proposal change for clusterset api

ldpliu · ldpliu · commit e8ad2ed521fc · 2022-08-09T13:56:39.000+08:00
Signed-off-by: ldpliu &lt;daliu@redhat.com&gt;
diff --git a/enhancements/sig-architecture/30-clusterset-override/README.md b/enhancements/sig-architecture/30-clusterset-override/README.md
@@ -7,7 +7,7 @@
 - [ ] Test plan is defined
 - [ ] Graduation criteria for dev preview, tech preview, GA
 - [ ] User-facing documentation is created in [website](https://github.com/open-cluster-management/website/)
- 
+
 ## Summary
 The proposed work enhances the managedClusterSet API to support managedClusterSet Override.
 
@@ -30,21 +30,18 @@ So, In this proposal, we change the managedClusterSets spec and want to provide
 
 ```go
 type ManagedClusterSetSpec struct {
-    // Selector represents a selector of ManagedClusters by labels and names.
+    // Selector represents a selector of ManagedClusters by labels.
     ClusterSelector ManagedClusterSelector  `json:"clusterSelector"`
 }
 
 type ManagedClusterSelector struct{
-    // "" means to use the current mechanism of matching label <cluster.open-cluster-management.io/clusterset:<ManagedClusterSet Name>.
-    // "LabelSelector" means to use the LabelSelector to select target managedClusters
-    // "ExclusiveLabel" means to use a particular cluster label. It is guaranteed that clustersets with same label key are exclusive with each others
-    // +optional
-    SelectorType SelectorType `json:"selectorType"`
-
-    // ExclusiveLabel defines one label which clusterset could use to select target managedClusters. In this way, we will:
-    // 1. Guarantee clustersets with same label key are exclusive
-    // 2. Enable additional permission check when cluster joining/leaving a clusterset (the label key should start with the reserved prefix "cluster.open-cluster-management.io/" and "info.open-cluster-management.io/");
-    ExclusiveLabel *ManagedClusterLabel `json:"exclusiveLabel"`
+    // SelectorType could only be "ExclusiveClusterSetLabel" or "LabelSelector"
+    // "ExclusiveClusterSetLabel" means to use label "cluster.open-cluster-management.io/clusterset:<ManagedClusterSet Name>"" to select target clusters.
+    // "LabelSelector" means to use LabelSelector to select target clusters
+    // +kubebuilder:validation:Enum=ExclusiveClusterSetLabel
+    // +kubebuilder:default:=ExclusiveClusterSetLabel
+    // +required
+    SelectorType SelectorType `json:"selectorType,omitempty"`
 
     // LabelSelector define the general labelSelector which clusterset will use to select target managedClusters
     LabelSelector *metav1.LabelSelector `json:"labelSelector"`
@@ -53,24 +50,22 @@ type ManagedClusterSelector struct{
 type SelectorType string
 
 const (
-    LabelSelector       SelectorType = "LabelSelector"
-    ExclusiveLabel        SelectorType = "ExclusiveLabel"
+    LabelSelector             SelectorType = "LabelSelector"
+	  ExclusiveClusterSetLabel  SelectorType = "ExclusiveClusterSetLabel"
 )
-
-//ManagedClusterLabel defines one label
-type ManagedClusterLabel struct {
-    Key string `json:"key"`
-    Value string `json:"value"`
-}
 ```
 
 ### RBAC
-Currently, When I want to create a managedClusterSet, I only should have `create` permission to the managedClusterSet. 
-In the managedCluster part, label `cluster.open-cluster-management.io/clusterset:<ManagedClusterSet Name>` means the cluster is in a managedClusterSet.
-If I want to add the label `cluster.open-cluster-management.io/clusterset:<ManagedClusterSet Name>` to a managedCluster, I must have `create` permission for subresource `managedclustersets/join`. 
-If I want to add/update other labels, I just need to have `update` permission to the managedCluster resource.
+Actually, what we discuss about RBAC is `who can add/remove a managedCluster to/from a managedClusterSet.`
+Currently, if a managedCluster which have label `cluster.open-cluster-management.io/clusterset:<ManagedClusterSet Name>` means the cluster is in a managedClusterSet.
+And If I want to add a managedCluster to a managedClusterSet, I must have `create` permission for subresource `managedclustersets/join` with resource name `<ManagedClusterSet Name>`.
 
-In this proposal, managedClusterSet could use any labels to select managedClusters. And if I add a label for a managedCluster, it may lead to the managedCluster added to a managedClusterSet. So we need to rethink about the rbac control of managedClusterSet
+In this proposal, managedClusterSet could use two ways to select target clusters, `ExclusiveClusterSetLabel` and `LabelSelector`.
+1. ExclusiveClusterSetLabel
+In this selector, managedClusterSet select target clusters using label `cluster.open-cluster-management.io/clusterset:<ManagedClusterSet Name>`. And the RBAC model will not change. If I want to add a managedCluster to a managedClusterSet, I must have `create` permission for subresource `managedclustersets/join` with resource name `<ManagedClusterSet Name>`.
+
+2. LabelSelector
+With the `LabelSelector`, any labels may be used to select target clusters. And we will not have external RBAC control for this kind of clustersets.
 
 We will use the following questions to consider the RBAC control.
 
@@ -81,7 +76,7 @@ c. Create a managedClusterSet which has the same capacity(like: all clusters ena
 d. Create a managedClusterSet for each squad for resource group purposes.
 
 #### Who can create a managedClusterSet
-Same as the current way, Anyone who has `create` permission to `managedclustersets` could create managedClusterSet. 
+Same as the current way, Anyone who has `create` permission to `managedclustersets` could create managedClusterSet.
 But generally, Cluster admin should not give this permission to others. So only cluster admin could create the managedClusterSet.
 
 #### What kinds of roles exist related to managedClusterSets and managedClusters
@@ -115,7 +110,7 @@ Currently, the controller/agent may add labels like: `region: apac`, `cloud: Ama
 3. Non Cluster admin users
 Currently, non cluster admin users may also `add/update` some managedClusters labels. 
 
-#### Add restrictions for adding/updating/deleting labels to managedClusters
+#### [Not Implement] Add restrictions for adding/updating/deleting labels to managedClusters
 In this proposal, managedClusterSet could use any labels to select managedClusters. And the new added labels may lead to this managedClusters added to a managedClusterSet.
 
 But for some managedClusterSet(like: Disaster Recovery ManagedClusterSet[a], Resource Group ManagedClusterSet[d]), we do not want any users to add their managedClusters to these managedClusterSets. 
@@ -174,8 +169,7 @@ Same as the current way, anyone who has `managedclustersets/bind` permission cou
 ### Example1: As cluster admin, I want to create a managedClusterSet which includes all clusters in the apac region. Then I can apply certain configurations to all clusters in apac.
 1. For each cluster, there should be an agent running on the managedCluster. The agent should have the following permissions:
 - `update` permission to its related managedCluster
-- `create` permission to subresource `managedclusters/label` with resourceName `info.open-cluster-management.io/region:*`
-2. For each cluster in the apac region, related agents should add a label: `info.open-cluster-management.io/region:apac` to these managedClusters.
+2. For each cluster in the apac region, related agents should add a label: `region:apac` to these managedClusters.
 3. As cluster admin, I could create a managedClusterSet to select all managedClusters in `apac` region
 ```yaml
 apiVersion: cluster.open-cluster-management.io/v1beta1
@@ -184,17 +178,16 @@ metadata:
   name: apacset
 spec:
   clusterSelector:
-    selectorType: ExclusiveLabel
-    exclusiveLabel:
-      key: info.open-cluster-management.io/region
-      value: apac
+    selectorType: LabelSelector
+    labelSelector:
+      matchLabels:
+        region: apac
 ```
 
 ### Example2: As a DEV team member, I want to use a managedClusterSet to select clusters based on the middleware enabled on these clusters, so I could run special applications in these clusters.
 1. For each cluster, there should be an agent running on the managedCluster. The agent should have the following permissions:
 - `update` permission to its related managedCluster
-- `create` permission to subresource `managedclusters/label` with resourceName `info.open-cluster-management.io/middlewareEnabled:true`
-2. For each cluster which enables the middleware, related agents should add a label: `info.open-cluster-management.io/middlewareEnabled:true` to these managedClusters.
+2. For each cluster which enables the middleware, related agents should add a label: `middlewareEnabled:true` to these managedClusters.
 3. Cluster admin create a managedClusterSet `middlewareenabledset`
 ```yaml
 apiVersion: cluster.open-cluster-management.io/v1beta1
@@ -206,11 +199,10 @@ spec:
     selectorType: LabelSelector
     labelSelector:
       matchLabels:
-        info.open-cluster-management.io/middlewareEnabled: true
+        middlewareEnabled: true
 ```
 4. Cluster admin should give following permissions to DEV team, so these team members could run applications in the managedClusterSet's clusters.
 - `get` permission to managedClusterSet `middlewareenabledset`
-- `create` permission to subresource `managedclustersets/bind` to managedClusterSet `middlewareenabledset`
 ```yaml
 kind: ClusterRole
 apiVersion: rbac.authorization.k8s.io/v1
@@ -234,26 +226,20 @@ rules:
 apiVersion: cluster.open-cluster-management.io/v1beta1
 kind: ManagedClusterSet
 metadata:
-  name: devset
+  name: dev
 spec:
   clusterSelector:
-    selectorType: ExclusiveLabel
-    exclusiveLabel:
-      key: cluster.open-cluster-management.io/clusterset
-      value: dev
+    selectorType: ExclusiveClusterSetLabel
 ```
 
 ```yaml
 apiVersion: cluster.open-cluster-management.io/v1beta1
 kind: ManagedClusterSet
 metadata:
-  name: qaset
+  name: qa
 spec:
   clusterSelector:
-    selectorType: ExclusiveLabel
-    exclusiveLabel:
-      key: cluster.open-cluster-management.io/clusterset
-      value: qa
+    selectorType: ExclusiveClusterSetLabel
 ```
 
 2. Cluster admin give the following permission to DEV team and QA team
@@ -269,15 +255,15 @@ rules:
    verbs: ["create"]
  - apiGroups: ["cluster.open-cluster-management.io"]
    resources: ["managedclustersets"]
-   resourceNames: ["devset"]
+   resourceNames: ["dev"]
    verbs: ["get"]
  - apiGroups: ["cluster.open-cluster-management.io"]
-   resources: ["managedclusters/label"]
-   resourceNames: ["cluster.open-cluster-management.io/clusterset:devset"]
+   resources: ["managedclustersets/join"]
+   resourceNames: ["dev"]
    verbs: ["create"]
  - apiGroups: ["cluster.open-cluster-management.io"]
    resources: ["managedclustersets/bind"]
-   resourceNames: ["devset"]
+   resourceNames: ["dev"]
    verbs: ["create"]
 ```
 
@@ -293,19 +279,19 @@ rules:
    verbs: ["create"]
  - apiGroups: ["cluster.open-cluster-management.io"]
    resources: ["managedclustersets"]
-   resourceNames: ["qaset"]
+   resourceNames: ["qa"]
    verbs: ["get"]
  - apiGroups: ["cluster.open-cluster-management.io"]
-   resources: ["managedclusters/label"]
-   resourceNames: ["cluster.open-cluster-management.io/clusterset:qaset"]
+   resources: ["managedclustersets/join"]
+   resourceNames: ["qa"]
    verbs: ["create"]
  - apiGroups: ["cluster.open-cluster-management.io"]
    resources: ["managedclustersets/bind"]
-   resourceNames: ["qaset"]
+   resourceNames: ["qa"]
    verbs: ["create"]
 ```
-3. As a DEV/QA team member, I can `create` a managedCluster, and add label `cluster.open-cluster-management.io/clusterset:devset/qaset` to the managedCluster.
-4. As a DEV/QA team member, I could `bind` the managedClusterSet `devset/qaset` to my namespace and run applications in the managedClusterSet.
+3. As a DEV/QA team member, I can `create` a managedCluster, and add label `cluster.open-cluster-management.io/clusterset:dev/qa` to the managedCluster.
+4. As a DEV/QA team member, I could `bind` the managedClusterSet `dev/qa` to my namespace and run applications in the managedClusterSet.
 
 ## Test Plan
 - Unit tests will cover the functionality of the controllers.
@@ -315,86 +301,10 @@ rules:
  - Create/update/delete managedClusters labels
  - Update managedClusterSet spec `clusterSelector` field
 
-## Migration
-Currently, managedClusterSet has three consumers: [placement](https://github.com/open-cluster-management-io/placement), [multicloud-operators-foundation](https://github.com/stolostron/multicloud-operators-foundation), [submariner-addon](https://github.com/stolostron/submariner-addon)
-
-- `multicloud-operators-foundation` uses managedClusterSet for resource group purposes. And it should only care about the exclusive managedClusterSet. 
-- `submariner-addon` uses managedClusterSet to group clusters based on the networks. And in different managedClusterSet, the clusters should be exclusive.
-- `placement` select all managedClusters in each managedClusterSet.
-
-So we could finish the migration by four steps, and step 1 and step 2 will be finished in OCM 0.7.0. and step 3 and step 4 will be finished in OCM 0.8.0
-
-1. [Implement in OCM 0.7.0]Update the managedClusterSet API which only includes an exclusive way to select target managedClusters.
-
-```go
-type ManagedClusterSetSpec struct {
-    // Selector represents a selector of ManagedClusters by labels and names.
-    ClusterSelector ManagedClusterSelector  `json:"clusterSelector"`
-}
-
-type ManagedClusterSelector struct{
-    // "" means to use the current mechanism of matching label <cluster.open-cluster-management.io/clusterset:<ManagedClusterSet Name>.
-    // (future) "LabelSelector" means to use the LabelSelector to select target managedClusters
-    // "ExclusiveLabel" means to use a particular cluster label. It is guaranteed that clustersets with same label key are exclusive with each others
-    // +optional
-    SelectorType SelectorType `json:"selectorType"`
-
-    // ExclusiveLabel defines one label which clusterset could use to select target managedClusters. In this way, we will:
-    // 1. Guarantee clustersets with same label key are exclusive
-    // 2. Enable additional permission check when cluster joining/leaving a clusterset (the label key should start with the reserved prefix "cluster.open-cluster-management.io/" and "info.open-cluster-management.io/");
-    ExclusiveLabel *ExclusiveLabel `json:"exclusiveLabel"`
-}
-
-type SelectorType string
-
-const (
-    ExclusiveLabel        SelectorType = "ExclusiveLabel"
-)
-
-//ExclusiveLabel defines one cluster label
-type ExclusiveLabel struct {
-    //Key is "cluster.open-cluster-management.io/clusterset" by default and can only be cluster.open-cluster-management.io/
-    Key string `json:"key"`
-    //Value can only be empty or the name of the clusterset.
-    Value string `json:"value"`
-}
-```
-
-- `LabelSelector` will not be included
-- `ExclusiveLabel.Key` must be `cluster.open-cluster-management.io/clusterset` and `ExclusiveLabel.Value` must be `ManagedClusterset Name`    
-- Both `managedclusterset/join` and `managedclusters/label` permission will be supported
-
-2. [Implement in OCM 0.7.0]`multicloud-operators-foundation`, `submariner-addon`, `placement` change the code to integrate with new managedClusterSet api
-
-  a. `multicloud-operators-foundation` uses managedClusterSet for resource group purpose. So it should only watch the following managedClusterSets:
-  - `spec.ClusterSelector.SelectorType` is `ExclusiveLabel` and the `ExclusiveLabel.Key` must be `cluster.open-cluster-management.io/clusterset`
-  - `spec.ClusterSelector.SelectorType` is ""
-
-  b. `multicloud-operators-foundation` gives the users `join` permission to a managedClusterSet if the user has "admin" permission to the managedClusterSet. So the `join` permission should be changed with the following rule:
-    ```yaml
-    - apiGroups: ["cluster.open-cluster-management.io"]
-      resources: ["managedclusters/label"]
-      resourceNames: ["cluster.open-cluster-management.io/clusterset:<ManagedClusterSet Name>"]
-      verbs: ["create"]
-    ```
-
-  c. `submariner-addon` uses managedClusterSet group clusters based on the network. And in different managedClusterSet, the clusters should be exclusive. So it should only watch the following managedClusterSet:
-  - `spec.ClusterSelector.SelectorType` is `ExclusiveLabel` and the `ExclusiveLabel.Key` must be `cluster.open-cluster-management.io/clusterset`, the `ExclusiveLabel.Value` must be managedClusterSet name.
-  - `spec.ClusterSelector.SelectorType` is ""
-
-  d. `placement` using new `ClusterSelector` to select target clusters.
-
-3. [Implement in OCM 0.8.0] Update full managedClusterSet api and RBAC
-- Include `LabelSelector`
-- Take off the restriction for “ExclusiveLabel.Key” and “ExclusiveLabel.Value”
-- Deprecate `managedclusterset/join` permission
-
-4. [Implement in OCM 0.8.0] `placement` uses the new managedClusterSet api to select managedClusters for each managedClusterSet.
-
 ## Upgrade / Downgrade Strategy 
 The new api is compatible with the previous version. So there is no external work needed when upgrading
 
 ## Graduation Criteria
 ### v1beta1
 1. The new APIs is reviewed and accepted;
-2. Implementation is completed to support the RBAC;
+2. Implementation is completed;
diff --git a/enhancements/sig-architecture/30-clusterset-override/metadata.yaml b/enhancements/sig-architecture/30-clusterset-override/metadata.yaml
@@ -9,5 +9,5 @@ approvers:
   - "@elgnay"
   - "@deads2k"
 creation-date: 2021-11-30
-last-updated: 2022-02-24
+last-updated: 2022-04-13
 status: provisional
