From db5b183c55311ee517f699c83570832303e3c2a1 Mon Sep 17 00:00:00 2001
From: fabriziopandini <fpandini@vmware.com>
Date: Tue, 16 Nov 2021 18:51:41 +0100
Subject: [PATCH] clusterctl discovery should ignore provider's resources

---
 cmd/clusterctl/client/cluster/proxy.go | 54 ++++++++++++++------------
 1 file changed, 29 insertions(+), 25 deletions(-)

diff --git a/cmd/clusterctl/client/cluster/proxy.go b/cmd/clusterctl/client/cluster/proxy.go
index eec7495ec959..828cf0f8b9d3 100644
--- a/cmd/clusterctl/client/cluster/proxy.go
+++ b/cmd/clusterctl/client/cluster/proxy.go
@@ -34,6 +34,7 @@ import (
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/clientcmd"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
+	clusterctlv1 "sigs.k8s.io/cluster-api/cmd/clusterctl/api/v1alpha3"
 	"sigs.k8s.io/cluster-api/cmd/clusterctl/internal/scheme"
 	"sigs.k8s.io/cluster-api/version"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -60,10 +61,12 @@ type Proxy interface {
 	// CheckClusterAvailable checks if a a cluster is available and reachable.
 	CheckClusterAvailable() error
 
-	// ListResources lists namespaced and cluster-wide resources matching the labels. Namespaced resources are only listed
+	// ListResources lists namespaced and cluster-wide resources for a component matching the labels. Namespaced resources are only listed
 	// in the given namespaces.
-	// If labels contains the ProviderLabelName label, CRDs of other providers are excluded.
-	// This is done to avoid errors when listing resources of providers which have already been deleted.
+	// Please note that we are not returning resources for the component's CRD (e.g. we are not returning
+	// Certificates for cert-manager, Clusters for CAPI, AWSCluster for CAPA and so on).
+	// This is done to avoid errors when listing resources of providers which have already been deleted/scaled down to 0 replicas/with
+	// malfunctioning webhooks.
 	ListResources(labels map[string]string, namespaces ...string) ([]unstructured.Unstructured, error)
 
 	// GetContexts returns the list of contexts in kubeconfig which begin with prefix.
@@ -205,10 +208,12 @@ func (k *proxy) CheckClusterAvailable() error {
 	return nil
 }
 
-// ListResources lists namespaced and cluster-wide resources matching the labels. Namespaced resources are only listed
+// ListResources lists namespaced and cluster-wide resources for a component matching the labels. Namespaced resources are only listed
 // in the given namespaces.
-// If labels contains the ProviderLabelName label, CRDs of other providers are excluded.
-// This is done to avoid errors when listing resources of providers which have already been deleted.
+// Please note that we are not returning resources for the component's CRD (e.g. we are not returning
+// Certificates for cert-manager, Clusters for CAPI, AWSCluster for CAPA and so on).
+// This is done to avoid errors when listing resources of providers which have already been deleted/scaled down to 0 replicas/with
+// malfunctioning webhooks.
 // For example:
 // * The AWS provider has already been deleted, but there are still cluster-wide resources of AWSClusterControllerIdentity.
 // * The AWSClusterControllerIdentity resources are still stored in an older version (e.g. v1alpha4, when the preferred
@@ -237,27 +242,26 @@ func (k *proxy) ListResources(labels map[string]string, namespaces ...string) ([
 		return nil, errors.Wrap(err, "failed to list api resources")
 	}
 
-	// If labels indicates that resources of a specific provider should be listed, exclude CRDs of other providers.
+	// Exclude from discovery the objects from the cert-manager/provider's CRDs.
+	// Those objects are not part of the components, and they will eventually be removed when removing the CRD definition.
 	crdsToExclude := sets.String{}
-	if providerName, ok := labels[clusterv1.ProviderLabelName]; ok {
-		// List all CRDs in the cluster.
-		crdList := &apiextensionsv1.CustomResourceDefinitionList{}
-		if err := retryWithExponentialBackoff(newReadBackoff(), func() error {
-			return c.List(ctx, crdList)
-		}); err != nil {
-			return nil, errors.Wrap(err, "failed to list CRDs")
-		}
 
-		// Exclude CRDs of other providers.
-		for _, crd := range crdList.Items {
-			if v, ok := crd.Labels[clusterv1.ProviderLabelName]; ok && v != providerName {
-				for _, version := range crd.Spec.Versions {
-					crdsToExclude.Insert(metav1.GroupVersionKind{
-						Group:   crd.Spec.Group,
-						Version: version.Name,
-						Kind:    crd.Spec.Names.Kind,
-					}.String())
-				}
+	crdList := &apiextensionsv1.CustomResourceDefinitionList{}
+	if err := retryWithExponentialBackoff(newReadBackoff(), func() error {
+		return c.List(ctx, crdList)
+	}); err != nil {
+		return nil, errors.Wrap(err, "failed to list CRDs")
+	}
+	for _, crd := range crdList.Items {
+		component, isCoreComponent := labels[clusterctlv1.ClusterctlCoreLabelName]
+		_, isProviderResource := crd.Labels[clusterv1.ProviderLabelName]
+		if (isCoreComponent && component == clusterctlv1.ClusterctlCoreLabelCertManagerValue) || isProviderResource {
+			for _, version := range crd.Spec.Versions {
+				crdsToExclude.Insert(metav1.GroupVersionKind{
+					Group:   crd.Spec.Group,
+					Version: version.Name,
+					Kind:    crd.Spec.Names.Kind,
+				}.String())
 			}
 		}
 	}