From 9d6c63c6dfd462183645c254f13a659cdf7e7515 Mon Sep 17 00:00:00 2001 From: Brett Tofel Date: Wed, 30 Jul 2025 16:09:59 -0400 Subject: [PATCH 1/9] Initial work to configuration PoC Signed-off-by: Brett Tofel --- api/v1/clusterextension_types.go | 5 ++ cmd/demo-config/main.go | 56 +++++++++++++++++++ .../testdata/api/v1/clusterextension_types.go | 5 ++ .../registryv1/generators/generators.go | 25 +++++++++ .../rukpak/render/render.go | 9 +++ test/regression/convert/generate-manifests.go | 16 +++++- .../bundles/test-operator/v2.0.0/config.yaml | 2 + 7 files changed, 116 insertions(+), 2 deletions(-) create mode 100644 cmd/demo-config/main.go create mode 100644 testdata/images/bundles/test-operator/v2.0.0/config.yaml diff --git a/api/v1/clusterextension_types.go b/api/v1/clusterextension_types.go index 0141f1a7a4..3f43f7ccde 100644 --- a/api/v1/clusterextension_types.go +++ b/api/v1/clusterextension_types.go @@ -92,6 +92,11 @@ type ClusterExtensionSpec struct { // // +optional Install *ClusterExtensionInstallConfig `json:"install,omitempty"` + + // config contains arbitrary configuration values to be applied at render time. + // These values will be merged into the bundle manifests during rendering. + // +optional + Config map[string]interface{} `json:"config,omitempty"` } const SourceTypeCatalog = "Catalog" diff --git a/cmd/demo-config/main.go b/cmd/demo-config/main.go new file mode 100644 index 0000000000..9578931c09 --- /dev/null +++ b/cmd/demo-config/main.go @@ -0,0 +1,56 @@ +package main + +import ( + "fmt" + "os" + "path/filepath" + + "sigs.k8s.io/yaml" + + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle/source" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/registryv1" +) + +func main() { + if len(os.Args) != 3 { + fmt.Fprintf(os.Stderr, "usage: %s \n", os.Args[0]) + os.Exit(1) + } + bundleDir := os.Args[1] + installNs := os.Args[2] + + // load optional configuration values from config.yaml + cfg := map[string]interface{}{} + configFile := filepath.Join(bundleDir, "config.yaml") + if data, err := os.ReadFile(configFile); err == nil { + if err := yaml.Unmarshal(data, &cfg); err != nil { + fmt.Fprintf(os.Stderr, "error unmarshalling config file %q: %v\n", configFile, err) + os.Exit(1) + } + } + + // parse registry+v1 bundle + regv1, err := source.FromFS(os.DirFS(bundleDir)).GetBundle() + if err != nil { + fmt.Fprintf(os.Stderr, "error parsing bundle directory: %v\n", err) + os.Exit(1) + } + + // render bundle with configuration + opts := []render.Option{render.WithConfig(cfg)} + objs, err := registryv1.Renderer.Render(regv1, installNs, opts...) + if err != nil { + fmt.Fprintf(os.Stderr, "error rendering bundle: %v\n", err) + os.Exit(1) + } + + for _, obj := range objs { + data, err := yaml.Marshal(obj) + if err != nil { + fmt.Fprintf(os.Stderr, "error marshaling object: %v\n", err) + os.Exit(1) + } + fmt.Printf("---\n%s", string(data)) + } +} diff --git a/hack/tools/crd-generator/testdata/api/v1/clusterextension_types.go b/hack/tools/crd-generator/testdata/api/v1/clusterextension_types.go index c5c04d5b93..4056138e25 100644 --- a/hack/tools/crd-generator/testdata/api/v1/clusterextension_types.go +++ b/hack/tools/crd-generator/testdata/api/v1/clusterextension_types.go @@ -93,6 +93,11 @@ type ClusterExtensionSpec struct { // // +optional Install *ClusterExtensionInstallConfig `json:"install,omitempty"` + + // config contains arbitrary configuration values to be applied at render time. + // These values will be merged into the bundle manifests during rendering. + // +optional + Config map[string]interface{} `json:"config,omitempty"` } const SourceTypeCatalog = "Catalog" diff --git a/internal/operator-controller/rukpak/render/registryv1/generators/generators.go b/internal/operator-controller/rukpak/render/registryv1/generators/generators.go index 233793308a..ddd9fc0ff6 100644 --- a/internal/operator-controller/rukpak/render/registryv1/generators/generators.go +++ b/internal/operator-controller/rukpak/render/registryv1/generators/generators.go @@ -13,6 +13,7 @@ import ( rbacv1 "k8s.io/api/rbac/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/utils/ptr" @@ -296,6 +297,30 @@ func BundleAdditionalResourcesGenerator(rv1 *bundle.RegistryV1, opts render.Opti objs = append(objs, obj) } + // apply overrides from configuration, if present + if opts.Config != nil { + for _, obj := range objs { + if obj.GetObjectKind().GroupVersionKind().Kind == "ConfigMap" { + u, ok := obj.(*unstructured.Unstructured) + if !ok { + continue + } + data, found, err := unstructured.NestedStringMap(u.Object, "data") + if err != nil { + return nil, err + } + if !found { + data = map[string]string{} + } + for k, v := range opts.Config { + data[k] = fmt.Sprintf("%v", v) + } + if err := unstructured.SetNestedStringMap(u.Object, data, "data"); err != nil { + return nil, err + } + } + } + } return objs, nil } diff --git a/internal/operator-controller/rukpak/render/render.go b/internal/operator-controller/rukpak/render/render.go index 70063f1d48..9bf8827a3e 100644 --- a/internal/operator-controller/rukpak/render/render.go +++ b/internal/operator-controller/rukpak/render/render.go @@ -61,6 +61,8 @@ type Options struct { TargetNamespaces []string UniqueNameGenerator UniqueNameGenerator CertificateProvider CertificateProvider + // Config holds arbitrary configuration values provided at render time + Config map[string]interface{} } func (o *Options) apply(opts ...Option) *Options { @@ -94,6 +96,13 @@ func WithTargetNamespaces(namespaces ...string) Option { } } +// WithConfig supplies configuration values to be used during bundle rendering +func WithConfig(cfg map[string]interface{}) Option { + return func(o *Options) { + o.Config = cfg + } +} + func WithUniqueNameGenerator(generator UniqueNameGenerator) Option { return func(o *Options) { o.UniqueNameGenerator = generator diff --git a/test/regression/convert/generate-manifests.go b/test/regression/convert/generate-manifests.go index b2a6565505..044a9ddd67 100644 --- a/test/regression/convert/generate-manifests.go +++ b/test/regression/convert/generate-manifests.go @@ -96,8 +96,20 @@ func generateManifests(outputPath, bundleDir, installNamespace, watchNamespace s os.Exit(1) } - // Convert RegistryV1 to plain manifests - objs, err := registryv1.Renderer.Render(regv1, installNamespace, render.WithTargetNamespaces(watchNamespace)) + // load optional configuration for registry+v1 bundle if present + cfg := map[string]interface{}{} + configFile := filepath.Join(bundleDir, "config.yaml") + if data, err := os.ReadFile(configFile); err == nil { + if err := yaml.Unmarshal(data, &cfg); err != nil { + return fmt.Errorf("error unmarshalling config file %q: %w", configFile, err) + } + } + // Convert RegistryV1 to plain manifests, applying any configuration + opts := []render.Option{render.WithTargetNamespaces(watchNamespace)} + if cfg != nil { + opts = append(opts, render.WithConfig(cfg)) + } + objs, err := registryv1.Renderer.Render(regv1, installNamespace, opts...) if err != nil { return fmt.Errorf("error converting registry+v1 bundle: %w", err) } diff --git a/testdata/images/bundles/test-operator/v2.0.0/config.yaml b/testdata/images/bundles/test-operator/v2.0.0/config.yaml new file mode 100644 index 0000000000..2f74f97587 --- /dev/null +++ b/testdata/images/bundles/test-operator/v2.0.0/config.yaml @@ -0,0 +1,2 @@ +version: "v2.0.0-demo" +name: "demo-configmap" From 269a9eec9616c592b7243c341067838816863669 Mon Sep 17 00:00:00 2001 From: Brett Tofel Date: Thu, 31 Jul 2025 14:56:39 -0400 Subject: [PATCH 2/9] Add helm chart and value processing to demo Signed-off-by: Brett Tofel --- cmd/demo-config/main.go | 70 +++++++- .../charts/test-operator/v2.0.0/Chart.yaml | 4 + .../charts/test-operator/v2.0.0/config.yaml | 2 + .../v2.0.0/templates/configmap.yaml | 7 + .../olm.operatorframework.com_olme2etest.yaml | 28 ++++ .../testoperator.clusterserviceversion.yaml | 151 ++++++++++++++++++ .../templates/testoperator.networkpolicy.yaml | 8 + .../charts/test-operator/v2.0.0/values.yaml | 2 + 8 files changed, 268 insertions(+), 4 deletions(-) create mode 100644 testdata/images/charts/test-operator/v2.0.0/Chart.yaml create mode 100644 testdata/images/charts/test-operator/v2.0.0/config.yaml create mode 100644 testdata/images/charts/test-operator/v2.0.0/templates/configmap.yaml create mode 100644 testdata/images/charts/test-operator/v2.0.0/templates/olm.operatorframework.com_olme2etest.yaml create mode 100644 testdata/images/charts/test-operator/v2.0.0/templates/testoperator.clusterserviceversion.yaml create mode 100644 testdata/images/charts/test-operator/v2.0.0/templates/testoperator.networkpolicy.yaml create mode 100644 testdata/images/charts/test-operator/v2.0.0/values.yaml diff --git a/cmd/demo-config/main.go b/cmd/demo-config/main.go index 9578931c09..625915fd38 100644 --- a/cmd/demo-config/main.go +++ b/cmd/demo-config/main.go @@ -1,24 +1,32 @@ package main import ( + "flag" "fmt" "os" "path/filepath" + "strings" "sigs.k8s.io/yaml" + "helm.sh/helm/v3/pkg/chart/loader" + "helm.sh/helm/v3/pkg/chartutil" + "helm.sh/helm/v3/pkg/engine" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle/source" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/registryv1" ) func main() { - if len(os.Args) != 3 { - fmt.Fprintf(os.Stderr, "usage: %s \n", os.Args[0]) + helmFlag := flag.Bool("helm", false, "render as Helm chart instead of registry+v1 bundle") + flag.Parse() + if flag.NArg() != 2 { + fmt.Fprintf(os.Stderr, "usage: %s [-helm] \n", os.Args[0]) os.Exit(1) } - bundleDir := os.Args[1] - installNs := os.Args[2] + bundleDir := flag.Arg(0) + installNs := flag.Arg(1) // load optional configuration values from config.yaml cfg := map[string]interface{}{} @@ -30,6 +38,60 @@ func main() { } } + if *helmFlag { + // ensure a Helm chart is present + // look for Chart.yaml in the specified dir, or try replacing "bundles" -> "charts" if not found + chartPath := filepath.Join(bundleDir, "Chart.yaml") + if _, err := os.Stat(chartPath); err != nil { + // fallback: swap a "bundles" segment to "charts" in the path + parts := strings.Split(bundleDir, string(os.PathSeparator)) + for i, p := range parts { + if p == "bundles" { + parts[i] = "charts" + altDir := filepath.Join(parts...) + if _, err2 := os.Stat(filepath.Join(altDir, "Chart.yaml")); err2 == nil { + bundleDir = altDir + chartPath = filepath.Join(bundleDir, "Chart.yaml") + break + } + } + } + if _, err := os.Stat(chartPath); err != nil { + fmt.Fprintf(os.Stderr, "error: helm chart not found in %q: %v\n", bundleDir, err) + os.Exit(1) + } + } + // render with the Helm engine + chrt, err := loader.Load(bundleDir) + if err != nil { + fmt.Fprintf(os.Stderr, "error loading Helm chart: %v\n", err) + os.Exit(1) + } + // load values.yaml and apply any config overrides + values := map[string]interface{}{} + valuesFile := filepath.Join(bundleDir, "values.yaml") + if data, err := os.ReadFile(valuesFile); err == nil { + if err := yaml.Unmarshal(data, &values); err != nil { + fmt.Fprintf(os.Stderr, "error unmarshalling values file %q: %v\n", valuesFile, err) + os.Exit(1) + } + } + for k, v := range cfg { + values[k] = v + } + // render the chart templates using the provided values under the 'Values' key + renderContext := chartutil.Values{"Values": values} + rendered, err := engine.Engine{}.Render(chrt, renderContext) + if err != nil { + fmt.Fprintf(os.Stderr, "error rendering Helm chart: %v\n", err) + os.Exit(1) + } + for name, content := range rendered { + fmt.Printf("---\n# Source: %s\n%s\n", name, content) + } + return + } + // parse registry+v1 bundle regv1, err := source.FromFS(os.DirFS(bundleDir)).GetBundle() if err != nil { diff --git a/testdata/images/charts/test-operator/v2.0.0/Chart.yaml b/testdata/images/charts/test-operator/v2.0.0/Chart.yaml new file mode 100644 index 0000000000..ca6a6c3948 --- /dev/null +++ b/testdata/images/charts/test-operator/v2.0.0/Chart.yaml @@ -0,0 +1,4 @@ +apiVersion: v2 +name: test-operator +version: 2.0.0 +appVersion: v2.0.0 diff --git a/testdata/images/charts/test-operator/v2.0.0/config.yaml b/testdata/images/charts/test-operator/v2.0.0/config.yaml new file mode 100644 index 0000000000..15bbf424eb --- /dev/null +++ b/testdata/images/charts/test-operator/v2.0.0/config.yaml @@ -0,0 +1,2 @@ +version: "v2.0.0-demo" +name: "demo-configmap" diff --git a/testdata/images/charts/test-operator/v2.0.0/templates/configmap.yaml b/testdata/images/charts/test-operator/v2.0.0/templates/configmap.yaml new file mode 100644 index 0000000000..b8ad793664 --- /dev/null +++ b/testdata/images/charts/test-operator/v2.0.0/templates/configmap.yaml @@ -0,0 +1,7 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: test-configmap +data: + version: "{{ index .Values "version" }}" + name: "{{ index .Values "name" }}" diff --git a/testdata/images/charts/test-operator/v2.0.0/templates/olm.operatorframework.com_olme2etest.yaml b/testdata/images/charts/test-operator/v2.0.0/templates/olm.operatorframework.com_olme2etest.yaml new file mode 100644 index 0000000000..fcfd4aeafe --- /dev/null +++ b/testdata/images/charts/test-operator/v2.0.0/templates/olm.operatorframework.com_olme2etest.yaml @@ -0,0 +1,28 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.16.1 + name: olme2etests.olm.operatorframework.io +spec: + group: olm.operatorframework.io + names: + kind: OLME2ETest + listKind: OLME2ETestList + plural: olme2etests + singular: olme2etest + scope: Cluster + versions: + - name: v1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + testField: + type: string diff --git a/testdata/images/charts/test-operator/v2.0.0/templates/testoperator.clusterserviceversion.yaml b/testdata/images/charts/test-operator/v2.0.0/templates/testoperator.clusterserviceversion.yaml new file mode 100644 index 0000000000..7a06196f20 --- /dev/null +++ b/testdata/images/charts/test-operator/v2.0.0/templates/testoperator.clusterserviceversion.yaml @@ -0,0 +1,151 @@ +apiVersion: operators.coreos.com/v1alpha1 +kind: ClusterServiceVersion +metadata: + annotations: + alm-examples: |- + [ + { + "apiVersion": "olme2etests.olm.operatorframework.io/v1", + "kind": "OLME2ETests", + "metadata": { + "labels": { + "app.kubernetes.io/managed-by": "kustomize", + "app.kubernetes.io/name": "test" + }, + "name": "test-sample" + }, + "spec": null + } + ] + capabilities: Basic Install + createdAt: "2024-10-24T19:21:40Z" + operators.operatorframework.io/builder: operator-sdk-v1.34.1 + operators.operatorframework.io/project_layout: go.kubebuilder.io/v4 + name: testoperator.v2.0.0 + namespace: placeholder +spec: + apiservicedefinitions: {} + customresourcedefinitions: + owned: + - description: Configures subsections of Alertmanager configuration specific to each namespace + displayName: OLME2ETest + kind: OLME2ETest + name: olme2etests.olm.operatorframework.io + version: v1 + description: OLM E2E Testing Operator + displayName: test-operator + icon: + - base64data: "" + mediatype: "" + install: + spec: + deployments: + - label: + app.kubernetes.io/component: controller + app.kubernetes.io/name: test-operator + app.kubernetes.io/version: 2.0.0 + name: test-operator + spec: + replicas: 1 + selector: + matchLabels: + app: olme2etest + template: + metadata: + labels: + app: olme2etest + spec: + terminationGracePeriodSeconds: 0 + containers: + - name: busybox + image: busybox + command: + - 'sleep' + - '1000' + securityContext: + runAsUser: 1000 + runAsNonRoot: true + serviceAccountName: simple-bundle-manager + clusterPermissions: + - rules: + - apiGroups: + - authentication.k8s.io + resources: + - tokenreviews + verbs: + - create + - apiGroups: + - authorization.k8s.io + resources: + - subjectaccessreviews + verbs: + - create + serviceAccountName: simple-bundle-manager + permissions: + - rules: + - apiGroups: + - "" + resources: + - configmaps + - serviceaccounts + verbs: + - get + - list + - watch + - create + - update + - patch + - delete + - apiGroups: + - networking.k8s.io + resources: + - networkpolicies + verbs: + - get + - list + - create + - update + - delete + - apiGroups: + - coordination.k8s.io + resources: + - leases + verbs: + - get + - list + - watch + - create + - update + - patch + - delete + - apiGroups: + - "" + resources: + - events + verbs: + - create + - patch + serviceAccountName: simple-bundle-manager + strategy: deployment + installModes: + - supported: false + type: OwnNamespace + - supported: false + type: SingleNamespace + - supported: false + type: MultiNamespace + - supported: true + type: AllNamespaces + keywords: + - registry + links: + - name: simple-bundle + url: https://simple-bundle.domain + maintainers: + - email: main#simple-bundle.domain + name: Simple Bundle + maturity: beta + provider: + name: Simple Bundle + url: https://simple-bundle.domain + version: 2.0.0 diff --git a/testdata/images/charts/test-operator/v2.0.0/templates/testoperator.networkpolicy.yaml b/testdata/images/charts/test-operator/v2.0.0/templates/testoperator.networkpolicy.yaml new file mode 100644 index 0000000000..d87648e6f3 --- /dev/null +++ b/testdata/images/charts/test-operator/v2.0.0/templates/testoperator.networkpolicy.yaml @@ -0,0 +1,8 @@ +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: test-operator-network-policy +spec: + podSelector: {} + policyTypes: + - Ingress diff --git a/testdata/images/charts/test-operator/v2.0.0/values.yaml b/testdata/images/charts/test-operator/v2.0.0/values.yaml new file mode 100644 index 0000000000..547b9d31aa --- /dev/null +++ b/testdata/images/charts/test-operator/v2.0.0/values.yaml @@ -0,0 +1,2 @@ +version: "v2.0.0" +name: "test-configmap" From 16b0c62661fc06e558afa282de816168b7dadeb6 Mon Sep 17 00:00:00 2001 From: Brett Tofel Date: Fri, 1 Aug 2025 08:48:44 -0400 Subject: [PATCH 3/9] Make helm applier use the values Signed-off-by: Brett Tofel --- internal/operator-controller/applier/helm.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/operator-controller/applier/helm.go b/internal/operator-controller/applier/helm.go index ecfb3fdc2b..0653c4fbb6 100644 --- a/internal/operator-controller/applier/helm.go +++ b/internal/operator-controller/applier/helm.go @@ -126,7 +126,8 @@ func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExte if err != nil { return nil, "", err } - values := chartutil.Values{} + // merge in any user-provided config values as Helm values + values := chartutil.Values{"Values": ext.Spec.Config} post := &postrenderer{ labels: objectLabels, From 95f3135126060e1ca099d865d7b0f6a364429081 Mon Sep 17 00:00:00 2001 From: Brett Tofel Date: Fri, 1 Aug 2025 09:56:08 -0400 Subject: [PATCH 4/9] Make helm applier deal in JSON since Interface no Signed-off-by: Brett Tofel --- api/v1/clusterextension_types.go | 7 +++++-- internal/operator-controller/applier/helm.go | 11 +++++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/api/v1/clusterextension_types.go b/api/v1/clusterextension_types.go index 3f43f7ccde..c7a7ef62c2 100644 --- a/api/v1/clusterextension_types.go +++ b/api/v1/clusterextension_types.go @@ -17,6 +17,7 @@ limitations under the License. package v1 import ( + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -93,10 +94,12 @@ type ClusterExtensionSpec struct { // +optional Install *ClusterExtensionInstallConfig `json:"install,omitempty"` - // config contains arbitrary configuration values to be applied at render time. + // config contains arbitrary JSON configuration values to be applied at render time. // These values will be merged into the bundle manifests during rendering. // +optional - Config map[string]interface{} `json:"config,omitempty"` + // +kubebuilder:validation:Type=object + // +kubebuilder:pruning:PreserveUnknownFields + Config *apiextensionsv1.JSON `json:"config,omitempty"` } const SourceTypeCatalog = "Catalog" diff --git a/internal/operator-controller/applier/helm.go b/internal/operator-controller/applier/helm.go index 0653c4fbb6..c6ecb68b2e 100644 --- a/internal/operator-controller/applier/helm.go +++ b/internal/operator-controller/applier/helm.go @@ -3,6 +3,7 @@ package applier import ( "bytes" "context" + "encoding/json" "errors" "fmt" "io" @@ -126,8 +127,14 @@ func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExte if err != nil { return nil, "", err } - // merge in any user-provided config values as Helm values - values := chartutil.Values{"Values": ext.Spec.Config} + // merge in any user-provided config JSON as Helm values + valuesMap := map[string]interface{}{} + if ext.Spec.Config != nil && ext.Spec.Config.Raw != nil { + if err := json.Unmarshal(ext.Spec.Config.Raw, &valuesMap); err != nil { + return nil, "", fmt.Errorf("invalid JSON in spec.config: %w", err) + } + } + values := chartutil.Values{"Values": valuesMap} post := &postrenderer{ labels: objectLabels, From 989ddb70b02553049591c360c3306da29236be1f Mon Sep 17 00:00:00 2001 From: Brett Tofel Date: Mon, 4 Aug 2025 14:34:00 -0400 Subject: [PATCH 5/9] Manifest changes associated w/ spec.config add Signed-off-by: Brett Tofel --- api/v1/zz_generated.deepcopy.go | 4 ++++ .../olm.operatorframework.io_clusterextensions.yaml | 6 ++++++ .../olm.operatorframework.io_clusterextensions.yaml | 6 ++++++ docs/tutorials/install-extension.md | 9 ++++++--- manifests/experimental-e2e.yaml | 6 ++++++ manifests/experimental.yaml | 6 ++++++ manifests/standard-e2e.yaml | 6 ++++++ manifests/standard.yaml | 6 ++++++ 8 files changed, 46 insertions(+), 3 deletions(-) diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index 23fcf7d85e..f0c98bc90b 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -330,6 +330,10 @@ func (in *ClusterExtensionSpec) DeepCopyInto(out *ClusterExtensionSpec) { *out = new(ClusterExtensionInstallConfig) (*in).DeepCopyInto(*out) } + if in.Config != nil { + in, out := &in.Config, &out.Config + *out = (*in).DeepCopy() + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterExtensionSpec. diff --git a/config/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml b/config/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml index 162683603d..eef8d5195c 100644 --- a/config/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml +++ b/config/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml @@ -57,6 +57,12 @@ spec: description: spec is an optional field that defines the desired state of the ClusterExtension. properties: + config: + description: |- + config contains arbitrary JSON configuration values to be applied at render time. + These values will be merged into the bundle manifests during rendering. + type: object + x-kubernetes-preserve-unknown-fields: true install: description: |- install is an optional field used to configure the installation options diff --git a/config/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml b/config/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml index 18faa59789..91fe222ec3 100644 --- a/config/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml +++ b/config/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml @@ -57,6 +57,12 @@ spec: description: spec is an optional field that defines the desired state of the ClusterExtension. properties: + config: + description: |- + config contains arbitrary JSON configuration values to be applied at render time. + These values will be merged into the bundle manifests during rendering. + type: object + x-kubernetes-preserve-unknown-fields: true install: description: |- install is an optional field used to configure the installation options diff --git a/docs/tutorials/install-extension.md b/docs/tutorials/install-extension.md index b71d5a58c0..72a2f689f8 100644 --- a/docs/tutorials/install-extension.md +++ b/docs/tutorials/install-extension.md @@ -31,9 +31,9 @@ For information on determining the ServiceAccount's permission, please see [Deri ## Procedure -1. Create a CR for the Kubernetes extension you want to install: +1. Create a CR for the Kubernetes extension you want to install. You can also specify arbitrary configuration values under `spec.config` (per [RFC: Registry+v1 Configuration Support](../../RFC_Config_registry+v1_bundle_config.md)): - ``` yaml title="Example CR" + ```yaml title="Example CR" apiVersion: olm.operatorframework.io/v1 kind: ClusterExtension metadata: @@ -46,8 +46,11 @@ For information on determining the ServiceAccount's permission, please see [Deri sourceType: Catalog catalog: packageName: - channels: [,,] version: "" + config: + version: "v2.0.0-demo" + name: "demo-configmap" ``` `extension_name` diff --git a/manifests/experimental-e2e.yaml b/manifests/experimental-e2e.yaml index a91833bd75..85def50e52 100644 --- a/manifests/experimental-e2e.yaml +++ b/manifests/experimental-e2e.yaml @@ -511,6 +511,12 @@ spec: description: spec is an optional field that defines the desired state of the ClusterExtension. properties: + config: + description: |- + config contains arbitrary JSON configuration values to be applied at render time. + These values will be merged into the bundle manifests during rendering. + type: object + x-kubernetes-preserve-unknown-fields: true install: description: |- install is an optional field used to configure the installation options diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml index 00dc141531..2f666b0fee 100644 --- a/manifests/experimental.yaml +++ b/manifests/experimental.yaml @@ -511,6 +511,12 @@ spec: description: spec is an optional field that defines the desired state of the ClusterExtension. properties: + config: + description: |- + config contains arbitrary JSON configuration values to be applied at render time. + These values will be merged into the bundle manifests during rendering. + type: object + x-kubernetes-preserve-unknown-fields: true install: description: |- install is an optional field used to configure the installation options diff --git a/manifests/standard-e2e.yaml b/manifests/standard-e2e.yaml index 1f46a03d47..7c51381a80 100644 --- a/manifests/standard-e2e.yaml +++ b/manifests/standard-e2e.yaml @@ -511,6 +511,12 @@ spec: description: spec is an optional field that defines the desired state of the ClusterExtension. properties: + config: + description: |- + config contains arbitrary JSON configuration values to be applied at render time. + These values will be merged into the bundle manifests during rendering. + type: object + x-kubernetes-preserve-unknown-fields: true install: description: |- install is an optional field used to configure the installation options diff --git a/manifests/standard.yaml b/manifests/standard.yaml index b4c70c252d..3809f74636 100644 --- a/manifests/standard.yaml +++ b/manifests/standard.yaml @@ -511,6 +511,12 @@ spec: description: spec is an optional field that defines the desired state of the ClusterExtension. properties: + config: + description: |- + config contains arbitrary JSON configuration values to be applied at render time. + These values will be merged into the bundle manifests during rendering. + type: object + x-kubernetes-preserve-unknown-fields: true install: description: |- install is an optional field used to configure the installation options From c6d77dca126cc80d22209af7561e7effd921f3a8 Mon Sep 17 00:00:00 2001 From: Brett Tofel Date: Mon, 18 Aug 2025 14:23:40 -0400 Subject: [PATCH 6/9] Add helm handling & tests Signed-off-by: Brett Tofel --- internal/operator-controller/applier/helm.go | 13 ++++- .../operator-controller/applier/helm_test.go | 5 ++ .../rukpak/convert/helm.go | 8 +++ .../rukpak/convert/helm_test.go | 58 +++++++++++++++++++ .../render/registryv1/registryv1_test.go | 42 ++++++++++++++ 5 files changed, 125 insertions(+), 1 deletion(-) diff --git a/internal/operator-controller/applier/helm.go b/internal/operator-controller/applier/helm.go index c6ecb68b2e..74c1086d1c 100644 --- a/internal/operator-controller/applier/helm.go +++ b/internal/operator-controller/applier/helm.go @@ -59,6 +59,7 @@ type Preflight interface { type BundleToHelmChartConverter interface { ToHelmChart(bundle source.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error) + ToHelmChartWithConfig(bundle source.BundleSource, installNamespace string, watchNamespace string, cfg map[string]interface{}) (*chart.Chart, error) } type Helm struct { @@ -230,7 +231,17 @@ func (h *Helm) buildHelmChart(bundleFS fs.FS, ext *ocv1.ClusterExtension) (*char } } - return h.BundleToHelmChartConverter.ToHelmChart(source.FromFS(bundleFS), ext.Spec.Namespace, watchNamespace) + // Unmarshal any provided configuration on the ClusterExtension and pass it + // to the registry+v1 renderer so that static manifests (e.g. ConfigMaps) + // can be overridden with values from spec.config. + cfg := map[string]interface{}{} + if ext.Spec.Config != nil && ext.Spec.Config.Raw != nil { + if err := json.Unmarshal(ext.Spec.Config.Raw, &cfg); err != nil { + return nil, fmt.Errorf("invalid JSON in spec.config: %w", err) + } + } + + return h.BundleToHelmChartConverter.ToHelmChartWithConfig(source.FromFS(bundleFS), ext.Spec.Namespace, watchNamespace, cfg) } func (h *Helm) renderClientOnlyRelease(ctx context.Context, ext *ocv1.ClusterExtension, chrt *chart.Chart, values chartutil.Values, post postrender.PostRenderer) (*release.Release, error) { diff --git a/internal/operator-controller/applier/helm_test.go b/internal/operator-controller/applier/helm_test.go index 89c94df88d..0c4d2f8902 100644 --- a/internal/operator-controller/applier/helm_test.go +++ b/internal/operator-controller/applier/helm_test.go @@ -628,3 +628,8 @@ type fakeBundleToHelmChartConverter struct { func (f fakeBundleToHelmChartConverter) ToHelmChart(bundle source.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error) { return f.fn(bundle, installNamespace, watchNamespace) } + +func (f fakeBundleToHelmChartConverter) ToHelmChartWithConfig(bundle source.BundleSource, installNamespace string, watchNamespace string, cfg map[string]interface{}) (*chart.Chart, error) { + // Tests don't inspect the config here; forward to the underlying function + return f.fn(bundle, installNamespace, watchNamespace) +} diff --git a/internal/operator-controller/rukpak/convert/helm.go b/internal/operator-controller/rukpak/convert/helm.go index 786601fdf8..369e6829fb 100644 --- a/internal/operator-controller/rukpak/convert/helm.go +++ b/internal/operator-controller/rukpak/convert/helm.go @@ -17,7 +17,14 @@ type BundleToHelmChartConverter struct { IsWebhookSupportEnabled bool } +// ToHelmChart is retained for compatibility and forwards to ToHelmChartWithConfig with a nil config. func (r *BundleToHelmChartConverter) ToHelmChart(bundle source.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error) { + return r.ToHelmChartWithConfig(bundle, installNamespace, watchNamespace, nil) +} + +// ToHelmChartWithConfig converts a registry+v1 bundle into a Helm chart and accepts +// an optional configuration map that will be supplied to the render pipeline. +func (r *BundleToHelmChartConverter) ToHelmChartWithConfig(bundle source.BundleSource, installNamespace string, watchNamespace string, cfg map[string]interface{}) (*chart.Chart, error) { rv1, err := bundle.GetBundle() if err != nil { return nil, err @@ -43,6 +50,7 @@ func (r *BundleToHelmChartConverter) ToHelmChart(bundle source.BundleSource, ins rv1, installNamespace, render.WithTargetNamespaces(watchNamespace), render.WithCertificateProvider(r.CertificateProvider), + render.WithConfig(cfg), ) if err != nil { diff --git a/internal/operator-controller/rukpak/convert/helm_test.go b/internal/operator-controller/rukpak/convert/helm_test.go index fdfa9812b0..b8a22d10a0 100644 --- a/internal/operator-controller/rukpak/convert/helm_test.go +++ b/internal/operator-controller/rukpak/convert/helm_test.go @@ -1,7 +1,9 @@ package convert_test import ( + "encoding/json" "errors" + "fmt" "testing" "github.com/stretchr/testify/require" @@ -192,3 +194,59 @@ func Test_BundleToHelmChartConverter_ToHelmChart_Success(t *testing.T) { t.Log("Check Chart templates have the same number of resources generated by the renderer") require.Len(t, chart.Templates, 1) } + +func Test_BundleToHelmChartConverter_ForwardsConfigToRenderer(t *testing.T) { + expectedCfg := map[string]interface{}{ + "version": "v2.0.0-demo", + "name": "demo-configmap", + } + + converter := convert.BundleToHelmChartConverter{ + BundleRenderer: render.BundleRenderer{ + ResourceGenerators: []render.ResourceGenerator{ + func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { + // generator emulates BundleAdditionalResourcesGenerator behavior + cm := &corev1.ConfigMap{ + TypeMeta: metav1.TypeMeta{ + APIVersion: corev1.SchemeGroupVersion.String(), + Kind: "ConfigMap", + }, + ObjectMeta: metav1.ObjectMeta{Name: "test-configmap"}, + Data: map[string]string{}, + } + if opts.Config != nil { + for k, v := range opts.Config { + cm.Data[k] = fmt.Sprintf("%v", v) + } + } + return []client.Object{cm}, nil + }, + }, + }, + } + + b := source.FromBundle( + bundle.RegistryV1{ + CSV: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces)), + }, + ) + + chart, err := converter.ToHelmChartWithConfig(b, "install-namespace", "", expectedCfg) + require.NoError(t, err) + require.NotNil(t, chart) + + found := false + for _, f := range chart.Files { + var obj map[string]interface{} + require.NoError(t, json.Unmarshal(f.Data, &obj)) + if kind, ok := obj["kind"].(string); ok && kind == "ConfigMap" { + data, ok := obj["data"].(map[string]interface{}) + require.True(t, ok) + require.Equal(t, expectedCfg["version"], data["version"]) + require.Equal(t, expectedCfg["name"], data["name"]) + found = true + break + } + } + require.True(t, found, "expected ConfigMap not found in chart files") +} diff --git a/internal/operator-controller/rukpak/render/registryv1/registryv1_test.go b/internal/operator-controller/rukpak/render/registryv1/registryv1_test.go index c75f1d602c..4220aee742 100644 --- a/internal/operator-controller/rukpak/render/registryv1/registryv1_test.go +++ b/internal/operator-controller/rukpak/render/registryv1/registryv1_test.go @@ -121,3 +121,45 @@ func Test_Renderer_Failure_UnsupportedKind(t *testing.T) { require.Contains(t, err.Error(), "unsupported resource") require.Empty(t, objs) } + +func Test_Renderer_AppliesConfigToConfigMap(t *testing.T) { + cfg := map[string]interface{}{ + "version": "v2.0.0-demo", + "name": "demo-configmap", + } + + bundle := bundle.RegistryV1{ + PackageName: "test", + CSV: MakeCSV( + WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces), + ), + Others: []unstructured.Unstructured{ + *ToUnstructuredT(t, &corev1.ConfigMap{ + TypeMeta: metav1.TypeMeta{ + Kind: "ConfigMap", + APIVersion: corev1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{Name: "test-configmap"}, + Data: map[string]string{ + "version": "v2.0.0", + "name": "test-configmap", + }, + }), + }, + } + + objs, err := registryv1.Renderer.Render(bundle, "install-namespace", render.WithConfig(cfg)) + require.NoError(t, err) + require.Len(t, objs, 1) + + cmObj := objs[0] + u, ok := cmObj.(*unstructured.Unstructured) + require.True(t, ok) + + data, found, err := unstructured.NestedStringMap(u.Object, "data") + require.NoError(t, err) + require.True(t, found) + require.Equal(t, "v2.0.0-demo", data["version"]) + require.Equal(t, "demo-configmap", data["name"]) + require.Equal(t, "install-namespace", u.GetNamespace()) +} From a507ca0884a6afa3d274041634a1e970bb32b6c2 Mon Sep 17 00:00:00 2001 From: Brett Tofel Date: Tue, 19 Aug 2025 15:03:27 -0400 Subject: [PATCH 7/9] Setup ClusterExtensionConfig allow for secrets Signed-off-by: Brett Tofel --- .bingo/go.mod | 4 +- api/v1/clusterextension_types.go | 19 +++++++- config/samples/olm_v1_clusterextension.yaml | 6 +++ hack/demo/resources/own-namespace-demo.yaml | 4 ++ .../demo/resources/single-namespace-demo.yaml | 3 ++ .../argocd-clusterextension.yaml | 3 ++ .../webhook-operator-extension.yaml | 4 ++ .../testdata/api/v1/clusterextension_types.go | 21 ++++++++- manifests/experimental-e2e.yaml | 45 +++++++++++++++++-- manifests/experimental.yaml | 45 +++++++++++++++++-- manifests/standard-e2e.yaml | 45 +++++++++++++++++-- manifests/standard.yaml | 45 +++++++++++++++++-- 12 files changed, 223 insertions(+), 21 deletions(-) diff --git a/.bingo/go.mod b/.bingo/go.mod index 610249af0b..c8b30a67ed 100644 --- a/.bingo/go.mod +++ b/.bingo/go.mod @@ -1 +1,3 @@ -module _ // Fake go.mod auto-created by 'bingo' for go -moddir compatibility with non-Go projects. Commit this file, together with other .mod files. \ No newline at end of file +module _ // Fake go.mod auto-created by 'bingo' for go -moddir compatibility with non-Go projects. Commit this file, together with other .mod files. + +go 1.23.6 diff --git a/api/v1/clusterextension_types.go b/api/v1/clusterextension_types.go index c7a7ef62c2..819d5b672d 100644 --- a/api/v1/clusterextension_types.go +++ b/api/v1/clusterextension_types.go @@ -17,6 +17,7 @@ limitations under the License. package v1 import ( + corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -94,12 +95,26 @@ type ClusterExtensionSpec struct { // +optional Install *ClusterExtensionInstallConfig `json:"install,omitempty"` - // config contains arbitrary JSON configuration values to be applied at render time. + // config contains configuration values to be applied at render time. + // +optional + Config *ClusterExtensionConfig `json:"config,omitempty"` +} + +// ClusterExtensionConfig provides configuration values to be applied at render time. +// +// +kubebuilder:validation:XValidation:rule="has(self.inline) || has(self.secretRef)",message="at least one of [inline, secretRef] is required when config is specified" +type ClusterExtensionConfig struct { + // inline contains arbitrary JSON configuration values to be applied at render time. // These values will be merged into the bundle manifests during rendering. // +optional // +kubebuilder:validation:Type=object // +kubebuilder:pruning:PreserveUnknownFields - Config *apiextensionsv1.JSON `json:"config,omitempty"` + Inline map[string]apiextensionsv1.JSON `json:"inline,omitempty"` + + // secretRef references a Secret containing configuration data. + // The secret must exist in the same namespace referenced in the spec. + // +optional + SecretRef *corev1.SecretKeySelector `json:"secretRef,omitempty"` } const SourceTypeCatalog = "Catalog" diff --git a/config/samples/olm_v1_clusterextension.yaml b/config/samples/olm_v1_clusterextension.yaml index 4438dfb76d..60ca6c9234 100644 --- a/config/samples/olm_v1_clusterextension.yaml +++ b/config/samples/olm_v1_clusterextension.yaml @@ -283,3 +283,9 @@ spec: catalog: packageName: argocd-operator version: 0.6.0 + config: + inline: + exampleKey: exampleValue + secretRef: + name: argocd-config + key: config.yaml diff --git a/hack/demo/resources/own-namespace-demo.yaml b/hack/demo/resources/own-namespace-demo.yaml index f1d6da927b..be5d0956f0 100644 --- a/hack/demo/resources/own-namespace-demo.yaml +++ b/hack/demo/resources/own-namespace-demo.yaml @@ -14,3 +14,7 @@ spec: catalog: packageName: argocd-operator version: 0.6.0 + config: + secretRef: + name: argocd-config + key: config.yaml diff --git a/hack/demo/resources/single-namespace-demo.yaml b/hack/demo/resources/single-namespace-demo.yaml index 2403bc6a8b..00559bac2a 100644 --- a/hack/demo/resources/single-namespace-demo.yaml +++ b/hack/demo/resources/single-namespace-demo.yaml @@ -14,3 +14,6 @@ spec: catalog: packageName: argocd-operator version: 0.6.0 + config: + inline: + exampleKey: exampleValue diff --git a/hack/demo/resources/synthetic-user-perms/argocd-clusterextension.yaml b/hack/demo/resources/synthetic-user-perms/argocd-clusterextension.yaml index 7eb5a7082b..23a66cfd7f 100644 --- a/hack/demo/resources/synthetic-user-perms/argocd-clusterextension.yaml +++ b/hack/demo/resources/synthetic-user-perms/argocd-clusterextension.yaml @@ -11,3 +11,6 @@ spec: catalog: packageName: argocd-operator version: 0.6.0 + config: + inline: + exampleKey: exampleValue diff --git a/hack/demo/resources/webhook-provider-certmanager/webhook-operator-extension.yaml b/hack/demo/resources/webhook-provider-certmanager/webhook-operator-extension.yaml index 19b7eceb08..bffb2e7de1 100644 --- a/hack/demo/resources/webhook-provider-certmanager/webhook-operator-extension.yaml +++ b/hack/demo/resources/webhook-provider-certmanager/webhook-operator-extension.yaml @@ -13,3 +13,7 @@ spec: selector: {} upgradeConstraintPolicy: CatalogProvided sourceType: Catalog + config: + secretRef: + name: webhook-config + key: config.yaml diff --git a/hack/tools/crd-generator/testdata/api/v1/clusterextension_types.go b/hack/tools/crd-generator/testdata/api/v1/clusterextension_types.go index 4056138e25..9c04727bd3 100644 --- a/hack/tools/crd-generator/testdata/api/v1/clusterextension_types.go +++ b/hack/tools/crd-generator/testdata/api/v1/clusterextension_types.go @@ -17,6 +17,8 @@ limitations under the License. package v1 import ( + corev1 "k8s.io/api/core/v1" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -94,10 +96,25 @@ type ClusterExtensionSpec struct { // +optional Install *ClusterExtensionInstallConfig `json:"install,omitempty"` - // config contains arbitrary configuration values to be applied at render time. + // config contains configuration values to be applied at render time. + // +optional + Config *ClusterExtensionConfig `json:"config,omitempty"` +} + +// ClusterExtensionConfig provides configuration values to be applied at render time. +// +// +type ClusterExtensionConfig struct { + // inline contains arbitrary configuration values to be applied at render time. // These values will be merged into the bundle manifests during rendering. // +optional - Config map[string]interface{} `json:"config,omitempty"` + // +kubebuilder:validation:Type=object + // +kubebuilder:pruning:PreserveUnknownFields + Inline map[string]apiextensionsv1.JSON `json:"inline,omitempty"` + + // secretRef references a Secret containing configuration data. + // +optional + SecretRef *corev1.SecretKeySelector `json:"secretRef,omitempty"` } const SourceTypeCatalog = "Catalog" diff --git a/manifests/experimental-e2e.yaml b/manifests/experimental-e2e.yaml index 85def50e52..d9fbcf9e0c 100644 --- a/manifests/experimental-e2e.yaml +++ b/manifests/experimental-e2e.yaml @@ -512,11 +512,48 @@ spec: of the ClusterExtension. properties: config: - description: |- - config contains arbitrary JSON configuration values to be applied at render time. - These values will be merged into the bundle manifests during rendering. + description: config contains configuration values to be applied at + render time. + properties: + inline: + additionalProperties: + x-kubernetes-preserve-unknown-fields: true + description: |- + inline contains arbitrary JSON configuration values to be applied at render time. + These values will be merged into the bundle manifests during rendering. + type: object + x-kubernetes-preserve-unknown-fields: true + secretRef: + description: |- + secretRef references a Secret containing configuration data. + The secret must exist in the same namespace referenced in the spec. + properties: + key: + description: The key of the secret to select from. Must be + a valid secret key. + 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 Secret or its key must be + defined + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic type: object - x-kubernetes-preserve-unknown-fields: true + x-kubernetes-validations: + - message: at least one of [inline, secretRef] is required when config + is specified + rule: has(self.inline) || has(self.secretRef) install: description: |- install is an optional field used to configure the installation options diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml index 2f666b0fee..2ca47b097d 100644 --- a/manifests/experimental.yaml +++ b/manifests/experimental.yaml @@ -512,11 +512,48 @@ spec: of the ClusterExtension. properties: config: - description: |- - config contains arbitrary JSON configuration values to be applied at render time. - These values will be merged into the bundle manifests during rendering. + description: config contains configuration values to be applied at + render time. + properties: + inline: + additionalProperties: + x-kubernetes-preserve-unknown-fields: true + description: |- + inline contains arbitrary JSON configuration values to be applied at render time. + These values will be merged into the bundle manifests during rendering. + type: object + x-kubernetes-preserve-unknown-fields: true + secretRef: + description: |- + secretRef references a Secret containing configuration data. + The secret must exist in the same namespace referenced in the spec. + properties: + key: + description: The key of the secret to select from. Must be + a valid secret key. + 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 Secret or its key must be + defined + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic type: object - x-kubernetes-preserve-unknown-fields: true + x-kubernetes-validations: + - message: at least one of [inline, secretRef] is required when config + is specified + rule: has(self.inline) || has(self.secretRef) install: description: |- install is an optional field used to configure the installation options diff --git a/manifests/standard-e2e.yaml b/manifests/standard-e2e.yaml index 7c51381a80..18a72755af 100644 --- a/manifests/standard-e2e.yaml +++ b/manifests/standard-e2e.yaml @@ -512,11 +512,48 @@ spec: of the ClusterExtension. properties: config: - description: |- - config contains arbitrary JSON configuration values to be applied at render time. - These values will be merged into the bundle manifests during rendering. + description: config contains configuration values to be applied at + render time. + properties: + inline: + additionalProperties: + x-kubernetes-preserve-unknown-fields: true + description: |- + inline contains arbitrary JSON configuration values to be applied at render time. + These values will be merged into the bundle manifests during rendering. + type: object + x-kubernetes-preserve-unknown-fields: true + secretRef: + description: |- + secretRef references a Secret containing configuration data. + The secret must exist in the same namespace referenced in the spec. + properties: + key: + description: The key of the secret to select from. Must be + a valid secret key. + 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 Secret or its key must be + defined + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic type: object - x-kubernetes-preserve-unknown-fields: true + x-kubernetes-validations: + - message: at least one of [inline, secretRef] is required when config + is specified + rule: has(self.inline) || has(self.secretRef) install: description: |- install is an optional field used to configure the installation options diff --git a/manifests/standard.yaml b/manifests/standard.yaml index 3809f74636..f553415494 100644 --- a/manifests/standard.yaml +++ b/manifests/standard.yaml @@ -512,11 +512,48 @@ spec: of the ClusterExtension. properties: config: - description: |- - config contains arbitrary JSON configuration values to be applied at render time. - These values will be merged into the bundle manifests during rendering. + description: config contains configuration values to be applied at + render time. + properties: + inline: + additionalProperties: + x-kubernetes-preserve-unknown-fields: true + description: |- + inline contains arbitrary JSON configuration values to be applied at render time. + These values will be merged into the bundle manifests during rendering. + type: object + x-kubernetes-preserve-unknown-fields: true + secretRef: + description: |- + secretRef references a Secret containing configuration data. + The secret must exist in the same namespace referenced in the spec. + properties: + key: + description: The key of the secret to select from. Must be + a valid secret key. + 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 Secret or its key must be + defined + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic type: object - x-kubernetes-preserve-unknown-fields: true + x-kubernetes-validations: + - message: at least one of [inline, secretRef] is required when config + is specified + rule: has(self.inline) || has(self.secretRef) install: description: |- install is an optional field used to configure the installation options From b7d6c6084092ec3d22b000a4dbf1e9a5585f2888 Mon Sep 17 00:00:00 2001 From: Brett Tofel Date: Wed, 20 Aug 2025 08:48:34 -0400 Subject: [PATCH 8/9] Plumb thru and work with secret config items Signed-off-by: Brett Tofel --- api/v1/clusterextension_types.go | 38 +++++------ api/v1/zz_generated.deepcopy.go | 26 +++++++- cmd/operator-controller/main.go | 1 + internal/operator-controller/applier/helm.go | 64 +++++++++++++++++-- .../operator-controller/applier/helm_test.go | 44 +++++++++++++ 5 files changed, 149 insertions(+), 24 deletions(-) diff --git a/api/v1/clusterextension_types.go b/api/v1/clusterextension_types.go index 819d5b672d..78b0408e28 100644 --- a/api/v1/clusterextension_types.go +++ b/api/v1/clusterextension_types.go @@ -95,28 +95,14 @@ type ClusterExtensionSpec struct { // +optional Install *ClusterExtensionInstallConfig `json:"install,omitempty"` - // config contains configuration values to be applied at render time. + // config contains configuration values applied during rendering of the + // ClusterExtension's manifests. Values can be specified inline or sourced + // from a referenced Secret. + // // +optional Config *ClusterExtensionConfig `json:"config,omitempty"` } -// ClusterExtensionConfig provides configuration values to be applied at render time. -// -// +kubebuilder:validation:XValidation:rule="has(self.inline) || has(self.secretRef)",message="at least one of [inline, secretRef] is required when config is specified" -type ClusterExtensionConfig struct { - // inline contains arbitrary JSON configuration values to be applied at render time. - // These values will be merged into the bundle manifests during rendering. - // +optional - // +kubebuilder:validation:Type=object - // +kubebuilder:pruning:PreserveUnknownFields - Inline map[string]apiextensionsv1.JSON `json:"inline,omitempty"` - - // secretRef references a Secret containing configuration data. - // The secret must exist in the same namespace referenced in the spec. - // +optional - SecretRef *corev1.SecretKeySelector `json:"secretRef,omitempty"` -} - const SourceTypeCatalog = "Catalog" // SourceConfig is a discriminated union which selects the installation source. @@ -161,6 +147,22 @@ type ClusterExtensionInstallConfig struct { Preflight *PreflightConfig `json:"preflight,omitempty"` } +// ClusterExtensionConfig defines configuration values to be merged into +// the ClusterExtension's rendered manifests. +type ClusterExtensionConfig struct { + // inline contains JSON or YAML values specified directly in the + // ClusterExtension. + // +optional + Inline *apiextensionsv1.JSON `json:"inline,omitempty"` + + // secretRef references a key in a Secret that contains JSON or YAML + // values. + // The referenced Secret must exist in the same namespace as the + // ClusterExtension. + // +optional + SecretRef *corev1.SecretKeySelector `json:"secretRef,omitempty"` +} + // CatalogFilter defines the attributes used to identify and filter content from a catalog. type CatalogFilter struct { // packageName is a reference to the name of the package to be installed diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index f0c98bc90b..241c166232 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -252,6 +252,29 @@ func (in *ClusterExtension) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ClusterExtensionConfig) DeepCopyInto(out *ClusterExtensionConfig) { + *out = *in + if in.Inline != nil { + in, out := &in.Inline, &out.Inline + *out = (*in).DeepCopy() + } + if in.SecretRef != nil { + in, out := &in.SecretRef, &out.SecretRef + *out = (*in).DeepCopy() + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterExtensionConfig. +func (in *ClusterExtensionConfig) DeepCopy() *ClusterExtensionConfig { + if in == nil { + return nil + } + out := new(ClusterExtensionConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ClusterExtensionInstallConfig) DeepCopyInto(out *ClusterExtensionInstallConfig) { *out = *in @@ -332,7 +355,8 @@ func (in *ClusterExtensionSpec) DeepCopyInto(out *ClusterExtensionSpec) { } if in.Config != nil { in, out := &in.Config, &out.Config - *out = (*in).DeepCopy() + *out = new(ClusterExtensionConfig) + (*in).DeepCopyInto(*out) } } diff --git a/cmd/operator-controller/main.go b/cmd/operator-controller/main.go index d426793d46..d80219ec9c 100644 --- a/cmd/operator-controller/main.go +++ b/cmd/operator-controller/main.go @@ -449,6 +449,7 @@ func run() error { IsWebhookSupportEnabled: isWebhookSupportEnabled, }, PreAuthorizer: preAuth, + Client: mgr.GetClient(), } cm := contentmanager.NewManager(clientRestConfigMapper, mgr.GetConfig(), mgr.GetRESTMapper()) diff --git a/internal/operator-controller/applier/helm.go b/internal/operator-controller/applier/helm.go index 74c1086d1c..972247c9bb 100644 --- a/internal/operator-controller/applier/helm.go +++ b/internal/operator-controller/applier/helm.go @@ -17,8 +17,10 @@ import ( "helm.sh/helm/v3/pkg/postrender" "helm.sh/helm/v3/pkg/release" "helm.sh/helm/v3/pkg/storage/driver" + corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + apitypes "k8s.io/apimachinery/pkg/types" apimachyaml "k8s.io/apimachinery/pkg/util/yaml" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/log" @@ -67,6 +69,34 @@ type Helm struct { Preflights []Preflight PreAuthorizer authorization.PreAuthorizer BundleToHelmChartConverter BundleToHelmChartConverter + Client client.Client +} + +func decodeValues(data []byte) (map[string]any, error) { + m := map[string]any{} + if len(bytes.TrimSpace(data)) == 0 { + return m, nil + } + j, err := apimachyaml.ToJSON(data) + if err != nil { + return nil, err + } + if err := json.Unmarshal(j, &m); err != nil { + return nil, err + } + return m, nil +} + +func mergeValueMaps(dst, src map[string]any) { + for k, v := range src { + if dstMap, ok := dst[k].(map[string]any); ok { + if srcMap, ok2 := v.(map[string]any); ok2 { + mergeValueMaps(dstMap, srcMap) + continue + } + } + dst[k] = v + } } // shouldSkipPreflight is a helper to determine if the preflight check is CRDUpgradeSafety AND @@ -128,11 +158,35 @@ func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExte if err != nil { return nil, "", err } - // merge in any user-provided config JSON as Helm values - valuesMap := map[string]interface{}{} - if ext.Spec.Config != nil && ext.Spec.Config.Raw != nil { - if err := json.Unmarshal(ext.Spec.Config.Raw, &valuesMap); err != nil { - return nil, "", fmt.Errorf("invalid JSON in spec.config: %w", err) + // gather configuration values from the ClusterExtension specification + // and any referenced Secrets, merging them into a single map + valuesMap := map[string]any{} + if ext.Spec.Config != nil { + if ext.Spec.Config.Inline != nil && len(ext.Spec.Config.Inline.Raw) > 0 { + inlineMap, err := decodeValues(ext.Spec.Config.Inline.Raw) + if err != nil { + return nil, "", fmt.Errorf("invalid spec.config.inline: %w", err) + } + mergeValueMaps(valuesMap, inlineMap) + } + if ext.Spec.Config.SecretRef != nil { + if h.Client == nil { + return nil, "", errors.New("client is nil") + } + secret := &corev1.Secret{} + nn := apitypes.NamespacedName{Name: ext.Spec.Config.SecretRef.Name, Namespace: ext.Spec.Namespace} + if err := h.Client.Get(ctx, nn, secret); err != nil { + return nil, "", fmt.Errorf("failed to get config secret: %w", err) + } + data, ok := secret.Data[ext.Spec.Config.SecretRef.Key] + if !ok { + return nil, "", fmt.Errorf("secret %q missing key %q", nn.Name, ext.Spec.Config.SecretRef.Key) + } + secretMap, err := decodeValues(data) + if err != nil { + return nil, "", fmt.Errorf("invalid config secret %q key %q: %w", nn.Name, ext.Spec.Config.SecretRef.Key, err) + } + mergeValueMaps(valuesMap, secretMap) } } values := chartutil.Values{"Values": valuesMap} diff --git a/internal/operator-controller/applier/helm_test.go b/internal/operator-controller/applier/helm_test.go index 0c4d2f8902..793561e2be 100644 --- a/internal/operator-controller/applier/helm_test.go +++ b/internal/operator-controller/applier/helm_test.go @@ -16,9 +16,11 @@ import ( "helm.sh/helm/v3/pkg/storage/driver" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" featuregatetesting "k8s.io/component-base/featuregate/testing" "sigs.k8s.io/controller-runtime/pkg/client" + clientfake "sigs.k8s.io/controller-runtime/pkg/client/fake" helmclient "github.com/operator-framework/helm-operator-plugins/pkg/client" @@ -66,6 +68,7 @@ type mockActionGetter struct { reconcileErr error desiredRel *release.Release currentRel *release.Release + vals map[string]interface{} } func (mag *mockActionGetter) ActionClientFor(ctx context.Context, obj client.Object) (helmclient.ActionInterface, error) { @@ -87,6 +90,7 @@ func (mag *mockActionGetter) Install(name, namespace string, chrt *chart.Chart, return nil, err } } + mag.vals = vals if i.DryRun { return mag.desiredRel, mag.dryRunInstallErr } @@ -100,6 +104,7 @@ func (mag *mockActionGetter) Upgrade(name, namespace string, chrt *chart.Chart, return nil, err } } + mag.vals = vals if i.DryRun { return mag.desiredRel, mag.dryRunUpgradeErr } @@ -621,6 +626,45 @@ func TestApply_RegistryV1ToChartConverterIntegration(t *testing.T) { }) } +func TestApply_ConfigMerging(t *testing.T) { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "cfg", Namespace: "ns"}, + Data: map[string][]byte{"values": []byte("foo:\n baz: 2\n")}, + } + fakeClient := clientfake.NewClientBuilder().WithObjects(secret).Build() + + mag := &mockActionGetter{ + getClientErr: driver.ErrReleaseNotFound, + desiredRel: &release.Release{Manifest: "---\n"}, + } + helmApplier := applier.Helm{ + ActionClientGetter: mag, + BundleToHelmChartConverter: &fakeBundleToHelmChartConverter{fn: func(bundle source.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error) { + return &chart.Chart{}, nil + }}, + Client: fakeClient, + } + + ext := &ocv1.ClusterExtension{ + ObjectMeta: metav1.ObjectMeta{Name: "test"}, + Spec: ocv1.ClusterExtensionSpec{ + Namespace: "ns", + ServiceAccount: ocv1.ServiceAccountReference{Name: "sa"}, + Source: ocv1.SourceConfig{SourceType: ocv1.SourceTypeCatalog, Catalog: &ocv1.CatalogFilter{PackageName: "pkg"}}, + Config: &ocv1.ClusterExtensionConfig{ + Inline: &apiextensionsv1.JSON{Raw: []byte("foo:\n bar: 1\n")}, + SecretRef: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "cfg"}, Key: "values"}, + }, + }, + } + + _, _, err := helmApplier.Apply(context.TODO(), validFS, ext, testObjectLabels, testStorageLabels) + require.NoError(t, err) + + expected := map[string]any{"foo": map[string]any{"bar": float64(1), "baz": float64(2)}} + assert.Equal(t, expected, mag.vals["Values"]) +} + type fakeBundleToHelmChartConverter struct { fn func(source.BundleSource, string, string) (*chart.Chart, error) } From ff69d61dea6c50097b84ea1d225ea994c8f67410 Mon Sep 17 00:00:00 2001 From: Brett Tofel Date: Wed, 20 Aug 2025 09:42:28 -0400 Subject: [PATCH 9/9] Redo config merging a bit and test thereof Signed-off-by: Brett Tofel --- api/v1/clusterextensionconfig_deepcopy.go | 35 ++++++ internal/operator-controller/applier/helm.go | 117 ++++++++---------- .../operator-controller/applier/helm_test.go | 84 +++++++------ 3 files changed, 128 insertions(+), 108 deletions(-) create mode 100644 api/v1/clusterextensionconfig_deepcopy.go diff --git a/api/v1/clusterextensionconfig_deepcopy.go b/api/v1/clusterextensionconfig_deepcopy.go new file mode 100644 index 0000000000..cf7d95a00c --- /dev/null +++ b/api/v1/clusterextensionconfig_deepcopy.go @@ -0,0 +1,35 @@ +package v1 + +import ( + corev1 "k8s.io/api/core/v1" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" +) + +// DeepCopyInto copies the receiver into out. in must be non-nil. +func (in *ClusterExtensionConfig) DeepCopyInto(out *ClusterExtensionConfig) { + *out = *in + if in.Inline != nil { + out.Inline = make(map[string]apiextensionsv1.JSON, len(in.Inline)) + for k, v := range in.Inline { + if v.Raw != nil { + out.Inline[k] = apiextensionsv1.JSON{Raw: append([]byte(nil), v.Raw...)} + } else { + out.Inline[k] = apiextensionsv1.JSON{} + } + } + } + if in.SecretRef != nil { + out.SecretRef = new(corev1.SecretKeySelector) + in.SecretRef.DeepCopyInto(out.SecretRef) + } +} + +// DeepCopy creates a new ClusterExtensionConfig by copying the receiver. +func (in *ClusterExtensionConfig) DeepCopy() *ClusterExtensionConfig { + if in == nil { + return nil + } + out := new(ClusterExtensionConfig) + in.DeepCopyInto(out) + return out +} diff --git a/internal/operator-controller/applier/helm.go b/internal/operator-controller/applier/helm.go index 972247c9bb..1de080b6ea 100644 --- a/internal/operator-controller/applier/helm.go +++ b/internal/operator-controller/applier/helm.go @@ -20,7 +20,6 @@ import ( corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - apitypes "k8s.io/apimachinery/pkg/types" apimachyaml "k8s.io/apimachinery/pkg/util/yaml" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/log" @@ -72,33 +71,6 @@ type Helm struct { Client client.Client } -func decodeValues(data []byte) (map[string]any, error) { - m := map[string]any{} - if len(bytes.TrimSpace(data)) == 0 { - return m, nil - } - j, err := apimachyaml.ToJSON(data) - if err != nil { - return nil, err - } - if err := json.Unmarshal(j, &m); err != nil { - return nil, err - } - return m, nil -} - -func mergeValueMaps(dst, src map[string]any) { - for k, v := range src { - if dstMap, ok := dst[k].(map[string]any); ok { - if srcMap, ok2 := v.(map[string]any); ok2 { - mergeValueMaps(dstMap, srcMap) - continue - } - } - dst[k] = v - } -} - // shouldSkipPreflight is a helper to determine if the preflight check is CRDUpgradeSafety AND // if it is set to enforcement None. func shouldSkipPreflight(ctx context.Context, preflight Preflight, ext *ocv1.ClusterExtension, state string) bool { @@ -153,41 +125,55 @@ func (h *Helm) runPreAuthorizationChecks(ctx context.Context, ext *ocv1.ClusterE return nil } -func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExtension, objectLabels map[string]string, storageLabels map[string]string) ([]client.Object, string, error) { - chrt, err := h.buildHelmChart(contentFS, ext) - if err != nil { - return nil, "", err +func (h *Helm) configValues(ctx context.Context, ext *ocv1.ClusterExtension) (map[string]interface{}, error) { + cfg := map[string]interface{}{} + if ext.Spec.Config == nil { + return cfg, nil } - // gather configuration values from the ClusterExtension specification - // and any referenced Secrets, merging them into a single map - valuesMap := map[string]any{} - if ext.Spec.Config != nil { - if ext.Spec.Config.Inline != nil && len(ext.Spec.Config.Inline.Raw) > 0 { - inlineMap, err := decodeValues(ext.Spec.Config.Inline.Raw) - if err != nil { - return nil, "", fmt.Errorf("invalid spec.config.inline: %w", err) - } - mergeValueMaps(valuesMap, inlineMap) - } - if ext.Spec.Config.SecretRef != nil { - if h.Client == nil { - return nil, "", errors.New("client is nil") - } - secret := &corev1.Secret{} - nn := apitypes.NamespacedName{Name: ext.Spec.Config.SecretRef.Name, Namespace: ext.Spec.Namespace} - if err := h.Client.Get(ctx, nn, secret); err != nil { - return nil, "", fmt.Errorf("failed to get config secret: %w", err) - } - data, ok := secret.Data[ext.Spec.Config.SecretRef.Key] - if !ok { - return nil, "", fmt.Errorf("secret %q missing key %q", nn.Name, ext.Spec.Config.SecretRef.Key) + if ext.Spec.Config.Inline != nil { + for k, v := range ext.Spec.Config.Inline { + if len(v.Raw) == 0 { + cfg[k] = nil + continue } - secretMap, err := decodeValues(data) - if err != nil { - return nil, "", fmt.Errorf("invalid config secret %q key %q: %w", nn.Name, ext.Spec.Config.SecretRef.Key, err) + var val interface{} + if err := json.Unmarshal(v.Raw, &val); err != nil { + return nil, fmt.Errorf("invalid JSON in spec.config.inline[%s]: %w", k, err) } - mergeValueMaps(valuesMap, secretMap) + cfg[k] = val + } + } + if ext.Spec.Config.SecretRef != nil { + if h.Client == nil { + return nil, fmt.Errorf("secretRef specified but Helm client is nil") + } + secret := &corev1.Secret{} + if err := h.Client.Get(ctx, client.ObjectKey{Name: ext.Spec.Config.SecretRef.Name, Namespace: ext.Spec.Namespace}, secret); err != nil { + return nil, fmt.Errorf("failed to get config secret: %w", err) } + data, ok := secret.Data[ext.Spec.Config.SecretRef.Key] + if !ok { + return nil, fmt.Errorf("missing key %s in secret %s", ext.Spec.Config.SecretRef.Key, ext.Spec.Config.SecretRef.Name) + } + var secretCfg map[string]interface{} + if err := json.Unmarshal(data, &secretCfg); err != nil { + return nil, fmt.Errorf("invalid JSON in secret %s/%s: %w", ext.Spec.Namespace, ext.Spec.Config.SecretRef.Name, err) + } + for k, v := range secretCfg { + cfg[k] = v + } + } + return cfg, nil +} + +func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExtension, objectLabels map[string]string, storageLabels map[string]string) ([]client.Object, string, error) { + chrt, err := h.buildHelmChart(ctx, contentFS, ext) + if err != nil { + return nil, "", err + } + valuesMap, err := h.configValues(ctx, ext) + if err != nil { + return nil, "", err } values := chartutil.Values{"Values": valuesMap} @@ -266,7 +252,7 @@ func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExte return relObjects, state, nil } -func (h *Helm) buildHelmChart(bundleFS fs.FS, ext *ocv1.ClusterExtension) (*chart.Chart, error) { +func (h *Helm) buildHelmChart(ctx context.Context, bundleFS fs.FS, ext *ocv1.ClusterExtension) (*chart.Chart, error) { if h.BundleToHelmChartConverter == nil { return nil, errors.New("BundleToHelmChartConverter is nil") } @@ -285,14 +271,9 @@ func (h *Helm) buildHelmChart(bundleFS fs.FS, ext *ocv1.ClusterExtension) (*char } } - // Unmarshal any provided configuration on the ClusterExtension and pass it - // to the registry+v1 renderer so that static manifests (e.g. ConfigMaps) - // can be overridden with values from spec.config. - cfg := map[string]interface{}{} - if ext.Spec.Config != nil && ext.Spec.Config.Raw != nil { - if err := json.Unmarshal(ext.Spec.Config.Raw, &cfg); err != nil { - return nil, fmt.Errorf("invalid JSON in spec.config: %w", err) - } + cfg, err := h.configValues(ctx, ext) + if err != nil { + return nil, err } return h.BundleToHelmChartConverter.ToHelmChartWithConfig(source.FromFS(bundleFS), ext.Spec.Namespace, watchNamespace, cfg) diff --git a/internal/operator-controller/applier/helm_test.go b/internal/operator-controller/applier/helm_test.go index 793561e2be..4c7e0c76a4 100644 --- a/internal/operator-controller/applier/helm_test.go +++ b/internal/operator-controller/applier/helm_test.go @@ -20,7 +20,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" featuregatetesting "k8s.io/component-base/featuregate/testing" "sigs.k8s.io/controller-runtime/pkg/client" - clientfake "sigs.k8s.io/controller-runtime/pkg/client/fake" + fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake" helmclient "github.com/operator-framework/helm-operator-plugins/pkg/client" @@ -228,6 +228,49 @@ func TestApply_Base(t *testing.T) { }) } +func TestApply_ConfigMerging(t *testing.T) { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "cfg", Namespace: "ns"}, + Data: map[string][]byte{ + "config": []byte(`{"secretKey":"secretValue","sharedKey":"secretVal"}`), + }, + } + cl := fakeclient.NewClientBuilder().WithObjects(secret).Build() + + ext := &ocv1.ClusterExtension{ + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "ns"}, + Spec: ocv1.ClusterExtensionSpec{ + Namespace: "ns", + ServiceAccount: ocv1.ServiceAccountReference{Name: "sa"}, + Source: ocv1.SourceConfig{SourceType: ocv1.SourceTypeCatalog, Catalog: &ocv1.CatalogFilter{PackageName: "pkg"}}, + Config: &ocv1.ClusterExtensionConfig{ + Inline: map[string]apiextensionsv1.JSON{ + "inlineKey": {Raw: []byte(`"inlineValue"`)}, + "sharedKey": {Raw: []byte(`"inlineVal"`)}, + }, + SecretRef: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "cfg"}, Key: "config"}, + }, + }, + } + + mag := &mockActionGetter{desiredRel: &release.Release{Manifest: validManifest}} + + helmApplier := applier.Helm{ + ActionClientGetter: mag, + BundleToHelmChartConverter: &fakeBundleToHelmChartConverter{fn: func(bundle source.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error) { + return &chart.Chart{}, nil + }}, + Client: cl, + } + + _, _, err := helmApplier.Apply(context.TODO(), validFS, ext, testObjectLabels, testStorageLabels) + require.NoError(t, err) + require.NotNil(t, mag.vals) + assert.Equal(t, "inlineValue", mag.vals["inlineKey"]) + assert.Equal(t, "secretValue", mag.vals["secretKey"]) + assert.Equal(t, "secretVal", mag.vals["sharedKey"]) +} + func TestApply_Installation(t *testing.T) { t.Run("fails during dry-run installation", func(t *testing.T) { mockAcg := &mockActionGetter{ @@ -626,45 +669,6 @@ func TestApply_RegistryV1ToChartConverterIntegration(t *testing.T) { }) } -func TestApply_ConfigMerging(t *testing.T) { - secret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{Name: "cfg", Namespace: "ns"}, - Data: map[string][]byte{"values": []byte("foo:\n baz: 2\n")}, - } - fakeClient := clientfake.NewClientBuilder().WithObjects(secret).Build() - - mag := &mockActionGetter{ - getClientErr: driver.ErrReleaseNotFound, - desiredRel: &release.Release{Manifest: "---\n"}, - } - helmApplier := applier.Helm{ - ActionClientGetter: mag, - BundleToHelmChartConverter: &fakeBundleToHelmChartConverter{fn: func(bundle source.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error) { - return &chart.Chart{}, nil - }}, - Client: fakeClient, - } - - ext := &ocv1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{Name: "test"}, - Spec: ocv1.ClusterExtensionSpec{ - Namespace: "ns", - ServiceAccount: ocv1.ServiceAccountReference{Name: "sa"}, - Source: ocv1.SourceConfig{SourceType: ocv1.SourceTypeCatalog, Catalog: &ocv1.CatalogFilter{PackageName: "pkg"}}, - Config: &ocv1.ClusterExtensionConfig{ - Inline: &apiextensionsv1.JSON{Raw: []byte("foo:\n bar: 1\n")}, - SecretRef: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "cfg"}, Key: "values"}, - }, - }, - } - - _, _, err := helmApplier.Apply(context.TODO(), validFS, ext, testObjectLabels, testStorageLabels) - require.NoError(t, err) - - expected := map[string]any{"foo": map[string]any{"bar": float64(1), "baz": float64(2)}} - assert.Equal(t, expected, mag.vals["Values"]) -} - type fakeBundleToHelmChartConverter struct { fn func(source.BundleSource, string, string) (*chart.Chart, error) }