diff --git a/api/install/install.go b/api/install/install.go index 306f384..3d9b6ae 100644 --- a/api/install/install.go +++ b/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)) diff --git a/docs/config/dns-service-config.md b/docs/config/dns-service-config.md index 9aa98c4..1074f6e 100644 --- a/docs/config/dns-service-config.md +++ b/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 diff --git a/internal/controllers/cluster/controller.go b/internal/controllers/cluster/controller.go index c8d39a3..5391624 100644 --- a/internal/controllers/cluster/controller.go +++ b/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) diff --git a/internal/controllers/cluster/controller_test.go b/internal/controllers/cluster/controller_test.go index db8e19f..bdbe77d 100644 --- a/internal/controllers/cluster/controller_test.go +++ b/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) { diff --git a/internal/controllers/cluster/testdata/test-01/platformservice.yaml b/internal/controllers/cluster/testdata/test-01/platformservice.yaml new file mode 100644 index 0000000..631a3d5 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/internal/controllers/cluster/testdata/test-02/platformservice.yaml b/internal/controllers/cluster/testdata/test-02/platformservice.yaml new file mode 100644 index 0000000..631a3d5 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/internal/controllers/cluster/testdata/test-03/platformservice.yaml b/internal/controllers/cluster/testdata/test-03/platformservice.yaml new file mode 100644 index 0000000..631a3d5 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/internal/controllers/cluster/testdata/test-04/platformservice.yaml b/internal/controllers/cluster/testdata/test-04/platformservice.yaml new file mode 100644 index 0000000..631a3d5 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/internal/controllers/cluster/testdata/test-05/platformservice.yaml b/internal/controllers/cluster/testdata/test-05/platformservice.yaml new file mode 100644 index 0000000..631a3d5 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/internal/controllers/cluster/testdata/test-06/cluster-01.yaml b/internal/controllers/cluster/testdata/test-06/cluster-01.yaml new file mode 100644 index 0000000..7b9b960 --- /dev/null +++ b/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 diff --git a/internal/controllers/cluster/testdata/test-06/config.yaml b/internal/controllers/cluster/testdata/test-06/config.yaml new file mode 100644 index 0000000..d1d720b --- /dev/null +++ b/internal/controllers/cluster/testdata/test-06/config.yaml @@ -0,0 +1,29 @@ +apiVersion: dns.openmcp.cloud/v1alpha1 +kind: DNSServiceConfig +metadata: + name: dns-service +spec: + secretsToCopy: + toPlatformCluster: + - source: + name: my-auth + target: + name: my-auth-copy + - source: + name: my-other-secret + externalDNSSource: + oci: + url: "oci://example.org/repo/charts" + interval: 10m + externalDNSForPurposes: + - name: foo + purposeSelector: + name: foo + helmValues: + foo: foo + - name: asdf + helmValues: + foo: bar + - name: qwer + helmValues: + foo: baz diff --git a/internal/controllers/cluster/testdata/test-06/platformservice.yaml b/internal/controllers/cluster/testdata/test-06/platformservice.yaml new file mode 100644 index 0000000..55c1a6d --- /dev/null +++ b/internal/controllers/cluster/testdata/test-06/platformservice.yaml @@ -0,0 +1,10 @@ +apiVersion: openmcp.cloud/v1alpha1 +kind: PlatformService +metadata: + name: dns-service +spec: + image: example.com/platform-service-dns:latest + verbosity: INFO + imagePullSecrets: + - name: my-auth + - name: another-auth \ No newline at end of file diff --git a/internal/controllers/cluster/testdata/test-06/secret-01.yaml b/internal/controllers/cluster/testdata/test-06/secret-01.yaml new file mode 100644 index 0000000..4941276 --- /dev/null +++ b/internal/controllers/cluster/testdata/test-06/secret-01.yaml @@ -0,0 +1,8 @@ +apiVersion: v1 +kind: Secret +metadata: + name: my-auth + namespace: test +data: + key: dmFsdWU= +type: Opaque diff --git a/internal/controllers/cluster/testdata/test-06/secret-02.yaml b/internal/controllers/cluster/testdata/test-06/secret-02.yaml new file mode 100644 index 0000000..64eb9e0 --- /dev/null +++ b/internal/controllers/cluster/testdata/test-06/secret-02.yaml @@ -0,0 +1,8 @@ +apiVersion: v1 +kind: Secret +metadata: + name: my-other-secret + namespace: test +data: + foo: YmFy +type: Opaque diff --git a/internal/controllers/cluster/testdata/test-06/secret-03.yaml b/internal/controllers/cluster/testdata/test-06/secret-03.yaml new file mode 100644 index 0000000..4043a6d --- /dev/null +++ b/internal/controllers/cluster/testdata/test-06/secret-03.yaml @@ -0,0 +1,8 @@ +apiVersion: v1 +kind: Secret +metadata: + name: another-auth + namespace: test +data: + auth: YXNkZg== +type: Opaque