drone-docker icon indicating copy to clipboard operation
drone-docker copied to clipboard

Secrets enable in Docker plugin with support for passing environment variables as secrets

Open wszychta opened this issue 4 years ago • 11 comments

PR purpose

This PR is providing three new settings to the plugin:

  • secrets_from_env - Pass environment variables as secret file into the build
  • secrets - Provide full secrets paths from local filesystem
  • secret_separator - separator between each of the secrets variable

Also when any secrets or secrets_from_env are provided then environment variable DOCKER_BUILDKIT is set to 1. If nothing is provided then plugin willl not create/change that variable.

Warning

Because of urfave/cli library is not able to Slice Flags with different sign than , then we have introduced another variable called secret_separator. The content of this variable will be replaced with , on docker build command creation.

Example usage

.drone.yml:

- name: test Docker
  image: plugins/docker
  environment:
    TEST_VAR:
      from_secret: testvar
  settings:
    dry_run: true
    repo: foo/bar
    tags: latest
    dockerfile: Dockerfile
    secrets_from_env:
      - TEST_VAR
    secret_separator: ":"
    secrets: 
      - id=test-secret:src=test-file.txt
      - id=test-secret2:src=test-file2.txt

Dockerfile:

# syntax=docker/dockerfile:1.2
ARG IMAGE_VERSION=latest
FROM ubuntu:${IMAGE_VERSION}
RUN --mount=type=secret,id=test_var,dst=/root/test_var.txt cat /root/test_var.txt
RUN --mount=type=secret,id=test-secret,dst=/root/test-secret.txt cat /root/test-secret.txt
RUN --mount=type=secret,id=test-secret2,dst=/root/test-secret2.txt cat /root/test-secret2.txt

Other work to do

This PR requires update to drone docker plugin documentation

wszychta avatar Sep 16 '21 13:09 wszychta

👋 Hey there, while I appreciate the pull request, I recommend reading this thread: https://discourse.drone.io/t/drone-netrc-username-documentation-usage/4485/2

The netrc file is considered Internal use only and is subject to change. We are working to narrow the use of the netrc file and if we can find a suitable alternative, we may consider removing in the future. If you want to inject git credentials into this plugin it should be done using secrets.

In terms of using docker secrets, it might make more sense to model this behavior after build_args_from_env as opposed to reading values from a file. I recommend working with the maintainers to design the yaml syntax before writing additional code.

bradrydzewski avatar Sep 16 '21 13:09 bradrydzewski

@bradrydzewski I have modified code as you suggested. I have also modified my Pull Request description with new changes. Right now this Pull Request doesn't create netrc file automatically.

wszychta avatar Sep 17 '21 12:09 wszychta

The build is failing on go vet, that doesn't seem expected?

@bradrydzewski this looks like something we could use today, can you take another look to help get this merged in?

bkk-bcd avatar Sep 21 '21 13:09 bkk-bcd

Looks like the build container is using go 1.11 while go mod requires go 1.13.

Go 1.13 has os.UserHomeDir()

Not until go 1.16 does os.WriteFile() exists...maybe we can use https://pkg.go.dev/[email protected]#OpenFile instead or https://pkg.go.dev/io/ioutil (see https://golang.org/doc/go1.16#ioutil for explanation)

bkk-bcd avatar Sep 21 '21 13:09 bkk-bcd

I think a more thorough design review is going to be required and I want to be up front that this will take some time. We will assign this to a team member in an upcoming sprint and we will post updates to this thread when we have more details to share. We have a strong commitment to backward compatibility which means once this feature is merged it cannot easily be changed, so we need to make sure we are happy with the design and implementation.

bradrydzewski avatar Sep 21 '21 15:09 bradrydzewski

@bradrydzewski ok, I will wait until one of your teammates will review this PR in next sprint. Also I would suggest to change go version on vet to newer than 1.11. Locally I'm working on go 1.16 and I didn't have any problems with docker and ECR plugins, but I was unable to check other plugins.

wszychta avatar Sep 27 '21 11:09 wszychta

Any luck getting some time to review this PR?

alexef avatar Oct 25 '21 17:10 alexef

@alexef it looks like plugin maintainers upgraded plugin test to golang 1.13 If I will have time, then mayby I will try to downgrade golang to the same version and rewrite my PR code. @bradrydzewski Do you have any plans on reviving this PR?

wszychta avatar Oct 26 '21 09:10 wszychta

Looks like this could be closed now that #359 was merged.

alexef avatar Oct 19 '22 08:10 alexef

@alexef I'm not working for the company which was requesting this feature. I will contact somebody from them and ask for comment here.

wszychta avatar Oct 19 '22 09:10 wszychta

We will, test out the release https://github.com/drone-plugins/drone-docker/releases/tag/v20.12.0, thats is refered in here.

ghost avatar Oct 19 '22 09:10 ghost