From d3e73d1e4a2bb3b79ef14c4a360c29b8fb432814 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 21 Nov 2024 15:42:02 +0100 Subject: [PATCH] Remove `secrets` in favor of `from_secret` (#4363) Co-authored-by: qwerty287 <80460567+qwerty287@users.noreply.github.com> Co-authored-by: Patrick Schratz Co-authored-by: Robert Kaussow Co-authored-by: Lauris BH --- .../frontend/yaml/compiler/compiler_test.go | 6 ++- pipeline/frontend/yaml/compiler/convert.go | 13 ----- .../frontend/yaml/compiler/environment.go | 53 ------------------- pipeline/frontend/yaml/linter/linter.go | 37 ++++++------- pipeline/frontend/yaml/linter/linter_test.go | 8 +++ .../frontend/yaml/types/base/deprecations.go | 19 ------- .../yaml/types/base/deprecations_test.go | 28 ---------- pipeline/frontend/yaml/types/container.go | 4 +- .../frontend/yaml/types/container_test.go | 6 +-- 9 files changed, 35 insertions(+), 139 deletions(-) delete mode 100644 pipeline/frontend/yaml/compiler/environment.go diff --git a/pipeline/frontend/yaml/compiler/compiler_test.go b/pipeline/frontend/yaml/compiler/compiler_test.go index dadbcd5dd..4eb8bc230 100644 --- a/pipeline/frontend/yaml/compiler/compiler_test.go +++ b/pipeline/frontend/yaml/compiler/compiler_test.go @@ -284,7 +284,9 @@ func TestCompilerCompile(t *testing.T) { Name: "step", Image: "bash", Commands: []string{"env"}, - Secrets: []string{"missing"}, + Environment: yaml_base_types.EnvironmentMap{ + "MISSING": map[string]any{"from_secret": "missing"}, + }, }}}}, backConf: nil, expectedErr: "secret \"missing\" not found", @@ -306,7 +308,7 @@ func TestCompilerCompile(t *testing.T) { backConf, err := compiler.Compile(test.fronConf) if test.expectedErr != "" { assert.Error(t, err) - assert.Equal(t, err.Error(), test.expectedErr) + 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 { diff --git a/pipeline/frontend/yaml/compiler/convert.go b/pipeline/frontend/yaml/compiler/convert.go index d61c9ecb3..fa88f9542 100644 --- a/pipeline/frontend/yaml/compiler/convert.go +++ b/pipeline/frontend/yaml/compiler/convert.go @@ -122,19 +122,6 @@ func (c *Compiler) createProcess(container *yaml_types.Container, stepType backe return nil, err } - for _, requested := range container.Secrets { - secretValue, err := getSecretValue(requested) - if err != nil { - return nil, err - } - - if !environmentAllowed(requested, stepType) { - continue - } - - environment[requested] = secretValue - } - if utils.MatchImageDynamic(container.Image, c.escalated...) && container.IsPlugin() { privileged = true } diff --git a/pipeline/frontend/yaml/compiler/environment.go b/pipeline/frontend/yaml/compiler/environment.go deleted file mode 100644 index f9061df45..000000000 --- a/pipeline/frontend/yaml/compiler/environment.go +++ /dev/null @@ -1,53 +0,0 @@ -// Copyright 2024 Woodpecker Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package compiler - -import backend_types "go.woodpecker-ci.org/woodpecker/v2/pipeline/backend/types" - -/* cSpell:disable */ - -var binaryVars = []string{ - "PATH", // Specifies directories to search for executable files - "PATH_SEPARATOR", // Defines the separator used in the PATH variable - "COMMAND_MODE", // (macOS): Can affect how certain commands are interpreted - "DYLD_FALLBACK_FRAMEWORK_PATH", // (macOS): Specifies additional locations to search for frameworks - "DYLD_FALLBACK_LIBRARY_PATH", // (macOS): Specifies additional locations to search for libraries -} - -var libraryVars = []string{ - "LD_PRELOAD", // Specifies shared libraries to be loaded before all others - "LD_LIBRARY_PATH", // Specifies directories to search for shared libraries before the standard locations - "LD_AUDIT", // Specifies a shared object to be used for auditing - "LD_BIND_NOW", // Forces all relocations to be processed immediately - "LD_PROFILE", // Specifies a shared object to be used for profiling - "LIBPATH", // (AIX): Similar to LD_LIBRARY_PATH on AIX systems - "DYLD_INSERT_LIBRARIES", // (macOS): Similar to LD_PRELOAD on macOS - "DYLD_LIBRARY_PATH", // (macOS): Similar to LD_LIBRARY_PATH on macOS -} - -/* cSpell:enable */ - -func environmentAllowed(envKey string, stepType backend_types.StepType) bool { - switch stepType { - case backend_types.StepTypePlugin, - backend_types.StepTypeClone: - for _, v := range append(binaryVars, libraryVars...) { - if envKey == v { - return false - } - } - } - return true -} diff --git a/pipeline/frontend/yaml/linter/linter.go b/pipeline/frontend/yaml/linter/linter.go index 105109a58..4bacf4345 100644 --- a/pipeline/frontend/yaml/linter/linter.go +++ b/pipeline/frontend/yaml/linter/linter.go @@ -158,6 +158,9 @@ func (l *Linter) lintContainers(config *WorkflowConfig, area string) error { if err := l.lintPrivilegedPlugins(config, container, area); err != nil { linterErr = multierr.Append(linterErr, err) } + if err := l.lintContainerDeprecations(config, container, area); err != nil { + linterErr = multierr.Append(linterErr, err) + } } return linterErr @@ -199,12 +202,25 @@ func (l *Linter) lintSettings(config *WorkflowConfig, c *types.Container, field if len(c.Environment) != 0 { return newLinterError("Should not configure both `environment` and `settings`", config.File, fmt.Sprintf("%s.%s", field, c.Name), true) } - if len(c.Secrets) != 0 { - return newLinterError("Should not configure both `secrets` and `settings`", config.File, fmt.Sprintf("%s.%s", field, c.Name), true) - } return nil } +func (l *Linter) lintContainerDeprecations(config *WorkflowConfig, c *types.Container, field string) (err error) { + if len(c.Secrets) != 0 { + err = multierr.Append(err, &errorTypes.PipelineError{ + Type: errorTypes.PipelineErrorTypeDeprecation, + Message: "Usage of `secrets` is deprecated, use `environment` in combination with `from_secret`", + Data: errors.DeprecationErrorData{ + File: config.File, + Field: fmt.Sprintf("%s.%s.secrets", field, c.Name), + Docs: "https://woodpecker-ci.org/docs/usage/secrets#use-secrets-in-settings-and-environment", + }, + }) + } + + return err +} + func (l *Linter) lintTrusted(config *WorkflowConfig, c *types.Container, area string) error { yamlPath := fmt.Sprintf("%s.%s", area, c.Name) errors := []string{} @@ -275,21 +291,6 @@ func (l *Linter) lintDeprecations(config *WorkflowConfig) (err error) { return err } - for _, container := range parsed.Steps.ContainerList { - if len(container.Secrets) > 0 { - err = multierr.Append(err, &errorTypes.PipelineError{ - Type: errorTypes.PipelineErrorTypeDeprecation, - Message: "Usage of `secrets` is deprecated, use `environment` with `from_secret`", - Data: errors.DeprecationErrorData{ - File: config.File, - Field: fmt.Sprintf("steps.%s.secrets", container.Name), - Docs: "https://woodpecker-ci.org/docs/usage/secrets#usage", - }, - IsWarning: true, - }) - } - } - return nil } diff --git a/pipeline/frontend/yaml/linter/linter_test.go b/pipeline/frontend/yaml/linter/linter_test.go index c826cf96d..b9c88687d 100644 --- a/pipeline/frontend/yaml/linter/linter_test.go +++ b/pipeline/frontend/yaml/linter/linter_test.go @@ -177,6 +177,14 @@ func TestLintErrors(t *testing.T) { from: "{steps: { build: { image: golang, settings: { test: 'true' } } }, when: { branch: main, event: push }, clone: { git: { image: some-other/plugin-git:v1.1.0 } } }", want: "Specified clone image does not match allow list, netrc is not injected", }, + { + from: "steps: { build: { image: golang, secrets: [ { source: mysql_username, target: mysql_username } ] } }", + want: "Usage of `secrets` is deprecated, use `environment` in combination with `from_secret`", + }, + { + from: "steps: { build: { image: golang, secrets: [ 'mysql_username' ] } }", + want: "Usage of `secrets` is deprecated, use `environment` in combination with `from_secret`", + }, } for _, test := range testdata { diff --git a/pipeline/frontend/yaml/types/base/deprecations.go b/pipeline/frontend/yaml/types/base/deprecations.go index cbb32abee..8504ae39a 100644 --- a/pipeline/frontend/yaml/types/base/deprecations.go +++ b/pipeline/frontend/yaml/types/base/deprecations.go @@ -38,22 +38,3 @@ func (s *EnvironmentMap) UnmarshalYAML(unmarshal func(any) error) error { return err } - -type SecretsSlice []string - -// UnmarshalYAML implements the Unmarshaler interface. -func (s *SecretsSlice) UnmarshalYAML(unmarshal func(any) error) error { - var stringSlice []string - err := unmarshal(&stringSlice) - if err == nil { - *s = stringSlice - return nil - } - - var objectSlice []any - if err := unmarshal(&objectSlice); err == nil { - return fmt.Errorf("'secrets' property has been removed, use 'from_secret' instead (https://woodpecker-ci.org/docs/usage/secrets)") - } - - return err -} diff --git a/pipeline/frontend/yaml/types/base/deprecations_test.go b/pipeline/frontend/yaml/types/base/deprecations_test.go index cfdd9bbfa..cb87f084c 100644 --- a/pipeline/frontend/yaml/types/base/deprecations_test.go +++ b/pipeline/frontend/yaml/types/base/deprecations_test.go @@ -27,10 +27,6 @@ type StructMap struct { Foos EnvironmentMap `yaml:"foos,omitempty"` } -type StructSecret struct { - Foos SecretsSlice `yaml:"foos,omitempty"` -} - func TestEnvironmentMapYaml(t *testing.T) { str := `{foos: [bar=baz, far=faz]}` s := StructMap{} @@ -53,27 +49,3 @@ func TestEnvironmentMapYaml(t *testing.T) { assert.Equal(t, EnvironmentMap{"bar": "baz", "far": "faz"}, s2.Foos) } - -func TestSecretsSlice(t *testing.T) { - str := `{foos: [ { source: mysql_username, target: mysql_username } ]}` - s := StructSecret{} - err := yaml.Unmarshal([]byte(str), &s) - if assert.Error(t, err) { - assert.EqualValues(t, "'secrets' property has been removed, use 'from_secret' instead (https://woodpecker-ci.org/docs/usage/secrets)", err.Error()) - } - - s.Foos = SecretsSlice{"bar", "baz", "faz"} - d, err := yaml.Marshal(&s) - assert.NoError(t, err) - str = `foos: - - bar - - baz - - faz -` - assert.EqualValues(t, str, string(d)) - - s2 := StructSecret{} - assert.NoError(t, yaml.Unmarshal(d, &s2)) - - assert.Equal(t, SecretsSlice{"bar", "baz", "faz"}, s2.Foos) -} diff --git a/pipeline/frontend/yaml/types/container.go b/pipeline/frontend/yaml/types/container.go index 75c2adbe0..73403f454 100644 --- a/pipeline/frontend/yaml/types/container.go +++ b/pipeline/frontend/yaml/types/container.go @@ -50,8 +50,8 @@ type ( // TODO: remove base.EnvironmentMap and use map[string]any after v3.0.0 release Environment base.EnvironmentMap `yaml:"environment,omitempty"` - // Deprecated - Secrets base.SecretsSlice `yaml:"secrets,omitempty"` + // Remove after v3.1.0 + Secrets []any `yaml:"secrets,omitempty"` // Docker and Kubernetes Specific Privileged bool `yaml:"privileged,omitempty"` diff --git a/pipeline/frontend/yaml/types/container_test.go b/pipeline/frontend/yaml/types/container_test.go index eaa5ca918..9b06045c7 100644 --- a/pipeline/frontend/yaml/types/container_test.go +++ b/pipeline/frontend/yaml/types/container_test.go @@ -158,15 +158,13 @@ func TestUnmarshalContainers(t *testing.T) { dry_run: true dockerfile: docker/Dockerfile.agent tag: [next, latest] - secrets: [docker_username, docker_password] when: branch: ${CI_REPO_DEFAULT_BRANCH} event: push`, want: []*Container{ { - Name: "publish-agent", - Image: "print/env", - Secrets: []string{"docker_username", "docker_password"}, + Name: "publish-agent", + Image: "print/env", Settings: map[string]any{ "repo": "woodpeckerci/woodpecker-agent", "dockerfile": "docker/Dockerfile.agent",