Make ordering configurable (#4019)

* api: Add new types for customizeable resource ordering Signed-off-by: Yannis Zarkadas <yanniszark@arrikto.com> * plugins: Implement SortOrderTransformer plugin Implement the SortOrderTransformer plugin. This plugin allows the user to customize the order that kustomize will output resources in. The API for the plugin is the following: sortOptions: order: legacy | fifo legacySortOptions: orderFirst: - {GVK} orderLast: - {GVK} Signed-off-by: Yannis Zarkadas <yanniszark@arrikto.com> * plugins: Add boilerplate and generate code for new SortOrderTransformer Signed-off-by: Yannis Zarkadas <yanniszark@arrikto.com> * build: Add option to denote if the reorder flag was set by the user We want to take different actions if the reorder flag was set by the user or filled by the default value. Thus, we propagate this information from build to the krusty options. Signed-off-by: Yannis Zarkadas <yanniszark@gmail.com> * api/krusty: Ensure sort ordering works with CLI flag and kustomization Sort order can be defined in two places: - (new) kustomization file - (old) CLI flag We want the kustomization file to take precedence over the CLI flag. Eventually, we may want to move away from having a CLI flag altogether: https://github.com/kubernetes-sigs/kustomize/issues/3947 Case 1: Sort order set in kustomization file AND in CLI flag. Print a warning and let the kustomization file take precedence. Case 2: Sort order set in CLI flag only or not at all. Follow the CLI flag (defaults to legacy) and reorder at the end. Case 3: Sort order set in kustomization file only. Simply build the kustomization. Signed-off-by: Yannis Zarkadas <yanniszark@gmail.com> * krusty: Add e2e test for SortOrderTransformer Signed-off-by: Yannis Zarkadas <yanniszark@arrikto.com> * plugins: Purge LegacyOrderTransformer Signed-off-by: Yannis Zarkadas <yanniszark@gmail.com> * Update go.work.sum Signed-off-by: Yannis Zarkadas <yanniszark@gmail.com> * review: Make review changes Signed-off-by: Yannis Zarkadas <yanniszark@gmail.com> Signed-off-by: Yannis Zarkadas <yanniszark@arrikto.com> Signed-off-by: Yannis Zarkadas <yanniszark@gmail.com>
2026-06-13 01:50:55 +00:00 · 2022-12-02 16:59:53 -05:00
parent 1e2e7bbc0b
commit a502717460
30 changed files with 1532 additions and 1092 deletions
--- a/api/krusty/kustomizer.go
+++ b/api/krusty/kustomizer.go
@@ -5,6 +5,7 @@ package krusty

 import (
 	"fmt"
+	"log"

 	"sigs.k8s.io/kustomize/api/internal/builtins"
 	pLdr "sigs.k8s.io/kustomize/api/internal/plugins/loader"
@@ -90,11 +91,9 @@ func (b *Kustomizer) Run(
 	if err != nil {
 		return nil, err
 	}
-	if b.options.DoLegacyResourceSort {
-		err = builtins.NewLegacyOrderTransformerPlugin().Transform(m)
-		if err != nil {
-			return nil, err
-		}
+	err = b.applySortOrder(m, kt)
+	if err != nil {
+		return nil, err
 	}
 	if b.options.AddManagedbyLabel || utils.StringSliceContains(kt.Kustomization().BuildMetadata, types.ManagedByLabelOption) {
 		t := builtins.LabelTransformerPlugin{
@@ -126,3 +125,39 @@ func (b *Kustomizer) Run(
 	}
 	return m, nil
 }
+
+func (b *Kustomizer) applySortOrder(m resmap.ResMap, kt *target.KustTarget) error {
+	// Sort order can be defined in two places:
+	// - (new) kustomization file
+	// - (old) CLI flag
+	//
+	// We want the kustomization file to take precedence over the CLI flag.
+	// Eventually, we may want to move away from having a CLI flag altogether:
+	// https://github.com/kubernetes-sigs/kustomize/issues/3947
+
+	// Case 1: Sort order set in kustomization file.
+	if kt.Kustomization().SortOptions != nil {
+		// If set in CLI flag too, warn the user.
+		if b.options.Reorder != ReorderOptionUnspecified {
+			log.Println("Warning: Sorting order is set both in 'kustomization.yaml'" +
+				" ('sortOptions') and in a CLI flag ('--reorder'). Using the" +
+				" kustomization file over the CLI flag.")
+		}
+		pl := &builtins.SortOrderTransformerPlugin{
+			SortOptions: kt.Kustomization().SortOptions,
+		}
+		err := pl.Transform(m)
+		if err != nil {
+			return errors.Wrap(err)
+		}
+	} else if b.options.Reorder == ReorderOptionLegacy || b.options.Reorder == ReorderOptionUnspecified {
+		// Case 2: Sort order set in CLI flag only or not at all.
+		pl := &builtins.SortOrderTransformerPlugin{
+			SortOptions: &types.SortOptions{
+				Order: types.LegacySortOrder,
+			},
+		}
+		return errors.Wrap(pl.Transform(m))
+	}
+	return nil
+}
--- a/api/krusty/legacy_order_test.go
+++ b/api/krusty/legacy_order_test.go
@@ -6,6 +6,7 @@ package krusty_test
 import (
 	"testing"

+	"sigs.k8s.io/kustomize/api/krusty"
 	kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest"
 )

@@ -38,7 +39,7 @@ data:
  key: value
 `)
 	opts := th.MakeDefaultOptions()
-	opts.DoLegacyResourceSort = true
+	opts.Reorder = krusty.ReorderOptionLegacy
 	m := th.Run(".", opts)
 	th.AssertActualEqualsExpected(m, `
 apiVersion: v1
--- a/api/krusty/options.go
+++ b/api/krusty/options.go
@@ -8,6 +8,14 @@ import (
 	"sigs.k8s.io/kustomize/api/types"
 )

+type ReorderOption string
+
+const (
+	ReorderOptionLegacy      ReorderOption = "legacy"
+	ReorderOptionNone        ReorderOption = "none"
+	ReorderOptionUnspecified ReorderOption = "unspecified"
+)
+
 // Options holds high-level kustomize configuration options,
 // e.g. are plugins enabled, should the loader be restricted
 // to the kustomization root, etc.
@@ -16,7 +24,15 @@ type Options struct {
 	// per a particular sort order.  When false, don't do the
 	// sort, and instead respect the depth-first resource input
 	// order as specified by the kustomization file(s).
-	DoLegacyResourceSort bool
+
+	// Sort the resources before emitting them. Possible values:
+	// - "legacy": Use a fixed order that kustomize provides for backwards
+	//   compatibility.
+	// - "none": Respect the depth-first resource input order as specified by the
+	//   kustomization file.
+	// - "unspecified": The user didn't specify any preference. Kustomize will
+	//   select the appropriate default.
+	Reorder ReorderOption

 	// When true, a label
 	//     app.kubernetes.io/managed-by: kustomize-<version>
@@ -37,11 +53,11 @@ type Options struct {
 // MakeDefaultOptions returns a default instance of Options.
 func MakeDefaultOptions() *Options {
 	return &Options{
-		DoLegacyResourceSort: false,
-		AddManagedbyLabel:    false,
-		LoadRestrictions:     types.LoadRestrictionsRootOnly,
-		DoPrune:              false,
-		PluginConfig:         types.DisabledPluginConfig(),
+		Reorder:           ReorderOptionNone,
+		AddManagedbyLabel: false,
+		LoadRestrictions:  types.LoadRestrictionsRootOnly,
+		DoPrune:           false,
+		PluginConfig:      types.DisabledPluginConfig(),
 	}
 }

--- a/api/krusty/sortordertransformer_test.go
+++ b/api/krusty/sortordertransformer_test.go
@@ -0,0 +1,331 @@
+// Copyright 2022 The Kubernetes Authors.
+// SPDX-License-Identifier: Apache-2.0
+
+package krusty_test
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"sigs.k8s.io/kustomize/api/krusty"
+	kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest"
+)
+
+//nolint:gochecknoglobals
+var sortOrderResources = `
+apiVersion: v1
+kind: Service
+metadata:
+  name: papaya
+---
+apiVersion: v1
+kind: Role
+metadata:
+  name: banana
+---
+apiVersion: v1
+kind: ValidatingWebhookConfiguration
+metadata:
+  name: pomegranate
+---
+apiVersion: v1
+kind: LimitRange
+metadata:
+  name: peach
+---
+apiVersion: v1
+kind: Deployment
+metadata:
+  name: pear
+---
+apiVersion: v1
+kind: Namespace
+metadata:
+  name: apple
+---
+apiVersion: v1
+kind: Secret
+metadata:
+  name: quince
+---
+apiVersion: v1
+kind: Ingress
+metadata:
+  name: durian
+---
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: apricot
+`
+
+//nolint:gochecknoglobals
+var legacyOrderResources = `
+apiVersion: v1
+kind: Namespace
+metadata:
+  name: apple
+---
+apiVersion: v1
+kind: Role
+metadata:
+  name: banana
+---
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: apricot
+---
+apiVersion: v1
+kind: Secret
+metadata:
+  name: quince
+---
+apiVersion: v1
+kind: Service
+metadata:
+  name: papaya
+---
+apiVersion: v1
+kind: LimitRange
+metadata:
+  name: peach
+---
+apiVersion: v1
+kind: Deployment
+metadata:
+  name: pear
+---
+apiVersion: v1
+kind: Ingress
+metadata:
+  name: durian
+---
+apiVersion: v1
+kind: ValidatingWebhookConfiguration
+metadata:
+  name: pomegranate
+`
+
+func TestCustomOrdering(t *testing.T) {
+	th := kusttest_test.MakeHarness(t)
+	th.WriteK("base", `
+resources:
+- resources.yaml
+
+sortOptions:
+  order: legacy
+  legacySortOptions:
+    orderFirst:
+    - MutatingWebhookConfiguration
+    - ValidatingWebhookConfiguration
+    - ResourceQuota
+    - StorageClass
+    - CustomResourceDefinition
+    - ServiceAccount
+    - PodSecurityPolicy
+    - Role
+    - ClusterRole
+    - RoleBinding
+    - ClusterRoleBinding
+    - ConfigMap
+    - Secret
+    - Endpoints
+    - Service
+    - LimitRange
+    - PriorityClass
+    - PersistentVolume
+    - PersistentVolumeClaim
+    - StatefulSet
+    - CronJob
+    - PodDisruptionBudget
+    orderLast:
+    - Namespace
+    - Deployment
+`)
+	th.WriteF("base/resources.yaml", sortOrderResources)
+
+	th.AssertActualEqualsExpected(
+		th.Run("base", th.MakeDefaultOptions()),
+		`
+apiVersion: v1
+kind: ValidatingWebhookConfiguration
+metadata:
+  name: pomegranate
+---
+apiVersion: v1
+kind: Role
+metadata:
+  name: banana
+---
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: apricot
+---
+apiVersion: v1
+kind: Secret
+metadata:
+  name: quince
+---
+apiVersion: v1
+kind: Service
+metadata:
+  name: papaya
+---
+apiVersion: v1
+kind: LimitRange
+metadata:
+  name: peach
+---
+apiVersion: v1
+kind: Ingress
+metadata:
+  name: durian
+---
+apiVersion: v1
+kind: Namespace
+metadata:
+  name: apple
+---
+apiVersion: v1
+kind: Deployment
+metadata:
+  name: pear
+`)
+}
+
+func TestDefaultLegacyOrdering(t *testing.T) {
+	th := kusttest_test.MakeHarness(t)
+	th.WriteK("base", `
+resources:
+- resources.yaml
+
+sortOptions:
+  order: legacy
+`)
+	th.WriteF("base/resources.yaml", sortOrderResources)
+	th.AssertActualEqualsExpected(
+		th.Run("base", th.MakeDefaultOptions()), legacyOrderResources)
+}
+
+func TestFIFOOrdering(t *testing.T) {
+	th := kusttest_test.MakeHarness(t)
+	th.WriteK("base", `
+resources:
+- resources.yaml
+
+sortOptions:
+  order: fifo
+`)
+	th.WriteF("base/resources.yaml", sortOrderResources)
+	th.AssertActualEqualsExpected(
+		th.Run("base", th.MakeDefaultOptions()), sortOrderResources)
+}
+
+func TestInvalidLegacySortOptionsWithoutOrderKey(t *testing.T) {
+	th := kusttest_test.MakeHarness(t)
+	th.WriteK("base", `
+resources:
+- resources.yaml
+
+sortOptions:
+  legacySortOptions: {}
+`)
+	th.WriteF("base/resources.yaml", sortOrderResources)
+	err := th.RunWithErr("base", th.MakeDefaultOptions())
+	require.ErrorContains(t, err, "the field 'sortOptions.order' must be one of [fifo, legacy]")
+}
+
+func TestInvalidLegacySortOptionsWithFIFOOrder(t *testing.T) {
+	th := kusttest_test.MakeHarness(t)
+	th.WriteK("base", `
+resources:
+- resources.yaml
+
+sortOptions:
+  order: fifo
+  legacySortOptions: {}
+`)
+	th.WriteF("base/resources.yaml", sortOrderResources)
+	err := th.RunWithErr("base", th.MakeDefaultOptions())
+	require.ErrorContains(t, err, "the field 'sortOptions.legacySortOptions' is set but the selected sort order is 'fifo', not 'legacy'")
+}
+
+// If the sort order is defined both in a CLI flag and the kustomization file,
+// the kustomization file takes precedence.
+func TestCLIAndKustomizationSet(t *testing.T) {
+	th := kusttest_test.MakeHarness(t)
+	th.WriteK("base", `
+resources:
+- resources.yaml
+
+sortOptions:
+  order: fifo
+`)
+	th.WriteF("base/resources.yaml", sortOrderResources)
+	kustOptions := th.MakeDefaultOptions()
+	kustOptions.Reorder = krusty.ReorderOptionLegacy
+	th.AssertActualEqualsExpected(th.Run("base", kustOptions), sortOrderResources)
+}
+
+// If no sort order is set in the kustomization file, validate that we fallback
+// to the default legacy sort ordering.
+func TestKustomizationSortOrderNotSet(t *testing.T) {
+	th := kusttest_test.MakeHarness(t)
+	th.WriteK("base", `
+resources:
+- resources.yaml
+`)
+	th.WriteF("base/resources.yaml", sortOrderResources)
+	kustOptions := th.MakeDefaultOptions()
+	kustOptions.Reorder = krusty.ReorderOptionUnspecified
+	th.AssertActualEqualsExpected(th.Run("base", kustOptions), legacyOrderResources)
+}
+
+func TestChildKustomizationSortOrder(t *testing.T) {
+	th := kusttest_test.MakeHarness(t)
+	th.WriteK("base", `
+resources:
+- resources.yaml
+
+sortOptions:
+  order: legacy
+`)
+	th.WriteF("base/resources.yaml", sortOrderResources)
+	th.WriteK("overlay", `
+resources:
+- ../base
+
+sortOptions:
+  order: fifo
+`)
+
+	kustOptions := th.MakeDefaultOptions()
+	// If child kustomization ordering takes effect, the order will be legacy,
+	// otherwise fifo.
+	th.AssertActualEqualsExpected(th.Run("overlay", kustOptions), sortOrderResources)
+}
+
+func TestSortOrderGivenAsTransformer(t *testing.T) {
+	th := kusttest_test.MakeHarness(t)
+	th.WriteK("base", `
+resources:
+- resources.yaml
+
+transformers:
+- sort_order.yaml
+`)
+	th.WriteF("base/resources.yaml", sortOrderResources)
+	th.WriteF("base/sort_order.yaml", `
+apiVersion: builtin
+kind: SortOrderTransformer
+metadata:
+  name: notImportantHere
+sortOptions:
+  order: fifo`)
+
+	kustOptions := th.MakeDefaultOptions()
+	err := th.RunWithErr("base", kustOptions)
+	require.ErrorContains(t, err, "unable to load builtin SortOrderTransformer.builtin.[noGrp]")
+}