diff --git a/Taskfile.yaml b/Taskfile.yaml index a6cfb5d..da38bb9 100644 --- a/Taskfile.yaml +++ b/Taskfile.yaml @@ -16,3 +16,25 @@ includes: CHART_COMPONENTS: "[]" CRDS_COMPONENTS: platform-service-dns CRDS_PATH: '{{.ROOT_DIR}}/api/crds/manifests' + +tasks: + platformservice: + desc: " Generates a PlatformService manifest for the current version. Set the VERBOSITY env var to overwrite the default verbosity level (INFO)." + requires: + vars: + - VERSION + vars: + VERBOSITY: + sh: echo "${VERBOSITY:-INFO}" + cmds: + - cmd: | + cat << EOF + apiVersion: openmcp.cloud/v1alpha1 + kind: PlatformService + metadata: + name: dns + spec: + image: ghcr.io/openmcp-project/images/platform-service-dns:{{.VERSION}} + verbosity: {{.VERBOSITY}} + EOF + silent: true diff --git a/VERSION b/VERSION index e3e41a5..90ab6e9 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -v0.0.1-dev \ No newline at end of file +v0.0.2 \ No newline at end of file diff --git a/api/crds/manifests/dns.openmcp.cloud_dnsserviceconfigs.yaml b/api/crds/manifests/dns.openmcp.cloud_dnsserviceconfigs.yaml index ab5a289..721b0f6 100644 --- a/api/crds/manifests/dns.openmcp.cloud_dnsserviceconfigs.yaml +++ b/api/crds/manifests/dns.openmcp.cloud_dnsserviceconfigs.yaml @@ -67,7 +67,8 @@ spec: - will be replaced with the environment name of the operator. - will be replaced with the name of the reconciled Cluster. - will be replaced with the namespace of the reconciled Cluster. - type: string + type: object + x-kubernetes-preserve-unknown-fields: true name: description: |- Name is an optional name. @@ -102,9 +103,9 @@ spec: chartName: description: |- ChartName specifies the name of the external-dns chart. - Depending on the source, this can also be a relative path within the repository. - When using a source that needs a version (helm or oci), append the version to the chart name using '@', e.g. 'external-dns@1.10.0' or omit for latest version. - minLength: 1 + Can be omitted for oci sources, required for git and helm sources. + For git sources, this is the path within the git repository to the chart. + For helm sources, append the version to the chart name using '@', e.g. 'external-dns@1.10.0' or omit for latest version. type: string git: description: |- @@ -638,10 +639,11 @@ spec: - interval - url type: object - required: - - chartName type: object x-kubernetes-validations: + - message: chartName must be set if git is used as source + rule: '(has(self.git) || has(self.helm)) ? (has(self.chartName) + && size(self.chartName) > 0) : true' - message: exactly one of the fields in [helm git oci] must be set rule: '[has(self.helm),has(self.git),has(self.oci)].filter(x,x==true).size() == 1' @@ -653,48 +655,100 @@ spec: type: string secretsToCopy: description: |- - SecretsToCopy specifies an optional list of secrets which will be copied from the provider namespace into the namespaces of the reconciled Clusters. - This can, for example, be used to distribute credentials for the registry holding the external-dns helm chart. - items: - description: |- - SecretCopy defines the name of the secret to copy and the name of the copied secret. - If target is nil or target.name is empty, the secret will be copied with the same name as the source secret. - properties: - source: - description: LocalObjectReference is a reference to an object - in the same namespace as the resource referencing it. + SecretsToCopy specifies secrets that should be copied to either the cluster's namespace on the platform cluster, + or the namespace on the target cluster where the helm chart will be installed into. + properties: + toPlatformCluster: + description: |- + ToPlatformCluster lists secrets from the provider namespace that should be copied into the cluster's namespace on the platform cluster. + This is useful e.g. for pull secrets for the helm chart registry. + items: + description: |- + SecretCopy defines the name of the secret to copy and the name of the copied secret. + If target is nil or target.name is empty, the secret will be copied with the same name as the source secret. properties: - name: - default: "" + source: description: |- - Name of the referent. - This field is effectively required, but due to backwards compatibility is - allowed to be empty. Instances of this type with an empty value here are - almost certainly wrong. - More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names - type: string + Source references the source secret to copy. + It has to be in the namespace the provider pod is running in. + properties: + name: + default: "" + description: |- + Name of the referent. + This field is effectively required, but due to backwards compatibility is + allowed to be empty. Instances of this type with an empty value here are + almost certainly wrong. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + type: string + type: object + x-kubernetes-map-type: atomic + target: + description: |- + Target is the name of the copied secret. + If not set, the secret will be copied with the same name as the source secret. + properties: + name: + default: "" + description: |- + Name of the referent. + This field is effectively required, but due to backwards compatibility is + allowed to be empty. Instances of this type with an empty value here are + almost certainly wrong. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + type: string + type: object + x-kubernetes-map-type: atomic + required: + - source type: object - x-kubernetes-map-type: atomic - target: - description: LocalObjectReference is a reference to an object - in the same namespace as the resource referencing it. + type: array + toTargetCluster: + description: |- + ToTargetCluster lists secrets from the provider namespace that should be copied into the cluster's namespace on the target cluster. + This allows propagating secrets that are required by the helm chart to the target cluster. + items: + description: |- + SecretCopy defines the name of the secret to copy and the name of the copied secret. + If target is nil or target.name is empty, the secret will be copied with the same name as the source secret. properties: - name: - default: "" + source: description: |- - Name of the referent. - This field is effectively required, but due to backwards compatibility is - allowed to be empty. Instances of this type with an empty value here are - almost certainly wrong. - More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names - type: string + Source references the source secret to copy. + It has to be in the namespace the provider pod is running in. + properties: + name: + default: "" + description: |- + Name of the referent. + This field is effectively required, but due to backwards compatibility is + allowed to be empty. Instances of this type with an empty value here are + almost certainly wrong. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + type: string + type: object + x-kubernetes-map-type: atomic + target: + description: |- + Target is the name of the copied secret. + If not set, the secret will be copied with the same name as the source secret. + properties: + name: + default: "" + description: |- + Name of the referent. + This field is effectively required, but due to backwards compatibility is + allowed to be empty. Instances of this type with an empty value here are + almost certainly wrong. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + type: string + type: object + x-kubernetes-map-type: atomic + required: + - source type: object - x-kubernetes-map-type: atomic - required: - - source - - target - type: object - type: array + type: array + type: object required: - externalDNSSource type: object diff --git a/api/dns/v1alpha1/config_types.go b/api/dns/v1alpha1/config_types.go index c51f8f7..5f1e04d 100644 --- a/api/dns/v1alpha1/config_types.go +++ b/api/dns/v1alpha1/config_types.go @@ -16,10 +16,10 @@ type DNSServiceConfigSpec struct { // ExternalDNSSource is the source of the external-dns helm chart. ExternalDNSSource ExternalDNSSource `json:"externalDNSSource"` - // SecretsToCopy specifies an optional list of secrets which will be copied from the provider namespace into the namespaces of the reconciled Clusters. - // This can, for example, be used to distribute credentials for the registry holding the external-dns helm chart. + // SecretsToCopy specifies secrets that should be copied to either the cluster's namespace on the platform cluster, + // or the namespace on the target cluster where the helm chart will be installed into. // +optional - SecretsToCopy []SecretCopy `json:"secretsToCopy,omitempty"` + SecretsToCopy *SecretsToCopy `json:"secretsToCopy,omitempty"` // HelmReleaseReconciliationInterval is the interval at which the HelmRelease for external-dns is reconciled. // The value can be overwritten for specific purposes using ExternalDNSForPurposes. @@ -34,15 +34,28 @@ type DNSServiceConfigSpec struct { ExternalDNSForPurposes []ExternalDNSPurposeConfig `json:"externalDNSForPurposes,omitempty"` } +type SecretsToCopy struct { + // ToPlatformCluster lists secrets from the provider namespace that should be copied into the cluster's namespace on the platform cluster. + // This is useful e.g. for pull secrets for the helm chart registry. + // +optional + ToPlatformCluster []SecretCopy `json:"toPlatformCluster,omitempty"` + // ToTargetCluster lists secrets from the provider namespace that should be copied into the cluster's namespace on the target cluster. + // This allows propagating secrets that are required by the helm chart to the target cluster. + // +optional + ToTargetCluster []SecretCopy `json:"toTargetCluster,omitempty"` +} + // ExternalDNSSource defines the source of the external-dns helm chart in form of a Flux source. // Exactly one of 'HelmRepository', 'GitRepository' or 'OCIRepository' must be set. // If 'copyAuthSecret' is set, the referenced source secret is copied into the namespace where the Flux resources are created with the specified target name. // +kubebuilder:validation:ExactlyOneOf=helm;git;oci +// +kubebuilder:validation:XValidation:rule="(has(self.git) || has(self.helm)) ? (has(self.chartName) && size(self.chartName) > 0) : true", message="chartName must be set if git is used as source" type ExternalDNSSource struct { // ChartName specifies the name of the external-dns chart. - // Depending on the source, this can also be a relative path within the repository. - // When using a source that needs a version (helm or oci), append the version to the chart name using '@', e.g. 'external-dns@1.10.0' or omit for latest version. - // +kubebuilder:validation:MinLength=1 + // Can be omitted for oci sources, required for git and helm sources. + // For git sources, this is the path within the git repository to the chart. + // For helm sources, append the version to the chart name using '@', e.g. 'external-dns@1.10.0' or omit for latest version. + // +optional ChartName string `json:"chartName"` Helm *fluxv1.HelmRepositorySpec `json:"helm,omitempty"` Git *fluxv1.GitRepositorySpec `json:"git,omitempty"` @@ -52,7 +65,12 @@ type ExternalDNSSource struct { // SecretCopy defines the name of the secret to copy and the name of the copied secret. // If target is nil or target.name is empty, the secret will be copied with the same name as the source secret. type SecretCopy struct { - Source commonapi.LocalObjectReference `json:"source"` + // Source references the source secret to copy. + // It has to be in the namespace the provider pod is running in. + Source commonapi.LocalObjectReference `json:"source"` + // Target is the name of the copied secret. + // If not set, the secret will be copied with the same name as the source secret. + // +optional Target *commonapi.LocalObjectReference `json:"target"` } @@ -80,8 +98,9 @@ type ExternalDNSPurposeConfig struct { // - will be replaced with the environment name of the operator. // - will be replaced with the name of the reconciled Cluster. // - will be replaced with the namespace of the reconciled Cluster. - // +kubebuilder:validation:Type=string + // +kubebuilder:validation:Type=object // +kubebuilder:validation:Schemaless + // +kubebuilder:pruning:PreserveUnknownFields HelmValues *apiextensionsv1.JSON `json:"helmValues"` } diff --git a/api/dns/v1alpha1/constants.go b/api/dns/v1alpha1/constants.go index b9cae29..84ecc0d 100644 --- a/api/dns/v1alpha1/constants.go +++ b/api/dns/v1alpha1/constants.go @@ -8,4 +8,6 @@ const ( OperationAnnotation = "dns." + openmcpconst.OperationAnnotation ExternalDNSFinalizerOnCluster = "platformservice." + openmcpconst.OpenMCPGroupName + "/dns" + + ReasonTargetClusterInteractionProblem = "TargetClusterInteractionProblem" ) diff --git a/api/dns/v1alpha1/zz_generated.deepcopy.go b/api/dns/v1alpha1/zz_generated.deepcopy.go index ad90150..0e427bc 100644 --- a/api/dns/v1alpha1/zz_generated.deepcopy.go +++ b/api/dns/v1alpha1/zz_generated.deepcopy.go @@ -76,10 +76,8 @@ func (in *DNSServiceConfigSpec) DeepCopyInto(out *DNSServiceConfigSpec) { in.ExternalDNSSource.DeepCopyInto(&out.ExternalDNSSource) if in.SecretsToCopy != nil { in, out := &in.SecretsToCopy, &out.SecretsToCopy - *out = make([]SecretCopy, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } + *out = new(SecretsToCopy) + (*in).DeepCopyInto(*out) } if in.HelmReleaseReconciliationInterval != nil { in, out := &in.HelmReleaseReconciliationInterval, &out.HelmReleaseReconciliationInterval @@ -235,3 +233,32 @@ func (in *SecretCopy) DeepCopy() *SecretCopy { in.DeepCopyInto(out) return out } + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SecretsToCopy) DeepCopyInto(out *SecretsToCopy) { + *out = *in + if in.ToPlatformCluster != nil { + in, out := &in.ToPlatformCluster, &out.ToPlatformCluster + *out = make([]SecretCopy, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + if in.ToTargetCluster != nil { + in, out := &in.ToTargetCluster, &out.ToTargetCluster + *out = make([]SecretCopy, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SecretsToCopy. +func (in *SecretsToCopy) DeepCopy() *SecretsToCopy { + if in == nil { + return nil + } + out := new(SecretsToCopy) + in.DeepCopyInto(out) + return out +} diff --git a/docs/config/dns-service-config.md b/docs/config/dns-service-config.md index af12d33..9aa98c4 100644 --- a/docs/config/dns-service-config.md +++ b/docs/config/dns-service-config.md @@ -10,6 +10,14 @@ metadata: name: dns namespace: openmcp-system spec: + secretsToCopy: # optional + toPlatformCluster: # optional + - source: + name: my-secret + target: # optional + name: my-copied-secret + toTargetCluster: [] # optional + externalDNSSource: chartName: charts/external-dns # path to the external-dns helm chart within the chosen repository git: @@ -31,6 +39,20 @@ spec: asdf: qwer ``` +#### 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. + +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. + +Secrets referenced in `secretsToCopy.toTargetCluster` will be copied into the namespace where the helm chart will be deployed into on the cluster represented by the reconciled `Cluster` resource. This is useful if secrets are referenced in the deployed chart's values. + +In both cases, if the entry's `target` field is set, the secret will be renamed to that name when copied, otherwise it will keep its source name. + +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. + +⚠️ **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 `spec.externalDNSSource` is required and describes where to find the helm chart for the [external-dns](https://github.com/kubernetes-sigs/external-dns) controller. Next to `chartName`, it has to contain exactly one of either `git`, `helm` or `oci`. The content of this field will then be used as `spec` for a Flux [`GitRepository`](https://fluxcd.io/flux/components/source/gitrepositories/), [`HelmRepository`](https://fluxcd.io/flux/components/source/helmrepositories/), or [`OCIRepository`](https://fluxcd.io/flux/components/source/ocirepositories/), respectively. @@ -97,3 +119,101 @@ purposeSelector: name: foo ``` Matches all `Cluster` resources that do not have `foo` in their purpose list. + +### Configuration Examples + +All examples below use a purpose selector that matches all `Cluster` resources which have `test` among their purposes. + +###### Example 1 - Git Repo with DNS Secret + +```yaml +apiVersion: dns.openmcp.cloud/v1alpha1 +kind: DNSServiceConfig +metadata: + name: dns + namespace: openmcp-system +spec: + secretsToCopy: + toTargetCluster: + - source: + name: route53-access + + externalDNSSource: + chartName: charts/external-dns + git: + url: https://github.com/kubernetes-sigs/external-dns + interval: 1h + ref: + tag: v0.19.0 + + externalDNSForPurposes: + - name: test + purposeSelector: + name: test + helmValues: + provider: + name: aws + env: + - name: AWS_DEFAULT_REGION + value: eu-central-1 + extraVolumes: + - name: aws-credentials + secret: + secretName: route53-access + extraVolumeMounts: + - name: aws-credentials + mountPath: /.aws + readOnly: true +``` + +###### Example 2 - OCI Repo with Auth Secret + +```yaml +apiVersion: dns.openmcp.cloud/v1alpha1 +kind: DNSServiceConfig +metadata: + name: dns + namespace: openmcp-system +spec: + secretsToCopy: + toPlatformCluster: + - source: + name: ghcr-access + + externalDNSSource: + oci: + url: oci://ghcr.io/my-user/external-dns + interval: 1h + ref: + tag: "1.19.0" + secretRef: + name: ghcr-access + + externalDNSForPurposes: + - name: test + purposeSelector: + name: test + helmValues: {} +``` + +###### Example 3 - Helm Repo + +```yaml +apiVersion: dns.openmcp.cloud/v1alpha1 +kind: DNSServiceConfig +metadata: + name: dns + namespace: openmcp-system +spec: + externalDNSSource: + chartName: external-dns@1.19.0 + helm: + url: https://kubernetes-sigs.github.io/external-dns/ + interval: 1h + + externalDNSForPurposes: + - name: test + purposeSelector: + name: test + helmValues: {} +``` diff --git a/go.mod b/go.mod index cf0c068..2e16ae2 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( github.com/openmcp-project/controller-utils v0.23.1 github.com/openmcp-project/openmcp-operator/api v0.15.1 github.com/openmcp-project/openmcp-operator/lib v0.15.1 - github.com/openmcp-project/platform-service-dns/api v0.0.1 + github.com/openmcp-project/platform-service-dns/api v0.0.2 github.com/spf13/cobra v1.10.1 k8s.io/api v0.34.1 k8s.io/apimachinery v0.34.1 diff --git a/internal/controllers/cluster/controller.go b/internal/controllers/cluster/controller.go index 30ca544..c8d39a3 100644 --- a/internal/controllers/cluster/controller.go +++ b/internal/controllers/cluster/controller.go @@ -16,10 +16,12 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/client-go/tools/clientcmd" "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/predicate" @@ -45,17 +47,25 @@ import ( dnsv1alpha1 "github.com/openmcp-project/platform-service-dns/api/dns/v1alpha1" ) -const ControllerName = "DNSCluster" -const defaultRequeueAfterDuration = 30 * time.Second +const ( + ControllerName = "DNSCluster" + defaultRequeueAfterDuration = 30 * time.Second + TargetClusterNamespace = "external-dns" + + SourceKindHelmRepository = "HelmRepository" + SourceKindGitRepository = "GitRepository" + SourceKindOCIRepository = "OCIRepository" +) type ClusterReconciler struct { - PlatformCluster *clusters.Cluster - eventRecorder record.EventRecorder - ProviderName string - ProviderNamespace string - Environment string - KnownClusters map[types.NamespacedName]struct{} - KnownClustersLock *sync.RWMutex + PlatformCluster *clusters.Cluster + eventRecorder record.EventRecorder + ProviderName string + ProviderNamespace string + Environment string + KnownClusters map[types.NamespacedName]struct{} + KnownClustersLock *sync.RWMutex + FakeClientMappings map[string]client.Client // only used for testing } func NewClusterReconciler(platformCluster *clusters.Cluster, recorder record.EventRecorder, providerName, providerNamespace, environment string) *ClusterReconciler { @@ -83,6 +93,8 @@ type ReconcileResult struct { SourceKind string // AccessRequest is the AccessRequest that provides access to the Cluster, if access was successfully obtained. AccessRequest *clustersv1alpha1.AccessRequest + // Access is the client that can be used to access the Cluster, if access was successfully obtained. + Access *clusters.Cluster // Message is an optional message to be printed in the generated event. Message string // ProviderConfig is the complete provider configuration. @@ -241,12 +253,47 @@ func (r *ClusterReconciler) handleCreateOrUpdate(ctx context.Context, c *cluster } rr.AccessRequest = ar - rr, copied := r.copySecrets(ctx, c, expectedLabels, rr) + // get access to Cluster + if ar.Status.SecretRef == nil { + rr.Message = fmt.Sprintf("AccessRequest '%s/%s' does not have a secretRef in its status despite being granted", ar.Namespace, ar.Name) + return rr + } + sec := &corev1.Secret{} + sec.Name = ar.Status.SecretRef.Name + sec.Namespace = ar.Status.SecretRef.Namespace + if err := r.PlatformCluster.Client().Get(ctx, client.ObjectKeyFromObject(sec), sec); err != nil { + rr.ReconcileError = errutils.WithReason(fmt.Errorf("error getting secret '%s/%s' referenced by AccessRequest '%s/%s': %w", sec.Namespace, sec.Name, ar.Namespace, ar.Name, err), clusterconst.ReasonPlatformClusterInteractionProblem) + return rr + } + kcfgData := sec.Data[clustersv1alpha1.SecretKeyKubeconfig] + if strings.HasPrefix(string(kcfgData), "fake:") && r.FakeClientMappings != nil { + log.Info("Using fake client for testing, this message should never appear outside of tests") + id := strings.TrimPrefix(string(kcfgData), "fake:") + fk := r.FakeClientMappings[id] + if fk == nil { + fk = fake.NewFakeClient() + r.FakeClientMappings[id] = fk + } + rr.Access = clusters.NewTestClusterFromClient(id, fk) + } else { + rest, err := clientcmd.RESTConfigFromKubeConfig(kcfgData) + if err != nil { + rr.ReconcileError = errutils.WithReason(fmt.Errorf("error creating REST config for Cluster from kubeconfig in secret '%s/%s': %w", sec.Namespace, sec.Name, err), clusterconst.ReasonInternalError) + return rr + } + rr.Access = clusters.New(ar.Name).WithRESTConfig(rest) + if err := rr.Access.InitializeClient(nil); err != nil { + rr.ReconcileError = errutils.WithReason(fmt.Errorf("error initializing client for Cluster from kubeconfig in secret '%s/%s': %w", sec.Namespace, sec.Name, err), clusterconst.ReasonInternalError) + return rr + } + } + + rr, copied := r.copySecrets(ctx, c.Namespace, expectedLabels, rr, platformClusterCopy) if rr.ReconcileError != nil || rr.Result.RequeueAfter > 0 { return rr } // remove any secrets that were copied in a previous run but are no longer configured to be copied - rr = r.removeSecrets(ctx, c, expectedLabels, rr, copied) + rr = r.removeSecrets(ctx, c.Namespace, expectedLabels, rr, platformClusterCopy, copied) if rr.ReconcileError != nil || rr.Result.RequeueAfter > 0 { return rr } @@ -256,6 +303,21 @@ func (r *ClusterReconciler) handleCreateOrUpdate(ctx context.Context, c *cluster return rr } + rr = r.ensureTargetClusterNamespace(ctx, TargetClusterNamespace, rr) + if rr.ReconcileError != nil || rr.Result.RequeueAfter > 0 { + return rr + } + + rr, copied = r.copySecrets(ctx, TargetClusterNamespace, expectedLabels, rr, targetClusterCopy) + if rr.ReconcileError != nil || rr.Result.RequeueAfter > 0 { + return rr + } + // remove any secrets that were copied in a previous run but are no longer configured to be copied + rr = r.removeSecrets(ctx, TargetClusterNamespace, expectedLabels, rr, targetClusterCopy, copied) + if rr.ReconcileError != nil || rr.Result.RequeueAfter > 0 { + return rr + } + rr = r.deployHelmRelease(ctx, c, expectedLabels, rr) if rr.ReconcileError != nil || rr.Result.RequeueAfter > 0 { return rr @@ -281,20 +343,76 @@ func (r *ClusterReconciler) handleDelete(ctx context.Context, c *clustersv1alpha return rr } + // get access to Cluster + accessRequestGone := false + ar := &clustersv1alpha1.AccessRequest{} + ar.SetName(accesslib.StableRequestNameFromLocalName(ControllerName, c.Name)) + ar.SetNamespace(c.Namespace) + if err := r.PlatformCluster.Client().Get(ctx, client.ObjectKeyFromObject(ar), ar); err != nil { + if !apierrors.IsNotFound(err) { + rr.ReconcileError = errutils.WithReason(fmt.Errorf("error getting AccessRequest '%s/%s': %w", ar.Namespace, ar.Name, err), clusterconst.ReasonPlatformClusterInteractionProblem) + return rr + } + accessRequestGone = true + } + if !accessRequestGone && ar.Status.IsGranted() { + if ar.Status.SecretRef == nil { + rr.Message = fmt.Sprintf("AccessRequest '%s/%s' does not have a secretRef in its status despite being granted", ar.Namespace, ar.Name) + return rr + } + sec := &corev1.Secret{} + sec.Name = ar.Status.SecretRef.Name + sec.Namespace = ar.Status.SecretRef.Namespace + if err := r.PlatformCluster.Client().Get(ctx, client.ObjectKeyFromObject(sec), sec); err != nil { + rr.ReconcileError = errutils.WithReason(fmt.Errorf("error getting secret '%s/%s' referenced by AccessRequest '%s/%s': %w", sec.Namespace, sec.Name, ar.Namespace, ar.Name, err), clusterconst.ReasonPlatformClusterInteractionProblem) + return rr + } + kcfgData := sec.Data[clustersv1alpha1.SecretKeyKubeconfig] + if strings.HasPrefix(string(kcfgData), "fake:") && r.FakeClientMappings != nil { + log.Info("Using fake client for testing, this message should never appear outside of tests") + id := strings.TrimPrefix(string(kcfgData), "fake:") + fk := r.FakeClientMappings[id] + if fk == nil { + fk = fake.NewFakeClient() + r.FakeClientMappings[id] = fk + } + rr.Access = clusters.NewTestClusterFromClient(id, fk) + } else { + rest, err := clientcmd.RESTConfigFromKubeConfig(kcfgData) + if err != nil { + rr.ReconcileError = errutils.WithReason(fmt.Errorf("error creating REST config for Cluster from kubeconfig in secret '%s/%s': %w", sec.Namespace, sec.Name, err), clusterconst.ReasonInternalError) + return rr + } + rr.Access = clusters.New(ar.Name).WithRESTConfig(rest) + if err := rr.Access.InitializeRESTConfig(); err != nil { + rr.ReconcileError = errutils.WithReason(fmt.Errorf("error initializing REST config for Cluster from kubeconfig in secret '%s/%s': %w", sec.Namespace, sec.Name, err), clusterconst.ReasonInternalError) + return rr + } + if err := rr.Access.InitializeClient(nil); err != nil { + rr.ReconcileError = errutils.WithReason(fmt.Errorf("error initializing client for Cluster from kubeconfig in secret '%s/%s': %w", sec.Namespace, sec.Name, err), clusterconst.ReasonInternalError) + return rr + } + } + + rr = r.removeSecrets(ctx, TargetClusterNamespace, expectedLabels, rr, targetClusterCopy, nil) + if rr.ReconcileError != nil || rr.Result.RequeueAfter > 0 { + return rr + } + } else { + log.Info("Skipping removal of copied secrets on target cluster because access is not available") + } + rr = r.undeployHelmChartSource(ctx, c, expectedLabels, rr) if rr.ReconcileError != nil || rr.Result.RequeueAfter > 0 { return rr } - rr = r.removeSecrets(ctx, c, expectedLabels, rr, nil) + rr = r.removeSecrets(ctx, c.Namespace, expectedLabels, rr, platformClusterCopy, nil) if rr.ReconcileError != nil || rr.Result.RequeueAfter > 0 { return rr } // delete AccessRequest - ar := &clustersv1alpha1.AccessRequest{} - ar.Name = accesslib.StableRequestNameFromLocalName(strings.ToLower(ControllerName), c.Name) - ar.Namespace = c.Namespace if err := r.PlatformCluster.Client().Delete(ctx, ar); err != nil { if !apierrors.IsNotFound(err) { rr.ReconcileError = errutils.WithReason(fmt.Errorf("error deleting AccessRequest '%s/%s': %w", ar.Namespace, ar.Name, err), clusterconst.ReasonPlatformClusterInteractionProblem) @@ -317,66 +435,94 @@ func (r *ClusterReconciler) handleDelete(ctx context.Context, c *clustersv1alpha return rr } +type copyTo string + +const ( + platformClusterCopy copyTo = "platform" + targetClusterCopy copyTo = "target" +) + // copySecrets copies the configured secrets into the Cluster namespace. // Returns a list of the names of the copied secrets. -func (r *ClusterReconciler) copySecrets(ctx context.Context, c *clustersv1alpha1.Cluster, expectedLabels map[string]string, rr ReconcileResult) (ReconcileResult, sets.Set[string]) { +func (r *ClusterReconciler) copySecrets(ctx context.Context, namespace string, expectedLabels map[string]string, rr ReconcileResult, copyTarget copyTo) (ReconcileResult, sets.Set[string]) { log := logging.FromContextOrPanic(ctx) copied := sets.New[string]() // copy secrets if configured - for i, stc := range rr.ProviderConfig.Spec.SecretsToCopy { - source := &corev1.Secret{} - source.Name = stc.Source.Name - source.Namespace = r.ProviderNamespace - log.Debug("Secret copying configured, getting source secret", "sourceNamespace", source.Namespace, "sourceName", source.Name, "index", i) - 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) + if rr.ProviderConfig.Spec.SecretsToCopy != nil { + var secretsToCopy []dnsv1alpha1.SecretCopy + var targetAccess client.Client + var interactionProblemReason string + switch copyTarget { + case platformClusterCopy: + secretsToCopy = rr.ProviderConfig.Spec.SecretsToCopy.ToPlatformCluster + targetAccess = r.PlatformCluster.Client() + interactionProblemReason = clusterconst.ReasonPlatformClusterInteractionProblem + case targetClusterCopy: + secretsToCopy = rr.ProviderConfig.Spec.SecretsToCopy.ToTargetCluster + targetAccess = rr.Access.Client() + interactionProblemReason = dnsv1alpha1.ReasonTargetClusterInteractionProblem + } + if targetAccess == nil { + rr.ReconcileError = errutils.WithReason(fmt.Errorf("no access to cluster '%s' for copying secrets", copyTarget), clusterconst.ReasonInternalError) 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.Namespace = c.Namespace - targetExists := true - if err := r.PlatformCluster.Client().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), clusterconst.ReasonPlatformClusterInteractionProblem) + for i, stc := range secretsToCopy { + source := &corev1.Secret{} + source.Name = stc.Source.Name + source.Namespace = r.ProviderNamespace + log.Debug("Secret copying configured, getting source secret", "sourceNamespace", source.Namespace, "sourceName", source.Name, "index", i) + 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) 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) - 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) + + // 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.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) + 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) return rr, copied } + targetExists = false } - } - log.Debug("Creating or updating target secret", "targetNamespace", target.Namespace, "targetName", target.Name, "index", i) - if _, err := controllerutil.CreateOrUpdate(ctx, r.PlatformCluster.Client(), target, func() error { - if err := controllerutil.SetOwnerReference(c, target, r.PlatformCluster.Scheme()); err != nil { - return fmt.Errorf("error setting owner reference on target secret '%s/%s' (index: %d): %w", target.Namespace, target.Name, i, err) + 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) + 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) + return rr, copied + } + } } - target.Labels = maputils.Merge(target.Labels, source.Labels, expectedLabels) - target.Annotations = maputils.Merge(target.Annotations, source.Annotations) - target.Data = make(map[string][]byte, len(source.Data)) - maps.Copy(target.Data, source.Data) - 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), clusterconst.ReasonPlatformClusterInteractionProblem) - return rr, copied + log.Debug("Creating or updating target secret", "targetNamespace", target.Namespace, "targetName", target.Name, "index", i) + 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) + target.Data = make(map[string][]byte, len(source.Data)) + maps.Copy(target.Data, source.Data) + 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) + return rr, copied + } + copied.Insert(target.Name) } - copied.Insert(target.Name) + rr.Message = fmt.Sprintf("Successfully copied %d secrets into namespace '%s' on cluster '%s'", len(secretsToCopy), namespace, copyTarget) } - rr.Message = fmt.Sprintf("Successfully copied %d secrets into Cluster namespace", len(rr.ProviderConfig.Spec.SecretsToCopy)) return rr, copied } @@ -418,7 +564,7 @@ func (r *ClusterReconciler) deployHelmChartSource(ctx context.Context, c *cluste helmRepo.Spec = *rr.ProviderConfig.Spec.ExternalDNSSource.Helm.DeepCopy() return nil } - rr.SourceKind = "HelmRepository" + rr.SourceKind = SourceKindHelmRepository for i := range existingHelm.Items { obj := &existingHelm.Items[i] if obj.GetName() != sourceName { @@ -437,7 +583,7 @@ func (r *ClusterReconciler) deployHelmChartSource(ctx context.Context, c *cluste gitRepo.Spec = *rr.ProviderConfig.Spec.ExternalDNSSource.Git.DeepCopy() return nil } - rr.SourceKind = "GitRepository" + rr.SourceKind = SourceKindGitRepository for i := range existingGit.Items { obj := &existingGit.Items[i] if obj.GetName() != sourceName { @@ -456,7 +602,7 @@ func (r *ClusterReconciler) deployHelmChartSource(ctx context.Context, c *cluste ociRepo.Spec = *rr.ProviderConfig.Spec.ExternalDNSSource.OCI.DeepCopy() return nil } - rr.SourceKind = "OCIRepository" + rr.SourceKind = SourceKindOCIRepository for i := range existingOCI.Items { obj := &existingOCI.Items[i] if obj.GetName() != sourceName { @@ -515,24 +661,36 @@ func (r *ClusterReconciler) deployHelmRelease(ctx context.Context, c *clustersv1 // labels hr.Labels = maputils.Merge(hr.Labels, expectedLabels) // chart - hr.Spec.Chart = &fluxhelmv2.HelmChartTemplate{ - Spec: fluxhelmv2.HelmChartTemplateSpec{ - SourceRef: fluxhelmv2.CrossNamespaceObjectReference{ - APIVersion: fluxsourcev1.SchemeBuilder.GroupVersion.String(), - Kind: rr.SourceKind, - Name: hr.Name, - Namespace: hr.Namespace, + if rr.SourceKind == SourceKindOCIRepository { + hr.Spec.Chart = nil + hr.Spec.ChartRef = &fluxhelmv2.CrossNamespaceSourceReference{ + APIVersion: fluxsourcev1.SchemeBuilder.GroupVersion.String(), + Kind: rr.SourceKind, + Name: hr.Name, + Namespace: hr.Namespace, + } + } else { + hr.Spec.ChartRef = nil + hr.Spec.Chart = &fluxhelmv2.HelmChartTemplate{ + Spec: fluxhelmv2.HelmChartTemplateSpec{ + SourceRef: fluxhelmv2.CrossNamespaceObjectReference{ + APIVersion: fluxsourcev1.SchemeBuilder.GroupVersion.String(), + Kind: rr.SourceKind, + Name: hr.Name, + Namespace: hr.Namespace, + }, }, - }, - } - chartNameVersion := strings.Split(rr.ProviderConfig.Spec.ExternalDNSSource.ChartName, "@") - hr.Spec.Chart.Spec.Chart = chartNameVersion[0] - if len(chartNameVersion) > 1 { - hr.Spec.Chart.Spec.Version = chartNameVersion[1] + } + chartNameVersion := strings.Split(rr.ProviderConfig.Spec.ExternalDNSSource.ChartName, "@") + hr.Spec.Chart.Spec.Chart = chartNameVersion[0] + if len(chartNameVersion) > 1 { + hr.Spec.Chart.Spec.Version = chartNameVersion[1] + } } // release information hr.Spec.ReleaseName = "external-dns" - hr.Spec.TargetNamespace = "external-dns" + hr.Spec.TargetNamespace = TargetClusterNamespace + hr.Spec.StorageNamespace = TargetClusterNamespace // values values := string(rr.Config.HelmValues.Raw) // at some point '<' and '>' get escaped and we have to match the escaped version here @@ -547,7 +705,6 @@ func (r *ClusterReconciler) deployHelmRelease(ctx context.Context, c *clustersv1 hr.Spec.Install = &fluxhelmv2.Install{} } hr.Spec.Install.CRDs = fluxhelmv2.CreateReplace - hr.Spec.Install.CreateNamespace = true if hr.Spec.Install.Remediation == nil { hr.Spec.Install.Remediation = &fluxhelmv2.InstallRemediation{} } @@ -657,19 +814,19 @@ func (r *ClusterReconciler) undeployHelmChartSource(ctx context.Context, c *clus toBeDeleted := []client.Object{} toBeDeleted = append(toBeDeleted, collections.ProjectSliceToSlice(existingHelm.Items, func(obj fluxsourcev1.HelmRepository) client.Object { if obj.GetObjectKind().GroupVersionKind().Kind == "" { - obj.SetGroupVersionKind(schema.GroupVersionKind{Group: fluxsourcev1.GroupVersion.Group, Version: fluxsourcev1.GroupVersion.Version, Kind: "HelmRepository"}) + obj.SetGroupVersionKind(schema.GroupVersionKind{Group: fluxsourcev1.GroupVersion.Group, Version: fluxsourcev1.GroupVersion.Version, Kind: SourceKindHelmRepository}) } return &obj })...) toBeDeleted = append(toBeDeleted, collections.ProjectSliceToSlice(existingGit.Items, func(obj fluxsourcev1.GitRepository) client.Object { if obj.GetObjectKind().GroupVersionKind().Kind == "" { - obj.SetGroupVersionKind(schema.GroupVersionKind{Group: fluxsourcev1.GroupVersion.Group, Version: fluxsourcev1.GroupVersion.Version, Kind: "GitRepository"}) + obj.SetGroupVersionKind(schema.GroupVersionKind{Group: fluxsourcev1.GroupVersion.Group, Version: fluxsourcev1.GroupVersion.Version, Kind: SourceKindGitRepository}) } return &obj })...) toBeDeleted = append(toBeDeleted, collections.ProjectSliceToSlice(existingOCI.Items, func(obj fluxsourcev1.OCIRepository) client.Object { if obj.GetObjectKind().GroupVersionKind().Kind == "" { - obj.SetGroupVersionKind(schema.GroupVersionKind{Group: fluxsourcev1.GroupVersion.Group, Version: fluxsourcev1.GroupVersion.Version, Kind: "OCIRepository"}) + obj.SetGroupVersionKind(schema.GroupVersionKind{Group: fluxsourcev1.GroupVersion.Group, Version: fluxsourcev1.GroupVersion.Version, Kind: SourceKindOCIRepository}) } return &obj })...) @@ -691,13 +848,28 @@ func (r *ClusterReconciler) undeployHelmChartSource(ctx context.Context, c *clus // removeSecrets removes all secrets from the Cluster namespace where the labels indicate they were created by this controller for the given Cluster. // Secrets listed in 'keep' are not deleted. // It does not wait for their deletion. -func (r *ClusterReconciler) removeSecrets(ctx context.Context, c *clustersv1alpha1.Cluster, expectedLabels map[string]string, rr ReconcileResult, keep sets.Set[string]) ReconcileResult { +func (r *ClusterReconciler) removeSecrets(ctx context.Context, namespace string, expectedLabels map[string]string, rr ReconcileResult, copyTarget copyTo, keep sets.Set[string]) ReconcileResult { log := logging.FromContextOrPanic(ctx) + var targetAccess client.Client + var interactionProblemReason string + switch copyTarget { + case platformClusterCopy: + targetAccess = r.PlatformCluster.Client() + interactionProblemReason = clusterconst.ReasonPlatformClusterInteractionProblem + case targetClusterCopy: + targetAccess = rr.Access.Client() + interactionProblemReason = dnsv1alpha1.ReasonTargetClusterInteractionProblem + } + if targetAccess == nil { + rr.ReconcileError = errutils.WithReason(fmt.Errorf("no access to cluster '%s' for removing secrets", copyTarget), clusterconst.ReasonInternalError) + return rr + } + // list existing secrets to detect obsolete ones existingSecrets := &corev1.SecretList{} - if err := r.PlatformCluster.Client().List(ctx, existingSecrets, client.InNamespace(c.Namespace), client.MatchingLabels(expectedLabels)); err != nil { - rr.ReconcileError = errutils.WithReason(fmt.Errorf("error listing existing Secret resources in target namespace '%s': %w", c.Namespace, err), clusterconst.ReasonPlatformClusterInteractionProblem) + if err := targetAccess.List(ctx, existingSecrets, client.InNamespace(namespace), client.MatchingLabels(expectedLabels)); err != nil { + rr.ReconcileError = errutils.WithReason(fmt.Errorf("error listing existing Secret resources in target namespace '%s': %w", namespace, err), interactionProblemReason) return rr } @@ -711,16 +883,47 @@ func (r *ClusterReconciler) removeSecrets(ctx context.Context, c *clustersv1alph continue } log.Info("Deleting copied secret", "resourceName", obj.GetName(), "resourceNamespace", obj.GetNamespace()) - if err := r.PlatformCluster.Client().Delete(ctx, obj); err != nil { + if err := targetAccess.Delete(ctx, obj); err != nil { if !apierrors.IsNotFound(err) { - rr.ReconcileError = errutils.WithReason(fmt.Errorf("error deleting Secret '%s/%s': %w", obj.GetNamespace(), obj.GetName(), err), clusterconst.ReasonPlatformClusterInteractionProblem) + rr.ReconcileError = errutils.WithReason(fmt.Errorf("error deleting Secret '%s/%s': %w", obj.GetNamespace(), obj.GetName(), err), interactionProblemReason) return rr } } deleted++ } - rr.Message = fmt.Sprintf("Deleted %d copied secrets from Cluster namespace, kept %d.", deleted, kept) + rr.Message = fmt.Sprintf("Deleted %d copied secrets from namespace '%s' on cluster '%s', kept %d.", deleted, namespace, copyTarget, kept) + return rr +} + +func (r *ClusterReconciler) ensureTargetClusterNamespace(ctx context.Context, namespace string, rr ReconcileResult) ReconcileResult { + log := logging.FromContextOrPanic(ctx) + + ns := &corev1.Namespace{} + ns.Name = namespace + + log.Debug("Checking if target namespace exists on target cluster", "targetNamespace", ns.Name) + if err := rr.Access.Client().Get(ctx, client.ObjectKeyFromObject(ns), ns); err != nil { + if !apierrors.IsNotFound(err) { + rr.ReconcileError = errutils.WithReason(fmt.Errorf("error getting target namespace '%s' on target cluster: %w", ns.Name, err), dnsv1alpha1.ReasonTargetClusterInteractionProblem) + return rr + } + } else { + log.Debug("Target namespace already exists on target cluster", "targetNamespace", ns.Name) + return rr + } + + // create target namespace + log.Info("Creating target namespace on target cluster", "targetNamespace", ns.Name) + // the namespace will not be removed again, so there is no need to add the usual managed labels + if err := rr.Access.Client().Create(ctx, ns); err != nil { + if !apierrors.IsAlreadyExists(err) { + rr.ReconcileError = errutils.WithReason(fmt.Errorf("error creating target namespace '%s' on target cluster: %w", ns.Name, err), dnsv1alpha1.ReasonTargetClusterInteractionProblem) + return rr + } + } + + rr.Message = fmt.Sprintf("Successfully created target namespace '%s' on target cluster.", ns.Name) return rr } diff --git a/internal/controllers/cluster/controller_test.go b/internal/controllers/cluster/controller_test.go index 5e70c1b..db8e19f 100644 --- a/internal/controllers/cluster/controller_test.go +++ b/internal/controllers/cluster/controller_test.go @@ -2,6 +2,7 @@ package cluster_test import ( + "fmt" "time" . "github.com/onsi/ginkgo/v2" @@ -42,23 +43,27 @@ const ( var platformScheme = install.InstallOperatorAPIsPlatform(runtime.NewScheme()) -func defaultTestSetup(testDirPathSegments ...string) *testutils.Environment { +func defaultTestSetup(testDirPathSegments ...string) (*testutils.Environment, *cluster.ClusterReconciler) { env := testutils.NewEnvironmentBuilder(). WithFakeClient(platformScheme). WithInitObjectPath(testDirPathSegments...). WithDynamicObjectsWithStatus(&clustersv1alpha1.AccessRequest{}). WithReconcilerConstructor(func(c client.Client) reconcile.Reconciler { - return cluster.NewClusterReconciler(clusters.NewTestClusterFromClient(platformCluster, c), nil, providerName, providerNamespace, environment) + rec := cluster.NewClusterReconciler(clusters.NewTestClusterFromClient(platformCluster, c), nil, providerName, providerNamespace, environment) + rec.FakeClientMappings = map[string]client.Client{} + return rec }). Build() - return env + rec, ok := env.Reconciler().(*cluster.ClusterReconciler) + Expect(ok).To(BeTrue(), "reconciler is not a ClusterReconciler") + return env, rec } var _ = Describe("ClusterReconciler", func() { It("should fail if no DNSServiceConfig exists", func() { - env := defaultTestSetup("testdata", "test-01") + env, _ := defaultTestSetup("testdata", "test-01") // delete any existing DNSServiceConfig Expect(env.Client().DeleteAllOf(env.Ctx, &dnsv1alpha1.DNSServiceConfig{})).To(Succeed()) @@ -69,7 +74,7 @@ var _ = Describe("ClusterReconciler", func() { }) It("should correctly match configs to clusters and create the flux resources", func() { - env := defaultTestSetup("testdata", "test-01") + env, rec := defaultTestSetup("testdata", "test-01") cfg := &dnsv1alpha1.DNSServiceConfig{} Expect(env.Client().Get(env.Ctx, client.ObjectKey{Name: providerName}, cfg)).To(Succeed()) @@ -127,7 +132,7 @@ var _ = Describe("ClusterReconciler", func() { "Name": Equal(c1.Name), "Namespace": Equal("foo"), }))) - // copied secrets (including deletion of the obsolete one) + // copied secrets (including deletion of the obsolete one) on platform cluster ss := &corev1.SecretList{} Expect(env.Client().List(env.Ctx, ss, client.InNamespace(c1.Namespace), client.MatchingLabels(expectedLabels))).To(Succeed()) Expect(ss.Items).To(ConsistOf( @@ -144,13 +149,26 @@ var _ = Describe("ClusterReconciler", func() { "Data": Equal(map[string][]byte{"foo": []byte("bar")}), }), )) - for _, s := range ss.Items { - Expect(s.OwnerReferences).To(ContainElements(MatchFields(IgnoreExtras, Fields{ - "APIVersion": Equal(clustersv1alpha1.GroupVersion.String()), - "Kind": Equal("Cluster"), - "Name": Equal(c1.Name), - })), "secret '%s/%s' does not have the expected owner reference", s.Namespace, s.Name) - } + // namespace on target cluster + ns := &corev1.Namespace{} + ns.Name = cluster.TargetClusterNamespace + Expect(rec.FakeClientMappings["foo/cluster-01"].Get(env.Ctx, client.ObjectKeyFromObject(ns), ns)).To(Succeed()) + // 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-target-secret-copy"), + }), + "Data": Equal(map[string][]byte{"foobar": []byte("foobar")}), + }), + MatchFields(IgnoreExtras, Fields{ + "ObjectMeta": MatchFields(IgnoreExtras, Fields{ + "Name": Equal("my-other-secret"), + }), + "Data": Equal(map[string][]byte{"foo": []byte("bar")}), + }), + )) c2 := &clustersv1alpha1.Cluster{} Expect(env.Client().Get(env.Ctx, client.ObjectKey{Name: "cluster-02", Namespace: "bar"}, c2)).To(Succeed()) @@ -202,7 +220,7 @@ var _ = Describe("ClusterReconciler", func() { "Name": Equal(c2.Name), "Namespace": Equal("bar"), }))) - // copied secrets + // copied secrets on platform cluster Expect(env.Client().List(env.Ctx, ss, client.InNamespace(c2.Namespace), client.MatchingLabels(expectedLabels))).To(Succeed()) Expect(ss.Items).To(ConsistOf( MatchFields(IgnoreExtras, Fields{ @@ -218,17 +236,28 @@ var _ = Describe("ClusterReconciler", func() { "Data": Equal(map[string][]byte{"foo": []byte("bar")}), }), )) - for _, s := range ss.Items { - Expect(s.OwnerReferences).To(ContainElements(MatchFields(IgnoreExtras, Fields{ - "APIVersion": Equal(clustersv1alpha1.GroupVersion.String()), - "Kind": Equal("Cluster"), - "Name": Equal(c2.Name), - })), "secret '%s/%s' does not have the expected owner reference", s.Namespace, s.Name) - } + // namespace on target cluster + Expect(rec.FakeClientMappings["bar/cluster-02"].Get(env.Ctx, client.ObjectKeyFromObject(ns), ns)).To(Succeed()) + // copied secrets on target cluster + Expect(rec.FakeClientMappings["bar/cluster-02"].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-target-secret-copy"), + }), + "Data": Equal(map[string][]byte{"foobar": []byte("foobar")}), + }), + MatchFields(IgnoreExtras, Fields{ + "ObjectMeta": MatchFields(IgnoreExtras, Fields{ + "Name": Equal("my-other-secret"), + }), + "Data": Equal(map[string][]byte{"foo": []byte("bar")}), + }), + )) }) It("should correctly match complex purpose selectors and don't create resources if no purpose selector matches", func() { - env := defaultTestSetup("testdata", "test-02") + env, _ := defaultTestSetup("testdata", "test-02") cfg := &dnsv1alpha1.DNSServiceConfig{} Expect(env.Client().Get(env.Ctx, client.ObjectKey{Name: providerName}, cfg)).To(Succeed()) @@ -293,7 +322,7 @@ var _ = Describe("ClusterReconciler", func() { }) It("should use finalizers and remove resources when the Cluster is being deleted", func() { - env := defaultTestSetup("testdata", "test-01") + env, _ := defaultTestSetup("testdata", "test-01") cfg := &dnsv1alpha1.DNSServiceConfig{} Expect(env.Client().Get(env.Ctx, client.ObjectKey{Name: providerName}, cfg)).To(Succeed()) @@ -363,7 +392,7 @@ var _ = Describe("ClusterReconciler", func() { }) It("should delete obsolete flux sources", func() { - env := defaultTestSetup("testdata", "test-01") + env, _ := defaultTestSetup("testdata", "test-01") cfg := &dnsv1alpha1.DNSServiceConfig{} Expect(env.Client().Get(env.Ctx, client.ObjectKey{Name: providerName}, cfg)).To(Succeed()) @@ -407,7 +436,7 @@ var _ = Describe("ClusterReconciler", func() { }) It("should create a GitRepository if configured", func() { - env := defaultTestSetup("testdata", "test-03") + env, _ := defaultTestSetup("testdata", "test-03") cfg := &dnsv1alpha1.DNSServiceConfig{} Expect(env.Client().Get(env.Ctx, client.ObjectKey{Name: providerName}, cfg)).To(Succeed()) @@ -440,7 +469,7 @@ var _ = Describe("ClusterReconciler", func() { }) It("should create a HelmRepository if configured", func() { - env := defaultTestSetup("testdata", "test-04") + env, _ := defaultTestSetup("testdata", "test-04") cfg := &dnsv1alpha1.DNSServiceConfig{} Expect(env.Client().Get(env.Ctx, client.ObjectKey{Name: providerName}, cfg)).To(Succeed()) @@ -473,7 +502,7 @@ var _ = Describe("ClusterReconciler", func() { }) It("should replace the special keywords in the values correctly", func() { - env := defaultTestSetup("testdata", "test-03") + env, _ := defaultTestSetup("testdata", "test-03") cfg := &dnsv1alpha1.DNSServiceConfig{} Expect(env.Client().Get(env.Ctx, client.ObjectKey{Name: providerName}, cfg)).To(Succeed()) @@ -503,6 +532,28 @@ var _ = Describe("ClusterReconciler", func() { Expect(valueData).To(HaveKeyWithValue("providerNamespace", providerNamespace)) }) + 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{} + Expect(env.Client().Get(env.Ctx, client.ObjectKey{Name: "my-auth", Namespace: "test"}, s1)).To(Succeed()) + Expect(s1.Labels).To(BeEmpty()) + 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()) + + Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(s1), s1)).To(Succeed()) + // verify that the secret was not copied (no label added) + Expect(s1.Labels).To(BeEmpty()) + }) + }) func fakeAccessRequestReadiness(env *testutils.Environment, c *clustersv1alpha1.Cluster) { @@ -521,7 +572,7 @@ func fakeAccessRequestReadiness(env *testutils.Environment, c *clustersv1alpha1. sec.Name = ar.Status.SecretRef.Name sec.Namespace = ar.Status.SecretRef.Namespace sec.Data = map[string][]byte{ - clustersv1alpha1.SecretKeyKubeconfig: []byte("fake"), + clustersv1alpha1.SecretKeyKubeconfig: fmt.Appendf(nil, "fake:%s/%s", c.Namespace, c.Name), } Expect(env.Client().Create(env.Ctx, sec)).To(Succeed()) } diff --git a/internal/controllers/cluster/testdata/test-01/config.yaml b/internal/controllers/cluster/testdata/test-01/config.yaml index 3dba821..65f492f 100644 --- a/internal/controllers/cluster/testdata/test-01/config.yaml +++ b/internal/controllers/cluster/testdata/test-01/config.yaml @@ -4,12 +4,20 @@ metadata: name: dns-service spec: secretsToCopy: - - source: - name: my-auth - target: - name: my-auth-copy - - source: - name: my-other-secret + toPlatformCluster: + - source: + name: my-auth + target: + name: my-auth-copy + - source: + name: my-other-secret + toTargetCluster: + - source: + name: my-target-secret + target: + name: my-target-secret-copy + - source: + name: my-other-secret externalDNSSource: oci: url: "oci://example.org/repo/charts" diff --git a/internal/controllers/cluster/testdata/test-01/secret-04.yaml b/internal/controllers/cluster/testdata/test-01/secret-04.yaml new file mode 100644 index 0000000..1b4db42 --- /dev/null +++ b/internal/controllers/cluster/testdata/test-01/secret-04.yaml @@ -0,0 +1,8 @@ +apiVersion: v1 +kind: Secret +metadata: + name: my-target-secret + namespace: test +data: + foobar: Zm9vYmFy +type: Opaque diff --git a/internal/controllers/cluster/testdata/test-05/cluster-01.yaml b/internal/controllers/cluster/testdata/test-05/cluster-01.yaml new file mode 100644 index 0000000..9bd910c --- /dev/null +++ b/internal/controllers/cluster/testdata/test-05/cluster-01.yaml @@ -0,0 +1,12 @@ +apiVersion: clusters.openmcp.cloud/v1alpha1 +kind: Cluster +metadata: + name: cluster-01 + namespace: test +spec: + kubernetes: {} + profile: my-profile + purposes: + - foo + - bar + tenancy: Shared diff --git a/internal/controllers/cluster/testdata/test-05/config.yaml b/internal/controllers/cluster/testdata/test-05/config.yaml new file mode 100644 index 0000000..f7b6db5 --- /dev/null +++ b/internal/controllers/cluster/testdata/test-05/config.yaml @@ -0,0 +1,25 @@ +apiVersion: dns.openmcp.cloud/v1alpha1 +kind: DNSServiceConfig +metadata: + name: dns-service +spec: + secretsToCopy: + toPlatformCluster: + - source: + name: my-auth + 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-05/secret-01.yaml b/internal/controllers/cluster/testdata/test-05/secret-01.yaml new file mode 100644 index 0000000..4941276 --- /dev/null +++ b/internal/controllers/cluster/testdata/test-05/secret-01.yaml @@ -0,0 +1,8 @@ +apiVersion: v1 +kind: Secret +metadata: + name: my-auth + namespace: test +data: + key: dmFsdWU= +type: Opaque