Remove secrets in favor of from_secret (#4363)

Co-authored-by: qwerty287 <80460567+qwerty287@users.noreply.github.com>
Co-authored-by: Patrick Schratz <patrick.schratz@gmail.com>
Co-authored-by: Robert Kaussow <xoxys@rknet.org>
Co-authored-by: Lauris BH <lauris@nix.lv>

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 {

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
 	}

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
 }
 

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 {

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
-}

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)
-}

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"`

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",