Skip to content
This repository was archived by the owner on Nov 28, 2024. It is now read-only.

Commit b3e5d74

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #344 from jpodivin/webhookrewrite
Moving CRD checks into webhook
2 parents 1003b1a + f00ab1b commit b3e5d74

File tree

9 files changed

+233
-73
lines changed

9 files changed

+233
-73
lines changed

‎api/go.mod

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,12 @@ module github.com/openstack-k8s-operators/openstack-ansibleee-operator/api
33
go 1.20
44

55
require (
6+
github.com/go-playground/validator/v10 v10.19.0
67
github.com/onsi/ginkgo/v2 v2.17.1
78
github.com/onsi/gomega v1.32.0
89
github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240326081751-56015b1ae3f6
910
github.com/openstack-k8s-operators/lib-common/modules/storage v0.3.1-0.20240326081751-56015b1ae3f6
11+
gopkg.in/yaml.v2 v2.4.0
1012
k8s.io/api v0.28.8
1113
k8s.io/apimachinery v0.28.8
1214
k8s.io/client-go v0.28.8
@@ -20,11 +22,14 @@ require (
2022
github.com/emicklei/go-restful/v3 v3.11.0 // indirect
2123
github.com/evanphx/json-patch/v5 v5.6.0 // indirect
2224
github.com/fsnotify/fsnotify v1.6.0 // indirect
25+
github.com/gabriel-vasile/mimetype v1.4.3 // indirect
2326
github.com/go-logr/logr v1.4.1 // indirect
2427
github.com/go-logr/zapr v1.2.4 // indirect
2528
github.com/go-openapi/jsonpointer v0.19.6 // indirect
2629
github.com/go-openapi/jsonreference v0.20.2 // indirect
2730
github.com/go-openapi/swag v0.22.3 // indirect
31+
github.com/go-playground/locales v0.14.1 // indirect
32+
github.com/go-playground/universal-translator v0.18.1 // indirect
2833
github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572 // indirect
2934
github.com/gogo/protobuf v1.3.2 // indirect
3035
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
@@ -37,6 +42,7 @@ require (
3742
github.com/imdario/mergo v0.3.12 // indirect
3843
github.com/josharian/intern v1.0.0 // indirect
3944
github.com/json-iterator/go v1.1.12 // indirect
45+
github.com/leodido/go-urn v1.4.0 // indirect
4046
github.com/mailru/easyjson v0.7.7 // indirect
4147
github.com/matttproud/golang_protobuf_extensions v1.0.4 // indirect
4248
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
@@ -48,9 +54,9 @@ require (
4854
github.com/prometheus/common v0.44.0 // indirect
4955
github.com/prometheus/procfs v0.10.1 // indirect
5056
github.com/spf13/pflag v1.0.5 // indirect
51-
github.com/stretchr/testify v1.8.4 // indirect
5257
go.uber.org/multierr v1.11.0 // indirect
5358
go.uber.org/zap v1.27.0 // indirect
59+
golang.org/x/crypto v0.19.0 // indirect
5460
golang.org/x/exp v0.0.0-20240119083558-1b970713d09a // indirect
5561
golang.org/x/net v0.21.0 // indirect
5662
golang.org/x/oauth2 v0.8.0 // indirect
@@ -63,7 +69,6 @@ require (
6369
google.golang.org/appengine v1.6.7 // indirect
6470
google.golang.org/protobuf v1.33.0 // indirect
6571
gopkg.in/inf.v0 v0.9.1 // indirect
66-
gopkg.in/yaml.v2 v2.4.0 // indirect
6772
gopkg.in/yaml.v3 v3.0.1 // indirect
6873
k8s.io/apiextensions-apiserver v0.28.8 // indirect
6974
k8s.io/component-base v0.28.8 // indirect

‎api/go.sum

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ github.com/evanphx/json-patch/v5 v5.6.0 h1:b91NhWfaz02IuVxO9faSllyAtNXHMPkC5J8sJ
1717
github.com/evanphx/json-patch/v5 v5.6.0/go.mod h1:G79N1coSVB93tBe7j6PhzjmR3/2VvlbKOFpnXhI9Bw4=
1818
github.com/fsnotify/fsnotify v1.6.0 h1:n+5WquG0fcWoWp6xPWfHdbskMCQaFnG6PfBrh1Ky4HY=
1919
github.com/fsnotify/fsnotify v1.6.0/go.mod h1:sl3t1tCWJFWoRz9R8WJCbQihKKwmorjAbSClcnxKAGw=
20+
github.com/gabriel-vasile/mimetype v1.4.3 h1:in2uUcidCuFcDKtdcBxlR0rJ1+fsokWf+uqxgUFjbI0=
2021
github.com/go-logr/logr v1.2.4/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A=
2122
github.com/go-logr/logr v1.3.0/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY=
2223
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
2930
github.com/go-openapi/jsonreference v0.20.2/go.mod h1:Bl1zwGIM8/wsvqjsOQLJ/SH+En5Ap4rVB5KVcIDZG2k=
3031
github.com/go-openapi/swag v0.22.3 h1:yMBqmnQ0gyZvEb/+KzuWZOXgllrXT4SADYbvDaXHv/g=
3132
github.com/go-openapi/swag v0.22.3/go.mod h1:UzaqsxGiab7freDnrUUra0MwWfN/q7tE4j+VcZ0yl14=
33+
github.com/go-playground/assert/v2 v2.2.0 h1:JvknZsQTYeFEAhQwI4qEt9cyV5ONwRHC+lYKSsYSR8s=
34+
github.com/go-playground/locales v0.14.1 h1:EWaQ/wswjilfKLTECiXz7Rh+3BjFhfDFKv/oXslEjJA=
35+
github.com/go-playground/universal-translator v0.18.1 h1:Bcnm0ZwsGyWbCzImXv+pAJnYK9S473LQFuzCbDbfSFY=
36+
github.com/go-playground/validator/v10 v10.19.0 h1:ol+5Fu+cSq9JD7SoSqe04GMI92cbn0+wvQ3bZ8b/AU4=
3237
github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572 h1:tfuBGBXKqDEevZMzYi5KSi8KkcZtzBcTgAUUtapy0OI=
3338
github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572/go.mod h1:9Pwr4B2jHnOSGXyyzV8ROjYa2ojvAY6HCGYYfMoC3Ls=
3439
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=
6873
github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
6974
github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
7075
github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
76+
github.com/leodido/go-urn v1.4.0 h1:WT9HwE9SGECu3lg4d/dIA+jxlljEa1/ffXKmRjqdmIQ=
7177
github.com/mailru/easyjson v0.7.7 h1:UGYAvKxe3sBsEDzO8ZeWOSlIQfWFlxbzLZe7hwFURr0=
7278
github.com/mailru/easyjson v0.7.7/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc=
7379
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/
113119
github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
114120
github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
115121
github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
116-
github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
117122
github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
118123
github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
119124
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=
129134
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
130135
golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
131136
golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
137+
golang.org/x/crypto v0.19.0 h1:ENy+Az/9Y1vSrlrvBSyna3PITt4tiZLf7sgCjZBX7Wo=
132138
golang.org/x/exp v0.0.0-20240119083558-1b970713d09a h1:Q8/wZp0KX97QFTc2ywcOE0YRjZPVIx+MXInMzdvQqcA=
133139
golang.org/x/exp v0.0.0-20240119083558-1b970713d09a/go.mod h1:idGWGoKP1toJGkd5/ig9ZLuPcZBC3ewk7SzmH0uou08=
134140
golang.org/x/lint v0.0.0-20190930215403-16217165b5de/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc=

‎api/v1beta1/openstackansibleee_webhook.go

Lines changed: 93 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,14 @@ package v1beta1
2424

2525
import (
2626
"fmt"
27+
"regexp"
2728

29+
"github.com/go-playground/validator/v10"
30+
"gopkg.in/yaml.v2"
31+
apierrors "k8s.io/apimachinery/pkg/api/errors"
2832
"k8s.io/apimachinery/pkg/runtime"
33+
"k8s.io/apimachinery/pkg/runtime/schema"
34+
"k8s.io/apimachinery/pkg/util/validation/field"
2935
ctrl "sigs.k8s.io/controller-runtime"
3036
logf "sigs.k8s.io/controller-runtime/pkg/log"
3137
"sigs.k8s.io/controller-runtime/pkg/webhook"
@@ -81,13 +87,78 @@ var _ webhook.Validator = &OpenStackAnsibleEE{}
8187
// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
8288
func (r *OpenStackAnsibleEE) ValidateCreate() (admission.Warnings, error) {
8389
openstackansibleeelog.Info("validate create", "name", r.Name)
90+
var errors field.ErrorList
91+
92+
errors = append(errors, r.Spec.ValidateCreate()...)
93+
if len(errors) != 0 {
94+
openstackansibleeelog.Info("validation failed", "name", r.Name)
95+
return nil, apierrors.NewInvalid(
96+
schema.GroupKind{Group: "ansibleee.openstack.org", Kind: "OpenStakAnsibleEE"},
97+
r.Name,
98+
errors,
99+
)
100+
}
101+
return nil, nil
102+
}
103+
104+
func (spec *OpenStackAnsibleEESpec) ValidateCreate() field.ErrorList {
105+
// Setting up input validation, including custom validators
106+
validate := validator.New()
107+
var errors field.ErrorList
108+
if valErr := validate.RegisterValidation("play", isPlay); valErr != nil {
109+
errors = append(errors,
110+
field.InternalError(
111+
field.NewPath("spec"),
112+
fmt.Errorf("error registering input validation")))
113+
}
114+
if valErr := validate.RegisterValidation("fqcn", isFQCN); valErr != nil {
115+
errors = append(errors,
116+
field.InternalError(
117+
field.NewPath("spec"),
118+
fmt.Errorf("error registering input validation")))
119+
}
120+
if len(spec.Play) > 0 {
121+
valErr := validate.Var(spec.Play, "play")
122+
if valErr != nil {
123+
errors = append(errors, field.Invalid(
124+
field.NewPath("spec.play"),
125+
spec.Play,
126+
fmt.Sprintf(
127+
"error checking sanity of an inline play: %s %s",
128+
spec.Play, valErr),
129+
))
130+
}
131+
} else if len(spec.Playbook) > 0 {
132+
// As we set "playbook.yaml" as default
133+
// we need to ensure that Play and Role are empty before addPlaybook
134+
if validate.Var(spec.Playbook, "fqcn") != nil {
135+
// First we check if the playbook isn't imported from a collection
136+
// if it is not we assume the variable holds a path.
137+
valErr := validate.Var(spec.Playbook, "filepath")
138+
if valErr != nil {
139+
errors = append(errors, field.Invalid(
140+
field.NewPath("spec.playbook"),
141+
spec.Playbook,
142+
fmt.Sprintf(
143+
"error checking sanity of playbook name/path: %s %s",
144+
spec.Playbook, valErr),
145+
))
146+
}
147+
}
84148

85-
for _, value := range r.Spec.Env {
149+
}
150+
151+
for _, value := range spec.Env {
86152
if value.Name == "ANSIBLE_ENABLE_TASK_DEBUGGER" {
87-
return nil, fmt.Errorf("ansible-runner does not support ANSIBLE_ENABLE_TASK_DEBUGGER")
153+
errors = append(errors, field.Invalid(
154+
field.NewPath("spec.env"),
155+
spec.Env,
156+
fmt.Sprintf("ansible-runner does not support ANSIBLE_ENABLE_TASK_DEBUGGER"),
157+
))
88158
}
89159
}
90-
return nil, nil
160+
161+
return errors
91162
}
92163

93164
// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
@@ -105,3 +176,22 @@ func (r *OpenStackAnsibleEE) ValidateDelete() (admission.Warnings, error) {
105176
// TODO(user): fill in your validation logic upon object deletion.
106177
return nil, nil
107178
}
179+
180+
// isPlay checks if the free form document has attributes of ansible play
181+
// Specifically if it is a parsable yaml with list as a root element
182+
func isPlay(document validator.FieldLevel) bool {
183+
var play []map[string]interface{}
184+
err := yaml.Unmarshal([]byte(document.Field().String()), &play)
185+
return err == nil
186+
}
187+
188+
// isFQCN checks if the string matches regular expression of ansible FQCN
189+
// The function doesn't check if the collection exists or is accessible
190+
// function only accepts FQCNs as defined by
191+
// https://old-galaxy.ansible.com/docs/contributing/namespaces.html#namespace-limitations
192+
// Regex derived from ansible-lint rule
193+
func isFQCN(name validator.FieldLevel) bool {
194+
pattern, compileErr := regexp.Compile(`^\w+(\.\w+){2,}$`)
195+
match := pattern.Match([]byte(name.Field().String()))
196+
return match && compileErr == nil
197+
}

‎controllers/openstack_ansibleee_controller.go

Lines changed: 1 addition & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ package controllers
1818

1919
import (
2020
"fmt"
21-
"regexp"
2221
"sort"
2322
"strings"
2423
"time"
@@ -33,7 +32,6 @@ import (
3332
"context"
3433

3534
"github.com/go-logr/logr"
36-
"github.com/go-playground/validator/v10"
3735
"github.com/openstack-k8s-operators/lib-common/modules/common/condition"
3836
helper "github.com/openstack-k8s-operators/lib-common/modules/common/helper"
3937
job "github.com/openstack-k8s-operators/lib-common/modules/common/job"
@@ -271,16 +269,6 @@ func (r *OpenStackAnsibleEEReconciler) jobForOpenStackAnsibleEE(ctx context.Cont
271269
Log := r.GetLogger(ctx)
272270
labels := instance.GetObjectMeta().GetLabels()
273271

274-
// Setting up input validation, including custom validators
275-
validate := validator.New()
276-
277-
if valErr := validate.RegisterValidation("play", isPlay); valErr != nil {
278-
return nil, fmt.Errorf("error registering input validation")
279-
}
280-
if valErr := validate.RegisterValidation("fqcn", isFQCN); valErr != nil {
281-
return nil, fmt.Errorf("error registering input validation")
282-
}
283-
284272
ls := labelsForOpenStackAnsibleEE(instance.Name, labels)
285273

286274
args := instance.Spec.Args
@@ -373,27 +361,10 @@ func (r *OpenStackAnsibleEEReconciler) jobForOpenStackAnsibleEE(ctx context.Cont
373361
}
374362

375363
if len(instance.Spec.Play) > 0 {
376-
valErr := validate.Var(instance.Spec.Play, "play")
377-
if valErr != nil {
378-
return nil, fmt.Errorf(
379-
"error checking sanity of an inline play: %s %w",
380-
instance.Spec.Play, valErr)
381-
}
382364
setRunnerEnvVar(h, "RUNNER_PLAYBOOK", instance.Spec.Play, "play", job, hashes)
383365
} else if len(playbook) > 0 {
384366
// As we set "playbook.yaml" as default
385-
// we need to ensure that Play and Role are empty before addPlaybook
386-
if validate.Var(playbook, "fqcn") != nil {
387-
// First we check if the playbook isn't imported from a collection
388-
// if it is not we assume the variable holds a path.
389-
valErr := validate.Var(playbook, "filepath")
390-
if valErr != nil {
391-
return nil, fmt.Errorf(
392-
"error checking sanity of playbook name/path: %s %w",
393-
playbook, valErr)
394-
}
395-
}
396-
367+
// we need to ensure that Play is empty before adding playbook
397368
setRunnerEnvVar(h, "RUNNER_PLAYBOOK", playbook, "playbooks", job, hashes)
398369
}
399370

@@ -556,22 +527,3 @@ func (r *OpenStackAnsibleEEReconciler) SetupWithManager(mgr ctrl.Manager) error
556527
Owns(&batchv1.Job{}).
557528
Complete(r)
558529
}
559-
560-
// isPlay checks if the free form document has attributes of ansible play
561-
// Specifically if it is a parsable yaml with list as a root element
562-
func isPlay(document validator.FieldLevel) bool {
563-
var play []map[string]interface{}
564-
err := yaml.Unmarshal([]byte(document.Field().String()), &play)
565-
return err == nil
566-
}
567-
568-
// isFQCN checks if the string matches regular expression of ansible FQCN
569-
// The function doesn't check if the collection exists or is accessible
570-
// function only accepts FQCNs as defined by
571-
// https://galaxy.ansible.com/docs/contributing/namespaces.html#namespace-limitations
572-
// Regex derive
573-
func isFQCN(name validator.FieldLevel) bool {
574-
pattern, compileErr := regexp.Compile(`^\w+(\.\w+){2,}$`)
575-
match := pattern.Match([]byte(name.Field().String()))
576-
return match && compileErr == nil
577-
}

‎go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ replace github.com/openstack-k8s-operators/openstack-ansibleee-operator/api => .
66

77
require (
88
github.com/go-logr/logr v1.4.1
9-
github.com/go-playground/validator/v10 v10.19.0
109
github.com/google/uuid v1.6.0
1110
github.com/k8snetworkplumbingwg/network-attachment-definition-client v1.4.0
1211
github.com/onsi/ginkgo/v2 v2.17.1
@@ -35,6 +34,7 @@ require (
3534
github.com/go-openapi/swag v0.22.3 // indirect
3635
github.com/go-playground/locales v0.14.1 // indirect
3736
github.com/go-playground/universal-translator v0.18.1 // indirect
37+
github.com/go-playground/validator/v10 v10.19.0 // indirect
3838
github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572 // indirect
3939
github.com/gogo/protobuf v1.3.2 // indirect
4040
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect

‎tests/functional/ansibleee_controller_test.go

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -188,23 +188,6 @@ var _ = Describe("Ansibleee controller", func() {
188188

189189
})
190190

191-
Context("with invalid playbook name/path", func() {
192-
BeforeEach(func() {
193-
DeferCleanup(th.DeleteInstance, CreateAnsibleeeWithParams(
194-
ansibleeeName, "/", "test-image", "", "", map[string]interface{}{}, []map[string]interface{}{}))
195-
})
196-
It("runs a job and reports when it succeeds", func() {
197-
th.ExpectConditionWithDetails(
198-
ansibleeeName,
199-
ConditionGetterFunc(AnsibleeeConditionGetter),
200-
v1beta1.AnsibleExecutionJobReadyCondition,
201-
corev1.ConditionUnknown,
202-
condition.InitReason,
203-
"AnsibleExecutionJob not started",
204-
)
205-
})
206-
})
207-
208191
Context("with an inline play", func() {
209192
BeforeEach(func() {
210193
DeferCleanup(th.DeleteInstance, CreateAnsibleeeWithParams(

‎tests/functional/base_test.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,15 @@ const (
3434
- name: Using debug statement
3535
ansible.builtin.debug:
3636
msg: "Hello, world this is ansibleee-play.yaml"`
37+
38+
// This constant is meant to represent malformed inline play
39+
malformedPlay = `
40+
- name: Print hello world
41+
hosts: all
42+
tasks:
43+
- name: Using debug statement
44+
ansible.builtin.debug:
45+
msg: "Hello, world this is ansibleee-play.yaml"`
3746
)
3847

3948
func GetAnsibleee(name types.NamespacedName) *v1beta1.OpenStackAnsibleEE {

0 commit comments

Comments
 (0)