diff --git a/pipeline/backend/kubernetes/kubernetes.go b/pipeline/backend/kubernetes/kubernetes.go index cb0a42176..70bc684b3 100644 --- a/pipeline/backend/kubernetes/kubernetes.go +++ b/pipeline/backend/kubernetes/kubernetes.go @@ -233,13 +233,6 @@ func (e *kube) StartStep(ctx context.Context, step *types.Step, taskUUID string) } } - if needsStepSecret(step) { - err = startStepSecret(ctx, e, step) - if err != nil { - return err - } - } - log.Trace().Str("taskUUID", taskUUID).Msgf("starting step: %s", step.Name) _, err = startPod(ctx, e, step, options) return err @@ -405,13 +398,6 @@ func (e *kube) DestroyStep(ctx context.Context, step *types.Step, taskUUID strin } } - if needsStepSecret(step) { - err := stopStepSecret(ctx, e, step, defaultDeleteOptions) - if err != nil { - errs = append(errs, err) - } - } - err := stopPod(ctx, e, step, defaultDeleteOptions) if err != nil { errs = append(errs, err) diff --git a/pipeline/backend/kubernetes/pod.go b/pipeline/backend/kubernetes/pod.go index c1a9e8b9f..460ed5bc8 100644 --- a/pipeline/backend/kubernetes/pod.go +++ b/pipeline/backend/kubernetes/pod.go @@ -235,15 +235,7 @@ func podContainer(step *types.Step, podName, goos string, options BackendOptions container.Command = step.Entrypoint } - stepSecret, err := stepSecretName(step) - if err != nil { - return container, err - } - - // filter environment variables to non-secrets and secrets, refer secrets from step secrets - envs, secs := filterSecrets(step.Environment, step.SecretMapping) - envsFromSecrets := mapToEnvVarsFromStepSecrets(secs, stepSecret) - container.Env = append(mapToEnvVars(envs), envsFromSecrets...) + container.Env = mapToEnvVars(step.Environment) container.Resources, err = resourceRequirements(options.Resources) if err != nil { @@ -262,38 +254,6 @@ func podContainer(step *types.Step, podName, goos string, options BackendOptions return container, nil } -func mapToEnvVarsFromStepSecrets(secs []string, stepSecretName string) []v1.EnvVar { - var ev []v1.EnvVar - for _, key := range secs { - ev = append(ev, v1.EnvVar{ - Name: key, - ValueFrom: &v1.EnvVarSource{ - SecretKeyRef: &v1.SecretKeySelector{ - LocalObjectReference: v1.LocalObjectReference{ - Name: stepSecretName, - }, - Key: key, - }, - }, - }) - } - return ev -} - -func filterSecrets(environment, secrets map[string]string) (map[string]string, []string) { - ev := map[string]string{} - var secs []string - - for k, v := range environment { - if _, found := secrets[k]; found { - secs = append(secs, k) - } else { - ev[k] = v - } - } - return ev, secs -} - func pvcVolumes(volumes []string) ([]v1.Volume, error) { var vols []v1.Volume diff --git a/pipeline/backend/kubernetes/pod_test.go b/pipeline/backend/kubernetes/pod_test.go index 927512517..48d0ed207 100644 --- a/pipeline/backend/kubernetes/pod_test.go +++ b/pipeline/backend/kubernetes/pod_test.go @@ -177,7 +177,6 @@ func TestTinyPod(t *testing.T) { pod, err := mkPod(&types.Step{ Name: "build-via-gradle", Image: "gradle:8.4.0-jdk21", - UUID: "01he8bebctabr3kgk0qj36d2me-0", WorkingDir: "/woodpecker/src", Pull: false, Privileged: false, @@ -416,7 +415,6 @@ func TestPodPrivilege(t *testing.T) { return mkPod(&types.Step{ Name: "go-test", Image: "golang:1.16", - UUID: "01he8bebctabr3kgk0qj36d2me-0", Privileged: stepPrivileged, }, &config{ Namespace: "woodpecker", @@ -527,7 +525,6 @@ func TestScratchPod(t *testing.T) { pod, err := mkPod(&types.Step{ Name: "curl-google", Image: "quay.io/curl/curl", - UUID: "01he8bebctabr3kgk0qj36d2me-0", Entrypoint: []string{"/usr/bin/curl", "-v", "google.com"}, }, &config{ Namespace: "woodpecker", @@ -626,7 +623,6 @@ func TestSecrets(t *testing.T) { pod, err := mkPod(&types.Step{ Name: "test-secrets", Image: "alpine", - UUID: "01he8bebctabr3kgk0qj36d2me-0", Environment: map[string]string{"CGO": "0"}, Volumes: []string{"workspace:/woodpecker/src"}, }, &config{ @@ -661,35 +657,3 @@ func TestSecrets(t *testing.T) { ja := jsonassert.New(t) ja.Assertf(string(podJSON), expected) } - -func TestStepSecret(t *testing.T) { - const expected = `{ - "metadata": { - "name": "wp-01he8bebctabr3kgk0qj36d2me-0-step-secret", - "namespace": "woodpecker", - "creationTimestamp": null - }, - "type": "Opaque", - "stringData": { - "VERY_SECRET": "secret_value" - } - }` - - secret, err := mkStepSecret(&types.Step{ - UUID: "01he8bebctabr3kgk0qj36d2me-0", - Name: "go-test", - Image: "meltwater/drone-cache", - SecretMapping: map[string]string{ - "VERY_SECRET": "secret_value", - }, - }, &config{ - Namespace: "woodpecker", - }) - assert.NoError(t, err) - - secretJSON, err := json.Marshal(secret) - assert.NoError(t, err) - - ja := jsonassert.New(t) - ja.Assertf(string(secretJSON), expected) -} diff --git a/pipeline/backend/kubernetes/secrets.go b/pipeline/backend/kubernetes/secrets.go index 560a6f873..d7543e795 100644 --- a/pipeline/backend/kubernetes/secrets.go +++ b/pipeline/backend/kubernetes/secrets.go @@ -308,58 +308,3 @@ func stopRegistrySecret(ctx context.Context, engine *kube, step *types.Step, del } return err } - -func needsStepSecret(step *types.Step) bool { - return len(step.SecretMapping) > 0 -} - -func startStepSecret(ctx context.Context, e *kube, step *types.Step) error { - secret, err := mkStepSecret(step, e.config) - if err != nil { - return err - } - log.Trace().Msgf("creating secret: %s", secret.Name) - _, err = e.client.CoreV1().Secrets(e.config.Namespace).Create(ctx, secret, meta_v1.CreateOptions{}) - if err != nil { - return err - } - return nil -} - -func mkStepSecret(step *types.Step, config *config) (*v1.Secret, error) { - name, err := stepSecretName(step) - if err != nil { - return nil, err - } - - return &v1.Secret{ - ObjectMeta: meta_v1.ObjectMeta{ - Namespace: config.Namespace, - Name: name, - }, - Type: v1.SecretTypeOpaque, - StringData: step.SecretMapping, - }, nil -} - -func stepSecretName(step *types.Step) (string, error) { - name, err := stepToPodName(step) - if err != nil { - return "", err - } - return fmt.Sprintf("%s-step-secret", name), nil -} - -func stopStepSecret(ctx context.Context, engine *kube, step *types.Step, deleteOpts meta_v1.DeleteOptions) error { - name, err := stepSecretName(step) - if err != nil { - return err - } - log.Trace().Str("name", name).Msg("deleting secret") - - err = engine.client.CoreV1().Secrets(engine.config.Namespace).Delete(ctx, name, deleteOpts) - if errors.IsNotFound(err) { - return nil - } - return err -} diff --git a/pipeline/backend/types/step.go b/pipeline/backend/types/step.go index 79e38c44c..3a658083d 100644 --- a/pipeline/backend/types/step.go +++ b/pipeline/backend/types/step.go @@ -26,7 +26,6 @@ type Step struct { WorkingDir string `json:"working_dir,omitempty"` WorkspaceBase string `json:"workspace_base,omitempty"` Environment map[string]string `json:"environment,omitempty"` - SecretMapping map[string]string `json:"secret_mapping,omitempty"` Entrypoint []string `json:"entrypoint,omitempty"` Commands []string `json:"commands,omitempty"` ExtraHosts []HostAlias `json:"extra_hosts,omitempty"` diff --git a/pipeline/frontend/yaml/compiler/compiler_test.go b/pipeline/frontend/yaml/compiler/compiler_test.go index a0d9c97ae..f8179e7c3 100644 --- a/pipeline/frontend/yaml/compiler/compiler_test.go +++ b/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 diff --git a/pipeline/frontend/yaml/compiler/convert.go b/pipeline/frontend/yaml/compiler/convert.go index 7a72c509c..913722c2b 100644 --- a/pipeline/frontend/yaml/compiler/convert.go +++ b/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, diff --git a/pipeline/frontend/yaml/compiler/settings/params.go b/pipeline/frontend/yaml/compiler/settings/params.go index b1fb202ae..f078643e6 100644 --- a/pipeline/frontend/yaml/compiler/settings/params.go +++ b/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 } diff --git a/pipeline/frontend/yaml/compiler/settings/params_test.go b/pipeline/frontend/yaml/compiler/settings/params_test.go index 70f6ba1f2..386b58d02 100644 --- a/pipeline/frontend/yaml/compiler/settings/params_test.go +++ b/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")) }