Use acm rh registry pull request secret for volsync-system (#513)

Allows non-Openshift clusters to be able to access the images in the redhat registry in the volsync-system namespace - Use ACM label that will copy the pull request secret to the volsync-system namespace (this is similar to other ACM addons) - Update the volsync service account to use this pull request secret Signed-off-by: Tesshu Flower <[email protected]>
stolostron · Dec 19, 2024 · 1edc9ed · 1edc9ed
1 parent bf54a28
commit 1edc9ed
Show file tree

Hide file tree

Showing 7 changed files with 73 additions and 23 deletions.
diff --git a/controllers/addoncontroller.go b/controllers/addoncontroller.go
@@ -80,6 +80,13 @@ const (
 	AnnotationVolSyncAddonDeployTypeOverrideOLMValue = "olm"
 
 	AnnotationHelmChartKey = "helm-chart-key"
+
+	// This is the name of the pull secret that is copied to the namespace (volsync-system) on the managed
+	// cluster.  This will allow pulls to the redhat registry.
+	// (Other addons get this copied to open-cluster-management-agent-addon namespace on the mgd cluster)
+	// Note this secret is automatically copied to volsync-system via putting the
+	// label "addon.open-cluster-management.io/namespace":"true" on the volsync-system ns
+	RHRegistryPullSecretName = "open-cluster-management-image-pull-credentials"
 )
 
 func init() {
@@ -156,7 +163,6 @@ func (h *volsyncAgent) GetAgentAddonOptions() agent.AgentAddonOptions {
 							Resource:  "deployments",
 							Name:      "volsync",
 							Namespace: "*",
-							//Namespace: "volsync-system",
 						},
 						ProbeRules: []workapiv1.FeedbackRule{
 							{

diff --git a/controllers/addoncontroller_test.go b/controllers/addoncontroller_test.go
@@ -40,6 +40,8 @@ var _ = Describe("Addoncontroller - helm deployment tests", func() {
 	genericCodecs := serializer.NewCodecFactory(scheme.Scheme)
 	genericCodec := genericCodecs.UniversalDeserializer()
 
+	expectedVolSyncNamespace := controllers.DefaultHelmInstallNamespace
+
 	// Make sure a ClusterManagementAddOn exists for volsync or addon-framework will not reconcile
 	// VolSync ManagedClusterAddOns
 	var clusterManagementAddon *addonv1alpha1.ClusterManagementAddOn
@@ -194,8 +196,6 @@ var _ = Describe("Addoncontroller - helm deployment tests", func() {
 				}
 
 				Context(fmt.Sprintf("When the managed cluster %s", whenText), func() {
-					expectedNamespace := controllers.DefaultHelmInstallNamespace
-
 					var namespaceObj *corev1.Namespace
 					var operatorPolicyObj *policyv1beta1.OperatorPolicy
 					var operatorPolicyAggregateClusterRoleObj *rbacv1.ClusterRole
@@ -302,18 +302,29 @@ var _ = Describe("Addoncontroller - helm deployment tests", func() {
 						}
 
 						// In all cases here we expect the namespace to be the default
-						Expect(namespaceObj.GetName()).To(Equal(expectedNamespace))
+						Expect(namespaceObj.GetName()).To(Equal(expectedVolSyncNamespace))
+						// Check that special label is set on the namespace to indicate that ACM should copy over the
+						// redhat registry pull secret (allows us to pull volsync images from registry.redhat.io in
+						// volsync-system)
+						nsLabels := namespaceObj.GetLabels()
+						Expect(len(nsLabels)).To(Equal(1))
+						pullSecretCopyLabel, ok := nsLabels["addon.open-cluster-management.io/namespace"]
+						Expect(ok).To(BeTrue())
+						Expect(pullSecretCopyLabel).To(Equal("true"))
 					})
 
 					Context("When the ManagedClusterAddOn spec does not set an installNamespace", func() {
 						It("Should install to default namespace", func() {
 							// should this get set in the managedclusteraddon.spec.InstallNamespace as well?
 							helmutilstest.VerifyHelmRenderedVolSyncObjects(helmChartObjs,
-								expectedNamespace, mgdClusterIsOpenShift)
+								expectedVolSyncNamespace, mgdClusterIsOpenShift)
 						})
 					})
 
 					Context("When the ManagedClusterAddOn spec sets an installNamespace", func() {
+						// ManagedClusterAddon.Spec.InstallNamespace is essentially deprecated and should not be used
+						// See: https://github.com/open-cluster-management-io/ocm/issues/298
+						// volsync-addon-controller Code should ignore it
 						BeforeEach(func() {
 							// Override to specifically set the ns in the spec - all the tests above in JustBeforeEach
 							// should still be valid here
@@ -325,7 +336,7 @@ var _ = Describe("Addoncontroller - helm deployment tests", func() {
 							Expect(mcAddon.Spec.InstallNamespace).To(Equal("test1234"))
 
 							helmutilstest.VerifyHelmRenderedVolSyncObjects(helmChartObjs,
-								expectedNamespace, mgdClusterIsOpenShift)
+								expectedVolSyncNamespace, mgdClusterIsOpenShift)
 						})
 					})
 				})
@@ -474,7 +485,7 @@ var _ = Describe("Addoncontroller - helm deployment tests", func() {
 					var err error
 					volsyncDeployment, err = getVolSyncDeploymentFromManifestWork(manifestWork, genericCodec)
 					Expect(err).NotTo(HaveOccurred())
-					Expect(volsyncDeployment.GetNamespace()).To(Equal("volsync-system"))
+					Expect(volsyncDeployment.GetNamespace()).To(Equal(expectedVolSyncNamespace))
 					Expect(volsyncDeployment.GetName()).To(Equal("volsync"))
 				})
 
@@ -1019,7 +1030,7 @@ var _ = Describe("Addoncontroller - helm deployment tests", func() {
 
 					Expect(manifestWork).ToNot(BeNil())
 					Expect(volsyncDeployment).NotTo(BeNil())
-					Expect(volsyncDeployment.GetNamespace()).To(Equal("volsync-system"))
+					Expect(volsyncDeployment.GetNamespace()).To(Equal(expectedVolSyncNamespace))
 				})
 
 				Context("When a ManagedClusterAddOn is created with no addonConfig specified (the default)", func() {

diff --git a/controllers/helmutils/helmutils.go b/controllers/helmutils/helmutils.go
@@ -8,6 +8,7 @@ import (
 	"sort"
 	"sync"
 
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/klog/v2"
@@ -24,6 +25,7 @@ import (
 const (
 	defaultEmbeddedChartsDir = "/helmcharts"
 	crdKind                  = "CustomResourceDefinition"
+	serviceAccountKind       = "ServiceAccount"
 )
 
 // List of kinds of objects in the manifestwork - anything in this list will not have
@@ -88,6 +90,7 @@ func RenderManifestsFromChart(
 	clusterIsOpenShift bool,
 	chartValues map[string]interface{},
 	runtimeDecoder runtime.Decoder,
+	pullSecretForServiceAccount string,
 ) ([]runtime.Object, error) {
 	helmObjs := []runtime.Object{}
 
@@ -164,20 +167,39 @@ func RenderManifestsFromChart(
 			return nil, err
 		}
 
-		if gvk != nil {
-			if !slices.Contains(globalKinds, gvk.Kind) {
-				// Helm rendering does not set namespace on the templates, it will rely on the kubectl install/apply
-				// to do it (which does not happen since these objects end up directly in our manifestwork).
-				// So set the namespace ourselves for any object with kind not in our globalKinds list
-				templateObj.(metav1.Object).SetNamespace(releaseOptions.Namespace)
-			}
+		if gvk == nil {
+			gvkErr := fmt.Errorf("no gvk for template")
+			klog.Error(gvkErr, "Error decoding gvk from rendered template", "fileName", fileName)
+			return nil, gvkErr
+		}
+
+		if !slices.Contains(globalKinds, gvk.Kind) {
+			// Helm rendering does not set namespace on the templates, it will rely on the kubectl install/apply
+			// to do it (which does not happen since these objects end up directly in our manifestwork).
+			// So set the namespace ourselves for any object with kind not in our globalKinds list
+			templateObj.(metav1.Object).SetNamespace(releaseOptions.Namespace)
+		}
 
-			if gvk.Kind == crdKind {
-				// Add annotation to indicate we do not want the CRD deleted when the manifestwork is deleted
-				// (i.e. when the managedclusteraddon is deleted)
-				crdAnnotations := templateObj.(metav1.Object).GetAnnotations()
-				crdAnnotations[addonapiv1alpha1.DeletionOrphanAnnotationKey] = ""
-				templateObj.(metav1.Object).SetAnnotations(crdAnnotations)
+		// Special cases for specific resource kinds
+		switch gvk.Kind {
+		case crdKind:
+			// Add annotation to indicate we do not want the CRD deleted when the manifestwork is deleted
+			// (i.e. when the managedclusteraddon is deleted)
+			crdAnnotations := templateObj.(metav1.Object).GetAnnotations()
+			crdAnnotations[addonapiv1alpha1.DeletionOrphanAnnotationKey] = ""
+			templateObj.(metav1.Object).SetAnnotations(crdAnnotations)
+		case serviceAccountKind:
+			// Add pull secret to svc account
+			if pullSecretForServiceAccount != "" {
+				svcAccount, ok := templateObj.(*corev1.ServiceAccount)
+				if !ok {
+					svcAcctErr := fmt.Errorf("unable to decode service account resource")
+					klog.Error(svcAcctErr, "Error rendering helm chart")
+					return nil, svcAcctErr
+				}
+				svcAccount.ImagePullSecrets = append(svcAccount.ImagePullSecrets, corev1.LocalObjectReference{
+					Name: pullSecretForServiceAccount,
+				})
 			}
 		}
 

diff --git a/controllers/helmutils/helmutils_test.go b/controllers/helmutils/helmutils_test.go
@@ -60,7 +60,7 @@ var _ = Describe("Helmutils", func() {
 			}
 
 			renderedObjs, err = helmutils.RenderManifestsFromChart(chart, testNamespace, testCluster, clusterIsOpenShift,
-				chartValues, genericCodec)
+				chartValues, genericCodec, controllers.RHRegistryPullSecretName)
 			Expect(err).NotTo(HaveOccurred())
 			Expect(renderedObjs).NotTo(BeNil())
 		})

diff --git a/controllers/helmutils/helmutilstest/testhelper.go b/controllers/helmutils/helmutilstest/testhelper.go
@@ -10,6 +10,8 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/klog/v2"
+
+	"github.com/stolostron/volsync-addon-controller/controllers"
 )
 
 //nolint:funlen
@@ -131,6 +133,13 @@ func VerifyHelmRenderedVolSyncObjects(objs []runtime.Object,
 		Expect(*deployment.Spec.Template.Spec.SecurityContext.RunAsUser).To(Equal(int64(65534)))
 	}
 
+	// Check ServiceAccount
+	// It should have the pull secret set
+	Expect(len(serviceAccount.ImagePullSecrets)).To(Equal(1))
+	Expect(serviceAccount.ImagePullSecrets[0]).To(Equal(corev1.LocalObjectReference{
+		Name: controllers.RHRegistryPullSecretName,
+	}))
+
 	// Return the deployment (can be checked further by tests)
 	return deployment
 }
diff --git a/controllers/manifesthelper_helmdeploy.go b/controllers/manifesthelper_helmdeploy.go
@@ -103,7 +103,7 @@ func (mh *manifestHelperHelmDeploy) loadManifestsFromHelmRepo(values addonfactor
 	}
 
 	return helmutils.RenderManifestsFromChart(chart, installNamespace,
-		mh.cluster, mh.clusterIsOpenShift, values, genericCodec)
+		mh.cluster, mh.clusterIsOpenShift, values, genericCodec, RHRegistryPullSecretName)
 }
 
 func (mh *manifestHelperHelmDeploy) getValuesForManifest() (addonfactory.Values, error) {

diff --git a/controllers/manifests/helm-chart/namespace.yaml b/controllers/manifests/helm-chart/namespace.yaml
@@ -2,3 +2,5 @@ apiVersion: v1
 kind: Namespace
 metadata:
   name: {{ .InstallNamespace }}
+  labels:
+    "addon.open-cluster-management.io/namespace": "true"