dockerfile: generate lint rules documentation #4990

crazy-max · 2024-06-04T10:38:59Z

relates to #4823 and #4980

Generates lint rules documentation in ./frontend/dockerfile/docs/rules that can then be used on https://github.com/docker/docs through vendoring with Hugo.

Keep in draft as some markdown files are incomplete.

crazy-max · 2024-06-04T10:40:44Z

frontend/dockerfile/linter/generate.go

+		if errors.Is(err, os.ErrNotExist) {
+			log.Printf("WARNING: Markdown file %q not found for %s\n", mdfilename, rule.Name)
+			return false, nil
+		}


It needs a markdown file that contains examples for the rule. Atm we skip docs generation if the markdown file is not found but I think it should be required and fails if not found (cc @dvdksn)

Updated to make validation fails if markdown files are missing.

crazy-max · 2024-06-04T10:43:07Z

frontend/dockerfile/linter/docs/FileConsistentCommandCasing.md

I just have a markdown file for this rule atm but we should have one for each of them.

@dvdksn Feel free to push to my branch other ones.

dvdksn · 2024-06-04T11:32:37Z

frontend/dockerfile/linter/generate.go

+description: {{.Rule.Description}}
+---
+
+{{.Content}}


here's a far out ask. Is there any way we can generate the sample error using the Format field? https://github.com/moby/buildkit/blob/master/frontend/dockerfile/linter/linter.go#L79

Just thinking if there's a way to make sure the sample error in the doc and the actual error look the same. If generating the sample error isn't easily doable, maybe we could update the tests to check that generated errors have the same format as what's in the doc.

I think we could but where to put Format in the template and what layout would this have?

I wonder if we could put the templating logic in the Markdown files directly. For example, consider frontend/dockerfile/linter/docs/RuleName.md

--- title: {{ .Name }} description: {{ .Description }} --- Example warning: {{ .Format "foo" "bar" }} Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod...

Yes sounds good

dvdksn · 2024-06-05T12:09:33Z

frontend/dockerfile/docs/rules/_index.md

This file is outdated, I created it a while ago. Would be nice to generate it too (I think we need an index for all of these rules)

Added commit to generate _index.md as well: https://github.com/crazy-max/buildkit/blob/docs-dockerfile-gen/frontend/dockerfile/docs/rules/_index.md

crazy-max · 2024-06-05T12:10:35Z

Picked changes from @dvdksn branch to include additional markdown files: crazy-max#18

frontend/dockerfile/linter/ruleset.go

crazy-max · 2024-06-06T08:16:46Z

frontend/dockerfile/linter/docs/UndeclaredArgInFrom.md

Missing content

As discussed it's fine to merge even if content is empty. We can update it later. We just want URL to be set.

crazy-max

@colinhemmings PTAL on rule's description related to #4980 (comment)

frontend/dockerfile/linter/ruleset.go

tonistiigi · 2024-06-06T20:00:04Z

frontend/dockerfile/linter/ruleset.go

 		URL:         "https://docs.docker.com/go/dockerfile/rule/reserved-stage-name/",
 		Format: func(reservedStageName string) string {
 			return fmt.Sprintf("Stage name should not use the same name as reserved stage %q", reservedStageName)
 		},
 	}
 	RuleJSONArgsRecommended = LinterRule[func(instructionName string) string]{
 		Name:        "JSONArgsRecommended",
-		Description: "JSON arguments recommended for ENTRYPOINT/CMD to prevent unintended behavior related to OS signals",
+		Description: "JSON arguments recommended for ENTRYPOINT/CMD to prevent unintended behavior related to OS signals, unless using a custom shell",


nit: "unless using a custom shell" is not needed here. If the user knows what "custom shell" is then they will not see this warning. Otherwise it will just confuse them.

Explaining the "custom shell" behavior in the linked URL is good.

tonistiigi · 2024-06-06T20:00:22Z

Dockerfile

@@ -378,7 +378,7 @@ exec dlv exec /usr/bin/buildkitd \\
  --continue \\
  -- "\$@"
 EOF
-ENV DELVE_PORT 5000
+ENV DELVE_PORT=5000


nit: these changes could be in another PR

💯 -- sorry about that, those were driving me nuts

tonistiigi · 2024-06-06T21:53:07Z

frontend/dockerfile/docs/rules/undefined-arg-in-from.md

@@ -0,0 +1,26 @@
+---
+title: UndefinedArgInFrom


These are for follow-up.

crazy-max

Needs to align generated docs: https://github.com/moby/buildkit/actions/runs/9408268148/job/25915879498?pr=4990#step:5:319

#20 [validate 1/1] RUN --mount=target=/context   --mount=target=.,type=tmpfs <<EOT (set -e...)
#20 0.814 ERROR: Dockerfile docs result differs. Please update with "make docs-dockerfile"
#20 0.842  M frontend/dockerfile/docs/rules/_index.md

Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>

Signed-off-by: David Karlsson <35727626+dvdksn@users.noreply.github.com>

Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>

Signed-off-by: Shaun Thompson <shaun.thompson@docker.com>

crazy-max commented Jun 4, 2024

View reviewed changes

crazy-max force-pushed the docs-dockerfile-gen branch from fe33f6a to bab6f7b Compare June 4, 2024 10:41

crazy-max commented Jun 4, 2024

View reviewed changes

crazy-max requested review from tonistiigi, jsternberg, daghack and dvdksn June 4, 2024 10:43

crazy-max force-pushed the docs-dockerfile-gen branch from bab6f7b to 4dc9006 Compare June 4, 2024 11:21

crazy-max added area/dockerfile kind/docs labels Jun 4, 2024

dvdksn reviewed Jun 4, 2024

View reviewed changes

dvdksn reviewed Jun 5, 2024

View reviewed changes

crazy-max commented Jun 5, 2024

View reviewed changes

frontend/dockerfile/linter/ruleset.go Show resolved Hide resolved

crazy-max force-pushed the docs-dockerfile-gen branch 2 times, most recently from dd2e9f4 to 5b63ac6 Compare June 5, 2024 12:34

crazy-max added this to the v0.14.0 milestone Jun 5, 2024

crazy-max force-pushed the docs-dockerfile-gen branch from 50dd7e9 to d8bfaf4 Compare June 5, 2024 16:01

This comment was marked as resolved.

Sign in to view

crazy-max force-pushed the docs-dockerfile-gen branch 3 times, most recently from d844815 to f6c970b Compare June 6, 2024 08:13

crazy-max commented Jun 6, 2024

View reviewed changes

crazy-max force-pushed the docs-dockerfile-gen branch from f6c970b to 53e3858 Compare June 6, 2024 15:30

thompson-shaun mentioned this pull request Jun 6, 2024

Verify linting rules descriptions and links #4980

Closed

crazy-max force-pushed the docs-dockerfile-gen branch 2 times, most recently from 8f0c7c5 to 989c53e Compare June 6, 2024 15:53

crazy-max commented Jun 6, 2024

View reviewed changes

crazy-max requested a review from colinhemmings June 6, 2024 16:04

This comment was marked as resolved.

Sign in to view

crazy-max marked this pull request as ready for review June 6, 2024 16:06

crazy-max force-pushed the docs-dockerfile-gen branch from 989c53e to 3bd7bc4 Compare June 6, 2024 16:27

crazy-max commented Jun 6, 2024

View reviewed changes

frontend/dockerfile/linter/ruleset.go Outdated Show resolved Hide resolved

thompson-shaun force-pushed the docs-dockerfile-gen branch 2 times, most recently from cc631ae to 7c85e86 Compare June 6, 2024 18:45

tonistiigi reviewed Jun 6, 2024

View reviewed changes

thompson-shaun force-pushed the docs-dockerfile-gen branch 3 times, most recently from 053d42c to 3b4139b Compare June 6, 2024 21:47

tonistiigi approved these changes Jun 6, 2024

View reviewed changes

tonistiigi reviewed Jun 6, 2024

View reviewed changes

frontend/dockerfile/docs/rules/undefined-arg-in-from.md

@@ -0,0 +1,26 @@

---

title: UndefinedArgInFrom

Copy link

Member

tonistiigi Jun 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are for follow-up.

crazy-max commented Jun 6, 2024

View reviewed changes

tonistiigi added the needs/follow-up label Jun 6, 2024

thompson-shaun force-pushed the docs-dockerfile-gen branch from 3b4139b to 25dbfdc Compare June 6, 2024 21:57

crazy-max and others added 6 commits June 6, 2024 15:11

dockerfile: generate lint rules documentation

7283eab

Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>

lint: add/generate docs for build rules

03a7663

Signed-off-by: David Karlsson <35727626+dvdksn@users.noreply.github.com>

lint: generate _index.md docs

d4351a3

Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>

lint: add alias to doc rule frontmatter

92698a6

Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>

lint: InvalidBaseImagePlatform does not need specific docs

138ee4c

Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>

lint: update descriptions, test and re-generate docs

37fc95a

Signed-off-by: Shaun Thompson <shaun.thompson@docker.com>

tonistiigi force-pushed the docs-dockerfile-gen branch from 25dbfdc to 37fc95a Compare June 6, 2024 22:18

tonistiigi approved these changes Jun 6, 2024

View reviewed changes

tonistiigi merged commit aebcc1f into moby:master Jun 6, 2024
75 checks passed

crazy-max deleted the docs-dockerfile-gen branch June 7, 2024 08:27

crazy-max mentioned this pull request Jun 10, 2024

vendor build checks reference docker/docs#20196

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dockerfile: generate lint rules documentation #4990

dockerfile: generate lint rules documentation #4990

crazy-max commented Jun 4, 2024 •

edited

Loading

crazy-max Jun 4, 2024

crazy-max Jun 4, 2024 •

edited

Loading

crazy-max Jun 4, 2024

dvdksn Jun 4, 2024 •

edited

Loading

crazy-max Jun 4, 2024

dvdksn Jun 4, 2024

crazy-max Jun 4, 2024

dvdksn Jun 5, 2024

crazy-max Jun 5, 2024 •

edited

Loading

crazy-max commented Jun 5, 2024

This comment was marked as resolved.

crazy-max Jun 6, 2024

crazy-max Jun 6, 2024

crazy-max left a comment •

edited

Loading

This comment was marked as resolved.

tonistiigi Jun 6, 2024 •

edited

Loading

tonistiigi Jun 6, 2024

thompson-shaun Jun 6, 2024

tonistiigi Jun 6, 2024

crazy-max left a comment

dockerfile: generate lint rules documentation #4990

dockerfile: generate lint rules documentation #4990

Conversation

crazy-max commented Jun 4, 2024 • edited Loading

Choose a reason for hiding this comment

crazy-max Jun 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dvdksn Jun 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crazy-max Jun 5, 2024 • edited Loading

Choose a reason for hiding this comment

crazy-max commented Jun 5, 2024

This comment was marked as resolved.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crazy-max left a comment • edited Loading

Choose a reason for hiding this comment

This comment was marked as resolved.

tonistiigi Jun 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crazy-max left a comment

Choose a reason for hiding this comment

crazy-max commented Jun 4, 2024 •

edited

Loading

crazy-max Jun 4, 2024 •

edited

Loading

dvdksn Jun 4, 2024 •

edited

Loading

crazy-max Jun 5, 2024 •

edited

Loading

crazy-max left a comment •

edited

Loading

tonistiigi Jun 6, 2024 •

edited

Loading