diff --git a/api/builtins/HashTransformer.go b/api/builtins/HashTransformer.go index 2e48923db..54586beeb 100644 --- a/api/builtins/HashTransformer.go +++ b/api/builtins/HashTransformer.go @@ -11,7 +11,7 @@ import ( ) type HashTransformerPlugin struct { - hasher ifc.KunstructuredHasher + hasher ifc.KustHasher } func (p *HashTransformerPlugin) Config( diff --git a/api/filters/namespace/namespace.go b/api/filters/namespace/namespace.go index 85e56c572..69458e198 100644 --- a/api/filters/namespace/namespace.go +++ b/api/filters/namespace/namespace.go @@ -118,7 +118,6 @@ func (ns Filter) roleBindingHack(obj *yaml.RNode, meta yaml.ResourceMeta) error // add the namespace to each "subject" with name: default err = obj.VisitElements(func(o *yaml.RNode) error { - // copied from kunstruct based kustomize NamespaceTransformer plugin // The only case we need to force the namespace // if for the "service account". "default" is // kind of hardcoded here for right now. diff --git a/api/hasher/hasher_test.go b/api/hasher/hasher_test.go index 5a31b934b..594cb6912 100644 --- a/api/hasher/hasher_test.go +++ b/api/hasher/hasher_test.go @@ -170,19 +170,18 @@ data: } } -func TestUnstructuredHash(t *testing.T) { - cases := []struct { - desc string - unstructured string - hash string - err string +func TestBasicHash(t *testing.T) { + cases := map[string]struct { + res string + hash string + err string }{ - {"minimal", ` + "minimal": {` apiVersion: test/v1 kind: TestResource metadata: name: my-resource`, "244782mkb7", ""}, - {"with spec", ` + "with spec": {` apiVersion: test/v1 kind: TestResource metadata: @@ -192,18 +191,21 @@ spec: bar: abc`, "59m2mdccg4", ""}, } h := &Hasher{} - for _, c := range cases { - node, err := yaml.Parse(c.unstructured) - if err != nil { - t.Fatal(err) - } - hashed, err := h.Hash(node) - if SkipRest(t, c.desc, err, c.err) { - continue - } - if c.hash != hashed { - t.Errorf("case %q, expect hash %q but got %q", c.desc, c.hash, h) - } + for n := range cases { + c := cases[n] + t.Run(n, func(t *testing.T) { + node, err := yaml.Parse(c.res) + if err != nil { + t.Fatal(err) + } + hashed, err := h.Hash(node) + if SkipRest(t, n, err, c.err) { + return + } + if c.hash != hashed { + t.Errorf("case %q, expect hash %q but got %q", n, c.hash, h) + } + }) } } diff --git a/api/ifc/ifc.go b/api/ifc/ifc.go index acbfa868b..9fd297478 100644 --- a/api/ifc/ifc.go +++ b/api/ifc/ifc.go @@ -5,7 +5,6 @@ package ifc import ( - "sigs.k8s.io/kustomize/api/resid" "sigs.k8s.io/kustomize/api/types" "sigs.k8s.io/kustomize/kyaml/yaml" ) @@ -39,91 +38,9 @@ type Loader interface { Cleanup() error } -// Kunstructured represents a Kubernetes Resource Model object. -type Kunstructured interface { - // Several uses. - Copy() Kunstructured - - // GetAnnotations returns the k8s annotations. - GetAnnotations() map[string]string - - // GetData returns a top-level "data" field, as in a ConfigMap. - GetDataMap() map[string]string - - // GetData returns a top-level "binaryData" field, as in a ConfigMap. - GetBinaryDataMap() map[string]string - - // Used by ResAccumulator and ReplacementTransformer. - GetFieldValue(string) (interface{}, error) - - // Used by Resource.OrgId - GetGvk() resid.Gvk - - // Used by resource.Factory.SliceFromBytes - GetKind() string - - // GetLabels returns the k8s labels. - GetLabels() map[string]string - - // Used by Resource.CurId and resource factory. - GetName() string - - // Used by special case code in - // ResMap.SubsetThatCouldBeReferencedByResource - GetSlice(path string) ([]interface{}, error) - - // GetString returns the value of a string field. - // Used by Resource.GetNamespace - GetString(string) (string, error) - - // Several uses. - Map() (map[string]interface{}, error) - - // Used by Resource.AsYAML and Resource.String - MarshalJSON() ([]byte, error) - - // Used by resWrangler.Select - MatchesAnnotationSelector(selector string) (bool, error) - - // Used by resWrangler.Select - MatchesLabelSelector(selector string) (bool, error) - - // SetAnnotations replaces the k8s annotations. - SetAnnotations(map[string]string) - - // SetDataMap sets a top-level "data" field, as in a ConfigMap. - SetDataMap(map[string]string) - - // SetDataMap sets a top-level "binaryData" field, as in a ConfigMap. - SetBinaryDataMap(map[string]string) - // Used by PatchStrategicMergeTransformer. - SetGvk(resid.Gvk) - - // SetLabels replaces the k8s labels. - SetLabels(map[string]string) - - // SetName changes the name. - SetName(string) - - // SetNamespace changes the namespace. - SetNamespace(string) - - // Needed, for now, by kyaml/filtersutil.ApplyToJSON. - UnmarshalJSON([]byte) error -} - -// KunstructuredFactory makes instances of Kunstructured. -type KunstructuredFactory interface { - SliceFromBytes([]byte) ([]Kunstructured, error) - FromMap(m map[string]interface{}) Kunstructured - Hasher() KunstructuredHasher - MakeConfigMap(kvLdr KvLoader, args *types.ConfigMapArgs) (Kunstructured, error) - MakeSecret(kvLdr KvLoader, args *types.SecretArgs) (Kunstructured, error) -} - -// KunstructuredHasher returns a hash of the argument +// KustHasher returns a hash of the argument // or an error. -type KunstructuredHasher interface { +type KustHasher interface { Hash(*yaml.RNode) (string, error) } diff --git a/api/provider/depprovider.go b/api/provider/depprovider.go index c6adba684..700b2e567 100644 --- a/api/provider/depprovider.go +++ b/api/provider/depprovider.go @@ -11,96 +11,14 @@ import ( "sigs.k8s.io/kustomize/api/resource" ) -// DepProvider is a dependency provider. +// DepProvider is a dependency provider, injecting different +// implementations depending on the context. // -// The instances it returns are either -// - old implementations backed by k8sdeps code, -// - new implementations backed by kyaml code. +// Notes on interfaces: // -// History: +// - resource.ConflictDetector // -// kubectl depends on k8s.io code, and at the time of writing, so -// does kustomize. Code that imports k8s.io/api* cannot be imported -// back into k8s.io/*, yet kustomize appears inside k8s.io/kubectl. -// -// To allow kustomize to appear inside kubectl, yet still be developed -// outside kubectl, the kustomize code was divided into the following -// packages -// -// api/ -// k8sdeps/ (and internal/ks8deps/) -// ifc/ -// krusty/ -// everythingElse/ -// -// with the following rules: -// -// - Only k8sdeps/ may import k8s.io/api*. -// -// - Only krusty/ (and its internals) may import k8sdeps/. -// I.e., ifc/ and everythingElse/ must not -// import k8sdeps/ or k8s.io/api*. -// -// - Code in krusty/ may use code in k8sdeps/ to create -// objects then inject said objects into -// everythingElse/ behind dependency neutral interfaces. -// -// The idea was to periodically copy, not import, the large k8sdeps/ -// tree (plus a snippet from krusty/kustomizer.go) into the kubectl -// codebase via a large PR, and have kubectl depend on the rest via -// normal importing. -// -// Over 2019, however, kubectl underwent large changes including -// a switch to Go modules, and a concerted attempt to extract kubectl -// from the k8s repo. This made large kustomize integration PRs too -// intrusive to review. -// -// In 2020, kubectl is based on Go modules, and almost entirely -// extracted from the k8s.io repositories, and further the kyaml -// library has a appeared as a viable replacement to k8s.io/api* -// KRM manipulation code. -// -// The new plan is to eliminate k8sdeps/ entirely, along with its -// k8s.io/api* dependence, allowing kustomize code to be imported -// into kubectl via normal Go module imports. Then the kustomize API -// code can then move into the github.com/kubernetes-sigs/cli-utils -// repo. The kustomize CLI in github.com/kubernetes-sigs/kustomize -// and the kubectl CLI can then both depend on the kustomize API. -// -// So, all code that depends on k8sdeps must go behind interfaces, -// and kustomize must be factored to choose the implementation. -// -// That problem has been reduced to three interfaces, each having -// two implementations. (1) is k8sdeps-based, (2) is kyaml-based. -// -// - ifc.Kunstructured -// -// 1) api/k8sdeps/kunstruct.UnstructAdapter -// -// This adapts structs in -// k8s.io/apimachinery/pkg/apis/meta/v1/unstructured -// to ifc.Kunstructured. -// -// 2) api/wrappy.WNode -// -// This adapts sigs.k8s.io/kustomize/kyaml/yaml.RNode -// to ifc.Unstructured. -// -// At time of writing, implementation started. -// Further reducing the size of ifc.Kunstructed -// would really reduce the work -// (e.g. drop Vars, drop ReplacementTranformer). -// -// - resource.ConflictDetector -// -// 1) api/internal/k8sdeps/conflict.conflictDetectorJson -// api/internal/k8sdeps/conflict.conflictDetectorSm -// -// Uses k8s.io/apimachinery/pkg/util/strategicpatch, -// apimachinery/pkg/util/mergepatch, etc. to merge -// resource.Resource instances. -// -// 2) api/internal/conflict.smPatchMergeOnlyDetector +// implemented by api/internal/conflict.smPatchMergeOnlyDetector // // At time of writing, this doesn't report conflicts, // but it does know how to merge patches. Conflict @@ -113,37 +31,14 @@ import ( // is plainly visible and usable in the output, even if // a conflict happened but wasn't reported as an error. // -// - ifc.Validator +// - ifc.Validator // -// 1) api/k8sdeps/validator.KustValidator -// -// Uses k8s.io/apimachinery/pkg/api/validation and -// friends to validate strings. -// -// 2) api/internal/validate.FieldValidator +// implemented by api/internal/validate.FieldValidator // // See TODO inside the validator for status. // At time of writing, this is a do-nothing // validator as it's not critical to kustomize function. // -// Proposed plan: -// [x] Ship kustomize with the ability to switch from 1 to 2 via -// an --enable_kyaml flag. -// [x] Make --enable_kyaml true by default. -// [x] When 2 is not noticeably more buggy than 1, delete 1. -// I.e. delete k8sdeps/, transitively deleting all k8s.io/api* deps. -// This DepProvider should be left in place to retain these -// comments, but it will have only one choice. -// [x] The way is now clear to reintegrate into kubectl. -// This should be done ASAP; the last step is cleanup. -// [ ] Cleanup. With only one impl of Kunstructure remaining, -// that interface and WNode can be deleted, along with this -// DepProvider. The other two interfaces could be dropped too. -// -// When the above is done, kustomize will use yaml.RNode and/or -// KRM Config Functions directly and exclusively. -// If you're reading this, plan not done. -// type DepProvider struct { resourceFactory *resource.Factory conflictDectectorFactory resource.ConflictDetectorFactory diff --git a/api/resmap/reswrangler.go b/api/resmap/reswrangler.go index a5e0db38c..acdceed23 100644 --- a/api/resmap/reswrangler.go +++ b/api/resmap/reswrangler.go @@ -323,9 +323,9 @@ func (m *resWrangler) ErrorIfNotEqualSets(other ResMap) error { "id in self matches %d in other; id: %s", len(others), id) } r2 := others[0] - if !r1.KunstructEqual(r2) { + if !r1.NodeEqual(r2) { return fmt.Errorf( - "kunstruct not equal: \n -- %s,\n -- %s\n\n--\n%#v\n------\n%#v\n", + "nodes unequal: \n -- %s,\n -- %s\n\n--\n%#v\n------\n%#v\n", r1, r2, r1, r2) } seen[m2.indexOfResource(r2)] = true diff --git a/api/resource/factory.go b/api/resource/factory.go index b8730b489..a9a1006bc 100644 --- a/api/resource/factory.go +++ b/api/resource/factory.go @@ -21,16 +21,16 @@ import ( // Factory makes instances of Resource. type Factory struct { - hasher ifc.KunstructuredHasher + hasher ifc.KustHasher } // NewFactory makes an instance of Factory. -func NewFactory(h ifc.KunstructuredHasher) *Factory { +func NewFactory(h ifc.KustHasher) *Factory { return &Factory{hasher: h} } -// Hasher returns an ifc.KunstructuredHasher -func (rf *Factory) Hasher() ifc.KunstructuredHasher { +// Hasher returns an ifc.KustHasher +func (rf *Factory) Hasher() ifc.KustHasher { return rf.hasher } @@ -69,7 +69,7 @@ func (rf *Factory) makeOne(rn *yaml.RNode, o *types.GenArgs) *Resource { if o == nil { o = types.NewGenArgs(nil) } - return &Resource{kunStr: rn, options: o} + return &Resource{node: rn, options: o} } // SliceFromPatches returns a slice of resources given a patch path diff --git a/api/resource/factory_test.go b/api/resource/factory_test.go index e9267e3ce..f40286210 100644 --- a/api/resource/factory_test.go +++ b/api/resource/factory_test.go @@ -337,7 +337,7 @@ kind: List // The error using kyaml is: // json: unsupported type: map[interface {}]interface {} // maybe arising from too many conversions between - // yaml, json, Resource, RNode, Unstructured etc. + // yaml, json, Resource, RNode, etc. // These conversions go away after closing #3506 // TODO(#3271) This shouldn't have an error, but does when kyaml is used. expectedErr: true, diff --git a/api/resource/resource.go b/api/resource/resource.go index bd7da5706..ba5e579d5 100644 --- a/api/resource/resource.go +++ b/api/resource/resource.go @@ -21,11 +21,10 @@ import ( "sigs.k8s.io/yaml" ) -// Resource is a representation of a Kubernetes Resource Model (KRM) object +// Resource is an RNode, representing a Kubernetes Resource Model object, // paired with metadata used by kustomize. -// For more history, see sigs.k8s.io/kustomize/api/ifc.Unstructured type Resource struct { - kunStr *kyaml.RNode + node *kyaml.RNode options *types.GenArgs refBy []resid.ResId refVarNames []string @@ -55,15 +54,15 @@ var buildAnnotations = []string{ } func (r *Resource) AsRNode() *kyaml.RNode { - return r.kunStr.Copy() + return r.node.Copy() } func (r *Resource) ResetPrimaryData(incoming *Resource) { - r.kunStr = incoming.kunStr.Copy() + r.node = incoming.node.Copy() } func (r *Resource) GetAnnotations() map[string]string { - annotations, err := r.kunStr.GetAnnotations() + annotations, err := r.node.GetAnnotations() if err != nil || annotations == nil { return make(map[string]string) } @@ -72,19 +71,19 @@ func (r *Resource) GetAnnotations() map[string]string { func (r *Resource) GetFieldValue(f string) (interface{}, error) { //nolint:staticcheck - return r.kunStr.GetFieldValue(f) + return r.node.GetFieldValue(f) } func (r *Resource) GetDataMap() map[string]string { - return r.kunStr.GetDataMap() + return r.node.GetDataMap() } func (r *Resource) GetBinaryDataMap() map[string]string { - return r.kunStr.GetBinaryDataMap() + return r.node.GetBinaryDataMap() } func (r *Resource) GetGvk() resid.Gvk { - meta, err := r.kunStr.GetMeta() + meta, err := r.node.GetMeta() if err != nil { return resid.GvkFromString("") } @@ -92,16 +91,16 @@ func (r *Resource) GetGvk() resid.Gvk { return resid.Gvk{Group: g, Version: v, Kind: meta.Kind} } -func (r *Resource) Hash(h ifc.KunstructuredHasher) (string, error) { - return h.Hash(r.kunStr) +func (r *Resource) Hash(h ifc.KustHasher) (string, error) { + return h.Hash(r.node) } func (r *Resource) GetKind() string { - return r.kunStr.GetKind() + return r.node.GetKind() } func (r *Resource) GetLabels() map[string]string { - l, err := r.kunStr.GetLabels() + l, err := r.node.GetLabels() if err != nil { return map[string]string{} } @@ -109,78 +108,78 @@ func (r *Resource) GetLabels() map[string]string { } func (r *Resource) GetName() string { - return r.kunStr.GetName() + return r.node.GetName() } func (r *Resource) GetSlice(p string) ([]interface{}, error) { //nolint:staticcheck - return r.kunStr.GetSlice(p) + return r.node.GetSlice(p) } func (r *Resource) GetString(p string) (string, error) { //nolint:staticcheck - return r.kunStr.GetString(p) + return r.node.GetString(p) } func (r *Resource) IsEmpty() bool { - return r.kunStr.IsNilOrEmpty() + return r.node.IsNilOrEmpty() } func (r *Resource) Map() (map[string]interface{}, error) { - return r.kunStr.Map() + return r.node.Map() } func (r *Resource) MarshalJSON() ([]byte, error) { - return r.kunStr.MarshalJSON() + return r.node.MarshalJSON() } func (r *Resource) MatchesLabelSelector(selector string) (bool, error) { - return r.kunStr.MatchesLabelSelector(selector) + return r.node.MatchesLabelSelector(selector) } func (r *Resource) MatchesAnnotationSelector(selector string) (bool, error) { - return r.kunStr.MatchesAnnotationSelector(selector) + return r.node.MatchesAnnotationSelector(selector) } func (r *Resource) SetAnnotations(m map[string]string) { if len(m) == 0 { // Force field erasure. - r.kunStr.SetAnnotations(nil) + r.node.SetAnnotations(nil) return } - r.kunStr.SetAnnotations(m) + r.node.SetAnnotations(m) } func (r *Resource) SetDataMap(m map[string]string) { - r.kunStr.SetDataMap(m) + r.node.SetDataMap(m) } func (r *Resource) SetBinaryDataMap(m map[string]string) { - r.kunStr.SetBinaryDataMap(m) + r.node.SetBinaryDataMap(m) } func (r *Resource) SetGvk(gvk resid.Gvk) { - r.kunStr.SetMapField( + r.node.SetMapField( kyaml.NewScalarRNode(gvk.Kind), kyaml.KindField) - r.kunStr.SetMapField( + r.node.SetMapField( kyaml.NewScalarRNode(gvk.ApiVersion()), kyaml.APIVersionField) } func (r *Resource) SetLabels(m map[string]string) { if len(m) == 0 { // Force field erasure. - r.kunStr.SetLabels(nil) + r.node.SetLabels(nil) return } - r.kunStr.SetLabels(m) + r.node.SetLabels(m) } func (r *Resource) SetName(n string) { - r.kunStr.SetName(n) + r.node.SetName(n) } func (r *Resource) SetNamespace(n string) { - r.kunStr.SetNamespace(n) + r.node.SetNamespace(n) } func (r *Resource) SetKind(k string) { @@ -190,7 +189,7 @@ func (r *Resource) SetKind(k string) { } func (r *Resource) UnmarshalJSON(s []byte) error { - return r.kunStr.UnmarshalJSON(s) + return r.node.UnmarshalJSON(s) } // ResCtx is an interface describing the contextual added @@ -210,14 +209,14 @@ type ResCtxMatcher func(ResCtx) bool // DeepCopy returns a new copy of resource func (r *Resource) DeepCopy() *Resource { rc := &Resource{ - kunStr: r.kunStr.Copy(), + node: r.node.Copy(), } rc.copyOtherFields(r) return rc } // CopyMergeMetaDataFields copies everything but the non-metadata in -// the ifc.Kunstructured map, merging labels and annotations. +// the resource. func (r *Resource) CopyMergeMetaDataFieldsFrom(other *Resource) { r.SetLabels(mergeStringMaps(other.GetLabels(), r.GetLabels())) r.SetAnnotations( @@ -283,8 +282,10 @@ func (r *Resource) ReferencesEqual(other *Resource) bool { return len(setSelf) == len(setOther) } -func (r *Resource) KunstructEqual(o *Resource) bool { - return reflect.DeepEqual(r.kunStr, o.kunStr) +// NodeEqual returns true if the resource's nodes are +// equal, ignoring ancillary information like genargs, refby, etc. +func (r *Resource) NodeEqual(o *Resource) bool { + return reflect.DeepEqual(r.node, o.node) } func (r *Resource) copyRefBy() []resid.ResId { @@ -572,10 +573,10 @@ func (r *Resource) ApplySmPatch(patch *Resource) error { } func (r *Resource) ApplyFilter(f kio.Filter) error { - l, err := f.Filter([]*kyaml.RNode{r.kunStr}) + l, err := f.Filter([]*kyaml.RNode{r.node}) if len(l) == 0 { // The node was deleted. The following makes r.IsEmpty() true. - r.kunStr = nil + r.node = nil } return err } diff --git a/api/types/fieldspec.go b/api/types/fieldspec.go index f5b711ee5..0bee70256 100644 --- a/api/types/fieldspec.go +++ b/api/types/fieldspec.go @@ -9,8 +9,7 @@ import ( "sigs.k8s.io/kustomize/api/resid" ) -// FieldSpec completely specifies a kustomizable field in -// an unstructured representation of a k8s API object. +// FieldSpec completely specifies a kustomizable field in a k8s API object. // It helps define the operands of transformations. // // For example, a directive to add a common label to objects diff --git a/kyaml/yaml/rnode.go b/kyaml/yaml/rnode.go index 9f6ed3984..9b1b5ff1b 100644 --- a/kyaml/yaml/rnode.go +++ b/kyaml/yaml/rnode.go @@ -808,9 +808,8 @@ func (rn *RNode) MatchesLabelSelector(selector string) (bool, error) { // HasNilEntryInList returns true if the RNode contains a list which has // a nil item, along with the path to the missing item. -// TODO(broken): This was copied from -// api/k8sdeps/kunstruct/factory.go//checkListItemNil -// and doesn't do what it claims to do (see TODO in unit test and pr 1513). +// TODO(broken): This doesn't do what it claims to do. +// (see TODO in unit test and pr 1513). func (rn *RNode) HasNilEntryInList() (bool, string) { return hasNilEntryInList(rn.value) } diff --git a/plugin/builtin/hashtransformer/HashTransformer.go b/plugin/builtin/hashtransformer/HashTransformer.go index 777c5fefa..49861ce94 100644 --- a/plugin/builtin/hashtransformer/HashTransformer.go +++ b/plugin/builtin/hashtransformer/HashTransformer.go @@ -12,7 +12,7 @@ import ( ) type plugin struct { - hasher ifc.KunstructuredHasher + hasher ifc.KustHasher } //noinspection GoUnusedGlobalVariable