diff --git a/cmd/clusterctl/client/cluster/objectgraph.go b/cmd/clusterctl/client/cluster/objectgraph.go index 3d6c86e1fc19..b0934b45c7dc 100644 --- a/cmd/clusterctl/client/cluster/objectgraph.go +++ b/cmd/clusterctl/client/cluster/objectgraph.go @@ -37,6 +37,7 @@ import ( ) const clusterTopologyNameKey = "cluster.spec.topology.class" +const clusterResourceSetBindingClusterNameKey = "clusterresourcesetbinding.spec.clustername" type empty struct{} @@ -146,6 +147,17 @@ func (n *node) captureAdditionalInformation(obj *unstructured.Unstructured) erro } } + // If the node is a ClusterResourceSetBinding capture the name of the cluster it is referencing to. + if n.identity.GroupVersionKind().GroupKind() == addonsv1.GroupVersion.WithKind("ClusterResourceSetBinding").GroupKind() { + binding := &addonsv1.ClusterResourceSetBinding{} + if err := localScheme.Convert(obj, binding, nil); err != nil { + return errors.Wrapf(err, "failed to convert object %s to ClusterResourceSetBinding", n.identityStr()) + } + if n.additionalInfo == nil { + n.additionalInfo = map[string]interface{}{} + } + n.additionalInfo[clusterResourceSetBindingClusterNameKey] = binding.Spec.ClusterName + } return nil } @@ -504,6 +516,17 @@ func (o *objectGraph) getClusterClasses() []*node { return clusterClasses } +// getClusterResourceSetBinding returns the list of ClusterResourceSetBinding existing in the object graph. +func (o *objectGraph) getClusterResourceSetBinding() []*node { + crs := []*node{} + for _, node := range o.uidToNode { + if node.identity.GroupVersionKind().GroupKind() == addonsv1.GroupVersion.WithKind("ClusterResourceSetBinding").GroupKind() { + crs = append(crs, node) + } + } + return crs +} + // getClusters returns the list of Secrets existing in the object graph. func (o *objectGraph) getSecrets() []*node { secrets := []*node{} @@ -588,7 +611,7 @@ func (o *objectGraph) setSoftOwnership() { // Cluster that uses a ClusterClass are soft owned by that ClusterClass. for _, clusterClass := range clusterClasses { for _, cluster := range clusters { - // if the cluster uses a managed topoloy and uses the clusterclass + // if the cluster uses a managed topology and uses the clusterclass // set the clusterclass as a soft owner of the cluster. if className, ok := cluster.additionalInfo[clusterTopologyNameKey]; ok { if className == clusterClass.identity.Name && clusterClass.identity.Namespace == cluster.identity.Namespace { @@ -597,6 +620,21 @@ func (o *objectGraph) setSoftOwnership() { } } } + + crsBindings := o.getClusterResourceSetBinding() + // ClusterResourceSetBinding that refers to a Cluster are soft owned by that Cluster. + for _, binding := range crsBindings { + clusterName, ok := binding.additionalInfo[clusterResourceSetBindingClusterNameKey] + if !ok { + continue + } + + for _, cluster := range clusters { + if clusterName == cluster.identity.Name && binding.identity.Namespace == cluster.identity.Namespace { + binding.addSoftOwner(cluster) + } + } + } } // setTenants identifies all the nodes linked to a parent with forceMoveHierarchy = true (e.g. Clusters or ClusterResourceSet) diff --git a/cmd/clusterctl/client/cluster/objectgraph_test.go b/cmd/clusterctl/client/cluster/objectgraph_test.go index 4f9ee1d1f89e..dcd856f48521 100644 --- a/cmd/clusterctl/client/cluster/objectgraph_test.go +++ b/cmd/clusterctl/client/cluster/objectgraph_test.go @@ -1120,6 +1120,8 @@ var objectGraphsTests = []struct { "addons.cluster.x-k8s.io/v1beta1, Kind=ClusterResourceSetBinding, ns1/cluster1": { owners: []string{ "addons.cluster.x-k8s.io/v1beta1, Kind=ClusterResourceSet, ns1/crs1", + }, + softOwners: []string{ "cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster1", }, }, @@ -1201,12 +1203,16 @@ var objectGraphsTests = []struct { "addons.cluster.x-k8s.io/v1beta1, Kind=ClusterResourceSetBinding, ns1/cluster1": { owners: []string{ "addons.cluster.x-k8s.io/v1beta1, Kind=ClusterResourceSet, ns1/crs1", + }, + softOwners: []string{ "cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster1", }, }, "addons.cluster.x-k8s.io/v1beta1, Kind=ClusterResourceSetBinding, ns1/cluster2": { owners: []string{ "addons.cluster.x-k8s.io/v1beta1, Kind=ClusterResourceSet, ns1/crs1", + }, + softOwners: []string{ "cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster2", }, }, @@ -1887,32 +1893,148 @@ func Test_objectGraph_setSoftOwnership(t *testing.T) { objs []client.Object } tests := []struct { - name string - fields fields - wantSecrets map[string][]string + name string + fields fields + want wantGraph }{ { name: "A cluster with a soft owned secret", fields: fields{ - objs: test.NewFakeCluster("ns1", "foo").Objs(), + objs: test.NewFakeCluster("ns1", "cluster1").Objs(), }, - wantSecrets: map[string][]string{ // wantSecrets is a map[node UID] --> list of soft owner UIDs - "/v1, Kind=Secret, ns1/foo-ca": { // the ca secret has no explicit OwnerRef to the cluster, so it should be identified as a soft ownership - "cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/foo", + want: wantGraph{ + nodes: map[string]wantGraphItem{ + "cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster1": { + forceMove: true, + forceMoveHierarchy: true, + }, + "infrastructure.cluster.x-k8s.io/v1beta1, Kind=GenericInfrastructureCluster, ns1/cluster1": { + owners: []string{ + "cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster1", + }, + }, + "/v1, Kind=Secret, ns1/cluster1-ca": { // the ca secret has no explicit OwnerRef to the cluster, so it should be identified as a soft ownership + softOwners: []string{ + "cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster1", + }, + }, + "/v1, Kind=Secret, ns1/cluster1-kubeconfig": { // the kubeconfig secret has explicit OwnerRef to the cluster, so it should NOT be identified as a soft ownership + owners: []string{ + "cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster1", + }, + }, }, - "/v1, Kind=Secret, ns1/foo-kubeconfig": {}, // the kubeconfig secret has explicit OwnerRef to the cluster, so it should NOT be identified as a soft ownership }, }, { - name: "A cluster with a soft owned secret (cluster name with - in the middle)", + name: "A ClusterClass with a soft owned Cluster", fields: fields{ - objs: test.NewFakeCluster("ns1", "foo-bar").Objs(), + objs: func() []client.Object { + objs := test.NewFakeClusterClass("ns1", "class1").Objs() + objs = append(objs, test.NewFakeCluster("ns1", "cluster1").WithTopologyClass("class1").Objs()...) + + return objs + }(), }, - wantSecrets: map[string][]string{ // wantSecrets is a map[node UID] --> list of soft owner UIDs - "/v1, Kind=Secret, ns1/foo-bar-ca": { // the ca secret has no explicit OwnerRef to the cluster, so it should be identified as a soft ownership - "cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/foo-bar", + want: wantGraph{ + nodes: map[string]wantGraphItem{ + "cluster.x-k8s.io/v1beta1, Kind=ClusterClass, ns1/class1": { + forceMove: true, + forceMoveHierarchy: true, + }, + "infrastructure.cluster.x-k8s.io/v1beta1, Kind=GenericInfrastructureClusterTemplate, ns1/class1": { + owners: []string{ + "cluster.x-k8s.io/v1beta1, Kind=ClusterClass, ns1/class1", + }, + }, + "controlplane.cluster.x-k8s.io/v1beta1, Kind=GenericControlPlaneTemplate, ns1/class1": { + owners: []string{ + "cluster.x-k8s.io/v1beta1, Kind=ClusterClass, ns1/class1", + }, + }, + "cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster1": { + forceMove: true, + forceMoveHierarchy: true, + softOwners: []string{ + "cluster.x-k8s.io/v1beta1, Kind=ClusterClass, ns1/class1", // NB. this cluster is not linked to the clusterclass through owner ref, but it is detected as soft ownership + }, + }, + "infrastructure.cluster.x-k8s.io/v1beta1, Kind=GenericInfrastructureCluster, ns1/cluster1": { + owners: []string{ + "cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster1", + }, + }, + "/v1, Kind=Secret, ns1/cluster1-ca": { + softOwners: []string{ + "cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster1", // NB. this secret is not linked to the cluster through owner ref, but it is detected as soft ownership + }, + }, + "/v1, Kind=Secret, ns1/cluster1-kubeconfig": { + owners: []string{ + "cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster1", + }, + }, + }, + }, + }, + { + name: "A Cluster with a soft owned ClusterResourceSetBinding", + fields: fields{ + objs: func() []client.Object { + objs := test.NewFakeCluster("ns1", "cluster1").Objs() + objs = append(objs, test.NewFakeClusterResourceSet("ns1", "crs1"). + WithSecret("resource-s1"). + WithConfigMap("resource-c1"). + ApplyToCluster(test.SelectClusterObj(objs, "ns1", "cluster1")). + Objs()...) + + return objs + }(), + }, + want: wantGraph{ + nodes: map[string]wantGraphItem{ + "cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster1": { + forceMove: true, + forceMoveHierarchy: true, + }, + "infrastructure.cluster.x-k8s.io/v1beta1, Kind=GenericInfrastructureCluster, ns1/cluster1": { + owners: []string{ + "cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster1", + }, + }, + "/v1, Kind=Secret, ns1/cluster1-ca": { + softOwners: []string{ + "cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster1", // NB. this secret is not linked to the cluster through owner ref, but it is detected as soft ownership + }, + }, + "/v1, Kind=Secret, ns1/cluster1-kubeconfig": { + owners: []string{ + "cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster1", + }, + }, + "addons.cluster.x-k8s.io/v1beta1, Kind=ClusterResourceSet, ns1/crs1": { + forceMove: true, + forceMoveHierarchy: true, + }, + "addons.cluster.x-k8s.io/v1beta1, Kind=ClusterResourceSetBinding, ns1/cluster1": { + owners: []string{ + "addons.cluster.x-k8s.io/v1beta1, Kind=ClusterResourceSet, ns1/crs1", + }, + softOwners: []string{ + "cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster1", // NB. this ClusterResourceSetBinding is not linked to the cluster through owner ref, but it is detected as soft ownership + }, + }, + "/v1, Kind=Secret, ns1/resource-s1": { + owners: []string{ + "addons.cluster.x-k8s.io/v1beta1, Kind=ClusterResourceSet, ns1/crs1", + }, + }, + "/v1, Kind=ConfigMap, ns1/resource-c1": { + owners: []string{ + "addons.cluster.x-k8s.io/v1beta1, Kind=ClusterResourceSet, ns1/crs1", + }, + }, }, - "/v1, Kind=Secret, ns1/foo-bar-kubeconfig": {}, // the kubeconfig secret has explicit OwnerRef to the cluster, so it should NOT be identified as a soft ownership }, }, } @@ -1925,20 +2047,7 @@ func Test_objectGraph_setSoftOwnership(t *testing.T) { graph.setSoftOwnership() - gotSecrets := graph.getSecrets() - g.Expect(gotSecrets).To(HaveLen(len(tt.wantSecrets))) - - for _, secret := range gotSecrets { - wantObjects, ok := tt.wantSecrets[string(secret.identity.UID)] - g.Expect(ok).To(BeTrue()) - - gotObjects := []string{} - for softOwners := range secret.softOwners { - gotObjects = append(gotObjects, string(softOwners.identity.UID)) - } - - g.Expect(gotObjects).To(ConsistOf(wantObjects)) - } + assertGraph(t, graph, tt.want) }) } } diff --git a/cmd/clusterctl/internal/test/fake_objects.go b/cmd/clusterctl/internal/test/fake_objects.go index 8bb404ce6791..ea67137f35af 100644 --- a/cmd/clusterctl/internal/test/fake_objects.go +++ b/cmd/clusterctl/internal/test/fake_objects.go @@ -1099,6 +1099,7 @@ func (f *FakeClusterResourceSet) Objs() []client.Object { Namespace: cluster.Namespace, }, Spec: addonsv1.ClusterResourceSetBindingSpec{ + ClusterName: cluster.Name, Bindings: []*addonsv1.ResourceSetBinding{ { ClusterResourceSetName: crs.Name, @@ -1119,14 +1120,6 @@ func (f *FakeClusterResourceSet) Objs() []client.Object { objs = append(objs, binding) - // binding are owned by the Cluster / ownership set by the ClusterResourceSet controller - binding.SetOwnerReferences(append(binding.OwnerReferences, metav1.OwnerReference{ - APIVersion: cluster.APIVersion, - Kind: cluster.Kind, - Name: cluster.Name, - UID: cluster.UID, - })) - resourceSetBinding := addonsv1.ResourceSetBinding{ ClusterResourceSetName: crs.Name, Resources: []addonsv1.ResourceBinding{},