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.

Sign up for GitHub

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

Jump to bottom

Use distroless image for operator image #533

Merged
merged 6 commits into from
Mar 14, 2023
Merged

Use distroless image for operator image #533

merged 6 commits into from
Mar 14, 2023

Conversation

AKamyshnikova
Copy link
Contributor

To avoid security issues switch to usage of distroless image.
Also bumped go version to 1.19

Fixes: #508

@AKamyshnikova AKamyshnikova force-pushed the distroless branch 2 times, most recently from 7922a54 to a25aef4 Compare February 24, 2023 12:29
@codecov
Copy link

codecov bot commented Feb 24, 2023 •
edited

Codecov Report

Patch and project coverage have no change.

Comparison is base (a4a345f) 85.12% compared to head (3590248) 85.12%.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master     #533   +/-   ##
=======================================
  Coverage   85.12%   85.12%           
=======================================
  Files          12       12           
  Lines        1613     1613           
=======================================
  Hits         1373     1373           
  Misses        155      155           
  Partials       85       85

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

aignatov
aignatov reviewed Feb 24, 2023
View reviewed changes
Dockerfile Outdated Show resolved Hide resolved
@AKamyshnikova
Use distroless image for operator image
9706b5f 
To avoid security issues switch to usage of distroless image.
Also bumped go version to 1.19

Fixes: pravega#508
Signed-off-by: Ann Taraday <akamyshnikova@mirantis.com>
anishakj
anishakj reviewed Feb 28, 2023
View reviewed changes
Dockerfile

ARG PROJECT_NAME=zookeeper-operator

COPY --from=go-builder /src/${PROJECT_NAME} /usr/local/bin/${PROJECT_NAME}

RUN adduser -D ${PROJECT_NAME}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AKamyshnikova why are we removing the user

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anishakj For distrolless image we cannot run adduser command. I will add update this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anishakj Seems this setting is no longer required.

@AKamyshnikova
Add user setting in Dockerfile
cabf888 
Signed-off-by: Ann Taraday <akamyshnikova@mirantis.com>
@AKamyshnikova
Revert setting USER in Dockerfile
d7e43b5 
Signed-off-by: Ann Taraday <akamyshnikova@mirantis.com>
@anishakj anishakj requested a review from derekm March 9, 2023 16:16
derekm
derekm requested changes Mar 9, 2023
View reviewed changes
Dockerfile Outdated
@@ -28,14 +28,10 @@ COPY controllers/ controllers/
RUN GOOS=linux GOARCH=amd64 CGO_ENABLED=0 go build -o /src/${PROJECT_NAME} \
-ldflags "-X ${REPO_PATH}/pkg/version.Version=${VERSION} -X ${REPO_PATH}/pkg/version.GitSHA=${GIT_SHA}" main.go

FROM ${DOCKER_REGISTRY:+$DOCKER_REGISTRY/}alpine:${ALPINE_VERSION} AS final

FROM gcr.io/distroless/static:nonroot AS final
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently available images are: https://github.com/GoogleContainerTools/distroless#what-images-are-available

That is, maybe we should use gcr.io/distroless/static-debian11 directly instead of gcr.io/distroless/static. I think static is a link to the latest build static-debian11, but we should use the latest build explicitly, so that we have a reproducible build and so we don't auto-upgrade images when the static link changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@AKamyshnikova
Use cr.io/distroless/static-debian11
4a455cb 
Signed-off-by: Ann Taraday <akamyshnikova@mirantis.com>
@AKamyshnikova AKamyshnikova requested review from derekm and removed request for jkhalack March 10, 2023 10:43
jkhalack
jkhalack suggested changes Mar 10, 2023
View reviewed changes
Dockerfile Outdated
@@ -28,14 +28,10 @@ COPY controllers/ controllers/
RUN GOOS=linux GOARCH=amd64 CGO_ENABLED=0 go build -o /src/${PROJECT_NAME} \
-ldflags "-X ${REPO_PATH}/pkg/version.Version=${VERSION} -X ${REPO_PATH}/pkg/version.GitSHA=${GIT_SHA}" main.go

FROM ${DOCKER_REGISTRY:+$DOCKER_REGISTRY/}alpine:${ALPINE_VERSION} AS final

FROM gcr.io/distroless/static-debian11:nonroot AS final
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep the option to use DOCKER_REGISTRY arg

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DOCKER_REGISTRY is different for golang build image and distroless final. I will introduce DISTROLESS_DOCKER_REGISTRY.

@AKamyshnikova
Introduce DISTROLESS_DOCKER_REGISTRY arg
864ecb9 
Signed-off-by: Ann Taraday <akamyshnikova@mirantis.com>
anishakj
anishakj reviewed Mar 10, 2023
View reviewed changes
Makefile Outdated
@@ -23,6 +23,7 @@ GIT_SHA=$(shell git rev-parse --short HEAD)
TEST_IMAGE=$(TEST_REPO)-testimages:$(VERSION)
DOCKER_TEST_PASS=testzkop@123
DOCKER_TEST_USER=testzkop
DISTROLESS_DOCKER_REGISTRY=gcr.io
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AKamyshnikova is this line needed, if we not mentioned this registry it will take gcr.io as default rt

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it can be dropped - I forgot to drop after I add default in Dockerfile

@AKamyshnikova
Drop DISTROLESS_DOCKER_REGISTRY from Makefile
3590248 
Signed-off-by: Ann Taraday <akamyshnikova@mirantis.com>
anishakj
anishakj approved these changes Mar 13, 2023
View reviewed changes
Copy link
Contributor

@anishakj anishakj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

derekm
derekm approved these changes Mar 13, 2023
View reviewed changes
Copy link
Contributor

@derekm derekm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

It might make sense to extract a distroless version like there is an alpine version, but this also looks great as-is.

@anishakj anishakj merged commit 3d3e647 into pravega:master Mar 14, 2023
@cheleb cheleb mentioned this pull request Apr 15, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derekm derekm derekm approved these changes

@aignatov aignatov aignatov approved these changes

@jkhalack jkhalack jkhalack approved these changes

@anishakj anishakj anishakj approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Critical CVE-2022-37434 in zookeeper-operator image
5 participants
@AKamyshnikova @derekm @aignatov @jkhalack @anishakj