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

checks: add check for constant in from platform flag #5140

Merged

Conversation

jsternberg
Copy link
Collaborator

@jsternberg jsternberg commented Jul 9, 2024

This linter rule triggers if a constant value has been used in the
FROM --platform flag and the stage name doesn't contain the OS or
architecture mentioned.

Fixes #5131.

@jsternberg jsternberg force-pushed the lint-constant-platform-disallowed branch from fc7a908 to 989d184 Compare July 9, 2024 21:20
@jsternberg jsternberg added this to the v0.15.0 milestone Jul 9, 2024
@jsternberg jsternberg self-assigned this Jul 9, 2024
@@ -315,6 +315,7 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
platMatch, err := shlex.ProcessWordWithMatches(v, platEnv)
reportUnusedFromArgs(metaArgsKeys(optMetaArgs), platMatch.Unmatched, st.Location, lint)
reportRedundantTargetPlatform(st.Platform, platMatch, st.Location, platEnv, lint)
reportFromConstDisallowed(st.Name, platMatch, st.Location, lint)
Copy link
Member

Choose a reason for hiding this comment

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

reportConstPlatformDisallowed

```dockerfile
FROM --platform=${BUILDPLATFORM} alpine AS base
RUN apk add --no-cache git
```
Copy link
Member

Choose a reason for hiding this comment

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

Add this multi-stage example as good example:

FROM --platform=linux/amd64 alpine:3.20 AS build_amd64
...

FROM --platform=linux/arm64 alpine:3.19 AS build_arm64
...

FROM build_${TARGETARCH} AS build
...
...

## Output

```text
FROM --platform=linux/amd64 should not use a constant value
Copy link
Member

Choose a reason for hiding this comment

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

"FROM --platform flag should not ... " ?


* Omit `FROM --platform` in the Dockerfile and use the `--platform` argument on the command line.
* Use `$BUILDPLATFORM` or some other combination of variables for the `--platform` argument.
* Stage name should include the platform, OS, or architecture name.
Copy link
Member

Choose a reason for hiding this comment

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

"Stage name should include the platform, OS, or architecture name to indicate that it only contains platform-specific instructions"

@tonistiigi tonistiigi requested a review from dvdksn July 9, 2024 22:12
@jsternberg jsternberg force-pushed the lint-constant-platform-disallowed branch 2 times, most recently from c48d349 to c644927 Compare July 10, 2024 14:50
This linter rule triggers if a constant value has been used in the
`FROM --platform` flag and the stage name doesn't contain the OS or
architecture mentioned.

Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
@jsternberg jsternberg force-pushed the lint-constant-platform-disallowed branch from c644927 to 58753e8 Compare July 10, 2024 15:09
@jsternberg
Copy link
Collaborator Author

Test failure seems to be unrelated:

=== FAIL: frontend/dockerfile TestIntegration/TestHistoryError/worker=containerd/frontend=gateway (1.80s)
    dockerfile_test.go:7498: 
        	Error Trace:	/src/frontend/dockerfile/dockerfile_test.go:7498
        	            				/src/util/testutil/integration/run.go:96
        	            				/src/util/testutil/integration/run.go:211
        	Error:      	Not equal: 
        	            	expected: true
        	            	actual  : false
        	Test:       	TestIntegration/TestHistoryError/worker=containerd/frontend=gateway

@tonistiigi
Copy link
Member

@jsternberg covered by #5142

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add check for usage of --platform= flag in Dockerfile FROM (FromPlatformFlagConstDisallowed)
2 participants