feat(pluginpresets): evaluate CEL expressions for owned Plugins (#1774) ver.2 by k-fabryczny · Pull Request #2012 · cloudoperators/greenhouse

k-fabryczny · 2026-05-21T09:13:23Z

feat(pluginpresets): evaluate CEL expressions for owned Plugins (#1774) ver.2

Description

What type of PR is this? (check all applicable)

Related Tickets & Documents

Added tests?

👍 yes
🙅 no, because they aren't needed
🙋 no, because I need help
Separate ticket for tests # (issue/pr)

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?

📜 README.md
🤝 Documentation pages updated
🙅 no documentation needed
(if applicable) generated OpenAPI docs for CRD changes

Checklist

My code follows the style guidelines of this project
I have performed a self-review of my code
I have commented my code, particularly in hard-to-understand areas
My changes generate no new warnings
New and existing unit tests pass locally with my changes

Copilot

Pull request overview

This PR moves CEL expression resolution for Plugins created via PluginPreset into the PluginPreset reconciliation flow, and introduces dedicated feature flags for enabling expression/integration behavior in the PluginPreset controller. It also updates the PluginPreset API shape (separating PluginPreset’s embedded plugin spec from PluginSpec) and regenerates CRDs/OpenAPI docs accordingly.

Changes:

Add expression resolution during PluginPreset reconciliation (apply overrides → evaluate CEL → write resolved .value into created Plugins).
Introduce pluginPreset feature-flag group and wire it into controller startup / Helm chart feature-flags ConfigMap.
Update PluginPreset API types and regenerate CRDs/OpenAPI docs; add controller/webhook test updates for the new API.

Reviewed changes

Copilot reviewed 29 out of 30 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
internal/webhook/v1alpha1/pluginpreset_webhook.go	Removes ClusterName validation/immutability checks after API reshaping.
internal/webhook/v1alpha1/pluginpreset_webhook_test.go	Updates admission tests for new PluginPreset plugin spec type; removes ClusterName-related cases.
internal/webhook/v1alpha1/plugin_webhook.go	Adds `staticcheck` suppression for deprecated expression field handling.
internal/webhook/v1alpha1/plugin_webhook_test.go	Adds `staticcheck` suppression in tests touching deprecated expression field.
internal/test/resources.go	Updates helper to populate `PluginPresetPluginSpec` fields from a `PluginSpec`.
internal/helm/helm.go	Adds `staticcheck` suppressions for deprecated option-value fields used in checksum calculation.
internal/helm/cel.go	Adds `staticcheck` suppression for deprecated expression-field usage in resolver.
internal/features/features.go	Adds `pluginPreset` feature group + getters for preset expression/integration flags.
internal/features/features_test.go	Adds unit tests for pluginPreset feature flags and independence from plugin flags.
internal/controller/plugin/suite_test.go	Enables preset expression/integration flags in controller test suite setup.
internal/controller/plugin/pluginpreset_values_resolver.go	New: resolves CEL expressions for PluginPreset option values and applies cluster overrides.
internal/controller/plugin/pluginpreset_controller.go	Wires override+expression resolution into PluginPreset reconciliation; maps preset plugin spec to PluginSpec.
internal/controller/plugin/pluginpreset_controller_test.go	Adds lifecycle tests verifying expression resolution results in `.value` only.
internal/controller/plugin/plugin_values_resolver.go	Adds `staticcheck` suppressions for deprecated `ValueFrom.Ref` usage.
internal/controller/plugin/plugin_controller_flux.go	Adds `staticcheck` suppression on deprecated expression/ref branches in release-value computation.
internal/cmd/plugin_template_test.go	Updates test fixtures for new `PluginPresetPluginSpec` type.
docs/reference/api/openapi.yaml	Updates generated OpenAPI: removes preset `clusterName`/`waitFor` from embedded spec; adds deprecation notes.
docs/reference/api/index.html	Updates generated API reference HTML for new PluginPreset types and deprecation notes.
dev-env/dev.values.yaml	Adds `global.pluginPreset.*` values for dev env.
cmd/greenhouse/controllers.go	Adds `startPluginPresetReconciler` to wire preset feature flags into the controller.
charts/manager/templates/manager/feature-flag.yaml	Adds `pluginPreset` feature-flag block to rendered ConfigMap.
charts/manager/templates/_helpers.tpl	Adds helper templates for `global.pluginPreset.*` values.
charts/manager/crds/greenhouse.sap_plugins.yaml	Adds deprecation notes for standalone Plugin `expression` / `valueFrom.ref`.
charts/manager/crds/greenhouse.sap_pluginpresets.yaml	Removes preset `clusterName`/`waitFor` from embedded spec; adds deprecation notes.
charts/manager/ci/test-values.yaml	Adds `global.pluginPreset.*` values for chart CI tests.
charts/greenhouse/values.yaml	Adds default `global.pluginPreset.*` values.
charts/greenhouse/ci/test-values.yaml	Adds `global.pluginPreset.*` values for chart CI tests.
api/v1alpha1/zz_generated.deepcopy.go	Regenerates deepcopy methods for new PluginPreset-related API types.
api/v1alpha1/pluginpreset_types.go	Introduces `PluginPresetPluginSpec` and related types; removes `clusterName` from preset plugin spec.
api/v1alpha1/plugin_types.go	Adds deprecation doc comments for standalone Plugin `Expression` and `ValueFrom.Ref`.

Files not reviewed (1)

api/v1alpha1/zz_generated.deepcopy.go: Language not supported

Comments suppressed due to low confidence (3)

internal/controller/plugin/pluginpreset_controller_test.go:980

This test assigns to the deprecated PluginOptionValue.Expression field without a //nolint:staticcheck suppression, which will likely trigger SA1019. Please add an inline suppression on this assignment (or migrate to a non-deprecated preset option value type).

				{
					Name:       "test.serviceHost",
					Expression: &expressionStr,
				},

internal/controller/plugin/pluginpreset_controller_test.go:1047

This test assigns to the deprecated PluginOptionValue.Expression field without a //nolint:staticcheck suppression, which will likely trigger SA1019. Please add an inline suppression on this assignment (or migrate to a non-deprecated preset option value type).

				{
					Name:       "expression.value",
					Expression: &expressionStr,
				},

internal/controller/plugin/pluginpreset_controller_test.go:1105

This test assigns to the deprecated PluginOptionValue.Expression field without a //nolint:staticcheck suppression, which will likely trigger SA1019. Please add an inline suppression on this assignment (or migrate to a non-deprecated preset option value type).

				{
					Name:       "test.invalid",
					Expression: &invalidExpressionStr,
				},

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

 // computeReleaseValues resolves Expressions and ValueFromRefs in the Plugin's option values
 // and inserts the Greenhouse values
 func computeReleaseValues(ctx context.Context, c client.Client, plugin *greenhousev1alpha1.Plugin, expressionEvaluation, integrationEnabled bool) ([]greenhousev1alpha1.PluginOptionValue, error) {
 	optionValues, err := helm.GetPluginOptionValuesForPlugin(ctx, c, plugin)
 	if err != nil {
 		return nil, err
 	}
 	trackedObjects := make([]string, 0)
 	// initialize CEL resolver
 	var celResolver *helm.CELResolver
 	if expressionEvaluation {
 		celResolver, err = helm.NewCELResolver(optionValues)
 		if err != nil {
 			return nil, fmt.Errorf("failed to initialize CEL resolver: %w", err)
 		}
 	}
 	for i, v := range optionValues {
 		switch {
 		case v.Value != nil:
 			// 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
 			if !expressionEvaluation {
 				// skip expression evaluation if not enabled
 				continue
 			}
 			resolvedOptionValue, err := celResolver.ResolveExpression(v, expressionEvaluation)
 			if err != nil {
 				return nil, err
 			}
 			optionValues[i] = *resolvedOptionValue



Copilot

Pull request overview

Copilot reviewed 31 out of 32 changed files in this pull request and generated 3 comments.

Files not reviewed (1)

api/v1alpha1/zz_generated.deepcopy.go: Language not supported

Comments suppressed due to low confidence (3)

internal/controller/plugin/pluginpreset_controller_test.go:980

PluginOptionValue.Expression is deprecated (SA1019). This struct literal assigns to Expression without a //nolint:staticcheck suppression. Add an inline suppression on this assignment (or use a helper that already suppresses SA1019).

				},
				{
					Name:       "test.serviceHost",
					Expression: &expressionStr,
				},

internal/controller/plugin/pluginpreset_controller_test.go:1047

PluginOptionValue.Expression is deprecated (SA1019). This struct literal assigns to Expression without a //nolint:staticcheck suppression. Add an inline suppression on this assignment (or use a helper that already suppresses SA1019).

				},
				{
					Name:       "expression.value",
					Expression: &expressionStr,
				},

internal/controller/plugin/pluginpreset_controller_test.go:1105

PluginOptionValue.Expression is deprecated (SA1019). This struct literal assigns to Expression without a //nolint:staticcheck suppression. Add an inline suppression on this assignment (or use a helper that already suppresses SA1019).

				},
				{
					Name:       "test.invalid",
					Expression: &invalidExpressionStr,
				},

+				},
+				{
+					Name:       "test.hostname",
+					Expression: &expressionStr,


Copilot

Pull request overview

Copilot reviewed 31 out of 32 changed files in this pull request and generated 1 comment.

Files not reviewed (1)

api/v1alpha1/zz_generated.deepcopy.go: Language not supported

Comments suppressed due to low confidence (3)

internal/controller/plugin/pluginpreset_controller_test.go:980

This test sets the deprecated PluginOptionValue.Expression field without a //nolint:staticcheck suppression. With staticcheck enabled, this usage can raise SA1019 and fail CI. Please add an inline suppression for this field assignment (or migrate tests to a non-deprecated preset-specific type when ready).

					Value: test.MustReturnJSONFor("myValue"),
				},
				{
					Name:       "test.serviceHost",
					Expression: &expressionStr,
				},

internal/controller/plugin/pluginpreset_controller_test.go:1047

This struct literal assigns to the deprecated PluginOptionValue.Expression field without //nolint:staticcheck. Since SA1019 is enabled, this can cause CI failures. Add an inline suppression on this assignment (or update tests to avoid the deprecated field).

				},
				{
					Name:       "expression.value",
					Expression: &expressionStr,
				},

internal/controller/plugin/pluginpreset_controller_test.go:1105

This struct literal assigns to deprecated PluginOptionValue.Expression without //nolint:staticcheck. With staticcheck enabled, this may raise SA1019 and fail CI. Add an inline suppression on this field assignment (or update tests to avoid using the deprecated field).

				},
				{
					Name:       "test.invalid",
					Expression: &invalidExpressionStr,
				},

Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>

Some fields (ClusterName, WaitFor) of the PluginSpec are not used by the PluginPreset With the upcoming changes of moving expression evaluation to the PluginPreset controller this will also effect the PluginOptionValues. This change brings a dedicated PluginSpec for PluginPresets and prepares the types for PluginOptionValues. The usage of the new PluginPresetPluginOptionValue will be implemented separately. Fields such as ClusterName, WaitFor are removed, both from types and webhook validation. Signed-off-by: Ivo Gosemann <ivo.gosemann@sap.com> Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>

Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>

… ver.2 Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>

Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>

…desToPreset Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>

Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>

k-fabryczny requested a review from a team as a code owner May 21, 2026 09:13

Copilot AI review requested due to automatic review settings May 21, 2026 09:13

github-actions Bot added size/XXL documentation Improvements or additions to documentation feature core-apis helm-charts labels May 21, 2026

Copilot started reviewing on behalf of k-fabryczny May 21, 2026 09:14 View session

Copilot AI reviewed May 21, 2026

View reviewed changes

k-fabryczny requested a review from Copilot May 25, 2026 11:15

Copilot started reviewing on behalf of k-fabryczny May 25, 2026 11:15 View session

Copilot AI reviewed May 25, 2026

View reviewed changes

Comment thread internal/features/features_test.go Outdated

Comment thread charts/manager/templates/manager/feature-flag.yaml Outdated

Comment thread internal/controller/plugin/pluginpreset_controller_test.go

},

{

Name: "test.hostname",

Expression: &expressionStr,

k-fabryczny requested a review from Copilot May 25, 2026 12:57

Copilot started reviewing on behalf of k-fabryczny May 25, 2026 12:58 View session

Copilot AI reviewed May 25, 2026

View reviewed changes

Comment thread internal/controller/plugin/pluginpreset_controller_test.go

k-fabryczny requested a review from a team as a code owner May 25, 2026 14:37

k-fabryczny and others added 12 commits May 26, 2026 12:17

feat(pluginpresets): add feature flag for pluginPreset

1850195

Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>

Automatic application of license header

5c5b38e

Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>

Automatic generation of CRD API Docs

2e0a3fb

Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>

add nolint:staticcheck // SA1019

3ded0fe

Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>

feat(pluginpresets): evaluate CEL expressions for owned Plugins (#1774)…

0ceb95a

… ver.2 Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>

fix linter warnings

f91355a

Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>

remove unused IntegrationEnabled flag until #1776

7f3a66e

Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>

remove dead overridesPluginOptionValues and add tests for applyOverri…

b4e905f

…desToPreset Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>

add TODO comments for Plugin expression/ref migration (#1775, #1776)

4c7e95c

Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>

remove integrationEnabled flag from tests and example

98ea326

Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>

docs(pluginpresets): document CEL expression evaluation for OptionValues

a32d0fa

Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>

k-fabryczny force-pushed the feat/v2-CEL-in-pp branch from 19c67dd to a32d0fa Compare May 26, 2026 10:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(pluginpresets): evaluate CEL expressions for owned Plugins (#1774) ver.2#2012

feat(pluginpresets): evaluate CEL expressions for owned Plugins (#1774) ver.2#2012
k-fabryczny wants to merge 12 commits into
mainfrom
feat/v2-CEL-in-pp

k-fabryczny commented May 21, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

k-fabryczny commented May 21, 2026

Description

What type of PR is this? (check all applicable)

Related Tickets & Documents

Added tests?

Added to documentation?

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants