diff --git a/kyaml/kio/byteio_reader.go b/kyaml/kio/byteio_reader.go index 9036b1739cf..fd0c45c6a24 100644 --- a/kyaml/kio/byteio_reader.go +++ b/kyaml/kio/byteio_reader.go @@ -316,6 +316,10 @@ func (r *ByteReader) decode(originalYAML string, index int, decoder *yaml.Decode r.SetAnnotations = map[string]string{} } if !r.OmitReaderAnnotations { + err := kioutil.CopyLegacyAnnotations(n) + if err != nil { + return nil, err + } r.SetAnnotations[kioutil.IndexAnnotation] = fmt.Sprintf("%d", index) r.SetAnnotations[kioutil.LegacyIndexAnnotation] = fmt.Sprintf("%d", index) diff --git a/kyaml/kio/byteio_writer_test.go b/kyaml/kio/byteio_writer_test.go index ce3c3732185..60bebf7165a 100644 --- a/kyaml/kio/byteio_writer_test.go +++ b/kyaml/kio/byteio_writer_test.go @@ -299,7 +299,13 @@ g: `, }, - expectedOutput: `a: b #first + expectedOutput: `e: f +g: + h: + - i # has a list + - j +--- +a: b #first metadata: annotations: internal.config.kubernetes.io/path: "a/b/a_test.yaml" @@ -310,12 +316,6 @@ metadata: annotations: internal.config.kubernetes.io/path: "a/b/a_test.yaml" config.kubernetes.io/path: 'a/b/a_test.yaml' ---- -e: f -g: - h: - - i # has a list - - j `, }, diff --git a/kyaml/kio/kio.go b/kyaml/kio/kio.go index 7b39438399f..4536551c16f 100644 --- a/kyaml/kio/kio.go +++ b/kyaml/kio/kio.go @@ -185,6 +185,22 @@ func storeInternalAnnotations(result []*yaml.RNode) (map[string]map[string]strin index := meta.Annotations[kioutil.IndexAnnotation] id := meta.Annotations[kioutil.IdAnnotation] + legacyPath := meta.Annotations[kioutil.LegacyPathAnnotation] + legacyIndex := meta.Annotations[kioutil.LegacyIndexAnnotation] + legacyId := meta.Annotations[kioutil.LegacyIdAnnotation] + + // if prior to running the functions, the legacy and internal annotations differ, + // throw an error as we cannot infer the user's intent. + if path != legacyPath { + return nil, fmt.Errorf("resource input to function has mismatched legacy and internal path annotations") + } + if index != legacyIndex { + return nil, fmt.Errorf("resource input to function has mismatched legacy and internal index annotations") + } + if id != legacyId { + return nil, fmt.Errorf("resource input to function has mismatched legacy and internal id annotations") + } + if _, ok := nodeAnnosMap[path]; !ok { nodeAnnosMap[path] = make(map[string]string) } diff --git a/kyaml/kio/kio_test.go b/kyaml/kio/kio_test.go index 4c73ecd349b..60027108eba 100644 --- a/kyaml/kio/kio_test.go +++ b/kyaml/kio/kio_test.go @@ -233,6 +233,17 @@ func TestLegacyAnnotationReconciliation(t *testing.T) { } return nodes, nil } + changeBothPathAnnos := func(nodes []*yaml.RNode) ([]*yaml.RNode, error) { + for _, rn := range nodes { + if err := rn.PipeE(yaml.SetAnnotation(kioutil.LegacyPathAnnotation, "legacy")); err != nil { + return nil, err + } + if err := rn.PipeE(yaml.SetAnnotation(kioutil.PathAnnotation, "new")); err != nil { + return nil, err + } + } + return nodes, nil + } noops := []Filter{ FilterFunc(noopFilter1), @@ -242,11 +253,13 @@ func TestLegacyAnnotationReconciliation(t *testing.T) { legacy := []Filter{FilterFunc(changeLegacyAnnos)} legacyId := []Filter{FilterFunc(changeLegacyId)} internalId := []Filter{FilterFunc(changeInternalId)} + changeBoth := []Filter{FilterFunc(changeBothPathAnnos), FilterFunc(noopFilter1)} testCases := map[string]struct { - input string - filters []Filter - expected string + input string + filters []Filter + expected string + expectedErr string }{ // the orchestrator should copy the legacy annotations to the new // annotations @@ -510,6 +523,32 @@ data: grpcPort: 8080 `, }, + // the function changes both the legacy and new path annotation, + // so we should get an error + "change both": { + input: `apiVersion: v1 +kind: ConfigMap +metadata: + name: ports-from + annotations: + config.kubernetes.io/path: 'configmap.yaml' + internal.kubernetes.io/path: 'configmap.yaml' +data: + grpcPort: 8080 +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: ports-to + annotations: + config.kubernetes.io/path: "configmap.yaml" + config.kubernetes.io/index: '1' +data: + grpcPort: 8081 +`, + filters: changeBoth, + expectedErr: "resource input to function has mismatched legacy and internal path annotations", + }, } for tn, tc := range testCases { @@ -526,8 +565,16 @@ data: Filters: tc.filters, Outputs: []Writer{&input}, } - assert.NoError(t, p.Execute()) - assert.Equal(t, tc.expected, out.String()) + + err := p.Execute() + if tc.expectedErr == "" { + assert.NoError(t, err) + assert.Equal(t, tc.expected, out.String()) + } else { + assert.Error(t, err) + assert.Equal(t, tc.expectedErr, err.Error()) + } + }) } } diff --git a/kyaml/kio/kioutil/kioutil.go b/kyaml/kio/kioutil/kioutil.go index bcd97303007..bfe2e9334c3 100644 --- a/kyaml/kio/kioutil/kioutil.go +++ b/kyaml/kio/kioutil/kioutil.go @@ -55,6 +55,10 @@ func GetFileAnnotations(rn *yaml.RNode) (string, string, error) { func CopyLegacyAnnotations(rn *yaml.RNode) error { meta, err := rn.GetMeta() if err != nil { + if err == yaml.ErrMissingMetadata { + // resource has no metadata, this should be a no-op + return nil + } return err } if err := copyAnnotations(meta, rn, LegacyPathAnnotation, PathAnnotation); err != nil { @@ -71,12 +75,14 @@ func CopyLegacyAnnotations(rn *yaml.RNode) error { func copyAnnotations(meta yaml.ResourceMeta, rn *yaml.RNode, legacyKey string, newKey string) error { newValue := meta.Annotations[newKey] + legacyValue := meta.Annotations[legacyKey] if newValue != "" { - if err := rn.PipeE(yaml.SetAnnotation(legacyKey, newValue)); err != nil { - return err + if legacyValue == "" { + if err := rn.PipeE(yaml.SetAnnotation(legacyKey, newValue)); err != nil { + return err + } } } else { - legacyValue := meta.Annotations[legacyKey] if legacyValue != "" { if err := rn.PipeE(yaml.SetAnnotation(newKey, legacyValue)); err != nil { return err diff --git a/kyaml/kio/kioutil/kioutil_test.go b/kyaml/kio/kioutil/kioutil_test.go index 6fedf21fb5a..bf6c07b4eaa 100644 --- a/kyaml/kio/kioutil/kioutil_test.go +++ b/kyaml/kio/kioutil/kioutil_test.go @@ -374,3 +374,80 @@ func TestCreatePathAnnotationValue(t *testing.T) { } } } + +func TestCopyLegacyAnnotations(t *testing.T) { + var tests = []struct { + input string + expected string + }{ + { + input: `apiVersion: v1 +kind: Foo +metadata: + name: foobar + annotations: + config.kubernetes.io/path: 'a/b.yaml' + config.kubernetes.io/index: '5' +`, + expected: `apiVersion: v1 +kind: Foo +metadata: + name: foobar + annotations: + config.kubernetes.io/path: 'a/b.yaml' + config.kubernetes.io/index: '5' + internal.config.kubernetes.io/path: 'a/b.yaml' + internal.config.kubernetes.io/index: '5' +`, + }, + { + input: `apiVersion: v1 +kind: Foo +metadata: + name: foobar + annotations: + internal.config.kubernetes.io/path: 'a/b.yaml' + internal.config.kubernetes.io/index: '5' +`, + expected: `apiVersion: v1 +kind: Foo +metadata: + name: foobar + annotations: + internal.config.kubernetes.io/path: 'a/b.yaml' + internal.config.kubernetes.io/index: '5' + config.kubernetes.io/path: 'a/b.yaml' + config.kubernetes.io/index: '5' +`, + }, + { + input: `apiVersion: v1 +kind: Foo +metadata: + name: foobar + annotations: + internal.config.kubernetes.io/path: 'a/b.yaml' + config.kubernetes.io/path: 'c/d.yaml' +`, + expected: `apiVersion: v1 +kind: Foo +metadata: + name: foobar + annotations: + internal.config.kubernetes.io/path: 'a/b.yaml' + config.kubernetes.io/path: 'c/d.yaml' +`, + }, + } + + for _, tc := range tests { + rw := kio.ByteReadWriter{ + Reader: bytes.NewBufferString(tc.input), + OmitReaderAnnotations: true, + } + nodes, err := rw.Read() + assert.NoError(t, err) + assert.NoError(t, kioutil.CopyLegacyAnnotations(nodes[0])) + assert.Equal(t, tc.expected, nodes[0].MustString()) + } +}