From 5eb5ab19c276b77554c62d983b4955ade690275d Mon Sep 17 00:00:00 2001 From: Aslak Knutsen Date: Mon, 2 Feb 2026 23:46:39 +0100 Subject: [PATCH 1/3] refactor: extract shared reconciliation logic into pkg/reconcile Move validation, Helm installation, and image digest logic from individual controllers into a shared pkg/reconcile package. This enables code reuse between operator controllers and (future) the install library, ensuring the same code path is used regardless of deployment mode. Changes: - Add pkg/reconcile with IstiodReconciler, CNIReconciler, ZTunnelReconciler - Each reconciler provides ValidateSpec(), Validate(), Install(), Uninstall() - Export ApplyCNIImageDigests() and ApplyZTunnelImageDigests() for reuse - Refactor IstioRevision, IstioCNI, ZTunnel controllers to delegate to shared reconcilers - Update controller tests to use shared reconcilers Design decisions: - Two-tier validation: ValidateSpec() for basic checks, Validate() for K8s API checks (supports library usage without K8s client) - Controller-agnostic error messages (e.g., "version not set" instead of "spec.version not set") Co-authored-by: Cursor noreply@cursor.com Signed-off-by: Aslak Knutsen --- controllers/istiocni/istiocni_controller.go | 110 ++-------- .../istiocni/istiocni_controller_test.go | 18 +- .../istiorevision/istiorevision_controller.go | 101 ++------- .../istiorevision_controller_test.go | 27 ++- controllers/ztunnel/ztunnel_controller.go | 115 ++-------- .../ztunnel/ztunnel_controller_test.go | 16 +- pkg/reconcile/cni.go | 176 +++++++++++++++ pkg/reconcile/cni_test.go | 154 +++++++++++++ pkg/reconcile/istiod.go | 190 ++++++++++++++++ pkg/reconcile/istiod_test.go | 203 ++++++++++++++++++ pkg/reconcile/types.go | 45 ++++ pkg/reconcile/ztunnel.go | 184 ++++++++++++++++ pkg/reconcile/ztunnel_test.go | 154 +++++++++++++ 13 files changed, 1196 insertions(+), 297 deletions(-) create mode 100644 pkg/reconcile/cni.go create mode 100644 pkg/reconcile/cni_test.go create mode 100644 pkg/reconcile/istiod.go create mode 100644 pkg/reconcile/istiod_test.go create mode 100644 pkg/reconcile/types.go create mode 100644 pkg/reconcile/ztunnel.go create mode 100644 pkg/reconcile/ztunnel_test.go diff --git a/controllers/istiocni/istiocni_controller.go b/controllers/istiocni/istiocni_controller.go index db9aaef78e..f679ce1a03 100644 --- a/controllers/istiocni/istiocni_controller.go +++ b/controllers/istiocni/istiocni_controller.go @@ -18,7 +18,6 @@ import ( "context" "errors" "fmt" - "path" "reflect" "github.com/go-logr/logr" @@ -28,12 +27,10 @@ import ( "github.com/istio-ecosystem/sail-operator/pkg/enqueuelogger" "github.com/istio-ecosystem/sail-operator/pkg/errlist" "github.com/istio-ecosystem/sail-operator/pkg/helm" - "github.com/istio-ecosystem/sail-operator/pkg/istiovalues" - "github.com/istio-ecosystem/sail-operator/pkg/istioversion" "github.com/istio-ecosystem/sail-operator/pkg/kube" "github.com/istio-ecosystem/sail-operator/pkg/predicate" + sharedreconcile "github.com/istio-ecosystem/sail-operator/pkg/reconcile" "github.com/istio-ecosystem/sail-operator/pkg/reconciler" - "github.com/istio-ecosystem/sail-operator/pkg/validation" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" @@ -53,11 +50,6 @@ import ( "istio.io/istio/pkg/ptr" ) -const ( - cniReleaseName = "istio-cni" - cniChartName = "cni" -) - // Reconciler reconciles an IstioCNI object type Reconciler struct { client.Client @@ -107,33 +99,19 @@ func (r *Reconciler) Reconcile(ctx context.Context, cni *v1.IstioCNI) (ctrl.Resu } func (r *Reconciler) Finalize(ctx context.Context, cni *v1.IstioCNI) error { - return r.uninstallHelmChart(ctx, cni) + cniReconciler := r.newCNIReconciler() + return cniReconciler.Uninstall(ctx, cni.Spec.Namespace) } func (r *Reconciler) doReconcile(ctx context.Context, cni *v1.IstioCNI) error { log := logf.FromContext(ctx) - if err := r.validate(ctx, cni); err != nil { - return err - } - - log.Info("Installing Helm chart") - return r.installHelmChart(ctx, cni) -} + cniReconciler := r.newCNIReconciler() -func (r *Reconciler) validate(ctx context.Context, cni *v1.IstioCNI) error { - if cni.Spec.Version == "" { - return reconciler.NewValidationError("spec.version not set") - } - if cni.Spec.Namespace == "" { - return reconciler.NewValidationError("spec.namespace not set") - } - if err := validation.ValidateTargetNamespace(ctx, r.Client, cni.Spec.Namespace); err != nil { + if err := cniReconciler.Validate(ctx, cni.Spec.Version, cni.Spec.Namespace); err != nil { return err } - return nil -} -func (r *Reconciler) installHelmChart(ctx context.Context, cni *v1.IstioCNI) error { + log.Info("Installing Helm chart") ownerReference := metav1.OwnerReference{ APIVersion: v1.GroupVersion.String(), Kind: v1.IstioCNIKind, @@ -142,75 +120,17 @@ func (r *Reconciler) installHelmChart(ctx context.Context, cni *v1.IstioCNI) err Controller: ptr.Of(true), BlockOwnerDeletion: ptr.Of(true), } - - version, err := istioversion.Resolve(cni.Spec.Version) - if err != nil { - return fmt.Errorf("failed to resolve IstioCNI version for %q: %w", cni.Name, err) - } - - // get userValues from Istio.spec.values - userValues := cni.Spec.Values - - // apply image digests from configuration, if not already set by user - userValues = applyImageDigests(version, userValues, config.Config) - - // apply vendor-specific default values - userValues, err = istiovalues.ApplyIstioCNIVendorDefaults(version, userValues) - if err != nil { - return fmt.Errorf("failed to apply vendor defaults: %w", err) - } - - // apply userValues on top of defaultValues from profiles - mergedHelmValues, err := istiovalues.ApplyProfilesAndPlatform( - r.Config.ResourceFS, version, r.Config.Platform, r.Config.DefaultProfile, cni.Spec.Profile, helm.FromValues(userValues)) - if err != nil { - return fmt.Errorf("failed to apply profile: %w", err) - } - - _, err = r.ChartManager.UpgradeOrInstallChart( - ctx, r.Config.ResourceFS, r.getChartPath(version), mergedHelmValues, cni.Spec.Namespace, cniReleaseName, &ownerReference) - if err != nil { - return fmt.Errorf("failed to install/update Helm chart %q: %w", cniChartName, err) - } - return nil + return cniReconciler.Install(ctx, cni.Spec.Version, cni.Spec.Namespace, cni.Spec.Values, cni.Spec.Profile, &ownerReference) } -func (r *Reconciler) getChartPath(version string) string { - return path.Join(version, "charts", cniChartName) -} - -func applyImageDigests(version string, values *v1.CNIValues, config config.OperatorConfig) *v1.CNIValues { - imageDigests, digestsDefined := config.ImageDigests[version] - // if we don't have default image digests defined for this version, it's a no-op - if !digestsDefined { - return values - } - - // if a global hub or tag value is configured by the user, don't set image digests - if values != nil && values.Global != nil && (values.Global.Hub != nil || values.Global.Tag != nil) { - return values - } - - if values == nil { - values = &v1.CNIValues{} - } - - // set image digest unless any part of the image has been configured by the user - if values.Cni == nil { - values.Cni = &v1.CNIConfig{} - } - if values.Cni.Image == nil && values.Cni.Hub == nil && values.Cni.Tag == nil { - values.Cni.Image = &imageDigests.CNIImage - } - return values -} - -func (r *Reconciler) uninstallHelmChart(ctx context.Context, cni *v1.IstioCNI) error { - _, err := r.ChartManager.UninstallChart(ctx, cniReleaseName, cni.Spec.Namespace) - if err != nil { - return fmt.Errorf("failed to uninstall Helm chart %q: %w", cniChartName, err) - } - return nil +func (r *Reconciler) newCNIReconciler() *sharedreconcile.CNIReconciler { + return sharedreconcile.NewCNIReconciler(sharedreconcile.Config{ + ResourceFS: r.Config.ResourceFS, + Platform: r.Config.Platform, + DefaultProfile: r.Config.DefaultProfile, + OperatorNamespace: r.Config.OperatorNamespace, + ChartManager: r.ChartManager, + }, r.Client) } // SetupWithManager sets up the controller with the Manager. diff --git a/controllers/istiocni/istiocni_controller_test.go b/controllers/istiocni/istiocni_controller_test.go index ee851d4244..041354b3b9 100644 --- a/controllers/istiocni/istiocni_controller_test.go +++ b/controllers/istiocni/istiocni_controller_test.go @@ -25,6 +25,7 @@ import ( "github.com/istio-ecosystem/sail-operator/pkg/config" "github.com/istio-ecosystem/sail-operator/pkg/istiovalues" "github.com/istio-ecosystem/sail-operator/pkg/istioversion" + sharedreconcile "github.com/istio-ecosystem/sail-operator/pkg/reconcile" "github.com/istio-ecosystem/sail-operator/pkg/scheme" . "github.com/onsi/gomega" appsv1 "k8s.io/api/apps/v1" @@ -77,7 +78,7 @@ func TestValidate(t *testing.T) { }, }, objects: []client.Object{ns}, - expectErr: "spec.version not set", + expectErr: "version not set", }, { name: "no namespace", @@ -90,7 +91,7 @@ func TestValidate(t *testing.T) { }, }, objects: []client.Object{ns}, - expectErr: "spec.namespace not set", + expectErr: "namespace not set", }, { name: "namespace not found", @@ -111,9 +112,14 @@ func TestValidate(t *testing.T) { t.Run(tc.name, func(t *testing.T) { g := NewWithT(t) cl := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(tc.objects...).Build() - r := NewReconciler(cfg, cl, scheme.Scheme, nil) - - err := r.validate(context.TODO(), tc.cni) + cniReconciler := sharedreconcile.NewCNIReconciler(sharedreconcile.Config{ + ResourceFS: cfg.ResourceFS, + Platform: cfg.Platform, + DefaultProfile: cfg.DefaultProfile, + OperatorNamespace: cfg.OperatorNamespace, + }, cl) + + err := cniReconciler.Validate(context.TODO(), tc.cni.Spec.Version, tc.cni.Spec.Namespace) if tc.expectErr == "" { g.Expect(err).ToNot(HaveOccurred()) } else { @@ -496,7 +502,7 @@ func TestApplyImageDigests(t *testing.T) { if err != nil { t.Errorf("failed to resolve IstioCNI version for %q: %v", tc.input.Name, err) } - result := applyImageDigests(version, tc.input.Spec.Values, tc.config) + result := sharedreconcile.ApplyCNIImageDigests(version, tc.input.Spec.Values, tc.config) if diff := cmp.Diff(tc.expectValues, result); diff != "" { t.Errorf("unexpected merge result; diff (-expected, +actual):\n%v", diff) } diff --git a/controllers/istiorevision/istiorevision_controller.go b/controllers/istiorevision/istiorevision_controller.go index c1f786e54e..b43e1a8094 100644 --- a/controllers/istiorevision/istiorevision_controller.go +++ b/controllers/istiorevision/istiorevision_controller.go @@ -18,7 +18,6 @@ import ( "context" "errors" "fmt" - "path" "reflect" "regexp" @@ -31,9 +30,9 @@ import ( "github.com/istio-ecosystem/sail-operator/pkg/helm" "github.com/istio-ecosystem/sail-operator/pkg/kube" predicate2 "github.com/istio-ecosystem/sail-operator/pkg/predicate" + sharedreconcile "github.com/istio-ecosystem/sail-operator/pkg/reconcile" "github.com/istio-ecosystem/sail-operator/pkg/reconciler" "github.com/istio-ecosystem/sail-operator/pkg/revision" - "github.com/istio-ecosystem/sail-operator/pkg/validation" admissionv1 "k8s.io/api/admissionregistration/v1" appsv1 "k8s.io/api/apps/v1" autoscalingv2 "k8s.io/api/autoscaling/v2" @@ -115,57 +114,13 @@ func (r *Reconciler) Reconcile(ctx context.Context, rev *v1.IstioRevision) (ctrl func (r *Reconciler) doReconcile(ctx context.Context, rev *v1.IstioRevision) error { log := logf.FromContext(ctx) - if err := r.validate(ctx, rev); err != nil { - return err - } - - log.Info("Installing Helm chart") - return r.installHelmCharts(ctx, rev) -} - -func (r *Reconciler) Finalize(ctx context.Context, rev *v1.IstioRevision) error { - return r.uninstallHelmCharts(ctx, rev) -} - -func (r *Reconciler) validate(ctx context.Context, rev *v1.IstioRevision) error { - if rev.Spec.Version == "" { - return reconciler.NewValidationError("spec.version not set") - } - if rev.Spec.Namespace == "" { - return reconciler.NewValidationError("spec.namespace not set") - } - if err := validation.ValidateTargetNamespace(ctx, r.Client, rev.Spec.Namespace); err != nil { - return err - } - - if rev.Spec.Values == nil { - return reconciler.NewValidationError("spec.values not set") - } + istiodReconciler := r.newIstiodReconciler() - revName := rev.Spec.Values.Revision - if rev.Name == v1.DefaultRevision && (revName != nil && *revName != "") { - return reconciler.NewValidationError(fmt.Sprintf("spec.values.revision must be \"\" when IstioRevision name is %s", v1.DefaultRevision)) - } else if rev.Name != v1.DefaultRevision && (revName == nil || *revName != rev.Name) { - return reconciler.NewValidationError("spec.values.revision does not match IstioRevision name") - } - - if rev.Spec.Values.Global == nil || rev.Spec.Values.Global.IstioNamespace == nil || *rev.Spec.Values.Global.IstioNamespace != rev.Spec.Namespace { - return reconciler.NewValidationError("spec.values.global.istioNamespace does not match spec.namespace") - } - - tag := v1.IstioRevisionTag{} - if err := r.Client.Get(ctx, types.NamespacedName{Name: rev.Name}, &tag); err == nil { - if validation.ResourceTakesPrecedence(&tag.ObjectMeta, &rev.ObjectMeta) { - return reconciler.NewNameAlreadyExistsError("an IstioRevisionTag exists with this name", nil) - } - } else if !apierrors.IsNotFound(err) { + if err := istiodReconciler.Validate(ctx, rev.Spec.Version, rev.Spec.Namespace, rev.Spec.Values, rev.Name, &rev.ObjectMeta); err != nil { return err } - return nil -} - -func (r *Reconciler) installHelmCharts(ctx context.Context, rev *v1.IstioRevision) error { + log.Info("Installing Helm chart") ownerReference := metav1.OwnerReference{ APIVersion: v1.GroupVersion.String(), Kind: v1.IstioRevisionKind, @@ -174,42 +129,22 @@ func (r *Reconciler) installHelmCharts(ctx context.Context, rev *v1.IstioRevisio Controller: ptr.Of(true), BlockOwnerDeletion: ptr.Of(true), } - - values := helm.FromValues(rev.Spec.Values) - _, err := r.ChartManager.UpgradeOrInstallChart(ctx, r.Config.ResourceFS, r.getChartPath(rev, constants.IstiodChartName), - values, rev.Spec.Namespace, getReleaseName(rev, constants.IstiodChartName), &ownerReference) - if err != nil { - return fmt.Errorf("failed to install/update Helm chart %q: %w", constants.IstiodChartName, err) - } - if rev.Name == v1.DefaultRevision { - _, err := r.ChartManager.UpgradeOrInstallChart(ctx, r.Config.ResourceFS, r.getChartPath(rev, constants.BaseChartName), - values, r.Config.OperatorNamespace, getReleaseName(rev, constants.BaseChartName), &ownerReference) - if err != nil { - return fmt.Errorf("failed to install/update Helm chart %q: %w", constants.BaseChartName, err) - } - } - return nil + return istiodReconciler.Install(ctx, rev.Spec.Version, rev.Spec.Namespace, rev.Spec.Values, rev.Name, &ownerReference) } -func getReleaseName(rev *v1.IstioRevision, chartName string) string { - return fmt.Sprintf("%s-%s", rev.Name, chartName) -} - -func (r *Reconciler) getChartPath(rev *v1.IstioRevision, chartName string) string { - return path.Join(rev.Spec.Version, "charts", chartName) -} - -func (r *Reconciler) uninstallHelmCharts(ctx context.Context, rev *v1.IstioRevision) error { - if _, err := r.ChartManager.UninstallChart(ctx, getReleaseName(rev, constants.IstiodChartName), rev.Spec.Namespace); err != nil { - return fmt.Errorf("failed to uninstall Helm chart %q: %w", constants.IstiodChartName, err) - } - if rev.Name == v1.DefaultRevision { - _, err := r.ChartManager.UninstallChart(ctx, getReleaseName(rev, constants.BaseChartName), r.Config.OperatorNamespace) - if err != nil { - return fmt.Errorf("failed to uninstall Helm chart %q: %w", constants.BaseChartName, err) - } - } - return nil +func (r *Reconciler) Finalize(ctx context.Context, rev *v1.IstioRevision) error { + istiodReconciler := r.newIstiodReconciler() + return istiodReconciler.Uninstall(ctx, rev.Spec.Namespace, rev.Name) +} + +func (r *Reconciler) newIstiodReconciler() *sharedreconcile.IstiodReconciler { + return sharedreconcile.NewIstiodReconciler(sharedreconcile.Config{ + ResourceFS: r.Config.ResourceFS, + Platform: r.Config.Platform, + DefaultProfile: r.Config.DefaultProfile, + OperatorNamespace: r.Config.OperatorNamespace, + ChartManager: r.ChartManager, + }, r.Client) } func (r *Reconciler) mapEndpointSliceToReconcileRequests(ctx context.Context, obj client.Object) []reconcile.Request { diff --git a/controllers/istiorevision/istiorevision_controller_test.go b/controllers/istiorevision/istiorevision_controller_test.go index 598f6c4e05..38c3adf732 100644 --- a/controllers/istiorevision/istiorevision_controller_test.go +++ b/controllers/istiorevision/istiorevision_controller_test.go @@ -25,6 +25,7 @@ import ( "github.com/istio-ecosystem/sail-operator/pkg/config" "github.com/istio-ecosystem/sail-operator/pkg/constants" "github.com/istio-ecosystem/sail-operator/pkg/istioversion" + sharedreconcile "github.com/istio-ecosystem/sail-operator/pkg/reconcile" "github.com/istio-ecosystem/sail-operator/pkg/scheme" . "github.com/onsi/gomega" admissionv1 "k8s.io/api/admissionregistration/v1" @@ -87,7 +88,7 @@ func TestValidate(t *testing.T) { }, }, objects: []client.Object{ns}, - expectErr: "spec.version not set", + expectErr: "version not set", }, { name: "no namespace", @@ -100,7 +101,7 @@ func TestValidate(t *testing.T) { }, }, objects: []client.Object{ns}, - expectErr: "spec.namespace not set", + expectErr: "namespace not set", }, { name: "namespace not found", @@ -111,6 +112,11 @@ func TestValidate(t *testing.T) { Spec: v1.IstioRevisionSpec{ Version: istioversion.Default, Namespace: "istio-system", + Values: &v1.Values{ + Global: &v1.GlobalConfig{ + IstioNamespace: ptr.Of("istio-system"), + }, + }, }, }, objects: []client.Object{}, @@ -128,7 +134,7 @@ func TestValidate(t *testing.T) { }, }, objects: []client.Object{ns}, - expectErr: "spec.values not set", + expectErr: "values not set", }, { name: "invalid istioNamespace", @@ -147,7 +153,7 @@ func TestValidate(t *testing.T) { }, }, objects: []client.Object{ns}, - expectErr: "spec.values.global.istioNamespace does not match spec.namespace", + expectErr: "values.global.istioNamespace does not match namespace", }, { name: "invalid revision default", @@ -167,7 +173,7 @@ func TestValidate(t *testing.T) { }, }, objects: []client.Object{ns}, - expectErr: `spec.values.revision must be "" when IstioRevision name is default`, + expectErr: `values.revision must be "" when revision name is default`, }, { name: "invalid revision non-default", @@ -187,16 +193,21 @@ func TestValidate(t *testing.T) { }, }, objects: []client.Object{ns}, - expectErr: `spec.values.revision does not match IstioRevision name`, + expectErr: `values.revision does not match revision name`, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { g := NewWithT(t) cl := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(tc.objects...).Build() - r := NewReconciler(cfg, cl, scheme.Scheme, nil) + istiodReconciler := sharedreconcile.NewIstiodReconciler(sharedreconcile.Config{ + ResourceFS: cfg.ResourceFS, + Platform: cfg.Platform, + DefaultProfile: cfg.DefaultProfile, + OperatorNamespace: cfg.OperatorNamespace, + }, cl) - err := r.validate(context.TODO(), tc.rev) + err := istiodReconciler.Validate(context.TODO(), tc.rev.Spec.Version, tc.rev.Spec.Namespace, tc.rev.Spec.Values, tc.rev.Name, &tc.rev.ObjectMeta) if tc.expectErr == "" { g.Expect(err).ToNot(HaveOccurred()) } else { diff --git a/controllers/ztunnel/ztunnel_controller.go b/controllers/ztunnel/ztunnel_controller.go index 7108029829..c8b07d0a96 100644 --- a/controllers/ztunnel/ztunnel_controller.go +++ b/controllers/ztunnel/ztunnel_controller.go @@ -18,7 +18,6 @@ import ( "context" "errors" "fmt" - "path" "reflect" "github.com/go-logr/logr" @@ -29,12 +28,10 @@ import ( "github.com/istio-ecosystem/sail-operator/pkg/enqueuelogger" "github.com/istio-ecosystem/sail-operator/pkg/errlist" "github.com/istio-ecosystem/sail-operator/pkg/helm" - "github.com/istio-ecosystem/sail-operator/pkg/istiovalues" - "github.com/istio-ecosystem/sail-operator/pkg/istioversion" "github.com/istio-ecosystem/sail-operator/pkg/kube" "github.com/istio-ecosystem/sail-operator/pkg/predicate" + sharedreconcile "github.com/istio-ecosystem/sail-operator/pkg/reconcile" "github.com/istio-ecosystem/sail-operator/pkg/reconciler" - "github.com/istio-ecosystem/sail-operator/pkg/validation" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" @@ -61,11 +58,6 @@ type Reconciler struct { ChartManager *helm.ChartManager } -const ( - ztunnelChart = "ztunnel" - defaultProfile = "ambient" -) - func NewReconciler(cfg config.ReconcilerConfig, client client.Client, scheme *runtime.Scheme, chartManager *helm.ChartManager) *Reconciler { return &Reconciler{ Config: cfg, @@ -101,33 +93,19 @@ func (r *Reconciler) Reconcile(ctx context.Context, ztunnel *v1.ZTunnel) (ctrl.R } func (r *Reconciler) Finalize(ctx context.Context, ztunnel *v1.ZTunnel) error { - return r.uninstallHelmChart(ctx, ztunnel) + ztunnelReconciler := r.newZTunnelReconciler() + return ztunnelReconciler.Uninstall(ctx, ztunnel.Spec.Namespace) } func (r *Reconciler) doReconcile(ctx context.Context, ztunnel *v1.ZTunnel) error { log := logf.FromContext(ctx) - if err := r.validate(ctx, ztunnel); err != nil { - return err - } - - log.Info("Installing ztunnel Helm chart") - return r.installHelmChart(ctx, ztunnel) -} + ztunnelReconciler := r.newZTunnelReconciler() -func (r *Reconciler) validate(ctx context.Context, ztunnel *v1.ZTunnel) error { - if ztunnel.Spec.Version == "" { - return reconciler.NewValidationError("spec.version not set") - } - if ztunnel.Spec.Namespace == "" { - return reconciler.NewValidationError("spec.namespace not set") - } - if err := validation.ValidateTargetNamespace(ctx, r.Client, ztunnel.Spec.Namespace); err != nil { + if err := ztunnelReconciler.Validate(ctx, ztunnel.Spec.Version, ztunnel.Spec.Namespace); err != nil { return err } - return nil -} -func (r *Reconciler) installHelmChart(ctx context.Context, ztunnel *v1.ZTunnel) error { + log.Info("Installing ztunnel Helm chart") ownerReference := metav1.OwnerReference{ APIVersion: v1.GroupVersion.String(), Kind: v1.ZTunnelKind, @@ -136,80 +114,17 @@ func (r *Reconciler) installHelmChart(ctx context.Context, ztunnel *v1.ZTunnel) Controller: ptr.Of(true), BlockOwnerDeletion: ptr.Of(true), } - - version, err := istioversion.Resolve(ztunnel.Spec.Version) - if err != nil { - return fmt.Errorf("failed to resolve Ztunnel version for %q: %w", ztunnel.Name, err) - } - // get userValues from ztunnel.spec.values - userValues := ztunnel.Spec.Values - if userValues == nil { - userValues = &v1.ZTunnelValues{} - } - - // apply image digests from configuration, if not already set by user - userValues = applyImageDigests(version, userValues, config.Config) - - // apply userValues on top of defaultValues from profiles - mergedHelmValues, err := istiovalues.ApplyProfilesAndPlatform( - r.Config.ResourceFS, version, r.Config.Platform, r.Config.DefaultProfile, defaultProfile, helm.FromValues(userValues)) - if err != nil { - return fmt.Errorf("failed to apply profile: %w", err) - } - - // Apply any user Overrides configured as part of values.ztunnel - // This step was not required for the IstioCNI resource because the Helm templates[*] automatically override values.cni - // [*]https://github.com/istio/istio/blob/0200fd0d4c3963a72f36987c2e8c2887df172abf/manifests/charts/istio-cni/templates/zzy_descope_legacy.yaml#L3 - // However, ztunnel charts do not have such a file, hence we are manually applying the mergeOperation here. - finalHelmValues, err := istiovalues.ApplyUserValues(helm.FromValues(mergedHelmValues), helm.FromValues(userValues.ZTunnel)) - if err != nil { - return fmt.Errorf("failed to apply user overrides: %w", err) - } - - _, err = r.ChartManager.UpgradeOrInstallChart( - ctx, r.Config.ResourceFS, r.getChartPath(version), finalHelmValues, ztunnel.Spec.Namespace, ztunnelChart, &ownerReference) - if err != nil { - return fmt.Errorf("failed to install/update Helm chart %q: %w", ztunnelChart, err) - } - return nil + return ztunnelReconciler.Install(ctx, ztunnel.Spec.Version, ztunnel.Spec.Namespace, ztunnel.Spec.Values, &ownerReference) } -func (r *Reconciler) getChartPath(version string) string { - return path.Join(version, "charts", ztunnelChart) -} - -func applyImageDigests(version string, values *v1.ZTunnelValues, config config.OperatorConfig) *v1.ZTunnelValues { - imageDigests, digestsDefined := config.ImageDigests[version] - // if we don't have default image digests defined for this version, it's a no-op - if !digestsDefined { - return values - } - - // if a global hub or tag value is configured by the user, don't set image digests - if values != nil && values.Global != nil && (values.Global.Hub != nil || values.Global.Tag != nil) { - return values - } - - if values == nil { - values = &v1.ZTunnelValues{} - } - - // set image digest unless any part of the image has been configured by the user - if values.ZTunnel == nil { - values.ZTunnel = &v1.ZTunnelConfig{} - } - if values.ZTunnel.Image == nil && values.ZTunnel.Hub == nil && values.ZTunnel.Tag == nil { - values.ZTunnel.Image = &imageDigests.ZTunnelImage - } - return values -} - -func (r *Reconciler) uninstallHelmChart(ctx context.Context, ztunnel *v1.ZTunnel) error { - _, err := r.ChartManager.UninstallChart(ctx, ztunnelChart, ztunnel.Spec.Namespace) - if err != nil { - return fmt.Errorf("failed to uninstall Helm chart %q: %w", ztunnelChart, err) - } - return nil +func (r *Reconciler) newZTunnelReconciler() *sharedreconcile.ZTunnelReconciler { + return sharedreconcile.NewZTunnelReconciler(sharedreconcile.Config{ + ResourceFS: r.Config.ResourceFS, + Platform: r.Config.Platform, + DefaultProfile: r.Config.DefaultProfile, + OperatorNamespace: r.Config.OperatorNamespace, + ChartManager: r.ChartManager, + }, r.Client) } // SetupWithManager sets up the controller with the Manager. diff --git a/controllers/ztunnel/ztunnel_controller_test.go b/controllers/ztunnel/ztunnel_controller_test.go index df879ac684..6e41b4eb1d 100644 --- a/controllers/ztunnel/ztunnel_controller_test.go +++ b/controllers/ztunnel/ztunnel_controller_test.go @@ -25,6 +25,7 @@ import ( v1 "github.com/istio-ecosystem/sail-operator/api/v1" "github.com/istio-ecosystem/sail-operator/pkg/config" "github.com/istio-ecosystem/sail-operator/pkg/istioversion" + sharedreconcile "github.com/istio-ecosystem/sail-operator/pkg/reconcile" "github.com/istio-ecosystem/sail-operator/pkg/scheme" . "github.com/onsi/gomega" appsv1 "k8s.io/api/apps/v1" @@ -81,7 +82,7 @@ func TestValidate(t *testing.T) { }, }, objects: []client.Object{ns}, - expectErr: "spec.version not set", + expectErr: "version not set", }, { name: "no namespace", @@ -94,7 +95,7 @@ func TestValidate(t *testing.T) { }, }, objects: []client.Object{ns}, - expectErr: "spec.namespace not set", + expectErr: "namespace not set", }, { name: "namespace not found", @@ -139,9 +140,14 @@ func TestValidate(t *testing.T) { t.Run(tc.name, func(t *testing.T) { g := NewWithT(t) cl := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(tc.objects...).Build() - r := NewReconciler(cfg, cl, scheme.Scheme, nil) + ztunnelReconciler := sharedreconcile.NewZTunnelReconciler(sharedreconcile.Config{ + ResourceFS: cfg.ResourceFS, + Platform: cfg.Platform, + DefaultProfile: cfg.DefaultProfile, + OperatorNamespace: cfg.OperatorNamespace, + }, cl) - err := r.validate(context.TODO(), tc.ztunnel) + err := ztunnelReconciler.Validate(context.TODO(), tc.ztunnel.Spec.Version, tc.ztunnel.Spec.Namespace) if tc.expectErr == "" { g.Expect(err).ToNot(HaveOccurred()) } else { @@ -519,7 +525,7 @@ func TestApplyImageDigests(t *testing.T) { } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - result := applyImageDigests(tc.input.Spec.Version, tc.input.Spec.Values, tc.config) + result := sharedreconcile.ApplyZTunnelImageDigests(tc.input.Spec.Version, tc.input.Spec.Values, tc.config) if diff := cmp.Diff(tc.expectValues, result); diff != "" { t.Errorf("unexpected merge result; diff (-expected, +actual):\n%v", diff) } diff --git a/pkg/reconcile/cni.go b/pkg/reconcile/cni.go new file mode 100644 index 0000000000..e5887003cf --- /dev/null +++ b/pkg/reconcile/cni.go @@ -0,0 +1,176 @@ +// Copyright Istio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package reconcile + +import ( + "context" + "fmt" + "path" + + v1 "github.com/istio-ecosystem/sail-operator/api/v1" + "github.com/istio-ecosystem/sail-operator/pkg/config" + "github.com/istio-ecosystem/sail-operator/pkg/helm" + "github.com/istio-ecosystem/sail-operator/pkg/istiovalues" + "github.com/istio-ecosystem/sail-operator/pkg/istioversion" + "github.com/istio-ecosystem/sail-operator/pkg/reconciler" + "github.com/istio-ecosystem/sail-operator/pkg/validation" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +const ( + cniReleaseName = "istio-cni" + cniChartName = "cni" +) + +// CNIReconciler handles reconciliation of the istio-cni component. +type CNIReconciler struct { + cfg Config + client client.Client +} + +// NewCNIReconciler creates a new CNIReconciler. +// The client parameter is optional - pass nil when using from the library +// where Kubernetes client operations are not needed. +func NewCNIReconciler(cfg Config, client client.Client) *CNIReconciler { + return &CNIReconciler{ + cfg: cfg, + client: client, + } +} + +// ValidateSpec validates the CNI specification. +// It performs basic validation that doesn't require Kubernetes API access. +func (r *CNIReconciler) ValidateSpec(version, namespace string) error { + if version == "" { + return reconciler.NewValidationError("version not set") + } + if namespace == "" { + return reconciler.NewValidationError("namespace not set") + } + return nil +} + +// Validate performs full validation including Kubernetes API checks. +// This requires a non-nil client to be set. +func (r *CNIReconciler) Validate(ctx context.Context, version, namespace string) error { + // First perform basic validation + if err := r.ValidateSpec(version, namespace); err != nil { + return err + } + + // Skip Kubernetes checks if no client is available + if r.client == nil { + return nil + } + + // Validate target namespace exists + if err := validation.ValidateTargetNamespace(ctx, r.client, namespace); err != nil { + return err + } + + return nil +} + +// ComputeValues computes the final Helm values by applying digests, vendor defaults, and profiles. +func (r *CNIReconciler) ComputeValues(version string, userValues *v1.CNIValues, profile string) (helm.Values, error) { + resolvedVersion, err := istioversion.Resolve(version) + if err != nil { + return nil, fmt.Errorf("failed to resolve CNI version: %w", err) + } + + // Apply image digests from configuration, if not already set by user + userValues = ApplyCNIImageDigests(resolvedVersion, userValues, config.Config) + + // Apply vendor-specific default values + userValues, err = istiovalues.ApplyIstioCNIVendorDefaults(resolvedVersion, userValues) + if err != nil { + return nil, fmt.Errorf("failed to apply vendor defaults: %w", err) + } + + // Apply userValues on top of defaultValues from profiles + mergedHelmValues, err := istiovalues.ApplyProfilesAndPlatform( + r.cfg.ResourceFS, resolvedVersion, r.cfg.Platform, r.cfg.DefaultProfile, profile, helm.FromValues(userValues)) + if err != nil { + return nil, fmt.Errorf("failed to apply profile: %w", err) + } + + return mergedHelmValues, nil +} + +// Install installs or upgrades the istio-cni Helm chart. +func (r *CNIReconciler) Install(ctx context.Context, version, namespace string, values *v1.CNIValues, profile string, ownerRef *metav1.OwnerReference) error { + mergedHelmValues, err := r.ComputeValues(version, values, profile) + if err != nil { + return err + } + + resolvedVersion, err := istioversion.Resolve(version) + if err != nil { + return fmt.Errorf("failed to resolve CNI version: %w", err) + } + + chartPath := path.Join(resolvedVersion, "charts", cniChartName) + _, err = r.cfg.ChartManager.UpgradeOrInstallChart( + ctx, + r.cfg.ResourceFS, + chartPath, + mergedHelmValues, + namespace, + cniReleaseName, + ownerRef, + ) + if err != nil { + return fmt.Errorf("failed to install/update Helm chart %q: %w", cniChartName, err) + } + return nil +} + +// Uninstall removes the istio-cni Helm chart. +func (r *CNIReconciler) Uninstall(ctx context.Context, namespace string) error { + _, err := r.cfg.ChartManager.UninstallChart(ctx, cniReleaseName, namespace) + if err != nil { + return fmt.Errorf("failed to uninstall Helm chart %q: %w", cniChartName, err) + } + return nil +} + +// ApplyCNIImageDigests applies image digests to CNI values if not already set by user. +// This function is exported for use by the controller and library. +func ApplyCNIImageDigests(version string, values *v1.CNIValues, cfg config.OperatorConfig) *v1.CNIValues { + imageDigests, digestsDefined := cfg.ImageDigests[version] + // if we don't have default image digests defined for this version, it's a no-op + if !digestsDefined { + return values + } + + // if a global hub or tag value is configured by the user, don't set image digests + if values != nil && values.Global != nil && (values.Global.Hub != nil || values.Global.Tag != nil) { + return values + } + + if values == nil { + values = &v1.CNIValues{} + } + + // set image digest unless any part of the image has been configured by the user + if values.Cni == nil { + values.Cni = &v1.CNIConfig{} + } + if values.Cni.Image == nil && values.Cni.Hub == nil && values.Cni.Tag == nil { + values.Cni.Image = &imageDigests.CNIImage + } + return values +} diff --git a/pkg/reconcile/cni_test.go b/pkg/reconcile/cni_test.go new file mode 100644 index 0000000000..eabcfa47fd --- /dev/null +++ b/pkg/reconcile/cni_test.go @@ -0,0 +1,154 @@ +// Copyright Istio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package reconcile + +import ( + "testing" + + v1 "github.com/istio-ecosystem/sail-operator/api/v1" + "github.com/istio-ecosystem/sail-operator/pkg/config" + "github.com/istio-ecosystem/sail-operator/pkg/reconciler" + "github.com/stretchr/testify/assert" + + "istio.io/istio/pkg/ptr" +) + +func TestCNIReconciler_ValidateSpec(t *testing.T) { + tests := []struct { + name string + version string + namespace string + wantErr bool + errContains string + }{ + { + name: "missing version", + version: "", + namespace: "istio-system", + wantErr: true, + errContains: "version not set", + }, + { + name: "missing namespace", + version: "v1.24.0", + namespace: "", + wantErr: true, + errContains: "namespace not set", + }, + { + name: "valid", + version: "v1.24.0", + namespace: "istio-system", + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := NewCNIReconciler(Config{}, nil) + err := r.ValidateSpec(tt.version, tt.namespace) + + if tt.wantErr { + assert.Error(t, err) + assert.True(t, reconciler.IsValidationError(err), "expected validation error") + if tt.errContains != "" { + assert.Contains(t, err.Error(), tt.errContains) + } + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestApplyCNIImageDigests(t *testing.T) { + tests := []struct { + name string + version string + values *v1.CNIValues + config config.OperatorConfig + expected *v1.CNIValues + }{ + { + name: "no digests defined", + version: "v1.24.0", + values: nil, + config: config.OperatorConfig{ + ImageDigests: map[string]config.IstioImageConfig{}, + }, + expected: nil, + }, + { + name: "applies digest when values is nil", + version: "v1.24.0", + values: nil, + config: config.OperatorConfig{ + ImageDigests: map[string]config.IstioImageConfig{ + "v1.24.0": {CNIImage: "istio/cni@sha256:abc123"}, + }, + }, + expected: &v1.CNIValues{ + Cni: &v1.CNIConfig{ + Image: ptr.Of("istio/cni@sha256:abc123"), + }, + }, + }, + { + name: "does not override user-set global hub", + version: "v1.24.0", + values: &v1.CNIValues{ + Global: &v1.CNIGlobalConfig{ + Hub: ptr.Of("my-registry.io"), + }, + }, + config: config.OperatorConfig{ + ImageDigests: map[string]config.IstioImageConfig{ + "v1.24.0": {CNIImage: "istio/cni@sha256:abc123"}, + }, + }, + expected: &v1.CNIValues{ + Global: &v1.CNIGlobalConfig{ + Hub: ptr.Of("my-registry.io"), + }, + }, + }, + { + name: "does not override user-set image", + version: "v1.24.0", + values: &v1.CNIValues{ + Cni: &v1.CNIConfig{ + Image: ptr.Of("my-custom-image"), + }, + }, + config: config.OperatorConfig{ + ImageDigests: map[string]config.IstioImageConfig{ + "v1.24.0": {CNIImage: "istio/cni@sha256:abc123"}, + }, + }, + expected: &v1.CNIValues{ + Cni: &v1.CNIConfig{ + Image: ptr.Of("my-custom-image"), + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := ApplyCNIImageDigests(tt.version, tt.values, tt.config) + assert.Equal(t, tt.expected, result) + }) + } +} diff --git a/pkg/reconcile/istiod.go b/pkg/reconcile/istiod.go new file mode 100644 index 0000000000..628282f104 --- /dev/null +++ b/pkg/reconcile/istiod.go @@ -0,0 +1,190 @@ +// Copyright Istio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package reconcile + +import ( + "context" + "fmt" + "path" + + v1 "github.com/istio-ecosystem/sail-operator/api/v1" + "github.com/istio-ecosystem/sail-operator/pkg/constants" + "github.com/istio-ecosystem/sail-operator/pkg/helm" + "github.com/istio-ecosystem/sail-operator/pkg/reconciler" + "github.com/istio-ecosystem/sail-operator/pkg/validation" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// IstiodReconciler handles reconciliation of the istiod component. +type IstiodReconciler struct { + cfg Config + client client.Client +} + +// NewIstiodReconciler creates a new IstiodReconciler. +// The client parameter is optional - pass nil when using from the library +// where Kubernetes client operations are not needed. +func NewIstiodReconciler(cfg Config, client client.Client) *IstiodReconciler { + return &IstiodReconciler{ + cfg: cfg, + client: client, + } +} + +// ValidateSpec validates the istiod specification. +// It performs basic validation that doesn't require Kubernetes API access. +func (r *IstiodReconciler) ValidateSpec(version, namespace string, values *v1.Values, revisionName string) error { + if version == "" { + return reconciler.NewValidationError("version not set") + } + if namespace == "" { + return reconciler.NewValidationError("namespace not set") + } + if values == nil { + return reconciler.NewValidationError("values not set") + } + + // Validate revision name consistency + revName := values.Revision + if revisionName == v1.DefaultRevision && (revName != nil && *revName != "") { + return reconciler.NewValidationError(fmt.Sprintf("values.revision must be \"\" when revision name is %s", v1.DefaultRevision)) + } else if revisionName != v1.DefaultRevision && (revName == nil || *revName != revisionName) { + return reconciler.NewValidationError("values.revision does not match revision name") + } + + // Validate namespace consistency + if values.Global == nil || values.Global.IstioNamespace == nil || *values.Global.IstioNamespace != namespace { + return reconciler.NewValidationError("values.global.istioNamespace does not match namespace") + } + + return nil +} + +// Validate performs full validation including Kubernetes API checks. +// This requires a non-nil client to be set. +func (r *IstiodReconciler) Validate( + ctx context.Context, + version, namespace string, + values *v1.Values, + revisionName string, + revisionMeta *metav1.ObjectMeta, +) error { + // First perform basic validation + if err := r.ValidateSpec(version, namespace, values, revisionName); err != nil { + return err + } + + // Skip Kubernetes checks if no client is available + if r.client == nil { + return nil + } + + // Validate target namespace exists + if err := validation.ValidateTargetNamespace(ctx, r.client, namespace); err != nil { + return err + } + + // Check for name conflicts with IstioRevisionTag (only when revisionMeta is provided) + if revisionMeta != nil { + tag := v1.IstioRevisionTag{} + if err := r.client.Get(ctx, types.NamespacedName{Name: revisionName}, &tag); err == nil { + if validation.ResourceTakesPrecedence(&tag.ObjectMeta, revisionMeta) { + return reconciler.NewNameAlreadyExistsError("an IstioRevisionTag exists with this name", nil) + } + } + } + + return nil +} + +// Install installs or upgrades the istiod Helm charts. +func (r *IstiodReconciler) Install( + ctx context.Context, + version, namespace string, + values *v1.Values, + revisionName string, + ownerRef *metav1.OwnerReference, +) error { + helmValues := helm.FromValues(values) + + // Install istiod chart + istiodChartPath := path.Join(version, "charts", constants.IstiodChartName) + istiodReleaseName := GetReleaseName(revisionName, constants.IstiodChartName) + + _, err := r.cfg.ChartManager.UpgradeOrInstallChart( + ctx, + r.cfg.ResourceFS, + istiodChartPath, + helmValues, + namespace, + istiodReleaseName, + ownerRef, + ) + if err != nil { + return fmt.Errorf("failed to install/update Helm chart %q: %w", constants.IstiodChartName, err) + } + + // Install base chart for default revision + if revisionName == v1.DefaultRevision { + baseChartPath := path.Join(version, "charts", constants.BaseChartName) + baseReleaseName := GetReleaseName(revisionName, constants.BaseChartName) + + _, err := r.cfg.ChartManager.UpgradeOrInstallChart( + ctx, + r.cfg.ResourceFS, + baseChartPath, + helmValues, + r.cfg.OperatorNamespace, + baseReleaseName, + ownerRef, + ) + if err != nil { + return fmt.Errorf("failed to install/update Helm chart %q: %w", constants.BaseChartName, err) + } + } + + return nil +} + +// Uninstall removes the istiod Helm charts. +func (r *IstiodReconciler) Uninstall(ctx context.Context, namespace, revisionName string) error { + // Uninstall istiod chart + istiodReleaseName := GetReleaseName(revisionName, constants.IstiodChartName) + if _, err := r.cfg.ChartManager.UninstallChart(ctx, istiodReleaseName, namespace); err != nil { + return fmt.Errorf("failed to uninstall Helm chart %q: %w", constants.IstiodChartName, err) + } + + // Uninstall base chart for default revision + if revisionName == v1.DefaultRevision { + baseReleaseName := GetReleaseName(revisionName, constants.BaseChartName) + if _, err := r.cfg.ChartManager.UninstallChart(ctx, baseReleaseName, r.cfg.OperatorNamespace); err != nil { + return fmt.Errorf("failed to uninstall Helm chart %q: %w", constants.BaseChartName, err) + } + } + + return nil +} + +// GetReleaseName returns the Helm release name for a given revision and chart. +func GetReleaseName(revisionName, chartName string) string { + return fmt.Sprintf("%s-%s", revisionName, chartName) +} + +// GetChartPath returns the path to a chart for a given version. +func GetChartPath(version, chartName string) string { + return path.Join(version, "charts", chartName) +} diff --git a/pkg/reconcile/istiod_test.go b/pkg/reconcile/istiod_test.go new file mode 100644 index 0000000000..1b9dfb7256 --- /dev/null +++ b/pkg/reconcile/istiod_test.go @@ -0,0 +1,203 @@ +// Copyright Istio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package reconcile + +import ( + "testing" + + v1 "github.com/istio-ecosystem/sail-operator/api/v1" + "github.com/istio-ecosystem/sail-operator/pkg/reconciler" + "github.com/stretchr/testify/assert" + + "istio.io/istio/pkg/ptr" +) + +func TestIstiodReconciler_ValidateSpec(t *testing.T) { + tests := []struct { + name string + version string + namespace string + values *v1.Values + revisionName string + wantErr bool + errContains string + }{ + { + name: "missing version", + version: "", + namespace: "istio-system", + values: &v1.Values{}, + wantErr: true, + errContains: "version not set", + }, + { + name: "missing namespace", + version: "v1.24.0", + namespace: "", + values: &v1.Values{}, + wantErr: true, + errContains: "namespace not set", + }, + { + name: "missing values", + version: "v1.24.0", + namespace: "istio-system", + values: nil, + wantErr: true, + errContains: "values not set", + }, + { + name: "default revision with non-empty revision name in values", + version: "v1.24.0", + namespace: "istio-system", + revisionName: v1.DefaultRevision, + values: &v1.Values{ + Revision: ptr.Of("canary"), + Global: &v1.GlobalConfig{ + IstioNamespace: ptr.Of("istio-system"), + }, + }, + wantErr: true, + errContains: "values.revision must be", + }, + { + name: "non-default revision with mismatched revision name", + version: "v1.24.0", + namespace: "istio-system", + revisionName: "canary", + values: &v1.Values{ + Revision: ptr.Of("other"), + Global: &v1.GlobalConfig{ + IstioNamespace: ptr.Of("istio-system"), + }, + }, + wantErr: true, + errContains: "values.revision does not match revision name", + }, + { + name: "namespace mismatch", + version: "v1.24.0", + namespace: "istio-system", + revisionName: v1.DefaultRevision, + values: &v1.Values{ + Global: &v1.GlobalConfig{ + IstioNamespace: ptr.Of("other-namespace"), + }, + }, + wantErr: true, + errContains: "values.global.istioNamespace does not match namespace", + }, + { + name: "valid default revision", + version: "v1.24.0", + namespace: "istio-system", + revisionName: v1.DefaultRevision, + values: &v1.Values{ + Revision: ptr.Of(""), + Global: &v1.GlobalConfig{ + IstioNamespace: ptr.Of("istio-system"), + }, + }, + wantErr: false, + }, + { + name: "valid canary revision", + version: "v1.24.0", + namespace: "istio-system", + revisionName: "canary", + values: &v1.Values{ + Revision: ptr.Of("canary"), + Global: &v1.GlobalConfig{ + IstioNamespace: ptr.Of("istio-system"), + }, + }, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := NewIstiodReconciler(Config{}, nil) + err := r.ValidateSpec(tt.version, tt.namespace, tt.values, tt.revisionName) + + if tt.wantErr { + assert.Error(t, err) + assert.True(t, reconciler.IsValidationError(err), "expected validation error") + if tt.errContains != "" { + assert.Contains(t, err.Error(), tt.errContains) + } + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestGetReleaseName(t *testing.T) { + tests := []struct { + revisionName string + chartName string + expected string + }{ + { + revisionName: "default", + chartName: "istiod", + expected: "default-istiod", + }, + { + revisionName: "canary", + chartName: "istiod", + expected: "canary-istiod", + }, + { + revisionName: "default", + chartName: "base", + expected: "default-base", + }, + } + + for _, tt := range tests { + t.Run(tt.revisionName+"-"+tt.chartName, func(t *testing.T) { + result := GetReleaseName(tt.revisionName, tt.chartName) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestGetChartPath(t *testing.T) { + tests := []struct { + version string + chartName string + expected string + }{ + { + version: "v1.24.0", + chartName: "istiod", + expected: "v1.24.0/charts/istiod", + }, + { + version: "v1.23.0", + chartName: "base", + expected: "v1.23.0/charts/base", + }, + } + + for _, tt := range tests { + t.Run(tt.version+"-"+tt.chartName, func(t *testing.T) { + result := GetChartPath(tt.version, tt.chartName) + assert.Equal(t, tt.expected, result) + }) + } +} diff --git a/pkg/reconcile/types.go b/pkg/reconcile/types.go new file mode 100644 index 0000000000..71ad1843a4 --- /dev/null +++ b/pkg/reconcile/types.go @@ -0,0 +1,45 @@ +// Copyright Istio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package reconcile provides shared reconciliation logic for Istio components. +// It is used by both the operator controllers and the install library to ensure +// consistent behavior across different deployment modes. +package reconcile + +import ( + "io/fs" + + "github.com/istio-ecosystem/sail-operator/pkg/config" + "github.com/istio-ecosystem/sail-operator/pkg/helm" +) + +// Config holds configuration needed for component reconciliation. +// It contains all the dependencies required by reconcilers to validate, +// compute values, and install Helm charts. +type Config struct { + // ResourceFS is the filesystem containing Istio charts and profiles + ResourceFS fs.FS + + // Platform is the target Kubernetes platform (e.g., OpenShift, vanilla Kubernetes) + Platform config.Platform + + // DefaultProfile is the base profile applied before user-selected profiles + DefaultProfile string + + // OperatorNamespace is the namespace where the operator is running + OperatorNamespace string + + // ChartManager handles Helm chart installation and upgrades + ChartManager *helm.ChartManager +} diff --git a/pkg/reconcile/ztunnel.go b/pkg/reconcile/ztunnel.go new file mode 100644 index 0000000000..7b6b695c12 --- /dev/null +++ b/pkg/reconcile/ztunnel.go @@ -0,0 +1,184 @@ +// Copyright Istio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package reconcile + +import ( + "context" + "fmt" + "path" + + v1 "github.com/istio-ecosystem/sail-operator/api/v1" + "github.com/istio-ecosystem/sail-operator/pkg/config" + "github.com/istio-ecosystem/sail-operator/pkg/helm" + "github.com/istio-ecosystem/sail-operator/pkg/istiovalues" + "github.com/istio-ecosystem/sail-operator/pkg/istioversion" + "github.com/istio-ecosystem/sail-operator/pkg/reconciler" + "github.com/istio-ecosystem/sail-operator/pkg/validation" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +const ( + ztunnelReleaseName = "ztunnel" + ztunnelChartName = "ztunnel" + ztunnelProfile = "ambient" +) + +// ZTunnelReconciler handles reconciliation of the ztunnel component. +type ZTunnelReconciler struct { + cfg Config + client client.Client +} + +// NewZTunnelReconciler creates a new ZTunnelReconciler. +// The client parameter is optional - pass nil when using from the library +// where Kubernetes client operations are not needed. +func NewZTunnelReconciler(cfg Config, client client.Client) *ZTunnelReconciler { + return &ZTunnelReconciler{ + cfg: cfg, + client: client, + } +} + +// ValidateSpec validates the ZTunnel specification. +// It performs basic validation that doesn't require Kubernetes API access. +func (r *ZTunnelReconciler) ValidateSpec(version, namespace string) error { + if version == "" { + return reconciler.NewValidationError("version not set") + } + if namespace == "" { + return reconciler.NewValidationError("namespace not set") + } + return nil +} + +// Validate performs full validation including Kubernetes API checks. +// This requires a non-nil client to be set. +func (r *ZTunnelReconciler) Validate(ctx context.Context, version, namespace string) error { + // First perform basic validation + if err := r.ValidateSpec(version, namespace); err != nil { + return err + } + + // Skip Kubernetes checks if no client is available + if r.client == nil { + return nil + } + + // Validate target namespace exists + if err := validation.ValidateTargetNamespace(ctx, r.client, namespace); err != nil { + return err + } + + return nil +} + +// ComputeValues computes the final Helm values by applying digests, profiles, and user overrides. +func (r *ZTunnelReconciler) ComputeValues(version string, userValues *v1.ZTunnelValues) (helm.Values, error) { + resolvedVersion, err := istioversion.Resolve(version) + if err != nil { + return nil, fmt.Errorf("failed to resolve ZTunnel version: %w", err) + } + + if userValues == nil { + userValues = &v1.ZTunnelValues{} + } + + // Apply image digests from configuration, if not already set by user + userValues = ApplyZTunnelImageDigests(resolvedVersion, userValues, config.Config) + + // Apply userValues on top of defaultValues from profiles + mergedHelmValues, err := istiovalues.ApplyProfilesAndPlatform( + r.cfg.ResourceFS, resolvedVersion, r.cfg.Platform, r.cfg.DefaultProfile, ztunnelProfile, helm.FromValues(userValues)) + if err != nil { + return nil, fmt.Errorf("failed to apply profile: %w", err) + } + + // Apply any user Overrides configured as part of values.ztunnel + // This step was not required for the IstioCNI resource because the Helm templates[*] automatically override values.cni + // [*]https://github.com/istio/istio/blob/0200fd0d4c3963a72f36987c2e8c2887df172abf/manifests/charts/istio-cni/templates/zzy_descope_legacy.yaml#L3 + // However, ztunnel charts do not have such a file, hence we are manually applying the mergeOperation here. + finalHelmValues, err := istiovalues.ApplyUserValues(helm.FromValues(mergedHelmValues), helm.FromValues(userValues.ZTunnel)) + if err != nil { + return nil, fmt.Errorf("failed to apply user overrides: %w", err) + } + + return finalHelmValues, nil +} + +// Install installs or upgrades the ztunnel Helm chart. +func (r *ZTunnelReconciler) Install(ctx context.Context, version, namespace string, values *v1.ZTunnelValues, ownerRef *metav1.OwnerReference) error { + finalHelmValues, err := r.ComputeValues(version, values) + if err != nil { + return err + } + + resolvedVersion, err := istioversion.Resolve(version) + if err != nil { + return fmt.Errorf("failed to resolve ZTunnel version: %w", err) + } + + chartPath := path.Join(resolvedVersion, "charts", ztunnelChartName) + _, err = r.cfg.ChartManager.UpgradeOrInstallChart( + ctx, + r.cfg.ResourceFS, + chartPath, + finalHelmValues, + namespace, + ztunnelReleaseName, + ownerRef, + ) + if err != nil { + return fmt.Errorf("failed to install/update Helm chart %q: %w", ztunnelChartName, err) + } + return nil +} + +// Uninstall removes the ztunnel Helm chart. +func (r *ZTunnelReconciler) Uninstall(ctx context.Context, namespace string) error { + _, err := r.cfg.ChartManager.UninstallChart(ctx, ztunnelReleaseName, namespace) + if err != nil { + return fmt.Errorf("failed to uninstall Helm chart %q: %w", ztunnelChartName, err) + } + return nil +} + +// ApplyZTunnelImageDigests applies image digests to ZTunnel values if not already set by user. +// This function is exported for use by the controller and library. +func ApplyZTunnelImageDigests(version string, values *v1.ZTunnelValues, cfg config.OperatorConfig) *v1.ZTunnelValues { + imageDigests, digestsDefined := cfg.ImageDigests[version] + // if we don't have default image digests defined for this version, it's a no-op + if !digestsDefined { + return values + } + + // if a global hub or tag value is configured by the user, don't set image digests + if values != nil && values.Global != nil && (values.Global.Hub != nil || values.Global.Tag != nil) { + return values + } + + if values == nil { + values = &v1.ZTunnelValues{} + } + + // set image digest unless any part of the image has been configured by the user + if values.ZTunnel == nil { + values.ZTunnel = &v1.ZTunnelConfig{} + } + if values.ZTunnel.Image == nil && values.ZTunnel.Hub == nil && values.ZTunnel.Tag == nil { + values.ZTunnel.Image = &imageDigests.ZTunnelImage + } + return values +} diff --git a/pkg/reconcile/ztunnel_test.go b/pkg/reconcile/ztunnel_test.go new file mode 100644 index 0000000000..d84cdc0c83 --- /dev/null +++ b/pkg/reconcile/ztunnel_test.go @@ -0,0 +1,154 @@ +// Copyright Istio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package reconcile + +import ( + "testing" + + v1 "github.com/istio-ecosystem/sail-operator/api/v1" + "github.com/istio-ecosystem/sail-operator/pkg/config" + "github.com/istio-ecosystem/sail-operator/pkg/reconciler" + "github.com/stretchr/testify/assert" + + "istio.io/istio/pkg/ptr" +) + +func TestZTunnelReconciler_ValidateSpec(t *testing.T) { + tests := []struct { + name string + version string + namespace string + wantErr bool + errContains string + }{ + { + name: "missing version", + version: "", + namespace: "istio-system", + wantErr: true, + errContains: "version not set", + }, + { + name: "missing namespace", + version: "v1.24.0", + namespace: "", + wantErr: true, + errContains: "namespace not set", + }, + { + name: "valid", + version: "v1.24.0", + namespace: "istio-system", + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := NewZTunnelReconciler(Config{}, nil) + err := r.ValidateSpec(tt.version, tt.namespace) + + if tt.wantErr { + assert.Error(t, err) + assert.True(t, reconciler.IsValidationError(err), "expected validation error") + if tt.errContains != "" { + assert.Contains(t, err.Error(), tt.errContains) + } + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestApplyZTunnelImageDigests(t *testing.T) { + tests := []struct { + name string + version string + values *v1.ZTunnelValues + config config.OperatorConfig + expected *v1.ZTunnelValues + }{ + { + name: "no digests defined", + version: "v1.24.0", + values: nil, + config: config.OperatorConfig{ + ImageDigests: map[string]config.IstioImageConfig{}, + }, + expected: nil, + }, + { + name: "applies digest when values is nil", + version: "v1.24.0", + values: nil, + config: config.OperatorConfig{ + ImageDigests: map[string]config.IstioImageConfig{ + "v1.24.0": {ZTunnelImage: "istio/ztunnel@sha256:abc123"}, + }, + }, + expected: &v1.ZTunnelValues{ + ZTunnel: &v1.ZTunnelConfig{ + Image: ptr.Of("istio/ztunnel@sha256:abc123"), + }, + }, + }, + { + name: "does not override user-set global hub", + version: "v1.24.0", + values: &v1.ZTunnelValues{ + Global: &v1.ZTunnelGlobalConfig{ + Hub: ptr.Of("my-registry.io"), + }, + }, + config: config.OperatorConfig{ + ImageDigests: map[string]config.IstioImageConfig{ + "v1.24.0": {ZTunnelImage: "istio/ztunnel@sha256:abc123"}, + }, + }, + expected: &v1.ZTunnelValues{ + Global: &v1.ZTunnelGlobalConfig{ + Hub: ptr.Of("my-registry.io"), + }, + }, + }, + { + name: "does not override user-set image", + version: "v1.24.0", + values: &v1.ZTunnelValues{ + ZTunnel: &v1.ZTunnelConfig{ + Image: ptr.Of("my-custom-image"), + }, + }, + config: config.OperatorConfig{ + ImageDigests: map[string]config.IstioImageConfig{ + "v1.24.0": {ZTunnelImage: "istio/ztunnel@sha256:abc123"}, + }, + }, + expected: &v1.ZTunnelValues{ + ZTunnel: &v1.ZTunnelConfig{ + Image: ptr.Of("my-custom-image"), + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := ApplyZTunnelImageDigests(tt.version, tt.values, tt.config) + assert.Equal(t, tt.expected, result) + }) + } +} From e96e291d32c916d997fd45ac166070e98efd308b Mon Sep 17 00:00:00 2001 From: Aslak Knutsen Date: Thu, 5 Feb 2026 22:53:15 +0100 Subject: [PATCH 2/3] refactor(validation): consolidate validation and move CRD-specific checks to controller The validation was awkwardly split between ValidateSpec (no client) and Validate (with client), but both paths actually need a client. This refactoring creates a cleaner separation: - General validations (version/namespace/values checks, target namespace exists) remain in pkg/reconcile - CRD-specific validations (revision name consistency, IstioRevisionTag conflict) move to the controller level Changes: - Remove ValidateSpec from IstiodReconciler, CNIReconciler, ZTunnelReconciler - Collapse validation into single Validate function that always requires client - Add validateRevisionConsistency and validateNoTagConflict to controller - Update tests to reflect new validation structure This enables library consumers to use pkg/reconcile without needing to implement operator-specific validation logic. Signed-off-by: Aslak Knutsen --- .../istiorevision/istiorevision_controller.go | 48 ++++++++- .../istiorevision_controller_test.go | 28 +++-- pkg/reconcile/cni.go | 23 +--- pkg/reconcile/cni_test.go | 35 +++++- pkg/reconcile/istiod.go | 56 +--------- pkg/reconcile/istiod_test.go | 102 +++++++----------- pkg/reconcile/ztunnel.go | 23 +--- pkg/reconcile/ztunnel_test.go | 35 +++++- 8 files changed, 183 insertions(+), 167 deletions(-) diff --git a/controllers/istiorevision/istiorevision_controller.go b/controllers/istiorevision/istiorevision_controller.go index b43e1a8094..b54b55eb52 100644 --- a/controllers/istiorevision/istiorevision_controller.go +++ b/controllers/istiorevision/istiorevision_controller.go @@ -33,6 +33,7 @@ import ( sharedreconcile "github.com/istio-ecosystem/sail-operator/pkg/reconcile" "github.com/istio-ecosystem/sail-operator/pkg/reconciler" "github.com/istio-ecosystem/sail-operator/pkg/revision" + "github.com/istio-ecosystem/sail-operator/pkg/validation" admissionv1 "k8s.io/api/admissionregistration/v1" appsv1 "k8s.io/api/apps/v1" autoscalingv2 "k8s.io/api/autoscaling/v2" @@ -116,7 +117,16 @@ func (r *Reconciler) doReconcile(ctx context.Context, rev *v1.IstioRevision) err log := logf.FromContext(ctx) istiodReconciler := r.newIstiodReconciler() - if err := istiodReconciler.Validate(ctx, rev.Spec.Version, rev.Spec.Namespace, rev.Spec.Values, rev.Name, &rev.ObjectMeta); err != nil { + // CRD-specific validations + if err := r.validateRevisionConsistency(rev); err != nil { + return err + } + if err := r.validateNoTagConflict(ctx, rev); err != nil { + return err + } + + // General validations + if err := istiodReconciler.Validate(ctx, rev.Spec.Version, rev.Spec.Namespace, rev.Spec.Values); err != nil { return err } @@ -147,6 +157,42 @@ func (r *Reconciler) newIstiodReconciler() *sharedreconcile.IstiodReconciler { }, r.Client) } +// validateRevisionConsistency validates that the IstioRevision CR fields are consistent +// with the Helm values. This is CRD-specific validation. +func (r *Reconciler) validateRevisionConsistency(rev *v1.IstioRevision) error { + values := rev.Spec.Values + if values == nil { + return nil // values nil check is done in general validation + } + + // Validate revision name consistency + revName := values.Revision + if rev.Name == v1.DefaultRevision && (revName != nil && *revName != "") { + return reconciler.NewValidationError(fmt.Sprintf("values.revision must be \"\" when revision name is %s", v1.DefaultRevision)) + } else if rev.Name != v1.DefaultRevision && (revName == nil || *revName != rev.Name) { + return reconciler.NewValidationError("values.revision does not match revision name") + } + + // Validate namespace consistency + if values.Global == nil || values.Global.IstioNamespace == nil || *values.Global.IstioNamespace != rev.Spec.Namespace { + return reconciler.NewValidationError("values.global.istioNamespace does not match namespace") + } + + return nil +} + +// validateNoTagConflict checks that no IstioRevisionTag exists with the same name +// as this IstioRevision. This is CRD-specific validation. +func (r *Reconciler) validateNoTagConflict(ctx context.Context, rev *v1.IstioRevision) error { + tag := v1.IstioRevisionTag{} + if err := r.Client.Get(ctx, types.NamespacedName{Name: rev.Name}, &tag); err == nil { + if validation.ResourceTakesPrecedence(&tag.ObjectMeta, &rev.ObjectMeta) { + return reconciler.NewNameAlreadyExistsError("an IstioRevisionTag exists with this name", nil) + } + } + return nil +} + func (r *Reconciler) mapEndpointSliceToReconcileRequests(ctx context.Context, obj client.Object) []reconcile.Request { // EndpointSlices may be owned by an Endpoints resource (if mirrored) which is in turn owned // by an IstioRevision, or in the future, EndpointSlices may be owned directly by an IstioRevision. diff --git a/controllers/istiorevision/istiorevision_controller_test.go b/controllers/istiorevision/istiorevision_controller_test.go index 38c3adf732..3dad5ee093 100644 --- a/controllers/istiorevision/istiorevision_controller_test.go +++ b/controllers/istiorevision/istiorevision_controller_test.go @@ -200,14 +200,28 @@ func TestValidate(t *testing.T) { t.Run(tc.name, func(t *testing.T) { g := NewWithT(t) cl := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(tc.objects...).Build() - istiodReconciler := sharedreconcile.NewIstiodReconciler(sharedreconcile.Config{ - ResourceFS: cfg.ResourceFS, - Platform: cfg.Platform, - DefaultProfile: cfg.DefaultProfile, - OperatorNamespace: cfg.OperatorNamespace, - }, cl) - err := istiodReconciler.Validate(context.TODO(), tc.rev.Spec.Version, tc.rev.Spec.Namespace, tc.rev.Spec.Values, tc.rev.Name, &tc.rev.ObjectMeta) + // Create controller reconciler for CRD-specific validations + reconciler := &Reconciler{ + Client: cl, + Config: cfg, + } + + // Run CRD-specific validations (same order as doReconcile) + var err error + if err = reconciler.validateRevisionConsistency(tc.rev); err == nil { + if err = reconciler.validateNoTagConflict(context.TODO(), tc.rev); err == nil { + // Run general validations + istiodReconciler := sharedreconcile.NewIstiodReconciler(sharedreconcile.Config{ + ResourceFS: cfg.ResourceFS, + Platform: cfg.Platform, + DefaultProfile: cfg.DefaultProfile, + OperatorNamespace: cfg.OperatorNamespace, + }, cl) + err = istiodReconciler.Validate(context.TODO(), tc.rev.Spec.Version, tc.rev.Spec.Namespace, tc.rev.Spec.Values) + } + } + if tc.expectErr == "" { g.Expect(err).ToNot(HaveOccurred()) } else { diff --git a/pkg/reconcile/cni.go b/pkg/reconcile/cni.go index e5887003cf..155ef0e06b 100644 --- a/pkg/reconcile/cni.go +++ b/pkg/reconcile/cni.go @@ -42,8 +42,6 @@ type CNIReconciler struct { } // NewCNIReconciler creates a new CNIReconciler. -// The client parameter is optional - pass nil when using from the library -// where Kubernetes client operations are not needed. func NewCNIReconciler(cfg Config, client client.Client) *CNIReconciler { return &CNIReconciler{ cfg: cfg, @@ -51,30 +49,15 @@ func NewCNIReconciler(cfg Config, client client.Client) *CNIReconciler { } } -// ValidateSpec validates the CNI specification. -// It performs basic validation that doesn't require Kubernetes API access. -func (r *CNIReconciler) ValidateSpec(version, namespace string) error { +// Validate performs general validation of the CNI specification. +// This includes basic field validation and Kubernetes API checks (namespace exists). +func (r *CNIReconciler) Validate(ctx context.Context, version, namespace string) error { if version == "" { return reconciler.NewValidationError("version not set") } if namespace == "" { return reconciler.NewValidationError("namespace not set") } - return nil -} - -// Validate performs full validation including Kubernetes API checks. -// This requires a non-nil client to be set. -func (r *CNIReconciler) Validate(ctx context.Context, version, namespace string) error { - // First perform basic validation - if err := r.ValidateSpec(version, namespace); err != nil { - return err - } - - // Skip Kubernetes checks if no client is available - if r.client == nil { - return nil - } // Validate target namespace exists if err := validation.ValidateTargetNamespace(ctx, r.client, namespace); err != nil { diff --git a/pkg/reconcile/cni_test.go b/pkg/reconcile/cni_test.go index eabcfa47fd..5a62022d3a 100644 --- a/pkg/reconcile/cni_test.go +++ b/pkg/reconcile/cni_test.go @@ -15,21 +15,33 @@ package reconcile import ( + "context" "testing" v1 "github.com/istio-ecosystem/sail-operator/api/v1" "github.com/istio-ecosystem/sail-operator/pkg/config" "github.com/istio-ecosystem/sail-operator/pkg/reconciler" + "github.com/istio-ecosystem/sail-operator/pkg/scheme" "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client/fake" "istio.io/istio/pkg/ptr" ) -func TestCNIReconciler_ValidateSpec(t *testing.T) { +func TestCNIReconciler_Validate(t *testing.T) { + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "istio-system", + }, + } + tests := []struct { name string version string namespace string + nsExists bool wantErr bool errContains string }{ @@ -37,6 +49,7 @@ func TestCNIReconciler_ValidateSpec(t *testing.T) { name: "missing version", version: "", namespace: "istio-system", + nsExists: true, wantErr: true, errContains: "version not set", }, @@ -44,21 +57,37 @@ func TestCNIReconciler_ValidateSpec(t *testing.T) { name: "missing namespace", version: "v1.24.0", namespace: "", + nsExists: true, wantErr: true, errContains: "namespace not set", }, + { + name: "namespace not found", + version: "v1.24.0", + namespace: "istio-system", + nsExists: false, + wantErr: true, + errContains: `namespace "istio-system" doesn't exist`, + }, { name: "valid", version: "v1.24.0", namespace: "istio-system", + nsExists: true, wantErr: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - r := NewCNIReconciler(Config{}, nil) - err := r.ValidateSpec(tt.version, tt.namespace) + clientBuilder := fake.NewClientBuilder().WithScheme(scheme.Scheme) + if tt.nsExists { + clientBuilder = clientBuilder.WithObjects(ns) + } + cl := clientBuilder.Build() + + r := NewCNIReconciler(Config{}, cl) + err := r.Validate(context.Background(), tt.version, tt.namespace) if tt.wantErr { assert.Error(t, err) diff --git a/pkg/reconcile/istiod.go b/pkg/reconcile/istiod.go index 628282f104..67bfaed3ab 100644 --- a/pkg/reconcile/istiod.go +++ b/pkg/reconcile/istiod.go @@ -25,7 +25,6 @@ import ( "github.com/istio-ecosystem/sail-operator/pkg/reconciler" "github.com/istio-ecosystem/sail-operator/pkg/validation" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -36,8 +35,6 @@ type IstiodReconciler struct { } // NewIstiodReconciler creates a new IstiodReconciler. -// The client parameter is optional - pass nil when using from the library -// where Kubernetes client operations are not needed. func NewIstiodReconciler(cfg Config, client client.Client) *IstiodReconciler { return &IstiodReconciler{ cfg: cfg, @@ -45,9 +42,11 @@ func NewIstiodReconciler(cfg Config, client client.Client) *IstiodReconciler { } } -// ValidateSpec validates the istiod specification. -// It performs basic validation that doesn't require Kubernetes API access. -func (r *IstiodReconciler) ValidateSpec(version, namespace string, values *v1.Values, revisionName string) error { +// Validate performs general validation of the istiod specification. +// This includes basic field validation and Kubernetes API checks (namespace exists). +// CRD-specific validations (revision name consistency, IstioRevisionTag conflicts) +// should be performed by the controller before calling this method. +func (r *IstiodReconciler) Validate(ctx context.Context, version, namespace string, values *v1.Values) error { if version == "" { return reconciler.NewValidationError("version not set") } @@ -58,56 +57,11 @@ func (r *IstiodReconciler) ValidateSpec(version, namespace string, values *v1.Va return reconciler.NewValidationError("values not set") } - // Validate revision name consistency - revName := values.Revision - if revisionName == v1.DefaultRevision && (revName != nil && *revName != "") { - return reconciler.NewValidationError(fmt.Sprintf("values.revision must be \"\" when revision name is %s", v1.DefaultRevision)) - } else if revisionName != v1.DefaultRevision && (revName == nil || *revName != revisionName) { - return reconciler.NewValidationError("values.revision does not match revision name") - } - - // Validate namespace consistency - if values.Global == nil || values.Global.IstioNamespace == nil || *values.Global.IstioNamespace != namespace { - return reconciler.NewValidationError("values.global.istioNamespace does not match namespace") - } - - return nil -} - -// Validate performs full validation including Kubernetes API checks. -// This requires a non-nil client to be set. -func (r *IstiodReconciler) Validate( - ctx context.Context, - version, namespace string, - values *v1.Values, - revisionName string, - revisionMeta *metav1.ObjectMeta, -) error { - // First perform basic validation - if err := r.ValidateSpec(version, namespace, values, revisionName); err != nil { - return err - } - - // Skip Kubernetes checks if no client is available - if r.client == nil { - return nil - } - // Validate target namespace exists if err := validation.ValidateTargetNamespace(ctx, r.client, namespace); err != nil { return err } - // Check for name conflicts with IstioRevisionTag (only when revisionMeta is provided) - if revisionMeta != nil { - tag := v1.IstioRevisionTag{} - if err := r.client.Get(ctx, types.NamespacedName{Name: revisionName}, &tag); err == nil { - if validation.ResourceTakesPrecedence(&tag.ObjectMeta, revisionMeta) { - return reconciler.NewNameAlreadyExistsError("an IstioRevisionTag exists with this name", nil) - } - } - } - return nil } diff --git a/pkg/reconcile/istiod_test.go b/pkg/reconcile/istiod_test.go index 1b9dfb7256..6486c5ec3e 100644 --- a/pkg/reconcile/istiod_test.go +++ b/pkg/reconcile/istiod_test.go @@ -15,30 +15,42 @@ package reconcile import ( + "context" "testing" v1 "github.com/istio-ecosystem/sail-operator/api/v1" "github.com/istio-ecosystem/sail-operator/pkg/reconciler" + "github.com/istio-ecosystem/sail-operator/pkg/scheme" "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client/fake" "istio.io/istio/pkg/ptr" ) -func TestIstiodReconciler_ValidateSpec(t *testing.T) { +func TestIstiodReconciler_Validate(t *testing.T) { + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "istio-system", + }, + } + tests := []struct { - name string - version string - namespace string - values *v1.Values - revisionName string - wantErr bool - errContains string + name string + version string + namespace string + values *v1.Values + nsExists bool + wantErr bool + errContains string }{ { name: "missing version", version: "", namespace: "istio-system", values: &v1.Values{}, + nsExists: true, wantErr: true, errContains: "version not set", }, @@ -47,6 +59,7 @@ func TestIstiodReconciler_ValidateSpec(t *testing.T) { version: "v1.24.0", namespace: "", values: &v1.Values{}, + nsExists: true, wantErr: true, errContains: "namespace not set", }, @@ -55,82 +68,47 @@ func TestIstiodReconciler_ValidateSpec(t *testing.T) { version: "v1.24.0", namespace: "istio-system", values: nil, + nsExists: true, wantErr: true, errContains: "values not set", }, { - name: "default revision with non-empty revision name in values", - version: "v1.24.0", - namespace: "istio-system", - revisionName: v1.DefaultRevision, - values: &v1.Values{ - Revision: ptr.Of("canary"), - Global: &v1.GlobalConfig{ - IstioNamespace: ptr.Of("istio-system"), - }, - }, - wantErr: true, - errContains: "values.revision must be", - }, - { - name: "non-default revision with mismatched revision name", - version: "v1.24.0", - namespace: "istio-system", - revisionName: "canary", + name: "namespace not found", + version: "v1.24.0", + namespace: "istio-system", values: &v1.Values{ - Revision: ptr.Of("other"), Global: &v1.GlobalConfig{ IstioNamespace: ptr.Of("istio-system"), }, }, + nsExists: false, wantErr: true, - errContains: "values.revision does not match revision name", - }, - { - name: "namespace mismatch", - version: "v1.24.0", - namespace: "istio-system", - revisionName: v1.DefaultRevision, - values: &v1.Values{ - Global: &v1.GlobalConfig{ - IstioNamespace: ptr.Of("other-namespace"), - }, - }, - wantErr: true, - errContains: "values.global.istioNamespace does not match namespace", - }, - { - name: "valid default revision", - version: "v1.24.0", - namespace: "istio-system", - revisionName: v1.DefaultRevision, - values: &v1.Values{ - Revision: ptr.Of(""), - Global: &v1.GlobalConfig{ - IstioNamespace: ptr.Of("istio-system"), - }, - }, - wantErr: false, + errContains: `namespace "istio-system" doesn't exist`, }, { - name: "valid canary revision", - version: "v1.24.0", - namespace: "istio-system", - revisionName: "canary", + name: "valid", + version: "v1.24.0", + namespace: "istio-system", values: &v1.Values{ - Revision: ptr.Of("canary"), Global: &v1.GlobalConfig{ IstioNamespace: ptr.Of("istio-system"), }, }, - wantErr: false, + nsExists: true, + wantErr: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - r := NewIstiodReconciler(Config{}, nil) - err := r.ValidateSpec(tt.version, tt.namespace, tt.values, tt.revisionName) + clientBuilder := fake.NewClientBuilder().WithScheme(scheme.Scheme) + if tt.nsExists { + clientBuilder = clientBuilder.WithObjects(ns) + } + cl := clientBuilder.Build() + + r := NewIstiodReconciler(Config{}, cl) + err := r.Validate(context.Background(), tt.version, tt.namespace, tt.values) if tt.wantErr { assert.Error(t, err) diff --git a/pkg/reconcile/ztunnel.go b/pkg/reconcile/ztunnel.go index 7b6b695c12..144367c679 100644 --- a/pkg/reconcile/ztunnel.go +++ b/pkg/reconcile/ztunnel.go @@ -43,8 +43,6 @@ type ZTunnelReconciler struct { } // NewZTunnelReconciler creates a new ZTunnelReconciler. -// The client parameter is optional - pass nil when using from the library -// where Kubernetes client operations are not needed. func NewZTunnelReconciler(cfg Config, client client.Client) *ZTunnelReconciler { return &ZTunnelReconciler{ cfg: cfg, @@ -52,30 +50,15 @@ func NewZTunnelReconciler(cfg Config, client client.Client) *ZTunnelReconciler { } } -// ValidateSpec validates the ZTunnel specification. -// It performs basic validation that doesn't require Kubernetes API access. -func (r *ZTunnelReconciler) ValidateSpec(version, namespace string) error { +// Validate performs general validation of the ZTunnel specification. +// This includes basic field validation and Kubernetes API checks (namespace exists). +func (r *ZTunnelReconciler) Validate(ctx context.Context, version, namespace string) error { if version == "" { return reconciler.NewValidationError("version not set") } if namespace == "" { return reconciler.NewValidationError("namespace not set") } - return nil -} - -// Validate performs full validation including Kubernetes API checks. -// This requires a non-nil client to be set. -func (r *ZTunnelReconciler) Validate(ctx context.Context, version, namespace string) error { - // First perform basic validation - if err := r.ValidateSpec(version, namespace); err != nil { - return err - } - - // Skip Kubernetes checks if no client is available - if r.client == nil { - return nil - } // Validate target namespace exists if err := validation.ValidateTargetNamespace(ctx, r.client, namespace); err != nil { diff --git a/pkg/reconcile/ztunnel_test.go b/pkg/reconcile/ztunnel_test.go index d84cdc0c83..d0164c71ae 100644 --- a/pkg/reconcile/ztunnel_test.go +++ b/pkg/reconcile/ztunnel_test.go @@ -15,21 +15,33 @@ package reconcile import ( + "context" "testing" v1 "github.com/istio-ecosystem/sail-operator/api/v1" "github.com/istio-ecosystem/sail-operator/pkg/config" "github.com/istio-ecosystem/sail-operator/pkg/reconciler" + "github.com/istio-ecosystem/sail-operator/pkg/scheme" "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client/fake" "istio.io/istio/pkg/ptr" ) -func TestZTunnelReconciler_ValidateSpec(t *testing.T) { +func TestZTunnelReconciler_Validate(t *testing.T) { + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "istio-system", + }, + } + tests := []struct { name string version string namespace string + nsExists bool wantErr bool errContains string }{ @@ -37,6 +49,7 @@ func TestZTunnelReconciler_ValidateSpec(t *testing.T) { name: "missing version", version: "", namespace: "istio-system", + nsExists: true, wantErr: true, errContains: "version not set", }, @@ -44,21 +57,37 @@ func TestZTunnelReconciler_ValidateSpec(t *testing.T) { name: "missing namespace", version: "v1.24.0", namespace: "", + nsExists: true, wantErr: true, errContains: "namespace not set", }, + { + name: "namespace not found", + version: "v1.24.0", + namespace: "istio-system", + nsExists: false, + wantErr: true, + errContains: `namespace "istio-system" doesn't exist`, + }, { name: "valid", version: "v1.24.0", namespace: "istio-system", + nsExists: true, wantErr: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - r := NewZTunnelReconciler(Config{}, nil) - err := r.ValidateSpec(tt.version, tt.namespace) + clientBuilder := fake.NewClientBuilder().WithScheme(scheme.Scheme) + if tt.nsExists { + clientBuilder = clientBuilder.WithObjects(ns) + } + cl := clientBuilder.Build() + + r := NewZTunnelReconciler(Config{}, cl) + err := r.Validate(context.Background(), tt.version, tt.namespace) if tt.wantErr { assert.Error(t, err) From c51614ac8f33c19ef5564bce4f7d0b656b1ef5e4 Mon Sep 17 00:00:00 2001 From: Aslak Knutsen Date: Fri, 6 Feb 2026 12:42:04 +0100 Subject: [PATCH 3/3] refactor(reconcile): use GetChartPath helper and tidy up exports Replace inline path.Join(version, "charts", ...) calls with the existing GetChartPath helper across all three reconcilers. Unexport getReleaseName since it's only used within istiod.go. Move GetChartPath and its test to common.go/common_test.go (renamed from types.go) since it's shared across packages. Signed-off-by: Aslak Knutsen --- pkg/reconcile/cni.go | 3 +- pkg/reconcile/{types.go => common.go} | 6 ++++ pkg/reconcile/common_test.go | 47 +++++++++++++++++++++++++++ pkg/reconcile/istiod.go | 22 +++++-------- pkg/reconcile/istiod_test.go | 30 ++--------------- pkg/reconcile/ztunnel.go | 3 +- 6 files changed, 65 insertions(+), 46 deletions(-) rename pkg/reconcile/{types.go => common.go} (90%) create mode 100644 pkg/reconcile/common_test.go diff --git a/pkg/reconcile/cni.go b/pkg/reconcile/cni.go index 155ef0e06b..e86f519220 100644 --- a/pkg/reconcile/cni.go +++ b/pkg/reconcile/cni.go @@ -17,7 +17,6 @@ package reconcile import ( "context" "fmt" - "path" v1 "github.com/istio-ecosystem/sail-operator/api/v1" "github.com/istio-ecosystem/sail-operator/pkg/config" @@ -105,7 +104,7 @@ func (r *CNIReconciler) Install(ctx context.Context, version, namespace string, return fmt.Errorf("failed to resolve CNI version: %w", err) } - chartPath := path.Join(resolvedVersion, "charts", cniChartName) + chartPath := GetChartPath(resolvedVersion, cniChartName) _, err = r.cfg.ChartManager.UpgradeOrInstallChart( ctx, r.cfg.ResourceFS, diff --git a/pkg/reconcile/types.go b/pkg/reconcile/common.go similarity index 90% rename from pkg/reconcile/types.go rename to pkg/reconcile/common.go index 71ad1843a4..047cc835a4 100644 --- a/pkg/reconcile/types.go +++ b/pkg/reconcile/common.go @@ -19,6 +19,7 @@ package reconcile import ( "io/fs" + "path" "github.com/istio-ecosystem/sail-operator/pkg/config" "github.com/istio-ecosystem/sail-operator/pkg/helm" @@ -43,3 +44,8 @@ type Config struct { // ChartManager handles Helm chart installation and upgrades ChartManager *helm.ChartManager } + +// GetChartPath returns the path to a chart for a given version. +func GetChartPath(version, chartName string) string { + return path.Join(version, "charts", chartName) +} diff --git a/pkg/reconcile/common_test.go b/pkg/reconcile/common_test.go new file mode 100644 index 0000000000..f2f7f3dc2b --- /dev/null +++ b/pkg/reconcile/common_test.go @@ -0,0 +1,47 @@ +// Copyright Istio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package reconcile + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestGetChartPath(t *testing.T) { + tests := []struct { + version string + chartName string + expected string + }{ + { + version: "v1.24.0", + chartName: "istiod", + expected: "v1.24.0/charts/istiod", + }, + { + version: "v1.23.0", + chartName: "base", + expected: "v1.23.0/charts/base", + }, + } + + for _, tt := range tests { + t.Run(tt.version+"-"+tt.chartName, func(t *testing.T) { + result := GetChartPath(tt.version, tt.chartName) + assert.Equal(t, tt.expected, result) + }) + } +} diff --git a/pkg/reconcile/istiod.go b/pkg/reconcile/istiod.go index 67bfaed3ab..752b91f6ab 100644 --- a/pkg/reconcile/istiod.go +++ b/pkg/reconcile/istiod.go @@ -17,7 +17,6 @@ package reconcile import ( "context" "fmt" - "path" v1 "github.com/istio-ecosystem/sail-operator/api/v1" "github.com/istio-ecosystem/sail-operator/pkg/constants" @@ -76,8 +75,8 @@ func (r *IstiodReconciler) Install( helmValues := helm.FromValues(values) // Install istiod chart - istiodChartPath := path.Join(version, "charts", constants.IstiodChartName) - istiodReleaseName := GetReleaseName(revisionName, constants.IstiodChartName) + istiodChartPath := GetChartPath(version, constants.IstiodChartName) + istiodReleaseName := getReleaseName(revisionName, constants.IstiodChartName) _, err := r.cfg.ChartManager.UpgradeOrInstallChart( ctx, @@ -94,8 +93,8 @@ func (r *IstiodReconciler) Install( // Install base chart for default revision if revisionName == v1.DefaultRevision { - baseChartPath := path.Join(version, "charts", constants.BaseChartName) - baseReleaseName := GetReleaseName(revisionName, constants.BaseChartName) + baseChartPath := GetChartPath(version, constants.BaseChartName) + baseReleaseName := getReleaseName(revisionName, constants.BaseChartName) _, err := r.cfg.ChartManager.UpgradeOrInstallChart( ctx, @@ -117,14 +116,14 @@ func (r *IstiodReconciler) Install( // Uninstall removes the istiod Helm charts. func (r *IstiodReconciler) Uninstall(ctx context.Context, namespace, revisionName string) error { // Uninstall istiod chart - istiodReleaseName := GetReleaseName(revisionName, constants.IstiodChartName) + istiodReleaseName := getReleaseName(revisionName, constants.IstiodChartName) if _, err := r.cfg.ChartManager.UninstallChart(ctx, istiodReleaseName, namespace); err != nil { return fmt.Errorf("failed to uninstall Helm chart %q: %w", constants.IstiodChartName, err) } // Uninstall base chart for default revision if revisionName == v1.DefaultRevision { - baseReleaseName := GetReleaseName(revisionName, constants.BaseChartName) + baseReleaseName := getReleaseName(revisionName, constants.BaseChartName) if _, err := r.cfg.ChartManager.UninstallChart(ctx, baseReleaseName, r.cfg.OperatorNamespace); err != nil { return fmt.Errorf("failed to uninstall Helm chart %q: %w", constants.BaseChartName, err) } @@ -133,12 +132,7 @@ func (r *IstiodReconciler) Uninstall(ctx context.Context, namespace, revisionNam return nil } -// GetReleaseName returns the Helm release name for a given revision and chart. -func GetReleaseName(revisionName, chartName string) string { +// getReleaseName returns the Helm release name for a given revision and chart. +func getReleaseName(revisionName, chartName string) string { return fmt.Sprintf("%s-%s", revisionName, chartName) } - -// GetChartPath returns the path to a chart for a given version. -func GetChartPath(version, chartName string) string { - return path.Join(version, "charts", chartName) -} diff --git a/pkg/reconcile/istiod_test.go b/pkg/reconcile/istiod_test.go index 6486c5ec3e..4f71858567 100644 --- a/pkg/reconcile/istiod_test.go +++ b/pkg/reconcile/istiod_test.go @@ -123,7 +123,7 @@ func TestIstiodReconciler_Validate(t *testing.T) { } } -func TestGetReleaseName(t *testing.T) { +func Test_getReleaseName(t *testing.T) { tests := []struct { revisionName string chartName string @@ -148,33 +148,7 @@ func TestGetReleaseName(t *testing.T) { for _, tt := range tests { t.Run(tt.revisionName+"-"+tt.chartName, func(t *testing.T) { - result := GetReleaseName(tt.revisionName, tt.chartName) - assert.Equal(t, tt.expected, result) - }) - } -} - -func TestGetChartPath(t *testing.T) { - tests := []struct { - version string - chartName string - expected string - }{ - { - version: "v1.24.0", - chartName: "istiod", - expected: "v1.24.0/charts/istiod", - }, - { - version: "v1.23.0", - chartName: "base", - expected: "v1.23.0/charts/base", - }, - } - - for _, tt := range tests { - t.Run(tt.version+"-"+tt.chartName, func(t *testing.T) { - result := GetChartPath(tt.version, tt.chartName) + result := getReleaseName(tt.revisionName, tt.chartName) assert.Equal(t, tt.expected, result) }) } diff --git a/pkg/reconcile/ztunnel.go b/pkg/reconcile/ztunnel.go index 144367c679..11712279e4 100644 --- a/pkg/reconcile/ztunnel.go +++ b/pkg/reconcile/ztunnel.go @@ -17,7 +17,6 @@ package reconcile import ( "context" "fmt" - "path" v1 "github.com/istio-ecosystem/sail-operator/api/v1" "github.com/istio-ecosystem/sail-operator/pkg/config" @@ -113,7 +112,7 @@ func (r *ZTunnelReconciler) Install(ctx context.Context, version, namespace stri return fmt.Errorf("failed to resolve ZTunnel version: %w", err) } - chartPath := path.Join(resolvedVersion, "charts", ztunnelChartName) + chartPath := GetChartPath(resolvedVersion, ztunnelChartName) _, err = r.cfg.ChartManager.UpgradeOrInstallChart( ctx, r.cfg.ResourceFS,