Skip to content

Change some of the Gardener components' base images to distroless #6159

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
Merged
merged 1 commit into from
Jun 23, 2022
Merged

Change some of the Gardener components' base images to distroless #6159

merged 1 commit into from
Jun 23, 2022

Conversation

dimityrmirchev
Copy link
Member

@dimityrmirchev dimityrmirchev commented Jun 22, 2022 •
edited by rfranzke
Loading

How to categorize this PR?

/area open-source security
/kind enhancement

What this PR does / why we need it:
This PR changes the base image to some of the Gardener's components from alpine to distroless. It changes the default user that is executing the binaries from root to a nonroot user. This will help reduce the attack surface of the images.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

  • tzdata is preinstalled in the distroless base image.
  • debian11 is hardcoded as a version so that we do not receive any unexpected changes when debian12 is released.
  • Gardenlet remains with alpine since it has a dependency to openvpn binary. I am not sure if this will still be needed when we deprecate the non reversed VPN.
  • Gardener API Server secure port is changed to 8443 so that it can be run as nonroot and avoid setting additional capabilities to that binary. See here for example.
  • There are several more usages of alpine in this repository but they will be separately evaluated.
  • Shells are removed from these images. For debugging purposes ephemeral containers can be used.

Release note:

`gardener-apiserver`, `gardener-controller-manager`, `gardener-scheduler`, `gardener-admission-controller`, `gardener-seed-admission-controller` and `gardener-resource-manager` are now using `gcr.io/distroless/static-debian11:nonroot` instead of versions of `alpine` as a base image.

@gardener-prow gardener-prow bot added area/open-source Open Source (community, enablement, contributions, conferences, CNCF, etc.) related area/security Security related kind/enhancement Enhancement, improvement, extension cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 22, 2022
@gardener-prow gardener-prow bot requested review from petersutter and timuthy June 22, 2022 09:21
@dimityrmirchev dimityrmirchev changed the title Change some Gardener components base image to distroless Change some of the Gardener components' base images to distroless Jun 22, 2022
@vpnachev
Copy link
Member

/lgtm

@gardener-prow gardener-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jun 22, 2022
@rfranzke
Copy link
Member

Thanks, nice PR!

Gardenlet remains with alpine since it has a dependency to openvpn binary. I am not sure if this will still be needed when we deprecate the non reversed VPN.

I think we still need if even with reversed VPN only.

Gardener API Server secure port is changed to 8443 so that it can be run as nonroot and avoid setting additional capabilities to that binary. See here for example.

Why did you decide to not adopt the same "fix" as in https://github.com/kubernetes/kubernetes/blob/f33ca2306548719e5116b53fccfc278bffb809a8/build/server-image/kube-apiserver/Dockerfile#L24?

@rfranzke rfranzke self-assigned this Jun 22, 2022
@vpnachev
Copy link
Member

Why did you decide to not adopt the same "fix" as in https://github.com/kubernetes/kubernetes/blob/f33ca2306548719e5116b53fccfc278bffb809a8/build/server-image/kube-apiserver/Dockerfile#L24?

Is there any hard requirement the gardener-apiserver to run on port 443? If not, the less privileges are assigned to the binary, the better the security is.

@dimityrmirchev
Copy link
Member Author

Why did you decide to not adopt the same "fix" as in https://github.com/kubernetes/kubernetes/blob/f33ca2306548719e5116b53fccfc278bffb809a8/build/server-image/kube-apiserver/Dockerfile#L24?

Yes, it is an alternative. In order to use the fix some code looking like this should be introduced in the Dockerfiles that are building the gardener-apiserver. If we do so then LD_LIBRARY_PATH will be disabled which is something that is of no concern I think. See here

RUN apt-get update && apt-get install -y libcap2-bin
RUN setcap cap_net_bind_service=+ep /gardener-apiserver

In the end my thinking was that we should not give any escalated permissions if we do not have a hard requirement to start this process on port lower than 1024.

@rfranzke
Copy link
Member

Is there any hard requirement the gardener-apiserver to run on port 443? If not, the less privileges are assigned to the binary, the better the security is.

I hope not, I cannot think of anything like it (and passing e2e tests are already a good indicator that it should be fine), but I cannot be 100% sure. 👀

OK, let's proceed then as suggested for now :) Thanks!

/lgtm
/approve

@gardener-prow
Copy link
Contributor

gardener-prow bot commented Jun 22, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rfranzke

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@gardener-prow gardener-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 22, 2022
@dimityrmirchev
Copy link
Member Author

I can follow up with a PR that will enable the configuration of this port in case this causes some troubles to someone. And if any hard requirements occur later on to run this service on port 443 we can adapt/adopt the "fix" in k/k

@rfranzke
Copy link
Member

Thanks @dimityrmirchev ❤️

@dimityrmirchev
Copy link
Member Author

/test pull-gardener-e2e-kind

1 similar comment
@dimityrmirchev
Copy link
Member Author

/test pull-gardener-e2e-kind

@gardener-prow gardener-prow bot merged commit 577c218 into gardener:master Jun 23, 2022
@ialidzhikov ialidzhikov mentioned this pull request Jun 24, 2022
Merged
@dimityrmirchev dimityrmirchev mentioned this pull request Jun 24, 2022
Merged
@vpnachev vpnachev mentioned this pull request Oct 10, 2022
Merged
@dimityrmirchev dimityrmirchev mentioned this pull request Dec 7, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@petersutter petersutter Awaiting requested review from petersutter

@timuthy timuthy Awaiting requested review from timuthy

Assignees

@rfranzke rfranzke

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/open-source Open Source (community, enablement, contributions, conferences, CNCF, etc.) related area/security Security related cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. kind/enhancement Enhancement, improvement, extension lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dimityrmirchev @vpnachev @rfranzke