diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 28e04e25..740ed9ba 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -11,6 +11,7 @@ repos: name: shellcheck language: system entry: shellcheck + args: ['--external-sources'] types: [shell] - id: kuttl-assert-collectors name: Validate KUTTL assert collectors diff --git a/Makefile b/Makefile index 99593645..d48c81b3 100644 --- a/Makefile +++ b/Makefile @@ -162,9 +162,18 @@ build: manifests generate fmt vet ## Build manager binary. go build -o bin/manager cmd/main.go .PHONY: run +run: export ENABLE_WEBHOOKS ?= false run: manifests generate fmt vet ## Run a controller from your host. source ./scripts/env.sh && go run ./cmd/main.go $(ARGS) +.PHONY: run-with-webhook +run-with-webhook: build ## Run locally with a validating webhook for Linux CRC. + OC="$(OC)" bash hack/run_with_local_webhook.sh $(ARGS) + +.PHONY: webhook-cleanup +webhook-cleanup: ## Remove the admission webhook used for local development. + OC="$(OC)" bash hack/clean_local_webhook.sh + # If you wish to build the manager image targeting other platforms you can use the --platform flag. # (i.e. docker build --platform linux/arm64). However, you must enable docker buildKit for it. # More info: https://docs.docker.com/develop/develop-images/build_enhancements/ @@ -231,6 +240,7 @@ $(LOCALBIN): ## Tool Binaries KUBECTL ?= kubectl +OC ?= oc KIND ?= kind KUSTOMIZE ?= $(LOCALBIN)/kustomize CONTROLLER_GEN ?= $(LOCALBIN)/controller-gen diff --git a/PROJECT b/PROJECT index 086da6a9..dd0ee3e9 100644 --- a/PROJECT +++ b/PROJECT @@ -20,4 +20,7 @@ resources: kind: OpenStackLightspeed path: github.com/openstack-k8s-operators/lightspeed-operator/api/v1beta1 version: v1beta1 + webhooks: + validation: true + webhookVersion: v1 version: "3" diff --git a/README.md b/README.md index eac65c4e..d6212104 100644 --- a/README.md +++ b/README.md @@ -180,10 +180,19 @@ Note: `--zap-devel` enable verbose (development) logging locally. This will: 1. Install the CRDs into your cluster. -2. Run the operator locally, connected to your cluster. +2. Run the operator locally, connected to your cluster, with admission webhooks disabled. Use this for quick development and testing. +To develop with the validating webhook enabled, use a local Linux CRC cluster +without a deployed Lightspeed operator and run: + +```bash +make install run-with-webhook +``` + +Use `make webhook-cleanup` after an unclean shutdown. + *Attention*: In this mode RBACs are ignored, so when changing those please run the operator in the OpenShift cluster with an image. diff --git a/cmd/main.go b/cmd/main.go index 37cb09f3..e1a5bea1 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -51,6 +51,7 @@ import ( lightspeedv1beta1 "github.com/openstack-k8s-operators/lightspeed-operator/api/v1beta1" "github.com/openstack-k8s-operators/lightspeed-operator/internal/controller" + webhookv1beta1 "github.com/openstack-k8s-operators/lightspeed-operator/internal/webhook/v1beta1" telemetryv1 "github.com/openstack-k8s-operators/telemetry-operator/api/v1beta1" // +kubebuilder:scaffold:imports ) @@ -80,6 +81,7 @@ func main() { var metricsAddr string var metricsCertPath, metricsCertName, metricsCertKey string var webhookCertPath, webhookCertName, webhookCertKey string + var webhookPort int var enableLeaderElection bool var probeAddr string var secureMetrics bool @@ -96,6 +98,7 @@ func main() { flag.StringVar(&webhookCertPath, "webhook-cert-path", "", "The directory that contains the webhook certificate.") flag.StringVar(&webhookCertName, "webhook-cert-name", "tls.crt", "The name of the webhook certificate file.") flag.StringVar(&webhookCertKey, "webhook-cert-key", "tls.key", "The name of the webhook key file.") + flag.IntVar(&webhookPort, "webhook-port", 9443, "The port the webhook server listens on.") flag.StringVar(&metricsCertPath, "metrics-cert-path", "", "The directory that contains the metrics server certificate.") flag.StringVar(&metricsCertName, "metrics-cert-name", "tls.crt", "The name of the metrics server certificate file.") @@ -158,6 +161,7 @@ func main() { } webhookServer := webhook.NewServer(webhook.Options{ + Port: webhookPort, TLSOpts: webhookTLSOpts, }) @@ -269,6 +273,16 @@ func main() { setupLog.Error(err, "unable to create controller", "controller", "OpenStackLightspeed") os.Exit(1) } + if os.Getenv("ENABLE_WEBHOOKS") != "false" { + if err := webhookv1beta1.SetupOpenStackLightspeedWebhookWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create webhook", "webhook", "OpenStackLightspeed") + os.Exit(1) + } + if err := mgr.AddReadyzCheck("webhook", webhookServer.StartedChecker()); err != nil { + setupLog.Error(err, "unable to set up webhook ready check") + os.Exit(1) + } + } // +kubebuilder:scaffold:builder if metricsCertWatcher != nil { diff --git a/config/default/kustomization.yaml b/config/default/kustomization.yaml index de271c16..a8a94393 100644 --- a/config/default/kustomization.yaml +++ b/config/default/kustomization.yaml @@ -18,9 +18,7 @@ resources: - ../crd - ../rbac - ../manager -# [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in -# crd/kustomization.yaml -#- ../webhook +- ../webhook # [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'. 'WEBHOOK' components are required. #- ../certmanager # [PROMETHEUS] To enable prometheus monitor, uncomment all sections with 'PROMETHEUS'. @@ -50,11 +48,14 @@ patches: target: kind: Deployment -# [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in -# crd/kustomization.yaml -#- path: manager_webhook_patch.yaml -# target: -# kind: Deployment +- path: manager_webhook_patch.yaml +- patch: |- + apiVersion: admissionregistration.k8s.io/v1 + kind: ValidatingWebhookConfiguration + metadata: + name: validating-webhook-configuration + annotations: + service.beta.openshift.io/inject-cabundle: "true" # [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER' prefix. # Uncomment the following replacements to add the cert-manager CA injection annotations diff --git a/config/default/manager_webhook_patch.yaml b/config/default/manager_webhook_patch.yaml new file mode 100644 index 00000000..37757b78 --- /dev/null +++ b/config/default/manager_webhook_patch.yaml @@ -0,0 +1,23 @@ +# OpenShift service-ca supplies the certificate for direct deployments. +apiVersion: apps/v1 +kind: Deployment +metadata: + name: controller-manager + namespace: system +spec: + template: + spec: + containers: + - name: manager + ports: + - containerPort: 9443 + name: webhook-server + protocol: TCP + volumeMounts: + - name: webhook-certs + mountPath: /tmp/k8s-webhook-server/serving-certs + readOnly: true + volumes: + - name: webhook-certs + secret: + secretName: webhook-server-cert diff --git a/config/manifests/kustomization.yaml b/config/manifests/kustomization.yaml index 6644db4f..8e2dc7d6 100644 --- a/config/manifests/kustomization.yaml +++ b/config/manifests/kustomization.yaml @@ -6,23 +6,22 @@ resources: - ../samples - ../scorecard -# [WEBHOOK] To enable webhooks, uncomment all the sections with [WEBHOOK] prefix. -# Do NOT uncomment sections with prefix [CERTMANAGER], as OLM does not support cert-manager. -# These patches remove the unnecessary "cert" volume and its manager container volumeMount. -#patches: -#- target: -# group: apps -# version: v1 -# kind: Deployment -# name: controller-manager -# namespace: system -# patch: |- -# # Remove the manager container's "cert" volumeMount, since OLM will create and mount a set of certs. -# # Update the indices in this path if adding or removing containers/volumeMounts in the manager's Deployment. -# - op: remove - -# path: /spec/template/spec/containers/0/volumeMounts/0 -# # Remove the "cert" volume, since OLM will create and mount a set of certs. -# # Update the indices in this path if adding or removing volumes in the manager's Deployment. -# - op: remove -# path: /spec/template/spec/volumes/0 +# OLM supplies and mounts its own webhook certificate at the same path. +patches: +- patch: |- + apiVersion: apps/v1 + kind: Deployment + metadata: + name: controller-manager + namespace: system + spec: + template: + spec: + containers: + - name: manager + volumeMounts: + - mountPath: /tmp/k8s-webhook-server/serving-certs + $patch: delete + volumes: + - name: webhook-certs + $patch: delete diff --git a/config/webhook/kustomization.yaml b/config/webhook/kustomization.yaml new file mode 100644 index 00000000..9cf26134 --- /dev/null +++ b/config/webhook/kustomization.yaml @@ -0,0 +1,6 @@ +resources: +- manifests.yaml +- service.yaml + +configurations: +- kustomizeconfig.yaml diff --git a/config/webhook/kustomizeconfig.yaml b/config/webhook/kustomizeconfig.yaml new file mode 100644 index 00000000..edc37567 --- /dev/null +++ b/config/webhook/kustomizeconfig.yaml @@ -0,0 +1,13 @@ +nameReference: +- kind: Service + version: v1 + fieldSpecs: + - kind: ValidatingWebhookConfiguration + group: admissionregistration.k8s.io + path: webhooks/clientConfig/service/name + +namespace: +- kind: ValidatingWebhookConfiguration + group: admissionregistration.k8s.io + path: webhooks/clientConfig/service/namespace + create: true diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml new file mode 100644 index 00000000..4bb0d293 --- /dev/null +++ b/config/webhook/manifests.yaml @@ -0,0 +1,25 @@ +--- +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingWebhookConfiguration +metadata: + name: validating-webhook-configuration +webhooks: +- admissionReviewVersions: + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /validate-lightspeed-openstack-org-v1beta1-openstacklightspeed + failurePolicy: Fail + name: vopenstacklightspeed-v1beta1.kb.io + rules: + - apiGroups: + - lightspeed.openstack.org + apiVersions: + - v1beta1 + operations: + - CREATE + resources: + - openstacklightspeeds + sideEffects: None diff --git a/config/webhook/service.yaml b/config/webhook/service.yaml new file mode 100644 index 00000000..fba6b4dd --- /dev/null +++ b/config/webhook/service.yaml @@ -0,0 +1,15 @@ +apiVersion: v1 +kind: Service +metadata: + name: webhook-service + namespace: system + annotations: + service.beta.openshift.io/serving-cert-secret-name: webhook-server-cert +spec: + ports: + - port: 443 + protocol: TCP + targetPort: 9443 + selector: + control-plane: controller-manager + app.kubernetes.io/name: openstack-lightspeed-operator diff --git a/docs/install_guide.md b/docs/install_guide.md index ff8a7651..35ad056b 100644 --- a/docs/install_guide.md +++ b/docs/install_guide.md @@ -150,6 +150,12 @@ spec: This deploys the full stack: the AI engine (lightspeed-stack and OGX), PostgreSQL, OKP, and the console plugin. +> [!NOTE] +> A single OpenStackLightspeed instance is supported +> in the openstack-lightspeed namespace. The validating webhook rejects +> additional instances at creation time. To replace an instance, delete it +> and wait for its removal before creating another. + ## Verifying the deployment ```bash diff --git a/hack/clean_local_webhook.sh b/hack/clean_local_webhook.sh new file mode 100755 index 00000000..80af35f0 --- /dev/null +++ b/hack/clean_local_webhook.sh @@ -0,0 +1,6 @@ +#!/bin/bash +set -euo pipefail + +# Only remove the configuration created by run_with_local_webhook.sh. +"${OC:-oc}" delete validatingwebhookconfiguration \ + openstack-lightspeed-local-webhook --ignore-not-found diff --git a/hack/run_with_local_webhook.sh b/hack/run_with_local_webhook.sh new file mode 100755 index 00000000..ce42f820 --- /dev/null +++ b/hack/run_with_local_webhook.sh @@ -0,0 +1,98 @@ +#!/bin/bash +set -euo pipefail + +cd "$(dirname "${BASH_SOURCE[0]}")/.." +# shellcheck source=scripts/env.sh +source scripts/env.sh + +OC=${OC:-oc} +WEBHOOK_PORT=${WEBHOOK_PORT:-9443} +HEALTH_PORT=${HEALTH_PORT:-8081} +WEBHOOK_CERT_DIR=${WEBHOOK_CERT_DIR:-"$PWD/bin/local-webhook"} +webhook_name=openstack-lightspeed-local-webhook + +for command in "$OC" openssl jq curl ip; do + command -v "$command" >/dev/null || { echo "Required command not found: $command" >&2; exit 1; } +done + +# Use the host-side CRC bridge address reachable from the VM. +crc_host_ip=$(ip -o -4 addr show dev crc 2>/dev/null | awk '{split($4, address, "/"); print address[1]; exit}' || true) +if [[ -z "$crc_host_ip" ]]; then + echo "No IPv4 address found on the crc interface. This helper requires a local Linux CRC cluster." >&2 + exit 1 +fi +if [[ -z "$WATCH_NAMESPACE" || "$WATCH_NAMESPACE" == *,* ]]; then + echo "Local webhook development requires one WATCH_NAMESPACE." >&2 + exit 1 +fi + +# Avoid running alongside another Lightspeed webhook, including one managed by OLM. +existing=$("$OC" get validatingwebhookconfigurations -o json | jq -r ' + .items[] | select(.metadata.name == "openstack-lightspeed-local-webhook" or + any(.webhooks[]; .name == "vopenstacklightspeed-v1beta1.kb.io")) | .metadata.name') +if [[ -n "$existing" ]]; then + echo "A Lightspeed webhook is already installed: $existing" >&2 + echo "Use make webhook-cleanup for a stale local webhook; uninstall the deployed operator before running locally." >&2 + exit 1 +fi + +umask 077 +mkdir -p "$WEBHOOK_CERT_DIR" +openssl req -newkey rsa:2048 -nodes -x509 -days 30 \ + -subj "/CN=lightspeed-local-webhook" -addext "subjectAltName=IP:$crc_host_ip" \ + -keyout "$WEBHOOK_CERT_DIR/tls.key" -out "$WEBHOOK_CERT_DIR/tls.crt" + +# The API server needs only the public certificate, never the private key. +ca_bundle=$(openssl base64 -A -in "$WEBHOOK_CERT_DIR/tls.crt") +"$OC" create --dry-run=client --validate=false -f config/webhook/manifests.yaml -o json | \ + jq --arg name "$webhook_name" --arg url "https://$crc_host_ip:$WEBHOOK_PORT" \ + --arg ca "$ca_bundle" --arg namespace "$WATCH_NAMESPACE" ' + .metadata.name = $name | + .webhooks |= map( + .clientConfig = {url: ($url + .clientConfig.service.path), caBundle: $ca} | + .namespaceSelector = {matchLabels: {"kubernetes.io/metadata.name": $namespace}} + )' > "$WEBHOOK_CERT_DIR/webhook.json" + +manager_pid="" +registered=false +# Invoked indirectly by the EXIT trap. +# shellcheck disable=SC2317,SC2329 +cleanup() { + local status=$? + trap - EXIT INT TERM + if [[ "$registered" == true ]]; then + bash hack/clean_local_webhook.sh || status=1 + fi + if [[ -n "$manager_pid" ]]; then + kill "$manager_pid" 2>/dev/null || true + wait "$manager_pid" 2>/dev/null || true + fi + exit "$status" +} +trap cleanup EXIT +trap 'exit 130' INT +trap 'exit 143' TERM + +export ENABLE_WEBHOOKS=true OC +./bin/manager "$@" --webhook-port="$WEBHOOK_PORT" --webhook-cert-path="$WEBHOOK_CERT_DIR" \ + --health-probe-bind-address="127.0.0.1:$HEALTH_PORT" & +manager_pid=$! + +# Register only after the local server is ready to answer admission requests. +for ((attempt=0; attempt<60; attempt++)); do + if ! kill -0 "$manager_pid" 2>/dev/null; then + wait "$manager_pid" + exit 1 + fi + if curl --noproxy '*' --fail --silent --max-time 1 "http://127.0.0.1:$HEALTH_PORT/readyz/webhook" >/dev/null; then + registered=true + "$OC" apply -f "$WEBHOOK_CERT_DIR/webhook.json" + echo "Local webhook registered at https://$crc_host_ip:$WEBHOOK_PORT for namespace $WATCH_NAMESPACE." + echo "The API server must be able to reach this address and port. Press Ctrl+C to stop and clean up." + wait "$manager_pid" + exit 0 + fi + sleep 1 +done +echo "Timed out waiting for the local webhook server to become ready." >&2 +exit 1 diff --git a/internal/controller/openstacklightspeed_controller.go b/internal/controller/openstacklightspeed_controller.go index 31fb01cf..143f553e 100644 --- a/internal/controller/openstacklightspeed_controller.go +++ b/internal/controller/openstacklightspeed_controller.go @@ -113,15 +113,15 @@ func (r *OpenStackLightspeedReconciler) Reconcile(ctx context.Context, req ctrl. Log := r.GetLogger(ctx) Log.Info("OpenStackLightspeed Reconciling") - instance := &apiv1beta1.OpenStackLightspeed{} - err := r.Get(ctx, req.NamespacedName, instance) + instance, err := r.GetOpenStackLightspeed(ctx, req) if err != nil { - if k8s_errors.IsNotFound(err) { - Log.Info("OpenStackLightspeed CR not found") - return ctrl.Result{}, nil - } + Log.Error(err, "Cannot reconcile OpenStackLightspeed") return ctrl.Result{}, err } + if instance == nil { + Log.Info("No OpenStackLightspeed CR matches the reconcile request", "name", req.Name, "namespace", req.Namespace) + return ctrl.Result{}, nil + } helper, err := common_helper.NewHelper( instance, @@ -402,6 +402,23 @@ func (r *OpenStackLightspeedReconciler) reconcileStatus( return ctrl.Result{}, nil } +// GetOpenStackLightspeed returns the instance matching the request, or nil if none matches. +// It returns an error if listing fails or more than one instance exists in the namespace. +func (r *OpenStackLightspeedReconciler) GetOpenStackLightspeed(ctx context.Context, req ctrl.Request) (*apiv1beta1.OpenStackLightspeed, error) { + instances := &apiv1beta1.OpenStackLightspeedList{} + if err := r.List(ctx, instances, client.InNamespace(req.Namespace)); err != nil { + return nil, err + } + if len(instances.Items) > 1 { + return nil, fmt.Errorf("only one OpenStackLightspeed instance per namespace is allowed; found %d in namespace %q", + len(instances.Items), req.Namespace) + } + if len(instances.Items) == 0 || instances.Items[0].Name != req.Name { + return nil, nil + } + return &instances.Items[0], nil +} + // SetupWithManager sets up the controller with the Manager. func (r *OpenStackLightspeedReconciler) SetupWithManager(mgr ctrl.Manager) error { if err := initClusterClient(mgr); err != nil { diff --git a/internal/controller/openstacklightspeed_controller_test.go b/internal/controller/openstacklightspeed_controller_test.go index 9d2fc93b..d52d2076 100644 --- a/internal/controller/openstacklightspeed_controller_test.go +++ b/internal/controller/openstacklightspeed_controller_test.go @@ -18,12 +18,18 @@ package controller import ( "context" + "strings" "sync/atomic" + "testing" "github.com/onsi/ginkgo/v2" "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" "sigs.k8s.io/controller-runtime/pkg/reconcile" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -98,3 +104,77 @@ var _ = ginkgo.Describe("OpenStackLightspeed Controller", func() { }) }) }) + +func TestReconcileInstanceCount(t *testing.T) { + scheme := runtime.NewScheme() + if err := apiv1beta1.AddToScheme(scheme); err != nil { + t.Fatalf("failed to add apiv1beta1 to scheme: %v", err) + } + first := &apiv1beta1.OpenStackLightspeed{ + ObjectMeta: metav1.ObjectMeta{Name: "first", Namespace: "ns"}, + } + second := first.DeepCopy() + second.Name = "second" + otherNamespace := first.DeepCopy() + otherNamespace.Namespace = "other-ns" + listErr := errors.NewServiceUnavailable("list failed") + + for _, tt := range []struct { + name string + instances []*apiv1beta1.OpenStackLightspeed + request string + listErr error + wantError string + reconcile bool + }{ + {name: "no instances", request: first.Name}, + {name: "single instance", instances: []*apiv1beta1.OpenStackLightspeed{first}, request: first.Name, reconcile: true}, + {name: "other namespace is ignored", instances: []*apiv1beta1.OpenStackLightspeed{first, otherNamespace}, request: first.Name, reconcile: true}, + {name: "deleted request is ignored", instances: []*apiv1beta1.OpenStackLightspeed{first}, request: "deleted"}, + {name: "multiple instances block first", instances: []*apiv1beta1.OpenStackLightspeed{first, second}, request: first.Name, wantError: "only one OpenStackLightspeed instance per namespace is allowed"}, + {name: "multiple instances block second", instances: []*apiv1beta1.OpenStackLightspeed{first, second}, request: second.Name, wantError: "only one OpenStackLightspeed instance per namespace is allowed"}, + {name: "list error is returned", instances: []*apiv1beta1.OpenStackLightspeed{first}, request: first.Name, listErr: listErr, wantError: listErr.Error()}, + } { + t.Run(tt.name, func(t *testing.T) { + builder := fake.NewClientBuilder().WithScheme(scheme). + WithStatusSubresource(&apiv1beta1.OpenStackLightspeed{}) + for _, instance := range tt.instances { + builder.WithObjects(instance.DeepCopy()) + } + fakeClient := builder.WithInterceptorFuncs(interceptor.Funcs{ + List: func(ctx context.Context, c client.WithWatch, list client.ObjectList, opts ...client.ListOption) error { + if tt.listErr != nil { + return tt.listErr + } + return c.List(ctx, list, opts...) + }, + }).Build() + r := &OpenStackLightspeedReconciler{Client: fakeClient, Scheme: scheme} + ctx := context.Background() + result, err := r.Reconcile(ctx, reconcile.Request{ + NamespacedName: types.NamespacedName{Name: tt.request, Namespace: first.Namespace}, + }) + if tt.wantError == "" { + if err != nil { + t.Fatalf("unexpected reconcile error: %v", err) + } + } else if err == nil || !strings.Contains(err.Error(), tt.wantError) { + t.Fatalf("expected error containing %q, got %v", tt.wantError, err) + } + if result != (reconcile.Result{}) { + t.Fatalf("unexpected requeue: %+v", result) + } + for _, instance := range tt.instances { + actual := &apiv1beta1.OpenStackLightspeed{} + if err := fakeClient.Get(ctx, client.ObjectKeyFromObject(instance), actual); err != nil { + t.Fatal(err) + } + wantReconciled := tt.reconcile && instance.Namespace == first.Namespace && instance.Name == tt.request + if (len(actual.Finalizers) > 0) != wantReconciled || (len(actual.Status.Conditions) > 0) != wantReconciled { + t.Errorf("unexpected reconciliation of %s/%s: finalizers=%v, conditions=%v", + actual.Namespace, actual.Name, actual.Finalizers, actual.Status.Conditions) + } + } + }) + } +} diff --git a/internal/webhook/v1beta1/openstacklightspeed_webhook.go b/internal/webhook/v1beta1/openstacklightspeed_webhook.go new file mode 100644 index 00000000..a6033fff --- /dev/null +++ b/internal/webhook/v1beta1/openstacklightspeed_webhook.go @@ -0,0 +1,89 @@ +/* +Copyright 2026. + +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 v1beta1 implements admission validation for OpenStackLightspeed. +package v1beta1 + +import ( + "context" + "fmt" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/validation/field" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + lightspeedv1beta1 "github.com/openstack-k8s-operators/lightspeed-operator/api/v1beta1" +) + +// SetupOpenStackLightspeedWebhookWithManager registers the validating webhook. +func SetupOpenStackLightspeedWebhookWithManager(mgr ctrl.Manager) error { + return ctrl.NewWebhookManagedBy(mgr). + For(&lightspeedv1beta1.OpenStackLightspeed{}). + WithValidator(&OpenStackLightspeedCustomValidator{Reader: mgr.GetAPIReader()}). + Complete() +} + +// +kubebuilder:webhook:path=/validate-lightspeed-openstack-org-v1beta1-openstacklightspeed,mutating=false,failurePolicy=fail,sideEffects=None,groups=lightspeed.openstack.org,resources=openstacklightspeeds,verbs=create,versions=v1beta1,name=vopenstacklightspeed-v1beta1.kb.io,admissionReviewVersions=v1 + +// OpenStackLightspeedCustomValidator rejects additional instances in a namespace. +type OpenStackLightspeedCustomValidator struct { + Reader client.Reader +} + +var _ webhook.CustomValidator = &OpenStackLightspeedCustomValidator{} + +// ValidateCreate allows creation only when the namespace has no existing instance. +func (v *OpenStackLightspeedCustomValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { + instance, ok := obj.(*lightspeedv1beta1.OpenStackLightspeed) + if !ok { + return nil, fmt.Errorf("expected an OpenStackLightspeed object but got %T", obj) + } + + // Read directly from the API server to avoid cache lag. Concurrent creates + // can still pass this check, so the controller retains its duplicate guard. + var instances lightspeedv1beta1.OpenStackLightspeedList + if err := v.Reader.List(ctx, &instances, client.InNamespace(instance.Namespace)); err != nil { + return nil, apierrors.NewInternalError(fmt.Errorf("failed to check existing OpenStackLightspeed instances: %w", err)) + } + if len(instances.Items) > 0 { + errs := field.ErrorList{ + field.Forbidden( + field.NewPath("metadata", "namespace"), + fmt.Sprintf("only one OpenStackLightspeed instance per namespace is allowed; %q already exists", instances.Items[0].Name), + ), + } + return nil, apierrors.NewInvalid( + lightspeedv1beta1.GroupVersion.WithKind("OpenStackLightspeed").GroupKind(), + instance.Name, + errs, + ) + } + return nil, nil +} + +// ValidateUpdate allows updates so existing instances can still be managed. +func (v *OpenStackLightspeedCustomValidator) ValidateUpdate(_ context.Context, _, _ runtime.Object) (admission.Warnings, error) { + return nil, nil +} + +// ValidateDelete allows deletion so existing instances can always be removed. +func (v *OpenStackLightspeedCustomValidator) ValidateDelete(_ context.Context, _ runtime.Object) (admission.Warnings, error) { + return nil, nil +} diff --git a/internal/webhook/v1beta1/openstacklightspeed_webhook_test.go b/internal/webhook/v1beta1/openstacklightspeed_webhook_test.go new file mode 100644 index 00000000..12dd3616 --- /dev/null +++ b/internal/webhook/v1beta1/openstacklightspeed_webhook_test.go @@ -0,0 +1,110 @@ +/* +Copyright 2026. + +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 v1beta1 + +import ( + "context" + "errors" + "strings" + "testing" + + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + 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" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + + lightspeedv1beta1 "github.com/openstack-k8s-operators/lightspeed-operator/api/v1beta1" +) + +func TestValidateCreate(t *testing.T) { + scheme := runtime.NewScheme() + if err := lightspeedv1beta1.AddToScheme(scheme); err != nil { + t.Fatal(err) + } + instance := &lightspeedv1beta1.OpenStackLightspeed{ + ObjectMeta: metav1.ObjectMeta{Name: "new", Namespace: "lightspeed"}, + } + existing := &lightspeedv1beta1.OpenStackLightspeed{ + ObjectMeta: metav1.ObjectMeta{Name: "existing", Namespace: instance.Namespace}, + } + otherNamespace := existing.DeepCopy() + otherNamespace.Namespace = "other" + terminating := existing.DeepCopy() + now := metav1.Now() + terminating.DeletionTimestamp = &now + terminating.Finalizers = []string{"lightspeed.openstack.org/finalizer"} + + for _, tt := range []struct { + name string + objects []client.Object + obj runtime.Object + listErr error + checkErr func(error) bool + message string + causeField string + }{ + {name: "first instance", obj: instance}, + {name: "different namespace", obj: instance, objects: []client.Object{otherNamespace}}, + {name: "duplicate", obj: instance, objects: []client.Object{existing}, checkErr: apierrors.IsInvalid, message: `metadata.namespace: Forbidden: only one OpenStackLightspeed instance per namespace is allowed; "existing" already exists`, causeField: "metadata.namespace"}, + {name: "terminating instance still blocks creation", obj: instance, objects: []client.Object{terminating}, checkErr: apierrors.IsInvalid, causeField: "metadata.namespace"}, + {name: "list failure denies creation", obj: instance, listErr: errors.New("API unavailable"), checkErr: apierrors.IsInternalError, message: "API unavailable"}, + {name: "unexpected object", obj: &corev1.Secret{}, checkErr: func(err error) bool { return err != nil }, message: "expected an OpenStackLightspeed object"}, + } { + t.Run(tt.name, func(t *testing.T) { + reader := fake.NewClientBuilder().WithScheme(scheme).WithObjects(tt.objects...).WithInterceptorFuncs(interceptor.Funcs{ + List: func(ctx context.Context, c client.WithWatch, list client.ObjectList, opts ...client.ListOption) error { + if tt.listErr != nil { + return tt.listErr + } + return c.List(ctx, list, opts...) + }, + }).Build() + validator := &OpenStackLightspeedCustomValidator{Reader: reader} + _, err := validator.ValidateCreate(context.Background(), tt.obj) + if tt.checkErr == nil { + if err != nil { + t.Fatalf("expected creation to be allowed: %v", err) + } + } else if !tt.checkErr(err) { + t.Fatalf("unexpected validation error: %v", err) + } + if tt.message != "" && (err == nil || !strings.Contains(err.Error(), tt.message)) { + t.Fatalf("expected error containing %q, got %v", tt.message, err) + } + if tt.causeField != "" { + cause, ok := apierrors.StatusCause(err, metav1.CauseTypeForbidden) + if !ok || cause.Field != tt.causeField { + t.Fatalf("expected forbidden field %q, got %+v", tt.causeField, cause) + } + } + }) + } +} + +func TestUpdatesAndDeletesDoNotRequireLookup(t *testing.T) { + validator := &OpenStackLightspeedCustomValidator{} + instance := &lightspeedv1beta1.OpenStackLightspeed{} + if _, err := validator.ValidateUpdate(context.Background(), instance, instance); err != nil { + t.Fatalf("expected update to be allowed: %v", err) + } + if _, err := validator.ValidateDelete(context.Background(), instance); err != nil { + t.Fatalf("expected deletion to be allowed: %v", err) + } +} diff --git a/test/kuttl/tests/duplicate-openstack-lightspeed-instance/00-create-primary-instance.yaml b/test/kuttl/tests/duplicate-openstack-lightspeed-instance/00-create-primary-instance.yaml new file mode 100644 index 00000000..e2597519 --- /dev/null +++ b/test/kuttl/tests/duplicate-openstack-lightspeed-instance/00-create-primary-instance.yaml @@ -0,0 +1,14 @@ +--- +apiVersion: lightspeed.openstack.org/v1beta1 +kind: OpenStackLightspeed +metadata: + name: openstack-lightspeed + namespace: openstack-lightspeed +spec: + defaultModel: default-model + models: + - name: default-model + llmEndpoint: http://mock-llm-api-server-pod:8000/v1 + llmEndpointType: openai + llmCredentials: openstack-lightspeed-apitoken + modelName: ibm-granite/granite-3.1-8b-instruct diff --git a/test/kuttl/tests/duplicate-openstack-lightspeed-instance/01-create-duplicate-instance.yaml b/test/kuttl/tests/duplicate-openstack-lightspeed-instance/01-create-duplicate-instance.yaml new file mode 100644 index 00000000..ab0bda8c --- /dev/null +++ b/test/kuttl/tests/duplicate-openstack-lightspeed-instance/01-create-duplicate-instance.yaml @@ -0,0 +1,30 @@ +--- +apiVersion: kuttl.dev/v1beta1 +kind: TestStep +commands: + - script: | + #!/bin/bash + set -euo pipefail + + if OUTPUT=$(oc create -f - 2>&1 <<'EOF' + apiVersion: lightspeed.openstack.org/v1beta1 + kind: OpenStackLightspeed + metadata: + name: openstack-lightspeed-duplicate + namespace: openstack-lightspeed + spec: + defaultModel: default-model + models: + - name: default-model + llmEndpoint: http://mock-llm-api-server-pod:8000/v1 + llmEndpointType: openai + llmCredentials: openstack-lightspeed-apitoken + modelName: ibm-granite/granite-3.1-8b-instruct + EOF + ); then + echo "ERROR: duplicate instance was accepted" + exit 1 + fi + + echo "$OUTPUT" + [[ "$OUTPUT" == *"is invalid: metadata.namespace: Forbidden: only one OpenStackLightspeed instance per namespace is allowed"* ]] diff --git a/test/kuttl/tests/duplicate-openstack-lightspeed-instance/02-cleanup-instances.yaml b/test/kuttl/tests/duplicate-openstack-lightspeed-instance/02-cleanup-instances.yaml new file mode 100644 index 00000000..7f187621 --- /dev/null +++ b/test/kuttl/tests/duplicate-openstack-lightspeed-instance/02-cleanup-instances.yaml @@ -0,0 +1,12 @@ +--- +apiVersion: kuttl.dev/v1beta1 +kind: TestStep +delete: + - apiVersion: lightspeed.openstack.org/v1beta1 + kind: OpenStackLightspeed + name: openstack-lightspeed + namespace: openstack-lightspeed + - apiVersion: v1 + kind: PersistentVolumeClaim + name: openstack-lightspeed-database + namespace: openstack-lightspeed diff --git a/test/kuttl/tests/duplicate-openstack-lightspeed-instance/03-errors-instances.yaml b/test/kuttl/tests/duplicate-openstack-lightspeed-instance/03-errors-instances.yaml new file mode 100644 index 00000000..503c1f32 --- /dev/null +++ b/test/kuttl/tests/duplicate-openstack-lightspeed-instance/03-errors-instances.yaml @@ -0,0 +1,12 @@ +--- +apiVersion: lightspeed.openstack.org/v1beta1 +kind: OpenStackLightspeed +metadata: + name: openstack-lightspeed + namespace: openstack-lightspeed +--- +apiVersion: lightspeed.openstack.org/v1beta1 +kind: OpenStackLightspeed +metadata: + name: openstack-lightspeed-duplicate + namespace: openstack-lightspeed diff --git a/test/kuttl/tests/dynamic-crd-watch-recovery/07-assert-openstack-lightspeed-instance.yaml b/test/kuttl/tests/dynamic-crd-watch-recovery/07-assert-openstack-lightspeed-instance.yaml index dc6bd12b..8e6797a0 100644 --- a/test/kuttl/tests/dynamic-crd-watch-recovery/07-assert-openstack-lightspeed-instance.yaml +++ b/test/kuttl/tests/dynamic-crd-watch-recovery/07-assert-openstack-lightspeed-instance.yaml @@ -4,7 +4,7 @@ kind: TestAssert collectors: - type: command command: ../../common/collectors/collect-workload-diagnostics.sh -timeout: 360 +timeout: 1380 # Script: Go omitempty on bool omits false from serialization, kuttl YAML assert fails with # "key is missing from map"; also parses MCP config string content within data fields commands: