From 911f85f5bd1dfcc1754f65395ce33494e1df25bf Mon Sep 17 00:00:00 2001 From: Geoffrey Israel Date: Mon, 12 Jun 2023 09:39:30 +0100 Subject: [PATCH] feat(operator): adapt TaskDefinition validation webhook to consider python and deno runtime (#1534) Signed-off-by: RealAnna <89971034+RealAnna@users.noreply.github.com> Co-authored-by: RealAnna <89971034+RealAnna@users.noreply.github.com> --- .../v1alpha3/keptntaskdefinition_webhook.go | 27 ++- .../keptntaskdefinition_webhook_test.go | 210 +++++++++++++++++- .../00-teststep-install.yaml | 19 +- .../01-teststep-assert.yaml | 13 +- ...nition.yaml => td_bad_container_deno.yaml} | 13 +- ...on.yaml => td_bad_container_function.yaml} | 11 +- .../td_bad_container_python.yaml | 24 ++ .../validate-taskdefinition/td_bad_empty.yaml | 6 + .../td_bad_function_deno.yaml | 14 ++ .../td_bad_function_python.yaml | 14 ++ .../td_bad_python_deno.yaml | 14 ++ .../td_good_container.yaml | 19 ++ .../validate-taskdefinition/td_good_deno.yaml | 9 + .../td_good_function.yaml | 9 + .../td_good_python.yaml | 9 + 15 files changed, 381 insertions(+), 30 deletions(-) rename test/integration/validate-taskdefinition/{badtaskdefinition.yaml => td_bad_container_deno.yaml} (64%) rename test/integration/validate-taskdefinition/{goodtaskdefinition.yaml => td_bad_container_function.yaml} (69%) create mode 100644 test/integration/validate-taskdefinition/td_bad_container_python.yaml create mode 100644 test/integration/validate-taskdefinition/td_bad_empty.yaml create mode 100644 test/integration/validate-taskdefinition/td_bad_function_deno.yaml create mode 100644 test/integration/validate-taskdefinition/td_bad_function_python.yaml create mode 100644 test/integration/validate-taskdefinition/td_bad_python_deno.yaml create mode 100644 test/integration/validate-taskdefinition/td_good_container.yaml create mode 100644 test/integration/validate-taskdefinition/td_good_deno.yaml create mode 100644 test/integration/validate-taskdefinition/td_good_function.yaml create mode 100644 test/integration/validate-taskdefinition/td_good_python.yaml diff --git a/operator/apis/lifecycle/v1alpha3/keptntaskdefinition_webhook.go b/operator/apis/lifecycle/v1alpha3/keptntaskdefinition_webhook.go index d06601747c..5512c74533 100644 --- a/operator/apis/lifecycle/v1alpha3/keptntaskdefinition_webhook.go +++ b/operator/apis/lifecycle/v1alpha3/keptntaskdefinition_webhook.go @@ -77,22 +77,39 @@ func (r *KeptnTaskDefinition) validateKeptnTaskDefinition() error { allErrs) } func (r *KeptnTaskDefinition) validateFields() *field.Error { - - if r.Spec.Function == nil && r.Spec.Container == nil { + count := countSpec(r) + if count == 0 { return field.Invalid( field.NewPath("spec"), r.Spec, - errors.New("Forbidden! Either Function or Container field must be defined").Error(), + errors.New("Forbidden! Either Function, Container, Python, or Deno field must be defined").Error(), ) } - if r.Spec.Function != nil && r.Spec.Container != nil { + if count > 1 { return field.Invalid( field.NewPath("spec"), r.Spec, - errors.New("Forbidden! Both Function and Container fields cannot be defined simultaneously").Error(), + errors.New("Forbidden! Only one of Function, Container, Python, or Deno field can be defined").Error(), ) } return nil } + +func countSpec(r *KeptnTaskDefinition) int { + count := 0 + if r.Spec.Function != nil { + count++ + } + if r.Spec.Container != nil { + count++ + } + if r.Spec.Python != nil { + count++ + } + if r.Spec.Deno != nil { + count++ + } + return count +} diff --git a/operator/apis/lifecycle/v1alpha3/keptntaskdefinition_webhook_test.go b/operator/apis/lifecycle/v1alpha3/keptntaskdefinition_webhook_test.go index b0db12eca9..e636389585 100644 --- a/operator/apis/lifecycle/v1alpha3/keptntaskdefinition_webhook_test.go +++ b/operator/apis/lifecycle/v1alpha3/keptntaskdefinition_webhook_test.go @@ -19,6 +19,31 @@ func TestKeptnTaskDefinition_ValidateFields(t *testing.T) { Container: &ContainerSpec{}, } + specWithFunctionAndPython := KeptnTaskDefinitionSpec{ + Function: &RuntimeSpec{}, + Python: &RuntimeSpec{}, + } + + specWithFunctionAndDeno := KeptnTaskDefinitionSpec{ + Function: &RuntimeSpec{}, + Deno: &RuntimeSpec{}, + } + + specWithContainerAndPython := KeptnTaskDefinitionSpec{ + Container: &ContainerSpec{}, + Python: &RuntimeSpec{}, + } + + specWithContainerAndDeno := KeptnTaskDefinitionSpec{ + Container: &ContainerSpec{}, + Deno: &RuntimeSpec{}, + } + + specWithPythonAndDeno := KeptnTaskDefinitionSpec{ + Python: &RuntimeSpec{}, + Deno: &RuntimeSpec{}, + } + emptySpec := KeptnTaskDefinitionSpec{} tests := []struct { @@ -29,15 +54,15 @@ func TestKeptnTaskDefinition_ValidateFields(t *testing.T) { oldSpec runtime.Object }{ { - name: "with-no-function-or-container", + name: "with-no-function-or-container-or-python-or-deno", spec: emptySpec, want: apierrors.NewInvalid( schema.GroupKind{Group: "lifecycle.keptn.sh", Kind: "KeptnTaskDefinition"}, - "with-no-function-or-container", + "with-no-function-or-container-or-python-or-deno", []*field.Error{field.Invalid( field.NewPath("spec"), emptySpec, - errors.New("Forbidden! Either Function or Container field must be defined").Error(), + errors.New("Forbidden! Either Function, Container, Python, or Deno field must be defined").Error(), )}, ), verb: "create", @@ -52,7 +77,7 @@ func TestKeptnTaskDefinition_ValidateFields(t *testing.T) { []*field.Error{field.Invalid( field.NewPath("spec"), specWithFunctionAndContainer, - errors.New("Forbidden! Both Function and Container fields cannot be defined simultaneously").Error(), + errors.New("Forbidden! Only one of Function, Container, Python, or Deno field can be defined").Error(), )}, ), }, @@ -70,6 +95,21 @@ func TestKeptnTaskDefinition_ValidateFields(t *testing.T) { }, verb: "create", }, + { + name: "with-python-only", + spec: KeptnTaskDefinitionSpec{ + Python: &RuntimeSpec{}, + }, + verb: "create", + }, + { + name: "with-deno-only", + spec: KeptnTaskDefinitionSpec{ + Deno: &RuntimeSpec{}, + }, + verb: "create", + }, + { name: "update-with-both-function-and-container", spec: specWithFunctionAndContainer, @@ -79,7 +119,71 @@ func TestKeptnTaskDefinition_ValidateFields(t *testing.T) { []*field.Error{field.Invalid( field.NewPath("spec"), specWithFunctionAndContainer, - errors.New("Forbidden! Both Function and Container fields cannot be defined simultaneously").Error(), + errors.New("Forbidden! Only one of Function, Container, Python, or Deno field can be defined").Error(), + )}, + ), + oldSpec: &KeptnTaskDefinition{ + Spec: KeptnTaskDefinitionSpec{}, + }, + verb: "update", + }, + + { + name: "with-both-function-and-python", + spec: specWithFunctionAndPython, + verb: "create", + want: apierrors.NewInvalid( + schema.GroupKind{Group: "lifecycle.keptn.sh", Kind: "KeptnTaskDefinition"}, + "with-both-function-and-python", + []*field.Error{field.Invalid( + field.NewPath("spec"), + specWithFunctionAndPython, + errors.New("Forbidden! Only one of Function, Container, Python, or Deno field can be defined").Error(), + )}, + ), + }, + { + name: "update-with-both-function-and-python", + spec: specWithFunctionAndPython, + want: apierrors.NewInvalid( + schema.GroupKind{Group: "lifecycle.keptn.sh", Kind: "KeptnTaskDefinition"}, + "update-with-both-function-and-python", + []*field.Error{field.Invalid( + field.NewPath("spec"), + specWithFunctionAndPython, + errors.New("Forbidden! Only one of Function, Container, Python, or Deno field can be defined").Error(), + )}, + ), + oldSpec: &KeptnTaskDefinition{ + Spec: KeptnTaskDefinitionSpec{}, + }, + verb: "update", + }, + + { + name: "with-both-function-and-deno", + spec: specWithFunctionAndDeno, + verb: "create", + want: apierrors.NewInvalid( + schema.GroupKind{Group: "lifecycle.keptn.sh", Kind: "KeptnTaskDefinition"}, + "with-both-function-and-deno", + []*field.Error{field.Invalid( + field.NewPath("spec"), + specWithFunctionAndDeno, + errors.New("Forbidden! Only one of Function, Container, Python, or Deno field can be defined").Error(), + )}, + ), + }, + { + name: "update-with-both-function-and-deno", + spec: specWithFunctionAndDeno, + want: apierrors.NewInvalid( + schema.GroupKind{Group: "lifecycle.keptn.sh", Kind: "KeptnTaskDefinition"}, + "update-with-both-function-and-deno", + []*field.Error{field.Invalid( + field.NewPath("spec"), + specWithFunctionAndDeno, + errors.New("Forbidden! Only one of Function, Container, Python, or Deno field can be defined").Error(), )}, ), oldSpec: &KeptnTaskDefinition{ @@ -87,6 +191,102 @@ func TestKeptnTaskDefinition_ValidateFields(t *testing.T) { }, verb: "update", }, + + { + name: "with-both-container-and-python", + spec: specWithContainerAndPython, + verb: "create", + want: apierrors.NewInvalid( + schema.GroupKind{Group: "lifecycle.keptn.sh", Kind: "KeptnTaskDefinition"}, + "with-both-container-and-python", + []*field.Error{field.Invalid( + field.NewPath("spec"), + specWithContainerAndPython, + errors.New("Forbidden! Only one of Function, Container, Python, or Deno field can be defined").Error(), + )}, + ), + }, + { + name: "update-with-both-container-and-python", + spec: specWithContainerAndPython, + want: apierrors.NewInvalid( + schema.GroupKind{Group: "lifecycle.keptn.sh", Kind: "KeptnTaskDefinition"}, + "update-with-both-container-and-python", + []*field.Error{field.Invalid( + field.NewPath("spec"), + specWithContainerAndPython, + errors.New("Forbidden! Only one of Function, Container, Python, or Deno field can be defined").Error(), + )}, + ), + oldSpec: &KeptnTaskDefinition{ + Spec: KeptnTaskDefinitionSpec{}, + }, + verb: "update", + }, + + { + name: "with-both-container-and-deno", + spec: specWithContainerAndDeno, + verb: "create", + want: apierrors.NewInvalid( + schema.GroupKind{Group: "lifecycle.keptn.sh", Kind: "KeptnTaskDefinition"}, + "with-both-container-and-deno", + []*field.Error{field.Invalid( + field.NewPath("spec"), + specWithContainerAndDeno, + errors.New("Forbidden! Only one of Function, Container, Python, or Deno field can be defined").Error(), + )}, + ), + }, + { + name: "update-with-both-container-and-deno", + spec: specWithContainerAndDeno, + want: apierrors.NewInvalid( + schema.GroupKind{Group: "lifecycle.keptn.sh", Kind: "KeptnTaskDefinition"}, + "update-with-both-container-and-deno", + []*field.Error{field.Invalid( + field.NewPath("spec"), + specWithContainerAndDeno, + errors.New("Forbidden! Only one of Function, Container, Python, or Deno field can be defined").Error(), + )}, + ), + oldSpec: &KeptnTaskDefinition{ + Spec: KeptnTaskDefinitionSpec{}, + }, + verb: "update", + }, + { + name: "with-both-python-and-deno", + spec: specWithPythonAndDeno, + verb: "create", + want: apierrors.NewInvalid( + schema.GroupKind{Group: "lifecycle.keptn.sh", Kind: "KeptnTaskDefinition"}, + "with-both-python-and-deno", + []*field.Error{field.Invalid( + field.NewPath("spec"), + specWithPythonAndDeno, + errors.New("Forbidden! Only one of Function, Container, Python, or Deno field can be defined").Error(), + )}, + ), + }, + { + name: "update-with-both-python-and-deno", + spec: specWithPythonAndDeno, + want: apierrors.NewInvalid( + schema.GroupKind{Group: "lifecycle.keptn.sh", Kind: "KeptnTaskDefinition"}, + "update-with-both-python-and-deno", + []*field.Error{field.Invalid( + field.NewPath("spec"), + specWithPythonAndDeno, + errors.New("Forbidden! Only one of Function, Container, Python, or Deno field can be defined").Error(), + )}, + ), + oldSpec: &KeptnTaskDefinition{ + Spec: KeptnTaskDefinitionSpec{}, + }, + verb: "update", + }, + { name: "delete", verb: "delete", diff --git a/test/integration/validate-taskdefinition/00-teststep-install.yaml b/test/integration/validate-taskdefinition/00-teststep-install.yaml index a65f372148..6a7aa16669 100644 --- a/test/integration/validate-taskdefinition/00-teststep-install.yaml +++ b/test/integration/validate-taskdefinition/00-teststep-install.yaml @@ -1,7 +1,22 @@ apiVersion: kuttl.dev/v1beta1 kind: TestStep apply: - - goodtaskdefinition.yaml + - td_good_container.yaml + - td_good_function.yaml + - td_good_python.yaml + - td_good_deno.yaml commands: - - command: kubectl apply -f badtaskdefinition.yaml + - command: kubectl apply -f td_bad_empty.yaml + ignoreFailure: true # we must install ignoring the validating webhook error to proceed with the test + - command: kubectl apply -f td_bad_container_function.yaml + ignoreFailure: true # we must install ignoring the validating webhook error to proceed with the test + - command: kubectl apply -f td_bad_container_python.yaml + ignoreFailure: true # we must install ignoring the validating webhook error to proceed with the test + - command: kubectl apply -f td_bad_container_deno.yaml + ignoreFailure: true # we must install ignoring the validating webhook error to proceed with the test + - command: kubectl apply -f td_bad_function_python.yaml + ignoreFailure: true # we must install ignoring the validating webhook error to proceed with the test + - command: kubectl apply -f td_bad_function_deno.yaml + ignoreFailure: true # we must install ignoring the validating webhook error to proceed with the test + - command: kubectl apply -f td_bad_python_deno.yaml ignoreFailure: true # we must install ignoring the validating webhook error to proceed with the test diff --git a/test/integration/validate-taskdefinition/01-teststep-assert.yaml b/test/integration/validate-taskdefinition/01-teststep-assert.yaml index 92a0b614a1..725a0e7bd1 100644 --- a/test/integration/validate-taskdefinition/01-teststep-assert.yaml +++ b/test/integration/validate-taskdefinition/01-teststep-assert.yaml @@ -1,6 +1,15 @@ apiVersion: kuttl.dev/v1beta1 kind: TestStep error: # this checks that kubectl get resource fails, AKA bad CRD not added - - badtaskdefinition.yaml + - td_bad_empty.yaml + - td_bad_container_function.yaml + - td_bad_container_python.yaml + - td_bad_container_deno.yaml + - td_bad_function_python.yaml + - td_bad_function_deno.yaml + - td_bad_python_deno.yaml assert: # this checks that kubectl get resource succeeds - - goodtaskdefinition.yaml + - td_good_container.yaml + - td_good_function.yaml + - td_good_python.yaml + - td_good_deno.yaml diff --git a/test/integration/validate-taskdefinition/badtaskdefinition.yaml b/test/integration/validate-taskdefinition/td_bad_container_deno.yaml similarity index 64% rename from test/integration/validate-taskdefinition/badtaskdefinition.yaml rename to test/integration/validate-taskdefinition/td_bad_container_deno.yaml index 3ddb285e7a..f66276903b 100644 --- a/test/integration/validate-taskdefinition/badtaskdefinition.yaml +++ b/test/integration/validate-taskdefinition/td_bad_container_deno.yaml @@ -2,7 +2,7 @@ apiVersion: lifecycle.keptn.sh/v1alpha3 kind: KeptnTaskDefinition metadata: - name: badtaskdefinition1 + name: badtaskdefinition4 spec: container: name: keptntaskdefinition @@ -18,10 +18,7 @@ spec: volumeMounts: - mountPath: /cache name: logger ---- -# This TaskDefinition will not be accepted by the validation webhook as it doesn't contain either containerSpec or functionSpec -apiVersion: lifecycle.keptn.sh/v1alpha3 -kind: KeptnTaskDefinition -metadata: - name: badtaskdefinition2 -spec: + deno: + inline: + code: | + console.log('hello'); diff --git a/test/integration/validate-taskdefinition/goodtaskdefinition.yaml b/test/integration/validate-taskdefinition/td_bad_container_function.yaml similarity index 69% rename from test/integration/validate-taskdefinition/goodtaskdefinition.yaml rename to test/integration/validate-taskdefinition/td_bad_container_function.yaml index 519f5dcbee..d9ec72b2fd 100644 --- a/test/integration/validate-taskdefinition/goodtaskdefinition.yaml +++ b/test/integration/validate-taskdefinition/td_bad_container_function.yaml @@ -1,10 +1,11 @@ +# This TaskDefinition will not be accepted by the validation webhook as it contains both containerSpec and functionSpec apiVersion: lifecycle.keptn.sh/v1alpha3 kind: KeptnTaskDefinition metadata: - name: goodtaskdefinition1 + name: badtaskdefinition1 spec: container: - name: keptntaskdefinition1 + name: keptntaskdefinition image: busybox:1.36.0 resources: limits: @@ -17,12 +18,6 @@ spec: volumeMounts: - mountPath: /cache name: logger ---- -apiVersion: lifecycle.keptn.sh/v1alpha3 -kind: KeptnTaskDefinition -metadata: - name: goodtaskdefinition2 -spec: function: inline: code: | diff --git a/test/integration/validate-taskdefinition/td_bad_container_python.yaml b/test/integration/validate-taskdefinition/td_bad_container_python.yaml new file mode 100644 index 0000000000..9d6e552a44 --- /dev/null +++ b/test/integration/validate-taskdefinition/td_bad_container_python.yaml @@ -0,0 +1,24 @@ +# This TaskDefinition will not be accepted by the validation webhook as it contains both containerSpec and functionSpec +apiVersion: lifecycle.keptn.sh/v1alpha3 +kind: KeptnTaskDefinition +metadata: + name: badtaskdefinition3 +spec: + container: + name: keptntaskdefinition + image: busybox:1.36.0 + resources: + limits: + memory: "200Mi" + command: + - 'echo' + - 'Hello World!' + - '>' + - '/cache/log.txt' + volumeMounts: + - mountPath: /cache + name: logger + python: + inline: + code: | + print('hello') diff --git a/test/integration/validate-taskdefinition/td_bad_empty.yaml b/test/integration/validate-taskdefinition/td_bad_empty.yaml new file mode 100644 index 0000000000..825f609a30 --- /dev/null +++ b/test/integration/validate-taskdefinition/td_bad_empty.yaml @@ -0,0 +1,6 @@ +# This TaskDefinition will not be accepted by the validation webhook as it doesn't contain either containerSpec or functionSpec +apiVersion: lifecycle.keptn.sh/v1alpha3 +kind: KeptnTaskDefinition +metadata: + name: badtaskdefinition2 +spec: diff --git a/test/integration/validate-taskdefinition/td_bad_function_deno.yaml b/test/integration/validate-taskdefinition/td_bad_function_deno.yaml new file mode 100644 index 0000000000..faeb29eadb --- /dev/null +++ b/test/integration/validate-taskdefinition/td_bad_function_deno.yaml @@ -0,0 +1,14 @@ +# This TaskDefinition will not be accepted by the validation webhook as it contains both containerSpec and functionSpec +apiVersion: lifecycle.keptn.sh/v1alpha3 +kind: KeptnTaskDefinition +metadata: + name: badtaskdefinition6 +spec: + function: + inline: + code: | + console.log('hello'); + deno: + inline: + code: | + console.log('hello'); diff --git a/test/integration/validate-taskdefinition/td_bad_function_python.yaml b/test/integration/validate-taskdefinition/td_bad_function_python.yaml new file mode 100644 index 0000000000..5fe710a220 --- /dev/null +++ b/test/integration/validate-taskdefinition/td_bad_function_python.yaml @@ -0,0 +1,14 @@ +# This TaskDefinition will not be accepted by the validation webhook as it contains both containerSpec and functionSpec +apiVersion: lifecycle.keptn.sh/v1alpha3 +kind: KeptnTaskDefinition +metadata: + name: badtaskdefinition5 +spec: + function: + inline: + code: | + console.log('hello'); + python: + inline: + code: | + print('hello') diff --git a/test/integration/validate-taskdefinition/td_bad_python_deno.yaml b/test/integration/validate-taskdefinition/td_bad_python_deno.yaml new file mode 100644 index 0000000000..91bbda0428 --- /dev/null +++ b/test/integration/validate-taskdefinition/td_bad_python_deno.yaml @@ -0,0 +1,14 @@ +# This TaskDefinition will not be accepted by the validation webhook as it contains both containerSpec and functionSpec +apiVersion: lifecycle.keptn.sh/v1alpha3 +kind: KeptnTaskDefinition +metadata: + name: badtaskdefinition7 +spec: + python: + inline: + code: | + print('hello') + deno: + inline: + code: | + console.log('hello'); diff --git a/test/integration/validate-taskdefinition/td_good_container.yaml b/test/integration/validate-taskdefinition/td_good_container.yaml new file mode 100644 index 0000000000..09ba626c36 --- /dev/null +++ b/test/integration/validate-taskdefinition/td_good_container.yaml @@ -0,0 +1,19 @@ +apiVersion: lifecycle.keptn.sh/v1alpha3 +kind: KeptnTaskDefinition +metadata: + name: goodtaskdefinition1 +spec: + container: + name: keptntaskdefinition1 + image: busybox:1.36.0 + resources: + limits: + memory: "200Mi" + command: + - 'echo' + - 'Hello World!' + - '>' + - '/cache/log.txt' + volumeMounts: + - mountPath: /cache + name: logger diff --git a/test/integration/validate-taskdefinition/td_good_deno.yaml b/test/integration/validate-taskdefinition/td_good_deno.yaml new file mode 100644 index 0000000000..1d9e592869 --- /dev/null +++ b/test/integration/validate-taskdefinition/td_good_deno.yaml @@ -0,0 +1,9 @@ +apiVersion: lifecycle.keptn.sh/v1alpha3 +kind: KeptnTaskDefinition +metadata: + name: goodtaskdefinition4 +spec: + deno: + inline: + code: | + console.log('hello'); diff --git a/test/integration/validate-taskdefinition/td_good_function.yaml b/test/integration/validate-taskdefinition/td_good_function.yaml new file mode 100644 index 0000000000..1482887faa --- /dev/null +++ b/test/integration/validate-taskdefinition/td_good_function.yaml @@ -0,0 +1,9 @@ +apiVersion: lifecycle.keptn.sh/v1alpha3 +kind: KeptnTaskDefinition +metadata: + name: goodtaskdefinition2 +spec: + function: + inline: + code: | + console.log('hello'); diff --git a/test/integration/validate-taskdefinition/td_good_python.yaml b/test/integration/validate-taskdefinition/td_good_python.yaml new file mode 100644 index 0000000000..19383bec89 --- /dev/null +++ b/test/integration/validate-taskdefinition/td_good_python.yaml @@ -0,0 +1,9 @@ +apiVersion: lifecycle.keptn.sh/v1alpha3 +kind: KeptnTaskDefinition +metadata: + name: goodtaskdefinition3 +spec: + python: + inline: + code: | + print('hello')