feat(pluginpresets): pluginpreset controller resolves values from references#2117
Open
k-fabryczny wants to merge 44 commits into
Open
feat(pluginpresets): pluginpreset controller resolves values from references#2117k-fabryczny wants to merge 44 commits into
k-fabryczny wants to merge 44 commits into
Conversation
Closed
24 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the PluginPreset controller to resolve valueFrom.ref references (to other PluginPresets) into concrete spec.optionValues[*].value entries on the generated Plugins, controlled via a new pluginPreset.integrationEnabled feature flag. It also updates Helm chart feature-flag wiring, adds extensive controller/e2e tests, and expands the PluginPreset documentation to cover cross-preset references.
Changes:
- Add
pluginPreset.integrationEnabledfeature flag plumbed through features, controller wiring, and Helm charts. - Implement
valueFrom.refresolution in the PluginPreset controller (including selector-based fan-out and CEL extraction). - Add/extend unit and E2E scenarios and documentation for cross-PluginPreset references.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/features/features.go | Adds IsPresetIntegrationEnabled() and supporting struct field. |
| internal/features/features_test.go | Extends feature-flag tests to cover preset integration flag. |
| internal/controller/plugin/suite_test.go | Enables preset integration in controller test suite setup. |
| internal/controller/plugin/pluginpreset_values_resolver.go | Implements reference resolution and CEL evaluation for valueFrom.ref. |
| internal/controller/plugin/pluginpreset_controller.go | Adds IntegrationEnabled flag to reconciler struct. |
| internal/controller/plugin/pluginpreset_controller_test.go | Adds multiple integration tests for cross-preset references. |
| e2e/pluginpreset/scenarios/selector_reference.go | New E2E scenario: selector-based cross-preset reference. |
| e2e/pluginpreset/scenarios/cross_preset_reference.go | New E2E scenario: name-based cross-preset reference. |
| e2e/pluginpreset/scenarios/cross_preset_reference_overrides.go | New E2E scenario: references respect source ClusterOptionOverrides. |
| e2e/pluginpreset/scenarios/constants.go | Adds constants for new E2E scenarios. |
| e2e/pluginpreset/e2e_test.go | Registers new PluginPreset reference E2E scenarios. |
| docs/reference/components/pluginpreset.md | Documents valueFrom.ref usage, syntax, and feature flags. |
| dev-env/dev.values.yaml | Enables preset integration in dev values. |
| cmd/greenhouse/controllers.go | Wires IsPresetIntegrationEnabled() into PluginPreset reconciler. |
| charts/manager/templates/manager/feature-flag.yaml | Adds template docs + rendered value for preset integration flag. |
| charts/manager/templates/_helpers.tpl | Adds helper for pluginPreset.integrationEnabled. |
| charts/greenhouse/values.yaml | Adds default global.pluginPreset.integrationEnabled: false. |
| charts/greenhouse/ci/test-values.yaml | Enables preset integration for chart CI values. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+44
to
46
| } else { | ||
| resolvedValues = util.ConvertToPluginOptionValues(preset.Spec.Plugin.OptionValues) | ||
| } |
Comment on lines
38
to
+56
| if r.ExpressionEvaluationEnabled { | ||
| return r.resolveExpressionsForPreset(ctx, preset, cluster) | ||
| var err error | ||
| resolvedValues, err = r.resolveExpressionsForPreset(ctx, preset, cluster) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to resolve expressions: %w", err) | ||
| } | ||
| } else { | ||
| resolvedValues = util.ConvertToPluginOptionValues(preset.Spec.Plugin.OptionValues) | ||
| } | ||
|
|
||
| for _, ov := range preset.Spec.Plugin.OptionValues { | ||
| if ov.Expression != nil { | ||
| return nil, fmt.Errorf("option %s has expression but expressionEvaluationEnabled is disabled for PluginPreset controller", ov.Name) | ||
| if r.IntegrationEnabled { | ||
| var err error | ||
| resolvedValues, err = r.resolveReferencesForPreset(ctx, cluster, preset.Namespace, preset.Spec.Plugin.OptionValues, resolvedValues) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to resolve references: %w", err) | ||
| } | ||
| } | ||
|
|
||
| return util.ConvertToPluginOptionValues(preset.Spec.Plugin.OptionValues), nil | ||
| return resolvedValues, nil |
Comment on lines
+177
to
+179
| > :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. |
Comment on lines
+345
to
+352
| #### New simplified syntax | ||
| ```expression: spec.optionValues.filter(v, v.name == "my.value")[0].value``` | ||
|
|
||
| #### With ${...} wrapper | ||
| ```expression: ${spec.optionValues.filter(v, v.name == "my.value")[0].value}``` | ||
|
|
||
| #### Legacy syntax (backward compatible) | ||
| ```expression: object.spec.optionValues.filter(v, v.name == "my.value")[0].value``` |
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
…esetToPluginOptionValues Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
…, fix tests Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
k-fabryczny
force-pushed
the
feat/v4-CEL-in-pp-1776
branch
from
July 20, 2026 14:44
965b705 to
05fcb76
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
What type of PR is this? (check all applicable)
Related Tickets & Documents
#1776
Added tests?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Added to documentation?
Checklist