diff --git a/api/filters/replacement/replacement.go b/api/filters/replacement/replacement.go index 70a1a4d75..39d047744 100644 --- a/api/filters/replacement/replacement.go +++ b/api/filters/replacement/replacement.go @@ -17,7 +17,6 @@ type Filter struct { } // Filter replaces values of targets with values from sources -// TODO (#3492): Connect this to a replacement transformer plugin func (f Filter) Filter(nodes []*yaml.RNode) ([]*yaml.RNode, error) { for _, r := range f.Replacements { if r.Source == nil || r.Targets == nil { @@ -37,6 +36,9 @@ func (f Filter) Filter(nodes []*yaml.RNode) ([]*yaml.RNode, error) { func applyReplacement(nodes []*yaml.RNode, value *yaml.RNode, targets []*types.TargetSelector) ([]*yaml.RNode, error) { for _, t := range targets { + if t.Select == nil { + return nil, fmt.Errorf("target must specify resources to select") + } if len(t.FieldPaths) == 0 { t.FieldPaths = []string{types.DefaultReplacementFieldPath} } @@ -54,13 +56,8 @@ func applyReplacement(nodes []*yaml.RNode, value *yaml.RNode, targets []*types.T } func applyToNode(node *yaml.RNode, value *yaml.RNode, target *types.TargetSelector) error { - if target.Select == nil { - return fmt.Errorf("target must specify resources to select") - } for _, fp := range target.FieldPaths { - fieldPath := strings.Split(fp, ".") - // TODO (#3492): Add tests for map keys in the fieldPath (e.g. .spec.containers[name=nginx]) - t, err := node.Pipe(yaml.Lookup(fieldPath...)) + t, err := node.Pipe(yaml.Lookup(strings.Split(fp, ".")...)) if err != nil { return err } @@ -83,7 +80,6 @@ func getReplacement(nodes []*yaml.RNode, r *types.Replacement) (*yaml.RNode, err } fieldPath := strings.Split(r.Source.FieldPath, ".") - // TODO (#3492): Add tests for map keys in the fieldPath (e.g. .spec.containers[name=nginx]) rn, err := source.Pipe(yaml.Lookup(fieldPath...)) if err != nil { return nil, err @@ -111,10 +107,17 @@ func selectSourceNode(nodes []*yaml.RNode, selector *types.SourceSelector) (*yam } func getKrmId(n *yaml.RNode) *types.KrmId { - ns, _ := n.GetNamespace() - apiVersion := yaml.GetValue(n.Field(yaml.APIVersionField).Value) - group, version := resid.ParseGroupVersion(apiVersion) - + ns, err := n.GetNamespace() + if err != nil { + // Resource has no metadata (no apiVersion, kind, nor metadata field). + // In this case, it cannot be selected. + return &types.KrmId{} + } + apiVersion := n.Field(yaml.APIVersionField) + var group, version string + if apiVersion != nil { + group, version = resid.ParseGroupVersion(yaml.GetValue(apiVersion.Value)) + } return &types.KrmId{ Gvk: resid.Gvk{Group: group, Version: version, Kind: n.GetKind()}, Name: n.GetName(), diff --git a/api/filters/replacement/replacement_test.go b/api/filters/replacement/replacement_test.go index bc3bca3ba..a9aaaa735 100644 --- a/api/filters/replacement/replacement_test.go +++ b/api/filters/replacement/replacement_test.go @@ -17,7 +17,7 @@ func TestFilter(t *testing.T) { input string replacements string expected string - expectedErr bool + expectedErr string }{ "simple": { input: `apiVersion: v1 @@ -45,6 +45,19 @@ spec: name: nginx-tagged - image: postgres:1.8.0 name: postgresdb +--- +apiVersion: v1 +kind: Deployment +metadata: + name: deploy3 +spec: + template: + spec: + containers: + - image: nginx:1.7.9 + name: nginx-tagged + - image: postgres:1.8.0 + name: postgresdb `, replacements: `replacements: - source: @@ -83,6 +96,19 @@ spec: name: nginx-tagged - image: postgres:1.8.0 name: postgresdb +--- +apiVersion: v1 +kind: Deployment +metadata: + name: deploy3 +spec: + template: + spec: + containers: + - image: nginx:1.7.9 + name: nginx-tagged + - image: postgres:1.8.0 + name: postgresdb `, }, "complex type": { @@ -295,7 +321,7 @@ spec: - select: kind: Deployment `, - expectedErr: true, + expectedErr: "more than one match for source ~G_~V_Deployment", }, "replacement has no source": { input: `apiVersion: v1 @@ -316,7 +342,246 @@ spec: - select: kind: Deployment `, - expectedErr: true, + expectedErr: "replacements must specify a source and at least one target", + }, + "field paths with key-value pairs": { + input: `apiVersion: v1 +kind: Deployment +metadata: + name: deploy1 +spec: + template: + spec: + containers: + - image: nginx:1.7.9 + name: nginx-tagged + - image: postgres:1.8.0 + name: postgresdb +--- +apiVersion: v1 +kind: Deployment +metadata: + name: deploy2 +spec: + template: + spec: + containers: + - image: nginx:1.7.9 + name: nginx-tagged + - image: postgres:1.8.0 + name: postgresdb +`, + replacements: `replacements: +- source: + kind: Deployment + name: deploy2 + fieldPath: spec.template.spec.containers.[name=nginx-tagged].image + targets: + - select: + kind: Deployment + name: deploy1 + fieldPaths: + - spec.template.spec.containers.[name=postgresdb].image +`, + expected: `apiVersion: v1 +kind: Deployment +metadata: + name: deploy1 +spec: + template: + spec: + containers: + - image: nginx:1.7.9 + name: nginx-tagged + - image: nginx:1.7.9 + name: postgresdb +--- +apiVersion: v1 +kind: Deployment +metadata: + name: deploy2 +spec: + template: + spec: + containers: + - image: nginx:1.7.9 + name: nginx-tagged + - image: postgres:1.8.0 + name: postgresdb +`, + }, + "select by group and version": { + input: `apiVersion: my-group-1/v1 +kind: Custom +metadata: + name: my-name +spec: + template: + spec: + containers: + - image: nginx:1.7.9 + name: nginx-tagged + - image: postgres:1.8.0 + name: postgresdb +--- +apiVersion: my-group-2/v2 +kind: Custom +metadata: + name: my-name +spec: + template: + spec: + containers: + - image: nginx:1.7.9 + name: nginx-tagged + - image: postgres:1.8.0 + name: postgresdb +--- +apiVersion: my-group-3/v3 +kind: Custom +metadata: + name: my-name +spec: + template: + spec: + containers: + - image: nginx:1.7.9 + name: nginx-tagged + - image: postgres:1.8.0 + name: postgresdb +`, + replacements: `replacements: +- source: + group: my-group-2 + fieldPath: spec.template.spec.containers.0.image + targets: + - select: + version: v3 + fieldPaths: + - spec.template.spec.containers.1.image +`, + expected: `apiVersion: my-group-1/v1 +kind: Custom +metadata: + name: my-name +spec: + template: + spec: + containers: + - image: nginx:1.7.9 + name: nginx-tagged + - image: postgres:1.8.0 + name: postgresdb +--- +apiVersion: my-group-2/v2 +kind: Custom +metadata: + name: my-name +spec: + template: + spec: + containers: + - image: nginx:1.7.9 + name: nginx-tagged + - image: postgres:1.8.0 + name: postgresdb +--- +apiVersion: my-group-3/v3 +kind: Custom +metadata: + name: my-name +spec: + template: + spec: + containers: + - image: nginx:1.7.9 + name: nginx-tagged + - image: nginx:1.7.9 + name: postgresdb +`, + }, + // regression test for missing metadata handling + "missing metadata": { + input: `spec: + template: + spec: + containers: + - image: nginx:1.7.9 + name: nginx-tagged + - image: postgres:1.8.0 + name: postgresdb +--- +apiVersion: my-group/v1 +kind: Custom +metadata: + name: my-name-1 +spec: + template: + spec: + containers: + - image: nginx:1.7.9 + name: nginx-tagged + - image: postgres:1.8.0 + name: postgresdb +--- +apiVersion: my-group/v1 +kind: Custom +metadata: + name: my-name-2 +spec: + template: + spec: + containers: + - image: nginx:1.7.9 + name: nginx-tagged + - image: postgres:1.8.0 + name: postgresdb +`, + replacements: `replacements: +- source: + name: my-name-1 + fieldPath: spec.template.spec.containers.0.image + targets: + - select: + name: my-name-2 + fieldPaths: + - spec.template.spec.containers.1.image +`, + expected: `spec: + template: + spec: + containers: + - image: nginx:1.7.9 + name: nginx-tagged + - image: postgres:1.8.0 + name: postgresdb +--- +apiVersion: my-group/v1 +kind: Custom +metadata: + name: my-name-1 +spec: + template: + spec: + containers: + - image: nginx:1.7.9 + name: nginx-tagged + - image: postgres:1.8.0 + name: postgresdb +--- +apiVersion: my-group/v1 +kind: Custom +metadata: + name: my-name-2 +spec: + template: + spec: + containers: + - image: nginx:1.7.9 + name: nginx-tagged + - image: nginx:1.7.9 + name: postgresdb +`, }, } @@ -324,11 +589,20 @@ spec: t.Run(tn, func(t *testing.T) { f := Filter{} err := yaml.Unmarshal([]byte(tc.replacements), &f) - assert.NoError(t, err) + if !assert.NoError(t, err) { + t.FailNow() + } actual, err := filtertest.RunFilterE(t, tc.input, f) - assert.Equal(t, tc.expectedErr, err != nil) - if !tc.expectedErr && - !assert.Equal(t, strings.TrimSpace(tc.expected), strings.TrimSpace(actual)) { + if err != nil { + if tc.expectedErr == "" { + t.Errorf("unexpected error: %s\n", err.Error()) + t.FailNow() + } + if !assert.Equal(t, tc.expectedErr, err.Error()) { + t.FailNow() + } + } + if !assert.Equal(t, strings.TrimSpace(tc.expected), strings.TrimSpace(actual)) { t.FailNow() } }) diff --git a/api/types/replacement.go b/api/types/replacement.go index cd9ecbb75..9af145512 100644 --- a/api/types/replacement.go +++ b/api/types/replacement.go @@ -23,7 +23,7 @@ type SourceSelector struct { // Structured field path expected in the allowed object. FieldPath string `json:"fieldPath" yaml:"fieldPath"` - // Used to refine the interpretation of the field + // Used to refine the interpretation of the field. Options *FieldOptions `json:"options" yaml:"options"` } @@ -33,20 +33,20 @@ type TargetSelector struct { Select *Selector `json:"select" yaml:"select"` // From the allowed set, remove objects that match this. - // TODO (#3492): Remove matches listed in the exclude field + // TODO (#3492): Remove matches listed in the `reject` field // Currently this field is unused Reject *Selector `json:"reject" yaml:"reject"` // Structured field paths expected in each allowed object. FieldPaths []string `json:"fieldPaths" yaml:"fieldPaths"` - // Used to refine the interpretation of the field + // Used to refine the interpretation of the field. Options *FieldOptions `json:"options" yaml:"options"` } -// FieldPath is a structured field path to the desired object +// FieldOptions refine the interpretation of FieldPaths. // TODO (#3492): Implement use of these options, they are -// currently used +// currently unused type FieldOptions struct { // Used to split/join the field. Delimiter string `json:"delimiter" yaml:"delimiter"` @@ -54,9 +54,9 @@ type FieldOptions struct { // Which position in the split to consider. Index int `json:"index" yaml:"index"` - // None, Base64, URL, Hex, etc + // None, Base64, URL, Hex, etc. Encoding string `json:"encoding" yaml:"index"` - // If field missing, add it + // If field missing, add it. Create bool `json:"create" yaml:"create"` }