Skip to content

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

Open
k-fabryczny wants to merge 19 commits into
mainfrom
feat/v2-CEL-in-pp
Open

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

Conversation

@k-fabryczny

@k-fabryczny k-fabryczny commented May 21, 2026

Copy link
Copy Markdown
Contributor

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

Description

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

  • 🍕 Feature
  • 🐛 Bug Fix
  • 📝 Documentation Update
  • 🎨 Style
  • 🧑‍💻 Code Refactor
  • 🔥 Performance Improvements
  • ✅ Test
  • 🤖 Build
  • 🔁 CI
  • 📦 Chore (Release)
  • ⏩ Revert

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

@k-fabryczny 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 github-actions Bot added size/XXL documentation Improvements or additions to documentation feature core-apis helm-charts labels May 21, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread internal/controller/plugin/pluginpreset_controller.go
Comment thread internal/controller/plugin/pluginpreset_controller.go
Comment thread internal/controller/plugin/plugin_controller_flux.go
Comment thread api/v1alpha1/pluginpreset_types.go
Comment thread internal/test/resources.go
Comment thread internal/controller/plugin/pluginpreset_values_resolver.go
Comment thread internal/controller/plugin/pluginpreset_controller_test.go

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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,
				},

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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,
				},

Comment thread internal/controller/plugin/pluginpreset_controller_test.go
@k-fabryczny k-fabryczny requested a review from a team as a code owner May 25, 2026 14:37
@github-actions github-actions Bot added size/L and removed size/XXL labels May 29, 2026
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
@github-actions github-actions Bot added size/XL and removed size/XXL labels Jun 1, 2026
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
@uwe-mayer uwe-mayer linked an issue Jun 2, 2026 that may be closed by this pull request
2 tasks
@uwe-mayer uwe-mayer changed the title feat(pluginpresets): evaluate CEL expressions for owned Plugins (#1774) ver.2 feat(pluginpresets): evaluate CEL expressions for owned Plugins (#1774) Jun 2, 2026
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>
@github-actions github-actions Bot added size/XXL and removed size/XL labels Jun 10, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 25 out of 27 changed files in this pull request and generated 4 comments.

Files not reviewed (1)
  • api/v1alpha1/zz_generated.deepcopy.go: Language not supported

Comment thread internal/controller/plugin/pluginpreset_controller.go
Comment thread internal/controller/plugin/pluginpreset_values_resolver.go
Comment thread internal/webhook/v1alpha1/pluginpreset_webhook.go
Comment thread internal/cmd/plugin_template.go
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 25 out of 27 changed files in this pull request and generated 2 comments.

Files not reviewed (1)
  • api/v1alpha1/zz_generated.deepcopy.go: Language not supported


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) {
)

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) {
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 25 out of 27 changed files in this pull request and generated 1 comment.

Files not reviewed (1)
  • api/v1alpha1/zz_generated.deepcopy.go: Language not supported

Comment thread internal/controller/plugin/plugin_controller_flux.go
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 25 out of 27 changed files in this pull request and generated 1 comment.

Files not reviewed (1)
  • api/v1alpha1/zz_generated.deepcopy.go: Language not supported

Comment thread internal/controller/plugin/plugin_controller_flux.go
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 25 out of 27 changed files in this pull request and generated 2 comments.

Files not reviewed (1)
  • api/v1alpha1/zz_generated.deepcopy.go: Language not supported

Comment on lines 271 to 272
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{
Comment on lines 339 to 340
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{
Signed-off-by: Klaudiusz Fabryczny <klaudiusz.fabryczny@sap.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core-apis documentation Improvements or additions to documentation feature helm-charts size/XXL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEAT] - PluginPreset controller evaluates CEL expressions for owned Plugins

3 participants