From f56253233479d127ec2bb9a2c61c73c361467d71 Mon Sep 17 00:00:00 2001 From: Ivo Gosemann Date: Wed, 20 May 2026 11:54:59 +0200 Subject: [PATCH 1/2] refactor(pluginpreset): create a dedicated PluginSpec 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 --- .golangci.yaml | 4 + api/v1alpha1/plugin_types.go | 6 + api/v1alpha1/pluginpreset_types.go | 63 ++++- api/v1alpha1/zz_generated.deepcopy.go | 85 ++++++ .../crds/greenhouse.sap_pluginpresets.yaml | 57 ++-- .../manager/crds/greenhouse.sap_plugins.yaml | 16 +- docs/reference/api/index.html | 243 +++++++++++++++++- internal/cmd/plugin_template_test.go | 8 +- .../plugin/pluginpreset_controller.go | 23 +- .../plugin/pluginpreset_controller_test.go | 6 +- internal/test/resources.go | 8 +- .../webhook/v1alpha1/pluginpreset_webhook.go | 9 - .../v1alpha1/pluginpreset_webhook_test.go | 40 +-- 13 files changed, 464 insertions(+), 104 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 900902415..5615edef8 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -208,6 +208,10 @@ linters: - linters: - goconst path: (.+)_test\.go + # Deprecated fields on v1alpha1.PluginOptionValue / PluginValueFromSource are intentionally + # still referenced during the deprecation window (see api/v1alpha1/plugin_types.go). + - linters: [staticcheck] + text: 'SA1019: .*\.(Expression|Ref) is deprecated' paths: - third_party$ - builtin$ diff --git a/api/v1alpha1/plugin_types.go b/api/v1alpha1/plugin_types.go index 0e690c4f4..f0e8c5f32 100644 --- a/api/v1alpha1/plugin_types.go +++ b/api/v1alpha1/plugin_types.go @@ -66,6 +66,9 @@ type PluginOptionValue struct { // ValueFrom references value in another source. ValueFrom *PluginValueFromSource `json:"valueFrom,omitempty"` // Expression is a YAML string with ${...} placeholders that will be evaluated as CEL expressions. + // + // Deprecated: Expression is deprecated on standalone Plugins and will be removed in a future release. + // Consider using a PluginPreset to deploy Plugins utilizing the Expression field. Expression *string `json:"expression,omitempty"` } @@ -77,6 +80,9 @@ type PluginValueFromSource struct { // Secret references the v1.Secret containing the value that needs to be extracted Secret *SecretKeyReference `json:"secret,omitempty"` // Ref references values defined in another resource (Plugin, PluginPreset) + // + // Deprecated: Ref is deprecated on standalone Plugins and will be removed in a future release. + // Consider using a PluginPreset to deploy Plugins utilizing the Ref field. Ref *ExternalValueSource `json:"ref,omitempty"` } diff --git a/api/v1alpha1/pluginpreset_types.go b/api/v1alpha1/pluginpreset_types.go index 80730076b..579072700 100644 --- a/api/v1alpha1/pluginpreset_types.go +++ b/api/v1alpha1/pluginpreset_types.go @@ -6,6 +6,7 @@ package v1alpha1 import ( "slices" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" greenhousemetav1alpha1 "github.com/cloudoperators/greenhouse/api/meta/v1alpha1" @@ -30,7 +31,7 @@ const ( type PluginPresetSpec struct { // PluginSpec is the spec of the plugin to be deployed by the PluginPreset. - Plugin PluginSpec `json:"plugin"` + Plugin PluginPresetPluginSpec `json:"plugin"` // ClusterSelector is a label selector to select the clusters the plugin bundle should be deployed to. ClusterSelector metav1.LabelSelector `json:"clusterSelector"` @@ -50,6 +51,66 @@ type PluginPresetSpec struct { DeletionPolicy string `json:"deletionPolicy,omitempty"` } +// PluginPresetPluginSpec defines the desired state of Plugin +type PluginPresetPluginSpec struct { + // PluginDefinitionRef is the reference to the (Cluster-)PluginDefinition. + PluginDefinitionRef PluginDefinitionReference `json:"pluginDefinitionRef"` + + // DisplayName is an optional name for the Plugin to be displayed in the Greenhouse UI. + // This is especially helpful to distinguish multiple instances of a PluginDefinition in the same context. + // Defaults to a normalized version of metadata.name. + DisplayName string `json:"displayName,omitempty"` + + // Values are the values for a PluginDefinition instance. + OptionValues []PluginOptionValue `json:"optionValues,omitempty"` + + // ReleaseNamespace is the namespace in the remote cluster to which the backend is deployed. + // Defaults to the Greenhouse managed namespace if not set. + ReleaseNamespace string `json:"releaseNamespace,omitempty"` + + // ReleaseName is the name of the helm release in the remote cluster to which the backend is deployed. + // If the Plugin was already deployed, the Plugin's name is used as the release name. + // If this Plugin is newly created, the releaseName is defaulted to the PluginDefinitions HelmChart name. + // +Optional + // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="ReleaseName is immutable" + // +kubebuilder:validation:MaxLength=53 + ReleaseName string `json:"releaseName,omitempty"` + + // DeletionPolicy defines how Helm Releases created by a Plugin are handled upon deletion of the Plugin. + // Supported values are "Delete" and "Retain". If not set, defaults to "Delete". + // +Optional + // +kubebuilder:default=Delete + // +kubebuilder:validation:Enum=Delete;Retain + DeletionPolicy string `json:"deletionPolicy,omitempty"` + + // IgnoreDifferences defines paths to ignore when detecting drift between desired and actual state. + // +Optional + IgnoreDifferences []IgnoreDifference `json:"ignoreDifferences,omitempty"` +} + +// PluginPresetPluginOptionValue is the value for a PluginOption. +type PluginPresetPluginOptionValue struct { + // Name of the values. + Name string `json:"name"` + // Value is the actual value in plain text. + Value *apiextensionsv1.JSON `json:"value,omitempty"` + // ValueFrom references value in another source. + ValueFrom *PluginPresetPluginValueFromSource `json:"valueFrom,omitempty"` + // Expression is a YAML string with ${...} placeholders that will be evaluated as CEL expressions. + Expression *string `json:"expression,omitempty"` +} + +// PluginPresetPluginValueFromSource defines how to extract dynamic values +// only one of secret or ref can be set +// +kubebuilder:validation:XValidation:rule="!(has(self.secret) && has(self.ref))",message="both secret and ref cannot be set" +// +kubebuilder:validation:XValidation:rule="has(self.secret) || has(self.ref)",message="one of secret or ref must be set" +type PluginPresetPluginValueFromSource struct { + // Secret references the v1.Secret containing the value that needs to be extracted + Secret *SecretKeyReference `json:"secret,omitempty"` + // Ref references values defined in another resource (Plugin, PluginPreset) + Ref *ExternalValueSource `json:"ref,omitempty"` +} + // ClusterOptionOverride defines which plugin option should be override in which cluster // +Optional type ClusterOptionOverride struct { diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 9a6a8fb42..f7b889c7a 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -1288,6 +1288,91 @@ func (in *PluginPresetList) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PluginPresetPluginOptionValue) DeepCopyInto(out *PluginPresetPluginOptionValue) { + *out = *in + if in.Value != nil { + in, out := &in.Value, &out.Value + *out = new(apiextensionsv1.JSON) + (*in).DeepCopyInto(*out) + } + if in.ValueFrom != nil { + in, out := &in.ValueFrom, &out.ValueFrom + *out = new(PluginPresetPluginValueFromSource) + (*in).DeepCopyInto(*out) + } + if in.Expression != nil { + in, out := &in.Expression, &out.Expression + *out = new(string) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PluginPresetPluginOptionValue. +func (in *PluginPresetPluginOptionValue) DeepCopy() *PluginPresetPluginOptionValue { + if in == nil { + return nil + } + out := new(PluginPresetPluginOptionValue) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PluginPresetPluginSpec) DeepCopyInto(out *PluginPresetPluginSpec) { + *out = *in + out.PluginDefinitionRef = in.PluginDefinitionRef + if in.OptionValues != nil { + in, out := &in.OptionValues, &out.OptionValues + *out = make([]PluginOptionValue, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + if in.IgnoreDifferences != nil { + in, out := &in.IgnoreDifferences, &out.IgnoreDifferences + *out = make([]IgnoreDifference, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PluginPresetPluginSpec. +func (in *PluginPresetPluginSpec) DeepCopy() *PluginPresetPluginSpec { + if in == nil { + return nil + } + out := new(PluginPresetPluginSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PluginPresetPluginValueFromSource) DeepCopyInto(out *PluginPresetPluginValueFromSource) { + *out = *in + if in.Secret != nil { + in, out := &in.Secret, &out.Secret + *out = new(SecretKeyReference) + **out = **in + } + if in.Ref != nil { + in, out := &in.Ref, &out.Ref + *out = new(ExternalValueSource) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PluginPresetPluginValueFromSource. +func (in *PluginPresetPluginValueFromSource) DeepCopy() *PluginPresetPluginValueFromSource { + if in == nil { + return nil + } + out := new(PluginPresetPluginValueFromSource) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PluginPresetSpec) DeepCopyInto(out *PluginPresetSpec) { *out = *in diff --git a/charts/manager/crds/greenhouse.sap_pluginpresets.yaml b/charts/manager/crds/greenhouse.sap_pluginpresets.yaml index 7d2cecd30..f18c3f5f5 100644 --- a/charts/manager/crds/greenhouse.sap_pluginpresets.yaml +++ b/charts/manager/crds/greenhouse.sap_pluginpresets.yaml @@ -6,7 +6,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.20.1 + controller-gen.kubebuilder.io/version: v0.20.0 name: pluginpresets.greenhouse.sap spec: group: greenhouse.sap @@ -71,8 +71,11 @@ spec: description: PluginOptionValue is the value for a PluginOption. properties: expression: - description: Expression is a YAML string with ${...} placeholders - that will be evaluated as CEL expressions. + description: |- + Expression is a YAML string with ${...} placeholders that will be evaluated as CEL expressions. + + Deprecated: Expression is deprecated on standalone Plugins and will be removed in a future release. + Consider using a PluginPreset to deploy Plugins utilizing the Expression field. type: string name: description: Name of the values. @@ -84,8 +87,11 @@ spec: description: ValueFrom references value in another source. properties: ref: - description: Ref references values defined in another - resource (Plugin, PluginPreset) + description: |- + Ref references values defined in another resource (Plugin, PluginPreset) + + Deprecated: Ref is deprecated on standalone Plugins and will be removed in a future release. + Consider using a PluginPreset to deploy Plugins utilizing the Ref field. properties: expression: description: Expression is a CEL expression to @@ -252,11 +258,6 @@ spec: description: PluginSpec is the spec of the plugin to be deployed by the PluginPreset. properties: - clusterName: - description: ClusterName is the name of the cluster the plugin - is deployed to. If not set, the plugin is deployed to the greenhouse - cluster. - type: string deletionPolicy: default: Delete description: |- @@ -309,8 +310,11 @@ spec: description: PluginOptionValue is the value for a PluginOption. properties: expression: - description: Expression is a YAML string with ${...} placeholders - that will be evaluated as CEL expressions. + description: |- + Expression is a YAML string with ${...} placeholders that will be evaluated as CEL expressions. + + Deprecated: Expression is deprecated on standalone Plugins and will be removed in a future release. + Consider using a PluginPreset to deploy Plugins utilizing the Expression field. type: string name: description: Name of the values. @@ -322,8 +326,11 @@ spec: description: ValueFrom references value in another source. properties: ref: - description: Ref references values defined in another - resource (Plugin, PluginPreset) + description: |- + Ref references values defined in another resource (Plugin, PluginPreset) + + Deprecated: Ref is deprecated on standalone Plugins and will be removed in a future release. + Consider using a PluginPreset to deploy Plugins utilizing the Ref field. properties: expression: description: Expression is a CEL expression to extract @@ -455,28 +462,6 @@ spec: ReleaseNamespace is the namespace in the remote cluster to which the backend is deployed. Defaults to the Greenhouse managed namespace if not set. type: string - waitFor: - description: WaitFor defines other Plugins to wait for before - installing this Plugin. - items: - description: WaitForItem is a wrapper around PluginRef to add - context for every WaitFor list item. - properties: - pluginRef: - description: PluginRef defines a reference to the Plugin. - properties: - name: - description: Name of the Plugin. - type: string - pluginPreset: - description: PluginPreset is the name of the PluginPreset - which creates the Plugin. - type: string - type: object - required: - - pluginRef - type: object - type: array required: - pluginDefinitionRef type: object diff --git a/charts/manager/crds/greenhouse.sap_plugins.yaml b/charts/manager/crds/greenhouse.sap_plugins.yaml index e016b0476..22c7c8835 100644 --- a/charts/manager/crds/greenhouse.sap_plugins.yaml +++ b/charts/manager/crds/greenhouse.sap_plugins.yaml @@ -6,7 +6,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.20.1 + controller-gen.kubebuilder.io/version: v0.20.0 name: plugins.greenhouse.sap spec: group: greenhouse.sap @@ -124,8 +124,11 @@ spec: description: PluginOptionValue is the value for a PluginOption. properties: expression: - description: Expression is a YAML string with ${...} placeholders - that will be evaluated as CEL expressions. + description: |- + Expression is a YAML string with ${...} placeholders that will be evaluated as CEL expressions. + + Deprecated: Expression is deprecated on standalone Plugins and will be removed in a future release. + Consider using a PluginPreset to deploy Plugins utilizing the Expression field. type: string name: description: Name of the values. @@ -137,8 +140,11 @@ spec: description: ValueFrom references value in another source. properties: ref: - description: Ref references values defined in another resource - (Plugin, PluginPreset) + description: |- + Ref references values defined in another resource (Plugin, PluginPreset) + + Deprecated: Ref is deprecated on standalone Plugins and will be removed in a future release. + Consider using a PluginPreset to deploy Plugins utilizing the Ref field. properties: expression: description: Expression is a CEL expression to extract diff --git a/docs/reference/api/index.html b/docs/reference/api/index.html index c0d619379..67f0c179c 100644 --- a/docs/reference/api/index.html +++ b/docs/reference/api/index.html @@ -1445,6 +1445,7 @@

ExternalValueSource

(Appears on: +PluginPresetPluginValueFromSource, PluginValueFromSource)

ExternalValueSource defines how to extract values from external resources

@@ -1700,6 +1701,7 @@

IgnoreDifference

(Appears on: +PluginPresetPluginSpec, PluginSpec)

IgnoreDifference defines a set of paths to ignore for matching resources.

@@ -2719,6 +2721,7 @@

PluginDefinitionRefer

(Appears on: +PluginPresetPluginSpec, PluginSpec)

PluginDefinitionReference defines the reference to the PluginDefinition or ClusterPluginDefinition.

@@ -3064,6 +3067,7 @@

PluginOptionValue

(Appears on: ClusterOptionOverride, +PluginPresetPluginSpec, PluginSpec)

PluginOptionValue is the value for a PluginOption.

@@ -3123,6 +3127,8 @@

PluginOptionValue

Expression is a YAML string with ${…} placeholders that will be evaluated as CEL expressions.

+

Deprecated: Expression is deprecated on standalone Plugins and will be removed in a future release. +Consider using a PluginPreset to deploy Plugins utilizing the Expression field.

@@ -3173,8 +3179,8 @@

PluginPreset plugin
- -PluginSpec + +PluginPresetPluginSpec @@ -3252,6 +3258,229 @@

PluginPreset +

PluginPresetPluginOptionValue +

+

PluginPresetPluginOptionValue is the value for a PluginOption.

+
+
+ + + + + + + + + + + + + + + + + + + + + + + + + +
FieldDescription
+name
+ +string + +		 +

Name of the values.

+
+value
+ + +Kubernetes apiextensions/v1.JSON + + +		 +

Value is the actual value in plain text.

+
+valueFrom
+ + +PluginPresetPluginValueFromSource + + +		 +

ValueFrom references value in another source.

+
+expression
+ +string + +		 +

Expression is a YAML string with ${…} placeholders that will be evaluated as CEL expressions.

+
+
+
+

PluginPresetPluginSpec +

+

+(Appears on: +PluginPresetSpec) +

+

PluginPresetPluginSpec defines the desired state of Plugin

+
+
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
FieldDescription
+pluginDefinitionRef
+ + +PluginDefinitionReference + + +		 +

PluginDefinitionRef is the reference to the (Cluster-)PluginDefinition.

+
+displayName
+ +string + +		 +

DisplayName is an optional name for the Plugin to be displayed in the Greenhouse UI. +This is especially helpful to distinguish multiple instances of a PluginDefinition in the same context. +Defaults to a normalized version of metadata.name.

+
+optionValues
+ + +[]PluginOptionValue + + +		 +

Values are the values for a PluginDefinition instance.

+
+releaseNamespace
+ +string + +		 +

ReleaseNamespace is the namespace in the remote cluster to which the backend is deployed. +Defaults to the Greenhouse managed namespace if not set.

+
+releaseName
+ +string + +		 +

ReleaseName is the name of the helm release in the remote cluster to which the backend is deployed. +If the Plugin was already deployed, the Plugin’s name is used as the release name. +If this Plugin is newly created, the releaseName is defaulted to the PluginDefinitions HelmChart name.

+
+deletionPolicy
+ +string + +		 +

DeletionPolicy defines how Helm Releases created by a Plugin are handled upon deletion of the Plugin. +Supported values are “Delete” and “Retain”. If not set, defaults to “Delete”.

+
+ignoreDifferences
+ + +[]IgnoreDifference + + +		 +

IgnoreDifferences defines paths to ignore when detecting drift between desired and actual state.

+
+
+
+

PluginPresetPluginValueFromSource +

+

+(Appears on: +PluginPresetPluginOptionValue) +

+

PluginPresetPluginValueFromSource defines how to extract dynamic values +only one of secret or ref can be set

+
+
+ + + + + + + + + + + + + + + + + +
FieldDescription
+secret
+ + +SecretKeyReference + + +		 +

Secret references the v1.Secret containing the value that needs to be extracted

+
+ref
+ + +ExternalValueSource + + +		 +

Ref references values defined in another resource (Plugin, PluginPreset)

+
+
+

PluginPresetSpec

@@ -3273,8 +3502,8 @@

PluginPresetSpec plugin
- -PluginSpec + +PluginPresetPluginSpec @@ -3464,8 +3693,7 @@

PluginSpec

(Appears on: -Plugin, -PluginPresetSpec) +Plugin)

PluginSpec defines the desired state of Plugin

@@ -3794,6 +4022,8 @@

PluginValueFromSource

Ref references values defined in another resource (Plugin, PluginPreset)

+

Deprecated: Ref is deprecated on standalone Plugins and will be removed in a future release. +Consider using a PluginPreset to deploy Plugins utilizing the Ref field.

@@ -3915,6 +4145,7 @@

SecretKeyReference

(Appears on: OIDCConfig, +PluginPresetPluginValueFromSource, PluginValueFromSource, ValueFromSource)

diff --git a/internal/cmd/plugin_template_test.go b/internal/cmd/plugin_template_test.go index 353428b9b..5ae43d9c0 100644 --- a/internal/cmd/plugin_template_test.go +++ b/internal/cmd/plugin_template_test.go @@ -25,7 +25,7 @@ var _ = Describe("validateCompatibility", func() { pluginPreset: &greenhousev1alpha1.PluginPreset{ TypeMeta: metav1.TypeMeta{Kind: PluginPresetKind}, Spec: greenhousev1alpha1.PluginPresetSpec{ - Plugin: greenhousev1alpha1.PluginSpec{ + Plugin: greenhousev1alpha1.PluginPresetPluginSpec{ PluginDefinitionRef: greenhousev1alpha1.PluginDefinitionReference{ Name: "apache", Kind: greenhousev1alpha1.ClusterPluginDefinitionKind, @@ -56,7 +56,7 @@ var _ = Describe("validateCompatibility", func() { pluginPreset: &greenhousev1alpha1.PluginPreset{ TypeMeta: metav1.TypeMeta{Kind: PluginPresetKind}, Spec: greenhousev1alpha1.PluginPresetSpec{ - Plugin: greenhousev1alpha1.PluginSpec{ + Plugin: greenhousev1alpha1.PluginPresetPluginSpec{ PluginDefinitionRef: greenhousev1alpha1.PluginDefinitionReference{ Name: "nginx", Kind: greenhousev1alpha1.ClusterPluginDefinitionKind, @@ -92,7 +92,7 @@ var _ = Describe("validateCompatibility", func() { pluginPreset: &greenhousev1alpha1.PluginPreset{ TypeMeta: metav1.TypeMeta{Kind: PluginPresetKind}, Spec: greenhousev1alpha1.PluginPresetSpec{ - Plugin: greenhousev1alpha1.PluginSpec{ + Plugin: greenhousev1alpha1.PluginPresetPluginSpec{ PluginDefinitionRef: greenhousev1alpha1.PluginDefinitionReference{ Name: "nginx", Kind: greenhousev1alpha1.ClusterPluginDefinitionKind, @@ -142,7 +142,7 @@ var _ = Describe("prepareValues", func() { Namespace: "test-org", }, Spec: greenhousev1alpha1.PluginPresetSpec{ - Plugin: greenhousev1alpha1.PluginSpec{ + Plugin: greenhousev1alpha1.PluginPresetPluginSpec{ PluginDefinitionRef: greenhousev1alpha1.PluginDefinitionReference{ Name: "nginx", Kind: greenhousev1alpha1.ClusterPluginDefinitionKind, diff --git a/internal/controller/plugin/pluginpreset_controller.go b/internal/controller/plugin/pluginpreset_controller.go index b78058582..aab4f0406 100644 --- a/internal/controller/plugin/pluginpreset_controller.go +++ b/internal/controller/plugin/pluginpreset_controller.go @@ -238,12 +238,8 @@ func (r *PluginPresetReconciler) reconcilePluginPreset(ctx context.Context, pres releaseName := getReleaseName(plugin, preset) - plugin.Spec = preset.Spec.Plugin + plugin.Spec = pluginSpecFromPluginPreset(preset, cluster.GetName()) plugin.Spec.ReleaseName = releaseName - // Set the cluster name to the name of the cluster. The PluginSpec contained in the PluginPreset does not have a cluster name. - plugin.Spec.ClusterName = cluster.GetName() - // Copy over the plugin dependencies - plugin.Spec.WaitFor = preset.Spec.WaitFor // transport plugin preset labels to plugin plugin = (lifecycle.NewPropagator(preset, plugin).Apply()).(*greenhousev1alpha1.Plugin) // overrides options based on preset definition @@ -512,3 +508,20 @@ func isPluginDefinitionNotFoundError(err error) bool { } return false } + +// pluginSpecFromPluginPreset returns a PluginSpec based on the given PluginPreset.Spec.Plugin. +// This maps all fields that can be copied 1:1 from the PluginPreset's PluginSpec, the given cluster name and the PluginPreset's WaitFor +func pluginSpecFromPluginPreset(preset *greenhousev1alpha1.PluginPreset, cluster string) greenhousev1alpha1.PluginSpec { + return greenhousev1alpha1.PluginSpec{ + PluginDefinitionRef: preset.Spec.Plugin.PluginDefinitionRef, + DisplayName: preset.Spec.Plugin.DisplayName, + OptionValues: preset.Spec.Plugin.OptionValues, + ReleaseNamespace: preset.Spec.Plugin.ReleaseNamespace, + DeletionPolicy: preset.Spec.Plugin.DeletionPolicy, + IgnoreDifferences: preset.Spec.Plugin.IgnoreDifferences, + // Set the cluster name to the name of the cluster. The PluginSpec contained in the PluginPreset does not have a cluster name. + ClusterName: cluster, + // Copy over the plugin dependencies + WaitFor: preset.Spec.WaitFor, + } +} diff --git a/internal/controller/plugin/pluginpreset_controller_test.go b/internal/controller/plugin/pluginpreset_controller_test.go index 6091c108f..1cbaf0874 100644 --- a/internal/controller/plugin/pluginpreset_controller_test.go +++ b/internal/controller/plugin/pluginpreset_controller_test.go @@ -1041,7 +1041,7 @@ var _ = Describe("overridesPluginOptionValues", Ordered, func() { var _ = Describe("getReleaseName", func() { It("returns plugin.Spec.ReleaseName if set", func() { plugin := &greenhousev1alpha1.Plugin{Spec: greenhousev1alpha1.PluginSpec{ReleaseName: "explicit-release"}} - preset := &greenhousev1alpha1.PluginPreset{Spec: greenhousev1alpha1.PluginPresetSpec{Plugin: greenhousev1alpha1.PluginSpec{ReleaseName: "preset-release"}}} + preset := &greenhousev1alpha1.PluginPreset{Spec: greenhousev1alpha1.PluginPresetSpec{Plugin: greenhousev1alpha1.PluginPresetPluginSpec{ReleaseName: "preset-release"}}} Expect(getReleaseName(plugin, preset)).To(Equal("explicit-release")) }) @@ -1051,13 +1051,13 @@ var _ = Describe("getReleaseName", func() { Spec: greenhousev1alpha1.PluginSpec{ReleaseName: ""}, Status: greenhousev1alpha1.PluginStatus{HelmReleaseStatus: &greenhousev1alpha1.HelmReleaseStatus{}}, } - preset := &greenhousev1alpha1.PluginPreset{Spec: greenhousev1alpha1.PluginPresetSpec{Plugin: greenhousev1alpha1.PluginSpec{ReleaseName: "preset-release"}}} + preset := &greenhousev1alpha1.PluginPreset{Spec: greenhousev1alpha1.PluginPresetSpec{Plugin: greenhousev1alpha1.PluginPresetPluginSpec{ReleaseName: "preset-release"}}} Expect(getReleaseName(plugin, preset)).To(Equal("plugin-name")) }) It("returns preset.Spec.Plugin.ReleaseName if plugin.Spec.ReleaseName is empty and no HelmReleaseStatus", func() { plugin := &greenhousev1alpha1.Plugin{Spec: greenhousev1alpha1.PluginSpec{ReleaseName: ""}} - preset := &greenhousev1alpha1.PluginPreset{Spec: greenhousev1alpha1.PluginPresetSpec{Plugin: greenhousev1alpha1.PluginSpec{ReleaseName: "preset-release"}}} + preset := &greenhousev1alpha1.PluginPreset{Spec: greenhousev1alpha1.PluginPresetSpec{Plugin: greenhousev1alpha1.PluginPresetPluginSpec{ReleaseName: "preset-release"}}} Expect(getReleaseName(plugin, preset)).To(Equal("preset-release")) }) }) diff --git a/internal/test/resources.go b/internal/test/resources.go index 07b62c663..190eedd4b 100644 --- a/internal/test/resources.go +++ b/internal/test/resources.go @@ -479,7 +479,13 @@ func WithPluginPresetClusterSelector(clusterSelector metav1.LabelSelector) func( // WithPluginPresetPluginSpec sets the PluginSpec on a PluginPreset. func WithPluginPresetPluginSpec(pluginSpec greenhousev1alpha1.PluginSpec) func(*greenhousev1alpha1.PluginPreset) { return func(pp *greenhousev1alpha1.PluginPreset) { - pp.Spec.Plugin = pluginSpec + pp.Spec.Plugin.PluginDefinitionRef = pluginSpec.PluginDefinitionRef + pp.Spec.Plugin.DisplayName = pluginSpec.DisplayName + pp.Spec.Plugin.OptionValues = pluginSpec.OptionValues + pp.Spec.Plugin.ReleaseNamespace = pluginSpec.ReleaseNamespace + pp.Spec.Plugin.ReleaseName = pluginSpec.ReleaseName + pp.Spec.Plugin.DeletionPolicy = pluginSpec.DeletionPolicy + pp.Spec.Plugin.IgnoreDifferences = pluginSpec.IgnoreDifferences } } diff --git a/internal/webhook/v1alpha1/pluginpreset_webhook.go b/internal/webhook/v1alpha1/pluginpreset_webhook.go index bf6c97438..4d1ddc4ee 100644 --- a/internal/webhook/v1alpha1/pluginpreset_webhook.go +++ b/internal/webhook/v1alpha1/pluginpreset_webhook.go @@ -88,11 +88,6 @@ func ValidateCreatePluginPreset(ctx context.Context, c client.Client, pluginPres allErrs = append(allErrs, field.Invalid(field.NewPath("spec").Child("clusterSelector"), pluginPreset.Spec.ClusterSelector, "ClusterSelector must be set")) } - // ensure ClusterName is not set - if pluginPreset.Spec.Plugin.ClusterName != "" { - allErrs = append(allErrs, field.Invalid(field.NewPath("spec").Child("plugin").Child("clusterName"), pluginPreset.Spec.Plugin.ClusterName, "ClusterName must not be set")) - } - if err := validateReleaseName(pluginPreset.Spec.Plugin.ReleaseName); err != nil { allErrs = append(allErrs, field.Invalid(field.NewPath("spec").Child("plugin").Child("releaseName"), pluginPreset.Spec.Plugin.ReleaseName, err.Error())) } @@ -117,10 +112,6 @@ func ValidateUpdatePluginPreset(ctx context.Context, c client.Client, oldPluginP 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...) diff --git a/internal/webhook/v1alpha1/pluginpreset_webhook_test.go b/internal/webhook/v1alpha1/pluginpreset_webhook_test.go index c18bcdc75..024169028 100644 --- a/internal/webhook/v1alpha1/pluginpreset_webhook_test.go +++ b/internal/webhook/v1alpha1/pluginpreset_webhook_test.go @@ -67,25 +67,6 @@ var _ = Describe("PluginPreset Admission Tests", Ordered, func() { Expect(err.Error()).To(ContainSubstring("PluginDefinition name must be set")) }) - It("should reject PluginPreset with a PluginSpec containing a ClusterName", func() { - cut := &greenhousev1alpha1.PluginPreset{ - ObjectMeta: metav1.ObjectMeta{ - Name: pluginPresetCreate, - Namespace: test.TestNamespace, - }, - Spec: greenhousev1alpha1.PluginPresetSpec{ - ClusterSelector: metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}}, - Plugin: greenhousev1alpha1.PluginSpec{ - ClusterName: "cluster", - }, - }, - } - - err := test.K8sClient.Create(test.Ctx, cut) - Expect(err).To(HaveOccurred(), "there should be an error creating the PluginPreset with invalid fields") - Expect(err.Error()).To(ContainSubstring("ClusterName must not be set"), "the error message should reflect that plugin.clusterName should not be set") - }) - It("should reject PluginPreset without ClusterSelector", func() { cut := &greenhousev1alpha1.PluginPreset{ ObjectMeta: metav1.ObjectMeta{ @@ -93,7 +74,7 @@ var _ = Describe("PluginPreset Admission Tests", Ordered, func() { Namespace: test.TestNamespace, }, Spec: greenhousev1alpha1.PluginPresetSpec{ - Plugin: greenhousev1alpha1.PluginSpec{ + Plugin: greenhousev1alpha1.PluginPresetPluginSpec{ PluginDefinitionRef: greenhousev1alpha1.PluginDefinitionReference{ Name: pluginPresetClusterDefinition, Kind: greenhousev1alpha1.ClusterPluginDefinitionKind, @@ -116,7 +97,7 @@ var _ = Describe("PluginPreset Admission Tests", Ordered, func() { Labels: map[string]string{greenhouseapis.LabelKeyOwnedBy: teamWithSupportGroupName}, }, Spec: greenhousev1alpha1.PluginPresetSpec{ - Plugin: greenhousev1alpha1.PluginSpec{ + Plugin: greenhousev1alpha1.PluginPresetPluginSpec{ PluginDefinitionRef: greenhousev1alpha1.PluginDefinitionReference{ Name: "non-existing", Kind: greenhousev1alpha1.ClusterPluginDefinitionKind, @@ -140,7 +121,7 @@ var _ = Describe("PluginPreset Admission Tests", Ordered, func() { Labels: map[string]string{greenhouseapis.LabelKeyOwnedBy: teamWithSupportGroupName}, }, Spec: greenhousev1alpha1.PluginPresetSpec{ - Plugin: greenhousev1alpha1.PluginSpec{ + Plugin: greenhousev1alpha1.PluginPresetPluginSpec{ PluginDefinitionRef: greenhousev1alpha1.PluginDefinitionReference{ Name: "non-existing", Kind: greenhousev1alpha1.PluginDefinitionKind, @@ -247,15 +228,6 @@ var _ = Describe("PluginPreset Admission Tests", Ordered, func() { Expect(err). NotTo(HaveOccurred(), "there must be no error updating the PluginPreset pluginDefinition kind") - _, err = clientutil.CreateOrPatch(test.Ctx, test.K8sClient, cut, func() error { - cut.Spec.Plugin.ClusterName = "foo" - return nil - }) - Expect(err). - To(HaveOccurred(), "there must be an error updating the PluginPreset clusterName") - Expect(err.Error()). - To(ContainSubstring("field is immutable"), "the error must reflect the field is immutable") - test.EventuallyDeleted(test.Ctx, test.K8sClient, cut) }) @@ -270,7 +242,7 @@ var _ = Describe("PluginPreset Admission Tests", Ordered, func() { }, }, Spec: greenhousev1alpha1.PluginPresetSpec{ - Plugin: greenhousev1alpha1.PluginSpec{ + Plugin: greenhousev1alpha1.PluginPresetPluginSpec{ PluginDefinitionRef: greenhousev1alpha1.PluginDefinitionReference{ Name: pluginPresetClusterDefinition, Kind: greenhousev1alpha1.ClusterPluginDefinitionKind, @@ -307,7 +279,7 @@ var _ = Describe("Validate Plugin OptionValues for PluginPreset", func() { Namespace: test.TestNamespace, }, Spec: greenhousev1alpha1.PluginPresetSpec{ - Plugin: greenhousev1alpha1.PluginSpec{ + Plugin: greenhousev1alpha1.PluginPresetPluginSpec{ PluginDefinitionRef: greenhousev1alpha1.PluginDefinitionReference{ Name: "test", Kind: greenhousev1alpha1.ClusterPluginDefinitionKind, @@ -375,7 +347,7 @@ var _ = Describe("Validate Plugin OptionValues for PluginPreset", func() { Namespace: test.TestNamespace, }, Spec: greenhousev1alpha1.PluginPresetSpec{ - Plugin: greenhousev1alpha1.PluginSpec{ + Plugin: greenhousev1alpha1.PluginPresetPluginSpec{ PluginDefinitionRef: greenhousev1alpha1.PluginDefinitionReference{ Name: "test", Kind: greenhousev1alpha1.ClusterPluginDefinitionKind, From 3be1c710edca6b9cc923a071810bb3469e41e013 Mon Sep 17 00:00:00 2001 From: "cloud-operator-bot[bot]" <224791424+cloud-operator-bot[bot]@users.noreply.github.com> Date: Wed, 20 May 2026 14:56:08 +0000 Subject: [PATCH 2/2] Automatic generation of CRD API Docs --- .../crds/greenhouse.sap_pluginpresets.yaml | 2 +- .../manager/crds/greenhouse.sap_plugins.yaml | 2 +- docs/reference/api/openapi.yaml | 58 ++++++++++--------- 3 files changed, 32 insertions(+), 30 deletions(-) diff --git a/charts/manager/crds/greenhouse.sap_pluginpresets.yaml b/charts/manager/crds/greenhouse.sap_pluginpresets.yaml index f18c3f5f5..fea9ad82d 100644 --- a/charts/manager/crds/greenhouse.sap_pluginpresets.yaml +++ b/charts/manager/crds/greenhouse.sap_pluginpresets.yaml @@ -6,7 +6,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.20.0 + controller-gen.kubebuilder.io/version: v0.20.1 name: pluginpresets.greenhouse.sap spec: group: greenhouse.sap diff --git a/charts/manager/crds/greenhouse.sap_plugins.yaml b/charts/manager/crds/greenhouse.sap_plugins.yaml index 22c7c8835..65676d7d1 100644 --- a/charts/manager/crds/greenhouse.sap_plugins.yaml +++ b/charts/manager/crds/greenhouse.sap_plugins.yaml @@ -6,7 +6,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.20.0 + controller-gen.kubebuilder.io/version: v0.20.1 name: plugins.greenhouse.sap spec: group: greenhouse.sap diff --git a/docs/reference/api/openapi.yaml b/docs/reference/api/openapi.yaml index 6fe406f38..95a099a30 100755 --- a/docs/reference/api/openapi.yaml +++ b/docs/reference/api/openapi.yaml @@ -1108,7 +1108,11 @@ components: description: PluginOptionValue is the value for a PluginOption. properties: expression: - description: Expression is a YAML string with ${...} placeholders that will be evaluated as CEL expressions. + description: |- + Expression is a YAML string with ${...} placeholders that will be evaluated as CEL expressions. + + Deprecated: Expression is deprecated on standalone Plugins and will be removed in a future release. + Consider using a PluginPreset to deploy Plugins utilizing the Expression field. type: string name: description: Name of the values. @@ -1120,7 +1124,11 @@ components: description: ValueFrom references value in another source. properties: ref: - description: Ref references values defined in another resource (Plugin, PluginPreset) + description: |- + Ref references values defined in another resource (Plugin, PluginPreset) + + Deprecated: Ref is deprecated on standalone Plugins and will be removed in a future release. + Consider using a PluginPreset to deploy Plugins utilizing the Ref field. properties: expression: description: Expression is a CEL expression to extract the value from the referenced resource @@ -1275,9 +1283,6 @@ components: plugin: description: PluginSpec is the spec of the plugin to be deployed by the PluginPreset. properties: - clusterName: - description: ClusterName is the name of the cluster the plugin is deployed to. If not set, the plugin is deployed to the greenhouse cluster. - type: string deletionPolicy: default: Delete description: |- @@ -1325,7 +1330,11 @@ components: description: PluginOptionValue is the value for a PluginOption. properties: expression: - description: Expression is a YAML string with ${...} placeholders that will be evaluated as CEL expressions. + description: |- + Expression is a YAML string with ${...} placeholders that will be evaluated as CEL expressions. + + Deprecated: Expression is deprecated on standalone Plugins and will be removed in a future release. + Consider using a PluginPreset to deploy Plugins utilizing the Expression field. type: string name: description: Name of the values. @@ -1337,7 +1346,11 @@ components: description: ValueFrom references value in another source. properties: ref: - description: Ref references values defined in another resource (Plugin, PluginPreset) + description: |- + Ref references values defined in another resource (Plugin, PluginPreset) + + Deprecated: Ref is deprecated on standalone Plugins and will be removed in a future release. + Consider using a PluginPreset to deploy Plugins utilizing the Ref field. properties: expression: description: Expression is a CEL expression to extract the value from the referenced resource @@ -1459,25 +1472,6 @@ components: ReleaseNamespace is the namespace in the remote cluster to which the backend is deployed. Defaults to the Greenhouse managed namespace if not set. type: string - waitFor: - description: WaitFor defines other Plugins to wait for before installing this Plugin. - items: - description: WaitForItem is a wrapper around PluginRef to add context for every WaitFor list item. - properties: - pluginRef: - description: PluginRef defines a reference to the Plugin. - properties: - name: - description: Name of the Plugin. - type: string - pluginPreset: - description: PluginPreset is the name of the PluginPreset which creates the Plugin. - type: string - type: object - required: - - pluginRef - type: object - type: array required: - pluginDefinitionRef type: object @@ -1661,7 +1655,11 @@ components: description: PluginOptionValue is the value for a PluginOption. properties: expression: - description: Expression is a YAML string with ${...} placeholders that will be evaluated as CEL expressions. + description: |- + Expression is a YAML string with ${...} placeholders that will be evaluated as CEL expressions. + + Deprecated: Expression is deprecated on standalone Plugins and will be removed in a future release. + Consider using a PluginPreset to deploy Plugins utilizing the Expression field. type: string name: description: Name of the values. @@ -1673,7 +1671,11 @@ components: description: ValueFrom references value in another source. properties: ref: - description: Ref references values defined in another resource (Plugin, PluginPreset) + description: |- + Ref references values defined in another resource (Plugin, PluginPreset) + + Deprecated: Ref is deprecated on standalone Plugins and will be removed in a future release. + Consider using a PluginPreset to deploy Plugins utilizing the Ref field. properties: expression: description: Expression is a CEL expression to extract the value from the referenced resource