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

bake: enable support for entitlements #2666

Merged
merged 2 commits into from
Sep 3, 2024

Conversation

tonistiigi
Copy link
Member

Ref #179

Add support for security.insecure and network.host entitlements via bake. User needs to confirm elevated privileges through a prompt or CLI flags.

const (
EntitlementKeyNetworkHost EntitlementKey = "network.host"
EntitlementKeySecurityInsecure EntitlementKey = "security.insecure"
EntitlementKeyFSRead EntitlementKey = "fs.read"
Copy link
Member Author

Choose a reason for hiding this comment

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

Even though these additional entitlements are defined here, the current PR does not perform any validation for them yet.

@tonistiigi
Copy link
Member Author

tonistiigi commented Aug 29, 2024

While the entitlements key in HCL is new and could not be used before, network: host was enabled via compose and may potentially break if entitlements is not passed by the user.

 # cat docker-compose.yml
services:
  app:
    build:
      network: "host"
 #
 # docker buildx bake app --print
[+] Building 0.0s (1/1) FINISHED
 => [internal] load local bake definitions                                                 0.0s
 => => reading docker-compose.yml 50B / 50B                                                0.0s
{
  "group": {
    "default": {
      "targets": [
        "app"
      ]
    }
  },
  "target": {
    "app": {
      "context": ".",
      "dockerfile": "Dockerfile",
      "entitlements": [
        "network.host"
      ]
    }
  }
}
 # docker buildx bake app
[+] Building 0.0s (1/1) FINISHED
 => [internal] load local bake definitions                                                 0.0s
 => => reading docker-compose.yml 50B / 50B                                                0.0s
Your build is requesting privileges for following possibly insecure capabilities:

 - Running build containers that can access host network

In order to not see this message in the future pass "--allow=network.host" to grant requested privileges.

Your full command with requested privileges:

docker buildx bake --allow=network.host app

Do you want to grant requested privileges and continue? [y/N]

Additionally, I guess we now want to expose network = "host" via HCL/JSON as well? Note that the recommended way is for Dockerfile to define when it needs host network by setting RUN --network=host instead of overwriting the network for the whole build here.

Add support for security.insecure and network.host
entitlements via bake. User needs to confirm elevated
privileges through a prompt or CLI flags.

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

While the entitlements key in HCL is new and could not be used before, network: host was enabled via compose and may potentially break if entitlements is not passed by the user.

I think this is fine looking at the recommendation printed during build.

Additionally, I guess we now want to expose network = "host" via HCL/JSON as well? Note that the recommended way is for Dockerfile to define when it needs host network by setting RUN --network=host instead of overwriting the network for the whole build here.

Yes we need that

}

args := append([]string(nil), os.Args...)
if filepath.Base(args[0]) == "docker-buildx" {
Copy link
Member

@crazy-max crazy-max Sep 2, 2024

Choose a reason for hiding this comment

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

If the idea is to check if buildx runs as docker plugin, then it would be better to check for !plugin.RunningStandalone()

Copy link
Member Author

Choose a reason for hiding this comment

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

This isn't quite it. It is because when docker invokes the command then the arg is always docker-buildx so we shouldn't just replace it based on any command name.

But I now see that there is DOCKER_CLI_PLUGIN_ORIGINAL_CLI_COMMAND . I'll see if I can reuse it for this.

@crazy-max
Copy link
Member

@dvdksn Seems a docs link is broken: https://github.com/docker/buildx/actions/runs/10636095098/job/29487219028?pr=2666#step:6:467

 #29 [validate-upstream 5/5] RUN htmltest
#29 0.066 htmltest started at 04:36:56 on public
#29 0.066 ========================================================================
#29 1.050 reference/cli/docker/buildx_bake/index.html
#29 1.050   hash does not exist --- reference/cli/docker/buildx_bake/index.html --> /build/attestations/slsa-provenance/#provenance-attestation-example
#29 1.050 reference/cli/docker/buildx_build/index.html
#29 1.205   hash does not exist --- reference/cli/docker/buildx_build/index.html --> /build/attestations/slsa-provenance/#provenance-attestation-example
#29 1.205 reference/cli/docker/buildx/build/index.html
#29 1.923   hash does not exist --- reference/cli/docker/buildx/build/index.html --> /build/attestations/slsa-provenance/#provenance-attestation-example
#29 1.923 reference/cli/docker/buildx/bake/index.html
#29 1.954   hash does not exist --- reference/cli/docker/buildx/bake/index.html --> /build/attestations/slsa-provenance/#provenance-attestation-example
#29 1.954 ========================================================================
#29 12.36 ✘✘✘ failed in 12.29200319s
#29 12.36 4 errors in 2598 documents

@dvdksn
Copy link
Contributor

dvdksn commented Sep 3, 2024

@crazy-max let me fix those in a follow-up

@tonistiigi
Copy link
Member Author

I'm not sure what this error is about. buildx bake validate-docs passes for me locally.

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

Looks like the docs thing went away. Green now.

@crazy-max
Copy link
Member

Looks like the docs thing went away. Green now.

Yes this has been sorted with #2652

@tonistiigi tonistiigi merged commit f369377 into docker:master Sep 3, 2024
108 checks passed
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.

3 participants