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

builder: do not set network.host entitlement flag if already set in buildkitd conf #2685

Merged
merged 1 commit into from
Sep 11, 2024

Conversation

crazy-max
Copy link
Member

@crazy-max crazy-max commented Sep 11, 2024

follow-up #2683 (comment)

On container builder creation, it's possible to define buildkitd configuration path that will injected and used inside the container.

If insecure-entitlements opts are set in this configuration they will be skipped because we are always setting the --allow-insecure-entitlement network.host #2266 if user does not set it explicitly and buildkitd overrides configuration values with flags: https://github.com/moby/buildkit/blob/3a7055008a5e58a2abbe0e0c21c919d9e014e062/cmd/buildkitd/main.go#L583-L584.

With this change we are now detecting if network.host insecure entitlement is set within the buildkitd configuration and skip setting --allow-insecure-entitlement network.host flag.

cc @dvdksn

…uildkitd conf

Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
@crazy-max crazy-max marked this pull request as ready for review September 11, 2024 11:02
@crazy-max
Copy link
Member Author

crazy-max commented Sep 11, 2024

@tonistiigi As follow-up I think it would make sense to merge insecure entitlements from flags and configuration in BuildKit https://github.com/moby/buildkit/blob/3a7055008a5e58a2abbe0e0c21c919d9e014e062/cmd/buildkitd/main.go#L583-L584. WDYT?

Didn't find any specific discussion around current behavior https://github.com/moby/buildkit/pull/570/files#diff-fb243a9185f7d24f7bfdf25097d6128f1b29b1a304bfc13d360667f8358551f6R375-R376

@crazy-max crazy-max added this to the v0.18.0 milestone Sep 11, 2024
@tonistiigi
Copy link
Member

tonistiigi commented Sep 11, 2024

v0.17.1 for this?

@tonistiigi tonistiigi merged commit 8fd27b8 into docker:master Sep 11, 2024
106 checks passed
@crazy-max crazy-max deleted the skip-networkhost-conf branch September 11, 2024 18:10
@crazy-max crazy-max modified the milestones: v0.18.0, v0.17.1 Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants