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

Make our container image "distroless" #15

Merged
merged 6 commits into from
Nov 8, 2022
Merged

Make our container image "distroless" #15

merged 6 commits into from
Nov 8, 2022

Conversation

boxofrad
Copy link
Contributor

Switches our release base image to harden our security posture.

See here: https://github.com/GoogleContainerTools/distroless

@boxofrad boxofrad requested a review from a team as a code owner September 23, 2022 10:20
@boxofrad boxofrad added the pr/no-changelog This PR does not introduce a user-facing change that should be reflected in the changelog label Sep 23, 2022

# release-default release image
# -----------------------------------
FROM alpine:3.16 AS release-default
FROM gcr.io/distroless/base-debian11 AS release-default
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to use the (smaller) static-debian11 image, but unfortunately, Envoy dynamically links system libraries so this is not possible.

@david-yu
Copy link
Contributor

Thanks we need to wait on testing with Consul K8s before merging this in, this can be done after beta.

Comment on lines +252 to +270
// readServiceIDFromFile reads the service ID from the file specified by the
// -proxy-service-id-path flag.
//
// We do this here, rather than in the consuldp package's config handling,
// because this option only really makes sense as a CLI flag (and we handle
// all flag parsing here).
func readServiceIDFromFile() {
if serviceID == "" && serviceIDPath != "" {
id, err := os.ReadFile(serviceIDPath)
if err != nil {
log.Fatalf("failed to read given -proxy-service-id-path: %v", err)
}
serviceID = string(id)
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @ishustava @thisisnotashwin 👋🏻

Do you have an explanation of why consul-k8s needs this, rather than putting the service ID directly in the CLI flags, that I could put here?

Choose a reason for hiding this comment

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

In K8s, we often have a different container responsible for getting the service ID to begin with. Information across containers can only be shared via files on a shared volume. It makes it significantly easier to ready what is going on if we are dealing with filepath. Putting the service ID directly to the CLI would require reading the contents of that file anyway.

Comment on lines +256 to +261
// because this option only really makes sense as a CLI flag (and we handle
// all flag parsing here).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think it'd make sense to add this to the config file, what do you think?

@boxofrad boxofrad removed the pr/no-changelog This PR does not introduce a user-facing change that should be reflected in the changelog label Oct 17, 2022
Copy link
Collaborator

@riddhi89 riddhi89 left a comment

Choose a reason for hiding this comment

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

LGTM

@david-yu
Copy link
Contributor

david-yu commented Oct 21, 2022

Small suggestion, the distroless envoy container is actually envoyproxy/envoy-distroless:v1.24.0 the one you are currently copying from for a multi-stage build is from the ubuntu based envoy image.

@boxofrad
Copy link
Contributor Author

Thanks, @david-yu! I think the Envoy binary itself is actually the same in the distroless and Ubuntu-based images (as it's built in a previous step).

That said, we could certainly shave some time off our builds by using the distroless base 👍🏻

Copy link
Contributor

@david-yu david-yu left a comment

Choose a reason for hiding this comment

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

Thanks, I assume we can merge after the next Consul K8s beta and try it out?

@pglass pglass merged commit 23748fb into main Nov 8, 2022
@pglass pglass deleted the boxofrad/distroless branch November 8, 2022 18:02
pglass pushed a commit that referenced this pull request Nov 14, 2022
* Make our container image "distroless"
* config: make it possible to read service ID from file
* docs: document how to add a shell to our container image
* docker: upgrade Envoy to 1.24.0
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

Successfully merging this pull request may close these issues.

5 participants