diff --git a/e2e/plugin/e2e_test.go b/e2e/plugin/e2e_test.go index 175e0670c..a0aca8409 100644 --- a/e2e/plugin/e2e_test.go +++ b/e2e/plugin/e2e_test.go @@ -70,11 +70,24 @@ var _ = Describe("Plugin E2E", Ordered, func() { It("should have a cluster resource created", func() { By("verifying if the cluster resource is created") Eventually(func(g Gomega) { - err := adminClient.Get(ctx, client.ObjectKey{Name: remoteClusterName, Namespace: env.TestNamespace}, &greenhousev1alpha1.Cluster{}) + var remoteCluster greenhousev1alpha1.Cluster + err := adminClient.Get(ctx, client.ObjectKey{Name: remoteClusterName, Namespace: env.TestNamespace}, &remoteCluster) + g.Expect(err).ToNot(HaveOccurred()) + if remoteCluster.Labels == nil { + remoteCluster.Labels = map[string]string{} + } + remoteCluster.Labels["app"] = "test" + err = adminClient.Update(ctx, &remoteCluster) g.Expect(err).ToNot(HaveOccurred()) - }).Should(Succeed(), "cluster resource should be created") + Eventually(func(g Gomega) { + var remoteCluster greenhousev1alpha1.Cluster + err := adminClient.Get(ctx, client.ObjectKey{Name: remoteClusterName, Namespace: env.TestNamespace}, &remoteCluster) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(remoteCluster.Labels["app"]).To(Equal("test")) + }).Should(Succeed(), "cluster resource should be updated") + By("verifying the cluster status is ready") shared.ClusterIsReady(ctx, adminClient, remoteClusterName, env.TestNamespace) }) @@ -119,6 +132,7 @@ var _ = Describe("Plugin E2E", Ordered, func() { g.Expect(len(pluginList.Items)).To(BeEquivalentTo(1)) g.Expect(pluginList.Items[0].Status.HelmReleaseStatus).ToNot(BeNil()) g.Expect(pluginList.Items[0].Status.HelmReleaseStatus.Status).To(BeEquivalentTo("deployed")) + testPlugin = &pluginList.Items[0] }).Should(Succeed()) By("Checking deployment") @@ -161,8 +175,29 @@ 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") - test.EventuallyDeleted(ctx, adminClient, testPluginPreset) + Eventually(func(g Gomega) { + err = adminClient.Delete(ctx, testPlugin) + g.Expect(err).NotTo(HaveOccurred()) + }).Should(Succeed()) By("Check, is deployment deleted") Eventually(func(g Gomega) bool { diff --git a/e2e/shared/cluster.go b/e2e/shared/cluster.go index c81b69c8d..5f86f7aed 100644 --- a/e2e/shared/cluster.go +++ b/e2e/shared/cluster.go @@ -31,9 +31,6 @@ func OnboardRemoteCluster(ctx context.Context, k8sClient client.Client, kubeConf ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: namespace, - Labels: map[string]string{ - "app": "test", - }, }, Type: greenhouseapis.SecretTypeKubeConfig, Data: map[string][]byte{ diff --git a/pkg/admission/plugin_webhook_test.go b/pkg/admission/plugin_webhook_test.go index 7e52a0851..b6c1b5561 100644 --- a/pkg/admission/plugin_webhook_test.go +++ b/pkg/admission/plugin_webhook_test.go @@ -228,20 +228,31 @@ var _ = Describe("Validate plugin spec fields", Ordered, func() { }) It("should not accept a plugin without a clusterName", func() { - testPlugin = test.NewPlugin(test.Ctx, "test-plugin", setup.Namespace(), test.WithPluginDefinition(testPluginDefinition.Name), test.WithReleaseNamespace("test-namespace")) + testPlugin = test.NewPlugin(test.Ctx, "test-plugin", setup.Namespace(), + test.WithPluginDefinition(testPluginDefinition.Name), + test.WithReleaseNamespace("test-namespace"), + test.WithAnnotation(greenhousev1alpha1.AllowCreateAnnotation, "true")) expectClusterMustBeSetError(test.K8sClient.Create(test.Ctx, testPlugin)) }) It("should reject the plugin when the cluster with clusterName does not exist", func() { By("creating the plugin") - testPlugin = test.NewPlugin(test.Ctx, "test-plugin", setup.Namespace(), test.WithPluginDefinition(testPluginDefinition.Name), test.WithCluster("non-existent-cluster"), test.WithReleaseNamespace("test-namespace")) + testPlugin = test.NewPlugin(test.Ctx, "test-plugin", setup.Namespace(), + test.WithPluginDefinition(testPluginDefinition.Name), + test.WithCluster("non-existent-cluster"), + test.WithReleaseNamespace("test-namespace"), + test.WithAnnotation(greenhousev1alpha1.AllowCreateAnnotation, "true")) expectClusterNotFoundError(test.K8sClient.Create(test.Ctx, testPlugin)) }) It("should accept the plugin when the cluster with clusterName exists", func() { By("creating the plugin") - testPlugin = setup.CreatePlugin(test.Ctx, "test-plugin", test.WithPluginDefinition(testPluginDefinition.Name), test.WithCluster(testCluster.Name), test.WithReleaseNamespace("test-namespace")) + testPlugin = setup.CreatePlugin(test.Ctx, "test-plugin", + test.WithPluginDefinition(testPluginDefinition.Name), + test.WithCluster(testCluster.Name), + test.WithReleaseNamespace("test-namespace"), + test.WithAnnotation(greenhousev1alpha1.AllowCreateAnnotation, "true")) By("checking the label on the plugin") actPlugin := &greenhousev1alpha1.Plugin{} @@ -253,7 +264,11 @@ var _ = Describe("Validate plugin spec fields", Ordered, func() { }) It("should reject to update a plugin when the clusterName changes", func() { - testPlugin = setup.CreatePlugin(test.Ctx, "test-plugin", test.WithPluginDefinition(testPluginDefinition.Name), test.WithCluster(testCluster.Name), test.WithReleaseNamespace("test-namespace")) + testPlugin = setup.CreatePlugin(test.Ctx, "test-plugin", + test.WithPluginDefinition(testPluginDefinition.Name), + test.WithCluster(testCluster.Name), + test.WithReleaseNamespace("test-namespace"), + test.WithAnnotation(greenhousev1alpha1.AllowCreateAnnotation, "true")) testPlugin.Spec.ClusterName = "wrong-cluster-name" err := test.K8sClient.Update(test.Ctx, testPlugin) @@ -262,7 +277,11 @@ var _ = Describe("Validate plugin spec fields", Ordered, func() { }) It("should reject to update a plugin when the clustername is removed", func() { - testPlugin = setup.CreatePlugin(test.Ctx, "test-plugin", test.WithPluginDefinition(testPluginDefinition.Name), test.WithCluster(testCluster.Name), test.WithReleaseNamespace("test-namespace")) + testPlugin = setup.CreatePlugin(test.Ctx, "test-plugin", + test.WithPluginDefinition(testPluginDefinition.Name), + test.WithCluster(testCluster.Name), + test.WithReleaseNamespace("test-namespace"), + test.WithAnnotation(greenhousev1alpha1.AllowCreateAnnotation, "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") @@ -270,7 +289,11 @@ var _ = Describe("Validate plugin spec fields", Ordered, func() { }) It("should reject to update a plugin when the releaseNamespace changes", func() { - testPlugin = setup.CreatePlugin(test.Ctx, "test-plugin", test.WithPluginDefinition(testPluginDefinition.Name), test.WithCluster(testCluster.Name), test.WithReleaseNamespace("test-namespace")) + testPlugin = setup.CreatePlugin(test.Ctx, "test-plugin", + test.WithPluginDefinition(testPluginDefinition.Name), + test.WithCluster(testCluster.Name), + test.WithReleaseNamespace("test-namespace"), + test.WithAnnotation(greenhousev1alpha1.AllowCreateAnnotation, "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") @@ -279,7 +302,11 @@ var _ = Describe("Validate plugin spec fields", Ordered, func() { 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")) + testPlugin = setup.CreatePlugin(test.Ctx, "test-plugin", + test.WithPluginDefinition(testPluginDefinition.Name), + test.WithCluster(testCluster.Name), + test.WithReleaseNamespace("test-namespace"), + test.WithAnnotation(greenhousev1alpha1.AllowCreateAnnotation, "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 a33c5def3..df7252a1e 100644 --- a/pkg/apis/greenhouse/v1alpha1/plugin_types.go +++ b/pkg/apis/greenhouse/v1alpha1/plugin_types.go @@ -80,6 +80,8 @@ 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/controllers/organization/service_proxy.go b/pkg/controllers/organization/service_proxy.go index 520bc3352..3a82ec292 100644 --- a/pkg/controllers/organization/service_proxy.go +++ b/pkg/controllers/organization/service_proxy.go @@ -51,6 +51,9 @@ func (r *OrganizationReconciler) reconcileServiceProxy(ctx context.Context, org ObjectMeta: metav1.ObjectMeta{ Name: serviceProxyName, Namespace: org.Name, + Annotations: map[string]string{ + greenhousesapv1alpha1.AllowCreateAnnotation: "true", + }, }, Spec: greenhousesapv1alpha1.PluginSpec{ PluginDefinition: serviceProxyName, diff --git a/pkg/controllers/plugin/helm_controller.go b/pkg/controllers/plugin/helm_controller.go index 539189102..d8ca9464d 100644 --- a/pkg/controllers/plugin/helm_controller.go +++ b/pkg/controllers/plugin/helm_controller.go @@ -106,7 +106,7 @@ func (r *HelmReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl. pluginStatus := initPluginStatus(plugin) defer func() { if statusErr := setPluginStatus(ctx, r.Client, plugin, pluginStatus); statusErr != nil { - log.FromContext(ctx).Error(statusErr, "failed to set status") + log.FromContext(ctx).Error(statusErr, "failed to set status", "plugin", plugin, "status", pluginStatus) } }() diff --git a/pkg/controllers/plugin/pluginpreset_controller.go b/pkg/controllers/plugin/pluginpreset_controller.go index cbc700821..eb173bf19 100644 --- a/pkg/controllers/plugin/pluginpreset_controller.go +++ b/pkg/controllers/plugin/pluginpreset_controller.go @@ -169,14 +169,13 @@ func (r *PluginPresetReconciler) reconcilePluginPreset(ctx context.Context, pres return err } - log.FromContext(ctx).Info("======================== OOOOOOOOOO", "cluster", cluster) _, err = clientutil.CreateOrPatch(ctx, r.Client, plugin, func() error { // Add annotation to allow plugin creation if plugin.Annotations == nil { plugin.Annotations = make(map[string]string) } - plugin.Annotations["greenhouse.sap/allow-create"] = "true" + plugin.Annotations[greenhousev1alpha1.AllowCreateAnnotation] = "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 5c39c0779..c0394322c 100644 --- a/pkg/controllers/plugin/remote_cluster_test.go +++ b/pkg/controllers/plugin/remote_cluster_test.go @@ -41,6 +41,9 @@ var ( ObjectMeta: metav1.ObjectMeta{ Name: "test-plugindefinition", Namespace: test.TestNamespace, + Annotations: map[string]string{ + greenhousev1alpha1.AllowCreateAnnotation: "true", + }, }, Spec: greenhousev1alpha1.PluginSpec{ ClusterName: "test-cluster", @@ -57,6 +60,9 @@ var ( ObjectMeta: metav1.ObjectMeta{ Name: "test-plugin-secretref", Namespace: test.TestNamespace, + Annotations: map[string]string{ + greenhousev1alpha1.AllowCreateAnnotation: "true", + }, }, Spec: greenhousev1alpha1.PluginSpec{ PluginDefinition: "test-plugindefinition", @@ -80,6 +86,9 @@ var ( ObjectMeta: metav1.ObjectMeta{ Name: "test-plugin-in-made-up-namespace", Namespace: test.TestNamespace, + Annotations: map[string]string{ + greenhousev1alpha1.AllowCreateAnnotation: "true", + }, }, Spec: greenhousev1alpha1.PluginSpec{ PluginDefinition: testPluginDefinition.GetName(), @@ -96,6 +105,9 @@ var ( ObjectMeta: metav1.ObjectMeta{ Name: "test-plugin-crd", Namespace: test.TestNamespace, + Annotations: map[string]string{ + greenhousev1alpha1.AllowCreateAnnotation: "true", + }, }, Spec: greenhousev1alpha1.PluginSpec{ ClusterName: "test-cluster", @@ -112,6 +124,9 @@ var ( ObjectMeta: metav1.ObjectMeta{ Name: "test-plugin-exposed", Namespace: test.TestNamespace, + Annotations: map[string]string{ + greenhousev1alpha1.AllowCreateAnnotation: "true", + }, }, Spec: greenhousev1alpha1.PluginSpec{ ClusterName: "test-cluster", diff --git a/pkg/controllers/plugin/suite_test.go b/pkg/controllers/plugin/suite_test.go index 4336c8b63..7fc58f767 100644 --- a/pkg/controllers/plugin/suite_test.go +++ b/pkg/controllers/plugin/suite_test.go @@ -164,6 +164,9 @@ var _ = Describe("HelmControllerTest", Serial, func() { ObjectMeta: metav1.ObjectMeta{ Name: PluginName, Namespace: Namespace, + Annotations: map[string]string{ + greenhousev1alpha1.AllowCreateAnnotation: "true", + }, }, Spec: greenhousev1alpha1.PluginSpec{ PluginDefinition: PluginDefinitionName, @@ -517,6 +520,9 @@ var _ = Describe("HelmControllerTest", Serial, func() { ObjectMeta: metav1.ObjectMeta{ Name: pluginName, Namespace: Namespace, + Annotations: map[string]string{ + greenhousev1alpha1.AllowCreateAnnotation: "true", + }, }, Spec: greenhousev1alpha1.PluginSpec{ PluginDefinition: pluginWithEveryOption, @@ -660,6 +666,9 @@ var _ = When("the pluginDefinition is UI only", func() { ObjectMeta: metav1.ObjectMeta{ Name: "uiplugin", Namespace: "default", + Annotations: map[string]string{ + greenhousev1alpha1.AllowCreateAnnotation: "true", + }, }, Spec: greenhousev1alpha1.PluginSpec{ PluginDefinition: "myuiplugin", diff --git a/pkg/test/resources.go b/pkg/test/resources.go index 94341973b..77a001181 100644 --- a/pkg/test/resources.go +++ b/pkg/test/resources.go @@ -242,6 +242,17 @@ func NewPlugin(ctx context.Context, name, namespace string, opts ...func(*greenh return plugin } +// WithAnnotation sets the annotation for plugin +func WithAnnotation(key, value string) func(plugin *greenhousev1alpha1.Plugin) { + return func(plugin *greenhousev1alpha1.Plugin) { + if plugin.Annotations == nil { + plugin.Annotations = map[string]string{} + } + + plugin.Annotations[key] = value + } +} + // WithRules overrides the default rules of a TeamRole func WithRules(rules []rbacv1.PolicyRule) func(*greenhousev1alpha1.TeamRole) { return func(tr *greenhousev1alpha1.TeamRole) {