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.
This commit is contained in:
Mengqi Yu
2021-11-17 14:06:20 -08:00
parent 2f115223cc
commit 5caed5b90a
5 changed files with 186 additions and 218 deletions

View File

@@ -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 {

View File

@@ -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"
}
}

View File

@@ -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
}

View File

@@ -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,

View File

@@ -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
}
}