From 8a8f5e09b0972a7e19bbb4a9c94778991e9d415a Mon Sep 17 00:00:00 2001
From: Yannis Zarkadas <yanniszark@gmail.com>
Date: Fri, 31 Dec 2021 16:08:07 +0000
Subject: [PATCH] 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>
---
 api/krusty/kustomizer.go                | 26 +++++--
 api/krusty/sortordertransformer_test.go | 94 +++++++++++++++++++++++++
 2 files changed, 116 insertions(+), 4 deletions(-)

diff --git a/api/krusty/kustomizer.go b/api/krusty/kustomizer.go
index 915fb26425c..846b5f05f53 100644
--- a/api/krusty/kustomizer.go
+++ b/api/krusty/kustomizer.go
@@ -5,6 +5,7 @@ package krusty
 
 import (
 	"fmt"
+	"log"
 	"path/filepath"
 
 	"sigs.k8s.io/kustomize/api/internal/builtins"
@@ -90,10 +91,23 @@ func (b *Kustomizer) Run(
 	if err != nil {
 		return nil, err
 	}
-	// Kustomization file overrides CLI ordering option. Apply the CLI sorting
-	// option only if the sort order is not set in the kustomization file.
-	if b.options.DoLegacyResourceSort &&
-		kt.Kustomization().SortOptions == nil {
+	// 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.
+	if b.options.SortModeSetExplicitly && kt.Kustomization().SortOptions != nil {
+		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.")
+	}
+
+	// Case 2: Sort order set in CLI flag only or not at all.
+	if b.options.DoLegacyResourceSort && kt.Kustomization().SortOptions == nil {
 		pl := &builtins.SortOrderTransformerPlugin{
 			SortOptions: &types.SortOptions{
 				Order: types.LegacySortOrder,
@@ -108,6 +122,10 @@ func (b *Kustomizer) Run(
 			return nil, err
 		}
 	}
+
+	// Case 3: Sort order set only in kustomization file. No need to do
+	// anything else here.
+
 	if b.options.AddManagedbyLabel {
 		t := builtins.LabelTransformerPlugin{
 			Labels: map[string]string{
diff --git a/api/krusty/sortordertransformer_test.go b/api/krusty/sortordertransformer_test.go
index eac2c999428..adc72f94c3c 100644
--- a/api/krusty/sortordertransformer_test.go
+++ b/api/krusty/sortordertransformer_test.go
@@ -213,3 +213,97 @@ sortOptions:
 	th.AssertActualEqualsExpected(
 		th.Run("base", th.MakeDefaultOptions()), sortOrderResources)
 }
+
+func TestIncorrectOptions(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())
+	t.Logf("Failed with error as expected: %+v", err)
+}
+
+// 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.DoLegacyResourceSort = true
+	kustOptions.SortModeSetExplicitly = true
+	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.DoLegacyResourceSort = true
+	kustOptions.SortModeSetExplicitly = false
+	th.AssertActualEqualsExpected(th.Run("base", kustOptions),
+		`
+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
+`)
+}