feat: also copy image pull secrets referencedin PlatformService spec (#29)

Author: Diaphteiros

api/install/install.go

@@ -10,6 +10,7 @@ import (
 	fluxsourcev1 "github.com/fluxcd/source-controller/api/v1"
 
 	clustersv1alpha1 "github.com/openmcp-project/openmcp-operator/api/clusters/v1alpha1"
+	providerv1alpha1 "github.com/openmcp-project/openmcp-operator/api/provider/v1alpha1"
 
 	dnsv1alpha1 "github.com/openmcp-project/platform-service-dns/api/dns/v1alpha1"
 )
@@ -27,6 +28,7 @@ func InstallOperatorAPIsPlatform(scheme *runtime.Scheme) *runtime.Scheme {
 	utilruntime.Must(clientgoscheme.AddToScheme(scheme))
 	utilruntime.Must(dnsv1alpha1.AddToScheme(scheme))
 	utilruntime.Must(clustersv1alpha1.AddToScheme(scheme))
+	utilruntime.Must(providerv1alpha1.AddToScheme(scheme))
 	utilruntime.Must(fluxsourcev1.AddToScheme(scheme))
 	utilruntime.Must(fluxhelmv2.AddToScheme(scheme))
 

docs/config/dns-service-config.md

@@ -41,7 +41,7 @@ spec:
 
 #### Secret Copying
 
-The `secretsToCopy` field allows to specify secrets that should be copied. The source of the secrets is always the provider namespace on the platform cluster.
+The `secretsToCopy` field allows to specify secrets that should be copied (in addition to the image pull secrets from the `PlatformService` resource). The source of the secrets is always the provider namespace on the platform cluster.
 
 Secrets referenced in `secretsToCopy.toPlatformCluster` will be copied into the reconciled `Cluster` resource's namespace on the platform cluster. This is the namespace that will host the Flux source resource and where pull secrets for the helm chart have to reside.
 
@@ -51,6 +51,8 @@ In both cases, if the entry's `target` field is set, the secret will be renamed
 
 If a secret that is to be created by the copy mechanism already exists, but is not managed by this controller (identified via labels), this will result in an error.
 
+⚠️ Note that the secrets referenced in `spec.imagePullSecrets` of the `PlatformService` resource will always be copied to both, platform cluster and target cluster.
+
 ⚠️ **Warning: This mechanism can copy secrets to other namespaces and even other clusters, therefore potentially making them accessible to users which do not have permissions to access the source secret. Use with caution!**
 
 #### Helm Chart Source

internal/controllers/cluster/controller.go

@@ -42,6 +42,7 @@ import (
 	clusterconst "github.com/openmcp-project/openmcp-operator/api/clusters/v1alpha1/constants"
 	commonapi "github.com/openmcp-project/openmcp-operator/api/common"
 	openmcpconst "github.com/openmcp-project/openmcp-operator/api/constants"
+	providerv1alpha1 "github.com/openmcp-project/openmcp-operator/api/provider/v1alpha1"
 	accesslib "github.com/openmcp-project/openmcp-operator/lib/clusteraccess"
 
 	dnsv1alpha1 "github.com/openmcp-project/platform-service-dns/api/dns/v1alpha1"
@@ -99,6 +100,8 @@ type ReconcileResult struct {
 	Message string
 	// ProviderConfig is the complete provider configuration.
 	ProviderConfig *dnsv1alpha1.DNSServiceConfig
+	// PlatformService is the DNS PlatformService resource.
+	PlatformService *providerv1alpha1.PlatformService
 }
 
 func (r *ClusterReconciler) Reconcile(ctx context.Context, req reconcile.Request) (reconcile.Result, error) {
@@ -173,6 +176,13 @@ func (r *ClusterReconciler) reconcile(ctx context.Context, c *clustersv1alpha1.C
 		}
 		return rr
 	}
+	// load PlatformService resource
+	rr.PlatformService = &providerv1alpha1.PlatformService{}
+	rr.PlatformService.Name = r.ProviderName
+	if err := r.PlatformCluster.Client().Get(ctx, client.ObjectKeyFromObject(rr.PlatformService), rr.PlatformService); err != nil {
+		rr.ReconcileError = errutils.WithReason(fmt.Errorf("error getting PlatformService '%s': %w", rr.PlatformService.Name, err), clusterconst.ReasonPlatformClusterInteractionProblem)
+		return rr
+	}
 
 	// iterate over configurations with purpose selectors and choose the first matching one
 	for i, cfg := range rr.ProviderConfig.Spec.ExternalDNSForPurposes {
@@ -450,64 +460,74 @@ func (r *ClusterReconciler) copySecrets(ctx context.Context, namespace string, e
 
 	// copy secrets if configured
 	if rr.ProviderConfig.Spec.SecretsToCopy != nil {
-		var secretsToCopy []dnsv1alpha1.SecretCopy
+		var configuredSecretsToCopy []dnsv1alpha1.SecretCopy
 		var targetAccess client.Client
 		var interactionProblemReason string
 		switch copyTarget {
 		case platformClusterCopy:
-			secretsToCopy = rr.ProviderConfig.Spec.SecretsToCopy.ToPlatformCluster
+			configuredSecretsToCopy = rr.ProviderConfig.Spec.SecretsToCopy.ToPlatformCluster
 			targetAccess = r.PlatformCluster.Client()
 			interactionProblemReason = clusterconst.ReasonPlatformClusterInteractionProblem
 		case targetClusterCopy:
-			secretsToCopy = rr.ProviderConfig.Spec.SecretsToCopy.ToTargetCluster
+			configuredSecretsToCopy = rr.ProviderConfig.Spec.SecretsToCopy.ToTargetCluster
 			targetAccess = rr.Access.Client()
 			interactionProblemReason = dnsv1alpha1.ReasonTargetClusterInteractionProblem
 		}
+		secretsToCopy := map[string]string{}
+		for _, ips := range rr.PlatformService.Spec.ImagePullSecrets {
+			secretsToCopy[ips.Name] = ips.Name
+		}
+		for _, sc := range configuredSecretsToCopy {
+			tName := ""
+			if sc.Target != nil && sc.Target.Name != "" {
+				tName = sc.Target.Name
+			} else {
+				tName = sc.Source.Name
+			}
+			secretsToCopy[sc.Source.Name] = tName
+		}
 		if targetAccess == nil {
 			rr.ReconcileError = errutils.WithReason(fmt.Errorf("no access to cluster '%s' for copying secrets", copyTarget), clusterconst.ReasonInternalError)
 			return rr, copied
 		}
-		for i, stc := range secretsToCopy {
+		for sourceName, targetName := range secretsToCopy {
 			source := &corev1.Secret{}
-			source.Name = stc.Source.Name
+			source.Name = sourceName
 			source.Namespace = r.ProviderNamespace
-			log.Debug("Secret copying configured, getting source secret", "sourceNamespace", source.Namespace, "sourceName", source.Name, "index", i)
+			log.Debug("Secret copying configured, getting source secret", "sourceNamespace", source.Namespace, "sourceName", source.Name)
 			if err := r.PlatformCluster.Client().Get(ctx, client.ObjectKeyFromObject(source), source); err != nil {
-				rr.ReconcileError = errutils.WithReason(fmt.Errorf("error getting source secret '%s/%s' (index: %d): %w", source.Namespace, source.Name, i, err), clusterconst.ReasonPlatformClusterInteractionProblem)
+				rr.ReconcileError = errutils.WithReason(fmt.Errorf("error getting source secret '%s/%s': %w", source.Namespace, source.Name, err), clusterconst.ReasonPlatformClusterInteractionProblem)
 				return rr, copied
 			}
 
 			// check if target secret already exists
 			target := &corev1.Secret{}
-			target.Name = source.Name
-			if stc.Target != nil && stc.Target.Name != "" {
-				target.Name = stc.Target.Name
-			}
+			target.Name = targetName
 			target.Namespace = namespace
 			if copyTarget == platformClusterCopy && target.Namespace == source.Namespace && target.Name == source.Name {
-				log.Debug("Skipping copying of secret because source and target are identical", "secretNamespace", target.Namespace, "secretName", target.Name, "index", i)
+				log.Debug("Skipping copying of secret because source and target are identical", "secretNamespace", target.Namespace, "secretName", target.Name)
 				copied.Insert(target.Name)
 				continue
 			}
 			targetExists := true
 			if err := targetAccess.Get(ctx, client.ObjectKeyFromObject(target), target); err != nil {
 				if !apierrors.IsNotFound(err) {
-					rr.ReconcileError = errutils.WithReason(fmt.Errorf("error getting target secret '%s/%s' (index: %d): %w", target.Namespace, target.Name, i, err), interactionProblemReason)
+					rr.ReconcileError = errutils.WithReason(fmt.Errorf("error getting target secret '%s/%s': %w", target.Namespace, target.Name, err), interactionProblemReason)
 					return rr, copied
 				}
 				targetExists = false
 			}
 			if targetExists {
 				// if target secret exists, verify that it is managed by us
-				log.Debug("Target secret already exists", "targetNamespace", target.Namespace, "targetName", target.Name, "index", i)
+				log.Debug("Target secret already exists", "targetNamespace", target.Namespace, "targetName", target.Name)
 				for k, v := range expectedLabels {
 					if v2, ok := target.Labels[k]; !ok || v2 != v {
-						rr.ReconcileError = errutils.WithReason(fmt.Errorf("target secret '%s/%s' (index: %d) already exists and is not managed by %s controller", target.Namespace, target.Name, i, ControllerName), clusterconst.ReasonConfigurationProblem)
+						rr.ReconcileError = errutils.WithReason(fmt.Errorf("target secret '%s/%s' already exists and is not managed by %s controller", target.Namespace, target.Name, ControllerName), clusterconst.ReasonConfigurationProblem)
 						return rr, copied
 					}
 				}
 			}
-			log.Debug("Creating or updating target secret", "targetNamespace", target.Namespace, "targetName", target.Name, "index", i)
+			log.Debug("Creating or updating target secret", "targetNamespace", target.Namespace, "targetName", target.Name)
 			if _, err := controllerutil.CreateOrUpdate(ctx, targetAccess, target, func() error {
 				target.Labels = maputils.Merge(target.Labels, source.Labels, expectedLabels)
 				target.Annotations = maputils.Merge(target.Annotations, source.Annotations)
@@ -516,7 +536,7 @@ func (r *ClusterReconciler) copySecrets(ctx context.Context, namespace string, e
 				target.Type = source.Type
 				return nil
 			}); err != nil {
-				rr.ReconcileError = errutils.WithReason(fmt.Errorf("error creating or updating target secret '%s/%s' (index: %d): %w", target.Namespace, target.Name, i, err), interactionProblemReason)
+				rr.ReconcileError = errutils.WithReason(fmt.Errorf("error creating or updating target secret '%s/%s': %w", target.Namespace, target.Name, err), interactionProblemReason)
 				return rr, copied
 			}
 			copied.Insert(target.Name)

internal/controllers/cluster/controller_test.go

@@ -324,9 +324,6 @@ var _ = Describe("ClusterReconciler", func() {
 	It("should use finalizers and remove resources when the Cluster is being deleted", func() {
 		env, _ := defaultTestSetup("testdata", "test-01")
 
-		cfg := &dnsv1alpha1.DNSServiceConfig{}
-		Expect(env.Client().Get(env.Ctx, client.ObjectKey{Name: providerName}, cfg)).To(Succeed())
-
 		c1 := &clustersv1alpha1.Cluster{}
 		Expect(env.Client().Get(env.Ctx, client.ObjectKey{Name: "cluster-01", Namespace: "foo"}, c1)).To(Succeed())
 		rr := env.ShouldReconcile(testutils.RequestFromObject(c1))
@@ -394,9 +391,6 @@ var _ = Describe("ClusterReconciler", func() {
 	It("should delete obsolete flux sources", func() {
 		env, _ := defaultTestSetup("testdata", "test-01")
 
-		cfg := &dnsv1alpha1.DNSServiceConfig{}
-		Expect(env.Client().Get(env.Ctx, client.ObjectKey{Name: providerName}, cfg)).To(Succeed())
-
 		// create dummy flux sources
 		expectedLabels := map[string]string{
 			openmcpconst.ManagedByLabel:      managedByValue,
@@ -438,9 +432,6 @@ var _ = Describe("ClusterReconciler", func() {
 	It("should create a GitRepository if configured", func() {
 		env, _ := defaultTestSetup("testdata", "test-03")
 
-		cfg := &dnsv1alpha1.DNSServiceConfig{}
-		Expect(env.Client().Get(env.Ctx, client.ObjectKey{Name: providerName}, cfg)).To(Succeed())
-
 		c1 := &clustersv1alpha1.Cluster{}
 		Expect(env.Client().Get(env.Ctx, client.ObjectKey{Name: "cluster-01", Namespace: "foo"}, c1)).To(Succeed())
 		rr := env.ShouldReconcile(testutils.RequestFromObject(c1))
@@ -471,9 +462,6 @@ var _ = Describe("ClusterReconciler", func() {
 	It("should create a HelmRepository if configured", func() {
 		env, _ := defaultTestSetup("testdata", "test-04")
 
-		cfg := &dnsv1alpha1.DNSServiceConfig{}
-		Expect(env.Client().Get(env.Ctx, client.ObjectKey{Name: providerName}, cfg)).To(Succeed())
-
 		c1 := &clustersv1alpha1.Cluster{}
 		Expect(env.Client().Get(env.Ctx, client.ObjectKey{Name: "cluster-01", Namespace: "foo"}, c1)).To(Succeed())
 		rr := env.ShouldReconcile(testutils.RequestFromObject(c1))
@@ -504,9 +492,6 @@ var _ = Describe("ClusterReconciler", func() {
 	It("should replace the special keywords in the values correctly", func() {
 		env, _ := defaultTestSetup("testdata", "test-03")
 
-		cfg := &dnsv1alpha1.DNSServiceConfig{}
-		Expect(env.Client().Get(env.Ctx, client.ObjectKey{Name: providerName}, cfg)).To(Succeed())
-
 		c1 := &clustersv1alpha1.Cluster{}
 		Expect(env.Client().Get(env.Ctx, client.ObjectKey{Name: "cluster-01", Namespace: "foo"}, c1)).To(Succeed())
 		rr := env.ShouldReconcile(testutils.RequestFromObject(c1))
@@ -535,9 +520,6 @@ var _ = Describe("ClusterReconciler", func() {
 	It("should not copy secrets if source and destination are identical", func() {
 		env, _ := defaultTestSetup("testdata", "test-05")
 
-		cfg := &dnsv1alpha1.DNSServiceConfig{}
-		Expect(env.Client().Get(env.Ctx, client.ObjectKey{Name: providerName}, cfg)).To(Succeed())
-
 		c1 := &clustersv1alpha1.Cluster{}
 		Expect(env.Client().Get(env.Ctx, client.ObjectKey{Name: "cluster-01", Namespace: "test"}, c1)).To(Succeed())
 		s1 := &corev1.Secret{}
@@ -554,6 +536,64 @@ var _ = Describe("ClusterReconciler", func() {
 		Expect(s1.Labels).To(BeEmpty())
 	})
 
+	It("should copy image pull secrets from the PlatformService resource", func() {
+		env, rec := defaultTestSetup("testdata", "test-06")
+
+		c1 := &clustersv1alpha1.Cluster{}
+		Expect(env.Client().Get(env.Ctx, client.ObjectKey{Name: "cluster-01", Namespace: "foo"}, c1)).To(Succeed())
+
+		rr := env.ShouldReconcile(testutils.RequestFromObject(c1))
+		Expect(rr.RequeueAfter).To(BeNumerically(">", 0))
+		fakeAccessRequestReadiness(env, c1)
+		rr = env.ShouldReconcile(testutils.RequestFromObject(c1))
+		Expect(rr.RequeueAfter).To(BeZero())
+
+		expectedLabels := map[string]string{
+			openmcpconst.ManagedByLabel:      managedByValue,
+			openmcpconst.ManagedPurposeLabel: "cluster-01",
+		}
+		ss := &corev1.SecretList{}
+		// copied secrets on platform cluster
+		Expect(env.Client().List(env.Ctx, ss, client.InNamespace(c1.Namespace), client.MatchingLabels(expectedLabels))).To(Succeed())
+		Expect(ss.Items).To(ConsistOf(
+			MatchFields(IgnoreExtras, Fields{
+				"ObjectMeta": MatchFields(IgnoreExtras, Fields{
+					"Name": Equal("my-auth-copy"),
+				}),
+				"Data": Equal(map[string][]byte{"key": []byte("value")}),
+			}),
+			MatchFields(IgnoreExtras, Fields{
+				"ObjectMeta": MatchFields(IgnoreExtras, Fields{
+					"Name": Equal("another-auth"),
+				}),
+				"Data": Equal(map[string][]byte{"auth": []byte("asdf")}),
+			}),
+			MatchFields(IgnoreExtras, Fields{
+				"ObjectMeta": MatchFields(IgnoreExtras, Fields{
+					"Name": Equal("my-other-secret"),
+				}),
+				"Data": Equal(map[string][]byte{"foo": []byte("bar")}),
+			}),
+		))
+		// copied secrets on target cluster
+		Expect(rec.FakeClientMappings["foo/cluster-01"].List(env.Ctx, ss, client.InNamespace(cluster.TargetClusterNamespace), client.MatchingLabels(expectedLabels))).To(Succeed())
+		Expect(ss.Items).To(ConsistOf(
+			MatchFields(IgnoreExtras, Fields{
+				"ObjectMeta": MatchFields(IgnoreExtras, Fields{
+					"Name": Equal("my-auth"),
+				}),
+				"Data": Equal(map[string][]byte{"key": []byte("value")}),
+			}),
+			MatchFields(IgnoreExtras, Fields{
+				"ObjectMeta": MatchFields(IgnoreExtras, Fields{
+					"Name": Equal("another-auth"),
+				}),
+				"Data": Equal(map[string][]byte{"auth": []byte("asdf")}),
+			}),
+		))
+
+	})
+
 })
 
 func fakeAccessRequestReadiness(env *testutils.Environment, c *clustersv1alpha1.Cluster) {

internal/controllers/cluster/testdata/test-01/platformservice.yaml

@@ -0,0 +1,7 @@
+apiVersion: openmcp.cloud/v1alpha1
+kind: PlatformService
+metadata:
+  name: dns-service
+spec:
+  image: example.com/platform-service-dns:latest
+  verbosity: INFO

internal/controllers/cluster/testdata/test-02/platformservice.yaml

@@ -0,0 +1,7 @@
+apiVersion: openmcp.cloud/v1alpha1
+kind: PlatformService
+metadata:
+  name: dns-service
+spec:
+  image: example.com/platform-service-dns:latest
+  verbosity: INFO

internal/controllers/cluster/testdata/test-03/platformservice.yaml

@@ -0,0 +1,7 @@
+apiVersion: openmcp.cloud/v1alpha1
+kind: PlatformService
+metadata:
+  name: dns-service
+spec:
+  image: example.com/platform-service-dns:latest
+  verbosity: INFO

internal/controllers/cluster/testdata/test-04/platformservice.yaml

@@ -0,0 +1,7 @@
+apiVersion: openmcp.cloud/v1alpha1
+kind: PlatformService
+metadata:
+  name: dns-service
+spec:
+  image: example.com/platform-service-dns:latest
+  verbosity: INFO

internal/controllers/cluster/testdata/test-05/platformservice.yaml

@@ -0,0 +1,7 @@
+apiVersion: openmcp.cloud/v1alpha1
+kind: PlatformService
+metadata:
+  name: dns-service
+spec:
+  image: example.com/platform-service-dns:latest
+  verbosity: INFO

internal/controllers/cluster/testdata/test-06/cluster-01.yaml

@@ -0,0 +1,12 @@
+apiVersion: clusters.openmcp.cloud/v1alpha1
+kind: Cluster
+metadata:
+  name: cluster-01
+  namespace: foo
+spec:
+  kubernetes: {}
+  profile: my-profile
+  purposes:
+  - foo
+  - bar
+  tenancy: Shared