Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve linter messages in UI #4351

Merged
merged 11 commits into from
Nov 11, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 17 additions & 17 deletions pipeline/frontend/yaml/linter/linter.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@
var linterErr error

if len(config.Workflow.Steps.ContainerList) == 0 {
linterErr = multierr.Append(linterErr, newLinterError("Invalid or missing steps section", config.File, "steps", false))
linterErr = multierr.Append(linterErr, newLinterError("Invalid or missing `steps` section", config.File, "steps", false))
}

if err := l.lintCloneSteps(config); err != nil {
Expand Down Expand Up @@ -123,7 +123,7 @@
if !utils.MatchImageDynamic(container.Image, trustedClonePlugins...) {
linterErr = multierr.Append(linterErr,
newLinterError(
"Specified clone image does not match allow list, netrc will not be injected",
"Specified clone image does not match allow list, netrc is not injected",
config.File, fmt.Sprintf("clone.%s", container.Name), true),
)
}
Expand Down Expand Up @@ -173,7 +173,7 @@
func (l *Linter) lintPrivilegedPlugins(config *WorkflowConfig, c *types.Container, area string) error {
// lint for conflicts of https://github.com/woodpecker-ci/woodpecker/pull/3918
if utils.MatchImage(c.Image, "plugins/docker", "plugins/gcr", "plugins/ecr", "woodpeckerci/plugin-docker-buildx") {
msg := fmt.Sprintf("The formerly privileged plugin '%s' is no longer privileged by default, if required, add it to WOODPECKER_PLUGINS_PRIVILEGED", c.Image)
msg := fmt.Sprintf("The formerly privileged plugin `%s` is no longer privileged by default, if required, add it to `WOODPECKER_PLUGINS_PRIVILEGED`", c.Image)
// check first if user did not add them back
if l.privilegedPlugins != nil && !utils.MatchImageDynamic(c.Image, *l.privilegedPlugins...) {
return newLinterError(msg, config.File, fmt.Sprintf("%s.%s", area, c.Name), false)
Expand All @@ -191,16 +191,16 @@
return nil
}
if len(c.Commands) != 0 {
return newLinterError("Cannot configure both commands and settings", config.File, fmt.Sprintf("%s.%s", field, c.Name), false)
return newLinterError("Cannot configure both `commands` and `settings`", config.File, fmt.Sprintf("%s.%s", field, c.Name), false)
}
if len(c.Entrypoint) != 0 {
return newLinterError("Cannot configure both entrypoint and settings", config.File, fmt.Sprintf("%s.%s", field, c.Name), false)
return newLinterError("Cannot configure both `entrypoint` and `settings`", config.File, fmt.Sprintf("%s.%s", field, c.Name), false)
}
if len(c.Environment) != 0 {
return newLinterError("Should not configure both environment and settings", config.File, fmt.Sprintf("%s.%s", field, c.Name), true)
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 newLinterError("Should not configure both `secrets` and `settings`", config.File, fmt.Sprintf("%s.%s", field, c.Name), true)

Check warning on line 203 in pipeline/frontend/yaml/linter/linter.go

View check run for this annotation

Codecov / codecov/patch

pipeline/frontend/yaml/linter/linter.go#L203

Added line #L203 was not covered by tests
}
return nil
}
Expand All @@ -210,32 +210,32 @@
errors := []string{}
if !l.trusted.Security {
if c.Privileged {
errors = append(errors, "Insufficient privileges to use privileged mode")
errors = append(errors, "Insufficient trust level to use `privileged` mode")
}
}
if !l.trusted.Network {
if len(c.DNS) != 0 {
errors = append(errors, "Insufficient privileges to use custom dns")
errors = append(errors, "Insufficient trust level to use custom `dns`")
}
if len(c.DNSSearch) != 0 {
errors = append(errors, "Insufficient privileges to use dns_search")
errors = append(errors, "Insufficient trust level to use `dns_search`")
}
if len(c.ExtraHosts) != 0 {
errors = append(errors, "Insufficient privileges to use extra_hosts")
errors = append(errors, "Insufficient trust level to use `extra_hosts`")
}
if len(c.NetworkMode) != 0 {
errors = append(errors, "Insufficient privileges to use network_mode")
errors = append(errors, "Insufficient trust level to use `network_mode`")
}
}
if !l.trusted.Volumes {
if len(c.Devices) != 0 {
errors = append(errors, "Insufficient privileges to use devices")
errors = append(errors, "Insufficient trust level to use `devices`")
}
if len(c.Volumes.Volumes) != 0 {
errors = append(errors, "Insufficient privileges to use volumes")
errors = append(errors, "Insufficient trust level to use `volumes`")
}
if len(c.Tmpfs) != 0 {
errors = append(errors, "Insufficient privileges to use tmpfs")
errors = append(errors, "Insufficient trust level to use `tmpfs`")

Check warning on line 238 in pipeline/frontend/yaml/linter/linter.go

View check run for this annotation

Codecov / codecov/patch

pipeline/frontend/yaml/linter/linter.go#L238

Added line #L238 was not covered by tests
}
}

Expand Down Expand Up @@ -279,7 +279,7 @@
if len(container.Secrets) > 0 {
err = multierr.Append(err, &errorTypes.PipelineError{
Type: errorTypes.PipelineErrorTypeDeprecation,
Message: "Secrets are deprecated, use environment with from_secret",
Message: "Usage of `secrets` is deprecated, use `environment` with `from_secret`",

Check warning on line 282 in pipeline/frontend/yaml/linter/linter.go

View check run for this annotation

Codecov / codecov/patch

pipeline/frontend/yaml/linter/linter.go#L282

Added line #L282 was not covered by tests
Data: errors.DeprecationErrorData{
File: config.File,
Field: fmt.Sprintf("steps.%s.secrets", container.Name),
Expand Down Expand Up @@ -328,7 +328,7 @@
if field != "" {
err = multierr.Append(err, &errorTypes.PipelineError{
Type: errorTypes.PipelineErrorTypeBadHabit,
Message: "Please set an event filter for all steps or the whole workflow on all items of the when block",
Message: "Set an event filter for all steps or the entire workflow on all items of the `when` block",
Data: errors.BadHabitErrorData{
File: config.File,
Field: field,
Expand Down
32 changes: 16 additions & 16 deletions pipeline/frontend/yaml/linter/linter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,68 +114,68 @@ func TestLintErrors(t *testing.T) {
}{
{
from: "",
want: "Invalid or missing steps section",
want: "Invalid or missing `steps` section",
},
{
from: "steps: { build: { image: '' } }",
want: "Invalid or missing image",
},
{
from: "steps: { build: { image: golang, privileged: true } }",
want: "Insufficient privileges to use privileged mode",
want: "Insufficient trust level to use `privileged` mode",
},
{
from: "steps: { build: { image: golang, dns: [ 8.8.8.8 ] } }",
want: "Insufficient privileges to use custom dns",
want: "Insufficient trust level to use custom `dns`",
},

{
from: "steps: { build: { image: golang, dns_search: [ example.com ] } }",
want: "Insufficient privileges to use dns_search",
want: "Insufficient trust level to use `dns_search`",
},
{
from: "steps: { build: { image: golang, devices: [ '/dev/tty0:/dev/tty0' ] } }",
want: "Insufficient privileges to use devices",
want: "Insufficient trust level to use `devices`",
},
{
from: "steps: { build: { image: golang, extra_hosts: [ 'somehost:162.242.195.82' ] } }",
want: "Insufficient privileges to use extra_hosts",
want: "Insufficient trust level to use `extra_hosts`",
},
{
from: "steps: { build: { image: golang, network_mode: host } }",
want: "Insufficient privileges to use network_mode",
want: "Insufficient trust level to use `network_mode`",
},
{
from: "steps: { build: { image: golang, volumes: [ '/opt/data:/var/lib/mysql' ] } }",
want: "Insufficient privileges to use volumes",
want: "Insufficient trust level to use `volumes`",
},
{
from: "steps: { build: { image: golang, network_mode: 'container:name' } }",
want: "Insufficient privileges to use network_mode",
want: "Insufficient trust level to use `network_mode`",
},
{
from: "steps: { build: { image: golang, settings: { test: 'true' }, commands: [ 'echo ja', 'echo nein' ] } }",
want: "Cannot configure both commands and settings",
want: "Cannot configure both `commands` and `settings`",
},
{
from: "steps: { build: { image: golang, settings: { test: 'true' }, entrypoint: [ '/bin/fish' ] } }",
want: "Cannot configure both entrypoint and settings",
want: "Cannot configure both `entrypoint` and `settings`",
},
{
from: "steps: { build: { image: golang, settings: { test: 'true' }, environment: { 'TEST': 'true' } } }",
want: "Should not configure both environment and settings",
want: "Should not configure both `environment` and `settings`",
},
{
from: "{pipeline: { build: { image: golang, settings: { test: 'true' } } }, when: { branch: main, event: push } }",
want: "Additional property pipeline is not allowed",
},
{
from: "{steps: { build: { image: plugins/docker, settings: { test: 'true' } } }, when: { branch: main, event: push } } }",
want: "The formerly privileged plugin 'plugins/docker' is no longer privileged by default, if required, add it to WOODPECKER_PLUGINS_PRIVILEGED",
want: "The formerly privileged plugin `plugins/docker` is no longer privileged by default, if required, add it to `WOODPECKER_PLUGINS_PRIVILEGED`",
},
{
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 will not be injected",
want: "Specified clone image does not match allow list, netrc is not injected",
},
}

Expand Down Expand Up @@ -209,11 +209,11 @@ func TestBadHabits(t *testing.T) {
}{
{
from: "steps: { build: { image: golang } }",
want: "Please set an event filter for all steps or the whole workflow on all items of the when block",
want: "Set an event filter for all steps or the entire workflow on all items of the `when` block",
},
{
from: "when: [{branch: xyz}, {event: push}]\nsteps: { build: { image: golang } }",
want: "Please set an event filter for all steps or the whole workflow on all items of the when block",
want: "Set an event filter for all steps or the entire workflow on all items of the `when` block",
},
}

Expand Down
1 change: 1 addition & 0 deletions web/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
"fuse.js": "^7.0.0",
"js-base64": "^3.7.7",
"lodash": "^4.17.21",
"marked": "^15.0.0",
"node-emoji": "^2.1.3",
"pinia": "^2.2.1",
"prismjs": "^1.29.0",
Expand Down
10 changes: 10 additions & 0 deletions web/pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 11 additions & 0 deletions web/src/components/atomic/RenderMarkdown.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<template>
<span v-html="marked(props.content)" />
</template>

<script setup lang="ts">
import { marked } from 'marked';

const props = defineProps<{
content: string;
}>();
</script>
4 changes: 2 additions & 2 deletions web/src/components/repo/settings/GeneralTab.vue
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@
</template>
<template #description>
<i18n-t keypath="repo.settings.general.pipeline_path.desc" tag="p" class="text-sm text-wp-text-alt-100">
<span class="code-box-inline px-1">{{ $t('repo.settings.general.pipeline_path.desc_path_example') }}</span>
<span class="code-box-inline">{{ $t('repo.settings.general.pipeline_path.desc_path_example') }}</span>
<!-- eslint-disable-next-line @intlify/vue-i18n/no-raw-text -->
<span class="code-box-inline px-1">/</span>
<span class="code-box-inline">/</span>
</i18n-t>
</template>
</InputField>
Expand Down
5 changes: 3 additions & 2 deletions web/src/style.css
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ body,
white-space: pre-wrap;
}

.code-box-inline {
@apply bg-wp-code-200 rounded-md text-wp-code-text-100;
.code-box-inline,
code {
@apply bg-wp-code-200 rounded-md text-wp-code-text-100 px-1 py-px;
}
80 changes: 42 additions & 38 deletions web/src/views/repo/pipeline/PipelineErrors.vue
Original file line number Diff line number Diff line change
@@ -1,37 +1,45 @@
<template>
<Panel>
<div class="grid justify-center gap-x-4 text-left grid-3-1">
<template v-for="(error, i) in pipeline!.errors" :key="i">
<Icon
name="attention"
class="flex-shrink-0 my-1"
:class="{
'text-wp-state-warn-100': error.is_warning,
'text-wp-state-error-100': !error.is_warning,
}"
/>
<!-- eslint-disable-next-line @intlify/vue-i18n/no-raw-text -->
<span>[{{ error.type }}]</span>
<span
v-if="isLinterError(error) || isDeprecationError(error) || isBadHabitError(error)"
class="whitespace-nowrap"
>
<!-- eslint-disable-next-line @intlify/vue-i18n/no-raw-text -->
<span v-if="error.data?.file" class="font-bold">{{ error.data?.file }}: </span>
<span>{{ error.data?.field }}</span>
</span>
<span v-else />
<a
v-if="isDeprecationError(error) || isBadHabitError(error)"
:href="error.data?.docs"
target="_blank"
class="underline col-span-full col-start-2 md:col-span-auto md:col-start-auto"
>
{{ error.message }}
</a>
<span v-else class="col-span-full col-start-2 md:col-span-auto md:col-start-auto">
{{ error.message }}
</span>
<div class="flex flex-col gap-y-4">
<template v-for="(error, _index) in pipeline!.errors" :key="_index">
<div>
<div class="grid grid-cols-[minmax(10rem,auto),4fr] items-center">
<span class="flex items-center gap-x-2">
<Icon
name="attention"
class="flex-shrink-0 my-1"
:class="{
'text-wp-state-warn-100': error.is_warning,
'text-wp-state-error-100': !error.is_warning,
}"
/>
<!-- eslint-disable-next-line @intlify/vue-i18n/no-raw-text -->
<span>
<code>{{ error.type }}</code>
</span>
</span>
<span
v-if="isLinterError(error) || isDeprecationError(error) || isBadHabitError(error)"
class="whitespace-nowrap"
>
<!-- eslint-disable-next-line @intlify/vue-i18n/no-raw-text -->
<span v-if="error.data?.file" class="font-bold">{{ error.data?.file }}: </span>
<span>{{ error.data?.field }}</span>
</span>
<span v-else />
</div>
<div class="grid grid-cols-[minmax(10rem,auto),4fr] col-start-2">
<span />
<span class="flex gap-x-2">
<RenderMarkdown :content="error.message" />
<DocsLink
v-if="isDeprecationError(error) || isBadHabitError(error)"
:topic="error.data?.field || ''"
:url="error.data?.docs || ''"
/>
</span>
</div>
</div>
</template>
</div>
</Panel>
Expand All @@ -40,7 +48,9 @@
<script lang="ts" setup>
import { inject, type Ref } from 'vue';

import DocsLink from '~/components/atomic/DocsLink.vue';
import Icon from '~/components/atomic/Icon.vue';
import RenderMarkdown from '~/components/atomic/RenderMarkdown.vue';
import Panel from '~/components/layout/Panel.vue';
import type { Pipeline, PipelineError } from '~/lib/api/types';

Expand All @@ -63,9 +73,3 @@ function isBadHabitError(error: PipelineError): error is PipelineError<{ file?:
return error.type === 'bad_habit';
}
</script>

<style scoped>
.grid-3-1 {
grid-template-columns: auto auto auto 1fr;
}
</style>