diff --git a/cmd/config/internal/commands/cmdcreatesubstitution_test.go b/cmd/config/internal/commands/cmdcreatesubstitution_test.go index 86e80cbe9..bc014d9c2 100644 --- a/cmd/config/internal/commands/cmdcreatesubstitution_test.go +++ b/cmd/config/internal/commands/cmdcreatesubstitution_test.go @@ -102,6 +102,72 @@ spec: image: nginx:1.7.9 # {"$ref":"#/definitions/io.k8s.cli.substitutions.image"} - name: sidecar image: sidecar:1.7.9 + `, + }, + { + name: "substitution and create setters 1", + args: []string{ + "image", "something/nginx::1.7.9/nginxotherthing", "--pattern", "something/IMAGE::TAG/nginxotherthing", + "--value", "IMAGE=image", "--value", "TAG=tag"}, + input: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + replicas: 3 + template: + spec: + containers: + - name: nginx + image: something/nginx::1.7.9/nginxotherthing + - name: sidecar + image: sidecar:1.7.9 + `, + inputOpenAPI: ` +apiVersion: v1alpha1 +kind: Example + `, + expectedOpenAPI: ` +apiVersion: v1alpha1 +kind: Example +openAPI: + definitions: + io.k8s.cli.setters.image: + x-k8s-cli: + setter: + name: image + value: nginx + io.k8s.cli.setters.tag: + x-k8s-cli: + setter: + name: tag + value: 1.7.9 + io.k8s.cli.substitutions.image: + x-k8s-cli: + substitution: + name: image + pattern: something/IMAGE::TAG/nginxotherthing + values: + - marker: IMAGE + ref: '#/definitions/io.k8s.cli.setters.image' + - marker: TAG + ref: '#/definitions/io.k8s.cli.setters.tag' + `, + expectedResources: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + replicas: 3 + template: + spec: + containers: + - name: nginx + image: something/nginx::1.7.9/nginxotherthing # {"$ref":"#/definitions/io.k8s.cli.substitutions.image"} + - name: sidecar + image: sidecar:1.7.9 `, }, } diff --git a/kyaml/go.mod b/kyaml/go.mod index de2170cd8..33eefaad1 100644 --- a/kyaml/go.mod +++ b/kyaml/go.mod @@ -9,6 +9,7 @@ require ( github.com/sergi/go-diff v1.1.0 github.com/stretchr/testify v1.4.0 github.com/xlab/treeprint v0.0.0-20181112141820-a009c3971eca - gopkg.in/yaml.v2 v2.2.4 - gopkg.in/yaml.v3 v3.0.0-20191026110619-0b21df46bc1d + golang.org/x/net v0.0.0-20191004110552-13f9640d40b9 // indirect + gopkg.in/yaml.v2 v2.2.7 + gopkg.in/yaml.v3 v3.0.0-20191120175047-4206685974f2 ) diff --git a/kyaml/go.sum b/kyaml/go.sum index 98d3f4c1c..05b1b96aa 100644 --- a/kyaml/go.sum +++ b/kyaml/go.sum @@ -39,18 +39,20 @@ golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACk golang.org/x/net v0.0.0-20190613194153-d28f0bde5980/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20190827160401-ba9fcec4b297 h1:k7pJ2yAPLPgbskkFdhRCsA77k2fySZ1zf2zCjvQCiIM= golang.org/x/net v0.0.0-20190827160401-ba9fcec4b297/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= +golang.org/x/net v0.0.0-20191004110552-13f9640d40b9 h1:rjwSpXsdiK0dV8/Naq3kAw9ymfAeJIyd0upUIElB+lI= +golang.org/x/net v0.0.0-20191004110552-13f9640d40b9/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.2 h1:tW2bmiBqwgJj/UpqtC8EpXEZVYOwU0yG4iWbprSVAcs= golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33q108Sa+fhmuc+sWQYwY= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 h1:YR8cESwS4TdDjEe65xsg0ogRM/Nc3DYOhEAlW+xobZo= gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= -gopkg.in/yaml.v2 v2.2.4 h1:/eiJrUcujPVeJ3xlSWaiNi3uSVmDGBK1pDHUHAnao1I= gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= -gopkg.in/yaml.v3 v3.0.0-20191026110619-0b21df46bc1d h1:LCPbGQ34PMrwad11aMZ+dbz5SAsq/0ySjRwQ8I9Qwd8= -gopkg.in/yaml.v3 v3.0.0-20191026110619-0b21df46bc1d/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +gopkg.in/yaml.v2 v2.2.7 h1:VUgggvou5XRW9mHwD/yXxIYSMtY0zoKQf/v226p2nyo= +gopkg.in/yaml.v2 v2.2.7/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= +gopkg.in/yaml.v3 v3.0.0-20191120175047-4206685974f2 h1:XZx7nhd5GMaZpmDaEHFVafUZC7ya0fuo7cSJ3UCKYmM= +gopkg.in/yaml.v3 v3.0.0-20191120175047-4206685974f2/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/kyaml/setters2/settersutil/substitutioncreator.go b/kyaml/setters2/settersutil/substitutioncreator.go index 094f34d1c..fc187c63e 100644 --- a/kyaml/setters2/settersutil/substitutioncreator.go +++ b/kyaml/setters2/settersutil/substitutioncreator.go @@ -4,9 +4,13 @@ package settersutil import ( + "strings" + + "sigs.k8s.io/kustomize/kyaml/errors" "sigs.k8s.io/kustomize/kyaml/kio" "sigs.k8s.io/kustomize/kyaml/openapi" "sigs.k8s.io/kustomize/kyaml/setters2" + "sigs.k8s.io/kustomize/kyaml/yaml" ) // SubstitutionCreator creates or updates a substitution in the OpenAPI definitions, and @@ -40,6 +44,12 @@ func (c SubstitutionCreator) Create(openAPIPath, resourcesPath string) error { Values: c.Values, Pattern: c.Pattern, } + + err := c.CreateSettersForSubstitution(openAPIPath) + if err != nil { + return err + } + if err := d.AddToFile(openAPIPath); err != nil { return err } @@ -62,3 +72,187 @@ func (c SubstitutionCreator) Create(openAPIPath, resourcesPath string) error { Outputs: []kio.Writer{inout}, }.Execute() } + +// CreateSettersForSubstitution creates the setters for all the references in the substitution +// values if they don't already exist in openAPIPath file. +func (c SubstitutionCreator) CreateSettersForSubstitution(openAPIPath string) error { + y, err := yaml.ReadFile(openAPIPath) + if err != nil { + return err + } + + m, err := c.GetValuesForMarkers() + if err != nil { + return err + } + + // for each ref in values, check if the setter already exists, if not create them + for _, value := range c.Values { + obj, err := y.Pipe(yaml.Lookup( + // get the setter key from ref. Ex: from #/definitions/io.k8s.cli.setters.image_setter + // extract io.k8s.cli.setters.image_setter + "openAPI", "definitions", strings.TrimPrefix(value.Ref, "#/definitions/"))) + + if err != nil { + return err + } + + if obj == nil { + sd := setters2.SetterDefinition{ + // get the setter name from ref. Ex: from #/definitions/io.k8s.cli.setters.image_setter + // extract image_setter + Name: strings.TrimPrefix(value.Ref, "#/definitions/io.k8s.cli.setters."), + Value: m[value.Marker], + } + err := sd.AddToFile(openAPIPath) + if err != nil { + return err + } + } + } + return nil +} + +// GetValuesForMarkers parses the pattern and field value to derive values for the +// markers in the pattern string. Returns error if the marker values can't be derived +func (c SubstitutionCreator) GetValuesForMarkers() (map[string]string, error) { + m := make(map[string]string) + indices, err := c.GetStartIndices() + if err != nil { + return nil, err + } + fv := c.FieldValue + pattern := c.Pattern + fvInd := 0 + patternInd := 0 + // iterate fv, pattern with indices fvInd, patternInd respectively and when patternInd hits the index of a marker, + // freeze patternInd and iterate fvInd and capture string till we find the substring just after current marker + // and before next marker + + // Ex: fv = "something/ubuntu:0.1.0", pattern = "something/IMAGE:VERSION", till patternInd reaches 10 + // just proceed fvInd and patternInd and check if fv[fvInd]==pattern[patternInd] when patternInd is 10, + // freeze patternInd and move fvInd till it sees substring ':' which derives IMAGE = ubuntu and so on. + for fvInd < len(fv) && patternInd < len(pattern) { + // if we hit marker index, extract its corresponding value + if marker, ok := indices[patternInd]; ok { + // increment the patternInd to end of marker. This helps us to extract the substring before next marker. + patternInd += len(marker) + var value string + if value, fvInd, err = c.extractValueForMarker(fvInd, fv, patternInd, indices); err != nil { + return nil, err + } + // if marker is repeated in the pattern, make sure that the corresponding values + // are same and throw error if not. + if prevValue, ok := m[marker]; ok && prevValue != value { + return nil, errors.Errorf( + "marker %s is found to have different values %s and %s", marker, prevValue, value) + } + m[marker] = value + } else { + // Ex: fv = "samething/ubuntu:0.1.0" pattern = "something/IMAGE:VERSION". Error out at 'a' in fv. + if fv[fvInd] != pattern[patternInd] { + return nil, errors.Errorf( + "unable to derive values for markers, " + + "create setters for all markers and then try again") + } + fvInd++ + patternInd++ + } + } + // check if both strings are completely visited or throw error + if fvInd < len(fv) || patternInd < len(pattern) { + return nil, errors.Errorf( + "unable to derive values for markers, " + + "create setters for all markers and then try again") + } + return m, nil +} + +// GetStartIndices returns the start indices of all the markers in the pattern +func (c SubstitutionCreator) GetStartIndices() (map[int]string, error) { + indices := make(map[int]string) + for _, value := range c.Values { + found := false + for i := range c.Pattern { + if strings.HasPrefix(c.Pattern[i:], value.Marker) { + indices[i] = value.Marker + found = true + } + } + if !found { + return nil, errors.Errorf("unable to find marker " + value.Marker + " in the pattern") + } + } + if err := validateMarkers(indices); err != nil { + return nil, err + } + return indices, nil +} + +// validateMarkers takes the indices map, checks if any of 2 markers not have delimiters, +// checks if any marker is substring of other and returns error +func validateMarkers(indices map[int]string) error { + for k1, v1 := range indices { + for k2, v2 := range indices { + if k1 != k2 && k1+len(v1) == k2 { + return errors.Errorf( + "markers %s and %s are found to have no delimiters between them,"+ + " pre-create setters and try again", v1, v2) + } + if v1 != v2 && strings.Contains(v1, v2) { + return errors.Errorf( + "markers %s is substring of %s,"+ + " no marker should be substring of other", v2, v1) + } + } + } + return nil +} + +// extractValueForMarker returns the value string for a marker and the incremented index +func (c SubstitutionCreator) extractValueForMarker(fvInd int, fv string, patternInd int, indices map[int]string) (string, int, error) { + nonMarkerStr := strTillNextMarker(indices, patternInd, c.Pattern) + + // return the remaining string of fv till end if patternInd is at end of pattern + if patternInd == len(c.Pattern) { + return fv[fvInd:], len(fv), nil + } + + // split remaining fv starting from fvInd with the non marker substring delimiter and get the first value + // In example fv = "something/ubuntu::0.1.0", pattern = "something/IMAGE::VERSION", + // split with "::" delimiter in fv which gives markerValue = ubuntu for marker IMAGE + // increment fvInd by length of extracted marker value and return fvInd + if markerValues := strings.Split(fv[fvInd:], nonMarkerStr); len(markerValues) > 0 { + return markerValues[0], fvInd + len(markerValues[0]), nil + } + + return "", -1, errors.Errorf( + "unable to derive values for markers," + + " create setters for all markers and then try again") +} + +// substrOfLen takes a string, start index and length and returns substring of given length +// or till end of string +func substrOfLen(str string, startInd int, length int) string { + return str[startInd:min(len(str), startInd+length)] +} + +// strTillNextMarker takes in the indices map, a start index and returns the substring till +// start of next marker +func strTillNextMarker(indices map[int]string, startInd int, pattern string) string { + // initialize with max value which is length of pattern + nextMarkerStartInd := len(pattern) + for ind := range indices { + if ind > startInd { + nextMarkerStartInd = min(ind-startInd, nextMarkerStartInd) + } + } + return substrOfLen(pattern, startInd, nextMarkerStartInd) +} + +func min(a int, b int) int { + if a < b { + return a + } + return b +} diff --git a/kyaml/setters2/settersutil/substitutioncreator_test.go b/kyaml/setters2/settersutil/substitutioncreator_test.go new file mode 100644 index 000000000..9c6b2951b --- /dev/null +++ b/kyaml/setters2/settersutil/substitutioncreator_test.go @@ -0,0 +1,111 @@ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package settersutil + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "sigs.k8s.io/kustomize/kyaml/errors" + "sigs.k8s.io/kustomize/kyaml/setters2" +) + +func TestGetValuesForMarkers(t *testing.T) { + var tests = []struct { + name string + pattern string + fieldValue string + markers []string + expectedError error + expectedOutput map[string]string + }{ + { + name: "positive example", + markers: []string{"IMAGE", "VERSION"}, + pattern: "something/IMAGE::VERSION/otherthing/IMAGE::VERSION/", + fieldValue: "something/nginx::0.1.0/otherthing/nginx::0.1.0/", + expectedOutput: map[string]string{"IMAGE": "nginx", "VERSION": "0.1.0"}, + }, + { + name: "marker with different values", + markers: []string{"IMAGE", "VERSION"}, + pattern: "something/IMAGE:VERSION/IMAGE", + fieldValue: "something/nginx:0.1.0/ubuntu", + expectedError: errors.Errorf("marker IMAGE is found to have different values nginx and ubuntu"), + }, + { + name: "unmatched pattern", + markers: []string{"IMAGE", "VERSION"}, + pattern: "something/IMAGE:VERSION", + fieldValue: "otherthing/nginx:0.1.0", + expectedError: errors.Errorf("unable to derive values for markers"), + }, + { + name: "unmatched pattern at the end", + markers: []string{"IMAGE", "VERSION"}, + pattern: "something/IMAGE:VERSION/abc", + fieldValue: "something/nginx:0.1.0/abcd", + expectedError: errors.Errorf("unable to derive values for markers"), + }, + { + name: "substring markers", + markers: []string{"IMAGE", "VERSION", "MAGE"}, + pattern: "something/IMAGE:VERSION/abc/MAGE", + fieldValue: "something/nginx:0.1.0/abc/ubuntu", + expectedError: errors.Errorf("no marker should be substring of other"), + }, + { + name: "markers with no delimiters", + markers: []string{"IMAGE", "VERSION"}, + pattern: "something/IMAGEVERSION/", + fieldValue: "something/nginx0.1.0/", + expectedError: errors.Errorf("no delimiters between them"), + }, + { + name: "unmatched delimiter", + markers: []string{"IMAGE", "VERSION"}, + pattern: "something/IMAGE:^VERSION/otherthing/IMAGE::VERSION/", + fieldValue: "something/nginx::0.1.0/otherthing/nginx::0.1.0/", + expectedError: errors.Errorf("unable to derive values for markers"), + }, + } + for i := range tests { + test := tests[i] + t.Run(test.name, func(t *testing.T) { + values := []setters2.Value{} + for _, marker := range test.markers { + value := setters2.Value{ + Marker: marker, + } + values = append(values, value) + } + + sc := SubstitutionCreator{ + Pattern: test.pattern, + Values: values, + FieldValue: test.fieldValue, + } + + m, err := sc.GetValuesForMarkers() + + if test.expectedError == nil { + // fail if expectedError is nil but actual error is not + if !assert.NoError(t, err) { + t.FailNow() + } + // check if all the expected markers and values are present in actual map + for k, v := range test.expectedOutput { + if val, ok := m[k]; ok { + assert.Equal(t, v, val) + } else { + t.FailNow() + } + } + } else { + //if expectedError is not nil, check for correctness of error message + assert.Contains(t, err.Error(), test.expectedError.Error()) + } + }) + } +}