Address reviews

Signed-off-by: Sergen Yalçın <[email protected]>

.github/workflows/pr-comment-trigger.yml

@@ -18,6 +18,12 @@ on:
         required: false
         type: string
       update-test-parameter:
+        description: 'Input parameter to use during update step. If this field
+        is empty, then the update step will be skipped.
+        The update parameter should be a serialized JSON object and if the JSON
+        object has nested children, then there must be only one leaf.
+        Example: {"tags":{"uptest":"update"}}
+        This parameter will add a tag to the test resource.'
         default: ''
         required: false
         type: string

internal/config/config.go

@@ -38,6 +38,9 @@ const (
 	// AnnotationKeyUpdateParameter defines the update parameter that will be
 	// used during the update step
 	AnnotationKeyUpdateParameter = "uptest.upbound.io/update-parameter"
+	// AnnotationKeyExampleId is id of example that populated from example
+	// manifest. This information will be used for determining the root resource
+	AnnotationKeyExampleId = "meta.upbound.io/example-id"
 )
 
 // AutomatedTest represents an automated test of resource example
@@ -87,4 +90,6 @@ type Resource struct {
 	UpdateParameter   string
 	UpdateAssertKey   string
 	UpdateAssertValue string
+
+	Root bool
 }

internal/templates/00-apply.yaml.tmpl

@@ -1,3 +1,4 @@
+# This file belongs to the resource apply step.
 {{ if .TestCase.SetupScriptPath -}}
 apiVersion: kuttl.dev/v1beta1
 kind: TestStep

internal/templates/00-assert.yaml.tmpl

@@ -1,9 +1,11 @@
+# This assert file belongs to the resource apply step.
 apiVersion: kuttl.dev/v1beta1
 kind: TestAssert
 timeout: {{ .TestCase.Timeout }}
 commands:
 - command: ${KUBECTL} annotate managed --all upjet.upbound.io/test=true --overwrite
-- command: ${KUBECTL} get managed -o yaml
+- script: echo "Dump MR manifests for the apply assertion step:"; ${KUBECTL} get managed -o yaml
+- script: echo "Dump Claim manifests for the apply assertion step:" || ${KUBECTL} get claim --all-namespaces -o yaml
 {{- range $resource := .Resources }}
 {{- if eq $resource.KindGroup "secret." -}}
   {{continue}}
@@ -13,7 +15,7 @@ commands:
 {{- end }}
 {{- range $condition := $resource.Conditions }}
 {{- if $resource.Namespace }}
-- script: ${KUBECTL} get --namespace {{ $resource.Namespace }} {{ $resource.KindGroup }}/{{ $resource.Name }} -o yaml && ${KUBECTL} wait {{ $resource.KindGroup }}/{{ $resource.Name }} --for=condition={{ $condition }} --timeout 10s --namespace {{ $resource.Namespace }}
+- command: ${KUBECTL} wait {{ $resource.KindGroup }}/{{ $resource.Name }} --for=condition={{ $condition }} --timeout 10s --namespace {{ $resource.Namespace }}
 {{- else }}
 - command: ${KUBECTL} wait {{ $resource.KindGroup }}/{{ $resource.Name }} --for=condition={{ $condition }} --timeout 10s
 {{- end }}

internal/templates/01-assert.yaml.tmpl

@@ -1,8 +1,9 @@
+# This assert file belongs to the resource update step.
 apiVersion: kuttl.dev/v1beta1
 kind: TestAssert
 timeout: {{ .TestCase.Timeout }}
 commands:
-- command: ${KUBECTL} get managed -o yaml
+- script: echo "Dump MR manifests for the update assertion step:"; ${KUBECTL} get managed -o yaml
 {{- range $resource := .Resources }}
 {{- if eq $resource.UpdateParameter "" -}}
   {{continue}}
@@ -11,6 +12,8 @@ commands:
   {{continue}}
 {{- end -}}
 {{- if not $resource.Namespace }}
+{{- if $resource.Root }}
 - script: ${KUBECTL} get {{ $resource.KindGroup }}/{{ $resource.Name }} -o=jsonpath='{.status.atProvider{{ $resource.UpdateAssertKey }}}' | grep -q "^{{ $resource.UpdateAssertValue }}$"
 {{- end }}
 {{- end }}
+{{- end }}

internal/templates/01-update.yaml.tmpl

@@ -1,3 +1,4 @@
+# This file belongs to the resource update step.
 apiVersion: kuttl.dev/v1beta1
 kind: TestStep
 commands:
@@ -9,6 +10,8 @@ commands:
   {{continue}}
 {{- end -}}
 {{- if not $resource.Namespace }}
+{{- if $resource.Root }}
 - command: ${KUBECTL} patch {{ $resource.KindGroup }}/{{ $resource.Name }} --type=merge -p '{"spec":{"forProvider":{{ $resource.UpdateParameter }}}}'
 {{- end }}
 {{- end }}
+{{- end }}

internal/templates/02-assert.yaml.tmpl

@@ -1,8 +1,9 @@
+# This assert file belongs to the resource import step.
 apiVersion: kuttl.dev/v1beta1
 kind: TestAssert
 timeout: {{ .TestCase.Timeout }}
 commands:
-- command: ${KUBECTL} get managed -o yaml
+- script: echo "Dump MR manifests for the import assertion step:"; ${KUBECTL} get managed -o yaml
 {{- range $resource := .Resources }}
 {{- if eq $resource.KindGroup "secret." -}}
   {{continue}}

internal/templates/02-import.yaml.tmpl

@@ -1,6 +1,9 @@
+# This file belongs to the resource import step.
 apiVersion: kuttl.dev/v1beta1
 kind: TestStep
 commands:
+- command: ${KUBECTL} scale deployment crossplane -n upbound-system --replicas=0
+- script: ${KUBECTL} -n upbound-system get deploy --no-headers -o custom-columns=":metadata.name" | grep "provider-" | xargs ${KUBECTL} -n upbound-system scale deploy --replicas=0
 {{- range $resource := .Resources }}
 {{- if eq $resource.KindGroup "secret." -}}
   {{continue}}
@@ -10,4 +13,5 @@ commands:
 - script: ${KUBECTL} annotate {{ $resource.KindGroup }}/{{ $resource.Name }} uptest-old-id=$(${KUBECTL} get {{ $resource.KindGroup }}/{{ $resource.Name }} -o=jsonpath='{.status.atProvider.id}') --overwrite
 {{- end }}
 {{- end }}
-- script: ${KUBECTL} -n upbound-system get pods --no-headers -o custom-columns=":metadata.name" | grep "provider-" | xargs ${KUBECTL} -n upbound-system delete pod
+- command: ${KUBECTL} scale deployment crossplane -n upbound-system --replicas=1
+- script: ${KUBECTL} -n upbound-system get deploy --no-headers -o custom-columns=":metadata.name" | grep "provider-" | xargs ${KUBECTL} -n upbound-system scale deploy --replicas=1

internal/templates/03-assert.yaml.tmpl

@@ -1,14 +1,16 @@
+# This assert file belongs to the resource delete step.
 apiVersion: kuttl.dev/v1beta1
 kind: TestAssert
 timeout: {{ .TestCase.Timeout }}
 commands:
-- command: ${KUBECTL} get managed -o yaml
+- script: echo "Dump MR manifests for the delete assertion step:"; ${KUBECTL} get managed -o yaml
+- script: echo "Dump Claim manifests for the delete assertion step:" || ${KUBECTL} get claim --all-namespaces -o yaml
 {{- range $resource := .Resources }}
 {{- if eq $resource.KindGroup "secret." -}}
   {{continue}}
 {{- end -}}
 {{- if $resource.Namespace }}
-- script: ${KUBECTL} get --namespace {{ $resource.Namespace }} {{ $resource.KindGroup }}/{{ $resource.Name }} -o yaml --ignore-not-found && ${KUBECTL} wait {{ $resource.KindGroup }}/{{ $resource.Name }} --for=delete --timeout 10s --namespace {{ $resource.Namespace }}
+- script: ${KUBECTL} wait {{ $resource.KindGroup }}/{{ $resource.Name }} --for=delete --timeout 10s --namespace {{ $resource.Namespace }}
 {{- else }}
 - command: ${KUBECTL} wait {{ $resource.KindGroup }}/{{ $resource.Name }} --for=delete --timeout 10s
 {{- end }}

internal/templates/03-delete.yaml.tmpl

@@ -1,3 +1,4 @@
+# This file belongs to the resource delete step.
 apiVersion: kuttl.dev/v1beta1
 kind: TestStep
 commands:

internal/templates/renderer_test.go

@@ -84,49 +84,62 @@ func TestRender(t *testing.T) {
 			},
 			want: want{
 				out: map[string]string{
-					"00-apply.yaml": "---\n" + bucketManifest,
-					"00-assert.yaml": `apiVersion: kuttl.dev/v1beta1
+					"00-apply.yaml": "# This file belongs to the resource apply step.\n---\n" + bucketManifest,
+					"00-assert.yaml": `# This assert file belongs to the resource apply step.
+apiVersion: kuttl.dev/v1beta1
 kind: TestAssert
 timeout: 10
 commands:
 - command: ${KUBECTL} annotate managed --all upjet.upbound.io/test=true --overwrite
-- command: ${KUBECTL} get managed -o yaml
+- script: echo "Dump MR manifests for the apply assertion step:"; ${KUBECTL} get managed -o yaml
+- script: echo "Dump Claim manifests for the apply assertion step:" || ${KUBECTL} get claim --all-namespaces -o yaml
 - command: ${KUBECTL} wait s3.aws.upbound.io/example-bucket --for=condition=Test --timeout 10s
 `,
-					"01-update.yaml": `apiVersion: kuttl.dev/v1beta1
+					"01-update.yaml": `# This file belongs to the resource update step.
+apiVersion: kuttl.dev/v1beta1
 kind: TestStep
 commands:
 `,
-					"01-assert.yaml": `apiVersion: kuttl.dev/v1beta1
+					"01-assert.yaml": `# This assert file belongs to the resource update step.
+apiVersion: kuttl.dev/v1beta1
 kind: TestAssert
 timeout: 10
 commands:
-- command: ${KUBECTL} get managed -o yaml
+- script: echo "Dump MR manifests for the update assertion step:"; ${KUBECTL} get managed -o yaml
 `,
-					"02-assert.yaml": `apiVersion: kuttl.dev/v1beta1
+					"02-assert.yaml": `# This assert file belongs to the resource import step.
+apiVersion: kuttl.dev/v1beta1
 kind: TestAssert
 timeout: 10
 commands:
-- command: ${KUBECTL} get managed -o yaml
+- script: echo "Dump MR manifests for the import assertion step:"; ${KUBECTL} get managed -o yaml
 - command: ${KUBECTL} wait s3.aws.upbound.io/example-bucket --for=condition=Test --timeout 10s
 - script: new_id=$(${KUBECTL} get s3.aws.upbound.io/example-bucket -o=jsonpath='{.status.atProvider.id}') && old_id=$(${KUBECTL} get s3.aws.upbound.io/example-bucket -o=jsonpath='{.metadata.annotations.uptest-old-id}') && [ "$new_id" == "$old_id" ]
 `,
-					"02-import.yaml": `apiVersion: kuttl.dev/v1beta1
+					"02-import.yaml": `# This file belongs to the resource import step.
+apiVersion: kuttl.dev/v1beta1
 kind: TestStep
 commands:
+- command: ${KUBECTL} scale deployment crossplane -n upbound-system --replicas=0
+- script: ${KUBECTL} -n upbound-system get deploy --no-headers -o custom-columns=":metadata.name" | grep "provider-" | xargs ${KUBECTL} -n upbound-system scale deploy --replicas=0
 - command: ${KUBECTL} --subresource=status patch s3.aws.upbound.io/example-bucket --type=merge -p '{"status":{"conditions":[]}}'
 - script: ${KUBECTL} annotate s3.aws.upbound.io/example-bucket uptest-old-id=$(${KUBECTL} get s3.aws.upbound.io/example-bucket -o=jsonpath='{.status.atProvider.id}') --overwrite
-- script: ${KUBECTL} -n upbound-system get pods --no-headers -o custom-columns=":metadata.name" | grep "provider-" | xargs ${KUBECTL} -n upbound-system delete pod
+- command: ${KUBECTL} scale deployment crossplane -n upbound-system --replicas=1
+- script: ${KUBECTL} -n upbound-system get deploy --no-headers -o custom-columns=":metadata.name" | grep "provider-" | xargs ${KUBECTL} -n upbound-system scale deploy --replicas=1
 `,
-					"03-assert.yaml": `apiVersion: kuttl.dev/v1beta1
+
+					"03-assert.yaml": `# This assert file belongs to the resource delete step.
+apiVersion: kuttl.dev/v1beta1
 kind: TestAssert
 timeout: 10
 commands:
-- command: ${KUBECTL} get managed -o yaml
+- script: echo "Dump MR manifests for the delete assertion step:"; ${KUBECTL} get managed -o yaml
+- script: echo "Dump Claim manifests for the delete assertion step:" || ${KUBECTL} get claim --all-namespaces -o yaml
 - command: ${KUBECTL} wait s3.aws.upbound.io/example-bucket --for=delete --timeout 10s
 - command: ${KUBECTL} wait managed --all --for=delete --timeout 10s
 `,
-					"03-delete.yaml": `apiVersion: kuttl.dev/v1beta1
+					"03-delete.yaml": `# This file belongs to the resource delete step.
+apiVersion: kuttl.dev/v1beta1
 kind: TestStep
 commands:
 - command: ${KUBECTL} delete s3.aws.upbound.io/example-bucket --wait=false --ignore-not-found
@@ -169,59 +182,72 @@ commands:
 			},
 			want: want{
 				out: map[string]string{
-					"00-apply.yaml": `apiVersion: kuttl.dev/v1beta1
+					"00-apply.yaml": `# This file belongs to the resource apply step.
+apiVersion: kuttl.dev/v1beta1
 kind: TestStep
 commands:
 - command: /tmp/setup.sh
 ` + "---\n" + bucketManifest + "---\n" + claimManifest + "---\n" + secretManifest,
-					"00-assert.yaml": `apiVersion: kuttl.dev/v1beta1
+					"00-assert.yaml": `# This assert file belongs to the resource apply step.
+apiVersion: kuttl.dev/v1beta1
 kind: TestAssert
 timeout: 10
 commands:
 - command: ${KUBECTL} annotate managed --all upjet.upbound.io/test=true --overwrite
-- command: ${KUBECTL} get managed -o yaml
+- script: echo "Dump MR manifests for the apply assertion step:"; ${KUBECTL} get managed -o yaml
+- script: echo "Dump Claim manifests for the apply assertion step:" || ${KUBECTL} get claim --all-namespaces -o yaml
 - command: /tmp/bucket/pre-assert.sh
 - command: ${KUBECTL} wait s3.aws.upbound.io/example-bucket --for=condition=Test --timeout 10s
-- script: ${KUBECTL} get --namespace upbound-system cluster.gcp.platformref.upbound.io/test-cluster-claim -o yaml && ${KUBECTL} wait cluster.gcp.platformref.upbound.io/test-cluster-claim --for=condition=Ready --timeout 10s --namespace upbound-system
-- script: ${KUBECTL} get --namespace upbound-system cluster.gcp.platformref.upbound.io/test-cluster-claim -o yaml && ${KUBECTL} wait cluster.gcp.platformref.upbound.io/test-cluster-claim --for=condition=Synced --timeout 10s --namespace upbound-system
+- command: ${KUBECTL} wait cluster.gcp.platformref.upbound.io/test-cluster-claim --for=condition=Ready --timeout 10s --namespace upbound-system
+- command: ${KUBECTL} wait cluster.gcp.platformref.upbound.io/test-cluster-claim --for=condition=Synced --timeout 10s --namespace upbound-system
 - command: /tmp/claim/post-assert.sh
 `,
-					"01-update.yaml": `apiVersion: kuttl.dev/v1beta1
+					"01-update.yaml": `# This file belongs to the resource update step.
+apiVersion: kuttl.dev/v1beta1
 kind: TestStep
 commands:
 `,
-					"01-assert.yaml": `apiVersion: kuttl.dev/v1beta1
+					"01-assert.yaml": `# This assert file belongs to the resource update step.
+apiVersion: kuttl.dev/v1beta1
 kind: TestAssert
 timeout: 10
 commands:
-- command: ${KUBECTL} get managed -o yaml
+- script: echo "Dump MR manifests for the update assertion step:"; ${KUBECTL} get managed -o yaml
 `,
-					"02-assert.yaml": `apiVersion: kuttl.dev/v1beta1
+					"02-assert.yaml": `# This assert file belongs to the resource import step.
+apiVersion: kuttl.dev/v1beta1
 kind: TestAssert
 timeout: 10
 commands:
-- command: ${KUBECTL} get managed -o yaml
+- script: echo "Dump MR manifests for the import assertion step:"; ${KUBECTL} get managed -o yaml
 - command: ${KUBECTL} wait s3.aws.upbound.io/example-bucket --for=condition=Test --timeout 10s
 - script: new_id=$(${KUBECTL} get s3.aws.upbound.io/example-bucket -o=jsonpath='{.status.atProvider.id}') && old_id=$(${KUBECTL} get s3.aws.upbound.io/example-bucket -o=jsonpath='{.metadata.annotations.uptest-old-id}') && [ "$new_id" == "$old_id" ]
 `,
-					"02-import.yaml": `apiVersion: kuttl.dev/v1beta1
+					"02-import.yaml": `# This file belongs to the resource import step.
+apiVersion: kuttl.dev/v1beta1
 kind: TestStep
 commands:
+- command: ${KUBECTL} scale deployment crossplane -n upbound-system --replicas=0
+- script: ${KUBECTL} -n upbound-system get deploy --no-headers -o custom-columns=":metadata.name" | grep "provider-" | xargs ${KUBECTL} -n upbound-system scale deploy --replicas=0
 - command: ${KUBECTL} --subresource=status patch s3.aws.upbound.io/example-bucket --type=merge -p '{"status":{"conditions":[]}}'
 - script: ${KUBECTL} annotate s3.aws.upbound.io/example-bucket uptest-old-id=$(${KUBECTL} get s3.aws.upbound.io/example-bucket -o=jsonpath='{.status.atProvider.id}') --overwrite
-- script: ${KUBECTL} -n upbound-system get pods --no-headers -o custom-columns=":metadata.name" | grep "provider-" | xargs ${KUBECTL} -n upbound-system delete pod
+- command: ${KUBECTL} scale deployment crossplane -n upbound-system --replicas=1
+- script: ${KUBECTL} -n upbound-system get deploy --no-headers -o custom-columns=":metadata.name" | grep "provider-" | xargs ${KUBECTL} -n upbound-system scale deploy --replicas=1
 `,
-					"03-assert.yaml": `apiVersion: kuttl.dev/v1beta1
+					"03-assert.yaml": `# This assert file belongs to the resource delete step.
+apiVersion: kuttl.dev/v1beta1
 kind: TestAssert
 timeout: 10
 commands:
-- command: ${KUBECTL} get managed -o yaml
+- script: echo "Dump MR manifests for the delete assertion step:"; ${KUBECTL} get managed -o yaml
+- script: echo "Dump Claim manifests for the delete assertion step:" || ${KUBECTL} get claim --all-namespaces -o yaml
 - command: ${KUBECTL} wait s3.aws.upbound.io/example-bucket --for=delete --timeout 10s
-- script: ${KUBECTL} get --namespace upbound-system cluster.gcp.platformref.upbound.io/test-cluster-claim -o yaml --ignore-not-found && ${KUBECTL} wait cluster.gcp.platformref.upbound.io/test-cluster-claim --for=delete --timeout 10s --namespace upbound-system
+- script: ${KUBECTL} wait cluster.gcp.platformref.upbound.io/test-cluster-claim --for=delete --timeout 10s --namespace upbound-system
 - command: ${KUBECTL} wait managed --all --for=delete --timeout 10s
 - command: /tmp/teardown.sh
 `,
-					"03-delete.yaml": `apiVersion: kuttl.dev/v1beta1
+					"03-delete.yaml": `# This file belongs to the resource delete step.
+apiVersion: kuttl.dev/v1beta1
 kind: TestStep
 commands:
 - command: ${KUBECTL} delete s3.aws.upbound.io/example-bucket --wait=false --ignore-not-found

internal/tester.go

@@ -71,7 +71,8 @@ func (t *tester) prepareConfig() (*config.TestCase, []config.Resource, error) {
 
 	for _, m := range t.manifests {
 		obj := m.Object
-		kg := strings.ToLower(obj.GroupVersionKind().Kind + "." + obj.GroupVersionKind().Group)
+		groupVersionKind := obj.GroupVersionKind()
+		kg := strings.ToLower(groupVersionKind.Kind + "." + groupVersionKind.Group)
 
 		example := config.Resource{
 			Name:       obj.GetName(),
@@ -82,15 +83,6 @@ func (t *tester) prepareConfig() (*config.TestCase, []config.Resource, error) {
 			Conditions: t.options.DefaultConditions,
 		}
 
-		if updateParameter := os.Getenv("UPTEST_UPDATE_PARAMETER"); updateParameter != "" {
-			example.UpdateParameter = updateParameter
-			var data map[string]interface{}
-			if err := json.Unmarshal([]byte(updateParameter), &data); err != nil {
-				return nil, nil, errors.Wrap(err, "cannot unmarshal JSON")
-			}
-			example.UpdateAssertKey, example.UpdateAssertValue = convertToJSONPath(data, "")
-		}
-
 		var err error
 		annotations := obj.GetAnnotations()
 		if v, ok := annotations[config.AnnotationKeyTimeout]; ok {
@@ -135,15 +127,25 @@ func (t *tester) prepareConfig() (*config.TestCase, []config.Resource, error) {
 			}
 		}
 
-		if v, ok := annotations[config.AnnotationKeyUpdateParameter]; ok {
-			example.UpdateParameter = v
+		updateParameter, ok := annotations[config.AnnotationKeyUpdateParameter]
+		if !ok {
+			updateParameter = os.Getenv("UPTEST_UPDATE_PARAMETER")
+		}
+		if updateParameter != "" {
+			example.UpdateParameter = updateParameter
 			var data map[string]interface{}
-			if err := json.Unmarshal([]byte(v), &data); err != nil {
-				return nil, nil, errors.Wrap(err, "cannot unmarshal JSON")
+			if err := json.Unmarshal([]byte(updateParameter), &data); err != nil {
+				return nil, nil, errors.Wrapf(err, "cannot unmarshal JSON object: %s", updateParameter)
 			}
 			example.UpdateAssertKey, example.UpdateAssertValue = convertToJSONPath(data, "")
 		}
 
+		if exampleId, ok := annotations["meta.upbound.io/example-id"]; ok {
+			if exampleId == strings.ToLower(fmt.Sprintf("%s/%s/%s", strings.Split(groupVersionKind.Group, ".")[0], groupVersionKind.Version, groupVersionKind.Kind)) {
+				example.Root = true
+			}
+		}
+
 		examples = append(examples, example)
 	}