From d7cb7029e34b3073bd50652acd2b4f64eea7bc5b Mon Sep 17 00:00:00 2001 From: Anik Bhattacharjee Date: Tue, 3 Feb 2026 11:56:37 -0500 Subject: [PATCH] feat: Extract and pass deploymentConfig to bundle renderer **Summary** Completes Phase 3 of the [Deployment Configuration RFC](https://docs.google.com/document/d/1bDo3W1asZqjJTgZy7BcVOGtKOEukp0vUi5CO1ac3vwc/edit?usp=sharing). Extracts `deploymentConfig` from validated ClusterExtension configuration and passes it to the bundle renderer. Builds on: - (PR2454)[https://github.com/operator-framework/operator-controller/pull/2454]: Config API and JSON schema validation - (PR2469)[https://github.com/operator-framework/operator-controller/pull/2469]: Renderer support for applying deployment customizations This completes the RFC - users can now customize operator deployments via ClusterExtension.spec.config.inline: ``` config: configType: Inline inline: watchNamespace: "some-namespace" deploymentConfig: env: - name: TEST_ENV value: test-value nodeSelector: kubernetes.io/os: linux ``` --- .../operator-controller/applier/provider.go | 66 +++++- .../applier/provider_test.go | 212 ++++++++++++++++++ 2 files changed, 268 insertions(+), 10 deletions(-) diff --git a/internal/operator-controller/applier/provider.go b/internal/operator-controller/applier/provider.go index 39461d26a..4602803ab 100644 --- a/internal/operator-controller/applier/provider.go +++ b/internal/operator-controller/applier/provider.go @@ -14,6 +14,7 @@ import ( ocv1 "github.com/operator-framework/operator-controller/api/v1" "github.com/operator-framework/operator-controller/internal/operator-controller/config" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle/source" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" errorutil "github.com/operator-framework/operator-controller/internal/shared/util/error" @@ -70,22 +71,44 @@ func (r *RegistryV1ManifestProvider) Get(bundleFS fs.FS, ext *ocv1.ClusterExtens } if r.IsSingleOwnNamespaceEnabled { - schema, err := rv1.GetConfigSchema() + configOpts, err := r.extractBundleConfigOptions(&rv1, ext) if err != nil { - return nil, fmt.Errorf("error getting configuration schema: %w", err) + return nil, err } + opts = append(opts, configOpts...) + } + return r.BundleRenderer.Render(rv1, ext.Spec.Namespace, opts...) +} - bundleConfigBytes := extensionConfigBytes(ext) - bundleConfig, err := config.UnmarshalConfig(bundleConfigBytes, schema, ext.Spec.Namespace) - if err != nil { - return nil, errorutil.NewTerminalError(ocv1.ReasonInvalidConfiguration, fmt.Errorf("invalid ClusterExtension configuration: %w", err)) - } +// extractBundleConfigOptions extracts and validates configuration options from a ClusterExtension. +// Returns render options for watchNamespace and deploymentConfig if present in the extension's configuration. +func (r *RegistryV1ManifestProvider) extractBundleConfigOptions(rv1 *bundle.RegistryV1, ext *ocv1.ClusterExtension) ([]render.Option, error) { + schema, err := rv1.GetConfigSchema() + if err != nil { + return nil, fmt.Errorf("error getting configuration schema: %w", err) + } + + bundleConfigBytes := extensionConfigBytes(ext) + bundleConfig, err := config.UnmarshalConfig(bundleConfigBytes, schema, ext.Spec.Namespace) + if err != nil { + return nil, errorutil.NewTerminalError(ocv1.ReasonInvalidConfiguration, fmt.Errorf("invalid ClusterExtension configuration: %w", err)) + } + + var opts []render.Option + if watchNS := bundleConfig.GetWatchNamespace(); watchNS != nil { + opts = append(opts, render.WithTargetNamespaces(*watchNS)) + } - if watchNS := bundleConfig.GetWatchNamespace(); watchNS != nil { - opts = append(opts, render.WithTargetNamespaces(*watchNS)) + // Extract and convert deploymentConfig if present + if deploymentConfigMap := bundleConfig.GetDeploymentConfig(); deploymentConfigMap != nil { + deploymentConfig, err := convertToDeploymentConfig(deploymentConfigMap) + if err != nil { + return nil, errorutil.NewTerminalError(ocv1.ReasonInvalidConfiguration, fmt.Errorf("invalid deploymentConfig: %w", err)) } + opts = append(opts, render.WithDeploymentConfig(deploymentConfig)) } - return r.BundleRenderer.Render(rv1, ext.Spec.Namespace, opts...) + + return opts, nil } // RegistryV1HelmChartProvider creates a Helm-Chart from a registry+v1 bundle and its associated ClusterExtension @@ -149,6 +172,29 @@ func extensionConfigBytes(ext *ocv1.ClusterExtension) []byte { return nil } +// convertToDeploymentConfig converts a map[string]any (from validated bundle config) +// to a *config.DeploymentConfig struct that can be passed to the renderer. +// Returns nil if the map is empty. +func convertToDeploymentConfig(deploymentConfigMap map[string]any) (*config.DeploymentConfig, error) { + if len(deploymentConfigMap) == 0 { + return nil, nil + } + + // Marshal the map to JSON + data, err := json.Marshal(deploymentConfigMap) + if err != nil { + return nil, fmt.Errorf("failed to marshal deploymentConfig: %w", err) + } + + // Unmarshal into the DeploymentConfig struct + var deploymentConfig config.DeploymentConfig + if err := json.Unmarshal(data, &deploymentConfig); err != nil { + return nil, fmt.Errorf("failed to unmarshal deploymentConfig: %w", err) + } + + return &deploymentConfig, nil +} + func getBundleAnnotations(bundleFS fs.FS) (map[string]string, error) { // The need to get the underlying bundle in order to extract its annotations // will go away once we have a bundle interface that can surface the annotations independently of the diff --git a/internal/operator-controller/applier/provider_test.go b/internal/operator-controller/applier/provider_test.go index c551432e5..26d565209 100644 --- a/internal/operator-controller/applier/provider_test.go +++ b/internal/operator-controller/applier/provider_test.go @@ -444,6 +444,218 @@ func Test_RegistryV1ManifestProvider_SingleOwnNamespaceSupport(t *testing.T) { }) } +func Test_RegistryV1ManifestProvider_DeploymentConfig(t *testing.T) { + t.Run("passes deploymentConfig to renderer when provided in configuration", func(t *testing.T) { + expectedEnvVars := []corev1.EnvVar{ + {Name: "TEST_ENV", Value: "test-value"}, + } + provider := applier.RegistryV1ManifestProvider{ + BundleRenderer: render.BundleRenderer{ + ResourceGenerators: []render.ResourceGenerator{ + func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { + t.Log("ensure deploymentConfig is passed to renderer") + require.NotNil(t, opts.DeploymentConfig) + require.Equal(t, expectedEnvVars, opts.DeploymentConfig.Env) + return nil, nil + }, + }, + }, + IsSingleOwnNamespaceEnabled: true, + } + + bundleFS := bundlefs.Builder().WithPackageName("test"). + WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces).Build()).Build() + + _, err := provider.Get(bundleFS, &ocv1.ClusterExtension{ + Spec: ocv1.ClusterExtensionSpec{ + Namespace: "install-namespace", + Config: &ocv1.ClusterExtensionConfig{ + ConfigType: ocv1.ClusterExtensionConfigTypeInline, + Inline: &apiextensionsv1.JSON{ + Raw: []byte(`{"deploymentConfig": {"env": [{"name": "TEST_ENV", "value": "test-value"}]}}`), + }, + }, + }, + }) + require.NoError(t, err) + }) + + t.Run("does not pass deploymentConfig to renderer when not provided in configuration", func(t *testing.T) { + provider := applier.RegistryV1ManifestProvider{ + BundleRenderer: render.BundleRenderer{ + ResourceGenerators: []render.ResourceGenerator{ + func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { + t.Log("ensure deploymentConfig is nil when not provided") + require.Nil(t, opts.DeploymentConfig) + return nil, nil + }, + }, + }, + IsSingleOwnNamespaceEnabled: true, + } + + bundleFS := bundlefs.Builder().WithPackageName("test"). + WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces).Build()).Build() + + _, err := provider.Get(bundleFS, &ocv1.ClusterExtension{ + Spec: ocv1.ClusterExtensionSpec{ + Namespace: "install-namespace", + // No config provided + }, + }) + require.NoError(t, err) + }) + + t.Run("passes deploymentConfig with multiple fields to renderer", func(t *testing.T) { + expectedNodeSelector := map[string]string{"kubernetes.io/os": "linux"} + expectedTolerations := []corev1.Toleration{ + {Key: "key1", Operator: "Equal", Value: "value1", Effect: "NoSchedule"}, + } + provider := applier.RegistryV1ManifestProvider{ + BundleRenderer: render.BundleRenderer{ + ResourceGenerators: []render.ResourceGenerator{ + func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { + t.Log("ensure all deploymentConfig fields are passed to renderer") + require.NotNil(t, opts.DeploymentConfig) + require.Equal(t, expectedNodeSelector, opts.DeploymentConfig.NodeSelector) + require.Equal(t, expectedTolerations, opts.DeploymentConfig.Tolerations) + return nil, nil + }, + }, + }, + IsSingleOwnNamespaceEnabled: true, + } + + bundleFS := bundlefs.Builder().WithPackageName("test"). + WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces).Build()).Build() + + _, err := provider.Get(bundleFS, &ocv1.ClusterExtension{ + Spec: ocv1.ClusterExtensionSpec{ + Namespace: "install-namespace", + Config: &ocv1.ClusterExtensionConfig{ + ConfigType: ocv1.ClusterExtensionConfigTypeInline, + Inline: &apiextensionsv1.JSON{ + Raw: []byte(`{ + "deploymentConfig": { + "nodeSelector": {"kubernetes.io/os": "linux"}, + "tolerations": [{"key": "key1", "operator": "Equal", "value": "value1", "effect": "NoSchedule"}] + } + }`), + }, + }, + }, + }) + require.NoError(t, err) + }) + + t.Run("passes both watchNamespace and deploymentConfig when both provided", func(t *testing.T) { + expectedWatchNamespace := "some-namespace" + expectedEnvVars := []corev1.EnvVar{ + {Name: "TEST_ENV", Value: "test-value"}, + } + provider := applier.RegistryV1ManifestProvider{ + BundleRenderer: render.BundleRenderer{ + ResourceGenerators: []render.ResourceGenerator{ + func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { + t.Log("ensure both watchNamespace and deploymentConfig are passed to renderer") + require.Equal(t, []string{expectedWatchNamespace}, opts.TargetNamespaces) + require.NotNil(t, opts.DeploymentConfig) + require.Equal(t, expectedEnvVars, opts.DeploymentConfig.Env) + return nil, nil + }, + }, + }, + IsSingleOwnNamespaceEnabled: true, + } + + bundleFS := bundlefs.Builder().WithPackageName("test"). + WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeSingleNamespace).Build()).Build() + + _, err := provider.Get(bundleFS, &ocv1.ClusterExtension{ + Spec: ocv1.ClusterExtensionSpec{ + Namespace: "install-namespace", + Config: &ocv1.ClusterExtensionConfig{ + ConfigType: ocv1.ClusterExtensionConfigTypeInline, + Inline: &apiextensionsv1.JSON{ + Raw: []byte(`{ + "watchNamespace": "some-namespace", + "deploymentConfig": { + "env": [{"name": "TEST_ENV", "value": "test-value"}] + } + }`), + }, + }, + }, + }) + require.NoError(t, err) + }) + + t.Run("handles empty deploymentConfig gracefully", func(t *testing.T) { + provider := applier.RegistryV1ManifestProvider{ + BundleRenderer: render.BundleRenderer{ + ResourceGenerators: []render.ResourceGenerator{ + func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { + t.Log("ensure deploymentConfig is nil for empty config object") + require.Nil(t, opts.DeploymentConfig) + return nil, nil + }, + }, + }, + IsSingleOwnNamespaceEnabled: true, + } + + bundleFS := bundlefs.Builder().WithPackageName("test"). + WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces).Build()).Build() + + _, err := provider.Get(bundleFS, &ocv1.ClusterExtension{ + Spec: ocv1.ClusterExtensionSpec{ + Namespace: "install-namespace", + Config: &ocv1.ClusterExtensionConfig{ + ConfigType: ocv1.ClusterExtensionConfigTypeInline, + Inline: &apiextensionsv1.JSON{ + Raw: []byte(`{"deploymentConfig": {}}`), + }, + }, + }, + }) + require.NoError(t, err) + }) + + t.Run("returns terminal error when deploymentConfig has invalid structure", func(t *testing.T) { + provider := applier.RegistryV1ManifestProvider{ + BundleRenderer: render.BundleRenderer{ + ResourceGenerators: []render.ResourceGenerator{ + func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { + return nil, nil + }, + }, + }, + IsSingleOwnNamespaceEnabled: true, + } + + bundleFS := bundlefs.Builder().WithPackageName("test"). + WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces).Build()).Build() + + // Provide deploymentConfig with invalid structure - env should be array, not string + // Schema validation catches this before conversion + _, err := provider.Get(bundleFS, &ocv1.ClusterExtension{ + Spec: ocv1.ClusterExtensionSpec{ + Namespace: "install-namespace", + Config: &ocv1.ClusterExtensionConfig{ + ConfigType: ocv1.ClusterExtensionConfigTypeInline, + Inline: &apiextensionsv1.JSON{ + Raw: []byte(`{"deploymentConfig": {"env": "not-an-array"}}`), + }, + }, + }, + }) + require.Error(t, err) + require.Contains(t, err.Error(), "invalid ClusterExtension configuration") + require.Contains(t, err.Error(), "deploymentConfig.env") + require.ErrorIs(t, err, reconcile.TerminalError(nil), "config validation errors should be terminal") + }) +} + func Test_RegistryV1HelmChartProvider_Integration(t *testing.T) { t.Run("surfaces bundle source errors", func(t *testing.T) { provider := applier.RegistryV1HelmChartProvider{