Skip to content

Commit aff0479

Browse files
daghackdaghack
committed
Add rule for arg / stage name comment descriptions
Signed-off-by: Talon Bowler <[email protected]>
1 parent c1dacbc commit aff0479

File tree

6 files changed

+275
-12
lines changed

6 files changed

+275
-12
lines changed

‎frontend/dockerfile/dockerfile_lint_test.go

Lines changed: 102 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,79 @@ var lintTests = integration.TestFuncs(
4646
testInvalidDefaultArgInFrom,
4747
testFromPlatformFlagConstDisallowed,
4848
testCopyIgnoredFiles,
49+
testDefinitionDescription,
4950
)
5051

52+
func testDefinitionDescription(t *testing.T, sb integration.Sandbox) {
53+
dockerfile := []byte(`
54+
# foo this is the foo
55+
ARG foo=bar
56+
57+
# base this is the base image
58+
FROM scratch AS base
59+
60+
# version this is the version number
61+
ARG version=latest
62+
63+
# baz this is the baz
64+
ARG foo=baz bar=qux baz=quux
65+
#
66+
ARG bit=bat
67+
68+
# comment for something other than ARG or FROM
69+
COPY Dockerfile .
70+
`)
71+
checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile})
72+
73+
dockerfile = []byte(`
74+
# bar this is the bar
75+
ARG foo=bar
76+
# BasE this is the BasE image
77+
FROM scratch AS base
78+
# definitely a bad comment
79+
ARG version=latest
80+
# definitely a bad comment
81+
ARG foo=baz bar=qux baz=quux
82+
`)
83+
checkLinterWarnings(t, sb, &lintTestParams{
84+
Dockerfile: dockerfile,
85+
Warnings: []expectedLintWarning{
86+
{
87+
RuleName: "InvalidDefinitionDescription",
88+
Description: "Comment for build stage or argument should follow the format: `# <arg/stage name> <description>`. If this is not intended to be a description comment, add an empty line or comment between the instruction and the comment.",
89+
URL: "https://docs.docker.com/go/dockerfile/rule/invalid-definition-description/",
90+
Detail: "Comment for ARG should follow the format: `# foo <description>`",
91+
Level: 1,
92+
Line: 3,
93+
},
94+
{
95+
RuleName: "InvalidDefinitionDescription",
96+
Description: "Comment for build stage or argument should follow the format: `# <arg/stage name> <description>`. If this is not intended to be a description comment, add an empty line or comment between the instruction and the comment.",
97+
URL: "https://docs.docker.com/go/dockerfile/rule/invalid-definition-description/",
98+
Detail: "Comment for FROM should follow the format: `# base <description>`",
99+
Level: 1,
100+
Line: 5,
101+
},
102+
{
103+
RuleName: "InvalidDefinitionDescription",
104+
Description: "Comment for build stage or argument should follow the format: `# <arg/stage name> <description>`. If this is not intended to be a description comment, add an empty line or comment between the instruction and the comment.",
105+
URL: "https://docs.docker.com/go/dockerfile/rule/invalid-definition-description/",
106+
Detail: "Comment for ARG should follow the format: `# version <description>`",
107+
Level: 1,
108+
Line: 7,
109+
},
110+
{
111+
RuleName: "InvalidDefinitionDescription",
112+
Description: "Comment for build stage or argument should follow the format: `# <arg/stage name> <description>`. If this is not intended to be a description comment, add an empty line or comment between the instruction and the comment.",
113+
URL: "https://docs.docker.com/go/dockerfile/rule/invalid-definition-description/",
114+
Detail: "Comment for ARG should follow the format: `# <arg_key> <description>`",
115+
Level: 1,
116+
Line: 9,
117+
},
118+
},
119+
})
120+
}
121+
51122
func testCopyIgnoredFiles(t *testing.T, sb integration.Sandbox) {
52123
dockerignore := []byte(`
53124
Dockerfile
@@ -267,30 +338,35 @@ COPY $bar .
267338

268339
func testRuleCheckOption(t *testing.T, sb integration.Sandbox) {
269340
dockerfile := []byte(`#check=skip=all
341+
#
270342
FROM scratch as base
271343
copy Dockerfile .
272344
`)
273345
checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile})
274346

275347
dockerfile = []byte(`#check=skip=all;error=true
348+
#
276349
FROM scratch as base
277350
copy Dockerfile .
278351
`)
279352
checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile})
280353

281354
dockerfile = []byte(`#check=skip=ConsistentInstructionCasing,FromAsCasing
355+
#
282356
FROM scratch as base
283357
copy Dockerfile .
284358
`)
285359
checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile})
286360

287361
dockerfile = []byte(`#check=skip=ConsistentInstructionCasing,FromAsCasing;error=true
362+
#
288363
FROM scratch as base
289364
copy Dockerfile .
290365
`)
291366
checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile})
292367

293368
dockerfile = []byte(`#check=skip=ConsistentInstructionCasing
369+
#
294370
FROM scratch as base
295371
copy Dockerfile .
296372
`)
@@ -302,13 +378,14 @@ copy Dockerfile .
302378
Description: "The 'as' keyword should match the case of the 'from' keyword",
303379
URL: "https://docs.docker.com/go/dockerfile/rule/from-as-casing/",
304380
Detail: "'as' and 'FROM' keywords' casing do not match",
305-
Line: 2,
381+
Line: 3,
306382
Level: 1,
307383
},
308384
},
309385
})
310386

311387
dockerfile = []byte(`#check=skip=ConsistentInstructionCasing;error=true
388+
#
312389
FROM scratch as base
313390
copy Dockerfile .
314391
`)
@@ -320,7 +397,7 @@ copy Dockerfile .
320397
Description: "The 'as' keyword should match the case of the 'from' keyword",
321398
URL: "https://docs.docker.com/go/dockerfile/rule/from-as-casing/",
322399
Detail: "'as' and 'FROM' keywords' casing do not match",
323-
Line: 2,
400+
Line: 3,
324401
Level: 1,
325402
},
326403
},
@@ -330,6 +407,7 @@ copy Dockerfile .
330407
})
331408

332409
dockerfile = []byte(`#check=skip=all
410+
#
333411
FROM scratch as base
334412
copy Dockerfile .
335413
`)
@@ -341,7 +419,7 @@ copy Dockerfile .
341419
Description: "The 'as' keyword should match the case of the 'from' keyword",
342420
URL: "https://docs.docker.com/go/dockerfile/rule/from-as-casing/",
343421
Detail: "'as' and 'FROM' keywords' casing do not match",
344-
Line: 2,
422+
Line: 3,
345423
Level: 1,
346424
},
347425
},
@@ -354,6 +432,7 @@ copy Dockerfile .
354432
})
355433

356434
dockerfile = []byte(`#check=error=true
435+
#
357436
FROM scratch as base
358437
copy Dockerfile .
359438
`)
@@ -368,9 +447,11 @@ copy Dockerfile .
368447
func testStageName(t *testing.T, sb integration.Sandbox) {
369448
dockerfile := []byte(`
370449
# warning: stage name should be lowercase
450+
#
371451
FROM scratch AS BadStageName
372452
373453
# warning: 'as' should match 'FROM' cmd casing.
454+
#
374455
FROM scratch as base2
375456
376457
FROM scratch AS base3
@@ -383,22 +464,23 @@ FROM scratch AS base3
383464
Description: "Stage names should be lowercase",
384465
URL: "https://docs.docker.com/go/dockerfile/rule/stage-name-casing/",
385466
Detail: "Stage name 'BadStageName' should be lowercase",
386-
Line: 3,
467+
Line: 4,
387468
Level: 1,
388469
},
389470
{
390471
RuleName: "FromAsCasing",
391472
Description: "The 'as' keyword should match the case of the 'from' keyword",
392473
URL: "https://docs.docker.com/go/dockerfile/rule/from-as-casing/",
393474
Detail: "'as' and 'FROM' keywords' casing do not match",
394-
Line: 6,
475+
Line: 8,
395476
Level: 1,
396477
},
397478
},
398479
})
399480

400481
dockerfile = []byte(`
401482
# warning: 'AS' should match 'from' cmd casing.
483+
#
402484
from scratch AS base
403485
404486
from scratch as base2
@@ -412,7 +494,7 @@ from scratch as base2
412494
Description: "The 'as' keyword should match the case of the 'from' keyword",
413495
URL: "https://docs.docker.com/go/dockerfile/rule/from-as-casing/",
414496
Detail: "'AS' and 'from' keywords' casing do not match",
415-
Line: 3,
497+
Line: 4,
416498
Level: 1,
417499
},
418500
},
@@ -448,6 +530,7 @@ COPY Dockerfile \
448530
func testConsistentInstructionCasing(t *testing.T, sb integration.Sandbox) {
449531
dockerfile := []byte(`
450532
# warning: 'FROM' should be either lowercased or uppercased
533+
#
451534
From scratch as base
452535
FROM scratch AS base2
453536
`)
@@ -460,7 +543,7 @@ FROM scratch AS base2
460543
URL: "https://docs.docker.com/go/dockerfile/rule/consistent-instruction-casing/",
461544
Detail: "Command 'From' should match the case of the command majority (uppercase)",
462545
Level: 1,
463-
Line: 3,
546+
Line: 4,
464547
},
465548
},
466549
})
@@ -488,6 +571,7 @@ COPY Dockerfile /bar
488571

489572
dockerfile = []byte(`
490573
# warning: 'frOM' should be either lowercased or uppercased
574+
#
491575
frOM scratch as base
492576
from scratch as base2
493577
`)
@@ -499,7 +583,7 @@ from scratch as base2
499583
Description: "All commands within the Dockerfile should use the same casing (either upper or lower)",
500584
URL: "https://docs.docker.com/go/dockerfile/rule/consistent-instruction-casing/",
501585
Detail: "Command 'frOM' should match the case of the command majority (lowercase)",
502-
Line: 3,
586+
Line: 4,
503587
Level: 1,
504588
},
505589
},
@@ -527,6 +611,7 @@ copy Dockerfile /bar
527611

528612
dockerfile = []byte(`
529613
# warning: 'from' should match command majority's casing (uppercase)
614+
#
530615
from scratch
531616
COPY Dockerfile /foo
532617
COPY Dockerfile /bar
@@ -540,14 +625,15 @@ COPY Dockerfile /baz
540625
Description: "All commands within the Dockerfile should use the same casing (either upper or lower)",
541626
URL: "https://docs.docker.com/go/dockerfile/rule/consistent-instruction-casing/",
542627
Detail: "Command 'from' should match the case of the command majority (uppercase)",
543-
Line: 3,
628+
Line: 4,
544629
Level: 1,
545630
},
546631
},
547632
})
548633

549634
dockerfile = []byte(`
550635
# warning: 'FROM' should match command majority's casing (lowercase)
636+
#
551637
FROM scratch
552638
copy Dockerfile /foo
553639
copy Dockerfile /bar
@@ -561,7 +647,7 @@ copy Dockerfile /baz
561647
Description: "All commands within the Dockerfile should use the same casing (either upper or lower)",
562648
URL: "https://docs.docker.com/go/dockerfile/rule/consistent-instruction-casing/",
563649
Detail: "Command 'FROM' should match the case of the command majority (lowercase)",
564-
Line: 3,
650+
Line: 4,
565651
Level: 1,
566652
},
567653
},
@@ -1382,6 +1468,12 @@ func checkUnmarshal(t *testing.T, sb integration.Sandbox, lintTest *lintTestPara
13821468
require.Less(t, lintResults.Error.Location.SourceIndex, int32(len(lintResults.Sources)))
13831469
}
13841470

1471+
if len(warnings) != len(lintResults.Warnings) {
1472+
for _, w := range lintResults.Warnings {
1473+
t.Logf("Warning Received: %s\n", w.Detail)
1474+
}
1475+
}
1476+
13851477
require.Equal(t, len(warnings), len(lintResults.Warnings))
13861478

13871479
sort.Slice(lintResults.Warnings, func(i, j int) bool {

‎frontend/dockerfile/docs/rules/_index.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,5 +103,9 @@ To learn more about how to use build checks, see
103103
<td><a href="./copy-ignored-file/">CopyIgnoredFile (experimental)</a></td>
104104
<td>Attempting to Copy file that is excluded by .dockerignore</td>
105105
</tr>
106+
<tr>
107+
<td><a href="./invalid-definition-description/">InvalidDefinitionDescription</a></td>
108+
<td>Comment for build stage or argument should follow the format: `# <arg/stage name> <description>`. If this is not intended to be a description comment, add an empty line or comment between the instruction and the comment.</td>
109+
</tr>
106110
</tbody>
107111
</table>

‎frontend/dockerfile/docs/rules/invalid-definition-description.md

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
---
2+
title: InvalidDefinitionDescription
3+
description: Comment for build stage or argument should follow the format: `# <arg/stage name> <description>`. If this is not intended to be a description comment, add an empty line or comment between the instruction and the comment.
4+
aliases:
5+
- /go/dockerfile/rule/invalid-definition-description/
6+
---
7+
8+
## Output
9+
10+
```text
11+
Comment for build stage or argument should follow the format: `# <arg/stage name> <description>`. If this is not intended to be a description comment, add an empty line or comment between the instruction and the comment.
12+
```
13+
14+
## Description
15+
16+
The [`--call=outline`](https://docs.docker.com/reference/cli/docker/buildx/build/#call-outline)
17+
and [`--call=targets`](https://docs.docker.com/reference/cli/docker/buildx/build/#call-outline)
18+
flags for the `docker build` command print descriptions for build targets and arguments.
19+
The descriptions are generated from [Dockerfile comments](https://docs.docker.com/reference/cli/docker/buildx/build/#descriptions)
20+
that immediately precede the `FROM` or `ARG` instruction
21+
and that begin with the name of the build stage or argument.
22+
For example:
23+
24+
```dockerfile
25+
# build-cli builds the CLI binary
26+
FROM alpine AS build-cli
27+
# VERSION controls the version of the program
28+
ARG VERSION=1
29+
```
30+
31+
In cases where preceding comments are not meant to be descriptions,
32+
add an empty line or comment between the instruction and the preceding comment.
33+
34+
## Examples
35+
36+
❌ Bad: A non-descriptive comment on the line preceding the `FROM` command.
37+
38+
```dockerfile
39+
# a non-descriptive comment
40+
FROM scratch AS base
41+
42+
# another non-descriptive comment
43+
ARG VERSION=1
44+
```
45+
46+
✅ Good: An empty line separating non-descriptive comments.
47+
48+
```dockerfile
49+
# a non-descriptive comment
50+
51+
FROM scratch AS base
52+
53+
# another non-descriptive comment
54+
55+
ARG VERSION=1
56+
```
57+
58+
✅ Good: Comments describing `ARG` keys and stages immediately proceeding the command.
59+
60+
```dockerfile
61+
# base is a stage for compiling source
62+
FROM scratch AS base
63+
# VERSION This is the version number.
64+
ARG VERSION=1
65+
```
66+

0 commit comments

Comments
 (0)