Moving CRD checks into webhook

api/go.mod

@@ -3,10 +3,12 @@ module github.com/openstack-k8s-operators/openstack-ansibleee-operator/api
 go 1.20
 
 require (
+	github.com/go-playground/validator/v10 v10.19.0
 	github.com/onsi/ginkgo/v2 v2.17.1
 	github.com/onsi/gomega v1.32.0
 	github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240326081751-56015b1ae3f6
 	github.com/openstack-k8s-operators/lib-common/modules/storage v0.3.1-0.20240326081751-56015b1ae3f6
+	gopkg.in/yaml.v2 v2.4.0
 	k8s.io/api v0.28.8
 	k8s.io/apimachinery v0.28.8
 	k8s.io/client-go v0.28.8
@@ -20,11 +22,14 @@ require (
 	github.com/emicklei/go-restful/v3 v3.11.0 // indirect
 	github.com/evanphx/json-patch/v5 v5.6.0 // indirect
 	github.com/fsnotify/fsnotify v1.6.0 // indirect
+	github.com/gabriel-vasile/mimetype v1.4.3 // indirect
 	github.com/go-logr/logr v1.4.1 // indirect
 	github.com/go-logr/zapr v1.2.4 // indirect
 	github.com/go-openapi/jsonpointer v0.19.6 // indirect
 	github.com/go-openapi/jsonreference v0.20.2 // indirect
 	github.com/go-openapi/swag v0.22.3 // indirect
+	github.com/go-playground/locales v0.14.1 // indirect
+	github.com/go-playground/universal-translator v0.18.1 // indirect
 	github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572 // indirect
 	github.com/gogo/protobuf v1.3.2 // indirect
 	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
@@ -37,6 +42,7 @@ require (
 	github.com/imdario/mergo v0.3.12 // indirect
 	github.com/josharian/intern v1.0.0 // indirect
 	github.com/json-iterator/go v1.1.12 // indirect
+	github.com/leodido/go-urn v1.4.0 // indirect
 	github.com/mailru/easyjson v0.7.7 // indirect
 	github.com/matttproud/golang_protobuf_extensions v1.0.4 // indirect
 	github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
@@ -48,9 +54,9 @@ require (
 	github.com/prometheus/common v0.44.0 // indirect
 	github.com/prometheus/procfs v0.10.1 // indirect
 	github.com/spf13/pflag v1.0.5 // indirect
-	github.com/stretchr/testify v1.8.4 // indirect
 	go.uber.org/multierr v1.11.0 // indirect
 	go.uber.org/zap v1.27.0 // indirect
+	golang.org/x/crypto v0.19.0 // indirect
 	golang.org/x/exp v0.0.0-20240119083558-1b970713d09a // indirect
 	golang.org/x/net v0.21.0 // indirect
 	golang.org/x/oauth2 v0.8.0 // indirect
@@ -63,7 +69,6 @@ require (
 	google.golang.org/appengine v1.6.7 // indirect
 	google.golang.org/protobuf v1.33.0 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect
-	gopkg.in/yaml.v2 v2.4.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 	k8s.io/apiextensions-apiserver v0.28.8 // indirect
 	k8s.io/component-base v0.28.8 // indirect

api/go.sum

@@ -17,6 +17,7 @@ github.com/evanphx/json-patch/v5 v5.6.0 h1:b91NhWfaz02IuVxO9faSllyAtNXHMPkC5J8sJ
 github.com/evanphx/json-patch/v5 v5.6.0/go.mod h1:G79N1coSVB93tBe7j6PhzjmR3/2VvlbKOFpnXhI9Bw4=
 github.com/fsnotify/fsnotify v1.6.0 h1:n+5WquG0fcWoWp6xPWfHdbskMCQaFnG6PfBrh1Ky4HY=
 github.com/fsnotify/fsnotify v1.6.0/go.mod h1:sl3t1tCWJFWoRz9R8WJCbQihKKwmorjAbSClcnxKAGw=
+github.com/gabriel-vasile/mimetype v1.4.3 h1:in2uUcidCuFcDKtdcBxlR0rJ1+fsokWf+uqxgUFjbI0=
 github.com/go-logr/logr v1.2.4/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A=
 github.com/go-logr/logr v1.3.0/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY=
 github.com/go-logr/logr v1.4.1 h1:pKouT5E8xu9zeFC39JXRDukb6JFQPXM5p5I91188VAQ=
@@ -29,6 +30,10 @@ github.com/go-openapi/jsonreference v0.20.2 h1:3sVjiK66+uXK/6oQ8xgcRKcFgQ5KXa2Kv
 github.com/go-openapi/jsonreference v0.20.2/go.mod h1:Bl1zwGIM8/wsvqjsOQLJ/SH+En5Ap4rVB5KVcIDZG2k=
 github.com/go-openapi/swag v0.22.3 h1:yMBqmnQ0gyZvEb/+KzuWZOXgllrXT4SADYbvDaXHv/g=
 github.com/go-openapi/swag v0.22.3/go.mod h1:UzaqsxGiab7freDnrUUra0MwWfN/q7tE4j+VcZ0yl14=
+github.com/go-playground/assert/v2 v2.2.0 h1:JvknZsQTYeFEAhQwI4qEt9cyV5ONwRHC+lYKSsYSR8s=
+github.com/go-playground/locales v0.14.1 h1:EWaQ/wswjilfKLTECiXz7Rh+3BjFhfDFKv/oXslEjJA=
+github.com/go-playground/universal-translator v0.18.1 h1:Bcnm0ZwsGyWbCzImXv+pAJnYK9S473LQFuzCbDbfSFY=
+github.com/go-playground/validator/v10 v10.19.0 h1:ol+5Fu+cSq9JD7SoSqe04GMI92cbn0+wvQ3bZ8b/AU4=
 github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572 h1:tfuBGBXKqDEevZMzYi5KSi8KkcZtzBcTgAUUtapy0OI=
 github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572/go.mod h1:9Pwr4B2jHnOSGXyyzV8ROjYa2ojvAY6HCGYYfMoC3Ls=
 github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q=
@@ -68,6 +73,7 @@ github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
 github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
 github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
 github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
+github.com/leodido/go-urn v1.4.0 h1:WT9HwE9SGECu3lg4d/dIA+jxlljEa1/ffXKmRjqdmIQ=
 github.com/mailru/easyjson v0.7.7 h1:UGYAvKxe3sBsEDzO8ZeWOSlIQfWFlxbzLZe7hwFURr0=
 github.com/mailru/easyjson v0.7.7/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc=
 github.com/matttproud/golang_protobuf_extensions v1.0.4 h1:mmDVorXM7PCGKw94cs5zkfA9PSy5pEvNWRP0ET0TIVo=
@@ -113,7 +119,6 @@ github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/
 github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
 github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
 github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
-github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
 github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k=
@@ -129,6 +134,7 @@ go.uber.org/zap v1.27.0/go.mod h1:GB2qFLM7cTU87MWRP2mPIjqfIDnGu+VIO4V/SdhGo2E=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
 golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
 golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
+golang.org/x/crypto v0.19.0 h1:ENy+Az/9Y1vSrlrvBSyna3PITt4tiZLf7sgCjZBX7Wo=
 golang.org/x/exp v0.0.0-20240119083558-1b970713d09a h1:Q8/wZp0KX97QFTc2ywcOE0YRjZPVIx+MXInMzdvQqcA=
 golang.org/x/exp v0.0.0-20240119083558-1b970713d09a/go.mod h1:idGWGoKP1toJGkd5/ig9ZLuPcZBC3ewk7SzmH0uou08=
 golang.org/x/lint v0.0.0-20190930215403-16217165b5de/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc=

api/v1beta1/openstackansibleee_webhook.go

@@ -24,8 +24,14 @@ package v1beta1
 
 import (
 	"fmt"
+	"regexp"
 
+	"github.com/go-playground/validator/v10"
+	"gopkg.in/yaml.v2"
+	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"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
@@ -81,13 +87,78 @@ var _ webhook.Validator = &OpenStackAnsibleEE{}
 // ValidateCreate implements webhook.Validator so a webhook will be registered for the type
 func (r *OpenStackAnsibleEE) ValidateCreate() (admission.Warnings, error) {
 	openstackansibleeelog.Info("validate create", "name", r.Name)
+	var errors field.ErrorList
+
+	errors = append(errors, r.Spec.ValidateCreate()...)
+	if len(errors) != 0 {
+		openstackansibleeelog.Info("validation failed", "name", r.Name)
+		return nil, apierrors.NewInvalid(
+			schema.GroupKind{Group: "ansibleee.openstack.org", Kind: "OpenStakAnsibleEE"},
+			r.Name,
+			errors,
+		)
+	}
+	return nil, nil
+}
+
+func (spec *OpenStackAnsibleEESpec) ValidateCreate() field.ErrorList {
+	// Setting up input validation, including custom validators
+	validate := validator.New()
+	var errors field.ErrorList
+	if valErr := validate.RegisterValidation("play", isPlay); valErr != nil {
+		errors = append(errors,
+			field.InternalError(
+				field.NewPath("spec"),
+				fmt.Errorf("error registering input validation")))
+	}
+	if valErr := validate.RegisterValidation("fqcn", isFQCN); valErr != nil {
+		errors = append(errors,
+			field.InternalError(
+				field.NewPath("spec"),
+				fmt.Errorf("error registering input validation")))
+	}
+	if len(spec.Play) > 0 {
+		valErr := validate.Var(spec.Play, "play")
+		if valErr != nil {
+			errors = append(errors, field.Invalid(
+				field.NewPath("spec.play"),
+				spec.Play,
+				fmt.Sprintf(
+					"error checking sanity of an inline play: %s %s",
+					spec.Play, valErr),
+			))
+		}
+	} else if len(spec.Playbook) > 0 {
+		// As we set "playbook.yaml" as default
+		// we need to ensure that Play and Role are empty before addPlaybook
+		if validate.Var(spec.Playbook, "fqcn") != nil {
+			// First we check if the playbook isn't imported from a collection
+			// if it is not we assume the variable holds a path.
+			valErr := validate.Var(spec.Playbook, "filepath")
+			if valErr != nil {
+				errors = append(errors, field.Invalid(
+					field.NewPath("spec.playbook"),
+					spec.Playbook,
+					fmt.Sprintf(
+						"error checking sanity of playbook name/path: %s %s",
+						spec.Playbook, valErr),
+				))
+			}
+		}
 
-	for _, value := range r.Spec.Env {
+	}
+
+	for _, value := range spec.Env {
 		if value.Name == "ANSIBLE_ENABLE_TASK_DEBUGGER" {
-			return nil, fmt.Errorf("ansible-runner does not support ANSIBLE_ENABLE_TASK_DEBUGGER")
+			errors = append(errors, field.Invalid(
+				field.NewPath("spec.env"),
+				spec.Env,
+				fmt.Sprintf("ansible-runner does not support ANSIBLE_ENABLE_TASK_DEBUGGER"),
+			))
 		}
 	}
-	return nil, nil
+
+	return errors
 }
 
 // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
@@ -105,3 +176,22 @@ func (r *OpenStackAnsibleEE) ValidateDelete() (admission.Warnings, error) {
 	// TODO(user): fill in your validation logic upon object deletion.
 	return nil, nil
 }
+
+// isPlay checks if the free form document has attributes of ansible play
+// Specifically if it is a parsable yaml with list as a root element
+func isPlay(document validator.FieldLevel) bool {
+	var play []map[string]interface{}
+	err := yaml.Unmarshal([]byte(document.Field().String()), &play)
+	return err == nil
+}
+
+// isFQCN checks if the string matches regular expression of ansible FQCN
+// The function doesn't check if the collection exists or is accessible
+// function only accepts FQCNs as defined by
+// https://old-galaxy.ansible.com/docs/contributing/namespaces.html#namespace-limitations
+// Regex derived from ansible-lint rule
+func isFQCN(name validator.FieldLevel) bool {
+	pattern, compileErr := regexp.Compile(`^\w+(\.\w+){2,}$`)
+	match := pattern.Match([]byte(name.Field().String()))
+	return match && compileErr == nil
+}

controllers/openstack_ansibleee_controller.go

@@ -18,7 +18,6 @@ package controllers
 
 import (
 	"fmt"
-	"regexp"
 	"sort"
 	"strings"
 	"time"
@@ -33,7 +32,6 @@ import (
 	"context"
 
 	"github.com/go-logr/logr"
-	"github.com/go-playground/validator/v10"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/condition"
 	helper "github.com/openstack-k8s-operators/lib-common/modules/common/helper"
 	job "github.com/openstack-k8s-operators/lib-common/modules/common/job"
@@ -271,16 +269,6 @@ func (r *OpenStackAnsibleEEReconciler) jobForOpenStackAnsibleEE(ctx context.Cont
 	Log := r.GetLogger(ctx)
 	labels := instance.GetObjectMeta().GetLabels()
 
-	// Setting up input validation, including custom validators
-	validate := validator.New()
-
-	if valErr := validate.RegisterValidation("play", isPlay); valErr != nil {
-		return nil, fmt.Errorf("error registering input validation")
-	}
-	if valErr := validate.RegisterValidation("fqcn", isFQCN); valErr != nil {
-		return nil, fmt.Errorf("error registering input validation")
-	}
-
 	ls := labelsForOpenStackAnsibleEE(instance.Name, labels)
 
 	args := instance.Spec.Args
@@ -373,27 +361,10 @@ func (r *OpenStackAnsibleEEReconciler) jobForOpenStackAnsibleEE(ctx context.Cont
 	}
 
 	if len(instance.Spec.Play) > 0 {
-		valErr := validate.Var(instance.Spec.Play, "play")
-		if valErr != nil {
-			return nil, fmt.Errorf(
-				"error checking sanity of an inline play: %s %w",
-				instance.Spec.Play, valErr)
-		}
 		setRunnerEnvVar(h, "RUNNER_PLAYBOOK", instance.Spec.Play, "play", job, hashes)
 	} else if len(playbook) > 0 {
 		// As we set "playbook.yaml" as default
-		// we need to ensure that Play and Role are empty before addPlaybook
-		if validate.Var(playbook, "fqcn") != nil {
-			// First we check if the playbook isn't imported from a collection
-			// if it is not we assume the variable holds a path.
-			valErr := validate.Var(playbook, "filepath")
-			if valErr != nil {
-				return nil, fmt.Errorf(
-					"error checking sanity of playbook name/path: %s %w",
-					playbook, valErr)
-			}
-		}
-
+		// we need to ensure that Play is empty before adding playbook
 		setRunnerEnvVar(h, "RUNNER_PLAYBOOK", playbook, "playbooks", job, hashes)
 	}
 
@@ -556,22 +527,3 @@ func (r *OpenStackAnsibleEEReconciler) SetupWithManager(mgr ctrl.Manager) error
 		Owns(&batchv1.Job{}).
 		Complete(r)
 }
-
-// isPlay checks if the free form document has attributes of ansible play
-// Specifically if it is a parsable yaml with list as a root element
-func isPlay(document validator.FieldLevel) bool {
-	var play []map[string]interface{}
-	err := yaml.Unmarshal([]byte(document.Field().String()), &play)
-	return err == nil
-}
-
-// isFQCN checks if the string matches regular expression of ansible FQCN
-// The function doesn't check if the collection exists or is accessible
-// function only accepts FQCNs as defined by
-// https://galaxy.ansible.com/docs/contributing/namespaces.html#namespace-limitations
-// Regex derive
-func isFQCN(name validator.FieldLevel) bool {
-	pattern, compileErr := regexp.Compile(`^\w+(\.\w+){2,}$`)
-	match := pattern.Match([]byte(name.Field().String()))
-	return match && compileErr == nil
-}

go.mod

@@ -6,7 +6,6 @@ replace github.com/openstack-k8s-operators/openstack-ansibleee-operator/api => .
 
 require (
 	github.com/go-logr/logr v1.4.1
-	github.com/go-playground/validator/v10 v10.19.0
 	github.com/google/uuid v1.6.0
 	github.com/k8snetworkplumbingwg/network-attachment-definition-client v1.4.0
 	github.com/onsi/ginkgo/v2 v2.17.1
@@ -35,6 +34,7 @@ require (
 	github.com/go-openapi/swag v0.22.3 // indirect
 	github.com/go-playground/locales v0.14.1 // indirect
 	github.com/go-playground/universal-translator v0.18.1 // indirect
+	github.com/go-playground/validator/v10 v10.19.0 // indirect
 	github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572 // indirect
 	github.com/gogo/protobuf v1.3.2 // indirect
 	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect

tests/functional/ansibleee_controller_test.go

@@ -188,23 +188,6 @@ var _ = Describe("Ansibleee controller", func() {
 
 		})
 
-		Context("with invalid playbook name/path", func() {
-			BeforeEach(func() {
-				DeferCleanup(th.DeleteInstance, CreateAnsibleeeWithParams(
-					ansibleeeName, "/", "test-image", "", "", map[string]interface{}{}, []map[string]interface{}{}))
-			})
-			It("runs a job and reports when it succeeds", func() {
-				th.ExpectConditionWithDetails(
-					ansibleeeName,
-					ConditionGetterFunc(AnsibleeeConditionGetter),
-					v1beta1.AnsibleExecutionJobReadyCondition,
-					corev1.ConditionUnknown,
-					condition.InitReason,
-					"AnsibleExecutionJob not started",
-				)
-			})
-		})
-
 		Context("with an inline play", func() {
 			BeforeEach(func() {
 				DeferCleanup(th.DeleteInstance, CreateAnsibleeeWithParams(

tests/functional/base_test.go

@@ -34,6 +34,15 @@ const (
     - name: Using debug statement
       ansible.builtin.debug:
         msg: "Hello, world this is ansibleee-play.yaml"`
+
+	// This constant is meant to represent malformed inline play
+	malformedPlay = `
+	- name: Print hello world
+	  hosts: all
+	  tasks:
+		- name: Using debug statement
+		  ansible.builtin.debug:
+			msg: "Hello, world this is ansibleee-play.yaml"`
 )
 
 func GetAnsibleee(name types.NamespacedName) *v1beta1.OpenStackAnsibleEE {