Merge remote-tracking branch 'origin/master' into issue5540

api/internal/builtins/PatchJson6902Transformer.go

@@ -6,7 +6,7 @@ package builtins
 import (
 	"fmt"
 
-	jsonpatch "github.com/evanphx/json-patch"
+	jsonpatch "gopkg.in/evanphx/json-patch.v4"
 	"sigs.k8s.io/kustomize/api/filters/patchjson6902"
 	"sigs.k8s.io/kustomize/api/ifc"
 	"sigs.k8s.io/kustomize/api/resmap"

api/internal/builtins/PatchTransformer.go

@@ -7,7 +7,7 @@ import (
 	"fmt"
 	"strings"
 
-	jsonpatch "github.com/evanphx/json-patch"
+	jsonpatch "gopkg.in/evanphx/json-patch.v4"
 	"sigs.k8s.io/kustomize/api/filters/patchjson6902"
 	"sigs.k8s.io/kustomize/api/resmap"
 	"sigs.k8s.io/kustomize/api/resource"

api/internal/plugins/builtinconfig/loaddefaultconfig.go

@@ -33,7 +33,7 @@ func loadDefaultConfig(
 // makeTransformerConfigFromBytes returns a TransformerConfig object from bytes
 func makeTransformerConfigFromBytes(data []byte) (*TransformerConfig, error) {
 	var t TransformerConfig
-	err := yaml.Unmarshal(data, &t)
+	err := yaml.UnmarshalStrict(data, &t)
 	if err != nil {
 		return nil, err
 	}

api/internal/plugins/builtinconfig/loaddefaultconfig_test.go

@@ -5,6 +5,7 @@ package builtinconfig
 
 import (
 	"reflect"
+	"strings"
 	"testing"
 
 	"sigs.k8s.io/kustomize/api/internal/loader"
@@ -44,3 +45,55 @@ namePrefix:
 		t.Fatalf("expected %v\n but go6t %v\n", expected, tCfg)
 	}
 }
+
+func TestLoadDefaultConfigsFromFilesWithMissingFields(t *testing.T) {
+	fSys := filesys.MakeFsInMemory()
+	filePathContainsTypo := "config_contains_typo.yaml"
+	if err := fSys.WriteFile(filePathContainsTypo, []byte(`
+namoPrefix:
+- path: nameprefix/path
+  kind: SomeKind
+`)); err != nil {
+		t.Fatal(err)
+	}
+	ldr, err := loader.NewLoader(
+		loader.RestrictionRootOnly, filesys.Separator, fSys)
+	if err != nil {
+		t.Fatal(err)
+	}
+	errMsg := "error unmarshaling JSON: while decoding JSON: json: unknown field"
+	_, err = loadDefaultConfig(ldr, []string{filePathContainsTypo})
+	if err == nil {
+		t.Fatalf("expected to fail unmarshal yaml, but got nil %s", filePathContainsTypo)
+	}
+	if !strings.Contains(err.Error(), errMsg) {
+		t.Fatalf("expected error %s, but got %s", errMsg, err)
+	}
+}
+
+// please remove this failing test after implements the labels support
+func TestLoadDefaultConfigsFromFilesWithMissingFieldsLabels(t *testing.T) {
+	fSys := filesys.MakeFsInMemory()
+	filePathContainsTypo := "config_contains_typo.yaml"
+	if err := fSys.WriteFile(filePathContainsTypo, []byte(`
+labels:
+  - path: spec/podTemplate/metadata/labels
+    create: true
+    kind: FlinkDeployment
+`)); err != nil {
+		t.Fatal(err)
+	}
+	ldr, err := loader.NewLoader(
+		loader.RestrictionRootOnly, filesys.Separator, fSys)
+	if err != nil {
+		t.Fatal(err)
+	}
+	errMsg := "error unmarshaling JSON: while decoding JSON: json: unknown field"
+	_, err = loadDefaultConfig(ldr, []string{filePathContainsTypo})
+	if err == nil {
+		t.Fatalf("expected to fail unmarshal yaml, but got nil %s", filePathContainsTypo)
+	}
+	if !strings.Contains(err.Error(), errMsg) {
+		t.Fatalf("expected error %s, but got %s", errMsg, err)
+	}
+}

api/internal/plugins/execplugin/execplugin.go

@@ -12,6 +12,7 @@ import (
 	"strings"
 
 	"github.com/google/shlex"
+	"k8s.io/klog"
 
 	"sigs.k8s.io/kustomize/api/internal/plugins/utils"
 	"sigs.k8s.io/kustomize/api/resmap"
@@ -21,6 +22,7 @@ import (
 
 const (
 	tmpConfigFilePrefix = "kust-plugin-config-"
+	maxArgStringLength  = 131071
 )
 
 // ExecPlugin record the name and args of an executable
@@ -169,23 +171,35 @@ func (p *ExecPlugin) invokePlugin(input []byte) ([]byte, error) {
 		p.path, append([]string{f.Name()}, p.args...)...)
 	cmd.Env = p.getEnv()
 	cmd.Stdin = bytes.NewReader(input)
-	cmd.Stderr = os.Stderr
+	var stdErr bytes.Buffer
+	cmd.Stderr = &stdErr
 	if _, err := os.Stat(p.h.Loader().Root()); err == nil {
 		cmd.Dir = p.h.Loader().Root()
 	}
 	result, err := cmd.Output()
 	if err != nil {
 		return nil, errors.WrapPrefixf(
-			err, "failure in plugin configured via %s; %v",
-			f.Name(), err.Error())
+			fmt.Errorf("failure in plugin configured via %s; %w",
+				f.Name(), err), stdErr.String())
 	}
 	return result, os.Remove(f.Name())
 }
 
 func (p *ExecPlugin) getEnv() []string {
 	env := os.Environ()
-	env = append(env,
-		"KUSTOMIZE_PLUGIN_CONFIG_STRING="+string(p.cfg),
-		"KUSTOMIZE_PLUGIN_CONFIG_ROOT="+p.h.Loader().Root())
+	pluginConfigString := "KUSTOMIZE_PLUGIN_CONFIG_STRING=" + string(p.cfg)
+	if len(pluginConfigString) <= maxArgStringLength {
+		env = append(env, pluginConfigString)
+	} else {
+		klog.Warningf("KUSTOMIZE_PLUGIN_CONFIG_STRING exceeds hard limit of %d characters, the environment variable "+
+			"will be omitted", maxArgStringLength)
+	}
+	pluginConfigRoot := "KUSTOMIZE_PLUGIN_CONFIG_ROOT=" + p.h.Loader().Root()
+	if len(pluginConfigRoot) <= maxArgStringLength {
+		env = append(env, pluginConfigRoot)
+	} else {
+		klog.Warningf("KUSTOMIZE_PLUGIN_CONFIG_ROOT exceeds hard limit of %d characters, the environment variable "+
+			"will be omitted", maxArgStringLength)
+	}
 	return env
 }

api/internal/plugins/execplugin/execplugin_test.go

@@ -20,6 +20,12 @@ import (
 	"sigs.k8s.io/kustomize/kyaml/filesys"
 )
 
+const (
+	expectedLargeConfigMap = `{"apiVersion":"v1","data":{"password":"password","username":"user"},"kind":"ConfigMap",` +
+		`"metadata":{"annotations":{"internal.config.kubernetes.io/generatorBehavior":"unspecified",` +
+		`"internal.config.kubernetes.io/needsHashSuffix":"enabled"},"name":"example-configmap-test"}}`
+)
+
 func TestExecPluginConfig(t *testing.T) {
 	fSys := filesys.MakeFsInMemory()
 	err := fSys.WriteFile("sed-input.txt", []byte(`
@@ -125,3 +131,104 @@ func TestExecPlugin_ErrIfNotExecutable(t *testing.T) {
 		t.Fatalf("unexpected err: %v", err)
 	}
 }
+
+// TestExecPluginLarge loads PluginConfigs of various (large) sizes. It tests if the env variable is kept below the
+// maximum of 131072 bytes.
+func TestExecPluginLarge(t *testing.T) {
+	// Skip this test on windows.
+	if runtime.GOOS == "windows" {
+		t.Skipf("always returns nil on Windows")
+	}
+
+	// Add executable plugin.
+	srcRoot, err := utils.DeterminePluginSrcRoot(filesys.MakeFsOnDisk())
+	if err != nil {
+		t.Error(err)
+	}
+	executablePlugin := filepath.Join(
+		srcRoot, "someteam.example.com", "v1", "bashedconfigmap", "BashedConfigMap")
+	p := NewExecPlugin(executablePlugin)
+	err = p.ErrIfNotExecutable()
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+
+	// Create a fake filesystem.
+	fSys := filesys.MakeFsInMemory()
+
+	// Load plugin config.
+	ldr, err := fLdr.NewLoader(
+		fLdr.RestrictionRootOnly, filesys.Separator, fSys)
+	if err != nil {
+		t.Fatal(err)
+	}
+	pvd := provider.NewDefaultDepProvider()
+	rf := resmap.NewFactory(pvd.GetResourceFactory())
+	pc := types.DisabledPluginConfig()
+
+	// Test for various lengths. 131071 is the max length that we can have for any given env var in Bytes.
+	tcs := []struct {
+		length int
+		char   rune
+	}{
+		{1000, 'a'},
+		{131071, 'a'},
+		{131072, 'a'},
+		{200000, 'a'},
+		{131071, '安'},
+		{131074, '安'},
+	}
+	for _, tc := range tcs {
+		t.Logf("Testing with an env var length of %d and character %c", tc.length, tc.char)
+		pluginConfig, err := rf.RF().FromBytes(buildLargePluginConfig(tc.length, tc.char))
+		if err != nil {
+			t.Fatalf("unexpected err: %v", err)
+		}
+		yaml, err := pluginConfig.AsYAML()
+		if err != nil {
+			t.Fatalf("unexpected err: %v", err)
+		}
+		err = p.Config(resmap.NewPluginHelpers(ldr, pvd.GetFieldValidator(), rf, pc), yaml)
+		if err != nil {
+			t.Fatalf("unexpected err: %v", err)
+		}
+		resMap, err := p.Generate()
+		if err != nil {
+			t.Fatalf("unexpected err: %v", err)
+		}
+		rNodeSlices := resMap.ToRNodeSlice()
+		for _, rNodeSlice := range rNodeSlices {
+			json, err := rNodeSlice.MarshalJSON()
+			if err != nil {
+				t.Fatalf("unexpected err: %v", err)
+			}
+			if string(json) != expectedLargeConfigMap {
+				t.Fatalf("expected generated JSON to match %q, but got %q instead",
+					expectedLargeConfigMap, string(json))
+			}
+		}
+	}
+}
+
+// buildLargePluginConfig builds a plugin configuration of length: length - len("KUSTOMIZE_PLUGIN_CONFIG_STRING=")
+// This allows us to create an environment variable KUSTOMIZE_PLUGIN_CONFIG_STRING=<plugin content> with the exact
+// length that's provided in the length parameter. Used as a helper for TestExecPluginLarge.
+func buildLargePluginConfig(length int, char rune) []byte {
+	length -= len("KUSTOMIZE_PLUGIN_CONFIG_STRING=")
+
+	var sb strings.Builder
+	sb.WriteString("apiVersion: someteam.example.com/v1\n")
+	sb.WriteString("kind: BashedConfigMap\n")
+	sb.WriteString("metadata:\n")
+	sb.WriteString("  name: some-random-name\n")
+	sb.WriteString("argsOneLiner: \"user password\"\n")
+	sb.WriteString("customArg: ")
+
+	// Now, fill up parameter customArg: until we reach the desired length. Account for the fact that runes can be
+	// 1 to 4 Bytes each.
+	upperBound := length - sb.Len()
+	for i := 0; i < upperBound-len(string(char)); i += len(string(char)) {
+		sb.WriteRune(char)
+	}
+	return []byte(sb.String())
+}

api/internal/plugins/loader/load_go_plugin.go

@@ -0,0 +1,62 @@
+// Copyright 2024 The Kubernetes Authors.
+// SPDX-License-Identifier: Apache-2.0
+//go:build !kustomize_disable_go_plugin_support
+
+package loader
+
+import (
+	"fmt"
+	"log"
+	"plugin"
+	"reflect"
+
+	"sigs.k8s.io/kustomize/api/internal/plugins/utils"
+	"sigs.k8s.io/kustomize/api/konfig"
+	"sigs.k8s.io/kustomize/api/resmap"
+	"sigs.k8s.io/kustomize/kyaml/errors"
+	"sigs.k8s.io/kustomize/kyaml/resid"
+)
+
+// registry is a means to avoid trying to load the same .so file
+// into memory more than once, which results in an error.
+// Each test makes its own loader, and tries to load its own plugins,
+// but the loaded .so files are in shared memory, so one will get
+// "this plugin already loaded" errors if the registry is maintained
+// as a Loader instance variable.  So make it a package variable.
+var registry = make(map[string]resmap.Configurable) //nolint:gochecknoglobals
+
+func copyPlugin(c resmap.Configurable) resmap.Configurable {
+	indirect := reflect.Indirect(reflect.ValueOf(c))
+	newIndirect := reflect.New(indirect.Type())
+	newIndirect.Elem().Set(reflect.ValueOf(indirect.Interface()))
+	newNamed := newIndirect.Interface()
+	return newNamed.(resmap.Configurable) //nolint:forcetypeassert
+}
+
+func (l *Loader) loadGoPlugin(id resid.ResId, absPath string) (resmap.Configurable, error) {
+	regId := relativePluginPath(id)
+	if c, ok := registry[regId]; ok {
+		return copyPlugin(c), nil
+	}
+	if !utils.FileExists(absPath) {
+		return nil, fmt.Errorf(
+			"expected file with Go object code at: %s", absPath)
+	}
+	log.Printf("Attempting plugin load from %q", absPath)
+	p, err := plugin.Open(absPath)
+	if err != nil {
+		return nil, errors.WrapPrefixf(err, "plugin %s fails to load", absPath)
+	}
+	symbol, err := p.Lookup(konfig.PluginSymbol)
+	if err != nil {
+		return nil, errors.WrapPrefixf(
+			err, "plugin %s doesn't have symbol %s",
+			regId, konfig.PluginSymbol)
+	}
+	c, ok := symbol.(resmap.Configurable)
+	if !ok {
+		return nil, fmt.Errorf("plugin %q not configurable", regId)
+	}
+	registry[regId] = c
+	return copyPlugin(c), nil
+}

api/internal/plugins/loader/load_go_plugin_disabled.go

@@ -0,0 +1,24 @@
+// Copyright 2024 The Kubernetes Authors.
+// SPDX-License-Identifier: Apache-2.0
+
+// The build tag "kustomize_disable_go_plugin_support" is used to deactivate the
+// kustomize API's dependency on the "plugins" package. This is beneficial for
+// applications that need to embed it but do not have requirements for dynamic
+// Go plugins.
+// Including plugins as a dependency can lead to an increase in binary size due
+// to the population of ELF's sections such as .dynsym and .dynstr.
+// By utilizing this flag, applications have the flexibility to exclude the
+// import if they do not require support for dynamic Go plugins.
+//go:build kustomize_disable_go_plugin_support
+
+package loader
+
+import (
+	"sigs.k8s.io/kustomize/api/resmap"
+	"sigs.k8s.io/kustomize/kyaml/errors"
+	"sigs.k8s.io/kustomize/kyaml/resid"
+)
+
+func (l *Loader) loadGoPlugin(_ resid.ResId, _ string) (resmap.Configurable, error) {
+	return nil, errors.New("plugin load is disabled")
+}

api/internal/plugins/loader/loader.go

@@ -5,18 +5,14 @@ package loader
 
 import (
 	"fmt"
-	"log"
 	"os"
 	"path/filepath"
-	"plugin"
-	"reflect"
 	"strings"
 
 	"sigs.k8s.io/kustomize/api/ifc"
 	"sigs.k8s.io/kustomize/api/internal/plugins/builtinhelpers"
 	"sigs.k8s.io/kustomize/api/internal/plugins/execplugin"
 	"sigs.k8s.io/kustomize/api/internal/plugins/fnplugin"
-	"sigs.k8s.io/kustomize/api/internal/plugins/utils"
 	"sigs.k8s.io/kustomize/api/konfig"
 	"sigs.k8s.io/kustomize/api/resmap"
 	"sigs.k8s.io/kustomize/api/resource"
@@ -287,46 +283,3 @@ func (l *Loader) loadExecOrGoPlugin(resId resid.ResId) (resmap.Configurable, err
 	return c, nil
 }
 
-// registry is a means to avoid trying to load the same .so file
-// into memory more than once, which results in an error.
-// Each test makes its own loader, and tries to load its own plugins,
-// but the loaded .so files are in shared memory, so one will get
-// "this plugin already loaded" errors if the registry is maintained
-// as a Loader instance variable.  So make it a package variable.
-var registry = make(map[string]resmap.Configurable)
-
-func (l *Loader) loadGoPlugin(id resid.ResId, absPath string) (resmap.Configurable, error) {
-	regId := relativePluginPath(id)
-	if c, ok := registry[regId]; ok {
-		return copyPlugin(c), nil
-	}
-	if !utils.FileExists(absPath) {
-		return nil, fmt.Errorf(
-			"expected file with Go object code at: %s", absPath)
-	}
-	log.Printf("Attempting plugin load from '%s'", absPath)
-	p, err := plugin.Open(absPath)
-	if err != nil {
-		return nil, errors.WrapPrefixf(err, "plugin %s fails to load", absPath)
-	}
-	symbol, err := p.Lookup(konfig.PluginSymbol)
-	if err != nil {
-		return nil, errors.WrapPrefixf(
-			err, "plugin %s doesn't have symbol %s",
-			regId, konfig.PluginSymbol)
-	}
-	c, ok := symbol.(resmap.Configurable)
-	if !ok {
-		return nil, fmt.Errorf("plugin '%s' not configurable", regId)
-	}
-	registry[regId] = c
-	return copyPlugin(c), nil
-}
-
-func copyPlugin(c resmap.Configurable) resmap.Configurable {
-	indirect := reflect.Indirect(reflect.ValueOf(c))
-	newIndirect := reflect.New(indirect.Type())
-	newIndirect.Elem().Set(reflect.ValueOf(indirect.Interface()))
-	newNamed := newIndirect.Interface()
-	return newNamed.(resmap.Configurable)
-}

api/internal/target/kusttarget_test.go

@@ -315,19 +315,21 @@ configurations:
 	th.WriteF("/merge-config/name-prefix-rules.yaml", `
 namePrefix:
 - path: metadata/name
-  apiVersion: v1
+  group: apps
+  version: v1
   kind: Deployment
 - path: metadata/name
-  apiVersion: v1
+  version: v1
   kind: Secret
 `)
 	th.WriteF("/merge-config/name-suffix-rules.yaml", `
 nameSuffix:
 - path: metadata/name
-  apiVersion: v1
+  version: v1
   kind: ConfigMap
 - path: metadata/name
-  apiVersion: v1
+  group: apps
+  version: v1
   kind: Deployment
 `)
 	th.WriteF("/merge-config/deployment.yaml", `