Modify tests to present expected data in unsorted order.

Modify all `build` tests to use the raw, non-sorted output of build. This makes every test provide coverage for how kustomize re-orders (or doesn't reorder) resources during processing. Going forward, the ordering of resources in _expected_ output should match the depth-first ordering specified in the `resources:` field used in the test's kustomization file. The only exception to this rule would be tests that actually confirmed some other output ordering, e.g. the test of the `LegacyOrderTransformer` plugin. Fixes kubernetes-sigs#756 Related kubernetes-sigs#821
monopole · Jun 17, 2019 · c69406f · c69406f
1 parent ed21e77
commit c69406f
Show file tree

Hide file tree

Showing 20 changed files with 591 additions and 560 deletions.
diff --git a/docs/v_2.1.0.md b/docs/v_2.1.0.md
@@ -6,6 +6,7 @@
 [imgModules]: images/goModules.png
 [imgPlugins]: images/plugins.png
 [imgPruning]: images/pruning.png
+[imgSorted]: images/sorted.jpg
 [imgWheels]: images/abandonedTrainingWheels.png
 [kustomization]: glossary.md#kustomization
 [_kustomization_]: glossary.md#kustomization
@@ -25,8 +26,6 @@ _Summary_: Go modules, inventory, plugins, eased
 loading restrictions, and about ~80 issues closed
 since [v2.0.3] (over 300 commits).
 
-
-
 ## Go modules
 
 ![gopher with boxes][imgModules]
@@ -38,6 +37,28 @@ as one or more [Go modules] for use in other
 programs (kubectl, kubebuilder, etc.) and in
 kustomize plugins (see below).
 
+## Resource ordering
+
+![sort order retained][imgSorted]
+
+Kustomize now retains the depth-first order of
+resources as read.
+
+The `build` command now accepts a `--reorder`
+flag with values `legacy` and `none`,
+with a default value of `legacy`.
+
+`legacy` means apply an ordering based on
+GVK, that currently emits `Namespace` objects
+first, and `ValidatingWebhookConfiguration`
+objects last.
+
+`none` means do not reorder the resources before
+output.  This means kustomize respects the input
+order of the data, giving the user a means to
+control output order.  Generated resources always
+come last.
+
 
 ## Inventory generation for pruning
 
@@ -131,29 +152,56 @@ path or URL to a kustomization directory in the
 
 * Generalized `resources` field.
 
-  The `resources` field has been generalized, and
-  can now accept what formerly could only
-  be specified in the `bases` field.
+  The `resources` field has been generalized; it
+  now accepts what formerly could only be
+  specified in the `bases` field.
 
   This change was made so that the `resources`,
   `generators` and `transformers` fields all
   accept the same argument format.
 
-  Each field's argument is a _string list_, where
-  each entry is either a _resource_ (a relative
-  path to a YAML file) or a [_kustomization_] (a
-  relative path or URL pointing to a directory
-  with a kustomization file).  A kustomization
-  directory used in this context is called a
-  [_base_].
-
-  The `bases` field still works, but is no longer
-  necessary, and will likely be deprecated in the
-  next release.  The _base_ as a concept is as
-  important as ever, it's just that two new fields
-  (`generators` and `tranformers`) and one existing
-  field (`resources`) now accept arguments
-  that were once accepted only by `bases`.
+  > Each field's argument is a _string list_,
+  > where each entry is either a _resource_ (a
+  > relative path to a YAML file) or a
+  > [_kustomization_] (a relative path or URL
+  > pointing to a directory with a kustomization
+  > file).  A kustomization directory used in this
+  > context is called a [_base_].
+
+  The `bases` field still works, but is
+  deprecated.  Deal with this by simply moving the
+  arguments of the `bases` field to the
+  `resources` field desired order, e.g.
+
+  > ```
+  > resources:
+  > - someFile.yaml
+  > - someOtherFile.yaml
+  > bases:
+  > - ../../someBaseDir
+  > ```
+
+  becomes
+
+  > ```
+  > resources:
+  > - someFile.yaml
+  > - ../../someBaseDir
+  > - someOtherFile.yaml
+  > ```
+  
+  The depth-first traversal order of items in the
+  `resources:` field is respected and retained by
+  kustomize, which might mean you want
+  `someBaseDir` processed between the other two
+  files.  See the `build --reorder` flag
+  discussion elsewhere in these notes.
+ 
+  The _base_ as a concept is as important as ever,
+  it's just that two new fields (`generators` and
+  `tranformers`) and one existing field
+  (`resources`) now accept arguments that were
+  once accepted only by `bases`.
 
   The fact that the `generators` and
   `transformers` field accept [bases]

diff --git a/pkg/kusttest/kusttestharness.go b/pkg/kusttest/kusttestharness.go
@@ -20,7 +20,6 @@ import (
 	"sigs.k8s.io/kustomize/pkg/target"
 	"sigs.k8s.io/kustomize/pkg/transformers/config/defaultconfig"
 	"sigs.k8s.io/kustomize/pkg/types"
-	"sigs.k8s.io/kustomize/plugin/builtin"
 )
 
 // KustTestHarness helps test kustomization generation and transformation.
@@ -190,16 +189,6 @@ func hint(a, b string) string {
 
 func (th *KustTestHarness) AssertActualEqualsExpected(
 	m resmap.ResMap, expected string) {
-	th.assertActualEqualsExpected(m, expected, true)
-}
-
-func (th *KustTestHarness) AssertActualEqualsExpectedNoSort(
-	m resmap.ResMap, expected string) {
-	th.assertActualEqualsExpected(m, expected, false)
-}
-
-func (th *KustTestHarness) assertActualEqualsExpected(
-	m resmap.ResMap, expected string, doLegacySort bool) {
 	if m == nil {
 		th.t.Fatalf("Map should not be nil.")
 	}
@@ -208,9 +197,6 @@ func (th *KustTestHarness) assertActualEqualsExpected(
 	if len(expected) > 0 && expected[0] == 10 {
 		expected = expected[1:]
 	}
-	if doLegacySort {
-		builtin.NewLegacyOrderTransformerPlugin().Transform(m)
-	}
 	actual, err := m.AsYaml()
 	if err != nil {
 		th.t.Fatalf("Unexpected err: %v", err)

diff --git a/pkg/target/baseandoverlaymedium_test.go b/pkg/target/baseandoverlaymedium_test.go
@@ -79,22 +79,6 @@ func TestMediumBase(t *testing.T) {
 		t.Fatalf("Err: %v", err)
 	}
 	th.AssertActualEqualsExpected(m, `
-apiVersion: v1
-kind: Service
-metadata:
-  annotations:
-    baseAnno: This is a base annotation
-  labels:
-    app: mungebot
-    foo: bar
-  name: baseprefix-mungebot-service
-spec:
-  ports:
-  - port: 7002
-  selector:
-    app: mungebot
-    foo: bar
----
 apiVersion: extensions/v1beta1
 kind: Deployment
 metadata:
@@ -125,6 +109,22 @@ spec:
         name: nginx
         ports:
         - containerPort: 80
+---
+apiVersion: v1
+kind: Service
+metadata:
+  annotations:
+    baseAnno: This is a base annotation
+  labels:
+    app: mungebot
+    foo: bar
+  name: baseprefix-mungebot-service
+spec:
+  ports:
+  - port: 7002
+  selector:
+    app: mungebot
+    foo: bar
 `)
 }
 
@@ -139,7 +139,7 @@ commonLabels:
   repo: test-infra
 commonAnnotations:
   note: This is a test annotation
-bases:
+resources:
 - ../base
 patchesStrategicMerge:
 - deployment/deployment.yaml
@@ -210,63 +210,7 @@ spec:
 	if err != nil {
 		t.Fatalf("Err: %v", err)
 	}
-	// TODO(#669): The name of the patched Deployment is
-	// test-infra-baseprefix-mungebot, retaining the base
-	// prefix (example of correct behavior).
 	th.AssertActualEqualsExpected(m, `
-apiVersion: v1
-data:
-  nonsense: "Lorem ipsum dolor sit amet, consectetur\nadipiscing elit, sed do eiusmod
-    tempor\nincididunt ut labore et dolore magna aliqua. \n"
-kind: ConfigMap
-metadata:
-  annotations:
-    note: This is a test annotation
-  labels:
-    app: mungebot
-    org: kubernetes
-    repo: test-infra
-  name: test-infra-app-config-f462h769f9
----
-apiVersion: v1
-data:
-  DB_PASSWORD: somepw
-  DB_USERNAME: admin
-  ENERGY: electronvolt
-  FRUIT: banana
-  LEGUME: chickpea
-  LENGTH: kilometer
-kind: ConfigMap
-metadata:
-  annotations:
-    note: This is a test annotation
-  labels:
-    app: mungebot
-    org: kubernetes
-    repo: test-infra
-  name: test-infra-app-env-ffmd9b969m
----
-apiVersion: v1
-kind: Service
-metadata:
-  annotations:
-    baseAnno: This is a base annotation
-    note: This is a test annotation
-  labels:
-    app: mungebot
-    foo: bar
-    org: kubernetes
-    repo: test-infra
-  name: test-infra-baseprefix-mungebot-service
-spec:
-  ports:
-  - port: 7002
-  selector:
-    app: mungebot
-    foo: bar
-    org: kubernetes
-    repo: test-infra
----
 apiVersion: extensions/v1beta1
 kind: Deployment
 metadata:
@@ -325,5 +269,58 @@ spec:
       - configMap:
           name: test-infra-app-env-ffmd9b969m
         name: app-env
+---
+apiVersion: v1
+kind: Service
+metadata:
+  annotations:
+    baseAnno: This is a base annotation
+    note: This is a test annotation
+  labels:
+    app: mungebot
+    foo: bar
+    org: kubernetes
+    repo: test-infra
+  name: test-infra-baseprefix-mungebot-service
+spec:
+  ports:
+  - port: 7002
+  selector:
+    app: mungebot
+    foo: bar
+    org: kubernetes
+    repo: test-infra
+---
+apiVersion: v1
+data:
+  DB_PASSWORD: somepw
+  DB_USERNAME: admin
+  ENERGY: electronvolt
+  FRUIT: banana
+  LEGUME: chickpea
+  LENGTH: kilometer
+kind: ConfigMap
+metadata:
+  annotations:
+    note: This is a test annotation
+  labels:
+    app: mungebot
+    org: kubernetes
+    repo: test-infra
+  name: test-infra-app-env-ffmd9b969m
+---
+apiVersion: v1
+data:
+  nonsense: "Lorem ipsum dolor sit amet, consectetur\nadipiscing elit, sed do eiusmod
+    tempor\nincididunt ut labore et dolore magna aliqua. \n"
+kind: ConfigMap
+metadata:
+  annotations:
+    note: This is a test annotation
+  labels:
+    app: mungebot
+    org: kubernetes
+    repo: test-infra
+  name: test-infra-app-config-f462h769f9
 `)
 }