From 325537204ddec534fdd02cdda9b9dd35c24c48ae Mon Sep 17 00:00:00 2001 From: Karl Isenberg Date: Mon, 13 Sep 2021 13:20:29 -0700 Subject: [PATCH] Refactor depends-on code - Move from pkg/object to pkg/object/dependson - Add DependencySet type to abstract some complexity - Add Marshal/Unmarshal funcs for ObjMetadata and DependencySet - Refactor primary API to ReadAnnotation and WriteAnnotation - Move Write/Marshal behavior from testutil into dependson pkg - Make constants private to encourage using functions as primary API --- pkg/object/annotations.go | 99 --------- pkg/object/annotations_test.go | 199 ----------------- pkg/object/dependson/annotation.go | 74 +++++++ pkg/object/dependson/annotation_test.go | 200 +++++++++++++++++ pkg/object/dependson/strings.go | 139 ++++++++++++ pkg/object/dependson/strings_test.go | 277 ++++++++++++++++++++++++ pkg/object/dependson/types.go | 19 ++ pkg/object/graph/depends.go | 3 +- pkg/testutil/object.go | 34 +-- 9 files changed, 717 insertions(+), 327 deletions(-) delete mode 100644 pkg/object/annotations.go delete mode 100644 pkg/object/annotations_test.go create mode 100644 pkg/object/dependson/annotation.go create mode 100644 pkg/object/dependson/annotation_test.go create mode 100644 pkg/object/dependson/strings.go create mode 100644 pkg/object/dependson/strings_test.go create mode 100644 pkg/object/dependson/types.go diff --git a/pkg/object/annotations.go b/pkg/object/annotations.go deleted file mode 100644 index ae250638..00000000 --- a/pkg/object/annotations.go +++ /dev/null @@ -1,99 +0,0 @@ -// Copyright 2021 The Kubernetes Authors. -// SPDX-License-Identifier: Apache-2.0 -// - -package object - -import ( - "fmt" - "strings" - - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/klog/v2" -) - -// Depends-on annotation constants. -const ( - DependsOnAnnotation = "config.kubernetes.io/depends-on" - // Number of fields for a cluster-scoped depends-on object value. Example: - // rbac.authorization.k8s.io/ClusterRole/my-cluster-role-name - NumFieldsClusterScoped = 3 - // Number of fields for a namespace-scoped depends-on object value. Example: - // apps/namespaces/my-namespace/Deployment/my-deployment-name - NumFieldsNamespacedScoped = 5 - // Used to separate multiple depends-on objects. - AnnotationSeparator = "," - // Used to separate the fields for a depends-on object value. - FieldSeparator = "/" - NamespacesField = "namespaces" -) - -// HasAnnotation returns the annotation value and true if the passed annotation -// is present in the as one of the keys in the annotations map for the passed -// object; empty string and false otherwise. -func HasAnnotation(u *unstructured.Unstructured, key string) (string, bool) { - if u == nil { - return "", false - } - annotations := u.GetAnnotations() - value, found := annotations[key] - return value, found -} - -// DependsOnObjs returns the slice of object references (ObjMetadata) -// that the passed unstructured object depends on based on an -// annotation within the passed unstructured object. -func DependsOnObjs(u *unstructured.Unstructured) ([]ObjMetadata, error) { - objs := []ObjMetadata{} - if u == nil { - return objs, nil - } - objsEncoded, found := HasAnnotation(u, DependsOnAnnotation) - if !found { - return objs, nil - } - klog.V(5).Infof("depends-on annotation found for %s/%s: %s\n", - u.GetNamespace(), u.GetName(), objsEncoded) - return DependsOnAnnotationToObjMetas(objsEncoded) -} - -// annotationToObjMeta parses the passed annotation as an -// object reference. The fields are separated by '/', and the -// string can have either three fields (cluster-scoped object) -// or five fields (namespace-scoped object). Examples are: -// Cluster-Scoped: // (3 fields) -// Namespaced: /namespaces/// (5 fields) -// For the "core" group, the string is empty. -// Return the parsed ObjMetadata object or an error if unable -// to parse the obj ref annotation string. -func DependsOnAnnotationToObjMetas(o string) ([]ObjMetadata, error) { - objs := []ObjMetadata{} - for _, objStr := range strings.Split(o, AnnotationSeparator) { - var group, kind, namespace, name string - objStr := strings.TrimSpace(objStr) - fields := strings.Split(objStr, FieldSeparator) - if len(fields) != NumFieldsClusterScoped && - len(fields) != NumFieldsNamespacedScoped { - return objs, fmt.Errorf("unable to parse depends on annotation into ObjMetadata: %s", o) - } - group = fields[0] - if len(fields) == 3 { - kind = fields[1] - name = fields[2] - } else { - if fields[1] != NamespacesField { - return objs, fmt.Errorf("depends on annotation missing 'namespaces' field: %s", o) - } - namespace = fields[2] - kind = fields[3] - name = fields[4] - } - obj, err := CreateObjMetadata(namespace, name, schema.GroupKind{Group: group, Kind: kind}) - if err != nil { - return objs, err - } - objs = append(objs, obj) - } - return objs, nil -} diff --git a/pkg/object/annotations_test.go b/pkg/object/annotations_test.go deleted file mode 100644 index 65160071..00000000 --- a/pkg/object/annotations_test.go +++ /dev/null @@ -1,199 +0,0 @@ -// Copyright 2021 The Kubernetes Authors. -// SPDX-License-Identifier: Apache-2.0 -// - -package object - -import ( - "testing" - - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime/schema" -) - -var ( - clusterScopedObj = ObjMetadata{Name: "cluster-obj", - GroupKind: schema.GroupKind{Group: "test-group", Kind: "test-kind"}} - namespacedObj = ObjMetadata{Namespace: "test-namespace", Name: "namespaced-obj", - GroupKind: schema.GroupKind{Group: "test-group", Kind: "test-kind"}} -) - -var u1 = &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "unused", - "namespace": "unused", - "annotations": map[string]interface{}{ - DependsOnAnnotation: "test-group/test-kind/cluster-obj", - }, - }, - }, -} - -var u2 = &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "unused", - "namespace": "unused", - "annotations": map[string]interface{}{ - DependsOnAnnotation: "test-group/namespaces/test-namespace/test-kind/namespaced-obj", - }, - }, - }, -} - -var multipleAnnotations = &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "unused", - "namespace": "unused", - "annotations": map[string]interface{}{ - DependsOnAnnotation: "test-group/namespaces/test-namespace/test-kind/namespaced-obj, " + - "test-group/test-kind/cluster-obj", - }, - }, - }, -} - -var noAnnotations = &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "unused", - "namespace": "unused", - }, - }, -} - -var badAnnotation = &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "unused", - "namespace": "unused", - "annotations": map[string]interface{}{ - DependsOnAnnotation: "test-group:namespaces:test-namespace:test-kind:namespaced-obj", - }, - }, - }, -} - -func TestDependsOnAnnotation(t *testing.T) { - testCases := map[string]struct { - obj *unstructured.Unstructured - expected []ObjMetadata - isError bool - }{ - "nil object is not found": { - obj: nil, - expected: []ObjMetadata{}, - }, - "Object with no annotations returns not found": { - obj: noAnnotations, - expected: []ObjMetadata{}, - }, - "Unparseable depends on annotation returns not found": { - obj: badAnnotation, - expected: []ObjMetadata{}, - isError: true, - }, - "Cluster-scoped object depends on annotation": { - obj: u1, - expected: []ObjMetadata{clusterScopedObj}, - }, - "Namespaced object depends on annotation": { - obj: u2, - expected: []ObjMetadata{namespacedObj}, - }, - "Multiple objects specified in annotation": { - obj: multipleAnnotations, - expected: []ObjMetadata{namespacedObj, clusterScopedObj}, - }, - } - - for tn, tc := range testCases { - t.Run(tn, func(t *testing.T) { - actual, err := DependsOnObjs(tc.obj) - if tc.isError { - if err == nil { - t.Fatalf("expected error not received") - } - } else { - if err != nil { - t.Fatalf("unexpected error received: %s", err) - } - if !SetEquals(tc.expected, actual) { - t.Errorf("expected (%s), got (%s)", tc.expected, actual) - } - } - }) - } -} - -func TestAnnotationToObjMetas(t *testing.T) { - testCases := map[string]struct { - annotation string - expected []ObjMetadata - isError bool - }{ - "empty annotation is error": { - annotation: "", - expected: []ObjMetadata{}, - isError: true, - }, - "wrong number of namespace-scoped fields in annotation is error": { - annotation: "test-group/test-namespace/test-kind/namespaced-obj", - expected: []ObjMetadata{}, - isError: true, - }, - "wrong number of cluster-scoped fields in annotation is error": { - annotation: "test-group/namespaces/test-kind/cluster-obj", - expected: []ObjMetadata{}, - isError: true, - }, - "cluster-scoped object annotation": { - annotation: "test-group/test-kind/cluster-obj", - expected: []ObjMetadata{clusterScopedObj}, - isError: false, - }, - "namespaced object annotation": { - annotation: "test-group/namespaces/test-namespace/test-kind/namespaced-obj", - expected: []ObjMetadata{namespacedObj}, - isError: false, - }, - "namespaced object annotation with whitespace at ends is valid": { - annotation: " test-group/namespaces/test-namespace/test-kind/namespaced-obj\n", - expected: []ObjMetadata{namespacedObj}, - isError: false, - }, - "multiple object annotation": { - annotation: "test-group/namespaces/test-namespace/test-kind/namespaced-obj," + - "test-group/test-kind/cluster-obj", - expected: []ObjMetadata{clusterScopedObj, namespacedObj}, - isError: false, - }, - } - - for tn, tc := range testCases { - t.Run(tn, func(t *testing.T) { - actual, err := DependsOnAnnotationToObjMetas(tc.annotation) - if err == nil && tc.isError { - t.Fatalf("expected error, but received none") - } - if err != nil && !tc.isError { - t.Errorf("unexpected error: %s", err) - } - if !SetEquals(tc.expected, actual) { - t.Errorf("expected (%s), got (%s)", tc.expected, actual) - } - }) - } -} diff --git a/pkg/object/dependson/annotation.go b/pkg/object/dependson/annotation.go new file mode 100644 index 00000000..2c8c21cc --- /dev/null +++ b/pkg/object/dependson/annotation.go @@ -0,0 +1,74 @@ +// Copyright 2021 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 +// + +package dependson + +import ( + "errors" + "fmt" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/klog/v2" +) + +const ( + Annotation = "config.kubernetes.io/depends-on" +) + +// HasAnnotation returns true if the config.kubernetes.io/depends-on annotation +// is present, false if not. +func HasAnnotation(u *unstructured.Unstructured) bool { + if u == nil { + return false + } + _, found := u.GetAnnotations()[Annotation] + return found +} + +// ReadAnnotation reads the depends-on annotation and parses the the set of +// object references. +func ReadAnnotation(u *unstructured.Unstructured) (DependencySet, error) { + depSet := DependencySet{} + if u == nil { + return depSet, nil + } + depSetStr, found := u.GetAnnotations()[Annotation] + if !found { + return depSet, nil + } + klog.V(5).Infof("depends-on annotation found for %s/%s: %q", + u.GetNamespace(), u.GetName(), depSetStr) + + depSet, err := UnmarshalDependencySet(depSetStr) + if err != nil { + return depSet, fmt.Errorf("failed to parse dependency set: %w", err) + } + return depSet, nil +} + +// WriteAnnotation updates the supplied unstructured object to add the +// depends-on annotation. The value is a string of objmetas delimited by commas. +// Each objmeta is formatted as "${group}/${kind}/${name}" if cluster-scoped or +// "${group}/namespaces/${namespace}/${kind}/${name}" if namespace-scoped. +func WriteAnnotation(obj *unstructured.Unstructured, depSet DependencySet) error { + if obj == nil { + return errors.New("object is nil") + } + if depSet.Equal(DependencySet{}) { + return errors.New("dependency set is empty") + } + + depSetStr, err := MarshalDependencySet(depSet) + if err != nil { + return fmt.Errorf("failed to format dependency set: %w", err) + } + + a := obj.GetAnnotations() + if a == nil { + a = map[string]string{} + } + a[Annotation] = depSetStr + obj.SetAnnotations(a) + return nil +} diff --git a/pkg/object/dependson/annotation_test.go b/pkg/object/dependson/annotation_test.go new file mode 100644 index 00000000..c71057c9 --- /dev/null +++ b/pkg/object/dependson/annotation_test.go @@ -0,0 +1,200 @@ +// Copyright 2021 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 +// + +package dependson + +import ( + "testing" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" +) + +var u1 = &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "unused", + "namespace": "unused", + "annotations": map[string]interface{}{ + Annotation: "test-group/test-kind/cluster-obj", + }, + }, + }, +} + +var u2 = &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "unused", + "namespace": "unused", + "annotations": map[string]interface{}{ + Annotation: "test-group/namespaces/test-namespace/test-kind/namespaced-obj", + }, + }, + }, +} + +var multipleAnnotations = &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "unused", + "namespace": "unused", + "annotations": map[string]interface{}{ + Annotation: "test-group/namespaces/test-namespace/test-kind/namespaced-obj," + + "test-group/test-kind/cluster-obj", + }, + }, + }, +} + +var noAnnotations = &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "unused", + "namespace": "unused", + }, + }, +} + +var badAnnotation = &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "unused", + "namespace": "unused", + "annotations": map[string]interface{}{ + Annotation: "test-group:namespaces:test-namespace:test-kind:namespaced-obj", + }, + }, + }, +} + +func TestReadAnnotation(t *testing.T) { + testCases := map[string]struct { + obj *unstructured.Unstructured + expected DependencySet + isError bool + }{ + "nil object is not found": { + obj: nil, + expected: DependencySet{}, + }, + "Object with no annotations returns not found": { + obj: noAnnotations, + expected: DependencySet{}, + }, + "Unparseable depends on annotation returns not found": { + obj: badAnnotation, + expected: DependencySet{}, + isError: true, + }, + "Cluster-scoped object depends on annotation": { + obj: u1, + expected: DependencySet{clusterScopedObj}, + }, + "Namespaced object depends on annotation": { + obj: u2, + expected: DependencySet{namespacedObj}, + }, + "Multiple objects specified in annotation": { + obj: multipleAnnotations, + expected: DependencySet{namespacedObj, clusterScopedObj}, + }, + } + + for tn, tc := range testCases { + t.Run(tn, func(t *testing.T) { + actual, err := ReadAnnotation(tc.obj) + if tc.isError { + if err == nil { + t.Fatalf("expected error not received") + } + } else { + if err != nil { + t.Fatalf("unexpected error received: %s", err) + } + if !actual.Equal(tc.expected) { + t.Errorf("expected (%s), got (%s)", tc.expected, actual) + } + } + }) + } +} + +// getDependsOnAnnotation wraps the depends-on annotation with a pointer. +// Returns nil if the annotation is missing. +func getDependsOnAnnotation(obj *unstructured.Unstructured) *string { + value, found := obj.GetAnnotations()[Annotation] + if !found { + return nil + } + return &value +} + +func TestWriteAnnotation(t *testing.T) { + testCases := map[string]struct { + obj *unstructured.Unstructured + dependson DependencySet + expected *string + isError bool + }{ + "nil object": { + obj: nil, + dependson: DependencySet{}, + expected: nil, + isError: true, + }, + "empty mutation": { + obj: &unstructured.Unstructured{}, + dependson: DependencySet{}, + expected: nil, + isError: true, + }, + "Namespace-scoped object": { + obj: &unstructured.Unstructured{}, + dependson: DependencySet{namespacedObj}, + expected: getDependsOnAnnotation(u2), + }, + "Cluster-scoped object": { + obj: &unstructured.Unstructured{}, + dependson: DependencySet{clusterScopedObj}, + expected: getDependsOnAnnotation(u1), + }, + "Multiple objects": { + obj: &unstructured.Unstructured{}, + dependson: DependencySet{namespacedObj, clusterScopedObj}, + expected: getDependsOnAnnotation(multipleAnnotations), + }, + } + + for tn, tc := range testCases { + t.Run(tn, func(t *testing.T) { + err := WriteAnnotation(tc.obj, tc.dependson) + if tc.isError { + if err == nil { + t.Fatalf("expected error not received") + } + } else { + if err != nil { + t.Fatalf("unexpected error received: %s", err) + } + received := getDependsOnAnnotation(tc.obj) + if received != tc.expected && (received == nil || tc.expected == nil) { + t.Errorf("\nexpected:\t%#v\nreceived:\t%#v", tc.expected, received) + } + if *received != *tc.expected { + t.Errorf("\nexpected:\t%#v\nreceived:\t%#v", *tc.expected, *received) + } + } + }) + } +} diff --git a/pkg/object/dependson/strings.go b/pkg/object/dependson/strings.go new file mode 100644 index 00000000..7510dac4 --- /dev/null +++ b/pkg/object/dependson/strings.go @@ -0,0 +1,139 @@ +// Copyright 2021 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 +// + +package dependson + +import ( + "fmt" + "strings" + + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/cli-utils/pkg/object" +) + +const ( + // Number of fields for a cluster-scoped depends-on object value. Example: + // rbac.authorization.k8s.io/ClusterRole/my-cluster-role-name + numFieldsClusterScoped = 3 + // Number of fields for a namespace-scoped depends-on object value. Example: + // apps/namespaces/my-namespace/Deployment/my-deployment-name + numFieldsNamespacedScoped = 5 + // Used to separate multiple depends-on objects. + annotationSeparator = "," + // Used to separate the fields for a depends-on object value. + fieldSeparator = "/" + namespacesField = "namespaces" +) + +// MarshalDependencySet formats the passed dependency set as a string. +// +// Object references are separated by ','. +// +// Returns the formatted DependencySet or an error if unable to format. +func MarshalDependencySet(depSet DependencySet) (string, error) { + var dependsOnStr string + for i, depObj := range depSet { + if i > 0 { + dependsOnStr += annotationSeparator + } + objStr, err := MarshalObjMetadata(depObj) + if err != nil { + return "", fmt.Errorf("failed to format object metadata (index: %d): %w", i, err) + } + dependsOnStr += objStr + } + return dependsOnStr, nil +} + +// UnmarshalDependencySet parses the passed string as a set of object +// references. +// +// Object references are separated by ','. +// +// Returns the parsed DependencySet or an error if unable to parse. +func UnmarshalDependencySet(depsStr string) (DependencySet, error) { + objs := DependencySet{} + for _, objStr := range strings.Split(depsStr, annotationSeparator) { + obj, err := UnmarshalObjMetadata(objStr) + if err != nil { + return objs, fmt.Errorf("failed to parse object metadata: %w", err) + } + objs = append(objs, obj) + } + return objs, nil +} + +// MarshalObjMetadata formats the passed object metadata as a string. +// +// Object references can have either three fields (cluster-scoped object) or +// five fields (namespace-scoped object). +// +// Fields are separated by '/'. +// +// Examples: +// Cluster-Scoped: // (3 fields) +// Namespaced: /namespaces/// (5 fields) +// +// Group and namespace may be empty, but name and kind may not. +// +// Returns the formatted ObjMetadata string or an error if unable to format. +func MarshalObjMetadata(obj object.ObjMetadata) (string, error) { + gk := obj.GroupKind + // group and namespace are allowed to be empty, but name and kind are not + if gk.Kind == "" { + return "", fmt.Errorf("invalid object metadata: kind is empty") + } + if obj.Name == "" { + return "", fmt.Errorf("invalid object metadata: name is empty") + } + if obj.Namespace != "" { + return fmt.Sprintf("%s/namespaces/%s/%s/%s", gk.Group, obj.Namespace, gk.Kind, obj.Name), nil + } + return fmt.Sprintf("%s/%s/%s", gk.Group, gk.Kind, obj.Name), nil +} + +// UnmarshalObjMetadata parses the passed string as a object metadata. +// +// Object references can have either three fields (cluster-scoped object) or +// five fields (namespace-scoped object). +// +// Fields are separated by '/'. +// +// Examples: +// Cluster-Scoped: // (3 fields) +// Namespaced: /namespaces/// (5 fields) +// +// Group and namespace may be empty, but name and kind may not. +// +// Returns the parsed ObjMetadata or an error if unable to parse. +func UnmarshalObjMetadata(objStr string) (object.ObjMetadata, error) { + var obj object.ObjMetadata + var group, kind, namespace, name string + objStr = strings.TrimSpace(objStr) + fields := strings.Split(objStr, fieldSeparator) + + if len(fields) != numFieldsClusterScoped && len(fields) != numFieldsNamespacedScoped { + return obj, fmt.Errorf("too many fields (expected %d or %d): %q", + numFieldsClusterScoped, numFieldsNamespacedScoped, objStr) + } + + group = fields[0] + if len(fields) == 3 { + kind = fields[1] + name = fields[2] + } else { + if fields[1] != namespacesField { + return obj, fmt.Errorf("missing %q field: %q", namespacesField, objStr) + } + namespace = fields[2] + kind = fields[3] + name = fields[4] + } + + obj, err := object.CreateObjMetadata(namespace, name, schema.GroupKind{Group: group, Kind: kind}) + if err != nil { + return obj, err + } + return obj, nil +} diff --git a/pkg/object/dependson/strings_test.go b/pkg/object/dependson/strings_test.go new file mode 100644 index 00000000..c815d553 --- /dev/null +++ b/pkg/object/dependson/strings_test.go @@ -0,0 +1,277 @@ +// Copyright 2021 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 +// + +package dependson + +import ( + "testing" + + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/cli-utils/pkg/object" +) + +var ( + clusterScopedObj = object.ObjMetadata{ + Name: "cluster-obj", + GroupKind: schema.GroupKind{ + Group: "test-group", + Kind: "test-kind", + }, + } + namespacedObj = object.ObjMetadata{ + Namespace: "test-namespace", + Name: "namespaced-obj", + GroupKind: schema.GroupKind{ + Group: "test-group", + Kind: "test-kind", + }, + } +) + +func TestUnmarshalDependencySet(t *testing.T) { + testCases := map[string]struct { + annotation string + expected DependencySet + isError bool + }{ + "empty annotation is error": { + annotation: "", + expected: DependencySet{}, + isError: true, + }, + "wrong number of namespace-scoped fields in annotation is error": { + annotation: "test-group/test-namespace/test-kind/namespaced-obj", + expected: DependencySet{}, + isError: true, + }, + "wrong number of cluster-scoped fields in annotation is error": { + annotation: "test-group/namespaces/test-kind/cluster-obj", + expected: DependencySet{}, + isError: true, + }, + "cluster-scoped object annotation": { + annotation: "test-group/test-kind/cluster-obj", + expected: DependencySet{clusterScopedObj}, + isError: false, + }, + "namespaced object annotation": { + annotation: "test-group/namespaces/test-namespace/test-kind/namespaced-obj", + expected: DependencySet{namespacedObj}, + isError: false, + }, + "namespaced object annotation with whitespace at ends is valid": { + annotation: " test-group/namespaces/test-namespace/test-kind/namespaced-obj\n", + expected: DependencySet{namespacedObj}, + isError: false, + }, + "multiple object annotation": { + annotation: "test-group/namespaces/test-namespace/test-kind/namespaced-obj," + + "test-group/test-kind/cluster-obj", + expected: DependencySet{clusterScopedObj, namespacedObj}, + isError: false, + }, + } + + for tn, tc := range testCases { + t.Run(tn, func(t *testing.T) { + actual, err := UnmarshalDependencySet(tc.annotation) + if err == nil && tc.isError { + t.Fatalf("expected error, but received none") + } + if err != nil && !tc.isError { + t.Errorf("unexpected error: %s", err) + } + if !actual.Equal(tc.expected) { + t.Errorf("expected (%s), got (%s)", tc.expected, actual) + } + }) + } +} + +func TestUnmarshalObjMetadata(t *testing.T) { + testCases := map[string]struct { + metaStr string + expected object.ObjMetadata + isError bool + }{ + "empty annotation is error": { + metaStr: "", + expected: object.ObjMetadata{}, + isError: true, + }, + "wrong number of namespace-scoped fields in annotation is error": { + metaStr: "test-group/test-namespace/test-kind/namespaced-obj", + expected: object.ObjMetadata{}, + isError: true, + }, + "wrong number of cluster-scoped fields in annotation is error": { + metaStr: "test-group/namespaces/test-kind/cluster-obj", + expected: object.ObjMetadata{}, + isError: true, + }, + "cluster-scoped object annotation": { + metaStr: "test-group/test-kind/cluster-obj", + expected: clusterScopedObj, + isError: false, + }, + "namespaced object annotation": { + metaStr: "test-group/namespaces/test-namespace/test-kind/namespaced-obj", + expected: namespacedObj, + isError: false, + }, + "namespaced object annotation with whitespace at ends is valid": { + metaStr: " test-group/namespaces/test-namespace/test-kind/namespaced-obj\n", + expected: namespacedObj, + isError: false, + }, + "multiple is error": { + metaStr: "test-group/namespaces/test-namespace/test-kind/namespaced-obj," + + "test-group/test-kind/cluster-obj", + expected: object.ObjMetadata{}, + isError: true, + }, + } + + for tn, tc := range testCases { + t.Run(tn, func(t *testing.T) { + actual, err := UnmarshalObjMetadata(tc.metaStr) + if err == nil && tc.isError { + t.Fatalf("expected error, but received none") + } + if err != nil && !tc.isError { + t.Errorf("unexpected error: %s", err) + } + if actual != tc.expected { + t.Errorf("expected (%s), got (%s)", tc.expected, actual) + } + }) + } +} + +func TestMarshalDependencySet(t *testing.T) { + testCases := map[string]struct { + depSet DependencySet + expected string + isError bool + }{ + "empty set is not error": { + depSet: DependencySet{}, + expected: "", + }, + "missing kind is error": { + depSet: DependencySet{ + { + Name: "cluster-obj", + GroupKind: schema.GroupKind{ + Group: "test-group", + }, + }, + }, + expected: "", + isError: true, + }, + "missing name is error": { + depSet: DependencySet{ + { + GroupKind: schema.GroupKind{ + Group: "test-group", + Kind: "test-kind", + }, + }, + }, + expected: "", + isError: true, + }, + "cluster-scoped": { + depSet: DependencySet{clusterScopedObj}, + expected: "test-group/test-kind/cluster-obj", + isError: false, + }, + "namespace-scoped": { + depSet: DependencySet{namespacedObj}, + expected: "test-group/namespaces/test-namespace/test-kind/namespaced-obj", + isError: false, + }, + "multiple dependencies": { + depSet: DependencySet{clusterScopedObj, namespacedObj}, + expected: "test-group/test-kind/cluster-obj," + + "test-group/namespaces/test-namespace/test-kind/namespaced-obj", + isError: false, + }, + } + + for tn, tc := range testCases { + t.Run(tn, func(t *testing.T) { + actual, err := MarshalDependencySet(tc.depSet) + if err == nil && tc.isError { + t.Fatalf("expected error, but received none") + } + if err != nil && !tc.isError { + t.Errorf("unexpected error: %s", err) + } + if actual != tc.expected { + t.Errorf("expected (%s), got (%s)", tc.expected, actual) + } + }) + } +} + +func TestMarshalObjMetadata(t *testing.T) { + testCases := map[string]struct { + objMeta object.ObjMetadata + expected string + isError bool + }{ + "empty is error": { + objMeta: object.ObjMetadata{}, + expected: "", + isError: true, + }, + "missing kind is error": { + objMeta: object.ObjMetadata{ + Name: "cluster-obj", + GroupKind: schema.GroupKind{ + Group: "test-group", + }, + }, + expected: "", + isError: true, + }, + "missing name is error": { + objMeta: object.ObjMetadata{ + GroupKind: schema.GroupKind{ + Group: "test-group", + Kind: "test-kind", + }, + }, + expected: "", + isError: true, + }, + "cluster-scoped": { + objMeta: clusterScopedObj, + expected: "test-group/test-kind/cluster-obj", + isError: false, + }, + "namespace-scoped": { + objMeta: namespacedObj, + expected: "test-group/namespaces/test-namespace/test-kind/namespaced-obj", + isError: false, + }, + } + + for tn, tc := range testCases { + t.Run(tn, func(t *testing.T) { + actual, err := MarshalObjMetadata(tc.objMeta) + if err == nil && tc.isError { + t.Fatalf("expected error, but received none") + } + if err != nil && !tc.isError { + t.Errorf("unexpected error: %s", err) + } + if actual != tc.expected { + t.Errorf("expected (%s), got (%s)", tc.expected, actual) + } + }) + } +} diff --git a/pkg/object/dependson/types.go b/pkg/object/dependson/types.go new file mode 100644 index 00000000..fc8a576a --- /dev/null +++ b/pkg/object/dependson/types.go @@ -0,0 +1,19 @@ +// Copyright 2021 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 +// + +package dependson + +import ( + "sigs.k8s.io/cli-utils/pkg/object" +) + +// DependencySet is a set of object references. +// When testing equality, order is not importent. +type DependencySet []object.ObjMetadata + +// Equal returns true if the ObjMetadata sets are eqivelent, ignoring order. +// Fulfills Equal interface from github.com/google/go-cmp +func (a DependencySet) Equal(b DependencySet) bool { + return object.SetEquals(a, b) +} diff --git a/pkg/object/graph/depends.go b/pkg/object/graph/depends.go index 2cc41544..c03953ab 100644 --- a/pkg/object/graph/depends.go +++ b/pkg/object/graph/depends.go @@ -10,6 +10,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/klog/v2" "sigs.k8s.io/cli-utils/pkg/object" + "sigs.k8s.io/cli-utils/pkg/object/dependson" ) // SortObjs returns a slice of the sets of objects to apply (in order). @@ -73,7 +74,7 @@ func addExplicitEdges(g *Graph, objs []*unstructured.Unstructured) { id := object.UnstructuredToObjMetaOrDie(obj) klog.V(3).Infof("adding vertex: %s", id) g.AddVertex(id) - deps, err := object.DependsOnObjs(obj) + deps, err := dependson.ReadAnnotation(obj) if err == nil { for _, dep := range deps { klog.V(3).Infof("adding edge from: %s, to: %s", id, dep) diff --git a/pkg/testutil/object.go b/pkg/testutil/object.go index dcf419e8..b73b74c8 100644 --- a/pkg/testutil/object.go +++ b/pkg/testutil/object.go @@ -6,7 +6,6 @@ package testutil import ( - "fmt" "testing" "github.com/stretchr/testify/assert" @@ -14,6 +13,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/kubectl/pkg/scheme" "sigs.k8s.io/cli-utils/pkg/object" + "sigs.k8s.io/cli-utils/pkg/object/dependson" ) var codec = scheme.Codecs.LegacyCodec(scheme.Scheme.PrioritizedVersionsAllGroups()...) @@ -101,37 +101,15 @@ type dependsOnMutator struct { depObjs []*unstructured.Unstructured } -// Mutate for dependsOnMutator adds the stored object dependencies -// in the depends-on annotation for the passed mutated object. +// Mutate writes a depends-on annotation on the supplied object. The value of +// the annotation is a set of dependencies referencing the dependsOnMutator's +// depObjs. func (d dependsOnMutator) Mutate(u *unstructured.Unstructured) { - // Add depends on object annotation to passed object. - var objStr string - // Iterate through all dependent objects to create the - // depends-on annotation string. - for i, depObj := range d.depObjs { - if i > 0 { - objStr += "," - } - groupKind := depObj.GroupVersionKind().GroupKind() - group := groupKind.Group - kind := groupKind.Kind - name := depObj.GetName() - if object.IsNamespaced(depObj) { - objStr += fmt.Sprintf("%s/namespaces/%s/%s/%s", - group, depObj.GetNamespace(), kind, name) - } else { - objStr += fmt.Sprintf("%s/%s/%s", group, kind, name) - } - } - annos, found, err := unstructured.NestedStringMap(u.Object, "metadata", "annotations") + objMetas, err := object.UnstructuredsToObjMetas(d.depObjs) if !assert.NoError(d.t, err) { d.t.FailNow() } - if !found { - annos = make(map[string]string) - } - annos[object.DependsOnAnnotation] = objStr - err = unstructured.SetNestedStringMap(u.Object, annos, "metadata", "annotations") + err = dependson.WriteAnnotation(u, objMetas) if !assert.NoError(d.t, err) { d.t.FailNow() }