diff --git a/cmd/config/internal/commands/e2e/e2e_test.go b/cmd/config/internal/commands/e2e/e2e_test.go index 962134c994..c74721d791 100644 --- a/cmd/config/internal/commands/e2e/e2e_test.go +++ b/cmd/config/internal/commands/e2e/e2e_test.go @@ -87,8 +87,18 @@ metadata: expectedFiles: func(d string) map[string]string { return map[string]string{ "deployment.json": ` -{"apiVersion": "apps/v1", "kind": "Deployment", "metadata": {"name": "foo", annotations: { - a-string-value: '', a-int-value: '0', a-bool-value: 'false'}}} +{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": { + "annotations": { + "a-bool-value": "false", + "a-int-value": "0", + "a-string-value": "" + }, + "name": "foo" + } +} `, } }, diff --git a/kyaml/kio/byteio_writer.go b/kyaml/kio/byteio_writer.go index 84097f1086..a243ed9113 100644 --- a/kyaml/kio/byteio_writer.go +++ b/kyaml/kio/byteio_writer.go @@ -6,13 +6,16 @@ package kio import ( "encoding/json" "io" - + "path/filepath" "sigs.k8s.io/kustomize/kyaml/errors" "sigs.k8s.io/kustomize/kyaml/kio/kioutil" "sigs.k8s.io/kustomize/kyaml/yaml" ) -// Writer writes ResourceNodes to bytes. +// ByteWriter writes ResourceNodes to bytes. Generally YAML encoding will be used but in the special +// case of writing a single, bare yaml.RNode that has a kioutil.PathAnnotation indicating that the +// target is a JSON file JSON encoding is used. See shouldJSONEncodeSingleBareNode below for more +// information. type ByteWriter struct { // Writer is where ResourceNodes are encoded. Writer io.Writer @@ -55,8 +58,9 @@ func (w ByteWriter) Write(nodes []*yaml.RNode) error { } } - encoder := yaml.NewEncoder(w.Writer) - defer encoder.Close() + // Even though we use the this value further down we must check this before removing annotations + jsonEncodeSingleBareNode := w.shouldJSONEncodeSingleBareNode(nodes) + for i := range nodes { // clean resources by removing annotations set by the Reader if !w.KeepReaderAnnotations { @@ -81,10 +85,18 @@ func (w ByteWriter) Write(nodes []*yaml.RNode) error { } } + if jsonEncodeSingleBareNode { + encoder := json.NewEncoder(w.Writer) + encoder.SetIndent("", " ") + return errors.Wrap(encoder.Encode(nodes[0])) + } + + encoder := yaml.NewEncoder(w.Writer) + defer encoder.Close() // don't wrap the elements if w.WrappingKind == "" { for i := range nodes { - if err := w.encode(encoder, nodes[i].Document()); err != nil { + if err := encoder.Encode(nodes[i].Document()); err != nil { return err } } @@ -118,23 +130,31 @@ func (w ByteWriter) Write(nodes []*yaml.RNode) error { for i := range nodes { items.Content = append(items.Content, nodes[i].YNode()) } - err := w.encode(encoder, doc) + err := encoder.Encode(doc) yaml.UndoSerializationHacksOnNodes(nodes) return err } -// encode encodes the input document node to appropriate node format -func (w ByteWriter) encode(encoder *yaml.Encoder, doc *yaml.Node) error { - rNode := &yaml.RNode{} - rNode.SetYNode(doc) - str, err := rNode.String() - if err != nil { - return errors.Wrap(err) - } - if json.Valid([]byte(str)) { - je := json.NewEncoder(w.Writer) - je.SetIndent("", " ") - return errors.Wrap(je.Encode(rNode)) +// shouldJSONEncodeSingleBareNode determines if nodes contain a single node that should not be +// wrapped and has a JSON file extension, which in turn means that the node should be JSON encoded. +// Note 1: this must be checked before any annotations to avoid losing information about the target +// filename extension. +// Note 2: JSON encoding should only be used for single, unwrapped nodes because multiple unwrapped +// nodes cannot be represented in JSON (no multi doc support). Furthermore, the typical use +// cases for wrapping nodes would likely not include later writing the whole wrapper to a +// .json file, i.e. there is no point risking any edge case information loss e.g. comments +// disappearing, that could come from JSON encoding the whole wrapper just to ensure that +// one (or all nodes) can be read as JSON. +func (w ByteWriter) shouldJSONEncodeSingleBareNode(nodes []*yaml.RNode) bool { + if w.WrappingKind == "" && len(nodes) == 1 { + if path, _, _ := kioutil.GetFileAnnotations(nodes[0]); path != "" { + filename := filepath.Base(path) + for _, glob := range JSONMatch { + if match, _ := filepath.Match(glob, filename); match { + return true + } + } + } } - return encoder.Encode(doc) -} + return false +} \ No newline at end of file diff --git a/kyaml/kio/byteio_writer_test.go b/kyaml/kio/byteio_writer_test.go index d038f529eb..c284d04a89 100644 --- a/kyaml/kio/byteio_writer_test.go +++ b/kyaml/kio/byteio_writer_test.go @@ -26,7 +26,7 @@ func TestByteWriter(t *testing.T) { testCases := []testCase{ // - // + // Test Case // { name: "wrap_resource_list", @@ -60,7 +60,7 @@ functionConfig: }, // - // + // Test Case // { name: "multiple_items", @@ -187,6 +187,110 @@ c: d # second metadata: annotations: config.kubernetes.io/path: "a/b/a_test.yaml" +`, + }, + + // + // Test Case + // + { + name: "encode_valid_json", + items: []string{ + `{ + "a": "a long string that would certainly see a newline introduced by the YAML marshaller abcd123", + metadata: { + annotations: { + config.kubernetes.io/path: test.json + } + } +}`, + }, + + expectedOutput: `{ + "a": "a long string that would certainly see a newline introduced by the YAML marshaller abcd123", + "metadata": { + "annotations": { + "config.kubernetes.io/path": "test.json" + } + } +}`, + }, + + // + // Test Case + // + { + name: "encode_unformatted_valid_json", + items: []string{ + `{ "a": "b", metadata: { annotations: { config.kubernetes.io/path: test.json } } }`, + }, + + expectedOutput: `{ + "a": "b", + "metadata": { + "annotations": { + "config.kubernetes.io/path": "test.json" + } + } +}`, + }, + + // + // Test Case + // + { + name: "encode_wrapped_json_as_yaml", + instance: ByteWriter{ + Sort: true, + WrappingKind: ResourceListKind, + WrappingAPIVersion: ResourceListAPIVersion, + }, + items: []string{ + `{ + "a": "b", + "metadata": { + "annotations": { + "config.kubernetes.io/path": "test.json" + } + } +}`, + }, + + expectedOutput: `apiVersion: config.kubernetes.io/v1alpha1 +kind: ResourceList +items: +- {"a": "b", "metadata": {"annotations": {"config.kubernetes.io/path": "test.json"}}} +`, + }, + + // + // Test Case + // + { + name: "encode_multi_doc_json_as_yaml", + items: []string{ + `{ + "a": "b", + "metadata": { + "annotations": { + "config.kubernetes.io/path": "test-1.json" + } + } +}`, + `{ + "c": "d", + "metadata": { + "annotations": { + "config.kubernetes.io/path": "test-2.json" + } + } +}`, + }, + + expectedOutput: ` +{"a": "b", "metadata": {"annotations": {"config.kubernetes.io/path": "test-1.json"}}} +--- +{"c": "d", "metadata": {"annotations": {"config.kubernetes.io/path": "test-2.json"}}} `, }, } diff --git a/kyaml/kio/filters/fmtr_test.go b/kyaml/kio/filters/fmtr_test.go index 87472bbe04..e988a3ff56 100644 --- a/kyaml/kio/filters/fmtr_test.go +++ b/kyaml/kio/filters/fmtr_test.go @@ -743,8 +743,8 @@ func TestFormatFileOrDirectory_ymlExtFile(t *testing.T) { assert.Equal(t, string(testyaml.FormattedYaml1), string(b)) } -// TestFormatFileOrDirectory_skipYamlExtFileWithJson verifies that the json content is formatted -// as yaml +// TestFormatFileOrDirectory_YamlExtFileWithJson verifies that the JSON compatible flow style +// YAML content is formatted as such. func TestFormatFileOrDirectory_YamlExtFileWithJson(t *testing.T) { // write the unformatted JSON file contents f, err := ioutil.TempFile("", "yamlfmt*.yaml") @@ -760,7 +760,27 @@ func TestFormatFileOrDirectory_YamlExtFileWithJson(t *testing.T) { // check the result is formatted as yaml b, err := ioutil.ReadFile(f.Name()) assert.NoError(t, err) - assert.Equal(t, string(testyaml.FormattedJSON1), string(b)) + assert.Equal(t, string(testyaml.FormattedFlowYAML1), string(b)) +} + +// TestFormatFileOrDirectory_JsonExtFileWithNotModified verifies that a file with .json extensions +// and JSON contents won't be modified. +func TestFormatFileOrDirectory_JsonExtFileWithNotModified(t *testing.T) { + // write the unformatted JSON file contents + f, err := ioutil.TempFile("", "yamlfmt*.json") + assert.NoError(t, err) + defer os.Remove(f.Name()) + err = ioutil.WriteFile(f.Name(), testyaml.UnformattedJSON1, 0600) + assert.NoError(t, err) + + // format the file + err = FormatFileOrDirectory(f.Name()) + assert.NoError(t, err) + + // check the result is formatted as yaml + b, err := ioutil.ReadFile(f.Name()) + assert.NoError(t, err) + assert.Equal(t, string(testyaml.UnformattedJSON1), string(b)) } // TestFormatFileOrDirectory_partialKubernetesYamlFile verifies that if a yaml file contains both diff --git a/kyaml/kio/filters/testyaml/testyaml.go b/kyaml/kio/filters/testyaml/testyaml.go index eae6692f42..01644b3112 100644 --- a/kyaml/kio/filters/testyaml/testyaml.go +++ b/kyaml/kio/filters/testyaml/testyaml.go @@ -97,18 +97,9 @@ status2: - 2 `) -var FormattedJSON1 = []byte(`{ - "apiVersion": "example.com/v1beta1", - "kind": "MyType", - "spec": "a", - "status": { - "conditions": [ - 3, - 1, - 2 - ] - } -} +var FormattedFlowYAML1 = []byte( + `{"apiVersion": "example.com/v1beta1", "kind": "MyType", "spec": "a", "status": {"conditions": [ + 3, 1, 2]}} `) var FormattedYaml3 = []byte(`apiVersion: v1