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..b54b55eb52 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,6 +30,7 @@ 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" @@ -115,57 +115,22 @@ 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) -} + istiodReconciler := r.newIstiodReconciler() -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 { + // CRD-specific validations + if err := r.validateRevisionConsistency(rev); err != nil { return err } - - if rev.Spec.Values == nil { - return reconciler.NewValidationError("spec.values not set") - } - - 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") + if err := r.validateNoTagConflict(ctx, rev); err != nil { + return err } - 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) { + // General validations + if err := istiodReconciler.Validate(ctx, rev.Spec.Version, rev.Spec.Namespace, rev.Spec.Values); 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,39 +139,55 @@ 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) Finalize(ctx context.Context, rev *v1.IstioRevision) error { + istiodReconciler := r.newIstiodReconciler() + return istiodReconciler.Uninstall(ctx, rev.Spec.Namespace, rev.Name) } -func (r *Reconciler) getChartPath(rev *v1.IstioRevision, chartName string) string { - return path.Join(rev.Spec.Version, "charts", chartName) +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) 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) +// 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") } - 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 +} + +// 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 diff --git a/controllers/istiorevision/istiorevision_controller_test.go b/controllers/istiorevision/istiorevision_controller_test.go index 598f6c4e05..3dad5ee093 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,35 @@ 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) - err := r.validate(context.TODO(), tc.rev) + // 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/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..e86f519220 --- /dev/null +++ b/pkg/reconcile/cni.go @@ -0,0 +1,158 @@ +// 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" + + 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. +func NewCNIReconciler(cfg Config, client client.Client) *CNIReconciler { + return &CNIReconciler{ + cfg: cfg, + client: client, + } +} + +// 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") + } + + // 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 := GetChartPath(resolvedVersion, 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..5a62022d3a --- /dev/null +++ b/pkg/reconcile/cni_test.go @@ -0,0 +1,183 @@ +// 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" + "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_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 + }{ + { + name: "missing version", + version: "", + namespace: "istio-system", + nsExists: true, + wantErr: true, + errContains: "version not set", + }, + { + 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) { + 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) + 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/common.go b/pkg/reconcile/common.go new file mode 100644 index 0000000000..047cc835a4 --- /dev/null +++ b/pkg/reconcile/common.go @@ -0,0 +1,51 @@ +// 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" + "path" + + "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 +} + +// 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 new file mode 100644 index 0000000000..752b91f6ab --- /dev/null +++ b/pkg/reconcile/istiod.go @@ -0,0 +1,138 @@ +// 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" + + 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" + "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. +func NewIstiodReconciler(cfg Config, client client.Client) *IstiodReconciler { + return &IstiodReconciler{ + cfg: cfg, + client: client, + } +} + +// 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") + } + if namespace == "" { + return reconciler.NewValidationError("namespace not set") + } + if values == nil { + return reconciler.NewValidationError("values not set") + } + + // Validate target namespace exists + if err := validation.ValidateTargetNamespace(ctx, r.client, namespace); err != nil { + return err + } + + 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 := GetChartPath(version, 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 := GetChartPath(version, 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) +} diff --git a/pkg/reconcile/istiod_test.go b/pkg/reconcile/istiod_test.go new file mode 100644 index 0000000000..4f71858567 --- /dev/null +++ b/pkg/reconcile/istiod_test.go @@ -0,0 +1,155 @@ +// 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" + "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_Validate(t *testing.T) { + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "istio-system", + }, + } + + tests := []struct { + 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", + }, + { + name: "missing namespace", + version: "v1.24.0", + namespace: "", + values: &v1.Values{}, + nsExists: true, + wantErr: true, + errContains: "namespace not set", + }, + { + name: "missing values", + version: "v1.24.0", + namespace: "istio-system", + values: nil, + nsExists: true, + wantErr: true, + errContains: "values not set", + }, + { + name: "namespace not found", + version: "v1.24.0", + namespace: "istio-system", + values: &v1.Values{ + Global: &v1.GlobalConfig{ + IstioNamespace: ptr.Of("istio-system"), + }, + }, + nsExists: false, + wantErr: true, + errContains: `namespace "istio-system" doesn't exist`, + }, + { + name: "valid", + version: "v1.24.0", + namespace: "istio-system", + values: &v1.Values{ + Global: &v1.GlobalConfig{ + IstioNamespace: ptr.Of("istio-system"), + }, + }, + nsExists: true, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + 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) + 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 Test_getReleaseName(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) + }) + } +} diff --git a/pkg/reconcile/ztunnel.go b/pkg/reconcile/ztunnel.go new file mode 100644 index 0000000000..11712279e4 --- /dev/null +++ b/pkg/reconcile/ztunnel.go @@ -0,0 +1,166 @@ +// 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" + + 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. +func NewZTunnelReconciler(cfg Config, client client.Client) *ZTunnelReconciler { + return &ZTunnelReconciler{ + cfg: cfg, + client: client, + } +} + +// 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") + } + + // 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 := GetChartPath(resolvedVersion, 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..d0164c71ae --- /dev/null +++ b/pkg/reconcile/ztunnel_test.go @@ -0,0 +1,183 @@ +// 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" + "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_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 + }{ + { + name: "missing version", + version: "", + namespace: "istio-system", + nsExists: true, + wantErr: true, + errContains: "version not set", + }, + { + 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) { + 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) + 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) + }) + } +}