diff --git a/cmd/config/runner/runner.go b/cmd/config/runner/runner.go index eb08b7be9..3d58d368a 100644 --- a/cmd/config/runner/runner.go +++ b/cmd/config/runner/runner.go @@ -60,37 +60,34 @@ func (e ExecuteCmdOnPkgs) Execute() error { } for i := range pkgsPaths { - pkgPath := pkgsPaths[i] - // Add schema present in openAPI file for current package - if e.NeedOpenAPI { - if err := openapi.AddSchemaFromFile(filepath.Join(pkgPath, ext.KRMFileName())); err != nil { - return err - } - } - - if !e.SkipPkgPathPrint { - fmt.Fprintf(e.Writer, "%s/\n", pkgPath) - } - - err := e.CmdRunner.ExecuteCmd(e.Writer, pkgPath) + err := e.processPkg(pkgsPaths[i]) if err != nil { return err } - if i != len(pkgsPaths)-1 { fmt.Fprint(e.Writer, "\n") } - - // Delete schema present in openAPI file for current package - if e.NeedOpenAPI { - if err := openapi.DeleteSchemaInFile(filepath.Join(pkgPath, ext.KRMFileName())); err != nil { - return err - } - } } return nil } +func (e ExecuteCmdOnPkgs) processPkg(pkgPath string) error { + // Add schema present in openAPI file for current package + if e.NeedOpenAPI { + clean, err := openapi.AddSchemaFromFile(filepath.Join(pkgPath, ext.KRMFileName())) + if err != nil { + return err + } + defer clean() + } + + if !e.SkipPkgPathPrint { + fmt.Fprintf(e.Writer, "%s/\n", pkgPath) + } + + return e.CmdRunner.ExecuteCmd(e.Writer, pkgPath) +} + // ParseFieldPath parse a flag value into a field path func ParseFieldPath(path string) ([]string, error) { // fixup '\.' so we don't split on it diff --git a/kyaml/openapi/openapi.go b/kyaml/openapi/openapi.go index 746214d54..c043c0ca0 100644 --- a/kyaml/openapi/openapi.go +++ b/kyaml/openapi/openapi.go @@ -65,28 +65,54 @@ const SupplementaryOpenAPIFieldName = "openAPI" const Definitions = "definitions" // AddSchemaFromFile reads the file at path and parses the OpenAPI definitions -// from the field "openAPI" -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(openAPIPath string) error { - fields, err := DefinitionRefs(openAPIPath) +// from the field "openAPI", also returns a function to clean the added definitions +// The returned clean function is a no-op on error, or else it's a function +// that the caller should use to remove the added openAPI definitions from +// global schema +func AddSchemaFromFile(path string) (func(), error) { + object, err := parseOpenAPI(path) if err != nil { - return err + return func() {}, err } - for _, field := range fields { - delete(globalSchema.schema.Definitions, field) + defs, err := definitionRefsFromRNode(object) + if err != nil { + return func() {}, err } - return nil + + clean := func() { + for _, def := range defs { + delete(globalSchema.schema.Definitions, def) + } + } + return clean, addSchemaUsingField(object, SupplementaryOpenAPIFieldName) } // DefinitionRefs returns the list of openAPI definition references present in the // input openAPIPath func DefinitionRefs(openAPIPath string) ([]string, error) { + object, err := parseOpenAPI(openAPIPath) + if err != nil { + return nil, err + } + return definitionRefsFromRNode(object) +} + +// definitionRefsFromRNode returns the list of openAPI definitions keys from input +// yaml RNode +func definitionRefsFromRNode(object *yaml.RNode) ([]string, error) { + definitions, err := object.Pipe(yaml.Lookup(SupplementaryOpenAPIFieldName, Definitions)) + if definitions == nil { + return nil, err + } + if err != nil { + return nil, err + } + return definitions.Fields() +} + +// parseOpenAPI reads openAPIPath yaml and converts it to RNode +func parseOpenAPI(openAPIPath string) (*yaml.RNode, error) { b, err := ioutil.ReadFile(openAPIPath) if err != nil { return nil, err @@ -96,44 +122,23 @@ func DefinitionRefs(openAPIPath string) ([]string, error) { if err != nil { return nil, err } - - definitions, err := object.Pipe(yaml.Lookup(SupplementaryOpenAPIFieldName, Definitions)) - if definitions == nil { - return nil, err - } - if err != nil { - return nil, err - } - - return definitions.Fields() + return object, 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. -func AddSchemaFromFileUsingField(path, field string) error { - b, err := ioutil.ReadFile(path) - if err != nil { - return err - } - - // parse the yaml file (json is a subset of yaml, so will also parse) - y, err := yaml.Parse(string(b)) - if err != nil { - return err - } - +// addSchemaUsingField parses the OpenAPI definitions from the specified field. +// If field is the empty string, use the whole document as OpenAPI. +func addSchemaUsingField(object *yaml.RNode, field string) error { if field != "" { // get the field containing the openAPI - m := y.Field(field) + m := object.Field(field) if m.IsNilOrEmpty() { // doesn't contain openAPI definitions return nil } - y = m.Value + object = m.Value } - oAPI, err := y.String() + oAPI, err := object.String() if err != nil { return err } diff --git a/kyaml/openapi/openapi_test.go b/kyaml/openapi/openapi_test.go index b9b96c036..a5d38c221 100644 --- a/kyaml/openapi/openapi_test.go +++ b/kyaml/openapi/openapi_test.go @@ -143,10 +143,11 @@ openAPI: t.FailNow() } - err = AddSchemaFromFile(f.Name()) + clean, err := AddSchemaFromFile(f.Name()) if !assert.NoError(t, err) { t.FailNow() } + defer clean() s, err := GetSchema(`{"$ref": "#/definitions/io.k8s.cli.setters.image-name"}`) @@ -179,7 +180,7 @@ openAPI: t.FailNow() } - err = AddSchemaFromFile(f.Name()) + clean, err := AddSchemaFromFile(f.Name()) if !assert.NoError(t, err) { t.FailNow() } @@ -195,10 +196,7 @@ openAPI: 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() - } + clean() _, err = GetSchema(`{"$ref": "#/definitions/io.k8s.cli.setters.image-name"}`) @@ -225,15 +223,11 @@ openAPI: t.FailNow() } - err = AddSchemaFromFile(f.Name()) - if !assert.NoError(t, err) { - t.FailNow() - } - - err = DeleteSchemaInFile(f.Name()) + clean, err := AddSchemaFromFile(f.Name()) if !assert.NoError(t, err) { t.FailNow() } + defer clean() } func TestPopulateDefsInOpenAPI_Substitution(t *testing.T) { @@ -271,9 +265,11 @@ openAPI: t.FailNow() } - if !assert.NoError(t, AddSchemaFromFile(f.Name())) { + clean, err := AddSchemaFromFile(f.Name()) + if !assert.NoError(t, err) { t.FailNow() } + defer clean() s, err := GetSchema(`{"$ref": "#/definitions/io.k8s.cli.substitutions.image"}`) @@ -305,9 +301,11 @@ kind: Example t.FailNow() } - if !assert.NoError(t, AddSchemaFromFile(f.Name())) { + clean, err := AddSchemaFromFile(f.Name()) + if !assert.NoError(t, err) { t.FailNow() } + defer clean() if !assert.Equal(t, len(globalSchema.schema.Definitions), 0) { t.FailNow() diff --git a/kyaml/setters2/add_test.go b/kyaml/setters2/add_test.go index 001b11796..c3e758f92 100644 --- a/kyaml/setters2/add_test.go +++ b/kyaml/setters2/add_test.go @@ -342,7 +342,7 @@ openAPI: func TestAddUpdateSubstitution(t *testing.T) { path := filepath.Join(os.TempDir(), "resourcefile") - //write initial resourcefile to temp path + // write initial resourcefile to temp path err := ioutil.WriteFile(path, []byte(resourcefile), 0666) if !assert.NoError(t, err) { t.FailNow() @@ -360,7 +360,7 @@ func TestAddUpdateSubstitution(t *testing.T) { values := []Value{value1, value2} - //add a setter definition + // add a setter definition subd := SubstitutionDefinition{ Name: "image", Pattern: "IMAGE_NAME:IMAGE_TAG", diff --git a/kyaml/setters2/delete_test.go b/kyaml/setters2/delete_test.go index 695f07c39..dad43b868 100644 --- a/kyaml/setters2/delete_test.go +++ b/kyaml/setters2/delete_test.go @@ -42,13 +42,13 @@ openAPI: func TestDelete_Filter2(t *testing.T) { path := filepath.Join(os.TempDir(), "resourcefile2") - //write initial resourcefile to temp path + // write initial resourcefile to temp path err := ioutil.WriteFile(path, []byte(resourcefile2), 0666) if !assert.NoError(t, err) { t.FailNow() } - //add a deleter definition + // add a deleter definition dd := DeleterDefinition{ Name: "image", DefinitionPrefix: fieldmeta.SetterDefinitionPrefix, diff --git a/kyaml/setters2/list.go b/kyaml/setters2/list.go index 0dda187cb..ed91d98e8 100644 --- a/kyaml/setters2/list.go +++ b/kyaml/setters2/list.go @@ -29,9 +29,11 @@ type List struct { // ListSetters initializes l.Setters with the setters from the OpenAPI definitions in the file func (l *List) ListSetters(openAPIPath, resourcePath string) error { - if err := openapi.AddSchemaFromFile(openAPIPath); err != nil { + clean, err := openapi.AddSchemaFromFile(openAPIPath) + if err != nil { return err } + defer clean() y, err := yaml.ReadFile(openAPIPath) if err != nil { return err @@ -41,9 +43,11 @@ func (l *List) ListSetters(openAPIPath, resourcePath string) error { // ListSubst initializes l.Substitutions with the substitutions from the OpenAPI definitions in the file func (l *List) ListSubst(openAPIPath string) error { - if err := openapi.AddSchemaFromFile(openAPIPath); err != nil { + clean, err := openapi.AddSchemaFromFile(openAPIPath) + if err != nil { return err } + defer clean() y, err := yaml.ReadFile(openAPIPath) if err != nil { return err diff --git a/kyaml/setters2/settersutil/deletecreator_test.go b/kyaml/setters2/settersutil/deletecreator_test.go index 34201a6de..d6987908b 100644 --- a/kyaml/setters2/settersutil/deletecreator_test.go +++ b/kyaml/setters2/settersutil/deletecreator_test.go @@ -71,10 +71,11 @@ func TestDeleterCreator_Delete(t *testing.T) { DefinitionPrefix: fieldmeta.SetterDefinitionPrefix, } - err = openapi.AddSchemaFromFile(openAPI.Name()) + clean, err := openapi.AddSchemaFromFile(openAPI.Name()) if !assert.NoError(t, err) { t.FailNow() } + defer clean() dc.OpenAPIPath = openAPI.Name() dc.ResourcesPath = resource.Name() diff --git a/kyaml/setters2/settersutil/deletercreator.go b/kyaml/setters2/settersutil/deletercreator.go index 2c86a3c96..0c695e3f5 100644 --- a/kyaml/setters2/settersutil/deletercreator.go +++ b/kyaml/setters2/settersutil/deletercreator.go @@ -39,9 +39,11 @@ func (d DeleterCreator) Delete() error { } // Load the updated definitions - if err := openapi.AddSchemaFromFile(d.OpenAPIPath); err != nil { + clean, err := openapi.AddSchemaFromFile(d.OpenAPIPath) + if err != nil { return err } + defer clean() // Update the resources with the deleter reference inout := &kio.LocalPackageReadWriter{PackagePath: d.ResourcesPath, PackageFileName: d.OpenAPIFileName} diff --git a/kyaml/setters2/settersutil/fieldsetter.go b/kyaml/setters2/settersutil/fieldsetter.go index 13b1bac58..60a3bae37 100644 --- a/kyaml/setters2/settersutil/fieldsetter.go +++ b/kyaml/setters2/settersutil/fieldsetter.go @@ -79,9 +79,11 @@ func (fs FieldSetter) Set() (int, error) { } // Load the updated definitions - if err := openapi.AddSchemaFromFile(fs.OpenAPIPath); err != nil { + clean, err := openapi.AddSchemaFromFile(fs.OpenAPIPath) + if err != nil { return 0, err } + defer clean() // 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 @@ -108,9 +110,11 @@ func (fs FieldSetter) Set() (int, error) { // If syncOpenAPI is true, the openAPI files in destination directories are also // updated with the setter values in the input openAPI file func SetAllSetterDefinitions(syncOpenAPI bool, openAPIPath string, dirs ...string) error { - if err := openapi.AddSchemaFromFile(openAPIPath); err != nil { + clean, err := openapi.AddSchemaFromFile(openAPIPath) + if err != nil { return err } + defer clean() for _, destDir := range dirs { if syncOpenAPI { openAPIFileName := filepath.Base(openAPIPath) @@ -122,7 +126,7 @@ func SetAllSetterDefinitions(syncOpenAPI bool, openAPIPath string, dirs ...strin rw := &kio.LocalPackageReadWriter{ PackagePath: destDir, // set output won't include resources from files which - //weren't modified. make sure we don't delete them. + // weren't modified. make sure we don't delete them. NoDeleteFiles: true, } diff --git a/kyaml/setters2/settersutil/settercreator.go b/kyaml/setters2/settersutil/settercreator.go index 9d8e04311..fa8d83013 100644 --- a/kyaml/setters2/settersutil/settercreator.go +++ b/kyaml/setters2/settersutil/settercreator.go @@ -103,9 +103,11 @@ func (c SetterCreator) Create() error { } // Load the updated definitions - if err := openapi.AddSchemaFromFile(c.OpenAPIPath); err != nil { + clean, err := openapi.AddSchemaFromFile(c.OpenAPIPath) + if err != nil { return err } + defer clean() // if the setter is of array type write the derived list values back to // openAPI definitions if len(a.ListValues) > 0 { diff --git a/kyaml/setters2/settersutil/substitutioncreator.go b/kyaml/setters2/settersutil/substitutioncreator.go index 51a14430e..66909f215 100644 --- a/kyaml/setters2/settersutil/substitutioncreator.go +++ b/kyaml/setters2/settersutil/substitutioncreator.go @@ -93,9 +93,11 @@ func (c SubstitutionCreator) Create() error { } // Load the updated definitions - if err := openapi.AddSchemaFromFile(c.OpenAPIPath); err != nil { + clean, err := openapi.AddSchemaFromFile(c.OpenAPIPath) + if err != nil { return err } + defer clean() visited := sets.String{} ref, err := spec.NewRef(fieldmeta.DefinitionsPrefix + fieldmeta.SubstitutionDefinitionPrefix + c.Name) @@ -119,9 +121,11 @@ func (c SubstitutionCreator) Create() error { } // Load the updated definitions after setters are created - if err := openapi.AddSchemaFromFile(c.OpenAPIPath); err != nil { + clean, err = openapi.AddSchemaFromFile(c.OpenAPIPath) + if err != nil { return err } + defer clean() // revert openAPI file if there are cycles detected in created input substitution if err := checkForCycles(ext, visited); err != nil { diff --git a/kyaml/setters2/settersutil/substitutioncreator_test.go b/kyaml/setters2/settersutil/substitutioncreator_test.go index 9c6b2951b..493856cea 100644 --- a/kyaml/setters2/settersutil/substitutioncreator_test.go +++ b/kyaml/setters2/settersutil/substitutioncreator_test.go @@ -103,7 +103,7 @@ func TestGetValuesForMarkers(t *testing.T) { } } } else { - //if expectedError is not nil, check for correctness of error message + // if expectedError is not nil, check for correctness of error message assert.Contains(t, err.Error(), test.expectedError.Error()) } }) diff --git a/kyaml/setters2/util_test.go b/kyaml/setters2/util_test.go index 415ed89bc..40ca5258a 100644 --- a/kyaml/setters2/util_test.go +++ b/kyaml/setters2/util_test.go @@ -154,7 +154,8 @@ openAPI: if !assert.NoError(t, err) { t.FailNow() } - err = openapi.AddSchemaFromFile(filepath.Join(dir, "Krmfile")) + clean, err := openapi.AddSchemaFromFile(filepath.Join(dir, "Krmfile")) + defer clean() if err != nil { // do nothing if openAPI file or schema doesn't exist, CheckRequiredSettersSet() // should not throw any error