From f432f4d75e82bb588336d9308ed75213651ea684 Mon Sep 17 00:00:00 2001 From: Phani Teja Marupaka Date: Thu, 27 Aug 2020 21:22:43 -0700 Subject: [PATCH] Setters with subpackages --- cmd/config/go.mod | 2 +- .../internal/commands/cmdcreatesetter.go | 97 ++++++++------ .../internal/commands/cmdcreatesetter_test.go | 122 +++++++++++++++--- .../commands/cmdcreatesubstitution.go | 85 ++++++------ .../commands/cmdcreatesubstitution_test.go | 116 ++++++++++++++--- .../internal/commands/cmdlistsetters.go | 11 +- .../internal/commands/cmdlistsetters_test.go | 11 +- cmd/config/internal/commands/cmdset.go | 50 ++++++- cmd/config/internal/commands/cmdset_test.go | 99 ++++++++++++-- cmd/config/internal/commands/cmdwrap_test.go | 6 +- .../commands/e2e/list_setters_test.go | 2 +- .../mysql/Krmfile | 0 .../mysql/deployment.yaml | 0 .../mysql/nosetters/Krmfile | 0 .../mysql/nosetters/deployment.yaml | 0 .../mysql/storage/Krmfile | 0 .../mysql/storage/deployment.yaml | 0 .../dataset-without-setters/mysql/Krmfile | 6 + .../mysql/deployment.yaml | 15 +++ .../mysql/storage/Krmfile | 6 + .../mysql/storage/deployment.yaml | 15 +++ kustomize/go.mod | 2 + kustomize/go.sum | 7 + kyaml/kio/ignorefilesmatcher.go | 12 +- kyaml/kio/ignorefilesmatcher_test.go | 2 +- kyaml/kio/pkgio_reader.go | 7 +- kyaml/openapi/openapi.go | 35 +++++ kyaml/openapi/openapi_test.go | 76 +++++++++++ kyaml/pathutil/pathutil.go | 24 ++-- kyaml/pathutil/pathutil_test.go | 59 +++++---- kyaml/setters2/set.go | 4 +- kyaml/setters2/set_test.go | 2 +- kyaml/setters2/settersutil/fieldsetter.go | 20 +-- kyaml/setters2/settersutil/settercreator.go | 53 ++++++-- .../settersutil/substitutioncreator.go | 59 +++++++-- 35 files changed, 795 insertions(+), 210 deletions(-) rename cmd/config/internal/commands/test/testdata/{dataset1 => dataset-with-setters}/mysql/Krmfile (100%) rename cmd/config/internal/commands/test/testdata/{dataset1 => dataset-with-setters}/mysql/deployment.yaml (100%) rename cmd/config/internal/commands/test/testdata/{dataset1 => dataset-with-setters}/mysql/nosetters/Krmfile (100%) rename cmd/config/internal/commands/test/testdata/{dataset1 => dataset-with-setters}/mysql/nosetters/deployment.yaml (100%) rename cmd/config/internal/commands/test/testdata/{dataset1 => dataset-with-setters}/mysql/storage/Krmfile (100%) rename cmd/config/internal/commands/test/testdata/{dataset1 => dataset-with-setters}/mysql/storage/deployment.yaml (100%) create mode 100644 cmd/config/internal/commands/test/testdata/dataset-without-setters/mysql/Krmfile create mode 100644 cmd/config/internal/commands/test/testdata/dataset-without-setters/mysql/deployment.yaml create mode 100644 cmd/config/internal/commands/test/testdata/dataset-without-setters/mysql/storage/Krmfile create mode 100644 cmd/config/internal/commands/test/testdata/dataset-without-setters/mysql/storage/deployment.yaml diff --git a/cmd/config/go.mod b/cmd/config/go.mod index 2d2c3e405..38ecb9580 100644 --- a/cmd/config/go.mod +++ b/cmd/config/go.mod @@ -18,4 +18,4 @@ require ( sigs.k8s.io/kustomize/kyaml v0.7.1 ) -replace sigs.k8s.io/kustomize/kyaml v0.7.1 => ../../kyaml +replace sigs.k8s.io/kustomize/kyaml => ../../kyaml diff --git a/cmd/config/internal/commands/cmdcreatesetter.go b/cmd/config/internal/commands/cmdcreatesetter.go index 42ad37349..38df7f96c 100644 --- a/cmd/config/internal/commands/cmdcreatesetter.go +++ b/cmd/config/internal/commands/cmdcreatesetter.go @@ -5,7 +5,9 @@ package commands import ( "encoding/json" + "fmt" "io/ioutil" + "path/filepath" "strings" "github.com/go-openapi/spec" @@ -13,9 +15,9 @@ import ( "sigs.k8s.io/kustomize/cmd/config/ext" "sigs.k8s.io/kustomize/cmd/config/internal/generateddocs/commands" "sigs.k8s.io/kustomize/kyaml/errors" - "sigs.k8s.io/kustomize/kyaml/fieldmeta" "sigs.k8s.io/kustomize/kyaml/kio" "sigs.k8s.io/kustomize/kyaml/openapi" + "sigs.k8s.io/kustomize/kyaml/pathutil" "sigs.k8s.io/kustomize/kyaml/setters" "sigs.k8s.io/kustomize/kyaml/setters2/settersutil" ) @@ -59,6 +61,8 @@ func NewCreateSetterRunner(parent string) *CreateSetterRunner { set.Flags().StringVar(&r.SchemaPath, "schema-path", "", `openAPI schema file path for setter constraints -- file content `+ `e.g. {"type": "string", "maxLength": 15, "enum": ["allowedValue1", "allowedValue2"]}`) + set.Flags().BoolVarP(&r.CreateSetter.RecurseSubPackages, "recurse-subpackages", "R", false, + "creates setter recursively in all the nested subpackages") set.Flags().MarkHidden("version") fixDocs(parent, set) r.Command = set @@ -111,42 +115,6 @@ func (r *CreateSetterRunner) preRunE(c *cobra.Command, args []string) error { } if setterVersion == "v2" { - var err error - r.OpenAPIFile, err = ext.GetOpenAPIFile(args) - if err != nil { - return err - } - - if err := openapi.AddSchemaFromFile(r.OpenAPIFile); err != nil { - return err - } - - // check if substitution with same name exists and throw error - ref, err := spec.NewRef(fieldmeta.DefinitionsPrefix + fieldmeta.SubstitutionDefinitionPrefix + r.CreateSetter.Name) - if err != nil { - return err - } - - subst, _ := openapi.Resolve(&ref) - // if substitution already exists with the input setter name, throw error - if subst != nil { - return errors.Errorf("substitution with name %s already exists, "+ - "substitution and setter can't have same name", r.CreateSetter.Name) - } - - // check if setter with same name exists and throw error - ref, err = spec.NewRef(fieldmeta.DefinitionsPrefix + fieldmeta.SetterDefinitionPrefix + r.CreateSetter.Name) - if err != nil { - return err - } - - setter, _ := openapi.Resolve(&ref) - // if setter already exists with the input setter name, throw error - if setter != nil { - return errors.Errorf("setter with name %s already exists, "+ - "if you want to modify it, please delete the existing setter and recreate it", r.CreateSetter.Name) - } - r.CreateSetter.Description = r.Set.SetPartialField.Description r.CreateSetter.SetBy = r.Set.SetPartialField.SetBy r.CreateSetter.Type = r.Set.SetPartialField.Type @@ -212,7 +180,60 @@ func (r *CreateSetterRunner) processSchema() error { func (r *CreateSetterRunner) set(c *cobra.Command, args []string) error { if setterVersion == "v2" { - return r.CreateSetter.Create(r.OpenAPIFile, args[0]) + openAPIFileName, err := ext.OpenAPIFileName() + if err != nil { + return err + } + + r.CreateSetter.OpenAPIFileName = openAPIFileName + resourcePackagesPaths, err := pathutil.DirsWithFile(args[0], openAPIFileName, r.CreateSetter.RecurseSubPackages) + if err != nil { + return err + } + + if len(resourcePackagesPaths) == 0 { + return errors.Errorf("unable to find %q in package %q", r.CreateSetter.OpenAPIFileName, args[0]) + } + for _, resourcesPath := range resourcePackagesPaths { + r.CreateSetter = settersutil.SetterCreator{ + Name: r.CreateSetter.Name, + SetBy: r.CreateSetter.SetBy, + Description: r.CreateSetter.Description, + Type: r.CreateSetter.Type, + Schema: r.CreateSetter.Schema, + FieldName: r.CreateSetter.FieldName, + FieldValue: r.CreateSetter.FieldValue, + Required: r.CreateSetter.Required, + RecurseSubPackages: r.CreateSetter.RecurseSubPackages, + OpenAPIFileName: openAPIFileName, + OpenAPIPath: filepath.Join(resourcesPath, openAPIFileName), + ResourcesPath: resourcesPath, + } + + // Add schema present in openAPI file for current package + if err := openapi.AddSchemaFromFile(r.CreateSetter.OpenAPIPath); err != nil { + return err + } + + err := r.CreateSetter.Create() + if err != nil { + // return err if there is only package + if len(resourcePackagesPaths) == 1 { + return err + } else { + // print error message and continue if there are multiple packages to set + fmt.Fprintf(c.OutOrStdout(), "%s in package %q\n", err.Error(), r.CreateSetter.ResourcesPath) + } + } else { + fmt.Fprintf(c.OutOrStdout(), "created setter %q in package %q\n", r.CreateSetter.Name, r.CreateSetter.ResourcesPath) + } + + // Delete schema present in openAPI file for current package + if err := openapi.DeleteSchemaInFile(r.CreateSetter.OpenAPIPath); err != nil { + return err + } + } + return nil } rw := &kio.LocalPackageReadWriter{PackagePath: args[0]} diff --git a/cmd/config/internal/commands/cmdcreatesetter_test.go b/cmd/config/internal/commands/cmdcreatesetter_test.go index c6822545e..048ddf721 100644 --- a/cmd/config/internal/commands/cmdcreatesetter_test.go +++ b/cmd/config/internal/commands/cmdcreatesetter_test.go @@ -7,12 +7,13 @@ import ( "bytes" "io/ioutil" "os" + "path/filepath" "strings" "testing" "github.com/stretchr/testify/assert" - "sigs.k8s.io/kustomize/cmd/config/ext" "sigs.k8s.io/kustomize/cmd/config/internal/commands" + "sigs.k8s.io/kustomize/kyaml/copyutil" "sigs.k8s.io/kustomize/kyaml/openapi" ) @@ -43,6 +44,7 @@ spec: apiVersion: v1alpha1 kind: Example `, + out: `created setter "replicas" in package "${baseDir}"`, expectedOpenAPI: ` apiVersion: v1alpha1 kind: Example @@ -81,6 +83,7 @@ spec: apiVersion: v1alpha1 kind: Example `, + out: `created setter "replicas" in package "${baseDir}"`, expectedOpenAPI: ` apiVersion: v1alpha1 kind: Example @@ -122,7 +125,7 @@ openAPI: - marker: ${my-tag-setter} ref: '#/definitions/io.k8s.cli.setters.my-tag-setter' `, - err: "substitution with name my-image already exists, substitution and setter can't have same name", + err: `substitution with name "my-image" already exists, substitution and setter can't have same name`, }, { @@ -140,7 +143,7 @@ openAPI: name: my-image value: "nginx" `, - err: "setter with name my-image already exists, if you want to modify it, please delete the existing setter and recreate it", + err: `setter with name "my-image" already exists, if you want to modify it, please delete the existing setter and recreate it`, }, { @@ -159,6 +162,7 @@ spec: apiVersion: v1alpha1 kind: Example `, + out: `created setter "replicas" in package "${baseDir}"`, expectedOpenAPI: ` apiVersion: v1alpha1 kind: Example @@ -200,6 +204,7 @@ spec: apiVersion: v1alpha1 kind: Example `, + out: `created setter replicas in package ${baseDir}`, expectedOpenAPI: ` apiVersion: v1alpha1 kind: Example @@ -254,6 +259,7 @@ spec: apiVersion: v1alpha1 kind: Example `, + out: `created setter "list" in package "${baseDir}"`, expectedOpenAPI: ` apiVersion: v1alpha1 kind: Example @@ -354,6 +360,7 @@ spec: apiVersion: v1alpha1 kind: Example `, + out: `created setter replicas in package ${baseDir}`, expectedOpenAPI: ` apiVersion: v1alpha1 kind: Example @@ -400,6 +407,7 @@ spec: apiVersion: v1alpha1 kind: Example `, + out: `created setter "replicas" in package "${baseDir}"`, expectedOpenAPI: ` apiVersion: v1alpha1 kind: Example @@ -437,6 +445,7 @@ spec: apiVersion: v1alpha1 kind: Example `, + out: `created setter "foo.bar" in package "${baseDir}"`, expectedOpenAPI: ` apiVersion: v1alpha1 kind: Example @@ -587,6 +596,7 @@ spec: apiVersion: v1alpha1 kind: Example `, + out: `created setter "replicas" in package "${baseDir}"`, expectedOpenAPI: ` apiVersion: v1alpha1 kind: Example @@ -712,6 +722,7 @@ spec: apiVersion: v1alpha1 kind: Example `, + out: `created setter "replicas" in package "${baseDir}"`, expectedOpenAPI: ` apiVersion: v1alpha1 kind: Example @@ -742,13 +753,14 @@ spec: openapi.ResetOpenAPI() defer openapi.ResetOpenAPI() - f, err := ioutil.TempFile("", "k8s-cli-") + baseDir, err := ioutil.TempDir("", "") if !assert.NoError(t, err) { t.FailNow() } - defer os.Remove(f.Name()) + defer os.RemoveAll(baseDir) - err = ioutil.WriteFile(f.Name(), []byte(test.inputOpenAPI), 0600) + f := filepath.Join(baseDir, "Krmfile") + err = ioutil.WriteFile(f, []byte(test.inputOpenAPI), 0600) if !assert.NoError(t, err) { t.FailNow() } @@ -768,13 +780,7 @@ spec: test.args = append(test.args, "--schema-path", sch.Name()) } - old := ext.GetOpenAPIFile - defer func() { ext.GetOpenAPIFile = old }() - ext.GetOpenAPIFile = func(args []string) (s string, err error) { - return f.Name(), nil - } - - r, err := ioutil.TempFile("", "k8s-cli-*.yaml") + r, err := ioutil.TempFile(baseDir, "k8s-cli-*.yaml") if !assert.NoError(t, err) { t.FailNow() } @@ -787,7 +793,7 @@ spec: runner := commands.NewCreateSetterRunner("") out := &bytes.Buffer{} runner.Command.SetOut(out) - runner.Command.SetArgs(append([]string{r.Name()}, test.args...)) + runner.Command.SetArgs(append([]string{baseDir}, test.args...)) err = runner.Command.Execute() if test.err != "" { if !assert.NotNil(t, err) { @@ -801,7 +807,14 @@ spec: t.FailNow() } - if !assert.Equal(t, test.out, out.String()) { + expectedOut := strings.Replace(test.out, "${baseDir}", baseDir, -1) + expectedNormalized := strings.Replace(expectedOut, "\\", "/", -1) + // normalize path format for windows + actualNormalized := strings.Replace( + strings.Replace(out.String(), "\\", "/", -1), + "//", "/", -1) + + if !assert.Equal(t, expectedNormalized, strings.TrimSpace(actualNormalized)) { t.FailNow() } @@ -815,7 +828,7 @@ spec: t.FailNow() } - actualOpenAPI, err := ioutil.ReadFile(f.Name()) + actualOpenAPI, err := ioutil.ReadFile(f) if !assert.NoError(t, err) { t.FailNow() } @@ -827,3 +840,80 @@ spec: }) } } + +func TestCreateSetterSubPackages(t *testing.T) { + var tests = []struct { + name string + dataset string + packagePath string + args []string + expected string + }{ + { + name: "create-setter-recurse-subpackages", + dataset: "dataset-without-setters", + args: []string{"namespace", "myspace", "-R"}, + expected: ` +created setter "namespace" in package "${baseDir}/mysql" +created setter "namespace" in package "${baseDir}/mysql/storage" +`, + }, + { + name: "create-setter-top-level-pkg-no-recurse-subpackages", + dataset: "dataset-without-setters", + packagePath: "mysql", + args: []string{"namespace", "myspace"}, + expected: `created setter "namespace" in package "${baseDir}/mysql"`, + }, + { + name: "create-setter-nested-pkg-no-recurse-subpackages", + dataset: "dataset-without-setters", + packagePath: "mysql/storage", + args: []string{"namespace", "myspace"}, + expected: `created setter "namespace" in package "${baseDir}/mysql/storage"`, + }, + { + name: "create-setter-already-exists", + dataset: "dataset-with-setters", + packagePath: "mysql", + args: []string{"namespace", "myspace", "-R"}, + expected: `setter with name "namespace" already exists, if you want to modify it, please delete the existing setter and recreate it in package "${baseDir}/mysql" +created setter "namespace" in package "${baseDir}/mysql/nosetters" +setter with name "namespace" already exists, if you want to modify it, please delete the existing setter and recreate it in package "${baseDir}/mysql/storage"`, + }, + } + for i := range tests { + test := tests[i] + t.Run(test.name, func(t *testing.T) { + // reset the openAPI afterward + openapi.ResetOpenAPI() + defer openapi.ResetOpenAPI() + sourceDir := filepath.Join("test", "testdata", test.dataset) + baseDir, err := ioutil.TempDir("", "") + if !assert.NoError(t, err) { + t.FailNow() + } + copyutil.CopyDir(sourceDir, baseDir) + defer os.RemoveAll(baseDir) + runner := commands.NewCreateSetterRunner("") + actual := &bytes.Buffer{} + runner.Command.SetOut(actual) + runner.Command.SetArgs(append([]string{filepath.Join(baseDir, test.packagePath)}, test.args...)) + err = runner.Command.Execute() + if !assert.NoError(t, err) { + t.FailNow() + } + + // normalize path format for windows + actualNormalized := strings.Replace( + strings.Replace(actual.String(), "\\", "/", -1), + "//", "/", -1) + + expected := strings.Replace(test.expected, "${baseDir}", baseDir, -1) + expectedNormalized := strings.Replace(expected, "\\", "/", -1) + if !assert.Equal(t, strings.TrimSpace(expectedNormalized), strings.TrimSpace(actualNormalized)) { + t.FailNow() + } + }) + } +} diff --git a/cmd/config/internal/commands/cmdcreatesubstitution.go b/cmd/config/internal/commands/cmdcreatesubstitution.go index a3657d76e..e7a112ce7 100644 --- a/cmd/config/internal/commands/cmdcreatesubstitution.go +++ b/cmd/config/internal/commands/cmdcreatesubstitution.go @@ -5,13 +5,13 @@ package commands import ( "fmt" + "path/filepath" - "github.com/go-openapi/spec" "github.com/spf13/cobra" "sigs.k8s.io/kustomize/cmd/config/ext" "sigs.k8s.io/kustomize/kyaml/errors" - "sigs.k8s.io/kustomize/kyaml/fieldmeta" "sigs.k8s.io/kustomize/kyaml/openapi" + "sigs.k8s.io/kustomize/kyaml/pathutil" "sigs.k8s.io/kustomize/kyaml/setters2/settersutil" ) @@ -19,10 +19,10 @@ import ( func NewCreateSubstitutionRunner(parent string) *CreateSubstitutionRunner { r := &CreateSubstitutionRunner{} cs := &cobra.Command{ - Use: "create-subst DIR NAME", - Args: cobra.ExactArgs(2), - PreRunE: r.preRunE, - RunE: r.runE, + Use: "create-subst DIR NAME", + Args: cobra.ExactArgs(2), + PreRun: r.preRun, + RunE: r.runE, } cs.Flags().StringVar(&r.CreateSubstitution.FieldName, "field", "", "name of the field to set -- e.g. --field image") @@ -30,6 +30,8 @@ func NewCreateSubstitutionRunner(parent string) *CreateSubstitutionRunner { "value of the field to create substitution for -- e.g. --field-value nginx:0.1.0") cs.Flags().StringVar(&r.CreateSubstitution.Pattern, "pattern", "", `substitution pattern -- e.g. --pattern \${my-image-setter}:\${my-tag-setter}`) + cs.Flags().BoolVarP(&r.CreateSubstitution.RecurseSubPackages, "recurse-subpackages", "R", false, + "creates substitution recursively in all the nested subpackages") _ = cs.MarkFlagRequired("pattern") _ = cs.MarkFlagRequired("field-value") fixDocs(parent, cs) @@ -49,49 +51,58 @@ type CreateSubstitutionRunner struct { } func (r *CreateSubstitutionRunner) runE(c *cobra.Command, args []string) error { - return handleError(c, r.CreateSubstitution.Create(r.OpenAPIFile, args[0])) -} - -func (r *CreateSubstitutionRunner) preRunE(c *cobra.Command, args []string) error { - var err error - r.CreateSubstitution.Name = args[1] + openAPIFileName, err := ext.OpenAPIFileName() if err != nil { return err } - r.OpenAPIFile, err = ext.GetOpenAPIFile(args) + r.CreateSubstitution.OpenAPIFileName = openAPIFileName + resourcePackagesPaths, err := pathutil.DirsWithFile(args[0], openAPIFileName, r.CreateSubstitution.RecurseSubPackages) if err != nil { return err } - if err := openapi.AddSchemaFromFile(r.OpenAPIFile); err != nil { - return err + if len(resourcePackagesPaths) == 0 { + return errors.Errorf("unable to find %q in package %q", r.CreateSubstitution.OpenAPIFileName, args[0]) } - // check if substitution with same name exists and throw error - ref, err := spec.NewRef(fieldmeta.DefinitionsPrefix + fieldmeta.SubstitutionDefinitionPrefix + r.CreateSubstitution.Name) - if err != nil { - return err - } + for _, resourcesPath := range resourcePackagesPaths { + r.CreateSubstitution = settersutil.SubstitutionCreator{ + Name: r.CreateSubstitution.Name, + FieldName: r.CreateSubstitution.FieldName, + FieldValue: r.CreateSubstitution.FieldValue, + RecurseSubPackages: r.CreateSubstitution.RecurseSubPackages, + Pattern: r.CreateSubstitution.Pattern, + OpenAPIFileName: openAPIFileName, + OpenAPIPath: filepath.Join(resourcesPath, openAPIFileName), + ResourcesPath: resourcesPath, + } - subst, _ := openapi.Resolve(&ref) - // if substitution already exists with the input substitution name, throw error - if subst != nil { - return errors.Errorf("substitution with name %s already exists", r.CreateSubstitution.Name) - } + // Add schema present in openAPI file for current package + if err := openapi.AddSchemaFromFile(r.CreateSubstitution.OpenAPIPath); err != nil { + return err + } + err := r.CreateSubstitution.Create() + if err != nil { + // return err if there is only package + if len(resourcePackagesPaths) == 1 { + return err + } else { + // print error message and continue if there are multiple packages to set + fmt.Fprintf(c.OutOrStdout(), "%s in package %q\n", err.Error(), r.CreateSubstitution.ResourcesPath) + } + } else { + fmt.Fprintf(c.OutOrStdout(), "created substitution %q in package %q\n", r.CreateSubstitution.Name, r.CreateSubstitution.ResourcesPath) + } - // check if setter with same name exists and throw error - ref, err = spec.NewRef(fieldmeta.DefinitionsPrefix + fieldmeta.SetterDefinitionPrefix + r.CreateSubstitution.Name) - if err != nil { - return err + // Delete schema present in openAPI file for current package + if err := openapi.DeleteSchemaInFile(r.CreateSubstitution.OpenAPIPath); err != nil { + return err + } } - - setter, _ := openapi.Resolve(&ref) - // if setter already exists with input substitution name, throw error - if setter != nil { - return errors.Errorf(fmt.Sprintf("setter with name %s already exists, "+ - "substitution and setter can't have same name", r.CreateSubstitution.Name)) - } - return nil } + +func (r *CreateSubstitutionRunner) preRun(c *cobra.Command, args []string) { + r.CreateSubstitution.Name = args[1] +} diff --git a/cmd/config/internal/commands/cmdcreatesubstitution_test.go b/cmd/config/internal/commands/cmdcreatesubstitution_test.go index ccc6a631a..f996488f3 100644 --- a/cmd/config/internal/commands/cmdcreatesubstitution_test.go +++ b/cmd/config/internal/commands/cmdcreatesubstitution_test.go @@ -7,12 +7,13 @@ import ( "bytes" "io/ioutil" "os" + "path/filepath" "strings" "testing" "github.com/stretchr/testify/assert" - "sigs.k8s.io/kustomize/cmd/config/ext" "sigs.k8s.io/kustomize/cmd/config/internal/commands" + "sigs.k8s.io/kustomize/kyaml/copyutil" "sigs.k8s.io/kustomize/kyaml/openapi" ) @@ -62,6 +63,7 @@ openAPI: name: my-tag-setter value: "1.7.9" `, + out: `created substitution "my-image-subst" in package "${baseDir}"`, expectedOpenAPI: ` apiVersion: v1alpha1 kind: Example @@ -123,7 +125,7 @@ openAPI: - marker: ${my-tag-setter} ref: '#/definitions/io.k8s.cli.setters.my-tag-setter' `, - err: "substitution with name my-image already exists", + err: `substitution with name "my-image" already exists`, }, { name: "error if setter with same name exists", @@ -140,7 +142,7 @@ openAPI: name: my-image value: "nginx" `, - err: "setter with name my-image already exists, substitution and setter can't have same name", + err: `setter with name "my-image" already exists, substitution and setter can't have same name`, }, { name: "substitution and create setters 1", @@ -165,6 +167,7 @@ spec: apiVersion: v1alpha1 kind: Example `, + out: `created substitution "my-image-subst" in package "${baseDir}"`, expectedOpenAPI: ` apiVersion: v1alpha1 kind: Example @@ -253,6 +256,7 @@ openAPI: - marker: ${my-tag-setter} ref: '#/definitions/io.k8s.cli.setters.my-tag-setter' `, + out: `created substitution "my-nested-subst" in package "${baseDir}"`, expectedOpenAPI: ` apiVersion: v1alpha1 kind: Example @@ -341,7 +345,7 @@ spec: substVal: prefix-1234 `, - err: "setters must have different name than the substitution: foo", + err: `setters must have different name than the substitution: foo`, }, } for i := range tests { @@ -351,22 +355,18 @@ spec: openapi.ResetOpenAPI() defer openapi.ResetOpenAPI() - f, err := ioutil.TempFile("", "k8s-cli-") + baseDir, err := ioutil.TempDir("", "") if !assert.NoError(t, err) { t.FailNow() } - defer os.Remove(f.Name()) - err = ioutil.WriteFile(f.Name(), []byte(test.inputOpenAPI), 0600) + defer os.RemoveAll(baseDir) + f := filepath.Join(baseDir, "Krmfile") + err = ioutil.WriteFile(f, []byte(test.inputOpenAPI), 0600) if !assert.NoError(t, err) { t.FailNow() } - old := ext.GetOpenAPIFile - defer func() { ext.GetOpenAPIFile = old }() - ext.GetOpenAPIFile = func(args []string) (s string, err error) { - return f.Name(), nil - } - r, err := ioutil.TempFile("", "k8s-cli-*.yaml") + r, err := ioutil.TempFile(baseDir, "k8s-cli-*.yaml") if !assert.NoError(t, err) { t.FailNow() } @@ -379,7 +379,7 @@ spec: runner := commands.NewCreateSubstitutionRunner("") out := &bytes.Buffer{} runner.Command.SetOut(out) - runner.Command.SetArgs(append([]string{r.Name()}, test.args...)) + runner.Command.SetArgs(append([]string{baseDir}, test.args...)) err = runner.Command.Execute() if test.err != "" { @@ -393,7 +393,14 @@ spec: t.FailNow() } - if !assert.Equal(t, test.out, out.String()) { + expectedOut := strings.Replace(test.out, "${baseDir}", baseDir, -1) + expectedNormalized := strings.Replace(expectedOut, "\\", "/", -1) + // normalize path format for windows + actualNormalized := strings.Replace( + strings.Replace(out.String(), "\\", "/", -1), + "//", "/", -1) + + if !assert.Equal(t, expectedNormalized, strings.TrimSpace(actualNormalized)) { t.FailNow() } @@ -407,7 +414,7 @@ spec: t.FailNow() } - actualOpenAPI, err := ioutil.ReadFile(f.Name()) + actualOpenAPI, err := ioutil.ReadFile(f) if !assert.NoError(t, err) { t.FailNow() } @@ -419,3 +426,80 @@ spec: }) } } + +func TestCreateSubstSubPackages(t *testing.T) { + var tests = []struct { + name string + dataset string + packagePath string + args []string + expected string + }{ + { + name: "create-subst-recurse-subpackages", + dataset: "dataset-without-setters", + args: []string{"image-tag", "--field-value", "mysql:1.7.9", "--pattern", "${image}:${tag}", "-R"}, + expected: ` +created substitution "image-tag" in package "${baseDir}/mysql" +created substitution "image-tag" in package "${baseDir}/mysql/storage" +`, + }, + { + name: "create-subst-top-level-pkg-no-recurse-subpackages", + dataset: "dataset-without-setters", + packagePath: "mysql", + args: []string{"image-tag", "--field-value", "mysql:1.7.9", "--pattern", "${image}:${tag}"}, + expected: `created substitution "image-tag" in package "${baseDir}/mysql"`, + }, + { + name: "create-subst-nested-pkg-no-recurse-subpackages", + dataset: "dataset-without-setters", + packagePath: "mysql/storage", + args: []string{"image-tag", "--field-value", "storage:1.7.9", "--pattern", "${image}:${tag}"}, + expected: `created substitution "image-tag" in package "${baseDir}/mysql/storage"`, + }, + { + name: "create-subst-already-exists", + dataset: "dataset-with-setters", + packagePath: "mysql", + args: []string{"image-tag", "--field-value", "mysql:1.7.9", "--pattern", "${image}:${tag}", "-R"}, + expected: `substitution with name "image-tag" already exists in package "${baseDir}/mysql" +created substitution "image-tag" in package "${baseDir}/mysql/nosetters" +created substitution "image-tag" in package "${baseDir}/mysql/storage"`, + }, + } + for i := range tests { + test := tests[i] + t.Run(test.name, func(t *testing.T) { + // reset the openAPI afterward + openapi.ResetOpenAPI() + defer openapi.ResetOpenAPI() + sourceDir := filepath.Join("test", "testdata", test.dataset) + baseDir, err := ioutil.TempDir("", "") + if !assert.NoError(t, err) { + t.FailNow() + } + copyutil.CopyDir(sourceDir, baseDir) + defer os.RemoveAll(baseDir) + runner := commands.NewCreateSubstitutionRunner("") + actual := &bytes.Buffer{} + runner.Command.SetOut(actual) + runner.Command.SetArgs(append([]string{filepath.Join(baseDir, test.packagePath)}, test.args...)) + err = runner.Command.Execute() + if !assert.NoError(t, err) { + t.FailNow() + } + + // normalize path format for windows + actualNormalized := strings.Replace( + strings.Replace(actual.String(), "\\", "/", -1), + "//", "/", -1) + + expected := strings.Replace(test.expected, "${baseDir}", baseDir, -1) + expectedNormalized := strings.Replace(expected, "\\", "/", -1) + if !assert.Equal(t, strings.TrimSpace(expectedNormalized), strings.TrimSpace(actualNormalized)) { + t.FailNow() + } + }) + } +} diff --git a/cmd/config/internal/commands/cmdlistsetters.go b/cmd/config/internal/commands/cmdlistsetters.go index a53a89a52..2080a1bb6 100644 --- a/cmd/config/internal/commands/cmdlistsetters.go +++ b/cmd/config/internal/commands/cmdlistsetters.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "os" + "path/filepath" "strings" "github.com/olekukonko/tablewriter" @@ -70,21 +71,21 @@ func (r *ListSettersRunner) runE(c *cobra.Command, args []string) error { return err } - openAPIPaths, err := pathutil.SubDirsWithFile(args[0], openAPIFileName) + resourcePaths, err := pathutil.DirsWithFile(args[0], openAPIFileName, true) if err != nil { return err } - if len(openAPIPaths) == 0 { - return errors.Errorf("unable to find %s in %s", openAPIFileName, args[0]) + if len(resourcePaths) == 0 { + return errors.Errorf("unable to find %s in package %s", openAPIFileName, args[0]) } // list setters for all the subpackages with openAPI file paths - for _, openAPIPath := range openAPIPaths { + for _, resourcePath := range resourcePaths { r.List = setters2.List{ Name: r.List.Name, OpenAPIFileName: openAPIFileName, } - resourcePath := strings.TrimSuffix(openAPIPath, openAPIFileName) + openAPIPath := filepath.Join(resourcePath, openAPIFileName) fmt.Fprintf(c.OutOrStdout(), "%s\n", resourcePath) if err := r.ListSetters(c, openAPIPath, resourcePath); err != nil { return err diff --git a/cmd/config/internal/commands/cmdlistsetters_test.go b/cmd/config/internal/commands/cmdlistsetters_test.go index 0b785e68a..921b4fdcf 100644 --- a/cmd/config/internal/commands/cmdlistsetters_test.go +++ b/cmd/config/internal/commands/cmdlistsetters_test.go @@ -471,9 +471,10 @@ func TestListSettersSubPackages(t *testing.T) { }{ { name: "list-replicas", - dataset: "dataset1", + dataset: "dataset-with-setters", args: []string{"--include-subst"}, - expected: `test/testdata/dataset1/mysql/ + expected: ` +test/testdata/dataset-with-setters/mysql NAME VALUE SET BY DESCRIPTION COUNT REQUIRED image mysql 1 No namespace myspace 1 No @@ -481,9 +482,9 @@ func TestListSettersSubPackages(t *testing.T) { --------------- ----------------- -------------- SUBSTITUTION PATTERN REFERENCES image-tag ${image}:${tag} [image,tag] -test/testdata/dataset1/mysql/nosetters/ +test/testdata/dataset-with-setters/mysql/nosetters NAME VALUE SET BY DESCRIPTION COUNT REQUIRED -test/testdata/dataset1/mysql/storage/ +test/testdata/dataset-with-setters/mysql/storage NAME VALUE SET BY DESCRIPTION COUNT REQUIRED namespace myspace 1 No `, @@ -509,7 +510,7 @@ test/testdata/dataset1/mysql/storage/ // normalize path format for windows actualNormalized := strings.Replace(actual.String(), "\\", "/", -1) - if !assert.Equal(t, test.expected, actualNormalized) { + if !assert.Equal(t, strings.TrimSpace(test.expected), strings.TrimSpace(actualNormalized)) { t.FailNow() } }) diff --git a/cmd/config/internal/commands/cmdset.go b/cmd/config/internal/commands/cmdset.go index 6d7143b7a..d3c106845 100644 --- a/cmd/config/internal/commands/cmdset.go +++ b/cmd/config/internal/commands/cmdset.go @@ -6,6 +6,7 @@ package commands import ( "fmt" "os" + "path/filepath" "github.com/olekukonko/tablewriter" "github.com/spf13/cobra" @@ -13,6 +14,7 @@ import ( "sigs.k8s.io/kustomize/cmd/config/internal/generateddocs/commands" "sigs.k8s.io/kustomize/kyaml/errors" "sigs.k8s.io/kustomize/kyaml/kio" + "sigs.k8s.io/kustomize/kyaml/pathutil" "sigs.k8s.io/kustomize/kyaml/setters" "sigs.k8s.io/kustomize/kyaml/setters2/settersutil" ) @@ -39,6 +41,8 @@ func NewSetRunner(parent string) *SetRunner { "annotate the field with a description of its value") c.Flags().StringVar(&setterVersion, "version", "", "use this version of the setter format") + c.Flags().BoolVarP(&r.Set.RecurseSubPackages, "recurse-subpackages", "R", false, + "sets recursively in all the nested subpackages") c.Flags().MarkHidden("version") return r @@ -126,15 +130,53 @@ func (r *SetRunner) preRunE(c *cobra.Command, args []string) error { return err } } - return nil } func (r *SetRunner) runE(c *cobra.Command, args []string) error { if setterVersion == "v2" { - count, err := r.Set.Set(r.OpenAPIFile, args[0]) - fmt.Fprintf(c.OutOrStdout(), "set %d fields\n", count) - return handleError(c, err) + openAPIFileName, err := ext.OpenAPIFileName() + if err != nil { + return err + } + + r.Set.OpenAPIFileName = openAPIFileName + resourcePackagesPaths, err := pathutil.DirsWithFile(args[0], openAPIFileName, r.Set.RecurseSubPackages) + if err != nil { + return err + } + + if len(resourcePackagesPaths) == 0 { + return errors.Errorf("unable to find %q in package %q", r.Set.OpenAPIFileName, args[0]) + } + + for _, resourcesPath := range resourcePackagesPaths { + r.Set = settersutil.FieldSetter{ + Name: r.Set.Name, + Value: r.Set.Value, + ListValues: r.Set.ListValues, + Description: r.Set.Description, + SetBy: r.Set.SetBy, + Count: 0, + OpenAPIPath: filepath.Join(resourcesPath, openAPIFileName), + OpenAPIFileName: openAPIFileName, + ResourcesPath: resourcesPath, + RecurseSubPackages: r.Set.RecurseSubPackages, + } + count, err := r.Set.Set() + if err != nil { + // return err if there is only package + if len(resourcePackagesPaths) == 1 { + return err + } else { + // print error message and continue if there are multiple packages to set + fmt.Fprintf(c.OutOrStdout(), "%s in package %q\n", err.Error(), r.Set.ResourcesPath) + } + } else { + fmt.Fprintf(c.OutOrStdout(), "set %d fields in package %q\n", count, r.Set.ResourcesPath) + } + } + return nil } if len(args) > 2 || c.Flag("values").Changed { return handleError(c, r.perform(c, args)) diff --git a/cmd/config/internal/commands/cmdset_test.go b/cmd/config/internal/commands/cmdset_test.go index 8340f464c..7681ff804 100644 --- a/cmd/config/internal/commands/cmdset_test.go +++ b/cmd/config/internal/commands/cmdset_test.go @@ -7,12 +7,13 @@ import ( "bytes" "io/ioutil" "os" + "path/filepath" "strings" "testing" "github.com/stretchr/testify/assert" - "sigs.k8s.io/kustomize/cmd/config/ext" "sigs.k8s.io/kustomize/cmd/config/internal/commands" + "sigs.k8s.io/kustomize/kyaml/copyutil" "sigs.k8s.io/kustomize/kyaml/openapi" ) @@ -1013,22 +1014,19 @@ spec: openapi.ResetOpenAPI() defer openapi.ResetOpenAPI() - f, err := ioutil.TempFile("", "k8s-cli-") + baseDir, err := ioutil.TempDir("", "") if !assert.NoError(t, err) { t.FailNow() } - defer os.Remove(f.Name()) - err = ioutil.WriteFile(f.Name(), []byte(test.inputOpenAPI), 0600) + defer os.RemoveAll(baseDir) + + f := filepath.Join(baseDir, "Krmfile") + err = ioutil.WriteFile(f, []byte(test.inputOpenAPI), 0600) if !assert.NoError(t, err) { t.FailNow() } - old := ext.GetOpenAPIFile - defer func() { ext.GetOpenAPIFile = old }() - ext.GetOpenAPIFile = func(args []string) (s string, err error) { - return f.Name(), nil - } - r, err := ioutil.TempFile("", "k8s-cli-*.yaml") + r, err := ioutil.TempFile(baseDir, "k8s-cli-*.yaml") if !assert.NoError(t, err) { t.FailNow() } @@ -1041,7 +1039,7 @@ spec: runner := commands.NewSetRunner("") out := &bytes.Buffer{} runner.Command.SetOut(out) - runner.Command.SetArgs(append([]string{r.Name()}, test.args...)) + runner.Command.SetArgs(append([]string{baseDir}, test.args...)) err = runner.Command.Execute() if test.errMsg != "" { if !assert.NotNil(t, err) { @@ -1056,7 +1054,7 @@ spec: t.FailNow() } - if test.errMsg == "" && !assert.Equal(t, test.out, out.String()) { + if test.errMsg == "" && !assert.Contains(t, out.String(), strings.TrimSpace(test.out)) { t.FailNow() } @@ -1070,7 +1068,7 @@ spec: t.FailNow() } - actualOpenAPI, err := ioutil.ReadFile(f.Name()) + actualOpenAPI, err := ioutil.ReadFile(f) if !assert.NoError(t, err) { t.FailNow() } @@ -1082,3 +1080,78 @@ spec: }) } } + +func TestSetSubPackages(t *testing.T) { + var tests = []struct { + name string + dataset string + packagePath string + args []string + expected string + }{ + { + name: "set-recurse-subpackages", + dataset: "dataset-with-setters", + args: []string{"namespace", "otherspace", "-R"}, + expected: ` +set 1 fields in package "${baseDir}/mysql" +setter "namespace" is not found in package "${baseDir}/mysql/nosetters" +set 1 fields in package "${baseDir}/mysql/storage" +`, + }, + { + name: "set-top-level-pkg-no-recurse-subpackages", + dataset: "dataset-with-setters", + packagePath: "mysql", + args: []string{"namespace", "otherspace"}, + expected: ` +set 1 fields in package "${baseDir}/mysql" +`, + }, + { + name: "set-nested-pkg-no-recurse-subpackages", + dataset: "dataset-with-setters", + packagePath: "mysql/storage", + args: []string{"namespace", "otherspace"}, + expected: ` +set 1 fields in package "${baseDir}/mysql/storage" +`, + }, + } + for i := range tests { + test := tests[i] + t.Run(test.name, func(t *testing.T) { + // reset the openAPI afterward + openapi.ResetOpenAPI() + defer openapi.ResetOpenAPI() + sourceDir := filepath.Join("test", "testdata", test.dataset) + baseDir, err := ioutil.TempDir("", "") + if !assert.NoError(t, err) { + t.FailNow() + } + copyutil.CopyDir(sourceDir, baseDir) + defer os.RemoveAll(baseDir) + runner := commands.NewSetRunner("") + actual := &bytes.Buffer{} + runner.Command.SetOut(actual) + runner.Command.SetArgs(append([]string{filepath.Join(baseDir, test.packagePath)}, test.args...)) + err = runner.Command.Execute() + if !assert.NoError(t, err) { + t.FailNow() + } + + // normalize path format for windows + actualNormalized := strings.Replace( + strings.Replace(actual.String(), "\\", "/", -1), + "//", "/", -1) + + expected := strings.Replace(test.expected, "${baseDir}", baseDir, -1) + expectedNormalized := strings.Replace( + strings.Replace(expected, "\\", "/", -1), + "//", "/", -1) + if !assert.Equal(t, strings.TrimSpace(expectedNormalized), strings.TrimSpace(actualNormalized)) { + t.FailNow() + } + }) + } +} diff --git a/cmd/config/internal/commands/cmdwrap_test.go b/cmd/config/internal/commands/cmdwrap_test.go index 49b37d5e9..988b644fb 100644 --- a/cmd/config/internal/commands/cmdwrap_test.go +++ b/cmd/config/internal/commands/cmdwrap_test.go @@ -182,7 +182,7 @@ items: kind: Deployment metadata: name: mysql-deployment - namespace: myspace # {"$openapi":"namespace"} + namespace: myspace annotations: config.kubernetes.io/index: '0' config.kubernetes.io/path: 'config/mysql-deployment_deployment.yaml' @@ -192,7 +192,7 @@ items: spec: containers: - name: mysql - image: mysql:1.7.9 # {"$openapi":"image-tag"} + image: mysql:1.7.9 - apiVersion: apps/v1 kind: Deployment metadata: @@ -212,7 +212,7 @@ items: kind: Deployment metadata: name: storage-deployment - namespace: myspace # {"$openapi":"namespace"} + namespace: myspace annotations: config.kubernetes.io/index: '0' config.kubernetes.io/path: 'config/storage-deployment_deployment.yaml' diff --git a/cmd/config/internal/commands/e2e/list_setters_test.go b/cmd/config/internal/commands/e2e/list_setters_test.go index 0d73132a3..6f5f78fd5 100644 --- a/cmd/config/internal/commands/e2e/list_setters_test.go +++ b/cmd/config/internal/commands/e2e/list_setters_test.go @@ -34,7 +34,7 @@ openAPI: `, }, expectedStdOut: ` -./ +. NAME VALUE SET BY DESCRIPTION COUNT REQUIRED replicas 3 1 No `, diff --git a/cmd/config/internal/commands/test/testdata/dataset1/mysql/Krmfile b/cmd/config/internal/commands/test/testdata/dataset-with-setters/mysql/Krmfile similarity index 100% rename from cmd/config/internal/commands/test/testdata/dataset1/mysql/Krmfile rename to cmd/config/internal/commands/test/testdata/dataset-with-setters/mysql/Krmfile diff --git a/cmd/config/internal/commands/test/testdata/dataset1/mysql/deployment.yaml b/cmd/config/internal/commands/test/testdata/dataset-with-setters/mysql/deployment.yaml similarity index 100% rename from cmd/config/internal/commands/test/testdata/dataset1/mysql/deployment.yaml rename to cmd/config/internal/commands/test/testdata/dataset-with-setters/mysql/deployment.yaml diff --git a/cmd/config/internal/commands/test/testdata/dataset1/mysql/nosetters/Krmfile b/cmd/config/internal/commands/test/testdata/dataset-with-setters/mysql/nosetters/Krmfile similarity index 100% rename from cmd/config/internal/commands/test/testdata/dataset1/mysql/nosetters/Krmfile rename to cmd/config/internal/commands/test/testdata/dataset-with-setters/mysql/nosetters/Krmfile diff --git a/cmd/config/internal/commands/test/testdata/dataset1/mysql/nosetters/deployment.yaml b/cmd/config/internal/commands/test/testdata/dataset-with-setters/mysql/nosetters/deployment.yaml similarity index 100% rename from cmd/config/internal/commands/test/testdata/dataset1/mysql/nosetters/deployment.yaml rename to cmd/config/internal/commands/test/testdata/dataset-with-setters/mysql/nosetters/deployment.yaml diff --git a/cmd/config/internal/commands/test/testdata/dataset1/mysql/storage/Krmfile b/cmd/config/internal/commands/test/testdata/dataset-with-setters/mysql/storage/Krmfile similarity index 100% rename from cmd/config/internal/commands/test/testdata/dataset1/mysql/storage/Krmfile rename to cmd/config/internal/commands/test/testdata/dataset-with-setters/mysql/storage/Krmfile diff --git a/cmd/config/internal/commands/test/testdata/dataset1/mysql/storage/deployment.yaml b/cmd/config/internal/commands/test/testdata/dataset-with-setters/mysql/storage/deployment.yaml similarity index 100% rename from cmd/config/internal/commands/test/testdata/dataset1/mysql/storage/deployment.yaml rename to cmd/config/internal/commands/test/testdata/dataset-with-setters/mysql/storage/deployment.yaml diff --git a/cmd/config/internal/commands/test/testdata/dataset-without-setters/mysql/Krmfile b/cmd/config/internal/commands/test/testdata/dataset-without-setters/mysql/Krmfile new file mode 100644 index 000000000..54355ba7d --- /dev/null +++ b/cmd/config/internal/commands/test/testdata/dataset-without-setters/mysql/Krmfile @@ -0,0 +1,6 @@ +apiVersion: krm.dev/v1alpha1 +kind: Krmfile +metadata: + name: mysql +packageMetadata: + shortDescription: sample description \ No newline at end of file diff --git a/cmd/config/internal/commands/test/testdata/dataset-without-setters/mysql/deployment.yaml b/cmd/config/internal/commands/test/testdata/dataset-without-setters/mysql/deployment.yaml new file mode 100644 index 000000000..b9e09b5fe --- /dev/null +++ b/cmd/config/internal/commands/test/testdata/dataset-without-setters/mysql/deployment.yaml @@ -0,0 +1,15 @@ +# Copyright 2019 The Kubernetes Authors. +# SPDX-License-Identifier: Apache-2.0 + +apiVersion: apps/v1 +kind: Deployment +metadata: + namespace: myspace + name: mysql-deployment +spec: + replicas: 3 + template: + spec: + containers: + - name: mysql + image: mysql:1.7.9 diff --git a/cmd/config/internal/commands/test/testdata/dataset-without-setters/mysql/storage/Krmfile b/cmd/config/internal/commands/test/testdata/dataset-without-setters/mysql/storage/Krmfile new file mode 100644 index 000000000..71a4c6ca1 --- /dev/null +++ b/cmd/config/internal/commands/test/testdata/dataset-without-setters/mysql/storage/Krmfile @@ -0,0 +1,6 @@ +apiVersion: krm.dev/v1alpha1 +kind: Krmfile +metadata: + name: storage +packageMetadata: + shortDescription: sample description diff --git a/cmd/config/internal/commands/test/testdata/dataset-without-setters/mysql/storage/deployment.yaml b/cmd/config/internal/commands/test/testdata/dataset-without-setters/mysql/storage/deployment.yaml new file mode 100644 index 000000000..89a04b35b --- /dev/null +++ b/cmd/config/internal/commands/test/testdata/dataset-without-setters/mysql/storage/deployment.yaml @@ -0,0 +1,15 @@ +# Copyright 2019 The Kubernetes Authors. +# SPDX-License-Identifier: Apache-2.0 + +apiVersion: apps/v1 +kind: Deployment +metadata: + namespace: myspace + name: storage-deployment +spec: + replicas: 4 + template: + spec: + containers: + - name: storage + image: storage:1.7.7 diff --git a/kustomize/go.mod b/kustomize/go.mod index 47ca295ae..a6a787b22 100644 --- a/kustomize/go.mod +++ b/kustomize/go.mod @@ -22,3 +22,5 @@ exclude ( replace sigs.k8s.io/kustomize/api v0.6.0 => ../api replace sigs.k8s.io/kustomize/cmd/config v0.6.0 => ../cmd/config + +replace sigs.k8s.io/kustomize/kyaml v0.7.1 => ../kyaml diff --git a/kustomize/go.sum b/kustomize/go.sum index 5cb8bacb8..a64f1f401 100644 --- a/kustomize/go.sum +++ b/kustomize/go.sum @@ -358,6 +358,8 @@ github.com/modern-go/reflect2 v0.0.0-20180701023420-4b7aa43c6742/go.mod h1:bx2lN github.com/modern-go/reflect2 v1.0.1 h1:9f412s+6RmYXLWZSEzVVgPGK7C2PphHj5RJrvfx9AWI= github.com/modern-go/reflect2 v1.0.1/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0= github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826/go.mod h1:TaXosZuwdSHYgviHp1DAtfrULt5eUgsSMsZf+YrPgl8= +github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00 h1:n6/2gBQ3RWajuToeY6ZtZTIKv2v7ThUy5KKusIT0yc0= +github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00/go.mod h1:Pm3mSP3c5uWn86xMLZ5Sa7JB9GsEZySvHYXCTK4E9q4= github.com/mozilla/tls-observatory v0.0.0-20190404164649-a3c1b6cfecfd/go.mod h1:SrKMQvPiws7F7iqYp8/TX+IhxCYhzr6N/1yb8cwHsGk= github.com/munnerz/goautoneg v0.0.0-20120707110453-a547fc61f48d/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ= github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ= @@ -742,12 +744,17 @@ modernc.org/xc v1.0.0/go.mod h1:mRNCo0bvLjGhHO9WsyuKVU4q0ceiDDDoEeWDJHrNx8I= mvdan.cc/interfacer v0.0.0-20180901003855-c20040233aed/go.mod h1:Xkxe497xwlCKkIaQYRfC7CSLworTXY9RMqwhhCm+8Nc= mvdan.cc/lint v0.0.0-20170908181259-adc824a0674b/go.mod h1:2odslEg/xrtNQqCYg2/jCoyKnw3vv5biOc3JnIcYfL4= mvdan.cc/unparam v0.0.0-20190720180237-d51796306d8f/go.mod h1:4G1h5nDURzA3bwVMZIVpwbkw+04kSxk3rAtzlimaUJw= +sigs.k8s.io/cli-utils v0.18.0 h1:PPFUhhwKsdMiYYm1DY9lZursNWSAEj8FYgMCZKVvOkQ= +sigs.k8s.io/cli-utils v0.18.0/go.mod h1:B7KdqkSkHNIUn3cFbaR4aKUZMKtr+Benboi1w/HW/Fg= sigs.k8s.io/cli-utils v0.19.2 h1:BwidWPZ3Qop4RBOl27MU8uN/BSEZPQ8rw1mwsNofXfw= sigs.k8s.io/cli-utils v0.19.2/go.mod h1:ulIQPERYwkYksNriRknJRbGECDR/ZZROpkiThlXBtB4= sigs.k8s.io/controller-runtime v0.4.0 h1:wATM6/m+3w8lj8FXNaO6Fs/rq/vqoOjO1Q116Z9NPsg= 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.6.0 h1:03tjs3SjvsumZjabctLicNUQwYlAeUqmRk1H4JBK+wI= +sigs.k8s.io/kustomize/cmd/config v0.6.0/go.mod h1:azYDRZ/lprMhKqF4DbquWS0cfBFaInLM3LN+zuAXRDI= +sigs.k8s.io/kustomize/kyaml v0.6.0/go.mod h1:bEzbO5pN9OvlEeCLvFHo8Pu7SA26Herc2m60UeWZBdI= sigs.k8s.io/kustomize/kyaml v0.7.1 h1:Ih6SJPvfKYfZaIFWUa2YAyg/0ZSTpA3LFjR/hv7+8ao= sigs.k8s.io/kustomize/kyaml v0.7.1/go.mod h1:ne3F9JPhW2wrVaLslxBsEe6MQJQ9YK5rUutrdhBWXwI= sigs.k8s.io/structured-merge-diff v0.0.0-20190525122527-15d366b2352e/go.mod h1:wWxsB5ozmmv/SG7nM11ayaAW51xMvak/t1r0CSlcokI= diff --git a/kyaml/kio/ignorefilesmatcher.go b/kyaml/kio/ignorefilesmatcher.go index 917c1f6a8..aa7bb98ad 100644 --- a/kyaml/kio/ignorefilesmatcher.go +++ b/kyaml/kio/ignorefilesmatcher.go @@ -12,7 +12,7 @@ import ( "sigs.k8s.io/kustomize/kyaml/ext" ) -// ignoreFilesMatcher handles `.krmignore` files, which allows for ignoring +// IgnoreFilesMatcher handles `.krmignore` files, which allows for ignoring // files or folders in a package. The format of this file is a subset of the // gitignore format, with recursive patterns (like a/**/c) not supported. If a // file or folder matches any of the patterns in the .krmignore file for the @@ -30,14 +30,14 @@ import ( // package contains a pattern that ignores the directory foo, if foo is a // subpackage, it will still be included if the IncludeSubpackages property // is set to true -type ignoreFilesMatcher struct { +type IgnoreFilesMatcher struct { matchers []matcher } // readIgnoreFile checks whether there is a .krmignore file in the path, and // if it is, reads it in and turns it into a matcher. If we can't find a file, // we just add a matcher that match nothing. -func (i *ignoreFilesMatcher) readIgnoreFile(path string) error { +func (i *IgnoreFilesMatcher) readIgnoreFile(path string) error { m, err := gitignore.NewGitIgnore(filepath.Join(path, ext.GetIgnoreFileName())) if err != nil { if os.IsNotExist(err) { @@ -60,7 +60,7 @@ func (i *ignoreFilesMatcher) readIgnoreFile(path string) error { // is correct for the provided filepath. Matchers are removed once // we encounter a filepath that is not a subpath of the basepath for // the matcher. -func (i *ignoreFilesMatcher) verifyPath(path string) { +func (i *IgnoreFilesMatcher) verifyPath(path string) { for j := len(i.matchers) - 1; j >= 0; j-- { matcher := i.matchers[j] if !strings.HasPrefix(path, matcher.basePath) { @@ -72,7 +72,7 @@ func (i *ignoreFilesMatcher) verifyPath(path string) { // matchFile checks whether the file given by the provided path matches // any of the patterns in the .krmignore file for the package. -func (i *ignoreFilesMatcher) matchFile(path string) bool { +func (i *IgnoreFilesMatcher) matchFile(path string) bool { if len(i.matchers) == 0 { return false } @@ -82,7 +82,7 @@ func (i *ignoreFilesMatcher) matchFile(path string) bool { // matchFile checks whether the directory given by the provided path matches // any of the patterns in the .krmignore file for the package. -func (i *ignoreFilesMatcher) matchDir(path string) bool { +func (i *IgnoreFilesMatcher) matchDir(path string) bool { if len(i.matchers) == 0 { return false } diff --git a/kyaml/kio/ignorefilesmatcher_test.go b/kyaml/kio/ignorefilesmatcher_test.go index 981274299..c2915463f 100644 --- a/kyaml/kio/ignorefilesmatcher_test.go +++ b/kyaml/kio/ignorefilesmatcher_test.go @@ -53,7 +53,7 @@ testfile.yaml assert.FailNow(t, err.Error()) } - ignoreFilesMatcher := ignoreFilesMatcher{} + ignoreFilesMatcher := IgnoreFilesMatcher{} err = ignoreFilesMatcher.readIgnoreFile(dir) if !assert.NoError(t, err) { t.FailNow() diff --git a/kyaml/kio/pkgio_reader.go b/kyaml/kio/pkgio_reader.go index 1a9077bb3..95364cb53 100644 --- a/kyaml/kio/pkgio_reader.go +++ b/kyaml/kio/pkgio_reader.go @@ -80,6 +80,7 @@ func (r *LocalPackageReadWriter) Read() ([]*yaml.RNode, error) { IncludeSubpackages: r.IncludeSubpackages, ErrorIfNonResources: r.ErrorIfNonResources, SetAnnotations: r.SetAnnotations, + PackageFileName: r.PackageFileName, }.Read() if err != nil { return nil, errors.Wrap(err) @@ -185,7 +186,7 @@ func (r LocalPackageReader) Read() ([]*yaml.RNode, error) { var operand ResourceNodeSlice var pathRelativeTo string var err error - ignoreFilesMatcher := &ignoreFilesMatcher{} + ignoreFilesMatcher := &IgnoreFilesMatcher{} r.PackagePath, err = filepath.Abs(r.PackagePath) if err != nil { return nil, errors.Wrap(err) @@ -257,7 +258,7 @@ func (r *LocalPackageReader) readFile(path string, _ os.FileInfo) ([]*yaml.RNode } // ShouldSkipFile returns true if the file should be skipped -func (r *LocalPackageReader) ShouldSkipFile(path string, matcher *ignoreFilesMatcher) (bool, error) { +func (r *LocalPackageReader) ShouldSkipFile(path string, matcher *IgnoreFilesMatcher) (bool, error) { // check if the file is covered by a .krmignore file. if matcher.matchFile(path) { return false, nil @@ -285,7 +286,7 @@ func (r *LocalPackageReader) initReaderAnnotations(path string, _ os.FileInfo) { } // ShouldSkipDir returns a filepath.SkipDir if the directory should be skipped -func (r *LocalPackageReader) ShouldSkipDir(path string, matcher *ignoreFilesMatcher) error { +func (r *LocalPackageReader) ShouldSkipDir(path string, matcher *IgnoreFilesMatcher) error { if r.PackageFileName == "" { // If the folder is not a package, but covered by the .krmignore file, // we skip it. diff --git a/kyaml/openapi/openapi.go b/kyaml/openapi/openapi.go index 3c935958b..b92321073 100644 --- a/kyaml/openapi/openapi.go +++ b/kyaml/openapi/openapi.go @@ -66,6 +66,41 @@ func AddSchemaFromFile(path string) error { return AddSchemaFromFileUsingField(path, SupplementaryOpenAPIFieldName) } +// DeleteSchemaInFile reads the file at path and removes all the openAPI definitions +// present in file from global schema +func DeleteSchemaInFile(path string) error { + b, err := ioutil.ReadFile(path) + if err != nil { + return err + } + + object, err := yaml.Parse(string(b)) + if err != nil { + return err + } + + definitions, err := object.Pipe(yaml.Lookup(SupplementaryOpenAPIFieldName, "definitions")) + if definitions == nil { + return nil + } + if err != nil { + return err + } + + fields, err := definitions.Fields() + if err != nil { + return err + } + + for _, field := range fields { + _, ok := globalSchema.schema.Definitions[field] + if ok { + delete(globalSchema.schema.Definitions, field) + } + } + return nil +} + // AddSchemaFromFileUsingField reads the file at path and parses the OpenAPI definitions // from the specified field. If field is the empty string, use the whole document as // OpenAPI. diff --git a/kyaml/openapi/openapi_test.go b/kyaml/openapi/openapi_test.go index bf0228986..766aa738b 100644 --- a/kyaml/openapi/openapi_test.go +++ b/kyaml/openapi/openapi_test.go @@ -160,6 +160,82 @@ openAPI: fmt.Sprintf("%v", s.Schema.Extensions)) } +func TestDeleteSchemaInFile(t *testing.T) { + ResetOpenAPI() + inputyaml := ` +openAPI: + definitions: + io.k8s.cli.setters.image-name: + x-k8s-cli: + setter: + name: image-name + value: "nginx" + ` + f, err := ioutil.TempFile("", "openapi-") + if !assert.NoError(t, err) { + t.FailNow() + } + if !assert.NoError(t, ioutil.WriteFile(f.Name(), []byte(inputyaml), 0600)) { + t.FailNow() + } + + err = AddSchemaFromFile(f.Name()) + if !assert.NoError(t, err) { + t.FailNow() + } + + s, err := GetSchema(`{"$ref": "#/definitions/io.k8s.cli.setters.image-name"}`) + + if !assert.NoError(t, err) { + t.FailNow() + } + if !assert.Greater(t, len(globalSchema.schema.Definitions), 200) { + t.FailNow() + } + assert.Equal(t, `map[x-k8s-cli:map[setter:map[name:image-name value:nginx]]]`, + fmt.Sprintf("%v", s.Schema.Extensions)) + + err = DeleteSchemaInFile(f.Name()) + if !assert.NoError(t, err) { + t.FailNow() + } + + _, err = GetSchema(`{"$ref": "#/definitions/io.k8s.cli.setters.image-name"}`) + + if !assert.Error(t, err) { + t.FailNow() + } + + if !assert.Equal(t, `object has no key "io.k8s.cli.setters.image-name"`, err.Error()) { + t.FailNow() + } +} + +func TestDeleteSchemaInFileNoDefs(t *testing.T) { + ResetOpenAPI() + inputyaml := ` +openAPI: + definitions: + ` + f, err := ioutil.TempFile("", "openapi-") + if !assert.NoError(t, err) { + t.FailNow() + } + if !assert.NoError(t, ioutil.WriteFile(f.Name(), []byte(inputyaml), 0600)) { + t.FailNow() + } + + err = AddSchemaFromFile(f.Name()) + if !assert.NoError(t, err) { + t.FailNow() + } + + err = DeleteSchemaInFile(f.Name()) + if !assert.NoError(t, err) { + t.FailNow() + } +} + func TestPopulateDefsInOpenAPI_Substitution(t *testing.T) { ResetOpenAPI() inputyaml := ` diff --git a/kyaml/pathutil/pathutil.go b/kyaml/pathutil/pathutil.go index 53f816237..d0a29926f 100644 --- a/kyaml/pathutil/pathutil.go +++ b/kyaml/pathutil/pathutil.go @@ -6,23 +6,29 @@ package pathutil import ( "os" "path/filepath" - "strings" ) -// SubDirsWithFile takes the root directory path and returns all the paths of -// sub-directories which contain file with input fileName including itself -func SubDirsWithFile(root, fileName string) ([]string, error) { +// DirsWithFile takes the root directory path and returns all the paths of +// sub-directories(including itself) which contain file with input fileName +// at top level if recurse is true +func DirsWithFile(root, fileName string, recurse bool) ([]string, error) { var res []string + if !recurse { + // check if the file with fileName is present in root and return it + // else return empty list + _, err := os.Stat(filepath.Join(root, fileName)) + if !os.IsNotExist(err) { + res = append(res, root) + } + return res, nil + } err := filepath.Walk(root, func(path string, info os.FileInfo, err error) error { if err != nil { return err } - if strings.HasSuffix(path, fileName) { - if root == "." { - path = root + "/" + path - } - res = append(res, path) + if filepath.Base(path) == fileName { + res = append(res, filepath.Dir(path)) } return nil }) diff --git a/kyaml/pathutil/pathutil_test.go b/kyaml/pathutil/pathutil_test.go index 00fdf9ab2..6069a581f 100644 --- a/kyaml/pathutil/pathutil_test.go +++ b/kyaml/pathutil/pathutil_test.go @@ -13,6 +13,32 @@ import ( ) func TestSubDirsWithFile(t *testing.T) { + var tests = []struct { + name string + fileName string + recurse bool + outFilesCount int + }{ + { + name: "dirs-with-file-recurse", + fileName: "Krmfile", + outFilesCount: 3, + recurse: true, + }, + { + name: "dirs-with-non-existent-file-recurse", + fileName: "non-existent-file.txt", + outFilesCount: 0, + recurse: true, + }, + { + name: "dir-with-file-no-recurse", + fileName: "Krmfile", + outFilesCount: 1, + recurse: false, + }, + } + dir, err := ioutil.TempDir("", "") if !assert.NoError(t, err) { t.FailNow() @@ -23,28 +49,17 @@ func TestSubDirsWithFile(t *testing.T) { t.FailNow() } - res, err := SubDirsWithFile(dir, "Krmfile") - if !assert.NoError(t, err) { - t.FailNow() - } - if !assert.Equal(t, 3, len(res)) { - t.FailNow() - } -} - -func TestSubDirsWithFileNoMatch(t *testing.T) { - dir, err := ioutil.TempDir("", "") - if !assert.NoError(t, err) { - t.FailNow() - } - defer os.RemoveAll(dir) - res, err := SubDirsWithFile(dir, "non-existent-file.txt") - if !assert.NoError(t, err) { - t.FailNow() - } - var expected []string - if !assert.Equal(t, expected, res) { - t.FailNow() + for i := range tests { + test := tests[i] + t.Run(test.name, func(t *testing.T) { + res, err := DirsWithFile(dir, test.fileName, test.recurse) + if !assert.NoError(t, err) { + t.FailNow() + } + if !assert.Equal(t, test.outFilesCount, len(res)) { + t.FailNow() + } + }) } } diff --git a/kyaml/setters2/set.go b/kyaml/setters2/set.go index 826973590..eba668af5 100644 --- a/kyaml/setters2/set.go +++ b/kyaml/setters2/set.go @@ -367,14 +367,14 @@ func (s SetOpenAPI) Filter(object *yaml.RNode) (*yaml.RNode, error) { return nil, err } if oa == nil { - return nil, errors.Errorf("no setter %s found", s.Name) + return nil, errors.Errorf("setter %q is not found", s.Name) } def, err := oa.Pipe(yaml.Lookup("x-k8s-cli", "setter")) if err != nil { return nil, err } if def == nil { - return nil, errors.Errorf("no setter %s found", s.Name) + return nil, errors.Errorf("setter %q is not found", s.Name) } // record the OpenAPI type for the setter diff --git a/kyaml/setters2/set_test.go b/kyaml/setters2/set_test.go index 41c01214f..6f9a247c1 100644 --- a/kyaml/setters2/set_test.go +++ b/kyaml/setters2/set_test.go @@ -1271,7 +1271,7 @@ openAPI: { name: "error", setter: "replicas", - err: "no setter replicas found", + err: `setter "replicas" is not found`, input: ` openAPI: definitions: diff --git a/kyaml/setters2/settersutil/fieldsetter.go b/kyaml/setters2/settersutil/fieldsetter.go index 1bd52ce12..6efafed41 100644 --- a/kyaml/setters2/settersutil/fieldsetter.go +++ b/kyaml/setters2/settersutil/fieldsetter.go @@ -32,16 +32,20 @@ type FieldSetter struct { OpenAPIPath string + OpenAPIFileName string + ResourcesPath string + + RecurseSubPackages bool } func (fs *FieldSetter) Filter(input []*yaml.RNode) ([]*yaml.RNode, error) { - fs.Count, _ = fs.Set(fs.OpenAPIPath, fs.ResourcesPath) + fs.Count, _ = fs.Set() return nil, nil } // Set updates the OpenAPI definitions and resources with the new setter value -func (fs FieldSetter) Set(openAPIPath, resourcesPath string) (int, error) { +func (fs FieldSetter) Set() (int, error) { // Update the OpenAPI definitions soa := setters2.SetOpenAPI{ Name: fs.Name, @@ -55,30 +59,30 @@ func (fs FieldSetter) Set(openAPIPath, resourcesPath string) (int, error) { // at to get the value and set it to resource files, but if there is error // after updating openAPI file and while updating resources, the openAPI // file should be reverted, as set operation failed - stat, err := os.Stat(openAPIPath) + stat, err := os.Stat(fs.OpenAPIPath) if err != nil { return 0, err } - curOpenAPI, err := ioutil.ReadFile(openAPIPath) + curOpenAPI, err := ioutil.ReadFile(fs.OpenAPIPath) if err != nil { return 0, err } // write the new input value to openAPI file - if err := soa.UpdateFile(openAPIPath); err != nil { + if err := soa.UpdateFile(fs.OpenAPIPath); err != nil { return 0, err } // Load the updated definitions - if err := openapi.AddSchemaFromFile(openAPIPath); err != nil { + if err := openapi.AddSchemaFromFile(fs.OpenAPIPath); err != nil { return 0, err } // Update the resources with the new value // Set NoDeleteFiles to true as SetAll will return only the nodes of files which should be updated and // hence, rest of the files should not be deleted - inout := &kio.LocalPackageReadWriter{PackagePath: resourcesPath, NoDeleteFiles: true} + inout := &kio.LocalPackageReadWriter{PackagePath: fs.ResourcesPath, NoDeleteFiles: true, PackageFileName: fs.OpenAPIFileName} s := &setters2.Set{Name: fs.Name} err = kio.Pipeline{ Inputs: []kio.Reader{inout}, @@ -88,7 +92,7 @@ func (fs FieldSetter) Set(openAPIPath, resourcesPath string) (int, error) { // revert openAPI file if set operation fails if err != nil { - if writeErr := ioutil.WriteFile(openAPIPath, curOpenAPI, stat.Mode().Perm()); writeErr != nil { + if writeErr := ioutil.WriteFile(fs.OpenAPIPath, curOpenAPI, stat.Mode().Perm()); writeErr != nil { return 0, writeErr } } diff --git a/kyaml/setters2/settersutil/settercreator.go b/kyaml/setters2/settersutil/settercreator.go index 0ea58d4fb..9d8e04311 100644 --- a/kyaml/setters2/settersutil/settercreator.go +++ b/kyaml/setters2/settersutil/settercreator.go @@ -47,6 +47,10 @@ type SetterCreator struct { // the package publisher's intent to make the setter required to be set. Required bool + RecurseSubPackages bool + + OpenAPIFileName string + // Path to openAPI file OpenAPIPath string @@ -55,17 +59,21 @@ type SetterCreator struct { } func (c *SetterCreator) Filter(input []*yaml.RNode) ([]*yaml.RNode, error) { - return nil, c.Create(c.OpenAPIPath, c.ResourcesPath) + return nil, c.Create() } -func (c SetterCreator) Create(openAPIPath, resourcesPath string) error { - err := validateSchema(c.Schema) +func (c SetterCreator) Create() error { + err := c.validateSetterInfo() + if err != nil { + return err + } + err = validateSchema(c.Schema) if err != nil { return errors.Errorf("invalid schema: %v", err) } // Update the resources with the setter reference - inout := &kio.LocalPackageReadWriter{PackagePath: resourcesPath} + inout := &kio.LocalPackageReadWriter{PackagePath: c.ResourcesPath} a := &setters2.Add{ FieldName: c.FieldName, FieldValue: c.FieldValue, @@ -78,7 +86,7 @@ func (c SetterCreator) Create(openAPIPath, resourcesPath string) error { Outputs: []kio.Writer{inout}, }.Execute() if a.Count == 0 { - fmt.Printf("setter %s doesn't match any field in resource configs, "+ + fmt.Printf("setter %q doesn't match any field in resource configs, "+ "but creating setter definition\n", c.Name) } if err != nil { @@ -90,12 +98,12 @@ func (c SetterCreator) Create(openAPIPath, resourcesPath string) error { Name: c.Name, Value: c.FieldValue, SetBy: c.SetBy, Description: c.Description, Type: c.Type, Schema: c.Schema, Required: c.Required, } - if err := sd.AddToFile(openAPIPath); err != nil { + if err := sd.AddToFile(c.OpenAPIPath); err != nil { return err } // Load the updated definitions - if err := openapi.AddSchemaFromFile(openAPIPath); err != nil { + if err := openapi.AddSchemaFromFile(c.OpenAPIPath); err != nil { return err } // if the setter is of array type write the derived list values back to @@ -103,7 +111,7 @@ func (c SetterCreator) Create(openAPIPath, resourcesPath string) error { if len(a.ListValues) > 0 { sd.ListValues = a.ListValues sd.Value = "" - if err := sd.AddToFile(openAPIPath); err != nil { + if err := sd.AddToFile(c.OpenAPIPath); err != nil { return err } } @@ -165,3 +173,32 @@ func validateSchemaTypes(sc *spec.Schema) error { } return nil } + +func (c SetterCreator) validateSetterInfo() error { + // check if substitution with same name exists and throw error + ref, err := spec.NewRef(fieldmeta.DefinitionsPrefix + fieldmeta.SubstitutionDefinitionPrefix + c.Name) + if err != nil { + return err + } + + subst, _ := openapi.Resolve(&ref) + // if substitution already exists with the input setter name, throw error + if subst != nil { + return errors.Errorf("substitution with name %q already exists, "+ + "substitution and setter can't have same name", c.Name) + } + + // check if setter with same name exists and throw error + ref, err = spec.NewRef(fieldmeta.DefinitionsPrefix + fieldmeta.SetterDefinitionPrefix + c.Name) + if err != nil { + return err + } + + setter, _ := openapi.Resolve(&ref) + // if setter already exists with the input setter name, throw error + if setter != nil { + return errors.Errorf("setter with name %q already exists, "+ + "if you want to modify it, please delete the existing setter and recreate it", c.Name) + } + return nil +} diff --git a/kyaml/setters2/settersutil/substitutioncreator.go b/kyaml/setters2/settersutil/substitutioncreator.go index 2260b6260..53ace3a57 100644 --- a/kyaml/setters2/settersutil/substitutioncreator.go +++ b/kyaml/setters2/settersutil/substitutioncreator.go @@ -47,15 +47,23 @@ type SubstitutionCreator struct { // Path to openAPI file OpenAPIPath string + OpenAPIFileName string + + RecurseSubPackages bool + // Path to resources folder ResourcesPath string } func (c *SubstitutionCreator) Filter(input []*yaml.RNode) ([]*yaml.RNode, error) { - return nil, c.Create(c.OpenAPIPath, c.ResourcesPath) + return nil, c.Create() } -func (c SubstitutionCreator) Create(openAPIPath, resourcesPath string) error { +func (c SubstitutionCreator) Create() error { + err := c.validateSubstitutionInfo() + if err != nil { + return err + } values, err := markersAndRefs(c.Name, c.Pattern) if err != nil { return err @@ -70,22 +78,22 @@ func (c SubstitutionCreator) Create(openAPIPath, resourcesPath string) error { // the input substitution definition is updated in the openAPI file and then parsed // to check if there are any cycles in nested substitutions, if there are // any, the openAPI file will be reverted to current state and error is thrown - stat, err := os.Stat(openAPIPath) + stat, err := os.Stat(c.OpenAPIPath) if err != nil { return err } - curOpenAPI, err := ioutil.ReadFile(openAPIPath) + curOpenAPI, err := ioutil.ReadFile(c.OpenAPIPath) if err != nil { return err } - if err := d.AddToFile(openAPIPath); err != nil { + if err := d.AddToFile(c.OpenAPIPath); err != nil { return err } // Load the updated definitions - if err := openapi.AddSchemaFromFile(openAPIPath); err != nil { + if err := openapi.AddSchemaFromFile(c.OpenAPIPath); err != nil { return err } @@ -105,19 +113,19 @@ func (c SubstitutionCreator) Create(openAPIPath, resourcesPath string) error { return err } - err = c.CreateSettersForSubstitution(openAPIPath) + err = c.CreateSettersForSubstitution(c.OpenAPIPath) if err != nil { return err } // Load the updated definitions after setters are created - if err := openapi.AddSchemaFromFile(openAPIPath); err != nil { + if err := openapi.AddSchemaFromFile(c.OpenAPIPath); err != nil { return err } // revert openAPI file if there are cycles detected in created input substitution if err := checkForCycles(ext, visited); err != nil { - if writeErr := ioutil.WriteFile(openAPIPath, curOpenAPI, stat.Mode().Perm()); writeErr != nil { + if writeErr := ioutil.WriteFile(c.OpenAPIPath, curOpenAPI, stat.Mode().Perm()); writeErr != nil { return writeErr } return err @@ -130,7 +138,7 @@ func (c SubstitutionCreator) Create(openAPIPath, resourcesPath string) error { } // Update the resources with the substitution reference - inout := &kio.LocalPackageReadWriter{PackagePath: resourcesPath} + inout := &kio.LocalPackageReadWriter{PackagePath: c.ResourcesPath} err = kio.Pipeline{ Inputs: []kio.Reader{inout}, Filters: []kio.Filter{kio.FilterAll(a)}, @@ -203,7 +211,7 @@ func (c SubstitutionCreator) CreateSettersForSubstitution(openAPIPath string) er // continue if ref is a substitution, as it has already been checked if it exists // as part of preRunE if strings.Contains(value.Ref, fieldmeta.SubstitutionDefinitionPrefix) { - fmt.Printf("found a substitution with name %s\n", value.Marker) + fmt.Printf("found a substitution with name %q\n", value.Marker) continue } setterObj, err := y.Pipe(yaml.Lookup( @@ -420,3 +428,32 @@ func min(a int, b int) int { } return b } + +func (c SubstitutionCreator) validateSubstitutionInfo() error { + // check if substitution with same name exists and throw error + ref, err := spec.NewRef(fieldmeta.DefinitionsPrefix + fieldmeta.SubstitutionDefinitionPrefix + c.Name) + if err != nil { + return err + } + + subst, _ := openapi.Resolve(&ref) + // if substitution already exists with the input substitution name, throw error + if subst != nil { + return errors.Errorf("substitution with name %q already exists", c.Name) + } + + // check if setter with same name exists and throw error + ref, err = spec.NewRef(fieldmeta.DefinitionsPrefix + fieldmeta.SetterDefinitionPrefix + c.Name) + if err != nil { + return err + } + + setter, _ := openapi.Resolve(&ref) + // if setter already exists with input substitution name, throw error + if setter != nil { + return errors.Errorf(fmt.Sprintf("setter with name %q already exists, "+ + "substitution and setter can't have same name", c.Name)) + } + + return nil +}