Merge pull request #2502 from monopole/testKustTarget

Add coverage for kusttarget.Load

api/internal/target/kusttarget.go

@@ -55,7 +55,10 @@ func (kt *KustTarget) Load() error {
 	if err != nil {
 		return err
 	}
-	content = types.FixKustomizationPreUnmarshalling(content)
+	content, err = types.FixKustomizationPreUnmarshalling(content)
+	if err != nil {
+		return err
+	}
 	var k types.Kustomization
 	err = unmarshal(content, &k)
 	if err != nil {

api/internal/target/kusttarget_test.go

@@ -7,6 +7,7 @@ import (
 	"encoding/base64"
 	"testing"
 
+	"github.com/stretchr/testify/require"
 	"sigs.k8s.io/kustomize/api/ifc"
 	"sigs.k8s.io/kustomize/api/k8sdeps/kunstruct"
 	"sigs.k8s.io/kustomize/api/resmap"
@@ -17,6 +18,66 @@ import (
 // KustTarget is primarily tested in the krusty package with
 // high level tests.
 
+func TestLoad(t *testing.T) {
+	th := kusttest_test.MakeHarness(t)
+	testCases := map[string]struct {
+		errContains string
+		content     string
+	}{
+		"empty": {
+			errContains: "unable to find one of 'kustomization.yaml', ",
+			// no content
+		},
+		"nonsenseLatin": {
+			errContains: "error converting YAML to JSON",
+			content: `
+Lorem ipsum dolor sit amet, consectetur
+adipiscing elit, sed do eiusmod tempor
+incididunt ut labore et dolore magna aliqua.
+Ut enim ad minim veniam, quis nostrud
+exercitation ullamco laboris nisi ut
+aliquip ex ea commodo consequat.
+`,
+		},
+		"implicitHeader": {
+			content: `
+commonLabels:
+  app: nginx
+`,
+		},
+		"explicitHeader": {
+			content: `
+apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+commonLabels:
+  app: nginx
+`,
+		},
+	}
+
+	kt := makeKustTargetWithRf(
+		t, th.GetFSys(), "/",
+		resource.NewFactory(kunstruct.NewKunstructuredFactoryImpl()))
+
+	for tn, tc := range testCases {
+		if tn != "nonsenseLatin" {
+			continue
+		}
+		t.Run(tn, func(t *testing.T) {
+			if tc.content != "" {
+				th.WriteK("/", tc.content)
+			}
+			err := kt.Load()
+			if tc.errContains != "" {
+				require.NotNilf(t, err, "expected error containing: `%s`", tc.errContains)
+				require.Contains(t, err.Error(), tc.errContains)
+			} else {
+				require.Nilf(t, err, "got error: %v", err)
+			}
+		})
+	}
+}
+
 func TestMakeCustomizedResMap(t *testing.T) {
 	th := kusttest_test.MakeHarness(t)
 	th.WriteK("/whatever", `
@@ -165,8 +226,13 @@ metadata:
 		}
 	}
 
-	actual, err := makeKustTargetWithRf(
+	kt := makeKustTargetWithRf(
-		t, th.GetFSys(), "/whatever", resFactory).MakeCustomizedResMap()
+		t, th.GetFSys(), "/whatever", resFactory)
+	err := kt.Load()
+	if err != nil {
+		t.Fatalf("unexpected Resources error %v", err)
+	}
+	actual, err := kt.MakeCustomizedResMap()
 	if err != nil {
 		t.Fatalf("unexpected Resources error %v", err)
 	}

api/internal/target/maker_test.go

@@ -18,13 +18,17 @@ import (
 	valtest_test "sigs.k8s.io/kustomize/api/testutils/valtest"
 )
 
-func makeKustTarget(
+func makeAndLoadKustTarget(
 	t *testing.T,
 	fSys filesys.FileSystem,
 	root string) *target.KustTarget {
-	return makeKustTargetWithRf(
+	kt := makeKustTargetWithRf(
 		t, fSys, root,
 		resource.NewFactory(kunstruct.NewKunstructuredFactoryImpl()))
+	if err := kt.Load(); err != nil {
+		t.Fatalf("Unexpected load error %v", err)
+	}
+	return kt
 }
 
 func makeKustTargetWithRf(
@@ -38,14 +42,10 @@ func makeKustTargetWithRf(
 	if err != nil {
 		t.Fatal(err)
 	}
-	kt := target.NewKustTarget(
+	return target.NewKustTarget(
 		ldr,
 		valtest_test.MakeFakeValidator(),
 		rf,
 		transformer.NewFactoryImpl(),
 		pLdr.NewLoader(pc, rf))
-	if err = kt.Load(); err != nil {
-		t.Fatalf("Unexpected construction error %v", err)
-	}
-	return kt
 }

api/internal/target/vars_test.go

@@ -64,7 +64,7 @@ vars:
       name: heron
       apiVersion: v300
 `)
-	ra, err := makeKustTarget(
+	ra, err := makeAndLoadKustTarget(
 		t, th.GetFSys(), "/app").AccumulateTarget()
 	if err != nil {
 		t.Fatalf("Err: %v", err)
@@ -119,7 +119,7 @@ resources:
 - ../o1
 `)
 
-	ra, err := makeKustTarget(
+	ra, err := makeAndLoadKustTarget(
 		t, th.GetFSys(), "/app/overlays/o2").AccumulateTarget()
 	if err != nil {
 		t.Fatalf("Err: %v", err)
@@ -176,7 +176,7 @@ vars:
 resources:
 - ../o1
 `)
-	_, err := makeKustTarget(
+	_, err := makeAndLoadKustTarget(
 		t, th.GetFSys(), "/app/overlays/o2").AccumulateTarget()
 	if err == nil {
 		t.Fatalf("expected var collision")

api/types/fix.go

@@ -4,7 +4,6 @@
 package types
 
 import (
-	"log"
 	"regexp"
 
 	"sigs.k8s.io/yaml"
@@ -13,7 +12,7 @@ import (
 // FixKustomizationPreUnmarshalling modies the raw data
 // before marshalling - e.g. changes old field names to
 // new field names.
-func FixKustomizationPreUnmarshalling(data []byte) []byte {
+func FixKustomizationPreUnmarshalling(data []byte) ([]byte, error) {
 	deprecateFieldsMap := map[string]string{
 		"imageTags:": "images:",
 	}
@@ -21,24 +20,28 @@ func FixKustomizationPreUnmarshalling(data []byte) []byte {
 		pattern := regexp.MustCompile(oldname)
 		data = pattern.ReplaceAll(data, []byte(newname))
 	}
-	if useLegacyPatch(data) {
+	doLegacy, err := useLegacyPatch(data)
+	if err != nil {
+		return nil, err
+	}
+	if doLegacy {
 		pattern := regexp.MustCompile("patches:")
 		data = pattern.ReplaceAll(data, []byte("patchesStrategicMerge:"))
 	}
-	return data
+	return data, nil
 }
 
-func useLegacyPatch(data []byte) bool {
+func useLegacyPatch(data []byte) (bool, error) {
 	found := false
 	var object map[string]interface{}
 	err := yaml.Unmarshal(data, &object)
 	if err != nil {
-		log.Fatalf("invalid content from %s\n", string(data))
+		return false, err
 	}
 	if rawPatches, ok := object["patches"]; ok {
 		patches, ok := rawPatches.([]interface{})
 		if !ok {
-			log.Fatalf("invalid patches from %v\n", rawPatches)
+			return false, err
 		}
 		for _, p := range patches {
 			_, ok := p.(string)
@@ -47,5 +50,5 @@ func useLegacyPatch(data []byte) bool {
 			}
 		}
 	}
-	return found
+	return found, nil
 }