diff --git a/pkg/apply/applier.go b/pkg/apply/applier.go index 77583eb..7e9f434 100644 --- a/pkg/apply/applier.go +++ b/pkg/apply/applier.go @@ -27,6 +27,7 @@ import ( "sigs.k8s.io/cli-utils/pkg/kstatus/polling" "sigs.k8s.io/cli-utils/pkg/kstatus/polling/engine" "sigs.k8s.io/cli-utils/pkg/object" + "sigs.k8s.io/cli-utils/pkg/object/validation" "sigs.k8s.io/cli-utils/pkg/ordering" ) @@ -143,9 +144,8 @@ func (a *Applier) Run(ctx context.Context, invInfo inventory.InventoryInfo, obje // Validate the resources to make sure we catch those problems early // before anything has been updated in the cluster. - if err := (&object.Validator{ - Mapper: mapper, - }).Validate(objects); err != nil { + validator := &validation.Validator{Mapper: mapper} + if err := validator.Validate(objects); err != nil { handleError(eventChannel, err) return } diff --git a/pkg/apply/mutator/apply_time_mutator.go b/pkg/apply/mutator/apply_time_mutator.go index ee8e20e..989c8a3 100644 --- a/pkg/apply/mutator/apply_time_mutator.go +++ b/pkg/apply/mutator/apply_time_mutator.go @@ -46,7 +46,7 @@ func (atm *ApplyTimeMutator) Mutate(ctx context.Context, obj *unstructured.Unstr mutated := false reason := "" - targetRef := mutation.NewResourceReference(obj) + targetRef := mutation.ResourceReferenceFromUnstructured(obj) if !mutation.HasAnnotation(obj) { return mutated, reason, nil @@ -177,7 +177,7 @@ func (atm *ApplyTimeMutator) getObject(ctx context.Context, mapping *meta.RESTMa if ref.Kind == "" { return nil, fmt.Errorf("invalid source reference: empty kind") } - id := ref.ObjMetadata() + id := ref.ToObjMetadata() // get resource from cache if atm.ResourceCache != nil { @@ -225,7 +225,7 @@ func computeStatus(obj *unstructured.Unstructured) cache.ResourceStatus { result, err := status.Compute(obj) if err != nil { if klog.V(3).Enabled() { - ref := mutation.NewResourceReference(obj) + ref := mutation.ResourceReferenceFromUnstructured(obj) klog.Info("failed to compute resource status (%s): %d", ref, err) } return cache.ResourceStatus{ diff --git a/pkg/apply/mutator/apply_time_mutator_test.go b/pkg/apply/mutator/apply_time_mutator_test.go index 96b6fd3..f28a3a6 100644 --- a/pkg/apply/mutator/apply_time_mutator_test.go +++ b/pkg/apply/mutator/apply_time_mutator_test.go @@ -410,7 +410,7 @@ func TestMutate(t *testing.T) { reason: "", // exact error message isn't very important. Feel free to update if the error text changes. errMsg: `failed to read annotation in resource (v1/namespaces/map-namespace/ConfigMap/map3-name): ` + - `failed to parse apply-time-mutation annotation: "not a valid substitution list": ` + + `failed to parse apply-time-mutation annotation: ` + `error unmarshaling JSON: ` + `while decoding JSON: ` + `json: cannot unmarshal string into Go value of type mutation.ApplyTimeMutation`, diff --git a/pkg/inventory/inventory-client.go b/pkg/inventory/inventory-client.go index a4d7756..cc6ad83 100644 --- a/pkg/inventory/inventory-client.go +++ b/pkg/inventory/inventory-client.go @@ -156,7 +156,7 @@ func (cic *ClusterInventoryClient) Replace(localInv InventoryInfo, objs object.O } clusterObjs, err := cic.GetClusterObjs(localInv, dryRun) if err != nil { - return err + return fmt.Errorf("failed to read inventory objects from cluster: %w", err) } if objs.Equal(clusterObjs) { klog.V(4).Infof("applied objects same as cluster inventory: do nothing") @@ -164,7 +164,7 @@ func (cic *ClusterInventoryClient) Replace(localInv InventoryInfo, objs object.O } clusterInv, err := cic.GetClusterInventoryInfo(localInv, dryRun) if err != nil { - return err + return fmt.Errorf("failed to read inventory from cluster: %w", err) } clusterInv, err = cic.replaceInventory(clusterInv, objs) if err != nil { @@ -173,7 +173,7 @@ func (cic *ClusterInventoryClient) Replace(localInv InventoryInfo, objs object.O klog.V(4).Infof("replace cluster inventory: %s/%s", clusterInv.GetNamespace(), clusterInv.GetName()) klog.V(4).Infof("replace cluster inventory %d objects", len(objs)) if err := cic.applyInventoryObj(clusterInv, dryRun); err != nil { - return err + return fmt.Errorf("failed to write updated inventory to cluster: %w", err) } return nil } @@ -225,7 +225,7 @@ func (cic *ClusterInventoryClient) GetClusterObjs(localInv InventoryInfo, dryRun var objs object.ObjMetadataSet clusterInv, err := cic.GetClusterInventoryInfo(localInv, dryRun) if err != nil { - return objs, err + return objs, fmt.Errorf("failed to read inventory from cluster: %w", err) } // First time; no inventory obj yet. if clusterInv == nil { @@ -247,7 +247,7 @@ func (cic *ClusterInventoryClient) GetClusterObjs(localInv InventoryInfo, dryRun func (cic *ClusterInventoryClient) GetClusterInventoryInfo(inv InventoryInfo, dryRun common.DryRunStrategy) (*unstructured.Unstructured, error) { clusterInvObjects, err := cic.GetClusterInventoryObjs(inv) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to read inventory objects from cluster: %w", err) } var clusterInv *unstructured.Unstructured diff --git a/pkg/jsonpath/jsonpath.go b/pkg/jsonpath/jsonpath.go index 0451448..e1ae5ac 100644 --- a/pkg/jsonpath/jsonpath.go +++ b/pkg/jsonpath/jsonpath.go @@ -17,9 +17,10 @@ import ( "github.com/spyzhov/ajson" ) -// Get evaluates the yq expression to extract values from the input map. +// Get evaluates the JSONPath expression to extract values from the input map. // Returns the node values that were found (zero or more), or an error. -// For details about the yq expression language, see: https://mikefarah.gitbook.io/yq/ +// For details about the JSONPath expression language, see: +// https://goessner.net/articles/JsonPath/ func Get(obj map[string]interface{}, expression string) ([]interface{}, error) { // format input object as json for input into jsonpath library jsonBytes, err := json.Marshal(obj) @@ -65,9 +66,10 @@ func Get(obj map[string]interface{}, expression string) ([]interface{}, error) { return result, nil } -// Set evaluates the yq expression to set a value in the input map. +// Set evaluates the JSONPath expression to set a value in the input map. // Returns the number of matching nodes that were updated, or an error. -// For details about the yq expression language, see: https://mikefarah.gitbook.io/yq/ +// For details about the JSONPath expression language, see: +// https://goessner.net/articles/JsonPath/ func Set(obj map[string]interface{}, expression string, value interface{}) (int, error) { // format input object as json for input into jsonpath library jsonBytes, err := json.Marshal(obj) diff --git a/pkg/multierror/multierror.go b/pkg/multierror/multierror.go new file mode 100644 index 0000000..016fed6 --- /dev/null +++ b/pkg/multierror/multierror.go @@ -0,0 +1,91 @@ +// Copyright 2022 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package multierror + +import ( + "fmt" + "strings" +) + +const Prefix = "- " +const Indent = " " + +type Interface interface { + Errors() []error +} + +// New returns a new MultiError wrapping the specified error list. +func New(causes ...error) *MultiError { + return &MultiError{ + Causes: causes, + } +} + +// MultiError wraps multiple errors and formats them for multi-line output. +type MultiError struct { + Causes []error +} + +func (mve *MultiError) Errors() []error { + return mve.Causes +} + +func (mve *MultiError) Error() string { + if len(mve.Causes) == 1 { + return mve.Causes[0].Error() + } + var b strings.Builder + _, _ = fmt.Fprintf(&b, "%d errors:\n", len(mve.Causes)) + for _, err := range mve.Causes { + _, _ = fmt.Fprintf(&b, "%s\n", formatError(err)) + } + return b.String() +} + +func formatError(err error) string { + lines := strings.Split(err.Error(), "\n") + return Prefix + strings.Join(lines, fmt.Sprintf("\n%s", Indent)) +} + +// Wrap merges zero or more errors and/or MultiErrors into one error. +// MultiErrors are recursively unwrapped to reduce depth. +// If only one error is received, that error is returned without a wrapper. +func Wrap(errs ...error) error { + if len(errs) == 0 { + return nil + } + errs = Unwrap(errs...) + var err error + switch { + case len(errs) == 0: + err = nil + case len(errs) == 1: + err = errs[0] + case len(errs) > 1: + err = &MultiError{ + Causes: errs, + } + } + return err +} + +// Unwrap flattens zero or more errors and/or MultiErrors into a list of errors. +// MultiErrors are recursively unwrapped to reduce depth. +func Unwrap(errs ...error) []error { + if len(errs) == 0 { + return nil + } + var errors []error + for _, err := range errs { + if mve, ok := err.(Interface); ok { + // Recursively unwrap MultiErrors + for _, cause := range mve.Errors() { + errors = append(errors, Unwrap(cause)...) + } + } else { + errors = append(errors, err) + } + } + return errors +} diff --git a/pkg/object/dependson/annotation.go b/pkg/object/dependson/annotation.go index 1bb0cfb..6ace01d 100644 --- a/pkg/object/dependson/annotation.go +++ b/pkg/object/dependson/annotation.go @@ -42,7 +42,7 @@ func ReadAnnotation(u *unstructured.Unstructured) (DependencySet, error) { depSet, err := ParseDependencySet(depSetStr) if err != nil { - return depSet, fmt.Errorf("failed to parse dependency set: %w", err) + return depSet, fmt.Errorf("failed to parse depends-on annotation: %w", err) } return depSet, nil } @@ -61,7 +61,7 @@ func WriteAnnotation(obj *unstructured.Unstructured, depSet DependencySet) error depSetStr, err := FormatDependencySet(depSet) if err != nil { - return fmt.Errorf("failed to format dependency set: %w", err) + return fmt.Errorf("failed to format depends-on annotation: %w", err) } a := obj.GetAnnotations() diff --git a/pkg/object/dependson/strings.go b/pkg/object/dependson/strings.go index b08e2ef..c092ecc 100644 --- a/pkg/object/dependson/strings.go +++ b/pkg/object/dependson/strings.go @@ -114,8 +114,8 @@ func ParseObjMetadata(objStr string) (object.ObjMetadata, error) { 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) + return obj, fmt.Errorf("expected %d or %d fields, found %d: %q", + numFieldsClusterScoped, numFieldsNamespacedScoped, len(fields), objStr) } group = fields[0] diff --git a/pkg/object/graph/depends.go b/pkg/object/graph/depends.go index 86e6908..f8a76a1 100644 --- a/pkg/object/graph/depends.go +++ b/pkg/object/graph/depends.go @@ -7,13 +7,16 @@ package graph import ( + "fmt" "sort" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/klog/v2" + "sigs.k8s.io/cli-utils/pkg/multierror" "sigs.k8s.io/cli-utils/pkg/object" "sigs.k8s.io/cli-utils/pkg/object/dependson" "sigs.k8s.io/cli-utils/pkg/object/mutation" + "sigs.k8s.io/cli-utils/pkg/object/validation" "sigs.k8s.io/cli-utils/pkg/ordering" ) @@ -22,28 +25,38 @@ import ( // the returned applied sets is a topological ordering of the sets to apply. // Returns an single empty apply set if there are no objects to apply. func SortObjs(objs object.UnstructuredSet) ([]object.UnstructuredSet, error) { + var objSets []object.UnstructuredSet if len(objs) == 0 { - return []object.UnstructuredSet{}, nil + return objSets, nil } + var errors []error + // Convert to IDs (same length & order as objs) + ids := object.UnstructuredSetToObjMetadataSet(objs) // Create the graph, and build a map of object metadata to the object (Unstructured). g := New() objToUnstructured := map[object.ObjMetadata]*unstructured.Unstructured{} - for _, obj := range objs { - id := object.UnstructuredToObjMetadata(obj) + for i, obj := range objs { + id := ids[i] objToUnstructured[id] = obj } - // Add object vertices and dependency edges to graph. - addApplyTimeMutationEdges(g, objs) - addDependsOnEdges(g, objs) - addNamespaceEdges(g, objs) - addCRDEdges(g, objs) + // Add objects as graph vertices + addVertices(g, ids) + // Add dependencies as graph edges + addCRDEdges(g, objs, ids) + addNamespaceEdges(g, objs, ids) + if err := addDependsOnEdges(g, objs, ids); err != nil { + errors = append(errors, err) + } + if err := addApplyTimeMutationEdges(g, objs, ids); err != nil { + errors = append(errors, err) + } // Run topological sort on the graph. - objSets := []object.UnstructuredSet{} sortedObjSets, err := g.Sort() if err != nil { - return []object.UnstructuredSet{}, err + errors = append(errors, err) } // Map the object metadata back to the sorted sets of unstructured objects. + // Ignore any edges that aren't part of the input set (don't wait for them). for _, objSet := range sortedObjSets { currentSet := object.UnstructuredSet{} for _, id := range objSet { @@ -57,6 +70,9 @@ func SortObjs(objs object.UnstructuredSet) ([]object.UnstructuredSet, error) { sort.Sort(ordering.SortableUnstructureds(currentSet)) objSets = append(objSets, currentSet) } + if len(errors) > 0 { + return objSets, multierror.Wrap(errors...) + } return objSets, nil } @@ -65,7 +81,7 @@ func ReverseSortObjs(objs object.UnstructuredSet) ([]object.UnstructuredSet, err // Sorted objects using normal ordering. s, err := SortObjs(objs) if err != nil { - return []object.UnstructuredSet{}, err + return s, err } // Reverse the ordering of the object sets using swaps. for i, j := 0, len(s)-1; i < j; i, j = i+1, j-1 { @@ -80,73 +96,140 @@ func ReverseSortObjs(objs object.UnstructuredSet) ([]object.UnstructuredSet, err return s, nil } -// addApplyTimeMutationEdges updates the graph with edges from objects -// with an explicit "apply-time-mutation" annotation. -func addApplyTimeMutationEdges(g *Graph, objs object.UnstructuredSet) { - for _, obj := range objs { - id := object.UnstructuredToObjMetadata(obj) +// addVertices adds all the IDs in the set as graph vertices. +func addVertices(g *Graph, ids object.ObjMetadataSet) { + for _, id := range ids { klog.V(3).Infof("adding vertex: %s", id) g.AddVertex(id) - if mutation.HasAnnotation(obj) { - subs, err := mutation.ReadAnnotation(obj) - if err != nil { - // TODO: fail task if parse errors? - klog.V(3).Infof("failed to add edges from: %s: %s", id, err) - return + } +} + +// addApplyTimeMutationEdges updates the graph with edges from objects +// with an explicit "apply-time-mutation" annotation. +// The objs and ids must match in order and length (optimization). +func addApplyTimeMutationEdges(g *Graph, objs object.UnstructuredSet, ids object.ObjMetadataSet) error { + var errors []error + for i, obj := range objs { + if !mutation.HasAnnotation(obj) { + continue + } + id := ids[i] + subs, err := mutation.ReadAnnotation(obj) + if err != nil { + klog.V(3).Infof("failed to add edges from: %s: %v", id, err) + errors = append(errors, validation.NewError(err, id)) + continue + } + seen := make(map[object.ObjMetadata]struct{}) + var objErrors []error + for _, sub := range subs { + dep := sub.SourceRef.ToObjMetadata() + // Duplicate dependencies can be safely skipped. + if _, found := seen[dep]; found { + continue } - for _, sub := range subs { - // TODO: fail task if it's not in the inventory? - dep := sub.SourceRef.ObjMetadata() - klog.V(3).Infof("adding edge from: %s, to: %s", id, dep) - g.AddEdge(id, dep) + // Mark as seen + seen[dep] = struct{}{} + // Require dependencies to be in the same resource group. + // Waiting for external dependencies isn't implemented (yet). + if !ids.Contains(dep) { + err := fmt.Errorf("invalid %q annotation: dependency not in object set: %s", + mutation.Annotation, sub.SourceRef) + objErrors = append(objErrors, err) + klog.V(3).Infof("failed to add edges from: %s: %v", id, err) + continue } + klog.V(3).Infof("adding edge from: %s, to: %s", id, dep) + g.AddEdge(id, dep) + } + if len(objErrors) > 0 { + errors = append(errors, + validation.NewError(multierror.Wrap(objErrors...), id)) } } + if len(errors) > 0 { + return multierror.Wrap(errors...) + } + return nil } // addDependsOnEdges updates the graph with edges from objects // with an explicit "depends-on" annotation. -func addDependsOnEdges(g *Graph, objs object.UnstructuredSet) { - for _, obj := range objs { - id := object.UnstructuredToObjMetadata(obj) - klog.V(3).Infof("adding vertex: %s", id) - g.AddVertex(id) - deps, err := dependson.ReadAnnotation(obj) - if err != nil { - // TODO: fail if annotation fails to parse? - klog.V(3).Infof("failed to add edges from: %s: %s", id, err) +// The objs and ids must match in order and length (optimization). +func addDependsOnEdges(g *Graph, objs object.UnstructuredSet, ids object.ObjMetadataSet) error { + var errors []error + for i, obj := range objs { + if !dependson.HasAnnotation(obj) { continue } + id := ids[i] + deps, err := dependson.ReadAnnotation(obj) + if err != nil { + klog.V(3).Infof("failed to add edges from: %s: %v", id, err) + errors = append(errors, validation.NewError(err, id)) + continue + } + seen := make(map[object.ObjMetadata]struct{}) + var objErrors []error for _, dep := range deps { - // TODO: fail if depe is not in the inventory? + // Duplicate dependencies in the same annotation are not allowed. + // Having duplicates won't break the graph, but skip it anyway. + if _, found := seen[dep]; found { + // Won't error - already passed validation + depStr, _ := dependson.FormatObjMetadata(dep) + err := fmt.Errorf("invalid %q annotation: duplicate reference: %s", + dependson.Annotation, depStr) + objErrors = append(objErrors, err) + klog.V(3).Infof("failed to add edges from: %s: %v", id, err) + continue + } + // Mark as seen + seen[dep] = struct{}{} + // Require dependencies to be in the same resource group. + // Waiting for external dependencies isn't implemented (yet). + if !ids.Contains(dep) { + err := fmt.Errorf("invalid %q annotation: dependency not in object set: %s", + dependson.Annotation, mutation.ResourceReferenceFromObjMetadata(dep)) + objErrors = append(objErrors, err) + klog.V(3).Infof("failed to add edges from: %s: %v", id, err) + continue + } klog.V(3).Infof("adding edge from: %s, to: %s", id, dep) g.AddEdge(id, dep) } + if len(objErrors) > 0 { + errors = append(errors, + validation.NewError(multierror.Wrap(objErrors...), id)) + } } + if len(errors) > 0 { + return multierror.Wrap(errors...) + } + return nil } // addCRDEdges adds edges to the dependency graph from custom // resources to their definitions to ensure the CRD's exist // before applying the custom resources created with the definition. -func addCRDEdges(g *Graph, objs object.UnstructuredSet) { +// The objs and ids must match in order and length (optimization). +func addCRDEdges(g *Graph, objs object.UnstructuredSet, ids object.ObjMetadataSet) { crds := map[string]object.ObjMetadata{} // First create a map of all the CRD's. - for _, u := range objs { + for i, u := range objs { if object.IsCRD(u) { groupKind, found := object.GetCRDGroupKind(u) if found { - obj := object.UnstructuredToObjMetadata(u) - crds[groupKind.String()] = obj + crds[groupKind.String()] = ids[i] } } } // Iterate through all resources to see if we are applying any // custom resources defined by previously recorded CRD's. - for _, u := range objs { + for i, u := range objs { gvk := u.GroupVersionKind() groupKind := gvk.GroupKind() if to, found := crds[groupKind.String()]; found { - from := object.UnstructuredToObjMetadata(u) + from := ids[i] klog.V(3).Infof("adding edge from: custom resource %s, to CRD: %s", from, to) g.AddEdge(from, to) } @@ -156,25 +239,25 @@ func addCRDEdges(g *Graph, objs object.UnstructuredSet) { // addNamespaceEdges adds edges to the dependency graph from namespaced // objects to the namespace objects. Ensures the namespaces exist // before the resources in those namespaces are applied. -func addNamespaceEdges(g *Graph, objs object.UnstructuredSet) { +// The objs and ids must match in order and length (optimization). +func addNamespaceEdges(g *Graph, objs object.UnstructuredSet, ids object.ObjMetadataSet) { namespaces := map[string]object.ObjMetadata{} // First create a map of all the namespaces objects live in. - for _, obj := range objs { + for i, obj := range objs { if object.IsKindNamespace(obj) { - id := object.UnstructuredToObjMetadata(obj) namespace := obj.GetName() - namespaces[namespace] = id + namespaces[namespace] = ids[i] } } // Next, if the namespace of a namespaced object is being applied, // then create an edge from the namespaced object to its namespace. - for _, obj := range objs { + for i, obj := range objs { if object.IsNamespaced(obj) { objNamespace := obj.GetNamespace() - if namespace, found := namespaces[objNamespace]; found { - id := object.UnstructuredToObjMetadata(obj) - klog.V(3).Infof("adding edge from: %s to namespace: %s", id, namespace) - g.AddEdge(id, namespace) + if to, found := namespaces[objNamespace]; found { + from := ids[i] + klog.V(3).Infof("adding edge from: %s to namespace: %s", from, to) + g.AddEdge(from, to) } } } diff --git a/pkg/object/graph/depends_test.go b/pkg/object/graph/depends_test.go index 4990cf2..9be0097 100644 --- a/pkg/object/graph/depends_test.go +++ b/pkg/object/graph/depends_test.go @@ -4,13 +4,18 @@ package graph import ( + "errors" "testing" "github.com/stretchr/testify/assert" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/cli-utils/pkg/multierror" "sigs.k8s.io/cli-utils/pkg/object" + "sigs.k8s.io/cli-utils/pkg/object/dependson" "sigs.k8s.io/cli-utils/pkg/object/mutation" mutationutil "sigs.k8s.io/cli-utils/pkg/object/mutation/testutil" + "sigs.k8s.io/cli-utils/pkg/object/validation" "sigs.k8s.io/cli-utils/pkg/testutil" ) @@ -371,8 +376,9 @@ func TestReverseSortObjs(t *testing.T) { func TestApplyTimeMutationEdges(t *testing.T) { testCases := map[string]struct { - objs []*unstructured.Unstructured - expected []Edge + objs []*unstructured.Unstructured + expected []Edge + expectedError error }{ "no objects adds no graph edges": { objs: []*unstructured.Unstructured{}, @@ -398,7 +404,9 @@ func TestApplyTimeMutationEdges(t *testing.T) { resources["deployment"], mutationutil.AddApplyTimeMutation(t, &mutation.ApplyTimeMutation{ { - SourceRef: mutation.NewResourceReference(testutil.Unstructured(t, resources["secret"])), + SourceRef: mutation.ResourceReferenceFromObjMetadata( + testutil.ToIdentifier(t, resources["secret"]), + ), SourcePath: "unused", TargetPath: "unused", Token: "unused", @@ -421,7 +429,9 @@ func TestApplyTimeMutationEdges(t *testing.T) { resources["deployment"], mutationutil.AddApplyTimeMutation(t, &mutation.ApplyTimeMutation{ { - SourceRef: mutation.NewResourceReference(testutil.Unstructured(t, resources["secret"])), + SourceRef: mutation.ResourceReferenceFromObjMetadata( + testutil.ToIdentifier(t, resources["secret"]), + ), SourcePath: "unused", TargetPath: "unused", Token: "unused", @@ -433,7 +443,9 @@ func TestApplyTimeMutationEdges(t *testing.T) { resources["pod"], mutationutil.AddApplyTimeMutation(t, &mutation.ApplyTimeMutation{ { - SourceRef: mutation.NewResourceReference(testutil.Unstructured(t, resources["secret"])), + SourceRef: mutation.ResourceReferenceFromObjMetadata( + testutil.ToIdentifier(t, resources["secret"]), + ), SourcePath: "unused", TargetPath: "unused", Token: "unused", @@ -460,13 +472,17 @@ func TestApplyTimeMutationEdges(t *testing.T) { resources["pod"], mutationutil.AddApplyTimeMutation(t, &mutation.ApplyTimeMutation{ { - SourceRef: mutation.NewResourceReference(testutil.Unstructured(t, resources["secret"])), + SourceRef: mutation.ResourceReferenceFromObjMetadata( + testutil.ToIdentifier(t, resources["secret"]), + ), SourcePath: "unused", TargetPath: "unused", Token: "unused", }, { - SourceRef: mutation.NewResourceReference(testutil.Unstructured(t, resources["deployment"])), + SourceRef: mutation.ResourceReferenceFromObjMetadata( + testutil.ToIdentifier(t, resources["deployment"]), + ), SourcePath: "unused", TargetPath: "unused", Token: "unused", @@ -487,12 +503,133 @@ func TestApplyTimeMutationEdges(t *testing.T) { }, }, }, + "error: invalid annotation": { + objs: []*unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "foo", + "namespace": "default", + "annotations": map[string]interface{}{ + mutation.Annotation: "invalid-mutation", + }, + }, + }, + }, + }, + expected: []Edge{}, + expectedError: validation.NewError( + errors.New("failed to parse apply-time-mutation annotation: "+ + "error unmarshaling JSON: "+ + "while decoding JSON: json: "+ + "cannot unmarshal string into Go value of type mutation.ApplyTimeMutation"), + object.ObjMetadata{ + GroupKind: schema.GroupKind{ + Group: "apps", + Kind: "Deployment", + }, + Name: "foo", + Namespace: "default", + }, + ), + }, + "error: dependency not in object set": { + objs: []*unstructured.Unstructured{ + testutil.Unstructured(t, resources["pod"], + mutationutil.AddApplyTimeMutation(t, &mutation.ApplyTimeMutation{ + { + SourceRef: mutation.ResourceReferenceFromObjMetadata( + testutil.ToIdentifier(t, resources["deployment"]), + ), + }, + }), + ), + }, + expected: []Edge{}, + expectedError: validation.NewError( + errors.New(`invalid "config.kubernetes.io/apply-time-mutation" annotation: `+ + "dependency not in object set: "+ + "apps/namespaces/test-namespace/Deployment/foo"), + object.ObjMetadata{ + GroupKind: schema.GroupKind{ + Group: "", + Kind: "Pod", + }, + Name: "test-pod", + Namespace: "test-namespace", + }, + ), + }, + "error: two invalid objects": { + objs: []*unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "foo", + "namespace": "default", + "annotations": map[string]interface{}{ + mutation.Annotation: "invalid-mutation", + }, + }, + }, + }, + testutil.Unstructured(t, resources["pod"], + mutationutil.AddApplyTimeMutation(t, &mutation.ApplyTimeMutation{ + { + SourceRef: mutation.ResourceReferenceFromObjMetadata( + testutil.ToIdentifier(t, resources["secret"]), + ), + }, + }), + ), + }, + expected: []Edge{}, + expectedError: multierror.New( + validation.NewError( + errors.New("failed to parse apply-time-mutation annotation: "+ + "error unmarshaling JSON: "+ + "while decoding JSON: json: "+ + "cannot unmarshal string into Go value of type mutation.ApplyTimeMutation"), + object.ObjMetadata{ + GroupKind: schema.GroupKind{ + Group: "apps", + Kind: "Deployment", + }, + Name: "foo", + Namespace: "default", + }, + ), + validation.NewError( + errors.New(`invalid "config.kubernetes.io/apply-time-mutation" annotation: `+ + "dependency not in object set: "+ + "/namespaces/test-namespace/Secret/secret"), + object.ObjMetadata{ + GroupKind: schema.GroupKind{ + Group: "", + Kind: "Pod", + }, + Name: "test-pod", + Namespace: "test-namespace", + }, + ), + ), + }, } for tn, tc := range testCases { t.Run(tn, func(t *testing.T) { g := New() - addApplyTimeMutationEdges(g, tc.objs) + ids := object.UnstructuredSetToObjMetadataSet(tc.objs) + err := addApplyTimeMutationEdges(g, tc.objs, ids) + if tc.expectedError != nil { + assert.EqualError(t, err, tc.expectedError.Error()) + } else { + assert.NoError(t, err) + } actual := g.GetEdges() verifyEdges(t, tc.expected, actual) }) @@ -501,8 +638,9 @@ func TestApplyTimeMutationEdges(t *testing.T) { func TestAddDependsOnEdges(t *testing.T) { testCases := map[string]struct { - objs []*unstructured.Unstructured - expected []Edge + objs []*unstructured.Unstructured + expected []Edge + expectedError error }{ "no objects adds no graph edges": { objs: []*unstructured.Unstructured{}, @@ -575,12 +713,184 @@ func TestAddDependsOnEdges(t *testing.T) { }, }, }, + "error: invalid annotation": { + objs: []*unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "foo", + "namespace": "default", + "annotations": map[string]interface{}{ + dependson.Annotation: "invalid-obj-ref", + }, + }, + }, + }, + }, + expected: []Edge{}, + expectedError: validation.NewError( + errors.New("failed to parse depends-on annotation: "+ + "failed to parse object metadata: "+ + "expected 3 or 5 fields, found 1: "+ + `"invalid-obj-ref"`), + object.ObjMetadata{ + GroupKind: schema.GroupKind{ + Group: "apps", + Kind: "Deployment", + }, + Name: "foo", + Namespace: "default", + }, + ), + }, + "error: duplicate reference": { + objs: []*unstructured.Unstructured{ + testutil.Unstructured(t, resources["pod"], + testutil.AddDependsOn(t, + testutil.ToIdentifier(t, resources["deployment"]), + testutil.ToIdentifier(t, resources["deployment"]), + ), + ), + testutil.Unstructured(t, resources["deployment"]), + }, + expected: []Edge{ + { + From: testutil.ToIdentifier(t, resources["pod"]), + To: testutil.ToIdentifier(t, resources["deployment"]), + }, + }, + expectedError: validation.NewError( + errors.New(`invalid "config.kubernetes.io/depends-on" annotation: `+ + "duplicate reference: "+ + "apps/namespaces/test-namespace/Deployment/foo"), + object.ObjMetadata{ + GroupKind: schema.GroupKind{ + Group: "", + Kind: "Pod", + }, + Name: "test-pod", + Namespace: "test-namespace", + }, + ), + }, + "error: dependency not in object set": { + objs: []*unstructured.Unstructured{ + testutil.Unstructured(t, resources["pod"], + testutil.AddDependsOn(t, + testutil.ToIdentifier(t, resources["deployment"]), + ), + ), + }, + expected: []Edge{}, + expectedError: validation.NewError( + errors.New(`invalid "config.kubernetes.io/depends-on" annotation: `+ + "dependency not in object set: "+ + "apps/namespaces/test-namespace/Deployment/foo"), + object.ObjMetadata{ + GroupKind: schema.GroupKind{ + Group: "", + Kind: "Pod", + }, + Name: "test-pod", + Namespace: "test-namespace", + }, + ), + }, + "error: two invalid objects": { + objs: []*unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "foo", + "namespace": "default", + "annotations": map[string]interface{}{ + dependson.Annotation: "invalid-obj-ref", + }, + }, + }, + }, + testutil.Unstructured(t, resources["pod"], + testutil.AddDependsOn(t, + testutil.ToIdentifier(t, resources["secret"]), + ), + ), + }, + expected: []Edge{}, + expectedError: multierror.New( + validation.NewError( + errors.New("failed to parse depends-on annotation: "+ + "failed to parse object metadata: "+ + "expected 3 or 5 fields, found 1: "+ + `"invalid-obj-ref"`), + object.ObjMetadata{ + GroupKind: schema.GroupKind{ + Group: "apps", + Kind: "Deployment", + }, + Name: "foo", + Namespace: "default", + }, + ), + validation.NewError( + errors.New(`invalid "config.kubernetes.io/depends-on" annotation: `+ + "dependency not in object set: "+ + "/namespaces/test-namespace/Secret/secret"), + object.ObjMetadata{ + GroupKind: schema.GroupKind{ + Group: "", + Kind: "Pod", + }, + Name: "test-pod", + Namespace: "test-namespace", + }, + ), + ), + }, + "error: one object with two errors": { + objs: []*unstructured.Unstructured{ + testutil.Unstructured(t, resources["pod"], + testutil.AddDependsOn(t, + testutil.ToIdentifier(t, resources["deployment"]), + testutil.ToIdentifier(t, resources["deployment"]), + ), + ), + }, + expected: []Edge{}, + expectedError: validation.NewError( + multierror.New( + errors.New(`invalid "config.kubernetes.io/depends-on" annotation: `+ + "dependency not in object set: "+ + "apps/namespaces/test-namespace/Deployment/foo"), + errors.New(`invalid "config.kubernetes.io/depends-on" annotation: `+ + "duplicate reference: "+ + "apps/namespaces/test-namespace/Deployment/foo"), + ), + object.ObjMetadata{ + GroupKind: schema.GroupKind{ + Group: "", + Kind: "Pod", + }, + Name: "test-pod", + Namespace: "test-namespace", + }, + ), + }, } for tn, tc := range testCases { t.Run(tn, func(t *testing.T) { g := New() - addDependsOnEdges(g, tc.objs) + ids := object.UnstructuredSetToObjMetadataSet(tc.objs) + err := addDependsOnEdges(g, tc.objs, ids) + if tc.expectedError != nil { + assert.EqualError(t, err, tc.expectedError.Error()) + } else { + assert.NoError(t, err) + } actual := g.GetEdges() verifyEdges(t, tc.expected, actual) }) @@ -656,7 +966,8 @@ func TestAddNamespaceEdges(t *testing.T) { for tn, tc := range testCases { t.Run(tn, func(t *testing.T) { g := New() - addNamespaceEdges(g, tc.objs) + ids := object.UnstructuredSetToObjMetadataSet(tc.objs) + addNamespaceEdges(g, tc.objs, ids) actual := g.GetEdges() verifyEdges(t, tc.expected, actual) }) @@ -707,7 +1018,8 @@ func TestAddCRDEdges(t *testing.T) { for tn, tc := range testCases { t.Run(tn, func(t *testing.T) { g := New() - addCRDEdges(g, tc.objs) + ids := object.UnstructuredSetToObjMetadataSet(tc.objs) + addCRDEdges(g, tc.objs, ids) actual := g.GetEdges() verifyEdges(t, tc.expected, actual) }) diff --git a/pkg/object/graph/graph.go b/pkg/object/graph/graph.go index 1286dec..d7deae8 100644 --- a/pkg/object/graph/graph.go +++ b/pkg/object/graph/graph.go @@ -10,7 +10,9 @@ import ( "bytes" "fmt" + "sigs.k8s.io/cli-utils/pkg/multierror" "sigs.k8s.io/cli-utils/pkg/object" + "sigs.k8s.io/cli-utils/pkg/object/validation" ) // Graph is contains a directed set of edges, implemented as @@ -43,6 +45,17 @@ func (g *Graph) AddVertex(v object.ObjMetadata) { } } +// GetVertices returns an unsorted set of unique vertices in the graph. +func (g *Graph) GetVertices() object.ObjMetadataSet { + keys := make(object.ObjMetadataSet, len(g.edges)) + i := 0 + for k := range g.edges { + keys[i] = k + i++ + } + return keys +} + // AddEdge adds a edge from one ObjMetadata vertex to another. The // direction of the edge is "from" -> "to". func (g *Graph) AddEdge(from object.ObjMetadata, to object.ObjMetadata) { @@ -121,9 +134,10 @@ func (g *Graph) Sort() ([]object.ObjMetadataSet, error) { // No leaf vertices means cycle in the directed graph, // where remaining edges define the cycle. if len(leafVertices) == 0 { - return []object.ObjMetadataSet{}, CyclicDependencyError{ + // Error can be ignored, so return the full set list + return sorted, validation.NewError(CyclicDependencyError{ Edges: g.GetEdges(), - } + }, g.GetVertices()...) } // Remove all edges to leaf vertices. for _, v := range leafVertices { @@ -142,11 +156,11 @@ type CyclicDependencyError struct { func (cde CyclicDependencyError) Error() string { var errorBuf bytes.Buffer - errorBuf.WriteString("cyclic dependency") + errorBuf.WriteString("cyclic dependency:\n") for _, edge := range cde.Edges { from := fmt.Sprintf("%s/%s", edge.From.Namespace, edge.From.Name) to := fmt.Sprintf("%s/%s", edge.To.Namespace, edge.To.Name) - errorBuf.WriteString(fmt.Sprintf("\n\t%s -> %s", from, to)) + errorBuf.WriteString(fmt.Sprintf("%s%s -> %s\n", multierror.Prefix, from, to)) } return errorBuf.String() } diff --git a/pkg/object/mutation/annotation.go b/pkg/object/mutation/annotation.go index d92cfe3..9b726e1 100644 --- a/pkg/object/mutation/annotation.go +++ b/pkg/object/mutation/annotation.go @@ -36,12 +36,12 @@ func ReadAnnotation(obj *unstructured.Unstructured) (ApplyTimeMutation, error) { return mutation, nil } if klog.V(5).Enabled() { - klog.Infof("resource (%v) has apply-time-mutation annotation:\n%s", NewResourceReference(obj), mutationYaml) + klog.Infof("resource (%v) has apply-time-mutation annotation:\n%s", ResourceReferenceFromUnstructured(obj), mutationYaml) } err := yaml.Unmarshal([]byte(mutationYaml), &mutation) if err != nil { - return nil, fmt.Errorf("failed to parse apply-time-mutation annotation: %q: %v", mutationYaml, err) + return nil, fmt.Errorf("failed to parse apply-time-mutation annotation: %v", err) } return mutation, nil } @@ -57,7 +57,7 @@ func WriteAnnotation(obj *unstructured.Unstructured, mutation ApplyTimeMutation) } yamlBytes, err := yaml.Marshal(mutation) if err != nil { - return fmt.Errorf("failed to format apply-time-mutation annotation: %#v: %v", mutation, err) + return fmt.Errorf("failed to format apply-time-mutation annotation: %v", err) } a := obj.GetAnnotations() if a == nil { diff --git a/pkg/object/mutation/types.go b/pkg/object/mutation/types.go index 7f894d5..dcf8c5d 100644 --- a/pkg/object/mutation/types.go +++ b/pkg/object/mutation/types.go @@ -95,8 +95,8 @@ type ResourceReference struct { Namespace string `json:"namespace,omitempty"` } -// NewResourceReference returns the object as a ResourceReference -func NewResourceReference(obj *unstructured.Unstructured) ResourceReference { +// ResourceReferenceFromUnstructured returns the object as a ResourceReference +func ResourceReferenceFromUnstructured(obj *unstructured.Unstructured) ResourceReference { return ResourceReference{ Name: obj.GetName(), Namespace: obj.GetNamespace(), @@ -105,6 +105,16 @@ func NewResourceReference(obj *unstructured.Unstructured) ResourceReference { } } +// ResourceReferenceFromObjMetadata returns the object as a ResourceReference +func ResourceReferenceFromObjMetadata(id object.ObjMetadata) ResourceReference { + return ResourceReference{ + Name: id.Name, + Namespace: id.Namespace, + Kind: id.GroupKind.Kind, + Group: id.GroupKind.Group, + } +} + // GroupVersionKind satisfies the ObjectKind interface for all objects that // embed TypeMeta. Prefers Group over APIVersion. func (r ResourceReference) GroupVersionKind() schema.GroupVersionKind { @@ -114,16 +124,6 @@ func (r ResourceReference) GroupVersionKind() schema.GroupVersionKind { return schema.FromAPIVersionAndKind(r.APIVersion, r.Kind) } -// ObjMetadata returns the name, namespace, group, and kind of the -// ResourceReference, wrapped in a new ObjMetadata object. -func (r ResourceReference) ObjMetadata() object.ObjMetadata { - return object.ObjMetadata{ - Name: r.Name, - Namespace: r.Namespace, - GroupKind: r.GroupVersionKind().GroupKind(), - } -} - // ToUnstructured returns the name, namespace, group, version, and kind of the // ResourceReference, wrapped in a new Unstructured object. // This is useful for performing operations with diff --git a/pkg/object/validate.go b/pkg/object/validate.go deleted file mode 100644 index 71cc812..0000000 --- a/pkg/object/validate.go +++ /dev/null @@ -1,157 +0,0 @@ -// Copyright 2021 The Kubernetes Authors. -// SPDX-License-Identifier: Apache-2.0 - -package object - -import ( - "errors" - "fmt" - "strings" - - "k8s.io/apimachinery/pkg/api/meta" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/util/validation/field" -) - -// MultiValidationError captures validation errors for multiple resources. -type MultiValidationError struct { - Errors []*ValidationError -} - -func (ae MultiValidationError) Error() string { - var b strings.Builder - _, _ = fmt.Fprintf(&b, "%d resources failed validation\n", len(ae.Errors)) - for _, e := range ae.Errors { - b.WriteString(e.Error()) - } - return b.String() -} - -// ValidationError captures errors resulting from validation of a resources. -type ValidationError struct { - GroupVersionKind schema.GroupVersionKind - Name string - Namespace string - FieldErrors field.ErrorList -} - -func (e *ValidationError) Error() string { - var b strings.Builder - b.WriteString(fmt.Sprintf("Resource: %q, Name: %q, Namespace: %q\n", - e.GroupVersionKind.String(), e.Name, e.Namespace)) - b.WriteString(e.FieldErrors.ToAggregate().Error()) - return b.String() -} - -// Validator contains functionality for validating a set of resources prior -// to being used by the Apply functionality. This imposes some constraint not -// always required, such as namespaced resources must have the namespace set. -type Validator struct { - Mapper meta.RESTMapper -} - -// Validate validates the provided resources. A RESTMapper will be used -// to fetch type information from the live cluster. -func (v *Validator) Validate(resources []*unstructured.Unstructured) error { - crds := findCRDs(resources) - var errs []*ValidationError - for _, r := range resources { - var errList field.ErrorList - if err := v.validateKind(r); err != nil { - if fieldErr, ok := isFieldError(err); ok { - errList = append(errList, fieldErr) - } else { - return err - } - } - if err := v.validateName(r); err != nil { - if fieldErr, ok := isFieldError(err); ok { - errList = append(errList, fieldErr) - } else { - return err - } - } - if err := v.validateNamespace(r, crds); err != nil { - if fieldErr, ok := isFieldError(err); ok { - errList = append(errList, fieldErr) - } else { - return err - } - } - if len(errList) > 0 { - errs = append(errs, &ValidationError{ - GroupVersionKind: r.GroupVersionKind(), - Name: r.GetName(), - Namespace: r.GetNamespace(), - FieldErrors: errList, - }) - } - } - - if len(errs) > 0 { - return &MultiValidationError{ - Errors: errs, - } - } - return nil -} - -// isFieldError checks if an error is of type *field.Error. If so, -// a reference to an error of that type is returned. -func isFieldError(err error) (*field.Error, bool) { - var fieldErr *field.Error - if errors.As(err, &fieldErr) { - return fieldErr, true - } - return nil, false -} - -// findCRDs looks through the provided resources and returns a slice with -// the resources that are CRDs. -func findCRDs(us []*unstructured.Unstructured) []*unstructured.Unstructured { - var crds []*unstructured.Unstructured - for _, u := range us { - if IsCRD(u) { - crds = append(crds, u) - } - } - return crds -} - -// validateKind validates the value of the kind field of the resource. -func (v *Validator) validateKind(u *unstructured.Unstructured) error { - if u.GetKind() == "" { - return field.Required(field.NewPath("kind"), "kind is required") - } - return nil -} - -// validateName validates the value of the name field of the resource. -func (v *Validator) validateName(u *unstructured.Unstructured) error { - if u.GetName() == "" { - return field.Required(field.NewPath("metadata", "name"), "name is required") - } - return nil -} - -// validateNamespace validates the value of the namespace field of the resource. -func (v *Validator) validateNamespace(u *unstructured.Unstructured, crds []*unstructured.Unstructured) error { - // skip namespace validation if kind is missing (avoid redundant error) - if u.GetKind() == "" { - return nil - } - scope, err := LookupResourceScope(u, crds, v.Mapper) - if err != nil { - return err - } - - ns := u.GetNamespace() - if scope == meta.RESTScopeNamespace && ns == "" { - return field.Required(field.NewPath("metadata", "namespace"), "namespace is required") - } - if scope == meta.RESTScopeRoot && ns != "" { - return field.Invalid(field.NewPath("metadata", "namespace"), ns, "namespace must be empty") - } - return nil -} diff --git a/pkg/object/validate_test.go b/pkg/object/validate_test.go deleted file mode 100644 index 4b9f369..0000000 --- a/pkg/object/validate_test.go +++ /dev/null @@ -1,289 +0,0 @@ -// Copyright 2021 The Kubernetes Authors. -// SPDX-License-Identifier: Apache-2.0 - -package object_test - -import ( - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "k8s.io/apimachinery/pkg/api/meta" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/util/validation/field" - cmdtesting "k8s.io/kubectl/pkg/cmd/testing" - "sigs.k8s.io/cli-utils/pkg/object" - "sigs.k8s.io/cli-utils/pkg/testutil" -) - -func TestValidate(t *testing.T) { - testCases := map[string]struct { - resources []*unstructured.Unstructured - expectedError error - }{ - "missing kind": { - resources: []*unstructured.Unstructured{ - { - Object: map[string]interface{}{ - "apiVersion": "apps/v1", - "metadata": map[string]interface{}{ - "name": "foo", - "namespace": "default", - }, - }, - }, - }, - expectedError: &object.MultiValidationError{ - Errors: []*object.ValidationError{ - { - GroupVersionKind: schema.GroupVersionKind{ - Group: "apps", - Version: "v1", - Kind: "", - }, - Name: "foo", - Namespace: "default", - FieldErrors: []*field.Error{ - { - Type: field.ErrorTypeRequired, - Field: "kind", - BadValue: "", - Detail: "kind is required", - }, - }, - }, - }, - }, - }, - "errors are reported for resources": { - resources: []*unstructured.Unstructured{ - testutil.Unstructured(t, ` -apiVersion: apps/v1 -kind: Deployment -`, - ), - }, - expectedError: &object.MultiValidationError{ - Errors: []*object.ValidationError{ - { - GroupVersionKind: schema.GroupVersionKind{ - Group: "apps", - Version: "v1", - Kind: "Deployment", - }, - Name: "", - Namespace: "", - FieldErrors: []*field.Error{ - { - Type: field.ErrorTypeRequired, - Field: "metadata.name", - BadValue: "", - Detail: "name is required", - }, - { - Type: field.ErrorTypeRequired, - Field: "metadata.namespace", - BadValue: "", - Detail: "namespace is required", - }, - }, - }, - }, - }, - }, - "error is reported for all resources": { - resources: []*unstructured.Unstructured{ - testutil.Unstructured(t, ` -apiVersion: apps/v1 -kind: Deployment -metadata: - namespace: default -`), - testutil.Unstructured(t, ` -apiVersion: apps/v1 -kind: StatefulSet -metadata: - namespace: default -`, - ), - }, - expectedError: &object.MultiValidationError{ - Errors: []*object.ValidationError{ - { - GroupVersionKind: schema.GroupVersionKind{ - Group: "apps", - Version: "v1", - Kind: "Deployment", - }, - Name: "", - Namespace: "default", - FieldErrors: []*field.Error{ - { - Type: field.ErrorTypeRequired, - Field: "metadata.name", - BadValue: "", - Detail: "name is required", - }, - }, - }, - { - GroupVersionKind: schema.GroupVersionKind{ - Group: "apps", - Version: "v1", - Kind: "StatefulSet", - }, - Name: "", - Namespace: "default", - FieldErrors: []*field.Error{ - { - Type: field.ErrorTypeRequired, - Field: "metadata.name", - BadValue: "", - Detail: "name is required", - }, - }, - }, - }, - }, - }, - "error is reported if a cluster-scoped resource has namespace set": { - resources: []*unstructured.Unstructured{ - testutil.Unstructured(t, ` -apiVersion: v1 -kind: Namespace -metadata: - name: foo - namespace: default -`, - ), - }, - expectedError: &object.MultiValidationError{ - Errors: []*object.ValidationError{ - { - GroupVersionKind: schema.GroupVersionKind{ - Group: "", - Version: "v1", - Kind: "Namespace", - }, - Name: "foo", - Namespace: "default", - FieldErrors: []*field.Error{ - { - Type: field.ErrorTypeInvalid, - Field: "metadata.namespace", - BadValue: "default", - Detail: "namespace must be empty", - }, - }, - }, - }, - }, - }, - "error is reported if a namespace-scoped resource doesn't have namespace set": { - resources: []*unstructured.Unstructured{ - testutil.Unstructured(t, ` -apiVersion: apps/v1 -kind: Deployment -metadata: - name: foo -`, - ), - }, - expectedError: &object.MultiValidationError{ - Errors: []*object.ValidationError{ - { - GroupVersionKind: schema.GroupVersionKind{ - Group: "apps", - Version: "v1", - Kind: "Deployment", - }, - Name: "foo", - Namespace: "", - FieldErrors: []*field.Error{ - { - Type: field.ErrorTypeRequired, - Field: "metadata.namespace", - BadValue: "", - Detail: "namespace is required", - }, - }, - }, - }, - }, - }, - "scope for CRs are found in CRDs if available": { - resources: []*unstructured.Unstructured{ - testutil.Unstructured(t, ` -apiVersion: apiextensions.k8s.io/v1 -kind: CustomResourceDefinition -metadata: - name: customs.custom.io -spec: - group: custom.io - names: - kind: Custom - scope: Cluster - versions: - - name: v1 -`, - ), - testutil.Unstructured(t, ` -apiVersion: custom.io/v1 -kind: Custom -metadata: - name: foo - namespace: default -`, - ), - }, - expectedError: &object.MultiValidationError{ - Errors: []*object.ValidationError{ - { - GroupVersionKind: schema.GroupVersionKind{ - Group: "custom.io", - Version: "v1", - Kind: "Custom", - }, - Name: "foo", - Namespace: "default", - FieldErrors: []*field.Error{ - { - Type: field.ErrorTypeInvalid, - Field: "metadata.namespace", - BadValue: "default", - Detail: "namespace must be empty", - }, - }, - }, - }, - }, - }, - } - - for tn, tc := range testCases { - t.Run(tn, func(t *testing.T) { - tf := cmdtesting.NewTestFactory().WithNamespace("test-ns") - defer tf.Cleanup() - - mapper, err := tf.ToRESTMapper() - require.NoError(t, err) - crdGV := schema.GroupVersion{Group: "apiextensions.k8s.io", Version: "v1"} - crdMapper := meta.NewDefaultRESTMapper([]schema.GroupVersion{crdGV}) - crdMapper.AddSpecific(crdGV.WithKind("CustomResourceDefinition"), - crdGV.WithResource("customresourcedefinitions"), - crdGV.WithResource("customresourcedefinition"), meta.RESTScopeRoot) - mapper = meta.MultiRESTMapper([]meta.RESTMapper{mapper, crdMapper}) - - validator := &object.Validator{ - Mapper: mapper, - } - err = validator.Validate(tc.resources) - if tc.expectedError == nil { - assert.NoError(t, err) - return - } - require.EqualError(t, err, tc.expectedError.Error()) - }) - } -} diff --git a/pkg/object/validation/error.go b/pkg/object/validation/error.go new file mode 100644 index 0000000..e781de1 --- /dev/null +++ b/pkg/object/validation/error.go @@ -0,0 +1,53 @@ +// Copyright 2022 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package validation + +import ( + "fmt" + "strings" + + "sigs.k8s.io/cli-utils/pkg/object" +) + +func NewError(cause error, ids ...object.ObjMetadata) *Error { + return &Error{ + ids: object.ObjMetadataSet(ids), + cause: cause, + } +} + +// Error wraps an error with the object or objects it applies to. +type Error struct { + ids object.ObjMetadataSet + cause error +} + +// Identifiers returns zero or more object IDs which are invalid. +func (ve *Error) Identifiers() object.ObjMetadataSet { + return ve.ids +} + +// Unwrap returns the cause of the error. +// This may be useful when printing the cause without printing the identifiers. +func (ve *Error) Unwrap() error { + return ve.cause +} + +// Error stringifies the the error. +func (ve *Error) Error() string { + switch { + case len(ve.ids) == 0: + return fmt.Sprintf("validation error: %v", ve.cause.Error()) + case len(ve.ids) == 1: + return fmt.Sprintf("invalid object: %q: %v", ve.ids[0], ve.cause.Error()) + default: + var b strings.Builder + _, _ = fmt.Fprintf(&b, "invalid objects: [%q", ve.ids[0]) + for _, id := range ve.ids[1:] { + _, _ = fmt.Fprintf(&b, ", %q", id) + } + _, _ = fmt.Fprintf(&b, "] %v", ve.cause) + return b.String() + } +} diff --git a/pkg/object/validation/validate.go b/pkg/object/validation/validate.go new file mode 100644 index 0000000..ad7b68f --- /dev/null +++ b/pkg/object/validation/validate.go @@ -0,0 +1,95 @@ +// Copyright 2022 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package validation + +import ( + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/cli-utils/pkg/multierror" + "sigs.k8s.io/cli-utils/pkg/object" +) + +// Validator contains functionality for validating a set of resources prior +// to being used by the Apply functionality. This imposes some constraint not +// always required, such as namespaced resources must have the namespace set. +type Validator struct { + Mapper meta.RESTMapper +} + +// Validate validates the provided resources. A RESTMapper will be used +// to fetch type information from the live cluster. +func (v *Validator) Validate(objs []*unstructured.Unstructured) error { + crds := findCRDs(objs) + var errs []error + for _, obj := range objs { + var objErrors []error + if err := v.validateKind(obj); err != nil { + objErrors = append(objErrors, err) + } + if err := v.validateName(obj); err != nil { + objErrors = append(objErrors, err) + } + if err := v.validateNamespace(obj, crds); err != nil { + objErrors = append(objErrors, err) + } + if len(objErrors) > 0 { + errs = append(errs, NewError(multierror.Wrap(objErrors...), + object.UnstructuredToObjMetadata(obj))) + } + } + if len(errs) > 0 { + return multierror.Wrap(errs...) + } + return nil +} + +// findCRDs looks through the provided resources and returns a slice with +// the resources that are CRDs. +func findCRDs(us []*unstructured.Unstructured) []*unstructured.Unstructured { + var crds []*unstructured.Unstructured + for _, u := range us { + if object.IsCRD(u) { + crds = append(crds, u) + } + } + return crds +} + +// validateKind validates the value of the kind field of the resource. +func (v *Validator) validateKind(u *unstructured.Unstructured) error { + if u.GetKind() == "" { + return field.Required(field.NewPath("kind"), "kind is required") + } + return nil +} + +// validateName validates the value of the name field of the resource. +func (v *Validator) validateName(u *unstructured.Unstructured) error { + if u.GetName() == "" { + return field.Required(field.NewPath("metadata", "name"), "name is required") + } + return nil +} + +// validateNamespace validates the value of the namespace field of the resource. +func (v *Validator) validateNamespace(u *unstructured.Unstructured, crds []*unstructured.Unstructured) error { + // skip namespace validation if kind is missing (avoid redundant error) + if u.GetKind() == "" { + return nil + } + scope, err := object.LookupResourceScope(u, crds, v.Mapper) + if err != nil { + return err + } + + ns := u.GetNamespace() + if scope == meta.RESTScopeNamespace && ns == "" { + return field.Required(field.NewPath("metadata", "namespace"), "namespace is required") + } + if scope == meta.RESTScopeRoot && ns != "" { + return field.Invalid(field.NewPath("metadata", "namespace"), ns, "namespace must be empty") + } + return nil +} diff --git a/pkg/object/validation/validate_test.go b/pkg/object/validation/validate_test.go new file mode 100644 index 0000000..d4283ad --- /dev/null +++ b/pkg/object/validation/validate_test.go @@ -0,0 +1,274 @@ +// Copyright 2022 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package validation_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/validation/field" + cmdtesting "k8s.io/kubectl/pkg/cmd/testing" + "sigs.k8s.io/cli-utils/pkg/multierror" + "sigs.k8s.io/cli-utils/pkg/object" + "sigs.k8s.io/cli-utils/pkg/object/validation" + "sigs.k8s.io/cli-utils/pkg/testutil" +) + +func TestValidate(t *testing.T) { + testCases := map[string]struct { + resources []*unstructured.Unstructured + expectedError error + }{ + "missing kind": { + resources: []*unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "metadata": map[string]interface{}{ + "name": "foo", + "namespace": "default", + }, + }, + }, + }, + expectedError: validation.NewError( + &field.Error{ + Type: field.ErrorTypeRequired, + Field: "kind", + BadValue: "", + Detail: "kind is required", + }, + object.ObjMetadata{ + GroupKind: schema.GroupKind{ + Group: "apps", + Kind: "", + }, + Name: "foo", + Namespace: "default", + }, + ), + }, + "multiple errors in one object": { + resources: []*unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + }, + }, + }, + expectedError: validation.NewError( + multierror.New( + &field.Error{ + Type: field.ErrorTypeRequired, + Field: "metadata.name", + BadValue: "", + Detail: "name is required", + }, + &field.Error{ + Type: field.ErrorTypeRequired, + Field: "metadata.namespace", + BadValue: "", + Detail: "namespace is required", + }, + ), + object.ObjMetadata{ + GroupKind: schema.GroupKind{ + Group: "apps", + Kind: "Deployment", + }, + Name: "", + Namespace: "", + }, + ), + }, + "one error in multiple object": { + resources: []*unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "namespace": "default", + }, + }, + }, + { + Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "StatefulSet", + "metadata": map[string]interface{}{ + "namespace": "default", + }, + }, + }, + }, + expectedError: multierror.New( + validation.NewError( + &field.Error{ + Type: field.ErrorTypeRequired, + Field: "metadata.name", + BadValue: "", + Detail: "name is required", + }, + object.ObjMetadata{ + GroupKind: schema.GroupKind{ + Group: "apps", + Kind: "Deployment", + }, + Name: "", + Namespace: "default", + }, + ), + validation.NewError( + &field.Error{ + Type: field.ErrorTypeRequired, + Field: "metadata.name", + BadValue: "", + Detail: "name is required", + }, + object.ObjMetadata{ + GroupKind: schema.GroupKind{ + Group: "apps", + Kind: "StatefulSet", + }, + Name: "", + Namespace: "default", + }, + ), + ), + }, + "namespace must be empty (cluster-scoped)": { + resources: []*unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Namespace", + "metadata": map[string]interface{}{ + "name": "foo", + "namespace": "default", + }, + }, + }, + }, + expectedError: validation.NewError( + &field.Error{ + Type: field.ErrorTypeInvalid, + Field: "metadata.namespace", + BadValue: "default", + Detail: "namespace must be empty", + }, + object.ObjMetadata{ + GroupKind: schema.GroupKind{ + Group: "", + Kind: "Namespace", + }, + Name: "foo", + Namespace: "default", + }, + ), + }, + "namespace is required (namespace-scoped)": { + resources: []*unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "foo", + }, + }, + }, + }, + expectedError: validation.NewError( + &field.Error{ + Type: field.ErrorTypeRequired, + Field: "metadata.namespace", + BadValue: "", + Detail: "namespace is required", + }, + object.ObjMetadata{ + GroupKind: schema.GroupKind{ + Group: "apps", + Kind: "Deployment", + }, + Name: "foo", + Namespace: "", + }, + ), + }, + "scope for CRs are found in CRDs if available": { + resources: []*unstructured.Unstructured{ + testutil.Unstructured(t, ` +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: customs.custom.io +spec: + group: custom.io + names: + kind: Custom + scope: Cluster + versions: + - name: v1 +`, + ), + testutil.Unstructured(t, ` +apiVersion: custom.io/v1 +kind: Custom +metadata: + name: foo + namespace: default +`, + ), + }, + expectedError: validation.NewError( + &field.Error{ + Type: field.ErrorTypeInvalid, + Field: "metadata.namespace", + BadValue: "default", + Detail: "namespace must be empty", + }, + object.ObjMetadata{ + GroupKind: schema.GroupKind{ + Group: "custom.io", + Kind: "Custom", + }, + Name: "foo", + Namespace: "default", + }, + ), + }, + } + + for tn, tc := range testCases { + t.Run(tn, func(t *testing.T) { + tf := cmdtesting.NewTestFactory().WithNamespace("test-ns") + defer tf.Cleanup() + + mapper, err := tf.ToRESTMapper() + require.NoError(t, err) + crdGV := schema.GroupVersion{Group: "apiextensions.k8s.io", Version: "v1"} + crdMapper := meta.NewDefaultRESTMapper([]schema.GroupVersion{crdGV}) + crdMapper.AddSpecific(crdGV.WithKind("CustomResourceDefinition"), + crdGV.WithResource("customresourcedefinitions"), + crdGV.WithResource("customresourcedefinition"), meta.RESTScopeRoot) + mapper = meta.MultiRESTMapper([]meta.RESTMapper{mapper, crdMapper}) + + validator := &validation.Validator{ + Mapper: mapper, + } + err = validator.Validate(tc.resources) + if tc.expectedError == nil { + assert.NoError(t, err) + return + } + require.EqualError(t, err, tc.expectedError.Error()) + }) + } +} diff --git a/test/e2e/artifacts_test.go b/test/e2e/artifacts_test.go index 1ed025f..c0327fa 100644 --- a/test/e2e/artifacts_test.go +++ b/test/e2e/artifacts_test.go @@ -95,23 +95,25 @@ spec: image: k8s.gcr.io/pause:2.0 `)) -var podA = []byte(strings.TrimSpace(` +var podATemplate = ` kind: Pod apiVersion: v1 metadata: name: pod-a - namespace: test + namespace: {{.Namespace}} annotations: config.kubernetes.io/apply-time-mutation: | - sourceRef: kind: Pod name: pod-b + namespace: {{.Namespace}} sourcePath: $.status.podIP targetPath: $.spec.containers[?(@.name=="nginx")].env[?(@.name=="SERVICE_HOST")].value token: ${pob-b-ip} - sourceRef: kind: Pod name: pod-b + namespace: {{.Namespace}} sourcePath: $.spec.containers[?(@.name=="nginx")].ports[?(@.name=="tcp")].containerPort targetPath: $.spec.containers[?(@.name=="nginx")].env[?(@.name=="SERVICE_HOST")].value token: ${pob-b-port} @@ -125,14 +127,14 @@ spec: env: - name: SERVICE_HOST value: "${pob-b-ip}:${pob-b-port}" -`)) +` -var podB = []byte(strings.TrimSpace(` +var podBTemplate = ` kind: Pod apiVersion: v1 metadata: name: pod-b - namespace: test + namespace: {{.Namespace}} spec: containers: - name: nginx @@ -140,4 +142,4 @@ spec: ports: - name: tcp containerPort: 80 -`)) +` diff --git a/test/e2e/common_test.go b/test/e2e/common_test.go index 5944d93..d723dc3 100644 --- a/test/e2e/common_test.go +++ b/test/e2e/common_test.go @@ -4,8 +4,10 @@ package e2e import ( + "bytes" "context" "fmt" + "html/template" "strings" "time" @@ -91,7 +93,7 @@ func withDependsOn(obj *unstructured.Unstructured, dep string) *unstructured.Uns } func deleteUnstructuredAndWait(ctx context.Context, c client.Client, obj *unstructured.Unstructured) { - ref := mutation.NewResourceReference(obj) + ref := mutation.ResourceReferenceFromUnstructured(obj) err := c.Delete(ctx, obj, client.PropagationPolicy(metav1.DeletePropagationForeground)) @@ -102,7 +104,7 @@ func deleteUnstructuredAndWait(ctx context.Context, c client.Client, obj *unstru } func waitForDeletion(ctx context.Context, c client.Client, obj *unstructured.Unstructured) { - ref := mutation.NewResourceReference(obj) + ref := mutation.ResourceReferenceFromUnstructured(obj) resultObj := ref.ToUnstructured() timeout := 30 * time.Second @@ -133,7 +135,7 @@ func waitForDeletion(ctx context.Context, c client.Client, obj *unstructured.Uns } func createUnstructuredAndWait(ctx context.Context, c client.Client, obj *unstructured.Unstructured) { - ref := mutation.NewResourceReference(obj) + ref := mutation.ResourceReferenceFromUnstructured(obj) err := c.Create(ctx, obj) Expect(err).NotTo(HaveOccurred(), @@ -143,7 +145,7 @@ func createUnstructuredAndWait(ctx context.Context, c client.Client, obj *unstru } func waitForCreation(ctx context.Context, c client.Client, obj *unstructured.Unstructured) { - ref := mutation.NewResourceReference(obj) + ref := mutation.ResourceReferenceFromUnstructured(obj) resultObj := ref.ToUnstructured() timeout := 30 * time.Second @@ -175,7 +177,7 @@ func waitForCreation(ctx context.Context, c client.Client, obj *unstructured.Uns } func assertUnstructuredExists(ctx context.Context, c client.Client, obj *unstructured.Unstructured) *unstructured.Unstructured { - ref := mutation.NewResourceReference(obj) + ref := mutation.ResourceReferenceFromUnstructured(obj) resultObj := ref.ToUnstructured() err := c.Get(ctx, types.NamespacedName{ @@ -188,7 +190,7 @@ func assertUnstructuredExists(ctx context.Context, c client.Client, obj *unstruc } func assertUnstructuredDoesNotExist(ctx context.Context, c client.Client, obj *unstructured.Unstructured) { - ref := mutation.NewResourceReference(obj) + ref := mutation.ResourceReferenceFromUnstructured(obj) resultObj := ref.ToUnstructured() err := c.Get(ctx, types.NamespacedName{ @@ -284,3 +286,16 @@ func manifestToUnstructured(manifest []byte) *unstructured.Unstructured { Object: u, } } + +func templateToUnstructured(tmpl string, data interface{}) *unstructured.Unstructured { + t, err := template.New("manifest").Parse(tmpl) + if err != nil { + panic(fmt.Errorf("failed to parse manifest go-template: %w", err)) + } + var buffer bytes.Buffer + err = t.Execute(&buffer, data) + if err != nil { + panic(fmt.Errorf("failed to execute manifest go-template: %w", err)) + } + return manifestToUnstructured(buffer.Bytes()) +} diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index aa66bda..a5c0aee 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -142,14 +142,15 @@ var _ = Describe("Applier", func() { ctx, cancel = context.WithTimeout(context.Background(), defaultAfterTestTimeout) defer cancel() // clean up resources created by the tests + fields := struct{ Namespace string }{Namespace: namespace.GetName()} objs := []*unstructured.Unstructured{ manifestToUnstructured(cr), manifestToUnstructured(crd), withNamespace(manifestToUnstructured(pod1), namespace.GetName()), withNamespace(manifestToUnstructured(pod2), namespace.GetName()), withNamespace(manifestToUnstructured(pod3), namespace.GetName()), - withNamespace(manifestToUnstructured(podA), namespace.GetName()), - withNamespace(manifestToUnstructured(podB), namespace.GetName()), + templateToUnstructured(podATemplate, fields), + templateToUnstructured(podBTemplate, fields), withNamespace(manifestToUnstructured(deployment1), namespace.GetName()), manifestToUnstructured(apiservice1), } diff --git a/test/e2e/mutation_test.go b/test/e2e/mutation_test.go index 200c15c..0a8a70e 100644 --- a/test/e2e/mutation_test.go +++ b/test/e2e/mutation_test.go @@ -38,8 +38,9 @@ func mutationTest(ctx context.Context, c client.Client, invConfig InventoryConfi inv := invConfig.InvWrapperFunc(invConfig.InventoryFactoryFunc(inventoryName, namespaceName, "test")) - podAObj := withNamespace(manifestToUnstructured(podA), namespaceName) - podBObj := withNamespace(manifestToUnstructured(podB), namespaceName) + fields := struct{ Namespace string }{Namespace: namespaceName} + podAObj := templateToUnstructured(podATemplate, fields) + podBObj := templateToUnstructured(podBTemplate, fields) // Dependency order: podA -> podB // Apply order: podB, podA