fix(plugin) Extended test case (#459)

pkg/helm/diff.go

@@ -55,7 +55,7 @@ func (d DiffObjectList) String() string {
 }
 
 // diffAgainstRelease returns the diff between the templated manifest and the manifest of the deployed Helm release.
-func diffAgainstRelease(restClientGetter genericclioptions.RESTClientGetter, namespace, manifest string, helmRelease *release.Release) (DiffObjectList, error) {
+func diffAgainstRelease(restClientGetter genericclioptions.RESTClientGetter, namespace string, helmTemplateRelease *release.Release, helmRelease *release.Release) (DiffObjectList, error) {
 	remoteObjs, err := ObjectMapFromRelease(restClientGetter, helmRelease, nil)
 	if err != nil {
 		return nil, err
@@ -69,11 +69,19 @@ func diffAgainstRelease(restClientGetter genericclioptions.RESTClientGetter, nam
 		maps.Copy(remoteObjs, hookObjs)
 	}
 
-	localObjs, err := ObjectMapFromManifest(restClientGetter, namespace, manifest, nil)
+	localObjs, err := ObjectMapFromManifest(restClientGetter, namespace, helmTemplateRelease.Manifest, nil)
 	if err != nil {
 		return nil, err
 	}
 
+	for _, hook := range helmTemplateRelease.Hooks {
+		hookObjs, err := ObjectMapFromManifest(restClientGetter, namespace, hook.Manifest, nil)
+		if err != nil {
+			return nil, err
+		}
+		maps.Copy(localObjs, hookObjs)
+	}
+
 	// create the set of all keys in local and remote objects
 	keys := make(map[ObjectKey]struct{})
 	for k := range remoteObjs {

pkg/helm/diff_test.go

@@ -78,20 +78,20 @@ var _ = Describe("ensure helm diff against the release manifest works as expecte
 
 	It("should no diff or drift if nothing changes", func() {
 		By("templating the Helm Chart from the Plugin")
-		manifestUT, err := helm.TemplateHelmChartFromPlugin(test.Ctx, test.K8sClient, test.RestClientGetter, pluginDefinitionUT, pluginUT)
+		templateUT, err := helm.TemplateHelmChartFromPlugin(test.Ctx, test.K8sClient, test.RestClientGetter, pluginDefinitionUT, pluginUT)
 		Expect(err).NotTo(HaveOccurred(), "there should be no error templating the helm chart")
 
 		By("retrieving the Release for the Plugin")
 		releaseUT, err := helm.GetReleaseForHelmChartFromPlugin(test.Ctx, test.RestClientGetter, pluginUT)
 		Expect(err).NotTo(HaveOccurred(), "there should be no error getting the release for the helm chart")
 
 		By("diffing the manifest against the helm release")
-		diff, err := helm.ExportDiffAgainstRelease(test.RestClientGetter, namespace, manifestUT, releaseUT)
+		diff, err := helm.ExportDiffAgainstRelease(test.RestClientGetter, namespace, templateUT, releaseUT)
 		Expect(err).NotTo(HaveOccurred(), "there should be no error diffing the manifest against the helm release")
 		Expect(diff).To(BeEmpty(), "the diff should be empty")
 
 		By("diffing the manifest against the live objects")
-		diff, err = helm.ExportDiffAgainstLiveObjects(test.RestClientGetter, namespace, manifestUT)
+		diff, err = helm.ExportDiffAgainstLiveObjects(test.RestClientGetter, namespace, templateUT.Manifest)
 		Expect(err).NotTo(HaveOccurred(), "there should be no error diffing the manifest against the helm release")
 		Expect(diff).To(BeEmpty(), "the diff should be empty")
 	})
@@ -106,20 +106,20 @@ var _ = Describe("ensure helm diff against the release manifest works as expecte
 		}
 
 		By("templating the Helm Chart from the Plugin")
-		manifestUT, err := helm.TemplateHelmChartFromPlugin(test.Ctx, test.K8sClient, test.RestClientGetter, pluginDefinitionUT, pluginUT)
+		templateUT, err := helm.TemplateHelmChartFromPlugin(test.Ctx, test.K8sClient, test.RestClientGetter, pluginDefinitionUT, pluginUT)
 		Expect(err).NotTo(HaveOccurred(), "there should be no error templating the helm chart")
 
 		By("retrieving the Release for the Plugin")
 		releaseUT, err := helm.GetReleaseForHelmChartFromPlugin(test.Ctx, test.RestClientGetter, pluginUT)
 		Expect(err).NotTo(HaveOccurred(), "there should be no error getting the release for the helm chart")
 
 		By("diffing the manifest against the helm release")
-		diff, err := helm.ExportDiffAgainstRelease(test.RestClientGetter, namespace, manifestUT, releaseUT)
+		diff, err := helm.ExportDiffAgainstRelease(test.RestClientGetter, namespace, templateUT, releaseUT)
 		Expect(err).NotTo(HaveOccurred(), "there should be no error diffing the manifest against the helm release")
 		Expect(diff).To(ContainSubstring("3.19"), "the diff should not be empty")
 
 		By("diffing the manifest against the live objects")
-		diff, err = helm.ExportDiffAgainstLiveObjects(test.RestClientGetter, namespace, manifestUT)
+		diff, err = helm.ExportDiffAgainstLiveObjects(test.RestClientGetter, namespace, templateUT.Manifest)
 		Expect(err).NotTo(HaveOccurred(), "there should be no error diffing the manifest against the helm release")
 		Expect(diff).To(ContainSubstring("3.19"), "the diff should not be empty")
 	})
@@ -158,20 +158,20 @@ var _ = Describe("ensure helm diff against the release manifest works as expecte
 		Expect(test.K8sClient.Update(test.Ctx, podUT)).To(Succeed(), "the pod should be updated")
 
 		By("templating the Helm Chart from the Plugin")
-		manifestUT, err := helm.TemplateHelmChartFromPlugin(test.Ctx, test.K8sClient, test.RestClientGetter, pluginDefinitionUT, pluginUT)
+		templateUT, err := helm.TemplateHelmChartFromPlugin(test.Ctx, test.K8sClient, test.RestClientGetter, pluginDefinitionUT, pluginUT)
 		Expect(err).NotTo(HaveOccurred(), "there should be no error templating the helm chart")
 
 		By("retrieving the Release for the Plugin")
 		releaseUT, err := helm.GetReleaseForHelmChartFromPlugin(test.Ctx, test.RestClientGetter, pluginUT)
 		Expect(err).NotTo(HaveOccurred(), "there should be no error getting the release for the helm chart")
 
 		By("diffing the manifest against the helm release")
-		diff, err := helm.ExportDiffAgainstRelease(test.RestClientGetter, namespace, manifestUT, releaseUT)
+		diff, err := helm.ExportDiffAgainstRelease(test.RestClientGetter, namespace, templateUT, releaseUT)
 		Expect(err).NotTo(HaveOccurred(), "there should be no error diffing the manifest against the helm release")
 		Expect(diff).To(BeEmpty(), "the diff should be empty")
 
 		By("diffing the manifest against the live objects")
-		diff, err = helm.ExportDiffAgainstLiveObjects(test.RestClientGetter, namespace, manifestUT)
+		diff, err = helm.ExportDiffAgainstLiveObjects(test.RestClientGetter, namespace, templateUT.Manifest)
 		Expect(err).NotTo(HaveOccurred(), "there should be no error diffing the manifest against the helm release")
 		Expect(diff).To(ContainSubstring("3.17"), "the diff should not be empty")
 	})
@@ -413,7 +413,7 @@ var _ = Describe("Ensure helm diff does not leak secrets", Ordered, func() {
 
 			helmRelease := &release.Release{Manifest: manifest}
 
-			diffs, err := helm.ExportDiffAgainstRelease(test.RestClientGetter, namespace, manifest, helmRelease)
+			diffs, err := helm.ExportDiffAgainstRelease(test.RestClientGetter, namespace, helmRelease, helmRelease)
 			Expect(err).NotTo(HaveOccurred(), "there should be no error diffing the helm release")
 			Expect(diffs).To(BeEmpty(), "the diff should be empty")
 			Expect(diffs).NotTo(ContainSubstring("dGVzdC12YWx1ZQ=="), "the diff should not contain the original value for test")

pkg/helm/helm.go

@@ -163,12 +163,12 @@ func DiffChartToDeployedResources(ctx context.Context, local client.Client, rest
 		return nil, true, nil
 	}
 
-	manifest, err := TemplateHelmChartFromPlugin(ctx, local, restClientGetter, pluginDefinition, plugin)
+	helmTemplateRelease, err := TemplateHelmChartFromPlugin(ctx, local, restClientGetter, pluginDefinition, plugin)
 	if err != nil {
 		return nil, false, err
 	}
 
-	diffObjects, err := diffAgainstRelease(restClientGetter, plugin.GetReleaseNamespace(), manifest, helmRelease)
+	diffObjects, err := diffAgainstRelease(restClientGetter, plugin.GetReleaseNamespace(), helmTemplateRelease, helmRelease)
 	if err != nil {
 		return nil, false, err
 	}
@@ -189,7 +189,7 @@ func DiffChartToDeployedResources(ctx context.Context, local client.Client, rest
 		return nil, false, nil
 	}
 
-	diffObjects, err = diffAgainstLiveObjects(restClientGetter, plugin.GetReleaseNamespace(), manifest)
+	diffObjects, err = diffAgainstLiveObjects(restClientGetter, plugin.GetReleaseNamespace(), helmTemplateRelease.Manifest)
 	if err != nil {
 		return nil, false, err
 	}
@@ -272,12 +272,12 @@ func GetReleaseForHelmChartFromPlugin(_ context.Context, restClientGetter generi
 }
 
 // TemplateHelmChartFromPlugin returns the rendered manifest or an error.
-func TemplateHelmChartFromPlugin(ctx context.Context, local client.Client, restClientGetter genericclioptions.RESTClientGetter, pluginDefinition *greenhousev1alpha1.PluginDefinition, plugin *greenhousev1alpha1.Plugin) (string, error) {
+func TemplateHelmChartFromPlugin(ctx context.Context, local client.Client, restClientGetter genericclioptions.RESTClientGetter, pluginDefinition *greenhousev1alpha1.PluginDefinition, plugin *greenhousev1alpha1.Plugin) (*release.Release, error) {
 	helmRelease, err := installRelease(ctx, local, restClientGetter, pluginDefinition, plugin, true)
 	if err != nil {
-		return "", err
+		return nil, err
 	}
-	return helmRelease.Manifest, nil
+	return helmRelease, nil
 }
 
 type ChartLoaderFunc func(name string) (*chart.Chart, error)

pkg/test/fixtures/testHook/Chart.yaml

@@ -2,8 +2,8 @@
 # SPDX-License-Identifier: Apache-2.0
 
 apiVersion: v2
-name: test-redis-hook
-description: Test for hook plugin deployment with redis image
+name: test-hook
+description: Test for hook plugin deployment
 
 # A chart can be either an 'application' or a 'library' chart.
 #

pkg/test/fixtures/testHook/templates/hook.yaml

@@ -1,16 +1,17 @@
+{{ if .Values.hook_enabled }}
 apiVersion: batch/v1
 kind: Job
 metadata:
-  name: post-delete-job
+  name: hook-job
   annotations:
-    "helm.sh/hook": post-delete
-    "helm.sh/hook-weight": "-5"
-    "helm.sh/hook-delete-policy": hook-succeeded
+    "helm.sh/hook": post-install, post-upgrade
+    "helm.sh/hook-weight": "10"
 spec:
   template:
     spec:
       containers:
-        - name: pre-install-container
-          image: busybox
-          command: ['sh', '-c', 'echo Pre-install hook executed!']
-      restartPolicy: Never
+        - name: pre-update-container
+          image: alpine:{{.Values.imageTag}}
+          command: ['sh', '-c', 'sleep 5']
+      restartPolicy: Never
+{{ end }}

pkg/test/fixtures/testHook/templates/pod.yaml

@@ -4,7 +4,6 @@
 apiVersion: v1
 kind: Pod
 metadata:
-  creationTimestamp: null
   labels:
     run: alpine
   name: alpine

pkg/test/fixtures/testHook/values.yaml

@@ -2,3 +2,5 @@
 # SPDX-License-Identifier: Apache-2.0
 
 imageTag: 3.18
+
+hook_enabled: false

test/e2e/fixtures/fixtures.go

@@ -80,7 +80,7 @@ var TestHookPluginDefinition = &greenhousev1alpha1.PluginDefinition{
 		APIVersion: greenhousev1alpha1.GroupVersion.String(),
 	},
 	ObjectMeta: metav1.ObjectMeta{
-		Name:      "test-redis-hooks",
+		Name:      "test-hooks",
 		Namespace: test.TestNamespace,
 	},
 	Spec: greenhousev1alpha1.PluginDefinitionSpec{
@@ -99,7 +99,13 @@ var TestHookPluginDefinition = &greenhousev1alpha1.PluginDefinition{
 			Name:    "supernova",
 			Version: "latest",
 		},
-		Options: []greenhousev1alpha1.PluginOption{},
+		Options: []greenhousev1alpha1.PluginOption{
+			{
+				Name:    "hook_enabled",
+				Type:    "bool",
+				Default: &apiextensionsv1.JSON{Raw: []byte("false")},
+			},
+		},
 	},
 }
 

test/e2e/plugin_lifecycle_test.go

@@ -11,6 +11,7 @@ import (
 	. "github.com/onsi/gomega"
 	. "github.com/onsi/gomega/gstruct"
 	appsv1 "k8s.io/api/apps/v1"
+	batchv1 "k8s.io/api/batch/v1"
 	v1 "k8s.io/api/core/v1"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -24,7 +25,7 @@ import (
 )
 
 var _ = Describe("PluginLifecycle", Ordered, func() {
-	Context("without webhook", func() {
+	Context("without helm hook", func() {
 		It("should deploy the plugin", func() {
 
 			const clusterName = "test-cluster-a"
@@ -149,7 +150,7 @@ var _ = Describe("PluginLifecycle", Ordered, func() {
 		})
 	})
 
-	Context("with webhooks", func() {
+	Context("with helm lifecycle hooks", func() {
 		It("should deploy the plugin", func() {
 
 			const clusterName = "test-cluster-b"
@@ -200,19 +201,29 @@ var _ = Describe("PluginLifecycle", Ordered, func() {
 
 			// Creating plugin
 			err = test.K8sClient.Create(ctx, testPlugin)
+
+			// Check jobs
+			SetDefaultEventuallyTimeout(240 * time.Second)
+			jobList := &batchv1.JobList{}
+			err = remoteClient.List(ctx, jobList, client.InNamespace(setup.Namespace()))
+			Expect(err).NotTo(HaveOccurred())
+			Expect(len(jobList.Items)).To(BeEquivalentTo(0))
+
+			// Check plugin list
+			var plugin greenhousev1alpha1.Plugin
 			Expect(err).NotTo(HaveOccurred())
 			Eventually(func(g Gomega) {
 				err = test.K8sClient.List(ctx, pluginList)
 				g.Expect(err).NotTo(HaveOccurred())
 				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"))
+				plugin = pluginList.Items[0]
 			}).Should(Succeed())
 
 			// Checking pod
 			err = remoteClient.List(ctx, podList, client.InNamespace(setup.Namespace()))
 			Expect(err).NotTo(HaveOccurred())
-			SetDefaultEventuallyTimeout(60 * time.Second)
 			Eventually(func(g Gomega) {
 				err = remoteClient.List(ctx, podList, client.InNamespace(setup.Namespace()))
 				g.Expect(err).NotTo(HaveOccurred())
@@ -228,6 +239,19 @@ var _ = Describe("PluginLifecycle", Ordered, func() {
 				}
 			}
 			Expect(podExists).To(BeTrue())
+
+			// Update plugin value
+			plugin.Spec.OptionValues[4].Value.Raw = []byte("true")
+			err = test.K8sClient.Update(ctx, &plugin)
+			Expect(err).NotTo(HaveOccurred())
+
+			// Check jobs
+			Eventually(func(g Gomega) {
+				err = remoteClient.List(ctx, jobList, client.InNamespace(setup.Namespace()))
+				g.Expect(err).NotTo(HaveOccurred())
+				g.Expect(len(jobList.Items)).To(BeEquivalentTo(1))
+			}).Should(Succeed())
+
 		})
 	})
 })