Add deprecation warnings (#2725)

cli/exec/exec.go

@@ -167,7 +167,11 @@ func execWithAxis(c *cli.Context, file, repoPath string, axis matrix.Axis) error
 	}
 
 	// lint the yaml file
-	if lerr := linter.New(linter.WithTrusted(true)).Lint(confstr, conf); lerr != nil {
+	if lerr := linter.New(linter.WithTrusted(true)).Lint([]*linter.WorkflowConfig{{
+		File:      path.Base(file),
+		RawConfig: confstr,
+		Workflow:  conf,
+	}}); lerr != nil {
 		return lerr
 	}
 

cli/lint/lint.go

@@ -93,9 +93,16 @@ func lintFile(_ *cli.Context, file string) error {
 		return err
 	}
 
-	err = linter.New(linter.WithTrusted(true)).Lint(string(buf), c)
+	config := &linter.WorkflowConfig{
+		File:      path.Base(file),
+		RawConfig: rawConfig,
+		Workflow:  c,
+	}
+
+	// TODO: lint multiple files at once to allow checks for sth like "depends_on" to work
+	err = linter.New(linter.WithTrusted(true)).Lint([]*linter.WorkflowConfig{config})
 	if err != nil {
-		fmt.Printf("🔥 %s has errors:\n", output.String(path.Base(file)).Underline())
+		fmt.Printf("🔥 %s has errors:\n", output.String(config.File).Underline())
 
 		hasErrors := true
 		for _, err := range pipeline_errors.GetPipelineErrors(err) {

pipeline/errors/error.go

@@ -24,9 +24,16 @@ type PipelineError struct {
 }
 
 type LinterErrorData struct {
+	File  string `json:"file"`
 	Field string `json:"field"`
 }
 
+type DeprecationErrorData struct {
+	File  string `json:"file"`
+	Field string `json:"field"`
+	Docs  string `json:"docs"`
+}
+
 func (e *PipelineError) Error() string {
 	return fmt.Sprintf("[%s] %s", e.Type, e.Message)
 }

pipeline/frontend/yaml/linter/error.go

@@ -18,11 +18,11 @@ import (
 	"github.com/woodpecker-ci/woodpecker/pipeline/errors"
 )
 
-func newLinterError(message, field string, isWarning bool) *errors.PipelineError {
+func newLinterError(message, file, field string, isWarning bool) *errors.PipelineError {
 	return &errors.PipelineError{
 		Type:      errors.PipelineErrorTypeLinter,
 		Message:   message,
-		Data:      &errors.LinterErrorData{Field: field},
+		Data:      &errors.LinterErrorData{File: file, Field: field},
 		IsWarning: isWarning,
 	}
 }

pipeline/frontend/yaml/linter/linter.go

@@ -17,8 +17,10 @@ package linter
 import (
 	"fmt"
 
+	"codeberg.org/6543/xyaml"
 	"go.uber.org/multierr"
 
+	"github.com/woodpecker-ci/woodpecker/pipeline/errors"
 	"github.com/woodpecker-ci/woodpecker/pipeline/frontend/yaml/linter/schema"
 	"github.com/woodpecker-ci/woodpecker/pipeline/frontend/yaml/types"
 )
@@ -37,50 +39,84 @@ func New(opts ...Option) *Linter {
 	return linter
 }
 
+type WorkflowConfig struct {
+	// File is the path to the configuration file.
+	File string
+
+	// RawConfig is the raw configuration.
+	RawConfig string
+
+	// Config is the parsed configuration.
+	Workflow *types.Workflow
+}
+
 // Lint lints the configuration.
-func (l *Linter) Lint(rawConfig string, c *types.Workflow) error {
+func (l *Linter) Lint(configs []*WorkflowConfig) error {
 	var linterErr error
 
-	if len(c.Steps.ContainerList) == 0 {
-		linterErr = multierr.Append(linterErr, newLinterError("Invalid or missing steps section", "steps", false))
+	for _, config := range configs {
+		if err := l.lintFile(config); err != nil {
+			linterErr = multierr.Append(linterErr, err)
+		}
 	}
 
-	if err := l.lintContainers(c.Clone.ContainerList); err != nil {
+	return linterErr
+}
+
+func (l *Linter) lintFile(config *WorkflowConfig) error {
+	var linterErr error
+
+	if len(config.Workflow.Steps.ContainerList) == 0 {
+		linterErr = multierr.Append(linterErr, newLinterError("Invalid or missing steps section", config.File, "steps", false))
+	}
+
+	if err := l.lintContainers(config, "clone"); err != nil {
 		linterErr = multierr.Append(linterErr, err)
 	}
-	if err := l.lintContainers(c.Steps.ContainerList); err != nil {
+	if err := l.lintContainers(config, "steps"); err != nil {
 		linterErr = multierr.Append(linterErr, err)
 	}
-	if err := l.lintContainers(c.Services.ContainerList); err != nil {
+	if err := l.lintContainers(config, "services"); err != nil {
 		linterErr = multierr.Append(linterErr, err)
 	}
 
-	if err := l.lintSchema(rawConfig); err != nil {
+	if err := l.lintSchema(config); err != nil {
 		linterErr = multierr.Append(linterErr, err)
 	}
-	if err := l.lintDeprecations(c); err != nil {
+	if err := l.lintDeprecations(config); err != nil {
 		linterErr = multierr.Append(linterErr, err)
 	}
-	if err := l.lintBadHabits(c); err != nil {
+	if err := l.lintBadHabits(config); err != nil {
 		linterErr = multierr.Append(linterErr, err)
 	}
 
 	return linterErr
 }
 
-func (l *Linter) lintContainers(containers []*types.Container) error {
+func (l *Linter) lintContainers(config *WorkflowConfig, area string) error {
 	var linterErr error
 
+	var containers []*types.Container
+
+	switch area {
+	case "clone":
+		containers = config.Workflow.Clone.ContainerList
+	case "steps":
+		containers = config.Workflow.Steps.ContainerList
+	case "services":
+		containers = config.Workflow.Services.ContainerList
+	}
+
 	for _, container := range containers {
-		if err := l.lintImage(container); err != nil {
+		if err := l.lintImage(config, container, area); err != nil {
 			linterErr = multierr.Append(linterErr, err)
 		}
 		if !l.trusted {
-			if err := l.lintTrusted(container); err != nil {
+			if err := l.lintTrusted(config, container, area); err != nil {
 				linterErr = multierr.Append(linterErr, err)
 			}
 		}
-		if err := l.lintCommands(container); err != nil {
+		if err := l.lintCommands(config, container, area); err != nil {
 			linterErr = multierr.Append(linterErr, err)
 		}
 	}
@@ -88,14 +124,14 @@ func (l *Linter) lintContainers(containers []*types.Container) error {
 	return linterErr
 }
 
-func (l *Linter) lintImage(c *types.Container) error {
+func (l *Linter) lintImage(config *WorkflowConfig, c *types.Container, area string) error {
 	if len(c.Image) == 0 {
-		return newLinterError("Invalid or missing image", fmt.Sprintf("steps.%s", c.Name), false)
+		return newLinterError("Invalid or missing image", config.File, fmt.Sprintf("%s.%s", area, c.Name), false)
 	}
 	return nil
 }
 
-func (l *Linter) lintCommands(c *types.Container) error {
+func (l *Linter) lintCommands(config *WorkflowConfig, c *types.Container, field string) error {
 	if len(c.Commands) == 0 {
 		return nil
 	}
@@ -104,59 +140,66 @@ func (l *Linter) lintCommands(c *types.Container) error {
 		for key := range c.Settings {
 			keys = append(keys, key)
 		}
-		return newLinterError(fmt.Sprintf("Cannot configure both commands and custom attributes %v", keys), fmt.Sprintf("steps.%s", c.Name), false)
+		return newLinterError(fmt.Sprintf("Cannot configure both commands and custom attributes %v", keys), config.File, fmt.Sprintf("%s.%s", field, c.Name), false)
 	}
 	return nil
 }
 
-func (l *Linter) lintTrusted(c *types.Container) error {
-	yamlPath := fmt.Sprintf("steps.%s", c.Name)
+func (l *Linter) lintTrusted(config *WorkflowConfig, c *types.Container, area string) error {
+	yamlPath := fmt.Sprintf("%s.%s", area, c.Name)
+	err := ""
 	if c.Privileged {
-		return newLinterError("Insufficient privileges to use privileged mode", yamlPath, false)
+		err = "Insufficient privileges to use privileged mode"
 	}
 	if c.ShmSize != 0 {
-		return newLinterError("Insufficient privileges to override shm_size", yamlPath, false)
+		err = "Insufficient privileges to override shm_size"
 	}
 	if len(c.DNS) != 0 {
-		return newLinterError("Insufficient privileges to use custom dns", yamlPath, false)
+		err = "Insufficient privileges to use custom dns"
 	}
 	if len(c.DNSSearch) != 0 {
-		return newLinterError("Insufficient privileges to use dns_search", yamlPath, false)
+		err = "Insufficient privileges to use dns_search"
 	}
 	if len(c.Devices) != 0 {
-		return newLinterError("Insufficient privileges to use devices", yamlPath, false)
+		err = "Insufficient privileges to use devices"
 	}
 	if len(c.ExtraHosts) != 0 {
-		return newLinterError("Insufficient privileges to use extra_hosts", yamlPath, false)
+		err = "Insufficient privileges to use extra_hosts"
 	}
 	if len(c.NetworkMode) != 0 {
-		return newLinterError("Insufficient privileges to use network_mode", yamlPath, false)
+		err = "Insufficient privileges to use network_mode"
 	}
 	if len(c.IpcMode) != 0 {
-		return newLinterError("Insufficient privileges to use ipc_mode", yamlPath, false)
+		err = "Insufficient privileges to use ipc_mode"
 	}
 	if len(c.Sysctls) != 0 {
-		return newLinterError("Insufficient privileges to use sysctls", yamlPath, false)
+		err = "Insufficient privileges to use sysctls"
 	}
 	if c.Networks.Networks != nil && len(c.Networks.Networks) != 0 {
-		return newLinterError("Insufficient privileges to use networks", yamlPath, false)
+		err = "Insufficient privileges to use networks"
 	}
 	if c.Volumes.Volumes != nil && len(c.Volumes.Volumes) != 0 {
-		return newLinterError("Insufficient privileges to use volumes", yamlPath, false)
+		err = "Insufficient privileges to use volumes"
 	}
 	if len(c.Tmpfs) != 0 {
-		return newLinterError("Insufficient privileges to use tmpfs", yamlPath, false)
+		err = "Insufficient privileges to use tmpfs"
 	}
+
+	if len(err) != 0 {
+		return newLinterError(err, config.File, yamlPath, false)
+	}
+
 	return nil
 }
 
-func (l *Linter) lintSchema(rawConfig string) error {
+func (l *Linter) lintSchema(config *WorkflowConfig) error {
 	var linterErr error
-	schemaErrors, err := schema.LintString(rawConfig)
+	schemaErrors, err := schema.LintString(config.RawConfig)
 	if err != nil {
 		for _, schemaError := range schemaErrors {
 			linterErr = multierr.Append(linterErr, newLinterError(
 				schemaError.Description(),
+				config.File,
 				schemaError.Field(),
 				true, // TODO: let pipelines fail if the schema is invalid
 			))
@@ -165,12 +208,56 @@ func (l *Linter) lintSchema(rawConfig string) error {
 	return linterErr
 }
 
-func (l *Linter) lintDeprecations(_ *types.Workflow) error {
-	// TODO: add deprecation warnings
-	return nil
+func (l *Linter) lintDeprecations(config *WorkflowConfig) (err error) {
+	parsed := new(types.Workflow)
+	err = xyaml.Unmarshal([]byte(config.RawConfig), parsed)
+	if err != nil {
+		return err
+	}
+
+	if parsed.PipelineDontUseIt.ContainerList != nil {
+		err = multierr.Append(err, &errors.PipelineError{
+			Type:    errors.PipelineErrorTypeDeprecation,
+			Message: "Please use 'steps:' instead of deprecated 'pipeline:' list",
+			Data: errors.DeprecationErrorData{
+				File:  config.File,
+				Field: "pipeline",
+				Docs:  "https://woodpecker-ci.org/docs/next/migrations#next-200",
+			},
+			IsWarning: true,
+		})
+	}
+
+	if parsed.PlatformDontUseIt != "" {
+		err = multierr.Append(err, &errors.PipelineError{
+			Type:    errors.PipelineErrorTypeDeprecation,
+			Message: "Please use labels instead of deprecated 'platform' filters",
+			Data: errors.DeprecationErrorData{
+				File:  config.File,
+				Field: "platform",
+				Docs:  "https://woodpecker-ci.org/docs/next/migrations#next-200",
+			},
+			IsWarning: true,
+		})
+	}
+
+	if parsed.BranchesDontUseIt != nil {
+		err = multierr.Append(err, &errors.PipelineError{
+			Type:    errors.PipelineErrorTypeDeprecation,
+			Message: "Please use global when instead of deprecated 'branches' filter",
+			Data: errors.DeprecationErrorData{
+				File:  config.File,
+				Field: "branches",
+				Docs:  "https://woodpecker-ci.org/docs/next/migrations#next-200",
+			},
+			IsWarning: true,
+		})
+	}
+
+	return err
 }
 
-func (l *Linter) lintBadHabits(_ *types.Workflow) error {
+func (l *Linter) lintBadHabits(_ *WorkflowConfig) error {
 	// TODO: add bad habit warnings
 	return nil
 }

pipeline/frontend/yaml/linter/linter_test.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package linter
+package linter_test
 
 import (
 	"testing"
@@ -20,6 +20,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/woodpecker-ci/woodpecker/pipeline/errors"
 	"github.com/woodpecker-ci/woodpecker/pipeline/frontend/yaml"
+	"github.com/woodpecker-ci/woodpecker/pipeline/frontend/yaml/linter"
 )
 
 func TestLint(t *testing.T) {
@@ -82,7 +83,11 @@ steps:
 				t.Fatalf("Cannot unmarshal yaml %q. Error: %s", testd.Title, err)
 			}
 
-			if err := New(WithTrusted(true)).Lint(testd.Data, conf); err != nil {
+			if err := linter.New(linter.WithTrusted(true)).Lint([]*linter.WorkflowConfig{{
+				File:      testd.Title,
+				RawConfig: testd.Data,
+				Workflow:  conf,
+			}}); err != nil {
 				t.Errorf("Expected lint returns no errors, got %q", err)
 			}
 		})
@@ -155,7 +160,11 @@ func TestLintErrors(t *testing.T) {
 			t.Fatalf("Cannot unmarshal yaml %q. Error: %s", test.from, err)
 		}
 
-		lerr := New().Lint(test.from, conf)
+		lerr := linter.New().Lint([]*linter.WorkflowConfig{{
+			File:      test.from,
+			RawConfig: test.from,
+			Workflow:  conf,
+		}})
 		if lerr == nil {
 			t.Errorf("Expected lint error for configuration %q", test.from)
 		}

pipeline/stepBuilder.go

@@ -144,7 +144,11 @@ func (b *StepBuilder) genItemForWorkflow(workflow *model.Workflow, axis matrix.A
 	// lint pipeline
 	errorsAndWarnings = multierr.Append(errorsAndWarnings, linter.New(
 		linter.WithTrusted(b.Repo.IsTrusted),
-	).Lint(substituted, parsed))
+	).Lint([]*linter.WorkflowConfig{{
+		Workflow:  parsed,
+		File:      workflow.Name,
+		RawConfig: data,
+	}}))
 	if pipeline_errors.HasBlockingErrors(errorsAndWarnings) {
 		return nil, errorsAndWarnings
 	}

web/src/lib/api/types/pipeline.ts

@@ -1,9 +1,9 @@
 import { WebhookEvents } from './webhook';
 
-export type PipelineError = {
+export type PipelineError<D = unknown> = {
   type: string;
   message: string;
-  data?: unknown;
+  data?: D;
   is_warning: boolean;
 };
 

web/src/views/repo/pipeline/PipelineErrors.vue

@@ -4,9 +4,17 @@
       <template v-for="(error, i) in pipeline.errors" :key="i">
         <span>{{ error.is_warning ? '⚠️' : '❌' }}</span>
         <span>[{{ error.type }}]</span>
-        <span v-if="error.type === 'linter'" class="underline">{{ (error.data as any)?.field }}</span>
+        <span v-if="isLinterError(error) || isDeprecationError(error)">
+          <span v-if="error.data?.file" class="font-bold">{{ error.data?.file }}: </span>
+          <span>{{ error.data?.field }}</span>
+        </span>
         <span v-else />
-        <span class="ml-4">{{ error.message }}</span>
+        <a v-if="isDeprecationError(error)" :href="error.data?.docs" target="_blank" class="underline ml-4">
+          {{ error.message }}
+        </a>
+        <span v-else class="ml-4">
+          {{ error.message }}
+        </span>
       </template>
     </div>
   </Panel>
@@ -16,12 +24,22 @@
 import { inject, Ref } from 'vue';
 
 import Panel from '~/components/layout/Panel.vue';
-import { Pipeline } from '~/lib/api/types';
+import type { Pipeline, PipelineError } from '~/lib/api/types';
 
 const pipeline = inject<Ref<Pipeline>>('pipeline');
 if (!pipeline) {
   throw new Error('Unexpected: "pipeline" should be provided at this place');
 }
+
+function isLinterError(error: PipelineError): error is PipelineError<{ file?: string; field: string }> {
+  return error.type === 'linter';
+}
+
+function isDeprecationError(
+  error: PipelineError,
+): error is PipelineError<{ file: string; field: string; docs: string }> {
+  return error.type === 'deprecation';
+}
 </script>
 
 <style scoped>