From 26a404ab84747a79e0b113dfe1a7a728690f4790 Mon Sep 17 00:00:00 2001 From: Klaudiusz Fabryczny Date: Mon, 1 Jun 2026 14:28:57 +0200 Subject: [PATCH 01/18] feat(pluginpresets): evaluate CEL expressions for owned Plugins (#1774) Signed-off-by: Klaudiusz Fabryczny --- .github/workflows/backport.yaml | 39 ++ charts/greenhouse/ci/test-values.yaml | 3 + charts/greenhouse/values.yaml | 3 + charts/manager/ci/test-values.yaml | 2 + charts/manager/templates/_helpers.tpl | 4 + .../templates/manager/feature-flag.yaml | 7 + cmd/greenhouse/controllers.go | 8 +- dev-env/dev.values.yaml | 2 + docs/reference/components/pluginpreset.md | 113 ++++- .../plugin/plugin_controller_flux.go | 12 +- .../plugin/plugin_values_resolver.go | 20 +- .../plugin/pluginpreset_controller.go | 38 +- .../plugin/pluginpreset_controller_test.go | 433 ++++++++++++++---- .../plugin/pluginpreset_values_resolver.go | 164 +++++++ internal/controller/plugin/suite_test.go | 4 +- internal/features/features.go | 43 +- internal/features/features_test.go | 121 +++++ internal/helm/cel.go | 1 + internal/helm/helm.go | 10 +- internal/test/resources.go | 8 +- internal/webhook/v1alpha1/plugin_webhook.go | 2 +- .../webhook/v1alpha1/plugin_webhook_test.go | 2 +- 22 files changed, 900 insertions(+), 139 deletions(-) create mode 100644 .github/workflows/backport.yaml create mode 100644 internal/controller/plugin/pluginpreset_values_resolver.go diff --git a/.github/workflows/backport.yaml b/.github/workflows/backport.yaml new file mode 100644 index 000000000..2fbe97393 --- /dev/null +++ b/.github/workflows/backport.yaml @@ -0,0 +1,39 @@ +name: backport +on: + pull_request_target: + types: [closed, labeled] + +jobs: + backport: + if: github.event.pull_request.state == 'closed' && github.event.pull_request.merged && (github.event_name != 'labeled' || startsWith('backport:', github.event.label.name)) + runs-on: [ ubuntu-latest ] + steps: + - name: Generate GitHub App Token + id: github-app-token + uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 + with: + app-id: ${{ secrets.CLOUDOPERATOR_APP_ID }} + private-key: ${{ secrets.CLOUDOPERATOR_APP_PRIVATE_KEY }} + permission-contents: write + permission-pull-requests: write + - name: Checkout + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + ref: ${{ github.event.pull_request.head.sha }} + token: ${{ steps.github-app-token.outputs.token }} + - name: Create backport PRs + uses: korthout/backport-action@66065406958f46e82238fd59546f5a99e69e22aa # v4.5.2 + # xref: https://github.com/korthout/backport-action#inputs + with: + # Use token to allow workflows to be triggered for the created PR + github_token: ${{ steps.github-app-token.outputs.token }} + # Match labels with a pattern `backport:` + label_pattern: '^backport:(release\/v[0-9]+.[0-9]+)$' + # A bit shorter pull-request title than the default + pull_title: 'backport(${target_branch}): ${pull_title}' + # Simpler PR description than default + pull_description: |- + Automated cherry-pick to `${target_branch}`, triggered by a label in PR#${pull_number}. + # Name and Email of the cloud-operator-bot, used commit the cherry-pick & create the PR + git_commiter_name: cloud-operator-bot[bot] + git_commiter_email: 224791424+cloud-operator-bot[bot]@users.noreply.github.com diff --git a/charts/greenhouse/ci/test-values.yaml b/charts/greenhouse/ci/test-values.yaml index c665f2b93..27fe37c41 100644 --- a/charts/greenhouse/ci/test-values.yaml +++ b/charts/greenhouse/ci/test-values.yaml @@ -15,6 +15,9 @@ global: expressionEvaluationEnabled: false integrationEnabled: false ociMirroringEnabled: false + # PluginPreset configuration for Greenhouse. + pluginPreset: + expressionEvaluationEnabled: true linkerd_enabled: false region: greenhouse registry: ghcr.io/cloudoperators/greenhouse diff --git a/charts/greenhouse/values.yaml b/charts/greenhouse/values.yaml index 1ea34bd10..9dc09a6c3 100644 --- a/charts/greenhouse/values.yaml +++ b/charts/greenhouse/values.yaml @@ -23,6 +23,9 @@ global: expressionEvaluationEnabled: false integrationEnabled: false ociMirroringEnabled: false + # PluginPreset configuration for Greenhouse. + pluginPreset: + expressionEvaluationEnabled: true postgresqlng: enabled: true diff --git a/charts/manager/ci/test-values.yaml b/charts/manager/ci/test-values.yaml index 897d90f33..57cb691bd 100644 --- a/charts/manager/ci/test-values.yaml +++ b/charts/manager/ci/test-values.yaml @@ -13,6 +13,8 @@ global: expressionEvaluationEnabled: false integrationEnabled: false ociMirroringEnabled: false + pluginPreset: + expressionEvaluationEnabled: true controllerManager: args: diff --git a/charts/manager/templates/_helpers.tpl b/charts/manager/templates/_helpers.tpl index de495a13b..338e96f37 100644 --- a/charts/manager/templates/_helpers.tpl +++ b/charts/manager/templates/_helpers.tpl @@ -117,3 +117,7 @@ Define postgresql helpers {{- define "plugin.ociMirroringEnabled" -}} {{- printf "%t" (required "global.plugin.ociMirroringEnabled missing" .Values.global.plugin.ociMirroringEnabled) }} {{- end }} +{{/* Render the pluginPreset expression evaluation flag */}} +{{- define "pluginPreset.expressionEvaluationEnabled" -}} + {{- printf "%t" (required "global.pluginPreset.expressionEvaluationEnabled missing" .Values.global.pluginPreset.expressionEvaluationEnabled) }} +{{- end }} \ No newline at end of file diff --git a/charts/manager/templates/manager/feature-flag.yaml b/charts/manager/templates/manager/feature-flag.yaml index 6bfb1ac94..87043f10d 100644 --- a/charts/manager/templates/manager/feature-flag.yaml +++ b/charts/manager/templates/manager/feature-flag.yaml @@ -26,9 +26,16 @@ data: expressionEvaluationEnabled: false / true integrationEnabled: false / true ociMirroringEnabled: false / true + # enable pluginPreset features + # expressionEvaluationEnabled allows you to enable or disable CEL expression evaluation in PluginPreset + # when enabled, expressions in PluginPreset.spec.plugin.optionValues are evaluated before creating the Plugin + pluginPreset: | + expressionEvaluationEnabled: false / true dex: | storage: {{ include "dex.backend" $ }} plugin: | expressionEvaluationEnabled: {{ include "plugin.expressionEvaluationEnabled" $ }} integrationEnabled: {{ include "plugin.integrationEnabled" $ }} ociMirroringEnabled: {{ include "plugin.ociMirroringEnabled" $ }} + pluginPreset: | + expressionEvaluationEnabled: {{ include "pluginPreset.expressionEvaluationEnabled" $ }} diff --git a/cmd/greenhouse/controllers.go b/cmd/greenhouse/controllers.go index 98992f02b..0ed5b6725 100644 --- a/cmd/greenhouse/controllers.go +++ b/cmd/greenhouse/controllers.go @@ -35,7 +35,7 @@ var knownControllers = map[string]func(controllerName string, mgr ctrl.Manager) // Plugin controllers. "plugin": startPluginReconciler, - "pluginPreset": (&plugincontrollers.PluginPresetReconciler{}).SetupWithManager, + "pluginPreset": startPluginPresetReconciler, "catalog": startCatalogReconciler, "pluginDefinition": startPluginDefinitionReconciler, @@ -93,6 +93,12 @@ func startPluginReconciler(name string, mgr ctrl.Manager) error { }).SetupWithManager(name, mgr) } +func startPluginPresetReconciler(name string, mgr ctrl.Manager) error { + return (&plugincontrollers.PluginPresetReconciler{ + ExpressionEvaluationEnabled: featureFlags.IsPresetExpressionEvaluationEnabled(), + }).SetupWithManager(name, mgr) +} + func startPluginDefinitionReconciler(name string, mgr ctrl.Manager) error { return (&plugindefinitioncontroller.PluginDefinitionReconciler{ OCIMirroringEnabled: featureFlags.IsOCIMirroringEnabled(), diff --git a/dev-env/dev.values.yaml b/dev-env/dev.values.yaml index e0a190fac..d32d1770a 100644 --- a/dev-env/dev.values.yaml +++ b/dev-env/dev.values.yaml @@ -9,6 +9,8 @@ global: expressionEvaluationEnabled: true integrationEnabled: true ociMirroringEnabled: true + pluginPreset: + expressionEvaluationEnabled: true alerts: enabled: false certManager: diff --git a/docs/reference/components/pluginpreset.md b/docs/reference/components/pluginpreset.md index 95737d8a9..f73661907 100644 --- a/docs/reference/components/pluginpreset.md +++ b/docs/reference/components/pluginpreset.md @@ -33,6 +33,9 @@ spec: optionValues: - name: perses.sidecar.enabled value: true + - name: perses.ingress.host + expression: | + "perses.${global.greenhouse.clusterName}.example.com" pluginDefinitionRef: kind: ClusterPluginDefinition name: perses @@ -86,8 +89,116 @@ spec: `.spec.deletionPolicy` is an optional field that specifies the behaviour when a PluginPreset is deleted. The possible values are `Delete` and `Retain`. If set to `Delete` (the default), all Plugins created by the PluginPreset will also be deleted when the PluginPreset is deleted. If set to `Retain`, the Plugins will remain after the PluginPreset is deleted or if the Cluster stops matching the selector. +## CEL Expressions in OptionValues + +PluginPresets support CEL (Common Expression Language) expressions in `optionValues`. Expressions are evaluated during PluginPreset reconciliation, and the resulting Plugin contains only the resolved values with no expression fields remaining. + +Expressions use the `${...}` syntax to reference dynamic values: + +```yaml +spec: + plugin: + optionValues: + - name: app.hostname + expression: | + "myapp.${global.greenhouse.clusterName}.example.com" +``` + +When this PluginPreset creates a Plugin for a cluster named `cluster-a`, the Plugin will contain: + +```yaml +spec: + optionValues: + - name: app.hostname + value: "myapp.cluster-a.example.com" +``` + +### Available Variables + +| Variable | Description | Example Value | +|--------------------------------------|----------------------------|------------------------------| +| `global.greenhouse.clusterName` | Name of the target cluster | `cluster-a` | +| `global.greenhouse.organizationName` | Organization namespace | `my-org` | +| `global.greenhouse.clusterNames` | List of all cluster names | `["cluster-a", "cluster-b"]` | +| `global.greenhouse.teamNames` | List of all team names | `["team-1", "team-2"]` | +| `global.greenhouse.baseDomain` | Base DNS domain | `greenhouse.example.com` | +| `global.greenhouse.metadata.*` | Cluster metadata labels | `eu-de-1` | + +> :information_source: `global.greenhouse.metadata.*` values are derived from cluster labels prefixed with `metadata.greenhouse.sap/`. For example, the label `metadata.greenhouse.sap/region: eu-de-1` becomes available as `global.greenhouse.metadata.region`. + +### Examples + +**Hostname per cluster:** + +```yaml +- name: ingress.host + expression: | + "service.${global.greenhouse.clusterName}.example.com" +# Result for cluster "cluster-a": "service.cluster-a.example.com" +``` + +**Using cluster metadata:** + +```yaml +- name: ingress.host + expression: | + "service.${global.greenhouse.metadata.region}.example.com" +# Result: "service.eu-de-1.example.com" +# Requires label metadata.greenhouse.sap/region on the cluster +``` + +**Combining variables:** + +```yaml +- name: app.fqdn + expression: | + "${global.greenhouse.clusterName}-${global.greenhouse.organizationName}" +# Result for cluster "cluster-a" in org "my-org": "cluster-a-my-org" +``` + +### Expressions in ClusterOptionOverrides + +Expressions can also be used in `clusterOptionOverrides`. Overrides are merged before expression evaluation, so override expressions are also resolved: + +```yaml +spec: + plugin: + optionValues: + - name: app.mode + value: "standard" + clusterOptionOverrides: + - clusterName: special-cluster + overrides: + - name: app.hostname + expression: | + "special.${global.greenhouse.metadata.region}.example.com" +``` + +> :information_source: Expressions are only evaluated in PluginPresets. + +> :warning: CEL expressions on standalone Plugins are deprecated and will be removed in a future release. Use PluginPresets for expression evaluation. + + +## Feature Flag + +CEL expression evaluation in PluginPresets requires the feature flag `pluginPreset.expressionEvaluationEnabled` to be set to `true` in the Greenhouse feature flags ConfigMap. + +When disabled (default: `true`), expressions are passed through as literal values without evaluation. + +```yaml +# greenhouse-feature-flags ConfigMap +apiVersion: v1 +kind: ConfigMap +metadata: + name: greenhouse-feature-flags + namespace: greenhouse +data: + pluginPreset: | + expressionEvaluationEnabled: true +``` + ## Next Steps - [Managing Plugins for multiple clusters](./../../../user-guides/plugin/plugin-management) - [Plugin reference](./../plugin) -- [PluginDefinition reference](./../plugindefinition) +- [PluginDefinition reference](./../plugindefinition) \ No newline at end of file diff --git a/internal/controller/plugin/plugin_controller_flux.go b/internal/controller/plugin/plugin_controller_flux.go index 38dd8218c..7ee1a9f35 100644 --- a/internal/controller/plugin/plugin_controller_flux.go +++ b/internal/controller/plugin/plugin_controller_flux.go @@ -468,9 +468,11 @@ func computeReleaseValues(ctx context.Context, c client.Client, plugin *greenhou // noop, direct values are already set continue - case v.Expression != nil: + case v.Expression != nil: //nolint:staticcheck // SA1019: deprecated field kept for later clean up + // TODO(#1775): Expressions should no longer be evaluated by Plugin controller. + // Once PluginPreset controller handles all expression evaluation, + // this branch should return an error instead of evaluating. if !expressionEvaluation { - // skip expression evaluation if not enabled continue } resolvedOptionValue, err := celResolver.ResolveExpression(v, expressionEvaluation) @@ -479,8 +481,10 @@ func computeReleaseValues(ctx context.Context, c client.Client, plugin *greenhou } optionValues[i] = *resolvedOptionValue - case v.ValueFrom != nil && v.ValueFrom.Ref != nil: - // skip if integration flag is not enabled + case v.ValueFrom != nil && v.ValueFrom.Ref != nil: //nolint:staticcheck // SA1019: deprecated field kept for later clean up + // TODO(#1776): References should no longer be resolved by Plugin controller. + // Once PluginPreset controller handles all reference resolution, + // this branch should return an error instead of resolving. if !integrationEnabled { continue } diff --git a/internal/controller/plugin/plugin_values_resolver.go b/internal/controller/plugin/plugin_values_resolver.go index 7248df010..ba6d3d4d9 100644 --- a/internal/controller/plugin/plugin_values_resolver.go +++ b/internal/controller/plugin/plugin_values_resolver.go @@ -38,7 +38,7 @@ const ( // filterValueRefOptions filters option values to only include those with external references (ValueFrom.Ref). func filterValueRefOptions(optionValues []greenhousev1alpha1.PluginOptionValue) []greenhousev1alpha1.PluginOptionValue { return slices.DeleteFunc(optionValues, func(o greenhousev1alpha1.PluginOptionValue) bool { - return o.ValueFrom == nil || o.ValueFrom.Ref == nil + return o.ValueFrom == nil || o.ValueFrom.Ref == nil //nolint:staticcheck // SA1019: deprecated field kept for later clean up }) } @@ -61,20 +61,20 @@ func resolveValueFromRef(ctx context.Context, c client.Client, plugin *greenhous var trackedObjects []string var err error resolveKind := defaultKind - if option.ValueFrom.Ref.Kind != "" { - resolveKind = option.ValueFrom.Ref.Kind + if option.ValueFrom.Ref.Kind != "" { //nolint:staticcheck // SA1019: deprecated fields kept for later clean up + resolveKind = option.ValueFrom.Ref.Kind //nolint:staticcheck // SA1019: deprecated fields kept for later clean up } gvk := buildGVK(resolveKind) // resolve by name - if option.ValueFrom.Ref.Name != "" { + if option.ValueFrom.Ref.Name != "" { //nolint:staticcheck // SA1019: deprecated fields kept for later clean up value, err = resolveByName(ctx, c, plugin, option, gvk, tracker) if err != nil { return nil, nil, err } - trackedObjects = append(trackedObjects, trackingID(resolveKind, option.ValueFrom.Ref.Name)) + trackedObjects = append(trackedObjects, trackingID(resolveKind, option.ValueFrom.Ref.Name)) //nolint:staticcheck // SA1019: deprecated fields kept for later clean up } // resolve by label selector - if option.ValueFrom.Ref.Selector != nil { + if option.ValueFrom.Ref.Selector != nil { //nolint:staticcheck // SA1019: deprecated fields kept for later clean up var selectorTrackedObjects []string value, selectorTrackedObjects, err = resolveBySelector(ctx, c, plugin, option, gvk, tracker) if err != nil { @@ -102,7 +102,7 @@ func resolveValueFromRef(ctx context.Context, c client.Client, plugin *greenhous // resolveByName resolves an option value by fetching a specific named resource. func resolveByName(ctx context.Context, c client.Client, plugin *greenhousev1alpha1.Plugin, option greenhousev1alpha1.PluginOptionValue, gvk schema.GroupVersionKind, tracker string) (any, error) { key := types.NamespacedName{ - Name: option.ValueFrom.Ref.Name, + Name: option.ValueFrom.Ref.Name, //nolint:staticcheck // SA1019: deprecated fields kept for later clean up Namespace: plugin.GetNamespace(), } uObject := &unstructured.Unstructured{} @@ -122,7 +122,7 @@ func resolveByName(ctx context.Context, c client.Client, plugin *greenhousev1alp "namespace", key.Namespace) return nil, err } - value, err := evaluateExpression(ctx, uObject, option.ValueFrom.Ref.Expression) + value, err := evaluateExpression(ctx, uObject, option.ValueFrom.Ref.Expression) //nolint:staticcheck // SA1019: deprecated fields kept for later clean up if err != nil { return nil, err } @@ -138,7 +138,7 @@ func resolveByName(ctx context.Context, c client.Client, plugin *greenhousev1alp // resolveBySelector resolves option values by fetching resources matching a label selector. func resolveBySelector(ctx context.Context, c client.Client, plugin *greenhousev1alpha1.Plugin, option greenhousev1alpha1.PluginOptionValue, gvk schema.GroupVersionKind, tracker string) (value any, trackedObjects []string, err error) { - selector, err := metav1.LabelSelectorAsSelector(option.ValueFrom.Ref.Selector) + selector, err := metav1.LabelSelectorAsSelector(option.ValueFrom.Ref.Selector) //nolint:staticcheck // SA1019: deprecated fields kept for later clean up if err != nil { log.FromContext(ctx).Error(err, "failed to parse label selector", "namespace", plugin.GetNamespace(), @@ -147,7 +147,7 @@ func resolveBySelector(ctx context.Context, c client.Client, plugin *greenhousev return nil, nil, err } - value, trackedObjects, err = resolveMany(ctx, c, gvk, selector, plugin.GetNamespace(), option.ValueFrom.Ref.Expression, tracker) + value, trackedObjects, err = resolveMany(ctx, c, gvk, selector, plugin.GetNamespace(), option.ValueFrom.Ref.Expression, tracker) //nolint:staticcheck // SA1019: deprecated fields kept for later clean up if err != nil { return nil, nil, err } diff --git a/internal/controller/plugin/pluginpreset_controller.go b/internal/controller/plugin/pluginpreset_controller.go index 84c9a8579..e793d8aa3 100644 --- a/internal/controller/plugin/pluginpreset_controller.go +++ b/internal/controller/plugin/pluginpreset_controller.go @@ -44,7 +44,8 @@ var presetExposedConditions = []greenhousemetav1alpha1.ConditionType{ // PluginPresetReconciler reconciles a PluginPreset object type PluginPresetReconciler struct { client.Client - recorder events.EventRecorder + recorder events.EventRecorder + ExpressionEvaluationEnabled bool } //+kubebuilder:rbac:groups=greenhouse.sap,resources=pluginpresets,verbs=get;list;watch;update @@ -238,12 +239,19 @@ func (r *PluginPresetReconciler) reconcilePluginPreset(ctx context.Context, pres releaseName := getReleaseName(plugin, preset) + // Apply overrides first, then resolve expressions + presetWithOverrides := applyOverridesToPreset(preset, cluster.GetName()) + + resolvedValues, err := r.resolvePluginOptionValuesForPreset(ctx, presetWithOverrides, &cluster) + if err != nil { + return fmt.Errorf("failed to resolve option values for plugin %s: %w", plugin.Name, err) + } + plugin.Spec = pluginSpecFromPluginPreset(preset, cluster.GetName()) + plugin.Spec.OptionValues = resolvedValues plugin.Spec.ReleaseName = releaseName // transport plugin preset labels to plugin plugin = (lifecycle.NewPropagator(preset, plugin).Apply()).(*greenhousev1alpha1.Plugin) - // overrides options based on preset definition - overridesPluginOptionValues(plugin, preset) return nil }) if err != nil { @@ -333,30 +341,6 @@ func isPluginManagedByPreset(plugin *greenhousev1alpha1.Plugin, presetName strin return plugin.Labels[greenhouseapis.LabelKeyPluginPreset] == presetName } -func overridesPluginOptionValues(plugin *greenhousev1alpha1.Plugin, preset *greenhousev1alpha1.PluginPreset) { - index := slices.IndexFunc(preset.Spec.ClusterOptionOverrides, func(override greenhousev1alpha1.ClusterOptionOverride) bool { - return override.ClusterName == plugin.Spec.ClusterName - }) - - // when plugin is running on different cluster then defined in - if index == -1 { - return - } - - // overrides value - for _, overrideValue := range preset.Spec.ClusterOptionOverrides[index].Overrides { - valueIndex := slices.IndexFunc(plugin.Spec.OptionValues, func(value greenhousev1alpha1.PluginOptionValue) bool { - return value.Name == overrideValue.Name - }) - - if valueIndex == -1 { - plugin.Spec.OptionValues = append(plugin.Spec.OptionValues, overrideValue) - } else { - plugin.Spec.OptionValues[valueIndex] = overrideValue - } - } -} - // generatePluginName generates a name for a plugin based on the used PluginPreset's name and the Cluster. func generatePluginName(p *greenhousev1alpha1.PluginPreset, cluster *greenhousev1alpha1.Cluster) string { return buildPluginName(p.Name, cluster.GetName()) diff --git a/internal/controller/plugin/pluginpreset_controller_test.go b/internal/controller/plugin/pluginpreset_controller_test.go index 1cbaf0874..7a189d21d 100644 --- a/internal/controller/plugin/pluginpreset_controller_test.go +++ b/internal/controller/plugin/pluginpreset_controller_test.go @@ -890,152 +890,427 @@ var _ = Describe("PluginPreset Controller Lifecycle", Ordered, func() { By("removing plugin preset") test.EventuallyDeleted(test.Ctx, test.K8sClient, pluginPreset) }) + + It("should resolve a simple expression using clusterName", func() { + By("creating a PluginPreset with an expression") + expressionStr := `"app-${global.greenhouse.clusterName}.example.com"` + pluginSpec := greenhousev1alpha1.PluginSpec{ + PluginDefinitionRef: greenhousev1alpha1.PluginDefinitionReference{ + Kind: greenhousev1alpha1.ClusterPluginDefinitionKind, + Name: pluginPresetDefinitionName, + }, + ReleaseName: releaseName, + ReleaseNamespace: releaseNamespace, + OptionValues: []greenhousev1alpha1.PluginOptionValue{ + { + Name: "myRequiredOption", + Value: test.MustReturnJSONFor("myValue"), + }, + { + Name: "test.hostname", + Expression: &expressionStr, + }, + }, + } + + pluginPreset := test.NewPluginPreset("expr-simple", test.TestNamespace, + test.WithPluginPresetLabel(greenhouseapis.LabelKeyOwnedBy, testTeam.Name), + test.WithPluginPresetPluginSpec(pluginSpec), + test.WithPluginPresetClusterSelector(metav1.LabelSelector{ + MatchLabels: map[string]string{ + "cluster": clusterA, + }, + })) + Expect(test.K8sClient.Create(test.Ctx, pluginPreset)).To(Succeed()) + + By("ensuring Plugin has resolved expression value") + expPluginName := types.NamespacedName{Name: "expr-simple-" + clusterA, Namespace: test.TestNamespace} + expPlugin := &greenhousev1alpha1.Plugin{} + Eventually(func(g Gomega) { + err := test.K8sClient.Get(test.Ctx, expPluginName, expPlugin) + g.Expect(err).ToNot(HaveOccurred(), "Plugin should exist") + + var hostnameFound bool + for _, ov := range expPlugin.Spec.OptionValues { + if ov.Name == "test.hostname" { + hostnameFound = true + g.Expect(ov.Expression).To(BeNil(), "Expression should be resolved") //nolint:staticcheck // SA1019: checking deprecated field in test + g.Expect(ov.Value).ToNot(BeNil(), "Value should be set") + g.Expect(string(ov.Value.Raw)).To(Equal(`"app-`+clusterA+`.example.com"`), + "Expression should resolve with cluster name") + } + } + g.Expect(hostnameFound).To(BeTrue(), "test.hostname should exist in Plugin") + }).Should(Succeed()) + + By("removing the PluginPreset") + test.EventuallyDeleted(test.Ctx, test.K8sClient, pluginPreset) + }) + + It("should resolve expression with cluster metadata", func() { + By("adding metadata labels to clusterA") + clusterAObj := &greenhousev1alpha1.Cluster{} + Expect(test.K8sClient.Get(test.Ctx, types.NamespacedName{ + Name: clusterA, Namespace: test.TestNamespace, + }, clusterAObj)).To(Succeed()) + + _, err := clientutil.CreateOrPatch(test.Ctx, test.K8sClient, clusterAObj, func() error { + clusterAObj.Labels["metadata.greenhouse.sap/region"] = "eu-de-1" + return nil + }) + Expect(err).NotTo(HaveOccurred()) + + By("creating a PluginPreset with metadata expression") + expressionStr := `"service.${global.greenhouse.metadata.region}.example.com"` + pluginSpec := greenhousev1alpha1.PluginSpec{ + PluginDefinitionRef: greenhousev1alpha1.PluginDefinitionReference{ + Kind: greenhousev1alpha1.ClusterPluginDefinitionKind, + Name: pluginPresetDefinitionName, + }, + ReleaseName: releaseName, + ReleaseNamespace: releaseNamespace, + OptionValues: []greenhousev1alpha1.PluginOptionValue{ + { + Name: "myRequiredOption", + Value: test.MustReturnJSONFor("myValue"), + }, + { + Name: "test.serviceHost", + Expression: &expressionStr, + }, + }, + } + + pluginPreset := test.NewPluginPreset("expr-metadata", test.TestNamespace, + test.WithPluginPresetLabel(greenhouseapis.LabelKeyOwnedBy, testTeam.Name), + test.WithPluginPresetPluginSpec(pluginSpec), + test.WithPluginPresetClusterSelector(metav1.LabelSelector{ + MatchLabels: map[string]string{ + "cluster": clusterA, + }, + })) + Expect(test.K8sClient.Create(test.Ctx, pluginPreset)).To(Succeed()) + + By("ensuring Plugin has resolved metadata expression") + expPluginName := types.NamespacedName{Name: "expr-metadata-" + clusterA, Namespace: test.TestNamespace} + expPlugin := &greenhousev1alpha1.Plugin{} + Eventually(func(g Gomega) { + err := test.K8sClient.Get(test.Ctx, expPluginName, expPlugin) + g.Expect(err).ToNot(HaveOccurred()) + + var found bool + for _, ov := range expPlugin.Spec.OptionValues { + if ov.Name == "test.serviceHost" { + found = true + g.Expect(ov.Expression).To(BeNil()) //nolint:staticcheck // SA1019 + g.Expect(ov.Value).ToNot(BeNil()) + g.Expect(string(ov.Value.Raw)).To(Equal(`"service.eu-de-1.example.com"`)) + } + } + g.Expect(found).To(BeTrue()) + }).Should(Succeed()) + + By("cleaning up metadata label") + Expect(test.K8sClient.Get(test.Ctx, types.NamespacedName{ + Name: clusterA, Namespace: test.TestNamespace, + }, clusterAObj)).To(Succeed()) + _, err = clientutil.CreateOrPatch(test.Ctx, test.K8sClient, clusterAObj, func() error { + delete(clusterAObj.Labels, "metadata.greenhouse.sap/region") + return nil + }) + Expect(err).NotTo(HaveOccurred()) + + test.EventuallyDeleted(test.Ctx, test.K8sClient, pluginPreset) + }) + + It("should keep direct values unchanged when resolving expressions", func() { + expressionStr := `"generated-${global.greenhouse.clusterName}"` + pluginSpec := greenhousev1alpha1.PluginSpec{ + PluginDefinitionRef: greenhousev1alpha1.PluginDefinitionReference{ + Kind: greenhousev1alpha1.ClusterPluginDefinitionKind, + Name: pluginPresetDefinitionName, + }, + ReleaseName: releaseName, + ReleaseNamespace: releaseNamespace, + OptionValues: []greenhousev1alpha1.PluginOptionValue{ + { + Name: "myRequiredOption", + Value: test.MustReturnJSONFor("myValue"), + }, + { + Name: "direct.value", + Value: test.MustReturnJSONFor("unchanged"), + }, + { + Name: "expression.value", + Expression: &expressionStr, + }, + }, + } + + pluginPreset := test.NewPluginPreset("expr-mixed", test.TestNamespace, + test.WithPluginPresetLabel(greenhouseapis.LabelKeyOwnedBy, testTeam.Name), + test.WithPluginPresetPluginSpec(pluginSpec), + test.WithPluginPresetClusterSelector(metav1.LabelSelector{ + MatchLabels: map[string]string{ + "cluster": clusterA, + }, + })) + Expect(test.K8sClient.Create(test.Ctx, pluginPreset)).To(Succeed()) + + expPluginName := types.NamespacedName{Name: "expr-mixed-" + clusterA, Namespace: test.TestNamespace} + expPlugin := &greenhousev1alpha1.Plugin{} + Eventually(func(g Gomega) { + err := test.K8sClient.Get(test.Ctx, expPluginName, expPlugin) + g.Expect(err).ToNot(HaveOccurred()) + + g.Expect(expPlugin.Spec.OptionValues).To(ContainElement( + greenhousev1alpha1.PluginOptionValue{ + Name: "direct.value", + Value: test.MustReturnJSONFor("unchanged"), + }), "Direct value should be unchanged") + + var exprResolved bool + for _, ov := range expPlugin.Spec.OptionValues { + if ov.Name == "expression.value" { + exprResolved = true + g.Expect(ov.Expression).To(BeNil()) //nolint:staticcheck // SA1019 + g.Expect(ov.Value).ToNot(BeNil()) + g.Expect(string(ov.Value.Raw)).To(Equal(`"generated-` + clusterA + `"`)) + } + } + g.Expect(exprResolved).To(BeTrue()) + }).Should(Succeed()) + + test.EventuallyDeleted(test.Ctx, test.K8sClient, pluginPreset) + }) + + It("should report error for invalid expression", func() { + invalidExpressionStr := `"service.${global.greenhouse.nonexistent.field}.example.com"` + pluginSpec := greenhousev1alpha1.PluginSpec{ + PluginDefinitionRef: greenhousev1alpha1.PluginDefinitionReference{ + Kind: greenhousev1alpha1.ClusterPluginDefinitionKind, + Name: pluginPresetDefinitionName, + }, + ReleaseName: releaseName, + ReleaseNamespace: releaseNamespace, + OptionValues: []greenhousev1alpha1.PluginOptionValue{ + { + Name: "myRequiredOption", + Value: test.MustReturnJSONFor("myValue"), + }, + { + Name: "test.invalid", + Expression: &invalidExpressionStr, + }, + }, + } + + pluginPreset := test.NewPluginPreset("expr-invalid", test.TestNamespace, + test.WithPluginPresetLabel(greenhouseapis.LabelKeyOwnedBy, testTeam.Name), + test.WithPluginPresetPluginSpec(pluginSpec), + test.WithPluginPresetClusterSelector(metav1.LabelSelector{ + MatchLabels: map[string]string{ + "cluster": clusterA, + }, + })) + Expect(test.K8sClient.Create(test.Ctx, pluginPreset)).To(Succeed()) + + Eventually(func(g Gomega) { + err := test.K8sClient.Get(test.Ctx, client.ObjectKeyFromObject(pluginPreset), pluginPreset) + g.Expect(err).ToNot(HaveOccurred()) + + pluginFailedCondition := pluginPreset.Status.GetConditionByType(greenhousev1alpha1.PluginFailedCondition) + g.Expect(pluginFailedCondition).ToNot(BeNil()) + g.Expect(pluginFailedCondition.Status).To(Equal(metav1.ConditionTrue)) + g.Expect(pluginFailedCondition.Message).To(ContainSubstring("failed to resolve")) + }).Should(Succeed()) + + test.EventuallyDeleted(test.Ctx, test.K8sClient, pluginPreset) + }) }) -var _ = Describe("overridesPluginOptionValues", Ordered, func() { - DescribeTable("test cases", func(plugin *greenhousev1alpha1.Plugin, preset *greenhousev1alpha1.PluginPreset, expectedPlugin *greenhousev1alpha1.Plugin) { - overridesPluginOptionValues(plugin, preset) - Expect(plugin).To(BeEquivalentTo(expectedPlugin)) - }, - Entry("with no defined pluginPresetOverrides", - test.NewPlugin(test.Ctx, "", "", test.WithPluginOptionValue("option-1", test.MustReturnJSONFor(2)), test.WithPluginLabel(greenhouseapis.LabelKeyOwnedBy, testTeam.Name)), +var _ = Describe("applyOverridesToPreset", func() { + DescribeTable("test cases", + func(preset *greenhousev1alpha1.PluginPreset, clusterName string, expectedOptionValues []greenhousev1alpha1.PluginOptionValue) { + result := applyOverridesToPreset(preset, clusterName) + Expect(result.Spec.Plugin.OptionValues).To(Equal(expectedOptionValues)) + }, + + Entry("with no overrides defined", &greenhousev1alpha1.PluginPreset{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{greenhouseapis.LabelKeyOwnedBy: testTeam.Name}, + Spec: greenhousev1alpha1.PluginPresetSpec{ + Plugin: greenhousev1alpha1.PluginPresetPluginSpec{ + OptionValues: []greenhousev1alpha1.PluginOptionValue{ + {Name: "option-1", Value: test.MustReturnJSONFor("value-1")}, + }, + }, }, - Spec: greenhousev1alpha1.PluginPresetSpec{}, }, - test.NewPlugin(test.Ctx, "", "", test.WithPluginOptionValue("option-1", test.MustReturnJSONFor(2)), test.WithPluginLabel(greenhouseapis.LabelKeyOwnedBy, testTeam.Name)), + clusterA, + []greenhousev1alpha1.PluginOptionValue{ + {Name: "option-1", Value: test.MustReturnJSONFor("value-1")}, + }, ), - Entry("with defined pluginPresetOverrides but for another cluster", - test.NewPlugin(test.Ctx, "", clusterA, test.WithPluginLabel(greenhouseapis.LabelKeyOwnedBy, testTeam.Name), - test.WithCluster(clusterA), test.WithPluginOptionValue("option-1", test.MustReturnJSONFor(2))), + + Entry("with overrides for a different cluster", &greenhousev1alpha1.PluginPreset{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{greenhouseapis.LabelKeyOwnedBy: testTeam.Name}, - }, Spec: greenhousev1alpha1.PluginPresetSpec{ + Plugin: greenhousev1alpha1.PluginPresetPluginSpec{ + OptionValues: []greenhousev1alpha1.PluginOptionValue{ + {Name: "option-1", Value: test.MustReturnJSONFor("value-1")}, + }, + }, ClusterOptionOverrides: []greenhousev1alpha1.ClusterOptionOverride{ { ClusterName: clusterB, Overrides: []greenhousev1alpha1.PluginOptionValue{ - { - Name: "option-1", - Value: test.MustReturnJSONFor(1), - }, + {Name: "option-1", Value: test.MustReturnJSONFor("overridden")}, }, }, }, }, }, - test.NewPlugin(test.Ctx, "", clusterA, test.WithPluginLabel(greenhouseapis.LabelKeyOwnedBy, testTeam.Name), - test.WithCluster(clusterA), test.WithPluginOptionValue("option-1", test.MustReturnJSONFor(2))), + clusterA, + []greenhousev1alpha1.PluginOptionValue{ + {Name: "option-1", Value: test.MustReturnJSONFor("value-1")}, + }, ), - Entry("with defined pluginPresetOverrides for the correct cluster", - test.NewPlugin(test.Ctx, "", clusterA, test.WithCluster(clusterA), test.WithPluginOptionValue("option-1", test.MustReturnJSONFor(2)), test.WithPluginLabel(greenhouseapis.LabelKeyOwnedBy, testTeam.Name)), + + Entry("with overrides for matching cluster - replaces existing value", &greenhousev1alpha1.PluginPreset{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{greenhouseapis.LabelKeyOwnedBy: testTeam.Name}, - }, Spec: greenhousev1alpha1.PluginPresetSpec{ + Plugin: greenhousev1alpha1.PluginPresetPluginSpec{ + OptionValues: []greenhousev1alpha1.PluginOptionValue{ + {Name: "option-1", Value: test.MustReturnJSONFor("original")}, + {Name: "option-2", Value: test.MustReturnJSONFor("unchanged")}, + }, + }, ClusterOptionOverrides: []greenhousev1alpha1.ClusterOptionOverride{ { ClusterName: clusterA, Overrides: []greenhousev1alpha1.PluginOptionValue{ - { - Name: "option-1", - Value: test.MustReturnJSONFor(1), - }, + {Name: "option-1", Value: test.MustReturnJSONFor("overridden")}, }, }, }, }, }, - test.NewPlugin(test.Ctx, "", clusterA, test.WithCluster(clusterA), test.WithPluginOptionValue("option-1", test.MustReturnJSONFor(1)), test.WithPluginLabel(greenhouseapis.LabelKeyOwnedBy, testTeam.Name)), + clusterA, + []greenhousev1alpha1.PluginOptionValue{ + {Name: "option-1", Value: test.MustReturnJSONFor("overridden")}, + {Name: "option-2", Value: test.MustReturnJSONFor("unchanged")}, + }, ), - Entry("with defined pluginPresetOverrides for the cluster and plugin with empty option values", - test.NewPlugin(test.Ctx, "", clusterA, test.WithCluster(clusterA), test.WithPluginLabel(greenhouseapis.LabelKeyOwnedBy, testTeam.Name)), + + Entry("with overrides for matching cluster - appends new value", &greenhousev1alpha1.PluginPreset{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{greenhouseapis.LabelKeyOwnedBy: testTeam.Name}, - }, Spec: greenhousev1alpha1.PluginPresetSpec{ + Plugin: greenhousev1alpha1.PluginPresetPluginSpec{ + OptionValues: []greenhousev1alpha1.PluginOptionValue{ + {Name: "option-1", Value: test.MustReturnJSONFor("value-1")}, + }, + }, ClusterOptionOverrides: []greenhousev1alpha1.ClusterOptionOverride{ { ClusterName: clusterA, Overrides: []greenhousev1alpha1.PluginOptionValue{ - { - Name: "option-1", - Value: test.MustReturnJSONFor(1), - }, + {Name: "option-new", Value: test.MustReturnJSONFor("new-value")}, }, }, }, }, }, - test.NewPlugin(test.Ctx, "", clusterA, test.WithCluster(clusterA), test.WithPluginOptionValue("option-1", test.MustReturnJSONFor(1)), test.WithPluginLabel(greenhouseapis.LabelKeyOwnedBy, testTeam.Name)), + clusterA, + []greenhousev1alpha1.PluginOptionValue{ + {Name: "option-1", Value: test.MustReturnJSONFor("value-1")}, + {Name: "option-new", Value: test.MustReturnJSONFor("new-value")}, + }, ), - Entry("with defined pluginPresetOverrides and plugin has two options", - test.NewPlugin(test.Ctx, "", clusterA, test.WithCluster(clusterA), test.WithPluginOptionValue("option-1", test.MustReturnJSONFor(1)), test.WithPluginOptionValue("option-2", test.MustReturnJSONFor(1)), test.WithPluginLabel(greenhouseapis.LabelKeyOwnedBy, testTeam.Name)), + + Entry("with multiple overrides - replaces and appends", &greenhousev1alpha1.PluginPreset{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{greenhouseapis.LabelKeyOwnedBy: testTeam.Name}, - }, Spec: greenhousev1alpha1.PluginPresetSpec{ + Plugin: greenhousev1alpha1.PluginPresetPluginSpec{ + OptionValues: []greenhousev1alpha1.PluginOptionValue{ + {Name: "option-1", Value: test.MustReturnJSONFor(1)}, + {Name: "option-2", Value: test.MustReturnJSONFor(2)}, + {Name: "option-3", Value: test.MustReturnJSONFor(3)}, + }, + }, ClusterOptionOverrides: []greenhousev1alpha1.ClusterOptionOverride{ { ClusterName: clusterA, Overrides: []greenhousev1alpha1.PluginOptionValue{ - { - Name: "option-2", - Value: test.MustReturnJSONFor(2), - }, + {Name: "option-2", Value: test.MustReturnJSONFor(22)}, + {Name: "option-3", Value: test.MustReturnJSONFor(33)}, + {Name: "option-4", Value: test.MustReturnJSONFor(44)}, }, }, }, }, }, - test.NewPlugin(test.Ctx, "", clusterA, test.WithCluster(clusterA), test.WithPluginOptionValue("option-1", test.MustReturnJSONFor(1)), test.WithPluginOptionValue("option-2", test.MustReturnJSONFor(2)), test.WithPluginLabel(greenhouseapis.LabelKeyOwnedBy, testTeam.Name)), + clusterA, + []greenhousev1alpha1.PluginOptionValue{ + {Name: "option-1", Value: test.MustReturnJSONFor(1)}, + {Name: "option-2", Value: test.MustReturnJSONFor(22)}, + {Name: "option-3", Value: test.MustReturnJSONFor(33)}, + {Name: "option-4", Value: test.MustReturnJSONFor(44)}, + }, ), - Entry("with defined pluginPresetOverrides has multiple options to override", - test.NewPlugin(test.Ctx, "", clusterA, test.WithCluster(clusterA), - test.WithPluginOptionValue("option-1", test.MustReturnJSONFor(1)), - test.WithPluginOptionValue("option-2", test.MustReturnJSONFor(1)), - test.WithPluginOptionValue("option-3", test.MustReturnJSONFor(1)), - test.WithPluginLabel(greenhouseapis.LabelKeyOwnedBy, testTeam.Name)), + + Entry("with empty option values and overrides adds values", &greenhousev1alpha1.PluginPreset{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{greenhouseapis.LabelKeyOwnedBy: testTeam.Name}, - }, Spec: greenhousev1alpha1.PluginPresetSpec{ + Plugin: greenhousev1alpha1.PluginPresetPluginSpec{ + OptionValues: []greenhousev1alpha1.PluginOptionValue{}, + }, ClusterOptionOverrides: []greenhousev1alpha1.ClusterOptionOverride{ { ClusterName: clusterA, Overrides: []greenhousev1alpha1.PluginOptionValue{ - { - Name: "option-2", - Value: test.MustReturnJSONFor(2), - }, - { - Name: "option-3", - Value: test.MustReturnJSONFor(2), - }, - { - Name: "option-4", - Value: test.MustReturnJSONFor(2), - }, + {Name: "option-1", Value: test.MustReturnJSONFor("added")}, }, }, }, }, }, - test.NewPlugin(test.Ctx, "", clusterA, test.WithCluster(clusterA), test.WithPluginLabel(greenhouseapis.LabelKeyOwnedBy, testTeam.Name), - test.WithPluginOptionValue("option-1", test.MustReturnJSONFor(1)), - test.WithPluginOptionValue("option-2", test.MustReturnJSONFor(2)), - test.WithPluginOptionValue("option-3", test.MustReturnJSONFor(2)), - test.WithPluginOptionValue("option-4", test.MustReturnJSONFor(2))), + clusterA, + []greenhousev1alpha1.PluginOptionValue{ + {Name: "option-1", Value: test.MustReturnJSONFor("added")}, + }, ), ) + + It("should not mutate the original preset", func() { + originalValue := test.MustReturnJSONFor("original") + preset := &greenhousev1alpha1.PluginPreset{ + Spec: greenhousev1alpha1.PluginPresetSpec{ + Plugin: greenhousev1alpha1.PluginPresetPluginSpec{ + OptionValues: []greenhousev1alpha1.PluginOptionValue{ + {Name: "option-1", Value: originalValue}, + }, + }, + ClusterOptionOverrides: []greenhousev1alpha1.ClusterOptionOverride{ + { + ClusterName: clusterA, + Overrides: []greenhousev1alpha1.PluginOptionValue{ + {Name: "option-1", Value: test.MustReturnJSONFor("overridden")}, + }, + }, + }, + }, + } + + result := applyOverridesToPreset(preset, clusterA) + + // Result should have overridden value + Expect(result.Spec.Plugin.OptionValues[0].Value).To(Equal(test.MustReturnJSONFor("overridden"))) + + // Original preset should NOT be mutated + Expect(preset.Spec.Plugin.OptionValues[0].Value).To(Equal(originalValue), + "original preset should not be mutated by applyOverridesToPreset") + }) }) var _ = Describe("getReleaseName", func() { diff --git a/internal/controller/plugin/pluginpreset_values_resolver.go b/internal/controller/plugin/pluginpreset_values_resolver.go new file mode 100644 index 000000000..70f6d77f2 --- /dev/null +++ b/internal/controller/plugin/pluginpreset_values_resolver.go @@ -0,0 +1,164 @@ +// SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company and Greenhouse contributors +// SPDX-License-Identifier: Apache-2.0 + +package plugin + +import ( + "context" + "encoding/json" + "fmt" + "slices" + "strings" + + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + greenhousev1alpha1 "github.com/cloudoperators/greenhouse/api/v1alpha1" + "github.com/cloudoperators/greenhouse/internal/helm" + "github.com/cloudoperators/greenhouse/pkg/cel" +) + +// resolvePluginOptionValuesForPreset resolves expressions in a PluginPreset's +// option values before writing to Plugin. +func (r *PluginPresetReconciler) resolvePluginOptionValuesForPreset( + ctx context.Context, + preset *greenhousev1alpha1.PluginPreset, + cluster *greenhousev1alpha1.Cluster, +) ([]greenhousev1alpha1.PluginOptionValue, error) { + + optionValues := preset.Spec.Plugin.OptionValues + + if r.ExpressionEvaluationEnabled { + var err error + optionValues, err = r.resolveExpressionsForPreset(ctx, preset, cluster) + if err != nil { + return nil, fmt.Errorf("failed to resolve expressions: %w", err) + } + } + + return optionValues, nil +} + +// resolveExpressionsForPreset evaluates all expression fields in PluginPreset option values. +func (r *PluginPresetReconciler) resolveExpressionsForPreset( + ctx context.Context, + preset *greenhousev1alpha1.PluginPreset, + cluster *greenhousev1alpha1.Cluster, +) ([]greenhousev1alpha1.PluginOptionValue, error) { + + hasExpressions := false + for _, ov := range preset.Spec.Plugin.OptionValues { + if ov.Expression != nil { //nolint:staticcheck // SA1019: deprecated field kept for later clean up + hasExpressions = true + break + } + } + if !hasExpressions { + return preset.Spec.Plugin.OptionValues, nil + } + + templateData, err := r.buildTemplateData(ctx, preset, cluster) + if err != nil { + return nil, fmt.Errorf("failed to build template data: %w", err) + } + + result := make([]greenhousev1alpha1.PluginOptionValue, 0, len(preset.Spec.Plugin.OptionValues)) + for _, optionValue := range preset.Spec.Plugin.OptionValues { + if optionValue.Expression != nil { //nolint:staticcheck // SA1019: deprecated field kept for later clean up + evaluatedValue, err := cel.EvaluateExpression(*optionValue.Expression, templateData) //nolint:staticcheck // SA1019: deprecated field kept for later clean up + if err != nil { + return nil, fmt.Errorf("failed to evaluate expression for option %s: %w", optionValue.Name, err) + } + result = append(result, greenhousev1alpha1.PluginOptionValue{ + Name: optionValue.Name, + Value: &apiextensionsv1.JSON{Raw: evaluatedValue}, + }) + } else { + result = append(result, optionValue) + } + } + + return result, nil +} + +// buildTemplateData creates the template data map for CEL expression evaluation. +func (r *PluginPresetReconciler) buildTemplateData( + ctx context.Context, + preset *greenhousev1alpha1.PluginPreset, + cluster *greenhousev1alpha1.Cluster, +) (map[string]any, error) { + + tempPlugin := greenhousev1alpha1.Plugin{ + ObjectMeta: metav1.ObjectMeta{ + Name: preset.Name, + Namespace: preset.Namespace, + Labels: preset.Labels, + }, + Spec: greenhousev1alpha1.PluginSpec{ + ClusterName: cluster.Name, + }, + } + + greenhouseValuesList, err := helm.GetGreenhouseValues(ctx, r.Client, tempPlugin) + if err != nil { + return nil, fmt.Errorf("failed to get greenhouse values: %w", err) + } + + templateData := make(map[string]any) + for _, gv := range greenhouseValuesList { + if gv.Value != nil { + var value any + if err := json.Unmarshal(gv.Value.Raw, &value); err != nil { + return nil, fmt.Errorf("failed to unmarshal greenhouse value %s: %w", gv.Name, err) + } + parts := strings.Split(gv.Name, ".") + setNestedValue(templateData, parts, value) + } + } + + return templateData, nil +} + +// setNestedValue sets a value in a nested map using a slice of keys. +func setNestedValue(m map[string]any, keys []string, value any) { + if len(keys) == 0 { + return + } + if len(keys) == 1 { + m[keys[0]] = value + return + } + if _, ok := m[keys[0]]; !ok { + m[keys[0]] = make(map[string]any) + } + if nested, ok := m[keys[0]].(map[string]any); ok { + setNestedValue(nested, keys[1:], value) + } +} + +// applyOverridesToPreset returns a copy of the preset with cluster-specific overrides merged. +func applyOverridesToPreset(preset *greenhousev1alpha1.PluginPreset, clusterName string) *greenhousev1alpha1.PluginPreset { + presetCopy := preset.DeepCopy() + + index := slices.IndexFunc(presetCopy.Spec.ClusterOptionOverrides, func(override greenhousev1alpha1.ClusterOptionOverride) bool { + return override.ClusterName == clusterName + }) + + if index == -1 { + return presetCopy + } + + for _, overrideValue := range presetCopy.Spec.ClusterOptionOverrides[index].Overrides { + valueIndex := slices.IndexFunc(presetCopy.Spec.Plugin.OptionValues, func(value greenhousev1alpha1.PluginOptionValue) bool { + return value.Name == overrideValue.Name + }) + + if valueIndex == -1 { + presetCopy.Spec.Plugin.OptionValues = append(presetCopy.Spec.Plugin.OptionValues, overrideValue) + } else { + presetCopy.Spec.Plugin.OptionValues[valueIndex] = overrideValue + } + } + + return presetCopy +} diff --git a/internal/controller/plugin/suite_test.go b/internal/controller/plugin/suite_test.go index 01d1e5391..76d68095e 100644 --- a/internal/controller/plugin/suite_test.go +++ b/internal/controller/plugin/suite_test.go @@ -39,7 +39,9 @@ var _ = BeforeSuite(func() { test.RegisterController("plugin", (&PluginReconciler{ KubeRuntimeOpts: clientutil.RuntimeOptions{QPS: 5, Burst: 10}, }).SetupWithManager) - test.RegisterController("pluginPreset", (&PluginPresetReconciler{}).SetupWithManager) + test.RegisterController("pluginPreset", (&PluginPresetReconciler{ + ExpressionEvaluationEnabled: true, + }).SetupWithManager) test.RegisterController("pluginDefinition", (&greenhouseDef.PluginDefinitionReconciler{}).SetupWithManager) test.RegisterController("clusterPluginDefinition", (&greenhouseDef.ClusterPluginDefinitionReconciler{}).SetupWithManager) test.RegisterController("cluster", (&greenhousecluster.RemoteClusterReconciler{}).SetupWithManager) diff --git a/internal/features/features.go b/internal/features/features.go index e52371f0d..3e637a16d 100644 --- a/internal/features/features.go +++ b/internal/features/features.go @@ -16,14 +16,16 @@ import ( ) const ( - DexFeatureKey = "dex" - PluginFeatureKey = "plugin" + DexFeatureKey = "dex" + PluginFeatureKey = "plugin" + PluginPresetFeatureKey = "pluginPreset" ) type Features struct { - raw map[string]string - dex *dexFeatures `yaml:"dex"` - plugin *pluginFeatures `yaml:"plugin"` + raw map[string]string + dex *dexFeatures `yaml:"dex"` + plugin *pluginFeatures `yaml:"plugin"` + pluginPreset *pluginPresetFeatures `yaml:"pluginPreset"` } type dexFeatures struct { @@ -36,6 +38,10 @@ type pluginFeatures struct { OCIMirroringEnabled bool `yaml:"ociMirroringEnabled"` } +type pluginPresetFeatures struct { + ExpressionEvaluationEnabled bool `yaml:"expressionEvaluationEnabled"` +} + func NewFeatures(ctx context.Context, k8sClient client.Reader, configMapName, namespace string) (*Features, error) { featureMap := &corev1.ConfigMap{} if err := k8sClient.Get(ctx, types.NamespacedName{Name: configMapName, Namespace: namespace}, featureMap); err != nil { @@ -95,6 +101,33 @@ func (f *Features) resolvePluginFeatures() error { return nil } +func (f *Features) resolvePluginPresetFeatures() error { + pluginPreset, err := resolve[pluginPresetFeatures](f, PluginPresetFeatureKey) + if err != nil { + return err + } + f.pluginPreset = pluginPreset + return nil +} + +// IsPresetExpressionEvaluationEnabled returns whether CEL expression evaluation +// is enabled in the PluginPreset controller. +// Returns false as default. +func (f *Features) IsPresetExpressionEvaluationEnabled() bool { + if f == nil { + return false + } + + if f.pluginPreset != nil { + return f.pluginPreset.ExpressionEvaluationEnabled + } + if err := f.resolvePluginPresetFeatures(); err != nil { + ctrl.LoggerFrom(context.Background()).Error(err, "failed to resolve pluginPreset features") + return false + } + return f.pluginPreset.ExpressionEvaluationEnabled +} + // IsExpressionEvaluationEnabled returns whether plugin option expression evaluation is enabled. // Returns false as default. func (f *Features) IsExpressionEvaluationEnabled() bool { diff --git a/internal/features/features_test.go b/internal/features/features_test.go index 4983f063a..97f3eac5b 100644 --- a/internal/features/features_test.go +++ b/internal/features/features_test.go @@ -242,3 +242,124 @@ func Test_PluginFeatures(t *testing.T) { }) } } + +func Test_PluginPresetFeatures(t *testing.T) { + type testCase struct { + name string + configMapData map[string]string + getError error + expectedExpressionEvaluation bool + } + testCases := []testCase{ + { + name: "it should return true when pluginPreset expression evaluation is enabled", + configMapData: map[string]string{PluginPresetFeatureKey: "expressionEvaluationEnabled: true\n"}, + expectedExpressionEvaluation: true, + }, + { + name: "it should return false when pluginPreset expression evaluation is disabled", + configMapData: map[string]string{PluginPresetFeatureKey: "expressionEvaluationEnabled: false\n"}, + expectedExpressionEvaluation: false, + }, + { + name: "it should return false when pluginPreset key is not found in feature-flags cm", + configMapData: map[string]string{"someOtherKey": "value\n"}, + expectedExpressionEvaluation: false, + }, + { + name: "it should return false when feature-flags cm is not found", + getError: apierrors.NewNotFound(schema.GroupResource{}, "configmap not found"), + expectedExpressionEvaluation: false, + }, + { + name: "it should return false when flag is malformed in feature-flags cm", + configMapData: map[string]string{PluginPresetFeatureKey: "expressionEvaluationEnabled:: invalid_yaml"}, + expectedExpressionEvaluation: false, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + ctx := context.Background() + ctx = log.IntoContext(ctx, log.Log) + + mockK8sClient := &mocks.MockClient{} + configMap := &corev1.ConfigMap{} + + if tc.getError != nil { + mockK8sClient.On("Get", ctx, types.NamespacedName{ + Name: clientutil.GetEnvOrDefault("FEATURE_FLAGS", "greenhouse-feature-flags"), Namespace: clientutil.GetEnvOrDefault("POD_NAMESPACE", "greenhouse"), + }, mock.Anything).Return(tc.getError) + } else { + configMap.Data = tc.configMapData + mockK8sClient.On("Get", ctx, types.NamespacedName{ + Name: clientutil.GetEnvOrDefault("FEATURE_FLAGS", "greenhouse-feature-flags"), Namespace: clientutil.GetEnvOrDefault("POD_NAMESPACE", "greenhouse"), + }, mock.Anything).Run(func(args mock.Arguments) { + arg := args.Get(2).(*corev1.ConfigMap) + *arg = *configMap + }).Return(nil) + } + + // Create Features instance + featuresInstance, err := NewFeatures(ctx, mockK8sClient, clientutil.GetEnvOrDefault("FEATURE_FLAGS", "greenhouse-feature-flags"), clientutil.GetEnvOrDefault("POD_NAMESPACE", "greenhouse")) + + if tc.getError != nil && client.IgnoreNotFound(tc.getError) == nil { + assert.NoError(t, client.IgnoreNotFound(err)) + assert.Nil(t, featuresInstance, "Expected nil when ConfigMap is missing") + + presetExpressionValue := featuresInstance.IsPresetExpressionEvaluationEnabled() + + assert.Equal(t, tc.expectedExpressionEvaluation, presetExpressionValue) + + mockK8sClient.AssertExpectations(t) + return + } + + assert.NoError(t, err) + + presetExpressionValue := featuresInstance.IsPresetExpressionEvaluationEnabled() + + // Assert expected values + assert.Equal(t, tc.expectedExpressionEvaluation, presetExpressionValue) + + // Verify plugin flags are NOT affected by pluginPreset flags + pluginExpressionValue := featuresInstance.IsExpressionEvaluationEnabled() + pluginIntegrationValue := featuresInstance.IsIntegrationEnabled() + assert.Equal(t, false, pluginExpressionValue, "plugin expression flag should be false when only pluginPreset is configured") + assert.Equal(t, false, pluginIntegrationValue, "plugin integration flag should be false when only pluginPreset is configured") + + mockK8sClient.AssertExpectations(t) + }) + } +} + +func Test_PluginAndPluginPresetFeaturesIndependent(t *testing.T) { + ctx := context.Background() + ctx = log.IntoContext(ctx, log.Log) + + mockK8sClient := &mocks.MockClient{} + configMap := &corev1.ConfigMap{ + Data: map[string]string{ + PluginFeatureKey: "expressionEvaluationEnabled: false\nintegrationEnabled: false\n", + PluginPresetFeatureKey: "expressionEvaluationEnabled: true\nintegrationEnabled: true\n", + }, + } + + mockK8sClient.On("Get", ctx, types.NamespacedName{ + Name: clientutil.GetEnvOrDefault("FEATURE_FLAGS", "greenhouse-feature-flags"), Namespace: clientutil.GetEnvOrDefault("POD_NAMESPACE", "greenhouse"), + }, mock.Anything).Run(func(args mock.Arguments) { + arg := args.Get(2).(*corev1.ConfigMap) + *arg = *configMap + }).Return(nil) + + featuresInstance, err := NewFeatures(ctx, mockK8sClient, clientutil.GetEnvOrDefault("FEATURE_FLAGS", "greenhouse-feature-flags"), clientutil.GetEnvOrDefault("POD_NAMESPACE", "greenhouse")) + assert.NoError(t, err) + + // Plugin flags should be false + assert.Equal(t, false, featuresInstance.IsExpressionEvaluationEnabled(), "plugin expression should be disabled") + assert.Equal(t, false, featuresInstance.IsIntegrationEnabled(), "plugin integration should be disabled") + + // PluginPreset flags should be true + assert.Equal(t, true, featuresInstance.IsPresetExpressionEvaluationEnabled(), "preset expression should be enabled") + + mockK8sClient.AssertExpectations(t) +} diff --git a/internal/helm/cel.go b/internal/helm/cel.go index e1a06254e..b5ad3ab4b 100644 --- a/internal/helm/cel.go +++ b/internal/helm/cel.go @@ -27,6 +27,7 @@ func NewCELResolver(optionValues []greenhousev1alpha1.PluginOptionValue) (*CELRe return &CELResolver{templateData: templateData}, nil } +//nolint:staticcheck // SA1019: deprecated fields kept for later clean up func (c *CELResolver) ResolveExpression(optionValue greenhousev1alpha1.PluginOptionValue, expressionEvaluationEnabled bool) (*greenhousev1alpha1.PluginOptionValue, error) { // early return if there is no expression to resolve if optionValue.Expression == nil { diff --git a/internal/helm/helm.go b/internal/helm/helm.go index 93c4e6f57..c7bc321ff 100644 --- a/internal/helm/helm.go +++ b/internal/helm/helm.go @@ -405,13 +405,13 @@ func CalculatePluginOptionChecksum(ctx context.Context, c client.Client, plugin case v.Value != nil: buf = append(buf, v.Value.Raw...) - case v.Expression != nil: + case v.Expression != nil: //nolint:staticcheck // SA1019: deprecated field kept for later clean up buf = append(buf, []byte(*v.Expression)...) - case v.ValueFrom != nil && v.ValueFrom.Ref != nil: - buf = append(buf, []byte(v.ValueFrom.Ref.Name)...) - buf = append(buf, []byte(v.ValueFrom.Ref.Kind)...) - buf = append(buf, []byte(v.ValueFrom.Ref.Expression)...) + case v.ValueFrom != nil && v.ValueFrom.Ref != nil: //nolint:staticcheck // SA1019: deprecated field kept for later clean up + buf = append(buf, []byte(v.ValueFrom.Ref.Name)...) //nolint:staticcheck // SA1019: deprecated fields kept for later clean up + buf = append(buf, []byte(v.ValueFrom.Ref.Kind)...) //nolint:staticcheck // SA1019: deprecated fields kept for later clean up + buf = append(buf, []byte(v.ValueFrom.Ref.Expression)...) //nolint:staticcheck // SA1019: deprecated fields kept for later clean up default: continue diff --git a/internal/test/resources.go b/internal/test/resources.go index 190eedd4b..fa42301da 100644 --- a/internal/test/resources.go +++ b/internal/test/resources.go @@ -337,7 +337,7 @@ func WithPluginOptionValue(name string, value *apiextensionsv1.JSON) func(*green if v.Name == name { v.Value = value v.ValueFrom = nil - v.Expression = nil + v.Expression = nil //nolint:staticcheck // SA1019: deprecated fields kept for later clean up p.Spec.OptionValues[i] = v return } @@ -360,7 +360,7 @@ func WithPluginOptionValueFrom(name string, valueFrom *greenhousev1alpha1.Plugin v.ValueFrom = &greenhousev1alpha1.PluginValueFromSource{ Secret: valueFrom.Secret, } - v.Expression = nil + v.Expression = nil //nolint:staticcheck // SA1019: deprecated fields kept for later clean up p.Spec.OptionValues[i] = v return } @@ -385,7 +385,7 @@ func WithPluginOptionValueFromRef(name string, ref *greenhousev1alpha1.ExternalV v.ValueFrom = &greenhousev1alpha1.PluginValueFromSource{ Ref: ref, } - v.Expression = nil + v.Expression = nil //nolint:staticcheck // SA1019: deprecated fields kept for later clean up p.Spec.OptionValues[i] = v return } @@ -407,7 +407,7 @@ func WithPluginOptionValueExpression(name string, expression *string) func(*gree for i, v := range p.Spec.OptionValues { if v.Name == name { v.Value = nil - v.Expression = expression + v.Expression = expression //nolint:staticcheck // SA1019: deprecated fields kept for later clean up v.ValueFrom = nil p.Spec.OptionValues[i] = v return diff --git a/internal/webhook/v1alpha1/plugin_webhook.go b/internal/webhook/v1alpha1/plugin_webhook.go index 8b7d0012a..e4936a522 100644 --- a/internal/webhook/v1alpha1/plugin_webhook.go +++ b/internal/webhook/v1alpha1/plugin_webhook.go @@ -391,7 +391,7 @@ func hasExactlyOneValueSource(val greenhousev1alpha1.PluginOptionValue) bool { sources := []bool{ val.Value != nil, val.ValueFrom != nil, - val.Expression != nil, + val.Expression != nil, //nolint:staticcheck // SA1019: deprecated fields kept for later clean up } count := 0 diff --git a/internal/webhook/v1alpha1/plugin_webhook_test.go b/internal/webhook/v1alpha1/plugin_webhook_test.go index a86f28df7..f3fe48797 100644 --- a/internal/webhook/v1alpha1/plugin_webhook_test.go +++ b/internal/webhook/v1alpha1/plugin_webhook_test.go @@ -347,7 +347,7 @@ var _ = Describe("Validate plugin spec fields", Ordered, func() { Eventually(func() bool { for _, actOption := range actPlugin.Spec.OptionValues { if actOption.Name == "expressionOption" { - return actOption.Expression != nil + return actOption.Expression != nil //nolint:staticcheck // SA1019: deprecated fields kept for later clean up } } return false From c56433232d87529445739bf59106daed81164397 Mon Sep 17 00:00:00 2001 From: Klaudiusz Fabryczny Date: Mon, 1 Jun 2026 15:06:40 +0200 Subject: [PATCH 02/18] Remove nolint:staticcheck Signed-off-by: Klaudiusz Fabryczny --- .../plugin/plugin_controller_flux.go | 4 ++-- .../plugin/plugin_values_resolver.go | 20 +++++++++---------- .../plugin/pluginpreset_controller_test.go | 6 +++--- .../plugin/pluginpreset_values_resolver.go | 6 +++--- internal/helm/cel.go | 1 - internal/helm/helm.go | 10 +++++----- internal/test/resources.go | 8 ++++---- internal/webhook/v1alpha1/plugin_webhook.go | 2 +- .../webhook/v1alpha1/plugin_webhook_test.go | 2 +- 9 files changed, 29 insertions(+), 30 deletions(-) diff --git a/internal/controller/plugin/plugin_controller_flux.go b/internal/controller/plugin/plugin_controller_flux.go index 7ee1a9f35..168355782 100644 --- a/internal/controller/plugin/plugin_controller_flux.go +++ b/internal/controller/plugin/plugin_controller_flux.go @@ -468,7 +468,7 @@ func computeReleaseValues(ctx context.Context, c client.Client, plugin *greenhou // noop, direct values are already set continue - case v.Expression != nil: //nolint:staticcheck // SA1019: deprecated field kept for later clean up + case v.Expression != nil: // TODO(#1775): Expressions should no longer be evaluated by Plugin controller. // Once PluginPreset controller handles all expression evaluation, // this branch should return an error instead of evaluating. @@ -481,7 +481,7 @@ func computeReleaseValues(ctx context.Context, c client.Client, plugin *greenhou } optionValues[i] = *resolvedOptionValue - case v.ValueFrom != nil && v.ValueFrom.Ref != nil: //nolint:staticcheck // SA1019: deprecated field kept for later clean up + case v.ValueFrom != nil && v.ValueFrom.Ref != nil: // TODO(#1776): References should no longer be resolved by Plugin controller. // Once PluginPreset controller handles all reference resolution, // this branch should return an error instead of resolving. diff --git a/internal/controller/plugin/plugin_values_resolver.go b/internal/controller/plugin/plugin_values_resolver.go index ba6d3d4d9..7248df010 100644 --- a/internal/controller/plugin/plugin_values_resolver.go +++ b/internal/controller/plugin/plugin_values_resolver.go @@ -38,7 +38,7 @@ const ( // filterValueRefOptions filters option values to only include those with external references (ValueFrom.Ref). func filterValueRefOptions(optionValues []greenhousev1alpha1.PluginOptionValue) []greenhousev1alpha1.PluginOptionValue { return slices.DeleteFunc(optionValues, func(o greenhousev1alpha1.PluginOptionValue) bool { - return o.ValueFrom == nil || o.ValueFrom.Ref == nil //nolint:staticcheck // SA1019: deprecated field kept for later clean up + return o.ValueFrom == nil || o.ValueFrom.Ref == nil }) } @@ -61,20 +61,20 @@ func resolveValueFromRef(ctx context.Context, c client.Client, plugin *greenhous var trackedObjects []string var err error resolveKind := defaultKind - if option.ValueFrom.Ref.Kind != "" { //nolint:staticcheck // SA1019: deprecated fields kept for later clean up - resolveKind = option.ValueFrom.Ref.Kind //nolint:staticcheck // SA1019: deprecated fields kept for later clean up + if option.ValueFrom.Ref.Kind != "" { + resolveKind = option.ValueFrom.Ref.Kind } gvk := buildGVK(resolveKind) // resolve by name - if option.ValueFrom.Ref.Name != "" { //nolint:staticcheck // SA1019: deprecated fields kept for later clean up + if option.ValueFrom.Ref.Name != "" { value, err = resolveByName(ctx, c, plugin, option, gvk, tracker) if err != nil { return nil, nil, err } - trackedObjects = append(trackedObjects, trackingID(resolveKind, option.ValueFrom.Ref.Name)) //nolint:staticcheck // SA1019: deprecated fields kept for later clean up + trackedObjects = append(trackedObjects, trackingID(resolveKind, option.ValueFrom.Ref.Name)) } // resolve by label selector - if option.ValueFrom.Ref.Selector != nil { //nolint:staticcheck // SA1019: deprecated fields kept for later clean up + if option.ValueFrom.Ref.Selector != nil { var selectorTrackedObjects []string value, selectorTrackedObjects, err = resolveBySelector(ctx, c, plugin, option, gvk, tracker) if err != nil { @@ -102,7 +102,7 @@ func resolveValueFromRef(ctx context.Context, c client.Client, plugin *greenhous // resolveByName resolves an option value by fetching a specific named resource. func resolveByName(ctx context.Context, c client.Client, plugin *greenhousev1alpha1.Plugin, option greenhousev1alpha1.PluginOptionValue, gvk schema.GroupVersionKind, tracker string) (any, error) { key := types.NamespacedName{ - Name: option.ValueFrom.Ref.Name, //nolint:staticcheck // SA1019: deprecated fields kept for later clean up + Name: option.ValueFrom.Ref.Name, Namespace: plugin.GetNamespace(), } uObject := &unstructured.Unstructured{} @@ -122,7 +122,7 @@ func resolveByName(ctx context.Context, c client.Client, plugin *greenhousev1alp "namespace", key.Namespace) return nil, err } - value, err := evaluateExpression(ctx, uObject, option.ValueFrom.Ref.Expression) //nolint:staticcheck // SA1019: deprecated fields kept for later clean up + value, err := evaluateExpression(ctx, uObject, option.ValueFrom.Ref.Expression) if err != nil { return nil, err } @@ -138,7 +138,7 @@ func resolveByName(ctx context.Context, c client.Client, plugin *greenhousev1alp // resolveBySelector resolves option values by fetching resources matching a label selector. func resolveBySelector(ctx context.Context, c client.Client, plugin *greenhousev1alpha1.Plugin, option greenhousev1alpha1.PluginOptionValue, gvk schema.GroupVersionKind, tracker string) (value any, trackedObjects []string, err error) { - selector, err := metav1.LabelSelectorAsSelector(option.ValueFrom.Ref.Selector) //nolint:staticcheck // SA1019: deprecated fields kept for later clean up + selector, err := metav1.LabelSelectorAsSelector(option.ValueFrom.Ref.Selector) if err != nil { log.FromContext(ctx).Error(err, "failed to parse label selector", "namespace", plugin.GetNamespace(), @@ -147,7 +147,7 @@ func resolveBySelector(ctx context.Context, c client.Client, plugin *greenhousev return nil, nil, err } - value, trackedObjects, err = resolveMany(ctx, c, gvk, selector, plugin.GetNamespace(), option.ValueFrom.Ref.Expression, tracker) //nolint:staticcheck // SA1019: deprecated fields kept for later clean up + value, trackedObjects, err = resolveMany(ctx, c, gvk, selector, plugin.GetNamespace(), option.ValueFrom.Ref.Expression, tracker) if err != nil { return nil, nil, err } diff --git a/internal/controller/plugin/pluginpreset_controller_test.go b/internal/controller/plugin/pluginpreset_controller_test.go index 7a189d21d..8c5e9d072 100644 --- a/internal/controller/plugin/pluginpreset_controller_test.go +++ b/internal/controller/plugin/pluginpreset_controller_test.go @@ -934,7 +934,7 @@ var _ = Describe("PluginPreset Controller Lifecycle", Ordered, func() { for _, ov := range expPlugin.Spec.OptionValues { if ov.Name == "test.hostname" { hostnameFound = true - g.Expect(ov.Expression).To(BeNil(), "Expression should be resolved") //nolint:staticcheck // SA1019: checking deprecated field in test + g.Expect(ov.Expression).To(BeNil(), "Expression should be resolved") g.Expect(ov.Value).ToNot(BeNil(), "Value should be set") g.Expect(string(ov.Value.Raw)).To(Equal(`"app-`+clusterA+`.example.com"`), "Expression should resolve with cluster name") @@ -1002,7 +1002,7 @@ var _ = Describe("PluginPreset Controller Lifecycle", Ordered, func() { for _, ov := range expPlugin.Spec.OptionValues { if ov.Name == "test.serviceHost" { found = true - g.Expect(ov.Expression).To(BeNil()) //nolint:staticcheck // SA1019 + g.Expect(ov.Expression).To(BeNil()) g.Expect(ov.Value).ToNot(BeNil()) g.Expect(string(ov.Value.Raw)).To(Equal(`"service.eu-de-1.example.com"`)) } @@ -1074,7 +1074,7 @@ var _ = Describe("PluginPreset Controller Lifecycle", Ordered, func() { for _, ov := range expPlugin.Spec.OptionValues { if ov.Name == "expression.value" { exprResolved = true - g.Expect(ov.Expression).To(BeNil()) //nolint:staticcheck // SA1019 + g.Expect(ov.Expression).To(BeNil()) g.Expect(ov.Value).ToNot(BeNil()) g.Expect(string(ov.Value.Raw)).To(Equal(`"generated-` + clusterA + `"`)) } diff --git a/internal/controller/plugin/pluginpreset_values_resolver.go b/internal/controller/plugin/pluginpreset_values_resolver.go index 70f6d77f2..3315a45ea 100644 --- a/internal/controller/plugin/pluginpreset_values_resolver.go +++ b/internal/controller/plugin/pluginpreset_values_resolver.go @@ -48,7 +48,7 @@ func (r *PluginPresetReconciler) resolveExpressionsForPreset( hasExpressions := false for _, ov := range preset.Spec.Plugin.OptionValues { - if ov.Expression != nil { //nolint:staticcheck // SA1019: deprecated field kept for later clean up + if ov.Expression != nil { hasExpressions = true break } @@ -64,8 +64,8 @@ func (r *PluginPresetReconciler) resolveExpressionsForPreset( result := make([]greenhousev1alpha1.PluginOptionValue, 0, len(preset.Spec.Plugin.OptionValues)) for _, optionValue := range preset.Spec.Plugin.OptionValues { - if optionValue.Expression != nil { //nolint:staticcheck // SA1019: deprecated field kept for later clean up - evaluatedValue, err := cel.EvaluateExpression(*optionValue.Expression, templateData) //nolint:staticcheck // SA1019: deprecated field kept for later clean up + if optionValue.Expression != nil { + evaluatedValue, err := cel.EvaluateExpression(*optionValue.Expression, templateData) if err != nil { return nil, fmt.Errorf("failed to evaluate expression for option %s: %w", optionValue.Name, err) } diff --git a/internal/helm/cel.go b/internal/helm/cel.go index b5ad3ab4b..e1a06254e 100644 --- a/internal/helm/cel.go +++ b/internal/helm/cel.go @@ -27,7 +27,6 @@ func NewCELResolver(optionValues []greenhousev1alpha1.PluginOptionValue) (*CELRe return &CELResolver{templateData: templateData}, nil } -//nolint:staticcheck // SA1019: deprecated fields kept for later clean up func (c *CELResolver) ResolveExpression(optionValue greenhousev1alpha1.PluginOptionValue, expressionEvaluationEnabled bool) (*greenhousev1alpha1.PluginOptionValue, error) { // early return if there is no expression to resolve if optionValue.Expression == nil { diff --git a/internal/helm/helm.go b/internal/helm/helm.go index c7bc321ff..93c4e6f57 100644 --- a/internal/helm/helm.go +++ b/internal/helm/helm.go @@ -405,13 +405,13 @@ func CalculatePluginOptionChecksum(ctx context.Context, c client.Client, plugin case v.Value != nil: buf = append(buf, v.Value.Raw...) - case v.Expression != nil: //nolint:staticcheck // SA1019: deprecated field kept for later clean up + case v.Expression != nil: buf = append(buf, []byte(*v.Expression)...) - case v.ValueFrom != nil && v.ValueFrom.Ref != nil: //nolint:staticcheck // SA1019: deprecated field kept for later clean up - buf = append(buf, []byte(v.ValueFrom.Ref.Name)...) //nolint:staticcheck // SA1019: deprecated fields kept for later clean up - buf = append(buf, []byte(v.ValueFrom.Ref.Kind)...) //nolint:staticcheck // SA1019: deprecated fields kept for later clean up - buf = append(buf, []byte(v.ValueFrom.Ref.Expression)...) //nolint:staticcheck // SA1019: deprecated fields kept for later clean up + case v.ValueFrom != nil && v.ValueFrom.Ref != nil: + buf = append(buf, []byte(v.ValueFrom.Ref.Name)...) + buf = append(buf, []byte(v.ValueFrom.Ref.Kind)...) + buf = append(buf, []byte(v.ValueFrom.Ref.Expression)...) default: continue diff --git a/internal/test/resources.go b/internal/test/resources.go index fa42301da..190eedd4b 100644 --- a/internal/test/resources.go +++ b/internal/test/resources.go @@ -337,7 +337,7 @@ func WithPluginOptionValue(name string, value *apiextensionsv1.JSON) func(*green if v.Name == name { v.Value = value v.ValueFrom = nil - v.Expression = nil //nolint:staticcheck // SA1019: deprecated fields kept for later clean up + v.Expression = nil p.Spec.OptionValues[i] = v return } @@ -360,7 +360,7 @@ func WithPluginOptionValueFrom(name string, valueFrom *greenhousev1alpha1.Plugin v.ValueFrom = &greenhousev1alpha1.PluginValueFromSource{ Secret: valueFrom.Secret, } - v.Expression = nil //nolint:staticcheck // SA1019: deprecated fields kept for later clean up + v.Expression = nil p.Spec.OptionValues[i] = v return } @@ -385,7 +385,7 @@ func WithPluginOptionValueFromRef(name string, ref *greenhousev1alpha1.ExternalV v.ValueFrom = &greenhousev1alpha1.PluginValueFromSource{ Ref: ref, } - v.Expression = nil //nolint:staticcheck // SA1019: deprecated fields kept for later clean up + v.Expression = nil p.Spec.OptionValues[i] = v return } @@ -407,7 +407,7 @@ func WithPluginOptionValueExpression(name string, expression *string) func(*gree for i, v := range p.Spec.OptionValues { if v.Name == name { v.Value = nil - v.Expression = expression //nolint:staticcheck // SA1019: deprecated fields kept for later clean up + v.Expression = expression v.ValueFrom = nil p.Spec.OptionValues[i] = v return diff --git a/internal/webhook/v1alpha1/plugin_webhook.go b/internal/webhook/v1alpha1/plugin_webhook.go index e4936a522..8b7d0012a 100644 --- a/internal/webhook/v1alpha1/plugin_webhook.go +++ b/internal/webhook/v1alpha1/plugin_webhook.go @@ -391,7 +391,7 @@ func hasExactlyOneValueSource(val greenhousev1alpha1.PluginOptionValue) bool { sources := []bool{ val.Value != nil, val.ValueFrom != nil, - val.Expression != nil, //nolint:staticcheck // SA1019: deprecated fields kept for later clean up + val.Expression != nil, } count := 0 diff --git a/internal/webhook/v1alpha1/plugin_webhook_test.go b/internal/webhook/v1alpha1/plugin_webhook_test.go index f3fe48797..a86f28df7 100644 --- a/internal/webhook/v1alpha1/plugin_webhook_test.go +++ b/internal/webhook/v1alpha1/plugin_webhook_test.go @@ -347,7 +347,7 @@ var _ = Describe("Validate plugin spec fields", Ordered, func() { Eventually(func() bool { for _, actOption := range actPlugin.Spec.OptionValues { if actOption.Name == "expressionOption" { - return actOption.Expression != nil //nolint:staticcheck // SA1019: deprecated fields kept for later clean up + return actOption.Expression != nil } } return false From 442bff011a9f7ef15fd17bad95779268fa90ea1e Mon Sep 17 00:00:00 2001 From: Klaudiusz Fabryczny Date: Mon, 1 Jun 2026 15:22:56 +0200 Subject: [PATCH 03/18] fix: remove backport workflow accidentally introduced during merge Signed-off-by: Klaudiusz Fabryczny --- .github/workflows/backport.yaml | 39 --------------------------------- 1 file changed, 39 deletions(-) delete mode 100644 .github/workflows/backport.yaml diff --git a/.github/workflows/backport.yaml b/.github/workflows/backport.yaml deleted file mode 100644 index 2fbe97393..000000000 --- a/.github/workflows/backport.yaml +++ /dev/null @@ -1,39 +0,0 @@ -name: backport -on: - pull_request_target: - types: [closed, labeled] - -jobs: - backport: - if: github.event.pull_request.state == 'closed' && github.event.pull_request.merged && (github.event_name != 'labeled' || startsWith('backport:', github.event.label.name)) - runs-on: [ ubuntu-latest ] - steps: - - name: Generate GitHub App Token - id: github-app-token - uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 - with: - app-id: ${{ secrets.CLOUDOPERATOR_APP_ID }} - private-key: ${{ secrets.CLOUDOPERATOR_APP_PRIVATE_KEY }} - permission-contents: write - permission-pull-requests: write - - name: Checkout - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - with: - ref: ${{ github.event.pull_request.head.sha }} - token: ${{ steps.github-app-token.outputs.token }} - - name: Create backport PRs - uses: korthout/backport-action@66065406958f46e82238fd59546f5a99e69e22aa # v4.5.2 - # xref: https://github.com/korthout/backport-action#inputs - with: - # Use token to allow workflows to be triggered for the created PR - github_token: ${{ steps.github-app-token.outputs.token }} - # Match labels with a pattern `backport:` - label_pattern: '^backport:(release\/v[0-9]+.[0-9]+)$' - # A bit shorter pull-request title than the default - pull_title: 'backport(${target_branch}): ${pull_title}' - # Simpler PR description than default - pull_description: |- - Automated cherry-pick to `${target_branch}`, triggered by a label in PR#${pull_number}. - # Name and Email of the cloud-operator-bot, used commit the cherry-pick & create the PR - git_commiter_name: cloud-operator-bot[bot] - git_commiter_email: 224791424+cloud-operator-bot[bot]@users.noreply.github.com From 291ee61a98271485d43e5c18792c7ea4df42f6ea Mon Sep 17 00:00:00 2001 From: Klaudiusz Fabryczny Date: Wed, 3 Jun 2026 09:37:47 +0200 Subject: [PATCH 04/18] fix pluginPreset doc Signed-off-by: Klaudiusz Fabryczny --- docs/reference/components/pluginpreset.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/reference/components/pluginpreset.md b/docs/reference/components/pluginpreset.md index f73661907..da9c361f5 100644 --- a/docs/reference/components/pluginpreset.md +++ b/docs/reference/components/pluginpreset.md @@ -91,7 +91,9 @@ spec: ## CEL Expressions in OptionValues -PluginPresets support CEL (Common Expression Language) expressions in `optionValues`. Expressions are evaluated during PluginPreset reconciliation, and the resulting Plugin contains only the resolved values with no expression fields remaining. +PluginPresets support CEL (Common Expression Language) expressions in `optionValues`. +When `pluginPreset.expressionEvaluationEnabled` is enabled, expressions are evaluated during PluginPreset reconciliation and the resulting Plugin contains only the resolved values +with no expression fields remaining. Expressions use the `${...}` syntax to reference dynamic values: @@ -183,7 +185,7 @@ spec: CEL expression evaluation in PluginPresets requires the feature flag `pluginPreset.expressionEvaluationEnabled` to be set to `true` in the Greenhouse feature flags ConfigMap. -When disabled (default: `true`), expressions are passed through as literal values without evaluation. +When disabled (default: `false`), expressions are passed through as literal values without evaluation. ```yaml # greenhouse-feature-flags ConfigMap From a75caa429d0da5be39e03054ccbe3645dd695a3d Mon Sep 17 00:00:00 2001 From: Klaudiusz Fabryczny Date: Wed, 3 Jun 2026 10:01:20 +0200 Subject: [PATCH 05/18] Remove unsupported flag Signed-off-by: Klaudiusz Fabryczny --- internal/features/features_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/features/features_test.go b/internal/features/features_test.go index 97f3eac5b..6d2ea4d40 100644 --- a/internal/features/features_test.go +++ b/internal/features/features_test.go @@ -340,7 +340,7 @@ func Test_PluginAndPluginPresetFeaturesIndependent(t *testing.T) { configMap := &corev1.ConfigMap{ Data: map[string]string{ PluginFeatureKey: "expressionEvaluationEnabled: false\nintegrationEnabled: false\n", - PluginPresetFeatureKey: "expressionEvaluationEnabled: true\nintegrationEnabled: true\n", + PluginPresetFeatureKey: "expressionEvaluationEnabled: true\n", }, } From c3b08efb06587cc4c2c4f88167248ffbcb331806 Mon Sep 17 00:00:00 2001 From: Klaudiusz Fabryczny Date: Wed, 3 Jun 2026 12:36:54 +0200 Subject: [PATCH 06/18] clarify Plugin controller expression deprecation scope Signed-off-by: Klaudiusz Fabryczny --- internal/controller/plugin/plugin_controller_flux.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/controller/plugin/plugin_controller_flux.go b/internal/controller/plugin/plugin_controller_flux.go index 168355782..5c9c9486d 100644 --- a/internal/controller/plugin/plugin_controller_flux.go +++ b/internal/controller/plugin/plugin_controller_flux.go @@ -469,9 +469,9 @@ func computeReleaseValues(ctx context.Context, c client.Client, plugin *greenhou continue case v.Expression != nil: - // TODO(#1775): Expressions should no longer be evaluated by Plugin controller. - // Once PluginPreset controller handles all expression evaluation, - // this branch should return an error instead of evaluating. + // This PR adds CEL expression evaluation to the PluginPreset controller (#1774). + // The Plugin controller's expression evaluation remains active (gated by feature flag) + // and will be disabled in a follow-up PR TODO(#1775). if !expressionEvaluation { continue } From 65ac595715d08ae3fa7c1e6b77054bbc6018f2a5 Mon Sep 17 00:00:00 2001 From: Klaudiusz Fabryczny Date: Wed, 3 Jun 2026 12:53:22 +0200 Subject: [PATCH 07/18] Set expressionEvaluationEnabled to false for pluginPreset Signed-off-by: Klaudiusz Fabryczny --- charts/greenhouse/values.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/charts/greenhouse/values.yaml b/charts/greenhouse/values.yaml index 9dc09a6c3..3404507bd 100644 --- a/charts/greenhouse/values.yaml +++ b/charts/greenhouse/values.yaml @@ -25,7 +25,7 @@ global: ociMirroringEnabled: false # PluginPreset configuration for Greenhouse. pluginPreset: - expressionEvaluationEnabled: true + expressionEvaluationEnabled: false postgresqlng: enabled: true From 48e9ce5a374c328484329dcaa1246b4a11f2054a Mon Sep 17 00:00:00 2001 From: Klaudiusz Fabryczny Date: Wed, 3 Jun 2026 13:01:40 +0200 Subject: [PATCH 08/18] Improve pluginPreset doc Signed-off-by: Klaudiusz Fabryczny --- docs/reference/components/pluginpreset.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/docs/reference/components/pluginpreset.md b/docs/reference/components/pluginpreset.md index da9c361f5..01a9f35f4 100644 --- a/docs/reference/components/pluginpreset.md +++ b/docs/reference/components/pluginpreset.md @@ -176,16 +176,15 @@ spec: "special.${global.greenhouse.metadata.region}.example.com" ``` -> :information_source: Expressions are only evaluated in PluginPresets. - -> :warning: CEL expressions on standalone Plugins are deprecated and will be removed in a future release. Use PluginPresets for expression evaluation. +> :information_source: Expressions are evaluated in PluginPresets when `pluginPreset.expressionEvaluationEnabled` is enabled. +Standalone Plugin expressions are still supported (deprecated) and may be evaluated by the Plugin controller depending on feature flags. ## Feature Flag CEL expression evaluation in PluginPresets requires the feature flag `pluginPreset.expressionEvaluationEnabled` to be set to `true` in the Greenhouse feature flags ConfigMap. -When disabled (default: `false`), expressions are passed through as literal values without evaluation. +When disabled (default: `false`), expressions are not evaluated by the PluginPreset controller and are copied to the created Plugin as `expression` fields. ```yaml # greenhouse-feature-flags ConfigMap From e9168cc06abf0e9542d6024b09d1fd84b56132e7 Mon Sep 17 00:00:00 2001 From: Klaudiusz Fabryczny Date: Wed, 3 Jun 2026 15:23:45 +0200 Subject: [PATCH 09/18] add RBAC permission for Teams required by expression evaluation Signed-off-by: Klaudiusz Fabryczny --- internal/controller/plugin/pluginpreset_controller.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/controller/plugin/pluginpreset_controller.go b/internal/controller/plugin/pluginpreset_controller.go index e793d8aa3..65d5401f3 100644 --- a/internal/controller/plugin/pluginpreset_controller.go +++ b/internal/controller/plugin/pluginpreset_controller.go @@ -55,6 +55,7 @@ type PluginPresetReconciler struct { //+kubebuilder:rbac:groups=greenhouse.sap,resources=clusters,verbs=get;list;watch; //+kubebuilder:rbac:groups=greenhouse.sap,resources=plugindefinitions,verbs=get;list;watch; //+kubebuilder:rbac:groups=greenhouse.sap,resources=clusterplugindefinitions,verbs=get;list;watch; +//+kubebuilder:rbac:groups=greenhouse.sap,resources=teams,verbs=get;list;watch // SetupWithManager sets up the controller with the Manager. func (r *PluginPresetReconciler) SetupWithManager(name string, mgr ctrl.Manager) error { From 3bb2004682ad9c4fd61490c52fa657cbf3e5c57d Mon Sep 17 00:00:00 2001 From: Klaudiusz Fabryczny Date: Wed, 3 Jun 2026 16:51:53 +0200 Subject: [PATCH 10/18] Always resolve expression option values regardless of feature flag Signed-off-by: Klaudiusz Fabryczny --- internal/controller/plugin/plugin_controller_flux.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/internal/controller/plugin/plugin_controller_flux.go b/internal/controller/plugin/plugin_controller_flux.go index 5c9c9486d..77ff9f37d 100644 --- a/internal/controller/plugin/plugin_controller_flux.go +++ b/internal/controller/plugin/plugin_controller_flux.go @@ -471,10 +471,6 @@ func computeReleaseValues(ctx context.Context, c client.Client, plugin *greenhou case v.Expression != nil: // This PR adds CEL expression evaluation to the PluginPreset controller (#1774). // The Plugin controller's expression evaluation remains active (gated by feature flag) - // and will be disabled in a follow-up PR TODO(#1775). - if !expressionEvaluation { - continue - } resolvedOptionValue, err := celResolver.ResolveExpression(v, expressionEvaluation) if err != nil { return nil, err From 9078af209cc47e052ca5883be4acaddb93ecfb30 Mon Sep 17 00:00:00 2001 From: Klaudiusz Fabryczny Date: Tue, 9 Jun 2026 14:44:22 +0200 Subject: [PATCH 11/18] Export BuildTemplateData and use it Signed-off-by: Klaudiusz Fabryczny --- .../plugin/pluginpreset_values_resolver.go | 74 ++++--------------- internal/helm/cel.go | 6 +- 2 files changed, 19 insertions(+), 61 deletions(-) diff --git a/internal/controller/plugin/pluginpreset_values_resolver.go b/internal/controller/plugin/pluginpreset_values_resolver.go index 3315a45ea..10f88a737 100644 --- a/internal/controller/plugin/pluginpreset_values_resolver.go +++ b/internal/controller/plugin/pluginpreset_values_resolver.go @@ -5,10 +5,8 @@ package plugin import ( "context" - "encoding/json" "fmt" "slices" - "strings" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -57,7 +55,22 @@ func (r *PluginPresetReconciler) resolveExpressionsForPreset( return preset.Spec.Plugin.OptionValues, nil } - templateData, err := r.buildTemplateData(ctx, preset, cluster) + tempPlugin := greenhousev1alpha1.Plugin{ + ObjectMeta: metav1.ObjectMeta{ + Name: preset.Name, + Namespace: preset.Namespace, + Labels: preset.Labels, + }, + Spec: greenhousev1alpha1.PluginSpec{ + ClusterName: cluster.Name, + }, + } + greenhouseValuesList, err := helm.GetGreenhouseValues(ctx, r.Client, tempPlugin) + if err != nil { + return nil, fmt.Errorf("failed to get greenhouse values: %w", err) + } + + templateData, err := helm.BuildTemplateData(greenhouseValuesList) if err != nil { return nil, fmt.Errorf("failed to build template data: %w", err) } @@ -81,61 +94,6 @@ func (r *PluginPresetReconciler) resolveExpressionsForPreset( return result, nil } -// buildTemplateData creates the template data map for CEL expression evaluation. -func (r *PluginPresetReconciler) buildTemplateData( - ctx context.Context, - preset *greenhousev1alpha1.PluginPreset, - cluster *greenhousev1alpha1.Cluster, -) (map[string]any, error) { - - tempPlugin := greenhousev1alpha1.Plugin{ - ObjectMeta: metav1.ObjectMeta{ - Name: preset.Name, - Namespace: preset.Namespace, - Labels: preset.Labels, - }, - Spec: greenhousev1alpha1.PluginSpec{ - ClusterName: cluster.Name, - }, - } - - greenhouseValuesList, err := helm.GetGreenhouseValues(ctx, r.Client, tempPlugin) - if err != nil { - return nil, fmt.Errorf("failed to get greenhouse values: %w", err) - } - - templateData := make(map[string]any) - for _, gv := range greenhouseValuesList { - if gv.Value != nil { - var value any - if err := json.Unmarshal(gv.Value.Raw, &value); err != nil { - return nil, fmt.Errorf("failed to unmarshal greenhouse value %s: %w", gv.Name, err) - } - parts := strings.Split(gv.Name, ".") - setNestedValue(templateData, parts, value) - } - } - - return templateData, nil -} - -// setNestedValue sets a value in a nested map using a slice of keys. -func setNestedValue(m map[string]any, keys []string, value any) { - if len(keys) == 0 { - return - } - if len(keys) == 1 { - m[keys[0]] = value - return - } - if _, ok := m[keys[0]]; !ok { - m[keys[0]] = make(map[string]any) - } - if nested, ok := m[keys[0]].(map[string]any); ok { - setNestedValue(nested, keys[1:], value) - } -} - // applyOverridesToPreset returns a copy of the preset with cluster-specific overrides merged. func applyOverridesToPreset(preset *greenhousev1alpha1.PluginPreset, clusterName string) *greenhousev1alpha1.PluginPreset { presetCopy := preset.DeepCopy() diff --git a/internal/helm/cel.go b/internal/helm/cel.go index e1a06254e..b917e927a 100644 --- a/internal/helm/cel.go +++ b/internal/helm/cel.go @@ -20,7 +20,7 @@ type CELResolver struct { // NewCELResolver creates a new CELResolver for a given Plugin. func NewCELResolver(optionValues []greenhousev1alpha1.PluginOptionValue) (*CELResolver, error) { - templateData, err := buildTemplateData(optionValues) + templateData, err := BuildTemplateData(optionValues) if err != nil { return nil, fmt.Errorf("failed to build template data: %w", err) } @@ -59,8 +59,8 @@ func (c *CELResolver) ResolveExpression(optionValue greenhousev1alpha1.PluginOpt }, nil } -// buildTemplateData extracts global.greenhouse.* values to build template data for CEL evaluation. -func buildTemplateData(optionValues []greenhousev1alpha1.PluginOptionValue) (map[string]any, error) { +// BuildTemplateData extracts global.greenhouse.* values to build template data for CEL evaluation. +func BuildTemplateData(optionValues []greenhousev1alpha1.PluginOptionValue) (map[string]any, error) { greenhouseValues := make([]greenhousev1alpha1.PluginOptionValue, 0) for _, optionValue := range optionValues { // Include global.greenhouse.* values for CEL evaluation. From f09d2c7df3deea9df7a7ed5c7d3e7b1afe41b17f Mon Sep 17 00:00:00 2001 From: Klaudiusz Fabryczny Date: Wed, 10 Jun 2026 14:19:03 +0200 Subject: [PATCH 12/18] Use PluginPresetPluginOptionValue in pluginPreset Signed-off-by: Klaudiusz Fabryczny --- api/v1alpha1/pluginpreset_types.go | 6 +-- api/v1alpha1/zz_generated.deepcopy.go | 4 +- .../crds/greenhouse.sap_pluginpresets.yaml | 34 +++++-------- docs/reference/api/index.html | 15 +++--- docs/reference/api/openapi.yaml | 28 +++------- internal/cmd/plugin_template.go | 15 +++++- internal/cmd/plugin_template_test.go | 4 +- .../plugin/pluginpreset_controller.go | 14 ++++- .../plugin/pluginpreset_controller_test.go | 51 +++++++++++-------- .../plugin/pluginpreset_values_resolver.go | 23 +++------ internal/test/resources.go | 27 ++++++++-- .../webhook/v1alpha1/pluginpreset_webhook.go | 21 +++++++- .../v1alpha1/pluginpreset_webhook_test.go | 18 +++---- types/typescript/schema.d.ts | 26 +++++----- 14 files changed, 161 insertions(+), 125 deletions(-) diff --git a/api/v1alpha1/pluginpreset_types.go b/api/v1alpha1/pluginpreset_types.go index 579072700..0615f1ff0 100644 --- a/api/v1alpha1/pluginpreset_types.go +++ b/api/v1alpha1/pluginpreset_types.go @@ -62,7 +62,7 @@ type PluginPresetPluginSpec struct { DisplayName string `json:"displayName,omitempty"` // Values are the values for a PluginDefinition instance. - OptionValues []PluginOptionValue `json:"optionValues,omitempty"` + OptionValues []PluginPresetPluginOptionValue `json:"optionValues,omitempty"` // ReleaseNamespace is the namespace in the remote cluster to which the backend is deployed. // Defaults to the Greenhouse managed namespace if not set. @@ -114,8 +114,8 @@ type PluginPresetPluginValueFromSource struct { // ClusterOptionOverride defines which plugin option should be override in which cluster // +Optional type ClusterOptionOverride struct { - ClusterName string `json:"clusterName"` - Overrides []PluginOptionValue `json:"overrides"` + ClusterName string `json:"clusterName"` + Overrides []PluginPresetPluginOptionValue `json:"overrides"` } const ( diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index f7b889c7a..2bca20dff 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -545,7 +545,7 @@ func (in *ClusterOptionOverride) DeepCopyInto(out *ClusterOptionOverride) { *out = *in if in.Overrides != nil { in, out := &in.Overrides, &out.Overrides - *out = make([]PluginOptionValue, len(*in)) + *out = make([]PluginPresetPluginOptionValue, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) } @@ -1324,7 +1324,7 @@ func (in *PluginPresetPluginSpec) DeepCopyInto(out *PluginPresetPluginSpec) { out.PluginDefinitionRef = in.PluginDefinitionRef if in.OptionValues != nil { in, out := &in.OptionValues, &out.OptionValues - *out = make([]PluginOptionValue, len(*in)) + *out = make([]PluginPresetPluginOptionValue, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) } diff --git a/charts/manager/crds/greenhouse.sap_pluginpresets.yaml b/charts/manager/crds/greenhouse.sap_pluginpresets.yaml index bc9445a14..628600c5a 100644 --- a/charts/manager/crds/greenhouse.sap_pluginpresets.yaml +++ b/charts/manager/crds/greenhouse.sap_pluginpresets.yaml @@ -68,14 +68,12 @@ spec: type: string overrides: items: - description: PluginOptionValue is the value for a PluginOption. + description: PluginPresetPluginOptionValue is the value for + a PluginOption. properties: expression: - description: |- - Expression is a YAML string with ${...} placeholders that will be evaluated as CEL expressions. - - Deprecated: Expression is deprecated on standalone Plugins and will be removed in a future release. - Consider using a PluginPreset to deploy Plugins utilizing the Expression field. + description: Expression is a YAML string with ${...} placeholders + that will be evaluated as CEL expressions. type: string name: description: Name of the values. @@ -87,11 +85,8 @@ spec: description: ValueFrom references value in another source. properties: ref: - description: |- - Ref references values defined in another resource (Plugin, PluginPreset) - - Deprecated: Ref is deprecated on standalone Plugins and will be removed in a future release. - Consider using a PluginPreset to deploy Plugins utilizing the Ref field. + description: Ref references values defined in another + resource (Plugin, PluginPreset) properties: expression: description: Expression is a CEL expression to @@ -307,14 +302,12 @@ spec: optionValues: description: Values are the values for a PluginDefinition instance. items: - description: PluginOptionValue is the value for a PluginOption. + description: PluginPresetPluginOptionValue is the value for + a PluginOption. properties: expression: - description: |- - Expression is a YAML string with ${...} placeholders that will be evaluated as CEL expressions. - - Deprecated: Expression is deprecated on standalone Plugins and will be removed in a future release. - Consider using a PluginPreset to deploy Plugins utilizing the Expression field. + description: Expression is a YAML string with ${...} placeholders + that will be evaluated as CEL expressions. type: string name: description: Name of the values. @@ -326,11 +319,8 @@ spec: description: ValueFrom references value in another source. properties: ref: - description: |- - Ref references values defined in another resource (Plugin, PluginPreset) - - Deprecated: Ref is deprecated on standalone Plugins and will be removed in a future release. - Consider using a PluginPreset to deploy Plugins utilizing the Ref field. + description: Ref references values defined in another + resource (Plugin, PluginPreset) properties: expression: description: Expression is a CEL expression to extract diff --git a/docs/reference/api/index.html b/docs/reference/api/index.html index 67f0c179c..c7507e842 100644 --- a/docs/reference/api/index.html +++ b/docs/reference/api/index.html @@ -1082,8 +1082,8 @@

ClusterOptionOverride overrides
- -[]PluginOptionValue + +[]PluginPresetPluginOptionValue @@ -3066,8 +3066,6 @@

PluginOptionValue

(Appears on: -ClusterOptionOverride, -PluginPresetPluginSpec, PluginSpec)

PluginOptionValue is the value for a PluginOption.

@@ -3260,6 +3258,11 @@

PluginPreset

PluginPresetPluginOptionValue

+

+(Appears on: +ClusterOptionOverride, +PluginPresetPluginSpec) +

PluginPresetPluginOptionValue is the value for a PluginOption.

@@ -3370,8 +3373,8 @@

PluginPresetPluginSpec optionValues
- -[]PluginOptionValue + +[]PluginPresetPluginOptionValue diff --git a/docs/reference/api/openapi.yaml b/docs/reference/api/openapi.yaml index 95a099a30..619f332ab 100755 --- a/docs/reference/api/openapi.yaml +++ b/docs/reference/api/openapi.yaml @@ -1105,14 +1105,10 @@ components: type: string overrides: items: - description: PluginOptionValue is the value for a PluginOption. + description: PluginPresetPluginOptionValue is the value for a PluginOption. properties: expression: - description: |- - Expression is a YAML string with ${...} placeholders that will be evaluated as CEL expressions. - - Deprecated: Expression is deprecated on standalone Plugins and will be removed in a future release. - Consider using a PluginPreset to deploy Plugins utilizing the Expression field. + description: Expression is a YAML string with ${...} placeholders that will be evaluated as CEL expressions. type: string name: description: Name of the values. @@ -1124,11 +1120,7 @@ components: description: ValueFrom references value in another source. properties: ref: - description: |- - Ref references values defined in another resource (Plugin, PluginPreset) - - Deprecated: Ref is deprecated on standalone Plugins and will be removed in a future release. - Consider using a PluginPreset to deploy Plugins utilizing the Ref field. + description: Ref references values defined in another resource (Plugin, PluginPreset) properties: expression: description: Expression is a CEL expression to extract the value from the referenced resource @@ -1327,14 +1319,10 @@ components: optionValues: description: Values are the values for a PluginDefinition instance. items: - description: PluginOptionValue is the value for a PluginOption. + description: PluginPresetPluginOptionValue is the value for a PluginOption. properties: expression: - description: |- - Expression is a YAML string with ${...} placeholders that will be evaluated as CEL expressions. - - Deprecated: Expression is deprecated on standalone Plugins and will be removed in a future release. - Consider using a PluginPreset to deploy Plugins utilizing the Expression field. + description: Expression is a YAML string with ${...} placeholders that will be evaluated as CEL expressions. type: string name: description: Name of the values. @@ -1346,11 +1334,7 @@ components: description: ValueFrom references value in another source. properties: ref: - description: |- - Ref references values defined in another resource (Plugin, PluginPreset) - - Deprecated: Ref is deprecated on standalone Plugins and will be removed in a future release. - Consider using a PluginPreset to deploy Plugins utilizing the Ref field. + description: Ref references values defined in another resource (Plugin, PluginPreset) properties: expression: description: Expression is a CEL expression to extract the value from the referenced resource diff --git a/internal/cmd/plugin_template.go b/internal/cmd/plugin_template.go index e12773a2d..9e0e09017 100644 --- a/internal/cmd/plugin_template.go +++ b/internal/cmd/plugin_template.go @@ -190,7 +190,7 @@ func (o *PluginTemplatePresetOptions) prepareValues() error { ) // Merge PluginPreset values. - values = helminternal.MergePluginOptionValues(values, o.pluginPreset.Spec.Plugin.OptionValues) + values = helminternal.MergePluginOptionValues(values, convertToPluginOptionValues(o.pluginPreset.Spec.Plugin.OptionValues)) // Merge cluster overrides. values = helminternal.MergePluginOptionValues(values, o.getClusterSpecificOverrides()) @@ -205,6 +205,17 @@ func (o *PluginTemplatePresetOptions) prepareValues() error { return nil } +func convertToPluginOptionValues(presetValues []greenhousev1alpha1.PluginPresetPluginOptionValue) []greenhousev1alpha1.PluginOptionValue { + result := make([]greenhousev1alpha1.PluginOptionValue, 0, len(presetValues)) + for _, pv := range presetValues { + result = append(result, greenhousev1alpha1.PluginOptionValue{ + Name: pv.Name, + Value: pv.Value, + }) + } + return result +} + func (o *PluginTemplatePresetOptions) runHelmTemplate(valuesFile string) error { chartRef := o.pluginDefinition.Spec.HelmChart @@ -289,7 +300,7 @@ func createPluginOptionValue(name, value string) (*greenhousev1alpha1.PluginOpti func (o *PluginTemplatePresetOptions) getClusterSpecificOverrides() []greenhousev1alpha1.PluginOptionValue { for _, override := range o.pluginPreset.Spec.ClusterOptionOverrides { if override.ClusterName == o.clusterName { - return override.Overrides + return convertToPluginOptionValues(override.Overrides) } } return []greenhousev1alpha1.PluginOptionValue{} diff --git a/internal/cmd/plugin_template_test.go b/internal/cmd/plugin_template_test.go index 5ae43d9c0..61315e286 100644 --- a/internal/cmd/plugin_template_test.go +++ b/internal/cmd/plugin_template_test.go @@ -191,7 +191,7 @@ var _ = Describe("prepareValues", func() { Context("with PluginPreset overrides", func() { BeforeEach(func() { - pluginPreset.Spec.Plugin.OptionValues = []greenhousev1alpha1.PluginOptionValue{ + pluginPreset.Spec.Plugin.OptionValues = []greenhousev1alpha1.PluginPresetPluginOptionValue{ { Name: "replicas", Value: &apiextensionsv1.JSON{Raw: []byte("3")}, @@ -221,7 +221,7 @@ var _ = Describe("prepareValues", func() { pluginPreset.Spec.ClusterOptionOverrides = []greenhousev1alpha1.ClusterOptionOverride{ { ClusterName: "test-cluster", - Overrides: []greenhousev1alpha1.PluginOptionValue{ + Overrides: []greenhousev1alpha1.PluginPresetPluginOptionValue{ { Name: "replicas", Value: &apiextensionsv1.JSON{Raw: []byte("5")}, diff --git a/internal/controller/plugin/pluginpreset_controller.go b/internal/controller/plugin/pluginpreset_controller.go index 65d5401f3..4adef8833 100644 --- a/internal/controller/plugin/pluginpreset_controller.go +++ b/internal/controller/plugin/pluginpreset_controller.go @@ -500,7 +500,7 @@ func pluginSpecFromPluginPreset(preset *greenhousev1alpha1.PluginPreset, cluster return greenhousev1alpha1.PluginSpec{ PluginDefinitionRef: preset.Spec.Plugin.PluginDefinitionRef, DisplayName: preset.Spec.Plugin.DisplayName, - OptionValues: preset.Spec.Plugin.OptionValues, + OptionValues: convertToPluginOptionValues(preset.Spec.Plugin.OptionValues), ReleaseNamespace: preset.Spec.Plugin.ReleaseNamespace, DeletionPolicy: preset.Spec.Plugin.DeletionPolicy, IgnoreDifferences: preset.Spec.Plugin.IgnoreDifferences, @@ -510,3 +510,15 @@ func pluginSpecFromPluginPreset(preset *greenhousev1alpha1.PluginPreset, cluster WaitFor: preset.Spec.WaitFor, } } + +// Convert PluginPresetPluginOptionValue → PluginOptionValue for the Plugin +func convertToPluginOptionValues(presetValues []greenhousev1alpha1.PluginPresetPluginOptionValue) []greenhousev1alpha1.PluginOptionValue { + result := make([]greenhousev1alpha1.PluginOptionValue, 0, len(presetValues)) + for _, pv := range presetValues { + result = append(result, greenhousev1alpha1.PluginOptionValue{ + Name: pv.Name, + Value: pv.Value, + }) + } + return result +} diff --git a/internal/controller/plugin/pluginpreset_controller_test.go b/internal/controller/plugin/pluginpreset_controller_test.go index 8c5e9d072..6c3807576 100644 --- a/internal/controller/plugin/pluginpreset_controller_test.go +++ b/internal/controller/plugin/pluginpreset_controller_test.go @@ -264,7 +264,11 @@ var _ = Describe("PluginPreset Controller Lifecycle", Ordered, func() { }).Should(Succeed(), "the Plugin should be created") By("checking plugin options with plugin definition defaults and plugin preset values") - Expect(expPlugin.Spec.OptionValues).To(ContainElement(pluginPreset.Spec.Plugin.OptionValues[0])) + Expect(expPlugin.Spec.OptionValues).To(ContainElement(greenhousev1alpha1.PluginOptionValue{ + Name: pluginPreset.Spec.Plugin.OptionValues[0].Name, + Value: pluginPreset.Spec.Plugin.OptionValues[0].Value, + })) + Expect(expPlugin.Spec.OptionValues).To(ContainElement(greenhousev1alpha1.PluginOptionValue{ Name: defaultPluginDefinition.Spec.Options[0].Name, Value: defaultPluginDefinition.Spec.Options[0].Default, @@ -550,7 +554,10 @@ var _ = Describe("PluginPreset Controller Lifecycle", Ordered, func() { Eventually(func(g Gomega) { plugin = verifyPluginCreatedWithHelmRelease(g, pluginObjectKey) }).Should(Succeed(), "the Plugin should be created successfully with HelmRelease") - Expect(plugin.Spec.OptionValues).To(ContainElement(pluginPreset.Spec.ClusterOptionOverrides[0].Overrides[0]), + Expect(plugin.Spec.OptionValues).To(ContainElement(greenhousev1alpha1.PluginOptionValue{ + Name: pluginPreset.Spec.ClusterOptionOverrides[0].Overrides[0].Name, + Value: pluginPreset.Spec.ClusterOptionOverrides[0].Overrides[0].Value, + }), "ClusterOptionOverrides should be applied to the Plugin OptionValues") By("removing plugin preset") @@ -1132,7 +1139,7 @@ var _ = Describe("PluginPreset Controller Lifecycle", Ordered, func() { var _ = Describe("applyOverridesToPreset", func() { DescribeTable("test cases", - func(preset *greenhousev1alpha1.PluginPreset, clusterName string, expectedOptionValues []greenhousev1alpha1.PluginOptionValue) { + func(preset *greenhousev1alpha1.PluginPreset, clusterName string, expectedOptionValues []greenhousev1alpha1.PluginPresetPluginOptionValue) { result := applyOverridesToPreset(preset, clusterName) Expect(result.Spec.Plugin.OptionValues).To(Equal(expectedOptionValues)) }, @@ -1141,14 +1148,14 @@ var _ = Describe("applyOverridesToPreset", func() { &greenhousev1alpha1.PluginPreset{ Spec: greenhousev1alpha1.PluginPresetSpec{ Plugin: greenhousev1alpha1.PluginPresetPluginSpec{ - OptionValues: []greenhousev1alpha1.PluginOptionValue{ + OptionValues: []greenhousev1alpha1.PluginPresetPluginOptionValue{ {Name: "option-1", Value: test.MustReturnJSONFor("value-1")}, }, }, }, }, clusterA, - []greenhousev1alpha1.PluginOptionValue{ + []greenhousev1alpha1.PluginPresetPluginOptionValue{ {Name: "option-1", Value: test.MustReturnJSONFor("value-1")}, }, ), @@ -1157,14 +1164,14 @@ var _ = Describe("applyOverridesToPreset", func() { &greenhousev1alpha1.PluginPreset{ Spec: greenhousev1alpha1.PluginPresetSpec{ Plugin: greenhousev1alpha1.PluginPresetPluginSpec{ - OptionValues: []greenhousev1alpha1.PluginOptionValue{ + OptionValues: []greenhousev1alpha1.PluginPresetPluginOptionValue{ {Name: "option-1", Value: test.MustReturnJSONFor("value-1")}, }, }, ClusterOptionOverrides: []greenhousev1alpha1.ClusterOptionOverride{ { ClusterName: clusterB, - Overrides: []greenhousev1alpha1.PluginOptionValue{ + Overrides: []greenhousev1alpha1.PluginPresetPluginOptionValue{ {Name: "option-1", Value: test.MustReturnJSONFor("overridden")}, }, }, @@ -1172,7 +1179,7 @@ var _ = Describe("applyOverridesToPreset", func() { }, }, clusterA, - []greenhousev1alpha1.PluginOptionValue{ + []greenhousev1alpha1.PluginPresetPluginOptionValue{ {Name: "option-1", Value: test.MustReturnJSONFor("value-1")}, }, ), @@ -1181,7 +1188,7 @@ var _ = Describe("applyOverridesToPreset", func() { &greenhousev1alpha1.PluginPreset{ Spec: greenhousev1alpha1.PluginPresetSpec{ Plugin: greenhousev1alpha1.PluginPresetPluginSpec{ - OptionValues: []greenhousev1alpha1.PluginOptionValue{ + OptionValues: []greenhousev1alpha1.PluginPresetPluginOptionValue{ {Name: "option-1", Value: test.MustReturnJSONFor("original")}, {Name: "option-2", Value: test.MustReturnJSONFor("unchanged")}, }, @@ -1189,7 +1196,7 @@ var _ = Describe("applyOverridesToPreset", func() { ClusterOptionOverrides: []greenhousev1alpha1.ClusterOptionOverride{ { ClusterName: clusterA, - Overrides: []greenhousev1alpha1.PluginOptionValue{ + Overrides: []greenhousev1alpha1.PluginPresetPluginOptionValue{ {Name: "option-1", Value: test.MustReturnJSONFor("overridden")}, }, }, @@ -1197,7 +1204,7 @@ var _ = Describe("applyOverridesToPreset", func() { }, }, clusterA, - []greenhousev1alpha1.PluginOptionValue{ + []greenhousev1alpha1.PluginPresetPluginOptionValue{ {Name: "option-1", Value: test.MustReturnJSONFor("overridden")}, {Name: "option-2", Value: test.MustReturnJSONFor("unchanged")}, }, @@ -1207,14 +1214,14 @@ var _ = Describe("applyOverridesToPreset", func() { &greenhousev1alpha1.PluginPreset{ Spec: greenhousev1alpha1.PluginPresetSpec{ Plugin: greenhousev1alpha1.PluginPresetPluginSpec{ - OptionValues: []greenhousev1alpha1.PluginOptionValue{ + OptionValues: []greenhousev1alpha1.PluginPresetPluginOptionValue{ {Name: "option-1", Value: test.MustReturnJSONFor("value-1")}, }, }, ClusterOptionOverrides: []greenhousev1alpha1.ClusterOptionOverride{ { ClusterName: clusterA, - Overrides: []greenhousev1alpha1.PluginOptionValue{ + Overrides: []greenhousev1alpha1.PluginPresetPluginOptionValue{ {Name: "option-new", Value: test.MustReturnJSONFor("new-value")}, }, }, @@ -1222,7 +1229,7 @@ var _ = Describe("applyOverridesToPreset", func() { }, }, clusterA, - []greenhousev1alpha1.PluginOptionValue{ + []greenhousev1alpha1.PluginPresetPluginOptionValue{ {Name: "option-1", Value: test.MustReturnJSONFor("value-1")}, {Name: "option-new", Value: test.MustReturnJSONFor("new-value")}, }, @@ -1232,7 +1239,7 @@ var _ = Describe("applyOverridesToPreset", func() { &greenhousev1alpha1.PluginPreset{ Spec: greenhousev1alpha1.PluginPresetSpec{ Plugin: greenhousev1alpha1.PluginPresetPluginSpec{ - OptionValues: []greenhousev1alpha1.PluginOptionValue{ + OptionValues: []greenhousev1alpha1.PluginPresetPluginOptionValue{ {Name: "option-1", Value: test.MustReturnJSONFor(1)}, {Name: "option-2", Value: test.MustReturnJSONFor(2)}, {Name: "option-3", Value: test.MustReturnJSONFor(3)}, @@ -1241,7 +1248,7 @@ var _ = Describe("applyOverridesToPreset", func() { ClusterOptionOverrides: []greenhousev1alpha1.ClusterOptionOverride{ { ClusterName: clusterA, - Overrides: []greenhousev1alpha1.PluginOptionValue{ + Overrides: []greenhousev1alpha1.PluginPresetPluginOptionValue{ {Name: "option-2", Value: test.MustReturnJSONFor(22)}, {Name: "option-3", Value: test.MustReturnJSONFor(33)}, {Name: "option-4", Value: test.MustReturnJSONFor(44)}, @@ -1251,7 +1258,7 @@ var _ = Describe("applyOverridesToPreset", func() { }, }, clusterA, - []greenhousev1alpha1.PluginOptionValue{ + []greenhousev1alpha1.PluginPresetPluginOptionValue{ {Name: "option-1", Value: test.MustReturnJSONFor(1)}, {Name: "option-2", Value: test.MustReturnJSONFor(22)}, {Name: "option-3", Value: test.MustReturnJSONFor(33)}, @@ -1263,12 +1270,12 @@ var _ = Describe("applyOverridesToPreset", func() { &greenhousev1alpha1.PluginPreset{ Spec: greenhousev1alpha1.PluginPresetSpec{ Plugin: greenhousev1alpha1.PluginPresetPluginSpec{ - OptionValues: []greenhousev1alpha1.PluginOptionValue{}, + OptionValues: []greenhousev1alpha1.PluginPresetPluginOptionValue{}, }, ClusterOptionOverrides: []greenhousev1alpha1.ClusterOptionOverride{ { ClusterName: clusterA, - Overrides: []greenhousev1alpha1.PluginOptionValue{ + Overrides: []greenhousev1alpha1.PluginPresetPluginOptionValue{ {Name: "option-1", Value: test.MustReturnJSONFor("added")}, }, }, @@ -1276,7 +1283,7 @@ var _ = Describe("applyOverridesToPreset", func() { }, }, clusterA, - []greenhousev1alpha1.PluginOptionValue{ + []greenhousev1alpha1.PluginPresetPluginOptionValue{ {Name: "option-1", Value: test.MustReturnJSONFor("added")}, }, ), @@ -1287,14 +1294,14 @@ var _ = Describe("applyOverridesToPreset", func() { preset := &greenhousev1alpha1.PluginPreset{ Spec: greenhousev1alpha1.PluginPresetSpec{ Plugin: greenhousev1alpha1.PluginPresetPluginSpec{ - OptionValues: []greenhousev1alpha1.PluginOptionValue{ + OptionValues: []greenhousev1alpha1.PluginPresetPluginOptionValue{ {Name: "option-1", Value: originalValue}, }, }, ClusterOptionOverrides: []greenhousev1alpha1.ClusterOptionOverride{ { ClusterName: clusterA, - Overrides: []greenhousev1alpha1.PluginOptionValue{ + Overrides: []greenhousev1alpha1.PluginPresetPluginOptionValue{ {Name: "option-1", Value: test.MustReturnJSONFor("overridden")}, }, }, diff --git a/internal/controller/plugin/pluginpreset_values_resolver.go b/internal/controller/plugin/pluginpreset_values_resolver.go index 10f88a737..3f72a7559 100644 --- a/internal/controller/plugin/pluginpreset_values_resolver.go +++ b/internal/controller/plugin/pluginpreset_values_resolver.go @@ -24,17 +24,10 @@ func (r *PluginPresetReconciler) resolvePluginOptionValuesForPreset( cluster *greenhousev1alpha1.Cluster, ) ([]greenhousev1alpha1.PluginOptionValue, error) { - optionValues := preset.Spec.Plugin.OptionValues - if r.ExpressionEvaluationEnabled { - var err error - optionValues, err = r.resolveExpressionsForPreset(ctx, preset, cluster) - if err != nil { - return nil, fmt.Errorf("failed to resolve expressions: %w", err) - } + return r.resolveExpressionsForPreset(ctx, preset, cluster) } - - return optionValues, nil + return convertToPluginOptionValues(preset.Spec.Plugin.OptionValues), nil } // resolveExpressionsForPreset evaluates all expression fields in PluginPreset option values. @@ -52,7 +45,7 @@ func (r *PluginPresetReconciler) resolveExpressionsForPreset( } } if !hasExpressions { - return preset.Spec.Plugin.OptionValues, nil + return convertToPluginOptionValues(preset.Spec.Plugin.OptionValues), nil } tempPlugin := greenhousev1alpha1.Plugin{ @@ -69,12 +62,10 @@ func (r *PluginPresetReconciler) resolveExpressionsForPreset( if err != nil { return nil, fmt.Errorf("failed to get greenhouse values: %w", err) } - templateData, err := helm.BuildTemplateData(greenhouseValuesList) if err != nil { return nil, fmt.Errorf("failed to build template data: %w", err) } - result := make([]greenhousev1alpha1.PluginOptionValue, 0, len(preset.Spec.Plugin.OptionValues)) for _, optionValue := range preset.Spec.Plugin.OptionValues { if optionValue.Expression != nil { @@ -87,10 +78,12 @@ func (r *PluginPresetReconciler) resolveExpressionsForPreset( Value: &apiextensionsv1.JSON{Raw: evaluatedValue}, }) } else { - result = append(result, optionValue) + result = append(result, greenhousev1alpha1.PluginOptionValue{ + Name: optionValue.Name, + Value: optionValue.Value, + }) } } - return result, nil } @@ -107,7 +100,7 @@ func applyOverridesToPreset(preset *greenhousev1alpha1.PluginPreset, clusterName } for _, overrideValue := range presetCopy.Spec.ClusterOptionOverrides[index].Overrides { - valueIndex := slices.IndexFunc(presetCopy.Spec.Plugin.OptionValues, func(value greenhousev1alpha1.PluginOptionValue) bool { + valueIndex := slices.IndexFunc(presetCopy.Spec.Plugin.OptionValues, func(value greenhousev1alpha1.PluginPresetPluginOptionValue) bool { return value.Name == overrideValue.Name }) diff --git a/internal/test/resources.go b/internal/test/resources.go index 190eedd4b..65ada08d6 100644 --- a/internal/test/resources.go +++ b/internal/test/resources.go @@ -481,7 +481,7 @@ func WithPluginPresetPluginSpec(pluginSpec greenhousev1alpha1.PluginSpec) func(* return func(pp *greenhousev1alpha1.PluginPreset) { pp.Spec.Plugin.PluginDefinitionRef = pluginSpec.PluginDefinitionRef pp.Spec.Plugin.DisplayName = pluginSpec.DisplayName - pp.Spec.Plugin.OptionValues = pluginSpec.OptionValues + pp.Spec.Plugin.OptionValues = convertToPresetOptionValues(pluginSpec.OptionValues) pp.Spec.Plugin.ReleaseNamespace = pluginSpec.ReleaseNamespace pp.Spec.Plugin.ReleaseName = pluginSpec.ReleaseName pp.Spec.Plugin.DeletionPolicy = pluginSpec.DeletionPolicy @@ -489,6 +489,26 @@ func WithPluginPresetPluginSpec(pluginSpec greenhousev1alpha1.PluginSpec) func(* } } +// convertToPresetOptionValues converts []PluginOptionValue to []PluginPresetPluginOptionValue +func convertToPresetOptionValues(values []greenhousev1alpha1.PluginOptionValue) []greenhousev1alpha1.PluginPresetPluginOptionValue { + result := make([]greenhousev1alpha1.PluginPresetPluginOptionValue, 0, len(values)) + for _, v := range values { + pv := greenhousev1alpha1.PluginPresetPluginOptionValue{ + Name: v.Name, + Value: v.Value, + Expression: v.Expression, + } + if v.ValueFrom != nil { + pv.ValueFrom = &greenhousev1alpha1.PluginPresetPluginValueFromSource{ + Secret: v.ValueFrom.Secret, + Ref: v.ValueFrom.Ref, + } + } + result = append(result, pv) + } + return result +} + // WithPluginPresetLabel sets the label on a PluginPreset func WithPluginPresetLabel(key, value string) func(*greenhousev1alpha1.PluginPreset) { return func(pp *greenhousev1alpha1.PluginPreset) { @@ -512,15 +532,16 @@ func WithPluginPresetAnnotation(key, value string) func(*greenhousev1alpha1.Plug // WithClusterOverrides sets the ClusterOverrides for a Cluster func WithClusterOverride(clusterName string, optionValues []greenhousev1alpha1.PluginOptionValue) func(*greenhousev1alpha1.PluginPreset) { return func(pp *greenhousev1alpha1.PluginPreset) { + presetOverrides := convertToPresetOptionValues(optionValues) for co := range pp.Spec.ClusterOptionOverrides { if pp.Spec.ClusterOptionOverrides[co].ClusterName == clusterName { - pp.Spec.ClusterOptionOverrides[co].Overrides = optionValues + pp.Spec.ClusterOptionOverrides[co].Overrides = presetOverrides return } } pp.Spec.ClusterOptionOverrides = append(pp.Spec.ClusterOptionOverrides, greenhousev1alpha1.ClusterOptionOverride{ ClusterName: clusterName, - Overrides: optionValues, + Overrides: presetOverrides, }) } } diff --git a/internal/webhook/v1alpha1/pluginpreset_webhook.go b/internal/webhook/v1alpha1/pluginpreset_webhook.go index 4d1ddc4ee..b81038ce0 100644 --- a/internal/webhook/v1alpha1/pluginpreset_webhook.go +++ b/internal/webhook/v1alpha1/pluginpreset_webhook.go @@ -142,17 +142,34 @@ func validatePluginOptionValuesForPreset(pluginPreset *greenhousev1alpha1.Plugin var allErrs field.ErrorList optionValuesPath := field.NewPath("spec").Child("plugin").Child("optionValues") - errors := validatePluginOptionValues(pluginPreset.Spec.Plugin.OptionValues, pluginDefinitionName, pluginDefinitionSpec, false, optionValuesPath) + errors := validatePluginOptionValues(convertPresetToPluginOptionValues(pluginPreset.Spec.Plugin.OptionValues), pluginDefinitionName, pluginDefinitionSpec, false, optionValuesPath) allErrs = append(allErrs, errors...) for idx, overridesForSingleCluster := range pluginPreset.Spec.ClusterOptionOverrides { optionOverridesPath := field.NewPath("spec").Child("clusterOptionOverrides").Index(idx).Child("overrides") - errors = validatePluginOptionValues(overridesForSingleCluster.Overrides, pluginDefinitionName, pluginDefinitionSpec, false, optionOverridesPath) + errors = validatePluginOptionValues(convertPresetToPluginOptionValues(overridesForSingleCluster.Overrides), pluginDefinitionName, pluginDefinitionSpec, false, optionOverridesPath) allErrs = append(allErrs, errors...) } return allErrs } +func convertPresetToPluginOptionValues(presetValues []greenhousev1alpha1.PluginPresetPluginOptionValue) []greenhousev1alpha1.PluginOptionValue { + result := make([]greenhousev1alpha1.PluginOptionValue, 0, len(presetValues)) + for _, pv := range presetValues { + ov := greenhousev1alpha1.PluginOptionValue{ + Name: pv.Name, + Value: pv.Value, + } + if pv.ValueFrom != nil { + ov.ValueFrom = &greenhousev1alpha1.PluginValueFromSource{ + Secret: pv.ValueFrom.Secret, + } + } + result = append(result, ov) + } + return result +} + // validateWaitForPluginRefs validates that the WaitFor list is unique and that each PluginRef has exactly one field set. func validateWaitForPluginRefs(items []greenhousev1alpha1.WaitForItem, isPluginInCentralCluster bool) field.ErrorList { itemsPath := field.NewPath("spec", "waitFor") diff --git a/internal/webhook/v1alpha1/pluginpreset_webhook_test.go b/internal/webhook/v1alpha1/pluginpreset_webhook_test.go index 024169028..30100823e 100644 --- a/internal/webhook/v1alpha1/pluginpreset_webhook_test.go +++ b/internal/webhook/v1alpha1/pluginpreset_webhook_test.go @@ -268,7 +268,7 @@ var _ = Describe("PluginPreset Admission Tests", Ordered, func() { }) var _ = Describe("Validate Plugin OptionValues for PluginPreset", func() { - DescribeTable("Validate OptionValues in .Spec.Plugin contain either Value or ValueFrom", func(value *apiextensionsv1.JSON, valueFrom *greenhousev1alpha1.PluginValueFromSource, expErr bool) { + DescribeTable("Validate OptionValues in .Spec.Plugin contain either Value or ValueFrom", func(value *apiextensionsv1.JSON, valueFrom *greenhousev1alpha1.PluginPresetPluginValueFromSource, expErr bool) { pluginPreset := &greenhousev1alpha1.PluginPreset{ TypeMeta: metav1.TypeMeta{ Kind: "PluginPreset", @@ -284,7 +284,7 @@ var _ = Describe("Validate Plugin OptionValues for PluginPreset", func() { Name: "test", Kind: greenhousev1alpha1.ClusterPluginDefinitionKind, }, - OptionValues: []greenhousev1alpha1.PluginOptionValue{ + OptionValues: []greenhousev1alpha1.PluginPresetPluginOptionValue{ { Name: "test", Value: value, @@ -331,12 +331,12 @@ var _ = Describe("Validate Plugin OptionValues for PluginPreset", func() { } }, Entry("Value and ValueFrom nil", nil, nil, true), - Entry("Value and ValueFrom not nil", test.MustReturnJSONFor("test"), &greenhousev1alpha1.PluginValueFromSource{Secret: &greenhousev1alpha1.SecretKeyReference{Name: "my-secret"}}, true), + Entry("Value and ValueFrom not nil", test.MustReturnJSONFor("test"), &greenhousev1alpha1.PluginPresetPluginValueFromSource{Secret: &greenhousev1alpha1.SecretKeyReference{Name: "my-secret"}}, true), Entry("Value not nil", test.MustReturnJSONFor("test"), nil, false), - Entry("ValueFrom not nil", nil, &greenhousev1alpha1.PluginValueFromSource{Secret: &greenhousev1alpha1.SecretKeyReference{Name: "my-secret", Key: "secret-key"}}, false), + Entry("ValueFrom not nil", nil, &greenhousev1alpha1.PluginPresetPluginValueFromSource{Secret: &greenhousev1alpha1.SecretKeyReference{Name: "my-secret", Key: "secret-key"}}, false), ) - DescribeTable("Validate OptionValues in .Spec.ClusterOptionOverrides contain either Value or ValueFrom", func(value *apiextensionsv1.JSON, valueFrom *greenhousev1alpha1.PluginValueFromSource, expErr bool) { + DescribeTable("Validate OptionValues in .Spec.ClusterOptionOverrides contain either Value or ValueFrom", func(value *apiextensionsv1.JSON, valueFrom *greenhousev1alpha1.PluginPresetPluginValueFromSource, expErr bool) { pluginPreset := &greenhousev1alpha1.PluginPreset{ TypeMeta: metav1.TypeMeta{ Kind: "PluginPreset", @@ -352,12 +352,12 @@ var _ = Describe("Validate Plugin OptionValues for PluginPreset", func() { Name: "test", Kind: greenhousev1alpha1.ClusterPluginDefinitionKind, }, - OptionValues: []greenhousev1alpha1.PluginOptionValue{}, + OptionValues: []greenhousev1alpha1.PluginPresetPluginOptionValue{}, }, ClusterOptionOverrides: []greenhousev1alpha1.ClusterOptionOverride{ { ClusterName: "test-cluster", - Overrides: []greenhousev1alpha1.PluginOptionValue{ + Overrides: []greenhousev1alpha1.PluginPresetPluginOptionValue{ { Name: "test", Value: value, @@ -405,9 +405,9 @@ var _ = Describe("Validate Plugin OptionValues for PluginPreset", func() { } }, Entry("Value and ValueFrom nil", nil, nil, true), - Entry("Value and ValueFrom not nil", test.MustReturnJSONFor("test"), &greenhousev1alpha1.PluginValueFromSource{Secret: &greenhousev1alpha1.SecretKeyReference{Name: "my-secret"}}, true), + Entry("Value and ValueFrom not nil", test.MustReturnJSONFor("test"), &greenhousev1alpha1.PluginPresetPluginValueFromSource{Secret: &greenhousev1alpha1.SecretKeyReference{Name: "my-secret"}}, true), Entry("Value not nil", test.MustReturnJSONFor("test"), nil, false), - Entry("ValueFrom not nil", nil, &greenhousev1alpha1.PluginValueFromSource{Secret: &greenhousev1alpha1.SecretKeyReference{Name: "my-secret", Key: "secret-key"}}, false), + Entry("ValueFrom not nil", nil, &greenhousev1alpha1.PluginPresetPluginValueFromSource{Secret: &greenhousev1alpha1.SecretKeyReference{Name: "my-secret", Key: "secret-key"}}, false), ) DescribeTable("Validate WaitFor PluginRefs", func(waitForItems []greenhousev1alpha1.WaitForItem, expErr bool) { diff --git a/types/typescript/schema.d.ts b/types/typescript/schema.d.ts index 5f3987e5c..6f78c16ff 100644 --- a/types/typescript/schema.d.ts +++ b/types/typescript/schema.d.ts @@ -944,8 +944,6 @@ export interface components { deletionPolicy: "Delete" | "Retain"; /** @description PluginSpec is the spec of the plugin to be deployed by the PluginPreset. */ plugin: { - /** @description ClusterName is the name of the cluster the plugin is deployed to. If not set, the plugin is deployed to the greenhouse cluster. */ - clusterName?: string; /** * @description DeletionPolicy defines how Helm Releases created by a Plugin are handled upon deletion of the Plugin. * Supported values are "Delete" and "Retain". If not set, defaults to "Delete". @@ -1059,16 +1057,6 @@ export interface components { * Defaults to the Greenhouse managed namespace if not set. */ releaseNamespace?: string; - /** @description WaitFor defines other Plugins to wait for before installing this Plugin. */ - waitFor?: { - /** @description PluginRef defines a reference to the Plugin. */ - pluginRef: { - /** @description Name of the Plugin. */ - name?: string; - /** @description PluginPreset is the name of the PluginPreset which creates the Plugin. */ - pluginPreset?: string; - }; - }[]; }; /** @description WaitFor defines other Plugins to wait for before creating the Plugin. */ waitFor?: { @@ -1198,7 +1186,12 @@ export interface components { }[]; /** @description Values are the values for a PluginDefinition instance. */ optionValues?: { - /** @description Expression is a YAML string with ${...} placeholders that will be evaluated as CEL expressions. */ + /** + * @description Expression is a YAML string with ${...} placeholders that will be evaluated as CEL expressions. + * + * Deprecated: Expression is deprecated on standalone Plugins and will be removed in a future release. + * Consider using a PluginPreset to deploy Plugins utilizing the Expression field. + */ expression?: string; /** @description Name of the values. */ name: string; @@ -1206,7 +1199,12 @@ export interface components { value?: unknown; /** @description ValueFrom references value in another source. */ valueFrom?: { - /** @description Ref references values defined in another resource (Plugin, PluginPreset) */ + /** + * @description Ref references values defined in another resource (Plugin, PluginPreset) + * + * Deprecated: Ref is deprecated on standalone Plugins and will be removed in a future release. + * Consider using a PluginPreset to deploy Plugins utilizing the Ref field. + */ ref?: { /** @description Expression is a CEL expression to extract the value from the referenced resource */ expression: string; From 5a5b9e85e7bbfb7a1434d4712771e46a9af7c643 Mon Sep 17 00:00:00 2001 From: "cloud-operator-bot[bot]" <224791424+cloud-operator-bot[bot]@users.noreply.github.com> Date: Wed, 10 Jun 2026 12:22:09 +0000 Subject: [PATCH 13/18] Automatic generation of CRD API Docs --- docs/reference/api/index.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/reference/api/index.html b/docs/reference/api/index.html index c7507e842..958efd535 100644 --- a/docs/reference/api/index.html +++ b/docs/reference/api/index.html @@ -3260,7 +3260,7 @@

PluginPresetPlugi

(Appears on: -ClusterOptionOverride, +ClusterOptionOverride, PluginPresetPluginSpec)

PluginPresetPluginOptionValue is the value for a PluginOption.

From 97e84fd7e56e940b6dde23c2478ed519d3ebf301 Mon Sep 17 00:00:00 2001 From: Klaudiusz Fabryczny Date: Wed, 10 Jun 2026 15:51:43 +0200 Subject: [PATCH 14/18] Add missing fields: Expression and ValueFrom Signed-off-by: Klaudiusz Fabryczny --- internal/cmd/plugin_template.go | 16 ++++++++++++++-- .../plugin/pluginpreset_controller.go | 19 +++++++++++++++++-- .../plugin/pluginpreset_values_resolver.go | 11 +++++++++-- .../webhook/v1alpha1/pluginpreset_webhook.go | 8 ++++++-- 4 files changed, 46 insertions(+), 8 deletions(-) diff --git a/internal/cmd/plugin_template.go b/internal/cmd/plugin_template.go index 9e0e09017..cda18866a 100644 --- a/internal/cmd/plugin_template.go +++ b/internal/cmd/plugin_template.go @@ -208,10 +208,22 @@ func (o *PluginTemplatePresetOptions) prepareValues() error { func convertToPluginOptionValues(presetValues []greenhousev1alpha1.PluginPresetPluginOptionValue) []greenhousev1alpha1.PluginOptionValue { result := make([]greenhousev1alpha1.PluginOptionValue, 0, len(presetValues)) for _, pv := range presetValues { - result = append(result, greenhousev1alpha1.PluginOptionValue{ + ov := greenhousev1alpha1.PluginOptionValue{ Name: pv.Name, Value: pv.Value, - }) + } + if pv.Expression != nil { + ov.Expression = pv.Expression + } + if pv.ValueFrom != nil { + ov.ValueFrom = &greenhousev1alpha1.PluginValueFromSource{ + Secret: pv.ValueFrom.Secret, + } + if pv.ValueFrom.Ref != nil { + ov.ValueFrom.Ref = pv.ValueFrom.Ref + } + } + result = append(result, ov) } return result } diff --git a/internal/controller/plugin/pluginpreset_controller.go b/internal/controller/plugin/pluginpreset_controller.go index 4adef8833..394ed0e42 100644 --- a/internal/controller/plugin/pluginpreset_controller.go +++ b/internal/controller/plugin/pluginpreset_controller.go @@ -515,10 +515,25 @@ func pluginSpecFromPluginPreset(preset *greenhousev1alpha1.PluginPreset, cluster func convertToPluginOptionValues(presetValues []greenhousev1alpha1.PluginPresetPluginOptionValue) []greenhousev1alpha1.PluginOptionValue { result := make([]greenhousev1alpha1.PluginOptionValue, 0, len(presetValues)) for _, pv := range presetValues { - result = append(result, greenhousev1alpha1.PluginOptionValue{ + ov := greenhousev1alpha1.PluginOptionValue{ Name: pv.Name, Value: pv.Value, - }) + } + + if pv.Expression != nil { + ov.Expression = pv.Expression + } + + if pv.ValueFrom != nil { + ov.ValueFrom = &greenhousev1alpha1.PluginValueFromSource{ + Secret: pv.ValueFrom.Secret, + } + + if pv.ValueFrom.Ref != nil { + ov.ValueFrom.Ref = pv.ValueFrom.Ref + } + } + result = append(result, ov) } return result } diff --git a/internal/controller/plugin/pluginpreset_values_resolver.go b/internal/controller/plugin/pluginpreset_values_resolver.go index 3f72a7559..da5ab58af 100644 --- a/internal/controller/plugin/pluginpreset_values_resolver.go +++ b/internal/controller/plugin/pluginpreset_values_resolver.go @@ -78,10 +78,17 @@ func (r *PluginPresetReconciler) resolveExpressionsForPreset( Value: &apiextensionsv1.JSON{Raw: evaluatedValue}, }) } else { - result = append(result, greenhousev1alpha1.PluginOptionValue{ + ov := greenhousev1alpha1.PluginOptionValue{ Name: optionValue.Name, Value: optionValue.Value, - }) + } + if optionValue.ValueFrom != nil { + ov.ValueFrom = &greenhousev1alpha1.PluginValueFromSource{ + Secret: optionValue.ValueFrom.Secret, + Ref: optionValue.ValueFrom.Ref, + } + } + result = append(result, ov) } } return result, nil diff --git a/internal/webhook/v1alpha1/pluginpreset_webhook.go b/internal/webhook/v1alpha1/pluginpreset_webhook.go index b81038ce0..c291dd5b0 100644 --- a/internal/webhook/v1alpha1/pluginpreset_webhook.go +++ b/internal/webhook/v1alpha1/pluginpreset_webhook.go @@ -157,13 +157,17 @@ func convertPresetToPluginOptionValues(presetValues []greenhousev1alpha1.PluginP result := make([]greenhousev1alpha1.PluginOptionValue, 0, len(presetValues)) for _, pv := range presetValues { ov := greenhousev1alpha1.PluginOptionValue{ - Name: pv.Name, - Value: pv.Value, + Name: pv.Name, + Value: pv.Value, + Expression: pv.Expression, } if pv.ValueFrom != nil { ov.ValueFrom = &greenhousev1alpha1.PluginValueFromSource{ Secret: pv.ValueFrom.Secret, } + if pv.ValueFrom.Ref != nil { + ov.ValueFrom.Ref = pv.ValueFrom.Ref + } } result = append(result, ov) } From a588f785702716dd4415817a340fa2f0a5444a62 Mon Sep 17 00:00:00 2001 From: Klaudiusz Fabryczny Date: Wed, 10 Jun 2026 16:37:58 +0200 Subject: [PATCH 15/18] Update tests descriptions Signed-off-by: Klaudiusz Fabryczny --- internal/webhook/v1alpha1/pluginpreset_webhook_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/webhook/v1alpha1/pluginpreset_webhook_test.go b/internal/webhook/v1alpha1/pluginpreset_webhook_test.go index 30100823e..37b6dd1cc 100644 --- a/internal/webhook/v1alpha1/pluginpreset_webhook_test.go +++ b/internal/webhook/v1alpha1/pluginpreset_webhook_test.go @@ -268,7 +268,7 @@ var _ = Describe("PluginPreset Admission Tests", Ordered, func() { }) var _ = Describe("Validate Plugin OptionValues for PluginPreset", func() { - DescribeTable("Validate OptionValues in .Spec.Plugin contain either Value or ValueFrom", func(value *apiextensionsv1.JSON, valueFrom *greenhousev1alpha1.PluginPresetPluginValueFromSource, expErr bool) { + DescribeTable("Validate OptionValues in .Spec.Plugin contain exactly one of Value, ValueFrom, or Expression", func(value *apiextensionsv1.JSON, valueFrom *greenhousev1alpha1.PluginPresetPluginValueFromSource, expErr bool) { pluginPreset := &greenhousev1alpha1.PluginPreset{ TypeMeta: metav1.TypeMeta{ Kind: "PluginPreset", @@ -336,7 +336,7 @@ var _ = Describe("Validate Plugin OptionValues for PluginPreset", func() { Entry("ValueFrom not nil", nil, &greenhousev1alpha1.PluginPresetPluginValueFromSource{Secret: &greenhousev1alpha1.SecretKeyReference{Name: "my-secret", Key: "secret-key"}}, false), ) - DescribeTable("Validate OptionValues in .Spec.ClusterOptionOverrides contain either Value or ValueFrom", func(value *apiextensionsv1.JSON, valueFrom *greenhousev1alpha1.PluginPresetPluginValueFromSource, expErr bool) { + DescribeTable("Validate OptionValues in .Spec.ClusterOptionOverrides contain exactly one of Value, ValueFrom, or Expression", func(value *apiextensionsv1.JSON, valueFrom *greenhousev1alpha1.PluginPresetPluginValueFromSource, expErr bool) { pluginPreset := &greenhousev1alpha1.PluginPreset{ TypeMeta: metav1.TypeMeta{ Kind: "PluginPreset", From 04c718a66da21672d6d9127eeb6b346ffce61327 Mon Sep 17 00:00:00 2001 From: Klaudiusz Fabryczny Date: Thu, 11 Jun 2026 11:11:21 +0200 Subject: [PATCH 16/18] Add nil guard for celResolver Signed-off-by: Klaudiusz Fabryczny --- internal/controller/plugin/plugin_controller_flux.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/controller/plugin/plugin_controller_flux.go b/internal/controller/plugin/plugin_controller_flux.go index 77ff9f37d..b345bf833 100644 --- a/internal/controller/plugin/plugin_controller_flux.go +++ b/internal/controller/plugin/plugin_controller_flux.go @@ -469,6 +469,9 @@ func computeReleaseValues(ctx context.Context, c client.Client, plugin *greenhou continue case v.Expression != nil: + if celResolver == nil { + continue + } // This PR adds CEL expression evaluation to the PluginPreset controller (#1774). // The Plugin controller's expression evaluation remains active (gated by feature flag) resolvedOptionValue, err := celResolver.ResolveExpression(v, expressionEvaluation) From df4487dfba6ec6db38b843df32ee3cc8deefbbe4 Mon Sep 17 00:00:00 2001 From: Klaudiusz Fabryczny Date: Thu, 11 Jun 2026 12:41:25 +0200 Subject: [PATCH 17/18] Always call ResolveExpression Signed-off-by: Klaudiusz Fabryczny --- internal/controller/plugin/plugin_controller_flux.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/controller/plugin/plugin_controller_flux.go b/internal/controller/plugin/plugin_controller_flux.go index b345bf833..e4375b5cc 100644 --- a/internal/controller/plugin/plugin_controller_flux.go +++ b/internal/controller/plugin/plugin_controller_flux.go @@ -470,7 +470,10 @@ func computeReleaseValues(ctx context.Context, c client.Client, plugin *greenhou case v.Expression != nil: if celResolver == nil { - continue + celResolver, err = helm.NewCELResolver(optionValues) + if err != nil { + return nil, fmt.Errorf("failed to initialize CEL resolver: %w", err) + } } // This PR adds CEL expression evaluation to the PluginPreset controller (#1774). // The Plugin controller's expression evaluation remains active (gated by feature flag) From 241d0b46ce0aba40939f6d2a24feaf007cf05f96 Mon Sep 17 00:00:00 2001 From: Klaudiusz Fabryczny Date: Thu, 11 Jun 2026 14:27:56 +0200 Subject: [PATCH 18/18] Add Expression case to tests Signed-off-by: Klaudiusz Fabryczny --- .../v1alpha1/pluginpreset_webhook_test.go | 49 +++++++++++++------ 1 file changed, 33 insertions(+), 16 deletions(-) diff --git a/internal/webhook/v1alpha1/pluginpreset_webhook_test.go b/internal/webhook/v1alpha1/pluginpreset_webhook_test.go index 37b6dd1cc..0d3fb39bf 100644 --- a/internal/webhook/v1alpha1/pluginpreset_webhook_test.go +++ b/internal/webhook/v1alpha1/pluginpreset_webhook_test.go @@ -12,6 +12,7 @@ import ( greenhouseapis "github.com/cloudoperators/greenhouse/api" greenhousev1alpha1 "github.com/cloudoperators/greenhouse/api/v1alpha1" "github.com/cloudoperators/greenhouse/internal/clientutil" + "github.com/cloudoperators/greenhouse/internal/local/utils" "github.com/cloudoperators/greenhouse/internal/test" ) @@ -268,7 +269,7 @@ var _ = Describe("PluginPreset Admission Tests", Ordered, func() { }) var _ = Describe("Validate Plugin OptionValues for PluginPreset", func() { - DescribeTable("Validate OptionValues in .Spec.Plugin contain exactly one of Value, ValueFrom, or Expression", func(value *apiextensionsv1.JSON, valueFrom *greenhousev1alpha1.PluginPresetPluginValueFromSource, expErr bool) { + DescribeTable("Validate OptionValues in .Spec.Plugin contain exactly one of Value, ValueFrom, or Expression", func(value *apiextensionsv1.JSON, valueFrom *greenhousev1alpha1.PluginPresetPluginValueFromSource, expression *string, expErr bool) { pluginPreset := &greenhousev1alpha1.PluginPreset{ TypeMeta: metav1.TypeMeta{ Kind: "PluginPreset", @@ -286,9 +287,10 @@ var _ = Describe("Validate Plugin OptionValues for PluginPreset", func() { }, OptionValues: []greenhousev1alpha1.PluginPresetPluginOptionValue{ { - Name: "test", - Value: value, - ValueFrom: valueFrom, + Name: "test", + Value: value, + ValueFrom: valueFrom, + Expression: expression, }, }, }, @@ -304,6 +306,9 @@ var _ = Describe("Validate Plugin OptionValues for PluginPreset", func() { case valueFrom != nil: defaultVal = test.MustReturnJSONFor(valueFrom.Secret.Name) optionType = greenhousev1alpha1.PluginOptionTypeSecret + case expression != nil: + defaultVal = test.MustReturnJSONFor("expression-default") + optionType = greenhousev1alpha1.PluginOptionTypeString } pluginDefinition := &greenhousev1alpha1.ClusterPluginDefinition{ @@ -330,13 +335,17 @@ var _ = Describe("Validate Plugin OptionValues for PluginPreset", func() { Expect(errList).To(BeEmpty(), "expected no error, got %v", errList) } }, - Entry("Value and ValueFrom nil", nil, nil, true), - Entry("Value and ValueFrom not nil", test.MustReturnJSONFor("test"), &greenhousev1alpha1.PluginPresetPluginValueFromSource{Secret: &greenhousev1alpha1.SecretKeyReference{Name: "my-secret"}}, true), - Entry("Value not nil", test.MustReturnJSONFor("test"), nil, false), - Entry("ValueFrom not nil", nil, &greenhousev1alpha1.PluginPresetPluginValueFromSource{Secret: &greenhousev1alpha1.SecretKeyReference{Name: "my-secret", Key: "secret-key"}}, false), + Entry("Value and ValueFrom nil", nil, nil, nil, true), + Entry("Value and ValueFrom not nil", test.MustReturnJSONFor("test"), &greenhousev1alpha1.PluginPresetPluginValueFromSource{Secret: &greenhousev1alpha1.SecretKeyReference{Name: "my-secret"}}, nil, true), + Entry("Value not nil", test.MustReturnJSONFor("test"), nil, nil, false), + Entry("ValueFrom not nil", nil, &greenhousev1alpha1.PluginPresetPluginValueFromSource{Secret: &greenhousev1alpha1.SecretKeyReference{Name: "my-secret", Key: "secret-key"}}, nil, false), + Entry("Expression only (valid)", nil, nil, utils.StringP(`"test-${global.greenhouse.clusterName}"`), false), + Entry("Expression and Value both set (invalid)", test.MustReturnJSONFor("test"), nil, utils.StringP(`"test-expression"`), true), + Entry("Expression and ValueFrom both set (invalid)", nil, &greenhousev1alpha1.PluginPresetPluginValueFromSource{Secret: &greenhousev1alpha1.SecretKeyReference{Name: "my-secret"}}, utils.StringP(`"test-expression"`), true), + Entry("All three set (invalid)", test.MustReturnJSONFor("test"), &greenhousev1alpha1.PluginPresetPluginValueFromSource{Secret: &greenhousev1alpha1.SecretKeyReference{Name: "my-secret"}}, utils.StringP(`"test-expression"`), true), ) - DescribeTable("Validate OptionValues in .Spec.ClusterOptionOverrides contain exactly one of Value, ValueFrom, or Expression", func(value *apiextensionsv1.JSON, valueFrom *greenhousev1alpha1.PluginPresetPluginValueFromSource, expErr bool) { + DescribeTable("Validate OptionValues in .Spec.ClusterOptionOverrides contain exactly one of Value, ValueFrom, or Expression", func(value *apiextensionsv1.JSON, valueFrom *greenhousev1alpha1.PluginPresetPluginValueFromSource, expression *string, expErr bool) { pluginPreset := &greenhousev1alpha1.PluginPreset{ TypeMeta: metav1.TypeMeta{ Kind: "PluginPreset", @@ -359,9 +368,10 @@ var _ = Describe("Validate Plugin OptionValues for PluginPreset", func() { ClusterName: "test-cluster", Overrides: []greenhousev1alpha1.PluginPresetPluginOptionValue{ { - Name: "test", - Value: value, - ValueFrom: valueFrom, + Name: "test", + Value: value, + ValueFrom: valueFrom, + Expression: expression, }, }, }, @@ -378,6 +388,9 @@ var _ = Describe("Validate Plugin OptionValues for PluginPreset", func() { case valueFrom != nil: defaultVal = test.MustReturnJSONFor(valueFrom.Secret.Name) optionType = greenhousev1alpha1.PluginOptionTypeSecret + case expression != nil: + defaultVal = test.MustReturnJSONFor("expression-default") + optionType = greenhousev1alpha1.PluginOptionTypeString } pluginDefinition := &greenhousev1alpha1.ClusterPluginDefinition{ @@ -404,10 +417,14 @@ var _ = Describe("Validate Plugin OptionValues for PluginPreset", func() { Expect(errList).To(BeEmpty(), "expected no error, got %v", errList) } }, - Entry("Value and ValueFrom nil", nil, nil, true), - Entry("Value and ValueFrom not nil", test.MustReturnJSONFor("test"), &greenhousev1alpha1.PluginPresetPluginValueFromSource{Secret: &greenhousev1alpha1.SecretKeyReference{Name: "my-secret"}}, true), - Entry("Value not nil", test.MustReturnJSONFor("test"), nil, false), - Entry("ValueFrom not nil", nil, &greenhousev1alpha1.PluginPresetPluginValueFromSource{Secret: &greenhousev1alpha1.SecretKeyReference{Name: "my-secret", Key: "secret-key"}}, false), + Entry("Value and ValueFrom nil", nil, nil, nil, true), + Entry("Value and ValueFrom not nil", test.MustReturnJSONFor("test"), &greenhousev1alpha1.PluginPresetPluginValueFromSource{Secret: &greenhousev1alpha1.SecretKeyReference{Name: "my-secret"}}, nil, true), + Entry("Value not nil", test.MustReturnJSONFor("test"), nil, nil, false), + Entry("ValueFrom not nil", nil, &greenhousev1alpha1.PluginPresetPluginValueFromSource{Secret: &greenhousev1alpha1.SecretKeyReference{Name: "my-secret", Key: "secret-key"}}, nil, false), + Entry("Expression only (valid)", nil, nil, utils.StringP(`"test-${global.greenhouse.clusterName}"`), false), + Entry("Expression and Value both set (invalid)", test.MustReturnJSONFor("test"), nil, utils.StringP(`"test-expression"`), true), + Entry("Expression and ValueFrom both set (invalid)", nil, &greenhousev1alpha1.PluginPresetPluginValueFromSource{Secret: &greenhousev1alpha1.SecretKeyReference{Name: "my-secret"}}, utils.StringP(`"test-expression"`), true), + Entry("All three set (invalid)", test.MustReturnJSONFor("test"), &greenhousev1alpha1.PluginPresetPluginValueFromSource{Secret: &greenhousev1alpha1.SecretKeyReference{Name: "my-secret"}}, utils.StringP(`"test-expression"`), true), ) DescribeTable("Validate WaitFor PluginRefs", func(waitForItems []greenhousev1alpha1.WaitForItem, expErr bool) {