Revert "kubernetes: prevent secrets from leaking to api-server logs" (#5293)

Co-authored-by: qwerty287 <80460567+qwerty287@users.noreply.github.com>

pipeline/frontend/yaml/compiler/compiler_test.go

@@ -317,124 +317,6 @@ func TestCompilerCompile(t *testing.T) {
 						assert.Truef(t, s.Environment["VERBOSE"] == "true", "expected to get value of global set environment")
 						assert.Truef(t, len(s.Environment) > 10, "expected to have a lot of built-in variables")
 						s.Environment = nil
-						s.SecretMapping = nil
-					}
-				}
-				// check if we get an expected backend config based on a frontend config
-				assert.EqualValues(t, *test.backConf, *backConf)
-			}
-		})
-	}
-}
-
-func TestCompilerCompileWithFromSecret(t *testing.T) {
-	repoURL := "https://github.com/octocat/hello-world"
-	compiler := New(
-		WithMetadata(metadata.Metadata{
-			Repo: metadata.Repo{
-				Owner:    "octacat",
-				Name:     "hello-world",
-				Private:  true,
-				ForgeURL: repoURL,
-				CloneURL: "https://github.com/octocat/hello-world.git",
-			},
-		}),
-		WithEnviron(map[string]string{
-			"VERBOSE": "true",
-			"COLORED": "true",
-		}),
-		WithSecret(Secret{
-			Name:  "secret_name",
-			Value: "VERY_SECRET",
-		}),
-		WithPrefix("test"),
-		// we use "/test" as custom workspace base to ensure the enforcement of the pluginWorkspaceBase is applied
-		WithWorkspaceFromURL("/test", repoURL),
-	)
-	defaultNetwork := &backend_types.Network{
-		Name: "test_default",
-	}
-	defaultVolume := &backend_types.Volume{
-		Name: "test_default",
-	}
-	defaultCloneStage := &backend_types.Stage{
-		Steps: []*backend_types.Step{{
-			Name:          "clone",
-			Type:          backend_types.StepTypeClone,
-			Image:         constant.DefaultClonePlugin,
-			OnSuccess:     true,
-			Failure:       "fail",
-			WorkingDir:    "/woodpecker/src/github.com/octocat/hello-world",
-			WorkspaceBase: "/woodpecker",
-			Volumes:       []string{defaultVolume.Name + ":/woodpecker"},
-			Networks:      []backend_types.Conn{{Name: "test_default", Aliases: []string{"clone"}}},
-			ExtraHosts:    []backend_types.HostAlias{},
-		}},
-	}
-	tests := []struct {
-		name        string
-		fronConf    *yaml_types.Workflow
-		backConf    *backend_types.Config
-		expectedErr string
-	}{
-		{
-			name: "workflow with missing secret",
-			fronConf: &yaml_types.Workflow{Steps: yaml_types.ContainerList{ContainerList: []*yaml_types.Container{{
-				Name:     "step",
-				Image:    "bash",
-				Commands: []string{"env"},
-				Environment: yaml_base_types.EnvironmentMap{
-					"SECRET": map[string]any{"from_secret": "secret_name"},
-				},
-			}}}},
-			backConf: &backend_types.Config{
-				Stages: []*backend_types.Stage{defaultCloneStage, {
-					Steps: []*backend_types.Step{{
-						Name:          "step",
-						Type:          backend_types.StepTypeCommands,
-						Image:         "bash",
-						Commands:      []string{"env"},
-						OnSuccess:     true,
-						Failure:       "fail",
-						WorkingDir:    "/test/src/github.com/octocat/hello-world",
-						WorkspaceBase: "/test",
-						Volumes:       []string{defaultVolume.Name + ":/test"},
-						Networks:      []backend_types.Conn{{Name: "test_default", Aliases: []string{"step"}}},
-						ExtraHosts:    []backend_types.HostAlias{},
-						SecretMapping: map[string]string{
-							"SECRET": "VERY_SECRET",
-						},
-					}},
-				}},
-				Volume:  defaultVolume,
-				Network: defaultNetwork,
-				Secrets: []*backend_types.Secret{{
-					Name:  "secret_name",
-					Value: "VERY_SECRET",
-				}},
-			},
-			expectedErr: "",
-		},
-	}
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			backConf, err := compiler.Compile(test.fronConf)
-			if test.expectedErr != "" {
-				assert.Error(t, err)
-				assert.Equal(t, test.expectedErr, err.Error())
-			} else {
-				// we ignore uuids in steps and only check if global env got set ...
-				for _, st := range backConf.Stages {
-					for _, s := range st.Steps {
-						s.UUID = ""
-						assert.Truef(t, s.Environment["VERBOSE"] == "true", "expected to get value of global set environment")
-						assert.Truef(t, len(s.Environment) > 10, "expected to have a lot of built-in variables")
-						s.Environment = nil
-
-						if len(s.SecretMapping) == 0 {
-							s.SecretMapping = nil
-						}
 					}
 				}
 				// check if we get an expected backend config based on a frontend config

pipeline/frontend/yaml/compiler/convert.go

@@ -114,13 +114,11 @@ func (c *Compiler) createProcess(container *yaml_types.Container, workflow *yaml
 		return secret.Value, nil
 	}
 
-	secretMapping := map[string]string{}
-
-	if err := settings.ParamsToEnv(container.Settings, environment, "PLUGIN_", true, getSecretValue, secretMapping); err != nil {
+	if err := settings.ParamsToEnv(container.Settings, environment, "PLUGIN_", true, getSecretValue); err != nil {
 		return nil, err
 	}
 
-	if err := settings.ParamsToEnv(container.Environment, environment, "", false, getSecretValue, secretMapping); err != nil {
+	if err := settings.ParamsToEnv(container.Environment, environment, "", false, getSecretValue); err != nil {
 		return nil, err
 	}
 
@@ -167,7 +165,6 @@ func (c *Compiler) createProcess(container *yaml_types.Container, workflow *yaml
 		WorkingDir:     workingDir,
 		WorkspaceBase:  workspaceBase,
 		Environment:    environment,
-		SecretMapping:  secretMapping,
 		Commands:       container.Commands,
 		Entrypoint:     container.Entrypoint,
 		ExtraHosts:     extraHosts,

pipeline/frontend/yaml/compiler/settings/params.go

@@ -24,17 +24,9 @@ import (
 	"gopkg.in/yaml.v3"
 )
 
-func captureInjectedSecret(k string, secretMapping map[string]string, getSecretValue func(name string) (string, error)) func(name string) (string, error) {
-	return func(name string) (string, error) {
-		v, err := getSecretValue(name)
-		secretMapping[k] = v
-		return v, err
-	}
-}
-
 // ParamsToEnv uses reflection to convert a map[string]interface to a list
 // of environment variables.
-func ParamsToEnv(from map[string]any, to map[string]string, prefix string, upper bool, getSecretValue func(name string) (string, error), secretMapping map[string]string) (err error) {
+func ParamsToEnv(from map[string]any, to map[string]string, prefix string, upper bool, getSecretValue func(name string) (string, error)) (err error) {
 	if to == nil {
 		return fmt.Errorf("no map to write to")
 	}
@@ -42,7 +34,7 @@ func ParamsToEnv(from map[string]any, to map[string]string, prefix string, upper
 		if v == nil || len(k) == 0 {
 			continue
 		}
-		to[sanitizeParamKey(prefix, upper, k)], err = sanitizeParamValue(v, captureInjectedSecret(sanitizeParamKey(prefix, upper, k), secretMapping, getSecretValue))
+		to[sanitizeParamKey(prefix, upper, k)], err = sanitizeParamValue(v, getSecretValue)
 		if err != nil {
 			return err
 		}

pipeline/frontend/yaml/compiler/settings/params_test.go

@@ -68,13 +68,13 @@ func TestParamsToEnv(t *testing.T) {
 
 		return "", fmt.Errorf("secret %q not found or not allowed to be used", name)
 	}
-	secretMapping := map[string]string{}
-	assert.NoError(t, ParamsToEnv(from, got, "PLUGIN_", true, getSecretValue, secretMapping))
+
+	assert.NoError(t, ParamsToEnv(from, got, "PLUGIN_", true, getSecretValue))
 	assert.EqualValues(t, want, got, "Problem converting plugin parameters to environment variables")
 
 	// handle edge cases (#1609)
 	got = map[string]string{}
-	assert.NoError(t, ParamsToEnv(map[string]any{"a": []any{"a", nil}}, got, "PLUGIN_", true, nil, nil))
+	assert.NoError(t, ParamsToEnv(map[string]any{"a": []any{"a", nil}}, got, "PLUGIN_", true, nil))
 	assert.EqualValues(t, map[string]string{"PLUGIN_A": "a,"}, got)
 }
 
@@ -92,7 +92,7 @@ func TestParamsToEnvPrefix(t *testing.T) {
 		return "", fmt.Errorf("secret %q not found or not allowed to be used", name)
 	}
 
-	assert.NoError(t, ParamsToEnv(from, got, "PLUGIN_", true, getSecretValue, nil))
+	assert.NoError(t, ParamsToEnv(from, got, "PLUGIN_", true, getSecretValue))
 	assert.EqualValues(t, wantPrefixPlugin, got, "Problem converting plugin parameters to environment variables")
 
 	wantNoPrefix := map[string]string{
@@ -102,7 +102,7 @@ func TestParamsToEnvPrefix(t *testing.T) {
 
 	// handle edge cases (#1609)
 	got = map[string]string{}
-	assert.NoError(t, ParamsToEnv(from, got, "", true, getSecretValue, nil))
+	assert.NoError(t, ParamsToEnv(from, got, "", true, getSecretValue))
 	assert.EqualValues(t, wantNoPrefix, got, "Problem converting plugin parameters to environment variables")
 }
 
@@ -166,14 +166,8 @@ list.map:
 
 		return "", fmt.Errorf("secret %q not found or not allowed to be used", name)
 	}
-	gotSecretMapping := map[string]string{}
-	wantSecretMapping := map[string]string{
-		"PLUGIN_MY_SECRET": "FooBar",
-		"PLUGIN_MAP":       "FooBar",
-		"PLUGIN_LIST_MAP":  "geheim",
-	}
-	assert.NoError(t, ParamsToEnv(from, got, "PLUGIN_", true, getSecretValue, gotSecretMapping))
-	assert.Equal(t, wantSecretMapping, gotSecretMapping, "Problem collecting secret mapping")
+
+	assert.NoError(t, ParamsToEnv(from, got, "PLUGIN_", true, getSecretValue))
 	assert.EqualValues(t, want, got, "Problem converting plugin parameters to environment variables")
 }
 
@@ -197,8 +191,7 @@ func TestYAMLToParamsToEnvError(t *testing.T) {
 		return "", fmt.Errorf("secret %q not found or not allowed to be used", name)
 	}
 
-	secretMapping := map[string]string{}
-	assert.Error(t, ParamsToEnv(from, make(map[string]string), "PLUGIN_", true, getSecretValue, secretMapping))
+	assert.Error(t, ParamsToEnv(from, make(map[string]string), "PLUGIN_", true, getSecretValue))
 }
 
 func stringsToInterface(val ...string) []any {
@@ -227,8 +220,8 @@ func TestSecretNotFound(t *testing.T) {
 		return "", fmt.Errorf("secret %q not found or not allowed to be used", name)
 	}
 	got := map[string]string{}
-	secretMapping := map[string]string{}
+
 	assert.ErrorContains(t,
-		ParamsToEnv(from, got, "PLUGIN_", true, getSecretValue, secretMapping),
+		ParamsToEnv(from, got, "PLUGIN_", true, getSecretValue),
 		fmt.Sprintf("secret %q not found or not allowed to be used", "secret_token"))
 }