From 67080b13988e24e2ec68ec17ae9a2fc3a76ef422 Mon Sep 17 00:00:00 2001 From: abhijith-darshan Date: Tue, 14 Apr 2026 05:18:58 +0200 Subject: [PATCH] (chore): add exposed service to checksum this effectively triggers reconcile on plugins tracking referred plugins Signed-off-by: abhijith-darshan --- e2e/plugin/e2e_test.go | 8 +- e2e/plugin/scenarios/plugin_integration.go | 152 +++++++++++++++++- .../controller/plugin/plugin_controller.go | 4 +- .../plugin/plugin_controller_flux.go | 20 ++- internal/helm/helm.go | 21 ++- internal/helm/helm_test.go | 4 +- pkg/cel/evaluator.go | 2 + 7 files changed, 190 insertions(+), 21 deletions(-) diff --git a/e2e/plugin/e2e_test.go b/e2e/plugin/e2e_test.go index bc7b32d8c..75ffc3962 100644 --- a/e2e/plugin/e2e_test.go +++ b/e2e/plugin/e2e_test.go @@ -103,7 +103,7 @@ var _ = Describe("Plugin E2E", Ordered, func() { scenarios.FluxControllerPluginDeletePolicyRetain(ctx, adminClient, env, remoteClusterName, team.Name) }) - It("should resolve option values from direct plugin reference", func() { + It("should onboard remote OIDC cluster", Label("scenario:p2p"), func() { By("setting up cluster role binding for OIDC on remote cluster") expect.SetupOIDCClusterRoleBinding(ctx, remoteClient, remoteOIDCClusterRoleBindingName, remoteIntegrationCluster, env.TestNamespace) @@ -118,7 +118,9 @@ var _ = Describe("Plugin E2E", Ordered, func() { By("verifying the cluster status is ready") shared.ClusterIsReady(ctx, adminClient, remoteIntegrationCluster, env.TestNamespace) + }) + It("should resolve option values from direct plugin reference", func() { By("executing the plugin integration scenario with direct plugin reference") scenarios.PluginIntegrationByDirectReference(ctx, adminClient, remoteClient, env, remoteIntegrationCluster) }) @@ -127,4 +129,8 @@ var _ = Describe("Plugin E2E", Ordered, func() { By("executing the plugin integration scenario with plugin reference by label selector") scenarios.PluginIntegrationBySelector(ctx, adminClient, remoteClient, env, remoteIntegrationCluster) }) + + It("should reconcile tracking plugin when referenced plugin exposed services change", Label("scenario:p2p-xsvc"), func() { + scenarios.PluginIntegrationExposedServiceChecksum(ctx, adminClient, env, remoteIntegrationCluster) + }) }) diff --git a/e2e/plugin/scenarios/plugin_integration.go b/e2e/plugin/scenarios/plugin_integration.go index d12f03aaf..644630979 100644 --- a/e2e/plugin/scenarios/plugin_integration.go +++ b/e2e/plugin/scenarios/plugin_integration.go @@ -9,6 +9,7 @@ import ( "encoding/json" "math/big" "slices" + "time" helmv2 "github.com/fluxcd/helm-controller/api/v2" . "github.com/onsi/ginkgo/v2" @@ -25,15 +26,18 @@ import ( "github.com/cloudoperators/greenhouse/e2e/plugin/fixtures" "github.com/cloudoperators/greenhouse/e2e/shared" "github.com/cloudoperators/greenhouse/internal/test" + "github.com/cloudoperators/greenhouse/pkg/lifecycle" ) const ( - multiRefPluginLabelKey = "e2e.greenhouse.sap/multi-ref-plugin" - selectorRefPluginA = "selector-ref-plugin-a" - selectorRefPluginB = "selector-ref-plugin-b" - selectorResolverPluginName = "selector-resolver-plugin" - directResolverPluginName = "direct-resolver-plugin" - directReferencePluginName = "direct-reference-plugin" + multiRefPluginLabelKey = "e2e.greenhouse.sap/multi-ref-plugin" + selectorRefPluginA = "selector-ref-plugin-a" + selectorRefPluginB = "selector-ref-plugin-b" + selectorResolverPluginName = "selector-resolver-plugin" + directResolverPluginName = "direct-resolver-plugin" + directReferencePluginName = "direct-reference-plugin" + exposedSvcReferencedPluginName = "exposed-svc-referenced-plugin" + exposedSvcResolverPluginName = "exposed-svc-resolver-plugin" ) func randIntn(n int) int { @@ -405,3 +409,139 @@ func PluginIntegrationBySelector(ctx context.Context, adminClient, remoteClient g.Expect(containsExpectedEnvs(envVars, extraEnvsFromHR)).To(BeTrue(), "the deployment should contain all expected environment variables from the HelmRelease") }).Should(Succeed(), "the deployment should be present in the remote cluster with expected envs") } + +func PluginIntegrationExposedServiceChecksum(ctx context.Context, adminClient client.Client, env *shared.TestEnv, remoteClusterName string) { + By("creating plugin definition for exposed service checksum test") + testPluginDefinition := fixtures.PreparePodInfoPluginDefinition("podinfo-expose", env.TestNamespace, "6.9.0") + err := adminClient.Create(ctx, testPluginDefinition) + Expect(client.IgnoreAlreadyExists(err)).ToNot(HaveOccurred()) + + By("checking the test plugin definition is ready") + Eventually(func(g Gomega) { + err = adminClient.Get(ctx, client.ObjectKeyFromObject(testPluginDefinition), testPluginDefinition) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(testPluginDefinition.Status.IsReadyTrue()).To(BeTrue(), "the plugin definition should be ready") + }).Should(Succeed()) + + By("creating the referenced plugin with an exposed service on port 9898") + referencedPlugin := test.NewPlugin(ctx, exposedSvcReferencedPluginName, env.TestNamespace) + _, err = controllerutil.CreateOrPatch(ctx, adminClient, referencedPlugin, func() error { + referencedPlugin.Spec = test.NewPlugin(ctx, exposedSvcReferencedPluginName, env.TestNamespace, + test.WithPluginDefinition(testPluginDefinition.Name), + test.WithPluginOptionValue("replicaCount", &apiextensionsv1.JSON{Raw: []byte("1")}), + test.WithPluginOptionValue("service.externalPort", &apiextensionsv1.JSON{Raw: []byte("9898")}), + test.WithPluginOptionValue("service.annotations", test.MustReturnJSONFor(map[string]string{ + "greenhouse.sap/expose": "true", + })), + test.WithReleaseName(exposedSvcReferencedPluginName+"-release"), + test.WithReleaseNamespace(exposedSvcReferencedPluginName+"-namespace"), + test.WithCluster(remoteClusterName), + ).Spec + return nil + }) + Expect(err).ToNot(HaveOccurred(), "there should be no error creating the referenced plugin") + + By("checking the referenced plugin is ready and has exposed services populated") + Eventually(func(g Gomega) { + err = adminClient.Get(ctx, client.ObjectKeyFromObject(referencedPlugin), referencedPlugin) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(referencedPlugin.Status.IsReadyTrue()).To(BeTrue(), "the referenced plugin should be ready") + g.Expect(referencedPlugin.Status.ExposedServices).NotTo(BeEmpty(), "the referenced plugin should have exposed services") + g.Expect(referencedPlugin.Status.HelmReleaseStatus).NotTo(BeNil()) + g.Expect(referencedPlugin.Status.HelmReleaseStatus.PluginOptionChecksum).NotTo(BeEmpty(), "the checksum should be set") + }).Should(Succeed(), "the referenced plugin should be ready with exposed services and a checksum") + + By("creating the resolver plugin referencing the port from the referenced plugin's exposed service") + resolverPlugin := test.NewPlugin(ctx, exposedSvcResolverPluginName, env.TestNamespace) + _, err = controllerutil.CreateOrPatch(ctx, adminClient, resolverPlugin, func() error { + resolverPlugin.Spec = test.NewPlugin(ctx, exposedSvcResolverPluginName, env.TestNamespace, + test.WithPluginDefinition(testPluginDefinition.Name), + test.WithPluginOptionValue("replicaCount", &apiextensionsv1.JSON{Raw: []byte("1")}), + test.WithReleaseName(exposedSvcResolverPluginName+"-release"), + test.WithReleaseNamespace(exposedSvcResolverPluginName+"-namespace"), + test.WithCluster(remoteClusterName), + test.WithPluginOptionValueFromRef("service.externalPort", &greenhousev1alpha1.ExternalValueSource{ + Name: referencedPlugin.Name, + Expression: "object.status.exposedServices.transformList(k, v, v.port)[0]", + }), + ).Spec + return nil + }) + Expect(err).ToNot(HaveOccurred(), "there should be no error creating the resolver plugin") + + By("checking the resolver plugin is ready") + Eventually(func(g Gomega) { + err = adminClient.Get(ctx, client.ObjectKeyFromObject(resolverPlugin), resolverPlugin) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(resolverPlugin.Status.IsReadyTrue()).To(BeTrue(), "the resolver plugin should be ready") + }).Should(Succeed(), "the resolver plugin should be ready") + + By("verifying the tracking-id annotation is set on the referenced plugin") + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(referencedPlugin), referencedPlugin)).To(Succeed()) + annotations := referencedPlugin.GetAnnotations() + g.Expect(annotations).NotTo(BeNil()) + g.Expect(annotations[greenhouseapis.AnnotationKeyPluginTackingID]).To(Equal("Plugin/"+resolverPlugin.Name), + "the tracking ID annotation should match the resolver plugin name") + }).Should(Succeed(), "the referenced plugin should have the tracking ID annotation") + + By("verifying the resolver plugin's HelmRelease has the port value from the referenced plugin") + hr := &helmv2.HelmRelease{} + hr.SetName(resolverPlugin.Name) + hr.SetNamespace(resolverPlugin.Namespace) + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(hr), hr)).To(Succeed()) + var valuesMap map[string]any + g.Expect(json.Unmarshal(hr.Spec.Values.Raw, &valuesMap)).To(Succeed()) + svc, ok := valuesMap["service"].(map[string]any) + g.Expect(ok).To(BeTrue(), "service key should be present in HelmRelease values") + g.Expect(svc["externalPort"]).To(BeEquivalentTo(9898), "the resolver plugin should have the port from the referenced plugin") + }).Should(Succeed(), "the resolver HelmRelease should have the port value from the referenced plugin's exposed service") + + By("recording initial checksum and exposed services from the referenced plugin") + err = adminClient.Get(ctx, client.ObjectKeyFromObject(referencedPlugin), referencedPlugin) + Expect(err).NotTo(HaveOccurred()) + initialChecksum := referencedPlugin.Status.HelmReleaseStatus.PluginOptionChecksum + initialExposedServices := referencedPlugin.Status.ExposedServices + + By("updating the referenced plugin's service externalPort to 8080") + _, err = controllerutil.CreateOrPatch(ctx, adminClient, referencedPlugin, func() error { + for i, v := range referencedPlugin.Spec.OptionValues { + if v.Name == "service.externalPort" { + referencedPlugin.Spec.OptionValues[i].Value = &apiextensionsv1.JSON{Raw: []byte("8080")} + break + } + } + return nil + }) + Expect(err).ToNot(HaveOccurred(), "there should be no error updating the referenced plugin's service externalPort") + + By("waiting for the referenced plugin to be ready with the new port") + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(referencedPlugin), referencedPlugin)).To(Succeed()) + g.Expect(referencedPlugin.Status.IsReadyTrue()).To(BeTrue(), "the referenced plugin should be ready after port update") + g.Expect(referencedPlugin.Status.ExposedServices).NotTo(Equal(initialExposedServices), + "the exposed services should reflect the new port") + g.Expect(referencedPlugin.Status.HelmReleaseStatus.PluginOptionChecksum).NotTo(Equal(initialChecksum), + "the checksum should change after the exposed services change") + }).Should(Succeed(), "the referenced plugin should reflect the port change in exposed services and checksum") + + By("verifying the resolver plugin picks up the new port from the referenced plugin's exposed service") + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(resolverPlugin), resolverPlugin)).To(Succeed()) + annotations := resolverPlugin.GetAnnotations() + if annotations == nil { + annotations = make(map[string]string) + } + annotations[lifecycle.ReconcileAnnotation] = metav1.Now().UTC().Format(time.DateTime) + resolverPlugin.SetAnnotations(annotations) + g.Expect(adminClient.Update(ctx, resolverPlugin)).To(Succeed()) + + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(hr), hr)).To(Succeed()) + var valuesMap map[string]any + g.Expect(json.Unmarshal(hr.Spec.Values.Raw, &valuesMap)).To(Succeed()) + svc, ok := valuesMap["service"].(map[string]any) + g.Expect(ok).To(BeTrue(), "service key should be present in HelmRelease values") + g.Expect(svc["externalPort"]).To(BeEquivalentTo(8080), "the resolver plugin should have the updated port") + }).Should(Succeed(), "the resolver plugin's HelmRelease should be updated with the new port from the referenced plugin") +} diff --git a/internal/controller/plugin/plugin_controller.go b/internal/controller/plugin/plugin_controller.go index ca69a1524..e7205523f 100644 --- a/internal/controller/plugin/plugin_controller.go +++ b/internal/controller/plugin/plugin_controller.go @@ -245,7 +245,7 @@ func getExposedServicesForPluginFromHelmRelease(restClientGetter genericclioptio if err != nil { return nil, err } - var exposedServices = make(map[string]greenhousev1alpha1.Service, 0) + var exposedServices = make(map[string]greenhousev1alpha1.Service) if len(exposedServiceList) == 0 { return exposedServices, nil } @@ -295,7 +295,7 @@ func getExposedIngressesForPluginFromHelmRelease(restClientGetter genericcliopti return nil, err } - var exposedServices = make(map[string]greenhousev1alpha1.Service, 0) + var exposedServices = make(map[string]greenhousev1alpha1.Service) for _, ingress := range exposedIngressList { fullURL, err := getURLForExposedIngress(ingress.Object) if err != nil { diff --git a/internal/controller/plugin/plugin_controller_flux.go b/internal/controller/plugin/plugin_controller_flux.go index 6d4fcaa94..abf6297fb 100644 --- a/internal/controller/plugin/plugin_controller_flux.go +++ b/internal/controller/plugin/plugin_controller_flux.go @@ -313,6 +313,12 @@ func (r *PluginReconciler) fetchReleaseStatus(ctx context.Context, } ) + // Capture the old checksum before plugin.Status is overwritten below. + oldChecksum := "" + if plugin.Status.HelmReleaseStatus != nil && plugin.Status.HelmReleaseStatus.PluginOptionChecksum != "" { + oldChecksum = plugin.Status.HelmReleaseStatus.PluginOptionChecksum + } + // early return if the plugin is not backed by a Helm chart, to avoid unnecessary attempts to fetch the Helm release and exposed services if pluginDefinitionSpec.HelmChart == nil { pluginStatus.HelmChart = nil @@ -427,14 +433,13 @@ func (r *PluginReconciler) fetchReleaseStatus(ctx context.Context, pluginStatus.Version = pluginVersion pluginStatus.HelmReleaseStatus = releaseStatus - oldChecksum := "" newChecksum := "" - if plugin.Status.HelmReleaseStatus != nil && plugin.Status.HelmReleaseStatus.PluginOptionChecksum != "" { - oldChecksum = plugin.Status.HelmReleaseStatus.PluginOptionChecksum - } if plugin.Spec.OptionValues != nil { - newChecksum, err = helm.CalculatePluginOptionChecksum(ctx, r.Client, plugin) + // Include exposed services in the checksum + // digest changes when either computed URL or exposed service struct changes (e.g. port or path) to trigger re-reconciliation of tracking plugins. + newChecksum, err = helm.CalculatePluginOptionChecksum(ctx, r.Client, plugin, pluginStatus.ExposedServices) if err != nil { + ctrl.LoggerFrom(ctx).Error(err, "failed to calculate plugin option checksum", "namespace", plugin.Namespace, "name", plugin.Name) releaseStatus.PluginOptionChecksum = "" } else { releaseStatus.PluginOptionChecksum = newChecksum @@ -621,15 +626,14 @@ func (r *PluginReconciler) triggerReconcileForTracker(ctx context.Context, plugi } // Update the resource with reconcile annotation - err = updateResourceWithAnnotation(ctx, r.Client, gvk, key) - - if err != nil { + if err = updateResourceWithAnnotation(ctx, r.Client, gvk, key); err != nil { log.FromContext(ctx).Error(err, "failed to annotate tracking object with reconcile request", "kind", kind, "namespace", plugin.GetNamespace(), "name", name) return err } + ctrl.LoggerFrom(ctx).Info("triggered reconcile request for tracking resource", "kind", kind, "namespace", plugin.GetNamespace(), "name", name) return nil } diff --git a/internal/helm/helm.go b/internal/helm/helm.go index af9970ad6..7d995ac3f 100644 --- a/internal/helm/helm.go +++ b/internal/helm/helm.go @@ -384,9 +384,10 @@ func replaceCustomResourceDefinitions(ctx context.Context, c client.Client, crdL return nil } -// CalculatePluginOptionChecksum calculates a hash of plugin option values. +// CalculatePluginOptionChecksum calculates a hash of plugin option values and exposed services. // Secret-type option values are extracted first and all values are sorted to ensure that order is not important when comparing checksums. -func CalculatePluginOptionChecksum(ctx context.Context, c client.Client, plugin *greenhousev1alpha1.Plugin) (string, error) { +// exposedServices is included in the checksum so that tracking plugins are re-reconciled when exposed service/ingress URLs change. +func CalculatePluginOptionChecksum(ctx context.Context, c client.Client, plugin *greenhousev1alpha1.Plugin, exposedServices map[string]greenhousev1alpha1.Service) (string, error) { values, err := resolvePluginOptionValueFrom(ctx, c, plugin.Namespace, plugin.Spec.OptionValues) if err != nil { return "", err @@ -417,6 +418,22 @@ func CalculatePluginOptionChecksum(ctx context.Context, c client.Client, plugin } } + if len(exposedServices) > 0 { + keys := make([]string, 0, len(exposedServices)) + for k := range exposedServices { + keys = append(keys, k) + } + sort.Strings(keys) + for _, k := range keys { + buf = append(buf, []byte(k)...) // add the computed exposed service URL to buf + svcBytes, err := json.Marshal(exposedServices[k]) + if err != nil { + return "", fmt.Errorf("failed to marshal exposed service %q: %w", k, err) + } + buf = append(buf, svcBytes...) // add the exposed svc struct to buf + } + } + checksum := sha256.Sum256(buf) return hex.EncodeToString(checksum[:]), nil } diff --git a/internal/helm/helm_test.go b/internal/helm/helm_test.go index 156923dea..8a9c4cea4 100644 --- a/internal/helm/helm_test.go +++ b/internal/helm/helm_test.go @@ -236,9 +236,9 @@ var _ = Describe("Plugin option checksum", Ordered, func() { } plugin1.Spec.OptionValues = optionValues1 plugin2.Spec.OptionValues = optionValues2 - hashedValues1, err := helm.CalculatePluginOptionChecksum(test.Ctx, test.K8sClient, &plugin1) + hashedValues1, err := helm.CalculatePluginOptionChecksum(test.Ctx, test.K8sClient, &plugin1, nil) Expect(err).ToNot(HaveOccurred(), "there should be no error calculating plugin option checksum") - hashedValues2, err := helm.CalculatePluginOptionChecksum(test.Ctx, test.K8sClient, &plugin2) + hashedValues2, err := helm.CalculatePluginOptionChecksum(test.Ctx, test.K8sClient, &plugin2, nil) Expect(err).ToNot(HaveOccurred(), "there should be no error calculating plugin option checksum") comparisonResult := hashedValues1 == hashedValues2 diff --git a/pkg/cel/evaluator.go b/pkg/cel/evaluator.go index 18efea1bf..e074fc878 100644 --- a/pkg/cel/evaluator.go +++ b/pkg/cel/evaluator.go @@ -11,6 +11,7 @@ import ( "github.com/google/cel-go/cel" "github.com/google/cel-go/common/types" "github.com/google/cel-go/common/types/ref" + "github.com/google/cel-go/ext" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -111,6 +112,7 @@ func EvaluateWithData(expression string, env *cel.Env, data map[string]any) (any func compileExpression(expression string) (cel.Program, error) { env, err := cel.NewEnv( cel.Variable("object", cel.DynType), + ext.TwoVarComprehensions(), ) if err != nil { return nil, fmt.Errorf("failed to create CEL environment: %w", err)