From bd77825f59be7c4fd58c905ad3c95a65f15e99a9 Mon Sep 17 00:00:00 2001 From: Kumar Mallikarjuna Date: Fri, 22 Nov 2024 15:45:54 +0530 Subject: [PATCH 01/31] [KEP-0009] feat: add expression based assertions This PR adds CEL-expression based assertions to `TestAsserts`. See https://github.com/kudobuilder/kuttl/blob/main/keps/0009-expression-based-assertions.md for more details. Signed-off-by: Kumar Mallikarjuna --- go.mod | 7 ++ go.sum | 9 +- pkg/apis/testharness/v1beta1/test_types.go | 17 ++++ .../v1beta1/zz_generated.deepcopy.go | 48 ++++++++++ pkg/test/step.go | 9 ++ pkg/test/utils/expressions.go | 88 +++++++++++++++++++ 6 files changed, 177 insertions(+), 1 deletion(-) create mode 100644 pkg/test/utils/expressions.go diff --git a/go.mod b/go.mod index 22f2e604..cbc450ea 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( github.com/docker/docker v27.4.1+incompatible github.com/dustin/go-humanize v1.0.1 github.com/dustinkirkland/golang-petname v0.0.0-20191129215211-8e5a1ed0cff0 + github.com/google/cel-go v0.22.0 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 github.com/spf13/cobra v1.8.1 @@ -25,10 +26,12 @@ require ( ) require ( + cel.dev/expr v0.18.0 // indirect github.com/Azure/go-ansiterm v0.0.0-20230124172434-306776ec8161 // indirect github.com/BurntSushi/toml v1.4.0 // indirect github.com/Microsoft/go-winio v0.5.1 // indirect github.com/alessio/shellescape v1.4.2 // indirect + github.com/antlr4-go/antlr/v4 v4.13.0 // indirect github.com/containerd/log v0.1.0 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/distribution/reference v0.6.0 // indirect @@ -68,12 +71,14 @@ require ( github.com/opencontainers/image-spec v1.0.2 // indirect github.com/pelletier/go-toml v1.9.5 // indirect github.com/pkg/errors v0.9.1 // indirect + github.com/stoewer/go-strcase v1.3.0 // indirect github.com/x448/float16 v0.8.4 // indirect go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.53.0 // indirect go.opentelemetry.io/otel v1.28.0 // indirect go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.27.0 // indirect go.opentelemetry.io/otel/metric v1.28.0 // indirect go.opentelemetry.io/otel/trace v1.28.0 // indirect + golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 // indirect golang.org/x/mod v0.22.0 // indirect golang.org/x/net v0.33.0 // indirect golang.org/x/oauth2 v0.23.0 // indirect @@ -83,6 +88,8 @@ require ( golang.org/x/text v0.21.0 // indirect golang.org/x/time v0.7.0 // indirect golang.org/x/tools v0.28.0 // indirect + google.golang.org/genproto/googleapis/api v0.0.0-20240826202546-f6391c0de4c7 // indirect + google.golang.org/genproto/googleapis/rpc v0.0.0-20240826202546-f6391c0de4c7 // indirect google.golang.org/protobuf v1.35.1 // indirect gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect gopkg.in/inf.v0 v0.9.1 // indirect diff --git a/go.sum b/go.sum index dd151a8a..06d7873e 100644 --- a/go.sum +++ b/go.sum @@ -1,3 +1,5 @@ +cel.dev/expr v0.18.0 h1:CJ6drgk+Hf96lkLikr4rFf19WrU0BOWEihyZnI2TAzo= +cel.dev/expr v0.18.0/go.mod h1:MrpN08Q+lEBs+bGYdLxxHkZoUSsCp0nSKTs0nTymJgw= github.com/Azure/go-ansiterm v0.0.0-20230124172434-306776ec8161 h1:L/gRVlceqvL25UVaW/CKtUDjefjrs0SPonmDGUVOYP0= github.com/Azure/go-ansiterm v0.0.0-20230124172434-306776ec8161/go.mod h1:xomTg63KZ2rFqZQzSB4Vz2SUXa1BpHTVz9L5PTmPC4E= github.com/BurntSushi/toml v1.4.0 h1:kuoIxZQy2WRRk1pttg9asf+WVv6tWQuBNVmK8+nqPr0= @@ -8,6 +10,8 @@ github.com/Microsoft/go-winio v0.5.1 h1:aPJp2QD7OOrhO5tQXqQoGSJc+DjDtWTGLOmNyAm6 github.com/Microsoft/go-winio v0.5.1/go.mod h1:JPGBdM1cNvN/6ISo+n8V5iA4v8pBzdOpzfwIujj1a84= github.com/alessio/shellescape v1.4.2 h1:MHPfaU+ddJ0/bYWpgIeUnQUqKrlJ1S7BfEYPM4uEoM0= github.com/alessio/shellescape v1.4.2/go.mod h1:PZAiSCk0LJaZkiCSkPv8qIobYglO3FPpyFjDCtHLS30= +github.com/antlr4-go/antlr/v4 v4.13.0 h1:lxCg3LAv+EUK6t1i0y1V6/SLeUi0eKEKdhQAlS8TVTI= +github.com/antlr4-go/antlr/v4 v4.13.0/go.mod h1:pfChB/xh/Unjila75QW7+VU4TSnWnnk9UTnmpPaOR2g= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= github.com/cenkalti/backoff/v4 v4.3.0 h1:MyRJ/UdXutAwSAT+s3wNd7MfTIcy71VQueUuFK343L8= @@ -69,6 +73,8 @@ github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q= github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q= github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek= github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps= +github.com/google/cel-go v0.22.0 h1:b3FJZxpiv1vTMo2/5RDUqAHPxkT8mmMfJIrq1llbf7g= +github.com/google/cel-go v0.22.0/go.mod h1:BuznPXXfQDpXKWQ9sPW3TzlAJN5zzFe+i9tIs0yC4s8= github.com/google/gnostic-models v0.6.8 h1:yo/ABAfM5IMRsS1VnXjTBvUb61tFIHozhlYvRgGre9I= github.com/google/gnostic-models v0.6.8/go.mod h1:5n7qKqH0f5wFt+aWF8CW6pZLLNOfYuF5OpfBSENuI8U= github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= @@ -162,6 +168,8 @@ github.com/spf13/cobra v1.8.1/go.mod h1:wHxEcudfqmLYa8iTfL+OuZPbBZkmvliBWKIezN3k github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= +github.com/stoewer/go-strcase v1.3.0 h1:g0eASXYtp+yvN9fK8sH94oCIk0fau9uV1/ZdJ0AVEzs= +github.com/stoewer/go-strcase v1.3.0/go.mod h1:fAH5hQ5pehh+j3nZfvwdk2RgEgQjAoM8wodgtPmh1xo= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= @@ -253,7 +261,6 @@ golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8T golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gomodules.xyz/jsonpatch/v2 v2.4.0 h1:Ci3iUJyx9UeRx7CeFN8ARgGbkESwJK+KB9lLcWxY/Zw= gomodules.xyz/jsonpatch/v2 v2.4.0/go.mod h1:AH3dM2RI6uoBZxn3LVrfvJ3E0/9dG4cSrbuBJT4moAY= -google.golang.org/genproto v0.0.0-20240123012728-ef4313101c80 h1:KAeGQVN3M9nD0/bQXnr/ClcEMJ968gUXJQ9pwfSynuQ= google.golang.org/genproto/googleapis/api v0.0.0-20240826202546-f6391c0de4c7 h1:YcyjlL1PRr2Q17/I0dPk2JmYS5CDXfcdb2Z3YRioEbw= google.golang.org/genproto/googleapis/api v0.0.0-20240826202546-f6391c0de4c7/go.mod h1:OCdP9MfskevB/rbYvHTsXTtKC+3bHWajPdoKgjcYkfo= google.golang.org/genproto/googleapis/rpc v0.0.0-20240826202546-f6391c0de4c7 h1:2035KHhUv+EpyB+hWgJnaWKJOdX1E95w2S8Rr4uWKTs= diff --git a/pkg/apis/testharness/v1beta1/test_types.go b/pkg/apis/testharness/v1beta1/test_types.go index 850ae4de..7f9b4ac0 100644 --- a/pkg/apis/testharness/v1beta1/test_types.go +++ b/pkg/apis/testharness/v1beta1/test_types.go @@ -157,6 +157,10 @@ type TestAssert struct { Collectors []*TestCollector `json:"collectors,omitempty"` // Commands is a set of commands to be run as assertions for the current step Commands []TestAssertCommand `json:"commands,omitempty"` + + ResourceRefs []TestResourceRef `json:"resourceRefs,omitempty"` + + AssertExpressions AnyAllExpressions `json:"assertExpressions,omitempty"` } // TestAssertCommand an assertion based on the result of the execution of a command @@ -227,6 +231,19 @@ type TestCollector struct { Cmd string `json:"command,omitempty"` } +type TestResourceRef struct { + ApiVersion string `json:"apiVersion,omitempty"` + Kind string `json:"kind,omitempty"` + Namespace string `json:"namespace,omitempty"` + Name string `json:"name,omitempty"` + Id string `json:"id,omitempty"` +} + +type AnyAllExpressions struct { + Any []string `json:"any,omitempty"` + All []string `json:"all,omitempty"` +} + // DefaultKINDContext defines the default kind context to use. const DefaultKINDContext = "kind" diff --git a/pkg/apis/testharness/v1beta1/zz_generated.deepcopy.go b/pkg/apis/testharness/v1beta1/zz_generated.deepcopy.go index 4698a0c6..ca953a91 100644 --- a/pkg/apis/testharness/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/testharness/v1beta1/zz_generated.deepcopy.go @@ -24,6 +24,32 @@ import ( runtime "k8s.io/apimachinery/pkg/runtime" ) +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AnyAllExpressions) DeepCopyInto(out *AnyAllExpressions) { + *out = *in + if in.Any != nil { + in, out := &in.Any, &out.Any + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.All != nil { + in, out := &in.All, &out.All + *out = make([]string, len(*in)) + copy(*out, *in) + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AnyAllExpressions. +func (in *AnyAllExpressions) DeepCopy() *AnyAllExpressions { + if in == nil { + return nil + } + out := new(AnyAllExpressions) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Command) DeepCopyInto(out *Command) { *out = *in @@ -95,6 +121,12 @@ func (in *TestAssert) DeepCopyInto(out *TestAssert) { *out = make([]TestAssertCommand, len(*in)) copy(*out, *in) } + if in.ResourceRefs != nil { + in, out := &in.ResourceRefs, &out.ResourceRefs + *out = make([]TestResourceRef, len(*in)) + copy(*out, *in) + } + in.AssertExpressions.DeepCopyInto(&out.AssertExpressions) return } @@ -179,6 +211,22 @@ func (in *TestFile) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *TestResourceRef) DeepCopyInto(out *TestResourceRef) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TestResourceRef. +func (in *TestResourceRef) DeepCopy() *TestResourceRef { + if in == nil { + return nil + } + out := new(TestResourceRef) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TestStep) DeepCopyInto(out *TestStep) { *out = *in diff --git a/pkg/test/step.go b/pkg/test/step.go index a076d963..a387ef33 100644 --- a/pkg/test/step.go +++ b/pkg/test/step.go @@ -412,6 +412,14 @@ func (s *Step) CheckAssertCommands(ctx context.Context, namespace string, comman return testErrors } +func (s *Step) CheckAssertExpressions( + ctx context.Context, + resourceRefs []harness.TestResourceRef, + expressions harness.AnyAllExpressions, +) []error { + return testutils.RunAssertExpressions(ctx, s.Logger, resourceRefs, expressions, s.Kubeconfig) +} + // Check checks if the resources defined in Asserts and Errors are in the correct state. func (s *Step) Check(namespace string, timeout int) []error { testErrors := []error{} @@ -422,6 +430,7 @@ func (s *Step) Check(namespace string, timeout int) []error { if s.Assert != nil { testErrors = append(testErrors, s.CheckAssertCommands(context.TODO(), namespace, s.Assert.Commands, timeout)...) + testErrors = append(testErrors, s.CheckAssertExpressions(context.TODO(), s.Assert.ResourceRefs, s.Assert.AssertExpressions)...) } for _, expected := range s.Errors { diff --git a/pkg/test/utils/expressions.go b/pkg/test/utils/expressions.go new file mode 100644 index 00000000..e100dad4 --- /dev/null +++ b/pkg/test/utils/expressions.go @@ -0,0 +1,88 @@ +package utils + +import ( + "context" + "fmt" + "os" + + "github.com/google/cel-go/cel" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/types" + + harness "github.com/kudobuilder/kuttl/pkg/apis/testharness/v1beta1" +) + +// RunAssertExpressions evaluates a set of CEL expressions specified as AnyAllExpressions +func RunAssertExpressions( + ctx context.Context, + logger Logger, + resourceRefs []harness.TestResourceRef, + expressions harness.AnyAllExpressions, + kubeconfigOverride string, +) []error { + errs := []error{} + + actualDir, err := os.Getwd() + if err != nil { + return []error{fmt.Errorf("failed to get current working director: %w", err)} + } + + kubeconfig := kubeconfigPath(actualDir, kubeconfigOverride) + cl, err := NewClient(kubeconfig, "")(false) + if err != nil { + return []error{fmt.Errorf("failed to construct client: %w", err)} + } + + variables := make(map[string]interface{}) + for _, resourceRef := range resourceRefs { + gvk := constructGVK(resourceRef.ApiVersion, resourceRef.Kind) + referencedResource := &unstructured.Unstructured{} + referencedResource.SetGroupVersionKind(gvk) + + if err := cl.Get( + ctx, + types.NamespacedName{Namespace: resourceRef.Namespace, Name: resourceRef.Name}, + referencedResource, + ); err != nil { + return []error{fmt.Errorf("failed to get referenced resource '%v': %w", gvk, err)} + } + + variables[resourceRef.Id] = referencedResource.Object + } + + env, err := cel.NewEnv() + if err != nil { + return []error{fmt.Errorf("failed to create environment: %w", err)} + } + + for k := range variables { + env, err = env.Extend(cel.Variable(k, cel.DynType)) + if err != nil { + return []error{fmt.Errorf("failed to add resource parameter '%v' to environment: %w", k, err)} + } + } + + for _, expr := range expressions.Any { + ast, issues := env.Compile(expr) + if issues != nil && issues.Err() != nil { + return []error{fmt.Errorf("type-check error: %s", issues.Err())} + } + + prg, err := env.Program(ast) + if err != nil { + return []error{fmt.Errorf("program constuction error: %w", err)} + } + + out, _, err := prg.Eval(variables) + if err != nil { + return []error{fmt.Errorf("failed to evaluate program: %w", err)} + } + + logger.Logf("expression '%v' evaluated to '%v'", expr, out.Value()) + if out.Value() != true { + errs = append(errs, fmt.Errorf("failed validation, expression '%v' evaluated to '%v'", expr, out.Value())) + } + } + + return errs +} From 68c60e33adbe3c3a3399d3f307698ab766a60b6a Mon Sep 17 00:00:00 2001 From: Kumar Mallikarjuna Date: Wed, 27 Nov 2024 11:08:18 +0530 Subject: [PATCH 02/31] chore: rename Id->Ref and make linter happy Signed-off-by: Kumar Mallikarjuna --- pkg/apis/testharness/v1beta1/test_types.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/apis/testharness/v1beta1/test_types.go b/pkg/apis/testharness/v1beta1/test_types.go index 7f9b4ac0..f6bcd397 100644 --- a/pkg/apis/testharness/v1beta1/test_types.go +++ b/pkg/apis/testharness/v1beta1/test_types.go @@ -232,11 +232,11 @@ type TestCollector struct { } type TestResourceRef struct { - ApiVersion string `json:"apiVersion,omitempty"` + APIVersion string `json:"apiVersion,omitempty"` Kind string `json:"kind,omitempty"` Namespace string `json:"namespace,omitempty"` Name string `json:"name,omitempty"` - Id string `json:"id,omitempty"` + Ref string `json:"ref,omitempty"` } type AnyAllExpressions struct { From 9a94a6b24ffd261adb78f4f9675e00d2af2fe08d Mon Sep 17 00:00:00 2001 From: Kumar Mallikarjuna Date: Wed, 27 Nov 2024 14:14:27 +0530 Subject: [PATCH 03/31] chore: add validation for resourceRefs Signed-off-by: Kumar Mallikarjuna --- pkg/apis/testharness/v1beta1/expression.go | 59 ++++++++++++++++++++++ pkg/test/step.go | 11 ++++ 2 files changed, 70 insertions(+) create mode 100644 pkg/apis/testharness/v1beta1/expression.go diff --git a/pkg/apis/testharness/v1beta1/expression.go b/pkg/apis/testharness/v1beta1/expression.go new file mode 100644 index 00000000..762f56ed --- /dev/null +++ b/pkg/apis/testharness/v1beta1/expression.go @@ -0,0 +1,59 @@ +package v1beta1 + +import ( + "errors" + "fmt" + "strings" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" +) + +func (t *TestResourceRef) BuildResourceReference() (namespacedName types.NamespacedName, referencedResource *unstructured.Unstructured) { + referencedResource = &unstructured.Unstructured{} + apiVersionSplit := strings.Split(t.APIVersion, "/") + gvk := schema.GroupVersionKind{ + Version: apiVersionSplit[len(apiVersionSplit)-1], + Kind: t.Kind, + } + if len(t.APIVersion) > 1 { + gvk.Group = apiVersionSplit[0] + } + referencedResource.SetGroupVersionKind(gvk) + + namespacedName = types.NamespacedName{ + Namespace: t.Namespace, + Name: t.Name, + } + + return +} + +func (t *TestResourceRef) ValidateResourceReference() error { + apiVersionSplit := strings.Split(t.APIVersion, "/") + if t.APIVersion == "" || (len(apiVersionSplit) != 1 && len(apiVersionSplit) != 2) { + return fmt.Errorf("apiVersion '%v' not of the format (/)", t.APIVersion) + } else if t.Kind == "" { + return errors.New("kind not specified") + } else if t.Namespace == "" { + return errors.New("namespace not specified") + } else if t.Name == "" { + return errors.New("name not specified") + } else if t.Ref == "" { + return errors.New("ref not specified") + } + + return nil +} + +func (t *TestResourceRef) String() string { + return fmt.Sprintf( + "apiVersion=%v, kind=%v, namespace=%v, name=%v, ref=%v", + t.APIVersion, + t.Kind, + t.Namespace, + t.Name, + t.Ref, + ) +} diff --git a/pkg/test/step.go b/pkg/test/step.go index a387ef33..f5edaa91 100644 --- a/pkg/test/step.go +++ b/pkg/test/step.go @@ -542,6 +542,17 @@ func (s *Step) LoadYAML(file string) error { } else { return fmt.Errorf("failed to load TestAssert object from %s: it contains an object of type %T", file, obj) } + + var errs []error + for _, resourceRef := range s.Assert.ResourceRefs { + if err := resourceRef.ValidateResourceReference(); err != nil { + errs = append(errs, fmt.Errorf("validation failed for reference '%v': %w", resourceRef.String(), err)) + } + } + + if len(errs) > 0 { + return fmt.Errorf("failed to load TestAssert object from %s: %w", file, errors.Join(errs...)) + } } else { asserts = append(asserts, obj) } From 38819ef3c2f0729f570485327dd074e6e897276e Mon Sep 17 00:00:00 2001 From: Kumar Mallikarjuna Date: Wed, 27 Nov 2024 14:31:00 +0530 Subject: [PATCH 04/31] chore: make assertion syntax consistent with the KEP Signed-off-by: Kumar Mallikarjuna --- pkg/apis/testharness/v1beta1/test_types.go | 10 ++++--- .../v1beta1/zz_generated.deepcopy.go | 29 +++++++++---------- pkg/test/step.go | 7 +++-- 3 files changed, 24 insertions(+), 22 deletions(-) diff --git a/pkg/apis/testharness/v1beta1/test_types.go b/pkg/apis/testharness/v1beta1/test_types.go index f6bcd397..9ec6324a 100644 --- a/pkg/apis/testharness/v1beta1/test_types.go +++ b/pkg/apis/testharness/v1beta1/test_types.go @@ -9,6 +9,8 @@ import ( const KubeconfigLoadingEager = "Eager" const KubeconfigLoadingLazy = "Lazy" +type CELExpression string + // Create embedded struct to implement custom DeepCopyInto method type RestConfig struct { RC *rest.Config @@ -160,7 +162,8 @@ type TestAssert struct { ResourceRefs []TestResourceRef `json:"resourceRefs,omitempty"` - AssertExpressions AnyAllExpressions `json:"assertExpressions,omitempty"` + AssertAny []Assertion `json:"assertAny,omitempty"` + AssertAll []Assertion `json:"assertAll,omitempty"` } // TestAssertCommand an assertion based on the result of the execution of a command @@ -239,9 +242,8 @@ type TestResourceRef struct { Ref string `json:"ref,omitempty"` } -type AnyAllExpressions struct { - Any []string `json:"any,omitempty"` - All []string `json:"all,omitempty"` +type Assertion struct { + CELExpression CELExpression `json:"celExpr,omitempty"` } // DefaultKINDContext defines the default kind context to use. diff --git a/pkg/apis/testharness/v1beta1/zz_generated.deepcopy.go b/pkg/apis/testharness/v1beta1/zz_generated.deepcopy.go index ca953a91..0966036f 100644 --- a/pkg/apis/testharness/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/testharness/v1beta1/zz_generated.deepcopy.go @@ -25,27 +25,17 @@ import ( ) // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *AnyAllExpressions) DeepCopyInto(out *AnyAllExpressions) { +func (in *Assertion) DeepCopyInto(out *Assertion) { *out = *in - if in.Any != nil { - in, out := &in.Any, &out.Any - *out = make([]string, len(*in)) - copy(*out, *in) - } - if in.All != nil { - in, out := &in.All, &out.All - *out = make([]string, len(*in)) - copy(*out, *in) - } return } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AnyAllExpressions. -func (in *AnyAllExpressions) DeepCopy() *AnyAllExpressions { +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Assertion. +func (in *Assertion) DeepCopy() *Assertion { if in == nil { return nil } - out := new(AnyAllExpressions) + out := new(Assertion) in.DeepCopyInto(out) return out } @@ -126,7 +116,16 @@ func (in *TestAssert) DeepCopyInto(out *TestAssert) { *out = make([]TestResourceRef, len(*in)) copy(*out, *in) } - in.AssertExpressions.DeepCopyInto(&out.AssertExpressions) + if in.AssertAny != nil { + in, out := &in.AssertAny, &out.AssertAny + *out = make([]Assertion, len(*in)) + copy(*out, *in) + } + if in.AssertAll != nil { + in, out := &in.AssertAll, &out.AssertAll + *out = make([]Assertion, len(*in)) + copy(*out, *in) + } return } diff --git a/pkg/test/step.go b/pkg/test/step.go index f5edaa91..38a6e5a0 100644 --- a/pkg/test/step.go +++ b/pkg/test/step.go @@ -415,9 +415,10 @@ func (s *Step) CheckAssertCommands(ctx context.Context, namespace string, comman func (s *Step) CheckAssertExpressions( ctx context.Context, resourceRefs []harness.TestResourceRef, - expressions harness.AnyAllExpressions, + assertAny, + assertAll []harness.Assertion, ) []error { - return testutils.RunAssertExpressions(ctx, s.Logger, resourceRefs, expressions, s.Kubeconfig) + return testutils.RunAssertExpressions(ctx, s.Logger, resourceRefs, assertAny, assertAll, s.Kubeconfig) } // Check checks if the resources defined in Asserts and Errors are in the correct state. @@ -430,7 +431,7 @@ func (s *Step) Check(namespace string, timeout int) []error { if s.Assert != nil { testErrors = append(testErrors, s.CheckAssertCommands(context.TODO(), namespace, s.Assert.Commands, timeout)...) - testErrors = append(testErrors, s.CheckAssertExpressions(context.TODO(), s.Assert.ResourceRefs, s.Assert.AssertExpressions)...) + testErrors = append(testErrors, s.CheckAssertExpressions(context.TODO(), s.Assert.ResourceRefs, s.Assert.AssertAny, s.Assert.AssertAll)...) } for _, expected := range s.Errors { From 3a6b02e3687f25ede64dfb89f760605a96c930cb Mon Sep 17 00:00:00 2001 From: Kumar Mallikarjuna Date: Wed, 27 Nov 2024 14:33:12 +0530 Subject: [PATCH 05/31] refactor: rename Validate method for `TestResourceRef` Signed-off-by: Kumar Mallikarjuna --- pkg/apis/testharness/v1beta1/expression.go | 2 +- pkg/test/step.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/apis/testharness/v1beta1/expression.go b/pkg/apis/testharness/v1beta1/expression.go index 762f56ed..dd2bc3af 100644 --- a/pkg/apis/testharness/v1beta1/expression.go +++ b/pkg/apis/testharness/v1beta1/expression.go @@ -30,7 +30,7 @@ func (t *TestResourceRef) BuildResourceReference() (namespacedName types.Namespa return } -func (t *TestResourceRef) ValidateResourceReference() error { +func (t *TestResourceRef) Validate() error { apiVersionSplit := strings.Split(t.APIVersion, "/") if t.APIVersion == "" || (len(apiVersionSplit) != 1 && len(apiVersionSplit) != 2) { return fmt.Errorf("apiVersion '%v' not of the format (/)", t.APIVersion) diff --git a/pkg/test/step.go b/pkg/test/step.go index 38a6e5a0..b3d2ecbc 100644 --- a/pkg/test/step.go +++ b/pkg/test/step.go @@ -546,7 +546,7 @@ func (s *Step) LoadYAML(file string) error { var errs []error for _, resourceRef := range s.Assert.ResourceRefs { - if err := resourceRef.ValidateResourceReference(); err != nil { + if err := resourceRef.Validate(); err != nil { errs = append(errs, fmt.Errorf("validation failed for reference '%v': %w", resourceRef.String(), err)) } } From bd362f213f082c1aed734893294ac9a382021d79 Mon Sep 17 00:00:00 2001 From: Kumar Mallikarjuna Date: Tue, 3 Dec 2024 13:16:00 +0530 Subject: [PATCH 06/31] chore: pre-build environment and program for expressions Signed-off-by: Kumar Mallikarjuna --- pkg/apis/testharness/v1beta1/expression.go | 15 ++++++++ pkg/apis/testharness/v1beta1/test_types.go | 8 ++--- .../v1beta1/zz_generated.deepcopy.go | 20 ++++++++--- pkg/test/step.go | 35 +++++++++++++++++-- pkg/test/utils/expression.go | 25 +++++++++++++ 5 files changed, 92 insertions(+), 11 deletions(-) create mode 100644 pkg/test/utils/expression.go diff --git a/pkg/apis/testharness/v1beta1/expression.go b/pkg/apis/testharness/v1beta1/expression.go index dd2bc3af..6c265099 100644 --- a/pkg/apis/testharness/v1beta1/expression.go +++ b/pkg/apis/testharness/v1beta1/expression.go @@ -5,6 +5,7 @@ import ( "fmt" "strings" + "github.com/google/cel-go/cel" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" @@ -57,3 +58,17 @@ func (t *TestResourceRef) String() string { t.Ref, ) } + +func (t *Assertion) BuildProgram(env *cel.Env) (cel.Program, error) { + ast, issues := env.Compile(t.CELExpression) + if issues != nil && issues.Err() != nil { + return nil, fmt.Errorf("type-check error: %s", issues.Err()) + } + + prg, err := env.Program(ast) + if err != nil { + return nil, fmt.Errorf("program construction error: %w", err) + } + + return prg, nil +} diff --git a/pkg/apis/testharness/v1beta1/test_types.go b/pkg/apis/testharness/v1beta1/test_types.go index 9ec6324a..ad726053 100644 --- a/pkg/apis/testharness/v1beta1/test_types.go +++ b/pkg/apis/testharness/v1beta1/test_types.go @@ -9,8 +9,6 @@ import ( const KubeconfigLoadingEager = "Eager" const KubeconfigLoadingLazy = "Lazy" -type CELExpression string - // Create embedded struct to implement custom DeepCopyInto method type RestConfig struct { RC *rest.Config @@ -162,8 +160,8 @@ type TestAssert struct { ResourceRefs []TestResourceRef `json:"resourceRefs,omitempty"` - AssertAny []Assertion `json:"assertAny,omitempty"` - AssertAll []Assertion `json:"assertAll,omitempty"` + AssertAny []*Assertion `json:"assertAny,omitempty"` + AssertAll []*Assertion `json:"assertAll,omitempty"` } // TestAssertCommand an assertion based on the result of the execution of a command @@ -243,7 +241,7 @@ type TestResourceRef struct { } type Assertion struct { - CELExpression CELExpression `json:"celExpr,omitempty"` + CELExpression string `json:"celExpr,omitempty"` } // DefaultKINDContext defines the default kind context to use. diff --git a/pkg/apis/testharness/v1beta1/zz_generated.deepcopy.go b/pkg/apis/testharness/v1beta1/zz_generated.deepcopy.go index 0966036f..303263f4 100644 --- a/pkg/apis/testharness/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/testharness/v1beta1/zz_generated.deepcopy.go @@ -118,13 +118,25 @@ func (in *TestAssert) DeepCopyInto(out *TestAssert) { } if in.AssertAny != nil { in, out := &in.AssertAny, &out.AssertAny - *out = make([]Assertion, len(*in)) - copy(*out, *in) + *out = make([]*Assertion, len(*in)) + for i := range *in { + if (*in)[i] != nil { + in, out := &(*in)[i], &(*out)[i] + *out = new(Assertion) + **out = **in + } + } } if in.AssertAll != nil { in, out := &in.AssertAll, &out.AssertAll - *out = make([]Assertion, len(*in)) - copy(*out, *in) + *out = make([]*Assertion, len(*in)) + for i := range *in { + if (*in)[i] != nil { + in, out := &(*in)[i], &(*out)[i] + *out = new(Assertion) + **out = **in + } + } } return } diff --git a/pkg/test/step.go b/pkg/test/step.go index b3d2ecbc..077665ee 100644 --- a/pkg/test/step.go +++ b/pkg/test/step.go @@ -10,6 +10,7 @@ import ( "testing" "time" + "github.com/google/cel-go/cel" k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -46,6 +47,8 @@ type Step struct { Step *harness.TestStep Assert *harness.TestAssert + Programs map[string]cel.Program + Asserts []client.Object Apply []client.Object Errors []client.Object @@ -416,9 +419,14 @@ func (s *Step) CheckAssertExpressions( ctx context.Context, resourceRefs []harness.TestResourceRef, assertAny, - assertAll []harness.Assertion, + assertAll []*harness.Assertion, ) []error { - return testutils.RunAssertExpressions(ctx, s.Logger, resourceRefs, assertAny, assertAll, s.Kubeconfig) + client, err := s.Client(false) + if err != nil { + return []error{err} + } + + return testutils.RunAssertExpressions(ctx, s.Logger, client, s.Programs, resourceRefs, assertAny, assertAll) } // Check checks if the resources defined in Asserts and Errors are in the correct state. @@ -525,6 +533,8 @@ func (s *Step) String() string { // if seen, mark a test immediately failed. // - All other YAML files are considered resources to create. func (s *Step) LoadYAML(file string) error { + s.Programs = make(map[string]cel.Program) + skipFile, objects, err := s.loadOrSkipFile(file) if skipFile || err != nil { return err @@ -554,6 +564,27 @@ func (s *Step) LoadYAML(file string) error { if len(errs) > 0 { return fmt.Errorf("failed to load TestAssert object from %s: %w", file, errors.Join(errs...)) } + + var assertions []*harness.Assertion + assertions = append(assertions, s.Assert.AssertAny...) + assertions = append(assertions, s.Assert.AssertAll...) + + env, err := testutils.BuildEnv(s.Assert.ResourceRefs) + if err != nil { + return fmt.Errorf("failed to load TestAssert object from %s: %w", file, err) + } + + for _, assertion := range assertions { + if prg, err := assertion.BuildProgram(env); err != nil { + errs = append(errs, err) + } else { + s.Programs[assertion.CELExpression] = prg + } + } + + if len(errs) > 0 { + return fmt.Errorf("failed to load TestAssert object from %s: %w", file, errors.Join(errs...)) + } } else { asserts = append(asserts, obj) } diff --git a/pkg/test/utils/expression.go b/pkg/test/utils/expression.go new file mode 100644 index 00000000..75b812f5 --- /dev/null +++ b/pkg/test/utils/expression.go @@ -0,0 +1,25 @@ +package utils + +import ( + "fmt" + + "github.com/google/cel-go/cel" + + "github.com/kudobuilder/kuttl/pkg/apis/testharness/v1beta1" +) + +func BuildEnv(resourceRefs []v1beta1.TestResourceRef) (*cel.Env, error) { + env, err := cel.NewEnv() + if err != nil { + return nil, fmt.Errorf("failed to create environment: %w", err) + } + + for _, resourceRef := range resourceRefs { + env, err = env.Extend(cel.Variable(resourceRef.Ref, cel.DynType)) + if err != nil { + return nil, fmt.Errorf("failed to add resource parameter '%v' to environment: %w", resourceRef.Ref, err) + } + } + + return env, nil +} From 14ad7a86cd47a9a85a6c069694ecccfdedb9b5fe Mon Sep 17 00:00:00 2001 From: Kumar Mallikarjuna Date: Tue, 3 Dec 2024 13:25:22 +0530 Subject: [PATCH 07/31] chore: make linter happy and initialize Programs only if assertions are present Signed-off-by: Kumar Mallikarjuna --- pkg/apis/testharness/v1beta1/expression.go | 11 ++++++----- pkg/test/step.go | 8 +++++--- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/pkg/apis/testharness/v1beta1/expression.go b/pkg/apis/testharness/v1beta1/expression.go index 6c265099..0739de2f 100644 --- a/pkg/apis/testharness/v1beta1/expression.go +++ b/pkg/apis/testharness/v1beta1/expression.go @@ -33,15 +33,16 @@ func (t *TestResourceRef) BuildResourceReference() (namespacedName types.Namespa func (t *TestResourceRef) Validate() error { apiVersionSplit := strings.Split(t.APIVersion, "/") - if t.APIVersion == "" || (len(apiVersionSplit) != 1 && len(apiVersionSplit) != 2) { + switch { + case t.APIVersion == "" || (len(apiVersionSplit) != 1 && len(apiVersionSplit) != 2): return fmt.Errorf("apiVersion '%v' not of the format (/)", t.APIVersion) - } else if t.Kind == "" { + case t.Kind == "": return errors.New("kind not specified") - } else if t.Namespace == "" { + case t.Namespace == "": return errors.New("namespace not specified") - } else if t.Name == "" { + case t.Name == "": return errors.New("name not specified") - } else if t.Ref == "" { + case t.Ref == "": return errors.New("ref not specified") } diff --git a/pkg/test/step.go b/pkg/test/step.go index 077665ee..66795659 100644 --- a/pkg/test/step.go +++ b/pkg/test/step.go @@ -426,7 +426,7 @@ func (s *Step) CheckAssertExpressions( return []error{err} } - return testutils.RunAssertExpressions(ctx, s.Logger, client, s.Programs, resourceRefs, assertAny, assertAll) + return testutils.RunAssertExpressions(ctx, client, s.Programs, resourceRefs, assertAny, assertAll) } // Check checks if the resources defined in Asserts and Errors are in the correct state. @@ -533,8 +533,6 @@ func (s *Step) String() string { // if seen, mark a test immediately failed. // - All other YAML files are considered resources to create. func (s *Step) LoadYAML(file string) error { - s.Programs = make(map[string]cel.Program) - skipFile, objects, err := s.loadOrSkipFile(file) if skipFile || err != nil { return err @@ -574,6 +572,10 @@ func (s *Step) LoadYAML(file string) error { return fmt.Errorf("failed to load TestAssert object from %s: %w", file, err) } + if len(assertions) > 0 { + s.Programs = make(map[string]cel.Program) + } + for _, assertion := range assertions { if prg, err := assertion.BuildProgram(env); err != nil { errs = append(errs, err) From 6df2e32c696219f120eb4309359f423ae247f225 Mon Sep 17 00:00:00 2001 From: Kumar Mallikarjuna Date: Tue, 3 Dec 2024 22:36:43 +0530 Subject: [PATCH 08/31] chore: incorporate review comments Signed-off-by: Kumar Mallikarjuna --- pkg/apis/testharness/v1beta1/expression.go | 19 +---------------- .../expression.go => expressions/cel.go} | 16 +++++++++++++- pkg/test/step.go | 21 ++++++++++++++++--- 3 files changed, 34 insertions(+), 22 deletions(-) rename pkg/{test/utils/expression.go => expressions/cel.go} (60%) diff --git a/pkg/apis/testharness/v1beta1/expression.go b/pkg/apis/testharness/v1beta1/expression.go index 0739de2f..de210a2d 100644 --- a/pkg/apis/testharness/v1beta1/expression.go +++ b/pkg/apis/testharness/v1beta1/expression.go @@ -5,7 +5,6 @@ import ( "fmt" "strings" - "github.com/google/cel-go/cel" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" @@ -18,7 +17,7 @@ func (t *TestResourceRef) BuildResourceReference() (namespacedName types.Namespa Version: apiVersionSplit[len(apiVersionSplit)-1], Kind: t.Kind, } - if len(t.APIVersion) > 1 { + if len(apiVersionSplit) > 1 { gvk.Group = apiVersionSplit[0] } referencedResource.SetGroupVersionKind(gvk) @@ -38,8 +37,6 @@ func (t *TestResourceRef) Validate() error { return fmt.Errorf("apiVersion '%v' not of the format (/)", t.APIVersion) case t.Kind == "": return errors.New("kind not specified") - case t.Namespace == "": - return errors.New("namespace not specified") case t.Name == "": return errors.New("name not specified") case t.Ref == "": @@ -59,17 +56,3 @@ func (t *TestResourceRef) String() string { t.Ref, ) } - -func (t *Assertion) BuildProgram(env *cel.Env) (cel.Program, error) { - ast, issues := env.Compile(t.CELExpression) - if issues != nil && issues.Err() != nil { - return nil, fmt.Errorf("type-check error: %s", issues.Err()) - } - - prg, err := env.Program(ast) - if err != nil { - return nil, fmt.Errorf("program construction error: %w", err) - } - - return prg, nil -} diff --git a/pkg/test/utils/expression.go b/pkg/expressions/cel.go similarity index 60% rename from pkg/test/utils/expression.go rename to pkg/expressions/cel.go index 75b812f5..cd52fe7d 100644 --- a/pkg/test/utils/expression.go +++ b/pkg/expressions/cel.go @@ -1,4 +1,4 @@ -package utils +package expressions import ( "fmt" @@ -8,6 +8,20 @@ import ( "github.com/kudobuilder/kuttl/pkg/apis/testharness/v1beta1" ) +func BuildProgram(expr string, env *cel.Env) (cel.Program, error) { + ast, issues := env.Compile(expr) + if issues != nil && issues.Err() != nil { + return nil, fmt.Errorf("type-check error: %s", issues.Err()) + } + + prg, err := env.Program(ast) + if err != nil { + return nil, fmt.Errorf("program construction error: %w", err) + } + + return prg, nil +} + func BuildEnv(resourceRefs []v1beta1.TestResourceRef) (*cel.Env, error) { env, err := cel.NewEnv() if err != nil { diff --git a/pkg/test/step.go b/pkg/test/step.go index 66795659..9f656f91 100644 --- a/pkg/test/step.go +++ b/pkg/test/step.go @@ -24,6 +24,7 @@ import ( harness "github.com/kudobuilder/kuttl/pkg/apis/testharness/v1beta1" "github.com/kudobuilder/kuttl/pkg/env" + "github.com/kudobuilder/kuttl/pkg/expressions" kfile "github.com/kudobuilder/kuttl/pkg/file" "github.com/kudobuilder/kuttl/pkg/http" "github.com/kudobuilder/kuttl/pkg/kubernetes" @@ -426,7 +427,21 @@ func (s *Step) CheckAssertExpressions( return []error{err} } - return testutils.RunAssertExpressions(ctx, client, s.Programs, resourceRefs, assertAny, assertAll) + variables := make(map[string]interface{}) + for _, resourceRef := range resourceRefs { + namespacedName, referencedResource := resourceRef.BuildResourceReference() + if err := client.Get( + ctx, + namespacedName, + referencedResource, + ); err != nil { + return []error{fmt.Errorf("failed to get referenced resource '%v': %w", namespacedName, err)} + } + + variables[resourceRef.Ref] = referencedResource.Object + } + + return testutils.RunAssertExpressions(s.Programs, variables, assertAny, assertAll) } // Check checks if the resources defined in Asserts and Errors are in the correct state. @@ -567,7 +582,7 @@ func (s *Step) LoadYAML(file string) error { assertions = append(assertions, s.Assert.AssertAny...) assertions = append(assertions, s.Assert.AssertAll...) - env, err := testutils.BuildEnv(s.Assert.ResourceRefs) + env, err := expressions.BuildEnv(s.Assert.ResourceRefs) if err != nil { return fmt.Errorf("failed to load TestAssert object from %s: %w", file, err) } @@ -577,7 +592,7 @@ func (s *Step) LoadYAML(file string) error { } for _, assertion := range assertions { - if prg, err := assertion.BuildProgram(env); err != nil { + if prg, err := expressions.BuildProgram(assertion.CELExpression, env); err != nil { errs = append(errs, err) } else { s.Programs[assertion.CELExpression] = prg From aa6cb65316c96a06f1a619d1b4b3b52c145a26b7 Mon Sep 17 00:00:00 2001 From: Kumar Mallikarjuna Date: Tue, 3 Dec 2024 22:39:39 +0530 Subject: [PATCH 09/31] chore: move RunAssertExpressions() to pkg/expressions Signed-off-by: Kumar Mallikarjuna --- pkg/expressions/cel.go | 55 ++++++++++++++++++++++++++++++++++++++++++ pkg/test/step.go | 2 +- 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/pkg/expressions/cel.go b/pkg/expressions/cel.go index cd52fe7d..55fb23b7 100644 --- a/pkg/expressions/cel.go +++ b/pkg/expressions/cel.go @@ -1,6 +1,7 @@ package expressions import ( + "errors" "fmt" "github.com/google/cel-go/cel" @@ -37,3 +38,57 @@ func BuildEnv(resourceRefs []v1beta1.TestResourceRef) (*cel.Env, error) { return env, nil } + +// RunAssertExpressions evaluates a set of CEL expressions specified as AnyAllExpressions +func RunAssertExpressions( + programs map[string]cel.Program, + variables map[string]interface{}, + assertAny, + assertAll []*v1beta1.Assertion, +) []error { + var errs []error + if len(assertAny) == 0 && len(assertAll) == 0 { + return errs + } + + var anyExpressionsEvaluation, allExpressionsEvaluation []error + for _, expr := range assertAny { + prg, ok := programs[expr.CELExpression] + if !ok { + return []error{fmt.Errorf("couldn't find pre-built program for expression: %v", expr.CELExpression)} + } + out, _, err := prg.Eval(variables) + if err != nil { + return []error{fmt.Errorf("failed to evaluate program: %w", err)} + } + + if out.Value() != true { + anyExpressionsEvaluation = append(anyExpressionsEvaluation, fmt.Errorf("expression '%v' evaluated to '%v'", expr.CELExpression, out.Value())) + } + } + + for _, expr := range assertAll { + prg, ok := programs[expr.CELExpression] + if !ok { + return []error{fmt.Errorf("couldn't find pre-built program for expression: %v", expr.CELExpression)} + } + out, _, err := prg.Eval(variables) + if err != nil { + return []error{fmt.Errorf("failed to evaluate program: %w", err)} + } + + if out.Value() != true { + allExpressionsEvaluation = append(allExpressionsEvaluation, fmt.Errorf("expression '%v' evaluated to '%v'", expr.CELExpression, out.Value())) + } + } + + if len(assertAny) != 0 && len(anyExpressionsEvaluation) == len(assertAny) { + errs = append(errs, fmt.Errorf("no expression evaluated to true: %w", errors.Join(anyExpressionsEvaluation...))) + } + + if len(allExpressionsEvaluation) != len(assertAll) { + errs = append(errs, fmt.Errorf("not all expressions evaluated to true: %w", errors.Join(allExpressionsEvaluation...))) + } + + return errs +} diff --git a/pkg/test/step.go b/pkg/test/step.go index 9f656f91..e9a2e43b 100644 --- a/pkg/test/step.go +++ b/pkg/test/step.go @@ -441,7 +441,7 @@ func (s *Step) CheckAssertExpressions( variables[resourceRef.Ref] = referencedResource.Object } - return testutils.RunAssertExpressions(s.Programs, variables, assertAny, assertAll) + return expressions.RunAssertExpressions(s.Programs, variables, assertAny, assertAll) } // Check checks if the resources defined in Asserts and Errors are in the correct state. From 832bf9ee5a3c51e18bc4029ffe5d3313ff60aa3e Mon Sep 17 00:00:00 2001 From: Kumar Mallikarjuna Date: Tue, 3 Dec 2024 22:49:22 +0530 Subject: [PATCH 10/31] refactor: move CEL program loading to a dedicated function Signed-off-by: Kumar Mallikarjuna --- pkg/expressions/cel.go | 49 ++++++++++++++++++++++++++++++++++++++---- pkg/test/step.go | 35 ++---------------------------- 2 files changed, 47 insertions(+), 37 deletions(-) diff --git a/pkg/expressions/cel.go b/pkg/expressions/cel.go index 55fb23b7..65558281 100644 --- a/pkg/expressions/cel.go +++ b/pkg/expressions/cel.go @@ -6,10 +6,11 @@ import ( "github.com/google/cel-go/cel" - "github.com/kudobuilder/kuttl/pkg/apis/testharness/v1beta1" + harness "github.com/kudobuilder/kuttl/pkg/apis/testharness/v1beta1" + "github.com/kudobuilder/kuttl/pkg/test" ) -func BuildProgram(expr string, env *cel.Env) (cel.Program, error) { +func buildProgram(expr string, env *cel.Env) (cel.Program, error) { ast, issues := env.Compile(expr) if issues != nil && issues.Err() != nil { return nil, fmt.Errorf("type-check error: %s", issues.Err()) @@ -23,7 +24,7 @@ func BuildProgram(expr string, env *cel.Env) (cel.Program, error) { return prg, nil } -func BuildEnv(resourceRefs []v1beta1.TestResourceRef) (*cel.Env, error) { +func buildEnv(resourceRefs []harness.TestResourceRef) (*cel.Env, error) { env, err := cel.NewEnv() if err != nil { return nil, fmt.Errorf("failed to create environment: %w", err) @@ -44,7 +45,7 @@ func RunAssertExpressions( programs map[string]cel.Program, variables map[string]interface{}, assertAny, - assertAll []*v1beta1.Assertion, + assertAll []*harness.Assertion, ) []error { var errs []error if len(assertAny) == 0 && len(assertAll) == 0 { @@ -92,3 +93,43 @@ func RunAssertExpressions( return errs } + +func LoadPrograms(s *test.Step) error { + var errs []error + for _, resourceRef := range s.Assert.ResourceRefs { + if err := resourceRef.Validate(); err != nil { + errs = append(errs, fmt.Errorf("validation failed for reference '%v': %w", resourceRef.String(), err)) + } + } + + if len(errs) > 0 { + return fmt.Errorf("failed to load resource reference(s): %w", errors.Join(errs...)) + } + + var assertions []*harness.Assertion + assertions = append(assertions, s.Assert.AssertAny...) + assertions = append(assertions, s.Assert.AssertAll...) + + env, err := buildEnv(s.Assert.ResourceRefs) + if err != nil { + return fmt.Errorf("failed to build environment: %w", err) + } + + if len(assertions) > 0 { + s.Programs = make(map[string]cel.Program) + } + + for _, assertion := range assertions { + if prg, err := buildProgram(assertion.CELExpression, env); err != nil { + errs = append(errs, err) + } else { + s.Programs[assertion.CELExpression] = prg + } + } + + if len(errs) > 0 { + return fmt.Errorf("failed to build program(s): %w", errors.Join(errs...)) + } + + return nil +} diff --git a/pkg/test/step.go b/pkg/test/step.go index e9a2e43b..2a64e259 100644 --- a/pkg/test/step.go +++ b/pkg/test/step.go @@ -567,40 +567,9 @@ func (s *Step) LoadYAML(file string) error { return fmt.Errorf("failed to load TestAssert object from %s: it contains an object of type %T", file, obj) } - var errs []error - for _, resourceRef := range s.Assert.ResourceRefs { - if err := resourceRef.Validate(); err != nil { - errs = append(errs, fmt.Errorf("validation failed for reference '%v': %w", resourceRef.String(), err)) - } - } - - if len(errs) > 0 { - return fmt.Errorf("failed to load TestAssert object from %s: %w", file, errors.Join(errs...)) - } - - var assertions []*harness.Assertion - assertions = append(assertions, s.Assert.AssertAny...) - assertions = append(assertions, s.Assert.AssertAll...) - - env, err := expressions.BuildEnv(s.Assert.ResourceRefs) + err := expressions.LoadPrograms(s) if err != nil { - return fmt.Errorf("failed to load TestAssert object from %s: %w", file, err) - } - - if len(assertions) > 0 { - s.Programs = make(map[string]cel.Program) - } - - for _, assertion := range assertions { - if prg, err := expressions.BuildProgram(assertion.CELExpression, env); err != nil { - errs = append(errs, err) - } else { - s.Programs[assertion.CELExpression] = prg - } - } - - if len(errs) > 0 { - return fmt.Errorf("failed to load TestAssert object from %s: %w", file, errors.Join(errs...)) + return fmt.Errorf("failed to load programs: %w", err) } } else { asserts = append(asserts, obj) From e32b53e7727a9fce5b4d5c2e8a106b10cea46ad7 Mon Sep 17 00:00:00 2001 From: Kumar Mallikarjuna Date: Tue, 3 Dec 2024 22:53:06 +0530 Subject: [PATCH 11/31] refactor: move program-loading to Step out of LoadPrograms() Signed-off-by: Kumar Mallikarjuna --- pkg/expressions/cel.go | 24 ++++++++++++------------ pkg/test/step.go | 2 +- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/pkg/expressions/cel.go b/pkg/expressions/cel.go index 65558281..e25a331c 100644 --- a/pkg/expressions/cel.go +++ b/pkg/expressions/cel.go @@ -7,7 +7,6 @@ import ( "github.com/google/cel-go/cel" harness "github.com/kudobuilder/kuttl/pkg/apis/testharness/v1beta1" - "github.com/kudobuilder/kuttl/pkg/test" ) func buildProgram(expr string, env *cel.Env) (cel.Program, error) { @@ -94,42 +93,43 @@ func RunAssertExpressions( return errs } -func LoadPrograms(s *test.Step) error { +func LoadPrograms(testAssert *harness.TestAssert) (map[string]cel.Program, error) { var errs []error - for _, resourceRef := range s.Assert.ResourceRefs { + for _, resourceRef := range testAssert.ResourceRefs { if err := resourceRef.Validate(); err != nil { errs = append(errs, fmt.Errorf("validation failed for reference '%v': %w", resourceRef.String(), err)) } } if len(errs) > 0 { - return fmt.Errorf("failed to load resource reference(s): %w", errors.Join(errs...)) + return nil, fmt.Errorf("failed to load resource reference(s): %w", errors.Join(errs...)) } var assertions []*harness.Assertion - assertions = append(assertions, s.Assert.AssertAny...) - assertions = append(assertions, s.Assert.AssertAll...) + assertions = append(assertions, testAssert.AssertAny...) + assertions = append(assertions, testAssert.AssertAll...) - env, err := buildEnv(s.Assert.ResourceRefs) + env, err := buildEnv(testAssert.ResourceRefs) if err != nil { - return fmt.Errorf("failed to build environment: %w", err) + return nil, fmt.Errorf("failed to build environment: %w", err) } + var programs map[string]cel.Program if len(assertions) > 0 { - s.Programs = make(map[string]cel.Program) + programs = make(map[string]cel.Program) } for _, assertion := range assertions { if prg, err := buildProgram(assertion.CELExpression, env); err != nil { errs = append(errs, err) } else { - s.Programs[assertion.CELExpression] = prg + programs[assertion.CELExpression] = prg } } if len(errs) > 0 { - return fmt.Errorf("failed to build program(s): %w", errors.Join(errs...)) + return nil, fmt.Errorf("failed to build program(s): %w", errors.Join(errs...)) } - return nil + return programs, nil } diff --git a/pkg/test/step.go b/pkg/test/step.go index 2a64e259..f80ba512 100644 --- a/pkg/test/step.go +++ b/pkg/test/step.go @@ -567,7 +567,7 @@ func (s *Step) LoadYAML(file string) error { return fmt.Errorf("failed to load TestAssert object from %s: it contains an object of type %T", file, obj) } - err := expressions.LoadPrograms(s) + s.Programs, err = expressions.LoadPrograms(s.Assert) if err != nil { return fmt.Errorf("failed to load programs: %w", err) } From 3223ada5a955bd12ab786560d5eb2385c11f8dfa Mon Sep 17 00:00:00 2001 From: Kumar Mallikarjuna Date: Wed, 4 Dec 2024 20:42:03 +0530 Subject: [PATCH 12/31] chore: add tests for `TestResourceRef` Signed-off-by: Kumar Mallikarjuna --- pkg/apis/testharness/v1beta1/expression.go | 15 +- .../testharness/v1beta1/expression_test.go | 181 ++++++++++++++++++ 2 files changed, 192 insertions(+), 4 deletions(-) create mode 100644 pkg/apis/testharness/v1beta1/expression_test.go diff --git a/pkg/apis/testharness/v1beta1/expression.go b/pkg/apis/testharness/v1beta1/expression.go index de210a2d..2209c074 100644 --- a/pkg/apis/testharness/v1beta1/expression.go +++ b/pkg/apis/testharness/v1beta1/expression.go @@ -10,6 +10,13 @@ import ( "k8s.io/apimachinery/pkg/types" ) +var ( + apiVersionInvalidErr = errors.New("apiVersion not of the format (/)") + kindNotSpecifiedErr = errors.New("kind not specified") + nameNotSpecifiedErr = errors.New("name not specified") + refNotSpecifiedErr = errors.New("ref not specified") +) + func (t *TestResourceRef) BuildResourceReference() (namespacedName types.NamespacedName, referencedResource *unstructured.Unstructured) { referencedResource = &unstructured.Unstructured{} apiVersionSplit := strings.Split(t.APIVersion, "/") @@ -34,13 +41,13 @@ func (t *TestResourceRef) Validate() error { apiVersionSplit := strings.Split(t.APIVersion, "/") switch { case t.APIVersion == "" || (len(apiVersionSplit) != 1 && len(apiVersionSplit) != 2): - return fmt.Errorf("apiVersion '%v' not of the format (/)", t.APIVersion) + return apiVersionInvalidErr case t.Kind == "": - return errors.New("kind not specified") + return kindNotSpecifiedErr case t.Name == "": - return errors.New("name not specified") + return nameNotSpecifiedErr case t.Ref == "": - return errors.New("ref not specified") + return refNotSpecifiedErr } return nil diff --git a/pkg/apis/testharness/v1beta1/expression_test.go b/pkg/apis/testharness/v1beta1/expression_test.go new file mode 100644 index 00000000..b69b6b00 --- /dev/null +++ b/pkg/apis/testharness/v1beta1/expression_test.go @@ -0,0 +1,181 @@ +package v1beta1 + +import ( + "reflect" + "testing" + + "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" +) + +func TestValidate(t *testing.T) { + testCases := []struct { + name string + testResourceRef TestResourceRef + errored bool + expectedError error + }{ + { + name: "apiVersion is not specified", + testResourceRef: TestResourceRef{ + Kind: "Pod", + Namespace: "test", + Name: "test-pod", + Ref: "testPod", + }, + errored: true, + expectedError: apiVersionInvalidErr, + }, + { + name: "apiVersion is invalid", + testResourceRef: TestResourceRef{ + APIVersion: "x/y/z", + Kind: "Pod", + Namespace: "test", + Name: "test-pod", + Ref: "testPod", + }, + errored: true, + expectedError: apiVersionInvalidErr, + }, + { + name: "apiVersion is valid and group is vacuous", + testResourceRef: TestResourceRef{ + APIVersion: "v1", + Kind: "Pod", + Namespace: "test", + Name: "test-pod", + Ref: "testPod", + }, + errored: false, + }, + { + name: "apiVersion has both group name and version", + testResourceRef: TestResourceRef{ + APIVersion: "apps/v1", + Kind: "Deployment", + Namespace: "test", + Name: "test-deployment", + Ref: "testDeployment", + }, + errored: false, + }, + { + name: "kind is not specified", + testResourceRef: TestResourceRef{ + APIVersion: "apps/v1", + Namespace: "test", + Name: "test-deployment", + Ref: "testDeployment", + }, + errored: true, + expectedError: kindNotSpecifiedErr, + }, + { + name: "name is not specified", + testResourceRef: TestResourceRef{ + APIVersion: "apps/v1", + Kind: "Deployment", + Namespace: "test", + Ref: "testDeployment", + }, + errored: true, + expectedError: nameNotSpecifiedErr, + }, + { + name: "ref is not specified", + testResourceRef: TestResourceRef{ + APIVersion: "apps/v1", + Kind: "Deployment", + Namespace: "test", + Name: "test-deployment", + }, + errored: true, + expectedError: refNotSpecifiedErr, + }, + { + name: "all attributes are present and valid", + testResourceRef: TestResourceRef{ + APIVersion: "apps/v1", + Kind: "Deployment", + Namespace: "test", + Name: "test-deployment", + Ref: "testDeployment", + }, + errored: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + err := tc.testResourceRef.Validate() + if !tc.errored { + assert.NoError(t, err) + } else { + assert.ErrorIs(t, err, tc.expectedError) + } + }) + } +} + +func TestBuildResourceReference(t *testing.T) { + buildObject := func(gvk schema.GroupVersionKind) *unstructured.Unstructured { + obj := &unstructured.Unstructured{} + obj.SetGroupVersionKind(gvk) + return obj + } + + testCases := []struct { + name string + testResourceRef TestResourceRef + namespacedName types.NamespacedName + resourceReference *unstructured.Unstructured + }{ + { + name: "group name is vacuous", + testResourceRef: TestResourceRef{ + APIVersion: "v1", + Kind: "Pod", + Namespace: "test", + Name: "test-pod", + Ref: "testPod", + }, + namespacedName: types.NamespacedName{ + Namespace: "test", + Name: "test-pod", + }, + resourceReference: buildObject(schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Pod"}), + }, + { + name: "group name is present", + testResourceRef: TestResourceRef{ + APIVersion: "apps/v1", + Kind: "Deployment", + Namespace: "test", + Name: "test-deployment", + Ref: "testDeployment", + }, + namespacedName: types.NamespacedName{ + Namespace: "test", + Name: "test-deployment", + }, + resourceReference: buildObject(schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"}), + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + namspacedName, referencedResource := tc.testResourceRef.BuildResourceReference() + assert.Equal(t, tc.namespacedName, namspacedName) + assert.True( + t, + reflect.DeepEqual(tc.resourceReference, referencedResource), + "constructed unstructured reference does not match, expected '%s', got '%s'", + tc.resourceReference, + referencedResource, + ) + }) + } +} From f08a27e63ebe523a532d9d83bd446de66d6fc78c Mon Sep 17 00:00:00 2001 From: Kumar Mallikarjuna Date: Wed, 4 Dec 2024 22:05:57 +0530 Subject: [PATCH 13/31] fix: correct evaluation for `assertAll` Signed-off-by: Kumar Mallikarjuna --- pkg/expressions/cel.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/expressions/cel.go b/pkg/expressions/cel.go index e25a331c..711ff028 100644 --- a/pkg/expressions/cel.go +++ b/pkg/expressions/cel.go @@ -86,7 +86,7 @@ func RunAssertExpressions( errs = append(errs, fmt.Errorf("no expression evaluated to true: %w", errors.Join(anyExpressionsEvaluation...))) } - if len(allExpressionsEvaluation) != len(assertAll) { + if len(allExpressionsEvaluation) > 0 { errs = append(errs, fmt.Errorf("not all expressions evaluated to true: %w", errors.Join(allExpressionsEvaluation...))) } From 7cb27fc8ce229e1670b940ab415d10799b77a025 Mon Sep 17 00:00:00 2001 From: Kumar Mallikarjuna Date: Wed, 4 Dec 2024 23:16:30 +0530 Subject: [PATCH 14/31] chore: add integration tests for expressions Signed-off-by: Kumar Mallikarjuna --- pkg/test/expression_integration_test.go | 118 ++++++++++++++++++ .../check_deployment_name/00-assert.yaml | 11 ++ .../00-assert.yaml | 11 ++ .../check_multiple_assert_all/00-assert.yaml | 17 +++ .../00-assert.yaml | 17 +++ .../check_multiple_assert_any/00-assert.yaml | 17 +++ .../00-assert.yaml | 17 +++ .../invalid_expression/00-assert.yaml | 11 ++ 8 files changed, 219 insertions(+) create mode 100644 pkg/test/expression_integration_test.go create mode 100644 pkg/test/step_integration_test_data/assert_expressions/check_deployment_name/00-assert.yaml create mode 100644 pkg/test/step_integration_test_data/assert_expressions/check_incorrect_deployment_name/00-assert.yaml create mode 100644 pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_all/00-assert.yaml create mode 100644 pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_all_with_one_failing/00-assert.yaml create mode 100644 pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_any/00-assert.yaml create mode 100644 pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_any_with_all_failing/00-assert.yaml create mode 100644 pkg/test/step_integration_test_data/assert_expressions/invalid_expression/00-assert.yaml diff --git a/pkg/test/expression_integration_test.go b/pkg/test/expression_integration_test.go new file mode 100644 index 00000000..529d0c29 --- /dev/null +++ b/pkg/test/expression_integration_test.go @@ -0,0 +1,118 @@ +package test + +import ( + "errors" + "fmt" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/discovery" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + testutils "github.com/kudobuilder/kuttl/pkg/test/utils" +) + +func TestAssertExpressions(t *testing.T) { + codednsDeployment := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "coredns", + Namespace: "kube-system", + }, + } + metricServerPod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "metrics-server-xyz-pqr", + Namespace: "kube-system", + Labels: map[string]string{ + "app": "metrics-server", + }, + }, + } + + buildTestStep := func(tcName string) *Step { + return &Step{ + Name: t.Name(), + Index: 0, + Logger: testutils.NewTestLogger(t, tcName), + Client: func(bool) (client.Client, error) { + return fake. + NewClientBuilder(). + WithObjects(codednsDeployment, metricServerPod). + WithScheme(testutils.Scheme()). + Build(), nil + }, + DiscoveryClient: func() (discovery.DiscoveryInterface, error) { + return testutils.FakeDiscoveryClient(), nil + }, + } + } + + testCases := []struct { + name string + loadingFailed bool + runFailed bool + errorMessage string + }{ + { + name: "invalid expression", + loadingFailed: true, + errorMessage: "undeclared reference", + }, + { + name: "check deployment name", + }, + { + name: "check incorrect deployment name", + runFailed: true, + errorMessage: "not all expressions evaluated to true", + }, + { + name: "check multiple assert all", + }, + { + name: "check multiple assert all with one failing", + runFailed: true, + errorMessage: "not all expressions evaluated to true", + }, + { + name: "check multiple assert any", + }, + { + name: "check multiple assert any with all failing", + runFailed: true, + errorMessage: "no expression evaluated to true", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + step := buildTestStep(tc.name) + + fName := fmt.Sprintf( + "step_integration_test_data/assert_expressions/%s/00-assert.yaml", + strings.Replace(tc.name, " ", "_", -1), + ) + + // Load test that has an invalid expression + err := step.LoadYAML(fName) + if !tc.loadingFailed { + assert.NoError(t, err) + } else { + assert.ErrorContains(t, err, tc.errorMessage) + return + } + + err = errors.Join(step.Run(t, "")...) + if !tc.runFailed { + assert.NoError(t, err) + } else { + assert.ErrorContains(t, err, tc.errorMessage) + } + }) + } +} diff --git a/pkg/test/step_integration_test_data/assert_expressions/check_deployment_name/00-assert.yaml b/pkg/test/step_integration_test_data/assert_expressions/check_deployment_name/00-assert.yaml new file mode 100644 index 00000000..9b0cda37 --- /dev/null +++ b/pkg/test/step_integration_test_data/assert_expressions/check_deployment_name/00-assert.yaml @@ -0,0 +1,11 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestAssert +resourceRefs: + - apiVersion: apps/v1 + kind: Deployment + namespace: kube-system + name: coredns + ref: coredns +assertAll: + - celExpr: "coredns.metadata.name == 'coredns'" +timeout: 1 diff --git a/pkg/test/step_integration_test_data/assert_expressions/check_incorrect_deployment_name/00-assert.yaml b/pkg/test/step_integration_test_data/assert_expressions/check_incorrect_deployment_name/00-assert.yaml new file mode 100644 index 00000000..3ec2e308 --- /dev/null +++ b/pkg/test/step_integration_test_data/assert_expressions/check_incorrect_deployment_name/00-assert.yaml @@ -0,0 +1,11 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestAssert +resourceRefs: + - apiVersion: apps/v1 + kind: Deployment + namespace: kube-system + name: coredns + ref: coredns +assertAll: + - celExpr: "coredns.metadata.name == 'metrics-server'" +timeout: 1 diff --git a/pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_all/00-assert.yaml b/pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_all/00-assert.yaml new file mode 100644 index 00000000..64855289 --- /dev/null +++ b/pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_all/00-assert.yaml @@ -0,0 +1,17 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestAssert +resourceRefs: + - apiVersion: apps/v1 + kind: Deployment + namespace: kube-system + name: coredns + ref: coredns + - apiVersion: v1 + kind: Pod + namespace: kube-system + name: metrics-server-xyz-pqr + ref: metricsServer +assertAll: + - celExpr: "coredns.metadata.name == 'coredns'" + - celExpr: "metricsServer.metadata.labels['app'] == 'metrics-server'" +timeout: 1 diff --git a/pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_all_with_one_failing/00-assert.yaml b/pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_all_with_one_failing/00-assert.yaml new file mode 100644 index 00000000..7f850071 --- /dev/null +++ b/pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_all_with_one_failing/00-assert.yaml @@ -0,0 +1,17 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestAssert +resourceRefs: + - apiVersion: apps/v1 + kind: Deployment + namespace: kube-system + name: coredns + ref: coredns + - apiVersion: v1 + kind: Pod + namespace: kube-system + name: metrics-server-xyz-pqr + ref: metricsServer +assertAll: + - celExpr: "coredns.metadata.name == 'metrics-server'" + - celExpr: "metricsServer.metadata.labels['app'] == 'metrics-server'" +timeout: 1 diff --git a/pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_any/00-assert.yaml b/pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_any/00-assert.yaml new file mode 100644 index 00000000..64659111 --- /dev/null +++ b/pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_any/00-assert.yaml @@ -0,0 +1,17 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestAssert +resourceRefs: + - apiVersion: apps/v1 + kind: Deployment + namespace: kube-system + name: coredns + ref: coredns + - apiVersion: v1 + kind: Pod + namespace: kube-system + name: metrics-server-xyz-pqr + ref: metricsServer +assertAny: + - celExpr: "coredns.metadata.name == 'coredns'" + - celExpr: "metricsServer.metadata.labels['app'] == 'metrics-server-1.6'" +timeout: 1 diff --git a/pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_any_with_all_failing/00-assert.yaml b/pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_any_with_all_failing/00-assert.yaml new file mode 100644 index 00000000..786b600f --- /dev/null +++ b/pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_any_with_all_failing/00-assert.yaml @@ -0,0 +1,17 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestAssert +resourceRefs: + - apiVersion: apps/v1 + kind: Deployment + namespace: kube-system + name: coredns + ref: coredns + - apiVersion: v1 + kind: Pod + namespace: kube-system + name: metrics-server-xyz-pqr + ref: metricsServer +assertAny: + - celExpr: "coredns.metadata.name == 'metrics-server'" + - celExpr: "metricsServer.metadata.labels['app'] == 'metrics-server-1.6'" +timeout: 1 diff --git a/pkg/test/step_integration_test_data/assert_expressions/invalid_expression/00-assert.yaml b/pkg/test/step_integration_test_data/assert_expressions/invalid_expression/00-assert.yaml new file mode 100644 index 00000000..86a8b392 --- /dev/null +++ b/pkg/test/step_integration_test_data/assert_expressions/invalid_expression/00-assert.yaml @@ -0,0 +1,11 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestAssert +resourceRefs: + - apiVersion: apps/v1 + kind: Deployment + namespace: kube-system + name: coredns + ref: coredns +assertAll: + - celExpr: "badVariable.metadata.name == 'coredns'" +timeout: 1 From 3fcf0187d1d41fc7b09be97a7682eab6d5e79c90 Mon Sep 17 00:00:00 2001 From: Kumar Mallikarjuna Date: Wed, 4 Dec 2024 23:46:41 +0530 Subject: [PATCH 15/31] chore: make linter happy Signed-off-by: Kumar Mallikarjuna --- pkg/apis/testharness/v1beta1/expression.go | 16 ++++++++-------- pkg/apis/testharness/v1beta1/expression_test.go | 10 +++++----- pkg/test/expression_integration_test.go | 2 +- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/pkg/apis/testharness/v1beta1/expression.go b/pkg/apis/testharness/v1beta1/expression.go index 2209c074..f696ec02 100644 --- a/pkg/apis/testharness/v1beta1/expression.go +++ b/pkg/apis/testharness/v1beta1/expression.go @@ -11,10 +11,10 @@ import ( ) var ( - apiVersionInvalidErr = errors.New("apiVersion not of the format (/)") - kindNotSpecifiedErr = errors.New("kind not specified") - nameNotSpecifiedErr = errors.New("name not specified") - refNotSpecifiedErr = errors.New("ref not specified") + errAPIVersionInvalid = errors.New("apiVersion not of the format (/)") + errKindNotSpecified = errors.New("kind not specified") + errNameNotSpecified = errors.New("name not specified") + errRefNotSpecified = errors.New("ref not specified") ) func (t *TestResourceRef) BuildResourceReference() (namespacedName types.NamespacedName, referencedResource *unstructured.Unstructured) { @@ -41,13 +41,13 @@ func (t *TestResourceRef) Validate() error { apiVersionSplit := strings.Split(t.APIVersion, "/") switch { case t.APIVersion == "" || (len(apiVersionSplit) != 1 && len(apiVersionSplit) != 2): - return apiVersionInvalidErr + return errAPIVersionInvalid case t.Kind == "": - return kindNotSpecifiedErr + return errKindNotSpecified case t.Name == "": - return nameNotSpecifiedErr + return errNameNotSpecified case t.Ref == "": - return refNotSpecifiedErr + return errRefNotSpecified } return nil diff --git a/pkg/apis/testharness/v1beta1/expression_test.go b/pkg/apis/testharness/v1beta1/expression_test.go index b69b6b00..cac25e4d 100644 --- a/pkg/apis/testharness/v1beta1/expression_test.go +++ b/pkg/apis/testharness/v1beta1/expression_test.go @@ -26,7 +26,7 @@ func TestValidate(t *testing.T) { Ref: "testPod", }, errored: true, - expectedError: apiVersionInvalidErr, + expectedError: errAPIVersionInvalid, }, { name: "apiVersion is invalid", @@ -38,7 +38,7 @@ func TestValidate(t *testing.T) { Ref: "testPod", }, errored: true, - expectedError: apiVersionInvalidErr, + expectedError: errAPIVersionInvalid, }, { name: "apiVersion is valid and group is vacuous", @@ -71,7 +71,7 @@ func TestValidate(t *testing.T) { Ref: "testDeployment", }, errored: true, - expectedError: kindNotSpecifiedErr, + expectedError: errKindNotSpecified, }, { name: "name is not specified", @@ -82,7 +82,7 @@ func TestValidate(t *testing.T) { Ref: "testDeployment", }, errored: true, - expectedError: nameNotSpecifiedErr, + expectedError: errNameNotSpecified, }, { name: "ref is not specified", @@ -93,7 +93,7 @@ func TestValidate(t *testing.T) { Name: "test-deployment", }, errored: true, - expectedError: refNotSpecifiedErr, + expectedError: errRefNotSpecified, }, { name: "all attributes are present and valid", diff --git a/pkg/test/expression_integration_test.go b/pkg/test/expression_integration_test.go index 529d0c29..2dac15bf 100644 --- a/pkg/test/expression_integration_test.go +++ b/pkg/test/expression_integration_test.go @@ -95,7 +95,7 @@ func TestAssertExpressions(t *testing.T) { fName := fmt.Sprintf( "step_integration_test_data/assert_expressions/%s/00-assert.yaml", - strings.Replace(tc.name, " ", "_", -1), + strings.ReplaceAll(tc.name, " ", "_"), ) // Load test that has an invalid expression From c6c3e8a333472499a25442614adb16ced01cc7ca Mon Sep 17 00:00:00 2001 From: Kumar Mallikarjuna Date: Sat, 14 Dec 2024 12:02:03 +0530 Subject: [PATCH 16/31] chore: incorporate review comments Signed-off-by: Kumar Mallikarjuna --- pkg/apis/testharness/v1beta1/expression.go | 2 +- pkg/expressions/cel.go | 79 ++++++++++++---------- pkg/test/expression_integration_test.go | 36 +++++----- pkg/test/step.go | 6 +- 4 files changed, 63 insertions(+), 60 deletions(-) diff --git a/pkg/apis/testharness/v1beta1/expression.go b/pkg/apis/testharness/v1beta1/expression.go index f696ec02..2ff215cb 100644 --- a/pkg/apis/testharness/v1beta1/expression.go +++ b/pkg/apis/testharness/v1beta1/expression.go @@ -40,7 +40,7 @@ func (t *TestResourceRef) BuildResourceReference() (namespacedName types.Namespa func (t *TestResourceRef) Validate() error { apiVersionSplit := strings.Split(t.APIVersion, "/") switch { - case t.APIVersion == "" || (len(apiVersionSplit) != 1 && len(apiVersionSplit) != 2): + case t.APIVersion == "" || len(apiVersionSplit) > 2: return errAPIVersionInvalid case t.Kind == "": return errKindNotSpecified diff --git a/pkg/expressions/cel.go b/pkg/expressions/cel.go index 711ff028..acb9f98c 100644 --- a/pkg/expressions/cel.go +++ b/pkg/expressions/cel.go @@ -24,6 +24,17 @@ func buildProgram(expr string, env *cel.Env) (cel.Program, error) { } func buildEnv(resourceRefs []harness.TestResourceRef) (*cel.Env, error) { + var errs []error + for _, resourceRef := range resourceRefs { + if err := resourceRef.Validate(); err != nil { + errs = append(errs, fmt.Errorf("validation failed for reference '%v': %w", resourceRef.String(), err)) + } + } + + if len(errs) > 0 { + return nil, fmt.Errorf("failed to load resource reference(s): %w", errors.Join(errs...)) + } + env, err := cel.NewEnv() if err != nil { return nil, fmt.Errorf("failed to create environment: %w", err) @@ -53,32 +64,14 @@ func RunAssertExpressions( var anyExpressionsEvaluation, allExpressionsEvaluation []error for _, expr := range assertAny { - prg, ok := programs[expr.CELExpression] - if !ok { - return []error{fmt.Errorf("couldn't find pre-built program for expression: %v", expr.CELExpression)} - } - out, _, err := prg.Eval(variables) - if err != nil { - return []error{fmt.Errorf("failed to evaluate program: %w", err)} - } - - if out.Value() != true { - anyExpressionsEvaluation = append(anyExpressionsEvaluation, fmt.Errorf("expression '%v' evaluated to '%v'", expr.CELExpression, out.Value())) + if err := evaluateExpression(expr.CELExpression, programs, variables); err != nil { + anyExpressionsEvaluation = append(anyExpressionsEvaluation, err) } } for _, expr := range assertAll { - prg, ok := programs[expr.CELExpression] - if !ok { - return []error{fmt.Errorf("couldn't find pre-built program for expression: %v", expr.CELExpression)} - } - out, _, err := prg.Eval(variables) - if err != nil { - return []error{fmt.Errorf("failed to evaluate program: %w", err)} - } - - if out.Value() != true { - allExpressionsEvaluation = append(allExpressionsEvaluation, fmt.Errorf("expression '%v' evaluated to '%v'", expr.CELExpression, out.Value())) + if err := evaluateExpression(expr.CELExpression, programs, variables); err != nil { + anyExpressionsEvaluation = append(anyExpressionsEvaluation, err) } } @@ -95,16 +88,6 @@ func RunAssertExpressions( func LoadPrograms(testAssert *harness.TestAssert) (map[string]cel.Program, error) { var errs []error - for _, resourceRef := range testAssert.ResourceRefs { - if err := resourceRef.Validate(); err != nil { - errs = append(errs, fmt.Errorf("validation failed for reference '%v': %w", resourceRef.String(), err)) - } - } - - if len(errs) > 0 { - return nil, fmt.Errorf("failed to load resource reference(s): %w", errors.Join(errs...)) - } - var assertions []*harness.Assertion assertions = append(assertions, testAssert.AssertAny...) assertions = append(assertions, testAssert.AssertAll...) @@ -115,21 +98,45 @@ func LoadPrograms(testAssert *harness.TestAssert) (map[string]cel.Program, error } var programs map[string]cel.Program - if len(assertions) > 0 { - programs = make(map[string]cel.Program) + if len(assertions) == 0 { + return programs, nil } + programs = make(map[string]cel.Program) for _, assertion := range assertions { if prg, err := buildProgram(assertion.CELExpression, env); err != nil { - errs = append(errs, err) + errs = append( + errs, + fmt.Errorf("failed to parse CEL expression '%v': %w", assertion.CELExpression, err), + ) } else { programs[assertion.CELExpression] = prg } } if len(errs) > 0 { - return nil, fmt.Errorf("failed to build program(s): %w", errors.Join(errs...)) + return nil, fmt.Errorf("failed to parse expression(s): %w", errors.Join(errs...)) } return programs, nil } + +func evaluateExpression(expr string, + programs map[string]cel.Program, + variables map[string]interface{}, +) error { + prg, ok := programs[expr] + if !ok { + return fmt.Errorf("couldn't find pre-built parsed CEL expression '%v'", expr) + } + out, _, err := prg.Eval(variables) + if err != nil { + return fmt.Errorf("failed to evaluate CEL expression: %w", err) + } + + if out.Value() != true { + return fmt.Errorf("expression '%v' evaluated to '%v'", expr, out.Value()) + } + + return nil +} diff --git a/pkg/test/expression_integration_test.go b/pkg/test/expression_integration_test.go index 2dac15bf..5c5693fd 100644 --- a/pkg/test/expression_integration_test.go +++ b/pkg/test/expression_integration_test.go @@ -17,7 +17,7 @@ import ( testutils "github.com/kudobuilder/kuttl/pkg/test/utils" ) -func TestAssertExpressions(t *testing.T) { +func buildTestStep(t *testing.T, tcName string) *Step { codednsDeployment := &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ Name: "coredns", @@ -34,24 +34,24 @@ func TestAssertExpressions(t *testing.T) { }, } - buildTestStep := func(tcName string) *Step { - return &Step{ - Name: t.Name(), - Index: 0, - Logger: testutils.NewTestLogger(t, tcName), - Client: func(bool) (client.Client, error) { - return fake. - NewClientBuilder(). - WithObjects(codednsDeployment, metricServerPod). - WithScheme(testutils.Scheme()). - Build(), nil - }, - DiscoveryClient: func() (discovery.DiscoveryInterface, error) { - return testutils.FakeDiscoveryClient(), nil - }, - } + return &Step{ + Name: t.Name(), + Index: 0, + Logger: testutils.NewTestLogger(t, tcName), + Client: func(bool) (client.Client, error) { + return fake. + NewClientBuilder(). + WithObjects(codednsDeployment, metricServerPod). + WithScheme(testutils.Scheme()). + Build(), nil + }, + DiscoveryClient: func() (discovery.DiscoveryInterface, error) { + return testutils.FakeDiscoveryClient(), nil + }, } +} +func TestAssertExpressions(t *testing.T) { testCases := []struct { name string loadingFailed bool @@ -91,7 +91,7 @@ func TestAssertExpressions(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - step := buildTestStep(tc.name) + step := buildTestStep(t, tc.name) fName := fmt.Sprintf( "step_integration_test_data/assert_expressions/%s/00-assert.yaml", diff --git a/pkg/test/step.go b/pkg/test/step.go index f80ba512..3842abb6 100644 --- a/pkg/test/step.go +++ b/pkg/test/step.go @@ -430,11 +430,7 @@ func (s *Step) CheckAssertExpressions( variables := make(map[string]interface{}) for _, resourceRef := range resourceRefs { namespacedName, referencedResource := resourceRef.BuildResourceReference() - if err := client.Get( - ctx, - namespacedName, - referencedResource, - ); err != nil { + if err := client.Get(ctx, namespacedName, referencedResource); err != nil { return []error{fmt.Errorf("failed to get referenced resource '%v': %w", namespacedName, err)} } From 51e3debd8aa37a573980302c36476a65cf8e64fd Mon Sep 17 00:00:00 2001 From: Kumar Mallikarjuna Date: Sat, 14 Dec 2024 13:23:25 +0530 Subject: [PATCH 17/31] fix: change array reference for all-asserts Signed-off-by: Kumar Mallikarjuna --- pkg/expressions/cel.go | 2 +- pkg/test/expression_integration_test.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/expressions/cel.go b/pkg/expressions/cel.go index acb9f98c..37ec9d48 100644 --- a/pkg/expressions/cel.go +++ b/pkg/expressions/cel.go @@ -71,7 +71,7 @@ func RunAssertExpressions( for _, expr := range assertAll { if err := evaluateExpression(expr.CELExpression, programs, variables); err != nil { - anyExpressionsEvaluation = append(anyExpressionsEvaluation, err) + allExpressionsEvaluation = append(allExpressionsEvaluation, err) } } diff --git a/pkg/test/expression_integration_test.go b/pkg/test/expression_integration_test.go index 5c5693fd..5c6e657e 100644 --- a/pkg/test/expression_integration_test.go +++ b/pkg/test/expression_integration_test.go @@ -17,7 +17,7 @@ import ( testutils "github.com/kudobuilder/kuttl/pkg/test/utils" ) -func buildTestStep(t *testing.T, tcName string) *Step { +func buildTestStep(t *testing.T) *Step { codednsDeployment := &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ Name: "coredns", @@ -37,7 +37,7 @@ func buildTestStep(t *testing.T, tcName string) *Step { return &Step{ Name: t.Name(), Index: 0, - Logger: testutils.NewTestLogger(t, tcName), + Logger: testutils.NewTestLogger(t, t.Name()), Client: func(bool) (client.Client, error) { return fake. NewClientBuilder(). @@ -91,7 +91,7 @@ func TestAssertExpressions(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - step := buildTestStep(t, tc.name) + step := buildTestStep(t) fName := fmt.Sprintf( "step_integration_test_data/assert_expressions/%s/00-assert.yaml", From 32330e9a655d46834014424249e4fe01da12af71 Mon Sep 17 00:00:00 2001 From: Kumar Mallikarjuna Date: Tue, 17 Dec 2024 16:44:12 +0530 Subject: [PATCH 18/31] chore: change signature for `CheckAssertExpressions()` Signed-off-by: Kumar Mallikarjuna --- pkg/test/step.go | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/pkg/test/step.go b/pkg/test/step.go index 3842abb6..fbfccac6 100644 --- a/pkg/test/step.go +++ b/pkg/test/step.go @@ -416,28 +416,23 @@ func (s *Step) CheckAssertCommands(ctx context.Context, namespace string, comman return testErrors } -func (s *Step) CheckAssertExpressions( - ctx context.Context, - resourceRefs []harness.TestResourceRef, - assertAny, - assertAll []*harness.Assertion, -) []error { +func (s *Step) CheckAssertExpressions() []error { client, err := s.Client(false) if err != nil { return []error{err} } variables := make(map[string]interface{}) - for _, resourceRef := range resourceRefs { + for _, resourceRef := range s.Assert.ResourceRefs { namespacedName, referencedResource := resourceRef.BuildResourceReference() - if err := client.Get(ctx, namespacedName, referencedResource); err != nil { + if err := client.Get(context.TODO(), namespacedName, referencedResource); err != nil { return []error{fmt.Errorf("failed to get referenced resource '%v': %w", namespacedName, err)} } variables[resourceRef.Ref] = referencedResource.Object } - return expressions.RunAssertExpressions(s.Programs, variables, assertAny, assertAll) + return expressions.RunAssertExpressions(s.Programs, variables, s.Assert.AssertAny, s.Assert.AssertAll) } // Check checks if the resources defined in Asserts and Errors are in the correct state. @@ -450,7 +445,7 @@ func (s *Step) Check(namespace string, timeout int) []error { if s.Assert != nil { testErrors = append(testErrors, s.CheckAssertCommands(context.TODO(), namespace, s.Assert.Commands, timeout)...) - testErrors = append(testErrors, s.CheckAssertExpressions(context.TODO(), s.Assert.ResourceRefs, s.Assert.AssertAny, s.Assert.AssertAll)...) + testErrors = append(testErrors, s.CheckAssertExpressions()...) } for _, expected := range s.Errors { From 5e252f5ceb3cd3881f06838c47f91d6b47df93f9 Mon Sep 17 00:00:00 2001 From: Kumar Mallikarjuna Date: Tue, 17 Dec 2024 17:05:09 +0530 Subject: [PATCH 19/31] chore: use envtest in `expression_integration_test.go` Signed-off-by: Kumar Mallikarjuna --- pkg/test/expression_integration_test.go | 44 ++++++++++++++++--------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/pkg/test/expression_integration_test.go b/pkg/test/expression_integration_test.go index 5c6e657e..4788e027 100644 --- a/pkg/test/expression_integration_test.go +++ b/pkg/test/expression_integration_test.go @@ -1,8 +1,13 @@ +//go:build integration + package test import ( + "context" "errors" "fmt" + "log" + "os" "strings" "testing" @@ -10,13 +15,25 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/discovery" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/client/fake" testutils "github.com/kudobuilder/kuttl/pkg/test/utils" ) +var testenv testutils.TestEnvironment + +func TestMain(m *testing.M) { + var err error + + testenv, err = testutils.StartTestEnvironment(false) + if err != nil { + log.Fatal(err) + } + + exitCode := m.Run() + testenv.Environment.Stop() + os.Exit(exitCode) +} + func buildTestStep(t *testing.T) *Step { codednsDeployment := &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ @@ -34,20 +51,15 @@ func buildTestStep(t *testing.T) *Step { }, } + assert.NoError(t, testenv.Client.Create(context.TODO(), codednsDeployment)) + assert.NoError(t, testenv.Client.Create(context.TODO(), metricServerPod)) + return &Step{ - Name: t.Name(), - Index: 0, - Logger: testutils.NewTestLogger(t, t.Name()), - Client: func(bool) (client.Client, error) { - return fake. - NewClientBuilder(). - WithObjects(codednsDeployment, metricServerPod). - WithScheme(testutils.Scheme()). - Build(), nil - }, - DiscoveryClient: func() (discovery.DiscoveryInterface, error) { - return testutils.FakeDiscoveryClient(), nil - }, + Name: t.Name(), + Index: 0, + Logger: testutils.NewTestLogger(t, t.Name()), + Client: testenv.Client, + DiscoveryClient: testenv.DiscoveryClient, } } From 56c1779d163f54668b1001dd991eba796cef9939 Mon Sep 17 00:00:00 2001 From: Kumar Mallikarjuna Date: Tue, 17 Dec 2024 17:33:28 +0530 Subject: [PATCH 20/31] fix: remove redundant definitions for integration test tools Signed-off-by: Kumar Mallikarjuna --- pkg/test/expression_integration_test.go | 33 +++++++++---------------- 1 file changed, 11 insertions(+), 22 deletions(-) diff --git a/pkg/test/expression_integration_test.go b/pkg/test/expression_integration_test.go index 4788e027..805e2320 100644 --- a/pkg/test/expression_integration_test.go +++ b/pkg/test/expression_integration_test.go @@ -6,8 +6,6 @@ import ( "context" "errors" "fmt" - "log" - "os" "strings" "testing" @@ -15,25 +13,12 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/discovery" + "sigs.k8s.io/controller-runtime/pkg/client" testutils "github.com/kudobuilder/kuttl/pkg/test/utils" ) -var testenv testutils.TestEnvironment - -func TestMain(m *testing.M) { - var err error - - testenv, err = testutils.StartTestEnvironment(false) - if err != nil { - log.Fatal(err) - } - - exitCode := m.Run() - testenv.Environment.Stop() - os.Exit(exitCode) -} - func buildTestStep(t *testing.T) *Step { codednsDeployment := &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ @@ -55,11 +40,15 @@ func buildTestStep(t *testing.T) *Step { assert.NoError(t, testenv.Client.Create(context.TODO(), metricServerPod)) return &Step{ - Name: t.Name(), - Index: 0, - Logger: testutils.NewTestLogger(t, t.Name()), - Client: testenv.Client, - DiscoveryClient: testenv.DiscoveryClient, + Name: t.Name(), + Index: 0, + Logger: testutils.NewTestLogger(t, t.Name()), + Client: func(bool) (client.Client, error) { + return testenv.Client, nil + }, + DiscoveryClient: func() (discovery.DiscoveryInterface, error) { + return testenv.DiscoveryClient, nil + }, } } From 2c58b635a8316b784d080739a48b3f06d2200bca Mon Sep 17 00:00:00 2001 From: Kumar Mallikarjuna Date: Wed, 18 Dec 2024 16:33:33 +0530 Subject: [PATCH 21/31] chore: use testenv in expression_integration_test.go Signed-off-by: Kumar Mallikarjuna --- pkg/test/expression_integration_test.go | 57 ++++++++++++++----- .../check_multiple_assert_all/00-assert.yaml | 2 +- .../00-assert.yaml | 2 +- .../check_multiple_assert_any/00-assert.yaml | 2 +- .../00-assert.yaml | 2 +- 5 files changed, 46 insertions(+), 19 deletions(-) diff --git a/pkg/test/expression_integration_test.go b/pkg/test/expression_integration_test.go index 805e2320..25c5098e 100644 --- a/pkg/test/expression_integration_test.go +++ b/pkg/test/expression_integration_test.go @@ -20,10 +20,43 @@ import ( ) func buildTestStep(t *testing.T) *Step { + return &Step{ + Name: t.Name(), + Index: 0, + Logger: testutils.NewTestLogger(t, t.Name()), + Client: func(bool) (client.Client, error) { + return testenv.Client, nil + }, + DiscoveryClient: func() (discovery.DiscoveryInterface, error) { + return testenv.DiscoveryClient, nil + }, + } +} + +func TestAssertExpressions(t *testing.T) { codednsDeployment := &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ Name: "coredns", Namespace: "kube-system", + Labels: map[string]string{"k8s-app": "kube-dns"}, + }, + Spec: appsv1.DeploymentSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"k8s-app": "kube-dns"}, + }, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"k8s-app": "kube-dns"}, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "coredns", + Image: "registry.k8s.io/coredns/coredns:v1.11.1", + }, + }, + }, + }, }, } metricServerPod := &corev1.Pod{ @@ -31,7 +64,15 @@ func buildTestStep(t *testing.T) *Step { Name: "metrics-server-xyz-pqr", Namespace: "kube-system", Labels: map[string]string{ - "app": "metrics-server", + "k8s-app": "metrics-server", + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "metrics-server", + Image: "registry.k8s.io/metrics-server/metrics-server:v0.7.2", + }, }, }, } @@ -39,20 +80,6 @@ func buildTestStep(t *testing.T) *Step { assert.NoError(t, testenv.Client.Create(context.TODO(), codednsDeployment)) assert.NoError(t, testenv.Client.Create(context.TODO(), metricServerPod)) - return &Step{ - Name: t.Name(), - Index: 0, - Logger: testutils.NewTestLogger(t, t.Name()), - Client: func(bool) (client.Client, error) { - return testenv.Client, nil - }, - DiscoveryClient: func() (discovery.DiscoveryInterface, error) { - return testenv.DiscoveryClient, nil - }, - } -} - -func TestAssertExpressions(t *testing.T) { testCases := []struct { name string loadingFailed bool diff --git a/pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_all/00-assert.yaml b/pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_all/00-assert.yaml index 64855289..c05990d4 100644 --- a/pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_all/00-assert.yaml +++ b/pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_all/00-assert.yaml @@ -13,5 +13,5 @@ resourceRefs: ref: metricsServer assertAll: - celExpr: "coredns.metadata.name == 'coredns'" - - celExpr: "metricsServer.metadata.labels['app'] == 'metrics-server'" + - celExpr: "metricsServer.metadata.labels['k8s-app'] == 'metrics-server'" timeout: 1 diff --git a/pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_all_with_one_failing/00-assert.yaml b/pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_all_with_one_failing/00-assert.yaml index 7f850071..0cd17ac4 100644 --- a/pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_all_with_one_failing/00-assert.yaml +++ b/pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_all_with_one_failing/00-assert.yaml @@ -13,5 +13,5 @@ resourceRefs: ref: metricsServer assertAll: - celExpr: "coredns.metadata.name == 'metrics-server'" - - celExpr: "metricsServer.metadata.labels['app'] == 'metrics-server'" + - celExpr: "metricsServer.metadata.labels['k8s-app'] == 'metrics-server'" timeout: 1 diff --git a/pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_any/00-assert.yaml b/pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_any/00-assert.yaml index 64659111..00e3f30e 100644 --- a/pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_any/00-assert.yaml +++ b/pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_any/00-assert.yaml @@ -13,5 +13,5 @@ resourceRefs: ref: metricsServer assertAny: - celExpr: "coredns.metadata.name == 'coredns'" - - celExpr: "metricsServer.metadata.labels['app'] == 'metrics-server-1.6'" + - celExpr: "metricsServer.metadata.labels['k8s-app'] == 'metrics-server-1.6'" timeout: 1 diff --git a/pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_any_with_all_failing/00-assert.yaml b/pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_any_with_all_failing/00-assert.yaml index 786b600f..8d559331 100644 --- a/pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_any_with_all_failing/00-assert.yaml +++ b/pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_any_with_all_failing/00-assert.yaml @@ -13,5 +13,5 @@ resourceRefs: ref: metricsServer assertAny: - celExpr: "coredns.metadata.name == 'metrics-server'" - - celExpr: "metricsServer.metadata.labels['app'] == 'metrics-server-1.6'" + - celExpr: "metricsServer.metadata.labels['k8s-app'] == 'metrics-server-1.6'" timeout: 1 From aa2fa55ce3093b6c0b1fac7c6bb230b11762e187 Mon Sep 17 00:00:00 2001 From: Kumar Mallikarjuna Date: Thu, 19 Dec 2024 12:30:24 +0530 Subject: [PATCH 22/31] fix: run target expressions test within a Kuttl owned ephemeral namespace Signed-off-by: Kumar Mallikarjuna --- pkg/test/expression_integration_test.go | 38 ++++++++++++++++--- pkg/test/step.go | 7 +++- .../00-create.yaml | 13 +++++++ .../01-assert.yaml | 10 +++++ 4 files changed, 60 insertions(+), 8 deletions(-) create mode 100644 pkg/test/step_integration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/00-create.yaml create mode 100644 pkg/test/step_integration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/01-assert.yaml diff --git a/pkg/test/expression_integration_test.go b/pkg/test/expression_integration_test.go index 25c5098e..0857a199 100644 --- a/pkg/test/expression_integration_test.go +++ b/pkg/test/expression_integration_test.go @@ -6,6 +6,7 @@ import ( "context" "errors" "fmt" + "os" "strings" "testing" @@ -115,19 +116,44 @@ func TestAssertExpressions(t *testing.T) { runFailed: true, errorMessage: "no expression evaluated to true", }, + { + name: "check expression for ephemeral namespace", + }, } + const testNamespace = "kuttl-ephemeral-xyz" + assert.NoError(t, testenv.Client.Create(context.TODO(), &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: testNamespace, + }, + TypeMeta: metav1.TypeMeta{ + Kind: "Namespace", + }, + })) + for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - step := buildTestStep(t) - - fName := fmt.Sprintf( - "step_integration_test_data/assert_expressions/%s/00-assert.yaml", + dirName := fmt.Sprintf( + "step_integration_test_data/assert_expressions/%s", strings.ReplaceAll(tc.name, " ", "_"), ) + files, err := os.ReadDir(dirName) + assert.NoError(t, err) + + for i := 0; i < len(files)-1; i++ { + fName := fmt.Sprintf("%s/%s", dirName, files[i].Name()) + step := buildTestStep(t) + assert.NoError(t, step.LoadYAML(fName)) + assert.NoError(t, errors.Join(errors.Join(step.Run(t, testNamespace)...))) + } + + step := buildTestStep(t) + + fName := fmt.Sprintf("%s/%s", dirName, files[len(files)-1].Name()) + // Load test that has an invalid expression - err := step.LoadYAML(fName) + err = step.LoadYAML(fName) if !tc.loadingFailed { assert.NoError(t, err) } else { @@ -135,7 +161,7 @@ func TestAssertExpressions(t *testing.T) { return } - err = errors.Join(step.Run(t, "")...) + err = errors.Join(step.Run(t, testNamespace)...) if !tc.runFailed { assert.NoError(t, err) } else { diff --git a/pkg/test/step.go b/pkg/test/step.go index fbfccac6..42a00305 100644 --- a/pkg/test/step.go +++ b/pkg/test/step.go @@ -416,7 +416,7 @@ func (s *Step) CheckAssertCommands(ctx context.Context, namespace string, comman return testErrors } -func (s *Step) CheckAssertExpressions() []error { +func (s *Step) CheckAssertExpressions(namespace string) []error { client, err := s.Client(false) if err != nil { return []error{err} @@ -424,6 +424,9 @@ func (s *Step) CheckAssertExpressions() []error { variables := make(map[string]interface{}) for _, resourceRef := range s.Assert.ResourceRefs { + if resourceRef.Namespace == "" { + resourceRef.Namespace = namespace + } namespacedName, referencedResource := resourceRef.BuildResourceReference() if err := client.Get(context.TODO(), namespacedName, referencedResource); err != nil { return []error{fmt.Errorf("failed to get referenced resource '%v': %w", namespacedName, err)} @@ -445,7 +448,7 @@ func (s *Step) Check(namespace string, timeout int) []error { if s.Assert != nil { testErrors = append(testErrors, s.CheckAssertCommands(context.TODO(), namespace, s.Assert.Commands, timeout)...) - testErrors = append(testErrors, s.CheckAssertExpressions()...) + testErrors = append(testErrors, s.CheckAssertExpressions(namespace)...) } for _, expected := range s.Errors { diff --git a/pkg/test/step_integration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/00-create.yaml b/pkg/test/step_integration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/00-create.yaml new file mode 100644 index 00000000..8f064b21 --- /dev/null +++ b/pkg/test/step_integration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/00-create.yaml @@ -0,0 +1,13 @@ +--- +apiVersion: v1 +kind: Pod +metadata: + name: nginx-pod + labels: + app: nginx +spec: + containers: + - name: nginx-container + image: nginx:latest + ports: + - containerPort: 80 diff --git a/pkg/test/step_integration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/01-assert.yaml b/pkg/test/step_integration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/01-assert.yaml new file mode 100644 index 00000000..fd6fb16a --- /dev/null +++ b/pkg/test/step_integration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/01-assert.yaml @@ -0,0 +1,10 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestAssert +resourceRefs: + - apiVersion: v1 + kind: Pod + name: nginx-pod + ref: nginxPod +assertAll: + - celExpr: "nginxPod.metadata.name == 'nginx-pod'" +timeout: 1 From 9e770600182465e19abb3b737307a4374de8885c Mon Sep 17 00:00:00 2001 From: Kumar Mallikarjuna Date: Thu, 19 Dec 2024 13:13:52 +0530 Subject: [PATCH 23/31] chore: remove redundant file and update go.mod Signed-off-by: Kumar Mallikarjuna --- pkg/test/utils/expressions.go | 88 ----------------------------------- 1 file changed, 88 deletions(-) delete mode 100644 pkg/test/utils/expressions.go diff --git a/pkg/test/utils/expressions.go b/pkg/test/utils/expressions.go deleted file mode 100644 index e100dad4..00000000 --- a/pkg/test/utils/expressions.go +++ /dev/null @@ -1,88 +0,0 @@ -package utils - -import ( - "context" - "fmt" - "os" - - "github.com/google/cel-go/cel" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/types" - - harness "github.com/kudobuilder/kuttl/pkg/apis/testharness/v1beta1" -) - -// RunAssertExpressions evaluates a set of CEL expressions specified as AnyAllExpressions -func RunAssertExpressions( - ctx context.Context, - logger Logger, - resourceRefs []harness.TestResourceRef, - expressions harness.AnyAllExpressions, - kubeconfigOverride string, -) []error { - errs := []error{} - - actualDir, err := os.Getwd() - if err != nil { - return []error{fmt.Errorf("failed to get current working director: %w", err)} - } - - kubeconfig := kubeconfigPath(actualDir, kubeconfigOverride) - cl, err := NewClient(kubeconfig, "")(false) - if err != nil { - return []error{fmt.Errorf("failed to construct client: %w", err)} - } - - variables := make(map[string]interface{}) - for _, resourceRef := range resourceRefs { - gvk := constructGVK(resourceRef.ApiVersion, resourceRef.Kind) - referencedResource := &unstructured.Unstructured{} - referencedResource.SetGroupVersionKind(gvk) - - if err := cl.Get( - ctx, - types.NamespacedName{Namespace: resourceRef.Namespace, Name: resourceRef.Name}, - referencedResource, - ); err != nil { - return []error{fmt.Errorf("failed to get referenced resource '%v': %w", gvk, err)} - } - - variables[resourceRef.Id] = referencedResource.Object - } - - env, err := cel.NewEnv() - if err != nil { - return []error{fmt.Errorf("failed to create environment: %w", err)} - } - - for k := range variables { - env, err = env.Extend(cel.Variable(k, cel.DynType)) - if err != nil { - return []error{fmt.Errorf("failed to add resource parameter '%v' to environment: %w", k, err)} - } - } - - for _, expr := range expressions.Any { - ast, issues := env.Compile(expr) - if issues != nil && issues.Err() != nil { - return []error{fmt.Errorf("type-check error: %s", issues.Err())} - } - - prg, err := env.Program(ast) - if err != nil { - return []error{fmt.Errorf("program constuction error: %w", err)} - } - - out, _, err := prg.Eval(variables) - if err != nil { - return []error{fmt.Errorf("failed to evaluate program: %w", err)} - } - - logger.Logf("expression '%v' evaluated to '%v'", expr, out.Value()) - if out.Value() != true { - errs = append(errs, fmt.Errorf("failed validation, expression '%v' evaluated to '%v'", expr, out.Value())) - } - } - - return errs -} From f824fb900d2cd4c0362f5b1323b5bcd61e6ece51 Mon Sep 17 00:00:00 2001 From: Kumar Mallikarjuna Date: Thu, 19 Dec 2024 22:31:42 +0530 Subject: [PATCH 24/31] chore: incorporate review comments Signed-off-by: Kumar Mallikarjuna --- pkg/expressions/cel.go | 25 ++++++----- pkg/test/expression_integration_test.go | 41 +++++++++---------- pkg/test/step.go | 2 +- .../{01-assert.yaml => 00-assert.yaml} | 0 .../00-create.yaml | 1 - 5 files changed, 33 insertions(+), 36 deletions(-) rename pkg/test/step_integration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/{01-assert.yaml => 00-assert.yaml} (100%) diff --git a/pkg/expressions/cel.go b/pkg/expressions/cel.go index 37ec9d48..0f8dc276 100644 --- a/pkg/expressions/cel.go +++ b/pkg/expressions/cel.go @@ -62,25 +62,25 @@ func RunAssertExpressions( return errs } - var anyExpressionsEvaluation, allExpressionsEvaluation []error + var anyExprErrors, allExprErrors []error for _, expr := range assertAny { if err := evaluateExpression(expr.CELExpression, programs, variables); err != nil { - anyExpressionsEvaluation = append(anyExpressionsEvaluation, err) + anyExprErrors = append(anyExprErrors, err) } } for _, expr := range assertAll { if err := evaluateExpression(expr.CELExpression, programs, variables); err != nil { - allExpressionsEvaluation = append(allExpressionsEvaluation, err) + allExprErrors = append(allExprErrors, err) } } - if len(assertAny) != 0 && len(anyExpressionsEvaluation) == len(assertAny) { - errs = append(errs, fmt.Errorf("no expression evaluated to true: %w", errors.Join(anyExpressionsEvaluation...))) + if len(assertAny) != 0 && len(anyExprErrors) == len(assertAny) { + errs = append(errs, fmt.Errorf("no expression evaluated to true: %w", errors.Join(anyExprErrors...))) } - if len(allExpressionsEvaluation) > 0 { - errs = append(errs, fmt.Errorf("not all expressions evaluated to true: %w", errors.Join(allExpressionsEvaluation...))) + if len(allExprErrors) > 0 { + errs = append(errs, fmt.Errorf("not all assertAll evaluated to true: %w", errors.Join(allExprErrors...))) } return errs @@ -94,20 +94,19 @@ func LoadPrograms(testAssert *harness.TestAssert) (map[string]cel.Program, error env, err := buildEnv(testAssert.ResourceRefs) if err != nil { - return nil, fmt.Errorf("failed to build environment: %w", err) + return nil, fmt.Errorf("failed to build CEL environment: %w", err) } - var programs map[string]cel.Program if len(assertions) == 0 { - return programs, nil + return nil, nil } - programs = make(map[string]cel.Program) + programs := make(map[string]cel.Program) for _, assertion := range assertions { if prg, err := buildProgram(assertion.CELExpression, env); err != nil { errs = append( errs, - fmt.Errorf("failed to parse CEL expression '%v': %w", assertion.CELExpression, err), + fmt.Errorf("failed to build CEL program from expression %q: %w", assertion.CELExpression, err), ) } else { programs[assertion.CELExpression] = prg @@ -115,7 +114,7 @@ func LoadPrograms(testAssert *harness.TestAssert) (map[string]cel.Program, error } if len(errs) > 0 { - return nil, fmt.Errorf("failed to parse expression(s): %w", errors.Join(errs...)) + return nil, fmt.Errorf("failed to load expression(s): %w", errors.Join(errs...)) } return programs, nil diff --git a/pkg/test/expression_integration_test.go b/pkg/test/expression_integration_test.go index 0857a199..02eb6cf0 100644 --- a/pkg/test/expression_integration_test.go +++ b/pkg/test/expression_integration_test.go @@ -82,39 +82,39 @@ func TestAssertExpressions(t *testing.T) { assert.NoError(t, testenv.Client.Create(context.TODO(), metricServerPod)) testCases := []struct { - name string - loadingFailed bool - runFailed bool - errorMessage string + name string + expectLoadFailure bool + expectRunFailure bool + expectedErrorMessage string }{ { - name: "invalid expression", - loadingFailed: true, - errorMessage: "undeclared reference", + name: "invalid expression", + expectLoadFailure: true, + expectedErrorMessage: "undeclared reference", }, { name: "check deployment name", }, { - name: "check incorrect deployment name", - runFailed: true, - errorMessage: "not all expressions evaluated to true", + name: "check incorrect deployment name", + expectRunFailure: true, + expectedErrorMessage: "not all expressions evaluated to true", }, { name: "check multiple assert all", }, { - name: "check multiple assert all with one failing", - runFailed: true, - errorMessage: "not all expressions evaluated to true", + name: "check multiple assert all with one failing", + expectRunFailure: true, + expectedErrorMessage: "not all expressions evaluated to true", }, { name: "check multiple assert any", }, { - name: "check multiple assert any with all failing", - runFailed: true, - errorMessage: "no expression evaluated to true", + name: "check multiple assert any with all failing", + expectRunFailure: true, + expectedErrorMessage: "no expression evaluated to true", }, { name: "check expression for ephemeral namespace", @@ -152,20 +152,19 @@ func TestAssertExpressions(t *testing.T) { fName := fmt.Sprintf("%s/%s", dirName, files[len(files)-1].Name()) - // Load test that has an invalid expression err = step.LoadYAML(fName) - if !tc.loadingFailed { + if !tc.expectLoadFailure { assert.NoError(t, err) } else { - assert.ErrorContains(t, err, tc.errorMessage) + assert.ErrorContains(t, err, tc.expectedErrorMessage) return } err = errors.Join(step.Run(t, testNamespace)...) - if !tc.runFailed { + if !tc.expectRunFailure { assert.NoError(t, err) } else { - assert.ErrorContains(t, err, tc.errorMessage) + assert.ErrorContains(t, err, tc.expectedErrorMessage) } }) } diff --git a/pkg/test/step.go b/pkg/test/step.go index 42a00305..52b072e5 100644 --- a/pkg/test/step.go +++ b/pkg/test/step.go @@ -563,7 +563,7 @@ func (s *Step) LoadYAML(file string) error { s.Programs, err = expressions.LoadPrograms(s.Assert) if err != nil { - return fmt.Errorf("failed to load programs: %w", err) + return fmt.Errorf("failed to prepare expression evaluation: %w", err) } } else { asserts = append(asserts, obj) diff --git a/pkg/test/step_integration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/01-assert.yaml b/pkg/test/step_integration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/00-assert.yaml similarity index 100% rename from pkg/test/step_integration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/01-assert.yaml rename to pkg/test/step_integration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/00-assert.yaml diff --git a/pkg/test/step_integration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/00-create.yaml b/pkg/test/step_integration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/00-create.yaml index 8f064b21..156afc8c 100644 --- a/pkg/test/step_integration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/00-create.yaml +++ b/pkg/test/step_integration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/00-create.yaml @@ -1,4 +1,3 @@ ---- apiVersion: v1 kind: Pod metadata: From 965b7eb73649c384a2cbab4460980125f457b526 Mon Sep 17 00:00:00 2001 From: Kumar Mallikarjuna Date: Thu, 19 Dec 2024 22:37:37 +0530 Subject: [PATCH 25/31] chore: update error messages Signed-off-by: Kumar Mallikarjuna --- pkg/test/expression_integration_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/test/expression_integration_test.go b/pkg/test/expression_integration_test.go index 02eb6cf0..51322946 100644 --- a/pkg/test/expression_integration_test.go +++ b/pkg/test/expression_integration_test.go @@ -98,7 +98,7 @@ func TestAssertExpressions(t *testing.T) { { name: "check incorrect deployment name", expectRunFailure: true, - expectedErrorMessage: "not all expressions evaluated to true", + expectedErrorMessage: "not all assertAll expressions evaluated to true", }, { name: "check multiple assert all", @@ -106,7 +106,7 @@ func TestAssertExpressions(t *testing.T) { { name: "check multiple assert all with one failing", expectRunFailure: true, - expectedErrorMessage: "not all expressions evaluated to true", + expectedErrorMessage: "not all assertAll expressions evaluated to true", }, { name: "check multiple assert any", From 5173e6c93e39025a397dcda57c0ccc7cb1ec8f26 Mon Sep 17 00:00:00 2001 From: Kumar Mallikarjuna Date: Thu, 19 Dec 2024 23:05:37 +0530 Subject: [PATCH 26/31] chore: update error message Signed-off-by: Kumar Mallikarjuna --- pkg/expressions/cel.go | 2 +- .../{00-assert.yaml => 01-assert.yaml} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename pkg/test/step_integration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/{00-assert.yaml => 01-assert.yaml} (100%) diff --git a/pkg/expressions/cel.go b/pkg/expressions/cel.go index 0f8dc276..0f675ad2 100644 --- a/pkg/expressions/cel.go +++ b/pkg/expressions/cel.go @@ -80,7 +80,7 @@ func RunAssertExpressions( } if len(allExprErrors) > 0 { - errs = append(errs, fmt.Errorf("not all assertAll evaluated to true: %w", errors.Join(allExprErrors...))) + errs = append(errs, fmt.Errorf("not all assertAll expressions evaluated to true: %w", errors.Join(allExprErrors...))) } return errs diff --git a/pkg/test/step_integration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/00-assert.yaml b/pkg/test/step_integration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/01-assert.yaml similarity index 100% rename from pkg/test/step_integration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/00-assert.yaml rename to pkg/test/step_integration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/01-assert.yaml From 4f358b0eb1d4f2dabddc757a0492918b06f926b5 Mon Sep 17 00:00:00 2001 From: Kumar Mallikarjuna Date: Fri, 3 Jan 2025 15:38:21 +0530 Subject: [PATCH 27/31] chore: incorporate review comments Signed-off-by: Kumar Mallikarjuna --- pkg/expressions/cel.go | 6 +++--- pkg/test/expression_integration_test.go | 18 ++++++++++++------ 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/pkg/expressions/cel.go b/pkg/expressions/cel.go index 0f675ad2..0f0976b5 100644 --- a/pkg/expressions/cel.go +++ b/pkg/expressions/cel.go @@ -50,7 +50,7 @@ func buildEnv(resourceRefs []harness.TestResourceRef) (*cel.Env, error) { return env, nil } -// RunAssertExpressions evaluates a set of CEL expressions specified as AnyAllExpressions +// RunAssertExpressions evaluates a set of CEL expressions. func RunAssertExpressions( programs map[string]cel.Program, variables map[string]interface{}, @@ -126,7 +126,7 @@ func evaluateExpression(expr string, ) error { prg, ok := programs[expr] if !ok { - return fmt.Errorf("couldn't find pre-built parsed CEL expression '%v'", expr) + return fmt.Errorf("couldn't find pre-built parsed CEL expression %q", expr) } out, _, err := prg.Eval(variables) if err != nil { @@ -134,7 +134,7 @@ func evaluateExpression(expr string, } if out.Value() != true { - return fmt.Errorf("expression '%v' evaluated to '%v'", expr, out.Value()) + return fmt.Errorf("expression %q evaluated to %q", expr, out.Value()) } return nil diff --git a/pkg/test/expression_integration_test.go b/pkg/test/expression_integration_test.go index 51322946..c7f0694e 100644 --- a/pkg/test/expression_integration_test.go +++ b/pkg/test/expression_integration_test.go @@ -9,6 +9,7 @@ import ( "os" "strings" "testing" + "time" "github.com/stretchr/testify/assert" appsv1 "k8s.io/api/apps/v1" @@ -17,10 +18,11 @@ import ( "k8s.io/client-go/discovery" "sigs.k8s.io/controller-runtime/pkg/client" + "github.com/kudobuilder/kuttl/pkg/kubernetes" testutils "github.com/kudobuilder/kuttl/pkg/test/utils" ) -func buildTestStep(t *testing.T) *Step { +func buildTestStep(t *testing.T, testenv kubernetes.TestEnvironment) *Step { return &Step{ Name: t.Name(), Index: 0, @@ -35,6 +37,10 @@ func buildTestStep(t *testing.T) *Step { } func TestAssertExpressions(t *testing.T) { + ctx := context.WithTimeout(context.Background(), 2*time.Minute) + testenv, err := kubernetes.StartTestEnvironment(false) + assert.NoError(t, err) + codednsDeployment := &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ Name: "coredns", @@ -78,8 +84,8 @@ func TestAssertExpressions(t *testing.T) { }, } - assert.NoError(t, testenv.Client.Create(context.TODO(), codednsDeployment)) - assert.NoError(t, testenv.Client.Create(context.TODO(), metricServerPod)) + assert.NoError(t, testenv.Client.Create(ctx, codednsDeployment)) + assert.NoError(t, testenv.Client.Create(ctx, metricServerPod)) testCases := []struct { name string @@ -122,7 +128,7 @@ func TestAssertExpressions(t *testing.T) { } const testNamespace = "kuttl-ephemeral-xyz" - assert.NoError(t, testenv.Client.Create(context.TODO(), &corev1.Namespace{ + assert.NoError(t, testenv.Client.Create(ctx, &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ Name: testNamespace, }, @@ -143,12 +149,12 @@ func TestAssertExpressions(t *testing.T) { for i := 0; i < len(files)-1; i++ { fName := fmt.Sprintf("%s/%s", dirName, files[i].Name()) - step := buildTestStep(t) + step := buildTestStep(t, testenv) assert.NoError(t, step.LoadYAML(fName)) assert.NoError(t, errors.Join(errors.Join(step.Run(t, testNamespace)...))) } - step := buildTestStep(t) + step := buildTestStep(t, testenv) fName := fmt.Sprintf("%s/%s", dirName, files[len(files)-1].Name()) From 60b818d1c981a37a73fb34336c8bb49523a11c63 Mon Sep 17 00:00:00 2001 From: Kumar Mallikarjuna Date: Fri, 3 Jan 2025 15:57:21 +0530 Subject: [PATCH 28/31] fix: update context creation Signed-off-by: Kumar Mallikarjuna --- pkg/test/expression_integration_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/test/expression_integration_test.go b/pkg/test/expression_integration_test.go index c7f0694e..95b0d8c1 100644 --- a/pkg/test/expression_integration_test.go +++ b/pkg/test/expression_integration_test.go @@ -37,7 +37,9 @@ func buildTestStep(t *testing.T, testenv kubernetes.TestEnvironment) *Step { } func TestAssertExpressions(t *testing.T) { - ctx := context.WithTimeout(context.Background(), 2*time.Minute) + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) + defer cancel() + testenv, err := kubernetes.StartTestEnvironment(false) assert.NoError(t, err) From dd85a98401fad0d125c01c093db2d03129f315b5 Mon Sep 17 00:00:00 2001 From: Kumar Mallikarjuna Date: Tue, 7 Jan 2025 18:02:57 +0530 Subject: [PATCH 29/31] chore: aggregate test steps into one Signed-off-by: Kumar Mallikarjuna --- pkg/test/expression_integration_test.go | 20 ++++++++----------- pkg/test/step.go | 7 +++++++ .../{01-assert.yaml => 00-assert.yaml} | 0 3 files changed, 15 insertions(+), 12 deletions(-) rename pkg/test/step_integration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/{01-assert.yaml => 00-assert.yaml} (100%) diff --git a/pkg/test/expression_integration_test.go b/pkg/test/expression_integration_test.go index 95b0d8c1..1caca12d 100644 --- a/pkg/test/expression_integration_test.go +++ b/pkg/test/expression_integration_test.go @@ -149,26 +149,22 @@ func TestAssertExpressions(t *testing.T) { files, err := os.ReadDir(dirName) assert.NoError(t, err) - for i := 0; i < len(files)-1; i++ { - fName := fmt.Sprintf("%s/%s", dirName, files[i].Name()) - step := buildTestStep(t, testenv) - assert.NoError(t, step.LoadYAML(fName)) - assert.NoError(t, errors.Join(errors.Join(step.Run(t, testNamespace)...))) - } - step := buildTestStep(t, testenv) + for _, file := range files { + fName := fmt.Sprintf("%s/%s", dirName, file.Name()) + if err = step.LoadYAML(fName); err != nil { + break + } + } - fName := fmt.Sprintf("%s/%s", dirName, files[len(files)-1].Name()) - - err = step.LoadYAML(fName) if !tc.expectLoadFailure { assert.NoError(t, err) - } else { + } else if tc.expectLoadFailure { assert.ErrorContains(t, err, tc.expectedErrorMessage) return } - err = errors.Join(step.Run(t, testNamespace)...) + err = errors.Join(errors.Join(step.Run(t, testNamespace)...)) if !tc.expectRunFailure { assert.NoError(t, err) } else { diff --git a/pkg/test/step.go b/pkg/test/step.go index 52b072e5..7fb75c78 100644 --- a/pkg/test/step.go +++ b/pkg/test/step.go @@ -194,6 +194,7 @@ func (s *Step) Create(test *testing.T, namespace string) []error { errors := []error{} + fmt.Println("HEREA1", len(s.Apply)) for _, obj := range s.Apply { _, _, err := kubernetes.Namespaced(dClient, obj, namespace) if err != nil { @@ -225,7 +226,10 @@ func (s *Step) Create(test *testing.T, namespace string) []error { } s.Logger.Log(kubernetes.ResourceID(obj), action) } + + fmt.Println("HEREA", obj.GetNamespace(), obj.GetName()) } + fmt.Println("HEREB", errors) return errors } @@ -429,6 +433,7 @@ func (s *Step) CheckAssertExpressions(namespace string) []error { } namespacedName, referencedResource := resourceRef.BuildResourceReference() if err := client.Get(context.TODO(), namespacedName, referencedResource); err != nil { + fmt.Println("HEREB", err) return []error{fmt.Errorf("failed to get referenced resource '%v': %w", namespacedName, err)} } @@ -679,6 +684,8 @@ func (s *Step) populateObjectsByFileName(fileName string, objects []client.Objec return fmt.Errorf("%s does not match file name regexp: %s", fileName, testStepRegex.String()) } + fmt.Println("HEREC", strings.ToLower(matches[1])) + switch fname := strings.ToLower(matches[1]); fname { case "assert": s.Asserts = append(s.Asserts, objects...) diff --git a/pkg/test/step_integration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/01-assert.yaml b/pkg/test/step_integration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/00-assert.yaml similarity index 100% rename from pkg/test/step_integration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/01-assert.yaml rename to pkg/test/step_integration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/00-assert.yaml From d5106564325142c9fd9a9d6174605992b1608f00 Mon Sep 17 00:00:00 2001 From: Kumar Mallikarjuna Date: Wed, 8 Jan 2025 22:21:58 +0530 Subject: [PATCH 30/31] chore: remove debug statements Signed-off-by: Kumar Mallikarjuna --- pkg/test/step.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/pkg/test/step.go b/pkg/test/step.go index 7fb75c78..52b072e5 100644 --- a/pkg/test/step.go +++ b/pkg/test/step.go @@ -194,7 +194,6 @@ func (s *Step) Create(test *testing.T, namespace string) []error { errors := []error{} - fmt.Println("HEREA1", len(s.Apply)) for _, obj := range s.Apply { _, _, err := kubernetes.Namespaced(dClient, obj, namespace) if err != nil { @@ -226,10 +225,7 @@ func (s *Step) Create(test *testing.T, namespace string) []error { } s.Logger.Log(kubernetes.ResourceID(obj), action) } - - fmt.Println("HEREA", obj.GetNamespace(), obj.GetName()) } - fmt.Println("HEREB", errors) return errors } @@ -433,7 +429,6 @@ func (s *Step) CheckAssertExpressions(namespace string) []error { } namespacedName, referencedResource := resourceRef.BuildResourceReference() if err := client.Get(context.TODO(), namespacedName, referencedResource); err != nil { - fmt.Println("HEREB", err) return []error{fmt.Errorf("failed to get referenced resource '%v': %w", namespacedName, err)} } @@ -684,8 +679,6 @@ func (s *Step) populateObjectsByFileName(fileName string, objects []client.Objec return fmt.Errorf("%s does not match file name regexp: %s", fileName, testStepRegex.String()) } - fmt.Println("HEREC", strings.ToLower(matches[1])) - switch fname := strings.ToLower(matches[1]); fname { case "assert": s.Asserts = append(s.Asserts, objects...) From 7c164ba4fa2d6d92eec95e7b526c9a818268dac9 Mon Sep 17 00:00:00 2001 From: Kumar Mallikarjuna Date: Wed, 8 Jan 2025 22:29:47 +0530 Subject: [PATCH 31/31] chore(tests): remove prefixes for expression integration test steps Signed-off-by: Kumar Mallikarjuna --- .../check_deployment_name/{00-assert.yaml => assert.yaml} | 0 .../{00-assert.yaml => assert.yaml} | 0 .../{00-create.yaml => pod.yaml} | 0 .../{00-assert.yaml => assert.yaml} | 0 .../check_multiple_assert_all/{00-assert.yaml => assert.yaml} | 0 .../{00-assert.yaml => assert.yaml} | 0 .../check_multiple_assert_any/{00-assert.yaml => assert.yaml} | 0 .../{00-assert.yaml => assert.yaml} | 0 .../invalid_expression/{00-assert.yaml => assert.yaml} | 0 9 files changed, 0 insertions(+), 0 deletions(-) rename pkg/test/step_integration_test_data/assert_expressions/check_deployment_name/{00-assert.yaml => assert.yaml} (100%) rename pkg/test/step_integration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/{00-assert.yaml => assert.yaml} (100%) rename pkg/test/step_integration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/{00-create.yaml => pod.yaml} (100%) rename pkg/test/step_integration_test_data/assert_expressions/check_incorrect_deployment_name/{00-assert.yaml => assert.yaml} (100%) rename pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_all/{00-assert.yaml => assert.yaml} (100%) rename pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_all_with_one_failing/{00-assert.yaml => assert.yaml} (100%) rename pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_any/{00-assert.yaml => assert.yaml} (100%) rename pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_any_with_all_failing/{00-assert.yaml => assert.yaml} (100%) rename pkg/test/step_integration_test_data/assert_expressions/invalid_expression/{00-assert.yaml => assert.yaml} (100%) diff --git a/pkg/test/step_integration_test_data/assert_expressions/check_deployment_name/00-assert.yaml b/pkg/test/step_integration_test_data/assert_expressions/check_deployment_name/assert.yaml similarity index 100% rename from pkg/test/step_integration_test_data/assert_expressions/check_deployment_name/00-assert.yaml rename to pkg/test/step_integration_test_data/assert_expressions/check_deployment_name/assert.yaml diff --git a/pkg/test/step_integration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/00-assert.yaml b/pkg/test/step_integration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/assert.yaml similarity index 100% rename from pkg/test/step_integration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/00-assert.yaml rename to pkg/test/step_integration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/assert.yaml diff --git a/pkg/test/step_integration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/00-create.yaml b/pkg/test/step_integration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/pod.yaml similarity index 100% rename from pkg/test/step_integration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/00-create.yaml rename to pkg/test/step_integration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/pod.yaml diff --git a/pkg/test/step_integration_test_data/assert_expressions/check_incorrect_deployment_name/00-assert.yaml b/pkg/test/step_integration_test_data/assert_expressions/check_incorrect_deployment_name/assert.yaml similarity index 100% rename from pkg/test/step_integration_test_data/assert_expressions/check_incorrect_deployment_name/00-assert.yaml rename to pkg/test/step_integration_test_data/assert_expressions/check_incorrect_deployment_name/assert.yaml diff --git a/pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_all/00-assert.yaml b/pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_all/assert.yaml similarity index 100% rename from pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_all/00-assert.yaml rename to pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_all/assert.yaml diff --git a/pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_all_with_one_failing/00-assert.yaml b/pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_all_with_one_failing/assert.yaml similarity index 100% rename from pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_all_with_one_failing/00-assert.yaml rename to pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_all_with_one_failing/assert.yaml diff --git a/pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_any/00-assert.yaml b/pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_any/assert.yaml similarity index 100% rename from pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_any/00-assert.yaml rename to pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_any/assert.yaml diff --git a/pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_any_with_all_failing/00-assert.yaml b/pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_any_with_all_failing/assert.yaml similarity index 100% rename from pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_any_with_all_failing/00-assert.yaml rename to pkg/test/step_integration_test_data/assert_expressions/check_multiple_assert_any_with_all_failing/assert.yaml diff --git a/pkg/test/step_integration_test_data/assert_expressions/invalid_expression/00-assert.yaml b/pkg/test/step_integration_test_data/assert_expressions/invalid_expression/assert.yaml similarity index 100% rename from pkg/test/step_integration_test_data/assert_expressions/invalid_expression/00-assert.yaml rename to pkg/test/step_integration_test_data/assert_expressions/invalid_expression/assert.yaml