diff --git a/bundle/bundle.go b/bundle/bundle.go new file mode 100644 index 0000000000..76e9080615 --- /dev/null +++ b/bundle/bundle.go @@ -0,0 +1,20 @@ +// 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 bundle + +import _ "embed" + +//go:embed manifests/servicemeshoperator3.clusterserviceversion.yaml +var CSV []byte diff --git a/chart/crds/crds.go b/chart/crds/crds.go new file mode 100644 index 0000000000..4c8e382daa --- /dev/null +++ b/chart/crds/crds.go @@ -0,0 +1,27 @@ +// 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 crds provides embedded CRD YAML files for Istio and Sail resources. +package crds + +import "embed" + +// FS contains all CRD YAML files from the chart/crds directory. +// This allows programmatic access to CRDs for installation. +// +// CRD files follow the naming convention: {group}_{plural}.yaml +// Example: extensions.istio.io_wasmplugins.yaml +// +//go:embed *.yaml +var FS embed.FS diff --git a/go.mod b/go.mod index fb727e7151..2398b09dc4 100644 --- a/go.mod +++ b/go.mod @@ -31,7 +31,9 @@ require ( k8s.io/apimachinery v0.35.1 k8s.io/cli-runtime v0.35.0 k8s.io/client-go v0.35.1 + k8s.io/utils v0.0.0-20260108192941-914a6e750570 sigs.k8s.io/controller-runtime v0.23.3 + sigs.k8s.io/yaml v1.6.0 ) require ( @@ -175,7 +177,6 @@ require ( k8s.io/klog/v2 v2.130.1 // indirect k8s.io/kube-openapi v0.0.0-20251125145642-4e65d59e963e // indirect k8s.io/kubectl v0.35.0 // indirect - k8s.io/utils v0.0.0-20260108192941-914a6e750570 // indirect oras.land/oras-go/v2 v2.6.0 // indirect sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.32.1 // indirect sigs.k8s.io/controller-tools v0.14.0 // indirect @@ -184,5 +185,4 @@ require ( sigs.k8s.io/kustomize/kyaml v0.20.1 // indirect sigs.k8s.io/randfill v1.0.0 // indirect sigs.k8s.io/structured-merge-diff/v6 v6.3.2 // indirect - sigs.k8s.io/yaml v1.6.0 // indirect ) diff --git a/pkg/install/README.md b/pkg/install/README.md new file mode 100644 index 0000000000..e8093920d2 --- /dev/null +++ b/pkg/install/README.md @@ -0,0 +1,109 @@ +# pkg/install + +Library for managing istiod installations without running the Sail Operator. +Designed for embedding in other operators (e.g. OpenShift Ingress) that need +to install and maintain Istio as an internal dependency. + +## Usage + +```go +lib, err := install.New(kubeConfig, resourceFS) +notifyCh := lib.Start(ctx) + +// In controller reconcile: +lib.Apply(install.Options{ + Namespace: "istio-system", + Values: install.GatewayAPIDefaults(), +}) + +// Read result after notification: +for range notifyCh { + status := lib.Status() + // update conditions from status +} + +// Teardown: +lib.Uninstall(ctx, "istio-system", "default") +``` + +## How it works + +The Library runs as an independent actor with a simple state model: + +1. **Apply** -- consumer sends desired state (version, namespace, values) +2. **Reconcile** -- Library installs/upgrades CRDs and istiod via Helm +3. **Drift detection** -- dynamic informers watch owned resources and CRDs, re-enqueuing reconciliation on changes +4. **Status** -- consumer reads the reconciliation result + +The reconciliation loop sits idle until the first `Apply()` call. After that, +it stays active with informers running until `Uninstall()` clears the desired +state and stops the loop. + +## Public API + +### Constructor + +- `New(kubeConfig, resourceFS)` -- creates a Library with Kubernetes clients, Helm chart manager, and CRD manager +- `FromDirectory(path)` -- creates an `fs.FS` from a filesystem path (alternative to embedded resources) + +### Library methods + +| Method | Description | +|---|---| +| `Start(ctx)` | Starts the reconciliation loop; returns a notification channel | +| `Apply(opts)` | Sets desired state; enqueues reconciliation if changed | +| `Enqueue()` | Forces re-reconciliation without changing desired state | +| `Status()` | Returns the latest reconciliation result | +| `Uninstall(ctx, ns, rev)` | Stops informers, waits for processing, then Helm-uninstalls | + +### Types + +- **Options** -- install options: `Namespace`, `Version`, `Revision`, `Values`, `ManageCRDs`, `IncludeAllCRDs`, `OverwriteOLMManagedCRD` +- **Status** -- reconciliation result: `CRDState`, `CRDMessage`, `CRDs`, `Installed`, `Version`, `Error` +- **CRDManagementState** -- aggregate CRD ownership: `ManagedByCIO`, `ManagedByOLM`, `UnknownManagement`, `MixedOwnership`, `NoneExist` +- **CRDInfo** -- per-CRD state: `Name`, `State`, `Found` +- **ImageNames** -- image names for each component: `Istiod`, `Proxy`, `CNI`, `ZTunnel` + +### Helper functions + +- `GatewayAPIDefaults()` -- pre-configured values for Gateway API mode on OpenShift +- `MergeValues(base, overlay)` -- deep-merge two Values structs (overlay wins) +- `DefaultVersion(resourceFS)` -- highest stable semver version from the resource FS +- `NormalizeVersion(version)` -- ensures a `v` prefix +- `ValidateVersion(resourceFS, version)` -- checks that a version directory exists +- `SetImageDefaults(resourceFS, registry, images)` -- populates image refs from version directories +- `LibraryRBACRules()` -- returns RBAC PolicyRules for a consumer's ClusterRole + +## CRD management + +The Library classifies existing CRDs by ownership labels before deciding what to do: + +| State | Meaning | Action | +|---|---|---| +| `NoneExist` | No target CRDs on cluster | Install with CIO labels | +| `ManagedByCIO` | All owned by Cluster Ingress Operator | Update as needed | +| `ManagedByOLM` | All owned by OLM (OSSM subscription) | Leave alone; Helm install proceeds | +| `UnknownManagement` | CRDs exist without recognized labels | Leave alone; set Status.Error | +| `MixedOwnership` | Inconsistent labels across CRDs | Leave alone; set Status.Error | + +The `OverwriteOLMManagedCRD` callback in Options lets the consumer decide +whether to take over OLM-managed CRDs (e.g. after an OSSM subscription is deleted). + +Which CRDs are targeted depends on `IncludeAllCRDs`: when false (default), only +CRDs matching `PILOT_INCLUDE_RESOURCES` / `PILOT_IGNORE_RESOURCES` are managed. + +## Files + +| File | Purpose | +|---|---| +| `library.go` | Public API, types (`Library`, `Status`, `Options`), constructor | +| `lifecycle.go` | Reconciliation loop, workqueue, `Start`/`Apply`/`Uninstall` | +| `installer.go` | Core install/uninstall logic, Helm values resolution, watch spec extraction | +| `crds.go` | CRD ownership classification, install, update | +| `crds_filter.go` | CRD selection based on `PILOT_INCLUDE_RESOURCES` / `PILOT_IGNORE_RESOURCES` | +| `values.go` | `GatewayAPIDefaults()`, `MergeValues()` | +| `predicates.go` | Event filtering for informers (ownership checks, status-only changes) | +| `informers.go` | Dynamic informer setup for drift detection | +| `version.go` | Version resolution and validation | +| `images.go` | Image configuration from resource FS | +| `rbac.go` | RBAC rules for library consumers | diff --git a/pkg/install/USAGE.md b/pkg/install/USAGE.md new file mode 100644 index 0000000000..2c825951ad --- /dev/null +++ b/pkg/install/USAGE.md @@ -0,0 +1,313 @@ +# Install Library - CIO Usage Example + +This document shows how the Cluster Ingress Operator (CIO) would wire the install library into its main and GatewayClass controller. + +## Architecture + +``` +┌──────────────────────────────────────────────────────────────┐ +│ main.go │ +│ │ +│ 1. Create install.Library │ +│ 2. Start it (returns notifyCh) │ +│ 3. Pass Library to GatewayClassReconciler │ +│ 4. Bridge notifyCh into a source.Func on the controller │ +│ 5. Start manager │ +└──────────────────────────────────────────────────────────────┘ + │ Apply(opts) ▲ notifyCh signal + ▼ │ +┌──────────────────────────────────────────────────────────────┐ +│ install.Library (internal goroutines) │ +│ │ +│ - Classifies + installs/updates CRDs │ +│ - Installs istiod via Helm │ +│ - Watches Helm resources for drift │ +│ - Watches CRDs for ownership changes │ +│ - Sends signal on notifyCh after each reconciliation │ +└──────────────────────────────────────────────────────────────┘ + │ Status() ▲ drift / CRD event + ▼ │ +┌──────────────────────────────────────────────────────────────┐ +│ GatewayClassReconciler.Reconcile() │ +│ │ +│ 1. Get GatewayClass │ +│ 2. Build values, call lib.Apply(opts) │ +│ 3. Read lib.Status() │ +│ 4. Map status → GatewayClass conditions │ +│ 5. Update GatewayClass status │ +└──────────────────────────────────────────────────────────────┘ +``` + +## main.go + +```go +package main + +import ( + "context" + "os" + + "github.com/istio-ecosystem/sail-operator/pkg/install" + "github.com/istio-ecosystem/sail-operator/resources" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/util/workqueue" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + "sigs.k8s.io/controller-runtime/pkg/source" + gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" +) + +func main() { + mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{}) + if err != nil { + os.Exit(1) + } + + // 1. Create the install library. + // resources.FS contains the embedded Helm charts, profiles, and CRDs. + lib, err := install.New(mgr.GetConfig(), resources.FS) + if err != nil { + os.Exit(1) + } + + // 2. Start the library. This returns a notification channel that fires + // every time the library finishes a reconciliation (install, drift + // repair, CRD ownership change). + // The library sits idle until the first Apply() call. + ctx := ctrl.SetupSignalHandler() + notifyCh := lib.Start(ctx) + + // 3. Create the GatewayClass controller with the library injected. + reconciler := &GatewayClassReconciler{ + Client: mgr.GetClient(), + Lib: lib, + } + + // 4. Bridge notifyCh into a controller-runtime source. + // notifyCh is <-chan struct{}, so we use source.Func to convert each + // signal into a reconcile.Request for our fixed GatewayClass name. + notifySource := source.Func(func(ctx context.Context, q workqueue.TypedRateLimitingInterface[reconcile.Request]) error { + go func() { + for { + select { + case <-ctx.Done(): + return + case _, ok := <-notifyCh: + if !ok { + return + } + q.Add(reconcile.Request{ + NamespacedName: types.NamespacedName{Name: gatewayClassName}, + }) + } + } + }() + return nil + }) + + err = ctrl.NewControllerManagedBy(mgr). + For(&gwapiv1.GatewayClass{}). + WatchesRawSource(notifySource). + Complete(reconciler) + if err != nil { + os.Exit(1) + } + + // 5. Start the manager. This blocks until ctx is cancelled. + if err := mgr.Start(ctx); err != nil { + os.Exit(1) + } +} +``` + +## GatewayClass Controller + +```go +package main + +import ( + "context" + "fmt" + + "github.com/istio-ecosystem/sail-operator/pkg/install" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" +) + +const ( + controllerName = "openshift.io/ingress-controller" + gatewayClassName = "openshift-default" +) + +// GatewayClassReconciler reconciles GatewayClass objects and drives +// the install library to manage istiod. +type GatewayClassReconciler struct { + client.Client + Lib *install.Library +} + +func (r *GatewayClassReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + log := ctrl.LoggerFrom(ctx) + + // 1. Get the GatewayClass. + gc := &gwapiv1.GatewayClass{} + if err := r.Get(ctx, req.NamespacedName, gc); err != nil { + return ctrl.Result{}, client.IgnoreNotFound(err) + } + + // Only handle our GatewayClass. + if string(gc.Spec.ControllerName) != controllerName { + return ctrl.Result{}, nil + } + + // 2. Build values and call Apply. + // Apply is idempotent — if nothing changed, it's a no-op. + values := install.GatewayAPIDefaults() + values.Pilot.Env["PILOT_GATEWAY_API_CONTROLLER_NAME"] = controllerName + values.Pilot.Env["PILOT_GATEWAY_API_DEFAULT_GATEWAYCLASS_NAME"] = gatewayClassName + + r.Lib.Apply(install.Options{ + Namespace: "openshift-ingress", + Values: values, + }) + + // 3. Read the latest status from the library. + status := r.Lib.Status() + + // 4. Map library status to GatewayClass conditions. + conditions := mapStatusToConditions(status, gc.Generation) + + // 5. Update GatewayClass status. + gc.Status.Conditions = conditions + if err := r.Status().Update(ctx, gc); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to update GatewayClass status: %w", err) + } + + log.Info("reconciled", + "installed", status.Installed, + "version", status.Version, + "crdState", status.CRDState, + ) + return ctrl.Result{}, nil +} + +// mapStatusToConditions translates the library Status into GatewayClass conditions. +func mapStatusToConditions(status install.Status, generation int64) []metav1.Condition { + var conditions []metav1.Condition + + // Accepted condition: true if istiod is installed. + accepted := metav1.Condition{ + Type: string(gwapiv1.GatewayClassConditionStatusAccepted), + ObservedGeneration: generation, + LastTransitionTime: metav1.Now(), + } + if status.Installed { + accepted.Status = metav1.ConditionTrue + accepted.Reason = "Installed" + accepted.Message = fmt.Sprintf("istiod %s installed", status.Version) + } else if status.Error != nil { + accepted.Status = metav1.ConditionFalse + accepted.Reason = "InstallFailed" + accepted.Message = status.Error.Error() + } else { + accepted.Status = metav1.ConditionUnknown + accepted.Reason = "Pending" + accepted.Message = "waiting for first reconciliation" + } + conditions = append(conditions, accepted) + + // CRD condition: reflects CRD ownership state. + crd := metav1.Condition{ + Type: "CRDsReady", + ObservedGeneration: generation, + LastTransitionTime: metav1.Now(), + } + switch status.CRDState { + case install.CRDManagedByCIO: + crd.Status = metav1.ConditionTrue + crd.Reason = "ManagedByCIO" + crd.Message = status.CRDMessage + case install.CRDManagedByOLM: + crd.Status = metav1.ConditionTrue + crd.Reason = "ManagedByOLM" + crd.Message = status.CRDMessage + case install.CRDNoneExist: + crd.Status = metav1.ConditionUnknown + crd.Reason = "NoneExist" + crd.Message = "CRDs not yet installed" + case install.CRDMixedOwnership: + crd.Status = metav1.ConditionFalse + crd.Reason = "MixedOwnership" + crd.Message = status.CRDMessage + case install.CRDUnknownManagement: + crd.Status = metav1.ConditionFalse + crd.Reason = "UnknownManagement" + crd.Message = status.CRDMessage + } + conditions = append(conditions, crd) + + return conditions +} +``` + +## Sequence: What Happens When + +### Initial startup (no CRDs, no istiod) + +1. Manager starts, controller watches GatewayClass +2. GatewayClass is created by admin or platform +3. Controller reconciles: calls `lib.Apply(opts)` +4. Library wakes up, classifies CRDs (none exist) → installs CRDs with CIO labels +5. Library installs istiod via Helm +6. Library sends signal on `notifyCh` +7. Controller reconciles again: reads `lib.Status()` → sets Accepted=True, CRDsReady=True + +### OSSM installed later (CRD ownership conflict) + +1. Admin installs OSSM via OLM +2. OLM updates CRDs, adds `olm.managed=true` label +3. Library's CRD informer detects label change → re-reconciles +4. Library classifies CRDs → `MixedOwnership` (some CIO, some OLM) +5. Library skips CRD install/update, sets `Status.Error` +6. Library sends signal on `notifyCh` +7. Controller reconciles: reads status → sets CRDsReady=False, reason=MixedOwnership + +### Drift detected (someone deletes a ConfigMap) + +1. Someone deletes `istio` ConfigMap in the target namespace +2. Library's Helm resource informer detects the delete event +3. Library re-reconciles: Helm reinstalls the ConfigMap +4. Library sends signal on `notifyCh` +5. Controller reconciles: reads status → still Accepted=True (everything healthy) + +### No-op Apply (same values) + +1. Controller reconciles for some other reason (e.g. GatewayClass spec unchanged) +2. Calls `lib.Apply(opts)` with identical options +3. Library detects no change via `optionsEqual()` → does nothing +4. No `notifyCh` signal, no unnecessary Helm work + +### External state change (e.g. OLM Subscription deleted) + +1. OLM Subscription managing Istio CRDs is deleted +2. Controller detects the Subscription change (via its own watch) +3. Calls `lib.Enqueue()` to force CRD re-classification +4. Library re-reconciles with the previously applied options, re-classifying CRD ownership +5. Library sends signal on `notifyCh` +6. Controller reconciles: reads status → CRD state may have changed (e.g. takeover now possible) + +Use `Enqueue()` instead of `Apply()` when the Options haven't changed but external cluster state +(CRD ownership, Subscription lifecycle, etc.) has. `Apply()` with identical options is a no-op, +so it won't trigger re-evaluation. `Enqueue()` bypasses that check. + +## RBAC + +The library needs cluster-wide permissions. Aggregate them into your ClusterRole: + +```go +rules := append(myOperatorRules, install.LibraryRBACRules()...) +``` + +Or in YAML, merge the rules from `install.LibraryRBACRules()` into your operator's ClusterRole manifest. diff --git a/pkg/install/crds.go b/pkg/install/crds.go new file mode 100644 index 0000000000..98d50e2d63 --- /dev/null +++ b/pkg/install/crds.go @@ -0,0 +1,413 @@ +// 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 install + +import ( + "context" + "fmt" + "io/fs" + "strings" + + v1 "github.com/istio-ecosystem/sail-operator/api/v1" + "github.com/istio-ecosystem/sail-operator/chart/crds" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/yaml" +) + +// OverwriteOLMManagedCRDFunc is called when a CRD is detected with OLM ownership +// labels. The CRD object is provided so the callback can inspect OLM +// annotations/labels to determine whether the owning subscription still exists. +// Return true to overwrite the CRD (take ownership), false to leave it alone. +type OverwriteOLMManagedCRDFunc func(ctx context.Context, crd *apiextensionsv1.CustomResourceDefinition) bool + +// CRD ownership labels and annotations. +const ( + // labelManagedByCIO indicates the CRD is managed by the Cluster Ingress Operator. + labelManagedByCIO = "ingress.operator.openshift.io/owned" + + // labelOLMManaged indicates the CRD is managed by OLM (OSSM subscription). + labelOLMManaged = "olm.managed" + + // annotationHelmKeep prevents Helm from deleting the CRD during uninstall. + annotationHelmKeep = "helm.sh/resource-policy" +) + +// CRDManagementState represents the aggregate ownership state of Istio CRDs on the cluster. +type CRDManagementState string + +const ( + // CRDManagedByCIO means all target CRDs are owned by the Cluster Ingress Operator. + // CRDs will be installed or updated. + CRDManagedByCIO CRDManagementState = "ManagedByCIO" + + // CRDManagedByOLM means all target CRDs are owned by an OSSM subscription via OLM. + // CRDs are left alone; Helm install still proceeds. + CRDManagedByOLM CRDManagementState = "ManagedByOLM" + + // CRDUnknownManagement means one or more CRDs exist but are not owned by CIO or OLM. + // CRDs are left alone; Helm install still proceeds; Status.Error is set. + CRDUnknownManagement CRDManagementState = "UnknownManagement" + + // CRDMixedOwnership means CRDs have inconsistent ownership (some CIO, some OLM, some unknown, or some missing). + // CRDs are left alone; Helm install still proceeds; Status.Error is set. + CRDMixedOwnership CRDManagementState = "MixedOwnership" + + // CRDNoneExist means no target CRDs exist on the cluster yet. + // CRDs will be installed with CIO ownership labels. + CRDNoneExist CRDManagementState = "NoneExist" +) + +// CRDInfo describes the state of a single CRD on the cluster. +type CRDInfo struct { + // Name is the CRD name, e.g. "wasmplugins.extensions.istio.io" + Name string + + // State is the ownership state of this specific CRD. + // Only meaningful when Found is true. + State CRDManagementState + + // Found indicates whether this CRD exists on the cluster. + Found bool +} + +// crdResult bundles the outcome of CRD reconciliation. +type crdResult struct { + // State is the aggregate ownership state of the target Istio CRDs. + State CRDManagementState + + // CRDs contains per-CRD detail (name, ownership, found on cluster). + CRDs []CRDInfo + + // Message is a human-readable description of the CRD state. + Message string + + // Error is non-nil if CRD management encountered a problem. + Error error +} + +// crdManager encapsulates all CRD classification, installation, and update logic. +// It provides a single entry point (Reconcile) and keeps a client dependency +// that can be swapped out in tests. +type crdManager struct { + cl client.Client +} + +// newCRDManager creates a crdManager with the given Kubernetes client. +func newCRDManager(cl client.Client) *crdManager { + return &crdManager{cl: cl} +} + +// classifyCRD checks a single CRD on the cluster and returns its ownership state. +// If overwriteOLM is non-nil and the CRD has OLM labels, it is called to decide +// whether to reclassify the CRD as CIO-managed (allowing adoption). +func (m *crdManager) classifyCRD(ctx context.Context, crdName string, overwriteOLM OverwriteOLMManagedCRDFunc) CRDInfo { + existing := &apiextensionsv1.CustomResourceDefinition{} + err := m.cl.Get(ctx, client.ObjectKey{Name: crdName}, existing) + if err != nil { + if apierrors.IsNotFound(err) { + return CRDInfo{Name: crdName, Found: false} + } + // Treat API errors as unknown management (we can't determine ownership) + return CRDInfo{Name: crdName, Found: true, State: CRDUnknownManagement} + } + + labels := existing.GetLabels() + + // Check CIO ownership + if _, ok := labels[labelManagedByCIO]; ok { + return CRDInfo{Name: crdName, Found: true, State: CRDManagedByCIO} + } + + // Check OLM ownership + if val, ok := labels[labelOLMManaged]; ok && val == "true" { + if overwriteOLM != nil && overwriteOLM(ctx, existing) { + return CRDInfo{Name: crdName, Found: true, State: CRDManagedByCIO} + } + return CRDInfo{Name: crdName, Found: true, State: CRDManagedByOLM} + } + + // No recognized ownership labels + return CRDInfo{Name: crdName, Found: true, State: CRDUnknownManagement} +} + +// classifyCRDs checks all target CRDs on the cluster and returns the aggregate state. +func (m *crdManager) classifyCRDs(ctx context.Context, targets []string, overwriteOLM OverwriteOLMManagedCRDFunc) (CRDManagementState, []CRDInfo) { + if len(targets) == 0 { + return CRDNoneExist, nil + } + + infos := make([]CRDInfo, len(targets)) + for i, target := range targets { + infos[i] = m.classifyCRD(ctx, target, overwriteOLM) + } + + return aggregateCRDState(infos), infos +} + +// aggregateCRDState derives the batch state from individual CRD states. +// +// Rules: +// - All not found → CRDNoneExist +// - All found are CIO or unknown (no OLM), with at least one CIO → CRDManagedByCIO +// (unknown = label drift, missing = deleted; both get fixed by updateCRDs) +// - All found OLM, all present → CRDManagedByOLM +// - Pure unknown (no CIO, no OLM) → CRDUnknownManagement +// - Any mix involving OLM → CRDMixedOwnership +func aggregateCRDState(infos []CRDInfo) CRDManagementState { + if len(infos) == 0 { + return CRDNoneExist + } + + var foundCount, cioCount, olmCount, unknownCount int + for _, info := range infos { + if !info.Found { + continue + } + foundCount++ + switch info.State { + case CRDManagedByCIO: + cioCount++ + case CRDManagedByOLM: + olmCount++ + default: + unknownCount++ + } + } + + total := len(infos) + + // None exist on cluster + if foundCount == 0 { + return CRDNoneExist + } + + // All found are CIO-owned (possibly with some missing or some that lost labels). + // No OLM involvement means we can safely reclaim unknowns and reinstall missing. + if cioCount > 0 && olmCount == 0 { + return CRDManagedByCIO + } + + // All found and all OLM — only if none are missing + if foundCount == total && olmCount == total { + return CRDManagedByOLM + } + + // Pure unknown — no CIO, no OLM labels on any found CRD + if unknownCount > 0 && cioCount == 0 && olmCount == 0 { + return CRDUnknownManagement + } + + // Anything else is mixed: CIO+OLM, OLM with missing, etc. + return CRDMixedOwnership +} + +// Reconcile classifies target CRDs and installs/updates them if we own them (or none exist). +// This is the single entry point for CRD management. +func (m *crdManager) Reconcile(ctx context.Context, values *v1.Values, includeAllCRDs bool, overwriteOLM OverwriteOLMManagedCRDFunc) crdResult { + targets, err := targetCRDsFromValues(values, includeAllCRDs) + if err != nil { + return crdResult{State: CRDNoneExist, Error: fmt.Errorf("failed to determine target CRDs: %w", err)} + } + if len(targets) == 0 { + return crdResult{State: CRDNoneExist, Message: "no target CRDs configured"} + } + + state, infos := m.classifyCRDs(ctx, targets, overwriteOLM) + + switch state { + case CRDNoneExist: + // Install all with CIO labels + if err := m.installCRDs(ctx, targets); err != nil { + return crdResult{State: state, CRDs: infos, Error: fmt.Errorf("failed to install CRDs: %w", err)} + } + // Update infos to reflect new state + for idx := range infos { + infos[idx].Found = true + infos[idx].State = CRDManagedByCIO + } + return crdResult{State: CRDManagedByCIO, CRDs: infos, Message: "CRDs installed by CIO"} + + case CRDManagedByCIO: + // Update existing, reinstall missing, re-label unknowns + missing := missingCRDNames(infos) + unlabeled := unlabeledCRDNames(infos) + if err := m.updateCRDs(ctx, targets); err != nil { + return crdResult{State: state, CRDs: infos, Error: fmt.Errorf("failed to update CRDs: %w", err)} + } + // Update infos for any previously-missing or unlabeled CRDs + for idx := range infos { + if !infos[idx].Found || infos[idx].State == CRDUnknownManagement { + infos[idx].Found = true + infos[idx].State = CRDManagedByCIO + } + } + msg := "CRDs updated by CIO" + if len(missing) > 0 { + msg = fmt.Sprintf("CRDs updated by CIO; reinstalled: %s", strings.Join(missing, ", ")) + } + if len(unlabeled) > 0 { + msg += fmt.Sprintf("; reclaimed: %s", strings.Join(unlabeled, ", ")) + } + return crdResult{State: CRDManagedByCIO, CRDs: infos, Message: msg} + + case CRDManagedByOLM: + return crdResult{State: CRDManagedByOLM, CRDs: infos, Message: "CRDs managed by OSSM subscription via OLM"} + + case CRDUnknownManagement: + missing := missingCRDNames(infos) + msg := "CRDs exist with unknown management" + if len(missing) > 0 { + msg += fmt.Sprintf("; missing from other owner: %s", strings.Join(missing, ", ")) + } + return crdResult{State: CRDUnknownManagement, CRDs: infos, Message: msg, Error: fmt.Errorf("Istio CRDs are managed by an unknown party")} + + case CRDMixedOwnership: + missing := missingCRDNames(infos) + msg := "CRDs have mixed ownership" + if len(missing) > 0 { + msg += fmt.Sprintf("; missing: %s", strings.Join(missing, ", ")) + } + return crdResult{State: CRDMixedOwnership, CRDs: infos, Message: msg, Error: fmt.Errorf("Istio CRDs have mixed ownership (CIO/OLM/other)")} + + default: + return crdResult{State: state, CRDs: infos} + } +} + +// WatchTargets computes the set of CRD names that should be watched for changes. +// Returns nil if the target set cannot be determined. +func (m *crdManager) WatchTargets(values *v1.Values, includeAllCRDs bool) map[string]struct{} { + var targets []string + var err error + + if includeAllCRDs { + targets, err = allIstioCRDs() + } else if values != nil && values.Pilot != nil && values.Pilot.Env != nil { + targets, err = targetCRDsFromValues(values, false) + } + + if err != nil || len(targets) == 0 { + return nil + } + + names := make(map[string]struct{}, len(targets)) + for _, name := range targets { + names[name] = struct{}{} + } + return names +} + +// missingCRDNames returns the names of CRDs that were not found on the cluster. +func missingCRDNames(infos []CRDInfo) []string { + var missing []string + for _, info := range infos { + if !info.Found { + missing = append(missing, info.Name) + } + } + return missing +} + +// unlabeledCRDNames returns names of CRDs that exist but have unknown management (no CIO/OLM labels). +func unlabeledCRDNames(infos []CRDInfo) []string { + var names []string + for _, info := range infos { + if info.Found && info.State == CRDUnknownManagement { + names = append(names, info.Name) + } + } + return names +} + +// installCRDs installs all target CRDs with CIO ownership labels and Helm keep annotation. +func (m *crdManager) installCRDs(ctx context.Context, targets []string) error { + for _, resource := range targets { + crd, err := loadCRD(resource) + if err != nil { + return err + } + applyCIOLabels(crd) + if err := m.cl.Create(ctx, crd); err != nil { + return fmt.Errorf("failed to create CRD %s: %w", crd.Name, err) + } + } + return nil +} + +// updateCRDs updates existing CIO-owned CRDs and creates any missing ones. +func (m *crdManager) updateCRDs(ctx context.Context, targets []string) error { + for _, resource := range targets { + crd, err := loadCRD(resource) + if err != nil { + return err + } + applyCIOLabels(crd) + + existing := &apiextensionsv1.CustomResourceDefinition{} + if err := m.cl.Get(ctx, client.ObjectKey{Name: crd.Name}, existing); err != nil { + if apierrors.IsNotFound(err) { + // CRD was deleted — reinstall it + if err := m.cl.Create(ctx, crd); err != nil { + return fmt.Errorf("failed to create CRD %s: %w", crd.Name, err) + } + continue + } + return fmt.Errorf("failed to get existing CRD %s: %w", crd.Name, err) + } + crd.ResourceVersion = existing.ResourceVersion + if err := m.cl.Update(ctx, crd); err != nil { + return fmt.Errorf("failed to update CRD %s: %w", crd.Name, err) + } + } + return nil +} + +// loadCRD reads and unmarshals a CRD from the embedded filesystem. +func loadCRD(resource string) (*apiextensionsv1.CustomResourceDefinition, error) { + filename := resourceToCRDFilename(resource) + if filename == "" { + return nil, fmt.Errorf("invalid resource name: %s", resource) + } + + data, err := fs.ReadFile(crds.FS, filename) + if err != nil { + return nil, fmt.Errorf("failed to read CRD file %s: %w", filename, err) + } + + crd := &apiextensionsv1.CustomResourceDefinition{} + if err := yaml.Unmarshal(data, crd); err != nil { + return nil, fmt.Errorf("failed to unmarshal CRD %s: %w", filename, err) + } + return crd, nil +} + +// applyCIOLabels sets the CIO ownership label and Helm keep annotation on a CRD. +func applyCIOLabels(crd *apiextensionsv1.CustomResourceDefinition) { + labels := crd.GetLabels() + if labels == nil { + labels = make(map[string]string) + } + labels[labelManagedByCIO] = "true" + crd.SetLabels(labels) + + annotations := crd.GetAnnotations() + if annotations == nil { + annotations = make(map[string]string) + } + annotations[annotationHelmKeep] = "keep" + crd.SetAnnotations(annotations) +} diff --git a/pkg/install/crds_filter.go b/pkg/install/crds_filter.go new file mode 100644 index 0000000000..f02c9b3240 --- /dev/null +++ b/pkg/install/crds_filter.go @@ -0,0 +1,180 @@ +// 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 install + +import ( + "fmt" + "io/fs" + "path/filepath" + "strings" + + v1 "github.com/istio-ecosystem/sail-operator/api/v1" + "github.com/istio-ecosystem/sail-operator/chart/crds" +) + +// Environment variable names for Istio resource filtering. +// X_ prefix prevents Istio from processing until the feature is ready. +// When activating, remove the X_ prefix. +// See: https://github.com/istio/istio/commit/7e58d08397ee7b7119bf49abc9bd7b4f550f7839 +const ( + envPilotIgnoreResources = "X_PILOT_IGNORE_RESOURCES" + envPilotIncludeResources = "X_PILOT_INCLUDE_RESOURCES" +) + +// Resource filtering values for Gateway API mode. +// Ignore all istio.io resources except the 3 needed for gateway customization. +const ( + gatewayAPIIgnoreResources = "*.istio.io" + gatewayAPIIncludeResources = "wasmplugins.extensions.istio.io,envoyfilters.networking.istio.io,destinationrules.networking.istio.io" +) + +// resourceToCRDFilename converts a resource name to its CRD filename. +// The naming convention is: "{plural}.{group}" -> "{group}_{plural}.yaml" +// Example: "wasmplugins.extensions.istio.io" -> "extensions.istio.io_wasmplugins.yaml" +func resourceToCRDFilename(resource string) string { + parts := strings.SplitN(resource, ".", 2) + if len(parts) != 2 { + return "" + } + plural := parts[0] // e.g., "wasmplugins" + group := parts[1] // e.g., "extensions.istio.io" + return fmt.Sprintf("%s_%s.yaml", group, plural) +} + +// crdFilenameToResource converts a CRD filename back to a resource name. +// The naming convention is: "{group}_{plural}.yaml" -> "{plural}.{group}" +// Example: "extensions.istio.io_wasmplugins.yaml" -> "wasmplugins.extensions.istio.io" +func crdFilenameToResource(filename string) string { + // Strip .yaml suffix + name := strings.TrimSuffix(filename, ".yaml") + // Split on underscore: group_plural + parts := strings.SplitN(name, "_", 2) + if len(parts) != 2 { + return "" + } + group := parts[0] // e.g., "extensions.istio.io" + plural := parts[1] // e.g., "wasmplugins" + return fmt.Sprintf("%s.%s", plural, group) +} + +// matchesPattern checks if a resource matches a filter pattern. +// Patterns support glob-style wildcards: +// - "*.istio.io" matches "virtualservices.networking.istio.io" +// - "virtualservices.networking.istio.io" matches exactly +// +// The resource format is "{plural}.{group}" (e.g., "wasmplugins.extensions.istio.io") +func matchesPattern(resource, pattern string) bool { + // Handle glob patterns using filepath.Match + // Pattern "*.istio.io" should match "virtualservices.networking.istio.io" + matched, err := filepath.Match(pattern, resource) + if err != nil { + return false + } + return matched +} + +// matchesAnyPattern checks if a resource matches any pattern in a comma-separated list. +func matchesAnyPattern(resource, patterns string) bool { + if patterns == "" { + return false + } + for _, pattern := range strings.Split(patterns, ",") { + pattern = strings.TrimSpace(pattern) + if matchesPattern(resource, pattern) { + return true + } + } + return false +} + +// shouldManageResource determines if a resource should be managed based on +// IGNORE and INCLUDE filters. The logic follows Istio's resource filtering: +// - If resource matches INCLUDE, manage it (INCLUDE overrides IGNORE) +// - If resource matches IGNORE (and not INCLUDE), skip it +// - If resource matches neither, manage it (default allow) +func shouldManageResource(resource, ignorePatterns, includePatterns string) bool { + // INCLUDE takes precedence - if explicitly included, always manage + if matchesAnyPattern(resource, includePatterns) { + return true + } + // If ignored (and not included), skip + if matchesAnyPattern(resource, ignorePatterns) { + return false + } + // Default: manage if not matched by any filter + return true +} + +// allIstioCRDs returns all *.istio.io CRD resource names from the embedded CRD filesystem. +// Sail operator CRDs (sailoperator.io) are excluded since those belong to the Sail operator. +func allIstioCRDs() ([]string, error) { + entries, err := fs.ReadDir(crds.FS, ".") + if err != nil { + return nil, fmt.Errorf("failed to read CRD directory: %w", err) + } + + var resources []string + for _, entry := range entries { + if entry.IsDir() || !strings.HasSuffix(entry.Name(), ".yaml") { + continue + } + // Skip Sail operator CRDs + if strings.HasPrefix(entry.Name(), "sailoperator.io_") { + continue + } + resource := crdFilenameToResource(entry.Name()) + if resource != "" { + resources = append(resources, resource) + } + } + return resources, nil +} + +// targetCRDsFromValues determines the target CRD set based on values and options. +// If includeAllCRDs is true, returns all *.istio.io CRDs. +// Otherwise, returns CRDs derived from PILOT_INCLUDE_RESOURCES in values. +func targetCRDsFromValues(values *v1.Values, includeAllCRDs bool) ([]string, error) { + if includeAllCRDs { + return allIstioCRDs() + } + + if values == nil || values.Pilot == nil || values.Pilot.Env == nil { + return nil, nil + } + + ignorePatterns := values.Pilot.Env[envPilotIgnoreResources] + includePatterns := values.Pilot.Env[envPilotIncludeResources] + + // If no filters defined, nothing to do + if ignorePatterns == "" && includePatterns == "" { + return nil, nil + } + + var targets []string + if includePatterns != "" { + for _, resource := range strings.Split(includePatterns, ",") { + resource = strings.TrimSpace(resource) + if !shouldManageResource(resource, ignorePatterns, includePatterns) { + continue + } + // Skip resources that don't map to a valid CRD filename (e.g. wildcards) + if resourceToCRDFilename(resource) == "" { + continue + } + targets = append(targets, resource) + } + } + return targets, nil +} diff --git a/pkg/install/crds_filter_test.go b/pkg/install/crds_filter_test.go new file mode 100644 index 0000000000..9ac5526e5c --- /dev/null +++ b/pkg/install/crds_filter_test.go @@ -0,0 +1,378 @@ +// 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 install + +import ( + "testing" + + v1 "github.com/istio-ecosystem/sail-operator/api/v1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestResourceToCRDFilename(t *testing.T) { + tests := []struct { + name string + resource string + expected string + }{ + { + name: "wasmplugins", + resource: "wasmplugins.extensions.istio.io", + expected: "extensions.istio.io_wasmplugins.yaml", + }, + { + name: "envoyfilters", + resource: "envoyfilters.networking.istio.io", + expected: "networking.istio.io_envoyfilters.yaml", + }, + { + name: "destinationrules", + resource: "destinationrules.networking.istio.io", + expected: "networking.istio.io_destinationrules.yaml", + }, + { + name: "virtualservices", + resource: "virtualservices.networking.istio.io", + expected: "networking.istio.io_virtualservices.yaml", + }, + { + name: "authorizationpolicies", + resource: "authorizationpolicies.security.istio.io", + expected: "security.istio.io_authorizationpolicies.yaml", + }, + { + name: "invalid - no group", + resource: "wasmplugins", + expected: "", + }, + { + name: "invalid - empty", + resource: "", + expected: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := resourceToCRDFilename(tt.resource) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestCRDFilenameToResource(t *testing.T) { + tests := []struct { + name string + filename string + expected string + }{ + { + name: "wasmplugins", + filename: "extensions.istio.io_wasmplugins.yaml", + expected: "wasmplugins.extensions.istio.io", + }, + { + name: "envoyfilters", + filename: "networking.istio.io_envoyfilters.yaml", + expected: "envoyfilters.networking.istio.io", + }, + { + name: "no underscore", + filename: "invalid.yaml", + expected: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := crdFilenameToResource(tt.filename) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestMatchesPattern(t *testing.T) { + tests := []struct { + name string + resource string + pattern string + expected bool + }{ + { + name: "exact match", + resource: "wasmplugins.extensions.istio.io", + pattern: "wasmplugins.extensions.istio.io", + expected: true, + }, + { + name: "wildcard suffix matches istio resources", + resource: "virtualservices.networking.istio.io", + pattern: "*.istio.io", + expected: true, // * matches any characters including dots + }, + { + name: "wildcard matches wasmplugins", + resource: "wasmplugins.extensions.istio.io", + pattern: "*.istio.io", + expected: true, + }, + { + name: "wildcard matches short name", + resource: "networking.istio.io", + pattern: "*.istio.io", + expected: true, + }, + { + name: "no match - different domain", + resource: "gateways.gateway.networking.k8s.io", + pattern: "*.istio.io", + expected: false, + }, + { + name: "no match - exact pattern mismatch", + resource: "wasmplugins.extensions.istio.io", + pattern: "virtualservices.networking.istio.io", + expected: false, + }, + { + name: "empty pattern", + resource: "wasmplugins.extensions.istio.io", + pattern: "", + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := matchesPattern(tt.resource, tt.pattern) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestMatchesAnyPattern(t *testing.T) { + tests := []struct { + name string + resource string + patterns string + expected bool + }{ + { + name: "matches first pattern", + resource: "wasmplugins.extensions.istio.io", + patterns: "wasmplugins.extensions.istio.io,envoyfilters.networking.istio.io", + expected: true, + }, + { + name: "matches second pattern", + resource: "envoyfilters.networking.istio.io", + patterns: "wasmplugins.extensions.istio.io,envoyfilters.networking.istio.io", + expected: true, + }, + { + name: "no match", + resource: "virtualservices.networking.istio.io", + patterns: "wasmplugins.extensions.istio.io,envoyfilters.networking.istio.io", + expected: false, + }, + { + name: "empty patterns", + resource: "wasmplugins.extensions.istio.io", + patterns: "", + expected: false, + }, + { + name: "patterns with spaces", + resource: "wasmplugins.extensions.istio.io", + patterns: " wasmplugins.extensions.istio.io , envoyfilters.networking.istio.io ", + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := matchesAnyPattern(tt.resource, tt.patterns) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestShouldManageResource(t *testing.T) { + tests := []struct { + name string + resource string + ignore string + include string + expected bool + }{ + { + name: "included resource - should manage", + resource: "wasmplugins.extensions.istio.io", + ignore: "", + include: "wasmplugins.extensions.istio.io", + expected: true, + }, + { + name: "include overrides ignore", + resource: "wasmplugins.extensions.istio.io", + ignore: "wasmplugins.extensions.istio.io", + include: "wasmplugins.extensions.istio.io", + expected: true, // INCLUDE takes precedence + }, + { + name: "ignored and not included - should not manage", + resource: "virtualservices.networking.istio.io", + ignore: "virtualservices.networking.istio.io", + include: "wasmplugins.extensions.istio.io", + expected: false, + }, + { + name: "not in any filter - default manage", + resource: "wasmplugins.extensions.istio.io", + ignore: "", + include: "", + expected: true, + }, + { + name: "Gateway API mode - included resource", + resource: "wasmplugins.extensions.istio.io", + ignore: gatewayAPIIgnoreResources, + include: gatewayAPIIncludeResources, + expected: true, + }, + { + name: "Gateway API mode - envoyfilters included", + resource: "envoyfilters.networking.istio.io", + ignore: gatewayAPIIgnoreResources, + include: gatewayAPIIncludeResources, + expected: true, + }, + { + name: "Gateway API mode - destinationrules included", + resource: "destinationrules.networking.istio.io", + ignore: gatewayAPIIgnoreResources, + include: gatewayAPIIncludeResources, + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := shouldManageResource(tt.resource, tt.ignore, tt.include) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestResourceFilteringConstants(t *testing.T) { + // Verify the constants have expected values + assert.Equal(t, "X_PILOT_IGNORE_RESOURCES", envPilotIgnoreResources) + assert.Equal(t, "X_PILOT_INCLUDE_RESOURCES", envPilotIncludeResources) + assert.Equal(t, "*.istio.io", gatewayAPIIgnoreResources) + assert.Contains(t, gatewayAPIIncludeResources, "wasmplugins.extensions.istio.io") + assert.Contains(t, gatewayAPIIncludeResources, "envoyfilters.networking.istio.io") + assert.Contains(t, gatewayAPIIncludeResources, "destinationrules.networking.istio.io") +} + +func TestGatewayAPIFiltersWorkTogether(t *testing.T) { + // Test that our Gateway API filter values work correctly together + // IGNORE: *.istio.io (all istio resources) + // INCLUDE: wasmplugins, envoyfilters, destinationrules (exceptions) + + // These should be managed (explicitly included, overrides IGNORE) + assert.True(t, shouldManageResource("wasmplugins.extensions.istio.io", gatewayAPIIgnoreResources, gatewayAPIIncludeResources), + "wasmplugins should be managed (in INCLUDE)") + assert.True(t, shouldManageResource("envoyfilters.networking.istio.io", gatewayAPIIgnoreResources, gatewayAPIIncludeResources), + "envoyfilters should be managed (in INCLUDE)") + assert.True(t, shouldManageResource("destinationrules.networking.istio.io", gatewayAPIIgnoreResources, gatewayAPIIncludeResources), + "destinationrules should be managed (in INCLUDE)") + + // These should NOT be managed (matched by IGNORE, not in INCLUDE) + assert.False(t, shouldManageResource("virtualservices.networking.istio.io", gatewayAPIIgnoreResources, gatewayAPIIncludeResources), + "virtualservices should NOT be managed (in IGNORE, not in INCLUDE)") + assert.False(t, shouldManageResource("gateways.networking.istio.io", gatewayAPIIgnoreResources, gatewayAPIIncludeResources), + "gateways should NOT be managed (in IGNORE, not in INCLUDE)") + assert.False(t, shouldManageResource("serviceentries.networking.istio.io", gatewayAPIIgnoreResources, gatewayAPIIncludeResources), + "serviceentries should NOT be managed (in IGNORE, not in INCLUDE)") + assert.False(t, shouldManageResource("authorizationpolicies.security.istio.io", gatewayAPIIgnoreResources, gatewayAPIIncludeResources), + "authorizationpolicies should NOT be managed (in IGNORE, not in INCLUDE)") + + // Non-istio resources should be managed (not matched by IGNORE) + assert.True(t, shouldManageResource("gateways.gateway.networking.k8s.io", gatewayAPIIgnoreResources, gatewayAPIIncludeResources), + "k8s gateway resources should be managed (not in IGNORE)") +} + +func TestAllIstioCRDs(t *testing.T) { + resources, err := allIstioCRDs() + require.NoError(t, err) + + // Should have Istio CRDs but no sailoperator.io CRDs + assert.NotEmpty(t, resources) + for _, r := range resources { + assert.NotContains(t, r, "sailoperator.io", "sailoperator.io CRDs should be excluded") + assert.Contains(t, r, "istio.io", "all CRDs should be *.istio.io") + } + + // Verify expected CRDs are present + assert.Contains(t, resources, "wasmplugins.extensions.istio.io") + assert.Contains(t, resources, "envoyfilters.networking.istio.io") + assert.Contains(t, resources, "destinationrules.networking.istio.io") + assert.Contains(t, resources, "virtualservices.networking.istio.io") +} + +func TestTargetCRDsFromValues(t *testing.T) { + t.Run("include all CRDs", func(t *testing.T) { + targets, err := targetCRDsFromValues(nil, true) + require.NoError(t, err) + assert.NotEmpty(t, targets) + // Should contain all istio CRDs + assert.Contains(t, targets, "wasmplugins.extensions.istio.io") + }) + + t.Run("filtered by PILOT_INCLUDE_RESOURCES", func(t *testing.T) { + values := &v1.Values{ + Pilot: &v1.PilotConfig{ + Env: map[string]string{ + envPilotIgnoreResources: gatewayAPIIgnoreResources, + envPilotIncludeResources: gatewayAPIIncludeResources, + }, + }, + } + targets, err := targetCRDsFromValues(values, false) + require.NoError(t, err) + assert.Len(t, targets, 3) + assert.Contains(t, targets, "wasmplugins.extensions.istio.io") + assert.Contains(t, targets, "envoyfilters.networking.istio.io") + assert.Contains(t, targets, "destinationrules.networking.istio.io") + }) + + t.Run("nil values", func(t *testing.T) { + targets, err := targetCRDsFromValues(nil, false) + require.NoError(t, err) + assert.Empty(t, targets) + }) + + t.Run("no filters defined", func(t *testing.T) { + values := &v1.Values{ + Pilot: &v1.PilotConfig{ + Env: map[string]string{}, + }, + } + targets, err := targetCRDsFromValues(values, false) + require.NoError(t, err) + assert.Empty(t, targets) + }) +} diff --git a/pkg/install/crds_test.go b/pkg/install/crds_test.go new file mode 100644 index 0000000000..d7c3cf4ee9 --- /dev/null +++ b/pkg/install/crds_test.go @@ -0,0 +1,669 @@ +// 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 install + +import ( + "context" + "testing" + + v1 "github.com/istio-ecosystem/sail-operator/api/v1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func TestAggregateCRDState(t *testing.T) { + tests := []struct { + name string + infos []CRDInfo + expected CRDManagementState + }{ + { + name: "empty list", + infos: []CRDInfo{}, + expected: CRDNoneExist, + }, + { + name: "all not found", + infos: []CRDInfo{ + {Name: "a.istio.io", Found: false}, + {Name: "b.istio.io", Found: false}, + }, + expected: CRDNoneExist, + }, + { + name: "all CIO", + infos: []CRDInfo{ + {Name: "a.istio.io", Found: true, State: CRDManagedByCIO}, + {Name: "b.istio.io", Found: true, State: CRDManagedByCIO}, + }, + expected: CRDManagedByCIO, + }, + { + name: "all OLM", + infos: []CRDInfo{ + {Name: "a.istio.io", Found: true, State: CRDManagedByOLM}, + {Name: "b.istio.io", Found: true, State: CRDManagedByOLM}, + }, + expected: CRDManagedByOLM, + }, + { + name: "CIO with unknown - reclaim as CIO", + infos: []CRDInfo{ + {Name: "a.istio.io", Found: true, State: CRDManagedByCIO}, + {Name: "b.istio.io", Found: true, State: CRDUnknownManagement}, + }, + expected: CRDManagedByCIO, + }, + { + name: "OLM with unknown - mixed", + infos: []CRDInfo{ + {Name: "a.istio.io", Found: true, State: CRDManagedByOLM}, + {Name: "b.istio.io", Found: true, State: CRDUnknownManagement}, + }, + expected: CRDMixedOwnership, + }, + { + name: "CIO and OLM mix", + infos: []CRDInfo{ + {Name: "a.istio.io", Found: true, State: CRDManagedByCIO}, + {Name: "b.istio.io", Found: true, State: CRDManagedByOLM}, + }, + expected: CRDMixedOwnership, + }, + { + name: "CIO with some missing - still CIO owned", + infos: []CRDInfo{ + {Name: "a.istio.io", Found: true, State: CRDManagedByCIO}, + {Name: "b.istio.io", Found: false}, + }, + expected: CRDManagedByCIO, + }, + { + name: "OLM with some missing - mixed", + infos: []CRDInfo{ + {Name: "a.istio.io", Found: true, State: CRDManagedByOLM}, + {Name: "b.istio.io", Found: false}, + }, + expected: CRDMixedOwnership, + }, + { + name: "all found but all unknown", + infos: []CRDInfo{ + {Name: "a.istio.io", Found: true, State: CRDUnknownManagement}, + {Name: "b.istio.io", Found: true, State: CRDUnknownManagement}, + }, + expected: CRDUnknownManagement, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := aggregateCRDState(tt.infos) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestMissingCRDNames(t *testing.T) { + infos := []CRDInfo{ + {Name: "a.istio.io", Found: true}, + {Name: "b.istio.io", Found: false}, + {Name: "c.istio.io", Found: true}, + {Name: "d.istio.io", Found: false}, + } + + missing := missingCRDNames(infos) + assert.Equal(t, []string{"b.istio.io", "d.istio.io"}, missing) +} + +func TestMissingCRDNamesAllFound(t *testing.T) { + infos := []CRDInfo{ + {Name: "a.istio.io", Found: true}, + {Name: "b.istio.io", Found: true}, + } + + missing := missingCRDNames(infos) + assert.Empty(t, missing) +} + +// crdScheme returns a runtime.Scheme with CRD types registered. +func crdScheme() *runtime.Scheme { + s := runtime.NewScheme() + _ = apiextensionsv1.AddToScheme(s) + return s +} + +func TestClassifyCRD(t *testing.T) { + tests := []struct { + name string + crd *apiextensionsv1.CustomResourceDefinition + expected CRDInfo + }{ + { + name: "CRD with CIO label", + crd: &apiextensionsv1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "wasmplugins.extensions.istio.io", + Labels: map[string]string{labelManagedByCIO: "true"}, + }, + }, + expected: CRDInfo{ + Name: "wasmplugins.extensions.istio.io", + Found: true, + State: CRDManagedByCIO, + }, + }, + { + name: "CRD with OLM label", + crd: &apiextensionsv1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "envoyfilters.networking.istio.io", + Labels: map[string]string{labelOLMManaged: "true"}, + }, + }, + expected: CRDInfo{ + Name: "envoyfilters.networking.istio.io", + Found: true, + State: CRDManagedByOLM, + }, + }, + { + name: "CRD with no labels", + crd: &apiextensionsv1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gateways.networking.istio.io", + }, + }, + expected: CRDInfo{ + Name: "gateways.networking.istio.io", + Found: true, + State: CRDUnknownManagement, + }, + }, + { + name: "CRD not found", + crd: nil, + expected: CRDInfo{ + Name: "missing.istio.io", + Found: false, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + builder := fake.NewClientBuilder().WithScheme(crdScheme()) + if tt.crd != nil { + builder = builder.WithObjects(tt.crd) + } + cl := builder.Build() + mgr := newCRDManager(cl) + + crdName := tt.expected.Name + result := mgr.classifyCRD(context.Background(), crdName, nil) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestClassifyCRD_OverwriteOLM(t *testing.T) { + olmCRD := &apiextensionsv1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "envoyfilters.networking.istio.io", + Labels: map[string]string{labelOLMManaged: "true"}, + }, + } + + t.Run("callback returns true - reclassify as CIO", func(t *testing.T) { + cl := fake.NewClientBuilder().WithScheme(crdScheme()).WithObjects(olmCRD).Build() + mgr := newCRDManager(cl) + + overwrite := func(_ context.Context, _ *apiextensionsv1.CustomResourceDefinition) bool { return true } + result := mgr.classifyCRD(context.Background(), "envoyfilters.networking.istio.io", overwrite) + assert.Equal(t, CRDManagedByCIO, result.State) + assert.True(t, result.Found) + }) + + t.Run("callback returns false - stays OLM", func(t *testing.T) { + cl := fake.NewClientBuilder().WithScheme(crdScheme()).WithObjects(olmCRD).Build() + mgr := newCRDManager(cl) + + overwrite := func(_ context.Context, _ *apiextensionsv1.CustomResourceDefinition) bool { return false } + result := mgr.classifyCRD(context.Background(), "envoyfilters.networking.istio.io", overwrite) + assert.Equal(t, CRDManagedByOLM, result.State) + assert.True(t, result.Found) + }) + + t.Run("nil callback - stays OLM", func(t *testing.T) { + cl := fake.NewClientBuilder().WithScheme(crdScheme()).WithObjects(olmCRD).Build() + mgr := newCRDManager(cl) + + result := mgr.classifyCRD(context.Background(), "envoyfilters.networking.istio.io", nil) + assert.Equal(t, CRDManagedByOLM, result.State) + assert.True(t, result.Found) + }) + + t.Run("callback receives the CRD object", func(t *testing.T) { + cl := fake.NewClientBuilder().WithScheme(crdScheme()).WithObjects(olmCRD).Build() + mgr := newCRDManager(cl) + + var receivedCRD *apiextensionsv1.CustomResourceDefinition + overwrite := func(_ context.Context, crd *apiextensionsv1.CustomResourceDefinition) bool { + receivedCRD = crd + return false + } + mgr.classifyCRD(context.Background(), "envoyfilters.networking.istio.io", overwrite) + require.NotNil(t, receivedCRD) + assert.Equal(t, "envoyfilters.networking.istio.io", receivedCRD.Name) + assert.Equal(t, "true", receivedCRD.Labels[labelOLMManaged]) + }) +} + +func TestUnlabeledCRDNames(t *testing.T) { + tests := []struct { + name string + infos []CRDInfo + expected []string + }{ + { + name: "mixed states", + infos: []CRDInfo{ + {Name: "a.istio.io", Found: true, State: CRDManagedByCIO}, + {Name: "b.istio.io", Found: true, State: CRDUnknownManagement}, + {Name: "c.istio.io", Found: false}, + {Name: "d.istio.io", Found: true, State: CRDUnknownManagement}, + }, + expected: []string{"b.istio.io", "d.istio.io"}, + }, + { + name: "all labeled", + infos: []CRDInfo{ + {Name: "a.istio.io", Found: true, State: CRDManagedByCIO}, + {Name: "b.istio.io", Found: true, State: CRDManagedByOLM}, + }, + expected: nil, + }, + { + name: "empty", + infos: nil, + expected: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := unlabeledCRDNames(tt.infos) + assert.Equal(t, tt.expected, result) + }) + } +} + +// --- Helpers for Reconcile / WatchTargets tests --- + +// The 3 CRD resource names used by Gateway API mode. +var gatewayAPICRDNames = []string{ + "wasmplugins.extensions.istio.io", + "envoyfilters.networking.istio.io", + "destinationrules.networking.istio.io", +} + +// makeGatewayAPIValues returns Values with the Gateway API env vars that +// produce the 3 target CRDs via targetCRDsFromValues. +func makeGatewayAPIValues() *v1.Values { + return &v1.Values{ + Pilot: &v1.PilotConfig{ + Env: map[string]string{ + envPilotIgnoreResources: gatewayAPIIgnoreResources, + envPilotIncludeResources: gatewayAPIIncludeResources, + }, + }, + } +} + +// makeCRDStub creates a minimal CRD object with the given name and labels. +func makeCRDStub(name string, labels map[string]string) *apiextensionsv1.CustomResourceDefinition { + return &apiextensionsv1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Labels: labels, + }, + } +} + +// getCRDFromClient fetches a CRD by name from the fake client. +func getCRDFromClient(t *testing.T, cl client.Client, name string) *apiextensionsv1.CustomResourceDefinition { + t.Helper() + crd := &apiextensionsv1.CustomResourceDefinition{} + err := cl.Get(context.Background(), client.ObjectKey{Name: name}, crd) + require.NoError(t, err, "expected CRD %s to exist on fake client", name) + return crd +} + +// --- TestApplyCIOLabels --- + +func TestApplyCIOLabels(t *testing.T) { + t.Run("bare CRD gets labels and annotations", func(t *testing.T) { + crd := &apiextensionsv1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "test.istio.io"}, + } + applyCIOLabels(crd) + assert.Equal(t, "true", crd.Labels[labelManagedByCIO]) + assert.Equal(t, "keep", crd.Annotations[annotationHelmKeep]) + }) + + t.Run("existing labels and annotations preserved", func(t *testing.T) { + crd := &apiextensionsv1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test.istio.io", + Labels: map[string]string{"existing": "label"}, + Annotations: map[string]string{"existing": "annotation"}, + }, + } + applyCIOLabels(crd) + assert.Equal(t, "true", crd.Labels[labelManagedByCIO]) + assert.Equal(t, "label", crd.Labels["existing"]) + assert.Equal(t, "keep", crd.Annotations[annotationHelmKeep]) + assert.Equal(t, "annotation", crd.Annotations["existing"]) + }) +} + +// --- TestLoadCRD --- + +func TestLoadCRD(t *testing.T) { + t.Run("valid resource", func(t *testing.T) { + crd, err := loadCRD("wasmplugins.extensions.istio.io") + require.NoError(t, err) + assert.Equal(t, "wasmplugins.extensions.istio.io", crd.Name) + assert.Equal(t, "extensions.istio.io", crd.Spec.Group) + assert.Equal(t, "WasmPlugin", crd.Spec.Names.Kind) + }) + + t.Run("invalid resource name", func(t *testing.T) { + _, err := loadCRD("invalid") + assert.Error(t, err) + assert.Contains(t, err.Error(), "invalid resource name") + }) + + t.Run("nonexistent resource", func(t *testing.T) { + _, err := loadCRD("fake.nonexistent.istio.io") + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to read CRD file") + }) +} + +// --- TestClassifyCRDs --- + +func TestClassifyCRDs(t *testing.T) { + t.Run("CIO with missing", func(t *testing.T) { + cl := fake.NewClientBuilder().WithScheme(crdScheme()).WithObjects( + makeCRDStub("wasmplugins.extensions.istio.io", map[string]string{labelManagedByCIO: "true"}), + makeCRDStub("envoyfilters.networking.istio.io", map[string]string{labelManagedByCIO: "true"}), + ).Build() + mgr := newCRDManager(cl) + + state, infos := mgr.classifyCRDs(context.Background(), gatewayAPICRDNames, nil) + assert.Equal(t, CRDManagedByCIO, state) + assert.Len(t, infos, 3) + // The missing one should be Found=false + for _, info := range infos { + if info.Name == "destinationrules.networking.istio.io" { + assert.False(t, info.Found) + } else { + assert.True(t, info.Found) + assert.Equal(t, CRDManagedByCIO, info.State) + } + } + }) + + t.Run("mixed CIO and OLM", func(t *testing.T) { + cl := fake.NewClientBuilder().WithScheme(crdScheme()).WithObjects( + makeCRDStub("wasmplugins.extensions.istio.io", map[string]string{labelManagedByCIO: "true"}), + makeCRDStub("envoyfilters.networking.istio.io", map[string]string{labelOLMManaged: "true"}), + makeCRDStub("destinationrules.networking.istio.io", nil), + ).Build() + mgr := newCRDManager(cl) + + state, _ := mgr.classifyCRDs(context.Background(), gatewayAPICRDNames, nil) + assert.Equal(t, CRDMixedOwnership, state) + }) + + t.Run("empty targets", func(t *testing.T) { + cl := fake.NewClientBuilder().WithScheme(crdScheme()).Build() + mgr := newCRDManager(cl) + + state, infos := mgr.classifyCRDs(context.Background(), nil, nil) + assert.Equal(t, CRDNoneExist, state) + assert.Nil(t, infos) + }) +} + +// --- TestReconcile --- + +func TestReconcile_NoneExist(t *testing.T) { + cl := fake.NewClientBuilder().WithScheme(crdScheme()).Build() + mgr := newCRDManager(cl) + + result := mgr.Reconcile(context.Background(), makeGatewayAPIValues(), false, nil) + assert.Equal(t, CRDManagedByCIO, result.State) + assert.NoError(t, result.Error) + assert.Contains(t, result.Message, "installed by CIO") + assert.Len(t, result.CRDs, 3) + + for _, info := range result.CRDs { + assert.True(t, info.Found) + assert.Equal(t, CRDManagedByCIO, info.State) + } + + // Verify CRDs were actually created with CIO labels + for _, name := range gatewayAPICRDNames { + crd := getCRDFromClient(t, cl, name) + assert.Equal(t, "true", crd.Labels[labelManagedByCIO], "CRD %s missing CIO label", name) + assert.Equal(t, "keep", crd.Annotations[annotationHelmKeep], "CRD %s missing helm keep annotation", name) + } +} + +func TestReconcile_CIOOwned_Updates(t *testing.T) { + // Pre-seed all 3 CRDs with CIO labels + var objs []client.Object + for _, name := range gatewayAPICRDNames { + objs = append(objs, makeCRDStub(name, map[string]string{labelManagedByCIO: "true"})) + } + cl := fake.NewClientBuilder().WithScheme(crdScheme()).WithObjects(objs...).Build() + mgr := newCRDManager(cl) + + result := mgr.Reconcile(context.Background(), makeGatewayAPIValues(), false, nil) + assert.Equal(t, CRDManagedByCIO, result.State) + assert.NoError(t, result.Error) + assert.Contains(t, result.Message, "updated by CIO") + + // Verify CRDs still have CIO labels + for _, name := range gatewayAPICRDNames { + crd := getCRDFromClient(t, cl, name) + assert.Equal(t, "true", crd.Labels[labelManagedByCIO]) + } +} + +func TestReconcile_CIOOwned_ReinstallsMissing(t *testing.T) { + // Pre-seed 2 of 3 with CIO labels; destinationrules is missing + cl := fake.NewClientBuilder().WithScheme(crdScheme()).WithObjects( + makeCRDStub("wasmplugins.extensions.istio.io", map[string]string{labelManagedByCIO: "true"}), + makeCRDStub("envoyfilters.networking.istio.io", map[string]string{labelManagedByCIO: "true"}), + ).Build() + mgr := newCRDManager(cl) + + result := mgr.Reconcile(context.Background(), makeGatewayAPIValues(), false, nil) + assert.Equal(t, CRDManagedByCIO, result.State) + assert.NoError(t, result.Error) + assert.Contains(t, result.Message, "reinstalled") + assert.Contains(t, result.Message, "destinationrules.networking.istio.io") + + // Verify the missing CRD was created + crd := getCRDFromClient(t, cl, "destinationrules.networking.istio.io") + assert.Equal(t, "true", crd.Labels[labelManagedByCIO]) +} + +func TestReconcile_CIOOwned_ReclaimsUnlabeled(t *testing.T) { + // 2 CIO-labeled + 1 unlabeled + cl := fake.NewClientBuilder().WithScheme(crdScheme()).WithObjects( + makeCRDStub("wasmplugins.extensions.istio.io", map[string]string{labelManagedByCIO: "true"}), + makeCRDStub("envoyfilters.networking.istio.io", map[string]string{labelManagedByCIO: "true"}), + makeCRDStub("destinationrules.networking.istio.io", nil), + ).Build() + mgr := newCRDManager(cl) + + result := mgr.Reconcile(context.Background(), makeGatewayAPIValues(), false, nil) + assert.Equal(t, CRDManagedByCIO, result.State) + assert.NoError(t, result.Error) + assert.Contains(t, result.Message, "reclaimed") + assert.Contains(t, result.Message, "destinationrules.networking.istio.io") + + // Verify the previously-unlabeled CRD now has CIO label + crd := getCRDFromClient(t, cl, "destinationrules.networking.istio.io") + assert.Equal(t, "true", crd.Labels[labelManagedByCIO]) +} + +func TestReconcile_OLMOwned(t *testing.T) { + var objs []client.Object + for _, name := range gatewayAPICRDNames { + objs = append(objs, makeCRDStub(name, map[string]string{labelOLMManaged: "true"})) + } + cl := fake.NewClientBuilder().WithScheme(crdScheme()).WithObjects(objs...).Build() + mgr := newCRDManager(cl) + + result := mgr.Reconcile(context.Background(), makeGatewayAPIValues(), false, nil) + assert.Equal(t, CRDManagedByOLM, result.State) + assert.NoError(t, result.Error) + assert.Contains(t, result.Message, "OLM") +} + +func TestReconcile_MixedOwnership(t *testing.T) { + cl := fake.NewClientBuilder().WithScheme(crdScheme()).WithObjects( + makeCRDStub("wasmplugins.extensions.istio.io", map[string]string{labelManagedByCIO: "true"}), + makeCRDStub("envoyfilters.networking.istio.io", map[string]string{labelOLMManaged: "true"}), + makeCRDStub("destinationrules.networking.istio.io", nil), + ).Build() + mgr := newCRDManager(cl) + + result := mgr.Reconcile(context.Background(), makeGatewayAPIValues(), false, nil) + assert.Equal(t, CRDMixedOwnership, result.State) + assert.Error(t, result.Error) + assert.Contains(t, result.Error.Error(), "mixed ownership") +} + +func TestReconcile_NoTargets(t *testing.T) { + cl := fake.NewClientBuilder().WithScheme(crdScheme()).Build() + mgr := newCRDManager(cl) + + result := mgr.Reconcile(context.Background(), nil, false, nil) + assert.Equal(t, CRDNoneExist, result.State) + assert.NoError(t, result.Error) + assert.Contains(t, result.Message, "no target CRDs configured") +} + +// --- TestReconcile_OverwriteOLM --- + +func TestReconcile_OLMOwned_OverwriteAdopts(t *testing.T) { + var objs []client.Object + for _, name := range gatewayAPICRDNames { + objs = append(objs, makeCRDStub(name, map[string]string{labelOLMManaged: "true"})) + } + cl := fake.NewClientBuilder().WithScheme(crdScheme()).WithObjects(objs...).Build() + mgr := newCRDManager(cl) + + overwrite := func(_ context.Context, _ *apiextensionsv1.CustomResourceDefinition) bool { return true } + result := mgr.Reconcile(context.Background(), makeGatewayAPIValues(), false, overwrite) + assert.Equal(t, CRDManagedByCIO, result.State) + assert.NoError(t, result.Error) + + for _, name := range gatewayAPICRDNames { + crd := getCRDFromClient(t, cl, name) + assert.Equal(t, "true", crd.Labels[labelManagedByCIO], "CRD %s should have CIO label after adoption", name) + _, hasOLM := crd.Labels[labelOLMManaged] + assert.False(t, hasOLM, "CRD %s should not have OLM label after adoption", name) + } +} + +func TestReconcile_OLMOwned_OverwriteFalse_LeavesAlone(t *testing.T) { + var objs []client.Object + for _, name := range gatewayAPICRDNames { + objs = append(objs, makeCRDStub(name, map[string]string{labelOLMManaged: "true"})) + } + cl := fake.NewClientBuilder().WithScheme(crdScheme()).WithObjects(objs...).Build() + mgr := newCRDManager(cl) + + overwrite := func(_ context.Context, _ *apiextensionsv1.CustomResourceDefinition) bool { return false } + result := mgr.Reconcile(context.Background(), makeGatewayAPIValues(), false, overwrite) + assert.Equal(t, CRDManagedByOLM, result.State) + assert.NoError(t, result.Error) +} + +func TestReconcile_MixedCIOAndOLM_OverwriteResolves(t *testing.T) { + cl := fake.NewClientBuilder().WithScheme(crdScheme()).WithObjects( + makeCRDStub("wasmplugins.extensions.istio.io", map[string]string{labelManagedByCIO: "true"}), + makeCRDStub("envoyfilters.networking.istio.io", map[string]string{labelOLMManaged: "true"}), + makeCRDStub("destinationrules.networking.istio.io", map[string]string{labelOLMManaged: "true"}), + ).Build() + mgr := newCRDManager(cl) + + overwrite := func(_ context.Context, _ *apiextensionsv1.CustomResourceDefinition) bool { return true } + result := mgr.Reconcile(context.Background(), makeGatewayAPIValues(), false, overwrite) + assert.Equal(t, CRDManagedByCIO, result.State) + assert.NoError(t, result.Error) + + for _, name := range gatewayAPICRDNames { + crd := getCRDFromClient(t, cl, name) + assert.Equal(t, "true", crd.Labels[labelManagedByCIO], "CRD %s should have CIO label", name) + } +} + +// --- TestWatchTargets --- + +func TestWatchTargets(t *testing.T) { + cl := fake.NewClientBuilder().WithScheme(crdScheme()).Build() + mgr := newCRDManager(cl) + + t.Run("includeAllCRDs returns all istio.io CRDs", func(t *testing.T) { + targets := mgr.WatchTargets(nil, true) + assert.NotNil(t, targets) + assert.Greater(t, len(targets), 0) + // Should include istio.io CRDs + _, hasWasm := targets["wasmplugins.extensions.istio.io"] + assert.True(t, hasWasm, "should include wasmplugins") + // Should not include sailoperator.io CRDs + for name := range targets { + assert.NotContains(t, name, "sailoperator.io", "should exclude sail operator CRDs") + } + }) + + t.Run("filtered by PILOT_INCLUDE_RESOURCES", func(t *testing.T) { + targets := mgr.WatchTargets(makeGatewayAPIValues(), false) + assert.NotNil(t, targets) + assert.Len(t, targets, 3) + for _, name := range gatewayAPICRDNames { + _, ok := targets[name] + assert.True(t, ok, "expected %s in watch targets", name) + } + }) + + t.Run("nil values returns nil", func(t *testing.T) { + targets := mgr.WatchTargets(nil, false) + assert.Nil(t, targets) + }) +} diff --git a/pkg/install/images.go b/pkg/install/images.go new file mode 100644 index 0000000000..831a568bbe --- /dev/null +++ b/pkg/install/images.go @@ -0,0 +1,167 @@ +// 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 install + +import ( + "fmt" + "io/fs" + "strings" + + "github.com/istio-ecosystem/sail-operator/pkg/config" + "gopkg.in/yaml.v3" +) + +// ImageNames defines the image name for each component (without registry or tag). +type ImageNames struct { + Istiod string + Proxy string + CNI string + ZTunnel string +} + +// SetImageDefaults populates config.Config.ImageDigests by scanning resourceFS +// for version directories and constructing image refs from the given registry +// and image names. +// +// This is a no-op if config.Config.ImageDigests is already populated (e.g. by +// config.Read() in the operator path). +// +// Example: +// +// install.SetImageDefaults(resourceFS, "registry.redhat.io/openshift-service-mesh", install.ImageNames{ +// Istiod: "istio-pilot-rhel9", +// Proxy: "istio-proxyv2-rhel9", +// CNI: "istio-cni-rhel9", +// ZTunnel: "istio-ztunnel-rhel9", +// }) +func SetImageDefaults(resourceFS fs.FS, registry string, images ImageNames) error { + if config.Config.ImageDigests != nil { + return nil + } + entries, err := fs.ReadDir(resourceFS, ".") + if err != nil { + return fmt.Errorf("failed to read resource directory: %w", err) + } + config.Config.ImageDigests = make(map[string]config.IstioImageConfig) + for _, e := range entries { + if !e.IsDir() { + continue + } + v := e.Name() + tag := strings.TrimPrefix(v, "v") + config.Config.ImageDigests[v] = config.IstioImageConfig{ + IstiodImage: registry + "/" + images.Istiod + ":" + tag, + ProxyImage: registry + "/" + images.Proxy + ":" + tag, + CNIImage: registry + "/" + images.CNI + ":" + tag, + ZTunnelImage: registry + "/" + images.ZTunnel + ":" + tag, + } + } + return nil +} + +// csvDeployment is the minimal structure needed to reach pod template annotations +// inside a ClusterServiceVersion YAML. +type csvDeployment struct { + Spec struct { + Install struct { + Spec struct { + Deployments []struct { + Spec struct { + Template struct { + Metadata struct { + Annotations map[string]string `yaml:"annotations"` + } `yaml:"metadata"` + } `yaml:"template"` + } `yaml:"spec"` + } `yaml:"deployments"` + } `yaml:"spec"` + } `yaml:"install"` + } `yaml:"spec"` +} + +// LoadImageDigestsFromCSV reads image references from ClusterServiceVersion +// YAML bytes and populates config.Config.ImageDigests. +// +// The CSV's pod template annotations are expected to contain keys of the form +// "images.." (e.g. "images.v1_27_0.istiod"). Underscores +// in the version segment are converted to dots (v1_27_0 -> v1.27.0), matching +// the convention used by config.Read(). +// +// This is a no-op if config.Config.ImageDigests is already populated. +func LoadImageDigestsFromCSV(csvData []byte) error { + if config.Config.ImageDigests != nil { + return nil + } + + var csv csvDeployment + if err := yaml.Unmarshal(csvData, &csv); err != nil { + return fmt.Errorf("failed to parse CSV YAML: %w", err) + } + + annotations := findImageAnnotations(csv) + if len(annotations) == 0 { + return fmt.Errorf("no image annotations found in CSV") + } + + config.Config.ImageDigests = buildImageDigests(annotations) + return nil +} + +// findImageAnnotations extracts annotations starting with "images." from the +// first deployment in the CSV. +func findImageAnnotations(csv csvDeployment) map[string]string { + deployments := csv.Spec.Install.Spec.Deployments + if len(deployments) == 0 { + return nil + } + all := deployments[0].Spec.Template.Metadata.Annotations + filtered := make(map[string]string, len(all)) + for k, v := range all { + if strings.HasPrefix(k, "images.") { + filtered[k] = v + } + } + return filtered +} + +// buildImageDigests converts flat annotation keys ("images.v1_27_0.istiod") +// into a map[version]IstioImageConfig, replacing underscores with dots in +// the version segment. +func buildImageDigests(annotations map[string]string) map[string]config.IstioImageConfig { + digests := make(map[string]config.IstioImageConfig) + for key, image := range annotations { + // key format: "images.." + parts := strings.SplitN(key, ".", 3) + if len(parts) != 3 { + continue + } + version := strings.ReplaceAll(parts[1], "_", ".") + component := parts[2] + + cfg := digests[version] + switch component { + case "istiod": + cfg.IstiodImage = image + case "proxy": + cfg.ProxyImage = image + case "cni": + cfg.CNIImage = image + case "ztunnel": + cfg.ZTunnelImage = image + } + digests[version] = cfg + } + return digests +} diff --git a/pkg/install/images_test.go b/pkg/install/images_test.go new file mode 100644 index 0000000000..40f2ce44d0 --- /dev/null +++ b/pkg/install/images_test.go @@ -0,0 +1,207 @@ +// 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 install + +import ( + "testing" + "testing/fstest" + + "github.com/istio-ecosystem/sail-operator/bundle" + "github.com/istio-ecosystem/sail-operator/pkg/config" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestSetImageDefaults(t *testing.T) { + registry := "registry.example.com/istio" + images := ImageNames{ + Istiod: "pilot-rhel9", + Proxy: "proxyv2-rhel9", + CNI: "cni-rhel9", + ZTunnel: "ztunnel-rhel9", + } + + t.Run("populates from FS with multiple version dirs", func(t *testing.T) { + config.Config = config.OperatorConfig{} + fs := fstest.MapFS{ + "v1.27.1/charts/istiod/Chart.yaml": &fstest.MapFile{}, + "v1.28.0/charts/istiod/Chart.yaml": &fstest.MapFile{}, + } + + err := SetImageDefaults(fs, registry, images) + require.NoError(t, err) + + assert.Len(t, config.Config.ImageDigests, 2) + + v1271 := config.Config.ImageDigests["v1.27.1"] + assert.Equal(t, "registry.example.com/istio/pilot-rhel9:1.27.1", v1271.IstiodImage) + assert.Equal(t, "registry.example.com/istio/proxyv2-rhel9:1.27.1", v1271.ProxyImage) + assert.Equal(t, "registry.example.com/istio/cni-rhel9:1.27.1", v1271.CNIImage) + assert.Equal(t, "registry.example.com/istio/ztunnel-rhel9:1.27.1", v1271.ZTunnelImage) + + v1280 := config.Config.ImageDigests["v1.28.0"] + assert.Equal(t, "registry.example.com/istio/pilot-rhel9:1.28.0", v1280.IstiodImage) + assert.Equal(t, "registry.example.com/istio/proxyv2-rhel9:1.28.0", v1280.ProxyImage) + assert.Equal(t, "registry.example.com/istio/cni-rhel9:1.28.0", v1280.CNIImage) + assert.Equal(t, "registry.example.com/istio/ztunnel-rhel9:1.28.0", v1280.ZTunnelImage) + }) + + t.Run("no-op when ImageDigests already set", func(t *testing.T) { + config.Config = config.OperatorConfig{ + ImageDigests: map[string]config.IstioImageConfig{ + "v1.27.1": {IstiodImage: "already-set"}, + }, + } + + fs := fstest.MapFS{ + "v1.28.0/charts/istiod/Chart.yaml": &fstest.MapFile{}, + } + + err := SetImageDefaults(fs, registry, images) + require.NoError(t, err) + + assert.Len(t, config.Config.ImageDigests, 1) + assert.Equal(t, "already-set", config.Config.ImageDigests["v1.27.1"].IstiodImage) + }) + + t.Run("skips non-directory entries", func(t *testing.T) { + config.Config = config.OperatorConfig{} + fs := fstest.MapFS{ + "v1.27.1/charts/istiod/Chart.yaml": &fstest.MapFile{}, + "resources.go": &fstest.MapFile{}, + "README.md": &fstest.MapFile{}, + } + + err := SetImageDefaults(fs, registry, images) + require.NoError(t, err) + + assert.Len(t, config.Config.ImageDigests, 1) + assert.Contains(t, config.Config.ImageDigests, "v1.27.1") + }) +} + +const minimalCSV = `apiVersion: operators.coreos.com/v1alpha1 +kind: ClusterServiceVersion +metadata: + name: test-operator.v1.0.0 +spec: + install: + spec: + deployments: + - name: test-operator + spec: + template: + metadata: + annotations: + images.v1_27_0.istiod: gcr.io/istio-release/pilot:1.27.0 + images.v1_27_0.proxy: gcr.io/istio-release/proxyv2:1.27.0 + images.v1_27_0.cni: gcr.io/istio-release/install-cni:1.27.0 + images.v1_27_0.ztunnel: gcr.io/istio-release/ztunnel:1.27.0 + images.v1_28_1.istiod: gcr.io/istio-release/pilot:1.28.1 + images.v1_28_1.proxy: gcr.io/istio-release/proxyv2:1.28.1 + images.v1_28_1.cni: gcr.io/istio-release/install-cni:1.28.1 + images.v1_28_1.ztunnel: gcr.io/istio-release/ztunnel:1.28.1 + unrelated-annotation: something-else +` + +func TestLoadImageDigestsFromCSV(t *testing.T) { + t.Run("parses image annotations by version", func(t *testing.T) { + config.Config = config.OperatorConfig{} + + err := LoadImageDigestsFromCSV([]byte(minimalCSV)) + require.NoError(t, err) + + assert.Len(t, config.Config.ImageDigests, 2) + + v1270 := config.Config.ImageDigests["v1.27.0"] + assert.Equal(t, "gcr.io/istio-release/pilot:1.27.0", v1270.IstiodImage) + assert.Equal(t, "gcr.io/istio-release/proxyv2:1.27.0", v1270.ProxyImage) + assert.Equal(t, "gcr.io/istio-release/install-cni:1.27.0", v1270.CNIImage) + assert.Equal(t, "gcr.io/istio-release/ztunnel:1.27.0", v1270.ZTunnelImage) + + v1281 := config.Config.ImageDigests["v1.28.1"] + assert.Equal(t, "gcr.io/istio-release/pilot:1.28.1", v1281.IstiodImage) + assert.Equal(t, "gcr.io/istio-release/proxyv2:1.28.1", v1281.ProxyImage) + assert.Equal(t, "gcr.io/istio-release/install-cni:1.28.1", v1281.CNIImage) + assert.Equal(t, "gcr.io/istio-release/ztunnel:1.28.1", v1281.ZTunnelImage) + }) + + t.Run("handles alpha version with commit hash", func(t *testing.T) { + config.Config = config.OperatorConfig{} + csv := `apiVersion: operators.coreos.com/v1alpha1 +kind: ClusterServiceVersion +spec: + install: + spec: + deployments: + - name: op + spec: + template: + metadata: + annotations: + images.v1_30-alpha_abc123.istiod: gcr.io/istio-testing/pilot:1.30-alpha.abc123 + images.v1_30-alpha_abc123.proxy: gcr.io/istio-testing/proxyv2:1.30-alpha.abc123 +` + + err := LoadImageDigestsFromCSV([]byte(csv)) + require.NoError(t, err) + + v := config.Config.ImageDigests["v1.30-alpha.abc123"] + assert.Equal(t, "gcr.io/istio-testing/pilot:1.30-alpha.abc123", v.IstiodImage) + assert.Equal(t, "gcr.io/istio-testing/proxyv2:1.30-alpha.abc123", v.ProxyImage) + }) + + t.Run("no-op when ImageDigests already set", func(t *testing.T) { + config.Config = config.OperatorConfig{ + ImageDigests: map[string]config.IstioImageConfig{ + "v1.27.0": {IstiodImage: "already-set"}, + }, + } + + err := LoadImageDigestsFromCSV([]byte(minimalCSV)) + require.NoError(t, err) + + assert.Len(t, config.Config.ImageDigests, 1) + assert.Equal(t, "already-set", config.Config.ImageDigests["v1.27.0"].IstiodImage) + }) + + t.Run("error when CSV has no image annotations", func(t *testing.T) { + config.Config = config.OperatorConfig{} + csv := `apiVersion: operators.coreos.com/v1alpha1 +kind: ClusterServiceVersion +spec: + install: + spec: + deployments: + - name: op + spec: + template: + metadata: + annotations: + unrelated: value +` + + err := LoadImageDigestsFromCSV([]byte(csv)) + require.Error(t, err) + assert.Contains(t, err.Error(), "no image annotations found") + }) +} + +func TestLoadImageDigestsFromEmbeddedCSV(t *testing.T) { + config.Config = config.OperatorConfig{} + err := LoadImageDigestsFromCSV(bundle.CSV) + require.NoError(t, err) + assert.NotEmpty(t, config.Config.ImageDigests, "expected at least one version in CSV image annotations") +} diff --git a/pkg/install/informers.go b/pkg/install/informers.go new file mode 100644 index 0000000000..b516a34dcc --- /dev/null +++ b/pkg/install/informers.go @@ -0,0 +1,241 @@ +// 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 install + +import ( + "time" + + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/dynamic/dynamicinformer" + "k8s.io/client-go/tools/cache" + "k8s.io/utils/ptr" + ctrllog "sigs.k8s.io/controller-runtime/pkg/log" +) + +const ( + // defaultResyncPeriod is the default resync period for informers. + defaultResyncPeriod = 30 * time.Minute +) + +// setupInformers creates dynamic informers for Helm resources and CRDs +// based on the current desired options. +func (l *Library) setupInformers(stopCh <-chan struct{}) { + log := ctrllog.Log.WithName("install") + + l.mu.RLock() + opts := *l.desiredOpts + l.mu.RUnlock() + + // Helm resource watches (drift detection) + specs, err := l.inst.getWatchSpecs(opts) + if err != nil { + log.Error(err, "Failed to compute watch specs; Helm drift detection disabled") + specs = nil + } + + // CRD watches (ownership changes, creation, deletion) + if ptr.Deref(opts.ManageCRDs, true) { + crdSpec := l.buildCRDWatchSpec(opts) + if crdSpec != nil { + specs = append(specs, *crdSpec) + } + } + + if len(specs) == 0 { + log.Info("No watch specs; informers not started") + return + } + + log.Info("Setting up informers", "count", len(specs)) + + namespacedFactory := dynamicinformer.NewFilteredDynamicSharedInformerFactory( + l.dynamicCl, + defaultResyncPeriod, + opts.Namespace, + nil, + ) + clusterScopedFactory := dynamicinformer.NewDynamicSharedInformerFactory( + l.dynamicCl, + defaultResyncPeriod, + ) + + // Capture revision and managedByValue once for all event handlers — no lock needed per event. + revision := opts.Revision + managedByValue := l.managedByValue + enqueue := l.enqueue + + for _, spec := range specs { + var informer cache.SharedIndexInformer + gvr := gvkToGVR(spec.GVK) + + if spec.ClusterScoped { + informer = clusterScopedFactory.ForResource(gvr).Informer() + } else { + informer = namespacedFactory.ForResource(gvr).Informer() + } + + var handler cache.ResourceEventHandler + if spec.watchType == watchTypeCRD { + handler = makeCRDEventHandler(spec.TargetNames, enqueue) + } else { + handler = makeOwnedEventHandler(spec.GVK, spec.watchType, revision, managedByValue, enqueue) + } + + if _, err := informer.AddEventHandler(handler); err != nil { + log.Error(err, "Failed to add event handler", "gvk", spec.GVK) + continue + } + log.V(1).Info("Watching", "gvk", spec.GVK, "type", spec.watchType, "clusterScoped", spec.ClusterScoped) + } + + namespacedFactory.Start(stopCh) + clusterScopedFactory.Start(stopCh) + namespacedFactory.WaitForCacheSync(stopCh) + clusterScopedFactory.WaitForCacheSync(stopCh) + log.Info("Informers synced and watching for drift") +} + +// buildCRDWatchSpec computes a watchSpec for Istio CRDs based on the current options. +// Returns nil if the target CRD set cannot be determined. +func (l *Library) buildCRDWatchSpec(opts Options) *watchSpec { + targetNames := l.inst.crdManager.WatchTargets(opts.Values, ptr.Deref(opts.IncludeAllCRDs, false)) + if len(targetNames) == 0 { + return nil + } + + return &watchSpec{ + GVK: crdGVK, + watchType: watchTypeCRD, + ClusterScoped: true, + TargetNames: targetNames, + } +} + +// makeOwnedEventHandler handles events for Helm-managed and namespace resources. +// It takes explicit dependencies instead of closing over Library, enabling unit testing +// without concurrency machinery. +func makeOwnedEventHandler(gvk schema.GroupVersionKind, wt watchType, revision, managedByValue string, enqueue func()) cache.ResourceEventHandler { + log := ctrllog.Log.WithName("install").WithValues("gvk", gvk, "watchType", wt) + return cache.ResourceEventHandlerFuncs{ + // Add events fire during initial cache sync — ignore them. + AddFunc: func(obj interface{}) {}, + UpdateFunc: func(oldObj, newObj interface{}) { + oldU, ok := oldObj.(*unstructured.Unstructured) + if !ok { + return + } + newU, ok := newObj.(*unstructured.Unstructured) + if !ok { + return + } + + if !shouldReconcileOnUpdate(gvk, oldU, newU) { + log.V(2).Info("Skipping update (predicate)", "name", newU.GetName()) + return + } + + if wt == watchTypeOwned && !isOwnedResource(newU, revision, managedByValue) { + log.V(1).Info("Skipping update (not owned)", "name", newU.GetName(), "labels", newU.GetLabels()) + return + } + + log.Info("Drift detected (update)", "name", newU.GetName()) + enqueue() + }, + DeleteFunc: func(obj interface{}) { + if tombstone, ok := obj.(cache.DeletedFinalStateUnknown); ok { + obj = tombstone.Obj + } + + u, ok := obj.(*unstructured.Unstructured) + if !ok { + return + } + + if !shouldReconcileOnDelete(u) { + return + } + + if wt == watchTypeOwned && !isOwnedResource(u, revision, managedByValue) { + log.V(1).Info("Skipping delete (not owned)", "name", u.GetName()) + return + } + + log.Info("Drift detected (delete)", "name", u.GetName()) + enqueue() + }, + } +} + +// makeCRDEventHandler handles events for CRD resources. Unlike owned resources, +// CRD events trigger on create (new CRD appeared), delete (CRD removed), and +// label/annotation changes (ownership transfer). Events are filtered by name +// to only watch target Istio CRDs. +func makeCRDEventHandler(targets map[string]struct{}, enqueue func()) cache.ResourceEventHandler { + log := ctrllog.Log.WithName("install").WithValues("watchType", watchTypeCRD) + return cache.ResourceEventHandlerFuncs{ + AddFunc: func(obj interface{}) { + u, ok := obj.(*unstructured.Unstructured) + if !ok { + return + } + if !isTargetCRD(u, targets) { + return + } + log.Info("Drift detected (CRD added)", "name", u.GetName()) + enqueue() + }, + UpdateFunc: func(oldObj, newObj interface{}) { + oldU, ok := oldObj.(*unstructured.Unstructured) + if !ok { + return + } + newU, ok := newObj.(*unstructured.Unstructured) + if !ok { + return + } + if !isTargetCRD(newU, targets) { + return + } + if !shouldReconcileCRDOnUpdate(oldU, newU) { + return + } + log.Info("Drift detected (CRD updated)", "name", newU.GetName()) + enqueue() + }, + DeleteFunc: func(obj interface{}) { + if tombstone, ok := obj.(cache.DeletedFinalStateUnknown); ok { + obj = tombstone.Obj + } + u, ok := obj.(*unstructured.Unstructured) + if !ok { + return + } + if !isTargetCRD(u, targets) { + return + } + log.Info("Drift detected (CRD deleted)", "name", u.GetName()) + enqueue() + }, + } +} + +// gvkToGVR converts a GroupVersionKind to a GroupVersionResource. +func gvkToGVR(gvk schema.GroupVersionKind) schema.GroupVersionResource { + plural, _ := meta.UnsafeGuessKindToResource(gvk) + return plural +} diff --git a/pkg/install/informers_test.go b/pkg/install/informers_test.go new file mode 100644 index 0000000000..132630a92a --- /dev/null +++ b/pkg/install/informers_test.go @@ -0,0 +1,245 @@ +// 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 install + +import ( + "sync/atomic" + "testing" + + "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/tools/cache" +) + +// enqueueCounter returns an enqueue func and a counter to check how many times it was called. +func enqueueCounter() (func(), *atomic.Int32) { + var count atomic.Int32 + return func() { count.Add(1) }, &count +} + +// makeObj creates an unstructured object with the given name and labels. +func makeObj(name string, labels map[string]string) *unstructured.Unstructured { + obj := &unstructured.Unstructured{} + obj.SetName(name) + if labels != nil { + obj.SetLabels(labels) + } + return obj +} + +// --- makeOwnedEventHandler tests --- + +func TestOwnedHandler_AddIsNoOp(t *testing.T) { + enqueue, count := enqueueCounter() + handler := makeOwnedEventHandler( + schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"}, + watchTypeOwned, + "default", + defaultManagedByValue, + enqueue, + ) + + handler.(cache.ResourceEventHandlerFuncs).AddFunc(makeObj("test", map[string]string{"istio.io/rev": "default"})) + assert.Equal(t, int32(0), count.Load(), "Add events should be ignored (initial cache sync)") +} + +func TestOwnedHandler_UpdateOwnedEnqueues(t *testing.T) { + enqueue, count := enqueueCounter() + gvk := schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"} + handler := makeOwnedEventHandler(gvk, watchTypeOwned, "default", defaultManagedByValue, enqueue) + + oldObj := makeObj("test", map[string]string{"istio.io/rev": "default"}) + oldObj.SetGeneration(1) + newObj := makeObj("test", map[string]string{"istio.io/rev": "default"}) + newObj.SetGeneration(2) + + handler.(cache.ResourceEventHandlerFuncs).UpdateFunc(oldObj, newObj) + assert.Equal(t, int32(1), count.Load(), "Owned resource update should enqueue") +} + +func TestOwnedHandler_UpdateNotOwnedSkips(t *testing.T) { + enqueue, count := enqueueCounter() + gvk := schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"} + handler := makeOwnedEventHandler(gvk, watchTypeOwned, "default", defaultManagedByValue, enqueue) + + oldObj := makeObj("test", map[string]string{"istio.io/rev": "other-revision"}) + oldObj.SetGeneration(1) + newObj := makeObj("test", map[string]string{"istio.io/rev": "other-revision"}) + newObj.SetGeneration(2) + + handler.(cache.ResourceEventHandlerFuncs).UpdateFunc(oldObj, newObj) + assert.Equal(t, int32(0), count.Load(), "Non-owned resource update should be skipped") +} + +func TestOwnedHandler_UpdateIgnoreAnnotationSkips(t *testing.T) { + enqueue, count := enqueueCounter() + gvk := schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"} + handler := makeOwnedEventHandler(gvk, watchTypeOwned, "default", defaultManagedByValue, enqueue) + + oldObj := makeObj("test", map[string]string{"istio.io/rev": "default"}) + oldObj.SetGeneration(1) + newObj := makeObj("test", map[string]string{"istio.io/rev": "default"}) + newObj.SetGeneration(2) + newObj.SetAnnotations(map[string]string{ignoreAnnotation: "true"}) + + handler.(cache.ResourceEventHandlerFuncs).UpdateFunc(oldObj, newObj) + assert.Equal(t, int32(0), count.Load(), "Resource with ignore annotation should be skipped") +} + +func TestOwnedHandler_UpdateNamespaceTypeSkipsOwnerCheck(t *testing.T) { + enqueue, count := enqueueCounter() + gvk := schema.GroupVersionKind{Version: "v1", Kind: "Namespace"} + // watchTypeNamespace does not check isOwnedResource + handler := makeOwnedEventHandler(gvk, watchTypeNamespace, "default", defaultManagedByValue, enqueue) + + oldObj := makeObj("istio-system", nil) // no ownership labels + oldObj.SetGeneration(1) + newObj := makeObj("istio-system", nil) + newObj.SetGeneration(2) + + handler.(cache.ResourceEventHandlerFuncs).UpdateFunc(oldObj, newObj) + assert.Equal(t, int32(1), count.Load(), "Namespace watch should not filter by ownership") +} + +func TestOwnedHandler_DeleteOwnedEnqueues(t *testing.T) { + enqueue, count := enqueueCounter() + gvk := schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"} + handler := makeOwnedEventHandler(gvk, watchTypeOwned, "default", defaultManagedByValue, enqueue) + + obj := makeObj("test", map[string]string{"istio.io/rev": "default"}) + handler.(cache.ResourceEventHandlerFuncs).DeleteFunc(obj) + assert.Equal(t, int32(1), count.Load(), "Owned resource delete should enqueue") +} + +func TestOwnedHandler_DeleteNotOwnedSkips(t *testing.T) { + enqueue, count := enqueueCounter() + gvk := schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"} + handler := makeOwnedEventHandler(gvk, watchTypeOwned, "default", defaultManagedByValue, enqueue) + + obj := makeObj("test", map[string]string{"istio.io/rev": "other"}) + handler.(cache.ResourceEventHandlerFuncs).DeleteFunc(obj) + assert.Equal(t, int32(0), count.Load(), "Non-owned resource delete should be skipped") +} + +func TestOwnedHandler_DeleteTombstoneEnqueues(t *testing.T) { + enqueue, count := enqueueCounter() + gvk := schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"} + handler := makeOwnedEventHandler(gvk, watchTypeOwned, "default", defaultManagedByValue, enqueue) + + obj := makeObj("test", map[string]string{"istio.io/rev": "default"}) + tombstone := cache.DeletedFinalStateUnknown{Key: "test", Obj: obj} + + handler.(cache.ResourceEventHandlerFuncs).DeleteFunc(tombstone) + assert.Equal(t, int32(1), count.Load(), "Tombstone delete of owned resource should enqueue") +} + +// --- makeCRDEventHandler tests --- + +func TestCRDHandler_AddTargetEnqueues(t *testing.T) { + enqueue, count := enqueueCounter() + targets := map[string]struct{}{"wasmplugins.extensions.istio.io": {}} + handler := makeCRDEventHandler(targets, enqueue) + + obj := makeObj("wasmplugins.extensions.istio.io", nil) + handler.(cache.ResourceEventHandlerFuncs).AddFunc(obj) + assert.Equal(t, int32(1), count.Load(), "Target CRD add should enqueue") +} + +func TestCRDHandler_AddNonTargetSkips(t *testing.T) { + enqueue, count := enqueueCounter() + targets := map[string]struct{}{"wasmplugins.extensions.istio.io": {}} + handler := makeCRDEventHandler(targets, enqueue) + + obj := makeObj("gateways.gateway.networking.k8s.io", nil) + handler.(cache.ResourceEventHandlerFuncs).AddFunc(obj) + assert.Equal(t, int32(0), count.Load(), "Non-target CRD add should be skipped") +} + +func TestCRDHandler_UpdateTargetWithLabelChangeEnqueues(t *testing.T) { + enqueue, count := enqueueCounter() + targets := map[string]struct{}{"wasmplugins.extensions.istio.io": {}} + handler := makeCRDEventHandler(targets, enqueue) + + oldObj := makeObj("wasmplugins.extensions.istio.io", nil) + oldObj.SetGeneration(1) + newObj := makeObj("wasmplugins.extensions.istio.io", map[string]string{"ingress.operator.openshift.io/owned": "true"}) + newObj.SetGeneration(1) + + handler.(cache.ResourceEventHandlerFuncs).UpdateFunc(oldObj, newObj) + assert.Equal(t, int32(1), count.Load(), "Target CRD label change should enqueue") +} + +func TestCRDHandler_UpdateTargetNoChangeSkips(t *testing.T) { + enqueue, count := enqueueCounter() + targets := map[string]struct{}{"wasmplugins.extensions.istio.io": {}} + handler := makeCRDEventHandler(targets, enqueue) + + oldObj := makeObj("wasmplugins.extensions.istio.io", map[string]string{"foo": "bar"}) + oldObj.SetGeneration(1) + oldObj.SetResourceVersion("100") + newObj := makeObj("wasmplugins.extensions.istio.io", map[string]string{"foo": "bar"}) + newObj.SetGeneration(1) + newObj.SetResourceVersion("101") // only RV changed + + handler.(cache.ResourceEventHandlerFuncs).UpdateFunc(oldObj, newObj) + assert.Equal(t, int32(0), count.Load(), "Target CRD with no meaningful change should be skipped") +} + +func TestCRDHandler_UpdateNonTargetSkips(t *testing.T) { + enqueue, count := enqueueCounter() + targets := map[string]struct{}{"wasmplugins.extensions.istio.io": {}} + handler := makeCRDEventHandler(targets, enqueue) + + oldObj := makeObj("gateways.gateway.networking.k8s.io", nil) + oldObj.SetGeneration(1) + newObj := makeObj("gateways.gateway.networking.k8s.io", map[string]string{"new": "label"}) + newObj.SetGeneration(1) + + handler.(cache.ResourceEventHandlerFuncs).UpdateFunc(oldObj, newObj) + assert.Equal(t, int32(0), count.Load(), "Non-target CRD update should be skipped") +} + +func TestCRDHandler_DeleteTargetEnqueues(t *testing.T) { + enqueue, count := enqueueCounter() + targets := map[string]struct{}{"wasmplugins.extensions.istio.io": {}} + handler := makeCRDEventHandler(targets, enqueue) + + obj := makeObj("wasmplugins.extensions.istio.io", nil) + handler.(cache.ResourceEventHandlerFuncs).DeleteFunc(obj) + assert.Equal(t, int32(1), count.Load(), "Target CRD delete should enqueue") +} + +func TestCRDHandler_DeleteNonTargetSkips(t *testing.T) { + enqueue, count := enqueueCounter() + targets := map[string]struct{}{"wasmplugins.extensions.istio.io": {}} + handler := makeCRDEventHandler(targets, enqueue) + + obj := makeObj("gateways.gateway.networking.k8s.io", nil) + handler.(cache.ResourceEventHandlerFuncs).DeleteFunc(obj) + assert.Equal(t, int32(0), count.Load(), "Non-target CRD delete should be skipped") +} + +func TestCRDHandler_DeleteTombstoneTargetEnqueues(t *testing.T) { + enqueue, count := enqueueCounter() + targets := map[string]struct{}{"wasmplugins.extensions.istio.io": {}} + handler := makeCRDEventHandler(targets, enqueue) + + obj := makeObj("wasmplugins.extensions.istio.io", nil) + tombstone := cache.DeletedFinalStateUnknown{Key: "wasmplugins.extensions.istio.io", Obj: obj} + + handler.(cache.ResourceEventHandlerFuncs).DeleteFunc(tombstone) + assert.Equal(t, int32(1), count.Load(), "Tombstone delete of target CRD should enqueue") +} diff --git a/pkg/install/installer.go b/pkg/install/installer.go new file mode 100644 index 0000000000..00c2a497c5 --- /dev/null +++ b/pkg/install/installer.go @@ -0,0 +1,350 @@ +// 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 install + +import ( + "bufio" + "context" + "errors" + "fmt" + "io" + "io/fs" + "sort" + "strings" + + v1 "github.com/istio-ecosystem/sail-operator/api/v1" + "github.com/istio-ecosystem/sail-operator/pkg/config" + "github.com/istio-ecosystem/sail-operator/pkg/constants" + "github.com/istio-ecosystem/sail-operator/pkg/helm" + sharedreconcile "github.com/istio-ecosystem/sail-operator/pkg/reconcile" + "github.com/istio-ecosystem/sail-operator/pkg/revision" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/yaml" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// watchType indicates how events for a resource type should be handled. +type watchType int + +const ( + // watchTypeOwned indicates resources owned by the installation. + // Use owner reference filtering to only watch resources created by the installer. + watchTypeOwned watchType = iota + + // watchTypeNamespace indicates namespace resources should be watched. + // Used to detect when the target namespace is created/deleted. + watchTypeNamespace + + // watchTypeCRD indicates CRD resources that should be watched for ownership changes. + // Events are filtered by TargetNames and trigger on label/annotation changes, + // creation, or deletion — allowing the Library to re-classify CRD ownership. + watchTypeCRD +) + +// String returns the string representation of the watchType. +func (w watchType) String() string { + switch w { + case watchTypeOwned: + return "Owned" + case watchTypeNamespace: + return "Namespace" + case watchTypeCRD: + return "CRD" + default: + return fmt.Sprintf("Unknown(%d)", w) + } +} + +// watchSpec describes a resource type that should be watched for drift detection. +type watchSpec struct { + // GVK is the GroupVersionKind of the resource to watch. + GVK schema.GroupVersionKind + + // watchType indicates how events should be handled. + watchType watchType + + // ClusterScoped indicates if this is a cluster-scoped resource (vs namespaced). + // Cluster-scoped resources require a different informer factory setup. + ClusterScoped bool + + // TargetNames is an optional set of resource names to filter on. + // Only used with watchTypeCRD to limit events to specific CRDs. + // When nil or empty, all resources of this GVK are watched. + TargetNames map[string]struct{} +} + +// installer owns the worker dependencies and all install/watch logic. +// It is separated from Library so that reconcile, uninstall, and watch-spec +// computation can be tested and reasoned about without concurrency machinery. +type installer struct { + resourceFS fs.FS + chartManager *helm.ChartManager + cl client.Client + crdManager *crdManager +} + +// resolveValues resolves the version and computes Helm values from Options. +// This eliminates the duplication between reconcile and getWatchSpecs. +func (inst *installer) resolveValues(opts Options) (string, *v1.Values, error) { + opts.applyDefaults() + + // Default version from FS if not specified + if opts.Version == "" { + v, err := DefaultVersion(inst.resourceFS) + if err != nil { + return "", nil, fmt.Errorf("failed to determine default version: %w", err) + } + opts.Version = v + } + + // Validate version + if err := ValidateVersion(inst.resourceFS, opts.Version); err != nil { + return "", nil, fmt.Errorf("invalid version %q: %w", opts.Version, err) + } + + // Compute values + values, err := revision.ComputeValues( + opts.Values, + opts.Namespace, + opts.Version, + config.PlatformOpenShift, + defaultProfile, + "", + inst.resourceFS, + opts.Revision, + ) + if err != nil { + return "", nil, fmt.Errorf("failed to compute values: %w", err) + } + + return opts.Version, values, nil +} + +// reconcile does the actual work: CRDs + Helm. Takes opts as an argument +// so the caller reads opts under lock and passes them in — no lock access here. +func (inst *installer) reconcile(ctx context.Context, opts Options) Status { + // Deep-copy Values so nothing in this function can mutate the + // stored desiredOpts through shared pointers. + if opts.Values != nil { + opts.Values = opts.Values.DeepCopy() + } + + status := Status{} + + resolvedVersion, values, err := inst.resolveValues(opts) + if err != nil { + status.Error = err + return status + } + status.Version = resolvedVersion + + // Manage CRDs + if ptr.Deref(opts.ManageCRDs, true) { + result := inst.crdManager.Reconcile(ctx, values, ptr.Deref(opts.IncludeAllCRDs, false), opts.OverwriteOLMManagedCRD) + status.CRDState = result.State + status.CRDs = result.CRDs + status.CRDMessage = result.Message + if result.Error != nil { + status.Error = result.Error + } + } + + // Helm install + reconcilerCfg := sharedreconcile.Config{ + ResourceFS: inst.resourceFS, + Platform: config.PlatformOpenShift, + DefaultProfile: defaultProfile, + OperatorNamespace: opts.Namespace, + ChartManager: inst.chartManager, + } + + istiodReconciler := sharedreconcile.NewIstiodReconciler(reconcilerCfg, inst.cl) + + if err := istiodReconciler.Validate(ctx, resolvedVersion, opts.Namespace, values); err != nil { + status.Error = errors.Join(status.Error, fmt.Errorf("validation failed: %w", err)) + return status + } + + if err := istiodReconciler.Install(ctx, resolvedVersion, opts.Namespace, values, opts.Revision, nil); err != nil { + status.Error = errors.Join(status.Error, fmt.Errorf("installation failed: %w", err)) + return status + } + + status.Installed = true + return status +} + +// uninstall removes the istiod Helm release. +func (inst *installer) uninstall(ctx context.Context, namespace, revision string) error { + reconcilerCfg := sharedreconcile.Config{ + ResourceFS: inst.resourceFS, + Platform: config.PlatformOpenShift, + DefaultProfile: defaultProfile, + OperatorNamespace: namespace, + ChartManager: inst.chartManager, + } + + istiodReconciler := sharedreconcile.NewIstiodReconciler(reconcilerCfg, inst.cl) + if err := istiodReconciler.Uninstall(ctx, namespace, revision); err != nil { + return fmt.Errorf("uninstallation failed: %w", err) + } + return nil +} + +// getWatchSpecs renders the istiod Helm charts and extracts the GVKs +// of all resources that would be created. Used internally by the Library +// to set up informers for drift detection. +// +// The returned specs include: +// - All resource types from the base and istiod charts (watchTypeOwned) +// - Namespace (watchTypeNamespace) for namespace existence checks +func (inst *installer) getWatchSpecs(opts Options) ([]watchSpec, error) { + resolvedVersion, values, err := inst.resolveValues(opts) + if err != nil { + return nil, err + } + helmValues := helm.FromValues(values) + + // Collect GVKs from both charts + gvkSet := make(map[schema.GroupVersionKind]struct{}) + + // Render base chart (for default revision) + if opts.Revision == defaultRevision { + baseChartPath := sharedreconcile.GetChartPath(resolvedVersion, constants.BaseChartName) + rendered, err := helm.RenderChart(inst.resourceFS, baseChartPath, helmValues, opts.Namespace, "base") + if err != nil { + return nil, fmt.Errorf("failed to render base chart: %w", err) + } + if err := extractGVKsFromRendered(rendered, gvkSet); err != nil { + return nil, fmt.Errorf("failed to extract GVKs from base chart: %w", err) + } + } + + // Render istiod chart + istiodChartPath := sharedreconcile.GetChartPath(resolvedVersion, constants.IstiodChartName) + rendered, err := helm.RenderChart(inst.resourceFS, istiodChartPath, helmValues, opts.Namespace, "istiod") + if err != nil { + return nil, fmt.Errorf("failed to render istiod chart: %w", err) + } + if err := extractGVKsFromRendered(rendered, gvkSet); err != nil { + return nil, fmt.Errorf("failed to extract GVKs from istiod chart: %w", err) + } + + // Convert to specs + specs := make([]watchSpec, 0, len(gvkSet)+1) + for gvk := range gvkSet { + specs = append(specs, watchSpec{ + GVK: gvk, + watchType: watchTypeOwned, + ClusterScoped: isClusterScoped(gvk), + }) + } + + // Add Namespace watch + specs = append(specs, watchSpec{ + GVK: corev1.SchemeGroupVersion.WithKind("Namespace"), + watchType: watchTypeNamespace, + ClusterScoped: true, // Namespaces are cluster-scoped + }) + + // Sort for deterministic output + sort.Slice(specs, func(i, j int) bool { + if specs[i].GVK.Group != specs[j].GVK.Group { + return specs[i].GVK.Group < specs[j].GVK.Group + } + if specs[i].GVK.Version != specs[j].GVK.Version { + return specs[i].GVK.Version < specs[j].GVK.Version + } + return specs[i].GVK.Kind < specs[j].GVK.Kind + }) + + return specs, nil +} + +// extractGVKsFromRendered parses rendered Helm output and extracts GVKs. +func extractGVKsFromRendered(rendered map[string]string, gvkSet map[schema.GroupVersionKind]struct{}) error { + for name, content := range rendered { + // Skip empty files and notes + if content == "" || strings.HasSuffix(name, "NOTES.txt") { + continue + } + + // Parse multi-document YAML + decoder := yaml.NewYAMLOrJSONDecoder( + bufio.NewReader(strings.NewReader(content)), + 4096, + ) + + for { + var obj map[string]interface{} + if err := decoder.Decode(&obj); err != nil { + if err == io.EOF { + break + } + return fmt.Errorf("failed to decode YAML from %s: %w", name, err) + } + + // Skip empty documents + if obj == nil { + continue + } + + // Extract apiVersion and kind + apiVersion, ok := obj["apiVersion"].(string) + if !ok { + continue + } + kind, ok := obj["kind"].(string) + if !ok { + continue + } + + gvk := apiVersionKindToGVK(apiVersion, kind) + gvkSet[gvk] = struct{}{} + } + } + return nil +} + +// apiVersionKindToGVK converts apiVersion and kind strings to a GroupVersionKind. +func apiVersionKindToGVK(apiVersion, kind string) schema.GroupVersionKind { + gv, err := schema.ParseGroupVersion(apiVersion) + if err != nil { + // Fallback for malformed apiVersion + return schema.GroupVersionKind{ + Group: "", + Version: apiVersion, + Kind: kind, + } + } + return gv.WithKind(kind) +} + +// clusterScopedKinds is the set of known cluster-scoped kinds used in Istio charts. +var clusterScopedKinds = map[string]bool{ + "Namespace": true, + "ClusterRole": true, + "ClusterRoleBinding": true, + "CustomResourceDefinition": true, + "MutatingWebhookConfiguration": true, + "ValidatingWebhookConfiguration": true, +} + +// isClusterScoped returns true if the given GVK is a cluster-scoped resource. +func isClusterScoped(gvk schema.GroupVersionKind) bool { + return clusterScopedKinds[gvk.Kind] +} diff --git a/pkg/install/installer_test.go b/pkg/install/installer_test.go new file mode 100644 index 0000000000..d948ec256b --- /dev/null +++ b/pkg/install/installer_test.go @@ -0,0 +1,415 @@ +// 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 install + +import ( + "context" + "io/fs" + "testing" + "testing/fstest" + + v1 "github.com/istio-ecosystem/sail-operator/api/v1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/utils/ptr" +) + +func TestWatchTypeString(t *testing.T) { + tests := []struct { + watchType watchType + expected string + }{ + {watchTypeOwned, "Owned"}, + {watchTypeNamespace, "Namespace"}, + {watchTypeCRD, "CRD"}, + {watchType(99), "Unknown(99)"}, + } + + for _, tt := range tests { + t.Run(tt.expected, func(t *testing.T) { + assert.Equal(t, tt.expected, tt.watchType.String()) + }) + } +} + +func TestAPIVersionKindToGVK(t *testing.T) { + tests := []struct { + name string + apiVersion string + kind string + expected schema.GroupVersionKind + }{ + { + name: "core v1 resource", + apiVersion: "v1", + kind: "ConfigMap", + expected: schema.GroupVersionKind{Group: "", Version: "v1", Kind: "ConfigMap"}, + }, + { + name: "apps v1 resource", + apiVersion: "apps/v1", + kind: "Deployment", + expected: schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"}, + }, + { + name: "rbac resource", + apiVersion: "rbac.authorization.k8s.io/v1", + kind: "ClusterRole", + expected: schema.GroupVersionKind{Group: "rbac.authorization.k8s.io", Version: "v1", Kind: "ClusterRole"}, + }, + { + name: "admissionregistration resource", + apiVersion: "admissionregistration.k8s.io/v1", + kind: "MutatingWebhookConfiguration", + expected: schema.GroupVersionKind{Group: "admissionregistration.k8s.io", Version: "v1", Kind: "MutatingWebhookConfiguration"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := apiVersionKindToGVK(tt.apiVersion, tt.kind) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestExtractGVKsFromRendered(t *testing.T) { + tests := []struct { + name string + rendered map[string]string + expected []schema.GroupVersionKind + }{ + { + name: "single resource", + rendered: map[string]string{ + "templates/configmap.yaml": `apiVersion: v1 +kind: ConfigMap +metadata: + name: test +`, + }, + expected: []schema.GroupVersionKind{ + {Group: "", Version: "v1", Kind: "ConfigMap"}, + }, + }, + { + name: "multiple resources in one file", + rendered: map[string]string{ + "templates/resources.yaml": `apiVersion: v1 +kind: ConfigMap +metadata: + name: test +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: test +`, + }, + expected: []schema.GroupVersionKind{ + {Group: "", Version: "v1", Kind: "ConfigMap"}, + {Group: "apps", Version: "v1", Kind: "Deployment"}, + }, + }, + { + name: "multiple files", + rendered: map[string]string{ + "templates/configmap.yaml": `apiVersion: v1 +kind: ConfigMap +metadata: + name: test +`, + "templates/service.yaml": `apiVersion: v1 +kind: Service +metadata: + name: test +`, + }, + expected: []schema.GroupVersionKind{ + {Group: "", Version: "v1", Kind: "ConfigMap"}, + {Group: "", Version: "v1", Kind: "Service"}, + }, + }, + { + name: "skip empty and notes", + rendered: map[string]string{ + "templates/NOTES.txt": "Some notes", + "templates/empty.yaml": "", + "templates/configmap.yaml": `apiVersion: v1 +kind: ConfigMap +metadata: + name: test +`, + }, + expected: []schema.GroupVersionKind{ + {Group: "", Version: "v1", Kind: "ConfigMap"}, + }, + }, + { + name: "deduplicate same GVK", + rendered: map[string]string{ + "templates/cm1.yaml": `apiVersion: v1 +kind: ConfigMap +metadata: + name: test1 +`, + "templates/cm2.yaml": `apiVersion: v1 +kind: ConfigMap +metadata: + name: test2 +`, + }, + expected: []schema.GroupVersionKind{ + {Group: "", Version: "v1", Kind: "ConfigMap"}, + }, + }, + { + name: "empty documents in multi-doc", + rendered: map[string]string{ + "templates/resources.yaml": `--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: test +--- +--- +`, + }, + expected: []schema.GroupVersionKind{ + {Group: "", Version: "v1", Kind: "ConfigMap"}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gvkSet := make(map[schema.GroupVersionKind]struct{}) + err := extractGVKsFromRendered(tt.rendered, gvkSet) + require.NoError(t, err) + + // Convert set to slice for comparison + var result []schema.GroupVersionKind + for gvk := range gvkSet { + result = append(result, gvk) + } + + assert.ElementsMatch(t, tt.expected, result) + }) + } +} + +func TestGVKToGVR(t *testing.T) { + tests := []struct { + name string + gvk schema.GroupVersionKind + expected schema.GroupVersionResource + }{ + { + name: "Service", + gvk: schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Service"}, + expected: schema.GroupVersionResource{ + Group: "", + Version: "v1", + Resource: "services", + }, + }, + { + name: "ConfigMap", + gvk: schema.GroupVersionKind{Group: "", Version: "v1", Kind: "ConfigMap"}, + expected: schema.GroupVersionResource{ + Group: "", + Version: "v1", + Resource: "configmaps", + }, + }, + { + name: "Deployment", + gvk: schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"}, + expected: schema.GroupVersionResource{ + Group: "apps", + Version: "v1", + Resource: "deployments", + }, + }, + { + name: "ClusterRole", + gvk: schema.GroupVersionKind{Group: "rbac.authorization.k8s.io", Version: "v1", Kind: "ClusterRole"}, + expected: schema.GroupVersionResource{ + Group: "rbac.authorization.k8s.io", + Version: "v1", + Resource: "clusterroles", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := gvkToGVR(tt.gvk) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestIsClusterScoped(t *testing.T) { + tests := []struct { + gvk schema.GroupVersionKind + expected bool + }{ + { + gvk: schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Namespace"}, + expected: true, + }, + { + gvk: schema.GroupVersionKind{Group: "rbac.authorization.k8s.io", Version: "v1", Kind: "ClusterRole"}, + expected: true, + }, + { + gvk: schema.GroupVersionKind{Group: "rbac.authorization.k8s.io", Version: "v1", Kind: "ClusterRoleBinding"}, + expected: true, + }, + { + gvk: schema.GroupVersionKind{Group: "admissionregistration.k8s.io", Version: "v1", Kind: "MutatingWebhookConfiguration"}, + expected: true, + }, + { + gvk: schema.GroupVersionKind{Group: "admissionregistration.k8s.io", Version: "v1", Kind: "ValidatingWebhookConfiguration"}, + expected: true, + }, + { + gvk: schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Service"}, + expected: false, + }, + { + gvk: schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"}, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.gvk.Kind, func(t *testing.T) { + result := isClusterScoped(tt.gvk) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestExtractGVKsFromRendered_Errors(t *testing.T) { + tests := []struct { + name string + rendered map[string]string + }{ + { + name: "invalid yaml", + rendered: map[string]string{ + "templates/bad.yaml": `this is not: valid: yaml: at: all`, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gvkSet := make(map[schema.GroupVersionKind]struct{}) + err := extractGVKsFromRendered(tt.rendered, gvkSet) + // Invalid YAML should either error or be skipped gracefully + // The current implementation may skip documents without apiVersion/kind + // which is acceptable behavior + _ = err + }) + } +} + +// --- installer unit tests --- + +func TestResolveValues_NoVersionInEmptyFS(t *testing.T) { + inst := &installer{ + resourceFS: fstest.MapFS{}, + } + _, _, err := inst.resolveValues(Options{}) + require.Error(t, err) + assert.Contains(t, err.Error(), "no stable version found") +} + +func TestResolveValues_InvalidVersion(t *testing.T) { + inst := &installer{ + resourceFS: fstest.MapFS{ + "v1.28.3": &fstest.MapFile{Mode: fs.ModeDir}, + }, + } + _, _, err := inst.resolveValues(Options{Version: "v9.99.99"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid version") +} + +func TestResolveValues_DefaultsToHighestStableVersion(t *testing.T) { + // This will fail at ComputeValues (no charts/profiles in FS), but + // the version resolution itself should pick v1.28.3 over v1.27.0. + inst := &installer{ + resourceFS: fstest.MapFS{ + "v1.27.0": &fstest.MapFile{Mode: fs.ModeDir}, + "v1.28.3": &fstest.MapFile{Mode: fs.ModeDir}, + "v1.29.0-alpha1": &fstest.MapFile{Mode: fs.ModeDir}, + }, + } + _, _, err := inst.resolveValues(Options{}) + // ComputeValues will fail because the FS has no profile files, but + // we can verify the error message includes the resolved version. + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to compute values") +} + +func TestReconcile_ResolveValuesFailure(t *testing.T) { + inst := &installer{ + resourceFS: fstest.MapFS{}, // no versions + } + status := inst.reconcile(context.Background(), Options{}) + assert.False(t, status.Installed) + assert.Empty(t, status.Version) + assert.Error(t, status.Error) + assert.Contains(t, status.Error.Error(), "no stable version found") + // CRD fields should be zero-valued (never reached) + assert.Empty(t, status.CRDState) + assert.Nil(t, status.CRDs) +} + +func TestReconcile_InvalidVersionEarlyExit(t *testing.T) { + inst := &installer{ + resourceFS: fstest.MapFS{ + "v1.28.3": &fstest.MapFile{Mode: fs.ModeDir}, + }, + } + status := inst.reconcile(context.Background(), Options{Version: "v0.0.1"}) + assert.False(t, status.Installed) + assert.Error(t, status.Error) + assert.Contains(t, status.Error.Error(), "invalid version") +} + +func TestReconcile_DeepCopiesValues(t *testing.T) { + // Verify that reconcile deep-copies Values so mutations inside + // reconcile don't affect the caller's pointer. + inst := &installer{ + resourceFS: fstest.MapFS{}, // will fail early, but after deep-copy + } + original := &v1.Values{ + Global: &v1.GlobalConfig{ + Hub: ptr.To("original-hub"), + }, + } + opts := Options{Values: original} + _ = inst.reconcile(context.Background(), opts) + + // The original Values should not have been mutated + assert.Equal(t, "original-hub", *original.Global.Hub) +} diff --git a/pkg/install/library.go b/pkg/install/library.go new file mode 100644 index 0000000000..8230f9cb96 --- /dev/null +++ b/pkg/install/library.go @@ -0,0 +1,273 @@ +// 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 install provides a library for managing istiod installations. +// It is designed for embedding in other operators (like OpenShift Ingress) +// that need to install and maintain Istio without running a separate operator. +// +// The Library runs as an independent actor: the consumer sends desired state +// via Apply(), and reads the result via Status(). A notification channel +// returned by Start() signals when the Library has reconciled. +// +// Usage: +// +// lib, _ := install.New(kubeConfig, resourceFS) +// notifyCh := lib.Start(ctx) +// +// // In controller reconcile: +// lib.Apply(install.Options{Values: values, Namespace: "openshift-ingress"}) +// status := lib.Status() +// // update GatewayClass conditions from status +// +// // In a goroutine or source.Channel watch: +// for range notifyCh { +// status := lib.Status() +// // ... +// } +package install + +import ( + "fmt" + "io/fs" + "os" + "sync" + + v1 "github.com/istio-ecosystem/sail-operator/api/v1" + "github.com/istio-ecosystem/sail-operator/bundle" + "github.com/istio-ecosystem/sail-operator/pkg/helm" + "k8s.io/client-go/dynamic" + "k8s.io/client-go/rest" + "k8s.io/client-go/util/workqueue" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +const ( + defaultNamespace = "istio-system" + defaultProfile = "openshift" + defaultHelmDriver = "secret" + defaultRevision = v1.DefaultRevision + defaultManagedByValue = "sail-library" +) + +// Status represents the result of a reconciliation, covering both +// CRD management and Helm installation. +type Status struct { + // CRDState is the aggregate ownership state of the target Istio CRDs. + CRDState CRDManagementState + + // CRDMessage is a human-readable description of the CRD state. + CRDMessage string + + // CRDs contains per-CRD detail (name, ownership, found on cluster). + CRDs []CRDInfo + + // Installed is true if the Helm install/upgrade completed successfully. + Installed bool + + // Version is the resolved Istio version (set even if Installed is false). + Version string + + // Error is non-nil if something went wrong during CRD management or Helm installation. + // CRD ownership problems (UnknownManagement, MixedOwnership) set this but do not + // prevent Helm installation from being attempted. + Error error +} + +// String returns a human-readable summary of the status. +func (s Status) String() string { + state := "not installed" + if s.Installed { + state = "installed" + } + + ver := s.Version + if ver == "" { + ver = "unknown" + } + + msg := fmt.Sprintf("%s version=%s crds=%s", state, ver, s.CRDState) + if s.CRDMessage != "" { + msg += fmt.Sprintf(" (%s)", s.CRDMessage) + } + if len(s.CRDs) > 0 { + msg += " [" + for i, crd := range s.CRDs { + if i > 0 { + msg += ", " + } + if crd.Found { + msg += fmt.Sprintf("%s:%s", crd.Name, crd.State) + } else { + msg += fmt.Sprintf("%s:missing", crd.Name) + } + } + msg += "]" + } + if s.Error != nil { + msg += fmt.Sprintf(" error=%v", s.Error) + } + return msg +} + +// Options for installing istiod. +type Options struct { + // Namespace is the target namespace for installation. + // Defaults to "istio-system" if not specified. + Namespace string + + // Version is the Istio version to install. + // Defaults to the latest supported version if not specified. + Version string + + // Revision is the Istio revision name. + // Defaults to "default" if not specified. + Revision string + + // Values are Helm value overrides. + // Use GatewayAPIDefaults() to get pre-configured values for Gateway API mode, + // then modify as needed before passing here. + Values *v1.Values + + // ManageCRDs controls whether the Library manages Istio CRDs. + // When true (default), CRDs are classified by ownership and installed/updated + // if we own them or none exist. + // Set to false to skip CRD management entirely. + ManageCRDs *bool + + // IncludeAllCRDs controls which CRDs are managed. + // When true, all *.istio.io CRDs from the embedded FS are managed. + // When false (default), only CRDs matching PILOT_INCLUDE_RESOURCES are managed. + IncludeAllCRDs *bool + + // OverwriteOLMManagedCRD is called when a CRD is detected with OLM ownership labels. + // If provided and returns true, the CRD is overwritten with CIO labels and adopted. + // If nil or returns false, OLM-labeled CRDs are left alone. + OverwriteOLMManagedCRD OverwriteOLMManagedCRDFunc +} + +// applyDefaults fills in default values for Options. +// Version is not defaulted here — it requires access to the resource FS, +// so it is resolved during reconciliation via DefaultVersion(). +func (o *Options) applyDefaults() { + o.Version = NormalizeVersion(o.Version) + if o.Namespace == "" { + o.Namespace = defaultNamespace + } + if o.Revision == "" { + o.Revision = defaultRevision + } + if o.ManageCRDs == nil { + o.ManageCRDs = ptr.To(true) + } + if o.IncludeAllCRDs == nil { + o.IncludeAllCRDs = ptr.To(false) + } +} + +// Library manages the lifecycle of an istiod installation. It runs as an +// independent actor: the consumer sends desired state via Apply() and reads +// the result via Status(). Start() returns a notification channel that +// signals when the Library has reconciled (drift repair, CRD change, or +// a new Apply). +type Library struct { + // Core install/uninstall logic (no concurrency concerns) + inst *installer + + // managedByValue is the value of the "managed-by" label set on all + // Helm-managed resources. Used both by the post-renderer (write) and + // by informer predicates (read) for ownership filtering. + managedByValue string + + // Infrastructure needed only by Library (informers, dynamic client) + kubeConfig *rest.Config + dynamicCl dynamic.Interface + + // Lifecycle serialization (Apply and Uninstall hold this) + lifecycleMu sync.Mutex + + // Desired state (set by Apply, read by worker) + mu sync.RWMutex + desiredOpts *Options // nil until first Apply(); nil again after Uninstall() + + // Informer lifecycle (per install cycle) + informerStop chan struct{} // closed to stop current informer cycle + processingDone chan struct{} // closed when processWorkQueue exits + + // Latest status (written by worker, read by Status()) + statusMu sync.RWMutex + status Status + + // Signal channel: Apply() sends on it to wake waitForDesiredState(). + // Buffered 1, non-blocking write — if the signal is already pending, + // the new one is dropped. + applySignal chan struct{} + + // Internal workqueue + workqueue workqueue.TypedRateLimitingInterface[string] +} + +// New creates a new Library. +// +// Parameters: +// - kubeConfig: Kubernetes client configuration (required) +// - resourceFS: Filesystem containing Helm charts and profiles (required). +// Use resources.FS for embedded resources or FromDirectory() for a filesystem path. +func New(kubeConfig *rest.Config, resourceFS fs.FS) (*Library, error) { + if kubeConfig == nil { + return nil, fmt.Errorf("kubeConfig is required") + } + if resourceFS == nil { + return nil, fmt.Errorf("resourceFS is required") + } + + cl, err := client.New(kubeConfig, client.Options{}) + if err != nil { + return nil, fmt.Errorf("failed to create client: %w", err) + } + + dynamicCl, err := dynamic.NewForConfig(kubeConfig) + if err != nil { + return nil, fmt.Errorf("failed to create dynamic client: %w", err) + } + + // Populate default image refs from the embedded CSV (no-op if already set by config.Read) + if err := LoadImageDigestsFromCSV(bundle.CSV); err != nil { + return nil, fmt.Errorf("failed to load image digests from CSV: %w", err) + } + + return &Library{ + inst: &installer{ + resourceFS: resourceFS, + chartManager: helm.NewChartManager(kubeConfig, defaultHelmDriver, helm.WithManagedByValue(defaultManagedByValue)), + cl: cl, + crdManager: newCRDManager(cl), + }, + managedByValue: defaultManagedByValue, + kubeConfig: kubeConfig, + dynamicCl: dynamicCl, + applySignal: make(chan struct{}, 1), + }, nil +} + +// FromDirectory creates an fs.FS from a filesystem directory path. +// This is a convenience function for consumers who want to load resources +// from the filesystem instead of using embedded resources. +// +// Example: +// +// lib, _ := install.New(kubeConfig, install.FromDirectory("/var/lib/sail-operator/resources")) +func FromDirectory(path string) fs.FS { + return os.DirFS(path) +} diff --git a/pkg/install/lifecycle.go b/pkg/install/lifecycle.go new file mode 100644 index 0000000000..9990997705 --- /dev/null +++ b/pkg/install/lifecycle.go @@ -0,0 +1,323 @@ +// 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 install + +import ( + "context" + "reflect" + + "github.com/istio-ecosystem/sail-operator/pkg/helm" + "k8s.io/client-go/util/workqueue" + "k8s.io/utils/ptr" + ctrllog "sigs.k8s.io/controller-runtime/pkg/log" +) + +const ( + // reconcileKey is the single key used in the workqueue. + // Since we always reconcile the entire installation, we use a single key + // to coalesce multiple events into one reconciliation. + reconcileKey = "reconcile" +) + +// Start begins the Library's internal reconciliation loop. It returns a +// notification channel that receives a signal every time the Library +// finishes a reconciliation (whether triggered by Apply, drift detection, +// or CRD changes). +// +// The channel is owned by the Library and closed when ctx is cancelled. +// Buffer of 1 with non-blocking write: if the consumer hasn't drained +// the previous notification, the new one is dropped (latest-wins). +// +// Start is non-blocking — it launches goroutines internally and returns +// immediately. The Library sits idle until the first Apply() call. +func (l *Library) Start(ctx context.Context) <-chan struct{} { + notifyCh := make(chan struct{}, 1) + l.workqueue = workqueue.NewTypedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[string]()) + + // Start worker goroutine + go l.run(ctx, notifyCh) + + return notifyCh +} + +// Apply sets the desired installation state. If the options differ from +// the previously applied options, a reconciliation is enqueued. If they +// are the same, this is a no-op. +// +// Apply blocks if Uninstall is in progress (they share a lifecycle lock). +// The result of the reconciliation will be available via Status() after +// the notification channel signals. +func (l *Library) Apply(opts Options) { + l.lifecycleMu.Lock() + defer l.lifecycleMu.Unlock() + + opts.applyDefaults() + + l.mu.Lock() + defer l.mu.Unlock() + + if l.desiredOpts != nil && optionsEqual(*l.desiredOpts, opts) { + return // no change + } + + copied := opts + if copied.Values != nil { + copied.Values = copied.Values.DeepCopy() + } + l.desiredOpts = &copied + // Bypass the rate limiter so explicit user intent is never delayed + // by backoff from a previous failure. + if l.workqueue != nil { + l.workqueue.Add(reconcileKey) + } + + // Wake waitForDesiredState if it's blocking + select { + case l.applySignal <- struct{}{}: + default: + } +} + +// Enqueue forces a reconciliation without changing the desired options. +// Use this when cluster state that affects reconciliation has changed +// externally but the Options passed to Apply remain the same. For example, +// if an OLM Subscription managing Istio CRDs is deleted, the CRD ownership +// labels haven't changed yet, but the consumer knows a takeover is now +// possible. Calling Enqueue triggers the Library to re-classify CRDs and +// re-run the Helm install using the previously applied options. +// +// Enqueue is a no-op if Apply has not been called yet (no desired state). +// It is safe to call from any goroutine. +func (l *Library) Enqueue() { + l.enqueue() +} + +// Status returns the latest reconciliation result. This is safe to call +// from any goroutine. Returns a zero-value Status if no reconciliation +// has completed yet. +func (l *Library) Status() Status { + l.statusMu.RLock() + defer l.statusMu.RUnlock() + return l.status +} + +// Uninstall stops drift detection and removes the istiod installation. +// It first stops informers and waits for the processing loop to exit, +// then performs the Helm uninstall. This prevents the drift-repair loop +// from fighting the uninstall. +// +// Uninstall is a synchronous, blocking operation. It holds the lifecycle +// lock, so Apply() will block until Uninstall completes. +// +// The caller provides the namespace and revision to uninstall. Empty strings +// default to "istio-system" and "default" respectively. This allows Uninstall +// to work even after a crash, when no prior Apply() state exists in memory. +// +// If an active reconcile loop is running, it is stopped before the Helm +// uninstall proceeds. After Uninstall, calling Apply() will start a new +// install cycle. +func (l *Library) Uninstall(ctx context.Context, namespace, revision string) error { + l.lifecycleMu.Lock() + defer l.lifecycleMu.Unlock() + + log := ctrllog.Log.WithName("install") + + if namespace == "" { + namespace = defaultNamespace + } + if revision == "" { + revision = defaultRevision + } + + // Clear desired state and capture loop handles so we can stop + // the processing loop if one is active. + l.mu.Lock() + l.desiredOpts = nil + informerStop := l.informerStop + processingDone := l.processingDone + l.informerStop = nil + l.processingDone = nil + l.mu.Unlock() + + log.Info("Uninstalling", "namespace", namespace, "revision", revision) + + // Stop informers so they don't fire events during teardown + if informerStop != nil { + close(informerStop) + } + + // Enqueue a sentinel so processWorkQueue unblocks from Get() + // and checks the nil desiredOpts condition + l.enqueue() + + // Wait for the processing loop to exit + if processingDone != nil { + <-processingDone + } + + // Now safe to Helm uninstall — nothing is watching or reconciling + if err := l.inst.uninstall(ctx, namespace, revision); err != nil { + return err + } + l.setStatus(Status{}) + + log.Info("Uninstall complete", "namespace", namespace, "revision", revision) + return nil +} + +// run is the main loop. It alternates between idle (waiting for Apply) and +// active (informers running, processing workqueue). Uninstall() nils +// desiredOpts, which causes processWorkQueue to exit and the loop to +// return to idle. The loop exits only when ctx is cancelled. +func (l *Library) run(ctx context.Context, notifyCh chan<- struct{}) { + log := ctrllog.Log.WithName("install") + defer close(notifyCh) + defer l.workqueue.ShutDown() + + for { + // Idle: block until Apply() sets desiredOpts (or ctx cancelled) + if !l.waitForDesiredState(ctx) { + return // ctx cancelled + } + + // Active: set up informers for this install cycle. + // Capture both channels as locals while the lock is held so + // Uninstall can't nil the struct fields before we use them. + l.mu.Lock() + l.informerStop = make(chan struct{}) + l.processingDone = make(chan struct{}) + informerStop := l.informerStop + processingDone := l.processingDone + l.mu.Unlock() + + l.setupInformers(informerStop) + + log.Info("Processing workqueue") + l.processWorkQueue(ctx, notifyCh, processingDone) + log.Info("Workqueue processing stopped, returning to idle") + + // Loop back to idle, waiting for next Apply() + } +} + +// processWorkQueue processes work items until desiredOpts goes nil (Uninstall) +// or the workqueue shuts down (ctx cancelled). It closes done on exit so +// Uninstall() can wait for processing to stop before doing Helm cleanup. +// The done channel is passed by the caller (run) which captures it under +// the lock, so Uninstall niling the struct field doesn't affect us. +func (l *Library) processWorkQueue(ctx context.Context, notifyCh chan<- struct{}, done chan struct{}) { + log := ctrllog.Log.WithName("install") + defer func() { + if done != nil { + close(done) + } + }() + + for { + key, shutdown := l.workqueue.Get() + if shutdown { + return + } + + // Read desiredOpts once under a single lock to avoid a race + // where Uninstall() nils it between a nil-check and dereference. + l.mu.RLock() + optsPtr := l.desiredOpts + l.mu.RUnlock() + + if optsPtr == nil { + l.workqueue.Done(key) + return // back to idle + } + opts := *optsPtr + + log.Info("Reconciling") + status := l.inst.reconcile(ctx, opts) + l.setStatus(status) + log.Info("Reconcile complete", "installed", status.Installed, "error", status.Error) + + // Non-blocking notify + select { + case notifyCh <- struct{}{}: + default: + } + + // On success, reset the rate limiter so the next drift event is + // processed immediately. On failure, leave the backoff counter + // intact so informer-driven re-enqueues get exponential delay. + if status.Error == nil { + l.workqueue.Forget(key) + } + l.workqueue.Done(key) + } +} + +// waitForDesiredState blocks until Apply() has been called or ctx is cancelled. +func (l *Library) waitForDesiredState(ctx context.Context) bool { + // Fast path: check if opts are already set (e.g. Uninstall->Apply cycle) + l.mu.RLock() + hasOpts := l.desiredOpts != nil + l.mu.RUnlock() + if hasOpts { + return true + } + + for { + select { + case <-ctx.Done(): + return false + case <-l.applySignal: + l.mu.RLock() + hasOpts := l.desiredOpts != nil + l.mu.RUnlock() + if hasOpts { + return true + } + } + } +} + +// setStatus atomically updates the stored status. +func (l *Library) setStatus(s Status) { + l.statusMu.Lock() + defer l.statusMu.Unlock() + l.status = s +} + +// enqueue adds a rate-limited reconciliation request to the workqueue. +// The rate limiter provides exponential backoff when reconciliations fail, +// preventing tight loops from informer events during Helm rollbacks. +func (l *Library) enqueue() { + if l.workqueue != nil { + l.workqueue.AddRateLimited(reconcileKey) + } +} + +// optionsEqual compares two Options for equality. +// Used by Apply() to skip no-op updates. +func optionsEqual(a, b Options) bool { + if a.Namespace != b.Namespace || + a.Version != b.Version || + a.Revision != b.Revision || + ptr.Deref(a.ManageCRDs, true) != ptr.Deref(b.ManageCRDs, true) || + ptr.Deref(a.IncludeAllCRDs, false) != ptr.Deref(b.IncludeAllCRDs, false) { + return false + } + + // Compare Values via map conversion for deep equality + aMap := helm.FromValues(a.Values) + bMap := helm.FromValues(b.Values) + return reflect.DeepEqual(aMap, bMap) +} diff --git a/pkg/install/lifecycle_test.go b/pkg/install/lifecycle_test.go new file mode 100644 index 0000000000..fcf1793a9c --- /dev/null +++ b/pkg/install/lifecycle_test.go @@ -0,0 +1,960 @@ +// 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 install + +import ( + "context" + "fmt" + "sync/atomic" + "testing" + "testing/fstest" + "time" + + v1 "github.com/istio-ecosystem/sail-operator/api/v1" + "github.com/istio-ecosystem/sail-operator/pkg/helm" + "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/client-go/rest" + "k8s.io/client-go/util/workqueue" + "k8s.io/utils/ptr" +) + +func TestNew(t *testing.T) { + testFS := fstest.MapFS{} + testConfig := &rest.Config{} + + t.Run("missing kubeConfig", func(t *testing.T) { + lib, err := New(nil, testFS) + assert.Error(t, err) + assert.Contains(t, err.Error(), "kubeConfig is required") + assert.Nil(t, lib) + }) + + t.Run("missing resourceFS", func(t *testing.T) { + lib, err := New(testConfig, nil) + assert.Error(t, err) + assert.Contains(t, err.Error(), "resourceFS is required") + assert.Nil(t, lib) + }) + + t.Run("valid inputs", func(t *testing.T) { + lib, err := New(testConfig, testFS) + assert.NoError(t, err) + assert.NotNil(t, lib) + }) +} + +func TestOptionsApplyDefaults(t *testing.T) { + tests := []struct { + name string + opts Options + expectedNamespace string + expectedVersion string + expectedRevision string + expectedManageCRDs bool + expectedIncludeAllCRDs bool + }{ + { + name: "all defaults", + opts: Options{}, + expectedNamespace: "istio-system", + expectedVersion: "", + expectedRevision: "default", + expectedManageCRDs: true, + expectedIncludeAllCRDs: false, + }, + { + name: "custom namespace preserved", + opts: Options{ + Namespace: "custom-ns", + }, + expectedNamespace: "custom-ns", + expectedVersion: "", + expectedRevision: "default", + expectedManageCRDs: true, + expectedIncludeAllCRDs: false, + }, + { + name: "custom version preserved", + opts: Options{ + Version: "v1.24.0", + }, + expectedNamespace: "istio-system", + expectedVersion: "v1.24.0", + expectedRevision: "default", + expectedManageCRDs: true, + expectedIncludeAllCRDs: false, + }, + { + name: "custom revision preserved", + opts: Options{ + Revision: "canary", + }, + expectedNamespace: "istio-system", + expectedVersion: "", + expectedRevision: "canary", + expectedManageCRDs: true, + expectedIncludeAllCRDs: false, + }, + { + name: "all custom values preserved", + opts: Options{ + Namespace: "my-namespace", + Version: "v1.23.0", + Revision: "my-revision", + }, + expectedNamespace: "my-namespace", + expectedVersion: "v1.23.0", + expectedRevision: "my-revision", + expectedManageCRDs: true, + expectedIncludeAllCRDs: false, + }, + { + name: "ManageCRDs false preserved", + opts: Options{ + ManageCRDs: ptr.To(false), + }, + expectedNamespace: "istio-system", + expectedVersion: "", + expectedRevision: "default", + expectedManageCRDs: false, + expectedIncludeAllCRDs: false, + }, + { + name: "IncludeAllCRDs true preserved", + opts: Options{ + IncludeAllCRDs: ptr.To(true), + }, + expectedNamespace: "istio-system", + expectedVersion: "", + expectedRevision: "default", + expectedManageCRDs: true, + expectedIncludeAllCRDs: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tt.opts.applyDefaults() + assert.Equal(t, tt.expectedNamespace, tt.opts.Namespace) + assert.Equal(t, tt.expectedVersion, tt.opts.Version) + assert.Equal(t, tt.expectedRevision, tt.opts.Revision) + assert.Equal(t, tt.expectedManageCRDs, *tt.opts.ManageCRDs) + assert.Equal(t, tt.expectedIncludeAllCRDs, *tt.opts.IncludeAllCRDs) + }) + } +} + +func TestOptionsEqual(t *testing.T) { + base := Options{ + Namespace: "ns", + Version: "1.24.0", + Revision: "default", + ManageCRDs: ptr.To(true), + IncludeAllCRDs: ptr.To(false), + Values: &v1.Values{ + Global: &v1.GlobalConfig{ + Hub: ptr.To("docker.io/istio"), + }, + }, + } + + t.Run("identical options", func(t *testing.T) { + other := Options{ + Namespace: "ns", + Version: "1.24.0", + Revision: "default", + ManageCRDs: ptr.To(true), + IncludeAllCRDs: ptr.To(false), + Values: &v1.Values{ + Global: &v1.GlobalConfig{ + Hub: ptr.To("docker.io/istio"), + }, + }, + } + assert.True(t, optionsEqual(base, other)) + }) + + t.Run("different namespace", func(t *testing.T) { + other := base + other.Namespace = "different" + assert.False(t, optionsEqual(base, other)) + }) + + t.Run("different values", func(t *testing.T) { + other := Options{ + Namespace: "ns", + Version: "1.24.0", + Revision: "default", + ManageCRDs: ptr.To(true), + IncludeAllCRDs: ptr.To(false), + Values: &v1.Values{ + Global: &v1.GlobalConfig{ + Hub: ptr.To("quay.io/other"), + }, + }, + } + assert.False(t, optionsEqual(base, other)) + }) + + t.Run("nil values equal", func(t *testing.T) { + a := Options{Namespace: "ns", Version: "1.24.0", Revision: "default", ManageCRDs: ptr.To(true), IncludeAllCRDs: ptr.To(false)} + b := Options{Namespace: "ns", Version: "1.24.0", Revision: "default", ManageCRDs: ptr.To(true), IncludeAllCRDs: ptr.To(false)} + assert.True(t, optionsEqual(a, b)) + }) +} + +func TestApplyIdempotency(t *testing.T) { + lib := &Library{ + workqueue: workqueue.NewTypedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[string]()), + } + defer lib.workqueue.ShutDown() + + opts := Options{ + Namespace: "test-ns", + Version: "1.24.0", + } + + // First Apply should store and enqueue + lib.Apply(opts) + assert.NotNil(t, lib.desiredOpts) + + // Drain the queue + key, _ := lib.workqueue.Get() + lib.workqueue.Done(key) + + // Second Apply with same opts should be a no-op + lib.Apply(opts) + // Queue should be empty (len check via shutdown trick not possible, so just verify desiredOpts unchanged) + assert.Equal(t, opts.Namespace, lib.desiredOpts.Namespace) +} + +func TestEnqueueBeforeApply(t *testing.T) { + lib := &Library{} + + // Enqueue before Start (no workqueue) should not panic + lib.Enqueue() + + // Enqueue after Start but before Apply should enqueue but not panic + lib.workqueue = workqueue.NewTypedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[string]()) + defer lib.workqueue.ShutDown() + + lib.Enqueue() +} + +func TestEnqueueBypassesOptionsEqual(t *testing.T) { + lib := &Library{ + workqueue: workqueue.NewTypedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[string]()), + } + defer lib.workqueue.ShutDown() + + opts := Options{ + Namespace: "test-ns", + Version: "1.24.0", + } + + // Apply and drain + lib.Apply(opts) + key, _ := lib.workqueue.Get() + lib.workqueue.Done(key) + + // Apply with same options is a no-op (idempotent) + lib.Apply(opts) + + // But Enqueue bypasses the equal check and adds a work item + lib.Enqueue() + + // Should be able to Get an item + done := make(chan struct{}) + go func() { + k, _ := lib.workqueue.Get() + lib.workqueue.Done(k) + close(done) + }() + + select { + case <-done: + // got the item — Enqueue worked + case <-time.After(time.Second): + t.Fatal("Enqueue did not add a work item to the queue") + } +} + +func TestStatusString(t *testing.T) { + tests := []struct { + name string + status Status + expected string + }{ + { + name: "zero value", + status: Status{}, + expected: "not installed version=unknown crds=", + }, + { + name: "installed ok with CRD details", + status: Status{ + Installed: true, + Version: "1.24.0", + CRDState: CRDManagedByCIO, + CRDMessage: "CRDs installed by CIO", + CRDs: []CRDInfo{ + {Name: "wasmplugins.extensions.istio.io", Found: true, State: CRDManagedByCIO}, + {Name: "envoyfilters.networking.istio.io", Found: true, State: CRDManagedByCIO}, + }, + }, + expected: "installed version=1.24.0 crds=ManagedByCIO (CRDs installed by CIO) " + + "[wasmplugins.extensions.istio.io:ManagedByCIO, envoyfilters.networking.istio.io:ManagedByCIO]", + }, + { + name: "mixed ownership with missing CRDs", + status: Status{ + Version: "1.24.0", + CRDState: CRDMixedOwnership, + CRDMessage: "CRDs have mixed ownership", + CRDs: []CRDInfo{ + {Name: "wasmplugins.extensions.istio.io", Found: true, State: CRDManagedByOLM}, + {Name: "envoyfilters.networking.istio.io", Found: false}, + }, + Error: fmt.Errorf("Istio CRDs have mixed ownership (CIO/OLM/other)"), + }, + expected: "not installed version=1.24.0 crds=MixedOwnership (CRDs have mixed ownership) " + + "[wasmplugins.extensions.istio.io:ManagedByOLM, envoyfilters.networking.istio.io:missing] error=Istio CRDs have mixed ownership (CIO/OLM/other)", + }, + { + name: "installed no CRD details", + status: Status{ + Installed: true, + Version: "1.24.0", + CRDState: CRDManagedByOLM, + CRDMessage: "CRDs managed by OSSM subscription via OLM", + }, + expected: "installed version=1.24.0 crds=ManagedByOLM (CRDs managed by OSSM subscription via OLM)", + }, + { + name: "error without CRDs", + status: Status{ + Version: "1.24.0", + Error: fmt.Errorf("validation failed"), + }, + expected: "not installed version=1.24.0 crds= error=validation failed", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, tt.status.String()) + }) + } +} + +func TestStatusReadWrite(t *testing.T) { + lib := &Library{} + + // Initial status is zero value + status := lib.Status() + assert.False(t, status.Installed) + assert.Empty(t, status.Version) + + // Set status + lib.setStatus(Status{ + Installed: true, + Version: "1.24.0", + CRDState: CRDManagedByCIO, + }) + + status = lib.Status() + assert.True(t, status.Installed) + assert.Equal(t, "1.24.0", status.Version) + assert.Equal(t, CRDManagedByCIO, status.CRDState) +} + +func TestIsOwnedResource(t *testing.T) { + const testManagedByValue = "test-operator" + + tests := []struct { + name string + labels map[string]string + revision string + expected bool + }{ + { + name: "no labels", + labels: nil, + revision: "default", + expected: false, + }, + { + name: "istio rev label matches default", + labels: map[string]string{"istio.io/rev": "default"}, + revision: "default", + expected: true, + }, + { + name: "istio rev label matches custom", + labels: map[string]string{"istio.io/rev": "canary"}, + revision: "canary", + expected: true, + }, + { + name: "istio rev label does not match", + labels: map[string]string{"istio.io/rev": "other"}, + revision: "default", + expected: false, + }, + { + name: "operator component label", + labels: map[string]string{"operator.istio.io/component": "pilot"}, + revision: "default", + expected: true, + }, + { + name: "managed-by label matches configured value", + labels: map[string]string{"managed-by": testManagedByValue}, + revision: "default", + expected: true, + }, + { + name: "managed-by label does not match configured value", + labels: map[string]string{"managed-by": "something-else"}, + revision: "default", + expected: false, + }, + { + name: "app.kubernetes.io/managed-by Helm fallback", + labels: map[string]string{"app.kubernetes.io/managed-by": "Helm"}, + revision: "default", + expected: true, + }, + { + name: "app.kubernetes.io/managed-by non-Helm rejected", + labels: map[string]string{"app.kubernetes.io/managed-by": "other"}, + revision: "default", + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + obj := &unstructured.Unstructured{} + obj.SetLabels(tt.labels) + + result := isOwnedResource(obj, tt.revision, testManagedByValue) + assert.Equal(t, tt.expected, result) + }) + } +} + +// TestOptionsEqualWithNilValues verifies that optionsEqual handles nil Values by +// comparing the map representation (both nil Values produce equal empty maps). +func TestOptionsEqualWithNilValues(t *testing.T) { + a := Options{Namespace: "ns", ManageCRDs: ptr.To(true), IncludeAllCRDs: ptr.To(false)} + b := Options{Namespace: "ns", ManageCRDs: ptr.To(true), IncludeAllCRDs: ptr.To(false)} + a.applyDefaults() + b.applyDefaults() + + // Both nil Values should be equal + assert.True(t, optionsEqual(a, b)) + + // nil vs non-nil Values should differ (non-nil with content) + b.Values = &v1.Values{Global: &v1.GlobalConfig{Hub: ptr.To("test")}} + assert.False(t, optionsEqual(a, b)) +} + +// TestFromValuesRoundTrip verifies that helm.FromValues produces comparable maps. +func TestFromValuesRoundTrip(t *testing.T) { + v := &v1.Values{ + Global: &v1.GlobalConfig{ + Hub: ptr.To("docker.io/istio"), + }, + } + m1 := helm.FromValues(v) + m2 := helm.FromValues(v) + assert.Equal(t, m1, m2) +} + +// buildCIOOptions replicates how the Cluster Ingress Operator builds Options +// via buildInstallerOptions + openshiftValues + GatewayAPIDefaults + MergeValues. +// See: https://github.com/rikatz/cluster-ingress-operator/blob/31d7e74fe6/pkg/operator/controller/gatewayclass/istio_sail_installer.go +func buildCIOOptions() Options { + // Step 1: GatewayAPIDefaults (from the sail library) + values := GatewayAPIDefaults() + + // Step 2: openshiftValues overlay (from CIO) + pilotEnv := map[string]string{ + "PILOT_ENABLE_GATEWAY_API": "true", + "PILOT_ENABLE_ALPHA_GATEWAY_API": "false", + "PILOT_ENABLE_GATEWAY_API_STATUS": "true", + "PILOT_ENABLE_GATEWAY_API_DEPLOYMENT_CONTROLLER": "true", + "PILOT_ENABLE_GATEWAY_API_GATEWAYCLASS_CONTROLLER": "false", + "PILOT_GATEWAY_API_DEFAULT_GATEWAYCLASS_NAME": "openshift-default", + "PILOT_GATEWAY_API_CONTROLLER_NAME": "openshift.io/gateway-controller", + "PILOT_MULTI_NETWORK_DISCOVER_GATEWAY_API": "false", + "ENABLE_GATEWAY_API_MANUAL_DEPLOYMENT": "false", + "PILOT_ENABLE_GATEWAY_API_CA_CERT_ONLY": "true", + "PILOT_ENABLE_GATEWAY_API_COPY_LABELS_ANNOTATIONS": "false", + } + openshiftOverrides := &v1.Values{ + Global: &v1.GlobalConfig{ + DefaultPodDisruptionBudget: &v1.DefaultPodDisruptionBudgetConfig{ + Enabled: ptr.To(false), + }, + IstioNamespace: ptr.To("openshift-ingress"), + PriorityClassName: ptr.To("system-cluster-critical"), + TrustBundleName: ptr.To("openshift-gateway-ca-root-cert"), + }, + + Pilot: &v1.PilotConfig{ + Env: pilotEnv, + PodAnnotations: map[string]string{ + "target.workload.openshift.io/management": `{"effect": "PreferredDuringScheduling"}`, + }, + }, + } + + // Step 3: MergeValues + values = MergeValues(values, openshiftOverrides) + + // Step 4: Build Options (same as CIO's buildInstallerOptions) + return Options{ + Namespace: "openshift-ingress", + Revision: "openshift-gateway", + Values: values, + Version: "v1.27.3", + ManageCRDs: ptr.To(true), + IncludeAllCRDs: ptr.To(true), + } +} + +// TestOptionsEqualWithCIOPattern verifies that two independently-built CIO +// option sets compare as equal after applyDefaults. +func TestOptionsEqualWithCIOPattern(t *testing.T) { + opts1 := buildCIOOptions() + opts2 := buildCIOOptions() + opts1.applyDefaults() + opts2.applyDefaults() + + assert.True(t, optionsEqual(opts1, opts2), + "options built the same way should be equal;\n map1: %v\n map2: %v", + helm.FromValues(opts1.Values), helm.FromValues(opts2.Values)) +} + +// TestCIOReconcileLoopConverges simulates the full deployment flow: +// +// Library workqueue ──Get──▶ reconcile ──notify──▶ controller ──Apply──▶ Library workqueue +// +// The test replicates the real ordering in run(): +// 1. Get() item from workqueue +// 2. reconcile (no-op here — no cluster) +// 3. Send notification BEFORE Forget+Done (matches production code) +// 4. Controller receives notification, builds fresh Options, calls Apply() +// 5. Forget + Done +// +// If Apply() correctly detects identical options and skips enqueue, the loop +// converges after exactly 1 reconcile. If it re-enqueues, the test fails. +func TestCIOReconcileLoopConverges(t *testing.T) { + lib := &Library{ + workqueue: workqueue.NewTypedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[string]()), + } + defer lib.workqueue.ShutDown() + + notifyCh := make(chan struct{}, 1) + + // Track Apply calls from the simulated controller. + var applyCount atomic.Int32 + // appliedCh signals that the controller finished calling Apply. + appliedCh := make(chan struct{}, 1) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + // Simulate the gatewayclass controller: + // on each notification, build fresh options and call Apply(). + go func() { + for { + select { + case <-ctx.Done(): + return + case _, ok := <-notifyCh: + if !ok { + return + } + applyCount.Add(1) + lib.Apply(buildCIOOptions()) + select { + case appliedCh <- struct{}{}: + default: + } + } + } + }() + + // Initial Apply (CIO's first reconcile triggered by GatewayClass creation) + lib.Apply(buildCIOOptions()) + + // Simulate the library's run() loop. + reconcileCount := 0 + for { + type getResult struct { + key string + shutdown bool + } + ch := make(chan getResult, 1) + go func() { + key, shutdown := lib.workqueue.Get() + ch <- getResult{key, shutdown} + }() + + select { + case r := <-ch: + if r.shutdown { + t.Fatal("unexpected queue shutdown") + } + reconcileCount++ + if reconcileCount > 10 { + t.Fatalf("reconcile loop did not converge after 10 iterations (apply count: %d)", + applyCount.Load()) + } + t.Logf("reconcile #%d", reconcileCount) + + // --- replicate run() ordering: notify BEFORE Forget+Done --- + select { + case notifyCh <- struct{}{}: + default: + } + + // Wait for the controller to process the notification and call Apply() + select { + case <-appliedCh: + t.Logf(" controller applied (total applies: %d)", applyCount.Load()) + case <-time.After(200 * time.Millisecond): + t.Log(" controller did not apply (notification may have been dropped)") + } + + lib.workqueue.Forget(r.key) + lib.workqueue.Done(r.key) + + case <-time.After(500 * time.Millisecond): + // Queue has been empty for 500ms — converged. + t.Logf("converged: %d reconcile(s), %d controller apply(s)", + reconcileCount, applyCount.Load()) + assert.Equal(t, 1, reconcileCount, + "expected exactly 1 reconcile; more means Apply() is re-enqueuing "+ + "when it should detect equal options") + return + } + } +} + +// TestUninstallWithoutApply verifies that Uninstall on an idle library (no +// prior Apply) still attempts the Helm uninstall using the provided args. +func TestUninstallWithoutApply(t *testing.T) { + lib := &Library{ + workqueue: workqueue.NewTypedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[string]()), + inst: &installer{ + chartManager: helm.NewChartManager(&rest.Config{Host: "https://localhost:1"}, "memory"), + }, + } + defer lib.workqueue.ShutDown() + + err := lib.Uninstall(context.Background(), "test-ns", "default") + // Helm fails since there's no real cluster — that's expected + assert.Error(t, err, "Helm uninstall should fail without a real cluster") + assert.Nil(t, lib.desiredOpts) +} + +// TestUninstallClearsDesiredOpts verifies that Uninstall nils desiredOpts and +// signals processingDone so the run() loop can return to idle. +func TestUninstallClearsDesiredOpts(t *testing.T) { + lib := &Library{ + workqueue: workqueue.NewTypedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[string]()), + inst: &installer{ + chartManager: helm.NewChartManager(&rest.Config{Host: "https://localhost:1"}, "memory"), + }, + } + defer lib.workqueue.ShutDown() + + // Simulate an active install cycle + opts := Options{Namespace: "test-ns", Version: "1.24.0"} + opts.applyDefaults() + lib.desiredOpts = &opts + lib.informerStop = make(chan struct{}) + lib.processingDone = make(chan struct{}) + + // Close processingDone to simulate the processing loop having exited + // (in real usage, enqueue() + nil check in processWorkQueue causes this) + close(lib.processingDone) + + // Uninstall clears desiredOpts and closes informerStop. + // The Helm uninstall will fail (no real cluster), but the state should + // already be cleared before that point. + err := lib.Uninstall(context.Background(), "test-ns", "default") + + // Helm fails since there's no real cluster — that's expected + assert.Error(t, err, "Helm uninstall should fail without a real cluster") + assert.Nil(t, lib.desiredOpts, "desiredOpts should be nil after Uninstall") +} + +// TestUninstallAfterCrash verifies that a freshly created Library (simulating +// a crash recovery) can uninstall using explicit namespace/revision args, +// without any prior Apply() call. +func TestUninstallAfterCrash(t *testing.T) { + lib := &Library{ + workqueue: workqueue.NewTypedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[string]()), + inst: &installer{ + chartManager: helm.NewChartManager(&rest.Config{Host: "https://localhost:1"}, "memory"), + }, + } + defer lib.workqueue.ShutDown() + + // No Apply() — simulates a fresh Library after crash. + // desiredOpts is nil, informerStop and processingDone are nil. + assert.Nil(t, lib.desiredOpts) + assert.Nil(t, lib.informerStop) + assert.Nil(t, lib.processingDone) + + // Uninstall should still attempt the Helm uninstall with the provided args. + err := lib.Uninstall(context.Background(), "my-namespace", "my-revision") + + // Helm fails (no real cluster) — the point is it attempted the uninstall + // instead of silently returning nil. + assert.Error(t, err, "Helm uninstall should fail without a real cluster") + assert.Nil(t, lib.desiredOpts) +} + +// TestUninstallDefaultsEmptyParams verifies that empty namespace and revision +// strings are defaulted to "istio-system" and "default". +func TestUninstallDefaultsEmptyParams(t *testing.T) { + lib := &Library{ + workqueue: workqueue.NewTypedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[string]()), + inst: &installer{ + chartManager: helm.NewChartManager(&rest.Config{Host: "https://localhost:1"}, "memory"), + }, + } + defer lib.workqueue.ShutDown() + + // Pass empty strings — should default and still attempt helm uninstall. + err := lib.Uninstall(context.Background(), "", "") + assert.Error(t, err, "Helm uninstall should fail without a real cluster") +} + +// TestApplyAfterUninstallSetsDesiredOpts verifies that Apply works after +// Uninstall — the library can be reused for a new install cycle. +func TestApplyAfterUninstallSetsDesiredOpts(t *testing.T) { + lib := &Library{ + workqueue: workqueue.NewTypedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[string]()), + } + defer lib.workqueue.ShutDown() + + // First cycle: Apply + lib.Apply(Options{Namespace: "ns1", Version: "1.24.0"}) + assert.NotNil(t, lib.desiredOpts) + assert.Equal(t, "ns1", lib.desiredOpts.Namespace) + + // Drain the queue + key, _ := lib.workqueue.Get() + lib.workqueue.Done(key) + + // Simulate Uninstall clearing state (without real Helm) + lib.mu.Lock() + lib.desiredOpts = nil + lib.mu.Unlock() + + assert.Nil(t, lib.desiredOpts) + + // Second cycle: Apply again + lib.Apply(Options{Namespace: "ns2", Version: "1.25.0"}) + assert.NotNil(t, lib.desiredOpts) + assert.Equal(t, "ns2", lib.desiredOpts.Namespace) +} + +// TestApplyBlocksDuringUninstall verifies that Apply blocks while Uninstall +// holds the lifecycle lock, and proceeds after Uninstall completes. +func TestApplyBlocksDuringUninstall(t *testing.T) { + lib := &Library{ + workqueue: workqueue.NewTypedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[string]()), + } + defer lib.workqueue.ShutDown() + + // Set up active state + opts := Options{Namespace: "test-ns", Version: "1.24.0"} + opts.applyDefaults() + lib.desiredOpts = &opts + lib.informerStop = make(chan struct{}) + lib.processingDone = make(chan struct{}) + + // Hold the lifecycle lock to simulate Uninstall in progress + lib.lifecycleMu.Lock() + + var applyStarted atomic.Int32 + var applyFinished atomic.Int32 + + go func() { + applyStarted.Store(1) + lib.Apply(Options{Namespace: "new-ns", Version: "1.25.0"}) + applyFinished.Store(1) + }() + + // Give the goroutine time to start and block on the lock + time.Sleep(50 * time.Millisecond) + assert.Equal(t, int32(1), applyStarted.Load(), "Apply goroutine should have started") + assert.Equal(t, int32(0), applyFinished.Load(), "Apply should be blocked by lifecycle lock") + + // Release the lock + lib.lifecycleMu.Unlock() + + // Apply should now complete + time.Sleep(50 * time.Millisecond) + assert.Equal(t, int32(1), applyFinished.Load(), "Apply should complete after lock release") +} + +// TestDoubleUninstallDoesNotPanic verifies that calling Uninstall twice does +// not panic on the second call (closing an already-closed channel), and that +// Apply still works afterward. +func TestDoubleUninstallDoesNotPanic(t *testing.T) { + lib := &Library{ + workqueue: workqueue.NewTypedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[string]()), + applySignal: make(chan struct{}, 1), + inst: &installer{ + chartManager: helm.NewChartManager(&rest.Config{Host: "https://localhost:1"}, "memory"), + }, + } + defer lib.workqueue.ShutDown() + + // Simulate an active install cycle + opts := Options{Namespace: "test-ns", Version: "1.24.0"} + opts.applyDefaults() + lib.desiredOpts = &opts + lib.informerStop = make(chan struct{}) + lib.processingDone = make(chan struct{}) + close(lib.processingDone) + + // First Uninstall + _ = lib.Uninstall(context.Background(), "test-ns", "default") + + // Second Uninstall must not panic + assert.NotPanics(t, func() { + _ = lib.Uninstall(context.Background(), "test-ns", "default") + }) + + assert.Nil(t, lib.desiredOpts) + assert.Nil(t, lib.informerStop) + assert.Nil(t, lib.processingDone) + + // Apply after double-uninstall should start a new cycle + lib.Apply(Options{Namespace: "new-ns", Version: "1.25.0"}) + assert.NotNil(t, lib.desiredOpts) + assert.Equal(t, "new-ns", lib.desiredOpts.Namespace) +} + +// TestUninstallDoesNotDeadlock reproduces a deadlock where processWorkQueue's +// defer reads a niled l.processingDone (set to nil by Uninstall) and skips +// closing the channel, causing Uninstall to block forever. +func TestUninstallDoesNotDeadlock(t *testing.T) { + lib := &Library{ + workqueue: workqueue.NewTypedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[string]()), + applySignal: make(chan struct{}, 1), + inst: &installer{ + chartManager: helm.NewChartManager(&rest.Config{Host: "https://localhost:1"}, "memory"), + }, + } + defer lib.workqueue.ShutDown() + + opts := Options{Namespace: "test-ns", Version: "1.24.0"} + opts.applyDefaults() + lib.desiredOpts = &opts + lib.informerStop = make(chan struct{}) + lib.processingDone = make(chan struct{}) + + notifyCh := make(chan struct{}, 1) + processingDone := lib.processingDone + go lib.processWorkQueue(context.Background(), notifyCh, processingDone) + + // Let processWorkQueue block on workqueue.Get() + time.Sleep(50 * time.Millisecond) + + done := make(chan error, 1) + go func() { + done <- lib.Uninstall(context.Background(), "test-ns", "default") + }() + + select { + case <-done: + // Uninstall completed — no deadlock + case <-time.After(5 * time.Second): + t.Fatal("Uninstall deadlocked waiting for processingDone") + } +} + +// TestEnqueueBackoffOnFailure verifies that after a failed reconcile (no Forget), +// informer-driven enqueues via AddRateLimited are delayed with exponential +// backoff, while Apply's direct Add() bypasses the rate limiter entirely. +func TestEnqueueBackoffOnFailure(t *testing.T) { + baseDelay := 100 * time.Millisecond + lib := &Library{ + workqueue: workqueue.NewTypedRateLimitingQueue( + workqueue.NewTypedItemExponentialFailureRateLimiter[string](baseDelay, 2*time.Second), + ), + } + defer lib.workqueue.ShutDown() + + // Apply uses Add() directly — should be immediate + lib.workqueue.Add(reconcileKey) + + start := time.Now() + key, _ := lib.workqueue.Get() + assert.Less(t, time.Since(start), 50*time.Millisecond, "Add() should be immediate") + + // Simulate failure: Done without Forget + lib.workqueue.Done(key) + + // First informer-driven enqueue after failure + lib.enqueue() + start = time.Now() + key, _ = lib.workqueue.Get() + firstDelay := time.Since(start) + assert.GreaterOrEqual(t, firstDelay, baseDelay/2, "first failure should be delayed") + lib.workqueue.Done(key) + + // Second failure: delay should grow (exponential backoff) + lib.enqueue() + start = time.Now() + key, _ = lib.workqueue.Get() + secondDelay := time.Since(start) + assert.Greater(t, secondDelay, firstDelay, "backoff should increase on repeated failure") + lib.workqueue.Done(key) + + // Apply's Add() bypasses rate limiter even with accumulated backoff + lib.workqueue.Add(reconcileKey) + start = time.Now() + key, _ = lib.workqueue.Get() + assert.Less(t, time.Since(start), 50*time.Millisecond, "Add() should bypass rate limiter") + + // Simulate success: Forget resets backoff + lib.workqueue.Forget(key) + lib.workqueue.Done(key) + + // After Forget, delay should be back to base (less than the exponential delay) + lib.enqueue() + start = time.Now() + key, _ = lib.workqueue.Get() + resetDelay := time.Since(start) + assert.Less(t, resetDelay, secondDelay, "delay after Forget should reset below previous exponential delay") + lib.workqueue.Forget(key) + lib.workqueue.Done(key) +} + +// Note: Value computation tests are in pkg/revision and pkg/istiovalues packages. +// The reconcile() method uses revision.ComputeValues() which is tested there. diff --git a/pkg/install/predicates.go b/pkg/install/predicates.go new file mode 100644 index 0000000000..e1cb9462aa --- /dev/null +++ b/pkg/install/predicates.go @@ -0,0 +1,272 @@ +// 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 install + +import ( + "reflect" + "regexp" + + "github.com/istio-ecosystem/sail-operator/pkg/constants" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +// istiodValidatorRe matches istiod-managed ValidatingWebhookConfiguration names. +// Precompiled to avoid recompilation on every update event. +var istiodValidatorRe = regexp.MustCompile(`^(istiod-.*-validator|istio-validator.*)$`) + +// Predicate filtering logic adapted from controllers/istiorevision for use with dynamic informers. +// These functions determine whether a resource change should trigger reconciliation. + +const ( + // ignoreAnnotation is the annotation that, when set to "true", causes updates to be ignored. + ignoreAnnotation = "sailoperator.io/ignore" +) + +// GVKs that need special predicate handling +var ( + serviceGVK = schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Service"} + serviceAccountGVK = schema.GroupVersionKind{Group: "", Version: "v1", Kind: "ServiceAccount"} + namespaceGVK = schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Namespace"} + networkPolicyGVK = schema.GroupVersionKind{Group: "networking.k8s.io", Version: "v1", Kind: "NetworkPolicy"} + pdbGVK = schema.GroupVersionKind{Group: "policy", Version: "v1", Kind: "PodDisruptionBudget"} + hpaGVK = schema.GroupVersionKind{Group: "autoscaling", Version: "v2", Kind: "HorizontalPodAutoscaler"} + validatingWebhookConfigurationGVK = schema.GroupVersionKind{Group: "admissionregistration.k8s.io", Version: "v1", Kind: "ValidatingWebhookConfiguration"} + crdGVK = schema.GroupVersionKind{Group: "apiextensions.k8s.io", Version: "v1", Kind: "CustomResourceDefinition"} +) + +// shouldReconcileOnCreate determines if a create event should trigger reconciliation. +func shouldReconcileOnCreate(obj *unstructured.Unstructured) bool { + // Always reconcile on create + return true +} + +// shouldReconcileOnDelete determines if a delete event should trigger reconciliation. +func shouldReconcileOnDelete(obj *unstructured.Unstructured) bool { + // Always reconcile on delete to restore the resource + return true +} + +// shouldReconcileOnUpdate determines if an update event should trigger reconciliation. +// Implements the same logic as the operator's predicates. +func shouldReconcileOnUpdate(gvk schema.GroupVersionKind, oldObj, newObj *unstructured.Unstructured) bool { + // Check ignore annotation first - applies to all resources + if hasIgnoreAnnotation(newObj) { + return false + } + + // ServiceAccounts: ignore all updates to prevent removing pull secrets added by other controllers + if gvk == serviceAccountGVK { + return false + } + + // ValidatingWebhookConfiguration: special handling for istiod-managed webhooks + if gvk == validatingWebhookConfigurationGVK { + return shouldReconcileValidatingWebhook(oldObj, newObj) + } + + // Resources that need status-change filtering + if shouldFilterStatusChanges(gvk) { + return !isStatusOnlyChange(gvk, oldObj, newObj) + } + + // Default: reconcile on any change + return true +} + +// shouldFilterStatusChanges returns true for GVKs that should ignore status-only changes. +func shouldFilterStatusChanges(gvk schema.GroupVersionKind) bool { + switch gvk { + case serviceGVK, networkPolicyGVK, pdbGVK, hpaGVK, namespaceGVK: + return true + default: + return false + } +} + +// isStatusOnlyChange returns true if only the status changed (no spec/labels/annotations/etc). +// This prevents unnecessary reconciliation when only status fields are updated. +func isStatusOnlyChange(gvk schema.GroupVersionKind, oldObj, newObj *unstructured.Unstructured) bool { + // Check if spec was updated + if specWasUpdated(gvk, oldObj, newObj) { + return false + } + + // Check if labels changed + if !reflect.DeepEqual(oldObj.GetLabels(), newObj.GetLabels()) { + return false + } + + // Check if annotations changed + if !reflect.DeepEqual(oldObj.GetAnnotations(), newObj.GetAnnotations()) { + return false + } + + // Check if owner references changed + if !reflect.DeepEqual(oldObj.GetOwnerReferences(), newObj.GetOwnerReferences()) { + return false + } + + // Check if finalizers changed + if !reflect.DeepEqual(oldObj.GetFinalizers(), newObj.GetFinalizers()) { + return false + } + + // Only status changed + return true +} + +// specWasUpdated checks if the spec of a resource was updated. +func specWasUpdated(gvk schema.GroupVersionKind, oldObj, newObj *unstructured.Unstructured) bool { + // For HPAs, k8s doesn't set metadata.generation, so we check the spec directly + if gvk == hpaGVK { + oldSpec, _, _ := unstructured.NestedMap(oldObj.Object, "spec") + newSpec, _, _ := unstructured.NestedMap(newObj.Object, "spec") + return !reflect.DeepEqual(oldSpec, newSpec) + } + + // For other resources, comparing metadata.generation suffices + return oldObj.GetGeneration() != newObj.GetGeneration() +} + +// hasIgnoreAnnotation checks if the resource has the sailoperator.io/ignore annotation set to "true". +func hasIgnoreAnnotation(obj *unstructured.Unstructured) bool { + annotations := obj.GetAnnotations() + if annotations == nil { + return false + } + return annotations[ignoreAnnotation] == "true" +} + +// shouldReconcileValidatingWebhook handles special filtering for ValidatingWebhookConfiguration. +// Istiod updates the caBundle and failurePolicy fields in its validator webhook configs, +// and we must ignore these changes to prevent an endless update loop. +func shouldReconcileValidatingWebhook(oldObj, newObj *unstructured.Unstructured) bool { + name := newObj.GetName() + + // Check if this is an istiod-managed validator webhook + if !istiodValidatorRe.MatchString(name) { + // Not an istiod validator, reconcile normally + return true + } + + // For istiod validators, compare objects after clearing fields that istiod updates + oldCopy := clearIgnoredFields(oldObj.DeepCopy()) + newCopy := clearIgnoredFields(newObj.DeepCopy()) + + return !reflect.DeepEqual(oldCopy.Object, newCopy.Object) +} + +// clearIgnoredFields clears fields that should be ignored when comparing webhook configs. +func clearIgnoredFields(obj *unstructured.Unstructured) *unstructured.Unstructured { + // Clear metadata fields that change frequently + obj.SetResourceVersion("") + obj.SetGeneration(0) + obj.SetManagedFields(nil) + + switch obj.GetKind() { + case "ValidatingWebhookConfiguration": + webhooks, found, _ := unstructured.NestedSlice(obj.Object, "webhooks") + if found { + for i := range webhooks { + if webhook, ok := webhooks[i].(map[string]interface{}); ok { + delete(webhook, "failurePolicy") + if clientConfig, ok := webhook["clientConfig"].(map[string]interface{}); ok { + delete(clientConfig, "caBundle") + } + webhooks[i] = webhook + } + } + _ = unstructured.SetNestedSlice(obj.Object, webhooks, "webhooks") + } + case "MutatingWebhookConfiguration": + webhooks, found, _ := unstructured.NestedSlice(obj.Object, "webhooks") + if found { + for i := range webhooks { + if webhook, ok := webhooks[i].(map[string]interface{}); ok { + if clientConfig, ok := webhook["clientConfig"].(map[string]interface{}); ok { + delete(clientConfig, "caBundle") + } + webhooks[i] = webhook + } + } + _ = unstructured.SetNestedSlice(obj.Object, webhooks, "webhooks") + } + } + + return obj +} + +// isOwnedResource checks if the resource is owned by our installation +// by examining Istio labels against the expected revision and the +// managed-by label set by the post-renderer. +func isOwnedResource(obj *unstructured.Unstructured, revision, managedByValue string) bool { + labels := obj.GetLabels() + if labels == nil { + return false + } + + if rev, ok := labels["istio.io/rev"]; ok { + expectedRev := revision + if expectedRev == defaultRevision { + expectedRev = "default" + } + return rev == expectedRev + } + + if _, ok := labels["operator.istio.io/component"]; ok { + return true + } + + // Check the managed-by label set by HelmPostRenderer. + if managedBy, ok := labels[constants.ManagedByLabelKey]; ok { + return managedBy == managedByValue + } + + // Fallback: Helm sets app.kubernetes.io/managed-by to "Helm" on chart resources. + if managedBy, ok := labels["app.kubernetes.io/managed-by"]; ok { + return managedBy == "Helm" + } + + return false +} + +// isTargetCRD checks if an unstructured CRD object's name is in the target set. +func isTargetCRD(obj *unstructured.Unstructured, targets map[string]struct{}) bool { + if len(targets) == 0 { + return false + } + _, ok := targets[obj.GetName()] + return ok +} + +// shouldReconcileCRDOnUpdate determines if a CRD update should trigger reconciliation. +// We care about ownership label changes (CIO/OLM labels being added/removed) and +// annotation changes (helm.sh/resource-policy). Spec/status changes on the CRD itself +// are not relevant for ownership classification. +func shouldReconcileCRDOnUpdate(oldObj, newObj *unstructured.Unstructured) bool { + if !reflect.DeepEqual(oldObj.GetLabels(), newObj.GetLabels()) { + return true + } + if !reflect.DeepEqual(oldObj.GetAnnotations(), newObj.GetAnnotations()) { + return true + } + // Spec changes on CRDs mean the CRD schema was updated — re-reconcile + // to ensure our version is still current. + if oldObj.GetGeneration() != newObj.GetGeneration() { + return true + } + return false +} diff --git a/pkg/install/predicates_test.go b/pkg/install/predicates_test.go new file mode 100644 index 0000000000..b0f884cd1a --- /dev/null +++ b/pkg/install/predicates_test.go @@ -0,0 +1,460 @@ +// 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 install + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +func TestShouldReconcileOnCreate(t *testing.T) { + obj := &unstructured.Unstructured{} + obj.SetName("test") + + // Create events should always trigger reconciliation + assert.True(t, shouldReconcileOnCreate(obj)) +} + +func TestShouldReconcileOnDelete(t *testing.T) { + obj := &unstructured.Unstructured{} + obj.SetName("test") + + // Delete events should always trigger reconciliation + assert.True(t, shouldReconcileOnDelete(obj)) +} + +func TestHasIgnoreAnnotation(t *testing.T) { + tests := []struct { + name string + annotations map[string]string + expected bool + }{ + { + name: "no annotations", + annotations: nil, + expected: false, + }, + { + name: "empty annotations", + annotations: map[string]string{}, + expected: false, + }, + { + name: "ignore annotation set to true", + annotations: map[string]string{ignoreAnnotation: "true"}, + expected: true, + }, + { + name: "ignore annotation set to false", + annotations: map[string]string{ignoreAnnotation: "false"}, + expected: false, + }, + { + name: "ignore annotation set to other value", + annotations: map[string]string{ignoreAnnotation: "yes"}, + expected: false, + }, + { + name: "other annotations present", + annotations: map[string]string{"other": "value"}, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + obj := &unstructured.Unstructured{} + obj.SetAnnotations(tt.annotations) + + result := hasIgnoreAnnotation(obj) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestShouldReconcileOnUpdate_IgnoreAnnotation(t *testing.T) { + gvk := schema.GroupVersionKind{Group: "", Version: "v1", Kind: "ConfigMap"} + + oldObj := &unstructured.Unstructured{} + oldObj.SetName("test") + + newObj := &unstructured.Unstructured{} + newObj.SetName("test") + newObj.SetAnnotations(map[string]string{ignoreAnnotation: "true"}) + + // Should not reconcile when ignore annotation is set + assert.False(t, shouldReconcileOnUpdate(gvk, oldObj, newObj)) +} + +func TestShouldReconcileOnUpdate_ServiceAccount(t *testing.T) { + gvk := serviceAccountGVK + + oldObj := &unstructured.Unstructured{} + oldObj.SetName("test") + oldObj.SetGeneration(1) + + newObj := &unstructured.Unstructured{} + newObj.SetName("test") + newObj.SetGeneration(2) + + // ServiceAccount updates should always be ignored + assert.False(t, shouldReconcileOnUpdate(gvk, oldObj, newObj)) +} + +func TestIsStatusOnlyChange(t *testing.T) { + gvk := schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Service"} + + tests := []struct { + name string + setupOld func(*unstructured.Unstructured) + setupNew func(*unstructured.Unstructured) + isStatusOnly bool + }{ + { + name: "same objects - status only", + setupOld: func(obj *unstructured.Unstructured) { + obj.SetGeneration(1) + }, + setupNew: func(obj *unstructured.Unstructured) { + obj.SetGeneration(1) + }, + isStatusOnly: true, + }, + { + name: "generation changed - not status only", + setupOld: func(obj *unstructured.Unstructured) { + obj.SetGeneration(1) + }, + setupNew: func(obj *unstructured.Unstructured) { + obj.SetGeneration(2) + }, + isStatusOnly: false, + }, + { + name: "labels changed - not status only", + setupOld: func(obj *unstructured.Unstructured) { + obj.SetGeneration(1) + }, + setupNew: func(obj *unstructured.Unstructured) { + obj.SetGeneration(1) + obj.SetLabels(map[string]string{"new": "label"}) + }, + isStatusOnly: false, + }, + { + name: "annotations changed - not status only", + setupOld: func(obj *unstructured.Unstructured) { + obj.SetGeneration(1) + }, + setupNew: func(obj *unstructured.Unstructured) { + obj.SetGeneration(1) + obj.SetAnnotations(map[string]string{"new": "annotation"}) + }, + isStatusOnly: false, + }, + { + name: "finalizers changed - not status only", + setupOld: func(obj *unstructured.Unstructured) { + obj.SetGeneration(1) + }, + setupNew: func(obj *unstructured.Unstructured) { + obj.SetGeneration(1) + obj.SetFinalizers([]string{"new-finalizer"}) + }, + isStatusOnly: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + oldObj := &unstructured.Unstructured{} + newObj := &unstructured.Unstructured{} + + tt.setupOld(oldObj) + tt.setupNew(newObj) + + result := isStatusOnlyChange(gvk, oldObj, newObj) + assert.Equal(t, tt.isStatusOnly, result) + }) + } +} + +func TestSpecWasUpdated_HPA(t *testing.T) { + gvk := hpaGVK + + tests := []struct { + name string + oldSpec map[string]interface{} + newSpec map[string]interface{} + expected bool + }{ + { + name: "same spec", + oldSpec: map[string]interface{}{"minReplicas": int64(1)}, + newSpec: map[string]interface{}{"minReplicas": int64(1)}, + expected: false, + }, + { + name: "different spec", + oldSpec: map[string]interface{}{"minReplicas": int64(1)}, + newSpec: map[string]interface{}{"minReplicas": int64(2)}, + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + oldObj := &unstructured.Unstructured{Object: map[string]interface{}{}} + newObj := &unstructured.Unstructured{Object: map[string]interface{}{}} + + _ = unstructured.SetNestedMap(oldObj.Object, tt.oldSpec, "spec") + _ = unstructured.SetNestedMap(newObj.Object, tt.newSpec, "spec") + + result := specWasUpdated(gvk, oldObj, newObj) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestShouldFilterStatusChanges(t *testing.T) { + tests := []struct { + gvk schema.GroupVersionKind + expected bool + }{ + {serviceGVK, true}, + {networkPolicyGVK, true}, + {pdbGVK, true}, + {hpaGVK, true}, + {namespaceGVK, true}, + {serviceAccountGVK, false}, + {validatingWebhookConfigurationGVK, false}, + {schema.GroupVersionKind{Group: "", Version: "v1", Kind: "ConfigMap"}, false}, + } + + for _, tt := range tests { + t.Run(tt.gvk.Kind, func(t *testing.T) { + result := shouldFilterStatusChanges(tt.gvk) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestShouldReconcileCRDOnUpdate(t *testing.T) { + tests := []struct { + name string + setupOld func(*unstructured.Unstructured) + setupNew func(*unstructured.Unstructured) + expected bool + }{ + { + name: "no changes", + setupOld: func(obj *unstructured.Unstructured) { + obj.SetGeneration(1) + obj.SetLabels(map[string]string{"foo": "bar"}) + }, + setupNew: func(obj *unstructured.Unstructured) { + obj.SetGeneration(1) + obj.SetLabels(map[string]string{"foo": "bar"}) + }, + expected: false, + }, + { + name: "label added", + setupOld: func(obj *unstructured.Unstructured) { + obj.SetGeneration(1) + }, + setupNew: func(obj *unstructured.Unstructured) { + obj.SetGeneration(1) + obj.SetLabels(map[string]string{"ingress.operator.openshift.io/owned": "true"}) + }, + expected: true, + }, + { + name: "label removed", + setupOld: func(obj *unstructured.Unstructured) { + obj.SetGeneration(1) + obj.SetLabels(map[string]string{"ingress.operator.openshift.io/owned": "true"}) + }, + setupNew: func(obj *unstructured.Unstructured) { + obj.SetGeneration(1) + }, + expected: true, + }, + { + name: "annotation changed", + setupOld: func(obj *unstructured.Unstructured) { + obj.SetGeneration(1) + }, + setupNew: func(obj *unstructured.Unstructured) { + obj.SetGeneration(1) + obj.SetAnnotations(map[string]string{"helm.sh/resource-policy": "keep"}) + }, + expected: true, + }, + { + name: "generation changed (spec update)", + setupOld: func(obj *unstructured.Unstructured) { + obj.SetGeneration(1) + }, + setupNew: func(obj *unstructured.Unstructured) { + obj.SetGeneration(2) + }, + expected: true, + }, + { + name: "only resourceVersion changed (status-like)", + setupOld: func(obj *unstructured.Unstructured) { + obj.SetGeneration(1) + obj.SetResourceVersion("100") + }, + setupNew: func(obj *unstructured.Unstructured) { + obj.SetGeneration(1) + obj.SetResourceVersion("101") + }, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + oldObj := &unstructured.Unstructured{Object: map[string]interface{}{}} + newObj := &unstructured.Unstructured{Object: map[string]interface{}{}} + tt.setupOld(oldObj) + tt.setupNew(newObj) + assert.Equal(t, tt.expected, shouldReconcileCRDOnUpdate(oldObj, newObj)) + }) + } +} + +func TestIsTargetCRD(t *testing.T) { + targets := map[string]struct{}{ + "wasmplugins.extensions.istio.io": {}, + "destinationrules.networking.istio.io": {}, + "envoyfilters.networking.istio.io": {}, + } + + tests := []struct { + name string + crdName string + targets map[string]struct{} + expected bool + }{ + { + name: "matching target", + crdName: "wasmplugins.extensions.istio.io", + targets: targets, + expected: true, + }, + { + name: "not a target", + crdName: "gateways.gateway.networking.k8s.io", + targets: targets, + expected: false, + }, + { + name: "empty targets", + crdName: "wasmplugins.extensions.istio.io", + targets: nil, + expected: false, + }, + { + name: "empty name", + crdName: "", + targets: targets, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + obj := &unstructured.Unstructured{} + obj.SetName(tt.crdName) + assert.Equal(t, tt.expected, isTargetCRD(obj, tt.targets)) + }) + } +} + +func TestShouldReconcileValidatingWebhook(t *testing.T) { + tests := []struct { + name string + objName string + oldObj func() *unstructured.Unstructured + newObj func() *unstructured.Unstructured + expected bool + }{ + { + name: "non-istiod webhook - always reconcile", + objName: "some-other-webhook", + oldObj: func() *unstructured.Unstructured { + obj := &unstructured.Unstructured{} + obj.SetGeneration(1) + return obj + }, + newObj: func() *unstructured.Unstructured { + obj := &unstructured.Unstructured{} + obj.SetGeneration(2) + return obj + }, + expected: true, + }, + { + name: "istiod validator - same content", + objName: "istiod-istio-system-validator", + oldObj: func() *unstructured.Unstructured { + obj := &unstructured.Unstructured{} + obj.SetName("istiod-istio-system-validator") + obj.SetResourceVersion("123") + return obj + }, + newObj: func() *unstructured.Unstructured { + obj := &unstructured.Unstructured{} + obj.SetName("istiod-istio-system-validator") + obj.SetResourceVersion("456") // Different resource version + return obj + }, + expected: false, // Resource version is cleared, so they're equal + }, + { + name: "istio-validator - same content", + objName: "istio-validator-istio-system", + oldObj: func() *unstructured.Unstructured { + obj := &unstructured.Unstructured{} + obj.SetName("istio-validator-istio-system") + return obj + }, + newObj: func() *unstructured.Unstructured { + obj := &unstructured.Unstructured{} + obj.SetName("istio-validator-istio-system") + return obj + }, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + oldObj := tt.oldObj() + oldObj.SetName(tt.objName) + newObj := tt.newObj() + newObj.SetName(tt.objName) + + result := shouldReconcileValidatingWebhook(oldObj, newObj) + assert.Equal(t, tt.expected, result) + }) + } +} diff --git a/pkg/install/rbac.go b/pkg/install/rbac.go new file mode 100644 index 0000000000..34a8325b05 --- /dev/null +++ b/pkg/install/rbac.go @@ -0,0 +1,113 @@ +// 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 install + +import rbacv1 "k8s.io/api/rbac/v1" + +// LibraryRBACRules returns the RBAC PolicyRules required when using the +// install library. Consumers should aggregate these into their own ClusterRole. +// +// These rules are derived from chart/templates/rbac/role.yaml with +// sailoperator.io CRs filtered out (those are for the operator, not the library). +// +// Example usage: +// +// rules := append(myOperatorRules, install.LibraryRBACRules()...) +// +// TODO: Consider generating this from role.yaml or adding a verification test +// to ensure these rules stay in sync with the Helm template. +func LibraryRBACRules() []rbacv1.PolicyRule { + return []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{ + "configmaps", + "endpoints", + "events", + "namespaces", + "nodes", + "persistentvolumeclaims", + "pods", + "replicationcontrollers", + "resourcequotas", + "secrets", + "serviceaccounts", + "services", + }, + Verbs: []string{"create", "delete", "get", "list", "patch", "update", "watch"}, + }, + { + APIGroups: []string{"admissionregistration.k8s.io"}, + Resources: []string{ + "mutatingwebhookconfigurations", + "validatingadmissionpolicies", + "validatingadmissionpolicybindings", + "validatingwebhookconfigurations", + }, + Verbs: []string{"create", "delete", "get", "list", "patch", "update", "watch"}, + }, + { + APIGroups: []string{"apiextensions.k8s.io"}, + Resources: []string{"customresourcedefinitions"}, + Verbs: []string{"get", "list", "watch"}, + }, + { + APIGroups: []string{"apps"}, + Resources: []string{"daemonsets", "deployments"}, + Verbs: []string{"create", "delete", "get", "list", "patch", "update", "watch"}, + }, + { + APIGroups: []string{"autoscaling"}, + Resources: []string{"horizontalpodautoscalers"}, + Verbs: []string{"create", "delete", "get", "list", "patch", "update", "watch"}, + }, + { + APIGroups: []string{"discovery.k8s.io"}, + Resources: []string{"endpointslices"}, + Verbs: []string{"create", "delete", "get", "list", "patch", "update", "watch"}, + }, + { + APIGroups: []string{"k8s.cni.cncf.io"}, + Resources: []string{"network-attachment-definitions"}, + Verbs: []string{"create", "delete", "get", "list", "patch", "update", "watch"}, + }, + { + APIGroups: []string{"networking.istio.io"}, + Resources: []string{"envoyfilters"}, + Verbs: []string{"create", "delete", "get", "list", "patch", "update", "watch"}, + }, + { + APIGroups: []string{"networking.k8s.io"}, + Resources: []string{"networkpolicies"}, + Verbs: []string{"create", "delete", "get", "list", "patch", "update", "watch"}, + }, + { + APIGroups: []string{"policy"}, + Resources: []string{"poddisruptionbudgets"}, + Verbs: []string{"create", "delete", "get", "list", "patch", "update", "watch"}, + }, + { + APIGroups: []string{"rbac.authorization.k8s.io"}, + Resources: []string{"clusterrolebindings", "clusterroles", "rolebindings", "roles"}, + Verbs: []string{"create", "delete", "get", "list", "patch", "update", "watch", "bind", "escalate"}, + }, + { + APIGroups: []string{"security.openshift.io"}, + Resources: []string{"securitycontextconstraints"}, + ResourceNames: []string{"privileged"}, + Verbs: []string{"use"}, + }, + } +} diff --git a/pkg/install/values.go b/pkg/install/values.go new file mode 100644 index 0000000000..303a6c4301 --- /dev/null +++ b/pkg/install/values.go @@ -0,0 +1,161 @@ +// 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 install + +import ( + "fmt" + + v1 "github.com/istio-ecosystem/sail-operator/api/v1" + "github.com/istio-ecosystem/sail-operator/pkg/helm" + "k8s.io/utils/ptr" +) + +// GatewayAPIDefaults returns pre-configured values for Gateway API mode on OpenShift. +// These values configure istiod to work as a Gateway API controller. +// +// Usage: +// +// values := install.GatewayAPIDefaults() +// values.Pilot.Env["PILOT_GATEWAY_API_CONTROLLER_NAME"] = "my-controller" +// values.Pilot.Env["PILOT_GATEWAY_API_DEFAULT_GATEWAYCLASS_NAME"] = "my-class" +// installer.Install(ctx, Options{Values: values}) +// +// Consumer must set: +// - pilot.env.PILOT_GATEWAY_API_CONTROLLER_NAME +// - pilot.env.PILOT_GATEWAY_API_DEFAULT_GATEWAYCLASS_NAME +// +// Consumer may optionally set: +// - global.trustBundleName (if using custom CA) +func GatewayAPIDefaults() *v1.Values { + return &v1.Values{ + Global: &v1.GlobalConfig{ + // Disable PodDisruptionBudget - managed externally + DefaultPodDisruptionBudget: &v1.DefaultPodDisruptionBudgetConfig{ + Enabled: ptr.To(false), + }, + // Use cluster-critical priority for control plane + PriorityClassName: ptr.To("system-cluster-critical"), + }, + Pilot: &v1.PilotConfig{ + // Disable CNI - not needed for Gateway API only mode + Cni: &v1.CNIUsageConfig{ + Enabled: ptr.To(false), + }, + Enabled: ptr.To(true), + Env: map[string]string{ + // Enable Gateway API support + "PILOT_ENABLE_GATEWAY_API": "true", + // Disable experimental/alpha Gateway API features + "PILOT_ENABLE_ALPHA_GATEWAY_API": "false", + // Enable status updates on Gateway API resources + "PILOT_ENABLE_GATEWAY_API_STATUS": "true", + // Enable automated deployment (creates Envoy proxy + service for gateways) + "PILOT_ENABLE_GATEWAY_API_DEPLOYMENT_CONTROLLER": "true", + // Disable gatewayclass controller (admin manages gatewayclass) + "PILOT_ENABLE_GATEWAY_API_GATEWAYCLASS_CONTROLLER": "false", + // Disable multi-network gateway discovery + "PILOT_MULTI_NETWORK_DISCOVER_GATEWAY_API": "false", + // Disable manual deployment (only automated deployment allowed) + "ENABLE_GATEWAY_API_MANUAL_DEPLOYMENT": "false", + // Only create CA bundle configmap in namespaces with gateways + "PILOT_ENABLE_GATEWAY_API_CA_CERT_ONLY": "true", + // Don't copy labels/annotations from gateways to generated resources + "PILOT_ENABLE_GATEWAY_API_COPY_LABELS_ANNOTATIONS": "false", + // Resource filtering for Gateway API mode (X_ prefix until Istio feature is ready) + // When active, istiod will only reconcile Gateway API + the 3 included Istio resources + envPilotIgnoreResources: gatewayAPIIgnoreResources, + envPilotIncludeResources: gatewayAPIIncludeResources, + }, + }, + SidecarInjectorWebhook: &v1.SidecarInjectorConfig{ + // Disable sidecar injection by default (Gateway API mode only) + EnableNamespacesByDefault: ptr.To(false), + }, + MeshConfig: &v1.MeshConfig{ + // Enable access logging + AccessLogFile: ptr.To("/dev/stdout"), + // Disable legacy ingress controller + IngressControllerMode: v1.MeshConfigIngressControllerModeOff, + // Configure proxy defaults + DefaultConfig: &v1.MeshConfigProxyConfig{ + ProxyHeaders: &v1.ProxyConfigProxyHeaders{ + // Don't set Server header + Server: &v1.ProxyConfigProxyHeadersServer{ + Disabled: ptr.To(true), + }, + // Don't set X-Envoy-* debug headers + EnvoyDebugHeaders: &v1.ProxyConfigProxyHeadersEnvoyDebugHeaders{ + Disabled: ptr.To(true), + }, + // Only exchange metadata headers for in-mesh traffic + MetadataExchangeHeaders: &v1.ProxyConfigProxyHeadersMetadataExchangeHeaders{ + Mode: v1.ProxyConfigProxyHeadersMetadataExchangeModeInMesh, + }, + }, + }, + }, + } +} + +// mergeOverwrite recursively merges overlay into base, with overlay taking precedence. +// NOTE: This is a copy of istiovalues.mergeOverwrite. Consider exporting the original +// to avoid duplication once the library API stabilizes. +func mergeOverwrite(base map[string]any, overrides map[string]any) map[string]any { + if base == nil { + base = make(map[string]any, 1) + } + for key, value := range overrides { + if _, exists := base[key]; !exists { + base[key] = value + continue + } + childOverrides, overrideValueIsMap := value.(map[string]any) + childBase, baseValueIsMap := base[key].(map[string]any) + if baseValueIsMap && overrideValueIsMap { + base[key] = mergeOverwrite(childBase, childOverrides) + } else { + base[key] = value + } + } + return base +} + +// MergeValues merges two Values structs, with overlay taking precedence. +// This is useful for combining GatewayAPIDefaults() with custom overrides. +// +// Maps are merged recursively (overlay keys override base keys). +// Lists are replaced entirely (overlay list replaces base list). +// +// Example: +// +// base := install.GatewayAPIDefaults() +// overrides := &v1.Values{Global: &v1.GlobalConfig{Hub: ptr.To("my-registry")}} +// merged := install.MergeValues(base, overrides) +func MergeValues(base, overlay *v1.Values) *v1.Values { + if base == nil { + return overlay + } + if overlay == nil { + return base + } + baseMap := helm.FromValues(base) + overlayMap := helm.FromValues(overlay) + merged := mergeOverwrite(baseMap, overlayMap) + result, err := helm.ToValues(merged, &v1.Values{}) + if err != nil { + panic(fmt.Sprintf("failed to convert merged values: %v", err)) + } + return result +} diff --git a/pkg/install/values_test.go b/pkg/install/values_test.go new file mode 100644 index 0000000000..7f4149b3ed --- /dev/null +++ b/pkg/install/values_test.go @@ -0,0 +1,167 @@ +// 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 install + +import ( + "testing" + + v1 "github.com/istio-ecosystem/sail-operator/api/v1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/utils/ptr" +) + +func TestGatewayAPIDefaults(t *testing.T) { + defaults := GatewayAPIDefaults() + + // Check Global settings + require.NotNil(t, defaults.Global) + assert.NotNil(t, defaults.Global.DefaultPodDisruptionBudget) + assert.Equal(t, false, *defaults.Global.DefaultPodDisruptionBudget.Enabled) + + // Check Pilot settings + require.NotNil(t, defaults.Pilot) + assert.Equal(t, true, *defaults.Pilot.Enabled) + assert.NotNil(t, defaults.Pilot.Cni) + assert.Equal(t, false, *defaults.Pilot.Cni.Enabled) + + // Check Gateway API env vars + require.NotNil(t, defaults.Pilot.Env) + assert.Equal(t, "true", defaults.Pilot.Env["PILOT_ENABLE_GATEWAY_API"]) + assert.Equal(t, "false", defaults.Pilot.Env["PILOT_ENABLE_ALPHA_GATEWAY_API"]) + assert.Equal(t, "true", defaults.Pilot.Env["PILOT_ENABLE_GATEWAY_API_STATUS"]) + assert.Equal(t, "true", defaults.Pilot.Env["PILOT_ENABLE_GATEWAY_API_DEPLOYMENT_CONTROLLER"]) + assert.Equal(t, "false", defaults.Pilot.Env["PILOT_ENABLE_GATEWAY_API_GATEWAYCLASS_CONTROLLER"]) + assert.Equal(t, "false", defaults.Pilot.Env["PILOT_MULTI_NETWORK_DISCOVER_GATEWAY_API"]) + assert.Equal(t, "false", defaults.Pilot.Env["ENABLE_GATEWAY_API_MANUAL_DEPLOYMENT"]) + assert.Equal(t, "true", defaults.Pilot.Env["PILOT_ENABLE_GATEWAY_API_CA_CERT_ONLY"]) + assert.Equal(t, "false", defaults.Pilot.Env["PILOT_ENABLE_GATEWAY_API_COPY_LABELS_ANNOTATIONS"]) + + // Check resource filtering env vars (X_ prefixed until Istio feature is ready) + assert.Equal(t, gatewayAPIIgnoreResources, defaults.Pilot.Env[envPilotIgnoreResources]) + assert.Equal(t, gatewayAPIIncludeResources, defaults.Pilot.Env[envPilotIncludeResources]) + + // Check SidecarInjectorWebhook settings + require.NotNil(t, defaults.SidecarInjectorWebhook) + assert.Equal(t, false, *defaults.SidecarInjectorWebhook.EnableNamespacesByDefault) + + // Check MeshConfig settings + require.NotNil(t, defaults.MeshConfig) + assert.Equal(t, "/dev/stdout", *defaults.MeshConfig.AccessLogFile) + assert.Equal(t, v1.MeshConfigIngressControllerModeOff, defaults.MeshConfig.IngressControllerMode) + + // Check proxy headers + require.NotNil(t, defaults.MeshConfig.DefaultConfig) + require.NotNil(t, defaults.MeshConfig.DefaultConfig.ProxyHeaders) + assert.Equal(t, true, *defaults.MeshConfig.DefaultConfig.ProxyHeaders.Server.Disabled) + assert.Equal(t, true, *defaults.MeshConfig.DefaultConfig.ProxyHeaders.EnvoyDebugHeaders.Disabled) + assert.Equal(t, v1.ProxyConfigProxyHeadersMetadataExchangeModeInMesh, defaults.MeshConfig.DefaultConfig.ProxyHeaders.MetadataExchangeHeaders.Mode) +} + +func TestMergeValues(t *testing.T) { + t.Run("nil base returns overlay", func(t *testing.T) { + overlay := &v1.Values{ + Global: &v1.GlobalConfig{Hub: ptr.To("overlay-hub")}, + } + + result := MergeValues(nil, overlay) + + assert.Equal(t, overlay, result) + }) + + t.Run("nil overlay returns base", func(t *testing.T) { + base := &v1.Values{ + Global: &v1.GlobalConfig{Hub: ptr.To("base-hub")}, + } + + result := MergeValues(base, nil) + + assert.Equal(t, base, result) + }) + + t.Run("overlay takes precedence", func(t *testing.T) { + base := &v1.Values{ + Global: &v1.GlobalConfig{ + Hub: ptr.To("base-hub"), + PriorityClassName: ptr.To("base-priority"), + }, + } + overlay := &v1.Values{ + Global: &v1.GlobalConfig{ + Hub: ptr.To("overlay-hub"), + }, + } + + result := MergeValues(base, overlay) + + assert.Equal(t, "overlay-hub", *result.Global.Hub) + }) + + t.Run("env maps are merged", func(t *testing.T) { + base := &v1.Values{ + Pilot: &v1.PilotConfig{ + Env: map[string]string{ + "KEY1": "value1", + "KEY2": "value2", + }, + }, + } + overlay := &v1.Values{ + Pilot: &v1.PilotConfig{ + Env: map[string]string{ + "KEY2": "overlay-value2", + "KEY3": "value3", + }, + }, + } + + result := MergeValues(base, overlay) + + assert.Equal(t, "value1", result.Pilot.Env["KEY1"]) + assert.Equal(t, "overlay-value2", result.Pilot.Env["KEY2"]) + assert.Equal(t, "value3", result.Pilot.Env["KEY3"]) + }) + + t.Run("does not mutate base", func(t *testing.T) { + base := &v1.Values{ + Global: &v1.GlobalConfig{Hub: ptr.To("base-hub")}, + } + overlay := &v1.Values{ + Global: &v1.GlobalConfig{Hub: ptr.To("overlay-hub")}, + } + + _ = MergeValues(base, overlay) + + assert.Equal(t, "base-hub", *base.Global.Hub) + }) + + t.Run("lists are replaced not merged", func(t *testing.T) { + base := &v1.Values{ + Pilot: &v1.PilotConfig{ + ExtraContainerArgs: []string{"--foo", "--bar"}, + }, + } + overlay := &v1.Values{ + Pilot: &v1.PilotConfig{ + ExtraContainerArgs: []string{"--baz"}, + }, + } + + result := MergeValues(base, overlay) + + // Lists are replaced entirely, not merged (matches Helm semantics) + assert.Equal(t, []string{"--baz"}, result.Pilot.ExtraContainerArgs) + }) +} diff --git a/pkg/install/version.go b/pkg/install/version.go new file mode 100644 index 0000000000..22d11a61c5 --- /dev/null +++ b/pkg/install/version.go @@ -0,0 +1,80 @@ +// 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 install + +import ( + "fmt" + "io/fs" + "strings" + + "github.com/Masterminds/semver/v3" +) + +// DefaultVersion scans resourceFS for version directories and returns the +// highest stable (non-prerelease) semver version found. +// Returns an error if no stable version is found. +func DefaultVersion(resourceFS fs.FS) (string, error) { + entries, err := fs.ReadDir(resourceFS, ".") + if err != nil { + return "", fmt.Errorf("failed to read resource directory: %w", err) + } + + var best *semver.Version + var bestName string + for _, e := range entries { + if !e.IsDir() { + continue + } + name := e.Name() + v, err := semver.NewVersion(strings.TrimPrefix(name, "v")) + if err != nil { + continue // skip non-semver directories + } + if v.Prerelease() != "" { + continue // skip alpha, beta, rc, etc. + } + if best == nil || v.GreaterThan(best) { + best = v + bestName = name + } + } + if best == nil { + return "", fmt.Errorf("no stable version found in resource filesystem") + } + return bestName, nil +} + +// NormalizeVersion ensures the version string has a "v" prefix. +// Returns the input unchanged if it is empty or already starts with "v". +func NormalizeVersion(version string) string { + if version != "" && !strings.HasPrefix(version, "v") { + return "v" + version + } + return version +} + +// ValidateVersion checks that a version directory exists in the resource filesystem. +// Unlike istioversion.Resolve, this does not support aliases — only concrete +// version directory names (e.g. "v1.28.3"). +func ValidateVersion(resourceFS fs.FS, version string) error { + info, err := fs.Stat(resourceFS, version) + if err != nil { + return fmt.Errorf("version %q not found in resource filesystem", version) + } + if !info.IsDir() { + return fmt.Errorf("version %q is not a directory", version) + } + return nil +} diff --git a/pkg/install/version_test.go b/pkg/install/version_test.go new file mode 100644 index 0000000000..a25ef6e43b --- /dev/null +++ b/pkg/install/version_test.go @@ -0,0 +1,126 @@ +// 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 install + +import ( + "testing" + "testing/fstest" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestDefaultVersion(t *testing.T) { + t.Run("multiple stable versions returns highest", func(t *testing.T) { + fs := fstest.MapFS{ + "v1.27.5/charts/.keep": &fstest.MapFile{}, + "v1.28.0/charts/.keep": &fstest.MapFile{}, + "v1.28.3/charts/.keep": &fstest.MapFile{}, + } + v, err := DefaultVersion(fs) + require.NoError(t, err) + assert.Equal(t, "v1.28.3", v) + }) + + t.Run("pre-release versions skipped", func(t *testing.T) { + fs := fstest.MapFS{ + "v1.28.3/charts/.keep": &fstest.MapFile{}, + "v1.30-alpha.abc/charts/.keep": &fstest.MapFile{}, + "v1.29.0-beta.1/charts/.keep": &fstest.MapFile{}, + "v1.29.0-rc.2/charts/.keep": &fstest.MapFile{}, + } + v, err := DefaultVersion(fs) + require.NoError(t, err) + assert.Equal(t, "v1.28.3", v) + }) + + t.Run("no stable versions returns error", func(t *testing.T) { + fs := fstest.MapFS{ + "v1.30-alpha.abc/charts/.keep": &fstest.MapFile{}, + } + _, err := DefaultVersion(fs) + assert.Error(t, err) + assert.Contains(t, err.Error(), "no stable version") + }) + + t.Run("non-semver directories silently skipped", func(t *testing.T) { + fs := fstest.MapFS{ + "not-a-version/charts/.keep": &fstest.MapFile{}, + "v1.27.0/charts/.keep": &fstest.MapFile{}, + "resources.go": &fstest.MapFile{}, + } + v, err := DefaultVersion(fs) + require.NoError(t, err) + assert.Equal(t, "v1.27.0", v) + }) + + t.Run("single version works", func(t *testing.T) { + fs := fstest.MapFS{ + "v1.28.2/charts/.keep": &fstest.MapFile{}, + } + v, err := DefaultVersion(fs) + require.NoError(t, err) + assert.Equal(t, "v1.28.2", v) + }) + + t.Run("empty FS returns error", func(t *testing.T) { + fs := fstest.MapFS{} + _, err := DefaultVersion(fs) + assert.Error(t, err) + assert.Contains(t, err.Error(), "no stable version") + }) +} + +func TestNormalizeVersion(t *testing.T) { + tests := []struct { + input string + expected string + }{ + {"", ""}, + {"v1.24.0", "v1.24.0"}, + {"1.24.0", "v1.24.0"}, + {"v1.28.3-rc.1", "v1.28.3-rc.1"}, + {"1.28.3-rc.1", "v1.28.3-rc.1"}, + } + for _, tt := range tests { + t.Run(tt.input, func(t *testing.T) { + assert.Equal(t, tt.expected, NormalizeVersion(tt.input)) + }) + } +} + +func TestValidateVersion(t *testing.T) { + fs := fstest.MapFS{ + "v1.28.3/charts/.keep": &fstest.MapFile{}, + "resources.go": &fstest.MapFile{}, + } + + t.Run("existing directory passes", func(t *testing.T) { + err := ValidateVersion(fs, "v1.28.3") + assert.NoError(t, err) + }) + + t.Run("missing directory returns error", func(t *testing.T) { + err := ValidateVersion(fs, "v1.99.0") + assert.Error(t, err) + assert.Contains(t, err.Error(), "not found") + }) + + t.Run("file instead of directory returns error", func(t *testing.T) { + err := ValidateVersion(fs, "resources.go") + assert.Error(t, err) + assert.Contains(t, err.Error(), "not a directory") + }) +}