From 4357d12a4d8c076132d56de283fd992bff710974 Mon Sep 17 00:00:00 2001 From: "Nico Andres (SAP)" Date: Thu, 28 May 2026 09:30:27 +0200 Subject: [PATCH 01/20] Add support for custom ca On-behalf-of: @SAP nico.andres@sap.com Signed-off-by: Nico Andres (SAP) --- README.md | 11 +- ...open-control-plane.io_providerconfigs.yaml | 9 + api/v1alpha1/providerconfig_types.go | 14 ++ api/v1alpha1/zz_generated.deepcopy.go | 2 +- docs/configuration/image-localization.md | 43 +++- internal/controller/flux_controller.go | 47 +++- pkg/flux/flux.go | 7 + pkg/flux/helm.go | 112 +++++++++ pkg/flux/helm_test.go | 230 ++++++++++++++++++ pkg/flux/secret.go | 16 +- pkg/flux/secret_test.go | 79 ++++-- 11 files changed, 539 insertions(+), 31 deletions(-) diff --git a/README.md b/README.md index f337c25..71d614e 100644 --- a/README.md +++ b/README.md @@ -146,6 +146,8 @@ metadata: spec: # Optional: Reconciliation interval pollInterval: "5m" + # Optional: Secret for custom CA certificate/bundle (will be copied to ManagedControlPlane) + certSecretRef: "custom-ca-cert" # The Flux versions that can be installed versions: - version: "2.8.3" @@ -170,10 +172,11 @@ spec: tag: v1.8.1 ``` -| Field | Type | Description | -| ------------------- | -------- | ---------------------------------------------- | -| `spec.pollInterval` | duration | How often to reconcile resources (default: 1m) | -| `spec.versions` | array | The versions of Flux that can be installed | +| Field | Type | Description | +| ------------------- | -------- | ------------------------------------------------ | +| `spec.pollInterval` | duration | How often to reconcile resources (default: 1m) | +| `certSecretRef` | object | SecretRef for chart registry trust establishment | +| `spec.versions` | array | The versions of Flux that can be installed | A version item is defined as follows: diff --git a/api/crds/manifests/flux.services.open-control-plane.io_providerconfigs.yaml b/api/crds/manifests/flux.services.open-control-plane.io_providerconfigs.yaml index a228b44..5083469 100644 --- a/api/crds/manifests/flux.services.open-control-plane.io_providerconfigs.yaml +++ b/api/crds/manifests/flux.services.open-control-plane.io_providerconfigs.yaml @@ -41,6 +41,15 @@ spec: spec: description: spec defines the desired state of ProviderConfig properties: + certSecretRef: + description: |- + certSecretRef is a reference to a Secret in the service provider's namespace + containing a single PEM-encoded CA certificate or a bundle used when pulling + the Helm chart from a private OCI registry. + The secret will be copied to the tenant namespace on the platform cluster + and referenced by the OCIRepository. + The secret must be of type kubernetes.io/opaque. + type: string pollInterval: default: 1m description: PollInterval determines how often to reconcile resources diff --git a/api/v1alpha1/providerconfig_types.go b/api/v1alpha1/providerconfig_types.go index f9c1d4e..35a26c2 100644 --- a/api/v1alpha1/providerconfig_types.go +++ b/api/v1alpha1/providerconfig_types.go @@ -35,6 +35,20 @@ type ProviderConfigSpec struct { // +kubebuilder:default:="1m" // +kubebuilder:validation:Format=duration PollInterval *metav1.Duration `json:"pollInterval,omitempty"` + + // CertSecretRef is a reference to a Secret in the service provider's namespace + // containing a single PEM-encoded CA certificate or a bundle used when pulling + // the Helm chart from a private OCI registry. + // The secret will be copied to the tenant namespace on the platform cluster + // and referenced by the OCIRepository. + // The secret is also copied to the flux-system namespace on the MCP cluster and is + // mounted into the source-controller. + // The secret must be of type kubernetes.io/opaque and store the PEM-encoded + // certificate/bundle under the ca.crt key. + // It can also contain tls.crt and tls.key keys for mTLS. + // (https://fluxcd.io/flux/components/source/ocirepositories/#mutual-tls-authentication) + // +optional + CertSecretRef string `json:"certSecretRef,omitempty"` } // FluxVersion defines a version of Flux that can be installed diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index cadbb0b..70e148d 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -6,7 +6,7 @@ package v1alpha1 import ( apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" ) diff --git a/docs/configuration/image-localization.md b/docs/configuration/image-localization.md index ee1759b..9d60541 100644 --- a/docs/configuration/image-localization.md +++ b/docs/configuration/image-localization.md @@ -13,6 +13,7 @@ In air-gapped environments, you typically need to: The Flux service provider handles this through: - **`chartPullSecret`**: Credentials for pulling the Helm chart from a private OCI registry +- **`certSecretRef`**: Private CA certificate for pulling the Helm chart from a private OCI registry - **`values.imagePullSecrets`**: Credentials for pulling Flux controller images (specified in Helm values) - **`values`**: Custom Helm values for image location overrides @@ -24,15 +25,18 @@ flowchart TB subgraph PC[Platform Cluster] subgraph SPN[Service Provider Namespace] chartsecret([chart-pull-secret]) + customcasecret([custom-ca-secret]) imgsecret1([image-pull-secret-1]) imgsecret2([image-pull-secret-2]) end subgraph TN[Tenant Namespace] chartsecretcopy([chart-pull-secret copy]) + customcasecretcopy([custom-ca-secret copy]) ocirepo([OCIRepository]) helmrel([HelmRelease]) ocirepo -. refs .-> chartsecretcopy + ocirepo -. refs .-> customcasecretcopy end end @@ -40,15 +44,19 @@ flowchart TB subgraph FS[flux-system namespace] imgcopy1([image-pull-secret-1]) imgcopy2([image-pull-secret-2]) + casecretcopy([custom-ca-secret]) fluxctrl[Flux Controllers] fluxctrl -. uses .-> imgcopy1 fluxctrl -. uses .-> imgcopy2 + fluxctrl -. uses .-> casecretcopy end end chartsecret -- copied to --> chartsecretcopy + customcasecret -- copied to --> customcasecretcopy imgsecret1 -- copied to --> imgcopy1 imgsecret2 -- copied to --> imgcopy2 + customcasecret -- copied to --> casecretcopy helmrel -- installs --> fluxctrl ``` @@ -63,6 +71,13 @@ metadata: name: flux-provider-config spec: versions: + # Secret with single PEM-encoded custom CA root certificate or bundle stored + # on the ca.crt key. + # Must exist in the service provider's namespace on the platform cluster + # Will be copied to the tenant namespace on the platform cluster and to the + # flux-system namespace on the ManagedControlPlane. + certSecretRef: "custom-ca-cert" + - version: "2.8.3" chartVersion: "2.18.2" # Flux Helm chart location (private OCI registry) @@ -110,6 +125,11 @@ kubectl create secret docker-registry image-registry-credentials \ --docker-server=registry.internal.corp \ --docker-username= \ --docker-password= + +# Custom CA certificate secret (for OCI registry connection) +kubectl create secret generic custom-ca-cert \ + --namespace \ + --ca.crt= ``` ## How It Works @@ -120,6 +140,22 @@ kubectl create secret docker-registry image-registry-credentials \ 2. The `OCIRepository` resource references this secret via `spec.secretRef` 3. The Flux Source Controller uses this secret to authenticate when pulling the Helm chart +### Custom CA Secret + +1. The secret specified in `certSecretRef` is copied from the service provider's namespace to the tenant namespace on the platform cluster +2. The `OCIRepository` resource references this secret via `spec.certSecretRef` +3. The Flux Source Controller uses this secret to establish a trusted connection with the OCI registry when pulling the Helm chart +4. The secret is also copied to the MCP cluster flux-system namespace and mounted into the Flux Source Controller +by adding a volume and volumeMount in to the helm values. Flux in the target MCP hence is also able to verify +self-signed certificates used by the OCI repository. + +> [!CAUTION] +> The custom CA certificate is not propagated to the MCP cluster nodes. If you want to pull images from the same OCI registry you must add the custom CA certificate to the MCP cluster nodes yourself. + +> [!NOTE] +> The secret referenced in certSecretRef can also contain two additional keys tls.key and tls.crt which will be +> used for mTLS (see here https://fluxcd.io/flux/components/source/ocirepositories/#mutual-tls-authentication) + ### Image Pull Secrets 1. Secrets specified in `values.imagePullSecrets` are extracted from the Helm values @@ -138,6 +174,8 @@ metadata: spec: chartUrl: "oci://harbor.corp.internal/charts/flux2" chartPullSecret: "harbor-credentials" + # Secret containing custom ca cert - will be copied to ManagedControlPlane + certSecretRef: "harbor-corp-internal-ca-cert" values: # Image pull secrets - will be copied to ManagedControlPlane imagePullSecrets: @@ -186,16 +224,17 @@ Verify secrets are copied to the correct namespaces: ```bash # Platform cluster - tenant namespace -kubectl get secrets -n mcp-- | grep -E "chart|image" +kubectl get secrets -n mcp-- | grep -E "chart|image|ca" # ManagedControlPlane - flux-system namespace -kubectl get secrets -n flux-system | grep -E "image" +kubectl get secrets -n flux-system | grep -E "image|ca" ``` ### Check OCIRepository Secret Reference ```bash kubectl get ocirepository flux -n mcp-- -o jsonpath='{.spec.secretRef}' +kubectl get ocirepository flux -n mcp-- -o jsonpath='{.spec.certSecretRef}' ``` ### Check HelmRelease Values diff --git a/internal/controller/flux_controller.go b/internal/controller/flux_controller.go index 7e1b1d8..96dc04d 100644 --- a/internal/controller/flux_controller.go +++ b/internal/controller/flux_controller.go @@ -149,10 +149,11 @@ func (r *FluxReconciler) createObjectManager(obj *apiv1alpha1.Flux, pc *apiv1alp mcpCluster := flux.NewManagedCluster(clusters.MCPCluster, clusters.MCPCluster.RESTConfig(), fluxNamespace, flux.ManagedControlPlane) // Sync image pull secrets from platform cluster to MCP - flux.ManagePullSecrets(mcpCluster, helmValues.ImagePullSecrets, flux.SecretCopyConfig{ + flux.ManageSecrets(mcpCluster, helmValues.ImagePullSecrets, flux.SecretCopyConfig{ SourceClient: r.PlatformCluster.Client(), SourceNamespace: r.PodNamespace, TargetNamespace: fluxNamespace, + TargetType: corev1.SecretTypeDockerConfigJson, }) // Sync chart pull secret within platform cluster from pod namespace to tenant namespace @@ -162,13 +163,48 @@ func (r *FluxReconciler) createObjectManager(obj *apiv1alpha1.Flux, pc *apiv1alp if err != nil { return nil, fmt.Errorf("error generating secret name: %w", err) } - flux.ManagePullSecrets(platformCluster, []corev1.LocalObjectReference{ + flux.ManageSecrets(platformCluster, []corev1.LocalObjectReference{ {Name: fluxVersion.ChartPullSecret}, }, flux.SecretCopyConfig{ SourceClient: r.PlatformCluster.Client(), SourceNamespace: r.PodNamespace, TargetNamespace: tenantNamespace, TargetName: prefixedChartPullSecret, + TargetType: corev1.SecretTypeDockerConfigJson, + }) + } + + var prefixedCertSecret string + if pc.Spec.CertSecretRef != "" { + // add custom ca volume and volumeMount to helm values + fluxVersion.Values, err = flux.AddCaToHelmValues(fluxVersion.Values, pc.Spec.CertSecretRef) + if err != nil { + return nil, fmt.Errorf("failed to add ca volume to helm values: %w", err) + } + + // Sync image pull secrets from platform cluster to MCP + flux.ManageSecrets(mcpCluster, []corev1.LocalObjectReference{ + {Name: pc.Spec.CertSecretRef}, + }, flux.SecretCopyConfig{ + SourceClient: r.PlatformCluster.Client(), + SourceNamespace: r.PodNamespace, + TargetNamespace: fluxNamespace, + TargetType: corev1.SecretTypeOpaque, + }) + + // Sync ca secret within platform cluster from pod namespace to tenant namespace + prefixedCertSecret, err = flux.PrefixSecretName(pc.Spec.CertSecretRef) + if err != nil { + return nil, fmt.Errorf("error generating secret name: %w", err) + } + flux.ManageSecrets(platformCluster, []corev1.LocalObjectReference{ + {Name: pc.Spec.CertSecretRef}, + }, flux.SecretCopyConfig{ + SourceClient: r.PlatformCluster.Client(), + SourceNamespace: r.PodNamespace, + TargetNamespace: tenantNamespace, + TargetName: prefixedCertSecret, + TargetType: corev1.SecretTypeOpaque, }) } @@ -177,6 +213,7 @@ func (r *FluxReconciler) createObjectManager(obj *apiv1alpha1.Flux, pc *apiv1alp Cluster: platformCluster, MCPNamespace: fluxNamespace, ChartPullSecretName: prefixedChartPullSecret, + CertSecretName: prefixedCertSecret, Obj: obj, ProviderConfig: pc, ClusterContext: clusters, @@ -193,8 +230,12 @@ func (r *FluxReconciler) createObjectManager(obj *apiv1alpha1.Flux, pc *apiv1alp { Name: prefixedChartPullSecret, }, + { + Name: prefixedCertSecret, + }, }) - controlPlaneCleaner := flux.NewSecretCleaner(mcpCluster, fluxNamespace, helmValues.ImagePullSecrets) + cpSecretsToKeep := append(helmValues.ImagePullSecrets, corev1.LocalObjectReference{Name: pc.Spec.CertSecretRef}) + controlPlaneCleaner := flux.NewSecretCleaner(mcpCluster, fluxNamespace, cpSecretsToKeep) mgr.AddCleaner(platformCleaner) mgr.AddCleaner(controlPlaneCleaner) diff --git a/pkg/flux/flux.go b/pkg/flux/flux.go index 3ea1099..468711a 100644 --- a/pkg/flux/flux.go +++ b/pkg/flux/flux.go @@ -48,6 +48,8 @@ type ManageFluxResourcesParams struct { MCPNamespace string // ChartPullSecretName defines the name of the secret copy that will be placed in the Cluster namespace ChartPullSecretName string + // CertSecretName defines the name of the secret copy that will be placed in the Cluster namespace + CertSecretName string // Obj is the tenant API object that is being reconciled Obj *apiv1alpha1.Flux // ProviderConfig of the current reconciliation context @@ -87,6 +89,11 @@ func ManageFluxResources(p ManageFluxResourcesParams) { Name: p.ChartPullSecretName, } } + if p.CertSecretName != "" { + ociRepo.Spec.CertSecretRef = &meta.LocalObjectReference{ + Name: p.CertSecretName, + } + } return nil }, DependsOn: []ManagedObject{}, diff --git a/pkg/flux/helm.go b/pkg/flux/helm.go index e9a9e86..87824aa 100644 --- a/pkg/flux/helm.go +++ b/pkg/flux/helm.go @@ -15,6 +15,7 @@ package flux import ( "encoding/json" + "fmt" corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" @@ -44,3 +45,114 @@ func ExtractHelmValues(values *apiextensionsv1.JSON) (*HelmValues, error) { return vals, nil } + +func AddCaToHelmValues(values *apiextensionsv1.JSON, secretName string) (*apiextensionsv1.JSON, error) { + var root = map[string]json.RawMessage{} + var sourceController = map[string]json.RawMessage{} + var volumes []corev1.Volume + var volumeMounts []corev1.VolumeMount + + caVolume := corev1.Volume{ + Name: "sp-flux-custom-ca", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: secretName, + Items: []corev1.KeyToPath{ + { + Key: "ca.crt", + Path: "sp-flux-custom-ca.crt", + }, + }, + }, + }, + } + + caVolumeMount := corev1.VolumeMount{ + Name: "sp-flux-custom-ca", + ReadOnly: true, + MountPath: "/etc/ssl/certs/sp-flux-custom-ca.crt", + SubPath: "sp-flux-custom-ca.crt", + } + + if values != nil && len(values.Raw) > 0 { + if err := json.Unmarshal(values.Raw, &root); err != nil { + return nil, fmt.Errorf("failed to unmarshal helm values: %w", err) + } + + if err := unmarshalIfPresent(root, "sourceController", &sourceController); err != nil { + return nil, fmt.Errorf("failed to unmarshal sourceController: %w", err) + } + + if err := unmarshalIfPresent(sourceController, "volumes", &volumes); err != nil { + return nil, fmt.Errorf("failed to unmarshal sourceController.volumes: %w", err) + } + + if err := unmarshalIfPresent(sourceController, "volumeMounts", &volumeMounts); err != nil { + return nil, fmt.Errorf("failed to unmarshal sourceController.volumeMounts: %w", err) + } + } + + volumes = removeConflictingVolumes(volumes, caVolume) + volumeMounts = removeConflictingVolumeMounts(volumeMounts, caVolumeMount) + + volumes = append(volumes, caVolume) + volumeMounts = append(volumeMounts, caVolumeMount) + + volumesRaw, err := json.Marshal(volumes) + if err != nil { + return nil, fmt.Errorf("failed to marshal sourceController.volumes: %w", err) + } + + volumeMountsRaw, err := json.Marshal(volumeMounts) + if err != nil { + return nil, fmt.Errorf("failed to marshal sourceController.volumeMounts: %w", err) + } + + sourceController["volumes"] = volumesRaw + sourceController["volumeMounts"] = volumeMountsRaw + + sourceControllerRaw, err := json.Marshal(sourceController) + if err != nil { + return nil, fmt.Errorf("failed to marshal sourceController: %w", err) + } + + root["sourceController"] = sourceControllerRaw + + out, err := json.Marshal(root) + if err != nil { + return nil, fmt.Errorf("failed to marshal helm values: %w", err) + } + + return &apiextensionsv1.JSON{Raw: out}, nil +} + +func removeConflictingVolumes(volumes []corev1.Volume, caVolume corev1.Volume) []corev1.Volume { + r := []corev1.Volume{} + for _, volume := range volumes { + if volume.Name != caVolume.Name { + r = append(r, volume) + } + } + return r +} + +func removeConflictingVolumeMounts(volumeMounts []corev1.VolumeMount, caVolumeMount corev1.VolumeMount) []corev1.VolumeMount { + r := []corev1.VolumeMount{} + for _, volumeMount := range volumeMounts { + if volumeMount.MountPath != caVolumeMount.MountPath && volumeMount.Name != caVolumeMount.Name { + r = append(r, volumeMount) + } + } + return r +} + +func unmarshalIfPresent(obj map[string]json.RawMessage, key string, out any) error { + raw, ok := obj[key] + if !ok || len(raw) == 0 { + return nil + } + if err := json.Unmarshal(raw, out); err != nil { + return fmt.Errorf("invalid %s JSON: %w", key, err) + } + return nil +} diff --git a/pkg/flux/helm_test.go b/pkg/flux/helm_test.go index a91dca7..e8e018d 100644 --- a/pkg/flux/helm_test.go +++ b/pkg/flux/helm_test.go @@ -19,6 +19,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" ) @@ -130,6 +131,235 @@ func TestExtractHelmValues(t *testing.T) { } } +func TestAddCaToHelmValues(t *testing.T) { + const ( + volumeName = "sp-flux-custom-ca" + secretName = "custom-ca-secret" + secretKey = "ca.crt" + secretPath = "sp-flux-custom-ca.crt" + mountPath = "/etc/ssl/certs/sp-flux-custom-ca.crt" + subPath = "sp-flux-custom-ca.crt" + ) + + expectedCaVolume := corev1.Volume{ + Name: volumeName, + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: secretName, + Items: []corev1.KeyToPath{{ + Key: secretKey, + Path: secretPath, + }}, + }, + }, + } + expectedCaVolumeMount := corev1.VolumeMount{ + Name: volumeName, + ReadOnly: true, + MountPath: mountPath, + SubPath: subPath, + } + + tests := []struct { + name string + values *apiextensionsv1.JSON + wantErr string + checkValue func(t *testing.T, out *apiextensionsv1.JSON) + }{ + { + name: "Adds SourceController Volumes and VolumeMounts when no helm values are set", + values: nil, + checkValue: func(t *testing.T, out *apiextensionsv1.JSON) { + require.NotNil(t, out) + + expected := buildHelmValues(t, + withSourceControllerVolumes(expectedCaVolume), + withSourceControllerVolumeMounts(expectedCaVolumeMount), + ) + + assert.JSONEq(t, string(expected.Raw), string(out.Raw)) + }, + }, + { + name: "Preserves existing helm values and adds CA entries", + values: buildHelmValues(t, + withRootField("namespace", "other-namespace"), + withSourceControllerField( + "resources", map[string]any{ + "limits": map[string]any{ + "memory": "256Mi", + }, + }), + withSourceControllerVolumes(corev1.Volume{Name: "existing-volume"}), + withSourceControllerVolumeMounts( + corev1.VolumeMount{Name: "existing-volume", MountPath: "/tmp/existing"}), + ), + checkValue: func(t *testing.T, out *apiextensionsv1.JSON) { + require.NotNil(t, out) + + expected := buildHelmValues(t, + withRootField("namespace", "other-namespace"), + withSourceControllerField("resources", map[string]any{ + "limits": map[string]any{"memory": "256Mi"}, + }), + withSourceControllerVolumes( + corev1.Volume{Name: "existing-volume"}, + expectedCaVolume, + ), + withSourceControllerVolumeMounts( + corev1.VolumeMount{Name: "existing-volume", MountPath: "/tmp/existing"}, + expectedCaVolumeMount, + ), + ) + assert.JSONEq(t, string(expected.Raw), string(out.Raw)) + }, + }, + { + name: "Removes VolumeMounts with same name and/or same MountPath", + values: buildHelmValues(t, + withSourceControllerVolumeMounts( + corev1.VolumeMount{Name: "volume1", MountPath: "/tmp/volume1"}, + corev1.VolumeMount{Name: "volume2", MountPath: mountPath}, + corev1.VolumeMount{Name: volumeName, MountPath: "/tmp/existing"}, + corev1.VolumeMount{Name: volumeName, MountPath: mountPath}, + ), + ), + checkValue: func(t *testing.T, out *apiextensionsv1.JSON) { + require.NotNil(t, out) + + expected := buildHelmValues(t, + withSourceControllerVolumes(expectedCaVolume), + withSourceControllerVolumeMounts( + corev1.VolumeMount{Name: "volume1", MountPath: "/tmp/volume1"}, + expectedCaVolumeMount, + ), + ) + + assert.JSONEq(t, string(expected.Raw), string(out.Raw)) + }, + }, + { + name: "Removes Volumes with same name", + values: buildHelmValues(t, + withSourceControllerVolumes( + corev1.Volume{Name: volumeName}, + corev1.Volume{Name: "volume1"}, + ), + ), + checkValue: func(t *testing.T, out *apiextensionsv1.JSON) { + require.NotNil(t, out) + + expected := buildHelmValues(t, + withSourceControllerVolumes( + corev1.Volume{Name: "volume1"}, + expectedCaVolume), + withSourceControllerVolumeMounts( + expectedCaVolumeMount, + ), + ) + assert.JSONEq(t, string(expected.Raw), string(out.Raw)) + }, + }, + { + name: "Returns error for invalid root json", + values: &apiextensionsv1.JSON{Raw: []byte("not-json")}, + wantErr: "failed to unmarshal helm values", + }, + { + name: "returns error for invalid sourceController json", + values: mustMarshalJSON(t, map[string]any{ + "sourceController": "not-an-object", + }), + wantErr: "failed to unmarshal sourceController", + }, + { + name: "returns error for invalid sourceController.volumes json", + values: mustMarshalJSON(t, map[string]any{ + "sourceController": map[string]any{ + "volumes": "not-a-list", + }, + }), + wantErr: "failed to unmarshal sourceController.volumes", + }, + { + name: "returns error for invalid sourceController.volumeMounts json", + values: mustMarshalJSON(t, map[string]any{ + "sourceController": map[string]any{ + "volumeMounts": "not-a-list", + }, + }), + wantErr: "failed to unmarshal sourceController.volumeMounts", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + out, err := AddCaToHelmValues(tt.values, secretName) + if tt.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + return + } + + require.NoError(t, err) + require.NotNil(t, out) + if tt.checkValue != nil { + tt.checkValue(t, out) + } + }) + } +} + +type helmValues struct { + root map[string]any + sourceController map[string]any +} + +type helmValuesOption func(*helmValues) + +func buildHelmValues(t *testing.T, opts ...helmValuesOption) *apiextensionsv1.JSON { + t.Helper() + + builder := &helmValues{ + root: map[string]any{}, + sourceController: map[string]any{}, + } + + for _, opt := range opts { + opt(builder) + } + + if len(builder.sourceController) > 0 { + builder.root["sourceController"] = builder.sourceController + } + + return mustMarshalJSON(t, builder.root) +} + +func withRootField(key string, value any) helmValuesOption { + return func(builder *helmValues) { + builder.root[key] = value + } +} + +func withSourceControllerField(key string, value any) helmValuesOption { + return func(builder *helmValues) { + builder.sourceController[key] = value + } +} + +func withSourceControllerVolumes(volumes ...corev1.Volume) helmValuesOption { + return func(builder *helmValues) { + builder.sourceController["volumes"] = volumes + } +} + +func withSourceControllerVolumeMounts(volumeMounts ...corev1.VolumeMount) helmValuesOption { + return func(builder *helmValues) { + builder.sourceController["volumeMounts"] = volumeMounts + } +} + func mustMarshalJSON(t *testing.T, v any) *apiextensionsv1.JSON { t.Helper() raw, err := json.Marshal(v) diff --git a/pkg/flux/secret.go b/pkg/flux/secret.go index 6bc6398..a5c0568 100644 --- a/pkg/flux/secret.go +++ b/pkg/flux/secret.go @@ -45,12 +45,14 @@ type SecretCopyConfig struct { // TargetName is an optional value to adjust the name of the target secret // instead of using the source secret name. TargetName string + // TargetType is the type of the target secret. + TargetType corev1.SecretType } -// ManagePullSecrets syncs every image pull secret to the target cluster. -func ManagePullSecrets(targetCluster ManagedCluster, imagePullSecrets []corev1.LocalObjectReference, config SecretCopyConfig) { - for _, pullSecret := range imagePullSecrets { - secretName := pullSecret.Name +// ManageSecrets syncs every secret to the target cluster. +func ManageSecrets(targetCluster ManagedCluster, secrets []corev1.LocalObjectReference, config SecretCopyConfig) { + for _, secret := range secrets { + secretName := secret.Name if config.TargetName != "" { secretName = config.TargetName } @@ -67,7 +69,7 @@ func ManagePullSecrets(targetCluster ManagedCluster, imagePullSecrets []corev1.L } sourceSecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: pullSecret.Name, + Name: secret.Name, Namespace: config.SourceNamespace, }, } @@ -75,7 +77,7 @@ func ManagePullSecrets(targetCluster ManagedCluster, imagePullSecrets []corev1.L if err := config.SourceClient.Get(ctx, client.ObjectKeyFromObject(sourceSecret), sourceSecret); err != nil { return err } - mutator := openmcpresources.NewSecretMutator(secretName, config.TargetNamespace, sourceSecret.Data, corev1.SecretTypeDockerConfigJson) + mutator := openmcpresources.NewSecretMutator(secretName, config.TargetNamespace, sourceSecret.Data, config.TargetType) return mutator.Mutate(oSecret) }, StatusFunc: SimpleStatus, @@ -123,7 +125,7 @@ type secretCleaner struct { secretsToKeep []corev1.LocalObjectReference } -// NewSecretCleaner removes redundant pull secrets in the given target namespace +// NewSecretCleaner removes redundant secrets in the given target namespace // by removing any secret labeled as managed by sp-flux that is not in secretsToKeep. func NewSecretCleaner(cluster ManagedCluster, namespace string, secretsToKeep []corev1.LocalObjectReference) OrphanCleaner { return &secretCleaner{ diff --git a/pkg/flux/secret_test.go b/pkg/flux/secret_test.go index 4cba8a0..99ae6b2 100644 --- a/pkg/flux/secret_test.go +++ b/pkg/flux/secret_test.go @@ -34,8 +34,8 @@ import ( "github.com/openmcp-project/service-provider-flux/pkg/testutils" ) -func TestManagePullSecrets(t *testing.T) { - sourceSecret := &corev1.Secret{ +func TestManageSecrets(t *testing.T) { + sourcePullSecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "test-pull-secret", Namespace: "source-ns", @@ -45,13 +45,25 @@ func TestManagePullSecrets(t *testing.T) { }, Type: corev1.SecretTypeDockerConfigJson, } - fakeCluster := testutils.CreateFakeCluster(t, "platform", sourceSecret) + sourceCaSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ca-secret", + Namespace: "source-ns", + }, + Data: map[string][]byte{ + "ca.crt": []byte("data"), + }, + Type: corev1.SecretTypeOpaque, + } + fakeCluster := testutils.CreateFakeCluster(t, "platform", sourcePullSecret, sourceCaSecret) tests := []struct { - name string - targetCluster ManagedCluster - imagePullSecrets []corev1.LocalObjectReference - config SecretCopyConfig + name string + targetCluster ManagedCluster + imagePullSecrets []corev1.LocalObjectReference + caSecrets []corev1.LocalObjectReference + configPullSecrets SecretCopyConfig + configCaSecrets SecretCopyConfig }{ { name: "syncs secret with correct type", @@ -59,10 +71,20 @@ func TestManagePullSecrets(t *testing.T) { imagePullSecrets: []corev1.LocalObjectReference{ {Name: "test-pull-secret"}, }, - config: SecretCopyConfig{ + caSecrets: []corev1.LocalObjectReference{ + {Name: "test-ca-secret"}, + }, + configPullSecrets: SecretCopyConfig{ SourceClient: fakeCluster.Client(), SourceNamespace: "source-ns", TargetNamespace: "target-ns", + TargetType: corev1.SecretTypeDockerConfigJson, + }, + configCaSecrets: SecretCopyConfig{ + SourceClient: fakeCluster.Client(), + SourceNamespace: "source-ns", + TargetNamespace: "target-ns", + TargetType: corev1.SecretTypeOpaque, }, }, { @@ -71,18 +93,30 @@ func TestManagePullSecrets(t *testing.T) { imagePullSecrets: []corev1.LocalObjectReference{ {Name: "test-pull-secret"}, }, - config: SecretCopyConfig{ + caSecrets: []corev1.LocalObjectReference{ + {Name: "test-ca-secret"}, + }, + configPullSecrets: SecretCopyConfig{ SourceClient: fakeCluster.Client(), SourceNamespace: "source-ns", TargetNamespace: "target-ns", TargetName: fmt.Sprintf("%s%s", secretNamePrefix, "test-pull-secret"), + TargetType: corev1.SecretTypeDockerConfigJson, + }, + configCaSecrets: SecretCopyConfig{ + SourceClient: fakeCluster.Client(), + SourceNamespace: "source-ns", + TargetNamespace: "target-ns", + TargetName: fmt.Sprintf("%s%s", secretNamePrefix, "test-ca-secret"), + TargetType: corev1.SecretTypeOpaque, }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - ManagePullSecrets(tt.targetCluster, tt.imagePullSecrets, tt.config) + ManageSecrets(tt.targetCluster, tt.imagePullSecrets, tt.configPullSecrets) + ManageSecrets(tt.targetCluster, tt.caSecrets, tt.configCaSecrets) // Apply managed objects mgr := NewManager() @@ -97,18 +131,35 @@ func TestManagePullSecrets(t *testing.T) { for _, pullSecret := range tt.imagePullSecrets { targetSecret := &corev1.Secret{} targetSecretName := pullSecret.Name - if tt.config.TargetName != "" { - targetSecretName = tt.config.TargetName + if tt.configPullSecrets.TargetName != "" { + targetSecretName = tt.configPullSecrets.TargetName } err := fakeCluster.Client().Get(context.Background(), client.ObjectKey{ Name: targetSecretName, - Namespace: tt.config.TargetNamespace, + Namespace: tt.configPullSecrets.TargetNamespace, }, targetSecret) require.NoError(t, err) - assert.Equal(t, sourceSecret.Data, targetSecret.Data) + assert.Equal(t, sourcePullSecret.Data, targetSecret.Data) assert.Equal(t, corev1.SecretTypeDockerConfigJson, targetSecret.Type, "target secret should have the correct type") } + + // Verify secret was synced with correct type + for _, caSecret := range tt.caSecrets { + targetSecret := &corev1.Secret{} + targetSecretName := caSecret.Name + if tt.configCaSecrets.TargetName != "" { + targetSecretName = tt.configCaSecrets.TargetName + } + err := fakeCluster.Client().Get(context.Background(), client.ObjectKey{ + Name: targetSecretName, + Namespace: tt.configCaSecrets.TargetNamespace, + }, targetSecret) + require.NoError(t, err) + + assert.Equal(t, sourceCaSecret.Data, targetSecret.Data) + assert.Equal(t, corev1.SecretTypeOpaque, targetSecret.Type, "target secret should have the correct type") + } }) } } From 4f4d7d3d38cbbbd24a1071fb805cf18a3e6896d6 Mon Sep 17 00:00:00 2001 From: "Nico Andres (SAP)" Date: Thu, 28 May 2026 09:40:41 +0200 Subject: [PATCH 02/20] generate On-behalf-of: @SAP nico.andres@sap.com Signed-off-by: Nico Andres (SAP) --- ...x.services.open-control-plane.io_providerconfigs.yaml | 9 +++++++-- api/v1alpha1/zz_generated.deepcopy.go | 2 +- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/api/crds/manifests/flux.services.open-control-plane.io_providerconfigs.yaml b/api/crds/manifests/flux.services.open-control-plane.io_providerconfigs.yaml index 5083469..77df47d 100644 --- a/api/crds/manifests/flux.services.open-control-plane.io_providerconfigs.yaml +++ b/api/crds/manifests/flux.services.open-control-plane.io_providerconfigs.yaml @@ -43,12 +43,17 @@ spec: properties: certSecretRef: description: |- - certSecretRef is a reference to a Secret in the service provider's namespace + CertSecretRef is a reference to a Secret in the service provider's namespace containing a single PEM-encoded CA certificate or a bundle used when pulling the Helm chart from a private OCI registry. The secret will be copied to the tenant namespace on the platform cluster and referenced by the OCIRepository. - The secret must be of type kubernetes.io/opaque. + The secret is also copied to the flux-system namespace on the MCP cluster and is + mounted into the source-controller. + The secret must be of type kubernetes.io/opaque and store the PEM-encoded + certificate/bundle under the ca.crt key. + It can also contain tls.crt and tls.key keys for mTLS. + (https://fluxcd.io/flux/components/source/ocirepositories/#mutual-tls-authentication) type: string pollInterval: default: 1m diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 70e148d..cadbb0b 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -6,7 +6,7 @@ package v1alpha1 import ( apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" ) From 192625a1798459ea7f6c9d75665242a782ccb3d2 Mon Sep 17 00:00:00 2001 From: "Nico Andres (SAP)" Date: Thu, 28 May 2026 10:38:51 +0200 Subject: [PATCH 03/20] Fix lint On-behalf-of: @SAP nico.andres@sap.com Signed-off-by: Nico Andres (SAP) --- internal/controller/flux_controller.go | 5 ++++- pkg/flux/helm.go | 3 +++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/internal/controller/flux_controller.go b/internal/controller/flux_controller.go index 96dc04d..55c9613 100644 --- a/internal/controller/flux_controller.go +++ b/internal/controller/flux_controller.go @@ -234,7 +234,10 @@ func (r *FluxReconciler) createObjectManager(obj *apiv1alpha1.Flux, pc *apiv1alp Name: prefixedCertSecret, }, }) - cpSecretsToKeep := append(helmValues.ImagePullSecrets, corev1.LocalObjectReference{Name: pc.Spec.CertSecretRef}) + + cpSecretsToKeep := make([]corev1.LocalObjectReference, 0, 1+len(helmValues.ImagePullSecrets)) + cpSecretsToKeep = append(cpSecretsToKeep, corev1.LocalObjectReference{Name: pc.Spec.CertSecretRef}) + cpSecretsToKeep = append(cpSecretsToKeep, helmValues.ImagePullSecrets...) controlPlaneCleaner := flux.NewSecretCleaner(mcpCluster, fluxNamespace, cpSecretsToKeep) mgr.AddCleaner(platformCleaner) diff --git a/pkg/flux/helm.go b/pkg/flux/helm.go index 87824aa..764b570 100644 --- a/pkg/flux/helm.go +++ b/pkg/flux/helm.go @@ -46,6 +46,9 @@ func ExtractHelmValues(values *apiextensionsv1.JSON) (*HelmValues, error) { return vals, nil } +// AddCaToHelmValues removes conflicting volume and volume mounts (matching by name and/or mountPath) and +// adds volume and volume mount on the sourceController helm values section. +// nolint:gocyclo func AddCaToHelmValues(values *apiextensionsv1.JSON, secretName string) (*apiextensionsv1.JSON, error) { var root = map[string]json.RawMessage{} var sourceController = map[string]json.RawMessage{} From 0a0e428ee86f38b878c1231a49032973d7c1ba74 Mon Sep 17 00:00:00 2001 From: "Nico Andres (SAP)" Date: Thu, 28 May 2026 14:18:28 +0200 Subject: [PATCH 04/20] Add e2e On-behalf-of: @SAP nico.andres@sap.com Signed-off-by: Nico Andres (SAP) --- test/e2e/flux_test.go | 27 +++++++++++++++++++++++---- test/e2e/platform/casecret.yaml | 8 ++++++++ test/e2e/platform/providerconfig.yaml | 1 + 3 files changed, 32 insertions(+), 4 deletions(-) create mode 100644 test/e2e/platform/casecret.yaml diff --git a/test/e2e/flux_test.go b/test/e2e/flux_test.go index 16bd5ab..b2e4518 100644 --- a/test/e2e/flux_test.go +++ b/test/e2e/flux_test.go @@ -89,6 +89,15 @@ func TestServiceProvider(t *testing.T) { if err := wait.For(conditions.New(c.Client().Resources()).ResourcesFound(pullSecrets), wait.WithTimeout(2*time.Minute)); err != nil { t.Errorf("pull secret not found: %v", err) } + caSecret := &corev1.Secret{} + caSecret.SetName("sp-flux-custom-ca-cert") + caSecret.SetNamespace(tenantNamespace) + caSecrets := &corev1.SecretList{ + Items: []corev1.Secret{*caSecret}, + } + if err := wait.For(conditions.New(c.Client().Resources()).ResourcesFound(caSecrets), wait.WithTimeout(2*time.Minute)); err != nil { + t.Errorf("ca secret not found: %v", err) + } return ctx }). Assess("ManagedControlPlane resources have been created", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { @@ -106,6 +115,15 @@ func TestServiceProvider(t *testing.T) { if err := wait.For(conditions.New(mcp.Client().Resources()).ResourcesFound(list), wait.WithTimeout(2*time.Minute)); err != nil { t.Errorf("image pull secret not found on control plane: %v", err) } + caSecret := &corev1.Secret{} + caSecret.SetName("custom-ca-cert") + caSecret.SetNamespace("flux-system") + caSecrets := &corev1.SecretList{ + Items: []corev1.Secret{*caSecret}, + } + if err := wait.For(conditions.New(mcp.Client().Resources()).ResourcesFound(caSecrets), wait.WithTimeout(2*time.Minute)); err != nil { + t.Errorf("ca secret not found on control plane: %v", err) + } return ctx }). Assess("domain objects can be created", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { @@ -238,6 +256,7 @@ func TestServiceProvider(t *testing.T) { } providerConfig.Spec.Versions[0].ChartPullSecret = "" providerConfig.Spec.Versions[0].Values = nil + providerConfig.Spec.CertSecretRef = "" if err := c.Client().Resources().Update(ctx, providerConfig); err != nil { t.Errorf("failed to update provider config: %v", err) } @@ -257,7 +276,7 @@ func TestServiceProvider(t *testing.T) { return ctx }). - Assess("platform chart pull secret deleted", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { + Assess("platform secrets deleted", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { tenantNamespace, err := libutils.StableMCPNamespace(mcpName, "default") if err != nil { t.Errorf("failed to get tenant namespace: %v", err) @@ -268,11 +287,11 @@ func TestServiceProvider(t *testing.T) { ResourceListN(spFluxSecrets, 0, klientresources.WithLabelSelector( labels.FormatLabels(map[string]string{flux.LabelManagedBy: "service-provider-flux"}))), wait.WithTimeout(2*time.Minute)); err != nil { - t.Errorf("orphaned chart pull secret is not deleted: %v", err) + t.Errorf("orphaned platform secret is not deleted: %v", err) } return ctx }). - Assess("control plane image pull secrets deleted", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { + Assess("control plane secrets deleted", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { mcp, err := clusterutils.MCPConfig(ctx, c, mcpName) if err != nil { t.Error(err) @@ -283,7 +302,7 @@ func TestServiceProvider(t *testing.T) { ResourceListN(spFluxSecrets, 0, klientresources.WithLabelSelector( labels.FormatLabels(map[string]string{flux.LabelManagedBy: "service-provider-flux"}))), wait.WithTimeout(2*time.Minute)); err != nil { - t.Errorf("orphaned image pull secret is not deleted: %v", err) + t.Errorf("orphaned secret is not deleted: %v", err) } return ctx }). diff --git a/test/e2e/platform/casecret.yaml b/test/e2e/platform/casecret.yaml new file mode 100644 index 0000000..24fe66f --- /dev/null +++ b/test/e2e/platform/casecret.yaml @@ -0,0 +1,8 @@ +apiVersion: v1 +data: + # dummy value for testing + ca.crt: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUZIVENDQXdXZ0F3SUJBZ0lVU29EQzZLUktnVlpTTVlubGQ4NmYrT2tBeldrd0RRWUpLb1pJaHZjTkFRRUwKQlFBd0hqRWNNQm9HQTFVRUF3d1RiRzlqWVd3dGNtVm5hWE4wY25rdGNtOXZkREFlRncweU5qQTFNakV4TURFeQpNekZhRncwek5qQTFNVGd4TURFeU16RmFNQjR4SERBYUJnTlZCQU1NRTJ4dlkyRnNMWEpsWjJsemRISjVMWEp2CmIzUXdnZ0lpTUEwR0NTcUdTSWIzRFFFQkFRVUFBNElDRHdBd2dnSUtBb0lDQVFETllrVThEc0NVVlNyU3NScFUKSGxaUUgxUU5hT0pHNDBsSnFmZkFHbHpLWis3d0ZSMzA4dkI0VGUrNUtqN0pzN3dQRWtaUnhPOVpmc05YOTdRUApQY0F2R0RtTUFIL2xDK0RsVndHK0RuQVZDYnVoWEdEcHhJd09ycC9Qckh1OEtFbHY4RDdFbXpFd0Y0YXdhNGFnCmRCeWRDc21yZm9oS3JaZG9UQnlhaTR3bWExaCsxc2JtQTByMUROVGxnLzlQSUFPL0NsSGpXZHgrMjhjc3VON2MKQ3NzSmtucHVBVXBLRkc0TjJldHlhK2xVUHFsa3dnd05NTjFkMGRVbCthUm9TeGRxS2xJZEtrVVdGNmNNMkRhZwpTLzE1ZTNOVWkrUFZVMUtPdDZ6c3kvVjVVRXdTYkliVmcxRE5mRjBFMHNCbFFucWxvTERBNm9kaVJnd0dLNHpFCnB3N2F3N2FaU3c5WEt3RjJWZ2hYL1ZVV0dvUkswU25OU1ZnNUdFQUtFb3ZLN2s3UkdXanpRRlJkYWt4Z1U4NkUKbWdxdXh1ZSs1OVROWnFqVnJCQjdnN2trTGQzNFRidHdQR1VqSU1vRHVtcGxDM1c4M0U0SDEvbnFPV2tJYWp5RQpPRmI0ZmtXM1NlcXlsTytyRmxQKzJVWDRFdHZHeE82a1lDWkdTcmR5TFVKZ21ZbjBDYlI1V3BlOXdJWExqWExmCm9QTGJmQWYzaDlwZkwrdlhnSm44WE0zYnUwTEVjY3RMZk03NFgwL0h4S3BDOTRtR2hmdFRnd3VEM3ZHUmluUDYKaE5NMXQ2Y0NqSzVWYXd6NVFMQzJVb2NQWkt0Zm1hSUhhU1JIOVdEaitxQkNjTXVQYThFMUlEZ1RlazMwaEZIVAp5SXFtNEd5VVBkZmtyNXJjMEc3WGNGUGRnUUlEQVFBQm8xTXdVVEFkQmdOVkhRNEVGZ1FVZ0Q3SzZLa1JMVDYyCnhWMG50NTUrTlJrdU9mMHdId1lEVlIwakJCZ3dGb0FVZ0Q3SzZLa1JMVDYyeFYwbnQ1NStOUmt1T2Ywd0R3WUQKVlIwVEFRSC9CQVV3QXdFQi96QU5CZ2txaGtpRzl3MEJBUXNGQUFPQ0FnRUFORjZwczJjT3hqdHFiMzF3S0drdgpqTG9ucm5VT0JvdTgxdkFMK0FycnZDMWtvdEF2Nnl2YTd1OWlTS2Q2VGN3Q2hFdUJtbE1ocFFDaUR5UDRrUU1hCkRsRFZqdStZbk55NHhGSHV4V1ovczhlanRBT21lTmJ3bzgyVStkd2NVZ2w4UVR6QU1rV3k1cmxDVXJScE9ZYmIKUE1nSFlTckd0YVZEelBkc2dkN0ZlUStGcEkwMWd6eXBFODd3TjdEZ0RoMnRyVGV6MFMwQWhKbk40K3NtbFltVwpRaTNkQXAzRFBUZmFMczZlSWEvOWJBRXhrR0lYS3JRQytaRU5IZG9vRDBEY2VFamNpeTJxVllYTXNicVhubEdTCmc5UmdnSFNpUHNtTWFMZVVhQ0doOHgvSGVVakVoU3ROVkxjWkRDOUlHc21UaHlBcjUrTjdYaG1ZbjRIVWxCbDAKZWF5ZzA2VTlWc3NRaU5HNDRvTkRrRXc1TVFIRDNwQUVnTWJrSWJYVEcvbVBhNDhycWRBcnBXSDRvR002OThLZApQTTRsWHVkTHF4SWN1N2JrclpnTGpVbFMzSnVXMVhqVTRNRlVSK2RhSnoveGVCSmlBekNFaHBaalVyOXdPcXFUClRYcGtBMFBvWnd0cFA5MER6emF6SkJiZWs4N3BSaGdsS3VZNmJuZ1hCVjlZZklNUERZSVJNMnhJV01QRzU0ekwKVUNjMEpaOVV6WExyc1QwcFBpWURsaVJIZEtUTGtNRk5LQTZEU3lIVFlVelFybkhudHZJZG5mOXo2UXpxRVBoWQpxWTJ6eXdQaEdlOVZPSXowNWxHdkU5WUxncEJDY1VxZHlJMGZpZzF6bFdrYXBGdFB5MHBBY2NJNXZyUmFXSTAyCm93cGo0eDFCVkJ6bWp0eFczSzdUa0U4PQotLS0tLUVORCBDRVJUSUZJQ0FURS0tLS0tCg== +kind: Secret +metadata: + name: custom-ca-cert + namespace: openmcp-system diff --git a/test/e2e/platform/providerconfig.yaml b/test/e2e/platform/providerconfig.yaml index 384490c..8c82abe 100644 --- a/test/e2e/platform/providerconfig.yaml +++ b/test/e2e/platform/providerconfig.yaml @@ -4,6 +4,7 @@ metadata: name: flux spec: pollInterval: "10s" + certSecretRef: "custom-ca-cert" versions: - version: "2.8.3" chartVersion: "2.18.2" From 7230b2ecfa6e78e8bf4a6b4adca58b4e7344fe3b Mon Sep 17 00:00:00 2001 From: "Nico Andres (SAP)" Date: Thu, 28 May 2026 14:34:02 +0200 Subject: [PATCH 05/20] Change e2e test name On-behalf-of: @SAP nico.andres@sap.com Signed-off-by: Nico Andres (SAP) --- test/e2e/flux_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/flux_test.go b/test/e2e/flux_test.go index b2e4518..22d133c 100644 --- a/test/e2e/flux_test.go +++ b/test/e2e/flux_test.go @@ -243,7 +243,7 @@ func TestServiceProvider(t *testing.T) { } return ctx }). - Assess("provider config update drops pull secrets", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { + Assess("provider config update secrets", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { if err := v1alpha1.AddToScheme(c.Client().Resources().GetScheme()); err != nil { t.Errorf("failed to add api types to client scheme: %s", err) return ctx From 804915616c3e374630a0cfb0a344ac7dfb9a86c9 Mon Sep 17 00:00:00 2001 From: "Nico Andres (SAP)" Date: Tue, 2 Jun 2026 16:12:21 +0200 Subject: [PATCH 06/20] Custom CA for all flux controllers + updated docs On-behalf-of: @SAP nico.andres@sap.com Signed-off-by: Nico Andres (SAP) --- README.md | 5 +- ...open-control-plane.io_providerconfigs.yaml | 2 +- api/v1alpha1/providerconfig_types.go | 2 +- docs/configuration/image-localization.md | 20 +- pkg/flux/helm.go | 150 ++++++++++----- pkg/flux/helm_test.go | 174 ++++++++++++------ 6 files changed, 240 insertions(+), 113 deletions(-) diff --git a/README.md b/README.md index 71d614e..b66e9c5 100644 --- a/README.md +++ b/README.md @@ -146,7 +146,10 @@ metadata: spec: # Optional: Reconciliation interval pollInterval: "5m" - # Optional: Secret for custom CA certificate/bundle (will be copied to ManagedControlPlane) + # Optional: Secret for custom CA certificate/bundle (will be copied to OpenControlPlane) + # CA certificate/bundle must be stored in key ca.crt + # Optionally: keys tls.key and tls.crt can be set for mTLS + # (https://fluxcd.io/flux/components/source/ocirepositories/#mutual-tls-authentication) certSecretRef: "custom-ca-cert" # The Flux versions that can be installed versions: diff --git a/api/crds/manifests/flux.services.open-control-plane.io_providerconfigs.yaml b/api/crds/manifests/flux.services.open-control-plane.io_providerconfigs.yaml index 77df47d..2894a7f 100644 --- a/api/crds/manifests/flux.services.open-control-plane.io_providerconfigs.yaml +++ b/api/crds/manifests/flux.services.open-control-plane.io_providerconfigs.yaml @@ -49,7 +49,7 @@ spec: The secret will be copied to the tenant namespace on the platform cluster and referenced by the OCIRepository. The secret is also copied to the flux-system namespace on the MCP cluster and is - mounted into the source-controller. + mounted into all Flux controllers deployed by the chart. The secret must be of type kubernetes.io/opaque and store the PEM-encoded certificate/bundle under the ca.crt key. It can also contain tls.crt and tls.key keys for mTLS. diff --git a/api/v1alpha1/providerconfig_types.go b/api/v1alpha1/providerconfig_types.go index 35a26c2..7efe166 100644 --- a/api/v1alpha1/providerconfig_types.go +++ b/api/v1alpha1/providerconfig_types.go @@ -42,7 +42,7 @@ type ProviderConfigSpec struct { // The secret will be copied to the tenant namespace on the platform cluster // and referenced by the OCIRepository. // The secret is also copied to the flux-system namespace on the MCP cluster and is - // mounted into the source-controller. + // mounted into all Flux controllers deployed by the chart. // The secret must be of type kubernetes.io/opaque and store the PEM-encoded // certificate/bundle under the ca.crt key. // It can also contain tls.crt and tls.key keys for mTLS. diff --git a/docs/configuration/image-localization.md b/docs/configuration/image-localization.md index 9d60541..72315b9 100644 --- a/docs/configuration/image-localization.md +++ b/docs/configuration/image-localization.md @@ -72,10 +72,10 @@ metadata: spec: versions: # Secret with single PEM-encoded custom CA root certificate or bundle stored - # on the ca.crt key. + # in the ca.crt key. # Must exist in the service provider's namespace on the platform cluster # Will be copied to the tenant namespace on the platform cluster and to the - # flux-system namespace on the ManagedControlPlane. + # flux-system namespace on the OpenControlPlane. certSecretRef: "custom-ca-cert" - version: "2.8.3" @@ -126,10 +126,12 @@ kubectl create secret docker-registry image-registry-credentials \ --docker-username= \ --docker-password= -# Custom CA certificate secret (for OCI registry connection) +# Custom CA certificate secret with optional mTLS key and cert (for OCI registry connection) kubectl create secret generic custom-ca-cert \ --namespace \ - --ca.crt= + --ca.crt= \ + --tls.key= \ + --tls.crt= ``` ## How It Works @@ -145,12 +147,12 @@ kubectl create secret generic custom-ca-cert \ 1. The secret specified in `certSecretRef` is copied from the service provider's namespace to the tenant namespace on the platform cluster 2. The `OCIRepository` resource references this secret via `spec.certSecretRef` 3. The Flux Source Controller uses this secret to establish a trusted connection with the OCI registry when pulling the Helm chart -4. The secret is also copied to the MCP cluster flux-system namespace and mounted into the Flux Source Controller -by adding a volume and volumeMount in to the helm values. Flux in the target MCP hence is also able to verify -self-signed certificates used by the OCI repository. +4. The secret is also copied to the MCP cluster flux-system namespace and mounted into the all Flux controllers +by adding a volume, volumeMount and envVar to the helm values. Flux in the target OpenControlPlane hence is also able to verify +certificates signed with the custom CA certificate. > [!CAUTION] -> The custom CA certificate is not propagated to the MCP cluster nodes. If you want to pull images from the same OCI registry you must add the custom CA certificate to the MCP cluster nodes yourself. +> The custom CA certificate is not propagated to the OpenControlPlane cluster nodes. If you want to pull images from the same OCI registry you must add the custom CA certificate to thecluster nodes yourself. > [!NOTE] > The secret referenced in certSecretRef can also contain two additional keys tls.key and tls.crt which will be @@ -174,7 +176,7 @@ metadata: spec: chartUrl: "oci://harbor.corp.internal/charts/flux2" chartPullSecret: "harbor-credentials" - # Secret containing custom ca cert - will be copied to ManagedControlPlane + # Secret containing custom ca cert under ca.crt - will be copied to ManagedControlPlane certSecretRef: "harbor-corp-internal-ca-cert" values: # Image pull secrets - will be copied to ManagedControlPlane diff --git a/pkg/flux/helm.go b/pkg/flux/helm.go index 764b570..e3d0081 100644 --- a/pkg/flux/helm.go +++ b/pkg/flux/helm.go @@ -16,11 +16,33 @@ package flux import ( "encoding/json" "fmt" + "strings" corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" ) +const ( + customCaVolumeName = "custom-ca-bundle" + customCaPath = "/etc/open-control-plane/custom-ca" +) + +// from x509 go standard lib (https://github.com/golang/go/blob/015343854b5d9e2829481df30dbcae2ca6682d25/src/crypto/x509/root_linux.go) +var certDirectories = []string{ + "/etc/ssl/certs", + "/etc/pki/tls/certs", +} + +var fluxControllers = []string{ + "helmController", + "imageAutomationController", + "imageReflectionController", + "kustomizeController", + "notificationController", + "sourceController", + "sourceWatcher", +} + // HelmValues defines the Helm values that are explicitly processed during reconciliation. // The Flux Helm chart uses imagePullSecrets at the top level, not under global. type HelmValues struct { @@ -46,24 +68,20 @@ func ExtractHelmValues(values *apiextensionsv1.JSON) (*HelmValues, error) { return vals, nil } -// AddCaToHelmValues removes conflicting volume and volume mounts (matching by name and/or mountPath) and -// adds volume and volume mount on the sourceController helm values section. -// nolint:gocyclo +// AddCaToHelmValues removes conflicting volumes, volumeMounts and envVars (matching by name and/or mountPath) and +// adds a volume, volumeMount and envVar on all Flux controller helm values sections to import the custom CA certificate. func AddCaToHelmValues(values *apiextensionsv1.JSON, secretName string) (*apiextensionsv1.JSON, error) { var root = map[string]json.RawMessage{} - var sourceController = map[string]json.RawMessage{} - var volumes []corev1.Volume - var volumeMounts []corev1.VolumeMount caVolume := corev1.Volume{ - Name: "sp-flux-custom-ca", + Name: customCaVolumeName, VolumeSource: corev1.VolumeSource{ Secret: &corev1.SecretVolumeSource{ SecretName: secretName, Items: []corev1.KeyToPath{ { Key: "ca.crt", - Path: "sp-flux-custom-ca.crt", + Path: "ca.crt", }, }, }, @@ -71,82 +89,130 @@ func AddCaToHelmValues(values *apiextensionsv1.JSON, secretName string) (*apiext } caVolumeMount := corev1.VolumeMount{ - Name: "sp-flux-custom-ca", + Name: customCaVolumeName, ReadOnly: true, - MountPath: "/etc/ssl/certs/sp-flux-custom-ca.crt", - SubPath: "sp-flux-custom-ca.crt", + MountPath: customCaPath, + } + + caEnvVar := corev1.EnvVar{ + Name: "SSL_CERT_DIR", + Value: strings.Join(append(certDirectories, customCaPath), ":"), } if values != nil && len(values.Raw) > 0 { if err := json.Unmarshal(values.Raw, &root); err != nil { return nil, fmt.Errorf("failed to unmarshal helm values: %w", err) } + } - if err := unmarshalIfPresent(root, "sourceController", &sourceController); err != nil { - return nil, fmt.Errorf("failed to unmarshal sourceController: %w", err) + for _, controller := range fluxControllers { + if err := addCaToController(root, controller, caVolume, caVolumeMount, caEnvVar); err != nil { + return nil, err } + } - if err := unmarshalIfPresent(sourceController, "volumes", &volumes); err != nil { - return nil, fmt.Errorf("failed to unmarshal sourceController.volumes: %w", err) - } + out, err := json.Marshal(root) + if err != nil { + return nil, fmt.Errorf("failed to marshal helm values: %w", err) + } - if err := unmarshalIfPresent(sourceController, "volumeMounts", &volumeMounts); err != nil { - return nil, fmt.Errorf("failed to unmarshal sourceController.volumeMounts: %w", err) - } + return &apiextensionsv1.JSON{Raw: out}, nil +} + +func addCaToController( + root map[string]json.RawMessage, + controller string, + caVolume corev1.Volume, + caVolumeMount corev1.VolumeMount, + caEnvVar corev1.EnvVar, +) error { + var controllerValues map[string]json.RawMessage + var volumes []corev1.Volume + var volumeMounts []corev1.VolumeMount + var envVars []corev1.EnvVar + + if err := unmarshalIfPresent(root, controller, &controllerValues); err != nil { + return fmt.Errorf("failed to unmarshal %s: %w", controller, err) + } + if controllerValues == nil { + controllerValues = map[string]json.RawMessage{} } - volumes = removeConflictingVolumes(volumes, caVolume) - volumeMounts = removeConflictingVolumeMounts(volumeMounts, caVolumeMount) + if err := unmarshalIfPresent(controllerValues, "volumes", &volumes); err != nil { + return fmt.Errorf("failed to unmarshal %s.volumes: %w", controller, err) + } + + if err := unmarshalIfPresent(controllerValues, "volumeMounts", &volumeMounts); err != nil { + return fmt.Errorf("failed to unmarshal %s.volumeMounts: %w", controller, err) + } + + if err := unmarshalIfPresent(controllerValues, "extraEnv", &envVars); err != nil { + return fmt.Errorf("failed to unmarshal %s.extraEnv: %w", controller, err) + } - volumes = append(volumes, caVolume) - volumeMounts = append(volumeMounts, caVolumeMount) + volumes = removeConflictingVolumesAndAppend(volumes, caVolume) + volumeMounts = removeConflictingVolumeMountsAndAppend(volumeMounts, caVolumeMount) + envVars = removeConflictingEnvVarsAndAppend(envVars, caEnvVar) volumesRaw, err := json.Marshal(volumes) if err != nil { - return nil, fmt.Errorf("failed to marshal sourceController.volumes: %w", err) + return fmt.Errorf("failed to marshal %s.volumes: %w", controller, err) } volumeMountsRaw, err := json.Marshal(volumeMounts) if err != nil { - return nil, fmt.Errorf("failed to marshal sourceController.volumeMounts: %w", err) + return fmt.Errorf("failed to marshal %s.volumeMounts: %w", controller, err) } - sourceController["volumes"] = volumesRaw - sourceController["volumeMounts"] = volumeMountsRaw - - sourceControllerRaw, err := json.Marshal(sourceController) + envVarsRaw, err := json.Marshal(envVars) if err != nil { - return nil, fmt.Errorf("failed to marshal sourceController: %w", err) + return fmt.Errorf("failed to marshal %s.extraEnv: %w", controller, err) } - root["sourceController"] = sourceControllerRaw + controllerValues["volumes"] = volumesRaw + controllerValues["volumeMounts"] = volumeMountsRaw + controllerValues["extraEnv"] = envVarsRaw - out, err := json.Marshal(root) + controllerRaw, err := json.Marshal(controllerValues) if err != nil { - return nil, fmt.Errorf("failed to marshal helm values: %w", err) + return fmt.Errorf("failed to marshal %s: %w", controller, err) } - return &apiextensionsv1.JSON{Raw: out}, nil + root[controller] = controllerRaw + return nil } -func removeConflictingVolumes(volumes []corev1.Volume, caVolume corev1.Volume) []corev1.Volume { - r := []corev1.Volume{} +func removeConflictingVolumesAndAppend(volumes []corev1.Volume, caVolume corev1.Volume) []corev1.Volume { + updated := []corev1.Volume{} for _, volume := range volumes { if volume.Name != caVolume.Name { - r = append(r, volume) + updated = append(updated, volume) } } - return r + updated = append(updated, caVolume) + return updated } -func removeConflictingVolumeMounts(volumeMounts []corev1.VolumeMount, caVolumeMount corev1.VolumeMount) []corev1.VolumeMount { - r := []corev1.VolumeMount{} +func removeConflictingVolumeMountsAndAppend(volumeMounts []corev1.VolumeMount, caVolumeMount corev1.VolumeMount) []corev1.VolumeMount { + updated := []corev1.VolumeMount{} for _, volumeMount := range volumeMounts { if volumeMount.MountPath != caVolumeMount.MountPath && volumeMount.Name != caVolumeMount.Name { - r = append(r, volumeMount) + updated = append(updated, volumeMount) + } + } + updated = append(updated, caVolumeMount) + return updated +} + +func removeConflictingEnvVarsAndAppend(envVars []corev1.EnvVar, caEnvVar corev1.EnvVar) []corev1.EnvVar { + updated := []corev1.EnvVar{} + for _, envVar := range envVars { + if envVar.Name != caEnvVar.Name { + updated = append(updated, envVar) } } - return r + updated = append(updated, caEnvVar) + return updated } func unmarshalIfPresent(obj map[string]json.RawMessage, key string, out any) error { diff --git a/pkg/flux/helm_test.go b/pkg/flux/helm_test.go index e8e018d..00f8241 100644 --- a/pkg/flux/helm_test.go +++ b/pkg/flux/helm_test.go @@ -15,6 +15,7 @@ package flux import ( "encoding/json" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -132,32 +133,26 @@ func TestExtractHelmValues(t *testing.T) { } func TestAddCaToHelmValues(t *testing.T) { - const ( - volumeName = "sp-flux-custom-ca" - secretName = "custom-ca-secret" - secretKey = "ca.crt" - secretPath = "sp-flux-custom-ca.crt" - mountPath = "/etc/ssl/certs/sp-flux-custom-ca.crt" - subPath = "sp-flux-custom-ca.crt" - ) - expectedCaVolume := corev1.Volume{ - Name: volumeName, + Name: customCaVolumeName, VolumeSource: corev1.VolumeSource{ Secret: &corev1.SecretVolumeSource{ - SecretName: secretName, + SecretName: "custom-ca-secret", Items: []corev1.KeyToPath{{ - Key: secretKey, - Path: secretPath, + Key: "ca.crt", + Path: "ca.crt", }}, }, }, } expectedCaVolumeMount := corev1.VolumeMount{ - Name: volumeName, + Name: customCaVolumeName, ReadOnly: true, - MountPath: mountPath, - SubPath: subPath, + MountPath: customCaPath, + } + expectedCaEnvVar := corev1.EnvVar{ + Name: "SSL_CERT_DIR", + Value: strings.Join(append(certDirectories, customCaPath), ":"), } tests := []struct { @@ -167,14 +162,15 @@ func TestAddCaToHelmValues(t *testing.T) { checkValue func(t *testing.T, out *apiextensionsv1.JSON) }{ { - name: "Adds SourceController Volumes and VolumeMounts when no helm values are set", + name: "Adds controller volumes, volumeMounts and extraEnv when no helm values are set", values: nil, checkValue: func(t *testing.T, out *apiextensionsv1.JSON) { require.NotNil(t, out) expected := buildHelmValues(t, - withSourceControllerVolumes(expectedCaVolume), - withSourceControllerVolumeMounts(expectedCaVolumeMount), + withAllControllerVolumes(expectedCaVolume), + withAllControllerVolumeMounts(expectedCaVolumeMount), + withAllControllerExtraEnv(expectedCaEnvVar), ) assert.JSONEq(t, string(expected.Raw), string(out.Raw)) @@ -184,14 +180,14 @@ func TestAddCaToHelmValues(t *testing.T) { name: "Preserves existing helm values and adds CA entries", values: buildHelmValues(t, withRootField("namespace", "other-namespace"), - withSourceControllerField( + withAllControllerField( "resources", map[string]any{ "limits": map[string]any{ "memory": "256Mi", }, }), - withSourceControllerVolumes(corev1.Volume{Name: "existing-volume"}), - withSourceControllerVolumeMounts( + withAllControllerVolumes(corev1.Volume{Name: "existing-volume"}), + withAllControllerVolumeMounts( corev1.VolumeMount{Name: "existing-volume", MountPath: "/tmp/existing"}), ), checkValue: func(t *testing.T, out *apiextensionsv1.JSON) { @@ -199,14 +195,17 @@ func TestAddCaToHelmValues(t *testing.T) { expected := buildHelmValues(t, withRootField("namespace", "other-namespace"), - withSourceControllerField("resources", map[string]any{ + withAllControllerField("resources", map[string]any{ "limits": map[string]any{"memory": "256Mi"}, }), - withSourceControllerVolumes( + withAllControllerVolumes(expectedCaVolume), + withAllControllerVolumeMounts(expectedCaVolumeMount), + withAllControllerExtraEnv(expectedCaEnvVar), + withAllControllerVolumes( corev1.Volume{Name: "existing-volume"}, expectedCaVolume, ), - withSourceControllerVolumeMounts( + withAllControllerVolumeMounts( corev1.VolumeMount{Name: "existing-volume", MountPath: "/tmp/existing"}, expectedCaVolumeMount, ), @@ -217,19 +216,20 @@ func TestAddCaToHelmValues(t *testing.T) { { name: "Removes VolumeMounts with same name and/or same MountPath", values: buildHelmValues(t, - withSourceControllerVolumeMounts( + withAllControllerVolumeMounts( corev1.VolumeMount{Name: "volume1", MountPath: "/tmp/volume1"}, - corev1.VolumeMount{Name: "volume2", MountPath: mountPath}, - corev1.VolumeMount{Name: volumeName, MountPath: "/tmp/existing"}, - corev1.VolumeMount{Name: volumeName, MountPath: mountPath}, + corev1.VolumeMount{Name: "volume2", MountPath: customCaPath}, + corev1.VolumeMount{Name: customCaVolumeName, MountPath: "/tmp/existing"}, + corev1.VolumeMount{Name: customCaVolumeName, MountPath: customCaPath}, ), ), checkValue: func(t *testing.T, out *apiextensionsv1.JSON) { require.NotNil(t, out) expected := buildHelmValues(t, - withSourceControllerVolumes(expectedCaVolume), - withSourceControllerVolumeMounts( + withAllControllerVolumes(expectedCaVolume), + withAllControllerExtraEnv(expectedCaEnvVar), + withAllControllerVolumeMounts( corev1.VolumeMount{Name: "volume1", MountPath: "/tmp/volume1"}, expectedCaVolumeMount, ), @@ -241,8 +241,8 @@ func TestAddCaToHelmValues(t *testing.T) { { name: "Removes Volumes with same name", values: buildHelmValues(t, - withSourceControllerVolumes( - corev1.Volume{Name: volumeName}, + withAllControllerVolumes( + corev1.Volume{Name: customCaVolumeName}, corev1.Volume{Name: "volume1"}, ), ), @@ -250,12 +250,34 @@ func TestAddCaToHelmValues(t *testing.T) { require.NotNil(t, out) expected := buildHelmValues(t, - withSourceControllerVolumes( + withAllControllerVolumes( corev1.Volume{Name: "volume1"}, - expectedCaVolume), - withSourceControllerVolumeMounts( - expectedCaVolumeMount, + expectedCaVolume, + ), + withAllControllerVolumeMounts(expectedCaVolumeMount), + withAllControllerExtraEnv(expectedCaEnvVar), + ) + assert.JSONEq(t, string(expected.Raw), string(out.Raw)) + }, + }, + { + name: "Removes EnvVars with same name", + values: buildHelmValues(t, + withAllControllerExtraEnv( + corev1.EnvVar{Name: "SSL_CERT_DIR"}, + corev1.EnvVar{Name: "ANOTHER_ENV_VAR"}, + ), + ), + checkValue: func(t *testing.T, out *apiextensionsv1.JSON) { + require.NotNil(t, out) + + expected := buildHelmValues(t, + withAllControllerExtraEnv( + corev1.EnvVar{Name: "ANOTHER_ENV_VAR"}, + expectedCaEnvVar, ), + withAllControllerVolumes(expectedCaVolume), + withAllControllerVolumeMounts(expectedCaVolumeMount), ) assert.JSONEq(t, string(expected.Raw), string(out.Raw)) }, @@ -266,35 +288,44 @@ func TestAddCaToHelmValues(t *testing.T) { wantErr: "failed to unmarshal helm values", }, { - name: "returns error for invalid sourceController json", + name: "returns error for invalid controller json", values: mustMarshalJSON(t, map[string]any{ - "sourceController": "not-an-object", + "helmController": "not-an-object", }), - wantErr: "failed to unmarshal sourceController", + wantErr: "failed to unmarshal helmController", }, { - name: "returns error for invalid sourceController.volumes json", + name: "returns error for invalid controller.volumes json", values: mustMarshalJSON(t, map[string]any{ - "sourceController": map[string]any{ + "helmController": map[string]any{ "volumes": "not-a-list", }, }), - wantErr: "failed to unmarshal sourceController.volumes", + wantErr: "failed to unmarshal helmController.volumes", }, { - name: "returns error for invalid sourceController.volumeMounts json", + name: "returns error for invalid controller.volumeMounts json", values: mustMarshalJSON(t, map[string]any{ - "sourceController": map[string]any{ + "helmController": map[string]any{ "volumeMounts": "not-a-list", }, }), - wantErr: "failed to unmarshal sourceController.volumeMounts", + wantErr: "failed to unmarshal helmController.volumeMounts", + }, + { + name: "returns error for invalid controller.extraEnv json", + values: mustMarshalJSON(t, map[string]any{ + "helmController": map[string]any{ + "extraEnv": "not-a-list", + }, + }), + wantErr: "failed to unmarshal helmController.extraEnv", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - out, err := AddCaToHelmValues(tt.values, secretName) + out, err := AddCaToHelmValues(tt.values, "custom-ca-secret") if tt.wantErr != "" { require.Error(t, err) assert.Contains(t, err.Error(), tt.wantErr) @@ -311,8 +342,8 @@ func TestAddCaToHelmValues(t *testing.T) { } type helmValues struct { - root map[string]any - sourceController map[string]any + root map[string]any + controllers map[string]map[string]any } type helmValuesOption func(*helmValues) @@ -321,16 +352,18 @@ func buildHelmValues(t *testing.T, opts ...helmValuesOption) *apiextensionsv1.JS t.Helper() builder := &helmValues{ - root: map[string]any{}, - sourceController: map[string]any{}, + root: map[string]any{}, + controllers: map[string]map[string]any{}, } for _, opt := range opts { opt(builder) } - if len(builder.sourceController) > 0 { - builder.root["sourceController"] = builder.sourceController + for controller, values := range builder.controllers { + if len(values) > 0 { + builder.root[controller] = values + } } return mustMarshalJSON(t, builder.root) @@ -342,21 +375,44 @@ func withRootField(key string, value any) helmValuesOption { } } -func withSourceControllerField(key string, value any) helmValuesOption { +func withAllControllerField(key string, value any) helmValuesOption { + return func(builder *helmValues) { + for _, controller := range fluxControllers { + withControllerField(controller, key, value)(builder) + } + } +} + +func withControllerField(controller string, key string, value any) helmValuesOption { + return func(builder *helmValues) { + if _, ok := builder.controllers[controller]; !ok { + builder.controllers[controller] = map[string]any{} + } + builder.controllers[controller][key] = value + } +} + +func withAllControllerVolumes(volumes ...corev1.Volume) helmValuesOption { return func(builder *helmValues) { - builder.sourceController[key] = value + for _, controller := range fluxControllers { + withControllerField(controller, "volumes", volumes)(builder) + } } } -func withSourceControllerVolumes(volumes ...corev1.Volume) helmValuesOption { +func withAllControllerVolumeMounts(volumeMounts ...corev1.VolumeMount) helmValuesOption { return func(builder *helmValues) { - builder.sourceController["volumes"] = volumes + for _, controller := range fluxControllers { + withControllerField(controller, "volumeMounts", volumeMounts)(builder) + } } } -func withSourceControllerVolumeMounts(volumeMounts ...corev1.VolumeMount) helmValuesOption { +func withAllControllerExtraEnv(envVars ...corev1.EnvVar) helmValuesOption { return func(builder *helmValues) { - builder.sourceController["volumeMounts"] = volumeMounts + for _, controller := range fluxControllers { + withControllerField(controller, "extraEnv", envVars)(builder) + } } } From 9396382288566ad7f807b17ed84dbd8d76c92768 Mon Sep 17 00:00:00 2001 From: "Nico Andres (SAP)" Date: Tue, 2 Jun 2026 16:12:38 +0200 Subject: [PATCH 07/20] fix lint On-behalf-of: @SAP nico.andres@sap.com Signed-off-by: Nico Andres (SAP) --- internal/controller/flux_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/controller/flux_controller.go b/internal/controller/flux_controller.go index 55c9613..0caa07b 100644 --- a/internal/controller/flux_controller.go +++ b/internal/controller/flux_controller.go @@ -42,7 +42,7 @@ import ( const conditionReasonError = "ReconcileError" // ErrManagedResources is an end-user facing error if errors are present inside Flux.Status.ManagedResources -var ErrManagedResources error = errors.New("resources contain reconcile errors") +var ErrManagedResources = errors.New("resources contain reconcile errors") // FluxReconciler reconciles a Flux object type FluxReconciler struct { From 5db7f931867fd1ab6455b7554e32865f3f530811 Mon Sep 17 00:00:00 2001 From: "Nico Andres (SAP)" Date: Mon, 8 Jun 2026 10:59:18 +0200 Subject: [PATCH 08/20] Revert changes On-behalf-of: @SAP nico.andres@sap.com Signed-off-by: Nico Andres (SAP) --- README.md | 5 - ...open-control-plane.io_providerconfigs.yaml | 14 - api/v1alpha1/providerconfig_types.go | 14 - docs/configuration/image-localization.md | 45 +-- internal/controller/flux_controller.go | 52 +--- pkg/flux/flux.go | 7 - pkg/flux/helm.go | 181 ----------- pkg/flux/helm_test.go | 286 ------------------ pkg/flux/secret.go | 16 +- pkg/flux/secret_test.go | 79 +---- test/e2e/flux_test.go | 29 +- test/e2e/platform/casecret.yaml | 8 - test/e2e/platform/providerconfig.yaml | 1 - 13 files changed, 32 insertions(+), 705 deletions(-) delete mode 100644 test/e2e/platform/casecret.yaml diff --git a/README.md b/README.md index b66e9c5..2de0681 100644 --- a/README.md +++ b/README.md @@ -146,11 +146,6 @@ metadata: spec: # Optional: Reconciliation interval pollInterval: "5m" - # Optional: Secret for custom CA certificate/bundle (will be copied to OpenControlPlane) - # CA certificate/bundle must be stored in key ca.crt - # Optionally: keys tls.key and tls.crt can be set for mTLS - # (https://fluxcd.io/flux/components/source/ocirepositories/#mutual-tls-authentication) - certSecretRef: "custom-ca-cert" # The Flux versions that can be installed versions: - version: "2.8.3" diff --git a/api/crds/manifests/flux.services.open-control-plane.io_providerconfigs.yaml b/api/crds/manifests/flux.services.open-control-plane.io_providerconfigs.yaml index 2894a7f..a228b44 100644 --- a/api/crds/manifests/flux.services.open-control-plane.io_providerconfigs.yaml +++ b/api/crds/manifests/flux.services.open-control-plane.io_providerconfigs.yaml @@ -41,20 +41,6 @@ spec: spec: description: spec defines the desired state of ProviderConfig properties: - certSecretRef: - description: |- - CertSecretRef is a reference to a Secret in the service provider's namespace - containing a single PEM-encoded CA certificate or a bundle used when pulling - the Helm chart from a private OCI registry. - The secret will be copied to the tenant namespace on the platform cluster - and referenced by the OCIRepository. - The secret is also copied to the flux-system namespace on the MCP cluster and is - mounted into all Flux controllers deployed by the chart. - The secret must be of type kubernetes.io/opaque and store the PEM-encoded - certificate/bundle under the ca.crt key. - It can also contain tls.crt and tls.key keys for mTLS. - (https://fluxcd.io/flux/components/source/ocirepositories/#mutual-tls-authentication) - type: string pollInterval: default: 1m description: PollInterval determines how often to reconcile resources diff --git a/api/v1alpha1/providerconfig_types.go b/api/v1alpha1/providerconfig_types.go index 7efe166..f9c1d4e 100644 --- a/api/v1alpha1/providerconfig_types.go +++ b/api/v1alpha1/providerconfig_types.go @@ -35,20 +35,6 @@ type ProviderConfigSpec struct { // +kubebuilder:default:="1m" // +kubebuilder:validation:Format=duration PollInterval *metav1.Duration `json:"pollInterval,omitempty"` - - // CertSecretRef is a reference to a Secret in the service provider's namespace - // containing a single PEM-encoded CA certificate or a bundle used when pulling - // the Helm chart from a private OCI registry. - // The secret will be copied to the tenant namespace on the platform cluster - // and referenced by the OCIRepository. - // The secret is also copied to the flux-system namespace on the MCP cluster and is - // mounted into all Flux controllers deployed by the chart. - // The secret must be of type kubernetes.io/opaque and store the PEM-encoded - // certificate/bundle under the ca.crt key. - // It can also contain tls.crt and tls.key keys for mTLS. - // (https://fluxcd.io/flux/components/source/ocirepositories/#mutual-tls-authentication) - // +optional - CertSecretRef string `json:"certSecretRef,omitempty"` } // FluxVersion defines a version of Flux that can be installed diff --git a/docs/configuration/image-localization.md b/docs/configuration/image-localization.md index 72315b9..ee1759b 100644 --- a/docs/configuration/image-localization.md +++ b/docs/configuration/image-localization.md @@ -13,7 +13,6 @@ In air-gapped environments, you typically need to: The Flux service provider handles this through: - **`chartPullSecret`**: Credentials for pulling the Helm chart from a private OCI registry -- **`certSecretRef`**: Private CA certificate for pulling the Helm chart from a private OCI registry - **`values.imagePullSecrets`**: Credentials for pulling Flux controller images (specified in Helm values) - **`values`**: Custom Helm values for image location overrides @@ -25,18 +24,15 @@ flowchart TB subgraph PC[Platform Cluster] subgraph SPN[Service Provider Namespace] chartsecret([chart-pull-secret]) - customcasecret([custom-ca-secret]) imgsecret1([image-pull-secret-1]) imgsecret2([image-pull-secret-2]) end subgraph TN[Tenant Namespace] chartsecretcopy([chart-pull-secret copy]) - customcasecretcopy([custom-ca-secret copy]) ocirepo([OCIRepository]) helmrel([HelmRelease]) ocirepo -. refs .-> chartsecretcopy - ocirepo -. refs .-> customcasecretcopy end end @@ -44,19 +40,15 @@ flowchart TB subgraph FS[flux-system namespace] imgcopy1([image-pull-secret-1]) imgcopy2([image-pull-secret-2]) - casecretcopy([custom-ca-secret]) fluxctrl[Flux Controllers] fluxctrl -. uses .-> imgcopy1 fluxctrl -. uses .-> imgcopy2 - fluxctrl -. uses .-> casecretcopy end end chartsecret -- copied to --> chartsecretcopy - customcasecret -- copied to --> customcasecretcopy imgsecret1 -- copied to --> imgcopy1 imgsecret2 -- copied to --> imgcopy2 - customcasecret -- copied to --> casecretcopy helmrel -- installs --> fluxctrl ``` @@ -71,13 +63,6 @@ metadata: name: flux-provider-config spec: versions: - # Secret with single PEM-encoded custom CA root certificate or bundle stored - # in the ca.crt key. - # Must exist in the service provider's namespace on the platform cluster - # Will be copied to the tenant namespace on the platform cluster and to the - # flux-system namespace on the OpenControlPlane. - certSecretRef: "custom-ca-cert" - - version: "2.8.3" chartVersion: "2.18.2" # Flux Helm chart location (private OCI registry) @@ -125,13 +110,6 @@ kubectl create secret docker-registry image-registry-credentials \ --docker-server=registry.internal.corp \ --docker-username= \ --docker-password= - -# Custom CA certificate secret with optional mTLS key and cert (for OCI registry connection) -kubectl create secret generic custom-ca-cert \ - --namespace \ - --ca.crt= \ - --tls.key= \ - --tls.crt= ``` ## How It Works @@ -142,22 +120,6 @@ kubectl create secret generic custom-ca-cert \ 2. The `OCIRepository` resource references this secret via `spec.secretRef` 3. The Flux Source Controller uses this secret to authenticate when pulling the Helm chart -### Custom CA Secret - -1. The secret specified in `certSecretRef` is copied from the service provider's namespace to the tenant namespace on the platform cluster -2. The `OCIRepository` resource references this secret via `spec.certSecretRef` -3. The Flux Source Controller uses this secret to establish a trusted connection with the OCI registry when pulling the Helm chart -4. The secret is also copied to the MCP cluster flux-system namespace and mounted into the all Flux controllers -by adding a volume, volumeMount and envVar to the helm values. Flux in the target OpenControlPlane hence is also able to verify -certificates signed with the custom CA certificate. - -> [!CAUTION] -> The custom CA certificate is not propagated to the OpenControlPlane cluster nodes. If you want to pull images from the same OCI registry you must add the custom CA certificate to thecluster nodes yourself. - -> [!NOTE] -> The secret referenced in certSecretRef can also contain two additional keys tls.key and tls.crt which will be -> used for mTLS (see here https://fluxcd.io/flux/components/source/ocirepositories/#mutual-tls-authentication) - ### Image Pull Secrets 1. Secrets specified in `values.imagePullSecrets` are extracted from the Helm values @@ -176,8 +138,6 @@ metadata: spec: chartUrl: "oci://harbor.corp.internal/charts/flux2" chartPullSecret: "harbor-credentials" - # Secret containing custom ca cert under ca.crt - will be copied to ManagedControlPlane - certSecretRef: "harbor-corp-internal-ca-cert" values: # Image pull secrets - will be copied to ManagedControlPlane imagePullSecrets: @@ -226,17 +186,16 @@ Verify secrets are copied to the correct namespaces: ```bash # Platform cluster - tenant namespace -kubectl get secrets -n mcp-- | grep -E "chart|image|ca" +kubectl get secrets -n mcp-- | grep -E "chart|image" # ManagedControlPlane - flux-system namespace -kubectl get secrets -n flux-system | grep -E "image|ca" +kubectl get secrets -n flux-system | grep -E "image" ``` ### Check OCIRepository Secret Reference ```bash kubectl get ocirepository flux -n mcp-- -o jsonpath='{.spec.secretRef}' -kubectl get ocirepository flux -n mcp-- -o jsonpath='{.spec.certSecretRef}' ``` ### Check HelmRelease Values diff --git a/internal/controller/flux_controller.go b/internal/controller/flux_controller.go index 0caa07b..7e1b1d8 100644 --- a/internal/controller/flux_controller.go +++ b/internal/controller/flux_controller.go @@ -42,7 +42,7 @@ import ( const conditionReasonError = "ReconcileError" // ErrManagedResources is an end-user facing error if errors are present inside Flux.Status.ManagedResources -var ErrManagedResources = errors.New("resources contain reconcile errors") +var ErrManagedResources error = errors.New("resources contain reconcile errors") // FluxReconciler reconciles a Flux object type FluxReconciler struct { @@ -149,11 +149,10 @@ func (r *FluxReconciler) createObjectManager(obj *apiv1alpha1.Flux, pc *apiv1alp mcpCluster := flux.NewManagedCluster(clusters.MCPCluster, clusters.MCPCluster.RESTConfig(), fluxNamespace, flux.ManagedControlPlane) // Sync image pull secrets from platform cluster to MCP - flux.ManageSecrets(mcpCluster, helmValues.ImagePullSecrets, flux.SecretCopyConfig{ + flux.ManagePullSecrets(mcpCluster, helmValues.ImagePullSecrets, flux.SecretCopyConfig{ SourceClient: r.PlatformCluster.Client(), SourceNamespace: r.PodNamespace, TargetNamespace: fluxNamespace, - TargetType: corev1.SecretTypeDockerConfigJson, }) // Sync chart pull secret within platform cluster from pod namespace to tenant namespace @@ -163,48 +162,13 @@ func (r *FluxReconciler) createObjectManager(obj *apiv1alpha1.Flux, pc *apiv1alp if err != nil { return nil, fmt.Errorf("error generating secret name: %w", err) } - flux.ManageSecrets(platformCluster, []corev1.LocalObjectReference{ + flux.ManagePullSecrets(platformCluster, []corev1.LocalObjectReference{ {Name: fluxVersion.ChartPullSecret}, }, flux.SecretCopyConfig{ SourceClient: r.PlatformCluster.Client(), SourceNamespace: r.PodNamespace, TargetNamespace: tenantNamespace, TargetName: prefixedChartPullSecret, - TargetType: corev1.SecretTypeDockerConfigJson, - }) - } - - var prefixedCertSecret string - if pc.Spec.CertSecretRef != "" { - // add custom ca volume and volumeMount to helm values - fluxVersion.Values, err = flux.AddCaToHelmValues(fluxVersion.Values, pc.Spec.CertSecretRef) - if err != nil { - return nil, fmt.Errorf("failed to add ca volume to helm values: %w", err) - } - - // Sync image pull secrets from platform cluster to MCP - flux.ManageSecrets(mcpCluster, []corev1.LocalObjectReference{ - {Name: pc.Spec.CertSecretRef}, - }, flux.SecretCopyConfig{ - SourceClient: r.PlatformCluster.Client(), - SourceNamespace: r.PodNamespace, - TargetNamespace: fluxNamespace, - TargetType: corev1.SecretTypeOpaque, - }) - - // Sync ca secret within platform cluster from pod namespace to tenant namespace - prefixedCertSecret, err = flux.PrefixSecretName(pc.Spec.CertSecretRef) - if err != nil { - return nil, fmt.Errorf("error generating secret name: %w", err) - } - flux.ManageSecrets(platformCluster, []corev1.LocalObjectReference{ - {Name: pc.Spec.CertSecretRef}, - }, flux.SecretCopyConfig{ - SourceClient: r.PlatformCluster.Client(), - SourceNamespace: r.PodNamespace, - TargetNamespace: tenantNamespace, - TargetName: prefixedCertSecret, - TargetType: corev1.SecretTypeOpaque, }) } @@ -213,7 +177,6 @@ func (r *FluxReconciler) createObjectManager(obj *apiv1alpha1.Flux, pc *apiv1alp Cluster: platformCluster, MCPNamespace: fluxNamespace, ChartPullSecretName: prefixedChartPullSecret, - CertSecretName: prefixedCertSecret, Obj: obj, ProviderConfig: pc, ClusterContext: clusters, @@ -230,15 +193,8 @@ func (r *FluxReconciler) createObjectManager(obj *apiv1alpha1.Flux, pc *apiv1alp { Name: prefixedChartPullSecret, }, - { - Name: prefixedCertSecret, - }, }) - - cpSecretsToKeep := make([]corev1.LocalObjectReference, 0, 1+len(helmValues.ImagePullSecrets)) - cpSecretsToKeep = append(cpSecretsToKeep, corev1.LocalObjectReference{Name: pc.Spec.CertSecretRef}) - cpSecretsToKeep = append(cpSecretsToKeep, helmValues.ImagePullSecrets...) - controlPlaneCleaner := flux.NewSecretCleaner(mcpCluster, fluxNamespace, cpSecretsToKeep) + controlPlaneCleaner := flux.NewSecretCleaner(mcpCluster, fluxNamespace, helmValues.ImagePullSecrets) mgr.AddCleaner(platformCleaner) mgr.AddCleaner(controlPlaneCleaner) diff --git a/pkg/flux/flux.go b/pkg/flux/flux.go index 468711a..3ea1099 100644 --- a/pkg/flux/flux.go +++ b/pkg/flux/flux.go @@ -48,8 +48,6 @@ type ManageFluxResourcesParams struct { MCPNamespace string // ChartPullSecretName defines the name of the secret copy that will be placed in the Cluster namespace ChartPullSecretName string - // CertSecretName defines the name of the secret copy that will be placed in the Cluster namespace - CertSecretName string // Obj is the tenant API object that is being reconciled Obj *apiv1alpha1.Flux // ProviderConfig of the current reconciliation context @@ -89,11 +87,6 @@ func ManageFluxResources(p ManageFluxResourcesParams) { Name: p.ChartPullSecretName, } } - if p.CertSecretName != "" { - ociRepo.Spec.CertSecretRef = &meta.LocalObjectReference{ - Name: p.CertSecretName, - } - } return nil }, DependsOn: []ManagedObject{}, diff --git a/pkg/flux/helm.go b/pkg/flux/helm.go index e3d0081..e9a9e86 100644 --- a/pkg/flux/helm.go +++ b/pkg/flux/helm.go @@ -15,34 +15,11 @@ package flux import ( "encoding/json" - "fmt" - "strings" corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" ) -const ( - customCaVolumeName = "custom-ca-bundle" - customCaPath = "/etc/open-control-plane/custom-ca" -) - -// from x509 go standard lib (https://github.com/golang/go/blob/015343854b5d9e2829481df30dbcae2ca6682d25/src/crypto/x509/root_linux.go) -var certDirectories = []string{ - "/etc/ssl/certs", - "/etc/pki/tls/certs", -} - -var fluxControllers = []string{ - "helmController", - "imageAutomationController", - "imageReflectionController", - "kustomizeController", - "notificationController", - "sourceController", - "sourceWatcher", -} - // HelmValues defines the Helm values that are explicitly processed during reconciliation. // The Flux Helm chart uses imagePullSecrets at the top level, not under global. type HelmValues struct { @@ -67,161 +44,3 @@ func ExtractHelmValues(values *apiextensionsv1.JSON) (*HelmValues, error) { return vals, nil } - -// AddCaToHelmValues removes conflicting volumes, volumeMounts and envVars (matching by name and/or mountPath) and -// adds a volume, volumeMount and envVar on all Flux controller helm values sections to import the custom CA certificate. -func AddCaToHelmValues(values *apiextensionsv1.JSON, secretName string) (*apiextensionsv1.JSON, error) { - var root = map[string]json.RawMessage{} - - caVolume := corev1.Volume{ - Name: customCaVolumeName, - VolumeSource: corev1.VolumeSource{ - Secret: &corev1.SecretVolumeSource{ - SecretName: secretName, - Items: []corev1.KeyToPath{ - { - Key: "ca.crt", - Path: "ca.crt", - }, - }, - }, - }, - } - - caVolumeMount := corev1.VolumeMount{ - Name: customCaVolumeName, - ReadOnly: true, - MountPath: customCaPath, - } - - caEnvVar := corev1.EnvVar{ - Name: "SSL_CERT_DIR", - Value: strings.Join(append(certDirectories, customCaPath), ":"), - } - - if values != nil && len(values.Raw) > 0 { - if err := json.Unmarshal(values.Raw, &root); err != nil { - return nil, fmt.Errorf("failed to unmarshal helm values: %w", err) - } - } - - for _, controller := range fluxControllers { - if err := addCaToController(root, controller, caVolume, caVolumeMount, caEnvVar); err != nil { - return nil, err - } - } - - out, err := json.Marshal(root) - if err != nil { - return nil, fmt.Errorf("failed to marshal helm values: %w", err) - } - - return &apiextensionsv1.JSON{Raw: out}, nil -} - -func addCaToController( - root map[string]json.RawMessage, - controller string, - caVolume corev1.Volume, - caVolumeMount corev1.VolumeMount, - caEnvVar corev1.EnvVar, -) error { - var controllerValues map[string]json.RawMessage - var volumes []corev1.Volume - var volumeMounts []corev1.VolumeMount - var envVars []corev1.EnvVar - - if err := unmarshalIfPresent(root, controller, &controllerValues); err != nil { - return fmt.Errorf("failed to unmarshal %s: %w", controller, err) - } - if controllerValues == nil { - controllerValues = map[string]json.RawMessage{} - } - - if err := unmarshalIfPresent(controllerValues, "volumes", &volumes); err != nil { - return fmt.Errorf("failed to unmarshal %s.volumes: %w", controller, err) - } - - if err := unmarshalIfPresent(controllerValues, "volumeMounts", &volumeMounts); err != nil { - return fmt.Errorf("failed to unmarshal %s.volumeMounts: %w", controller, err) - } - - if err := unmarshalIfPresent(controllerValues, "extraEnv", &envVars); err != nil { - return fmt.Errorf("failed to unmarshal %s.extraEnv: %w", controller, err) - } - - volumes = removeConflictingVolumesAndAppend(volumes, caVolume) - volumeMounts = removeConflictingVolumeMountsAndAppend(volumeMounts, caVolumeMount) - envVars = removeConflictingEnvVarsAndAppend(envVars, caEnvVar) - - volumesRaw, err := json.Marshal(volumes) - if err != nil { - return fmt.Errorf("failed to marshal %s.volumes: %w", controller, err) - } - - volumeMountsRaw, err := json.Marshal(volumeMounts) - if err != nil { - return fmt.Errorf("failed to marshal %s.volumeMounts: %w", controller, err) - } - - envVarsRaw, err := json.Marshal(envVars) - if err != nil { - return fmt.Errorf("failed to marshal %s.extraEnv: %w", controller, err) - } - - controllerValues["volumes"] = volumesRaw - controllerValues["volumeMounts"] = volumeMountsRaw - controllerValues["extraEnv"] = envVarsRaw - - controllerRaw, err := json.Marshal(controllerValues) - if err != nil { - return fmt.Errorf("failed to marshal %s: %w", controller, err) - } - - root[controller] = controllerRaw - return nil -} - -func removeConflictingVolumesAndAppend(volumes []corev1.Volume, caVolume corev1.Volume) []corev1.Volume { - updated := []corev1.Volume{} - for _, volume := range volumes { - if volume.Name != caVolume.Name { - updated = append(updated, volume) - } - } - updated = append(updated, caVolume) - return updated -} - -func removeConflictingVolumeMountsAndAppend(volumeMounts []corev1.VolumeMount, caVolumeMount corev1.VolumeMount) []corev1.VolumeMount { - updated := []corev1.VolumeMount{} - for _, volumeMount := range volumeMounts { - if volumeMount.MountPath != caVolumeMount.MountPath && volumeMount.Name != caVolumeMount.Name { - updated = append(updated, volumeMount) - } - } - updated = append(updated, caVolumeMount) - return updated -} - -func removeConflictingEnvVarsAndAppend(envVars []corev1.EnvVar, caEnvVar corev1.EnvVar) []corev1.EnvVar { - updated := []corev1.EnvVar{} - for _, envVar := range envVars { - if envVar.Name != caEnvVar.Name { - updated = append(updated, envVar) - } - } - updated = append(updated, caEnvVar) - return updated -} - -func unmarshalIfPresent(obj map[string]json.RawMessage, key string, out any) error { - raw, ok := obj[key] - if !ok || len(raw) == 0 { - return nil - } - if err := json.Unmarshal(raw, out); err != nil { - return fmt.Errorf("invalid %s JSON: %w", key, err) - } - return nil -} diff --git a/pkg/flux/helm_test.go b/pkg/flux/helm_test.go index 00f8241..a91dca7 100644 --- a/pkg/flux/helm_test.go +++ b/pkg/flux/helm_test.go @@ -15,12 +15,10 @@ package flux import ( "encoding/json" - "strings" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" ) @@ -132,290 +130,6 @@ func TestExtractHelmValues(t *testing.T) { } } -func TestAddCaToHelmValues(t *testing.T) { - expectedCaVolume := corev1.Volume{ - Name: customCaVolumeName, - VolumeSource: corev1.VolumeSource{ - Secret: &corev1.SecretVolumeSource{ - SecretName: "custom-ca-secret", - Items: []corev1.KeyToPath{{ - Key: "ca.crt", - Path: "ca.crt", - }}, - }, - }, - } - expectedCaVolumeMount := corev1.VolumeMount{ - Name: customCaVolumeName, - ReadOnly: true, - MountPath: customCaPath, - } - expectedCaEnvVar := corev1.EnvVar{ - Name: "SSL_CERT_DIR", - Value: strings.Join(append(certDirectories, customCaPath), ":"), - } - - tests := []struct { - name string - values *apiextensionsv1.JSON - wantErr string - checkValue func(t *testing.T, out *apiextensionsv1.JSON) - }{ - { - name: "Adds controller volumes, volumeMounts and extraEnv when no helm values are set", - values: nil, - checkValue: func(t *testing.T, out *apiextensionsv1.JSON) { - require.NotNil(t, out) - - expected := buildHelmValues(t, - withAllControllerVolumes(expectedCaVolume), - withAllControllerVolumeMounts(expectedCaVolumeMount), - withAllControllerExtraEnv(expectedCaEnvVar), - ) - - assert.JSONEq(t, string(expected.Raw), string(out.Raw)) - }, - }, - { - name: "Preserves existing helm values and adds CA entries", - values: buildHelmValues(t, - withRootField("namespace", "other-namespace"), - withAllControllerField( - "resources", map[string]any{ - "limits": map[string]any{ - "memory": "256Mi", - }, - }), - withAllControllerVolumes(corev1.Volume{Name: "existing-volume"}), - withAllControllerVolumeMounts( - corev1.VolumeMount{Name: "existing-volume", MountPath: "/tmp/existing"}), - ), - checkValue: func(t *testing.T, out *apiextensionsv1.JSON) { - require.NotNil(t, out) - - expected := buildHelmValues(t, - withRootField("namespace", "other-namespace"), - withAllControllerField("resources", map[string]any{ - "limits": map[string]any{"memory": "256Mi"}, - }), - withAllControllerVolumes(expectedCaVolume), - withAllControllerVolumeMounts(expectedCaVolumeMount), - withAllControllerExtraEnv(expectedCaEnvVar), - withAllControllerVolumes( - corev1.Volume{Name: "existing-volume"}, - expectedCaVolume, - ), - withAllControllerVolumeMounts( - corev1.VolumeMount{Name: "existing-volume", MountPath: "/tmp/existing"}, - expectedCaVolumeMount, - ), - ) - assert.JSONEq(t, string(expected.Raw), string(out.Raw)) - }, - }, - { - name: "Removes VolumeMounts with same name and/or same MountPath", - values: buildHelmValues(t, - withAllControllerVolumeMounts( - corev1.VolumeMount{Name: "volume1", MountPath: "/tmp/volume1"}, - corev1.VolumeMount{Name: "volume2", MountPath: customCaPath}, - corev1.VolumeMount{Name: customCaVolumeName, MountPath: "/tmp/existing"}, - corev1.VolumeMount{Name: customCaVolumeName, MountPath: customCaPath}, - ), - ), - checkValue: func(t *testing.T, out *apiextensionsv1.JSON) { - require.NotNil(t, out) - - expected := buildHelmValues(t, - withAllControllerVolumes(expectedCaVolume), - withAllControllerExtraEnv(expectedCaEnvVar), - withAllControllerVolumeMounts( - corev1.VolumeMount{Name: "volume1", MountPath: "/tmp/volume1"}, - expectedCaVolumeMount, - ), - ) - - assert.JSONEq(t, string(expected.Raw), string(out.Raw)) - }, - }, - { - name: "Removes Volumes with same name", - values: buildHelmValues(t, - withAllControllerVolumes( - corev1.Volume{Name: customCaVolumeName}, - corev1.Volume{Name: "volume1"}, - ), - ), - checkValue: func(t *testing.T, out *apiextensionsv1.JSON) { - require.NotNil(t, out) - - expected := buildHelmValues(t, - withAllControllerVolumes( - corev1.Volume{Name: "volume1"}, - expectedCaVolume, - ), - withAllControllerVolumeMounts(expectedCaVolumeMount), - withAllControllerExtraEnv(expectedCaEnvVar), - ) - assert.JSONEq(t, string(expected.Raw), string(out.Raw)) - }, - }, - { - name: "Removes EnvVars with same name", - values: buildHelmValues(t, - withAllControllerExtraEnv( - corev1.EnvVar{Name: "SSL_CERT_DIR"}, - corev1.EnvVar{Name: "ANOTHER_ENV_VAR"}, - ), - ), - checkValue: func(t *testing.T, out *apiextensionsv1.JSON) { - require.NotNil(t, out) - - expected := buildHelmValues(t, - withAllControllerExtraEnv( - corev1.EnvVar{Name: "ANOTHER_ENV_VAR"}, - expectedCaEnvVar, - ), - withAllControllerVolumes(expectedCaVolume), - withAllControllerVolumeMounts(expectedCaVolumeMount), - ) - assert.JSONEq(t, string(expected.Raw), string(out.Raw)) - }, - }, - { - name: "Returns error for invalid root json", - values: &apiextensionsv1.JSON{Raw: []byte("not-json")}, - wantErr: "failed to unmarshal helm values", - }, - { - name: "returns error for invalid controller json", - values: mustMarshalJSON(t, map[string]any{ - "helmController": "not-an-object", - }), - wantErr: "failed to unmarshal helmController", - }, - { - name: "returns error for invalid controller.volumes json", - values: mustMarshalJSON(t, map[string]any{ - "helmController": map[string]any{ - "volumes": "not-a-list", - }, - }), - wantErr: "failed to unmarshal helmController.volumes", - }, - { - name: "returns error for invalid controller.volumeMounts json", - values: mustMarshalJSON(t, map[string]any{ - "helmController": map[string]any{ - "volumeMounts": "not-a-list", - }, - }), - wantErr: "failed to unmarshal helmController.volumeMounts", - }, - { - name: "returns error for invalid controller.extraEnv json", - values: mustMarshalJSON(t, map[string]any{ - "helmController": map[string]any{ - "extraEnv": "not-a-list", - }, - }), - wantErr: "failed to unmarshal helmController.extraEnv", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - out, err := AddCaToHelmValues(tt.values, "custom-ca-secret") - if tt.wantErr != "" { - require.Error(t, err) - assert.Contains(t, err.Error(), tt.wantErr) - return - } - - require.NoError(t, err) - require.NotNil(t, out) - if tt.checkValue != nil { - tt.checkValue(t, out) - } - }) - } -} - -type helmValues struct { - root map[string]any - controllers map[string]map[string]any -} - -type helmValuesOption func(*helmValues) - -func buildHelmValues(t *testing.T, opts ...helmValuesOption) *apiextensionsv1.JSON { - t.Helper() - - builder := &helmValues{ - root: map[string]any{}, - controllers: map[string]map[string]any{}, - } - - for _, opt := range opts { - opt(builder) - } - - for controller, values := range builder.controllers { - if len(values) > 0 { - builder.root[controller] = values - } - } - - return mustMarshalJSON(t, builder.root) -} - -func withRootField(key string, value any) helmValuesOption { - return func(builder *helmValues) { - builder.root[key] = value - } -} - -func withAllControllerField(key string, value any) helmValuesOption { - return func(builder *helmValues) { - for _, controller := range fluxControllers { - withControllerField(controller, key, value)(builder) - } - } -} - -func withControllerField(controller string, key string, value any) helmValuesOption { - return func(builder *helmValues) { - if _, ok := builder.controllers[controller]; !ok { - builder.controllers[controller] = map[string]any{} - } - builder.controllers[controller][key] = value - } -} - -func withAllControllerVolumes(volumes ...corev1.Volume) helmValuesOption { - return func(builder *helmValues) { - for _, controller := range fluxControllers { - withControllerField(controller, "volumes", volumes)(builder) - } - } -} - -func withAllControllerVolumeMounts(volumeMounts ...corev1.VolumeMount) helmValuesOption { - return func(builder *helmValues) { - for _, controller := range fluxControllers { - withControllerField(controller, "volumeMounts", volumeMounts)(builder) - } - } -} - -func withAllControllerExtraEnv(envVars ...corev1.EnvVar) helmValuesOption { - return func(builder *helmValues) { - for _, controller := range fluxControllers { - withControllerField(controller, "extraEnv", envVars)(builder) - } - } -} - func mustMarshalJSON(t *testing.T, v any) *apiextensionsv1.JSON { t.Helper() raw, err := json.Marshal(v) diff --git a/pkg/flux/secret.go b/pkg/flux/secret.go index a5c0568..6bc6398 100644 --- a/pkg/flux/secret.go +++ b/pkg/flux/secret.go @@ -45,14 +45,12 @@ type SecretCopyConfig struct { // TargetName is an optional value to adjust the name of the target secret // instead of using the source secret name. TargetName string - // TargetType is the type of the target secret. - TargetType corev1.SecretType } -// ManageSecrets syncs every secret to the target cluster. -func ManageSecrets(targetCluster ManagedCluster, secrets []corev1.LocalObjectReference, config SecretCopyConfig) { - for _, secret := range secrets { - secretName := secret.Name +// ManagePullSecrets syncs every image pull secret to the target cluster. +func ManagePullSecrets(targetCluster ManagedCluster, imagePullSecrets []corev1.LocalObjectReference, config SecretCopyConfig) { + for _, pullSecret := range imagePullSecrets { + secretName := pullSecret.Name if config.TargetName != "" { secretName = config.TargetName } @@ -69,7 +67,7 @@ func ManageSecrets(targetCluster ManagedCluster, secrets []corev1.LocalObjectRef } sourceSecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: secret.Name, + Name: pullSecret.Name, Namespace: config.SourceNamespace, }, } @@ -77,7 +75,7 @@ func ManageSecrets(targetCluster ManagedCluster, secrets []corev1.LocalObjectRef if err := config.SourceClient.Get(ctx, client.ObjectKeyFromObject(sourceSecret), sourceSecret); err != nil { return err } - mutator := openmcpresources.NewSecretMutator(secretName, config.TargetNamespace, sourceSecret.Data, config.TargetType) + mutator := openmcpresources.NewSecretMutator(secretName, config.TargetNamespace, sourceSecret.Data, corev1.SecretTypeDockerConfigJson) return mutator.Mutate(oSecret) }, StatusFunc: SimpleStatus, @@ -125,7 +123,7 @@ type secretCleaner struct { secretsToKeep []corev1.LocalObjectReference } -// NewSecretCleaner removes redundant secrets in the given target namespace +// NewSecretCleaner removes redundant pull secrets in the given target namespace // by removing any secret labeled as managed by sp-flux that is not in secretsToKeep. func NewSecretCleaner(cluster ManagedCluster, namespace string, secretsToKeep []corev1.LocalObjectReference) OrphanCleaner { return &secretCleaner{ diff --git a/pkg/flux/secret_test.go b/pkg/flux/secret_test.go index 99ae6b2..4cba8a0 100644 --- a/pkg/flux/secret_test.go +++ b/pkg/flux/secret_test.go @@ -34,8 +34,8 @@ import ( "github.com/openmcp-project/service-provider-flux/pkg/testutils" ) -func TestManageSecrets(t *testing.T) { - sourcePullSecret := &corev1.Secret{ +func TestManagePullSecrets(t *testing.T) { + sourceSecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "test-pull-secret", Namespace: "source-ns", @@ -45,25 +45,13 @@ func TestManageSecrets(t *testing.T) { }, Type: corev1.SecretTypeDockerConfigJson, } - sourceCaSecret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-ca-secret", - Namespace: "source-ns", - }, - Data: map[string][]byte{ - "ca.crt": []byte("data"), - }, - Type: corev1.SecretTypeOpaque, - } - fakeCluster := testutils.CreateFakeCluster(t, "platform", sourcePullSecret, sourceCaSecret) + fakeCluster := testutils.CreateFakeCluster(t, "platform", sourceSecret) tests := []struct { - name string - targetCluster ManagedCluster - imagePullSecrets []corev1.LocalObjectReference - caSecrets []corev1.LocalObjectReference - configPullSecrets SecretCopyConfig - configCaSecrets SecretCopyConfig + name string + targetCluster ManagedCluster + imagePullSecrets []corev1.LocalObjectReference + config SecretCopyConfig }{ { name: "syncs secret with correct type", @@ -71,20 +59,10 @@ func TestManageSecrets(t *testing.T) { imagePullSecrets: []corev1.LocalObjectReference{ {Name: "test-pull-secret"}, }, - caSecrets: []corev1.LocalObjectReference{ - {Name: "test-ca-secret"}, - }, - configPullSecrets: SecretCopyConfig{ + config: SecretCopyConfig{ SourceClient: fakeCluster.Client(), SourceNamespace: "source-ns", TargetNamespace: "target-ns", - TargetType: corev1.SecretTypeDockerConfigJson, - }, - configCaSecrets: SecretCopyConfig{ - SourceClient: fakeCluster.Client(), - SourceNamespace: "source-ns", - TargetNamespace: "target-ns", - TargetType: corev1.SecretTypeOpaque, }, }, { @@ -93,30 +71,18 @@ func TestManageSecrets(t *testing.T) { imagePullSecrets: []corev1.LocalObjectReference{ {Name: "test-pull-secret"}, }, - caSecrets: []corev1.LocalObjectReference{ - {Name: "test-ca-secret"}, - }, - configPullSecrets: SecretCopyConfig{ + config: SecretCopyConfig{ SourceClient: fakeCluster.Client(), SourceNamespace: "source-ns", TargetNamespace: "target-ns", TargetName: fmt.Sprintf("%s%s", secretNamePrefix, "test-pull-secret"), - TargetType: corev1.SecretTypeDockerConfigJson, - }, - configCaSecrets: SecretCopyConfig{ - SourceClient: fakeCluster.Client(), - SourceNamespace: "source-ns", - TargetNamespace: "target-ns", - TargetName: fmt.Sprintf("%s%s", secretNamePrefix, "test-ca-secret"), - TargetType: corev1.SecretTypeOpaque, }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - ManageSecrets(tt.targetCluster, tt.imagePullSecrets, tt.configPullSecrets) - ManageSecrets(tt.targetCluster, tt.caSecrets, tt.configCaSecrets) + ManagePullSecrets(tt.targetCluster, tt.imagePullSecrets, tt.config) // Apply managed objects mgr := NewManager() @@ -131,35 +97,18 @@ func TestManageSecrets(t *testing.T) { for _, pullSecret := range tt.imagePullSecrets { targetSecret := &corev1.Secret{} targetSecretName := pullSecret.Name - if tt.configPullSecrets.TargetName != "" { - targetSecretName = tt.configPullSecrets.TargetName + if tt.config.TargetName != "" { + targetSecretName = tt.config.TargetName } err := fakeCluster.Client().Get(context.Background(), client.ObjectKey{ Name: targetSecretName, - Namespace: tt.configPullSecrets.TargetNamespace, + Namespace: tt.config.TargetNamespace, }, targetSecret) require.NoError(t, err) - assert.Equal(t, sourcePullSecret.Data, targetSecret.Data) + assert.Equal(t, sourceSecret.Data, targetSecret.Data) assert.Equal(t, corev1.SecretTypeDockerConfigJson, targetSecret.Type, "target secret should have the correct type") } - - // Verify secret was synced with correct type - for _, caSecret := range tt.caSecrets { - targetSecret := &corev1.Secret{} - targetSecretName := caSecret.Name - if tt.configCaSecrets.TargetName != "" { - targetSecretName = tt.configCaSecrets.TargetName - } - err := fakeCluster.Client().Get(context.Background(), client.ObjectKey{ - Name: targetSecretName, - Namespace: tt.configCaSecrets.TargetNamespace, - }, targetSecret) - require.NoError(t, err) - - assert.Equal(t, sourceCaSecret.Data, targetSecret.Data) - assert.Equal(t, corev1.SecretTypeOpaque, targetSecret.Type, "target secret should have the correct type") - } }) } } diff --git a/test/e2e/flux_test.go b/test/e2e/flux_test.go index 22d133c..16bd5ab 100644 --- a/test/e2e/flux_test.go +++ b/test/e2e/flux_test.go @@ -89,15 +89,6 @@ func TestServiceProvider(t *testing.T) { if err := wait.For(conditions.New(c.Client().Resources()).ResourcesFound(pullSecrets), wait.WithTimeout(2*time.Minute)); err != nil { t.Errorf("pull secret not found: %v", err) } - caSecret := &corev1.Secret{} - caSecret.SetName("sp-flux-custom-ca-cert") - caSecret.SetNamespace(tenantNamespace) - caSecrets := &corev1.SecretList{ - Items: []corev1.Secret{*caSecret}, - } - if err := wait.For(conditions.New(c.Client().Resources()).ResourcesFound(caSecrets), wait.WithTimeout(2*time.Minute)); err != nil { - t.Errorf("ca secret not found: %v", err) - } return ctx }). Assess("ManagedControlPlane resources have been created", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { @@ -115,15 +106,6 @@ func TestServiceProvider(t *testing.T) { if err := wait.For(conditions.New(mcp.Client().Resources()).ResourcesFound(list), wait.WithTimeout(2*time.Minute)); err != nil { t.Errorf("image pull secret not found on control plane: %v", err) } - caSecret := &corev1.Secret{} - caSecret.SetName("custom-ca-cert") - caSecret.SetNamespace("flux-system") - caSecrets := &corev1.SecretList{ - Items: []corev1.Secret{*caSecret}, - } - if err := wait.For(conditions.New(mcp.Client().Resources()).ResourcesFound(caSecrets), wait.WithTimeout(2*time.Minute)); err != nil { - t.Errorf("ca secret not found on control plane: %v", err) - } return ctx }). Assess("domain objects can be created", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { @@ -243,7 +225,7 @@ func TestServiceProvider(t *testing.T) { } return ctx }). - Assess("provider config update secrets", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { + Assess("provider config update drops pull secrets", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { if err := v1alpha1.AddToScheme(c.Client().Resources().GetScheme()); err != nil { t.Errorf("failed to add api types to client scheme: %s", err) return ctx @@ -256,7 +238,6 @@ func TestServiceProvider(t *testing.T) { } providerConfig.Spec.Versions[0].ChartPullSecret = "" providerConfig.Spec.Versions[0].Values = nil - providerConfig.Spec.CertSecretRef = "" if err := c.Client().Resources().Update(ctx, providerConfig); err != nil { t.Errorf("failed to update provider config: %v", err) } @@ -276,7 +257,7 @@ func TestServiceProvider(t *testing.T) { return ctx }). - Assess("platform secrets deleted", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { + Assess("platform chart pull secret deleted", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { tenantNamespace, err := libutils.StableMCPNamespace(mcpName, "default") if err != nil { t.Errorf("failed to get tenant namespace: %v", err) @@ -287,11 +268,11 @@ func TestServiceProvider(t *testing.T) { ResourceListN(spFluxSecrets, 0, klientresources.WithLabelSelector( labels.FormatLabels(map[string]string{flux.LabelManagedBy: "service-provider-flux"}))), wait.WithTimeout(2*time.Minute)); err != nil { - t.Errorf("orphaned platform secret is not deleted: %v", err) + t.Errorf("orphaned chart pull secret is not deleted: %v", err) } return ctx }). - Assess("control plane secrets deleted", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { + Assess("control plane image pull secrets deleted", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { mcp, err := clusterutils.MCPConfig(ctx, c, mcpName) if err != nil { t.Error(err) @@ -302,7 +283,7 @@ func TestServiceProvider(t *testing.T) { ResourceListN(spFluxSecrets, 0, klientresources.WithLabelSelector( labels.FormatLabels(map[string]string{flux.LabelManagedBy: "service-provider-flux"}))), wait.WithTimeout(2*time.Minute)); err != nil { - t.Errorf("orphaned secret is not deleted: %v", err) + t.Errorf("orphaned image pull secret is not deleted: %v", err) } return ctx }). diff --git a/test/e2e/platform/casecret.yaml b/test/e2e/platform/casecret.yaml deleted file mode 100644 index 24fe66f..0000000 --- a/test/e2e/platform/casecret.yaml +++ /dev/null @@ -1,8 +0,0 @@ -apiVersion: v1 -data: - # dummy value for testing - ca.crt: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUZIVENDQXdXZ0F3SUJBZ0lVU29EQzZLUktnVlpTTVlubGQ4NmYrT2tBeldrd0RRWUpLb1pJaHZjTkFRRUwKQlFBd0hqRWNNQm9HQTFVRUF3d1RiRzlqWVd3dGNtVm5hWE4wY25rdGNtOXZkREFlRncweU5qQTFNakV4TURFeQpNekZhRncwek5qQTFNVGd4TURFeU16RmFNQjR4SERBYUJnTlZCQU1NRTJ4dlkyRnNMWEpsWjJsemRISjVMWEp2CmIzUXdnZ0lpTUEwR0NTcUdTSWIzRFFFQkFRVUFBNElDRHdBd2dnSUtBb0lDQVFETllrVThEc0NVVlNyU3NScFUKSGxaUUgxUU5hT0pHNDBsSnFmZkFHbHpLWis3d0ZSMzA4dkI0VGUrNUtqN0pzN3dQRWtaUnhPOVpmc05YOTdRUApQY0F2R0RtTUFIL2xDK0RsVndHK0RuQVZDYnVoWEdEcHhJd09ycC9Qckh1OEtFbHY4RDdFbXpFd0Y0YXdhNGFnCmRCeWRDc21yZm9oS3JaZG9UQnlhaTR3bWExaCsxc2JtQTByMUROVGxnLzlQSUFPL0NsSGpXZHgrMjhjc3VON2MKQ3NzSmtucHVBVXBLRkc0TjJldHlhK2xVUHFsa3dnd05NTjFkMGRVbCthUm9TeGRxS2xJZEtrVVdGNmNNMkRhZwpTLzE1ZTNOVWkrUFZVMUtPdDZ6c3kvVjVVRXdTYkliVmcxRE5mRjBFMHNCbFFucWxvTERBNm9kaVJnd0dLNHpFCnB3N2F3N2FaU3c5WEt3RjJWZ2hYL1ZVV0dvUkswU25OU1ZnNUdFQUtFb3ZLN2s3UkdXanpRRlJkYWt4Z1U4NkUKbWdxdXh1ZSs1OVROWnFqVnJCQjdnN2trTGQzNFRidHdQR1VqSU1vRHVtcGxDM1c4M0U0SDEvbnFPV2tJYWp5RQpPRmI0ZmtXM1NlcXlsTytyRmxQKzJVWDRFdHZHeE82a1lDWkdTcmR5TFVKZ21ZbjBDYlI1V3BlOXdJWExqWExmCm9QTGJmQWYzaDlwZkwrdlhnSm44WE0zYnUwTEVjY3RMZk03NFgwL0h4S3BDOTRtR2hmdFRnd3VEM3ZHUmluUDYKaE5NMXQ2Y0NqSzVWYXd6NVFMQzJVb2NQWkt0Zm1hSUhhU1JIOVdEaitxQkNjTXVQYThFMUlEZ1RlazMwaEZIVAp5SXFtNEd5VVBkZmtyNXJjMEc3WGNGUGRnUUlEQVFBQm8xTXdVVEFkQmdOVkhRNEVGZ1FVZ0Q3SzZLa1JMVDYyCnhWMG50NTUrTlJrdU9mMHdId1lEVlIwakJCZ3dGb0FVZ0Q3SzZLa1JMVDYyeFYwbnQ1NStOUmt1T2Ywd0R3WUQKVlIwVEFRSC9CQVV3QXdFQi96QU5CZ2txaGtpRzl3MEJBUXNGQUFPQ0FnRUFORjZwczJjT3hqdHFiMzF3S0drdgpqTG9ucm5VT0JvdTgxdkFMK0FycnZDMWtvdEF2Nnl2YTd1OWlTS2Q2VGN3Q2hFdUJtbE1ocFFDaUR5UDRrUU1hCkRsRFZqdStZbk55NHhGSHV4V1ovczhlanRBT21lTmJ3bzgyVStkd2NVZ2w4UVR6QU1rV3k1cmxDVXJScE9ZYmIKUE1nSFlTckd0YVZEelBkc2dkN0ZlUStGcEkwMWd6eXBFODd3TjdEZ0RoMnRyVGV6MFMwQWhKbk40K3NtbFltVwpRaTNkQXAzRFBUZmFMczZlSWEvOWJBRXhrR0lYS3JRQytaRU5IZG9vRDBEY2VFamNpeTJxVllYTXNicVhubEdTCmc5UmdnSFNpUHNtTWFMZVVhQ0doOHgvSGVVakVoU3ROVkxjWkRDOUlHc21UaHlBcjUrTjdYaG1ZbjRIVWxCbDAKZWF5ZzA2VTlWc3NRaU5HNDRvTkRrRXc1TVFIRDNwQUVnTWJrSWJYVEcvbVBhNDhycWRBcnBXSDRvR002OThLZApQTTRsWHVkTHF4SWN1N2JrclpnTGpVbFMzSnVXMVhqVTRNRlVSK2RhSnoveGVCSmlBekNFaHBaalVyOXdPcXFUClRYcGtBMFBvWnd0cFA5MER6emF6SkJiZWs4N3BSaGdsS3VZNmJuZ1hCVjlZZklNUERZSVJNMnhJV01QRzU0ekwKVUNjMEpaOVV6WExyc1QwcFBpWURsaVJIZEtUTGtNRk5LQTZEU3lIVFlVelFybkhudHZJZG5mOXo2UXpxRVBoWQpxWTJ6eXdQaEdlOVZPSXowNWxHdkU5WUxncEJDY1VxZHlJMGZpZzF6bFdrYXBGdFB5MHBBY2NJNXZyUmFXSTAyCm93cGo0eDFCVkJ6bWp0eFczSzdUa0U4PQotLS0tLUVORCBDRVJUSUZJQ0FURS0tLS0tCg== -kind: Secret -metadata: - name: custom-ca-cert - namespace: openmcp-system diff --git a/test/e2e/platform/providerconfig.yaml b/test/e2e/platform/providerconfig.yaml index 8c82abe..384490c 100644 --- a/test/e2e/platform/providerconfig.yaml +++ b/test/e2e/platform/providerconfig.yaml @@ -4,7 +4,6 @@ metadata: name: flux spec: pollInterval: "10s" - certSecretRef: "custom-ca-cert" versions: - version: "2.8.3" chartVersion: "2.18.2" From ca43196265c1917ac2c6734e2989012ceb0296e5 Mon Sep 17 00:00:00 2001 From: "Nico Andres (SAP)" Date: Mon, 8 Jun 2026 18:58:29 +0200 Subject: [PATCH 09/20] Add support for custom ca On-behalf-of: @SAP nico.andres@sap.com Signed-off-by: Nico Andres (SAP) --- README.md | 26 +- ...open-control-plane.io_providerconfigs.yaml | 25 ++ api/v1alpha1/providerconfig_types.go | 6 + api/v1alpha1/zz_generated.deepcopy.go | 6 + docs/configuration/image-localization.md | 58 ++++ internal/controller/flux_controller.go | 33 +- internal/controller/flux_controller_test.go | 18 +- pkg/flux/configmap.go | 163 ++++++++++ pkg/flux/configmap_test.go | 229 ++++++++++++++ pkg/flux/helm.go | 186 +++++++++++ pkg/flux/helm_test.go | 291 ++++++++++++++++++ pkg/flux/secret_test.go | 35 --- pkg/flux/testutils_test.go | 56 ++++ test/e2e/flux_test.go | 161 +++++++++- test/e2e/platform/caconfigmap.yaml | 33 ++ test/e2e/platform/providerconfig.yaml | 3 + 16 files changed, 1282 insertions(+), 47 deletions(-) create mode 100644 pkg/flux/configmap.go create mode 100644 pkg/flux/configmap_test.go create mode 100644 pkg/flux/testutils_test.go create mode 100644 test/e2e/platform/caconfigmap.yaml diff --git a/README.md b/README.md index 2de0681..b62405b 100644 --- a/README.md +++ b/README.md @@ -37,6 +37,7 @@ flowchart LR subgraph FS[flux-system namespace] fluxctrl[Flux Controllers] pullsecret([image-pull-secret]) + caconfigmap([custom-ca-configmap]) end end @@ -45,6 +46,7 @@ flowchart LR spflux -- creates --> helmrel helmrel -- installs --> fluxctrl spflux -- copies secrets --> pullsecret + spflux -- copies configmap --> caconfigmap mcpapi -- represents --> mcp ``` @@ -146,6 +148,10 @@ metadata: spec: # Optional: Reconciliation interval pollInterval: "5m" + # Optional: ConfigMapKeySelector for a custom ca bundle (configmap will be copied to ManagedControlPlane) + caBundleRef: + name: "custom-ca-bundle" + key: "ca-bundle.crt" # The Flux versions that can be installed versions: - version: "2.8.3" @@ -170,13 +176,21 @@ spec: tag: v1.8.1 ``` -| Field | Type | Description | -| ------------------- | -------- | ------------------------------------------------ | -| `spec.pollInterval` | duration | How often to reconcile resources (default: 1m) | -| `certSecretRef` | object | SecretRef for chart registry trust establishment | -| `spec.versions` | array | The versions of Flux that can be installed | +| Field | Type | Description | +| ------------------- | -------------------- | ------------------------------------------------------------------ | +| `spec.pollInterval` | duration | How often to reconcile resources (default: 1m) | +| `spec.certSecretRef`| object | SecretRef for chart registry trust establishment | +| `spec.caBundleRef` | ConfigMapKeySelector | A configmap with a ca bundle used by Flux to verify certificates | +| `spec.versions` | array | The versions of Flux that can be installed | -A version item is defined as follows: +A **caBundleref** is defined as follows: + +| Field | Type | Description | +|-------|------|-------------| +| `name` | string | The name of the configmap which holds the ca bundle | +| `key` | string | The key in the configmap under which the ca bundle is stored | + +A **version** item is defined as follows: | Field | Type | Description | | ----------------- | ------ | --------------------------------------------- | diff --git a/api/crds/manifests/flux.services.open-control-plane.io_providerconfigs.yaml b/api/crds/manifests/flux.services.open-control-plane.io_providerconfigs.yaml index a228b44..ce91c6e 100644 --- a/api/crds/manifests/flux.services.open-control-plane.io_providerconfigs.yaml +++ b/api/crds/manifests/flux.services.open-control-plane.io_providerconfigs.yaml @@ -41,6 +41,31 @@ spec: spec: description: spec defines the desired state of ProviderConfig properties: + caBundleRef: + description: |- + CABundleRef is a reference to a config map containing a certificate bundle. + It will be installed on the ManagedControlPlane and configured for the Flux controllers. + properties: + key: + description: The key to select. + type: string + 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 + optional: + description: Specify whether the ConfigMap or its key must be + defined + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic pollInterval: default: 1m description: PollInterval determines how often to reconcile resources diff --git a/api/v1alpha1/providerconfig_types.go b/api/v1alpha1/providerconfig_types.go index f9c1d4e..1906792 100644 --- a/api/v1alpha1/providerconfig_types.go +++ b/api/v1alpha1/providerconfig_types.go @@ -19,6 +19,7 @@ package v1alpha1 import ( "time" + corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -35,6 +36,11 @@ type ProviderConfigSpec struct { // +kubebuilder:default:="1m" // +kubebuilder:validation:Format=duration PollInterval *metav1.Duration `json:"pollInterval,omitempty"` + + // CABundleRef is a reference to a config map containing a certificate bundle. + // It will be installed on the ManagedControlPlane and configured for the Flux controllers. + // +kubebuilder:validation:Optional + CaBundleRef *corev1.ConfigMapKeySelector `json:"caBundleRef,omitempty"` } // FluxVersion defines a version of Flux that can be installed diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index cadbb0b..18945de 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -5,6 +5,7 @@ package v1alpha1 import ( + corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -228,6 +229,11 @@ func (in *ProviderConfigSpec) DeepCopyInto(out *ProviderConfigSpec) { *out = new(v1.Duration) **out = **in } + if in.CaBundleRef != nil { + in, out := &in.CaBundleRef, &out.CaBundleRef + *out = new(corev1.ConfigMapKeySelector) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ProviderConfigSpec. diff --git a/docs/configuration/image-localization.md b/docs/configuration/image-localization.md index ee1759b..26b51bb 100644 --- a/docs/configuration/image-localization.md +++ b/docs/configuration/image-localization.md @@ -15,6 +15,7 @@ The Flux service provider handles this through: - **`chartPullSecret`**: Credentials for pulling the Helm chart from a private OCI registry - **`values.imagePullSecrets`**: Credentials for pulling Flux controller images (specified in Helm values) - **`values`**: Custom Helm values for image location overrides +- **`caBundleRef`**: PEM-encoded custom ca bundle if your private OCI registry uses self-signed certificates ## Secret Flow @@ -24,6 +25,7 @@ flowchart TB subgraph PC[Platform Cluster] subgraph SPN[Service Provider Namespace] chartsecret([chart-pull-secret]) + caconfigmap([custom-ca-configmap]) imgsecret1([image-pull-secret-1]) imgsecret2([image-pull-secret-2]) end @@ -40,15 +42,18 @@ flowchart TB subgraph FS[flux-system namespace] imgcopy1([image-pull-secret-1]) imgcopy2([image-pull-secret-2]) + caconfigmapcopy([custom-ca-configmap copy]) fluxctrl[Flux Controllers] fluxctrl -. uses .-> imgcopy1 fluxctrl -. uses .-> imgcopy2 + fluxctrl -. uses .-> caconfigmapcopy end end chartsecret -- copied to --> chartsecretcopy imgsecret1 -- copied to --> imgcopy1 imgsecret2 -- copied to --> imgcopy2 + caconfigmap -- copied to --> caconfigmapcopy helmrel -- installs --> fluxctrl ``` @@ -62,6 +67,14 @@ kind: ProviderConfig metadata: name: flux-provider-config spec: + # ConfigMapKeySelector pointing to a configmap which holds a PEM-encoded custom CA bundle. + # Must exist in the service provider's namespace on the platform cluster + # The configmap will be automatically copied from the service provider's namespace + # to the flux-system namespace on the ManagedControlPlane and configured + # for the flux-controllers + caBundleRef: + name: "custom-ca-bundle" + key: "ca-bundle.crt" versions: - version: "2.8.3" chartVersion: "2.18.2" @@ -112,6 +125,33 @@ kubectl create secret docker-registry image-registry-credentials \ --docker-password= ``` +### Creating Custom CA ConfigMap + +Concatenate all your custom CA certificates into a single PEM file. Each certificate must use the standard PEM format. + +```shell +cat /path/to/ca1.crt /path/to/ca2.crt > ca-bundle.crt +``` + +The resulting file should look like this: + +```text title="ca-bundle.crt" +-----BEGIN CERTIFICATE----- +MIIDXTCCAkWgAwIBAgIJAMSO... +-----END CERTIFICATE----- +-----BEGIN CERTIFICATE----- +MIIDXTCCAkWgAwIBAgIJANPQ... +-----END CERTIFICATE----- +``` + +Create the configmap + +```shell +kubectl create configmap custom-ca-bundle \ + --from-file=ca-bundle.crt=ca-bundle.crt \ + --namespace=openmcp-system +``` + ## How It Works ### Chart Pull Secret @@ -126,6 +166,13 @@ kubectl create secret docker-registry image-registry-credentials \ 2. These secrets are copied from the service provider's namespace on the platform cluster to `flux-system` on the ManagedControlPlane 3. The Helm values are passed through to Flux, which configures the controller pods with these secrets +### Custom CA Bundle +1. The configmap specified in `caBundleRef` is copied from the service provider's namespace on the platform cluster to `flux-system` on the ManagedControlPlane +2. For each Flux controller, the Helm values are adjusted so that it mounts the provided `caBundleRef.key` and sets the `SSL_CERT_DIR` environment variable to add the bundle to the pool of known certificates +3. The Helm values are passed through to Flux, and each Flux controller is able to verify certificates signed by the provided custom CA + +[!CAUTION] The custom CA certificate is not propagated to the OpenControlPlane cluster nodes. If you want to pull images from the same OCI registry you must add the custom CA certificate to the cluster nodes yourself. + ## Complete Example ### Air-Gapped Setup @@ -138,6 +185,9 @@ metadata: spec: chartUrl: "oci://harbor.corp.internal/charts/flux2" chartPullSecret: "harbor-credentials" + caBundleRef: + name: "harbor-ca-bundle" + key: "harbor-ca-bundle.crt" values: # Image pull secrets - will be copied to ManagedControlPlane imagePullSecrets: @@ -192,6 +242,14 @@ kubectl get secrets -n mcp-- | grep -E "chart|image" kubectl get secrets -n flux-system | grep -E "image" ``` +### Check ConfigMap Copying +Verify configmaps are copied to the correct namespaces: + +```bash +# ManagedControlPlane - flux-system namespace +kubectl get cm -n flux-system | grep -E "" +``` + ### Check OCIRepository Secret Reference ```bash diff --git a/internal/controller/flux_controller.go b/internal/controller/flux_controller.go index 7e1b1d8..5005cae 100644 --- a/internal/controller/flux_controller.go +++ b/internal/controller/flux_controller.go @@ -42,7 +42,7 @@ import ( const conditionReasonError = "ReconcileError" // ErrManagedResources is an end-user facing error if errors are present inside Flux.Status.ManagedResources -var ErrManagedResources error = errors.New("resources contain reconcile errors") +var ErrManagedResources = errors.New("resources contain reconcile errors") // FluxReconciler reconciles a Flux object type FluxReconciler struct { @@ -117,6 +117,9 @@ func userErrorMessage(err error) string { if errors.Is(err, flux.ErrSecretCleanup) { errorMessages = append(errorMessages, flux.ErrSecretCleanup.Error()) } + if errors.Is(err, flux.ErrConfigMapCleanup) { + errorMessages = append(errorMessages, flux.ErrConfigMapCleanup.Error()) + } return strings.Join(errorMessages, "; ") } @@ -172,6 +175,22 @@ func (r *FluxReconciler) createObjectManager(obj *apiv1alpha1.Flux, pc *apiv1alp }) } + if pc.Spec.CaBundleRef != nil { + // add custom ca volume and volumeMount to helm values + fluxVersion.Values, err = flux.AddCaToHelmValues(fluxVersion.Values, pc.Spec.CaBundleRef) + if err != nil { + return nil, fmt.Errorf("failed to add ca volume to helm values: %w", err) + } + + // Sync ca configmap from platform cluster to MCP + flux.ManageCaConfigMap(mcpCluster, pc.Spec.CaBundleRef.LocalObjectReference, flux.ConfigMapCopyConfig{ + SourceClient: r.PlatformCluster.Client(), + SourceNamespace: r.PodNamespace, + TargetNamespace: fluxNamespace, + }) + + } + // Configure Flux resources (OCIRepository and HelmRelease) flux.ManageFluxResources(flux.ManageFluxResourcesParams{ Cluster: platformCluster, @@ -194,10 +213,18 @@ func (r *FluxReconciler) createObjectManager(obj *apiv1alpha1.Flux, pc *apiv1alp Name: prefixedChartPullSecret, }, }) - controlPlaneCleaner := flux.NewSecretCleaner(mcpCluster, fluxNamespace, helmValues.ImagePullSecrets) + controlPlaneSecretCleaner := flux.NewSecretCleaner(mcpCluster, fluxNamespace, helmValues.ImagePullSecrets) + + configMapsToKeep := []corev1.LocalObjectReference{} + if pc.Spec.CaBundleRef != nil { + configMapsToKeep = append(configMapsToKeep, pc.Spec.CaBundleRef.LocalObjectReference) + } + + controlPlaneConfigMapCleaner := flux.NewConfigMapCleaner(mcpCluster, fluxNamespace, configMapsToKeep) mgr.AddCleaner(platformCleaner) - mgr.AddCleaner(controlPlaneCleaner) + mgr.AddCleaner(controlPlaneSecretCleaner) + mgr.AddCleaner(controlPlaneConfigMapCleaner) return mgr, nil } diff --git a/internal/controller/flux_controller_test.go b/internal/controller/flux_controller_test.go index 695b021..8626725 100644 --- a/internal/controller/flux_controller_test.go +++ b/internal/controller/flux_controller_test.go @@ -296,19 +296,33 @@ func Test_updateStatusError(t *testing.T) { wantMessage: ErrManagedResources.Error(), }, { - name: "cleanup error", + name: "secret cleanup error", obj: &apiv1alpha1.Flux{}, resourceErrors: false, err: flux.ErrSecretCleanup, wantMessage: flux.ErrSecretCleanup.Error(), }, { - name: "combined resource and cleanup error", + name: "combined resource and secret cleanup error", obj: &apiv1alpha1.Flux{}, resourceErrors: true, err: flux.ErrSecretCleanup, wantMessage: fmt.Sprintf("%s; %s", ErrManagedResources.Error(), flux.ErrSecretCleanup.Error()), }, + { + name: "configmap cleanup error", + obj: &apiv1alpha1.Flux{}, + resourceErrors: false, + err: flux.ErrConfigMapCleanup, + wantMessage: flux.ErrConfigMapCleanup.Error(), + }, + { + name: "combined resource and configmap cleanup error", + obj: &apiv1alpha1.Flux{}, + resourceErrors: true, + err: flux.ErrConfigMapCleanup, + wantMessage: fmt.Sprintf("%s; %s", ErrManagedResources.Error(), flux.ErrConfigMapCleanup.Error()), + }, { name: "resource error and no end-user error", obj: &apiv1alpha1.Flux{}, diff --git a/pkg/flux/configmap.go b/pkg/flux/configmap.go new file mode 100644 index 0000000..5f7460f --- /dev/null +++ b/pkg/flux/configmap.go @@ -0,0 +1,163 @@ +// Copyright 2025. +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package flux + +import ( + "context" + "errors" + "fmt" + "slices" + + openmcpresources "github.com/openmcp-project/controller-utils/pkg/resources" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" + + apiv1alpha1 "github.com/openmcp-project/service-provider-flux/api/v1alpha1" +) + +// ErrConfigMapCleanup is an user-facing error that indicates configmap cleanup failures +var ErrConfigMapCleanup = errors.New("configmap cleanup failed") + +// ConfigMapCopyConfig holds the configuration for copying configmap. +type ConfigMapCopyConfig struct { + // SourceClient is the client to read the source configmap from. + SourceClient client.Client + // SourceNamespace is the namespace of the source configmap. + SourceNamespace string + // TargetNamespace is the namespace of the target configmap. + TargetNamespace string + // TargetName is an optional value to adjust the name of the target configmap + // instead of using the source configmap name. + TargetName string +} + +// ManageCaConfigMap syncs the ca configmap to the target cluster. +func ManageCaConfigMap(targetCluster ManagedCluster, caConfigMap corev1.LocalObjectReference, config ConfigMapCopyConfig) { + caConfigMapName := caConfigMap.Name + if config.TargetName != "" { + caConfigMapName = config.TargetName + } + configMap := NewManagedObject(&corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: caConfigMapName, + Namespace: config.TargetNamespace, + }, + }, ManagedObjectContext{ + ReconcileFunc: func(ctx context.Context, o client.Object) error { + oConfigMap, ok := o.(*corev1.ConfigMap) + if !ok { + return fmt.Errorf("expected *corev1.ConfigMap, got %T", o) + } + sourceConfigMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: caConfigMap.Name, + Namespace: config.SourceNamespace, + }, + } + // retrieve source configmap from platform cluster + if err := config.SourceClient.Get(ctx, client.ObjectKeyFromObject(sourceConfigMap), sourceConfigMap); err != nil { + return err + } + mutator := openmcpresources.NewConfigMapMutator(caConfigMapName, config.TargetNamespace, sourceConfigMap.Data) + return mutator.Mutate(oConfigMap) + }, + StatusFunc: SimpleStatus, + }) + targetCluster.AddObject(configMap) +} + +// ConfigMapStatus returns the status of a configmap object. +func ConfigMapStatus(o client.Object, rl apiv1alpha1.ResourceLocation) Status { + if !o.GetDeletionTimestamp().IsZero() { + return Status{ + Phase: apiv1alpha1.Terminating, + Message: "ConfigMap is terminating.", + Location: rl, + } + } + if o.GetUID() == "" { + return Status{ + Phase: apiv1alpha1.Pending, + Message: "ConfigMap has not been created yet.", + Location: rl, + } + } + return Status{ + Phase: apiv1alpha1.Ready, + Message: "ConfigMap exists.", + Location: rl, + } +} + +var _ OrphanCleaner = &configMapCleaner{} + +type configMapCleaner struct { + cluster ManagedCluster + namespace string + configMapsToKeep []corev1.LocalObjectReference +} + +// NewConfigMapCleaner removes redundant configmaps in the given target namespace +// by removing any configmap labeled as managed by sp-flux that is not in configMapsToKeep. +func NewConfigMapCleaner(cluster ManagedCluster, namespace string, configMapsToKeep []corev1.LocalObjectReference) OrphanCleaner { + return &configMapCleaner{ + cluster: cluster, + namespace: namespace, + configMapsToKeep: configMapsToKeep, + } +} + +func (c *configMapCleaner) Cleanup(ctx context.Context) ([]Result, error) { + results := []Result{} + configMapCopies := &corev1.ConfigMapList{} + cl := c.cluster.GetClient() + if err := cl.List(ctx, configMapCopies, + client.InNamespace(c.namespace), + client.MatchingLabels{LabelManagedBy: labelServiceProviderFlux}, + ); err != nil { + log.FromContext(ctx).Error(err, "failed to list configmap for orphan cleanup") + return nil, ErrConfigMapCleanup + } + for _, configMap := range configMapCopies.Items { + if !slices.ContainsFunc(c.configMapsToKeep, func(ref corev1.LocalObjectReference) bool { return configMap.Name == ref.Name }) { + if err := cl.Delete(ctx, &configMap); client.IgnoreNotFound(err) != nil { + results = append(results, c.cleanupErrorResult(&configMap, err)) + } + } + } + return results, nil +} + +func (c *configMapCleaner) cleanupErrorResult(obj *corev1.ConfigMap, err error) Result { + return Result{ + Object: &managedObject{ + object: obj, + statusFunc: cleanupErrorStatusConfigMap, + deletionPolicy: Delete, + }, + Cluster: c.cluster, + OperationResult: OperationResultDeletionFailed, + Error: err, + } +} + +func cleanupErrorStatusConfigMap(_ client.Object, rl apiv1alpha1.ResourceLocation) Status { + return Status{ + Phase: apiv1alpha1.Terminating, + Message: "ConfigMap cleanup failed", + Location: rl, + } +} diff --git a/pkg/flux/configmap_test.go b/pkg/flux/configmap_test.go new file mode 100644 index 0000000..a115b8c --- /dev/null +++ b/pkg/flux/configmap_test.go @@ -0,0 +1,229 @@ +// Copyright 2025. +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package flux + +import ( + "context" + "errors" + "slices" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/rest" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/openmcp-project/service-provider-flux/pkg/testutils" +) + +func TestManageCaConfigMap(t *testing.T) { + sourceConfigMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "custom-ca", + Namespace: "source-ns", + }, + Data: map[string]string{ + "ca.crt": "CERTDATA", + }, + } + fakeCluster := testutils.CreateFakeCluster(t, "platform", sourceConfigMap) + + tests := []struct { + name string + targetCluster ManagedCluster + caConfigMap corev1.LocalObjectReference + config ConfigMapCopyConfig + }{ + { + name: "syncs configmap", + targetCluster: NewManagedCluster(fakeCluster, &rest.Config{}, "target-ns", ManagedControlPlane), + caConfigMap: corev1.LocalObjectReference{Name: "custom-ca"}, + config: ConfigMapCopyConfig{ + SourceClient: fakeCluster.Client(), + SourceNamespace: "source-ns", + TargetNamespace: "target-ns", + }, + }, + { + name: "syncs configmap with target name adjustment", + targetCluster: NewManagedCluster(fakeCluster, &rest.Config{}, "target-ns", ManagedControlPlane), + caConfigMap: corev1.LocalObjectReference{Name: "custom-ca"}, + config: ConfigMapCopyConfig{ + SourceClient: fakeCluster.Client(), + SourceNamespace: "source-ns", + TargetNamespace: "target-ns", + TargetName: "sp-flux-custom-ca", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ManageCaConfigMap(tt.targetCluster, tt.caConfigMap, tt.config) + + mgr := NewManager() + mgr.AddCluster(tt.targetCluster) + results, gotErr := mgr.Apply(context.Background()) + require.NoError(t, gotErr) + for _, r := range results { + require.NoError(t, r.Error) + } + + targetConfigMap := &corev1.ConfigMap{} + targetName := tt.caConfigMap.Name + if tt.config.TargetName != "" { + targetName = tt.config.TargetName + } + err := fakeCluster.Client().Get(context.Background(), client.ObjectKey{ + Name: targetName, + Namespace: tt.config.TargetNamespace, + }, targetConfigMap) + require.NoError(t, err) + assert.Equal(t, sourceConfigMap.Data, targetConfigMap.Data) + }) + } +} + +func Test_configMapCleaner_Cleanup(t *testing.T) { + tests := []struct { + name string + cluster ManagedCluster + targetNamespace string + configMapsToKeep []corev1.LocalObjectReference + want []corev1.ConfigMap + wantResults bool + wantErr bool + }{ + { + name: "only managed configmaps are deleted", + targetNamespace: "flux-system", + cluster: createFakeCluster(createFakeClient([]client.Object{ + testConfigMap("a", "flux-system", true), + testConfigMap("b", "flux-system", false), + })), + configMapsToKeep: []corev1.LocalObjectReference{}, + want: []corev1.ConfigMap{ + *testConfigMap("b", "flux-system", false), + }, + wantErr: false, + }, + { + name: "configmaps in other namespaces are not deleted", + targetNamespace: "openmcp-system", + cluster: createFakeCluster(createFakeClient([]client.Object{ + testConfigMap("a", "flux-system", true), + testConfigMap("b", "flux-system", false), + })), + configMapsToKeep: []corev1.LocalObjectReference{}, + want: []corev1.ConfigMap{ + *testConfigMap("a", "flux-system", true), + *testConfigMap("b", "flux-system", false), + }, + wantErr: false, + }, + { + name: "configmaps to keep are not deleted", + targetNamespace: "flux-system", + cluster: createFakeCluster(createFakeClient([]client.Object{ + testConfigMap("a", "flux-system", true), + testConfigMap("b", "flux-system", false), + })), + configMapsToKeep: []corev1.LocalObjectReference{ + { + Name: "a", + }, + }, + want: []corev1.ConfigMap{ + *testConfigMap("a", "flux-system", true), + *testConfigMap("b", "flux-system", false), + }, + wantErr: false, + }, + { + name: "error is returned when list fails", + cluster: createFakeCluster(listErrorClient{}), + targetNamespace: "flux-system", + configMapsToKeep: []corev1.LocalObjectReference{}, + want: []corev1.ConfigMap{}, + wantErr: true, + }, + { + name: "error is returned when delete fails", + cluster: createFakeCluster(deleteErrorConfigMapClient{ + fakeConfigMap: *testConfigMap("a", "flux-system", true), + }), + targetNamespace: "flux-system", + configMapsToKeep: []corev1.LocalObjectReference{}, + want: []corev1.ConfigMap{}, + wantErr: false, + wantResults: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := NewConfigMapCleaner(tt.cluster, tt.targetNamespace, tt.configMapsToKeep) + results, gotErr := c.Cleanup(context.Background()) + if gotErr != nil { + if !tt.wantErr { + t.Errorf("Cleanup() failed: %v", gotErr) + } + return + } + if len(results) > 0 { + if !tt.wantResults { + t.Errorf("Cleanup() failed %v", results) + } + return + } + configMapList := &corev1.ConfigMapList{} + require.NoError(t, tt.cluster.GetClient().List(context.Background(), configMapList)) + for _, gotConfigMap := range configMapList.Items { + assert.True(t, slices.ContainsFunc(tt.want, func(cm corev1.ConfigMap) bool { + return cm.Name == gotConfigMap.Name && cm.Namespace == gotConfigMap.Namespace + })) + } + }) + } +} + +func testConfigMap(name, namespace string, managedByFlux bool) *corev1.ConfigMap { + labels := map[string]string{} + if managedByFlux { + labels[LabelManagedBy] = labelServiceProviderFlux + } + return &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Labels: labels, + }, + } +} + +type deleteErrorConfigMapClient struct { + client.Client + fakeConfigMap corev1.ConfigMap +} + +func (d deleteErrorConfigMapClient) List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error { + configMapList := list.(*corev1.ConfigMapList) + configMapList.Items = []corev1.ConfigMap{d.fakeConfigMap} + return nil +} + +func (d deleteErrorConfigMapClient) Delete(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error { + return errors.New("delete failed") +} diff --git a/pkg/flux/helm.go b/pkg/flux/helm.go index e9a9e86..034990d 100644 --- a/pkg/flux/helm.go +++ b/pkg/flux/helm.go @@ -15,11 +15,35 @@ package flux import ( "encoding/json" + "errors" + "fmt" + "strings" corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" ) +const ( + customCaVolumeName = "custom-ca-bundle" + customCaPath = "/etc/open-control-plane/custom-ca" +) + +// from x509 go lib (https://github.com/golang/go/blob/015343854b5d9e2829481df30dbcae2ca6682d25/src/crypto/x509/root_linux.go) +var certDirectories = []string{ + "/etc/ssl/certs", + "/etc/pki/tls/certs", +} + +var fluxControllers = []string{ + "helmController", + "imageAutomationController", + "imageReflectionController", + "kustomizeController", + "notificationController", + "sourceController", + "sourceWatcher", +} + // HelmValues defines the Helm values that are explicitly processed during reconciliation. // The Flux Helm chart uses imagePullSecrets at the top level, not under global. type HelmValues struct { @@ -44,3 +68,165 @@ func ExtractHelmValues(values *apiextensionsv1.JSON) (*HelmValues, error) { return vals, nil } + +// AddCaToHelmValues removes conflicting volumes, volumeMounts and envVars (matching by name and/or mountPath) and +// adds a volume, volumeMount and envVar on all Flux controller helm values sections to import the custom CA certificate. +func AddCaToHelmValues(values *apiextensionsv1.JSON, configMap *corev1.ConfigMapKeySelector) (*apiextensionsv1.JSON, error) { + if configMap == nil { + return nil, errors.New("cannot add custom CA to Helm values: ConfigMapKeySelector is nil") + } + + var root = map[string]json.RawMessage{} + + caVolume := corev1.Volume{ + Name: customCaVolumeName, + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: configMap.LocalObjectReference, + Items: []corev1.KeyToPath{ + { + Key: configMap.Key, + Path: configMap.Key, + }, + }, + }, + }, + } + + caVolumeMount := corev1.VolumeMount{ + Name: customCaVolumeName, + ReadOnly: true, + MountPath: customCaPath, + } + + caEnvVar := corev1.EnvVar{ + Name: "SSL_CERT_DIR", + Value: strings.Join(append(certDirectories, customCaPath), ":"), + } + + if values != nil && len(values.Raw) > 0 { + if err := json.Unmarshal(values.Raw, &root); err != nil { + return nil, fmt.Errorf("failed to unmarshal helm values: %w", err) + } + } + + for _, controller := range fluxControllers { + if err := addCaToController(root, controller, caVolume, caVolumeMount, caEnvVar); err != nil { + return nil, err + } + } + + out, err := json.Marshal(root) + if err != nil { + return nil, fmt.Errorf("failed to marshal helm values: %w", err) + } + + return &apiextensionsv1.JSON{Raw: out}, nil +} + +func addCaToController( + root map[string]json.RawMessage, + controller string, + caVolume corev1.Volume, + caVolumeMount corev1.VolumeMount, + caEnvVar corev1.EnvVar, +) error { + var controllerValues map[string]json.RawMessage + var volumes []corev1.Volume + var volumeMounts []corev1.VolumeMount + var envVars []corev1.EnvVar + + if err := unmarshalIfPresent(root, controller, &controllerValues); err != nil { + return fmt.Errorf("failed to unmarshal %s: %w", controller, err) + } + if controllerValues == nil { + controllerValues = map[string]json.RawMessage{} + } + + if err := unmarshalIfPresent(controllerValues, "volumes", &volumes); err != nil { + return fmt.Errorf("failed to unmarshal %s.volumes: %w", controller, err) + } + + if err := unmarshalIfPresent(controllerValues, "volumeMounts", &volumeMounts); err != nil { + return fmt.Errorf("failed to unmarshal %s.volumeMounts: %w", controller, err) + } + + if err := unmarshalIfPresent(controllerValues, "extraEnv", &envVars); err != nil { + return fmt.Errorf("failed to unmarshal %s.extraEnv: %w", controller, err) + } + + volumes = removeConflictingVolumesAndAppend(volumes, caVolume) + volumeMounts = removeConflictingVolumeMountsAndAppend(volumeMounts, caVolumeMount) + envVars = removeConflictingEnvVarsAndAppend(envVars, caEnvVar) + + volumesRaw, err := json.Marshal(volumes) + if err != nil { + return fmt.Errorf("failed to marshal %s.volumes: %w", controller, err) + } + + volumeMountsRaw, err := json.Marshal(volumeMounts) + if err != nil { + return fmt.Errorf("failed to marshal %s.volumeMounts: %w", controller, err) + } + + envVarsRaw, err := json.Marshal(envVars) + if err != nil { + return fmt.Errorf("failed to marshal %s.extraEnv: %w", controller, err) + } + + controllerValues["volumes"] = volumesRaw + controllerValues["volumeMounts"] = volumeMountsRaw + controllerValues["extraEnv"] = envVarsRaw + + controllerRaw, err := json.Marshal(controllerValues) + if err != nil { + return fmt.Errorf("failed to marshal %s: %w", controller, err) + } + + root[controller] = controllerRaw + return nil +} + +func removeConflictingVolumesAndAppend(volumes []corev1.Volume, caVolume corev1.Volume) []corev1.Volume { + updated := []corev1.Volume{} + for _, volume := range volumes { + if volume.Name != caVolume.Name { + updated = append(updated, volume) + } + } + updated = append(updated, caVolume) + return updated +} + +func removeConflictingVolumeMountsAndAppend(volumeMounts []corev1.VolumeMount, caVolumeMount corev1.VolumeMount) []corev1.VolumeMount { + updated := []corev1.VolumeMount{} + for _, volumeMount := range volumeMounts { + if volumeMount.MountPath != caVolumeMount.MountPath && volumeMount.Name != caVolumeMount.Name { + updated = append(updated, volumeMount) + } + } + updated = append(updated, caVolumeMount) + return updated +} + +func removeConflictingEnvVarsAndAppend(envVars []corev1.EnvVar, caEnvVar corev1.EnvVar) []corev1.EnvVar { + updated := []corev1.EnvVar{} + for _, envVar := range envVars { + if envVar.Name != caEnvVar.Name { + updated = append(updated, envVar) + } + } + updated = append(updated, caEnvVar) + return updated +} + +func unmarshalIfPresent(obj map[string]json.RawMessage, key string, out any) error { + raw, ok := obj[key] + if !ok || len(raw) == 0 { + return nil + } + if err := json.Unmarshal(raw, out); err != nil { + return fmt.Errorf("invalid %s JSON: %w", key, err) + } + return nil +} diff --git a/pkg/flux/helm_test.go b/pkg/flux/helm_test.go index a91dca7..423bdd0 100644 --- a/pkg/flux/helm_test.go +++ b/pkg/flux/helm_test.go @@ -15,10 +15,12 @@ package flux import ( "encoding/json" + "strings" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" ) @@ -130,6 +132,295 @@ func TestExtractHelmValues(t *testing.T) { } } +func TestAddCaToHelmValues(t *testing.T) { + caBundleRef := &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "custom-ca-configmap"}, + Key: "ca.crt", + } + + expectedCaVolume := corev1.Volume{ + Name: customCaVolumeName, + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{Name: "custom-ca-configmap"}, + Items: []corev1.KeyToPath{{ + Key: "ca.crt", + Path: "ca.crt", + }}, + }, + }, + } + expectedCaVolumeMount := corev1.VolumeMount{ + Name: customCaVolumeName, + ReadOnly: true, + MountPath: customCaPath, + } + expectedCaEnvVar := corev1.EnvVar{ + Name: "SSL_CERT_DIR", + Value: strings.Join(append(certDirectories, customCaPath), ":"), + } + + tests := []struct { + name string + values *apiextensionsv1.JSON + wantErr string + checkValue func(t *testing.T, out *apiextensionsv1.JSON) + }{ + { + name: "Adds controller volumes, volumeMounts and extraEnv when no helm values are set", + values: nil, + checkValue: func(t *testing.T, out *apiextensionsv1.JSON) { + require.NotNil(t, out) + + expected := buildHelmValues(t, + withAllControllerVolumes(expectedCaVolume), + withAllControllerVolumeMounts(expectedCaVolumeMount), + withAllControllerExtraEnv(expectedCaEnvVar), + ) + + assert.JSONEq(t, string(expected.Raw), string(out.Raw)) + }, + }, + { + name: "Preserves existing helm values and adds CA entries", + values: buildHelmValues(t, + withRootField("namespace", "other-namespace"), + withAllControllerField( + "resources", map[string]any{ + "limits": map[string]any{ + "memory": "256Mi", + }, + }), + withAllControllerVolumes(corev1.Volume{Name: "existing-volume"}), + withAllControllerVolumeMounts( + corev1.VolumeMount{Name: "existing-volume", MountPath: "/tmp/existing"}), + ), + checkValue: func(t *testing.T, out *apiextensionsv1.JSON) { + require.NotNil(t, out) + + expected := buildHelmValues(t, + withRootField("namespace", "other-namespace"), + withAllControllerField("resources", map[string]any{ + "limits": map[string]any{"memory": "256Mi"}, + }), + withAllControllerVolumes(expectedCaVolume), + withAllControllerVolumeMounts(expectedCaVolumeMount), + withAllControllerExtraEnv(expectedCaEnvVar), + withAllControllerVolumes( + corev1.Volume{Name: "existing-volume"}, + expectedCaVolume, + ), + withAllControllerVolumeMounts( + corev1.VolumeMount{Name: "existing-volume", MountPath: "/tmp/existing"}, + expectedCaVolumeMount, + ), + ) + assert.JSONEq(t, string(expected.Raw), string(out.Raw)) + }, + }, + { + name: "Removes VolumeMounts with same name and/or same MountPath", + values: buildHelmValues(t, + withAllControllerVolumeMounts( + corev1.VolumeMount{Name: "volume1", MountPath: "/tmp/volume1"}, + corev1.VolumeMount{Name: "volume2", MountPath: customCaPath}, + corev1.VolumeMount{Name: customCaVolumeName, MountPath: "/tmp/existing"}, + corev1.VolumeMount{Name: customCaVolumeName, MountPath: customCaPath}, + ), + ), + checkValue: func(t *testing.T, out *apiextensionsv1.JSON) { + require.NotNil(t, out) + + expected := buildHelmValues(t, + withAllControllerVolumes(expectedCaVolume), + withAllControllerExtraEnv(expectedCaEnvVar), + withAllControllerVolumeMounts( + corev1.VolumeMount{Name: "volume1", MountPath: "/tmp/volume1"}, + expectedCaVolumeMount, + ), + ) + + assert.JSONEq(t, string(expected.Raw), string(out.Raw)) + }, + }, + { + name: "Removes Volumes with same name", + values: buildHelmValues(t, + withAllControllerVolumes( + corev1.Volume{Name: customCaVolumeName}, + corev1.Volume{Name: "volume1"}, + ), + ), + checkValue: func(t *testing.T, out *apiextensionsv1.JSON) { + require.NotNil(t, out) + + expected := buildHelmValues(t, + withAllControllerVolumes( + corev1.Volume{Name: "volume1"}, + expectedCaVolume, + ), + withAllControllerVolumeMounts(expectedCaVolumeMount), + withAllControllerExtraEnv(expectedCaEnvVar), + ) + assert.JSONEq(t, string(expected.Raw), string(out.Raw)) + }, + }, + { + name: "Removes EnvVars with same name", + values: buildHelmValues(t, + withAllControllerExtraEnv( + corev1.EnvVar{Name: "SSL_CERT_DIR"}, + corev1.EnvVar{Name: "ANOTHER_ENV_VAR"}, + ), + ), + checkValue: func(t *testing.T, out *apiextensionsv1.JSON) { + require.NotNil(t, out) + + expected := buildHelmValues(t, + withAllControllerExtraEnv( + corev1.EnvVar{Name: "ANOTHER_ENV_VAR"}, + expectedCaEnvVar, + ), + withAllControllerVolumes(expectedCaVolume), + withAllControllerVolumeMounts(expectedCaVolumeMount), + ) + assert.JSONEq(t, string(expected.Raw), string(out.Raw)) + }, + }, + { + name: "Returns error for invalid root json", + values: &apiextensionsv1.JSON{Raw: []byte("not-json")}, + wantErr: "failed to unmarshal helm values", + }, + { + name: "returns error for invalid controller json", + values: mustMarshalJSON(t, map[string]any{ + "helmController": "not-an-object", + }), + wantErr: "failed to unmarshal helmController", + }, + { + name: "returns error for invalid controller.volumes json", + values: mustMarshalJSON(t, map[string]any{ + "helmController": map[string]any{ + "volumes": "not-a-list", + }, + }), + wantErr: "failed to unmarshal helmController.volumes", + }, + { + name: "returns error for invalid controller.volumeMounts json", + values: mustMarshalJSON(t, map[string]any{ + "helmController": map[string]any{ + "volumeMounts": "not-a-list", + }, + }), + wantErr: "failed to unmarshal helmController.volumeMounts", + }, + { + name: "returns error for invalid controller.extraEnv json", + values: mustMarshalJSON(t, map[string]any{ + "helmController": map[string]any{ + "extraEnv": "not-a-list", + }, + }), + wantErr: "failed to unmarshal helmController.extraEnv", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + out, err := AddCaToHelmValues(tt.values, caBundleRef) + if tt.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + return + } + + require.NoError(t, err) + require.NotNil(t, out) + if tt.checkValue != nil { + tt.checkValue(t, out) + } + }) + } +} + +type helmValues struct { + root map[string]any + controllers map[string]map[string]any +} + +type helmValuesOption func(*helmValues) + +func buildHelmValues(t *testing.T, opts ...helmValuesOption) *apiextensionsv1.JSON { + t.Helper() + + builder := &helmValues{ + root: map[string]any{}, + controllers: map[string]map[string]any{}, + } + + for _, opt := range opts { + opt(builder) + } + + for controller, values := range builder.controllers { + if len(values) > 0 { + builder.root[controller] = values + } + } + + return mustMarshalJSON(t, builder.root) +} + +func withRootField(key string, value any) helmValuesOption { + return func(builder *helmValues) { + builder.root[key] = value + } +} + +func withAllControllerField(key string, value any) helmValuesOption { + return func(builder *helmValues) { + for _, controller := range fluxControllers { + withControllerField(controller, key, value)(builder) + } + } +} + +func withControllerField(controller string, key string, value any) helmValuesOption { + return func(builder *helmValues) { + if _, ok := builder.controllers[controller]; !ok { + builder.controllers[controller] = map[string]any{} + } + builder.controllers[controller][key] = value + } +} + +func withAllControllerVolumes(volumes ...corev1.Volume) helmValuesOption { + return func(builder *helmValues) { + for _, controller := range fluxControllers { + withControllerField(controller, "volumes", volumes)(builder) + } + } +} + +func withAllControllerVolumeMounts(volumeMounts ...corev1.VolumeMount) helmValuesOption { + return func(builder *helmValues) { + for _, controller := range fluxControllers { + withControllerField(controller, "volumeMounts", volumeMounts)(builder) + } + } +} + +func withAllControllerExtraEnv(envVars ...corev1.EnvVar) helmValuesOption { + return func(builder *helmValues) { + for _, controller := range fluxControllers { + withControllerField(controller, "extraEnv", envVars)(builder) + } + } +} + func mustMarshalJSON(t *testing.T, v any) *apiextensionsv1.JSON { t.Helper() raw, err := json.Marshal(v) diff --git a/pkg/flux/secret_test.go b/pkg/flux/secret_test.go index 4cba8a0..e8bca48 100644 --- a/pkg/flux/secret_test.go +++ b/pkg/flux/secret_test.go @@ -25,11 +25,8 @@ import ( "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - clientgoscheme "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/client/fake" "github.com/openmcp-project/service-provider-flux/pkg/testutils" ) @@ -233,24 +230,6 @@ func Test_secretCleaner_Cleanup(t *testing.T) { } } -var _ ManagedCluster = &fakeCluster{} - -type fakeCluster struct { - managedCluster - fakeClient client.Client -} - -// GetClient implements [ManagedCluster]. -func (f *fakeCluster) GetClient() client.Client { - return f.fakeClient -} - -func createFakeCluster(client client.Client) ManagedCluster { - return &fakeCluster{ - fakeClient: client, - } -} - func testSecret(name, namespace string, managedByFlux bool) *corev1.Secret { labels := map[string]string{} if managedByFlux { @@ -265,20 +244,6 @@ func testSecret(name, namespace string, managedByFlux bool) *corev1.Secret { } } -func createFakeClient(clusterObjects []client.Object) client.Client { - scheme := runtime.NewScheme() - _ = clientgoscheme.AddToScheme(scheme) - return fake.NewClientBuilder().WithObjects(clusterObjects...).WithScheme(scheme).Build() -} - -type listErrorClient struct { - client.Client -} - -func (l listErrorClient) List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error { - return errors.New("list failed") -} - type deleteErrorClient struct { client.Client fakeSecret corev1.Secret diff --git a/pkg/flux/testutils_test.go b/pkg/flux/testutils_test.go new file mode 100644 index 0000000..34981ad --- /dev/null +++ b/pkg/flux/testutils_test.go @@ -0,0 +1,56 @@ +// Copyright 2025. +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package flux + +import ( + "context" + "errors" + + "k8s.io/apimachinery/pkg/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +var _ ManagedCluster = &fakeCluster{} + +type fakeCluster struct { + managedCluster + fakeClient client.Client +} + +// GetClient implements [ManagedCluster]. +func (f *fakeCluster) GetClient() client.Client { + return f.fakeClient +} + +func createFakeCluster(client client.Client) ManagedCluster { + return &fakeCluster{ + fakeClient: client, + } +} + +func createFakeClient(clusterObjects []client.Object) client.Client { + scheme := runtime.NewScheme() + _ = clientgoscheme.AddToScheme(scheme) + return fake.NewClientBuilder().WithObjects(clusterObjects...).WithScheme(scheme).Build() +} + +type listErrorClient struct { + client.Client +} + +func (l listErrorClient) List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error { + return errors.New("list failed") +} diff --git a/test/e2e/flux_test.go b/test/e2e/flux_test.go index 16bd5ab..a464f9d 100644 --- a/test/e2e/flux_test.go +++ b/test/e2e/flux_test.go @@ -6,6 +6,7 @@ import ( "testing" "time" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -28,7 +29,16 @@ import ( "github.com/openmcp-project/openmcp-testing/pkg/resources" ) -const mcpName = "test-mcp" +const ( + mcpName = "test-mcp" + caConfigMapName = "flux-ca-bundle" + caConfigMapNameUpdate = "flux-ca-bundle-update" + caConfigMapKey = "ca.crt" + caVolumeName = "custom-ca-bundle" + caMountPath = "/etc/open-control-plane/custom-ca" + sslCertDirEnvName = "SSL_CERT_DIR" + sslCertDirEnvValue = "/etc/ssl/certs:/etc/pki/tls/certs:/etc/open-control-plane/custom-ca" +) func TestServiceProvider(t *testing.T) { var onboardingList unstructured.UnstructuredList @@ -106,6 +116,16 @@ func TestServiceProvider(t *testing.T) { if err := wait.For(conditions.New(mcp.Client().Resources()).ResourcesFound(list), wait.WithTimeout(2*time.Minute)); err != nil { t.Errorf("image pull secret not found on control plane: %v", err) } + + caBundleConfigMap := &corev1.ConfigMap{} + caBundleConfigMap.SetName(caConfigMapName) + caBundleConfigMap.SetNamespace("flux-system") + cmList := &corev1.ConfigMapList{ + Items: []corev1.ConfigMap{*caBundleConfigMap}, + } + if err := wait.For(conditions.New(mcp.Client().Resources()).ResourcesFound(cmList), wait.WithTimeout(2*time.Minute)); err != nil { + t.Errorf("ca configmap not found on control plane: %v", err) + } return ctx }). Assess("domain objects can be created", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { @@ -141,6 +161,69 @@ func TestServiceProvider(t *testing.T) { } return ctx }). + Assess("flux deployments mount custom ca and set SSL_CERT_DIR", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { + mcp, err := clusterutils.MCPConfig(ctx, c, mcpName) + if err != nil { + t.Error(err) + return ctx + } + + deploymentNames := []string{ + "helm-controller", + "image-automation-controller", + "image-reflector-controller", + "kustomize-controller", + "notification-controller", + "source-controller", + "source-watcher", + } + + for _, deploymentName := range deploymentNames { + deployment := &appsv1.Deployment{} + if err := mcp.Client().Resources().Get(ctx, deploymentName, "flux-system", deployment); err != nil { + t.Errorf("failed to get deployment %s: %v", deploymentName, err) + continue + } + + hasCAVolume := false + for _, volume := range deployment.Spec.Template.Spec.Volumes { + if volume.Name == caVolumeName { + hasCAVolume = true + break + } + } + if !hasCAVolume { + t.Errorf("deployment %s does not have %s volume", deploymentName, caVolumeName) + continue + } + + hasCAMount := false + hasSSLCertDirEnv := false + for _, container := range deployment.Spec.Template.Spec.Containers { + for _, volumeMount := range container.VolumeMounts { + if volumeMount.Name == caVolumeName && volumeMount.MountPath == caMountPath { + hasCAMount = true + break + } + } + for _, envVar := range container.Env { + if envVar.Name == sslCertDirEnvName && envVar.Value == sslCertDirEnvValue { + hasSSLCertDirEnv = true + break + } + } + } + + if !hasCAMount { + t.Errorf("deployment %s does not mount %s at %s", deploymentName, caVolumeName, caMountPath) + } + if !hasSSLCertDirEnv { + t.Errorf("deployment %s does not set %s=%s", deploymentName, sslCertDirEnvName, sslCertDirEnvValue) + } + } + + return ctx + }). Assess("provider config update with new secret references", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { if err := v1alpha1.AddToScheme(c.Client().Resources().GetScheme()); err != nil { t.Errorf("failed to add api types to client scheme: %s", err) @@ -225,6 +308,66 @@ func TestServiceProvider(t *testing.T) { } return ctx }). + Assess("provider config update with new ca bundle reference", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { + if err := v1alpha1.AddToScheme(c.Client().Resources().GetScheme()); err != nil { + t.Errorf("failed to add api types to client scheme: %s", err) + return ctx + } + providerConfig := &v1alpha1.ProviderConfig{} + providerConfig.SetName("flux") + if err := c.Client().Resources().Get(ctx, "flux", "openmcp-system", providerConfig); err != nil { + t.Errorf("failed to get provider config: %v", err) + return ctx + } + providerConfig.Spec.CaBundleRef = &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: caConfigMapNameUpdate}, + Key: caConfigMapKey, + } + if err := c.Client().Resources().Update(ctx, providerConfig); err != nil { + t.Errorf("failed to update provider config: %v", err) + return ctx + } + + onboardingConfig, err := clusterutils.OnboardingConfig() + v1alpha1.AddToScheme(onboardingConfig.GetClient().Resources().GetScheme()) + if err != nil { + t.Error(err) + return ctx + } + + fluxObj := &v1alpha1.Flux{} + fluxObj.SetName(mcpName) + fluxObj.SetNamespace(corev1.NamespaceDefault) + if err := wait.For(openmcpconditions.Match(fluxObj, onboardingConfig, "Ready", corev1.ConditionTrue), wait.WithTimeout(2*time.Minute)); err != nil { + t.Errorf("Flux not ready after provider config update: %v", err) + } + return ctx + }). + Assess("control plane ca configmap updated", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { + mcp, err := clusterutils.MCPConfig(ctx, c, mcpName) + if err != nil { + t.Error(err) + return ctx + } + + oldCaConfigMap := &corev1.ConfigMap{} + oldCaConfigMap.SetName(caConfigMapName) + oldCaConfigMap.SetNamespace("flux-system") + if err := wait.For(conditions.New(mcp.Client().Resources()).ResourceDeleted(oldCaConfigMap), wait.WithTimeout(2*time.Minute)); err != nil { + t.Errorf("orphaned ca configmap is not deleted: %v", err) + } + + newCaConfigMap := &corev1.ConfigMap{} + newCaConfigMap.SetName(caConfigMapNameUpdate) + newCaConfigMap.SetNamespace("flux-system") + list := &corev1.ConfigMapList{ + Items: []corev1.ConfigMap{*newCaConfigMap}, + } + if err := wait.For(conditions.New(mcp.Client().Resources()).ResourcesFound(list), wait.WithTimeout(2*time.Minute)); err != nil { + t.Errorf("updated ca configmap not found on control plane: %v", err) + } + return ctx + }). Assess("provider config update drops pull secrets", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { if err := v1alpha1.AddToScheme(c.Client().Resources().GetScheme()); err != nil { t.Errorf("failed to add api types to client scheme: %s", err) @@ -238,6 +381,7 @@ func TestServiceProvider(t *testing.T) { } providerConfig.Spec.Versions[0].ChartPullSecret = "" providerConfig.Spec.Versions[0].Values = nil + providerConfig.Spec.CaBundleRef = nil if err := c.Client().Resources().Update(ctx, providerConfig); err != nil { t.Errorf("failed to update provider config: %v", err) } @@ -287,6 +431,21 @@ func TestServiceProvider(t *testing.T) { } return ctx }). + Assess("control plane ca configmap deleted", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { + mcp, err := clusterutils.MCPConfig(ctx, c, mcpName) + if err != nil { + t.Error(err) + return ctx + } + caConfigMap := &corev1.ConfigMapList{} + if err := wait.For(conditions.New(mcp.Client().Resources().WithNamespace("flux-system")). + ResourceListN(caConfigMap, 0, klientresources.WithLabelSelector( + labels.FormatLabels(map[string]string{flux.LabelManagedBy: "service-provider-flux"}))), + wait.WithTimeout(2*time.Minute)); err != nil { + t.Errorf("orphaned ca configmap is not deleted: %v", err) + } + return ctx + }). Teardown(func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { mcp, err := clusterutils.MCPConfig(ctx, c, mcpName) if err != nil { diff --git a/test/e2e/platform/caconfigmap.yaml b/test/e2e/platform/caconfigmap.yaml new file mode 100644 index 0000000..5b6b64a --- /dev/null +++ b/test/e2e/platform/caconfigmap.yaml @@ -0,0 +1,33 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: flux-ca-bundle + namespace: openmcp-system +data: + # dummy value for testing + ca.crt: | + -----BEGIN CERTIFICATE----- + MIIBsjCCAVigAwIBAgIUDummyCertificateForE2ETestsOnlywCgYIKoZIzj0EAwIw + EzERMA8GA1UEAwwIdGVzdC1jYTAeFw0yNjAxMDEwMDAwMDBaFw0zNjAxMDEwMDAw + MDBaMBMxETAPBgNVBAMMCHRlc3QtY2EwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNC + AASxDummyCertDataOnlyForE2EFlowValidationDoNotUseInProd2Yxk7T7x7C1q5 + i8E2zX2S9qQwCgYIKoZIzj0EAwIDSAAwRQIhANdummyW9J6g4QYjv2Q6u8X9q8lQ + 3S7V8E5U2bY6uY6QhAiBiDummylX8Yv3n9vQj8m8xjz0H2xB3g9C2u8vA== + -----END CERTIFICATE----- +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: flux-ca-bundle-update + namespace: openmcp-system +data: + # dummy value for testing + ca.crt: | + -----BEGIN CERTIFICATE----- + MIIBsjCCAVigAwIBAgIUDummyCertificateForE2ETestsOnlywCgYIKoZIzj0EAwIw + EzERMA8GA1UEAwwIdGVzdC1jYTAeFw0yNjAxMDEwMDAwMDBaFw0zNjAxMDEwMDAw + MDBaMBMxETAPBgNVBAMMCHRlc3QtY2EwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNC + AASxDummyCertDataOnlyForE2EFlowValidationDoNotUseInProd2Yxk7T7x7C1q5 + i8E2zX2S9qQwCgYIKoZIzj0EAwIDSAAwRQIhANdummyW9J6g4QYjv2Q6u8X9q8lQ + 3S7V8E5U2bY6uY6QhAiBiDummylX8Yv3n9vQj8m8xjz0H2xB3g9C2u8vA== + -----END CERTIFICATE----- diff --git a/test/e2e/platform/providerconfig.yaml b/test/e2e/platform/providerconfig.yaml index 384490c..e606ec4 100644 --- a/test/e2e/platform/providerconfig.yaml +++ b/test/e2e/platform/providerconfig.yaml @@ -4,6 +4,9 @@ metadata: name: flux spec: pollInterval: "10s" + caBundleRef: + name: flux-ca-bundle + key: ca.crt versions: - version: "2.8.3" chartVersion: "2.18.2" From 68539aadf2edda7467d8d2fd0afcd6763444b82b Mon Sep 17 00:00:00 2001 From: "Nico Andres (SAP)" Date: Mon, 8 Jun 2026 20:12:43 +0200 Subject: [PATCH 10/20] Fixed name for target configmap On-behalf-of: @SAP nico.andres@sap.com Signed-off-by: Nico Andres (SAP) --- docs/configuration/image-localization.md | 2 +- internal/controller/flux_controller.go | 3 ++- pkg/flux/helm.go | 13 +++++++++++-- pkg/flux/helm_test.go | 2 +- test/e2e/flux_test.go | 21 +++++++-------------- 5 files changed, 22 insertions(+), 19 deletions(-) diff --git a/docs/configuration/image-localization.md b/docs/configuration/image-localization.md index 26b51bb..50ba890 100644 --- a/docs/configuration/image-localization.md +++ b/docs/configuration/image-localization.md @@ -247,7 +247,7 @@ Verify configmaps are copied to the correct namespaces: ```bash # ManagedControlPlane - flux-system namespace -kubectl get cm -n flux-system | grep -E "" +kubectl get cm -n flux-system | grep -E "ca" ``` ### Check OCIRepository Secret Reference diff --git a/internal/controller/flux_controller.go b/internal/controller/flux_controller.go index 5005cae..420d6dc 100644 --- a/internal/controller/flux_controller.go +++ b/internal/controller/flux_controller.go @@ -187,6 +187,7 @@ func (r *FluxReconciler) createObjectManager(obj *apiv1alpha1.Flux, pc *apiv1alp SourceClient: r.PlatformCluster.Client(), SourceNamespace: r.PodNamespace, TargetNamespace: fluxNamespace, + TargetName: flux.CustomCABundleConfigMapName, }) } @@ -217,7 +218,7 @@ func (r *FluxReconciler) createObjectManager(obj *apiv1alpha1.Flux, pc *apiv1alp configMapsToKeep := []corev1.LocalObjectReference{} if pc.Spec.CaBundleRef != nil { - configMapsToKeep = append(configMapsToKeep, pc.Spec.CaBundleRef.LocalObjectReference) + configMapsToKeep = append(configMapsToKeep, corev1.LocalObjectReference{Name: flux.CustomCABundleConfigMapName}) } controlPlaneConfigMapCleaner := flux.NewConfigMapCleaner(mcpCluster, fluxNamespace, configMapsToKeep) diff --git a/pkg/flux/helm.go b/pkg/flux/helm.go index 034990d..2390495 100644 --- a/pkg/flux/helm.go +++ b/pkg/flux/helm.go @@ -24,10 +24,17 @@ import ( ) const ( + // customCaVolumeName is the name of the custom ca volume and volume mount customCaVolumeName = "custom-ca-bundle" - customCaPath = "/etc/open-control-plane/custom-ca" + + // customCaPath is the path the ca bundle will be mounted into + customCaPath = "/etc/open-control-plane/custom-ca" + + // CustomCABundleConfigMapName is the fixed name for the copied CA bundle ConfigMap on the MCP cluster. + CustomCABundleConfigMapName = "custom-ca-bundle" ) +// certDirectories contains a list of places where the default system certs are stored in addition to caBundleMountDir // from x509 go lib (https://github.com/golang/go/blob/015343854b5d9e2829481df30dbcae2ca6682d25/src/crypto/x509/root_linux.go) var certDirectories = []string{ "/etc/ssl/certs", @@ -82,7 +89,9 @@ func AddCaToHelmValues(values *apiextensionsv1.JSON, configMap *corev1.ConfigMap Name: customCaVolumeName, VolumeSource: corev1.VolumeSource{ ConfigMap: &corev1.ConfigMapVolumeSource{ - LocalObjectReference: configMap.LocalObjectReference, + LocalObjectReference: corev1.LocalObjectReference{ + Name: CustomCABundleConfigMapName, + }, Items: []corev1.KeyToPath{ { Key: configMap.Key, diff --git a/pkg/flux/helm_test.go b/pkg/flux/helm_test.go index 423bdd0..1f80689 100644 --- a/pkg/flux/helm_test.go +++ b/pkg/flux/helm_test.go @@ -142,7 +142,7 @@ func TestAddCaToHelmValues(t *testing.T) { Name: customCaVolumeName, VolumeSource: corev1.VolumeSource{ ConfigMap: &corev1.ConfigMapVolumeSource{ - LocalObjectReference: corev1.LocalObjectReference{Name: "custom-ca-configmap"}, + LocalObjectReference: corev1.LocalObjectReference{Name: CustomCABundleConfigMapName}, Items: []corev1.KeyToPath{{ Key: "ca.crt", Path: "ca.crt", diff --git a/test/e2e/flux_test.go b/test/e2e/flux_test.go index a464f9d..f7a93eb 100644 --- a/test/e2e/flux_test.go +++ b/test/e2e/flux_test.go @@ -31,7 +31,7 @@ import ( const ( mcpName = "test-mcp" - caConfigMapName = "flux-ca-bundle" + mcpCAConfigMapName = "custom-ca-bundle" caConfigMapNameUpdate = "flux-ca-bundle-update" caConfigMapKey = "ca.crt" caVolumeName = "custom-ca-bundle" @@ -118,7 +118,7 @@ func TestServiceProvider(t *testing.T) { } caBundleConfigMap := &corev1.ConfigMap{} - caBundleConfigMap.SetName(caConfigMapName) + caBundleConfigMap.SetName(mcpCAConfigMapName) caBundleConfigMap.SetNamespace("flux-system") cmList := &corev1.ConfigMapList{ Items: []corev1.ConfigMap{*caBundleConfigMap}, @@ -350,21 +350,14 @@ func TestServiceProvider(t *testing.T) { return ctx } - oldCaConfigMap := &corev1.ConfigMap{} - oldCaConfigMap.SetName(caConfigMapName) - oldCaConfigMap.SetNamespace("flux-system") - if err := wait.For(conditions.New(mcp.Client().Resources()).ResourceDeleted(oldCaConfigMap), wait.WithTimeout(2*time.Minute)); err != nil { - t.Errorf("orphaned ca configmap is not deleted: %v", err) - } - - newCaConfigMap := &corev1.ConfigMap{} - newCaConfigMap.SetName(caConfigMapNameUpdate) - newCaConfigMap.SetNamespace("flux-system") + mcpCaConfigMap := &corev1.ConfigMap{} + mcpCaConfigMap.SetName(mcpCAConfigMapName) + mcpCaConfigMap.SetNamespace("flux-system") list := &corev1.ConfigMapList{ - Items: []corev1.ConfigMap{*newCaConfigMap}, + Items: []corev1.ConfigMap{*mcpCaConfigMap}, } if err := wait.For(conditions.New(mcp.Client().Resources()).ResourcesFound(list), wait.WithTimeout(2*time.Minute)); err != nil { - t.Errorf("updated ca configmap not found on control plane: %v", err) + t.Errorf("ca configmap not found on control plane: %v", err) } return ctx }). From e8d727817cee696ece5c4c9098ad4714be607a82 Mon Sep 17 00:00:00 2001 From: "Nico Andres (SAP)" Date: Mon, 8 Jun 2026 20:39:39 +0200 Subject: [PATCH 11/20] Update docs On-behalf-of: @SAP nico.andres@sap.com Signed-off-by: Nico Andres (SAP) --- docs/configuration/image-localization.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/configuration/image-localization.md b/docs/configuration/image-localization.md index 50ba890..3ea67ca 100644 --- a/docs/configuration/image-localization.md +++ b/docs/configuration/image-localization.md @@ -9,6 +9,7 @@ In air-gapped environments, you typically need to: 1. **Mirror the Flux Helm chart** to your internal OCI registry 2. **Mirror Flux controller images** to your internal container registry 3. **Configure authentication** for both chart and image pulls +4. **Add custom CA certificates** if you use private PKI with self-signed certificates The Flux service provider handles this through: From 7c9837c6417313dba928cd8874ff54513505c8de Mon Sep 17 00:00:00 2001 From: "Nico Andres (SAP)" Date: Tue, 9 Jun 2026 08:20:25 +0200 Subject: [PATCH 12/20] Fix comments On-behalf-of: @SAP nico.andres@sap.com Signed-off-by: Nico Andres (SAP) --- docs/configuration/image-localization.md | 5 ++--- internal/controller/flux_controller.go | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/docs/configuration/image-localization.md b/docs/configuration/image-localization.md index 3ea67ca..7298a04 100644 --- a/docs/configuration/image-localization.md +++ b/docs/configuration/image-localization.md @@ -43,7 +43,7 @@ flowchart TB subgraph FS[flux-system namespace] imgcopy1([image-pull-secret-1]) imgcopy2([image-pull-secret-2]) - caconfigmapcopy([custom-ca-configmap copy]) + caconfigmapcopy([custom-ca-configmap]) fluxctrl[Flux Controllers] fluxctrl -. uses .-> imgcopy1 fluxctrl -. uses .-> imgcopy2 @@ -71,8 +71,7 @@ spec: # ConfigMapKeySelector pointing to a configmap which holds a PEM-encoded custom CA bundle. # Must exist in the service provider's namespace on the platform cluster # The configmap will be automatically copied from the service provider's namespace - # to the flux-system namespace on the ManagedControlPlane and configured - # for the flux-controllers + # and configured for the Flux controlers. caBundleRef: name: "custom-ca-bundle" key: "ca-bundle.crt" diff --git a/internal/controller/flux_controller.go b/internal/controller/flux_controller.go index 420d6dc..28a9dea 100644 --- a/internal/controller/flux_controller.go +++ b/internal/controller/flux_controller.go @@ -176,7 +176,7 @@ func (r *FluxReconciler) createObjectManager(obj *apiv1alpha1.Flux, pc *apiv1alp } if pc.Spec.CaBundleRef != nil { - // add custom ca volume and volumeMount to helm values + // add custom ca volume, volumeMount and envVar to helm values fluxVersion.Values, err = flux.AddCaToHelmValues(fluxVersion.Values, pc.Spec.CaBundleRef) if err != nil { return nil, fmt.Errorf("failed to add ca volume to helm values: %w", err) From 00155dfd98d17d10f8d1d47acc28d850607c60ca Mon Sep 17 00:00:00 2001 From: "Nico Andres (SAP)" Date: Tue, 16 Jun 2026 12:00:36 +0200 Subject: [PATCH 13/20] fix remarks On-behalf-of: @SAP nico.andres@sap.com Signed-off-by: Nico Andres (SAP) --- README.md | 2 +- api/v1alpha1/providerconfig_types.go | 2 +- api/v1alpha1/zz_generated.deepcopy.go | 4 +- internal/controller/flux_controller.go | 8 ++-- pkg/flux/configmap.go | 2 +- pkg/flux/configmap_test.go | 59 ++++++++++++++++++++++++++ pkg/flux/flux_test.go | 56 ------------------------ pkg/flux/helm.go | 9 ++-- pkg/flux/helm_test.go | 2 +- pkg/flux/secret.go | 2 +- pkg/flux/secret_test.go | 59 ++++++++++++++++++++++++++ test/e2e/flux_test.go | 23 +++++++++- test/e2e/platform/caconfigmap.yaml | 2 +- 13 files changed, 156 insertions(+), 74 deletions(-) diff --git a/README.md b/README.md index b62405b..69cbf34 100644 --- a/README.md +++ b/README.md @@ -183,7 +183,7 @@ spec: | `spec.caBundleRef` | ConfigMapKeySelector | A configmap with a ca bundle used by Flux to verify certificates | | `spec.versions` | array | The versions of Flux that can be installed | -A **caBundleref** is defined as follows: +A **caBundleRef** is defined as follows: | Field | Type | Description | |-------|------|-------------| diff --git a/api/v1alpha1/providerconfig_types.go b/api/v1alpha1/providerconfig_types.go index 1906792..0c33c37 100644 --- a/api/v1alpha1/providerconfig_types.go +++ b/api/v1alpha1/providerconfig_types.go @@ -40,7 +40,7 @@ type ProviderConfigSpec struct { // CABundleRef is a reference to a config map containing a certificate bundle. // It will be installed on the ManagedControlPlane and configured for the Flux controllers. // +kubebuilder:validation:Optional - CaBundleRef *corev1.ConfigMapKeySelector `json:"caBundleRef,omitempty"` + CABundleRef *corev1.ConfigMapKeySelector `json:"caBundleRef,omitempty"` } // FluxVersion defines a version of Flux that can be installed diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 18945de..c55646d 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -229,8 +229,8 @@ func (in *ProviderConfigSpec) DeepCopyInto(out *ProviderConfigSpec) { *out = new(v1.Duration) **out = **in } - if in.CaBundleRef != nil { - in, out := &in.CaBundleRef, &out.CaBundleRef + if in.CABundleRef != nil { + in, out := &in.CABundleRef, &out.CABundleRef *out = new(corev1.ConfigMapKeySelector) (*in).DeepCopyInto(*out) } diff --git a/internal/controller/flux_controller.go b/internal/controller/flux_controller.go index 28a9dea..92b1a60 100644 --- a/internal/controller/flux_controller.go +++ b/internal/controller/flux_controller.go @@ -175,15 +175,15 @@ func (r *FluxReconciler) createObjectManager(obj *apiv1alpha1.Flux, pc *apiv1alp }) } - if pc.Spec.CaBundleRef != nil { + if pc.Spec.CABundleRef != nil { // add custom ca volume, volumeMount and envVar to helm values - fluxVersion.Values, err = flux.AddCaToHelmValues(fluxVersion.Values, pc.Spec.CaBundleRef) + fluxVersion.Values, err = flux.AddCAToHelmValues(fluxVersion.Values, pc.Spec.CABundleRef) if err != nil { return nil, fmt.Errorf("failed to add ca volume to helm values: %w", err) } // Sync ca configmap from platform cluster to MCP - flux.ManageCaConfigMap(mcpCluster, pc.Spec.CaBundleRef.LocalObjectReference, flux.ConfigMapCopyConfig{ + flux.ManageCaConfigMap(mcpCluster, pc.Spec.CABundleRef.LocalObjectReference, flux.ConfigMapCopyConfig{ SourceClient: r.PlatformCluster.Client(), SourceNamespace: r.PodNamespace, TargetNamespace: fluxNamespace, @@ -217,7 +217,7 @@ func (r *FluxReconciler) createObjectManager(obj *apiv1alpha1.Flux, pc *apiv1alp controlPlaneSecretCleaner := flux.NewSecretCleaner(mcpCluster, fluxNamespace, helmValues.ImagePullSecrets) configMapsToKeep := []corev1.LocalObjectReference{} - if pc.Spec.CaBundleRef != nil { + if pc.Spec.CABundleRef != nil { configMapsToKeep = append(configMapsToKeep, corev1.LocalObjectReference{Name: flux.CustomCABundleConfigMapName}) } diff --git a/pkg/flux/configmap.go b/pkg/flux/configmap.go index 5f7460f..d93a542 100644 --- a/pkg/flux/configmap.go +++ b/pkg/flux/configmap.go @@ -74,7 +74,7 @@ func ManageCaConfigMap(targetCluster ManagedCluster, caConfigMap corev1.LocalObj mutator := openmcpresources.NewConfigMapMutator(caConfigMapName, config.TargetNamespace, sourceConfigMap.Data) return mutator.Mutate(oConfigMap) }, - StatusFunc: SimpleStatus, + StatusFunc: ConfigMapStatus, }) targetCluster.AddObject(configMap) } diff --git a/pkg/flux/configmap_test.go b/pkg/flux/configmap_test.go index a115b8c..65b73b3 100644 --- a/pkg/flux/configmap_test.go +++ b/pkg/flux/configmap_test.go @@ -18,6 +18,7 @@ import ( "errors" "slices" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -26,6 +27,8 @@ import ( "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/client" + apiv1alpha1 "github.com/openmcp-project/service-provider-flux/api/v1alpha1" + "github.com/openmcp-project/service-provider-flux/pkg/testutils" ) @@ -213,6 +216,62 @@ func testConfigMap(name, namespace string, managedByFlux bool) *corev1.ConfigMap } } +// TestConfigMapStatus tests the ConfigMapStatus function +func TestConfigMapStatus(t *testing.T) { + tests := []struct { + name string + obj client.Object + rl apiv1alpha1.ResourceLocation + expected apiv1alpha1.InstancePhase + }{ + { + name: "configmap with UID - ready", + obj: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "test-ns", + UID: "test-uid", + }, + }, + rl: apiv1alpha1.ManagedControlPlane, + expected: apiv1alpha1.Ready, + }, + { + name: "configmap without UID - pending", + obj: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "test-ns", + }, + }, + rl: apiv1alpha1.ManagedControlPlane, + expected: apiv1alpha1.Pending, + }, + { + name: "configmap being deleted - terminating", + obj: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "test-ns", + UID: "test-uid", + DeletionTimestamp: &metav1.Time{Time: time.Now()}, + Finalizers: []string{"test-finalizer"}, + }, + }, + rl: apiv1alpha1.ManagedControlPlane, + expected: apiv1alpha1.Terminating, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + status := ConfigMapStatus(tt.obj, tt.rl) + assert.Equal(t, tt.expected, status.Phase) + assert.Equal(t, tt.rl, status.Location) + }) + } +} + type deleteErrorConfigMapClient struct { client.Client fakeConfigMap corev1.ConfigMap diff --git a/pkg/flux/flux_test.go b/pkg/flux/flux_test.go index ee43f27..cb2c440 100644 --- a/pkg/flux/flux_test.go +++ b/pkg/flux/flux_test.go @@ -172,62 +172,6 @@ func TestSimpleStatus(t *testing.T) { } } -// TestSecretStatus tests the SecretStatus function -func TestSecretStatus(t *testing.T) { - tests := []struct { - name string - obj client.Object - rl apiv1alpha1.ResourceLocation - expected apiv1alpha1.InstancePhase - }{ - { - name: "secret with UID - ready", - obj: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: "test-ns", - UID: "test-uid", - }, - }, - rl: apiv1alpha1.ManagedControlPlane, - expected: apiv1alpha1.Ready, - }, - { - name: "secret without UID - pending", - obj: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: "test-ns", - }, - }, - rl: apiv1alpha1.ManagedControlPlane, - expected: apiv1alpha1.Pending, - }, - { - name: "secret being deleted - terminating", - obj: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: "test-ns", - UID: "test-uid", - DeletionTimestamp: &metav1.Time{Time: time.Now()}, - Finalizers: []string{"test-finalizer"}, - }, - }, - rl: apiv1alpha1.ManagedControlPlane, - expected: apiv1alpha1.Terminating, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - status := SecretStatus(tt.obj, tt.rl) - assert.Equal(t, tt.expected, status.Phase) - assert.Equal(t, tt.rl, status.Location) - }) - } -} - // TestSetManagedBy tests the SetManagedBy function func TestSetManagedBy(t *testing.T) { tests := []struct { diff --git a/pkg/flux/helm.go b/pkg/flux/helm.go index 2390495..c5d007f 100644 --- a/pkg/flux/helm.go +++ b/pkg/flux/helm.go @@ -76,9 +76,9 @@ func ExtractHelmValues(values *apiextensionsv1.JSON) (*HelmValues, error) { return vals, nil } -// AddCaToHelmValues removes conflicting volumes, volumeMounts and envVars (matching by name and/or mountPath) and +// AddCAToHelmValues removes conflicting volumes, volumeMounts and envVars (matching by name and/or mountPath) and // adds a volume, volumeMount and envVar on all Flux controller helm values sections to import the custom CA certificate. -func AddCaToHelmValues(values *apiextensionsv1.JSON, configMap *corev1.ConfigMapKeySelector) (*apiextensionsv1.JSON, error) { +func AddCAToHelmValues(values *apiextensionsv1.JSON, configMap *corev1.ConfigMapKeySelector) (*apiextensionsv1.JSON, error) { if configMap == nil { return nil, errors.New("cannot add custom CA to Helm values: ConfigMapKeySelector is nil") } @@ -120,7 +120,7 @@ func AddCaToHelmValues(values *apiextensionsv1.JSON, configMap *corev1.ConfigMap } for _, controller := range fluxControllers { - if err := addCaToController(root, controller, caVolume, caVolumeMount, caEnvVar); err != nil { + if err := addCAToController(root, controller, caVolume, caVolumeMount, caEnvVar); err != nil { return nil, err } } @@ -133,7 +133,7 @@ func AddCaToHelmValues(values *apiextensionsv1.JSON, configMap *corev1.ConfigMap return &apiextensionsv1.JSON{Raw: out}, nil } -func addCaToController( +func addCAToController( root map[string]json.RawMessage, controller string, caVolume corev1.Volume, @@ -148,6 +148,7 @@ func addCaToController( if err := unmarshalIfPresent(root, controller, &controllerValues); err != nil { return fmt.Errorf("failed to unmarshal %s: %w", controller, err) } + if controllerValues == nil { controllerValues = map[string]json.RawMessage{} } diff --git a/pkg/flux/helm_test.go b/pkg/flux/helm_test.go index 1f80689..6c61fd8 100644 --- a/pkg/flux/helm_test.go +++ b/pkg/flux/helm_test.go @@ -330,7 +330,7 @@ func TestAddCaToHelmValues(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - out, err := AddCaToHelmValues(tt.values, caBundleRef) + out, err := AddCAToHelmValues(tt.values, caBundleRef) if tt.wantErr != "" { require.Error(t, err) assert.Contains(t, err.Error(), tt.wantErr) diff --git a/pkg/flux/secret.go b/pkg/flux/secret.go index 6bc6398..23fa6d5 100644 --- a/pkg/flux/secret.go +++ b/pkg/flux/secret.go @@ -78,7 +78,7 @@ func ManagePullSecrets(targetCluster ManagedCluster, imagePullSecrets []corev1.L mutator := openmcpresources.NewSecretMutator(secretName, config.TargetNamespace, sourceSecret.Data, corev1.SecretTypeDockerConfigJson) return mutator.Mutate(oSecret) }, - StatusFunc: SimpleStatus, + StatusFunc: SecretStatus, }) targetCluster.AddObject(secret) } diff --git a/pkg/flux/secret_test.go b/pkg/flux/secret_test.go index e8bca48..17e01e6 100644 --- a/pkg/flux/secret_test.go +++ b/pkg/flux/secret_test.go @@ -20,6 +20,7 @@ import ( "slices" "strings" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -28,6 +29,8 @@ import ( "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/client" + apiv1alpha1 "github.com/openmcp-project/service-provider-flux/api/v1alpha1" + "github.com/openmcp-project/service-provider-flux/pkg/testutils" ) @@ -244,6 +247,62 @@ func testSecret(name, namespace string, managedByFlux bool) *corev1.Secret { } } +// TestSecretStatus tests the SecretStatus function +func TestSecretStatus(t *testing.T) { + tests := []struct { + name string + obj client.Object + rl apiv1alpha1.ResourceLocation + expected apiv1alpha1.InstancePhase + }{ + { + name: "secret with UID - ready", + obj: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "test-ns", + UID: "test-uid", + }, + }, + rl: apiv1alpha1.ManagedControlPlane, + expected: apiv1alpha1.Ready, + }, + { + name: "secret without UID - pending", + obj: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "test-ns", + }, + }, + rl: apiv1alpha1.ManagedControlPlane, + expected: apiv1alpha1.Pending, + }, + { + name: "secret being deleted - terminating", + obj: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "test-ns", + UID: "test-uid", + DeletionTimestamp: &metav1.Time{Time: time.Now()}, + Finalizers: []string{"test-finalizer"}, + }, + }, + rl: apiv1alpha1.ManagedControlPlane, + expected: apiv1alpha1.Terminating, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + status := SecretStatus(tt.obj, tt.rl) + assert.Equal(t, tt.expected, status.Phase) + assert.Equal(t, tt.rl, status.Location) + }) + } +} + type deleteErrorClient struct { client.Client fakeSecret corev1.Secret diff --git a/test/e2e/flux_test.go b/test/e2e/flux_test.go index f7a93eb..a56f310 100644 --- a/test/e2e/flux_test.go +++ b/test/e2e/flux_test.go @@ -3,6 +3,7 @@ package e2e import ( "context" "encoding/json" + "strings" "testing" "time" @@ -319,7 +320,7 @@ func TestServiceProvider(t *testing.T) { t.Errorf("failed to get provider config: %v", err) return ctx } - providerConfig.Spec.CaBundleRef = &corev1.ConfigMapKeySelector{ + providerConfig.Spec.CABundleRef = &corev1.ConfigMapKeySelector{ LocalObjectReference: corev1.LocalObjectReference{Name: caConfigMapNameUpdate}, Key: caConfigMapKey, } @@ -350,14 +351,32 @@ func TestServiceProvider(t *testing.T) { return ctx } + // Verify that updated configmap exists mcpCaConfigMap := &corev1.ConfigMap{} mcpCaConfigMap.SetName(mcpCAConfigMapName) mcpCaConfigMap.SetNamespace("flux-system") list := &corev1.ConfigMapList{ Items: []corev1.ConfigMap{*mcpCaConfigMap}, } + if err := wait.For(conditions.New(mcp.Client().Resources()).ResourcesFound(list), wait.WithTimeout(2*time.Minute)); err != nil { t.Errorf("ca configmap not found on control plane: %v", err) + return ctx + } + + // Verify the configmap contains updated certificate data + if err := mcp.Client().Resources().Get(ctx, mcpCAConfigMapName, "flux-system", mcpCaConfigMap); err != nil { + t.Errorf("failed to get ca configmap data: %v", err) + return ctx + } + caData, ok := mcpCaConfigMap.Data[caConfigMapKey] + if !ok { + t.Errorf("ca configmap missing key %s", caConfigMapKey) + return ctx + } + // Verify the data contains the expected updated certificate marker + if !strings.Contains(caData, "UpdatedDummyCertificate") { + t.Errorf("ca configmap does not contain expected updated certificate data. Got: %s", caData) } return ctx }). @@ -374,7 +393,7 @@ func TestServiceProvider(t *testing.T) { } providerConfig.Spec.Versions[0].ChartPullSecret = "" providerConfig.Spec.Versions[0].Values = nil - providerConfig.Spec.CaBundleRef = nil + providerConfig.Spec.CABundleRef = nil if err := c.Client().Resources().Update(ctx, providerConfig); err != nil { t.Errorf("failed to update provider config: %v", err) } diff --git a/test/e2e/platform/caconfigmap.yaml b/test/e2e/platform/caconfigmap.yaml index 5b6b64a..26b57c3 100644 --- a/test/e2e/platform/caconfigmap.yaml +++ b/test/e2e/platform/caconfigmap.yaml @@ -24,7 +24,7 @@ data: # dummy value for testing ca.crt: | -----BEGIN CERTIFICATE----- - MIIBsjCCAVigAwIBAgIUDummyCertificateForE2ETestsOnlywCgYIKoZIzj0EAwIw + MIIBsjCCAVigAwIBAgIUUpdatedDummyCertificateForE2ETestsOnlywCgYIKoZIzj0EAwIw EzERMA8GA1UEAwwIdGVzdC1jYTAeFw0yNjAxMDEwMDAwMDBaFw0zNjAxMDEwMDAw MDBaMBMxETAPBgNVBAMMCHRlc3QtY2EwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNC AASxDummyCertDataOnlyForE2EFlowValidationDoNotUseInProd2Yxk7T7x7C1q5 From 16cf350676b934c4c33e9870d9e6d0cfd39331cf Mon Sep 17 00:00:00 2001 From: "Nico Andres (SAP)" Date: Tue, 16 Jun 2026 13:46:59 +0200 Subject: [PATCH 14/20] Change ca quality critera On-behalf-of: @SAP nico.andres@sap.com Signed-off-by: Nico Andres (SAP) --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 69cbf34..d07cac1 100644 --- a/README.md +++ b/README.md @@ -225,7 +225,7 @@ For air-gapped or enterprise environments, see the [Image Localization Guide](do | Status reporting & error messages | ✅ | | | Operation annotations | ⚠️ | `openmcp.cloud/operation: ignore` is processed by [opencontrolplane-runtime](https://github.com/openmcp-project/opencontrolplane-runtime). `openmcp.cloud/operation: reconcile` is not processed. | | API stability policy | ✅ | | -| Custom CA support | ⚠️ | Private-registry pull secrets supported; custom CA bundle propagation to Flux components is not implemented. | +| Custom CA support | ✅ | Private-registry pull secrets and custom CA bundle propagation to Flux components is supported. | | Release artifacts (image + OCM) | ✅ | | | Testing | ✅ | | | Ownership and maintenance docs | ✅ | | From 98a4cf32e0ab523a365fe0a6efbc5c859ac5e83b Mon Sep 17 00:00:00 2001 From: "Nico Andres (SAP)" Date: Tue, 16 Jun 2026 13:58:27 +0200 Subject: [PATCH 15/20] Update ca quality criteria On-behalf-of: @SAP nico.andres@sap.com Signed-off-by: Nico Andres (SAP) --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index d07cac1..45f828f 100644 --- a/README.md +++ b/README.md @@ -225,7 +225,8 @@ For air-gapped or enterprise environments, see the [Image Localization Guide](do | Status reporting & error messages | ✅ | | | Operation annotations | ⚠️ | `openmcp.cloud/operation: ignore` is processed by [opencontrolplane-runtime](https://github.com/openmcp-project/opencontrolplane-runtime). `openmcp.cloud/operation: reconcile` is not processed. | | API stability policy | ✅ | | -| Custom CA support | ✅ | Private-registry pull secrets and custom CA bundle propagation to Flux components is supported. | +| Custom CA support | +✅ | | | Release artifacts (image + OCM) | ✅ | | | Testing | ✅ | | | Ownership and maintenance docs | ✅ | | From 7be7622edf877fc90832e3d0e08de588127afa9d Mon Sep 17 00:00:00 2001 From: "Nico Andres (SAP)" Date: Tue, 16 Jun 2026 14:02:13 +0200 Subject: [PATCH 16/20] Fix ca quality criteria formatting On-behalf-of: @SAP nico.andres@sap.com Signed-off-by: Nico Andres (SAP) --- README.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/README.md b/README.md index 45f828f..434cf6a 100644 --- a/README.md +++ b/README.md @@ -225,8 +225,7 @@ For air-gapped or enterprise environments, see the [Image Localization Guide](do | Status reporting & error messages | ✅ | | | Operation annotations | ⚠️ | `openmcp.cloud/operation: ignore` is processed by [opencontrolplane-runtime](https://github.com/openmcp-project/opencontrolplane-runtime). `openmcp.cloud/operation: reconcile` is not processed. | | API stability policy | ✅ | | -| Custom CA support | -✅ | | +| Custom CA support | ✅ | | | Release artifacts (image + OCM) | ✅ | | | Testing | ✅ | | | Ownership and maintenance docs | ✅ | | From 452ab35bd67352829ecce2d6ebcacf57a1721bc1 Mon Sep 17 00:00:00 2001 From: Nico Andres Date: Thu, 18 Jun 2026 13:16:31 +0200 Subject: [PATCH 17/20] Fix docs On-behalf-of: @SAP nico.andres@sap.com Signed-off-by: Nico Andres --- docs/configuration/image-localization.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/configuration/image-localization.md b/docs/configuration/image-localization.md index 7298a04..1606ef2 100644 --- a/docs/configuration/image-localization.md +++ b/docs/configuration/image-localization.md @@ -71,7 +71,7 @@ spec: # ConfigMapKeySelector pointing to a configmap which holds a PEM-encoded custom CA bundle. # Must exist in the service provider's namespace on the platform cluster # The configmap will be automatically copied from the service provider's namespace - # and configured for the Flux controlers. + # and configured for the Flux controllers. caBundleRef: name: "custom-ca-bundle" key: "ca-bundle.crt" @@ -171,7 +171,8 @@ kubectl create configmap custom-ca-bundle \ 2. For each Flux controller, the Helm values are adjusted so that it mounts the provided `caBundleRef.key` and sets the `SSL_CERT_DIR` environment variable to add the bundle to the pool of known certificates 3. The Helm values are passed through to Flux, and each Flux controller is able to verify certificates signed by the provided custom CA -[!CAUTION] The custom CA certificate is not propagated to the OpenControlPlane cluster nodes. If you want to pull images from the same OCI registry you must add the custom CA certificate to the cluster nodes yourself. +[!CAUTION] +The custom CA certificate is not propagated to the OpenControlPlane cluster nodes. If you want to pull images from the same OCI registry you must add the custom CA certificate to the cluster nodes yourself. ## Complete Example From aee4caea9dcefa2273799ecb3825212b327598e3 Mon Sep 17 00:00:00 2001 From: Nico Andres Date: Thu, 18 Jun 2026 13:17:18 +0200 Subject: [PATCH 18/20] Add root nil check On-behalf-of: @SAP nico.andres@sap.com Signed-off-by: Nico Andres --- pkg/flux/helm.go | 3 +++ pkg/flux/helm_test.go | 15 +++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/pkg/flux/helm.go b/pkg/flux/helm.go index c5d007f..864a8b6 100644 --- a/pkg/flux/helm.go +++ b/pkg/flux/helm.go @@ -117,6 +117,9 @@ func AddCAToHelmValues(values *apiextensionsv1.JSON, configMap *corev1.ConfigMap if err := json.Unmarshal(values.Raw, &root); err != nil { return nil, fmt.Errorf("failed to unmarshal helm values: %w", err) } + if root == nil { + root = make(map[string]json.RawMessage) + } } for _, controller := range fluxControllers { diff --git a/pkg/flux/helm_test.go b/pkg/flux/helm_test.go index 6c61fd8..5acaa65 100644 --- a/pkg/flux/helm_test.go +++ b/pkg/flux/helm_test.go @@ -287,6 +287,21 @@ func TestAddCaToHelmValues(t *testing.T) { assert.JSONEq(t, string(expected.Raw), string(out.Raw)) }, }, + { + name: "null JSON value treated as empty", + values: &apiextensionsv1.JSON{Raw: []byte("null")}, + checkValue: func(t *testing.T, out *apiextensionsv1.JSON) { + require.NotNil(t, out) + + expected := buildHelmValues(t, + withAllControllerVolumes(expectedCaVolume), + withAllControllerVolumeMounts(expectedCaVolumeMount), + withAllControllerExtraEnv(expectedCaEnvVar), + ) + + assert.JSONEq(t, string(expected.Raw), string(out.Raw)) + }, + }, { name: "Returns error for invalid root json", values: &apiextensionsv1.JSON{Raw: []byte("not-json")}, From 4eb203fab87f1464a64a80360b1f4552d6552ed5 Mon Sep 17 00:00:00 2001 From: Nico Andres Date: Thu, 18 Jun 2026 13:19:17 +0200 Subject: [PATCH 19/20] Check if configmap key and name is set On-behalf-of: @SAP nico.andres@sap.com Signed-off-by: Nico Andres --- pkg/flux/helm.go | 8 ++++++++ pkg/flux/helm_test.go | 35 ++++++++++++++++++++++++++++++----- 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/pkg/flux/helm.go b/pkg/flux/helm.go index 864a8b6..565603f 100644 --- a/pkg/flux/helm.go +++ b/pkg/flux/helm.go @@ -83,6 +83,14 @@ func AddCAToHelmValues(values *apiextensionsv1.JSON, configMap *corev1.ConfigMap return nil, errors.New("cannot add custom CA to Helm values: ConfigMapKeySelector is nil") } + if configMap.Name == "" { + return nil, errors.New("cannot add custom CA to Helm values: caBundleRef.Name is unset") + } + + if configMap.Key == "" { + return nil, errors.New("cannot add custom CA to Helm values: caBundleRef.Key is unset") + } + var root = map[string]json.RawMessage{} caVolume := corev1.Volume{ diff --git a/pkg/flux/helm_test.go b/pkg/flux/helm_test.go index 5acaa65..b8dd70c 100644 --- a/pkg/flux/helm_test.go +++ b/pkg/flux/helm_test.go @@ -161,10 +161,11 @@ func TestAddCaToHelmValues(t *testing.T) { } tests := []struct { - name string - values *apiextensionsv1.JSON - wantErr string - checkValue func(t *testing.T, out *apiextensionsv1.JSON) + name string + caBundleRef *corev1.ConfigMapKeySelector + values *apiextensionsv1.JSON + wantErr string + checkValue func(t *testing.T, out *apiextensionsv1.JSON) }{ { name: "Adds controller volumes, volumeMounts and extraEnv when no helm values are set", @@ -341,11 +342,35 @@ func TestAddCaToHelmValues(t *testing.T) { }), wantErr: "failed to unmarshal helmController.extraEnv", }, + { + name: "returns error if configMap name is unset", + wantErr: "caBundleRef.Name is unset", + caBundleRef: &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: ""}, + Key: "ca.crt", + }, + }, + { + name: "returns error if configMap key is unset", + wantErr: "caBundleRef.Key is unset", + caBundleRef: &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "custom-ca-configmap"}, + Key: "", + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - out, err := AddCAToHelmValues(tt.values, caBundleRef) + var out *apiextensionsv1.JSON + var err error + + if tt.caBundleRef != nil { + out, err = AddCAToHelmValues(tt.values, tt.caBundleRef) + } else { + out, err = AddCAToHelmValues(tt.values, caBundleRef) + } + if tt.wantErr != "" { require.Error(t, err) assert.Contains(t, err.Error(), tt.wantErr) From ba6e36081838376438b77b089f7508cf7aed6ced Mon Sep 17 00:00:00 2001 From: Nico Andres Date: Thu, 18 Jun 2026 14:18:28 +0200 Subject: [PATCH 20/20] Change error message + nolint On-behalf-of: @SAP nico.andres@sap.com Signed-off-by: Nico Andres --- pkg/flux/helm.go | 5 +++-- pkg/flux/helm_test.go | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/pkg/flux/helm.go b/pkg/flux/helm.go index 565603f..dfe230e 100644 --- a/pkg/flux/helm.go +++ b/pkg/flux/helm.go @@ -78,17 +78,18 @@ func ExtractHelmValues(values *apiextensionsv1.JSON) (*HelmValues, error) { // AddCAToHelmValues removes conflicting volumes, volumeMounts and envVars (matching by name and/or mountPath) and // adds a volume, volumeMount and envVar on all Flux controller helm values sections to import the custom CA certificate. +// nolint:gocyclo func AddCAToHelmValues(values *apiextensionsv1.JSON, configMap *corev1.ConfigMapKeySelector) (*apiextensionsv1.JSON, error) { if configMap == nil { return nil, errors.New("cannot add custom CA to Helm values: ConfigMapKeySelector is nil") } if configMap.Name == "" { - return nil, errors.New("cannot add custom CA to Helm values: caBundleRef.Name is unset") + return nil, errors.New("cannot add custom CA to Helm values: caBundleRef.Name must be set") } if configMap.Key == "" { - return nil, errors.New("cannot add custom CA to Helm values: caBundleRef.Key is unset") + return nil, errors.New("cannot add custom CA to Helm values: caBundleRef.Key must be set") } var root = map[string]json.RawMessage{} diff --git a/pkg/flux/helm_test.go b/pkg/flux/helm_test.go index b8dd70c..24e4632 100644 --- a/pkg/flux/helm_test.go +++ b/pkg/flux/helm_test.go @@ -344,7 +344,7 @@ func TestAddCaToHelmValues(t *testing.T) { }, { name: "returns error if configMap name is unset", - wantErr: "caBundleRef.Name is unset", + wantErr: "caBundleRef.Name must be set", caBundleRef: &corev1.ConfigMapKeySelector{ LocalObjectReference: corev1.LocalObjectReference{Name: ""}, Key: "ca.crt", @@ -352,7 +352,7 @@ func TestAddCaToHelmValues(t *testing.T) { }, { name: "returns error if configMap key is unset", - wantErr: "caBundleRef.Key is unset", + wantErr: "caBundleRef.Key must be set", caBundleRef: &corev1.ConfigMapKeySelector{ LocalObjectReference: corev1.LocalObjectReference{Name: "custom-ca-configmap"}, Key: "",