fix: restrict configmap lookup to the provider namespace

When we use configmaps as a provider source, we filter them by provided labels and then take the latest version. Unfortunatelly, we list configmaps in all namespaces, which is not correct. This PR restricts the search by the provider namespace only.
kubernetes-sigs · Feb 15, 2024 · 82d7655 · 82d7655
1 parent c76a729
commit 82d7655
Show file tree

Hide file tree

Showing 7 changed files with 31 additions and 29 deletions.
diff --git a/internal/controller/manifests_downloader.go b/internal/controller/manifests_downloader.go
@@ -127,6 +127,7 @@ func (p *phaseReconciler) checkConfigMapExists(ctx context.Context, labelSelecto
 	labelSet := labels.Set(labelSelector.MatchLabels)
 	listOpts := []client.ListOption{
 		client.MatchingLabelsSelector{Selector: labels.SelectorFromSet(labelSet)},
+		client.InNamespace(p.provider.GetNamespace()),
 	}
 
 	var configMapList corev1.ConfigMapList

diff --git a/internal/controller/phases.go b/internal/controller/phases.go
@@ -176,7 +176,7 @@ func (p *phaseReconciler) load(ctx context.Context) (reconcile.Result, error) {
 		return reconcile.Result{}, wrapPhaseError(err, "failed to load additional manifests", operatorv1.ProviderInstalledCondition)
 	}
 
-	p.repo, err = p.configmapRepository(ctx, labelSelector, additionalManifests)
+	p.repo, err = p.configmapRepository(ctx, labelSelector, p.provider.GetNamespace(), additionalManifests)
 	if err != nil {
 		return reconcile.Result{}, wrapPhaseError(err, "failed to load the repository", operatorv1.ProviderInstalledCondition)
 	}
@@ -269,7 +269,7 @@ func (p *phaseReconciler) secretReader(ctx context.Context, providers ...configc
 
 // configmapRepository use clusterctl NewMemoryRepository structure to store the manifests
 // and metadata from a given configmap.
-func (p *phaseReconciler) configmapRepository(ctx context.Context, labelSelector *metav1.LabelSelector, additionalManifests string) (repository.Repository, error) {
+func (p *phaseReconciler) configmapRepository(ctx context.Context, labelSelector *metav1.LabelSelector, namespace, additionalManifests string) (repository.Repository, error) {
 	mr := repository.NewMemoryRepository()
 	mr.WithPaths("", "components.yaml")
 
@@ -280,7 +280,7 @@ func (p *phaseReconciler) configmapRepository(ctx context.Context, labelSelector
 		return nil, err
 	}
 
-	if err = p.ctrlClient.List(ctx, cml, &client.ListOptions{LabelSelector: selector}); err != nil {
+	if err = p.ctrlClient.List(ctx, cml, &client.ListOptions{LabelSelector: selector, Namespace: namespace}); err != nil {
 		return nil, err
 	}
 

diff --git a/internal/controller/phases_test.go b/internal/controller/phases_test.go
@@ -388,7 +388,7 @@ metadata:
 				g.Expect(fakeclient.Create(ctx, &tt.configMaps[i])).To(Succeed())
 			}
 
-			got, err := p.configmapRepository(context.TODO(), p.provider.GetSpec().FetchConfig.Selector, tt.additionalManifests)
+			got, err := p.configmapRepository(context.TODO(), p.provider.GetSpec().FetchConfig.Selector, "ns1", tt.additionalManifests)
 			if len(tt.wantErr) > 0 {
 				g.Expect(err).Should(MatchError(tt.wantErr))
 				return

diff --git a/test/e2e/air_gapped_test.go b/test/e2e/air_gapped_test.go
@@ -54,6 +54,14 @@ var _ = Describe("Install Core Provider in an air-gapped environment", func() {
 			configMaps = append(configMaps, configMap)
 		}
 
+		By("Creating capi-system namespace")
+		namespace := &corev1.Namespace{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: capiSystemNamespace,
+			},
+		}
+		Expect(bootstrapCluster.Create(ctx, namespace)).To(Succeed())
+
 		By("Applying core provider manifests to the cluster")
 		for _, cm := range configMaps {
 			Expect(bootstrapCluster.Create(ctx, &cm)).To(Succeed())
@@ -65,7 +73,7 @@ var _ = Describe("Install Core Provider in an air-gapped environment", func() {
 		coreProvider := &operatorv1.CoreProvider{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      coreProviderName,
-				Namespace: operatorNamespace,
+				Namespace: capiSystemNamespace,
 			},
 			Spec: operatorv1.CoreProviderSpec{
 				ProviderSpec: operatorv1.ProviderSpec{
@@ -87,7 +95,7 @@ var _ = Describe("Install Core Provider in an air-gapped environment", func() {
 		By("Waiting for the core provider deployment to be ready")
 		framework.WaitForDeploymentsAvailable(ctx, framework.WaitForDeploymentsAvailableInput{
 			Getter:     bootstrapClusterProxy.GetClient(),
-			Deployment: &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: coreProviderDeploymentName, Namespace: operatorNamespace}},
+			Deployment: &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: coreProviderDeploymentName, Namespace: capiSystemNamespace}},
 		}, e2eConfig.GetIntervals(bootstrapClusterProxy.GetName(), "wait-controllers")...)
 
 		By("Waiting for core provider to be ready")
@@ -104,7 +112,7 @@ var _ = Describe("Install Core Provider in an air-gapped environment", func() {
 	It("should successfully upgrade a CoreProvider (v1.5.4 -> latest)", func() {
 		bootstrapCluster := bootstrapClusterProxy.GetClient()
 		coreProvider := &operatorv1.CoreProvider{}
-		key := client.ObjectKey{Namespace: operatorNamespace, Name: coreProviderName}
+		key := client.ObjectKey{Namespace: capiSystemNamespace, Name: coreProviderName}
 		Expect(bootstrapCluster.Get(ctx, key, coreProvider)).To(Succeed())
 
 		coreProvider.Spec.Version = ""
@@ -114,7 +122,7 @@ var _ = Describe("Install Core Provider in an air-gapped environment", func() {
 		By("Waiting for the core provider deployment to be ready")
 		framework.WaitForDeploymentsAvailable(ctx, framework.WaitForDeploymentsAvailableInput{
 			Getter:     bootstrapClusterProxy.GetClient(),
-			Deployment: &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: coreProviderDeploymentName, Namespace: operatorNamespace}},
+			Deployment: &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: coreProviderDeploymentName, Namespace: capiSystemNamespace}},
 		}, e2eConfig.GetIntervals(bootstrapClusterProxy.GetName(), "wait-controllers")...)
 
 		By("Waiting for core provider to be ready")
@@ -132,15 +140,15 @@ var _ = Describe("Install Core Provider in an air-gapped environment", func() {
 		bootstrapCluster := bootstrapClusterProxy.GetClient()
 		coreProvider := &operatorv1.CoreProvider{ObjectMeta: metav1.ObjectMeta{
 			Name:      coreProviderName,
-			Namespace: operatorNamespace,
+			Namespace: capiSystemNamespace,
 		}}
 
 		Expect(bootstrapCluster.Delete(ctx, coreProvider)).To(Succeed())
 
 		By("Waiting for the core provider deployment to be deleted")
 		WaitForDelete(ctx, For(&appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{
 			Name:      coreProviderDeploymentName,
-			Namespace: operatorNamespace,
+			Namespace: capiSystemNamespace,
 		}}).In(bootstrapCluster), e2eConfig.GetIntervals(bootstrapClusterProxy.GetName(), "wait-controllers")...)
 
 		By("Waiting for the core provider object to be deleted")
@@ -168,5 +176,13 @@ var _ = Describe("Install Core Provider in an air-gapped environment", func() {
 		for _, cm := range configMaps {
 			Expect(bootstrapCluster.Delete(ctx, &cm)).To(Succeed())
 		}
+
+		By("Deleting capi-system namespace")
+		namespace := &corev1.Namespace{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: capiSystemNamespace,
+			},
+		}
+		Expect(bootstrapCluster.Delete(ctx, namespace)).To(Succeed())
 	})
 })
diff --git a/test/e2e/helpers_test.go b/test/e2e/helpers_test.go
@@ -28,7 +28,8 @@ var (
 )
 
 const (
-	operatorNamespace = "capi-operator-system"
+	operatorNamespace   = "capi-operator-system"
+	capiSystemNamespace = "capi-system"
 
 	previousCAPIVersion = "v1.5.4"
 

diff --git a/test/e2e/resources/core-cluster-api-v1.5.4.yaml b/test/e2e/resources/core-cluster-api-v1.5.4.yaml
@@ -1,14 +1,6 @@
 apiVersion: v1
 data:
   components: |
-    apiVersion: v1
-    kind: Namespace
-    metadata:
-      labels:
-        cluster.x-k8s.io/provider: cluster-api
-        control-plane: controller-manager
-      name: capi-system
-    ---
     apiVersion: apiextensions.k8s.io/v1
     kind: CustomResourceDefinition
     metadata:
@@ -11797,4 +11789,4 @@ metadata:
     provider.cluster.x-k8s.io/type: core
     provider.cluster.x-k8s.io/version: v1.5.4
   name: core-cluster-api-v1.5.4
-  namespace: capi-operator-system
+  namespace: capi-system
diff --git a/test/e2e/resources/core-cluster-api-v1.6.0.yaml b/test/e2e/resources/core-cluster-api-v1.6.0.yaml
@@ -1,14 +1,6 @@
 apiVersion: v1
 data:
   components: |
-    apiVersion: v1
-    kind: Namespace
-    metadata:
-      labels:
-        cluster.x-k8s.io/provider: cluster-api
-        control-plane: controller-manager
-      name: capi-system
-    ---
     apiVersion: apiextensions.k8s.io/v1
     kind: CustomResourceDefinition
     metadata:
@@ -9860,4 +9852,4 @@ metadata:
     provider.cluster.x-k8s.io/type: core
     provider.cluster.x-k8s.io/version: v1.6.0
   name: core-cluster-api-v1.6.0
-  namespace: capi-operator-system
+  namespace: capi-system