search

rog-golang-buddies / golang-template-repository

Kickstarter repository for a golang service
Apache License 2.0
17 stars 7 forks source link

Improvements to Dockerfile #42

Closed mridulganga closed 2 years ago

mridulganga commented 2 years ago

The existing Dockerfile is good for a starter project, but for a template repo we should follow some best practices. I am including the below code for reference.

FROM golang:1.18 as builder

RUN apk update && apk upgrade && \
    apk add --no-cache make

WORKDIR /src
COPY . .

# get packages and build
RUN make tidy
RUN make build

# Using a distroless image from https://github.com/GoogleContainerTools/distroless
FROM gcr.io/distroless/static:nonroot

COPY --from=build /app/bin/app /

USER 65532:65532
CMD["/app"]

ENV PORT=8080
EXPOSE $PORT
mridulganga commented 2 years ago

Additionally we should also consider making use of golangci-lint which will allow us to add multiple linting/vet/fmt tools in its configuration.

haani-niyaz commented 2 years ago

@mridulganga thanks for raising this! A few thoughts...

RUN make tidy

I wouldn't do this inside the builder. To reference the docs:

go mod tidy ensures that the go.mod file matches the source code in the module. It adds any missing module requirements necessary to build the current module’s packages and dependencies, and it removes requirements on modules that don’t provide any relevant packages. It also adds any missing entries to go.sum and removes unnecessary entries.

This means, you'll have a working build however it does not represent the state of your go.mod and go.sum file in your repo, as it is dynamically fixed. IMO, it is better to have the build fail because of outdated module files than to have it transparently fixed, with the fixed state not represented in your source code.

ENV PORT=8080
EXPOSE $PORT

I originally debated this one because you may have outbound-only services that are not meant to be exposed e.g: queue worker cli etc. From a template generation perspective, it would make sense to opt-in for this though.

haani-niyaz commented 2 years ago

Additionally we should also consider making use of golangci-lint which will allow us to add multiple linting/vet/fmt tools in its configuration.

We already have golangci-lint as a github workflow and in the pre-commit hook setup.

mridulganga commented 2 years ago 
FROM golang:1.18.3-alpine3.16 as builder

RUN apk update && apk upgrade && \
    apk add --no-cache make

WORKDIR /src
COPY . .

# build executable
RUN make build

# Using a distroless image from https://github.com/GoogleContainerTools/distroless
FROM gcr.io/distroless/static:nonroot

COPY --from=build /app/bin/app /

USER 65532:65532
CMD["/app"]

ENV PORT=8080
EXPOSE $PORT

@haani-niyaz I have updated it as you suggested, additionally I also added golang:1.18.3-alpine3.16 image as the 1.18 golang image is massive 2GB+

I was not aware of golangci-lint as it wasn't mentioned in readme, perhaps we can plan and add it there too.

haani-niyaz commented 2 years ago

@mridulganga good pickup on the image size 👍. Feel free to raise a PR to fix the image tag.

I was not aware of golangci-lint as it wasn't mentioned in readme, perhaps we can plan and add it there too.

Yeah the README needs an updated. It is grossly outdated.

mridulganga commented 2 years ago

@rog-golang-buddies/platform-team @haani-niyaz I have created a PR with the changes we discussed in this issue and some additional minor changes (more info in PR description) https://github.com/rog-golang-buddies/golang-template-repository/pull/45