From 6faff2d0316d6d8315edf600d3214d9d514b761f Mon Sep 17 00:00:00 2001 From: Phani Teja Marupaka Date: Mon, 27 Jul 2020 00:50:25 -0700 Subject: [PATCH] Fix V1 Setters and migrate to latest --- cmd/config/go.sum | 2 + .../commands/cmdcreatesubstitution.go | 38 -- kyaml/fix/fixsetters/fieldmetav1.go | 74 ++++ kyaml/fix/fixsetters/fixsetters.go | 141 ++++++ kyaml/fix/fixsetters/fixsetters_test.go | 405 ++++++++++++++++++ kyaml/fix/fixsetters/lookupupgrade.go | 115 +++++ kyaml/fix/fixsetters/lookupupgradekio.go | 81 ++++ kyaml/go.mod | 5 +- kyaml/go.sum | 9 +- kyaml/setters2/settersutil/settercreator.go | 11 + .../settersutil/substitutioncreator.go | 57 +++ 11 files changed, 893 insertions(+), 45 deletions(-) create mode 100644 kyaml/fix/fixsetters/fieldmetav1.go create mode 100644 kyaml/fix/fixsetters/fixsetters.go create mode 100644 kyaml/fix/fixsetters/fixsetters_test.go create mode 100644 kyaml/fix/fixsetters/lookupupgrade.go create mode 100644 kyaml/fix/fixsetters/lookupupgradekio.go diff --git a/cmd/config/go.sum b/cmd/config/go.sum index 865620d95..caa359f92 100644 --- a/cmd/config/go.sum +++ b/cmd/config/go.sum @@ -452,6 +452,7 @@ golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLL golang.org/x/net v0.0.0-20190812203447-cdfb69ac37fc/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20190813141303-74dc4d7220e7/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= 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/net v0.0.0-20200226121028-0de0cce0169b h1:0mm1VjtFUOIlE1SbDlwjYaDxZVDP2S5ou6y0gSgXHu8= golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= @@ -622,6 +623,7 @@ sigs.k8s.io/controller-runtime v0.4.0 h1:wATM6/m+3w8lj8FXNaO6Fs/rq/vqoOjO1Q116Z9 sigs.k8s.io/controller-runtime v0.4.0/go.mod h1:ApC79lpY3PHW9xj/w9pj+lYkLgwAAUZwfXkME1Lajns= sigs.k8s.io/kustomize v2.0.3+incompatible h1:JUufWFNlI44MdtnjUqVnvh29rR37PQFzPbLXqhyOyX0= sigs.k8s.io/kustomize v2.0.3+incompatible/go.mod h1:MkjgH3RdOWrievjo6c9T245dYlB5QeXV4WCbnt/PEpU= +sigs.k8s.io/kustomize/cmd/config v0.4.2/go.mod h1:w9l5mB/TKCMvBa8Syek1MP5dDrM2lBL05IyaEPN+PMc= sigs.k8s.io/kustomize/kyaml v0.4.0/go.mod h1:XJL84E6sOFeNrQ7CADiemc1B0EjIxHo3OhW4o1aJYNw= sigs.k8s.io/kustomize/kyaml v0.4.2 h1:9/Tb90gnThv4vgUldZOLnrT+9Esdh7+Og2UIq024Ykg= sigs.k8s.io/kustomize/kyaml v0.4.2/go.mod h1:XJL84E6sOFeNrQ7CADiemc1B0EjIxHo3OhW4o1aJYNw= diff --git a/cmd/config/internal/commands/cmdcreatesubstitution.go b/cmd/config/internal/commands/cmdcreatesubstitution.go index 2c99422c9..838273bc5 100644 --- a/cmd/config/internal/commands/cmdcreatesubstitution.go +++ b/cmd/config/internal/commands/cmdcreatesubstitution.go @@ -5,8 +5,6 @@ package commands import ( "fmt" - "regexp" - "strings" "github.com/go-openapi/spec" "github.com/spf13/cobra" @@ -14,7 +12,6 @@ import ( "sigs.k8s.io/kustomize/kyaml/errors" "sigs.k8s.io/kustomize/kyaml/fieldmeta" "sigs.k8s.io/kustomize/kyaml/openapi" - "sigs.k8s.io/kustomize/kyaml/setters2" "sigs.k8s.io/kustomize/kyaml/setters2/settersutil" ) @@ -84,40 +81,5 @@ func (r *CreateSubstitutionRunner) preRunE(c *cobra.Command, args []string) erro "substitution and setter can't have same name", r.CreateSubstitution.Name)) } - // extract setter name tokens from pattern enclosed in ${} - re := regexp.MustCompile(`\$\{([^}]*)\}`) - markers := re.FindAllString(r.CreateSubstitution.Pattern, -1) - if len(markers) == 0 { - return errors.Errorf("unable to find setter or substitution names in pattern, " + - "setter names must be enclosed in ${}") - } - - for _, marker := range markers { - name := strings.TrimSuffix(strings.TrimPrefix(marker, "${"), "}") - if name == r.CreateSubstitution.Name { - return fmt.Errorf("setters must have different name than the substitution: %s", name) - } - - ref, err := spec.NewRef(fieldmeta.DefinitionsPrefix + fieldmeta.SubstitutionDefinitionPrefix + name) - if err != nil { - return err - } - - var markerRef string - subst, _ := openapi.Resolve(&ref) - // check if the substitution exists with the marker name or fall back to creating setter - // ref with the name - if subst != nil { - markerRef = fieldmeta.DefinitionsPrefix + fieldmeta.SubstitutionDefinitionPrefix + name - } else { - markerRef = fieldmeta.DefinitionsPrefix + fieldmeta.SetterDefinitionPrefix + name - } - - r.CreateSubstitution.Values = append( - r.CreateSubstitution.Values, - setters2.Value{Marker: marker, Ref: markerRef}, - ) - } - return nil } diff --git a/kyaml/fix/fixsetters/fieldmetav1.go b/kyaml/fix/fixsetters/fieldmetav1.go new file mode 100644 index 000000000..7f794b56a --- /dev/null +++ b/kyaml/fix/fixsetters/fieldmetav1.go @@ -0,0 +1,74 @@ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package fixsetters + +import ( + "encoding/json" + "fmt" + "strings" + + "github.com/go-openapi/spec" + "sigs.k8s.io/kustomize/kyaml/errors" + "sigs.k8s.io/kustomize/kyaml/fieldmeta" + "sigs.k8s.io/kustomize/kyaml/yaml" +) + +// FieldMeta contains metadata that may be attached to fields as comments +type FieldMetaV1 struct { + Schema spec.Schema + + Extensions XKustomize +} + +type XKustomize struct { + SetBy string `yaml:"setBy,omitempty" json:"setBy,omitempty"` + PartialFieldSetters []PartialFieldSetter `yaml:"partialSetters,omitempty" json:"partialSetters,omitempty"` + FieldSetter *PartialFieldSetter `yaml:"setter,omitempty" json:"setter,omitempty"` +} + +// PartialFieldSetter defines how to set part of a field rather than the full field +// value. e.g. the tag part of an image field +type PartialFieldSetter struct { + // Name is the name of this setter. + Name string `yaml:"name" json:"name"` + + // Value is the current value that has been set. + Value string `yaml:"value" json:"value"` +} + +// UpgradeV1SetterComment reads the FieldMeta from a node and upgrade the +// setters comment to latest +func (fm *FieldMetaV1) UpgradeV1SetterComment(n *yaml.RNode) error { + // check for metadata on head and line comments + comments := []string{n.YNode().LineComment, n.YNode().HeadComment} + for _, c := range comments { + if c == "" { + continue + } + c := strings.TrimLeft(c, "#") + + if err := fm.Schema.UnmarshalJSON([]byte(c)); err != nil { + // note: don't return an error if the comment isn't a fieldmeta struct + return nil + } + + fe := fm.Schema.VendorExtensible.Extensions["x-kustomize"] + if fe == nil { + return nil + } + b, err := json.Marshal(fe) + if err != nil { + return errors.Wrap(err) + } + // delete line comment after parsing info into fieldmeta + n.YNode().HeadComment = "" + n.YNode().LineComment = "" + err = json.Unmarshal(b, &fm.Extensions) + if fm.Extensions.FieldSetter != nil { + n.YNode().LineComment = fmt.Sprintf(`{"%s":"%s"}`, fieldmeta.ShortHandRef(), fm.Extensions.FieldSetter.Name) + } + return err + } + return nil +} diff --git a/kyaml/fix/fixsetters/fixsetters.go b/kyaml/fix/fixsetters/fixsetters.go new file mode 100644 index 000000000..072fcd850 --- /dev/null +++ b/kyaml/fix/fixsetters/fixsetters.go @@ -0,0 +1,141 @@ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package fixsetters + +import ( + "os" + + "sigs.k8s.io/kustomize/kyaml/kio" + "sigs.k8s.io/kustomize/kyaml/setters2" + "sigs.k8s.io/kustomize/kyaml/setters2/settersutil" +) + +// SetterFixer fixes setters in the input package +type SetterFixer struct { + // PkgPath is path to the resource package + PkgPath string + + // OpenAPIPath is path to the openAPI file in the package + OpenAPIPath string + + // DryRun only displays the actions without performing + DryRun bool +} + +// SetterFixerV1Result holds the results of V1 setters fix +type SetterFixerV1Result struct { + // NeedFix indicates if the resource in pkgPath are on V1 version of setters + // and need to be fixed + NeedFix bool + + // CreatedSetters are setters created as part of this fix + CreatedSetters []string + + // CreatedSubst are substitutions created as part of this fix + CreatedSubst []string + + // FailedSetters are setters failed to create from current V1 setters + FailedSetters map[string]error + + // FailedSubst are substitutions failed to be created from V1 partial setters + FailedSubst map[string]error +} + +// FixSettersV1 reads the package and upgrades v1 version of setters +// to latest +func (f *SetterFixer) FixV1Setters() (SetterFixerV1Result, error) { + sfr := SetterFixerV1Result{ + FailedSetters: make(map[string]error), + FailedSubst: make(map[string]error), + } + // KrmFile need not exist for dryRun + if !f.DryRun { + _, err := os.Stat(f.OpenAPIPath) + if err != nil { + return sfr, err + } + } + + var err error + // lookup for all setters and partial setters in v1 format + // delete the v1 format comments after lookup + l := UpgradeV1Setters{} + if f.DryRun { + err = applyReadFilter(&l, f.PkgPath) + } else { + err = applyWriteFilter(&l, f.PkgPath) + } + if err != nil { + return sfr, err + } + if len(l.SetterCounts) > 0 { + sfr.NeedFix = true + } else { + return sfr, nil + } + + // for each v1 setter create the equivalent in v2, + for _, setter := range l.SetterCounts { + sd := setters2.SetterDefinition{ + Name: setter.Name, + Value: setter.Value, + Description: setter.Description, + SetBy: setter.SetBy, + Type: setter.Type, + } + var err error + if !f.DryRun { + err = sd.AddToFile(f.OpenAPIPath) + } + + if err != nil { + sfr.FailedSetters[setter.Name] = err + } else { + sfr.CreatedSetters = append(sfr.CreatedSetters, setter.Name) + } + } + + // for each group of partial setters, create equivalent substitution + for _, subst := range l.Substitutions { + sc := settersutil.SubstitutionCreator{ + Name: subst.Name, + FieldValue: subst.FieldVale, + Pattern: subst.Pattern, + ResourcesPath: f.PkgPath, + OpenAPIPath: f.OpenAPIPath, + } + var err error + if !f.DryRun { + err = applyWriteFilter(&sc, f.PkgPath) + } + if err != nil { + sfr.FailedSubst[subst.Name] = err + } else { + sfr.CreatedSubst = append(sfr.CreatedSubst, subst.Name) + } + } + + return sfr, nil +} + +func applyWriteFilter(f kio.Filter, pkgPath string) error { + rw := &kio.LocalPackageReadWriter{ + PackagePath: pkgPath, + } + return kio.Pipeline{ + Inputs: []kio.Reader{rw}, + Filters: []kio.Filter{f}, + Outputs: []kio.Writer{rw}, + }.Execute() +} + +func applyReadFilter(f kio.Filter, pkgPath string) error { + rw := &kio.LocalPackageReader{ + PackagePath: pkgPath, + } + return kio.Pipeline{ + Inputs: []kio.Reader{rw}, + Filters: []kio.Filter{f}, + }.Execute() +} diff --git a/kyaml/fix/fixsetters/fixsetters_test.go b/kyaml/fix/fixsetters/fixsetters_test.go new file mode 100644 index 000000000..f3cde88de --- /dev/null +++ b/kyaml/fix/fixsetters/fixsetters_test.go @@ -0,0 +1,405 @@ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package fixsetters + +import ( + "io/ioutil" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestFixSettersV1(t *testing.T) { + var tests = []struct { + name string + input string + err string + dryRun bool + openAPIFile string + expectedOutput string + expectedOpenAPI string + needFix bool + createdSetters []string + createdSubst []string + failedSetters map[string]error + failedSubst map[string]error + }{ + { + name: "upgrade-delete-partial-setters", + input: ` +apiVersion: install.istio.io/v1alpha2 +kind: IstioControlPlane +metadata: + cluster: "someproj/someclus" # {"type":"string","x-kustomize":{"partialSetters":[{"name":"project","value":"someproj"},{"name":"cluster","value":"someclus"}]}} +spec: + profile: asm # {"type":"string","x-kustomize":{"setter":{"name":"profile","value":"asm"}}} + cluster: "someproj/someclus" # {"type":"string","x-kustomize":{"partialSetters":[{"name":"project","value":"someproj"},{"name":"cluster","value":"someclus"}]}} + `, + + openAPIFile: `apiVersion: kustomization.dev/v1alpha1 +kind: Kustomization`, + + needFix: true, + createdSetters: []string{"cluster", "profile", "project"}, + createdSubst: []string{"subst-project-cluster"}, + failedSetters: map[string]error{}, + failedSubst: map[string]error{}, + + expectedOutput: `apiVersion: install.istio.io/v1alpha2 +kind: IstioControlPlane +metadata: + cluster: "someproj/someclus" # {"$openapi":"subst-project-cluster"} +spec: + profile: asm # {"$openapi":"profile"} + cluster: "someproj/someclus" # {"$openapi":"subst-project-cluster"} +`, + + expectedOpenAPI: `apiVersion: kustomization.dev/v1alpha1 +kind: Kustomization +openAPI: + definitions: + io.k8s.cli.setters.cluster: + type: string + x-k8s-cli: + setter: + name: cluster + value: someclus + io.k8s.cli.setters.profile: + type: string + x-k8s-cli: + setter: + name: profile + value: asm + io.k8s.cli.setters.project: + type: string + x-k8s-cli: + setter: + name: project + value: someproj + io.k8s.cli.substitutions.subst-project-cluster: + x-k8s-cli: + substitution: + name: subst-project-cluster + pattern: ${project}/${cluster} + values: + - marker: ${project} + ref: '#/definitions/io.k8s.cli.setters.project' + - marker: ${cluster} + ref: '#/definitions/io.k8s.cli.setters.cluster' +`, + }, + + { + name: "upgrade-delete-partial-setters-dryRun", + dryRun: true, + input: `apiVersion: install.istio.io/v1alpha2 +kind: IstioControlPlane +metadata: + cluster: "someproj/someclus" # {"type":"string","x-kustomize":{"partialSetters":[{"name":"project","value":"someproj"},{"name":"cluster","value":"someclus"}]}} +spec: + profile: asm # {"type":"string","x-kustomize":{"setter":{"name":"profile","value":"asm"}}} + cluster: "someproj/someclus" # {"type":"string","x-kustomize":{"partialSetters":[{"name":"project","value":"someproj"},{"name":"cluster","value":"someclus"}]}} +`, + needFix: true, + createdSetters: []string{"cluster", "profile", "project"}, + createdSubst: []string{"subst-project-cluster"}, + failedSetters: map[string]error{}, + failedSubst: map[string]error{}, + + expectedOutput: `apiVersion: install.istio.io/v1alpha2 +kind: IstioControlPlane +metadata: + cluster: "someproj/someclus" # {"type":"string","x-kustomize":{"partialSetters":[{"name":"project","value":"someproj"},{"name":"cluster","value":"someclus"}]}} +spec: + profile: asm # {"type":"string","x-kustomize":{"setter":{"name":"profile","value":"asm"}}} + cluster: "someproj/someclus" # {"type":"string","x-kustomize":{"partialSetters":[{"name":"project","value":"someproj"},{"name":"cluster","value":"someclus"}]}} +`, + }, + + { + name: "partial-setters-same-value", + input: ` +apiVersion: install.istio.io/v1alpha2 +kind: IstioControlPlane +spec: + profile: asm # {"type":"string","x-kustomize":{"setter":{"name":"profile","value":"asm"}}} + team: asm # {"type":"string","x-kustomize":{"setter":{"name":"team","value":"asm"}}} + profile-team: asm/asm # {"type":"string","x-kustomize":{"partialSetters":[{"name":"profile","value":"asm"},{"name":"team","value":"asm"}]}} + `, + + openAPIFile: `apiVersion: kustomization.dev/v1alpha1 +kind: Kustomization`, + + needFix: true, + createdSetters: []string{"profile", "team"}, + createdSubst: []string{"subst-profile-team"}, + failedSetters: map[string]error{}, + failedSubst: map[string]error{}, + + expectedOutput: `apiVersion: install.istio.io/v1alpha2 +kind: IstioControlPlane +spec: + profile: asm # {"$openapi":"profile"} + team: asm # {"$openapi":"team"} + profile-team: asm/asm # {"$openapi":"subst-profile-team"} +`, + + expectedOpenAPI: `apiVersion: kustomization.dev/v1alpha1 +kind: Kustomization +openAPI: + definitions: + io.k8s.cli.setters.profile: + type: string + x-k8s-cli: + setter: + name: profile + value: asm + io.k8s.cli.setters.team: + type: string + x-k8s-cli: + setter: + name: team + value: asm + io.k8s.cli.substitutions.subst-profile-team: + x-k8s-cli: + substitution: + name: subst-profile-team + pattern: ${profile}/${team} + values: + - marker: ${profile} + ref: '#/definitions/io.k8s.cli.setters.profile' + - marker: ${team} + ref: '#/definitions/io.k8s.cli.setters.team' +`, + }, + + { + name: "upgrade-with-both-versions", + + input: ` +apiVersion: install.istio.io/v1alpha2 +kind: IstioControlPlane +metadata: + clusterName: "project-id/us-east1-d/cluster-name" +spec: + profile: asm # {"type":"string","x-kustomize":{"setter":{"name":"profilesetter","value":"asm"}}} + hub: gcr.io/asm-testing # {"$openapi":"hubsetter"} + `, + + openAPIFile: `apiVersion: kustomization.dev/v1alpha1 +kind: Kustomization +openAPI: + definitions: + io.k8s.cli.setters.hubsetter: + type: string + x-k8s-cli: + setter: + name: hubsetter + value: gcr.io/asm-testing`, + + needFix: true, + createdSetters: []string{"profilesetter"}, + failedSetters: map[string]error{}, + failedSubst: map[string]error{}, + + expectedOutput: `apiVersion: install.istio.io/v1alpha2 +kind: IstioControlPlane +metadata: + clusterName: "project-id/us-east1-d/cluster-name" +spec: + profile: asm # {"$openapi":"profilesetter"} + hub: gcr.io/asm-testing # {"$openapi":"hubsetter"} +`, + + expectedOpenAPI: `apiVersion: kustomization.dev/v1alpha1 +kind: Kustomization +openAPI: + definitions: + io.k8s.cli.setters.hubsetter: + type: string + x-k8s-cli: + setter: + name: hubsetter + value: gcr.io/asm-testing + io.k8s.cli.setters.profilesetter: + type: string + x-k8s-cli: + setter: + name: profilesetter + value: asm +`, + }, + + { + name: "setter-already-exists", + + input: ` +apiVersion: install.istio.io/v1alpha2 +kind: IstioControlPlane +metadata: + clusterName: "project-id/us-east1-d/cluster-name" +spec: + profile: asm # {"type":"string","x-kustomize":{"setter":{"name":"profilesetter","value":"asm"}}} + hub: asm # {"$openapi":"profilesetter"} + `, + + openAPIFile: `apiVersion: kustomization.dev/v1alpha1 +kind: Kustomization +openAPI: + definitions: + io.k8s.cli.setters.profilesetter: + type: string + x-k8s-cli: + setter: + name: profilesetter + value: asm +`, + + needFix: true, + createdSetters: []string{"profilesetter"}, + failedSetters: map[string]error{}, + failedSubst: map[string]error{}, + expectedOutput: `apiVersion: install.istio.io/v1alpha2 +kind: IstioControlPlane +metadata: + clusterName: "project-id/us-east1-d/cluster-name" +spec: + profile: asm # {"$openapi":"profilesetter"} + hub: asm # {"$openapi":"profilesetter"} +`, + + expectedOpenAPI: `apiVersion: kustomization.dev/v1alpha1 +kind: Kustomization +openAPI: + definitions: + io.k8s.cli.setters.profilesetter: + type: string + x-k8s-cli: + setter: + name: profilesetter + value: asm +`, + }, + { + name: "do-not-delete-latest setters", + + input: ` +apiVersion: install.istio.io/v1alpha2 +kind: IstioControlPlane +metadata: + clusterName: "project-id/us-east1-d/cluster-name" +spec: + profile: asm # {"$openapi":"profilesetter"} + hub: gcr.io/asm-testing + `, + failedSetters: map[string]error{}, + failedSubst: map[string]error{}, + + openAPIFile: `apiVersion: kustomization.dev/v1alpha1 +kind: Kustomization +openAPI: + definitions: + io.k8s.cli.setters.profilesetter: + type: string + x-k8s-cli: + setter: + name: profilesetter + value: asm +`, + + expectedOutput: `apiVersion: install.istio.io/v1alpha2 +kind: IstioControlPlane +metadata: + clusterName: "project-id/us-east1-d/cluster-name" +spec: + profile: asm # {"$openapi":"profilesetter"} + hub: gcr.io/asm-testing +`, + + expectedOpenAPI: `apiVersion: kustomization.dev/v1alpha1 +kind: Kustomization +openAPI: + definitions: + io.k8s.cli.setters.profilesetter: + type: string + x-k8s-cli: + setter: + name: profilesetter + value: asm +`, + }, + + { + name: "no-openAPI-file-error", + input: ` +apiVersion: install.istio.io/v1alpha2 +kind: IstioControlPlane +metadata: + clusterName: "project-id/us-east1-d/cluster-name" +spec: + profile: asm # {"type":"string","x-kustomize":{"setter":{"name":"profilesetter","value":"asm"}}} + hub: gcr.io/asm-testing + `, + + err: "Krmfile:", + }, + } + for i := range tests { + test := tests[i] + t.Run(test.name, func(t *testing.T) { + openAPIFileName := "Krmfile" + + dir, err := ioutil.TempDir("", "") + if !assert.NoError(t, err) { + t.FailNow() + } + + defer os.RemoveAll(dir) + + err = ioutil.WriteFile(filepath.Join(dir, "deploy.yaml"), []byte(test.input), 0600) + if !assert.NoError(t, err) { + t.FailNow() + } + + if test.openAPIFile != "" { + err = ioutil.WriteFile(filepath.Join(dir, openAPIFileName), []byte(test.openAPIFile), 0600) + if !assert.NoError(t, err) { + t.FailNow() + } + } + sf := SetterFixer{ + PkgPath: dir, + DryRun: test.dryRun, + OpenAPIPath: filepath.Join(dir, "Krmfile"), + } + + sfr, err := sf.FixV1Setters() + if test.err == "" { + if !assert.NoError(t, err) { + t.FailNow() + } + } else { + if !assert.Contains(t, err.Error(), test.err) { + t.FailNow() + } + return + } + + if test.expectedOpenAPI != "" { + actualOpenAPI, err := ioutil.ReadFile(filepath.Join(dir, openAPIFileName)) + if !assert.NoError(t, err) { + t.FailNow() + } + assert.Equal(t, test.expectedOpenAPI, string(actualOpenAPI)) + } + assert.Equal(t, test.needFix, sfr.NeedFix) + assert.Equal(t, test.createdSetters, sfr.CreatedSetters) + assert.Equal(t, test.createdSubst, sfr.CreatedSubst) + assert.Equal(t, test.failedSubst, sfr.FailedSubst) + assert.Equal(t, test.failedSetters, sfr.FailedSetters) + }) + } +} diff --git a/kyaml/fix/fixsetters/lookupupgrade.go b/kyaml/fix/fixsetters/lookupupgrade.go new file mode 100644 index 000000000..f05119f87 --- /dev/null +++ b/kyaml/fix/fixsetters/lookupupgrade.go @@ -0,0 +1,115 @@ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package fixsetters + +import ( + "strings" + + "sigs.k8s.io/kustomize/kyaml/yaml" +) + +var _ yaml.Filter = &upgradeV1Setters{} + +// upgradeV1Setters looks up v1 setters in a Resource and upgrades +// all the setters related comments +type upgradeV1Setters struct { + // Name of the setter to lookup. Optional + Name string + + // Setters is a list of setters that were found + Setters []setter + + // Substitutions is a list of substitutions that were found + Substitutions []substitution +} + +type substitution struct { + Name string + FieldVale string + Pattern string +} + +type setter struct { + PartialFieldSetter + Description string + Type string + SetBy string + SubstName string +} + +func (ls *upgradeV1Setters) Filter(object *yaml.RNode) (*yaml.RNode, error) { + switch object.YNode().Kind { + case yaml.DocumentNode: + // skip the document node + return ls.Filter(yaml.NewRNode(object.YNode().Content[0])) + case yaml.MappingNode: + return object, object.VisitFields(func(node *yaml.MapNode) error { + return node.Value.PipeE(ls) + }) + case yaml.SequenceNode: + return object, object.VisitElements(func(node *yaml.RNode) error { + return node.PipeE(ls) + }) + case yaml.ScalarNode: + return object, ls.lookupAndUpgrade(object) + default: + return object, nil + } +} + +// lookupAndUpgrade finds any setters for a field and upgrades the setters comment +func (ls *upgradeV1Setters) lookupAndUpgrade(field *yaml.RNode) error { + // check if there is a substitution for this field + var fm = FieldMetaV1{} + if err := fm.UpgradeV1SetterComment(field); err != nil { + return err + } + + if fm.Extensions.FieldSetter != nil { + if ls.Name != "" && ls.Name != fm.Extensions.FieldSetter.Name { + // skip this setter, it doesn't match the specified setter + return nil + } + // full setter + ls.Setters = append(ls.Setters, setter{ + PartialFieldSetter: *fm.Extensions.FieldSetter, + Description: fm.Schema.Description, + Type: fm.Schema.Type[0], + SetBy: fm.Extensions.SetBy, + }) + return nil + } + + if len(fm.Extensions.PartialFieldSetters) > 0 { + substName := "subst" + filedValue := field.YNode().Value + pattern := filedValue + + // derive substitution pattern from partial setters + for i := range fm.Extensions.PartialFieldSetters { + substName += "-" + fm.Extensions.PartialFieldSetters[i].Name + pattern = strings.Replace(pattern, fm.Extensions.PartialFieldSetters[i].Value, `${`+fm.Extensions.PartialFieldSetters[i].Name+"}", 1) + } + + ls.Substitutions = append(ls.Substitutions, substitution{ + Name: substName, + FieldVale: filedValue, + Pattern: pattern, + }) + } + + for i := range fm.Extensions.PartialFieldSetters { + if ls.Name != "" && ls.Name != fm.Extensions.PartialFieldSetters[i].Name { + // skip this setter + continue + } + ls.Setters = append(ls.Setters, setter{ + PartialFieldSetter: fm.Extensions.PartialFieldSetters[i], + Description: fm.Schema.Description, + Type: fm.Schema.Type[0], + SetBy: fm.Extensions.SetBy, + }) + } + return nil +} diff --git a/kyaml/fix/fixsetters/lookupupgradekio.go b/kyaml/fix/fixsetters/lookupupgradekio.go new file mode 100644 index 000000000..8f94a2cc6 --- /dev/null +++ b/kyaml/fix/fixsetters/lookupupgradekio.go @@ -0,0 +1,81 @@ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package fixsetters + +import ( + "sort" + + "sigs.k8s.io/kustomize/kyaml/kio" + "sigs.k8s.io/kustomize/kyaml/yaml" +) + +var _ kio.Filter = &UpgradeV1Setters{} + +// UpgradeV1Setters identifies setters for a collection of Resources to upgrade +type UpgradeV1Setters struct { + // Name is the name of the setter to match. Optional. + Name string + + // SetterCounts is populated by Filter and contains the count of fields matching each setter. + SetterCounts []setterCount + + // Substitutions are groups of partial setters + Substitutions []substitution +} + +// setterCount records the identified setters and number of fields matching those setters +type setterCount struct { + // Count is the number of substitutions possible to perform + Count int + + // setter is the substitution found + setter +} + +// Filter implements kio.Filter +func (l *UpgradeV1Setters) Filter(input []*yaml.RNode) ([]*yaml.RNode, error) { + setters := map[string]*setterCount{} + substitutions := map[string]*substitution{} + + for i := range input { + // lookup substitutions for this object + ls := &upgradeV1Setters{Name: l.Name} + if err := input[i].PipeE(ls); err != nil { + return nil, err + } + + // aggregate counts for each setter by name. takes the description and value from + // the first setter for each name encountered. + for j := range ls.Setters { + setter := ls.Setters[j] + curr, found := setters[setter.Name] + if !found { + curr = &setterCount{setter: setter} + setters[setter.Name] = curr + } + curr.Count++ + } + + for j := range ls.Substitutions { + subst := ls.Substitutions[j] + _, found := substitutions[subst.Name] + if !found { + substitutions[subst.Name] = &subst + } + } + } + + // pull out and sort the results by setter name + for _, v := range setters { + l.SetterCounts = append(l.SetterCounts, *v) + } + + for _, subst := range substitutions { + l.Substitutions = append(l.Substitutions, *subst) + } + sort.Slice(l.SetterCounts, func(i, j int) bool { + return l.SetterCounts[i].Name < l.SetterCounts[j].Name + }) + return input, nil +} diff --git a/kyaml/go.mod b/kyaml/go.mod index e6d96d846..6b683303b 100644 --- a/kyaml/go.mod +++ b/kyaml/go.mod @@ -11,11 +11,10 @@ require ( github.com/qri-io/starlib v0.4.2-0.20200213133954-ff2e8cd5ef8d github.com/sergi/go-diff v1.1.0 github.com/spf13/cobra v1.0.0 - github.com/spf13/pflag v1.0.3 + github.com/spf13/pflag v1.0.5 github.com/stretchr/testify v1.4.0 github.com/xlab/treeprint v0.0.0-20181112141820-a009c3971eca go.starlark.net v0.0.0-20200306205701-8dd3e2ee1dd5 - golang.org/x/net v0.0.0-20200226121028-0de0cce0169b // indirect gopkg.in/yaml.v2 v2.3.0 - gopkg.in/yaml.v3 v3.0.0-20191120175047-4206685974f2 + gopkg.in/yaml.v3 v3.0.0-20200121175148-a6ecf24a6d71 ) diff --git a/kyaml/go.sum b/kyaml/go.sum index 095a1bc08..3f59e9e66 100644 --- a/kyaml/go.sum +++ b/kyaml/go.sum @@ -180,6 +180,8 @@ github.com/spf13/cobra v1.0.0/go.mod h1:/6GTrnGXV9HjY+aR4k0oJ5tcvakLuG6EuKReYlHN github.com/spf13/jwalterweatherman v1.0.0/go.mod h1:cQK4TGJAtQXfYWX+Ddv3mKDzgVb68N+wFjFa4jdeBTo= github.com/spf13/pflag v1.0.3 h1:zPAT6CGy6wXeQ7NtTnaTerfKOsV6V6F8agHXFiazDkg= 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/spf13/viper v1.4.0/go.mod h1:PTJ7Z/lr49W6bUbkmS1V3by4uWynFiR9p7+dSq/yZzE= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= @@ -226,9 +228,8 @@ golang.org/x/net v0.0.0-20190320064053-1272bf9dcd53/go.mod h1:t9HGtf8HONx5eT2rtn golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190522155817-f3200d17e092/go.mod h1:HSz+uSET+XFnRR8LxR5pz3Of3rY3CfYBVs4xY44aLks= 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-20200226121028-0de0cce0169b h1:0mm1VjtFUOIlE1SbDlwjYaDxZVDP2S5ou6y0gSgXHu8= -golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= @@ -270,6 +271,6 @@ gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.3.0 h1:clyUAQHOM3G0M3f5vQj7LuJrETvjVot3Z5el9nffUtU= gopkg.in/yaml.v2 v2.3.0/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= +gopkg.in/yaml.v3 v3.0.0-20200121175148-a6ecf24a6d71 h1:Xe2gvTZUJpsvOWUnvmL/tmhVBZUmHSvLbMjRj6NUUKo= +gopkg.in/yaml.v3 v3.0.0-20200121175148-a6ecf24a6d71/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= diff --git a/kyaml/setters2/settersutil/settercreator.go b/kyaml/setters2/settersutil/settercreator.go index 7f123e1c7..24c3b57ca 100644 --- a/kyaml/setters2/settersutil/settercreator.go +++ b/kyaml/setters2/settersutil/settercreator.go @@ -11,6 +11,7 @@ import ( "sigs.k8s.io/kustomize/kyaml/kio" "sigs.k8s.io/kustomize/kyaml/openapi" "sigs.k8s.io/kustomize/kyaml/setters2" + "sigs.k8s.io/kustomize/kyaml/yaml" ) // SetterCreator creates or updates a setter in the OpenAPI definitions, and inserts references @@ -43,6 +44,16 @@ type SetterCreator struct { // live apply/preview. This field is added to the setter definition to record // the package publisher's intent to make the setter required to be set. Required bool + + // Path to openAPI file + OpenAPIPath string + + // Path to resources folder + ResourcesPath string +} + +func (c *SetterCreator) Filter(input []*yaml.RNode) ([]*yaml.RNode, error) { + return nil, c.Create(c.OpenAPIPath, c.ResourcesPath) } func (c SetterCreator) Create(openAPIPath, resourcesPath string) error { diff --git a/kyaml/setters2/settersutil/substitutioncreator.go b/kyaml/setters2/settersutil/substitutioncreator.go index a48905ebd..1aa49e9db 100644 --- a/kyaml/setters2/settersutil/substitutioncreator.go +++ b/kyaml/setters2/settersutil/substitutioncreator.go @@ -7,6 +7,7 @@ import ( "fmt" "io/ioutil" "os" + "regexp" "strings" "github.com/go-openapi/spec" @@ -42,9 +43,24 @@ type SubstitutionCreator struct { // FieldValue if set will add the OpenAPI reference to fields if they have this value. // Optional. If unspecified match all field values. FieldValue string + + // Path to openAPI file + OpenAPIPath string + + // Path to resources folder + ResourcesPath string +} + +func (c *SubstitutionCreator) Filter(input []*yaml.RNode) ([]*yaml.RNode, error) { + return nil, c.Create(c.OpenAPIPath, c.ResourcesPath) } func (c SubstitutionCreator) Create(openAPIPath, resourcesPath string) error { + values, err := markersAndRefs(c.Name, c.Pattern) + if err != nil { + return err + } + c.Values = values d := setters2.SubstitutionDefinition{ Name: c.Name, Values: c.Values, @@ -121,6 +137,47 @@ func (c SubstitutionCreator) Create(openAPIPath, resourcesPath string) error { }.Execute() } +// createMarkersAndRefs takes the input pattern, creates setter/substitution markers +// and corresponding openAPI refs +func markersAndRefs(substName, pattern string) ([]setters2.Value, error) { + var values []setters2.Value + // extract setter name tokens from pattern enclosed in ${} + re := regexp.MustCompile(`\$\{([^}]*)\}`) + markers := re.FindAllString(pattern, -1) + if len(markers) == 0 { + return nil, errors.Errorf("unable to find setter or substitution names in pattern, " + + "setter names must be enclosed in ${}") + } + + for _, marker := range markers { + name := strings.TrimSuffix(strings.TrimPrefix(marker, "${"), "}") + if name == substName { + return nil, fmt.Errorf("setters must have different name than the substitution: %s", name) + } + + ref, err := spec.NewRef(fieldmeta.DefinitionsPrefix + fieldmeta.SubstitutionDefinitionPrefix + name) + if err != nil { + return nil, err + } + + var markerRef string + subst, _ := openapi.Resolve(&ref) + // check if the substitution exists with the marker name or fall back to creating setter + // ref with the name + if subst != nil { + markerRef = fieldmeta.DefinitionsPrefix + fieldmeta.SubstitutionDefinitionPrefix + name + } else { + markerRef = fieldmeta.DefinitionsPrefix + fieldmeta.SetterDefinitionPrefix + name + } + + values = append( + values, + setters2.Value{Marker: marker, Ref: markerRef}, + ) + } + return values, nil +} + // 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 {