From 664cf8b9c6dd3cb598dda3fecac792e4b2d89634 Mon Sep 17 00:00:00 2001 From: jzywieck Date: Fri, 17 Apr 2026 14:16:25 +0200 Subject: [PATCH 1/9] feat: move changes to internal/webook/apps Signed-off-by: jzywieck --- Makefile | 2 +- cmd/main.go | 9 + config/app-webhook/kustomization.yaml | 6 + config/app-webhook/kustomizeconfig.yaml | 18 ++ config/app-webhook/manifests.yaml | 26 ++ config/app-webhook/service.yaml | 12 + config/default/kustomization-cluster.yaml | 65 ++-- config/default/kustomization-namespace.yaml | 65 ++-- config/default/kustomization.yaml | 65 ++-- config/default/manager_webhook_patch.yaml | 20 ++ config/default/webhookcainjection_patch.yaml | 14 + internal/webhook/apps/v1alpha1/app_webhook.go | 220 ++++++++++++++ .../webhook/apps/v1alpha1/app_webhook_test.go | 279 ++++++++++++++++++ 13 files changed, 707 insertions(+), 94 deletions(-) create mode 100644 config/app-webhook/kustomization.yaml create mode 100644 config/app-webhook/kustomizeconfig.yaml create mode 100644 config/app-webhook/manifests.yaml create mode 100644 config/app-webhook/service.yaml create mode 100644 config/default/manager_webhook_patch.yaml create mode 100644 config/default/webhookcainjection_patch.yaml create mode 100644 internal/webhook/apps/v1alpha1/app_webhook.go create mode 100644 internal/webhook/apps/v1alpha1/app_webhook_test.go diff --git a/Makefile b/Makefile index 8a5d8c307..3fb11c158 100644 --- a/Makefile +++ b/Makefile @@ -121,7 +121,7 @@ help: ## Display this help. ##@ Development manifests: controller-gen ## Generate WebhookConfiguration, ClusterRole and CustomResourceDefinition objects. - $(CONTROLLER_GEN) rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases + $(CONTROLLER_GEN) rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases output:webhook:artifacts:config=config/app-webhook rm config/crd/bases/_.yaml generate: controller-gen ## Generate code containing DeepCopy, DeepCopyInto, and DeepCopyObject method implementations. diff --git a/cmd/main.go b/cmd/main.go index 468037185..d3caef145 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -50,11 +50,13 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log/zap" "sigs.k8s.io/controller-runtime/pkg/manager" metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" + ctrlwebhook "sigs.k8s.io/controller-runtime/pkg/webhook" appsv1alpha1 "github.com/splunk/splunk-operator/api/apps/v1alpha1" enterpriseApiV3 "github.com/splunk/splunk-operator/api/enterprise/v3" enterpriseApi "github.com/splunk/splunk-operator/api/enterprise/v4" appscontroller "github.com/splunk/splunk-operator/internal/controller/apps" + webhookappsv1alpha1 "github.com/splunk/splunk-operator/internal/webhook/apps/v1alpha1" //+kubebuilder:scaffold:imports //extapi "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" ) @@ -64,6 +66,8 @@ var ( setupLog = ctrl.Log.WithName("setup") ) +const appWebhookPort = 9444 + func init() { utilruntime.Must(clientgoscheme.AddToScheme(scheme)) utilruntime.Must(enterpriseApi.AddToScheme(scheme)) @@ -189,6 +193,7 @@ func main() { LeaderElectionID: "270bec8c.splunk.com", LeaseDuration: &leaseDuration, RenewDeadline: &renewDeadline, + WebhookServer: ctrlwebhook.NewServer(ctrlwebhook.Options{Port: appWebhookPort}), } // Apply namespace-specific configuration @@ -333,6 +338,10 @@ func main() { setupLog.Error(err, "unable to create controller", "controller", "App") os.Exit(1) } + if err := webhookappsv1alpha1.SetupWebhookWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create webhook", "webhook", "App") + os.Exit(1) + } //+kubebuilder:scaffold:builder // Register certificate watchers with the manager diff --git a/config/app-webhook/kustomization.yaml b/config/app-webhook/kustomization.yaml new file mode 100644 index 000000000..9cf26134e --- /dev/null +++ b/config/app-webhook/kustomization.yaml @@ -0,0 +1,6 @@ +resources: +- manifests.yaml +- service.yaml + +configurations: +- kustomizeconfig.yaml diff --git a/config/app-webhook/kustomizeconfig.yaml b/config/app-webhook/kustomizeconfig.yaml new file mode 100644 index 000000000..e809f7820 --- /dev/null +++ b/config/app-webhook/kustomizeconfig.yaml @@ -0,0 +1,18 @@ +# the following config is for teaching kustomize where to look at when substituting vars. +# It requires kustomize v2.1.0 or newer to work properly. +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 + +varReference: +- path: metadata/annotations diff --git a/config/app-webhook/manifests.yaml b/config/app-webhook/manifests.yaml new file mode 100644 index 000000000..8968e23c6 --- /dev/null +++ b/config/app-webhook/manifests.yaml @@ -0,0 +1,26 @@ +--- +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingWebhookConfiguration +metadata: + name: validating-webhook-configuration +webhooks: +- admissionReviewVersions: + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /validate-apps-splunk-com-v1alpha1-app + failurePolicy: Fail + name: vapp-v1alpha1.kb.io + rules: + - apiGroups: + - apps.splunk.com + apiVersions: + - v1alpha1 + operations: + - CREATE + - UPDATE + resources: + - apps + sideEffects: None diff --git a/config/app-webhook/service.yaml b/config/app-webhook/service.yaml new file mode 100644 index 000000000..86a71867b --- /dev/null +++ b/config/app-webhook/service.yaml @@ -0,0 +1,12 @@ +apiVersion: v1 +kind: Service +metadata: + name: webhook-service + namespace: system +spec: + ports: + - port: 443 + protocol: TCP + targetPort: 9444 + selector: + control-plane: controller-manager diff --git a/config/default/kustomization-cluster.yaml b/config/default/kustomization-cluster.yaml index 15c98e24a..96718b872 100644 --- a/config/default/kustomization-cluster.yaml +++ b/config/default/kustomization-cluster.yaml @@ -20,9 +20,9 @@ bases: - ../manager # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in # crd/kustomization.yaml -#- ../webhook +- ../app-webhook # [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'. 'WEBHOOK' components are required. -#- ../certmanager +- ../certmanager # [PROMETHEUS] To enable prometheus monitor, uncomment all sections with 'PROMETHEUS'. #- ../prometheus # [METRICS] Expose the controller manager metrics service. @@ -40,37 +40,36 @@ patchesStrategicMerge: # [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'. # Uncomment 'CERTMANAGER' sections in crd/kustomization.yaml to enable the CA injection in the admission webhooks. # 'CERTMANAGER' needs to be enabled to use ca injection -#- webhookcainjection_patch.yaml +- webhookcainjection_patch.yaml # the following config is for teaching kustomize how to do var substitution vars: -# [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER' prefix. -#- name: CERTIFICATE_NAMESPACE # namespace of the certificate CR -# objref: -# kind: Certificate -# group: cert-manager.io -# version: v1 -# name: serving-cert # this name should match the one in certificate.yaml -# fieldref: -# fieldpath: metadata.namespace -#- name: CERTIFICATE_NAME -# objref: -# kind: Certificate -# group: cert-manager.io -# version: v1 -# name: serving-cert # this name should match the one in certificate.yaml -#- name: SERVICE_NAMESPACE # namespace of the service -# objref: -# kind: Service -# version: v1 -# name: webhook-service -# fieldref: -# fieldpath: metadata.namespace -#- name: SERVICE_NAME -# objref: -# kind: Service -# version: v1 -# name: webhook-service +- name: CERTIFICATE_NAMESPACE + objref: + kind: Certificate + group: cert-manager.io + version: v1 + name: serving-cert + fieldref: + fieldpath: metadata.namespace +- name: CERTIFICATE_NAME + objref: + kind: Certificate + group: cert-manager.io + version: v1 + name: serving-cert +- name: SERVICE_NAMESPACE + objref: + kind: Service + version: v1 + name: webhook-service + fieldref: + fieldpath: metadata.namespace +- name: SERVICE_NAME + objref: + kind: Service + version: v1 + name: webhook-service #patches: #- target: @@ -111,6 +110,10 @@ vars: # currently patch is set to change deployment environment variables patches: +- path: manager_webhook_patch.yaml + target: + kind: Deployment + name: controller-manager - target: kind: Deployment name: controller-manager @@ -134,4 +137,4 @@ patches: # More info: https://book.kubebuilder.io/reference/metrics - path: manager_metrics_patch.yaml target: - kind: Deployment \ No newline at end of file + kind: Deployment diff --git a/config/default/kustomization-namespace.yaml b/config/default/kustomization-namespace.yaml index 1053b4785..0774b6c30 100644 --- a/config/default/kustomization-namespace.yaml +++ b/config/default/kustomization-namespace.yaml @@ -20,9 +20,9 @@ bases: - ../manager # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in # crd/kustomization.yaml -#- ../webhook +- ../app-webhook # [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'. 'WEBHOOK' components are required. -#- ../certmanager +- ../certmanager # [PROMETHEUS] To enable prometheus monitor, uncomment all sections with 'PROMETHEUS'. #- ../prometheus # [METRICS] Expose the controller manager metrics service. @@ -40,37 +40,36 @@ patchesStrategicMerge: # [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'. # Uncomment 'CERTMANAGER' sections in crd/kustomization.yaml to enable the CA injection in the admission webhooks. # 'CERTMANAGER' needs to be enabled to use ca injection -#- webhookcainjection_patch.yaml +- webhookcainjection_patch.yaml # the following config is for teaching kustomize how to do var substitution vars: -# [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER' prefix. -#- name: CERTIFICATE_NAMESPACE # namespace of the certificate CR -# objref: -# kind: Certificate -# group: cert-manager.io -# version: v1 -# name: serving-cert # this name should match the one in certificate.yaml -# fieldref: -# fieldpath: metadata.namespace -#- name: CERTIFICATE_NAME -# objref: -# kind: Certificate -# group: cert-manager.io -# version: v1 -# name: serving-cert # this name should match the one in certificate.yaml -#- name: SERVICE_NAMESPACE # namespace of the service -# objref: -# kind: Service -# version: v1 -# name: webhook-service -# fieldref: -# fieldpath: metadata.namespace -#- name: SERVICE_NAME -# objref: -# kind: Service -# version: v1 -# name: webhook-service +- name: CERTIFICATE_NAMESPACE + objref: + kind: Certificate + group: cert-manager.io + version: v1 + name: serving-cert + fieldref: + fieldpath: metadata.namespace +- name: CERTIFICATE_NAME + objref: + kind: Certificate + group: cert-manager.io + version: v1 + name: serving-cert +- name: SERVICE_NAMESPACE + objref: + kind: Service + version: v1 + name: webhook-service + fieldref: + fieldpath: metadata.namespace +- name: SERVICE_NAME + objref: + kind: Service + version: v1 + name: webhook-service #patches: #- target: @@ -111,6 +110,10 @@ vars: # currently patch is set to change deployment environment variables patches: +- path: manager_webhook_patch.yaml + target: + kind: Deployment + name: controller-manager - target: kind: Deployment name: controller-manager @@ -136,4 +139,4 @@ patches: # More info: https://book.kubebuilder.io/reference/metrics - path: manager_metrics_patch.yaml target: - kind: Deployment \ No newline at end of file + kind: Deployment diff --git a/config/default/kustomization.yaml b/config/default/kustomization.yaml index 15c98e24a..96718b872 100644 --- a/config/default/kustomization.yaml +++ b/config/default/kustomization.yaml @@ -20,9 +20,9 @@ bases: - ../manager # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in # crd/kustomization.yaml -#- ../webhook +- ../app-webhook # [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'. 'WEBHOOK' components are required. -#- ../certmanager +- ../certmanager # [PROMETHEUS] To enable prometheus monitor, uncomment all sections with 'PROMETHEUS'. #- ../prometheus # [METRICS] Expose the controller manager metrics service. @@ -40,37 +40,36 @@ patchesStrategicMerge: # [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'. # Uncomment 'CERTMANAGER' sections in crd/kustomization.yaml to enable the CA injection in the admission webhooks. # 'CERTMANAGER' needs to be enabled to use ca injection -#- webhookcainjection_patch.yaml +- webhookcainjection_patch.yaml # the following config is for teaching kustomize how to do var substitution vars: -# [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER' prefix. -#- name: CERTIFICATE_NAMESPACE # namespace of the certificate CR -# objref: -# kind: Certificate -# group: cert-manager.io -# version: v1 -# name: serving-cert # this name should match the one in certificate.yaml -# fieldref: -# fieldpath: metadata.namespace -#- name: CERTIFICATE_NAME -# objref: -# kind: Certificate -# group: cert-manager.io -# version: v1 -# name: serving-cert # this name should match the one in certificate.yaml -#- name: SERVICE_NAMESPACE # namespace of the service -# objref: -# kind: Service -# version: v1 -# name: webhook-service -# fieldref: -# fieldpath: metadata.namespace -#- name: SERVICE_NAME -# objref: -# kind: Service -# version: v1 -# name: webhook-service +- name: CERTIFICATE_NAMESPACE + objref: + kind: Certificate + group: cert-manager.io + version: v1 + name: serving-cert + fieldref: + fieldpath: metadata.namespace +- name: CERTIFICATE_NAME + objref: + kind: Certificate + group: cert-manager.io + version: v1 + name: serving-cert +- name: SERVICE_NAMESPACE + objref: + kind: Service + version: v1 + name: webhook-service + fieldref: + fieldpath: metadata.namespace +- name: SERVICE_NAME + objref: + kind: Service + version: v1 + name: webhook-service #patches: #- target: @@ -111,6 +110,10 @@ vars: # currently patch is set to change deployment environment variables patches: +- path: manager_webhook_patch.yaml + target: + kind: Deployment + name: controller-manager - target: kind: Deployment name: controller-manager @@ -134,4 +137,4 @@ patches: # More info: https://book.kubebuilder.io/reference/metrics - path: manager_metrics_patch.yaml target: - kind: Deployment \ No newline at end of file + kind: Deployment diff --git a/config/default/manager_webhook_patch.yaml b/config/default/manager_webhook_patch.yaml new file mode 100644 index 000000000..e097f669f --- /dev/null +++ b/config/default/manager_webhook_patch.yaml @@ -0,0 +1,20 @@ +- op: add + path: /spec/template/spec/containers/0/volumeMounts/- + value: + mountPath: /tmp/k8s-webhook-server/serving-certs + name: cert + readOnly: true + +- op: add + path: /spec/template/spec/containers/0/ports/- + value: + containerPort: 9444 + name: webhook-server + protocol: TCP + +- op: add + path: /spec/template/spec/volumes/- + value: + name: cert + secret: + secretName: webhook-server-cert diff --git a/config/default/webhookcainjection_patch.yaml b/config/default/webhookcainjection_patch.yaml new file mode 100644 index 000000000..da72b3cdb --- /dev/null +++ b/config/default/webhookcainjection_patch.yaml @@ -0,0 +1,14 @@ +# This patch adds the cert-manager CA injection annotation to the default App webhook configuration. +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingWebhookConfiguration +metadata: + labels: + app.kubernetes.io/name: validatingwebhookconfiguration + app.kubernetes.io/instance: validating-webhook-configuration + app.kubernetes.io/component: webhook + app.kubernetes.io/created-by: splunk-operator + app.kubernetes.io/part-of: splunk-operator + app.kubernetes.io/managed-by: kustomize + name: validating-webhook-configuration + annotations: + cert-manager.io/inject-ca-from: $(CERTIFICATE_NAMESPACE)/$(CERTIFICATE_NAME) diff --git a/internal/webhook/apps/v1alpha1/app_webhook.go b/internal/webhook/apps/v1alpha1/app_webhook.go new file mode 100644 index 000000000..b7cf43187 --- /dev/null +++ b/internal/webhook/apps/v1alpha1/app_webhook.go @@ -0,0 +1,220 @@ +/* +Copyright 2021. + +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 v1alpha1 + +import ( + "context" + "fmt" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/validation/field" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + appsv1alpha1 "github.com/splunk/splunk-operator/api/apps/v1alpha1" +) + +// nolint:unused +// log is for logging in this package. +var applog = logf.Log.WithName("app-resource") + +const AppValidationPath = "/validate-apps-splunk-com-v1alpha1-app" + +// SetupWebhookWithManager registers the webhook for App in the manager. +func SetupWebhookWithManager(mgr ctrl.Manager) error { + return ctrl.NewWebhookManagedBy(mgr). + For(&appsv1alpha1.App{}). + WithValidator(&AppValidator{Client: mgr.GetClient()}). + Complete() +} + +// TODO(user): change verbs to "verbs=create;update;delete" if you want to enable deletion validation. +// NOTE: If you want to customise the 'path', use the flags '--defaulting-path' or '--validation-path'. +// +kubebuilder:webhook:path=/validate-apps-splunk-com-v1alpha1-app,mutating=false,failurePolicy=fail,sideEffects=None,groups=apps.splunk.com,resources=apps,verbs=create;update,versions=v1alpha1,name=vapp-v1alpha1.kb.io,admissionReviewVersions=v1 + +// AppValidator is a scaffold validator for the App resource. +type AppValidator struct { + Client client.Client +} + +// NewAppValidator creates an App validator backed by a Kubernetes client. +func NewAppValidator(k8sClient client.Client) *AppValidator { + return &AppValidator{Client: k8sClient} +} + +func appFromObject(obj runtime.Object) (*appsv1alpha1.App, error) { + app, ok := obj.(*appsv1alpha1.App) + if !ok { + return nil, fmt.Errorf("expected *appsv1alpha1.App but got %T", obj) + } + + return app, nil +} + +// ValidateCreate implements admission.CustomValidator so a webhook will be registered for the type App. +func (v *AppValidator) ValidateCreate(_ context.Context, obj runtime.Object) (admission.Warnings, error) { + app, err := appFromObject(obj) + if err != nil { + return nil, err + } + + applog.Info("Validation for App upon creation", "name", app.GetName()) + + allErrs := ValidateAppCreate(v.Client, app) + if len(allErrs) > 0 { + return GetAppWarningsOnCreate(app), apierrors.NewInvalid(v.GetGroupKind(obj), v.GetName(obj), allErrs) + } + + return GetAppWarningsOnCreate(app), nil +} + +// ValidateUpdate implements admission.CustomValidator so a webhook will be registered for the type App. +func (v *AppValidator) ValidateUpdate(_ context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { + oldApp, err := appFromObject(oldObj) + if err != nil { + return nil, err + } + + app, err := appFromObject(newObj) + if err != nil { + return nil, err + } + + applog.Info("Validation for App upon update", "name", app.GetName()) + + allErrs := ValidateAppUpdate(v.Client, app, oldApp) + if len(allErrs) > 0 { + return GetAppWarningsOnUpdate(app, oldApp), apierrors.NewInvalid(v.GetGroupKind(newObj), v.GetName(newObj), allErrs) + } + + return GetAppWarningsOnUpdate(app, oldApp), nil +} + +// ValidateDelete implements admission.CustomValidator so a webhook will be registered for the type App. +func (v *AppValidator) ValidateDelete(_ context.Context, obj runtime.Object) (admission.Warnings, error) { + app, err := appFromObject(obj) + if err != nil { + return nil, err + } + + applog.Info("Validation for App upon deletion", "name", app.GetName()) + + return nil, nil +} + +// GetGroupKind returns the GroupKind for App. +func (v *AppValidator) GetGroupKind(runtime.Object) schema.GroupKind { + return schema.GroupKind{Group: appsv1alpha1.GroupVersion.Group, Kind: "App"} +} + +// GetName returns the App name. +func (v *AppValidator) GetName(obj runtime.Object) string { + app, err := appFromObject(obj) + if err != nil { + return "" + } + + return app.GetName() +} + +// GetAppWarningsOnCreate returns warnings for App CREATE. +func GetAppWarningsOnCreate(*appsv1alpha1.App) []string { + return nil +} + +// GetAppWarningsOnUpdate returns warnings for App UPDATE. +func GetAppWarningsOnUpdate(*appsv1alpha1.App, *appsv1alpha1.App) []string { + return nil +} + +// ValidateAppCreate validates an App on CREATE. +func ValidateAppCreate(k8sClient client.Client, obj *appsv1alpha1.App) field.ErrorList { + return validateApp(context.Background(), k8sClient, obj) +} + +// ValidateAppUpdate validates an App on UPDATE. +func ValidateAppUpdate(k8sClient client.Client, obj, _ *appsv1alpha1.App) field.ErrorList { + return validateApp(context.Background(), k8sClient, obj) +} + +func validateApp(ctx context.Context, k8sClient client.Client, app *appsv1alpha1.App) field.ErrorList { + if k8sClient == nil { + return field.ErrorList{ + field.InternalError(field.NewPath("spec"), fmt.Errorf("kubernetes client is required for App validation")), + } + } + + var allErrs field.ErrorList + allErrs = append(allErrs, validateAppSourceRef(ctx, k8sClient, app)...) + allErrs = append(allErrs, validateAppUniqueness(ctx, k8sClient, app)...) + + return allErrs +} + +func validateAppSourceRef(ctx context.Context, k8sClient client.Client, app *appsv1alpha1.App) field.ErrorList { + var allErrs field.ErrorList + + sourceRefPath := field.NewPath("spec").Child("sourceRef").Child("name") + key := client.ObjectKey{Name: app.Spec.SourceRef.Name, Namespace: app.Namespace} + + var source appsv1alpha1.AppSource + if err := k8sClient.Get(ctx, key, &source); err != nil { + if apierrors.IsNotFound(err) { + allErrs = append(allErrs, field.NotFound(sourceRefPath, app.Spec.SourceRef.Name)) + return allErrs + } + + allErrs = append(allErrs, field.InternalError(sourceRefPath, fmt.Errorf("failed to validate AppSource reference: %w", err))) + } + + return allErrs +} + +func validateAppUniqueness(ctx context.Context, k8sClient client.Client, app *appsv1alpha1.App) field.ErrorList { + var allErrs field.ErrorList + + var appList appsv1alpha1.AppList + if err := k8sClient.List(ctx, &appList, client.InNamespace(app.Namespace)); err != nil { + return field.ErrorList{ + field.InternalError(field.NewPath("spec"), fmt.Errorf("failed to validate App uniqueness: %w", err)), + } + } + + for i := range appList.Items { + other := &appList.Items[i] + if other.Name == app.Name { + continue + } + + if other.Spec.AppID == app.Spec.AppID && + other.Spec.Scope == app.Spec.Scope && + other.Spec.TargetRef == app.Spec.TargetRef { + allErrs = append(allErrs, field.Invalid( + field.NewPath("spec"), + fmt.Sprintf("%s/%s:%s:%s/%s", app.Namespace, app.Name, app.Spec.AppID, app.Spec.TargetRef.Kind, app.Spec.TargetRef.Name), + fmt.Sprintf("another App %q already exists in namespace %q with the same targetRef, appID, and scope", other.Name, app.Namespace), + )) + break + } + } + + return allErrs +} diff --git a/internal/webhook/apps/v1alpha1/app_webhook_test.go b/internal/webhook/apps/v1alpha1/app_webhook_test.go new file mode 100644 index 000000000..857164899 --- /dev/null +++ b/internal/webhook/apps/v1alpha1/app_webhook_test.go @@ -0,0 +1,279 @@ +/* +Copyright 2021. + +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 v1alpha1 + +import ( + "context" + "net/http" + "net/url" + "testing" + + "github.com/go-logr/logr" + appsv1alpha1 "github.com/splunk/splunk-operator/api/apps/v1alpha1" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/rest" + "k8s.io/client-go/tools/record" + "sigs.k8s.io/controller-runtime/pkg/cache" + "sigs.k8s.io/controller-runtime/pkg/cache/informertest" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/config" + "sigs.k8s.io/controller-runtime/pkg/healthz" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/webhook" +) + +func TestAppValidatorAllowsValidObjects(t *testing.T) { + scheme := runtime.NewScheme() + if err := appsv1alpha1.AddToScheme(scheme); err != nil { + t.Fatalf("failed to add App scheme: %v", err) + } + + validator := NewAppValidator(fake.NewClientBuilder().WithScheme(scheme).WithObjects( + &appsv1alpha1.AppSource{ + ObjectMeta: metav1.ObjectMeta{ + Name: "source-a", + Namespace: "test", + }, + }, + ).Build()) + app := &appsv1alpha1.App{ + ObjectMeta: metav1.ObjectMeta{ + Name: "app-a", + Namespace: "test", + }, + Spec: appsv1alpha1.AppSpec{ + AppID: "my-app", + Version: "1.0.0", + TargetRef: appsv1alpha1.AppTargetRef{ + Kind: "Standalone", + Name: "target-a", + }, + SourceRef: appsv1alpha1.AppSourceRef{ + Name: "source-a", + }, + Package: appsv1alpha1.AppPackageSpec{ + Path: "apps/my-app.tgz", + }, + Scope: "local", + }, + } + + tests := []struct { + name string + run func() error + }{ + { + name: "create", + run: func() error { + warnings, err := validator.ValidateCreate(context.Background(), app) + if len(warnings) != 0 { + t.Fatalf("expected no warnings, got %v", warnings) + } + return err + }, + }, + { + name: "update", + run: func() error { + warnings, err := validator.ValidateUpdate(context.Background(), app, app) + if len(warnings) != 0 { + t.Fatalf("expected no warnings, got %v", warnings) + } + return err + }, + }, + { + name: "delete", + run: func() error { + warnings, err := validator.ValidateDelete(context.Background(), app) + if len(warnings) != 0 { + t.Fatalf("expected no warnings, got %v", warnings) + } + return err + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + if err := test.run(); err != nil { + t.Fatalf("expected no error, got %v", err) + } + }) + } +} + +func TestAppValidatorRejectsMissingAppSource(t *testing.T) { + scheme := runtime.NewScheme() + if err := appsv1alpha1.AddToScheme(scheme); err != nil { + t.Fatalf("failed to add App scheme: %v", err) + } + + validator := NewAppValidator(fake.NewClientBuilder().WithScheme(scheme).Build()) + app := &appsv1alpha1.App{ + ObjectMeta: metav1.ObjectMeta{ + Name: "app-a", + Namespace: "test", + }, + Spec: appsv1alpha1.AppSpec{ + AppID: "my-app", + Version: "1.0.0", + TargetRef: appsv1alpha1.AppTargetRef{ + Kind: "Standalone", + Name: "target-a", + }, + SourceRef: appsv1alpha1.AppSourceRef{ + Name: "missing-source", + }, + Package: appsv1alpha1.AppPackageSpec{ + Path: "apps/my-app.tgz", + }, + Scope: "local", + }, + } + + if _, err := validator.ValidateCreate(context.Background(), app); err == nil || !apierrors.IsInvalid(err) { + t.Fatalf("expected Invalid validation error, got %v", err) + } +} + +func TestAppValidatorRejectsDuplicateAppTarget(t *testing.T) { + scheme := runtime.NewScheme() + if err := appsv1alpha1.AddToScheme(scheme); err != nil { + t.Fatalf("failed to add App scheme: %v", err) + } + + existing := &appsv1alpha1.App{ + ObjectMeta: metav1.ObjectMeta{ + Name: "app-existing", + Namespace: "test", + }, + Spec: appsv1alpha1.AppSpec{ + AppID: "my-app", + Version: "1.0.0", + TargetRef: appsv1alpha1.AppTargetRef{ + Kind: "Standalone", + Name: "target-a", + }, + SourceRef: appsv1alpha1.AppSourceRef{ + Name: "source-a", + }, + Package: appsv1alpha1.AppPackageSpec{ + Path: "apps/my-app.tgz", + }, + Scope: "local", + }, + } + + validator := NewAppValidator(fake.NewClientBuilder().WithScheme(scheme).WithObjects( + &appsv1alpha1.AppSource{ + ObjectMeta: metav1.ObjectMeta{ + Name: "source-a", + Namespace: "test", + }, + }, + existing, + ).Build()) + + app := existing.DeepCopy() + app.Name = "app-new" + + if _, err := validator.ValidateCreate(context.Background(), app); err == nil || !apierrors.IsInvalid(err) { + t.Fatalf("expected Invalid validation error, got %v", err) + } +} + +func TestAppValidatorRejectsWrongTypes(t *testing.T) { + validator := &AppValidator{} + wrongObj := &corev1.ConfigMap{} + + if _, err := validator.ValidateCreate(context.Background(), wrongObj); err == nil { + t.Fatalf("expected create validation to reject wrong object type") + } + if _, err := validator.ValidateUpdate(context.Background(), wrongObj, &appsv1alpha1.App{}); err == nil { + t.Fatalf("expected update validation to reject wrong old object type") + } + if _, err := validator.ValidateUpdate(context.Background(), &appsv1alpha1.App{}, wrongObj); err == nil { + t.Fatalf("expected update validation to reject wrong new object type") + } + if _, err := validator.ValidateDelete(context.Background(), wrongObj); err == nil { + t.Fatalf("expected delete validation to reject wrong object type") + } +} + +func TestSetupWebhookWithManager(t *testing.T) { + scheme := runtime.NewScheme() + if err := appsv1alpha1.AddToScheme(scheme); err != nil { + t.Fatalf("failed to add App scheme: %v", err) + } + + webhookServer := webhook.NewServer(webhook.Options{Port: 0}) + mgr := &testManager{ + scheme: scheme, + client: fake.NewClientBuilder().WithScheme(scheme).Build(), + webhookServer: webhookServer, + } + + if err := SetupWebhookWithManager(mgr); err != nil { + t.Fatalf("expected setup to succeed, got %v", err) + } + + handler, path := webhookServer.WebhookMux().Handler(&http.Request{ + URL: &url.URL{Path: AppValidationPath}, + }) + if path != AppValidationPath { + t.Fatalf("expected handler path %q, got %q", AppValidationPath, path) + } + if handler == nil { + t.Fatalf("expected webhook handler to be registered") + } +} + +var _ manager.Manager = &testManager{} + +type testManager struct { + scheme *runtime.Scheme + client client.Client + webhookServer webhook.Server +} + +func (m *testManager) AddMetricsServerExtraHandler(string, http.Handler) error { return nil } +func (m *testManager) Add(manager.Runnable) error { return nil } +func (m *testManager) Elected() <-chan struct{} { return nil } +func (m *testManager) AddHealthzCheck(string, healthz.Checker) error { return nil } +func (m *testManager) AddReadyzCheck(string, healthz.Checker) error { return nil } +func (m *testManager) Start(context.Context) error { return nil } +func (m *testManager) GetWebhookServer() webhook.Server { return m.webhookServer } +func (m *testManager) GetLogger() logr.Logger { return logf.Log.WithName("test-manager") } +func (m *testManager) GetControllerOptions() config.Controller { return config.Controller{} } +func (m *testManager) GetHTTPClient() *http.Client { return http.DefaultClient } +func (m *testManager) GetConfig() *rest.Config { return &rest.Config{} } +func (m *testManager) GetScheme() *runtime.Scheme { return m.scheme } +func (m *testManager) GetClient() client.Client { return m.client } +func (m *testManager) GetFieldIndexer() client.FieldIndexer { return nil } +func (m *testManager) GetCache() cache.Cache { return &informertest.FakeInformers{} } +func (m *testManager) GetEventRecorderFor(string) record.EventRecorder { + return record.NewFakeRecorder(1) +} +func (m *testManager) GetRESTMapper() meta.RESTMapper { return nil } +func (m *testManager) GetAPIReader() client.Reader { return m.client } From f4617169ecec91c5c97aef0e48f75cf3a93dd002 Mon Sep 17 00:00:00 2001 From: jzywieck Date: Fri, 17 Apr 2026 15:58:09 +0200 Subject: [PATCH 2/9] feat: add internal/webhook to docker Signed-off-by: jzywieck --- Dockerfile | 3 ++- config/app-webhook/manifests.yaml | 2 +- internal/webhook/apps/v1alpha1/app_webhook.go | 4 ++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Dockerfile b/Dockerfile index 4c7171a05..eaadbadc8 100644 --- a/Dockerfile +++ b/Dockerfile @@ -19,6 +19,7 @@ RUN go mod download COPY cmd/main.go cmd/main.go COPY api/ api/ COPY internal/controller/ internal/controller/ +COPY internal/webhook/ internal/webhook/ COPY pkg/ pkg/ COPY tools/ tools/ COPY hack hack/ @@ -92,4 +93,4 @@ COPY tools/k8_probes/startupProbe.sh /tools/k8_probes/ USER 1001 # Start the manager -ENTRYPOINT ["/manager"] \ No newline at end of file +ENTRYPOINT ["/manager"] diff --git a/config/app-webhook/manifests.yaml b/config/app-webhook/manifests.yaml index 8968e23c6..b7ebebcbb 100644 --- a/config/app-webhook/manifests.yaml +++ b/config/app-webhook/manifests.yaml @@ -10,7 +10,7 @@ webhooks: service: name: webhook-service namespace: system - path: /validate-apps-splunk-com-v1alpha1-app + path: /validate-apps failurePolicy: Fail name: vapp-v1alpha1.kb.io rules: diff --git a/internal/webhook/apps/v1alpha1/app_webhook.go b/internal/webhook/apps/v1alpha1/app_webhook.go index b7cf43187..f25fa9665 100644 --- a/internal/webhook/apps/v1alpha1/app_webhook.go +++ b/internal/webhook/apps/v1alpha1/app_webhook.go @@ -36,7 +36,7 @@ import ( // log is for logging in this package. var applog = logf.Log.WithName("app-resource") -const AppValidationPath = "/validate-apps-splunk-com-v1alpha1-app" +const AppValidationPath = "/validate-apps" // SetupWebhookWithManager registers the webhook for App in the manager. func SetupWebhookWithManager(mgr ctrl.Manager) error { @@ -48,7 +48,7 @@ func SetupWebhookWithManager(mgr ctrl.Manager) error { // TODO(user): change verbs to "verbs=create;update;delete" if you want to enable deletion validation. // NOTE: If you want to customise the 'path', use the flags '--defaulting-path' or '--validation-path'. -// +kubebuilder:webhook:path=/validate-apps-splunk-com-v1alpha1-app,mutating=false,failurePolicy=fail,sideEffects=None,groups=apps.splunk.com,resources=apps,verbs=create;update,versions=v1alpha1,name=vapp-v1alpha1.kb.io,admissionReviewVersions=v1 +// +kubebuilder:webhook:path=/validate-apps,mutating=false,failurePolicy=fail,sideEffects=None,groups=apps.splunk.com,resources=apps,verbs=create;update,versions=v1alpha1,name=vapp-v1alpha1.kb.io,admissionReviewVersions=v1 // AppValidator is a scaffold validator for the App resource. type AppValidator struct { From 1690c1cda69480376be6ac37ba72da2701bfda9b Mon Sep 17 00:00:00 2001 From: jzywieck Date: Fri, 17 Apr 2026 17:08:59 +0200 Subject: [PATCH 3/9] feat: revert back to default naming Signed-off-by: jzywieck --- config/app-webhook/manifests.yaml | 2 +- internal/webhook/apps/v1alpha1/app_webhook.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/config/app-webhook/manifests.yaml b/config/app-webhook/manifests.yaml index b7ebebcbb..8968e23c6 100644 --- a/config/app-webhook/manifests.yaml +++ b/config/app-webhook/manifests.yaml @@ -10,7 +10,7 @@ webhooks: service: name: webhook-service namespace: system - path: /validate-apps + path: /validate-apps-splunk-com-v1alpha1-app failurePolicy: Fail name: vapp-v1alpha1.kb.io rules: diff --git a/internal/webhook/apps/v1alpha1/app_webhook.go b/internal/webhook/apps/v1alpha1/app_webhook.go index f25fa9665..b7cf43187 100644 --- a/internal/webhook/apps/v1alpha1/app_webhook.go +++ b/internal/webhook/apps/v1alpha1/app_webhook.go @@ -36,7 +36,7 @@ import ( // log is for logging in this package. var applog = logf.Log.WithName("app-resource") -const AppValidationPath = "/validate-apps" +const AppValidationPath = "/validate-apps-splunk-com-v1alpha1-app" // SetupWebhookWithManager registers the webhook for App in the manager. func SetupWebhookWithManager(mgr ctrl.Manager) error { @@ -48,7 +48,7 @@ func SetupWebhookWithManager(mgr ctrl.Manager) error { // TODO(user): change verbs to "verbs=create;update;delete" if you want to enable deletion validation. // NOTE: If you want to customise the 'path', use the flags '--defaulting-path' or '--validation-path'. -// +kubebuilder:webhook:path=/validate-apps,mutating=false,failurePolicy=fail,sideEffects=None,groups=apps.splunk.com,resources=apps,verbs=create;update,versions=v1alpha1,name=vapp-v1alpha1.kb.io,admissionReviewVersions=v1 +// +kubebuilder:webhook:path=/validate-apps-splunk-com-v1alpha1-app,mutating=false,failurePolicy=fail,sideEffects=None,groups=apps.splunk.com,resources=apps,verbs=create;update,versions=v1alpha1,name=vapp-v1alpha1.kb.io,admissionReviewVersions=v1 // AppValidator is a scaffold validator for the App resource. type AppValidator struct { From a54be6046d14b7deab69922c5dc1184fe9ef51ae Mon Sep 17 00:00:00 2001 From: jzywieck Date: Fri, 17 Apr 2026 18:00:40 +0200 Subject: [PATCH 4/9] feat: tests changes and comments improvements Signed-off-by: jzywieck --- config/default/kustomization-cluster.yaml | 6 +- config/default/kustomization-namespace.yaml | 6 +- config/default/kustomization.yaml | 6 +- internal/webhook/apps/v1alpha1/app_webhook.go | 48 +++---- .../webhook/apps/v1alpha1/app_webhook_test.go | 118 ++---------------- 5 files changed, 33 insertions(+), 151 deletions(-) diff --git a/config/default/kustomization-cluster.yaml b/config/default/kustomization-cluster.yaml index 96718b872..b2e82a3ee 100644 --- a/config/default/kustomization-cluster.yaml +++ b/config/default/kustomization-cluster.yaml @@ -21,7 +21,7 @@ bases: # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in # crd/kustomization.yaml - ../app-webhook -# [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'. 'WEBHOOK' components are required. +# [CERTMANAGER] Required for apps webhook - ../certmanager # [PROMETHEUS] To enable prometheus monitor, uncomment all sections with 'PROMETHEUS'. #- ../prometheus @@ -37,9 +37,7 @@ patchesStrategicMerge: # crd/kustomization.yaml #- manager_webhook_patch.yaml -# [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'. -# Uncomment 'CERTMANAGER' sections in crd/kustomization.yaml to enable the CA injection in the admission webhooks. -# 'CERTMANAGER' needs to be enabled to use ca injection +# [CERTMANAGER] Enabled for CA injection in the admission webhooks - webhookcainjection_patch.yaml # the following config is for teaching kustomize how to do var substitution diff --git a/config/default/kustomization-namespace.yaml b/config/default/kustomization-namespace.yaml index 0774b6c30..d8d3c1b00 100644 --- a/config/default/kustomization-namespace.yaml +++ b/config/default/kustomization-namespace.yaml @@ -21,7 +21,7 @@ bases: # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in # crd/kustomization.yaml - ../app-webhook -# [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'. 'WEBHOOK' components are required. +# [CERTMANAGER] Required for apps webhook - ../certmanager # [PROMETHEUS] To enable prometheus monitor, uncomment all sections with 'PROMETHEUS'. #- ../prometheus @@ -37,9 +37,7 @@ patchesStrategicMerge: # crd/kustomization.yaml #- manager_webhook_patch.yaml -# [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'. -# Uncomment 'CERTMANAGER' sections in crd/kustomization.yaml to enable the CA injection in the admission webhooks. -# 'CERTMANAGER' needs to be enabled to use ca injection +# [CERTMANAGER] Enabled for CA injection in the admission webhooks - webhookcainjection_patch.yaml # the following config is for teaching kustomize how to do var substitution diff --git a/config/default/kustomization.yaml b/config/default/kustomization.yaml index 96718b872..b2e82a3ee 100644 --- a/config/default/kustomization.yaml +++ b/config/default/kustomization.yaml @@ -21,7 +21,7 @@ bases: # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in # crd/kustomization.yaml - ../app-webhook -# [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'. 'WEBHOOK' components are required. +# [CERTMANAGER] Required for apps webhook - ../certmanager # [PROMETHEUS] To enable prometheus monitor, uncomment all sections with 'PROMETHEUS'. #- ../prometheus @@ -37,9 +37,7 @@ patchesStrategicMerge: # crd/kustomization.yaml #- manager_webhook_patch.yaml -# [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'. -# Uncomment 'CERTMANAGER' sections in crd/kustomization.yaml to enable the CA injection in the admission webhooks. -# 'CERTMANAGER' needs to be enabled to use ca injection +# [CERTMANAGER] Enabled for CA injection in the admission webhooks - webhookcainjection_patch.yaml # the following config is for teaching kustomize how to do var substitution diff --git a/internal/webhook/apps/v1alpha1/app_webhook.go b/internal/webhook/apps/v1alpha1/app_webhook.go index b7cf43187..2fa00cd9f 100644 --- a/internal/webhook/apps/v1alpha1/app_webhook.go +++ b/internal/webhook/apps/v1alpha1/app_webhook.go @@ -1,18 +1,16 @@ -/* -Copyright 2021. - -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. -*/ +// Copyright (c) 2018-2026 Splunk Inc. All rights reserved. + +// 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 v1alpha1 @@ -26,16 +24,11 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" - logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" appsv1alpha1 "github.com/splunk/splunk-operator/api/apps/v1alpha1" ) -// nolint:unused -// log is for logging in this package. -var applog = logf.Log.WithName("app-resource") - const AppValidationPath = "/validate-apps-splunk-com-v1alpha1-app" // SetupWebhookWithManager registers the webhook for App in the manager. @@ -46,11 +39,9 @@ func SetupWebhookWithManager(mgr ctrl.Manager) error { Complete() } -// TODO(user): change verbs to "verbs=create;update;delete" if you want to enable deletion validation. -// NOTE: If you want to customise the 'path', use the flags '--defaulting-path' or '--validation-path'. // +kubebuilder:webhook:path=/validate-apps-splunk-com-v1alpha1-app,mutating=false,failurePolicy=fail,sideEffects=None,groups=apps.splunk.com,resources=apps,verbs=create;update,versions=v1alpha1,name=vapp-v1alpha1.kb.io,admissionReviewVersions=v1 -// AppValidator is a scaffold validator for the App resource. +// AppValidator is a validator for the App resource. type AppValidator struct { Client client.Client } @@ -76,8 +67,6 @@ func (v *AppValidator) ValidateCreate(_ context.Context, obj runtime.Object) (ad return nil, err } - applog.Info("Validation for App upon creation", "name", app.GetName()) - allErrs := ValidateAppCreate(v.Client, app) if len(allErrs) > 0 { return GetAppWarningsOnCreate(app), apierrors.NewInvalid(v.GetGroupKind(obj), v.GetName(obj), allErrs) @@ -98,8 +87,6 @@ func (v *AppValidator) ValidateUpdate(_ context.Context, oldObj, newObj runtime. return nil, err } - applog.Info("Validation for App upon update", "name", app.GetName()) - allErrs := ValidateAppUpdate(v.Client, app, oldApp) if len(allErrs) > 0 { return GetAppWarningsOnUpdate(app, oldApp), apierrors.NewInvalid(v.GetGroupKind(newObj), v.GetName(newObj), allErrs) @@ -110,13 +97,6 @@ func (v *AppValidator) ValidateUpdate(_ context.Context, oldObj, newObj runtime. // ValidateDelete implements admission.CustomValidator so a webhook will be registered for the type App. func (v *AppValidator) ValidateDelete(_ context.Context, obj runtime.Object) (admission.Warnings, error) { - app, err := appFromObject(obj) - if err != nil { - return nil, err - } - - applog.Info("Validation for App upon deletion", "name", app.GetName()) - return nil, nil } diff --git a/internal/webhook/apps/v1alpha1/app_webhook_test.go b/internal/webhook/apps/v1alpha1/app_webhook_test.go index 857164899..bb4d39ee6 100644 --- a/internal/webhook/apps/v1alpha1/app_webhook_test.go +++ b/internal/webhook/apps/v1alpha1/app_webhook_test.go @@ -1,45 +1,28 @@ -/* -Copyright 2021. - -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. -*/ +// Copyright (c) 2018-2026 Splunk Inc. All rights reserved. + +// 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 v1alpha1 import ( "context" - "net/http" - "net/url" "testing" - "github.com/go-logr/logr" appsv1alpha1 "github.com/splunk/splunk-operator/api/apps/v1alpha1" - corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/client-go/rest" - "k8s.io/client-go/tools/record" - "sigs.k8s.io/controller-runtime/pkg/cache" - "sigs.k8s.io/controller-runtime/pkg/cache/informertest" - "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" - "sigs.k8s.io/controller-runtime/pkg/config" - "sigs.k8s.io/controller-runtime/pkg/healthz" - logf "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/manager" - "sigs.k8s.io/controller-runtime/pkg/webhook" ) func TestAppValidatorAllowsValidObjects(t *testing.T) { @@ -202,78 +185,3 @@ func TestAppValidatorRejectsDuplicateAppTarget(t *testing.T) { t.Fatalf("expected Invalid validation error, got %v", err) } } - -func TestAppValidatorRejectsWrongTypes(t *testing.T) { - validator := &AppValidator{} - wrongObj := &corev1.ConfigMap{} - - if _, err := validator.ValidateCreate(context.Background(), wrongObj); err == nil { - t.Fatalf("expected create validation to reject wrong object type") - } - if _, err := validator.ValidateUpdate(context.Background(), wrongObj, &appsv1alpha1.App{}); err == nil { - t.Fatalf("expected update validation to reject wrong old object type") - } - if _, err := validator.ValidateUpdate(context.Background(), &appsv1alpha1.App{}, wrongObj); err == nil { - t.Fatalf("expected update validation to reject wrong new object type") - } - if _, err := validator.ValidateDelete(context.Background(), wrongObj); err == nil { - t.Fatalf("expected delete validation to reject wrong object type") - } -} - -func TestSetupWebhookWithManager(t *testing.T) { - scheme := runtime.NewScheme() - if err := appsv1alpha1.AddToScheme(scheme); err != nil { - t.Fatalf("failed to add App scheme: %v", err) - } - - webhookServer := webhook.NewServer(webhook.Options{Port: 0}) - mgr := &testManager{ - scheme: scheme, - client: fake.NewClientBuilder().WithScheme(scheme).Build(), - webhookServer: webhookServer, - } - - if err := SetupWebhookWithManager(mgr); err != nil { - t.Fatalf("expected setup to succeed, got %v", err) - } - - handler, path := webhookServer.WebhookMux().Handler(&http.Request{ - URL: &url.URL{Path: AppValidationPath}, - }) - if path != AppValidationPath { - t.Fatalf("expected handler path %q, got %q", AppValidationPath, path) - } - if handler == nil { - t.Fatalf("expected webhook handler to be registered") - } -} - -var _ manager.Manager = &testManager{} - -type testManager struct { - scheme *runtime.Scheme - client client.Client - webhookServer webhook.Server -} - -func (m *testManager) AddMetricsServerExtraHandler(string, http.Handler) error { return nil } -func (m *testManager) Add(manager.Runnable) error { return nil } -func (m *testManager) Elected() <-chan struct{} { return nil } -func (m *testManager) AddHealthzCheck(string, healthz.Checker) error { return nil } -func (m *testManager) AddReadyzCheck(string, healthz.Checker) error { return nil } -func (m *testManager) Start(context.Context) error { return nil } -func (m *testManager) GetWebhookServer() webhook.Server { return m.webhookServer } -func (m *testManager) GetLogger() logr.Logger { return logf.Log.WithName("test-manager") } -func (m *testManager) GetControllerOptions() config.Controller { return config.Controller{} } -func (m *testManager) GetHTTPClient() *http.Client { return http.DefaultClient } -func (m *testManager) GetConfig() *rest.Config { return &rest.Config{} } -func (m *testManager) GetScheme() *runtime.Scheme { return m.scheme } -func (m *testManager) GetClient() client.Client { return m.client } -func (m *testManager) GetFieldIndexer() client.FieldIndexer { return nil } -func (m *testManager) GetCache() cache.Cache { return &informertest.FakeInformers{} } -func (m *testManager) GetEventRecorderFor(string) record.EventRecorder { - return record.NewFakeRecorder(1) -} -func (m *testManager) GetRESTMapper() meta.RESTMapper { return nil } -func (m *testManager) GetAPIReader() client.Reader { return m.client } From 4c0e01fd3d023fbb8aacd3fab12deac21d894aae Mon Sep 17 00:00:00 2001 From: jzywieck Date: Fri, 17 Apr 2026 18:05:01 +0200 Subject: [PATCH 5/9] fix: remove comment Signed-off-by: jzywieck --- config/app-webhook/kustomizeconfig.yaml | 2 -- 1 file changed, 2 deletions(-) diff --git a/config/app-webhook/kustomizeconfig.yaml b/config/app-webhook/kustomizeconfig.yaml index e809f7820..c612da16a 100644 --- a/config/app-webhook/kustomizeconfig.yaml +++ b/config/app-webhook/kustomizeconfig.yaml @@ -1,5 +1,3 @@ -# the following config is for teaching kustomize where to look at when substituting vars. -# It requires kustomize v2.1.0 or newer to work properly. nameReference: - kind: Service version: v1 From 11c1b89ef2fe5b14d528353e19d433be028ec3a0 Mon Sep 17 00:00:00 2001 From: jzywieck Date: Fri, 17 Apr 2026 18:09:09 +0200 Subject: [PATCH 6/9] docs: add relevant comments to app webhook Signed-off-by: jzywieck --- internal/webhook/apps/v1alpha1/app_webhook.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/internal/webhook/apps/v1alpha1/app_webhook.go b/internal/webhook/apps/v1alpha1/app_webhook.go index 2fa00cd9f..fbcd8c7d4 100644 --- a/internal/webhook/apps/v1alpha1/app_webhook.go +++ b/internal/webhook/apps/v1alpha1/app_webhook.go @@ -51,6 +51,7 @@ func NewAppValidator(k8sClient client.Client) *AppValidator { return &AppValidator{Client: k8sClient} } +// appFromObject converts a runtime.Object to an *appsv1alpha1.App. func appFromObject(obj runtime.Object) (*appsv1alpha1.App, error) { app, ok := obj.(*appsv1alpha1.App) if !ok { @@ -60,7 +61,7 @@ func appFromObject(obj runtime.Object) (*appsv1alpha1.App, error) { return app, nil } -// ValidateCreate implements admission.CustomValidator so a webhook will be registered for the type App. +// ValidateAppCreate validates an App on CREATE. func (v *AppValidator) ValidateCreate(_ context.Context, obj runtime.Object) (admission.Warnings, error) { app, err := appFromObject(obj) if err != nil { @@ -75,7 +76,7 @@ func (v *AppValidator) ValidateCreate(_ context.Context, obj runtime.Object) (ad return GetAppWarningsOnCreate(app), nil } -// ValidateUpdate implements admission.CustomValidator so a webhook will be registered for the type App. +// ValidateAppUpdate validates an App on UPDATE. func (v *AppValidator) ValidateUpdate(_ context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { oldApp, err := appFromObject(oldObj) if err != nil { @@ -95,7 +96,7 @@ func (v *AppValidator) ValidateUpdate(_ context.Context, oldObj, newObj runtime. return GetAppWarningsOnUpdate(app, oldApp), nil } -// ValidateDelete implements admission.CustomValidator so a webhook will be registered for the type App. +// ValidateAppDelete validates an App on DELETE. No validation is needed for deletion, so it always returns nil. func (v *AppValidator) ValidateDelete(_ context.Context, obj runtime.Object) (admission.Warnings, error) { return nil, nil } @@ -135,6 +136,7 @@ func ValidateAppUpdate(k8sClient client.Client, obj, _ *appsv1alpha1.App) field. return validateApp(context.Background(), k8sClient, obj) } +// validateApp performs common validation for both CREATE and UPDATE operations on App. func validateApp(ctx context.Context, k8sClient client.Client, app *appsv1alpha1.App) field.ErrorList { if k8sClient == nil { return field.ErrorList{ @@ -149,6 +151,7 @@ func validateApp(ctx context.Context, k8sClient client.Client, app *appsv1alpha1 return allErrs } +// validateAppSourceRef checks if the App's sourceRef points to an existing AppSource. func validateAppSourceRef(ctx context.Context, k8sClient client.Client, app *appsv1alpha1.App) field.ErrorList { var allErrs field.ErrorList @@ -168,6 +171,7 @@ func validateAppSourceRef(ctx context.Context, k8sClient client.Client, app *app return allErrs } +// validateAppUniqueness checks that there are no other Apps in the same namespace with the same appID, scope, and targetRef combination. func validateAppUniqueness(ctx context.Context, k8sClient client.Client, app *appsv1alpha1.App) field.ErrorList { var allErrs field.ErrorList From fa323f8b99875db0113d85a3367eac49ef14c05f Mon Sep 17 00:00:00 2001 From: jzywieck Date: Fri, 17 Apr 2026 18:27:13 +0200 Subject: [PATCH 7/9] docs: improve the comments readability in configs Signed-off-by: jzywieck --- config/default-with-webhook/kustomization-cluster.yaml | 9 +++++---- config/default-with-webhook/kustomization-namespace.yaml | 9 +++++---- config/default-with-webhook/kustomization.yaml | 9 +++++---- config/default/kustomization-cluster.yaml | 1 + config/default/kustomization-namespace.yaml | 1 + config/default/kustomization.yaml | 1 + 6 files changed, 18 insertions(+), 12 deletions(-) diff --git a/config/default-with-webhook/kustomization-cluster.yaml b/config/default-with-webhook/kustomization-cluster.yaml index c596f0c68..38eb28573 100644 --- a/config/default-with-webhook/kustomization-cluster.yaml +++ b/config/default-with-webhook/kustomization-cluster.yaml @@ -1,6 +1,7 @@ # Adds namespace to all resources. -# Cluster-scoped deployment WITH webhook enabled (opt-in) -# Requires cert-manager to be installed in the cluster +# Cluster-scoped deployment with enterprise validation webhooks enabled (opt-in). +# The standalone App webhook is enabled by default via config/default. +# Requires cert-manager to be installed in the cluster. namespace: splunk-operator # Value of this field is prepended to the @@ -20,7 +21,7 @@ bases: - ../persistent-volume - ../service - ../manager -# [WEBHOOK] Enabled for opt-in webhook deployment +# [WEBHOOK] Enabled for enterprise validation webhooks - ../webhook # [CERTMANAGER] Required for webhook TLS - ../certmanager @@ -34,7 +35,7 @@ patchesStrategicMerge: # through a ComponentConfig type #- manager_config_patch.yaml -# [WEBHOOK] Enabled for webhook deployment +# [WEBHOOK] Enabled for enterprise validation webhooks - manager_webhook_patch.yaml # [CERTMANAGER] Enabled for CA injection in the admission webhooks diff --git a/config/default-with-webhook/kustomization-namespace.yaml b/config/default-with-webhook/kustomization-namespace.yaml index 193791601..38439c974 100644 --- a/config/default-with-webhook/kustomization-namespace.yaml +++ b/config/default-with-webhook/kustomization-namespace.yaml @@ -1,6 +1,7 @@ # Adds namespace to all resources. -# Namespace-scoped deployment WITH webhook enabled (opt-in) -# Requires cert-manager to be installed in the cluster +# Namespace-scoped deployment with enterprise validation webhooks enabled (opt-in). +# The standalone App webhook is enabled by default via config/default. +# Requires cert-manager to be installed in the cluster. namespace: splunk-operator # Value of this field is prepended to the @@ -20,7 +21,7 @@ bases: - ../persistent-volume - ../service - ../manager -# [WEBHOOK] Enabled for opt-in webhook deployment +# [WEBHOOK] Enabled for enterprise validation webhooks - ../webhook # [CERTMANAGER] Required for webhook TLS - ../certmanager @@ -34,7 +35,7 @@ patchesStrategicMerge: # through a ComponentConfig type #- manager_config_patch.yaml -# [WEBHOOK] Enabled for webhook deployment +# [WEBHOOK] Enabled for enterprise validation webhooks - manager_webhook_patch.yaml # [CERTMANAGER] Enabled for CA injection in the admission webhooks diff --git a/config/default-with-webhook/kustomization.yaml b/config/default-with-webhook/kustomization.yaml index 5ba87fec1..2fc8f750d 100644 --- a/config/default-with-webhook/kustomization.yaml +++ b/config/default-with-webhook/kustomization.yaml @@ -1,6 +1,7 @@ # Adds namespace to all resources. -# Cluster-scoped deployment WITH webhook enabled (opt-in) -# Requires cert-manager to be installed in the cluster +# Cluster-scoped deployment with enterprise validation webhooks enabled (opt-in). +# The standalone App webhook is enabled by default via config/default. +# Requires cert-manager to be installed in the cluster. namespace: splunk-operator # Value of this field is prepended to the @@ -20,7 +21,7 @@ bases: - ../persistent-volume - ../service - ../manager -# [WEBHOOK] Enabled for opt-in webhook deployment +# [WEBHOOK] Enabled for enterprise validation webhooks - ../webhook # [CERTMANAGER] Required for webhook TLS - ../certmanager @@ -34,7 +35,7 @@ patchesStrategicMerge: # through a ComponentConfig type #- manager_config_patch.yaml -# [WEBHOOK] Enabled for webhook deployment +# [WEBHOOK] Enabled for enterprise validation webhooks - manager_webhook_patch.yaml # [CERTMANAGER] Enabled for CA injection in the admission webhooks diff --git a/config/default/kustomization-cluster.yaml b/config/default/kustomization-cluster.yaml index b2e82a3ee..f3704e363 100644 --- a/config/default/kustomization-cluster.yaml +++ b/config/default/kustomization-cluster.yaml @@ -20,6 +20,7 @@ bases: - ../manager # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in # crd/kustomization.yaml +# - ../webhook - ../app-webhook # [CERTMANAGER] Required for apps webhook - ../certmanager diff --git a/config/default/kustomization-namespace.yaml b/config/default/kustomization-namespace.yaml index d8d3c1b00..a84de5624 100644 --- a/config/default/kustomization-namespace.yaml +++ b/config/default/kustomization-namespace.yaml @@ -20,6 +20,7 @@ bases: - ../manager # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in # crd/kustomization.yaml +# - ../webhook - ../app-webhook # [CERTMANAGER] Required for apps webhook - ../certmanager diff --git a/config/default/kustomization.yaml b/config/default/kustomization.yaml index b2e82a3ee..f3704e363 100644 --- a/config/default/kustomization.yaml +++ b/config/default/kustomization.yaml @@ -20,6 +20,7 @@ bases: - ../manager # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in # crd/kustomization.yaml +# - ../webhook - ../app-webhook # [CERTMANAGER] Required for apps webhook - ../certmanager From 79c8aa695ae7e5d20314d02dfe5989b614ddb969 Mon Sep 17 00:00:00 2001 From: jzywieck Date: Fri, 17 Apr 2026 18:30:23 +0200 Subject: [PATCH 8/9] fix: small styling fix Signed-off-by: jzywieck --- config/default/kustomization-cluster.yaml | 2 +- config/default/kustomization-namespace.yaml | 2 +- config/default/kustomization.yaml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/config/default/kustomization-cluster.yaml b/config/default/kustomization-cluster.yaml index f3704e363..62e53d1e1 100644 --- a/config/default/kustomization-cluster.yaml +++ b/config/default/kustomization-cluster.yaml @@ -20,7 +20,7 @@ bases: - ../manager # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in # crd/kustomization.yaml -# - ../webhook +#- ../webhook - ../app-webhook # [CERTMANAGER] Required for apps webhook - ../certmanager diff --git a/config/default/kustomization-namespace.yaml b/config/default/kustomization-namespace.yaml index a84de5624..d2196e071 100644 --- a/config/default/kustomization-namespace.yaml +++ b/config/default/kustomization-namespace.yaml @@ -20,7 +20,7 @@ bases: - ../manager # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in # crd/kustomization.yaml -# - ../webhook +#- ../webhook - ../app-webhook # [CERTMANAGER] Required for apps webhook - ../certmanager diff --git a/config/default/kustomization.yaml b/config/default/kustomization.yaml index f3704e363..62e53d1e1 100644 --- a/config/default/kustomization.yaml +++ b/config/default/kustomization.yaml @@ -20,7 +20,7 @@ bases: - ../manager # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in # crd/kustomization.yaml -# - ../webhook +#- ../webhook - ../app-webhook # [CERTMANAGER] Required for apps webhook - ../certmanager From 6389a22e7a52d91714de2e4005fd90fc2ffeb0dc Mon Sep 17 00:00:00 2001 From: jzywieck Date: Mon, 20 Apr 2026 17:36:58 +0200 Subject: [PATCH 9/9] feat: add cel rules for fields immutability validation Signed-off-by: jzywieck --- api/apps/v1alpha1/app_types.go | 4 ++++ config/crd/bases/apps.splunk.com_apps.yaml | 9 +++++++++ 2 files changed, 13 insertions(+) diff --git a/api/apps/v1alpha1/app_types.go b/api/apps/v1alpha1/app_types.go index 4bc31a0e9..bd55431f1 100644 --- a/api/apps/v1alpha1/app_types.go +++ b/api/apps/v1alpha1/app_types.go @@ -49,6 +49,10 @@ type AppSourceRef struct { Name string `json:"name"` } +// +kubebuilder:validation:XValidation:rule="self.appID == oldSelf.appID",message="spec.appID is immutable" +// +kubebuilder:validation:XValidation:rule="self.targetRef == oldSelf.targetRef",message="spec.targetRef is immutable" +// +kubebuilder:validation:XValidation:rule="self.sourceRef == oldSelf.sourceRef",message="spec.sourceRef is immutable" +// +kubebuilder:validation:XValidation:rule="self.scope == oldSelf.scope",message="spec.scope is immutable" // AppSpec defines the desired state of App. type AppSpec struct { // +kubebuilder:validation:Required diff --git a/config/crd/bases/apps.splunk.com_apps.yaml b/config/crd/bases/apps.splunk.com_apps.yaml index fe148c898..337508964 100644 --- a/config/crd/bases/apps.splunk.com_apps.yaml +++ b/config/crd/bases/apps.splunk.com_apps.yaml @@ -113,6 +113,15 @@ spec: - targetRef - version type: object + x-kubernetes-validations: + - message: spec.appID is immutable + rule: self.appID == oldSelf.appID + - message: spec.targetRef is immutable + rule: self.targetRef == oldSelf.targetRef + - message: spec.sourceRef is immutable + rule: self.sourceRef == oldSelf.sourceRef + - message: spec.scope is immutable + rule: self.scope == oldSelf.scope status: description: AppStatus defines the observed state of App. properties: