From 5caed5b90ac08af141ccd195f8c7c264148320a2 Mon Sep 17 00:00:00 2001
From: Mengqi Yu <mengqiy@google.com>
Date: Wed, 17 Nov 2021 14:06:20 -0800
Subject: [PATCH 1/7] kyaml/fn/framework ensures the annotation output format
 matches the input

If the input only contains legacy format anntations (path, index, id), the
output will be the same.
---
 kyaml/fn/framework/framework.go |  13 ++
 kyaml/kio/byteio_writer_test.go |   3 -
 kyaml/kio/kio.go                | 275 ++++++++++++++++++--------------
 kyaml/kio/kio_test.go           |  82 ----------
 kyaml/kio/kioutil/kioutil.go    |  31 ++--
 5 files changed, 186 insertions(+), 218 deletions(-)

diff --git a/kyaml/fn/framework/framework.go b/kyaml/fn/framework/framework.go
index b1ddbc331..62b11a9b0 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.StoreInternalAnnotations(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/kio/byteio_writer_test.go b/kyaml/kio/byteio_writer_test.go
index 835ed20cb..4897287b0 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 410c0a23f..e2361e693 100644
--- a/kyaml/kio/kio.go
+++ b/kyaml/kio/kio.go
@@ -7,7 +7,6 @@ package kio
 
 import (
 	"fmt"
-
 	"sigs.k8s.io/kustomize/kyaml/errors"
 	"sigs.k8s.io/kustomize/kyaml/kio/kioutil"
 	"sigs.k8s.io/kustomize/kyaml/yaml"
@@ -114,13 +113,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 := StoreInternalAnnotations(result)
+		if err != nil {
 			return err
 		}
 
@@ -138,8 +135,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, false)
+		if err != nil {
 			return err
 		}
 	}
@@ -166,55 +163,51 @@ func FilterAll(filter yaml.Filter) Filter {
 	})
 }
 
-// Store the original path, index, and id annotations so that we can reconcile
+// StoreInternalAnnotations stores the original path, index, and id annotations so that we can reconcile
 // it 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)
+func StoreInternalAnnotations(result []*yaml.RNode) (map[nodeAnnotations]map[string]string, error) {
+	nodeAnnosMap := make(map[nodeAnnotations]map[string]string)
 
 	for i := range result {
+		id := kioutil.GetIdAnnotation(result[i])
+		path, index, _ := kioutil.GetFileAnnotations(result[i])
+		annoKey := nodeAnnotations{
+			path:  path,
+			index: index,
+			id:    id,
+		}
+		nodeAnnosMap[annoKey] = kioutil.GetInternalAnnotations(result[i])
 		if err := kioutil.CopyLegacyAnnotations(result[i]); err != nil {
 			return nil, err
 		}
-		meta, err := result[i].GetMeta()
-		if err != nil {
+
+		if err := checkMismatchedAnnos(result[i].GetAnnotations()); err != nil {
 			return nil, err
 		}
-		if err := checkMismatchedAnnos(meta); 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)
-		}
-		nodeAnnosMap[path][index] = id
 	}
 	return nodeAnnosMap, 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,30 +219,41 @@ 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 {
+// ReconcileInternalAnnotations reconciles the annotation format for path, index and id annotations.
+func ReconcileInternalAnnotations(result []*yaml.RNode, nodeAnnosMap map[nodeAnnotations]map[string]string) error {
+	return reconcileInternalAnnotations(result, nodeAnnosMap, true)
+}
+
+func reconcileInternalAnnotations(result []*yaml.RNode, nodeAnnosMap map[nodeAnnotations]map[string]string, enforceAnnotationsFormat bool) error {
+	var useInternal, useLegacy bool
+	var err error
+	if enforceAnnotationsFormat {
+		if useInternal, useLegacy, err = determineAnnotationsFormat(nodeAnnosMap); 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 enforceAnnotationsFormat {
+			err = enforceConsistentAnnotationFormat(result[i], useInternal, useLegacy)
+			if err != nil {
+				return err
+			}
+		}
 		// if the annotations are still somehow out of sync, throw an error
-		meta, err = node.GetMeta()
-		if err != nil {
-			return err
-		}
-		err = checkMismatchedAnnos(meta)
+		err = checkMismatchedAnnos(result[i].GetAnnotations())
 		if err != nil {
 			return err
 		}
@@ -257,22 +261,63 @@ func reconcileInternalAnnotations(result []*yaml.RNode, nodeAnnosMap map[string]
 	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[nodeAnnotations]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]
+		foundOneOf := foundPath || foundIndex || foundId
+		if internal == nil {
+			internal = &foundOneOf
+		}
+		if (foundOneOf && !*internal) || (!foundOneOf && *internal) {
+			err = fmt.Errorf("the formatting in the input resources is not consistent")
+			return
+		}
+
+		_, foundPath = annos[kioutil.LegacyPathAnnotation]
+		_, foundIndex = annos[kioutil.LegacyIndexAnnotation]
+		_, foundId = annos[kioutil.LegacyIdAnnotation]
+		foundOneOf = foundPath || foundIndex || foundId
+		if legacy == nil {
+			legacy = &foundOneOf
+		}
+		if (foundOneOf && !*legacy) || (!foundOneOf && *legacy) {
+			err = fmt.Errorf("the 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 {
+	value := rn.GetAnnotations()[newKey]
+	legacyValue := rn.GetAnnotations()[legacyKey]
 
 	if value == "" && legacyValue == "" {
 		// do nothing
@@ -293,98 +338,82 @@ 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[nodeAnnotations]map[string]string) error {
+	annotations := rn.GetAnnotations()
 	// 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
-		return nil
+	originalAnnotations, found := nodeAnnosMap[internal]
+	if !found {
+		originalAnnotations, found = nodeAnnosMap[legacy]
 	}
-
-	// 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 {
+			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 {
+			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")
-	}
-	for i := range keys {
-		_, ok := meta.Annotations[keys[i]]
-		if values[i] == "" && !ok {
-			// don't set "" if annotation is not already there
-			continue
-		}
-		if err := rn.PipeE(yaml.SetAnnotation(keys[i], values[i])); err != nil {
+func enforceConsistentAnnotationFormat(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
+		}
+	}
+	if !useLegacy {
+		if err := rn.PipeE(yaml.ClearAnnotation(kioutil.LegacyIdAnnotation)); err != nil {
+			return err
+		}
+		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 8fccf4b07..b63b59b01 100644
--- a/kyaml/kio/kio_test.go
+++ b/kyaml/kio/kio_test.go
@@ -217,22 +217,6 @@ func TestLegacyAnnotationReconciliation(t *testing.T) {
 		}
 		return nodes, nil
 	}
-	changeLegacyId := func(nodes []*yaml.RNode) ([]*yaml.RNode, error) {
-		for _, rn := range nodes {
-			if err := rn.PipeE(yaml.SetAnnotation(kioutil.LegacyIdAnnotation, "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 {
-				return nil, err
-			}
-		}
-		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 {
@@ -251,8 +235,6 @@ 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)}
 
 	testCases := map[string]struct {
@@ -457,70 +439,6 @@ metadata:
     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": {
-			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: '1'
-data:
-  grpcPort: 8080
-`,
-			filters: internalId,
-			expected: `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'
-    internal.config.kubernetes.io/path: 'configmap.yaml'
-    internal.config.kubernetes.io/index: '0'
-    internal.config.kubernetes.io/id: 'new'
-data:
-  grpcPort: 8080
-`,
-		},
-		// 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": {
-			input: `apiVersion: v1
-kind: ConfigMap
-metadata:
-  name: ports-from
-  annotations:
-    internal.config.kubernetes.io/path: 'configmap.yaml'
-    internal.config.kubernetes.io/index: '0'
-    internal.config.kubernetes.io/id: '1'
-data:
-  grpcPort: 8080
-`,
-			filters: legacyId,
-			expected: `apiVersion: v1
-kind: ConfigMap
-metadata:
-  name: ports-from
-  annotations:
-    internal.config.kubernetes.io/path: 'configmap.yaml'
-    internal.config.kubernetes.io/index: '0'
-    internal.config.kubernetes.io/id: 'new'
-    config.kubernetes.io/path: 'configmap.yaml'
-    config.kubernetes.io/index: '0'
-    config.k8s.io/id: 'new'
-data:
-  grpcPort: 8080
 `,
 		},
 		// the function changes both the legacy and new path annotation,
diff --git a/kyaml/kio/kioutil/kioutil.go b/kyaml/kio/kioutil/kioutil.go
index 12cf85b11..800dcfe45 100644
--- a/kyaml/kio/kioutil/kioutil.go
+++ b/kyaml/kio/kioutil/kioutil.go
@@ -44,18 +44,27 @@ const (
 )
 
 func GetFileAnnotations(rn *yaml.RNode) (string, string, error) {
-	if err := CopyLegacyAnnotations(rn); err != nil {
-		return "", "", err
+	annotations := rn.GetAnnotations()
+	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 {
+	annotations := rn.GetAnnotations()
+	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 +386,15 @@ 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()
 	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
 		}
 	}

From 4e7aebc20c969838396b93056f382eda8a7a0101 Mon Sep 17 00:00:00 2001
From: Mengqi Yu <mengqiy@google.com>
Date: Thu, 18 Nov 2021 14:58:04 -0800
Subject: [PATCH 2/7] address comments

---
 kyaml/fn/framework/framework.go |  2 +-
 kyaml/kio/kio.go                | 24 ++++++++++++++++--------
 2 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/kyaml/fn/framework/framework.go b/kyaml/fn/framework/framework.go
index 62b11a9b0..2f7983dcf 100644
--- a/kyaml/fn/framework/framework.go
+++ b/kyaml/fn/framework/framework.go
@@ -117,7 +117,7 @@ func Execute(p ResourceListProcessor, rlSource *kio.ByteReadWriter) error {
 	rl.FunctionConfig = rlSource.FunctionConfig
 
 	// We store the original
-	nodeAnnos, err := kio.StoreInternalAnnotations(rl.Items)
+	nodeAnnos, err := kio.GetInternalAnnotationsFromResourceList(rl.Items)
 	if err != nil {
 		return err
 	}
diff --git a/kyaml/kio/kio.go b/kyaml/kio/kio.go
index e2361e693..d3c017ff0 100644
--- a/kyaml/kio/kio.go
+++ b/kyaml/kio/kio.go
@@ -7,6 +7,7 @@ package kio
 
 import (
 	"fmt"
+
 	"sigs.k8s.io/kustomize/kyaml/errors"
 	"sigs.k8s.io/kustomize/kyaml/kio/kioutil"
 	"sigs.k8s.io/kustomize/kyaml/yaml"
@@ -116,7 +117,7 @@ func (p Pipeline) ExecuteWithCallback(callback PipelineExecuteCallbackFunc) erro
 	for i := range p.Filters {
 		// Not all RNodes passed through kio.Pipeline have metadata nor should
 		// they all be required to.
-		nodeAnnos, err := StoreInternalAnnotations(result)
+		nodeAnnos, err := GetInternalAnnotationsFromResourceList(result)
 		if err != nil {
 			return err
 		}
@@ -163,11 +164,11 @@ func FilterAll(filter yaml.Filter) Filter {
 	})
 }
 
-// StoreInternalAnnotations stores the original path, index, and id annotations so that we can reconcile
+// GetInternalAnnotationsFromResourceList stores the original path, index, and id annotations so that we can reconcile
 // it 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[nodeAnnotations]map[string]string, error) {
+func GetInternalAnnotationsFromResourceList(result []*yaml.RNode) (map[nodeAnnotations]map[string]string, error) {
 	nodeAnnosMap := make(map[nodeAnnotations]map[string]string)
 
 	for i := range result {
@@ -224,10 +225,14 @@ func ReconcileInternalAnnotations(result []*yaml.RNode, nodeAnnosMap map[nodeAnn
 	return reconcileInternalAnnotations(result, nodeAnnosMap, true)
 }
 
-func reconcileInternalAnnotations(result []*yaml.RNode, nodeAnnosMap map[nodeAnnotations]map[string]string, enforceAnnotationsFormat bool) error {
+// reconcileInternalAnnotations reconciles the annotation format for path, index and id annotations.
+// If formatAnnotations is true, we 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[nodeAnnotations]map[string]string, formatAnnotations bool) error {
 	var useInternal, useLegacy bool
 	var err error
-	if enforceAnnotationsFormat {
+	if formatAnnotations {
 		if useInternal, useLegacy, err = determineAnnotationsFormat(nodeAnnosMap); err != nil {
 			return err
 		}
@@ -246,8 +251,11 @@ func reconcileInternalAnnotations(result []*yaml.RNode, nodeAnnosMap map[nodeAnn
 		if err != nil {
 			return err
 		}
-		if enforceAnnotationsFormat {
-			err = enforceConsistentAnnotationFormat(result[i], useInternal, useLegacy)
+		if formatAnnotations {
+			// 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
 			}
@@ -392,7 +400,7 @@ func checkAnnotationsAltered(rn *yaml.RNode, nodeAnnosMap map[nodeAnnotations]ma
 	return nil
 }
 
-func enforceConsistentAnnotationFormat(rn *yaml.RNode, useInternal, useLegacy bool) error {
+func formatInternalAnnotations(rn *yaml.RNode, useInternal, useLegacy bool) error {
 	if !useInternal {
 		if err := rn.PipeE(yaml.ClearAnnotation(kioutil.IdAnnotation)); err != nil {
 			return err

From 81edfb7ee80d8cb6b4f4049b88734d08e7d0a869 Mon Sep 17 00:00:00 2001
From: Mengqi Yu <mengqiy@google.com>
Date: Fri, 19 Nov 2021 01:25:03 -0800
Subject: [PATCH 3/7] default result.severity to info when formatting string

---
 kyaml/fn/framework/result.go | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/kyaml/fn/framework/result.go b/kyaml/fn/framework/result.go
index b0db3717a..9d9065ea6 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, "/"))

From 2ee2d3e389d33b8019fdcf678765e124bc60bd2e Mon Sep 17 00:00:00 2001
From: Mengqi Yu <mengqiy@google.com>
Date: Thu, 18 Nov 2021 19:53:46 -0800
Subject: [PATCH 4/7] use custom id as key of the mapping and make kio.Pipeline
 behave the same as framework.Execute

---
 kyaml/fn/framework/framework.go |   2 +-
 kyaml/kio/kio.go                | 136 +++++-----
 kyaml/kio/kio_test.go           | 451 +++++++++++++++++++++++++++++---
 kyaml/kio/kioutil/kioutil.go    |   9 +-
 4 files changed, 502 insertions(+), 96 deletions(-)

diff --git a/kyaml/fn/framework/framework.go b/kyaml/fn/framework/framework.go
index 2f7983dcf..d37c88f8a 100644
--- a/kyaml/fn/framework/framework.go
+++ b/kyaml/fn/framework/framework.go
@@ -117,7 +117,7 @@ func Execute(p ResourceListProcessor, rlSource *kio.ByteReadWriter) error {
 	rl.FunctionConfig = rlSource.FunctionConfig
 
 	// We store the original
-	nodeAnnos, err := kio.GetInternalAnnotationsFromResourceList(rl.Items)
+	nodeAnnos, err := kio.PreprocessResourcesForInternalAnnotationMigration(rl.Items)
 	if err != nil {
 		return err
 	}
diff --git a/kyaml/kio/kio.go b/kyaml/kio/kio.go
index d3c017ff0..46047152a 100644
--- a/kyaml/kio/kio.go
+++ b/kyaml/kio/kio.go
@@ -7,12 +7,17 @@ package kio
 
 import (
 	"fmt"
+	"strconv"
 
 	"sigs.k8s.io/kustomize/kyaml/errors"
 	"sigs.k8s.io/kustomize/kyaml/kio/kioutil"
 	"sigs.k8s.io/kustomize/kyaml/yaml"
 )
 
+// resourceIDAnnotation is used to uniquely identify the resource during round trip
+// to and from a function execution.
+const resourceIDAnnotation = "internal.config.k8s.io/annotations-migration-resource-id"
+
 // Reader reads ResourceNodes. Analogous to io.Reader.
 type Reader interface {
 	Read() ([]*yaml.RNode, error)
@@ -117,7 +122,7 @@ func (p Pipeline) ExecuteWithCallback(callback PipelineExecuteCallbackFunc) erro
 	for i := range p.Filters {
 		// Not all RNodes passed through kio.Pipeline have metadata nor should
 		// they all be required to.
-		nodeAnnos, err := GetInternalAnnotationsFromResourceList(result)
+		nodeAnnos, err := PreprocessResourcesForInternalAnnotationMigration(result)
 		if err != nil {
 			return err
 		}
@@ -136,7 +141,7 @@ 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, false)
+		err = ReconcileInternalAnnotations(result, nodeAnnos)
 		if err != nil {
 			return err
 		}
@@ -164,31 +169,29 @@ func FilterAll(filter yaml.Filter) Filter {
 	})
 }
 
-// GetInternalAnnotationsFromResourceList stores 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 GetInternalAnnotationsFromResourceList(result []*yaml.RNode) (map[nodeAnnotations]map[string]string, error) {
-	nodeAnnosMap := make(map[nodeAnnotations]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 {
-		id := kioutil.GetIdAnnotation(result[i])
-		path, index, _ := kioutil.GetFileAnnotations(result[i])
-		annoKey := nodeAnnotations{
-			path:  path,
-			index: index,
-			id:    id,
-		}
-		nodeAnnosMap[annoKey] = kioutil.GetInternalAnnotations(result[i])
-		if err := kioutil.CopyLegacyAnnotations(result[i]); err != nil {
+		idStr := strconv.Itoa(i)
+		err := result[i].PipeE(yaml.SetAnnotation(resourceIDAnnotation, idStr))
+		if err != nil {
 			return nil, err
 		}
-
-		if err := checkMismatchedAnnos(result[i].GetAnnotations()); err != nil {
+		idToAnnosMap[idStr] = kioutil.GetInternalAnnotations(result[i])
+		if err = kioutil.CopyLegacyAnnotations(result[i]); err != nil {
+			return nil, err
+		}
+		meta, _ := result[i].GetMeta()
+		if err = checkMismatchedAnnos(meta.Annotations); err != nil {
 			return nil, err
 		}
 	}
-	return nodeAnnosMap, nil
+	return idToAnnosMap, nil
 }
 
 func checkMismatchedAnnos(annotations map[string]string) error {
@@ -221,26 +224,17 @@ type nodeAnnotations struct {
 }
 
 // ReconcileInternalAnnotations reconciles the annotation format for path, index and id annotations.
-func ReconcileInternalAnnotations(result []*yaml.RNode, nodeAnnosMap map[nodeAnnotations]map[string]string) error {
-	return reconcileInternalAnnotations(result, nodeAnnosMap, true)
-}
-
-// reconcileInternalAnnotations reconciles the annotation format for path, index and id annotations.
-// If formatAnnotations is true, we 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[nodeAnnotations]map[string]string, formatAnnotations bool) error {
-	var useInternal, useLegacy bool
-	var err error
-	if formatAnnotations {
-		if useInternal, useLegacy, err = determineAnnotationsFormat(nodeAnnosMap); err != nil {
-			return err
-		}
+// 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(result[i])
+		err = missingInternalOrLegacyAnnotations(result[i])
 		if err != nil {
 			return err
 		}
@@ -251,26 +245,29 @@ func reconcileInternalAnnotations(result []*yaml.RNode, nodeAnnosMap map[nodeAnn
 		if err != nil {
 			return err
 		}
-		if formatAnnotations {
-			// 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
-			}
+		// 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
 		}
 		// if the annotations are still somehow out of sync, throw an error
-		err = checkMismatchedAnnos(result[i].GetAnnotations())
+		meta, _ := result[i].GetMeta()
+		err = checkMismatchedAnnos(meta.Annotations)
 		if err != nil {
 			return err
 		}
+
+		if _, err = result[i].Pipe(yaml.ClearAnnotation(resourceIDAnnotation)); err != nil {
+			return err
+		}
 	}
 	return nil
 }
 
 // determineAnnotationsFormat determines if the resources are using one of the internal and legacy annotation format or both of them.
-func determineAnnotationsFormat(nodeAnnosMap map[nodeAnnotations]map[string]string) (useInternal, useLegacy bool, err error) {
+func determineAnnotationsFormat(nodeAnnosMap map[string]map[string]string) (useInternal, useLegacy bool, err error) {
 	if len(nodeAnnosMap) == 0 {
 		return true, true, nil
 	}
@@ -280,24 +277,31 @@ func determineAnnotationsFormat(nodeAnnosMap map[nodeAnnotations]map[string]stri
 		_, 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 {
-			internal = &foundOneOf
+			f := foundOneOf
+			internal = &f
 		}
 		if (foundOneOf && !*internal) || (!foundOneOf && *internal) {
-			err = fmt.Errorf("the formatting in the input resources is not consistent")
+			err = fmt.Errorf("the annotation formatting in the input resources is not consistent")
 			return
 		}
 
-		_, foundPath = annos[kioutil.LegacyPathAnnotation]
-		_, foundIndex = annos[kioutil.LegacyIndexAnnotation]
-		_, foundId = annos[kioutil.LegacyIdAnnotation]
-		foundOneOf = foundPath || foundIndex || foundId
+		foundOneOf = foundLegacyPath || foundLegacyIndex || foundLegacyId
 		if legacy == nil {
-			legacy = &foundOneOf
+			f := foundOneOf
+			legacy = &f
 		}
 		if (foundOneOf && !*legacy) || (!foundOneOf && *legacy) {
-			err = fmt.Errorf("the formatting in the input resources is not consistent")
+			err = fmt.Errorf("the annotation formatting in the input resources is not consistent")
 			return
 		}
 	}
@@ -324,8 +328,10 @@ func missingInternalOrLegacyAnnotations(rn *yaml.RNode) error {
 }
 
 func missingInternalOrLegacyAnnotation(rn *yaml.RNode, newKey string, legacyKey string) error {
-	value := rn.GetAnnotations()[newKey]
-	legacyValue := rn.GetAnnotations()[legacyKey]
+	meta, _ := rn.GetMeta()
+	annotations := meta.Annotations
+	value := annotations[newKey]
+	legacyValue := annotations[legacyKey]
 
 	if value == "" && legacyValue == "" {
 		// do nothing
@@ -346,8 +352,9 @@ func missingInternalOrLegacyAnnotation(rn *yaml.RNode, newKey string, legacyKey
 	return nil
 }
 
-func checkAnnotationsAltered(rn *yaml.RNode, nodeAnnosMap map[nodeAnnotations]map[string]string) error {
-	annotations := rn.GetAnnotations()
+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:  annotations[kioutil.PathAnnotation],
@@ -362,16 +369,19 @@ func checkAnnotationsAltered(rn *yaml.RNode, nodeAnnosMap map[nodeAnnotations]ma
 		id:    annotations[kioutil.LegacyIdAnnotation],
 	}
 
-	originalAnnotations, found := nodeAnnosMap[internal]
+	rid := annotations[resourceIDAnnotation]
+	originalAnnotations, found := nodeAnnosMap[rid]
 	if !found {
-		originalAnnotations, found = nodeAnnosMap[legacy]
+		return nil
 	}
 	originalPath, found := originalAnnotations[kioutil.PathAnnotation]
 	if !found {
 		originalPath = originalAnnotations[kioutil.LegacyPathAnnotation]
 	}
 	if originalPath != "" {
-		if originalPath != internal.path {
+		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
 			}
@@ -387,7 +397,9 @@ func checkAnnotationsAltered(rn *yaml.RNode, nodeAnnosMap map[nodeAnnotations]ma
 		originalIndex = originalAnnotations[kioutil.LegacyIndexAnnotation]
 	}
 	if originalIndex != "" {
-		if originalIndex != internal.index {
+		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
 			}
diff --git a/kyaml/kio/kio_test.go b/kyaml/kio/kio_test.go
index b63b59b01..e7e5d046a 100644
--- a/kyaml/kio/kio_test.go
+++ b/kyaml/kio/kio_test.go
@@ -217,9 +217,9 @@ func TestLegacyAnnotationReconciliation(t *testing.T) {
 		}
 		return nodes, nil
 	}
-	changeBothPathAnnos := 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.LegacyPathAnnotation, "legacy")); err != nil {
+			if err := rn.PipeE(yaml.SetAnnotation(kioutil.LegacyPathAnnotation, "new")); err != nil {
 				return nil, err
 			}
 			if err := rn.PipeE(yaml.SetAnnotation(kioutil.PathAnnotation, "new")); err != nil {
@@ -228,6 +228,17 @@ func TestLegacyAnnotationReconciliation(t *testing.T) {
 		}
 		return nodes, nil
 	}
+	changeBothPathAnnosMismatch := func(nodes []*yaml.RNode) ([]*yaml.RNode, error) {
+		for _, rn := range nodes {
+			if err := rn.PipeE(yaml.SetAnnotation(kioutil.LegacyPathAnnotation, "foo")); err != nil {
+				return nil, err
+			}
+			if err := rn.PipeE(yaml.SetAnnotation(kioutil.PathAnnotation, "bar")); err != nil {
+				return nil, err
+			}
+		}
+		return nodes, nil
+	}
 
 	noops := []Filter{
 		FilterFunc(noopFilter1),
@@ -235,7 +246,8 @@ func TestLegacyAnnotationReconciliation(t *testing.T) {
 	}
 	internal := []Filter{FilterFunc(changeInternalAnnos)}
 	legacy := []Filter{FilterFunc(changeLegacyAnnos)}
-	changeBoth := []Filter{FilterFunc(changeBothPathAnnos), FilterFunc(noopFilter1)}
+	changeBothMatch := []Filter{FilterFunc(changeBothPathAnnosMatch), FilterFunc(noopFilter1)}
+	changeBothMismatch := []Filter{FilterFunc(changeBothPathAnnosMismatch), FilterFunc(noopFilter1)}
 
 	testCases := map[string]struct {
 		input       string
@@ -274,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
 ---
@@ -286,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
 `,
@@ -323,8 +331,6 @@ metadata:
   annotations:
     internal.config.kubernetes.io/path: 'configmap.yaml'
     internal.config.kubernetes.io/index: '0'
-    config.kubernetes.io/path: 'configmap.yaml'
-    config.kubernetes.io/index: '0'
 data:
   grpcPort: 8080
 ---
@@ -335,15 +341,12 @@ metadata:
   annotations:
     internal.config.kubernetes.io/path: "configmap.yaml"
     internal.config.kubernetes.io/index: '1'
-    config.kubernetes.io/path: 'configmap.yaml'
-    config.kubernetes.io/index: '1'
 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
+		// have been changed by the function
 		"change only legacy annotations": {
 			input: `apiVersion: v1
 kind: ConfigMap
@@ -368,6 +371,190 @@ data:
 			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
+		"change only legacy annotations while input has both": {
+			input: `apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: ports-from
+  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
+---
+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: legacy,
+			expected: `apiVersion: v1
+kind: ConfigMap
 metadata:
   name: ports-from
   annotations:
@@ -392,9 +579,204 @@ 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/index: '0'
+    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: internal,
+			expected: `apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: ports-from
+  annotations:
+    config.kubernetes.io/path: "new"
+    config.kubernetes.io/index: 'new'
+    internal.config.kubernetes.io/path: 'new'
+    internal.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'
+    internal.config.kubernetes.io/path: "new"
+    internal.config.kubernetes.io/index: 'new'
+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 has both": {
+			input: `apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: ports-from
+  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
+---
+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: changeBothMatch,
+			expected: `apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: ports-from
+  annotations:
+    config.kubernetes.io/path: "new"
+    config.kubernetes.io/index: '0'
+    internal.config.kubernetes.io/path: 'new'
+    internal.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'
+    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 new internal annotations
+		// have been changed by the function
+		"change both to matching value 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: changeBothMatch,
+			expected: `apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: ports-from
+  annotations:
+    internal.config.kubernetes.io/path: 'new'
+    internal.config.kubernetes.io/index: '0'
+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:
@@ -415,16 +797,19 @@ metadata:
 data:
   grpcPort: 8081
 `,
-			filters: internal,
-			expected: `apiVersion: v1
+			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:
-    config.kubernetes.io/path: 'new'
-    config.kubernetes.io/index: 'new'
-    internal.config.kubernetes.io/path: 'new'
-    internal.config.kubernetes.io/index: 'new'
+    internal.config.kubernetes.io/path: "configmap.yaml"
+    internal.config.kubernetes.io/index: '0'
 data:
   grpcPort: 8080
 ---
@@ -433,24 +818,27 @@ kind: ConfigMap
 metadata:
   name: ports-to
   annotations:
-    config.kubernetes.io/path: "new"
-    config.kubernetes.io/index: 'new'
-    internal.config.kubernetes.io/path: 'new'
-    internal.config.kubernetes.io/index: 'new'
+    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
 ---
@@ -461,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 800dcfe45..9f117e95a 100644
--- a/kyaml/kio/kioutil/kioutil.go
+++ b/kyaml/kio/kioutil/kioutil.go
@@ -44,7 +44,8 @@ const (
 )
 
 func GetFileAnnotations(rn *yaml.RNode) (string, string, error) {
-	annotations := rn.GetAnnotations()
+	rm, _ := rn.GetMeta()
+	annotations := rm.Annotations
 	path, found := annotations[PathAnnotation]
 	if !found {
 		path = annotations[LegacyPathAnnotation]
@@ -57,7 +58,8 @@ func GetFileAnnotations(rn *yaml.RNode) (string, string, error) {
 }
 
 func GetIdAnnotation(rn *yaml.RNode) string {
-	annotations := rn.GetAnnotations()
+	rm, _ := rn.GetMeta()
+	annotations := rm.Annotations
 	id, found := annotations[IdAnnotation]
 	if !found {
 		id = annotations[LegacyIdAnnotation]
@@ -391,7 +393,8 @@ func ConfirmInternalAnnotationUnchanged(r1 *yaml.RNode, r2 *yaml.RNode, exclusio
 // `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) || k == LegacyPathAnnotation || k == LegacyIndexAnnotation || k == LegacyIdAnnotation {

From 166c2e766b5b770f02e5b07c74410b9de93c5262 Mon Sep 17 00:00:00 2001
From: Mengqi Yu <mengqiy@google.com>
Date: Fri, 19 Nov 2021 07:28:42 -0800
Subject: [PATCH 5/7] update tests for api module

---
 api/filters/fieldspec/fieldspec_test.go | 6 ++++++
 api/filters/refvar/refvar_test.go       | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/api/filters/fieldspec/fieldspec_test.go b/api/filters/fieldspec/fieldspec_test.go
index d416191b5..49f38e6ca 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 3fd3eddf0..67e1ee03d 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 + `'`,

From dfc627068bf40270c298f8acaa97244e22c8c21f Mon Sep 17 00:00:00 2001
From: Mengqi Yu <mengqiy@google.com>
Date: Fri, 19 Nov 2021 07:29:39 -0800
Subject: [PATCH 6/7] update tests for cmd/config module

---
 cmd/config/internal/commands/e2e/e2e_test.go | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/cmd/config/internal/commands/e2e/e2e_test.go b/cmd/config/internal/commands/e2e/e2e_test.go
index 54749ed9f..d0bf7d092 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'
 `,
 				}
 			},

From bd7bad19a1f45e39097dbca6464382bc57dba6b3 Mon Sep 17 00:00:00 2001
From: Mengqi Yu <mengqiy@google.com>
Date: Fri, 19 Nov 2021 12:13:12 -0800
Subject: [PATCH 7/7] add to BuildAnnotations

---
 api/resource/resource.go     |  3 +++
 kyaml/kio/kio.go             | 10 +++-------
 kyaml/kio/kioutil/kioutil.go |  5 +++++
 3 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/api/resource/resource.go b/api/resource/resource.go
index 4985f7255..7f8bfbd7d 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/kyaml/kio/kio.go b/kyaml/kio/kio.go
index 46047152a..10388acd3 100644
--- a/kyaml/kio/kio.go
+++ b/kyaml/kio/kio.go
@@ -14,10 +14,6 @@ import (
 	"sigs.k8s.io/kustomize/kyaml/yaml"
 )
 
-// resourceIDAnnotation is used to uniquely identify the resource during round trip
-// to and from a function execution.
-const resourceIDAnnotation = "internal.config.k8s.io/annotations-migration-resource-id"
-
 // Reader reads ResourceNodes. Analogous to io.Reader.
 type Reader interface {
 	Read() ([]*yaml.RNode, error)
@@ -178,7 +174,7 @@ func PreprocessResourcesForInternalAnnotationMigration(result []*yaml.RNode) (ma
 	idToAnnosMap := make(map[string]map[string]string)
 	for i := range result {
 		idStr := strconv.Itoa(i)
-		err := result[i].PipeE(yaml.SetAnnotation(resourceIDAnnotation, idStr))
+		err := result[i].PipeE(yaml.SetAnnotation(kioutil.InternalAnnotationsMigrationResourceIDAnnotation, idStr))
 		if err != nil {
 			return nil, err
 		}
@@ -259,7 +255,7 @@ func ReconcileInternalAnnotations(result []*yaml.RNode, nodeAnnosMap map[string]
 			return err
 		}
 
-		if _, err = result[i].Pipe(yaml.ClearAnnotation(resourceIDAnnotation)); err != nil {
+		if _, err = result[i].Pipe(yaml.ClearAnnotation(kioutil.InternalAnnotationsMigrationResourceIDAnnotation)); err != nil {
 			return err
 		}
 	}
@@ -369,7 +365,7 @@ func checkAnnotationsAltered(rn *yaml.RNode, nodeAnnosMap map[string]map[string]
 		id:    annotations[kioutil.LegacyIdAnnotation],
 	}
 
-	rid := annotations[resourceIDAnnotation]
+	rid := annotations[kioutil.InternalAnnotationsMigrationResourceIDAnnotation]
 	originalAnnotations, found := nodeAnnosMap[rid]
 	if !found {
 		return nil
diff --git a/kyaml/kio/kioutil/kioutil.go b/kyaml/kio/kioutil/kioutil.go
index 9f117e95a..7d1a85273 100644
--- a/kyaml/kio/kioutil/kioutil.go
+++ b/kyaml/kio/kioutil/kioutil.go
@@ -41,6 +41,11 @@ 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) {