Overlaid OS's environment variables with the ones specified in the CredentialProviderConfig

- Removed dependency with cmd.Run's stub
- Added test cases

Signed-off-by: Neeraj Shah <neerajx86@gmail.com>

pkg/credentialprovider/plugin/plugin.go

@@ -135,6 +135,7 @@ func newPluginProvider(pluginBinDir string, provider kubeletconfig.CredentialPro
 			pluginBinDir: pluginBinDir,
 			args:         provider.Args,
 			envVars:      provider.Env,
+			environ:      os.Environ,
 		},
 	}, nil
 }
@@ -354,6 +355,7 @@ type execPlugin struct {
 	args         []string
 	envVars      []kubeletconfig.ExecEnvVar
 	pluginBinDir string
+	environ      func() []string
 }
 
 // ExecPlugin executes the plugin binary with arguments and environment variables specified in CredentialProviderConfig:
@@ -385,11 +387,17 @@ func (e *execPlugin) ExecPlugin(ctx context.Context, image string) (*credentialp
 	cmd := exec.CommandContext(ctx, filepath.Join(e.pluginBinDir, e.name), e.args...)
 	cmd.Stdout, cmd.Stderr, cmd.Stdin = stdout, stderr, stdin
 
-	cmd.Env = []string{}
+	var configEnvVars []string
-	for _, envVar := range e.envVars {
+	for _, v := range e.envVars {
-		cmd.Env = append(cmd.Env, fmt.Sprintf("%s=%s", envVar.Name, envVar.Value))
+		configEnvVars = append(configEnvVars, fmt.Sprintf("%s=%s", v.Name, v.Value))
 	}
 
+	// Append current system environment variables, to the ones configured in the
+	// credential provider file. Failing to do so may result in unsuccessful execution
+	// of the provider binary, see https://github.com/kubernetes/kubernetes/issues/102750
+	// also, this behaviour is inline with Credential Provider Config spec
+	cmd.Env = mergeEnvVars(e.environ(), configEnvVars)
+
 	err = cmd.Run()
 	if ctx.Err() != nil {
 		return nil, fmt.Errorf("error execing credential provider plugin %s for image %s: %w", e.name, image, ctx.Err())
@@ -457,3 +465,14 @@ func parseRegistry(image string) string {
 	imageParts := strings.Split(image, "/")
 	return imageParts[0]
 }
+
+// mergedEnvVars overlays system defined env vars with credential provider env vars,
+// it gives priority to the credential provider vars allowing user to override system
+// env vars
+func mergeEnvVars(sysEnvVars, credProviderVars []string) []string {
+	mergedEnvVars := sysEnvVars
+	for _, credProviderVar := range credProviderVars {
+		mergedEnvVars = append(mergedEnvVars, credProviderVar)
+	}
+	return mergedEnvVars
+}

pkg/credentialprovider/plugin/plugin_test.go

@@ -18,7 +18,9 @@ package plugin
 
 import (
 	"context"
+	"errors"
 	"fmt"
+	kubeletconfig "k8s.io/kubernetes/pkg/kubelet/apis/config"
 	"reflect"
 	"sync"
 	"testing"
@@ -713,3 +715,110 @@ func Test_NoCacheResponse(t *testing.T) {
 		t.Error("unexpected cache keys")
 	}
 }
+
+func Test_ExecPluginEnvVars(t *testing.T) {
+	testcases := []struct {
+		name            string
+		systemEnvVars   []string
+		execPlugin      *execPlugin
+		expectedEnvVars []string
+	}{
+		{
+			name:          "positive append system env vars",
+			systemEnvVars: []string{"HOME=/home/foo", "PATH=/usr/bin"},
+			execPlugin: &execPlugin{
+				envVars: []kubeletconfig.ExecEnvVar{
+					{
+						Name:  "SUPER_SECRET_STRONG_ACCESS_KEY",
+						Value: "123456789",
+					},
+				},
+			},
+			expectedEnvVars: []string{
+				"HOME=/home/foo",
+				"PATH=/usr/bin",
+				"SUPER_SECRET_STRONG_ACCESS_KEY=123456789",
+			},
+		},
+		{
+			name:          "positive no env vars provided in plugin",
+			systemEnvVars: []string{"HOME=/home/foo", "PATH=/usr/bin"},
+			execPlugin:    &execPlugin{},
+			expectedEnvVars: []string{
+				"HOME=/home/foo",
+				"PATH=/usr/bin",
+			},
+		},
+		{
+			name: "positive no system env vars but env vars are provided in plugin",
+			execPlugin: &execPlugin{
+				envVars: []kubeletconfig.ExecEnvVar{
+					{
+						Name:  "SUPER_SECRET_STRONG_ACCESS_KEY",
+						Value: "123456789",
+					},
+				},
+			},
+			expectedEnvVars: []string{
+				"SUPER_SECRET_STRONG_ACCESS_KEY=123456789",
+			},
+		},
+		{
+			name:            "positive no system or plugin provided env vars",
+			execPlugin:      &execPlugin{},
+			expectedEnvVars: nil,
+		},
+		{
+			name:          "positive plugin provided vars takes priority",
+			systemEnvVars: []string{"HOME=/home/foo", "PATH=/usr/bin", "SUPER_SECRET_STRONG_ACCESS_KEY=1111"},
+			execPlugin: &execPlugin{
+				envVars: []kubeletconfig.ExecEnvVar{
+					{
+						Name:  "SUPER_SECRET_STRONG_ACCESS_KEY",
+						Value: "123456789",
+					},
+				},
+			},
+			expectedEnvVars: []string{
+				"HOME=/home/foo",
+				"PATH=/usr/bin",
+				"SUPER_SECRET_STRONG_ACCESS_KEY=1111",
+				"SUPER_SECRET_STRONG_ACCESS_KEY=123456789",
+			},
+		},
+	}
+
+	for _, testcase := range testcases {
+		t.Run(testcase.name, func(t *testing.T) {
+			testcase.execPlugin.environ = func() []string {
+				return testcase.systemEnvVars
+			}
+
+			var configVars []string
+			for _, envVar := range testcase.execPlugin.envVars {
+				configVars = append(configVars, fmt.Sprintf("%s=%s", envVar.Name, envVar.Value))
+			}
+			merged := mergeEnvVars(testcase.systemEnvVars, configVars)
+
+			err := validate(testcase.expectedEnvVars, merged)
+			if err != nil {
+				t.Logf("unexpecged error %v", err)
+			}
+		})
+	}
+}
+
+func validate(expected, actual []string) error {
+	if len(actual) != len(expected) {
+		return errors.New(fmt.Sprintf("actual env var length [%d] and expected env var length [%d] don't match",
+			len(actual), len(expected)))
+	}
+
+	for i := range actual {
+		if actual[i] != expected[i] {
+			return fmt.Errorf("mismatch in expected env var %s and actual env var %s", actual[i], expected[i])
+		}
+	}
+
+	return nil
+}