buildkit icon indicating copy to clipboard operation
buildkit copied to clipboard

Proposal: lint disable controls

Open daghack opened this issue 1 year ago • 2 comments

Proposal

Currently, lint warnings in the Dockerfile frontend send their output to stderr and there is no convenient way to disable, either on a lint-rule-by-lint-rule basis, on a line-by-line basis, or as a whole.

After discussion with @tonistiigi, there are a few use-cases we want to support for the Dockerfile linting:

  • Disabling specific rules
  • Disabling all rules
  • Error-on-warning
  • Overwriting lint-controls via build-arg (for the case when being able to set/update lint exceptions from within the Dockerfile itself is not desirable, such as within an org that has some desired quality controls around the lint rules).

The proposal is as follows - that we add a new dockerfile directive, lint, as well as a new built-in build arg, BUILDKIT_DOCKERFILE_LINT.

#syntax=docker/dockerfile
#lint=skip=StageNameCasing,NoEmptyContinuations;error=true
FROM alpine
...
#lint-skip
ARG MYPASSWORD
docker build --build-arg="BUILDKIT_DOCKERFILE_LINT=skip=StageNameCasing,NoEmptyContinuations;error=true

The lint parser directive

Like other parser directives, the lint directive would be found at the top of the Dockerfile, and indicates both the lint tests to skip over as well as whether errors ought to be emitted in place of warnings.

#lint=skip=StageNameCasing,NoEmptyContinuations;error=true

The skip= clause of the value is a comma-separated list of lint rule names to, naturally, skip over, with all being a special key to indicate all tests. The error= clause of the value is a boolean and defaults to false.

Examples:

Skip no lint checks, but error on a lint warning:

#lint=error=true

Skip all lint checks:

#lint=skip=all

Skip StageNameCasing and NoEmptyContinuations rules, but error on a lint warning

#lint=skip=StageNameCasing,NoEmptyContinuations;error=true

Built-in build arg: BUILDKIT_DOCKERFILE_LINT

In the case where it is not desirable that linting behavior can be set from within the Dockerfile, we can overwrite the value of the lint directive via a build-arg.

daghack avatar May 13 '24 15:05 daghack

BUILDKIT_LINT_SKIP=skip=StageNameCasing,NoEmptyContinuations;error=true

Is this intentionally ..._SKIP=skip=... (redundant), or was that a typo?

tianon avatar May 14 '24 23:05 tianon

Not a typo so much that BUILDKIT_LINT felt a little too... vague? I'm not especially attached to any specific build-arg name. I'll update to BUILDKIT_DOCKERFILE_LINT, which feels specific enough for me and less redundant.

daghack avatar May 15 '24 17:05 daghack

After some discussion around linting with @colinhemmings, I'm updating this proposal from using lint as the directive and replacing it with check, as well as updating the build-arg to be BUILDKIT_DOCKERFILE_CHECK.

daghack avatar May 29 '24 16:05 daghack

Regarding the comment on the implementation. I think check is appropriate for the action of executing, but rule maybe be a better term for the specific configuration.

I feel like the syntax for these directives is kind of unintuitive. #check=skip=StageNameCasing,NoEmptyContinuations;error=true

colinhemmings avatar May 30 '24 16:05 colinhemmings