From b39c522cc1b76a874729343eb366dd9e5adcce30 Mon Sep 17 00:00:00 2001 From: Phani Teja Marupaka Date: Sun, 7 Jun 2020 15:13:21 -0700 Subject: [PATCH] Suggested changes --- cmd/config/internal/commands/sink_test.go | 133 ++++++++++++- cmd/config/internal/commands/source.go | 2 +- cmd/config/internal/commands/source_test.go | 203 ++++++++++++++++---- kyaml/kio/byteio_reader.go | 46 +++-- kyaml/kio/byteio_writer.go | 26 +-- kyaml/kio/kioutil/kioutil.go | 10 +- kyaml/kio/pkgio_reader.go | 19 +- kyaml/kio/pkgio_writer.go | 1 - kyaml/setters2/add.go | 2 +- kyaml/setters2/set.go | 4 +- kyaml/yaml/types.go | 24 +-- kyaml/yaml/types_test.go | 31 ++- 12 files changed, 379 insertions(+), 122 deletions(-) diff --git a/cmd/config/internal/commands/sink_test.go b/cmd/config/internal/commands/sink_test.go index 3e992ac3b..cf70c6554 100644 --- a/cmd/config/internal/commands/sink_test.go +++ b/cmd/config/internal/commands/sink_test.go @@ -33,7 +33,7 @@ items: annotations: app: nginx2 config.kubernetes.io/index: '0' - config.kubernetes.io/path: 'f1.json' + config.kubernetes.io/path: 'f1.yaml' spec: replicas: 1 - kind: Service @@ -42,7 +42,7 @@ items: annotations: app: nginx config.kubernetes.io/index: '1' - config.kubernetes.io/path: 'f1.json' + config.kubernetes.io/path: 'f1.yaml' spec: selector: app: nginx @@ -77,13 +77,28 @@ items: t.FailNow() } - actual, err := ioutil.ReadFile(filepath.Join(d, "f1.json")) + actual, err := ioutil.ReadFile(filepath.Join(d, "f1.yaml")) if !assert.NoError(t, err) { t.FailNow() } - expected := `'{"kind":"Deployment","metadata":{"annotations":{"app":"nginx2"},"labels":{"app":"nginx2"},"name":"foo"},"spec":{"replicas":1}}' + expected := `kind: Deployment +metadata: + labels: + app: nginx2 + name: foo + annotations: + app: nginx2 +spec: + replicas: 1 --- -'{"kind":"Service","metadata":{"annotations":{"app":"nginx"},"name":"foo"},"spec":{"selector":{"app":"nginx"}}}' +kind: Service +metadata: + name: foo + annotations: + app: nginx +spec: + selector: + app: nginx ` if !assert.Equal(t, expected, string(actual)) { t.FailNow() @@ -121,6 +136,60 @@ spec: } } +func TestSinkCommandJSON(t *testing.T) { + d, err := ioutil.TempDir("", "kustomize-source-test") + if !assert.NoError(t, err) { + t.FailNow() + } + defer os.RemoveAll(d) + + r := commands.GetSinkRunner("") + r.Command.SetIn(bytes.NewBufferString(`apiVersion: config.kubernetes.io/v1alpha1 +kind: ResourceList +items: +- kind: Deployment + metadata: + labels: + app: nginx2 + name: foo + annotations: + app: nginx2 + config.kubernetes.io/index: '0' + config.kubernetes.io/path: 'f1.json' + config.kubernetes.io/format: 'json' + spec: + replicas: 1 +`)) + r.Command.SetArgs([]string{d}) + if !assert.NoError(t, r.Command.Execute()) { + t.FailNow() + } + + actual, err := ioutil.ReadFile(filepath.Join(d, "f1.json")) + if !assert.NoError(t, err) { + t.FailNow() + } + expected := `{ + "kind": "Deployment", + "metadata": { + "annotations": { + "app": "nginx2" + }, + "labels": { + "app": "nginx2" + }, + "name": "foo" + }, + "spec": { + "replicas": 1 + } +} +` + if !assert.Equal(t, expected, string(actual)) { + t.FailNow() + } +} + func TestSinkCommand_Stdout(t *testing.T) { d, err := ioutil.TempDir("", "kustomize-source-test") if !assert.NoError(t, err) { @@ -234,3 +303,57 @@ spec: t.FailNow() } } + +func TestSinkCommandJSON_Stdout(t *testing.T) { + d, err := ioutil.TempDir("", "kustomize-source-test") + if !assert.NoError(t, err) { + t.FailNow() + } + defer os.RemoveAll(d) + + // fmt the files + out := &bytes.Buffer{} + r := commands.GetSinkRunner("") + r.Command.SetIn(bytes.NewBufferString(`apiVersion: config.kubernetes.io/v1alpha1 +kind: ResourceList +items: +- kind: Deployment + metadata: + labels: + app: nginx2 + name: foo + annotations: + app: nginx2 + config.kubernetes.io/index: '0' + config.kubernetes.io/format: 'json' + spec: + replicas: 1 +`)) + + r.Command.SetOut(out) + r.Command.SetArgs([]string{}) + if !assert.NoError(t, r.Command.Execute()) { + t.FailNow() + } + + expected := `{ + "kind": "Deployment", + "metadata": { + "annotations": { + "app": "nginx2" + }, + "labels": { + "app": "nginx2" + }, + "name": "foo" + }, + "spec": { + "replicas": 1 + } +} +` + if !assert.Equal(t, expected, out.String()) { + println(out.String()) + t.FailNow() + } +} diff --git a/cmd/config/internal/commands/source.go b/cmd/config/internal/commands/source.go index 7873956ce..8dc6296ae 100644 --- a/cmd/config/internal/commands/source.go +++ b/cmd/config/internal/commands/source.go @@ -74,7 +74,7 @@ func (r *SourceRunner) runE(c *cobra.Command, args []string) error { inputs = append(inputs, kio.LocalPackageReader{PackagePath: a, MatchFilesGlob: kio.MatchAll}) } if len(inputs) == 0 { - inputs = []kio.Reader{&kio.ByteReader{Reader: c.InOrStdin()}} + inputs = []kio.Reader{&kio.ByteReader{Reader: c.InOrStdin(), AcceptJSON: true}} } err := kio.Pipeline{Inputs: inputs, Outputs: outputs}.Execute() diff --git a/cmd/config/internal/commands/source_test.go b/cmd/config/internal/commands/source_test.go index b7f77d2ca..082306623 100644 --- a/cmd/config/internal/commands/source_test.go +++ b/cmd/config/internal/commands/source_test.go @@ -21,37 +21,25 @@ func TestSourceCommand(t *testing.T) { } defer os.RemoveAll(d) - err = ioutil.WriteFile(filepath.Join(d, "f1.json"), []byte(` -{ - "kind": "Deployment", - "metadata": { - "labels": { - "app": "nginx2" - }, - "name": "foo", - "annotations": { - "app": "nginx2" - } - }, - "spec": { - "replicas": 1 - } -} + err = ioutil.WriteFile(filepath.Join(d, "f1.yaml"), []byte(` +kind: Deployment +metadata: + labels: + app: nginx2 + name: foo + annotations: + app: nginx2 +spec: + replicas: 1 --- -{ - "kind": "Service", - "metadata": { - "name": "foo", - "annotations": { - "app": "nginx" - } - }, - "spec": { - "selector": { - "app": "nginx" - } - } -} +kind: Service +metadata: + name: foo + annotations: + app: nginx +spec: + selector: + app: nginx `), 0600) if !assert.NoError(t, err) { return @@ -98,22 +86,22 @@ kind: ResourceList items: - kind: Deployment metadata: - annotations: - app: nginx2 - config.kubernetes.io/index: '0' - config.kubernetes.io/path: 'f1.json' labels: app: nginx2 name: foo + annotations: + app: nginx2 + config.kubernetes.io/index: '0' + config.kubernetes.io/path: 'f1.yaml' spec: replicas: 1 - kind: Service metadata: + name: foo annotations: app: nginx config.kubernetes.io/index: '1' - config.kubernetes.io/path: 'f1.json' - name: foo + config.kubernetes.io/path: 'f1.yaml' spec: selector: app: nginx @@ -147,6 +135,96 @@ items: } } +func TestSourceCommandJSON(t *testing.T) { + d, err := ioutil.TempDir("", "kustomize-source-test") + if !assert.NoError(t, err) { + return + } + defer os.RemoveAll(d) + + err = ioutil.WriteFile(filepath.Join(d, "f1.json"), []byte(` +{ + "kind": "Deployment", + "metadata": { + "labels": { + "app": "nginx2" + }, + "name": "foo", + "annotations": { + "app": "nginx2" + } + }, + "spec": { + "replicas": 1 + } +} +`), 0600) + if !assert.NoError(t, err) { + return + } + err = ioutil.WriteFile(filepath.Join(d, "f2.json"), []byte(` +{ + "apiVersion": "v1", + "kind": "Abstraction", + "metadata": { + "name": "foo", + "annotations": { + "config.kubernetes.io/function": "container:\n image: gcr.io/example/reconciler:v1\n", + "config.kubernetes.io/local-config": "true" + } + }, + "spec": { + "replicas": 3 + } +} +`), 0600) + if !assert.NoError(t, err) { + return + } + + // fmt the files + b := &bytes.Buffer{} + r := commands.GetSourceRunner("") + r.Command.SetArgs([]string{d}) + r.Command.SetOut(b) + if !assert.NoError(t, r.Command.Execute()) { + return + } + + if !assert.Equal(t, `apiVersion: config.kubernetes.io/v1alpha1 +kind: ResourceList +items: +- kind: Deployment + metadata: + annotations: + app: nginx2 + config.kubernetes.io/format: 'json' + config.kubernetes.io/index: '0' + config.kubernetes.io/path: 'f1.json' + labels: + app: nginx2 + name: foo + spec: + replicas: 1 +- apiVersion: v1 + kind: Abstraction + metadata: + annotations: + config.kubernetes.io/function: | + container: + image: gcr.io/example/reconciler:v1 + config.kubernetes.io/local-config: "true" + config.kubernetes.io/format: 'json' + config.kubernetes.io/index: '0' + config.kubernetes.io/path: 'f2.json' + name: foo + spec: + replicas: 3 +`, b.String()) { + return + } +} + func TestSourceCommand_Stdin(t *testing.T) { d, err := ioutil.TempDir("", "kustomize-source-test") if !assert.NoError(t, err) { @@ -210,3 +288,56 @@ items: return } } + +func TestSourceCommandJSON_Stdin(t *testing.T) { + d, err := ioutil.TempDir("", "kustomize-source-test") + if !assert.NoError(t, err) { + return + } + defer os.RemoveAll(d) + + in := bytes.NewBufferString(` +{ + "kind": "Deployment", + "metadata": { + "labels": { + "app": "nginx2" + }, + "name": "foo", + "annotations": { + "app": "nginx2" + } + }, + "spec": { + "replicas": 1 + } +} +`) + + out := &bytes.Buffer{} + r := commands.GetSourceRunner("") + r.Command.SetArgs([]string{}) + r.Command.SetIn(in) + r.Command.SetOut(out) + if !assert.NoError(t, r.Command.Execute()) { + return + } + + if !assert.Equal(t, `apiVersion: config.kubernetes.io/v1alpha1 +kind: ResourceList +items: +- kind: Deployment + metadata: + annotations: + app: nginx2 + config.kubernetes.io/format: 'json' + config.kubernetes.io/index: '0' + labels: + app: nginx2 + name: foo + spec: + replicas: 1 +`, out.String()) { + return + } +} diff --git a/kyaml/kio/byteio_reader.go b/kyaml/kio/byteio_reader.go index e49dae6b6..86899d724 100644 --- a/kyaml/kio/byteio_reader.go +++ b/kyaml/kio/byteio_reader.go @@ -5,9 +5,9 @@ package kio import ( "bytes" + "encoding/json" "fmt" "io" - "path/filepath" "sort" "strings" @@ -103,11 +103,15 @@ type ByteReader struct { // the read objects were originally wrapped in. WrappingKind string - // Path indicates file path which is being read - // empty if it is being read from stdin - Path string + // AcceptJSON indicates if the input json bytes should be processed + AcceptJSON bool } +const ( + YAML = "yaml" + JSON = "json" +) + var _ Reader = &ByteReader{} func (r *ByteReader) Read() ([]*yaml.RNode, error) { @@ -120,25 +124,31 @@ func (r *ByteReader) Read() ([]*yaml.RNode, error) { if err != nil { return nil, errors.Wrap(err) } - values := strings.Split(input.String(), "\n---\n") + inputStr := input.String() + + // check if json is accepted and if input bytes are in json format + if r.AcceptJSON && json.Valid([]byte(inputStr)) { + // convert json to yaml string to generate resource list object + // with appropriate format annotation + yamlValue, err := yaml.ConvertJSONToYAMLString(input.String()) + if err != nil { + return nil, err + } + if r.SetAnnotations == nil { + r.SetAnnotations = map[string]string{} + } + if !r.OmitReaderAnnotations { + r.SetAnnotations[kioutil.FormatAnnotation] = JSON + } + inputStr = yamlValue + } + + values := strings.Split(inputStr, "\n---\n") index := 0 for i := range values { value := values[i] - if r.Path != "" { - for _, g := range JSONMatch { - if match, err := filepath.Match(g, filepath.Ext(r.Path)); err != nil { - return nil, errors.Wrap(err) - } else if match { - value, err = yaml.ConvertJSONToYamlString(value) - if err != nil { - return nil, err - } - } - } - } - decoder := yaml.NewDecoder(bytes.NewBufferString(value)) node, err := r.decode(index, decoder) if err == io.EOF { diff --git a/kyaml/kio/byteio_writer.go b/kyaml/kio/byteio_writer.go index e44c52a7c..72b80331f 100644 --- a/kyaml/kio/byteio_writer.go +++ b/kyaml/kio/byteio_writer.go @@ -4,8 +4,8 @@ package kio import ( + "encoding/json" "io" - "path/filepath" "sigs.k8s.io/kustomize/kyaml/errors" "sigs.k8s.io/kustomize/kyaml/kio/kioutil" @@ -43,9 +43,6 @@ type ByteWriter struct { // Sort if set, will cause ByteWriter to sort the the nodes before writing them. Sort bool - - // Path is the output file path to write to - Path string } var _ Writer = ByteWriter{} @@ -137,20 +134,17 @@ func (w ByteWriter) Write(nodes []*yaml.RNode) error { // encode encodes the input RNode to appropriate node format based on file path extension func (w ByteWriter) encode(encoder *yaml.Encoder, node *yaml.RNode) error { - for _, g := range JSONMatch { - if match, err := filepath.Match(g, filepath.Ext(w.Path)); err != nil { + _, _, format, err := kioutil.GetFileAnnotations(node) + if !w.KeepReaderAnnotations { + _, err := node.Pipe(yaml.ClearAnnotation(kioutil.FormatAnnotation)) + if err != nil { return errors.Wrap(err) - } else if match { - json, err := yaml.ConvertYamlNodeToJSONString(node) - if err != nil { - return errors.Wrap(err) - } - err = encoder.Encode(json) - if err != nil { - return errors.Wrap(err) - } - return nil } } + if err == nil && format == JSON { + je := json.NewEncoder(w.Writer) + je.SetIndent("", " ") + return errors.Wrap(je.Encode(node)) + } return encoder.Encode(node.Document()) } diff --git a/kyaml/kio/kioutil/kioutil.go b/kyaml/kio/kioutil/kioutil.go index 1d5e3bf58..8839b7ca4 100644 --- a/kyaml/kio/kioutil/kioutil.go +++ b/kyaml/kio/kioutil/kioutil.go @@ -22,16 +22,20 @@ const ( // PathAnnotation records the path to the file the Resource was read from PathAnnotation AnnotationKey = "config.kubernetes.io/path" + + // FormatAnnotation records the format of the resource ex: json + FormatAnnotation AnnotationKey = "config.kubernetes.io/format" ) -func GetFileAnnotations(rn *yaml.RNode) (string, string, error) { +func GetFileAnnotations(rn *yaml.RNode) (string, string, string, error) { meta, err := rn.GetMeta() if err != nil { - return "", "", err + return "", "", "", err } path := meta.Annotations[PathAnnotation] index := meta.Annotations[IndexAnnotation] - return path, index, nil + format := meta.Annotations[FormatAnnotation] + return path, index, format, nil } // ErrorIfMissingAnnotation validates the provided annotations are present on the given resources diff --git a/kyaml/kio/pkgio_reader.go b/kyaml/kio/pkgio_reader.go index f02aa54a3..3b6fa9c3d 100644 --- a/kyaml/kio/pkgio_reader.go +++ b/kyaml/kio/pkgio_reader.go @@ -123,7 +123,7 @@ func (r *LocalPackageReadWriter) Write(nodes []*yaml.RNode) error { func (r *LocalPackageReadWriter) getFiles(nodes []*yaml.RNode) (sets.String, error) { val := sets.String{} for _, n := range nodes { - path, _, err := kioutil.GetFileAnnotations(n) + path, _, _, err := kioutil.GetFileAnnotations(n) if err != nil { return nil, errors.Wrap(err) } @@ -240,16 +240,31 @@ func (r *LocalPackageReader) readFile(path string, _ os.FileInfo) ([]*yaml.RNode return nil, err } defer f.Close() + rr := &ByteReader{ DisableUnwrapping: true, Reader: f, OmitReaderAnnotations: r.OmitReaderAnnotations, SetAnnotations: r.SetAnnotations, - Path: path, + AcceptJSON: r.acceptJSON(path), } return rr.Read() } +// acceptJSON returns true if the input path has json ext and if the MatchFilesGlob +// has any of the JSONMatch ext +func (r *LocalPackageReader) acceptJSON(path string) bool { + for _, gm := range r.MatchFilesGlob { + for _, jm := range JSONMatch { + if filepath.Ext(path) == filepath.Ext(jm) && + filepath.Ext(gm) == filepath.Ext(jm) { + return true + } + } + } + return false +} + // ShouldSkipFile returns true if the file should be skipped func (r *LocalPackageReader) ShouldSkipFile(info os.FileInfo) (bool, error) { // check if the files are in scope diff --git a/kyaml/kio/pkgio_writer.go b/kyaml/kio/pkgio_writer.go index dbbc52fd1..b671512e3 100644 --- a/kyaml/kio/pkgio_writer.go +++ b/kyaml/kio/pkgio_writer.go @@ -98,7 +98,6 @@ func (r LocalPackageWriter) Write(nodes []*yaml.RNode) error { Writer: f, KeepReaderAnnotations: r.KeepReaderAnnotations, ClearAnnotations: r.ClearAnnotations, - Path: outputPath, } if err = w.Write(outputFiles[path]); err != nil { return errors.Wrap(err) diff --git a/kyaml/setters2/add.go b/kyaml/setters2/add.go index 8517c66a4..e12ed82d5 100644 --- a/kyaml/setters2/add.go +++ b/kyaml/setters2/add.go @@ -183,7 +183,7 @@ func (sd SetterDefinition) Filter(object *yaml.RNode) (*yaml.RNode, error) { } if sd.Schema != "" { - schNode, err := yaml.ConvertJSONToYamlNode(sd.Schema) + schNode, err := yaml.ConvertJSONToYAMLNode(sd.Schema) if err != nil { return nil, err } diff --git a/kyaml/setters2/set.go b/kyaml/setters2/set.go index c95b0f0bc..8e08cd83d 100644 --- a/kyaml/setters2/set.go +++ b/kyaml/setters2/set.go @@ -447,7 +447,7 @@ func SetAll(s *Set) kio.Filter { return nil, errors.Wrap(err) } if s.Count > preCount { - path, _, err := kioutil.GetFileAnnotations(nodes[i]) + path, _, _, err := kioutil.GetFileAnnotations(nodes[i]) if err != nil { return nil, errors.Wrap(err) } @@ -457,7 +457,7 @@ func SetAll(s *Set) kio.Filter { var nodesInUpdatedFiles []*yaml.RNode // return only the nodes whose corresponding file has at least one node with input setter for i := range nodes { - path, _, err := kioutil.GetFileAnnotations(nodes[i]) + path, _, _, err := kioutil.GetFileAnnotations(nodes[i]) if err != nil { return nil, errors.Wrap(err) } diff --git a/kyaml/yaml/types.go b/kyaml/yaml/types.go index 9d715337a..a1ab793e5 100644 --- a/kyaml/yaml/types.go +++ b/kyaml/yaml/types.go @@ -722,9 +722,9 @@ func (rn *RNode) UnmarshalJSON(b []byte) error { return nil } -// ConvertJSONToYamlNode parses input json string and returns equivalent yaml node -func ConvertJSONToYamlNode(jsonStr string) (*RNode, error) { - yml, err := ConvertJSONToYamlString(jsonStr) +// ConvertJSONToYAMLNode parses input json string and returns equivalent yaml node +func ConvertJSONToYAMLNode(jsonStr string) (*RNode, error) { + yml, err := ConvertJSONToYAMLString(jsonStr) if err != nil { return nil, err } @@ -735,8 +735,8 @@ func ConvertJSONToYamlNode(jsonStr string) (*RNode, error) { return node, nil } -// ConvertJSONToYamlString parses input json string and returns equivalent yaml string -func ConvertJSONToYamlString(jsonStr string) (string, error) { +// ConvertJSONToYAMLString parses input json string and returns equivalent yaml string +func ConvertJSONToYAMLString(jsonStr string) (string, error) { var body map[string]interface{} err := json.Unmarshal([]byte(jsonStr), &body) if err != nil { @@ -749,20 +749,6 @@ func ConvertJSONToYamlString(jsonStr string) (string, error) { return string(yml), nil } -// ConvertYamlNodeToJSONString returns json string from input yaml RNode -func ConvertYamlNodeToJSONString(node *RNode) (string, error) { - nodeStr, err := node.String() - if err != nil { - return "", err - } - var body interface{} - if err := yaml.Unmarshal([]byte(nodeStr), &body); err != nil { - return "", err - } - res, err := json.Marshal(body) - return string(res), err -} - // checkKey returns true if all elems have the key func checkKey(key string, elems []*Node) bool { count := 0 diff --git a/kyaml/yaml/types_test.go b/kyaml/yaml/types_test.go index 23a2a3873..fa9aab215 100644 --- a/kyaml/yaml/types_test.go +++ b/kyaml/yaml/types_test.go @@ -147,7 +147,7 @@ hello: world } } -func TestConvertJSONToYamlNode(t *testing.T) { +func TestConvertJSONToYAMLNode(t *testing.T) { inputJSON := `{"type": "string", "maxLength": 15, "enum": ["allowedValue1", "allowedValue2"]}` expected := `enum: - allowedValue1 @@ -156,7 +156,7 @@ maxLength: 15 type: string ` - node, err := ConvertJSONToYamlNode(inputJSON) + node, err := ConvertJSONToYAMLNode(inputJSON) if !assert.NoError(t, err) { t.FailNow() } @@ -167,7 +167,7 @@ type: string assert.Equal(t, expected, actual) } -func TestConvertJSONToYamlString(t *testing.T) { +func TestConvertJSONToYAMLString(t *testing.T) { inputJSON := `{"type": "string", "maxLength": 15, "enum": ["allowedValue1", "allowedValue2"]}` expected := `enum: - allowedValue1 @@ -176,27 +176,22 @@ maxLength: 15 type: string ` - actual, err := ConvertJSONToYamlString(inputJSON) + actual, err := ConvertJSONToYAMLString(inputJSON) if !assert.NoError(t, err) { t.FailNow() } assert.Equal(t, expected, actual) } -func TestConvertYamlNodeToJSONStr(t *testing.T) { - yl := `enum: - - allowedValue1 - - allowedValue2 -maxLength: 15 -type: string -` - node, err := Parse(yl) - if !assert.NoError(t, err) { +// error if there are multiple json blobs in input string +func TestConvertJSONToYAMLStringError(t *testing.T) { + inputJSON := `{"type": "string", "maxLength": 15, "enum": ["allowedValue1", "allowedValue2"]} +{"type": "string", "maxLength": 5, "enum": ["allowedValue2", "allowedValue3"]}` + expected := `invalid character '{' after top-level value` + + _, err := ConvertJSONToYAMLString(inputJSON) + if !assert.Error(t, err) { t.FailNow() } - res, err := ConvertYamlNodeToJSONString(node) - if !assert.NoError(t, err) { - t.FailNow() - } - assert.Equal(t, `{"enum":["allowedValue1","allowedValue2"],"maxLength":15,"type":"string"}`, res) + assert.Equal(t, expected, err.Error()) }