diff --git a/api/filters/fieldspec/fieldspec_test.go b/api/filters/fieldspec/fieldspec_test.go index d416191b5d..49f38e6caa 100644 --- a/api/filters/fieldspec/fieldspec_test.go +++ b/api/filters/fieldspec/fieldspec_test.go @@ -63,6 +63,9 @@ xxx: apiVersion: foo/v1 kind: Bar xxx: +metadata: + annotations: + internal.config.k8s.io/annotations-migration-resource-id: '0' : cannot set or create an empty field name`, filter: fieldspec.Filter{ SetValue: filtersutil.SetScalar("e"), @@ -220,6 +223,9 @@ a: kind: Bar a: b: a +metadata: + annotations: + internal.config.k8s.io/annotations-migration-resource-id: '0' : expected sequence or mapping node`, filter: fieldspec.Filter{ SetValue: filtersutil.SetScalar("e"), diff --git a/api/filters/refvar/refvar_test.go b/api/filters/refvar/refvar_test.go index 3fd3eddf08..67e1ee03db 100644 --- a/api/filters/refvar/refvar_test.go +++ b/api/filters/refvar/refvar_test.go @@ -251,6 +251,7 @@ metadata: annotations: config.kubernetes.io/index: '0' internal.config.kubernetes.io/index: '0' + internal.config.k8s.io/annotations-migration-resource-id: '0' data: slice: - false @@ -278,6 +279,7 @@ metadata: annotations: config.kubernetes.io/index: '0' internal.config.kubernetes.io/index: '0' + internal.config.k8s.io/annotations-migration-resource-id: '0' data: 1: str : invalid map key: value='1', tag='` + yaml.NodeTagInt + `'`, diff --git a/api/resource/resource.go b/api/resource/resource.go index 4985f72556..7f8bfbd7db 100644 --- a/api/resource/resource.go +++ b/api/resource/resource.go @@ -42,9 +42,12 @@ var BuildAnnotations = []string{ kioutil.PathAnnotation, kioutil.IndexAnnotation, kioutil.SeqIndentAnnotation, + kioutil.IdAnnotation, + kioutil.InternalAnnotationsMigrationResourceIDAnnotation, kioutil.LegacyPathAnnotation, kioutil.LegacyIndexAnnotation, + kioutil.LegacyIdAnnotation, } func (r *Resource) ResetRNode(incoming *Resource) { diff --git a/cmd/config/internal/commands/e2e/e2e_test.go b/cmd/config/internal/commands/e2e/e2e_test.go index 54749ed9f2..d0bf7d0927 100644 --- a/cmd/config/internal/commands/e2e/e2e_test.go +++ b/cmd/config/internal/commands/e2e/e2e_test.go @@ -293,6 +293,8 @@ apiVersion: apps/v1 kind: Deployment metadata: name: foo + annotations: + internal.config.k8s.io/annotations-migration-resource-id: '1' `, } }, diff --git a/kyaml/fn/framework/framework.go b/kyaml/fn/framework/framework.go index b1ddbc3311..d37c88f8ad 100644 --- a/kyaml/fn/framework/framework.go +++ b/kyaml/fn/framework/framework.go @@ -116,8 +116,21 @@ func Execute(p ResourceListProcessor, rlSource *kio.ByteReadWriter) error { } rl.FunctionConfig = rlSource.FunctionConfig + // We store the original + nodeAnnos, err := kio.PreprocessResourcesForInternalAnnotationMigration(rl.Items) + if err != nil { + return err + } + retErr := p.Process(&rl) + // If either the internal annotations for path, index, and id OR the legacy + // annotations for path, index, and id are changed, we have to update the other. + err = kio.ReconcileInternalAnnotations(rl.Items, nodeAnnos) + if err != nil { + return err + } + // Write the results // Set the ResourceList.results for validating functions if len(rl.Results) > 0 { diff --git a/kyaml/fn/framework/result.go b/kyaml/fn/framework/result.go index b0db3717a1..9d9065ea60 100644 --- a/kyaml/fn/framework/result.go +++ b/kyaml/fn/framework/result.go @@ -64,7 +64,12 @@ func (i Result) String() string { } } formatString := "[%s]" - list := []interface{}{i.Severity} + severity := i.Severity + // We default Severity to Info when converting a result to a message. + if i.Severity == "" { + severity = Info + } + list := []interface{}{severity} if len(idStringList) > 0 { formatString += " %s" list = append(list, strings.Join(idStringList, "/")) diff --git a/kyaml/kio/byteio_writer_test.go b/kyaml/kio/byteio_writer_test.go index 835ed20cb4..4897287b04 100644 --- a/kyaml/kio/byteio_writer_test.go +++ b/kyaml/kio/byteio_writer_test.go @@ -400,7 +400,6 @@ metadata: "a": "a long string that would certainly see a newline introduced by the YAML marshaller abcd123", "metadata": { "annotations": { - "config.kubernetes.io/path": "test.json", "internal.config.kubernetes.io/path": "test.json" } } @@ -429,7 +428,6 @@ metadata: "a": "a long string that would certainly see a newline introduced by the YAML marshaller abcd123", "metadata": { "annotations": { - "config.kubernetes.io/path": "test.json", "internal.config.kubernetes.io/path": "test.json" } } @@ -449,7 +447,6 @@ metadata: "a": "b", "metadata": { "annotations": { - "config.kubernetes.io/path": "test.json", "internal.config.kubernetes.io/path": "test.json" } } diff --git a/kyaml/kio/kio.go b/kyaml/kio/kio.go index 410c0a23fe..10388acd3b 100644 --- a/kyaml/kio/kio.go +++ b/kyaml/kio/kio.go @@ -7,6 +7,7 @@ package kio import ( "fmt" + "strconv" "sigs.k8s.io/kustomize/kyaml/errors" "sigs.k8s.io/kustomize/kyaml/kio/kioutil" @@ -114,13 +115,11 @@ func (p Pipeline) ExecuteWithCallback(callback PipelineExecuteCallbackFunc) erro } // apply operations - var err error for i := range p.Filters { // Not all RNodes passed through kio.Pipeline have metadata nor should // they all be required to. - var nodeAnnos map[string]map[string]string - nodeAnnos, err = storeInternalAnnotations(result) - if err != nil && err != yaml.ErrMissingMetadata { + nodeAnnos, err := PreprocessResourcesForInternalAnnotationMigration(result) + if err != nil { return err } @@ -138,8 +137,8 @@ func (p Pipeline) ExecuteWithCallback(callback PipelineExecuteCallbackFunc) erro // If either the internal annotations for path, index, and id OR the legacy // annotations for path, index, and id are changed, we have to update the other. - err = reconcileInternalAnnotations(result, nodeAnnos) - if err != nil && err != yaml.ErrMissingMetadata { + err = ReconcileInternalAnnotations(result, nodeAnnos) + if err != nil { return err } } @@ -166,55 +165,49 @@ func FilterAll(filter yaml.Filter) Filter { }) } -// Store the original path, index, and id annotations so that we can reconcile -// it later. This is necessary because currently both internal-prefixed annotations +// PreprocessResourcesForInternalAnnotationMigration returns a mapping from id to all +// internal annotations, so that we can use it to reconcile the annotations +// later. This is necessary because currently both internal-prefixed annotations // and legacy annotations are currently supported, and a change to one must be -// reflected in the other. -func storeInternalAnnotations(result []*yaml.RNode) (map[string]map[string]string, error) { - nodeAnnosMap := make(map[string]map[string]string) - +// reflected in the other if needed. +func PreprocessResourcesForInternalAnnotationMigration(result []*yaml.RNode) (map[string]map[string]string, error) { + idToAnnosMap := make(map[string]map[string]string) for i := range result { - if err := kioutil.CopyLegacyAnnotations(result[i]); err != nil { - return nil, err - } - meta, err := result[i].GetMeta() + idStr := strconv.Itoa(i) + err := result[i].PipeE(yaml.SetAnnotation(kioutil.InternalAnnotationsMigrationResourceIDAnnotation, idStr)) if err != nil { return nil, err } - if err := checkMismatchedAnnos(meta); err != nil { + idToAnnosMap[idStr] = kioutil.GetInternalAnnotations(result[i]) + if err = kioutil.CopyLegacyAnnotations(result[i]); err != nil { return nil, err } - - path := meta.Annotations[kioutil.PathAnnotation] - index := meta.Annotations[kioutil.IndexAnnotation] - id := meta.Annotations[kioutil.IdAnnotation] - - if _, ok := nodeAnnosMap[path]; !ok { - nodeAnnosMap[path] = make(map[string]string) + meta, _ := result[i].GetMeta() + if err = checkMismatchedAnnos(meta.Annotations); err != nil { + return nil, err } - nodeAnnosMap[path][index] = id } - return nodeAnnosMap, nil + return idToAnnosMap, nil } -func checkMismatchedAnnos(meta yaml.ResourceMeta) error { - path := meta.Annotations[kioutil.PathAnnotation] - index := meta.Annotations[kioutil.IndexAnnotation] - id := meta.Annotations[kioutil.IdAnnotation] +func checkMismatchedAnnos(annotations map[string]string) error { + path := annotations[kioutil.PathAnnotation] + index := annotations[kioutil.IndexAnnotation] + id := annotations[kioutil.IdAnnotation] - legacyPath := meta.Annotations[kioutil.LegacyPathAnnotation] - legacyIndex := meta.Annotations[kioutil.LegacyIndexAnnotation] - legacyId := meta.Annotations[kioutil.LegacyIdAnnotation] + legacyPath := annotations[kioutil.LegacyPathAnnotation] + legacyIndex := annotations[kioutil.LegacyIndexAnnotation] + legacyId := 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 { + if path != "" && legacyPath != "" && path != legacyPath { return fmt.Errorf("resource input to function has mismatched legacy and internal path annotations") } - if index != legacyIndex { + if index != "" && legacyIndex != "" && index != legacyIndex { return fmt.Errorf("resource input to function has mismatched legacy and internal index annotations") } - if id != legacyId { + if id != "" && legacyId != "" && id != legacyId { return fmt.Errorf("resource input to function has mismatched legacy and internal id annotations") } return nil @@ -226,53 +219,115 @@ type nodeAnnotations struct { id string } -func reconcileInternalAnnotations(result []*yaml.RNode, nodeAnnosMap map[string]map[string]string) error { - for _, node := range result { - meta, err := node.GetMeta() - if err != nil { - return err - } +// ReconcileInternalAnnotations reconciles the annotation format for path, index and id annotations. +// It will ensure the output annotation format matches the format in the input. e.g. if the input +// format uses the legacy format and the output will be converted to the legacy format if it's not. +func ReconcileInternalAnnotations(result []*yaml.RNode, nodeAnnosMap map[string]map[string]string) error { + useInternal, useLegacy, err := determineAnnotationsFormat(nodeAnnosMap) + if err != nil { + return err + } + + for i := range result { // if only one annotation is set, set the other. - err = missingInternalOrLegacyAnnotations(node, meta) + err = missingInternalOrLegacyAnnotations(result[i]) if err != nil { return err } // we must check to see if the function changed either the new internal annotations // or the old legacy annotations. If one is changed, the change must be reflected // in the other. - err = checkAnnotationsAltered(node, meta, nodeAnnosMap) + err = checkAnnotationsAltered(result[i], nodeAnnosMap) if err != nil { return err } - // if the annotations are still somehow out of sync, throw an error - meta, err = node.GetMeta() + // We invoke determineAnnotationsFormat to find out if the original annotations + // use the internal or (and) the legacy format. We format the resources to + // make them consistent with original format. + err = formatInternalAnnotations(result[i], useInternal, useLegacy) if err != nil { return err } - err = checkMismatchedAnnos(meta) + // if the annotations are still somehow out of sync, throw an error + meta, _ := result[i].GetMeta() + err = checkMismatchedAnnos(meta.Annotations) if err != nil { return err } + + if _, err = result[i].Pipe(yaml.ClearAnnotation(kioutil.InternalAnnotationsMigrationResourceIDAnnotation)); err != nil { + return err + } } return nil } -func missingInternalOrLegacyAnnotations(rn *yaml.RNode, meta yaml.ResourceMeta) error { - if err := missingInternalOrLegacyAnnotation(rn, meta, kioutil.PathAnnotation, kioutil.LegacyPathAnnotation); err != nil { +// determineAnnotationsFormat determines if the resources are using one of the internal and legacy annotation format or both of them. +func determineAnnotationsFormat(nodeAnnosMap map[string]map[string]string) (useInternal, useLegacy bool, err error) { + if len(nodeAnnosMap) == 0 { + return true, true, nil + } + + var internal, legacy *bool + for _, annos := range nodeAnnosMap { + _, foundPath := annos[kioutil.PathAnnotation] + _, foundIndex := annos[kioutil.IndexAnnotation] + _, foundId := annos[kioutil.IdAnnotation] + _, foundLegacyPath := annos[kioutil.LegacyPathAnnotation] + _, foundLegacyIndex := annos[kioutil.LegacyIndexAnnotation] + _, foundLegacyId := annos[kioutil.LegacyIdAnnotation] + + if !(foundPath || foundIndex || foundId || foundLegacyPath || foundLegacyIndex || foundLegacyId) { + continue + } + + foundOneOf := foundPath || foundIndex || foundId + if internal == nil { + f := foundOneOf + internal = &f + } + if (foundOneOf && !*internal) || (!foundOneOf && *internal) { + err = fmt.Errorf("the annotation formatting in the input resources is not consistent") + return + } + + foundOneOf = foundLegacyPath || foundLegacyIndex || foundLegacyId + if legacy == nil { + f := foundOneOf + legacy = &f + } + if (foundOneOf && !*legacy) || (!foundOneOf && *legacy) { + err = fmt.Errorf("the annotation formatting in the input resources is not consistent") + return + } + } + if internal != nil { + useInternal = *internal + } + if legacy != nil { + useLegacy = *legacy + } + return +} + +func missingInternalOrLegacyAnnotations(rn *yaml.RNode) error { + if err := missingInternalOrLegacyAnnotation(rn, kioutil.PathAnnotation, kioutil.LegacyPathAnnotation); err != nil { return err } - if err := missingInternalOrLegacyAnnotation(rn, meta, kioutil.IndexAnnotation, kioutil.LegacyIndexAnnotation); err != nil { + if err := missingInternalOrLegacyAnnotation(rn, kioutil.IndexAnnotation, kioutil.LegacyIndexAnnotation); err != nil { return err } - if err := missingInternalOrLegacyAnnotation(rn, meta, kioutil.IdAnnotation, kioutil.LegacyIdAnnotation); err != nil { + if err := missingInternalOrLegacyAnnotation(rn, kioutil.IdAnnotation, kioutil.LegacyIdAnnotation); err != nil { return err } return nil } -func missingInternalOrLegacyAnnotation(rn *yaml.RNode, meta yaml.ResourceMeta, newKey string, legacyKey string) error { - value := meta.Annotations[newKey] - legacyValue := meta.Annotations[legacyKey] +func missingInternalOrLegacyAnnotation(rn *yaml.RNode, newKey string, legacyKey string) error { + meta, _ := rn.GetMeta() + annotations := meta.Annotations + value := annotations[newKey] + legacyValue := annotations[legacyKey] if value == "" && legacyValue == "" { // do nothing @@ -293,98 +348,88 @@ func missingInternalOrLegacyAnnotation(rn *yaml.RNode, meta yaml.ResourceMeta, n return nil } -func checkAnnotationsAltered(rn *yaml.RNode, meta yaml.ResourceMeta, nodeAnnosMap map[string]map[string]string) error { +func checkAnnotationsAltered(rn *yaml.RNode, nodeAnnosMap map[string]map[string]string) error { + meta, _ := rn.GetMeta() + annotations := meta.Annotations // get the resource's current path, index, and ids from the new annotations internal := nodeAnnotations{ - path: meta.Annotations[kioutil.PathAnnotation], - index: meta.Annotations[kioutil.IndexAnnotation], - id: meta.Annotations[kioutil.IdAnnotation], + path: annotations[kioutil.PathAnnotation], + index: annotations[kioutil.IndexAnnotation], + id: annotations[kioutil.IdAnnotation], } // get the resource's current path, index, and ids from the legacy annotations legacy := nodeAnnotations{ - path: meta.Annotations[kioutil.LegacyPathAnnotation], - index: meta.Annotations[kioutil.LegacyIndexAnnotation], - id: meta.Annotations[kioutil.LegacyIdAnnotation], + path: annotations[kioutil.LegacyPathAnnotation], + index: annotations[kioutil.LegacyIndexAnnotation], + id: annotations[kioutil.LegacyIdAnnotation], } - if internal.path == legacy.path && - internal.index == legacy.index && - internal.id == legacy.id { - // none of the annotations differ, so no reconciliation is needed + rid := annotations[kioutil.InternalAnnotationsMigrationResourceIDAnnotation] + originalAnnotations, found := nodeAnnosMap[rid] + if !found { return nil } - - // nodeAnnosMap is a map of structure path -> index -> id that stores - // all of the resources' path/index/id annotations prior to the functions - // being run. We use that to check whether the legacy or new internal - // annotations have been changed, and make sure the change is reflected - // in the other. - - // first, check if the internal annotations are found in nodeAnnosMap - if indexIdMap, ok := nodeAnnosMap[internal.path]; ok { - if id, ok := indexIdMap[internal.index]; ok { - if id == internal.id { - // the internal annotations of the resource match the ones stored in - // nodeAnnosMap, so we should copy the legacy annotations to the - // internal ones - if err := updateAnnotations(rn, meta, - []string{ - kioutil.PathAnnotation, - kioutil.IndexAnnotation, - kioutil.IdAnnotation, - }, - []string{ - legacy.path, - legacy.index, - legacy.id, - }); err != nil { - return err - } + originalPath, found := originalAnnotations[kioutil.PathAnnotation] + if !found { + originalPath = originalAnnotations[kioutil.LegacyPathAnnotation] + } + if originalPath != "" { + if originalPath != internal.path && originalPath != legacy.path && internal.path != legacy.path { + return fmt.Errorf("resource input to function has mismatched legacy and internal path annotations") + } else if originalPath != internal.path { + if _, err := rn.Pipe(yaml.SetAnnotation(kioutil.LegacyPathAnnotation, internal.path)); err != nil { + return err + } + } else if originalPath != legacy.path { + if _, err := rn.Pipe(yaml.SetAnnotation(kioutil.PathAnnotation, legacy.path)); err != nil { + return err } } } - // check the opposite, to see if the legacy annotations are in nodeAnnosMap - if indexIdMap, ok := nodeAnnosMap[legacy.path]; ok { - if id, ok := indexIdMap[legacy.index]; ok { - if id == legacy.id { - // the legacy annotations of the resource match the ones stored in - // nodeAnnosMap, so we should copy the internal annotations to the - // legacy ones - if err := updateAnnotations(rn, meta, - []string{ - kioutil.LegacyPathAnnotation, - kioutil.LegacyIndexAnnotation, - kioutil.LegacyIdAnnotation, - }, - []string{ - internal.path, - internal.index, - internal.id, - }); err != nil { - return err - } + originalIndex, found := originalAnnotations[kioutil.IndexAnnotation] + if !found { + originalIndex = originalAnnotations[kioutil.LegacyIndexAnnotation] + } + if originalIndex != "" { + if originalIndex != internal.index && originalIndex != legacy.index && internal.index != legacy.index { + return fmt.Errorf("resource input to function has mismatched legacy and internal index annotations") + } else if originalIndex != internal.index { + if _, err := rn.Pipe(yaml.SetAnnotation(kioutil.LegacyIndexAnnotation, internal.index)); err != nil { + return err + } + } else if originalIndex != legacy.index { + if _, err := rn.Pipe(yaml.SetAnnotation(kioutil.IndexAnnotation, legacy.index)); err != nil { + return err } } } return nil } -func updateAnnotations(rn *yaml.RNode, meta yaml.ResourceMeta, keys []string, values []string) error { - if len(keys) != len(values) { - return fmt.Errorf("keys is not same length as values") +func formatInternalAnnotations(rn *yaml.RNode, useInternal, useLegacy bool) error { + if !useInternal { + if err := rn.PipeE(yaml.ClearAnnotation(kioutil.IdAnnotation)); err != nil { + return err + } + if err := rn.PipeE(yaml.ClearAnnotation(kioutil.PathAnnotation)); err != nil { + return err + } + if err := rn.PipeE(yaml.ClearAnnotation(kioutil.IndexAnnotation)); err != nil { + return err + } } - for i := range keys { - _, ok := meta.Annotations[keys[i]] - if values[i] == "" && !ok { - // don't set "" if annotation is not already there - continue + if !useLegacy { + if err := rn.PipeE(yaml.ClearAnnotation(kioutil.LegacyIdAnnotation)); err != nil { + return err } - if err := rn.PipeE(yaml.SetAnnotation(keys[i], values[i])); err != nil { + if err := rn.PipeE(yaml.ClearAnnotation(kioutil.LegacyPathAnnotation)); err != nil { + return err + } + if err := rn.PipeE(yaml.ClearAnnotation(kioutil.LegacyIndexAnnotation)); err != nil { return err } - } return nil } diff --git a/kyaml/kio/kio_test.go b/kyaml/kio/kio_test.go index 8fccf4b07f..e7e5d046a9 100644 --- a/kyaml/kio/kio_test.go +++ b/kyaml/kio/kio_test.go @@ -217,28 +217,23 @@ func TestLegacyAnnotationReconciliation(t *testing.T) { } return nodes, nil } - changeLegacyId := func(nodes []*yaml.RNode) ([]*yaml.RNode, error) { + changeBothPathAnnosMatch := func(nodes []*yaml.RNode) ([]*yaml.RNode, error) { for _, rn := range nodes { - if err := rn.PipeE(yaml.SetAnnotation(kioutil.LegacyIdAnnotation, "new")); err != nil { + if err := rn.PipeE(yaml.SetAnnotation(kioutil.LegacyPathAnnotation, "new")); err != nil { return nil, err } - } - return nodes, nil - } - changeInternalId := func(nodes []*yaml.RNode) ([]*yaml.RNode, error) { - for _, rn := range nodes { - if err := rn.PipeE(yaml.SetAnnotation(kioutil.IdAnnotation, "new")); err != nil { + if err := rn.PipeE(yaml.SetAnnotation(kioutil.PathAnnotation, "new")); err != nil { return nil, err } } return nodes, nil } - changeBothPathAnnos := func(nodes []*yaml.RNode) ([]*yaml.RNode, error) { + changeBothPathAnnosMismatch := func(nodes []*yaml.RNode) ([]*yaml.RNode, error) { for _, rn := range nodes { - if err := rn.PipeE(yaml.SetAnnotation(kioutil.LegacyPathAnnotation, "legacy")); err != nil { + if err := rn.PipeE(yaml.SetAnnotation(kioutil.LegacyPathAnnotation, "foo")); err != nil { return nil, err } - if err := rn.PipeE(yaml.SetAnnotation(kioutil.PathAnnotation, "new")); err != nil { + if err := rn.PipeE(yaml.SetAnnotation(kioutil.PathAnnotation, "bar")); err != nil { return nil, err } } @@ -251,9 +246,8 @@ func TestLegacyAnnotationReconciliation(t *testing.T) { } internal := []Filter{FilterFunc(changeInternalAnnos)} legacy := []Filter{FilterFunc(changeLegacyAnnos)} - legacyId := []Filter{FilterFunc(changeLegacyId)} - internalId := []Filter{FilterFunc(changeInternalId)} - changeBoth := []Filter{FilterFunc(changeBothPathAnnos), FilterFunc(noopFilter1)} + changeBothMatch := []Filter{FilterFunc(changeBothPathAnnosMatch), FilterFunc(noopFilter1)} + changeBothMismatch := []Filter{FilterFunc(changeBothPathAnnosMismatch), FilterFunc(noopFilter1)} testCases := map[string]struct { input string @@ -292,8 +286,6 @@ metadata: annotations: config.kubernetes.io/path: 'configmap.yaml' config.kubernetes.io/index: '0' - internal.config.kubernetes.io/path: 'configmap.yaml' - internal.config.kubernetes.io/index: '0' data: grpcPort: 8080 --- @@ -304,8 +296,6 @@ metadata: annotations: config.kubernetes.io/path: "configmap.yaml" config.kubernetes.io/index: '1' - internal.config.kubernetes.io/path: 'configmap.yaml' - internal.config.kubernetes.io/index: '1' data: grpcPort: 8081 `, @@ -341,6 +331,28 @@ metadata: annotations: internal.config.kubernetes.io/path: 'configmap.yaml' internal.config.kubernetes.io/index: '0' +data: + grpcPort: 8080 +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: ports-to + annotations: + internal.config.kubernetes.io/path: "configmap.yaml" + internal.config.kubernetes.io/index: '1' +data: + grpcPort: 8081 +`, + }, + // the orchestrator should detect that the legacy annotations + // have been changed by the function + "change only legacy annotations": { + input: `apiVersion: v1 +kind: ConfigMap +metadata: + name: ports-from + annotations: config.kubernetes.io/path: 'configmap.yaml' config.kubernetes.io/index: '0' data: @@ -348,21 +360,174 @@ data: --- apiVersion: v1 kind: ConfigMap +metadata: + name: ports-to + annotations: + config.kubernetes.io/path: "configmap.yaml" + config.kubernetes.io/index: '1' +data: + grpcPort: 8081 +`, + filters: legacy, + expected: `apiVersion: v1 +kind: ConfigMap +metadata: + name: ports-from + annotations: + config.kubernetes.io/path: 'new' + config.kubernetes.io/index: 'new' +data: + grpcPort: 8080 +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: ports-to + annotations: + config.kubernetes.io/path: "new" + config.kubernetes.io/index: 'new' +data: + grpcPort: 8081 +`, + }, + // the orchestrator should detect that the new internal annotations + // have been changed by the function + "change only internal annotations": { + input: `apiVersion: v1 +kind: ConfigMap +metadata: + name: ports-from + annotations: + internal.config.kubernetes.io/path: 'configmap.yaml' + internal.config.kubernetes.io/index: '0' +data: + grpcPort: 8080 +--- +apiVersion: v1 +kind: ConfigMap metadata: name: ports-to annotations: internal.config.kubernetes.io/path: "configmap.yaml" internal.config.kubernetes.io/index: '1' +data: + grpcPort: 8081 +`, + filters: internal, + expected: `apiVersion: v1 +kind: ConfigMap +metadata: + name: ports-from + annotations: + internal.config.kubernetes.io/path: 'new' + internal.config.kubernetes.io/index: 'new' +data: + grpcPort: 8080 +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: ports-to + annotations: + internal.config.kubernetes.io/path: "new" + internal.config.kubernetes.io/index: 'new' +data: + grpcPort: 8081 +`, + }, + // the orchestrator should detect that the legacy annotations + // have been changed by the function + "change only internal annotations while input is legacy annotations": { + input: `apiVersion: v1 +kind: ConfigMap +metadata: + name: ports-from + annotations: config.kubernetes.io/path: 'configmap.yaml' + config.kubernetes.io/index: '0' +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: internal, + expected: `apiVersion: v1 +kind: ConfigMap +metadata: + name: ports-from + annotations: + config.kubernetes.io/path: 'new' + config.kubernetes.io/index: 'new' +data: + grpcPort: 8080 +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: ports-to + annotations: + config.kubernetes.io/path: "new" + config.kubernetes.io/index: 'new' +data: + grpcPort: 8081 +`, + }, + // the orchestrator should detect that the new internal annotations + // have been changed by the function + "change only legacy annotations while input is internal annotations": { + input: `apiVersion: v1 +kind: ConfigMap +metadata: + name: ports-from + annotations: + internal.config.kubernetes.io/path: 'configmap.yaml' + internal.config.kubernetes.io/index: '0' +data: + grpcPort: 8080 +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: ports-to + annotations: + internal.config.kubernetes.io/path: "configmap.yaml" + internal.config.kubernetes.io/index: '1' +data: + grpcPort: 8081 +`, + filters: legacy, + expected: `apiVersion: v1 +kind: ConfigMap +metadata: + name: ports-from + annotations: + internal.config.kubernetes.io/path: 'new' + internal.config.kubernetes.io/index: 'new' +data: + grpcPort: 8080 +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: ports-to + annotations: + internal.config.kubernetes.io/path: "new" + internal.config.kubernetes.io/index: 'new' +data: + grpcPort: 8081 `, }, // the orchestrator should detect that the legacy annotations - // have been changed by the function, and should update the - // new internal annotations to reflect the same change - "change only legacy annotations": { + // have been changed by the function + "change only legacy annotations while input has both": { input: `apiVersion: v1 kind: ConfigMap metadata: @@ -370,6 +535,8 @@ metadata: annotations: config.kubernetes.io/path: 'configmap.yaml' config.kubernetes.io/index: '0' + internal.config.kubernetes.io/path: 'configmap.yaml' + internal.config.kubernetes.io/index: '0' data: grpcPort: 8080 --- @@ -380,6 +547,8 @@ metadata: annotations: config.kubernetes.io/path: "configmap.yaml" config.kubernetes.io/index: '1' + internal.config.kubernetes.io/path: 'configmap.yaml' + internal.config.kubernetes.io/index: '1' data: grpcPort: 8081 `, @@ -410,16 +579,17 @@ data: `, }, // the orchestrator should detect that the new internal annotations - // have been changed by the function, and should update the - // legacy annotations to reflect the same change - "change only internal annotations": { + // have been changed by the function + "change only internal annotations while input has both": { input: `apiVersion: v1 kind: ConfigMap metadata: name: ports-from annotations: - config.kubernetes.io/path: 'configmap.yaml' + config.kubernetes.io/path: "configmap.yaml" config.kubernetes.io/index: '0' + internal.config.kubernetes.io/path: 'configmap.yaml' + internal.config.kubernetes.io/index: '0' data: grpcPort: 8080 --- @@ -430,6 +600,8 @@ metadata: annotations: config.kubernetes.io/path: "configmap.yaml" config.kubernetes.io/index: '1' + internal.config.kubernetes.io/path: "configmap.yaml" + internal.config.kubernetes.io/index: '1' data: grpcPort: 8081 `, @@ -439,7 +611,7 @@ kind: ConfigMap metadata: name: ports-from annotations: - config.kubernetes.io/path: 'new' + config.kubernetes.io/path: "new" config.kubernetes.io/index: 'new' internal.config.kubernetes.io/path: 'new' internal.config.kubernetes.io/index: 'new' @@ -453,49 +625,113 @@ metadata: annotations: config.kubernetes.io/path: "new" config.kubernetes.io/index: 'new' - internal.config.kubernetes.io/path: 'new' + internal.config.kubernetes.io/path: "new" internal.config.kubernetes.io/index: 'new' data: grpcPort: 8081 `, }, - // the orchestrator should detect that the new internal id annotation - // has been changed, and copy it over to the legacy one, and also - // copy the path and index legacy annotations to the new internal - // ones - "change only internal id when original legacy set": { + // the orchestrator should detect that the new internal annotations + // have been changed by the function + "change both to matching value while input has both": { input: `apiVersion: v1 kind: ConfigMap metadata: name: ports-from annotations: - config.kubernetes.io/path: 'configmap.yaml' + config.kubernetes.io/path: "configmap.yaml" config.kubernetes.io/index: '0' - config.k8s.io/id: '1' + internal.config.kubernetes.io/path: 'configmap.yaml' + internal.config.kubernetes.io/index: '0' data: grpcPort: 8080 +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: ports-to + annotations: + config.kubernetes.io/path: "configmap.yaml" + config.kubernetes.io/index: '1' + internal.config.kubernetes.io/path: "configmap.yaml" + internal.config.kubernetes.io/index: '1' +data: + grpcPort: 8081 `, - filters: internalId, + filters: changeBothMatch, expected: `apiVersion: v1 kind: ConfigMap metadata: name: ports-from annotations: - config.kubernetes.io/path: 'configmap.yaml' + config.kubernetes.io/path: "new" config.kubernetes.io/index: '0' - config.k8s.io/id: 'new' - internal.config.kubernetes.io/path: 'configmap.yaml' + internal.config.kubernetes.io/path: 'new' internal.config.kubernetes.io/index: '0' - internal.config.kubernetes.io/id: 'new' data: grpcPort: 8080 +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: ports-to + annotations: + config.kubernetes.io/path: "new" + config.kubernetes.io/index: '1' + internal.config.kubernetes.io/path: "new" + internal.config.kubernetes.io/index: '1' +data: + grpcPort: 8081 +`, + }, + // the orchestrator should detect that the new internal annotations + // have been changed by the function + "change both to matching value while input is legacy": { + input: `apiVersion: v1 +kind: ConfigMap +metadata: + name: ports-from + annotations: + config.kubernetes.io/path: "configmap.yaml" + config.kubernetes.io/index: '0' +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: changeBothMatch, + expected: `apiVersion: v1 +kind: ConfigMap +metadata: + name: ports-from + annotations: + config.kubernetes.io/path: "new" + config.kubernetes.io/index: '0' +data: + grpcPort: 8080 +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: ports-to + annotations: + config.kubernetes.io/path: "new" + config.kubernetes.io/index: '1' +data: + grpcPort: 8081 `, }, - // the orchestrator should detect that the legacy id annotation - // has been changed, and copy it over to the internal one, and also - // copy the path and index internal annotations to the legacy - // ones - "change only legacy id when internal legacy set": { + // the orchestrator should detect that the new internal annotations + // have been changed by the function + "change both to matching value while input is internal": { input: `apiVersion: v1 kind: ConfigMap metadata: @@ -503,36 +739,106 @@ metadata: annotations: internal.config.kubernetes.io/path: 'configmap.yaml' internal.config.kubernetes.io/index: '0' - internal.config.kubernetes.io/id: '1' data: grpcPort: 8080 +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: ports-to + annotations: + internal.config.kubernetes.io/path: "configmap.yaml" + internal.config.kubernetes.io/index: '1' +data: + grpcPort: 8081 `, - filters: legacyId, + filters: changeBothMatch, expected: `apiVersion: v1 kind: ConfigMap metadata: name: ports-from annotations: - internal.config.kubernetes.io/path: 'configmap.yaml' + internal.config.kubernetes.io/path: 'new' internal.config.kubernetes.io/index: '0' - internal.config.kubernetes.io/id: 'new' +data: + grpcPort: 8080 +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: ports-to + annotations: + internal.config.kubernetes.io/path: "new" + internal.config.kubernetes.io/index: '1' +data: + grpcPort: 8081 +`, + }, + // the function changes both the legacy and new path annotation, and they mismatch, + // so we should get an error + "change both but mismatch while input is legacy": { + input: `apiVersion: v1 +kind: ConfigMap +metadata: + name: ports-from + annotations: config.kubernetes.io/path: 'configmap.yaml' config.kubernetes.io/index: '0' - config.k8s.io/id: 'new' 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: changeBothMismatch, + expectedErr: "resource input to function has mismatched legacy and internal path annotations", + }, + // the function changes both the legacy and new path annotation, and they mismatch, + // so we should get an error + "change both but mismatch while input is internal": { + input: `apiVersion: v1 +kind: ConfigMap +metadata: + name: ports-from + annotations: + internal.config.kubernetes.io/path: "configmap.yaml" + internal.config.kubernetes.io/index: '0' +data: + grpcPort: 8080 +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: ports-to + annotations: + internal.config.kubernetes.io/path: "configmap.yaml" + internal.config.kubernetes.io/index: '1' +data: + grpcPort: 8081 `, + filters: changeBothMismatch, + expectedErr: "resource input to function has mismatched legacy and internal path annotations", }, - // the function changes both the legacy and new path annotation, + // the function changes both the legacy and new path annotation, and they mismatch, // so we should get an error - "change both": { + "change both but mismatch while input has both": { input: `apiVersion: v1 kind: ConfigMap metadata: name: ports-from annotations: config.kubernetes.io/path: 'configmap.yaml' - internal.kubernetes.io/path: 'configmap.yaml' + config.kubernetes.io/index: '0' + config.k8s.io/id: '1' + internal.config.kubernetes.io/path: "configmap.yaml" + internal.config.kubernetes.io/index: '0' data: grpcPort: 8080 --- @@ -543,10 +849,13 @@ metadata: annotations: config.kubernetes.io/path: "configmap.yaml" config.kubernetes.io/index: '1' + config.k8s.io/id: '2' + internal.config.kubernetes.io/path: "configmap.yaml" + internal.config.kubernetes.io/index: '1' data: grpcPort: 8081 `, - filters: changeBoth, + filters: changeBothMismatch, expectedErr: "resource input to function has mismatched legacy and internal path annotations", }, } diff --git a/kyaml/kio/kioutil/kioutil.go b/kyaml/kio/kioutil/kioutil.go index 12cf85b11e..7d1a852738 100644 --- a/kyaml/kio/kioutil/kioutil.go +++ b/kyaml/kio/kioutil/kioutil.go @@ -41,21 +41,37 @@ const ( // Deprecated: use IdAnnotation instead. LegacyIdAnnotation = "config.k8s.io/id" + + // InternalAnnotationsMigrationResourceIDAnnotation is used to uniquely identify + // resources during round trip to and from a function execution. We will use it + // to track the internal annotations and reconcile them if needed. + InternalAnnotationsMigrationResourceIDAnnotation = "internal.config.k8s.io/annotations-migration-resource-id" ) func GetFileAnnotations(rn *yaml.RNode) (string, string, error) { - if err := CopyLegacyAnnotations(rn); err != nil { - return "", "", err + rm, _ := rn.GetMeta() + annotations := rm.Annotations + path, found := annotations[PathAnnotation] + if !found { + path = annotations[LegacyPathAnnotation] } - meta, err := rn.GetMeta() - if err != nil { - return "", "", err + index, found := annotations[IndexAnnotation] + if !found { + index = annotations[LegacyIndexAnnotation] } - path := meta.Annotations[PathAnnotation] - index := meta.Annotations[IndexAnnotation] return path, index, nil } +func GetIdAnnotation(rn *yaml.RNode) string { + rm, _ := rn.GetMeta() + annotations := rm.Annotations + id, found := annotations[IdAnnotation] + if !found { + id = annotations[LegacyIdAnnotation] + } + return id +} + func CopyLegacyAnnotations(rn *yaml.RNode) error { meta, err := rn.GetMeta() if err != nil { @@ -377,13 +393,16 @@ func ConfirmInternalAnnotationUnchanged(r1 *yaml.RNode, r2 *yaml.RNode, exclusio return nil } -// GetInternalAnnotations returns a map of all the annotations of the provided RNode that begin -// with the prefix `internal.config.kubernetes.io` +// GetInternalAnnotations returns a map of all the annotations of the provided +// RNode that satisfies one of the following: 1) begin with the prefix +// `internal.config.kubernetes.io` 2) is one of `config.kubernetes.io/path`, +// `config.kubernetes.io/index` and `config.k8s.io/id`. func GetInternalAnnotations(rn *yaml.RNode) map[string]string { - annotations := rn.GetAnnotations() + meta, _ := rn.GetMeta() + annotations := meta.Annotations result := make(map[string]string) for k, v := range annotations { - if strings.HasPrefix(k, internalPrefix) { + if strings.HasPrefix(k, internalPrefix) || k == LegacyPathAnnotation || k == LegacyIndexAnnotation || k == LegacyIdAnnotation { result[k] = v } }