Skip to content

feat(plugins): remove immutability of pluginDefinitionRef.Name#1941

Merged
IvoGoman merged 1 commit intomainfrom
fix/plugins/remove-pdref-name
Apr 23, 2026
Merged

feat(plugins): remove immutability of pluginDefinitionRef.Name#1941
IvoGoman merged 1 commit intomainfrom
fix/plugins/remove-pdref-name

Conversation

@IvoGoman
Copy link
Copy Markdown
Contributor

@IvoGoman IvoGoman commented Apr 23, 2026

with this change both Plugins & PluginPresets may allow to change
the name of the referenced PluginDefintion

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

@IvoGoman IvoGoman requested a review from a team as a code owner April 23, 2026 10:02
with this change both Plugins & PluginPresets may allow to change
the name of the referenced PluginDefintion
@IvoGoman IvoGoman force-pushed the fix/plugins/remove-pdref-name branch from 10f4c6b to dfeb6d8 Compare April 23, 2026 10:10
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 changes the admission behavior for Plugin and PluginPreset resources so that the referenced pluginDefinitionRef.name is no longer treated as immutable, enabling users to switch the referenced (Cluster-)PluginDefinition after creation.

Changes:

  • Removed webhook immutability validation for spec.pluginDefinitionRef.name on Plugin updates.
  • Removed webhook immutability validation for spec.plugin.pluginDefinitionRef.name on PluginPreset updates.
  • Updated webhook tests to expect pluginDefinition reference name changes to be accepted.

Reviewed changes

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

File Description
internal/webhook/v1alpha1/plugin_webhook.go Removes immutability enforcement for Plugin’s pluginDefinitionRef.name on update.
internal/webhook/v1alpha1/plugin_webhook_test.go Updates tests to ensure Plugin updates can change pluginDefinitionRef.name (including changing back).
internal/webhook/v1alpha1/pluginpreset_webhook.go Removes immutability enforcement for PluginPreset’s pluginDefinitionRef.name on update.
internal/webhook/v1alpha1/pluginpreset_webhook_test.go Updates tests to expect PluginPreset updates can change pluginDefinitionRef.name.
Comments suppressed due to low confidence (1)

internal/webhook/v1alpha1/pluginpreset_webhook.go:178

  • ValidateUpdatePluginPreset no longer enforces immutability of pluginDefinitionRef.name, but it also doesn't validate that the updated pluginDefinitionRef (name/kind) is set and refers to an existing (Cluster-)PluginDefinition, nor does it re-validate optionValues / clusterOptionOverrides against the new definition. This allows updating a PluginPreset to an invalid/non-existent PluginDefinition (or clearing the name entirely), which will likely break reconciliation. Consider reusing the same validation as ValidateCreatePluginPreset (or extracting shared validation) inside ValidateUpdatePluginPreset so updates still require a valid definition reference and valid option values.
func ValidateUpdatePluginPreset(ctx context.Context, c client.Client, oldPluginPreset, pluginPreset *greenhousev1alpha1.PluginPreset) (admission.Warnings, error) {
	var allErrs field.ErrorList
	var allWarns admission.Warnings

	if warn := webhook.ValidateLabelOwnedBy(ctx, c, pluginPreset); warn != "" {
		allWarns = append(allWarns, "PluginPreset should have a support-group Team set as its owner", warn)
	}

	if err := webhook.ValidateImmutableField(oldPluginPreset.Spec.Plugin.ClusterName, pluginPreset.Spec.Plugin.ClusterName, field.NewPath("spec", "plugin", "clusterName")); err != nil {
		allErrs = append(allErrs, err)
	}

	// validate WaitFor items are unique and that PluginRef's fields are mutually exclusive
	if errList := validateWaitForPluginRefs(pluginPreset.Spec.WaitFor, false); len(errList) > 0 {
		allErrs = append(allErrs, errList...)
	}

	if len(allErrs) > 0 {
		return allWarns, apierrors.NewInvalid(pluginPreset.GroupVersionKind().GroupKind(), pluginPreset.Name, allErrs)
	}
	return allWarns, nil

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

Comment thread internal/webhook/v1alpha1/pluginpreset_webhook_test.go
Copy link
Copy Markdown
Contributor

@uwe-mayer uwe-mayer left a comment

Choose a reason for hiding this comment

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

🚀
Fastest from discussion to PR ever?
TY!

@IvoGoman IvoGoman merged commit b5cb9ea into main Apr 23, 2026
24 checks passed
@IvoGoman IvoGoman deleted the fix/plugins/remove-pdref-name branch April 23, 2026 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEAT] - PluginDefinition reference of a PluginPreset can be reset on a running PP

4 participants