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

Introduce a new env var "COMPOSE_DOCKER_CLI_BUILD_EXTRA_ARGS" #7296

Conversation

moriyoshi
Copy link

This patch allows one to pass arbitrary arguments in the docker command invocation through COMPOSE_DOCKER_CLI_BUILD_EXTRA_ARGS.

Resolves #7025.

which allows one to pass arbitrary arguments in the docker command
invocation.

Signed-off-by: Moriyoshi Koizumi <mozo@mozo.jp>
@Syntactical01
Copy link

Only note, to be conistent with COMPOSE_DOCKER_CLI_BUILD you should probably use COMPOSE_DOCKER_CLI_BUILD_EXTRA_ARGS. That being said can someone review this? Docker compose REALLY needs support for the --ssh argument.

@mecampbellsoup
Copy link

Please merge!! Like many others we want to use --ssh when running builds thru docker compose. Thanks.

@itsjef
Copy link

itsjef commented Apr 21, 2020

Any progress update on this PR? 🤔

@mecampbellsoup
Copy link

Workaround:

COMPOSE_DOCKER_CLI_BUILD=1 docker-compose build {app-name-that-uses-build-kit}

Also here is my Docker daemon config:

{
  "debug": true,
  "experimental": true,
  "features": {
    "buildkit": true
  }
}

@ppg
Copy link

ppg commented May 11, 2020

That workaround doesn't help with --ssh or any other buildkit options that aren't explicitly supported by compose right? That's just a way to save typing DOCKER_BUILDKIT=1, i.e. you could simply do:

COMPOSE_DOCKER_CLI_BUILD=1 DOCKER_BUILDKIT=1 docker-compose build {app-name-that-uses-build-kit}

But neither of them help with the fact the argument list is hardcoded (see

compose/compose/service.py

Lines 1790 to 1803 in 440c94e

command_builder = _CommandBuilder()
command_builder.add_params("--build-arg", buildargs)
command_builder.add_list("--cache-from", cache_from)
command_builder.add_arg("--file", dockerfile)
command_builder.add_flag("--force-rm", forcerm)
command_builder.add_params("--label", labels)
command_builder.add_arg("--memory", container_limits.get("memory"))
command_builder.add_flag("--no-cache", nocache)
command_builder.add_arg("--progress", self._progress)
command_builder.add_flag("--pull", pull)
command_builder.add_arg("--tag", tag)
command_builder.add_arg("--target", target)
command_builder.add_arg("--iidfile", iidfile)
args = command_builder.build([path])
) and thus we will always be playing catchup to a buildkit build's options unless we have some pass through like this, specifically in this case the desire to pass --ssh.

@thaJeztah
Copy link
Member

I don't think we should implement this; this would be a global option, which means that in a compose-file that has multiple build instructions, this option would grant all builds access to the ssh-agent, which is not desirable.

@ppg
Copy link

ppg commented May 11, 2020

Is there an alternative plan? #6865 mentions #7046 but that is again hard-coding a new argument, so it will suffer the same issue of playing keep up, but also it's only adding --secrets; I didn't think you could implement the --ssh flag with --secrets?

@thaJeztah
Copy link
Member

#7046 might still be the way forward, but might need to be proposed in https://github.com/compose-spec/compose-spec

@hannahkamundson
Copy link

Would love to see this merge request put in!

@tommie
Copy link

tommie commented May 22, 2020

I filed compose-spec/compose-spec#81 to continue the compose-spec discussion there.

@philomory
Copy link

I'd very much prefer to see this in the docker-compose.yaml rather than adding more and more environment variables. In fact, I'd really prefer if COMPOSE_DOCKER_CLI_BUILD was moved to the docker-compose.yaml as well (but that probably belongs in a separate issue).

@moriyoshi
Copy link
Author

I don't think we should implement this; this would be a global option, which means that in a compose-file that has multiple build instructions, this option would grant all builds access to the ssh-agent, which is not desirable.

Passing a different key to each build sounds pretty much unusual to me. As for the compatibility, I don't think the build will be broken if SSH_AUTH_SOCK is present because its Dockerfile should have explicitly seeded the required key somewhere in the container in the first place.

And in case the behavior is undesirable, one can always choose not to use the option at all.

@eliliam
Copy link

eliliam commented Jul 8, 2020

Lets get this merged in guys, any devs know why the build it failed?

@goetas
Copy link

goetas commented Jul 9, 2020

I do not think that this is the best solution. COMPOSE_DOCKER_CLI_BUILD_EXTRA_ARGS would be a global option, but not all the services being built might need those extra args, or might need them different. #6358 (in particular #6358 (comment)) proposes a much safer solution (still a bit hacky but IMO good enough)

@Billy-
Copy link

Billy- commented Jul 10, 2020

I don't think we should implement this; this would be a global option, which means that in a compose-file that has multiple build instructions, this option would grant all builds access to the ssh-agent, which is not desirable.

@thaJeztah how can we get a definitive decision? If we definitely don't want this, should we close this PR to avoid confusion? And people spending time on it?

I do not think that this is the best solution. COMPOSE_DOCKER_CLI_BUILD_EXTRA_ARGS would be a global option, but not all the services being built might need those extra args, or might need them different. #6358 (in particular #6358 (comment)) proposes a much safer solution (still a bit hacky but IMO good enough)

I agree, this is a good suggestion. Looks like maybe we should either discuss further on that issue or file a new issue (as that issue is specifically about secrets, and this PR is about passing any flags you like).

@Seros
Copy link

Seros commented Mar 19, 2021

When can we expect to have this feature done or is there something we can support to speed up the process?

@glours
Copy link
Contributor

glours commented Jul 27, 2022

Thanks for taking the time to create this issue/pull request!

Unfortunately, Docker Compose V1 has reached end-of-life and we are not accepting any more changes (except for security issues). Please try and reproduce your issue with Compose V2 or rewrite your pull request to be based on the v2 branch and create a new issue or PR with the relevant Compose V2 information.

@glours glours closed this Jul 27, 2022
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.

Enable ssh option as build parameter for buildkit