diff --git a/e2e/plugin/e2e_test.go b/e2e/plugin/e2e_test.go index 66c93e125..0bffdaa36 100644 --- a/e2e/plugin/e2e_test.go +++ b/e2e/plugin/e2e_test.go @@ -24,6 +24,7 @@ import ( "github.com/cloudoperators/greenhouse/e2e/plugin/fixtures" "github.com/cloudoperators/greenhouse/e2e/shared" + "github.com/cloudoperators/greenhouse/pkg/apis" greenhousev1alpha1 "github.com/cloudoperators/greenhouse/pkg/apis/greenhouse/v1alpha1" "github.com/cloudoperators/greenhouse/pkg/clientutil" "github.com/cloudoperators/greenhouse/pkg/test" @@ -105,8 +106,7 @@ var _ = Describe("Plugin E2E", Ordered, func() { Expect(err).NotTo(HaveOccurred()) Expect(len(pluginDefinitionList.Items)).To(BeEquivalentTo(1)) - By("Try to creating the plugin") - // Creating plugin + By("Trying to create the plugin without annotation") testPlugin := fixtures.PreparePlugin("test-nginx-plugin", env.TestNamespace, test.WithPluginDefinition(testPluginDefinition.Name), test.WithReleaseNamespace(env.TestNamespace), @@ -114,17 +114,16 @@ var _ = Describe("Plugin E2E", Ordered, func() { err = adminClient.Create(ctx, testPlugin) Expect(err).To(HaveOccurred()) - By("Creating the plugin preset") - testPluginPreset := fixtures.PreparePluginPreset("test-nginx-preset", env.TestNamespace, testPlugin.Spec) - err = adminClient.Create(ctx, testPluginPreset) + By("Trying to create the plugin without annotation") + testPlugin = fixtures.PreparePlugin("test-nginx-plugin", env.TestNamespace, + test.WithPluginDefinition(testPluginDefinition.Name), + test.WithReleaseNamespace(env.TestNamespace), + test.WithPluginOptionValue("replicaCount", &apiextensionsv1.JSON{Raw: []byte("1")}, nil), + test.WithCluster(remoteClusterName), + test.WithAnnotation(apis.AllowPluginCreateAnnotation, "true")) + err = adminClient.Create(ctx, testPlugin) Expect(err).ToNot(HaveOccurred()) - By("Checking the plugin preset is ready") - Eventually(func(g Gomega) { - err = adminClient.Get(ctx, client.ObjectKeyFromObject(testPluginPreset), testPluginPreset) - g.Expect(err).ToNot(HaveOccurred()) - }).Should(Succeed()) - By("Checking the plugin status is ready") pluginList := &greenhousev1alpha1.PluginList{} Eventually(func(g Gomega) { @@ -157,11 +156,11 @@ var _ = Describe("Plugin E2E", Ordered, func() { By("Updating replicas") Eventually(func(g Gomega) { - namespacedName := types.NamespacedName{Name: testPluginPreset.Name, Namespace: testPluginPreset.Namespace} - err = adminClient.Get(ctx, namespacedName, testPluginPreset) + namespacedName := types.NamespacedName{Name: testPlugin.Name, Namespace: testPlugin.Namespace} + err = adminClient.Get(ctx, namespacedName, testPlugin) g.Expect(err).NotTo(HaveOccurred()) - test.SetOptionValueForPluginPreset(testPluginPreset, "replicaCount", "2") - err = adminClient.Update(ctx, testPluginPreset) + test.SetOptionValueForPlugin(testPlugin, "replicaCount", "2") + err = adminClient.Update(ctx, testPlugin) g.Expect(err).NotTo(HaveOccurred()) }).Should(Succeed()) @@ -176,24 +175,6 @@ var _ = Describe("Plugin E2E", Ordered, func() { } }).Should(Succeed()) - By("Add annotation to allow delete plugin preset") - Eventually(func(g Gomega) { - err = adminClient.Get(ctx, client.ObjectKeyFromObject(testPluginPreset), testPluginPreset) - g.Expect(err).NotTo(HaveOccurred()) - if testPluginPreset.Annotations == nil { - testPluginPreset.Annotations = map[string]string{} - } - delete(testPluginPreset.Annotations, "greenhouse.sap/prevent-deletion") - err = adminClient.Update(ctx, testPluginPreset) - g.Expect(err).NotTo(HaveOccurred()) - }).Should(Succeed()) - - By("Deleting plugin preset") - Eventually(func(g Gomega) { - err = adminClient.Delete(ctx, testPluginPreset) - g.Expect(err).NotTo(HaveOccurred()) - }).Should(Succeed()) - By("Deleting plugin") Eventually(func(g Gomega) { err = adminClient.Delete(ctx, testPlugin) diff --git a/pkg/admission/plugin_webhook.go b/pkg/admission/plugin_webhook.go index b1e5b3b05..48e5ee054 100644 --- a/pkg/admission/plugin_webhook.go +++ b/pkg/admission/plugin_webhook.go @@ -150,8 +150,8 @@ func ValidateDeletePlugin(_ context.Context, _ client.Client, _ runtime.Object) } func allowCreatePlugin(plugin *greenhousev1alpha1.Plugin) bool { - _, ok := plugin.Annotations["greenhouse.sap/allow-create"] - delete(plugin.Annotations, "greenhouse.sap/allow-create") + _, ok := plugin.Annotations[greenhouseapis.AllowPluginCreateAnnotation] + delete(plugin.Annotations, greenhouseapis.AllowPluginCreateAnnotation) return ok } diff --git a/pkg/admission/plugin_webhook_test.go b/pkg/admission/plugin_webhook_test.go index b6c1b5561..6d526bd0f 100644 --- a/pkg/admission/plugin_webhook_test.go +++ b/pkg/admission/plugin_webhook_test.go @@ -231,7 +231,7 @@ var _ = Describe("Validate plugin spec fields", Ordered, func() { testPlugin = test.NewPlugin(test.Ctx, "test-plugin", setup.Namespace(), test.WithPluginDefinition(testPluginDefinition.Name), test.WithReleaseNamespace("test-namespace"), - test.WithAnnotation(greenhousev1alpha1.AllowCreateAnnotation, "true")) + test.WithAnnotation(greenhouseapis.AllowPluginCreateAnnotation, "true")) expectClusterMustBeSetError(test.K8sClient.Create(test.Ctx, testPlugin)) }) @@ -241,7 +241,7 @@ var _ = Describe("Validate plugin spec fields", Ordered, func() { test.WithPluginDefinition(testPluginDefinition.Name), test.WithCluster("non-existent-cluster"), test.WithReleaseNamespace("test-namespace"), - test.WithAnnotation(greenhousev1alpha1.AllowCreateAnnotation, "true")) + test.WithAnnotation(greenhouseapis.AllowPluginCreateAnnotation, "true")) expectClusterNotFoundError(test.K8sClient.Create(test.Ctx, testPlugin)) }) @@ -252,7 +252,7 @@ var _ = Describe("Validate plugin spec fields", Ordered, func() { test.WithPluginDefinition(testPluginDefinition.Name), test.WithCluster(testCluster.Name), test.WithReleaseNamespace("test-namespace"), - test.WithAnnotation(greenhousev1alpha1.AllowCreateAnnotation, "true")) + test.WithAnnotation(greenhouseapis.AllowPluginCreateAnnotation, "true")) By("checking the label on the plugin") actPlugin := &greenhousev1alpha1.Plugin{} @@ -268,7 +268,7 @@ var _ = Describe("Validate plugin spec fields", Ordered, func() { test.WithPluginDefinition(testPluginDefinition.Name), test.WithCluster(testCluster.Name), test.WithReleaseNamespace("test-namespace"), - test.WithAnnotation(greenhousev1alpha1.AllowCreateAnnotation, "true")) + test.WithAnnotation(greenhouseapis.AllowPluginCreateAnnotation, "true")) testPlugin.Spec.ClusterName = "wrong-cluster-name" err := test.K8sClient.Update(test.Ctx, testPlugin) @@ -281,7 +281,7 @@ var _ = Describe("Validate plugin spec fields", Ordered, func() { test.WithPluginDefinition(testPluginDefinition.Name), test.WithCluster(testCluster.Name), test.WithReleaseNamespace("test-namespace"), - test.WithAnnotation(greenhousev1alpha1.AllowCreateAnnotation, "true")) + test.WithAnnotation(greenhouseapis.AllowPluginCreateAnnotation, "true")) testPlugin.Spec.ClusterName = "" err := test.K8sClient.Update(test.Ctx, testPlugin) Expect(err).To(HaveOccurred(), "there should be an error changing the plugin's clusterName") @@ -293,20 +293,30 @@ var _ = Describe("Validate plugin spec fields", Ordered, func() { test.WithPluginDefinition(testPluginDefinition.Name), test.WithCluster(testCluster.Name), test.WithReleaseNamespace("test-namespace"), - test.WithAnnotation(greenhousev1alpha1.AllowCreateAnnotation, "true")) + test.WithAnnotation(greenhouseapis.AllowPluginCreateAnnotation, "true")) testPlugin.Spec.ReleaseNamespace = "foo-bar" err := test.K8sClient.Update(test.Ctx, testPlugin) Expect(err).To(HaveOccurred(), "there should be an error changing the plugin's releaseNamespace") Expect(err.Error()).To(ContainSubstring(validation.FieldImmutableErrorMsg)) }) + It("should deny creation of a Plugin without AllowPluginCreationAnnotation", func() { + testPlugin = test.NewPlugin(test.Ctx, "test-plugin-2", setup.Namespace(), + test.WithPluginDefinition(testPluginDefinition.Name), + test.WithCluster(testCluster.Name), + test.WithReleaseNamespace("test-namespace")) + err := test.K8sClient.Create(test.Ctx, testPlugin) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("plugin creation is not allowed")) + }) + It("should reject to update a plugin when the pluginDefinition changes", func() { secondPluginDefinition := setup.CreatePluginDefinition(test.Ctx, "foo-bar") testPlugin = setup.CreatePlugin(test.Ctx, "test-plugin", test.WithPluginDefinition(testPluginDefinition.Name), test.WithCluster(testCluster.Name), test.WithReleaseNamespace("test-namespace"), - test.WithAnnotation(greenhousev1alpha1.AllowCreateAnnotation, "true")) + test.WithAnnotation(greenhouseapis.AllowPluginCreateAnnotation, "true")) testPlugin.Spec.PluginDefinition = secondPluginDefinition.Name err := test.K8sClient.Update(test.Ctx, testPlugin) diff --git a/pkg/apis/greenhouse/v1alpha1/plugin_types.go b/pkg/apis/greenhouse/v1alpha1/plugin_types.go index df7252a1e..a33c5def3 100644 --- a/pkg/apis/greenhouse/v1alpha1/plugin_types.go +++ b/pkg/apis/greenhouse/v1alpha1/plugin_types.go @@ -80,8 +80,6 @@ const ( HelmUninstallFailedReason ConditionReason = "HelmUninstallFailed" ) -const AllowCreateAnnotation = "greenhouse.sap/allow-create" - // PluginStatus defines the observed state of Plugin type PluginStatus struct { // HelmReleaseStatus reflects the status of the latest HelmChart release. diff --git a/pkg/apis/well_known.go b/pkg/apis/well_known.go index d5890e111..8a8bbb1fd 100644 --- a/pkg/apis/well_known.go +++ b/pkg/apis/well_known.go @@ -48,6 +48,9 @@ const ( // LabelKeyExposeNamedPort is specifying the port to be exposed by name. LabelKeyExposeService needs to be set. Defaults to the first port if the named port is not found. LabelKeyExposeNamedPort = "greenhouse.sap/exposeNamedPort" + + // AllowPluginCreateAnnotation enables the creation of a plugin + AllowPluginCreateAnnotation = "greenhouse.sap/allow-plugin-create" ) // TeamRole and TeamRoleBinding constants diff --git a/pkg/controllers/organization/service_proxy.go b/pkg/controllers/organization/service_proxy.go index 3a82ec292..be07cda57 100644 --- a/pkg/controllers/organization/service_proxy.go +++ b/pkg/controllers/organization/service_proxy.go @@ -18,6 +18,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/log" + greenhouseapis "github.com/cloudoperators/greenhouse/pkg/apis" greenhousesapv1alpha1 "github.com/cloudoperators/greenhouse/pkg/apis/greenhouse/v1alpha1" "github.com/cloudoperators/greenhouse/pkg/clientutil" "github.com/cloudoperators/greenhouse/pkg/common" @@ -52,7 +53,7 @@ func (r *OrganizationReconciler) reconcileServiceProxy(ctx context.Context, org Name: serviceProxyName, Namespace: org.Name, Annotations: map[string]string{ - greenhousesapv1alpha1.AllowCreateAnnotation: "true", + greenhouseapis.AllowPluginCreateAnnotation: "true", }, }, Spec: greenhousesapv1alpha1.PluginSpec{ diff --git a/pkg/controllers/plugin/pluginpreset_controller.go b/pkg/controllers/plugin/pluginpreset_controller.go index a6dfc1f1a..fe8e85527 100644 --- a/pkg/controllers/plugin/pluginpreset_controller.go +++ b/pkg/controllers/plugin/pluginpreset_controller.go @@ -175,7 +175,7 @@ func (r *PluginPresetReconciler) reconcilePluginPreset(ctx context.Context, pres plugin.Annotations = make(map[string]string) } - plugin.Annotations[greenhousev1alpha1.AllowCreateAnnotation] = "true" + plugin.Annotations[greenhouseapis.AllowPluginCreateAnnotation] = "true" // Label the plugin with the managed resource label to identify it as managed by the PluginPreset. plugin.SetLabels(map[string]string{greenhouseapis.LabelKeyPluginPreset: preset.Name}) diff --git a/pkg/controllers/plugin/remote_cluster_test.go b/pkg/controllers/plugin/remote_cluster_test.go index c0394322c..0ac433ac1 100644 --- a/pkg/controllers/plugin/remote_cluster_test.go +++ b/pkg/controllers/plugin/remote_cluster_test.go @@ -42,7 +42,7 @@ var ( Name: "test-plugindefinition", Namespace: test.TestNamespace, Annotations: map[string]string{ - greenhousev1alpha1.AllowCreateAnnotation: "true", + greenhouseapis.AllowPluginCreateAnnotation: "true", }, }, Spec: greenhousev1alpha1.PluginSpec{ @@ -61,7 +61,7 @@ var ( Name: "test-plugin-secretref", Namespace: test.TestNamespace, Annotations: map[string]string{ - greenhousev1alpha1.AllowCreateAnnotation: "true", + greenhouseapis.AllowPluginCreateAnnotation: "true", }, }, Spec: greenhousev1alpha1.PluginSpec{ @@ -87,7 +87,7 @@ var ( Name: "test-plugin-in-made-up-namespace", Namespace: test.TestNamespace, Annotations: map[string]string{ - greenhousev1alpha1.AllowCreateAnnotation: "true", + greenhouseapis.AllowPluginCreateAnnotation: "true", }, }, Spec: greenhousev1alpha1.PluginSpec{ @@ -106,7 +106,7 @@ var ( Name: "test-plugin-crd", Namespace: test.TestNamespace, Annotations: map[string]string{ - greenhousev1alpha1.AllowCreateAnnotation: "true", + greenhouseapis.AllowPluginCreateAnnotation: "true", }, }, Spec: greenhousev1alpha1.PluginSpec{ @@ -125,7 +125,7 @@ var ( Name: "test-plugin-exposed", Namespace: test.TestNamespace, Annotations: map[string]string{ - greenhousev1alpha1.AllowCreateAnnotation: "true", + greenhouseapis.AllowPluginCreateAnnotation: "true", }, }, Spec: greenhousev1alpha1.PluginSpec{ diff --git a/pkg/controllers/plugin/suite_test.go b/pkg/controllers/plugin/suite_test.go index 7fc58f767..a1b1d59a6 100644 --- a/pkg/controllers/plugin/suite_test.go +++ b/pkg/controllers/plugin/suite_test.go @@ -26,6 +26,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "github.com/cloudoperators/greenhouse/pkg/admission" + greenhouseapis "github.com/cloudoperators/greenhouse/pkg/apis" greenhousev1alpha1 "github.com/cloudoperators/greenhouse/pkg/apis/greenhouse/v1alpha1" "github.com/cloudoperators/greenhouse/pkg/clientutil" "github.com/cloudoperators/greenhouse/pkg/helm" @@ -165,7 +166,7 @@ var _ = Describe("HelmControllerTest", Serial, func() { Name: PluginName, Namespace: Namespace, Annotations: map[string]string{ - greenhousev1alpha1.AllowCreateAnnotation: "true", + greenhouseapis.AllowPluginCreateAnnotation: "true", }, }, Spec: greenhousev1alpha1.PluginSpec{ @@ -521,7 +522,7 @@ var _ = Describe("HelmControllerTest", Serial, func() { Name: pluginName, Namespace: Namespace, Annotations: map[string]string{ - greenhousev1alpha1.AllowCreateAnnotation: "true", + greenhouseapis.AllowPluginCreateAnnotation: "true", }, }, Spec: greenhousev1alpha1.PluginSpec{ @@ -667,7 +668,7 @@ var _ = When("the pluginDefinition is UI only", func() { Name: "uiplugin", Namespace: "default", Annotations: map[string]string{ - greenhousev1alpha1.AllowCreateAnnotation: "true", + greenhouseapis.AllowPluginCreateAnnotation: "true", }, }, Spec: greenhousev1alpha1.PluginSpec{ diff --git a/pkg/rbac/role.go b/pkg/rbac/role.go index 5ca7253bd..2f8a45886 100644 --- a/pkg/rbac/role.go +++ b/pkg/rbac/role.go @@ -79,7 +79,7 @@ func OrganizationPluginAdminPolicyRules() []rbacv1.PolicyRule { APIGroups: []string{greenhouseapisv1alpha1.GroupVersion.Group}, Resources: []string{"pluginpresets"}, }, - // Grant read, update and delete permissions for PluginPresets to organization plugin admins. No create + // Grant read, update and delete permissions for Plugins to organization plugin admins. No create { Verbs: []string{"get", "list", "watch", "update", "patch", "delete"}, APIGroups: []string{greenhouseapisv1alpha1.GroupVersion.Group}, diff --git a/pkg/test/resources.go b/pkg/test/resources.go index 77a001181..d12f86092 100644 --- a/pkg/test/resources.go +++ b/pkg/test/resources.go @@ -208,21 +208,6 @@ func SetOptionValueForPlugin(plugin *greenhousev1alpha1.Plugin, key, value strin }) } -// SetOptionValueForPluginPreset sets the value of a PluginOtionValue -func SetOptionValueForPluginPreset(pluginPreset *greenhousev1alpha1.PluginPreset, key, value string) { - for i, keyValue := range pluginPreset.Spec.Plugin.OptionValues { - if keyValue.Name == key { - pluginPreset.Spec.Plugin.OptionValues[i].Value.Raw = []byte(value) - return - } - } - - pluginPreset.Spec.Plugin.OptionValues = append(pluginPreset.Spec.Plugin.OptionValues, greenhousev1alpha1.PluginOptionValue{ - Name: key, - Value: &apiextensionsv1.JSON{Raw: []byte(value)}, - }) -} - // NewPlugin returns a greenhousev1alpha1.Plugin object. Opts can be used to set the desired state of the Plugin. func NewPlugin(ctx context.Context, name, namespace string, opts ...func(*greenhousev1alpha1.Plugin)) *greenhousev1alpha1.Plugin { GinkgoHelper()