From 4ed5e4e04df4661f5a2a8b53a03a18730d48b538 Mon Sep 17 00:00:00 2001
From: 6543 <6543@obermui.de>
Date: Wed, 13 Nov 2024 14:07:18 +0100
Subject: [PATCH] On wrong secrets/environment config syntax, fail with good
 error message (#4359)

Co-authored-by: Robert Kaussow <xoxys@rknet.org>
---
 .../frontend/yaml/types/base/deprecations.go  | 59 ++++++++++++++
 .../yaml/types/base/deprecations_test.go      | 79 +++++++++++++++++++
 pipeline/frontend/yaml/types/container.go     |  5 +-
 3 files changed, 141 insertions(+), 2 deletions(-)
 create mode 100644 pipeline/frontend/yaml/types/base/deprecations.go
 create mode 100644 pipeline/frontend/yaml/types/base/deprecations_test.go

diff --git a/pipeline/frontend/yaml/types/base/deprecations.go b/pipeline/frontend/yaml/types/base/deprecations.go
new file mode 100644
index 000000000..cbb32abee
--- /dev/null
+++ b/pipeline/frontend/yaml/types/base/deprecations.go
@@ -0,0 +1,59 @@
+// 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.
+
+// TODO: delete file after v3.0.0 release
+
+package base
+
+import (
+	"fmt"
+)
+
+type EnvironmentMap map[string]any
+
+// UnmarshalYAML implements the Unmarshaler interface.
+func (s *EnvironmentMap) UnmarshalYAML(unmarshal func(any) error) error {
+	var mapType map[string]any
+	err := unmarshal(&mapType)
+	if err == nil {
+		*s = mapType
+		return nil
+	}
+
+	var sliceType []any
+	if err := unmarshal(&sliceType); err == nil {
+		return fmt.Errorf("list syntax for 'environment' has been removed, use map syntax instead (https://woodpecker-ci.org/docs/usage/environment)")
+	}
+
+	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
new file mode 100644
index 000000000..cfdd9bbfa
--- /dev/null
+++ b/pipeline/frontend/yaml/types/base/deprecations_test.go
@@ -0,0 +1,79 @@
+// 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.
+
+// TODO: delete file after v3.0.0 release
+
+package base
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"gopkg.in/yaml.v3"
+)
+
+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{}
+	err := yaml.Unmarshal([]byte(str), &s)
+	if assert.Error(t, err) {
+		assert.EqualValues(t, "list syntax for 'environment' has been removed, use map syntax instead (https://woodpecker-ci.org/docs/usage/environment)", err.Error())
+	}
+
+	s.Foos = EnvironmentMap{"bar": "baz", "far": "faz"}
+	d, err := yaml.Marshal(&s)
+	assert.NoError(t, err)
+	str = `foos:
+    bar: baz
+    far: faz
+`
+	assert.EqualValues(t, str, string(d))
+
+	s2 := StructMap{}
+	assert.NoError(t, yaml.Unmarshal(d, &s2))
+
+	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 8d2bbbf64..75c2adbe0 100644
--- a/pipeline/frontend/yaml/types/container.go
+++ b/pipeline/frontend/yaml/types/container.go
@@ -47,10 +47,11 @@ type (
 		Ports          []string           `yaml:"ports,omitempty"`
 		DependsOn      base.StringOrSlice `yaml:"depends_on,omitempty"`
 
-		Environment map[string]any `yaml:"environment,omitempty"`
+		// TODO: remove base.EnvironmentMap and use map[string]any after v3.0.0 release
+		Environment base.EnvironmentMap `yaml:"environment,omitempty"`
 
 		// Deprecated
-		Secrets []string `yaml:"secrets,omitempty"`
+		Secrets base.SecretsSlice `yaml:"secrets,omitempty"`
 
 		// Docker and Kubernetes Specific
 		Privileged bool `yaml:"privileged,omitempty"`