Skip to content

Commit 278d27f

Browse files
anik120anik120
committed
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)[#2454]: Config API and JSON schema validation - (PR2469)[#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 ```
1 parent e41eb44 commit 278d27f

2 files changed

Lines changed: 268 additions & 10 deletions

File tree

‎internal/operator-controller/applier/provider.go‎

Lines changed: 56 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414

1515
ocv1 "github.com/operator-framework/operator-controller/api/v1"
1616
"github.com/operator-framework/operator-controller/internal/operator-controller/config"
17+
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle"
1718
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle/source"
1819
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render"
1920
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
7071
}
7172

7273
if r.IsSingleOwnNamespaceEnabled {
73-
schema, err := rv1.GetConfigSchema()
74+
configOpts, err := r.extractBundleConfigOptions(&rv1, ext)
7475
if err != nil {
75-
return nil, fmt.Errorf("error getting configuration schema: %w", err)
76+
return nil, err
7677
}
78+
opts = append(opts, configOpts...)
79+
}
80+
return r.BundleRenderer.Render(rv1, ext.Spec.Namespace, opts...)
81+
}
7782

78-
bundleConfigBytes := extensionConfigBytes(ext)
79-
bundleConfig, err := config.UnmarshalConfig(bundleConfigBytes, schema, ext.Spec.Namespace)
80-
if err != nil {
81-
return nil, errorutil.NewTerminalError(ocv1.ReasonInvalidConfiguration, fmt.Errorf("invalid ClusterExtension configuration: %w", err))
82-
}
83+
// extractBundleConfigOptions extracts and validates configuration options from a ClusterExtension.
84+
// Returns render options for watchNamespace and deploymentConfig if present in the extension's configuration.
85+
func (r *RegistryV1ManifestProvider) extractBundleConfigOptions(rv1 *bundle.RegistryV1, ext *ocv1.ClusterExtension) ([]render.Option, error) {
86+
schema, err := rv1.GetConfigSchema()
87+
if err != nil {
88+
return nil, fmt.Errorf("error getting configuration schema: %w", err)
89+
}
90+
91+
bundleConfigBytes := extensionConfigBytes(ext)
92+
bundleConfig, err := config.UnmarshalConfig(bundleConfigBytes, schema, ext.Spec.Namespace)
93+
if err != nil {
94+
return nil, errorutil.NewTerminalError(ocv1.ReasonInvalidConfiguration, fmt.Errorf("invalid ClusterExtension configuration: %w", err))
95+
}
96+
97+
var opts []render.Option
98+
if watchNS := bundleConfig.GetWatchNamespace(); watchNS != nil {
99+
opts = append(opts, render.WithTargetNamespaces(*watchNS))
100+
}
83101

84-
if watchNS := bundleConfig.GetWatchNamespace(); watchNS != nil {
85-
opts = append(opts, render.WithTargetNamespaces(*watchNS))
102+
// Extract and convert deploymentConfig if present
103+
if deploymentConfigMap := bundleConfig.GetDeploymentConfig(); deploymentConfigMap != nil {
104+
deploymentConfig, err := convertToDeploymentConfig(deploymentConfigMap)
105+
if err != nil {
106+
return nil, errorutil.NewTerminalError(ocv1.ReasonInvalidConfiguration, fmt.Errorf("invalid deploymentConfig: %w", err))
86107
}
108+
opts = append(opts, render.WithDeploymentConfig(deploymentConfig))
87109
}
88-
return r.BundleRenderer.Render(rv1, ext.Spec.Namespace, opts...)
110+
111+
return opts, nil
89112
}
90113

91114
// RegistryV1HelmChartProvider creates a Helm-Chart from a registry+v1 bundle and its associated ClusterExtension
@@ -149,6 +172,29 @@ func extensionConfigBytes(ext *ocv1.ClusterExtension) []byte {
149172
return nil
150173
}
151174

175+
// convertToDeploymentConfig converts a map[string]any (from validated bundle config)
176+
// to a *config.DeploymentConfig struct that can be passed to the renderer.
177+
// Returns nil if the map is empty.
178+
func convertToDeploymentConfig(deploymentConfigMap map[string]any) (*config.DeploymentConfig, error) {
179+
if len(deploymentConfigMap) == 0 {
180+
return nil, nil
181+
}
182+
183+
// Marshal the map to JSON
184+
data, err := json.Marshal(deploymentConfigMap)
185+
if err != nil {
186+
return nil, fmt.Errorf("failed to marshal deploymentConfig: %w", err)
187+
}
188+
189+
// Unmarshal into the DeploymentConfig struct
190+
var deploymentConfig config.DeploymentConfig
191+
if err := json.Unmarshal(data, &deploymentConfig); err != nil {
192+
return nil, fmt.Errorf("failed to unmarshal deploymentConfig: %w", err)
193+
}
194+
195+
return &deploymentConfig, nil
196+
}
197+
152198
func getBundleAnnotations(bundleFS fs.FS) (map[string]string, error) {
153199
// The need to get the underlying bundle in order to extract its annotations
154200
// will go away once we have a bundle interface that can surface the annotations independently of the

‎internal/operator-controller/applier/provider_test.go‎

Lines changed: 212 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -444,6 +444,218 @@ func Test_RegistryV1ManifestProvider_SingleOwnNamespaceSupport(t *testing.T) {
444444
})
445445
}
446446

447+
func Test_RegistryV1ManifestProvider_DeploymentConfig(t *testing.T) {
448+
t.Run("passes deploymentConfig to renderer when provided in configuration", func(t *testing.T) {
449+
expectedEnvVars := []corev1.EnvVar{
450+
{Name: "TEST_ENV", Value: "test-value"},
451+
}
452+
provider := applier.RegistryV1ManifestProvider{
453+
BundleRenderer: render.BundleRenderer{
454+
ResourceGenerators: []render.ResourceGenerator{
455+
func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) {
456+
t.Log("ensure deploymentConfig is passed to renderer")
457+
require.NotNil(t, opts.DeploymentConfig)
458+
require.Equal(t, expectedEnvVars, opts.DeploymentConfig.Env)
459+
return nil, nil
460+
},
461+
},
462+
},
463+
IsSingleOwnNamespaceEnabled: true,
464+
}
465+
466+
bundleFS := bundlefs.Builder().WithPackageName("test").
467+
WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces).Build()).Build()
468+
469+
_, err := provider.Get(bundleFS, &ocv1.ClusterExtension{
470+
Spec: ocv1.ClusterExtensionSpec{
471+
Namespace: "install-namespace",
472+
Config: &ocv1.ClusterExtensionConfig{
473+
ConfigType: ocv1.ClusterExtensionConfigTypeInline,
474+
Inline: &apiextensionsv1.JSON{
475+
Raw: []byte(`{"deploymentConfig": {"env": [{"name": "TEST_ENV", "value": "test-value"}]}}`),
476+
},
477+
},
478+
},
479+
})
480+
require.NoError(t, err)
481+
})
482+
483+
t.Run("does not pass deploymentConfig to renderer when not provided in configuration", func(t *testing.T) {
484+
provider := applier.RegistryV1ManifestProvider{
485+
BundleRenderer: render.BundleRenderer{
486+
ResourceGenerators: []render.ResourceGenerator{
487+
func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) {
488+
t.Log("ensure deploymentConfig is nil when not provided")
489+
require.Nil(t, opts.DeploymentConfig)
490+
return nil, nil
491+
},
492+
},
493+
},
494+
IsSingleOwnNamespaceEnabled: true,
495+
}
496+
497+
bundleFS := bundlefs.Builder().WithPackageName("test").
498+
WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces).Build()).Build()
499+
500+
_, err := provider.Get(bundleFS, &ocv1.ClusterExtension{
501+
Spec: ocv1.ClusterExtensionSpec{
502+
Namespace: "install-namespace",
503+
// No config provided
504+
},
505+
})
506+
require.NoError(t, err)
507+
})
508+
509+
t.Run("passes deploymentConfig with multiple fields to renderer", func(t *testing.T) {
510+
expectedNodeSelector := map[string]string{"kubernetes.io/os": "linux"}
511+
expectedTolerations := []corev1.Toleration{
512+
{Key: "key1", Operator: "Equal", Value: "value1", Effect: "NoSchedule"},
513+
}
514+
provider := applier.RegistryV1ManifestProvider{
515+
BundleRenderer: render.BundleRenderer{
516+
ResourceGenerators: []render.ResourceGenerator{
517+
func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) {
518+
t.Log("ensure all deploymentConfig fields are passed to renderer")
519+
require.NotNil(t, opts.DeploymentConfig)
520+
require.Equal(t, expectedNodeSelector, opts.DeploymentConfig.NodeSelector)
521+
require.Equal(t, expectedTolerations, opts.DeploymentConfig.Tolerations)
522+
return nil, nil
523+
},
524+
},
525+
},
526+
IsSingleOwnNamespaceEnabled: true,
527+
}
528+
529+
bundleFS := bundlefs.Builder().WithPackageName("test").
530+
WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces).Build()).Build()
531+
532+
_, err := provider.Get(bundleFS, &ocv1.ClusterExtension{
533+
Spec: ocv1.ClusterExtensionSpec{
534+
Namespace: "install-namespace",
535+
Config: &ocv1.ClusterExtensionConfig{
536+
ConfigType: ocv1.ClusterExtensionConfigTypeInline,
537+
Inline: &apiextensionsv1.JSON{
538+
Raw: []byte(`{
539+
"deploymentConfig": {
540+
"nodeSelector": {"kubernetes.io/os": "linux"},
541+
"tolerations": [{"key": "key1", "operator": "Equal", "value": "value1", "effect": "NoSchedule"}]
542+
}
543+
}`),
544+
},
545+
},
546+
},
547+
})
548+
require.NoError(t, err)
549+
})
550+
551+
t.Run("passes both watchNamespace and deploymentConfig when both provided", func(t *testing.T) {
552+
expectedWatchNamespace := "some-namespace"
553+
expectedEnvVars := []corev1.EnvVar{
554+
{Name: "TEST_ENV", Value: "test-value"},
555+
}
556+
provider := applier.RegistryV1ManifestProvider{
557+
BundleRenderer: render.BundleRenderer{
558+
ResourceGenerators: []render.ResourceGenerator{
559+
func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) {
560+
t.Log("ensure both watchNamespace and deploymentConfig are passed to renderer")
561+
require.Equal(t, []string{expectedWatchNamespace}, opts.TargetNamespaces)
562+
require.NotNil(t, opts.DeploymentConfig)
563+
require.Equal(t, expectedEnvVars, opts.DeploymentConfig.Env)
564+
return nil, nil
565+
},
566+
},
567+
},
568+
IsSingleOwnNamespaceEnabled: true,
569+
}
570+
571+
bundleFS := bundlefs.Builder().WithPackageName("test").
572+
WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeSingleNamespace).Build()).Build()
573+
574+
_, err := provider.Get(bundleFS, &ocv1.ClusterExtension{
575+
Spec: ocv1.ClusterExtensionSpec{
576+
Namespace: "install-namespace",
577+
Config: &ocv1.ClusterExtensionConfig{
578+
ConfigType: ocv1.ClusterExtensionConfigTypeInline,
579+
Inline: &apiextensionsv1.JSON{
580+
Raw: []byte(`{
581+
"watchNamespace": "some-namespace",
582+
"deploymentConfig": {
583+
"env": [{"name": "TEST_ENV", "value": "test-value"}]
584+
}
585+
}`),
586+
},
587+
},
588+
},
589+
})
590+
require.NoError(t, err)
591+
})
592+
593+
t.Run("handles empty deploymentConfig gracefully", func(t *testing.T) {
594+
provider := applier.RegistryV1ManifestProvider{
595+
BundleRenderer: render.BundleRenderer{
596+
ResourceGenerators: []render.ResourceGenerator{
597+
func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) {
598+
t.Log("ensure deploymentConfig is nil for empty config object")
599+
require.Nil(t, opts.DeploymentConfig)
600+
return nil, nil
601+
},
602+
},
603+
},
604+
IsSingleOwnNamespaceEnabled: true,
605+
}
606+
607+
bundleFS := bundlefs.Builder().WithPackageName("test").
608+
WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces).Build()).Build()
609+
610+
_, err := provider.Get(bundleFS, &ocv1.ClusterExtension{
611+
Spec: ocv1.ClusterExtensionSpec{
612+
Namespace: "install-namespace",
613+
Config: &ocv1.ClusterExtensionConfig{
614+
ConfigType: ocv1.ClusterExtensionConfigTypeInline,
615+
Inline: &apiextensionsv1.JSON{
616+
Raw: []byte(`{"deploymentConfig": {}}`),
617+
},
618+
},
619+
},
620+
})
621+
require.NoError(t, err)
622+
})
623+
624+
t.Run("returns terminal error when deploymentConfig has invalid structure", func(t *testing.T) {
625+
provider := applier.RegistryV1ManifestProvider{
626+
BundleRenderer: render.BundleRenderer{
627+
ResourceGenerators: []render.ResourceGenerator{
628+
func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) {
629+
return nil, nil
630+
},
631+
},
632+
},
633+
IsSingleOwnNamespaceEnabled: true,
634+
}
635+
636+
bundleFS := bundlefs.Builder().WithPackageName("test").
637+
WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces).Build()).Build()
638+
639+
// Provide deploymentConfig with invalid structure that will fail JSON unmarshaling
640+
_, err := provider.Get(bundleFS, &ocv1.ClusterExtension{
641+
Spec: ocv1.ClusterExtensionSpec{
642+
Namespace: "install-namespace",
643+
Config: &ocv1.ClusterExtensionConfig{
644+
ConfigType: ocv1.ClusterExtensionConfigTypeInline,
645+
Inline: &apiextensionsv1.JSON{
646+
// env should be an array of objects, but we provide an invalid string value
647+
// This will pass JSON schema validation (if schema allows it) but fail unmarshaling
648+
Raw: []byte(`{"deploymentConfig": {"env": "not-an-array"}}`),
649+
},
650+
},
651+
},
652+
})
653+
require.Error(t, err)
654+
require.Contains(t, err.Error(), "invalid deploymentConfig")
655+
require.ErrorIs(t, err, reconcile.TerminalError(nil), "deploymentConfig conversion errors should be terminal")
656+
})
657+
}
658+
447659
func Test_RegistryV1HelmChartProvider_Integration(t *testing.T) {
448660
t.Run("surfaces bundle source errors", func(t *testing.T) {
449661
provider := applier.RegistryV1HelmChartProvider{

0 commit comments

Comments
 (0)