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

gateway: enable named contexts for gateway frontend #3633

Merged
merged 2 commits into from
Jun 28, 2023

Conversation

tonistiigi
Copy link
Member

This allows overwriting the definition of #syntax names same as can be done with other dockerfile names.

Signed-off-by: Tonis Tiigi tonistiigi@gmail.com

frontend/gateway/gateway.go Outdated Show resolved Hide resolved
@jedevc
Copy link
Member

jedevc commented Feb 22, 2023

I think we might need to modify some of the caps detection logic as well - I gave this a spin on a frontend that doesn't have the moby.buildkit.frontend.contexts cap, and I get the following:

$ docker buildx build . -f Testfile --build-context custom-frontend=oci-layout://my-custom-frontend
...
ERROR: failed to build: current frontend does not support --build-context. Named contexts are supported since Dockerfile v1.4. Use #syntax directive in Dockerfile or update to latest BuildKit.

Even though my frontend doesn't use build-contexts, because of the propagation of build-contexts, the frontend needs to declare the cap.

@tonistiigi
Copy link
Member Author

I think we might need to modify some of the caps detection logic as well - I gave this a spin on a frontend that doesn't have the moby.buildkit.frontend.contexts cap, and I get the following:

Not sure I understand. Isn't it correct that if the user passes --build-context and the frontend doesn't support it, then it is an error? Isn't this the behavior before this PR as well?

@jedevc
Copy link
Member

jedevc commented Feb 22, 2023

Because with this patch we can determine the frontend based on the build-context, if you want to use this feature to only change the frontend, that frontend must support build-contexts, or it's propagated through, and immediately errors.

@tonistiigi
Copy link
Member Author

I think it is just something we need to accept that if you wish to use --build-context with #syntax to switch frontends, the frontend itself needs to support it as well. The only other way would be to start passing 2 different sets of contexts (one for gateway and another for redirect) what will get messy quickly.

@tonistiigi tonistiigi added this to the v0.12.0 milestone May 22, 2023
Copy link
Member

@jedevc jedevc left a comment

Choose a reason for hiding this comment

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

LGTM, but can we also get a test 👀

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
@tonistiigi
Copy link
Member Author

Testing this would mean building a custom frontend inside the test. Gateway testing is specific to Dockerfile frontend atm and is built into the test script that loads the frontend image.

@jedevc jedevc merged commit b85b5ab into moby:master Jun 28, 2023
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.

2 participants