From 6e2b71c63a790b4a47fbfe53d95e2047420cb1b7 Mon Sep 17 00:00:00 2001 From: Jay Pipes Date: Thu, 10 Aug 2023 13:01:15 -0400 Subject: [PATCH 1/3] rework parsing of the KubeSpec in prep for Action reworks the parsing of the KubeSpec struct and identifying parse errors earlier (along with a line/column for the failure of `With` struct parsing). Signed-off-by: Jay Pipes --- errors.go | 7 +- parse.go | 173 +++++++++++++----- .../parse/fail/more-than-one-kube-action.yaml | 4 +- .../parse/fail/more-than-one-shortcut.yaml | 4 +- .../parse/fail/shortcut-and-long-kube.yaml | 4 +- 5 files changed, 134 insertions(+), 58 deletions(-) diff --git a/errors.go b/errors.go index 8c1ee6b..de055bb 100644 --- a/errors.go +++ b/errors.go @@ -158,8 +158,11 @@ func InvalidResourceSpecifierOrFilepath(subject string) error { // InvalidWithLabels returns ErrWithLabels with an error containing more // context. -func InvalidWithLabels(err error) error { - return fmt.Errorf("%w: %s", ErrWithLabelsInvalid, err) +func InvalidWithLabels(err error, node *yaml.Node) error { + return fmt.Errorf( + "%w: %s at line %d, column %d", + ErrWithLabelsInvalid, err, node.Line, node.Column, + ) } // ResourceUnknown returns ErrRuntimeResourceUnknown for a given kind diff --git a/parse.go b/parse.go index eee74a6..de8c18b 100644 --- a/parse.go +++ b/parse.go @@ -43,12 +43,20 @@ func (s *Spec) UnmarshalYAML(node *yaml.Node) error { if valNode.Kind != yaml.ScalarNode { return errors.ExpectedScalarAt(valNode) } - s.KubeGet = valNode.Value + v := valNode.Value + if err := validateResourceIdentifier(v); err != nil { + return err + } + s.KubeGet = v case "kube.create": if valNode.Kind != yaml.ScalarNode { return errors.ExpectedScalarAt(valNode) } - s.KubeCreate = valNode.Value + v := valNode.Value + if err := validateFileExists(v); err != nil { + return err + } + s.KubeCreate = v case "kube.apply": if valNode.Kind != yaml.ScalarNode { return errors.ExpectedScalarAt(valNode) @@ -58,7 +66,14 @@ func (s *Spec) UnmarshalYAML(node *yaml.Node) error { if valNode.Kind != yaml.ScalarNode { return errors.ExpectedScalarAt(valNode) } - s.KubeDelete = valNode.Value + v := valNode.Value + if err := validateResourceIdentifierOrFilepath(v); err != nil { + return err + } + if err := validateFileExists(v); err != nil { + return err + } + s.KubeDelete = v case "assert": if valNode.Kind != yaml.MappingNode { return errors.ExpectedMapAt(valNode) @@ -79,8 +94,111 @@ func (s *Spec) UnmarshalYAML(node *yaml.Node) error { return err } expandShortcut(s) - if err := validateKubeSpec(s); err != nil { - return err + if moreThanOneAction(s) { + return ErrMoreThanOneKubeAction + } + with := s.Kube.With + if with != nil { + if s.Kube.Get == "" && s.Kube.Delete == "" { + return ErrWithLabelsOnlyGetDelete + } + } + return nil +} + +func (s *KubeSpec) UnmarshalYAML(node *yaml.Node) error { + if node.Kind != yaml.MappingNode { + return errors.ExpectedMapAt(node) + } + // maps/structs are stored in a top-level Node.Content field which is a + // concatenated slice of Node pointers in pairs of key/values. + for i := 0; i < len(node.Content); i += 2 { + keyNode := node.Content[i] + if keyNode.Kind != yaml.ScalarNode { + return errors.ExpectedScalarAt(keyNode) + } + key := keyNode.Value + valNode := node.Content[i+1] + switch key { + case "config": + if valNode.Kind != yaml.ScalarNode { + return errors.ExpectedScalarAt(valNode) + } + fp := valNode.Value + if err := validateFileExists(fp); err != nil { + return err + } + s.Config = fp + case "context": + if valNode.Kind != yaml.ScalarNode { + return errors.ExpectedScalarAt(valNode) + } + // NOTE(jaypipes): We can't validate the kubectx exists yet because + // fixtures may advertise a kube config and we look up the context + // in s.Config() method + s.Context = valNode.Value + case "namespace": + if valNode.Kind != yaml.ScalarNode { + return errors.ExpectedScalarAt(valNode) + } + s.Namespace = valNode.Value + case "apply": + if valNode.Kind != yaml.ScalarNode { + return errors.ExpectedScalarAt(valNode) + } + v := valNode.Value + if err := validateFileExists(v); err != nil { + return err + } + s.Apply = v + case "create": + if valNode.Kind != yaml.ScalarNode { + return errors.ExpectedScalarAt(valNode) + } + v := valNode.Value + if err := validateFileExists(v); err != nil { + return err + } + s.Create = v + case "get": + if valNode.Kind != yaml.ScalarNode { + return errors.ExpectedScalarAt(valNode) + } + v := valNode.Value + if err := validateResourceIdentifier(v); err != nil { + return err + } + s.Get = v + case "delete": + if valNode.Kind != yaml.ScalarNode { + return errors.ExpectedScalarAt(valNode) + } + v := valNode.Value + if err := validateResourceIdentifierOrFilepath(v); err != nil { + return err + } + if err := validateFileExists(v); err != nil { + return err + } + s.Delete = v + case "with": + if valNode.Kind != yaml.MappingNode { + return errors.ExpectedMapAt(valNode) + } + var w *With + if err := valNode.Decode(&w); err != nil { + return err + } + if w.Labels != nil { + _, err := labels.ValidatedSelectorFromSet(w.Labels) + if err != nil { + return InvalidWithLabels(err, valNode) + } + } + s.With = w + default: + return errors.UnknownFieldAt(key, keyNode) + } } return nil } @@ -260,51 +378,6 @@ func moreThanOneAction(s *Spec) bool { return foundActions > 1 } -// validateKubeSpec ensures that the test author has specified only a single -// action in the KubeSpec and that various KubeSpec fields are set -// appropriately. -func validateKubeSpec(s *Spec) error { - if moreThanOneAction(s) { - return ErrMoreThanOneKubeAction - } - if s.Kube.Get != "" { - if err := validateResourceIdentifier(s.Kube.Get); err != nil { - return err - } - } - if s.Kube.Delete != "" { - if err := validateResourceIdentifierOrFilepath(s.Kube.Delete); err != nil { - return err - } - if err := validateFileExists(s.Kube.Delete); err != nil { - return err - } - } - if s.Kube.Create != "" { - if err := validateFileExists(s.Kube.Create); err != nil { - return err - } - } - if s.Kube.Apply != "" { - if err := validateFileExists(s.Kube.Apply); err != nil { - return err - } - } - with := s.Kube.With - if with != nil { - if s.Kube.Get == "" && s.Kube.Delete == "" { - return ErrWithLabelsOnlyGetDelete - } - if with.Labels != nil { - _, err := labels.ValidatedSelectorFromSet(with.Labels) - if err != nil { - return InvalidWithLabels(err) - } - } - } - return nil -} - // validateFileExists returns an error if the supplied path looks like a file // path but the file does not exist. func validateFileExists(path string) error { diff --git a/testdata/parse/fail/more-than-one-kube-action.yaml b/testdata/parse/fail/more-than-one-kube-action.yaml index a0f75ab..2062821 100644 --- a/testdata/parse/fail/more-than-one-kube-action.yaml +++ b/testdata/parse/fail/more-than-one-kube-action.yaml @@ -2,5 +2,5 @@ name: more-than-one-kube-action description: invalid kube spec with more than one Kubernetes action in kube field tests: - kube: - create: testdata/pod.yaml - apply: testdata/pod.yaml + create: testdata/manifests/nginx-pod.yaml + apply: testdata/manifests/nginx-pod.yaml diff --git a/testdata/parse/fail/more-than-one-shortcut.yaml b/testdata/parse/fail/more-than-one-shortcut.yaml index 7c983df..fb2527d 100644 --- a/testdata/parse/fail/more-than-one-shortcut.yaml +++ b/testdata/parse/fail/more-than-one-shortcut.yaml @@ -1,5 +1,5 @@ name: more-than-one-shortcut description: invalid kube spec with more than one shortcut field tests: - - kube.create: testdata/pod.yaml - kube.apply: testdata/pod.yaml + - kube.create: testdata/manifests/nginx-pod.yaml + kube.apply: testdata/manifests/nginx-pod.yaml diff --git a/testdata/parse/fail/shortcut-and-long-kube.yaml b/testdata/parse/fail/shortcut-and-long-kube.yaml index e8e42f9..ae09565 100644 --- a/testdata/parse/fail/shortcut-and-long-kube.yaml +++ b/testdata/parse/fail/shortcut-and-long-kube.yaml @@ -1,7 +1,7 @@ name: shortcut-and-long-kube description: invalid kube spec with both shortcut and long-form kube tests: - - kube.create: testdata/pod.yaml + - kube.create: testdata/manifests/nginx-pod.yaml # The kube object is redundant when there is a kube.create shortcut kube: - create: testdata/pod.yaml + create: testdata/manifests/nginx-pod.yaml From 54b193e2e603e54f411acb52696f51f04b91e69f Mon Sep 17 00:00:00 2001 From: Jay Pipes Date: Fri, 11 Aug 2023 15:55:35 -0400 Subject: [PATCH 2/3] separate Action into own file and parsing Introducing the `on.fail` field for the kube gdt plugin involves parsing a kubernetes action (get, create, apply, etc) separate from the main `kube` field in the KubeSpec. So, in this patch we separate out the Action and then embed the Action struct inside the KubeSpec, separating out the parsing of the Action struct. Signed-off-by: Jay Pipes --- action.go | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++ go.mod | 2 +- go.sum | 4 ++-- parse.go | 53 ++++++++++++++++++++++++++++++++---------- parse_test.go | 28 ++++++++++++++++------ spec.go | 59 +++-------------------------------------------- 6 files changed, 132 insertions(+), 78 deletions(-) create mode 100644 action.go diff --git a/action.go b/action.go new file mode 100644 index 0000000..b52ef11 --- /dev/null +++ b/action.go @@ -0,0 +1,64 @@ +// Use and distribution licensed under the Apache license version 2. +// +// See the COPYING file in the root project directory for full text. + +package kube + +// With houses one or more selectors that the Get and Delete fields may use to +// select the resources to operate against. +type With struct { + // Labels is a map, keyed by metadata Label, of Label values to select a + // resource by + Labels map[string]string `yaml:"labels,omitempty"` +} + +// Action describes the the Kubernetes-specific action that is performed by the +// test. +type Action struct { + // Create is a string containing a file path or raw YAML content describing + // a Kubernetes resource to call `kubectl create` with. + Create string `yaml:"create,omitempty"` + // Apply is a string containing a file path or raw YAML content describing + // a Kubernetes resource to call `kubectl apply` with. + Apply string `yaml:"apply,omitempty"` + // Delete is a string containing an argument to `kubectl delete` and must + // be one of the following: + // + // - a file path to a manifest that will be read and the resources + // described in the manifest will be deleted + // - a resource kind or kind alias, e.g. "pods", "po", followed by one of + // the following: + // * a space or `/` character followed by the resource name to delete + // only a resource with that name. + // * a space followed by `-l ` followed by a label to delete resources + // having such a label. + // * the string `--all` to delete all resources of that kind. + Delete string `yaml:"delete,omitempty"` + // Get is a string containing an argument to `kubectl get` and must be one + // of the following: + // + // - a file path to a manifest that will be read and the resources within + // retrieved via `kubectl get` + // - a resource kind or kind alias, e.g. "pods", "po", followed by one of + // the following: + // * a space or `/` character followed by the resource name to get only a + // resource with that name. + // * a space followed by `-l ` followed by a label to get resources + // having such a label. + Get string `yaml:"get,omitempty"` + // With houses one or more selectors that the Get and Delete fields may use + // to select the resources to operate against. + // + // Use in conjunction with Get and Delete to filter resources: + // + // ```yaml + // tests: + // - name: delete pods with app:nginx label + // kube: + // delete: pods + // with: + // labels: + // app: nginx + // ``` + With *With `yaml:"with,omitempty"` +} diff --git a/go.mod b/go.mod index bc1fbe9..4e8c485 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.19 require ( github.com/cenkalti/backoff/v4 v4.2.1 - github.com/gdt-dev/gdt v1.1.1 + github.com/gdt-dev/gdt v1.2.0 github.com/samber/lo v1.38.1 github.com/stretchr/testify v1.8.4 gopkg.in/yaml.v3 v3.0.1 diff --git a/go.sum b/go.sum index 224bdcc..d7681ae 100644 --- a/go.sum +++ b/go.sum @@ -65,8 +65,8 @@ github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7 github.com/evanphx/json-patch v4.12.0+incompatible h1:4onqiflcdA9EOZ4RxV643DvftH5pOlLGNtQ5lPWQu84= github.com/evanphx/json-patch/v5 v5.6.0 h1:b91NhWfaz02IuVxO9faSllyAtNXHMPkC5J8sJCLunww= github.com/evanphx/json-patch/v5 v5.6.0/go.mod h1:G79N1coSVB93tBe7j6PhzjmR3/2VvlbKOFpnXhI9Bw4= -github.com/gdt-dev/gdt v1.1.1 h1:863WjQr2Oa+1eVKJspw1SRqW72S0Y3UydN0j8WwPYJU= -github.com/gdt-dev/gdt v1.1.1/go.mod h1:StnyGjC/67u59La2u6fh3HwW9MmodVhKdXcLlkgvNSY= +github.com/gdt-dev/gdt v1.2.0 h1:wXHeXjbjHBLkOCDU0/Wmy8zC0/u9468JDKRhYqxPI5s= +github.com/gdt-dev/gdt v1.2.0/go.mod h1:StnyGjC/67u59La2u6fh3HwW9MmodVhKdXcLlkgvNSY= github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU= github.com/go-gl/glfw/v3.3/glfw v0.0.0-20191125211704-12ad95a8df72/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= diff --git a/parse.go b/parse.go index de8c18b..03ae03b 100644 --- a/parse.go +++ b/parse.go @@ -142,6 +142,35 @@ func (s *KubeSpec) UnmarshalYAML(node *yaml.Node) error { return errors.ExpectedScalarAt(valNode) } s.Namespace = valNode.Value + case "get", "create", "apply", "delete", "with": + // Because Action is an embedded struct and we parse it below, just + // ignore these fields in the top-level `kube:` field for now. + default: + return errors.UnknownFieldAt(key, keyNode) + } + } + var a Action + if err := node.Decode(&a); err != nil { + return err + } + s.Action = a + return nil +} + +func (a *Action) UnmarshalYAML(node *yaml.Node) error { + if node.Kind != yaml.MappingNode { + return errors.ExpectedMapAt(node) + } + // maps/structs are stored in a top-level Node.Content field which is a + // concatenated slice of Node pointers in pairs of key/values. + for i := 0; i < len(node.Content); i += 2 { + keyNode := node.Content[i] + if keyNode.Kind != yaml.ScalarNode { + return errors.ExpectedScalarAt(keyNode) + } + key := keyNode.Value + valNode := node.Content[i+1] + switch key { case "apply": if valNode.Kind != yaml.ScalarNode { return errors.ExpectedScalarAt(valNode) @@ -150,7 +179,7 @@ func (s *KubeSpec) UnmarshalYAML(node *yaml.Node) error { if err := validateFileExists(v); err != nil { return err } - s.Apply = v + a.Apply = v case "create": if valNode.Kind != yaml.ScalarNode { return errors.ExpectedScalarAt(valNode) @@ -159,7 +188,7 @@ func (s *KubeSpec) UnmarshalYAML(node *yaml.Node) error { if err := validateFileExists(v); err != nil { return err } - s.Create = v + a.Create = v case "get": if valNode.Kind != yaml.ScalarNode { return errors.ExpectedScalarAt(valNode) @@ -168,7 +197,7 @@ func (s *KubeSpec) UnmarshalYAML(node *yaml.Node) error { if err := validateResourceIdentifier(v); err != nil { return err } - s.Get = v + a.Get = v case "delete": if valNode.Kind != yaml.ScalarNode { return errors.ExpectedScalarAt(valNode) @@ -180,7 +209,7 @@ func (s *KubeSpec) UnmarshalYAML(node *yaml.Node) error { if err := validateFileExists(v); err != nil { return err } - s.Delete = v + a.Delete = v case "with": if valNode.Kind != yaml.MappingNode { return errors.ExpectedMapAt(valNode) @@ -195,9 +224,7 @@ func (s *KubeSpec) UnmarshalYAML(node *yaml.Node) error { return InvalidWithLabels(err, valNode) } } - s.With = w - default: - return errors.UnknownFieldAt(key, keyNode) + a.With = w } } return nil @@ -343,18 +370,20 @@ func expandShortcut(s *Spec) { if s.Kube != nil { return } - ks := &KubeSpec{} + ks := &KubeSpec{ + Action: Action{}, + } if s.KubeGet != "" { - ks.Get = s.KubeGet + ks.Action.Get = s.KubeGet } if s.KubeCreate != "" { - ks.Create = s.KubeCreate + ks.Action.Create = s.KubeCreate } if s.KubeApply != "" { - ks.Apply = s.KubeApply + ks.Action.Apply = s.KubeApply } if s.KubeDelete != "" { - ks.Delete = s.KubeDelete + ks.Action.Delete = s.KubeDelete } s.Kube = ks } diff --git a/parse_test.go b/parse_test.go index 284e962..cda2a2e 100644 --- a/parse_test.go +++ b/parse_test.go @@ -264,7 +264,9 @@ spec: }, KubeCreate: podYAML, Kube: &gdtkube.KubeSpec{ - Create: podYAML, + Action: gdtkube.Action{ + Create: podYAML, + }, }, }, &gdtkube.Spec{ @@ -275,7 +277,9 @@ spec: }, KubeApply: "testdata/manifests/nginx-pod.yaml", Kube: &gdtkube.KubeSpec{ - Apply: "testdata/manifests/nginx-pod.yaml", + Action: gdtkube.Action{ + Apply: "testdata/manifests/nginx-pod.yaml", + }, }, }, &gdtkube.Spec{ @@ -285,7 +289,9 @@ spec: Defaults: &gdttypes.Defaults{}, }, Kube: &gdtkube.KubeSpec{ - Create: podYAML, + Action: gdtkube.Action{ + Create: podYAML, + }, }, }, &gdtkube.Spec{ @@ -295,7 +301,9 @@ spec: Defaults: &gdttypes.Defaults{}, }, Kube: &gdtkube.KubeSpec{ - Delete: "testdata/manifests/nginx-pod.yaml", + Action: gdtkube.Action{ + Delete: "testdata/manifests/nginx-pod.yaml", + }, }, }, &gdtkube.Spec{ @@ -306,7 +314,9 @@ spec: }, KubeGet: "pods/name", Kube: &gdtkube.KubeSpec{ - Get: "pods/name", + Action: gdtkube.Action{ + Get: "pods/name", + }, }, }, &gdtkube.Spec{ @@ -316,7 +326,9 @@ spec: Defaults: &gdttypes.Defaults{}, }, Kube: &gdtkube.KubeSpec{ - Get: "pods/name", + Action: gdtkube.Action{ + Get: "pods/name", + }, }, }, &gdtkube.Spec{ @@ -326,7 +338,9 @@ spec: Defaults: &gdttypes.Defaults{}, }, Kube: &gdtkube.KubeSpec{ - Get: "pods/foo", + Action: gdtkube.Action{ + Get: "pods/foo", + }, }, Assert: &gdtkube.Expect{ Len: &zero, diff --git a/spec.go b/spec.go index cd8512b..e2975bb 100644 --- a/spec.go +++ b/spec.go @@ -11,18 +11,11 @@ import ( gdttypes "github.com/gdt-dev/gdt/types" ) -// With houses one or more selectors that the Get and Delete fields may use to -// select the resources to operate against. -type With struct { - // Labels is a map, keyed by metadata Label, of Label values to select a - // resource by - Labels map[string]string `yaml:"labels,omitempty"` -} - // KubeSpec is the complex type containing all of the Kubernetes-specific -// actions and assertions. Most users will use the `kube.create`, `kube.apply` -// and `kube.describe` shortcut fields. +// actions. Most users will use the `kube.create`, `kube.apply` and +// `kube.describe` shortcut fields. type KubeSpec struct { + Action // Config is the path of the kubeconfig to use in executing Kubernetes // client calls for this Spec. If empty, the `kube` defaults' `config` // value will be used. If that is empty, the following precedence is used: @@ -39,52 +32,6 @@ type KubeSpec struct { // calling the Kubernetes API. If empty, any namespace specified in the // Defaults is used and then the string "default" is used. Namespace string `yaml:"namespace,omitempty"` - // Create is a string containing a file path or raw YAML content describing - // a Kubernetes resource to call `kubectl create` with. - Create string `yaml:"create,omitempty"` - // Apply is a string containing a file path or raw YAML content describing - // a Kubernetes resource to call `kubectl apply` with. - Apply string `yaml:"apply,omitempty"` - // Delete is a string containing an argument to `kubectl delete` and must - // be one of the following: - // - // - a file path to a manifest that will be read and the resources - // described in the manifest will be deleted - // - a resource kind or kind alias, e.g. "pods", "po", followed by one of - // the following: - // * a space or `/` character followed by the resource name to delete - // only a resource with that name. - // * a space followed by `-l ` followed by a label to delete resources - // having such a label. - // * the string `--all` to delete all resources of that kind. - Delete string `yaml:"delete,omitempty"` - // Get is a string containing an argument to `kubectl get` and must be one - // of the following: - // - // - a file path to a manifest that will be read and the resources within - // retrieved via `kubectl get` - // - a resource kind or kind alias, e.g. "pods", "po", followed by one of - // the following: - // * a space or `/` character followed by the resource name to get only a - // resource with that name. - // * a space followed by `-l ` followed by a label to get resources - // having such a label. - Get string `yaml:"get,omitempty"` - // With houses one or more selectors that the Get and Delete fields may use - // to select the resources to operate against. - // - // Use in conjunction with Get and Delete to filter resources: - // - // ```yaml - // tests: - // - name: delete pods with app:nginx label - // kube: - // delete: pods - // with: - // labels: - // app: nginx - // ``` - With *With `yaml:"with,omitempty"` } // Spec describes a test of a *single* Kubernetes API request and response. From 42953f07a270839558f0136d3d90cf297752c87e Mon Sep 17 00:00:00 2001 From: Jay Pipes Date: Mon, 14 Aug 2023 17:36:47 -0400 Subject: [PATCH 3/3] rework label selector and resource identifier type **This introduces a breaking API change** Previously, the following YAML was used to select (or delete) resources in a `gdt-kube` test spec: ```yaml tests: - kube.get: pods with: labels: app: nginx ``` This functionality has been changed to use the following format instead: ```yaml tests: - kube: get: type: pods labels: app: nginx ``` or using the `kube.get` shortcut, like so: ```yaml tests: - kube.get: type: pods labels: app: nginx ``` This was changed in order to better accomodate additional Kubernetes actions coming in future PRs, including `logs` and `exec` actions, as well as to standardize the parsing of resource identifiers and label selectors. Signed-off-by: Jay Pipes --- README.md | 46 ++-- action.go | 52 +--- connect.go | 5 - errors.go | 40 ++- eval.go | 30 +- go.mod | 2 +- go.sum | 4 +- identifier.go | 193 +++++++++++++ parse.go | 257 +++++++----------- parse_test.go | 78 +++--- spec.go | 14 +- testdata/list-pods-with-labels.yaml | 8 +- testdata/parse.yaml | 11 + .../parse/fail/more-than-one-shortcut.yaml | 5 - testdata/parse/fail/with-labels-invalid.yaml | 4 +- .../fail/with-labels-only-get-delete.yaml | 8 - 16 files changed, 448 insertions(+), 309 deletions(-) create mode 100644 identifier.go delete mode 100644 testdata/parse/fail/more-than-one-shortcut.yaml delete mode 100644 testdata/parse/fail/with-labels-only-get-delete.yaml diff --git a/README.md b/README.md index 787b101..1f7367b 100644 --- a/README.md +++ b/README.md @@ -141,22 +141,17 @@ matches some expectation: allows you to override the `defaults.namespace` value from the test scenario. * `kube`: (optional) an object containing actions and assertions the test takes against the Kubernetes API server. -* `kube.get`: (optional) string containing either a resource specifier (e.g. - `pods`, `po/nginx` or a file path to a YAML manifest containing resources - that will be read from the Kubernetes API server. +* `kube.get`: (optional) string or object containing a resource identifier + (e.g. `pods`, `po/nginx` or label selector for resources that will be read + from the Kubernetes API server. * `kube.create`: (optional) string containing either a file path to a YAML manifest or a string of raw YAML containing the resource(s) to create. * `kube.apply`: (optional) string containing either a file path to a YAML manifest or a string of raw YAML containing the resource(s) for which `gdt-kube` will perform a Kubernetes Apply call. -* `kube.delete`: (optional) string containing either a resource specifier (e.g. - `pods`, `po/nginx` or a file path to a YAML manifest containing resources - that will be deleted. -* `kube.with`: (optional) object containing selectors with which to filter - `get` and `delete` operations. -* `kube.with.labels`: (optional) `map[string]string` containing the label keys - and values to use in constructing an equality label selector (for all listed - labels) +* `kube.delete`: (optional) string or object containing either a resource + identifier (e.g. `pods`, `po/nginx` , a file path to a YAML manifest, or a + label selector for resources that will be deleted. * `assert`: (optional) object containing assertions to make about the action performed by the test. * `assert.error`: (optional) string to match a returned error from the @@ -236,14 +231,34 @@ Testing that there are two Pods having the label `app:nginx`: ```yaml name: list-pods-with-labels tests: + # You can use the shortcut kube.get + - name: verify-pods-with-app-nginx-label + kube.get: + type: pods + labels: + app: nginx + assert: + len: 2 + # Or the long-form kube:get - name: verify-pods-with-app-nginx-label kube: - get: pods - with: + get: + type: pods labels: app: nginx assert: len: 2 + # Like "kube.get", you can pass a label selector for "kube.delete" + - kube.delete: + type: pods + labels: + app: nginx + # And you can use the long-form kube:delete as well + - kube: + delete: + type: pods + labels: + app: nginx ``` Testing that a Pod with the name `nginx` exists by the specified timeout @@ -253,9 +268,8 @@ the timeout): ```yaml name: test-nginx-pod-exists-within-1-minute tests: - - kube: - get: pods/nginx - timeout: 1m + - kube.get: pods/nginx + timeout: 1m ``` Testing creation and subsequent fetch then delete of a Pod, specifying the Pod diff --git a/action.go b/action.go index b52ef11..a774278 100644 --- a/action.go +++ b/action.go @@ -4,14 +4,6 @@ package kube -// With houses one or more selectors that the Get and Delete fields may use to -// select the resources to operate against. -type With struct { - // Labels is a map, keyed by metadata Label, of Label values to select a - // resource by - Labels map[string]string `yaml:"labels,omitempty"` -} - // Action describes the the Kubernetes-specific action that is performed by the // test. type Action struct { @@ -21,8 +13,9 @@ type Action struct { // Apply is a string containing a file path or raw YAML content describing // a Kubernetes resource to call `kubectl apply` with. Apply string `yaml:"apply,omitempty"` - // Delete is a string containing an argument to `kubectl delete` and must - // be one of the following: + // Delete is a string or object containing arguments to `kubectl delete`. + // + // It must be one of the following: // // - a file path to a manifest that will be read and the resources // described in the manifest will be deleted @@ -30,35 +23,18 @@ type Action struct { // the following: // * a space or `/` character followed by the resource name to delete // only a resource with that name. - // * a space followed by `-l ` followed by a label to delete resources - // having such a label. - // * the string `--all` to delete all resources of that kind. - Delete string `yaml:"delete,omitempty"` - // Get is a string containing an argument to `kubectl get` and must be one - // of the following: + // - an object with a `type` and optional `labels` field containing a label + // selector that should be used to select that `type` of resource. + Delete *ResourceIdentifierOrFile `yaml:"delete,omitempty"` + // Get is a string or object containing arguments to `kubectl get`. // - // - a file path to a manifest that will be read and the resources within - // retrieved via `kubectl get` - // - a resource kind or kind alias, e.g. "pods", "po", followed by one of - // the following: + // It must be one of the following: + // + // - a string with a resource kind or kind alias, e.g. "pods", "po", + // followed by one of the following: // * a space or `/` character followed by the resource name to get only a // resource with that name. - // * a space followed by `-l ` followed by a label to get resources - // having such a label. - Get string `yaml:"get,omitempty"` - // With houses one or more selectors that the Get and Delete fields may use - // to select the resources to operate against. - // - // Use in conjunction with Get and Delete to filter resources: - // - // ```yaml - // tests: - // - name: delete pods with app:nginx label - // kube: - // delete: pods - // with: - // labels: - // app: nginx - // ``` - With *With `yaml:"with,omitempty"` + // - an object with a `type` and optional `labels` field containing a label + // selector that should be used to select that `type` of resource. + Get *ResourceIdentifier `yaml:"get,omitempty"` } diff --git a/connect.go b/connect.go index 36f7c09..f80ace5 100644 --- a/connect.go +++ b/connect.go @@ -88,11 +88,6 @@ func (s *Spec) Config(ctx context.Context) (*rest.Config, error) { ).ClientConfig() } -type groupVersion struct { - group string - version string -} - // connection is a struct containing a discovery client and a dynamic client // that the Spec uses to communicate with Kubernetes. type connection struct { diff --git a/errors.go b/errors.go index de055bb..f6a4607 100644 --- a/errors.go +++ b/errors.go @@ -22,14 +22,6 @@ var ( "or a string with embedded YAML", gdterrors.ErrParse, ) - // ErrMoreThanOneShortcut is returned when the test author included - // more than one shortcut (e.g. `kube.create` or `kube.apply`) in the same - // test spec. - ErrMoreThanOneShortcut = fmt.Errorf( - "%w: you may only specify a single shortcut field (e.g. "+ - "`kube.create` or `kube.apply`", - gdterrors.ErrParse, - ) // ErrEitherShortcutOrKubeSpec is returned when the test author // included both a shortcut (e.g. `kube.create` or `kube.apply`) AND the // long-form `kube` object in the same test spec. @@ -128,6 +120,24 @@ var ( ) ) +// EitherShortcutOrKubeSpecAt returns ErrEitherShortcutOrKubeSpec for a given +// YAML node +func EitherShortcutOrKubeSpecAt(node *yaml.Node) error { + return fmt.Errorf( + "%w at line %d, column %d", + ErrEitherShortcutOrKubeSpec, node.Line, node.Column, + ) +} + +// MoreThanOneKubeActionAt returns ErrMoreThanOneKubeAction for a given YAML +// node +func MoreThanOneKubeActionAt(node *yaml.Node) error { + return fmt.Errorf( + "%w at line %d, column %d", + ErrMoreThanOneKubeAction, node.Line, node.Column, + ) +} + // ExpectedMapOrYAMLStringAt returns ErrExpectedMapOrYAMLString for a given // YAML node func ExpectedMapOrYAMLStringAt(node *yaml.Node) error { @@ -144,15 +154,21 @@ func KubeConfigNotFound(path string) error { // InvalidResourceSpecifier returns ErrResourceSpecifier for a given // supplied resource specifier. -func InvalidResourceSpecifier(subject string) error { - return fmt.Errorf("%w: %s", ErrResourceSpecifierInvalid, subject) +func InvalidResourceSpecifier(subject string, node *yaml.Node) error { + return fmt.Errorf( + "%w: %s at line %d, column %d", + ErrResourceSpecifierInvalid, subject, node.Line, node.Column, + ) } // InvalidResourceSpecifierOrFilepath returns // ErrResourceSpecifierOrFilepath for a given supplied subject. -func InvalidResourceSpecifierOrFilepath(subject string) error { +func InvalidResourceSpecifierOrFilepath( + subject string, node *yaml.Node, +) error { return fmt.Errorf( - "%w: %s", ErrResourceSpecifierInvalidOrFilepath, subject, + "%w: %s at line %d, column %d", + ErrResourceSpecifierInvalidOrFilepath, subject, node.Line, node.Column, ) } diff --git a/eval.go b/eval.go index 7104bf2..cdf92ae 100644 --- a/eval.go +++ b/eval.go @@ -49,13 +49,13 @@ func (s *Spec) Eval(ctx context.Context, t *testing.T) *result.Result { } var res *result.Result t.Run(s.Title(), func(t *testing.T) { - if s.Kube.Get != "" { + if s.Kube.Get != nil { res = s.get(ctx, t, c) } if s.Kube.Create != "" { res = s.create(ctx, t, c) } - if s.Kube.Delete != "" { + if s.Kube.Delete != nil { res = s.delete(ctx, t, c) } if s.Kube.Apply != "" { @@ -83,7 +83,7 @@ func (s *Spec) get( t *testing.T, c *connection, ) *result.Result { - kind, name := splitKindName(s.Kube.Get) + kind, name := s.Kube.Get.KindName() gvk := schema.GroupVersionKind{ Kind: kind, } @@ -146,12 +146,10 @@ func (s *Spec) doList( namespace string, ) gdttypes.Assertions { opts := metav1.ListOptions{} - with := s.Kube.With - if with != nil { - if with.Labels != nil { - // We already validated the label selector during parse-time - opts.LabelSelector = labels.Set(with.Labels).String() - } + withlabels := s.Kube.Get.Labels() + if withlabels != nil { + // We already validated the label selector during parse-time + opts.LabelSelector = labels.Set(withlabels).String() } list, err := c.client.Resource(res).Namespace(namespace).List( ctx, opts, @@ -346,8 +344,8 @@ func (s *Spec) delete( t *testing.T, c *connection, ) *result.Result { - if probablyFilePath(s.Kube.Delete) { - path := s.Kube.Delete + if s.Kube.Delete.FilePath() != "" { + path := s.Kube.Delete.FilePath() f, err := os.Open(path) if err != nil { // This should never happen because we check during parse time @@ -385,7 +383,7 @@ func (s *Spec) delete( return result.New() } - kind, name := splitKindName(s.Kube.Delete) + kind, name := s.Kube.Delete.KindName() gvk := schema.GroupVersionKind{ Kind: kind, } @@ -428,10 +426,16 @@ func (s *Spec) doDeleteCollection( res schema.GroupVersionResource, namespace string, ) *result.Result { + listOpts := metav1.ListOptions{} + withlabels := s.Kube.Delete.Labels() + if withlabels != nil { + // We already validated the label selector during parse-time + listOpts.LabelSelector = labels.Set(withlabels).String() + } err := c.client.Resource(res).Namespace(namespace).DeleteCollection( ctx, metav1.DeleteOptions{}, - metav1.ListOptions{}, + listOpts, ) a := newAssertions(s.Assert, err, nil) return result.New(result.WithFailures(a.Failures()...)) diff --git a/go.mod b/go.mod index 4e8c485..12b3939 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.19 require ( github.com/cenkalti/backoff/v4 v4.2.1 - github.com/gdt-dev/gdt v1.2.0 + github.com/gdt-dev/gdt v1.2.1 github.com/samber/lo v1.38.1 github.com/stretchr/testify v1.8.4 gopkg.in/yaml.v3 v3.0.1 diff --git a/go.sum b/go.sum index d7681ae..c009cec 100644 --- a/go.sum +++ b/go.sum @@ -65,8 +65,8 @@ github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7 github.com/evanphx/json-patch v4.12.0+incompatible h1:4onqiflcdA9EOZ4RxV643DvftH5pOlLGNtQ5lPWQu84= github.com/evanphx/json-patch/v5 v5.6.0 h1:b91NhWfaz02IuVxO9faSllyAtNXHMPkC5J8sJCLunww= github.com/evanphx/json-patch/v5 v5.6.0/go.mod h1:G79N1coSVB93tBe7j6PhzjmR3/2VvlbKOFpnXhI9Bw4= -github.com/gdt-dev/gdt v1.2.0 h1:wXHeXjbjHBLkOCDU0/Wmy8zC0/u9468JDKRhYqxPI5s= -github.com/gdt-dev/gdt v1.2.0/go.mod h1:StnyGjC/67u59La2u6fh3HwW9MmodVhKdXcLlkgvNSY= +github.com/gdt-dev/gdt v1.2.1 h1:tNIpBPLatk8Rb0YFSK+FOzKIhHPYgmLpXQL8qottNcI= +github.com/gdt-dev/gdt v1.2.1/go.mod h1:StnyGjC/67u59La2u6fh3HwW9MmodVhKdXcLlkgvNSY= github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU= github.com/go-gl/glfw/v3.3/glfw v0.0.0-20191125211704-12ad95a8df72/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= diff --git a/identifier.go b/identifier.go new file mode 100644 index 0000000..3242cec --- /dev/null +++ b/identifier.go @@ -0,0 +1,193 @@ +// Use and distribution licensed under the Apache license version 2. +// +// See the COPYING file in the root project directory for full text. + +package kube + +import ( + "path/filepath" + "strings" + + "gopkg.in/yaml.v3" + "k8s.io/apimachinery/pkg/labels" + + gdterrors "github.com/gdt-dev/gdt/errors" +) + +// resourceIdentifierWithSelector is the full long-form resource identifier as +// a struct +type resourceIdentifierWithSelector struct { + // Type is the resource type to select. This should *not* be a type/name + // combination. + Type string `yaml:"type"` + // Labels is a map, keyed by metadata Label, of Label values to select a + // resource by + Labels map[string]string `yaml:"labels,omitempty"` +} + +// ResourceIdentifier is a struct used to parse an interface{} that can be +// either a string or a struct containing a selector with things like a label +// key/value map. +type ResourceIdentifier struct { + kind string `yaml:"-"` + name string `yaml:"-"` + labels map[string]string `yaml:"-"` +} + +// Title returns the resource identifier's kind and name, if present +func (r *ResourceIdentifier) Title() string { + if r.name == "" { + return r.kind + } + return r.kind + "/" + r.name +} + +// KindName returns the resource identifier's kind and name +func (r *ResourceIdentifier) KindName() (string, string) { + return r.kind, r.name +} + +// Labels returns the resource identifier's labels map, if present +func (r *ResourceIdentifier) Labels() map[string]string { + return r.labels +} + +// UnmarshalYAML is a custom unmarshaler that understands that the value of the +// ResourceIdentifier can be either a string or a selector. +func (r *ResourceIdentifier) UnmarshalYAML(node *yaml.Node) error { + if node.Kind != yaml.ScalarNode && node.Kind != yaml.MappingNode { + return gdterrors.ExpectedScalarOrMapAt(node) + } + var s string + // A resource identifier can be a string of the form {type}/{name} or + // {type}. + if err := node.Decode(&s); err == nil { + if strings.ContainsAny(s, " ,;\n\t\r") { + return InvalidResourceSpecifier(s, node) + } + if strings.Count(s, "/") > 1 { + return InvalidResourceSpecifier(s, node) + } + r.kind, r.name = splitKindName(s) + return nil + } + // Otherwise the resource identifier should be specified broken out as a + // struct with a `type` and `labels` field. + var ri resourceIdentifierWithSelector + if err := node.Decode(&ri); err != nil { + return err + } + _, err := labels.ValidatedSelectorFromSet(ri.Labels) + if err != nil { + return InvalidWithLabels(err, node) + } + r.kind = ri.Type + r.name = "" + r.labels = ri.Labels + return nil +} + +func NewResourceIdentifier( + kind string, + name string, + labels map[string]string, +) *ResourceIdentifier { + return &ResourceIdentifier{ + kind: kind, + name: name, + labels: labels, + } +} + +// ResourceIdentifierOrFile is a struct used to parse an interface{} that can +// be either a string, a filepath or a struct containing a selector with things +// like a label key/value map. +type ResourceIdentifierOrFile struct { + fp string `yaml:"-"` + kind string `yaml:"-"` + name string `yaml:"-"` + labels map[string]string `yaml:"-"` +} + +// FilePath returns the resource identifier's file path, if present +func (r *ResourceIdentifierOrFile) FilePath() string { + return r.fp +} + +// Title returns the resource identifier's file name, if present, or the kind +// and name, if present +func (r *ResourceIdentifierOrFile) Title() string { + if r.fp != "" { + return filepath.Base(r.fp) + } + if r.name == "" { + return r.kind + } + return r.kind + "/" + r.name +} + +// KindName returns the resource identifier's kind and name +func (r *ResourceIdentifierOrFile) KindName() (string, string) { + return r.kind, r.name +} + +// Labels returns the resource identifier's labels map, if present +func (r *ResourceIdentifierOrFile) Labels() map[string]string { + return r.labels +} + +// UnmarshalYAML is a custom unmarshaler that understands that the value of the +// ResourceIdentifierOrFile can be either a string or a selector. +func (r *ResourceIdentifierOrFile) UnmarshalYAML(node *yaml.Node) error { + if node.Kind != yaml.ScalarNode && node.Kind != yaml.MappingNode { + return gdterrors.ExpectedScalarOrMapAt(node) + } + var s string + // A resource identifier can be a filepath, a string of the form + // {type}/{name} or {type}. + if err := node.Decode(&s); err == nil { + if probablyFilePath(s) { + if !fileExists(s) { + return gdterrors.FileNotFound(s, node) + } + r.fp = s + return nil + } + if strings.ContainsAny(s, " ,;\n\t\r") { + return InvalidResourceSpecifierOrFilepath(s, node) + } + if strings.Count(s, "/") > 1 { + return InvalidResourceSpecifierOrFilepath(s, node) + } + r.kind, r.name = splitKindName(s) + return nil + } + // Otherwise the resource identifier should be specified broken out as a + // struct with a `type` and `labels` field. + var ri resourceIdentifierWithSelector + if err := node.Decode(&ri); err != nil { + return err + } + _, err := labels.ValidatedSelectorFromSet(ri.Labels) + if err != nil { + return InvalidWithLabels(err, node) + } + r.kind = ri.Type + r.name = "" + r.labels = ri.Labels + return nil +} + +func NewResourceIdentifierOrFile( + fp string, + kind string, + name string, + labels map[string]string, +) *ResourceIdentifierOrFile { + return &ResourceIdentifierOrFile{ + fp: fp, + kind: kind, + name: name, + labels: labels, + } +} diff --git a/parse.go b/parse.go index 03ae03b..f4c0a40 100644 --- a/parse.go +++ b/parse.go @@ -6,20 +6,22 @@ package kube import ( "os" - "strings" gdtjson "github.com/gdt-dev/gdt/assertion/json" "github.com/gdt-dev/gdt/errors" gdttypes "github.com/gdt-dev/gdt/types" "github.com/samber/lo" "gopkg.in/yaml.v3" - "k8s.io/apimachinery/pkg/labels" ) func (s *Spec) UnmarshalYAML(node *yaml.Node) error { if node.Kind != yaml.MappingNode { return errors.ExpectedMapAt(node) } + // We do an initial pass over the shortcut fields, then all the + // non-shortcut fields after that. + var ks *KubeSpec + // maps/structs are stored in a top-level Node.Content field which is a // concatenated slice of Node pointers in pairs of key/values. for i := 0; i < len(node.Content); i += 2 { @@ -30,50 +32,83 @@ func (s *Spec) UnmarshalYAML(node *yaml.Node) error { key := keyNode.Value valNode := node.Content[i+1] switch key { - case "kube": - if valNode.Kind != yaml.MappingNode { - return errors.ExpectedMapAt(valNode) - } - var ks *KubeSpec - if err := valNode.Decode(&ks); err != nil { - return err - } - s.Kube = ks case "kube.get": - if valNode.Kind != yaml.ScalarNode { + if valNode.Kind != yaml.ScalarNode && valNode.Kind != yaml.MappingNode { return errors.ExpectedScalarAt(valNode) } - v := valNode.Value - if err := validateResourceIdentifier(v); err != nil { + if ks != nil { + return MoreThanOneKubeActionAt(valNode) + } + var v *ResourceIdentifier + if err := valNode.Decode(&v); err != nil { return err } - s.KubeGet = v + ks = &KubeSpec{} + ks.Get = v + s.Kube = ks case "kube.create": if valNode.Kind != yaml.ScalarNode { return errors.ExpectedScalarAt(valNode) } + if ks != nil { + return MoreThanOneKubeActionAt(valNode) + } v := valNode.Value - if err := validateFileExists(v); err != nil { - return err + if probablyFilePath(v) { + if !fileExists(v) { + return errors.FileNotFound(v, valNode) + } } - s.KubeCreate = v + ks = &KubeSpec{} + ks.Create = v + s.Kube = ks case "kube.apply": if valNode.Kind != yaml.ScalarNode { return errors.ExpectedScalarAt(valNode) } - s.KubeApply = valNode.Value + if ks != nil { + return MoreThanOneKubeActionAt(valNode) + } + v := valNode.Value + ks = &KubeSpec{} + ks.Apply = v + s.Kube = ks case "kube.delete": - if valNode.Kind != yaml.ScalarNode { + if valNode.Kind != yaml.ScalarNode && valNode.Kind != yaml.MappingNode { return errors.ExpectedScalarAt(valNode) } - v := valNode.Value - if err := validateResourceIdentifierOrFilepath(v); err != nil { + if ks != nil { + return MoreThanOneKubeActionAt(valNode) + } + var v *ResourceIdentifierOrFile + if err := valNode.Decode(&v); err != nil { return err } - if err := validateFileExists(v); err != nil { + ks = &KubeSpec{} + ks.Delete = v + s.Kube = ks + } + } + + for i := 0; i < len(node.Content); i += 2 { + keyNode := node.Content[i] + if keyNode.Kind != yaml.ScalarNode { + return errors.ExpectedScalarAt(keyNode) + } + key := keyNode.Value + valNode := node.Content[i+1] + switch key { + case "kube": + if valNode.Kind != yaml.MappingNode { + return errors.ExpectedMapAt(valNode) + } + if ks != nil { + return EitherShortcutOrKubeSpecAt(valNode) + } + if err := valNode.Decode(&ks); err != nil { return err } - s.KubeDelete = v + s.Kube = ks case "assert": if valNode.Kind != yaml.MappingNode { return errors.ExpectedMapAt(valNode) @@ -83,6 +118,8 @@ func (s *Spec) UnmarshalYAML(node *yaml.Node) error { return err } s.Assert = e + case "kube.get", "kube.create", "kube.delete", "kube.apply": + continue default: if lo.Contains(gdttypes.BaseSpecFields, key) { continue @@ -90,19 +127,6 @@ func (s *Spec) UnmarshalYAML(node *yaml.Node) error { return errors.UnknownFieldAt(key, keyNode) } } - if err := validateShortcuts(s); err != nil { - return err - } - expandShortcut(s) - if moreThanOneAction(s) { - return ErrMoreThanOneKubeAction - } - with := s.Kube.With - if with != nil { - if s.Kube.Get == "" && s.Kube.Delete == "" { - return ErrWithLabelsOnlyGetDelete - } - } return nil } @@ -125,8 +149,8 @@ func (s *KubeSpec) UnmarshalYAML(node *yaml.Node) error { return errors.ExpectedScalarAt(valNode) } fp := valNode.Value - if err := validateFileExists(fp); err != nil { - return err + if !fileExists(fp) { + return errors.FileNotFound(fp, valNode) } s.Config = fp case "context": @@ -142,7 +166,7 @@ func (s *KubeSpec) UnmarshalYAML(node *yaml.Node) error { return errors.ExpectedScalarAt(valNode) } s.Namespace = valNode.Value - case "get", "create", "apply", "delete", "with": + case "get", "create", "apply", "delete": // Because Action is an embedded struct and we parse it below, just // ignore these fields in the top-level `kube:` field for now. default: @@ -176,8 +200,10 @@ func (a *Action) UnmarshalYAML(node *yaml.Node) error { return errors.ExpectedScalarAt(valNode) } v := valNode.Value - if err := validateFileExists(v); err != nil { - return err + if probablyFilePath(v) { + if !fileExists(v) { + return errors.FileNotFound(v, valNode) + } } a.Apply = v case "create": @@ -185,48 +211,35 @@ func (a *Action) UnmarshalYAML(node *yaml.Node) error { return errors.ExpectedScalarAt(valNode) } v := valNode.Value - if err := validateFileExists(v); err != nil { - return err + if probablyFilePath(v) { + if !fileExists(v) { + return errors.FileNotFound(v, valNode) + } } a.Create = v case "get": - if valNode.Kind != yaml.ScalarNode { - return errors.ExpectedScalarAt(valNode) + if valNode.Kind != yaml.ScalarNode && valNode.Kind != yaml.MappingNode { + return errors.ExpectedScalarOrMapAt(valNode) } - v := valNode.Value - if err := validateResourceIdentifier(v); err != nil { + var v *ResourceIdentifier + if err := valNode.Decode(&v); err != nil { return err } a.Get = v case "delete": - if valNode.Kind != yaml.ScalarNode { - return errors.ExpectedScalarAt(valNode) - } - v := valNode.Value - if err := validateResourceIdentifierOrFilepath(v); err != nil { - return err + if valNode.Kind != yaml.ScalarNode && valNode.Kind != yaml.MappingNode { + return errors.ExpectedScalarOrMapAt(valNode) } - if err := validateFileExists(v); err != nil { + var v *ResourceIdentifierOrFile + if err := valNode.Decode(&v); err != nil { return err } a.Delete = v - case "with": - if valNode.Kind != yaml.MappingNode { - return errors.ExpectedMapAt(valNode) - } - var w *With - if err := valNode.Decode(&w); err != nil { - return err - } - if w.Labels != nil { - _, err := labels.ValidatedSelectorFromSet(w.Labels) - if err != nil { - return InvalidWithLabels(err, valNode) - } - } - a.With = w } } + if moreThanOneAction(a) { + return ErrMoreThanOneKubeAction + } return nil } @@ -313,8 +326,10 @@ func (e *Expect) UnmarshalYAML(node *yaml.Node) error { if err := valNode.Decode(&v); err != nil { return err } - if err := validateFileExists(v); err != nil { - return err + if probablyFilePath(v) { + if !fileExists(v) { + return errors.FileNotFound(v, valNode) + } } // inline YAML. check it can be unmarshaled into a // map[string]interface{} @@ -333,37 +348,6 @@ func (e *Expect) UnmarshalYAML(node *yaml.Node) error { return nil } -// validateShortcuts ensures that the test author has specified only a single -// shortcut (e.g. `kube.create`) and that if a shortcut is specified, any -// long-form KubeSpec is not present. -func validateShortcuts(s *Spec) error { - foundShortcuts := 0 - if s.KubeGet != "" { - foundShortcuts += 1 - } - if s.KubeCreate != "" { - foundShortcuts += 1 - } - if s.KubeApply != "" { - foundShortcuts += 1 - } - if s.KubeDelete != "" { - foundShortcuts += 1 - } - if s.Kube == nil { - if foundShortcuts > 1 { - return ErrMoreThanOneShortcut - } else if foundShortcuts == 0 { - return ErrEitherShortcutOrKubeSpec - } - } else { - if foundShortcuts > 0 { - return ErrEitherShortcutOrKubeSpec - } - } - return nil -} - // expandShortcut looks at the shortcut fields (e.g. `kube.create`) and expands // the shortcut into a full KubeSpec. func expandShortcut(s *Spec) { @@ -373,82 +357,35 @@ func expandShortcut(s *Spec) { ks := &KubeSpec{ Action: Action{}, } - if s.KubeGet != "" { - ks.Action.Get = s.KubeGet - } if s.KubeCreate != "" { - ks.Action.Create = s.KubeCreate + ks.Create = s.KubeCreate } if s.KubeApply != "" { - ks.Action.Apply = s.KubeApply - } - if s.KubeDelete != "" { - ks.Action.Delete = s.KubeDelete + ks.Apply = s.KubeApply } s.Kube = ks } // moreThanOneAction returns true if the test author has specified more than a // single action in the KubeSpec. -func moreThanOneAction(s *Spec) bool { +func moreThanOneAction(a *Action) bool { foundActions := 0 - if s.Kube.Get != "" { + if a.Get != nil { foundActions += 1 } - if s.Kube.Create != "" { + if a.Create != "" { foundActions += 1 } - if s.Kube.Apply != "" { + if a.Apply != "" { foundActions += 1 } - if s.Kube.Delete != "" { + if a.Delete != nil { foundActions += 1 } return foundActions > 1 } -// validateFileExists returns an error if the supplied path looks like a file -// path but the file does not exist. -func validateFileExists(path string) error { - if probablyFilePath(path) { - _, err := os.Stat(path) - if err != nil { - if os.IsNotExist(err) { - return errors.FileNotFound(path) - } - return err - } - } - return nil -} - -// validateResourceIdentifierOrFilepath returns an error if the supplied -// argument is not a filepath and contains an ill-formed Kind, Alias or -// Kind/Name specifier. Only a single Kind may be specified (i.e. no commas or -// spaces are allowed in the supplied string.) -func validateResourceIdentifierOrFilepath(subject string) error { - if probablyFilePath(subject) { - return nil - } - if strings.ContainsAny(subject, " ,;\n\t\r") { - return InvalidResourceSpecifierOrFilepath(subject) - } - if strings.Count(subject, "/") > 1 { - return InvalidResourceSpecifierOrFilepath(subject) - } - return nil -} - -// validateResourceIdentifier returns an error if the supplied argument -// contains an ill-formed Kind, Alias or Kind/Name specifier. Only a single -// Kind may be specified (i.e. no commas or spaces are allowed in the supplied -// string.) -func validateResourceIdentifier(subject string) error { - if strings.ContainsAny(subject, " ,;\n\t\r") { - return InvalidResourceSpecifier(subject) - } - if strings.Count(subject, "/") > 1 { - return InvalidResourceSpecifier(subject) - } - return nil +func fileExists(path string) bool { + _, err := os.Stat(path) + return err == nil } diff --git a/parse_test.go b/parse_test.go index cda2a2e..e46a0a6 100644 --- a/parse_test.go +++ b/parse_test.go @@ -60,19 +60,6 @@ func TestFailureBothShortcutAndKubeSpec(t *testing.T) { require.Nil(s) } -func TestFailureMoreThanOneShortcut(t *testing.T) { - assert := assert.New(t) - require := require.New(t) - - fp := filepath.Join("testdata", "parse", "fail", "more-than-one-shortcut.yaml") - - s, err := gdt.From(fp) - require.NotNil(err) - assert.ErrorIs(err, gdtkube.ErrMoreThanOneShortcut) - assert.ErrorIs(err, errors.ErrParse) - require.Nil(s) -} - func TestFailureMoreThanOneKubeAction(t *testing.T) { assert := assert.New(t) require := require.New(t) @@ -203,19 +190,6 @@ func TestFailureBadMatchesNotMapAny(t *testing.T) { require.Nil(s) } -func TestWithLabelsOnlyGetDelete(t *testing.T) { - assert := assert.New(t) - require := require.New(t) - - fp := filepath.Join("testdata", "parse", "fail", "with-labels-only-get-delete.yaml") - - s, err := gdt.From(fp) - require.NotNil(err) - assert.ErrorIs(err, gdtkube.ErrWithLabelsOnlyGetDelete) - assert.ErrorIs(err, errors.ErrParse) - require.Nil(s) -} - func TestWithLabelsInvalid(t *testing.T) { assert := assert.New(t) require := require.New(t) @@ -262,7 +236,6 @@ spec: Name: "create a pod from YAML using kube.create shortcut", Defaults: &gdttypes.Defaults{}, }, - KubeCreate: podYAML, Kube: &gdtkube.KubeSpec{ Action: gdtkube.Action{ Create: podYAML, @@ -275,7 +248,6 @@ spec: Name: "apply a pod from a file using kube.apply shortcut", Defaults: &gdttypes.Defaults{}, }, - KubeApply: "testdata/manifests/nginx-pod.yaml", Kube: &gdtkube.KubeSpec{ Action: gdtkube.Action{ Apply: "testdata/manifests/nginx-pod.yaml", @@ -302,7 +274,10 @@ spec: }, Kube: &gdtkube.KubeSpec{ Action: gdtkube.Action{ - Delete: "testdata/manifests/nginx-pod.yaml", + Delete: gdtkube.NewResourceIdentifierOrFile( + "testdata/manifests/nginx-pod.yaml", + "", "", nil, + ), }, }, }, @@ -312,10 +287,11 @@ spec: Name: "fetch a pod via kube.get shortcut", Defaults: &gdttypes.Defaults{}, }, - KubeGet: "pods/name", Kube: &gdtkube.KubeSpec{ Action: gdtkube.Action{ - Get: "pods/name", + Get: gdtkube.NewResourceIdentifier( + "pods", "name", nil, + ), }, }, }, @@ -327,19 +303,55 @@ spec: }, Kube: &gdtkube.KubeSpec{ Action: gdtkube.Action{ - Get: "pods/name", + Get: gdtkube.NewResourceIdentifier( + "pods", "name", nil, + ), }, }, }, &gdtkube.Spec{ Spec: gdttypes.Spec{ Index: 6, + Name: "fetch a pod via kube.get shortcut to long-form resource identifier with labels", + Defaults: &gdttypes.Defaults{}, + }, + Kube: &gdtkube.KubeSpec{ + Action: gdtkube.Action{ + Get: gdtkube.NewResourceIdentifier( + "pods", "", map[string]string{ + "app": "nginx", + }, + ), + }, + }, + }, + &gdtkube.Spec{ + Spec: gdttypes.Spec{ + Index: 7, + Name: "fetch a pod via kube:get long-form resource identifier with labels", + Defaults: &gdttypes.Defaults{}, + }, + Kube: &gdtkube.KubeSpec{ + Action: gdtkube.Action{ + Get: gdtkube.NewResourceIdentifier( + "pods", "", map[string]string{ + "app": "nginx", + }, + ), + }, + }, + }, + &gdtkube.Spec{ + Spec: gdttypes.Spec{ + Index: 8, Name: "fetch a pod with envvar substitution", Defaults: &gdttypes.Defaults{}, }, Kube: &gdtkube.KubeSpec{ Action: gdtkube.Action{ - Get: "pods/foo", + Get: gdtkube.NewResourceIdentifier( + "pods", "foo", nil, + ), }, }, Assert: &gdtkube.Expect{ diff --git a/spec.go b/spec.go index e2975bb..7dd44ef 100644 --- a/spec.go +++ b/spec.go @@ -93,11 +93,8 @@ func (s *Spec) Title() string { // Shouldn't happen because of parsing, but you never know... return "" } - if s.Kube.Get != "" { - get := s.Kube.Get - if probablyFilePath(get) { - return "kube.get:" + filepath.Base(get) - } + if s.Kube.Get != nil { + return "kube.get:" + s.Kube.Get.Title() } if s.Kube.Create != "" { create := s.Kube.Create @@ -111,11 +108,8 @@ func (s *Spec) Title() string { return "kube.apply:" + filepath.Base(apply) } } - if s.Kube.Delete != "" { - delete := s.Kube.Delete - if probablyFilePath(delete) { - return "kube.delete:" + filepath.Base(delete) - } + if s.Kube.Delete != nil { + return "kube.delete:" + s.Kube.Delete.Title() } return "" } diff --git a/testdata/list-pods-with-labels.yaml b/testdata/list-pods-with-labels.yaml index 45cd872..4ff72ac 100644 --- a/testdata/list-pods-with-labels.yaml +++ b/testdata/list-pods-with-labels.yaml @@ -8,16 +8,16 @@ tests: create: testdata/manifests/nginx-deployment.yaml - name: verify-pods-with-app-nginx-label kube: - get: pods - with: + get: + type: pods labels: app: nginx assert: len: 2 - name: verify-no-pods-with-app-noexist-label kube: - get: pods - with: + get: + type: pods labels: app: noexist assert: diff --git a/testdata/parse.yaml b/testdata/parse.yaml index 9db0037..0ec4a21 100644 --- a/testdata/parse.yaml +++ b/testdata/parse.yaml @@ -32,6 +32,17 @@ tests: - name: fetch a pod via long-form kube:get kube: get: pods/name + - name: fetch a pod via kube.get shortcut to long-form resource identifier with labels + kube.get: + type: pods + labels: + app: nginx + - name: fetch a pod via kube:get long-form resource identifier with labels + kube: + get: + type: pods + labels: + app: nginx - name: fetch a pod with envvar substitution kube: get: pods/${pod_name} diff --git a/testdata/parse/fail/more-than-one-shortcut.yaml b/testdata/parse/fail/more-than-one-shortcut.yaml deleted file mode 100644 index fb2527d..0000000 --- a/testdata/parse/fail/more-than-one-shortcut.yaml +++ /dev/null @@ -1,5 +0,0 @@ -name: more-than-one-shortcut -description: invalid kube spec with more than one shortcut field -tests: - - kube.create: testdata/manifests/nginx-pod.yaml - kube.apply: testdata/manifests/nginx-pod.yaml diff --git a/testdata/parse/fail/with-labels-invalid.yaml b/testdata/parse/fail/with-labels-invalid.yaml index f9fe564..72669c7 100644 --- a/testdata/parse/fail/with-labels-invalid.yaml +++ b/testdata/parse/fail/with-labels-invalid.yaml @@ -2,8 +2,8 @@ name: with-labels-invalid description: invalid label selector specified tests: - kube: - get: pods - with: + get: + type: pods labels: -app: nginx _env: _qa_ diff --git a/testdata/parse/fail/with-labels-only-get-delete.yaml b/testdata/parse/fail/with-labels-only-get-delete.yaml deleted file mode 100644 index e3c77d7..0000000 --- a/testdata/parse/fail/with-labels-only-get-delete.yaml +++ /dev/null @@ -1,8 +0,0 @@ -name: with-labels-only-get-delete -description: invalid kube spec with label selector specified for create -tests: - - kube: - create: testdata/manifests/nginx-deployment.yaml - with: - labels: - app: nginx