Merge pull request #422 from Fedosin/configmap_namespace

🐛 Restrict configmap lookup to the provider namespace
kubernetes-sigs · Feb 19, 2024 · 9ccc6d1 · 9ccc6d1
2 parents e23d296 + 82d7655
commit 9ccc6d1
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