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

Arm64 build? #41

Closed
tiagoposse opened this issue Dec 3, 2020 · 9 comments
Closed

Arm64 build? #41

tiagoposse opened this issue Dec 3, 2020 · 9 comments

Comments

@tiagoposse
Copy link

Thanks for the amazing work with this extension!

Any chance you'll make an arm64 image available?

@jimsheldon
Copy link
Contributor

Thanks for the request @tiagoposse

We have added linux-arm64 support in v0.3.1, can you give it a try?

Thanks again!

@jimsheldon
Copy link
Contributor

I will close this out, if you find any problems with this image we can reopen this.

@tiagoposse
Copy link
Author

tiagoposse commented Feb 12, 2021

Hi @jimsheldon, apologies for the delay on testing this, always on the back of my mind but never really had time to test it. I can confirm that the main binary works, but trying to execute anything else inside the container fails.
Works:

root@master1:~# docker run -ti --rm meltwater/drone-convert-pathschanged:0.4.0
{"level":"fatal","msg":"missing secret key","time":"2021-02-12T09:27:43Z"}

Fails:

root@master1:~# docker run -ti --rm --entrypoint sh meltwater/drone-convert-pathschanged:0.4.0
standard_init_linux.go:211: exec user process caused "exec format error"

I'm running this on kubernetes and would love to copy a .env file into pwd. My current way of doing this would be executing a script as the entrypoint to the image, but this is failing due to the reason above. Would there be a better way to do this? I really miss a DRONE_ENV_FILE env var or so.

@jimsheldon
Copy link
Contributor

You found a bug @tiagoposse!

I ran this on my macbook pro:

$ docker run -ti --rm --entrypoint sh meltwater/drone-convert-pathschanged:0.4.0-linux-arm64
Unable to find image 'meltwater/drone-convert-pathschanged:0.4.0-linux-arm64' locally
0.4.0-linux-arm64: Pulling from meltwater/drone-convert-pathschanged
21c83c524219: Already exists
66205347a480: Pull complete
fbf185e11599: Pull complete
Digest: sha256:c753df6728b3deff178f2f937c106f0aa291b0b74ffc6c8ac3b5b66b2376e311
Status: Downloaded newer image for meltwater/drone-convert-pathschanged:0.4.0-linux-arm64
/ # apk add file
fetch http://dl-cdn.alpinelinux.org/alpine/v3.10/main/x86_64/APKINDEX.tar.gz
fetch http://dl-cdn.alpinelinux.org/alpine/v3.10/community/x86_64/APKINDEX.tar.gz
(1/2) Installing libmagic (5.37-r1)
(2/2) Installing file (5.37-r1)
Executing busybox-1.30.1-r3.trigger
OK: 11 MiB in 16 packages
/ # file /bin/drone-convert-pathschanged
/bin/drone-convert-pathschanged: ELF 64-bit LSB executable, ARM aarch64, version 1 (SYSV), statically linked, Go BuildID=P4UyAsezFydLhZ9DoyiS/6XclKVXfEkchm5Y2M0h2/ODgYFy1l-eEhezHntOh-/Cjro6c_dsV0QEGJfaCUt, not stripped
/ # file /bin/busybox
/bin/busybox: ELF 64-bit LSB pie executable, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib/ld-musl-x86_64.so.1, stripped

The drone-convert-pathschanged binary is arm64, but all other binaries in the image are amd64. I'm sure this is because we are building this project on amd64 hosts. It likely runs for you on arm64 because the entrypoint binary is arm64, but if you try to run any other binary (like /bin/sh) it fails.

We based our build process on https://github.com/drone/drone-convert-starlark which appears to have the same issue.

I will reopen this and work on a fix.

@jimsheldon jimsheldon reopened this Feb 12, 2021
@jimsheldon
Copy link
Contributor

@tiagoposse I have good news and bad news :)

The good news is I have a fix in #47, the bad news is it won't help your use case.

I think we should be building our images off the 'scratch' image, same as the docker runner (see Dockerfile.linux.arm64). For more info on the 'scratch' image see https://docs.docker.com/develop/develop-images/baseimages/#create-a-simple-parent-image-using-scratch.

This will increase the security for this docker image, since any potential exploit wouldn't have access to other binaries in the image (like /bin/sh).

What environment variables are you trying to pass to this plugin?

@tiagoposse
Copy link
Author

tiagoposse commented Feb 12, 2021

I'm using Hashicorp Vault to distribute secrets in the cluster and I want to include drone in this, which means I want to load the following variables from files:

  • DRONE_GITHUB_CLIENT_ID, DRONE_GITHUB_CLIENT_SECRET, DRONE_RPC_SECRET, DRONE_CONVERT_PLUGIN_SECRET to drone/drone
  • DRONE_RPC_SECRET to drone-runners/drone-runner-kube
  • DRONE_SECRET, TOKEN to meltwater/drone-convert-pathschanged

Drone supports this by running /bin/drone-server --env-file=<path>. (not supported by the offical helm chart).
Runner-kube supports this by running /bin/drone-runner-kube <path> (not supported by the offical helm chart).
Regarding this repo, I did not find such a use case. Since the base image was alpine, I could override the entrypoint in kubernetes and run sh -c "cp /vault/secrets/env /.env && /bin/drone-convert-pathschanged"

I fully support the move to scratch and will just deal with building upon the image locally if this behaviour is not supported.

Hope this is clear!

I also could not find a helm chart for this, is there one at all/is there one in the plans/is it open for contribution?

@jimsheldon
Copy link
Contributor

Interesting, so sh -c "cp /vault/secrets/env /.env && /bin/drone-convert-pathschanged" works for you with the current arm64 docker image? I'm not sure how the cp binary works when it is amd64...

Providing a --env-file option is definitely something we can do, I'll open a feature request for that.

A helm chart would be another good feature, I'll create something for that as well.

@tiagoposse
Copy link
Author

tiagoposse commented Feb 12, 2021

I built 0.4.0 locally, not the current official image.

Full of good news today :)

If you're interested in a PR for the chart at all, I have a working one that supports multiple use cases.

@jimsheldon
Copy link
Contributor

Version 0.5.0 has been pushed using 'scratch' for base images https://github.com/meltwater/drone-convert-pathschanged/releases/tag/v0.5.0

I created issues for the --env-file and helm chart features:

#48
#49

Feel free to subscribe to them and add any additional information.

Thanks for contributing! I'll close this out.

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

No branches or pull requests

2 participants