feat(pluginpresets): validate PluginDefinition existence in PluginPreset reconciler by IvoGoman · Pull Request #1862 · cloudoperators/greenhouse

IvoGoman · 2026-03-19T21:08:46Z

Description

Ensures e.g. changes bringing new PluginDefinition via Catalog does not break a HelmRelease if a PluginPreset using it comes with the same change

Removes strict existence validation of referenced (Cluster-)PluginDefinition and values in PluginPreset webhook. Webhook only does structural validation of PluginOptionValues & ClusterOptionOverrides.

Add early validation in PluginPreset reconciliation to check whether the
referenced PluginDefinition or ClusterPluginDefinition exists before
proceeding with plugin creation. When the definition is missing, the
ReadyCondition is set to false with an appropriate reason and the
reconciler requeues after 10 seconds.

Add PluginDefinitionNotExistsReason and ClusterPluginDefinitionNotExistsReason constants
Add pluginDefinitionExists helper method supporting both definition kinds
Preserve the not-exists ReadyCondition in computeReadyCondition
Clear stale not-exists condition once the definition becomes available

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

…set reconciler Ensures e.g. changes bringing new PluginDefinition via Catalog does not break a HelmRelease if a PluginPreset using it comes with the same change Removes strict existince validation of referenced (Cluster-)PluginDefinition and values in PluginPreset webhook. Webhook only does structural validation of PluginOptionValues & ClusterOptionOverrides. Add early validation in PluginPreset reconciliation to check whether the referenced PluginDefinition or ClusterPluginDefinition exists before proceeding with plugin creation. When the definition is missing, the ReadyCondition is set to false with an appropriate reason and the reconciler requeues after 10 seconds. - Add PluginDefinitionNotExistsReason and ClusterPluginDefinitionNotExistsReason constants - Add pluginDefinitionExists helper method supporting both definition kinds - Preserve the not-exists ReadyCondition in computeReadyCondition - Clear stale not-exists condition once the definition becomes available

Copilot

Pull request overview

This PR shifts (Cluster-)PluginDefinition existence validation from the PluginPreset admission webhook to the PluginPreset controller reconciliation path to avoid admission-time failures when presets and definitions are introduced together (e.g., via Catalog).

Changes:

Relax PluginPreset webhook validation to structural checks for option values (value/valueFrom/expression) instead of requiring referenced PluginDefinitions to exist at admission time.
Add early reconciliation-time validation for referenced PluginDefinition/ClusterPluginDefinition existence, setting ReadyCondition=false with a specific reason and requeueing.
Add/update unit and controller tests to cover the new admission and reconciliation behavior.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`internal/webhook/v1alpha1/pluginpreset_webhook_test.go`	Updates admission tests to accept missing definitions; refactors option-value validation tests to match new structural-only webhook behavior.
`internal/webhook/v1alpha1/pluginpreset_webhook.go`	Removes admission-time existence/type validation against PluginDefinitions; adds structural validation helper for option value sources.
`internal/controller/plugin/pluginpreset_controller.go`	Adds reconciliation-time existence checks for referenced definitions; preserves/clears “definition missing” ReadyCondition reasons.
`internal/controller/plugin/pluginpreset_controller_test.go`	Adds controller test verifying ReadyCondition behavior when a referenced ClusterPluginDefinition is initially missing then created.
`api/v1alpha1/pluginpreset_types.go`	Introduces new ReadyCondition reason constants for missing PluginDefinition/ClusterPluginDefinition.

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

Copilot · 2026-03-19T21:14:03Z

 		Entry("PluginOption ValueFrom is missing SecretReference Name", &greenhousev1alpha1.PluginValueFromSource{Secret: &greenhousev1alpha1.SecretKeyReference{Key: "key"}}, true),
 		Entry("PluginOption ValueFrom is missing SecretReference Key", &greenhousev1alpha1.PluginValueFromSource{Secret: &greenhousev1alpha1.SecretKeyReference{Name: "secret"}}, true),
-		Entry("PluginOption ValueFrom does not contain a SecretReference", nil, true),
+		Entry("PluginOption ValueFrom without a SecretReference is valid", &greenhousev1alpha1.PluginValueFromSource{}, false),


The test case treats valueFrom: {} as valid, but PluginValueFromSource has CRD XValidations requiring exactly one of secret or ref to be set. This should be an error (or the test should provide a valid ref/secret). Otherwise this test will diverge from (and fail under) API server validation.

Suggested change

Entry("PluginOption ValueFrom without a SecretReference is valid", &greenhousev1alpha1.PluginValueFromSource{}, false),

Entry("PluginOption ValueFrom without a SecretReference is invalid", &greenhousev1alpha1.PluginValueFromSource{}, true),

Copilot · 2026-03-19T21:14:03Z

 		Entry("PluginOption ValueFrom is missing SecretReference Name", &greenhousev1alpha1.PluginValueFromSource{Secret: &greenhousev1alpha1.SecretKeyReference{Key: "key"}}, true),
 		Entry("PluginOption ValueFrom is missing SecretReference Key", &greenhousev1alpha1.PluginValueFromSource{Secret: &greenhousev1alpha1.SecretKeyReference{Name: "secret"}}, true),
-		Entry("PluginOption ValueFrom does not contain a SecretReference", nil, true),
+		Entry("PluginOption ValueFrom without a SecretReference is valid", &greenhousev1alpha1.PluginValueFromSource{}, false),


Same issue as above: PluginValueFromSource{} violates the CRD XValidation rules (must set exactly one of secret or ref). This entry should expect an error or be changed to include a valid ref/secret.

Suggested change

Entry("PluginOption ValueFrom without a SecretReference is valid", &greenhousev1alpha1.PluginValueFromSource{}, false),

Entry("PluginOption ValueFrom without a SecretReference is invalid", &greenhousev1alpha1.PluginValueFromSource{}, true),

Copilot · 2026-03-19T21:14:04Z

+		// Validate that ValueFrom secret references are well-formed if a secret is referenced.
+		if val.ValueFrom != nil && val.ValueFrom.Secret != nil {
+			if val.ValueFrom.Secret.Name == "" {


validateOptionValuesStructure currently allows ValueFrom to be set with neither secret nor ref (it only validates secret when present). The API type PluginValueFromSource enforces “exactly one of secret or ref” via XValidation, so this helper should mirror that to keep webhook/unit-test validation consistent and to provide clearer field errors.

Copilot · 2026-03-19T21:14:04Z

+	// Check if the referenced PluginDefinition exists. If not, set ReadyCondition to false and return early.
+	pluginDefExists, err := r.pluginDefinitionExists(ctx, pluginPreset)
+	if err != nil {
+		return ctrl.Result{}, lifecycle.Failed, err


If pluginDefinitionExists returns an error, EnsureCreated returns early without setting any condition that computeReadyCondition uses to mark the resource not-ready; the status update can end up reporting Ready=true despite a failed reconcile. Consider setting an explicit ReadyCondition/other condition for lookup failures (and preserving it in computeReadyCondition) or handling the error by setting Ready=false and requeueing without returning an error.

Suggested change

return ctrl.Result{}, lifecycle.Failed, err

pluginPreset.SetCondition(greenhousemetav1alpha1.FalseCondition(

greenhousemetav1alpha1.ReadyCondition,

"PluginDefinitionLookupFailed",

fmt.Sprintf("Failed to look up %s %q: %v",

pluginPreset.Spec.Plugin.PluginDefinitionRef.Kind,

pluginPreset.Spec.Plugin.PluginDefinitionRef.Name,

err,

),

))

return ctrl.Result{RequeueAfter: 10 * time.Second}, lifecycle.Success, nil

Copilot · 2026-03-19T21:14:04Z

+		}
+		pluginPreset.SetCondition(greenhousemetav1alpha1.FalseCondition(greenhousemetav1alpha1.ReadyCondition, reason,
+			fmt.Sprintf("The referenced %s %q does not exist", pluginPreset.Spec.Plugin.PluginDefinitionRef.Kind, pluginPreset.Spec.Plugin.PluginDefinitionRef.Name)))
+		return ctrl.Result{RequeueAfter: 10 * time.Second}, lifecycle.Success, nil


Returning RequeueAfter: 10s on missing PluginDefinition will cause periodic reconciles for as long as the definition is absent. Since this controller already watches PluginDefinition/ClusterPluginDefinition and enqueues PluginPresets via the corresponding labels, consider relying on those watch events (or using a much longer/backoff requeue) to reduce steady-state load when a definition is missing long-term.

Suggested change

return ctrl.Result{RequeueAfter: 10 * time.Second}, lifecycle.Success, nil

// Do not requeue periodically; rely on PluginDefinition/ClusterPluginDefinition watch events to trigger reconciliation.

return ctrl.Result{}, lifecycle.Success, nil

IvoGoman requested a review from a team as a code owner March 19, 2026 21:08

IvoGoman requested a review from Copilot March 19, 2026 21:08

github-actions bot added feature core-apis size/XL labels Mar 19, 2026

Copilot started reviewing on behalf of IvoGoman March 19, 2026 21:09 View session

IvoGoman linked an issue Mar 19, 2026 that may be closed by this pull request

[FEAT] - Move existence check of (Cluster-)PluginDefinition for PluginPreset to reconciliation #1848

Open

3 tasks

Copilot AI reviewed Mar 19, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(pluginpresets): validate PluginDefinition existence in PluginPreset reconciler#1862

feat(pluginpresets): validate PluginDefinition existence in PluginPreset reconciler#1862
IvoGoman wants to merge 1 commit intomainfrom
feat/pluginpreset-definition-validation

IvoGoman commented Mar 19, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 19, 2026

Uh oh!

Copilot AI Mar 19, 2026

Uh oh!

Copilot AI Mar 19, 2026

Uh oh!

Copilot AI Mar 19, 2026

Uh oh!

Copilot AI Mar 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	Entry("PluginOption ValueFrom without a SecretReference is valid", &greenhousev1alpha1.PluginValueFromSource{}, false),
	Entry("PluginOption ValueFrom without a SecretReference is invalid", &greenhousev1alpha1.PluginValueFromSource{}, true),

	return ctrl.Result{RequeueAfter: 10 * time.Second}, lifecycle.Success, nil
	// Do not requeue periodically; rely on PluginDefinition/ClusterPluginDefinition watch events to trigger reconciliation.
	return ctrl.Result{}, lifecycle.Success, nil

Conversation

IvoGoman commented Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

IvoGoman commented Mar 19, 2026 •

edited

Loading