-
Notifications
You must be signed in to change notification settings - Fork 349
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
Add Dockerfile for building cloud-sql-proxy images. #296
Conversation
Dockerfile
Outdated
WORKDIR /go/src/cloudsql-proxy | ||
COPY . . | ||
|
||
RUN go get ./cmd/cloud_sql_proxy && go build -ldflags "-X 'main.versionString=$VERSION'" \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be safer if you don't rely on the libc
from the distroless image, and use Go’s static compilation flags.
Most of the time people use this to truly statically compile Go binaries:
go build -a -tags netgo -ldflags '-w -extldflags "-static"' ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why's it safer? The glibc dependency is the most tested and most used in production. Unless there's a concrete reason not to use it, I'd go with the defaults.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only reason would be if someone tries to use this binary like:
FROM this_image AS dep
FROM their_alpine
COPY -from=dep /this_binary /bin
And libc would cause issues
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to revert for now - it's probably safer for users to dl the binary from one of the links over pulling it out of this image.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM after ahmets comments are resolved
Dockerfile
Outdated
# Build Stage | ||
FROM golang:1 as build | ||
|
||
ARG VERSION="1.14-develop" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does this work? Can we please instead read the version from the git repo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So right now the git repo isn't using any version, it has to be handed a version when it's compiled (see here)
This changes it so the argument can be passed in during the docker build command, and has a reasonable default so if compiled by hand it lets us know.
Addresses #286