chainguard-images / images

Public Chainguard Images
https://chainguard.dev/chainguard-images
Apache License 2.0
542 stars 141 forks source link

Set default value of `groupname` and `username` to `nonroot` #1672

Open Dentrax opened 11 months ago

Dentrax commented 11 months ago

_This is a follow-up proposal from a PR review: https://github.com/chainguard-images/images/pull/1624#discussion_r1364101948 by @imjasonh_

Idea is to set both of accounts.groups.groupname and accounts.users.username to nonroot. And enforce it by introducing a new rule in wolfictl.

We usually set those to name of the package. It'd be great to make those fields consistent across within other packages and some standardization. In distoless image, its also set to nonroot: https://github.com/GoogleContainerTools/distroless/blob/e040feb731bdd61f7f9795773d7fa430e5a9ca4a/examples/nonroot/BUILD#L15

imjasonh commented 11 months ago

+1 to any effort to make this more consistent, and ideally automatic with defaults and/or enforced with linting.

New image configs use the config pattern, where the config is exposed via TF, possibly reading the rest of the config from the YAML file, but possibly not -- cert-manager only has config/main.tf, for example: https://github.com/chainguard-images/images/blob/main/images/cert-manager/config/main.tf

This model makes it easier to share a standard accounts block, from tflib/accts -- this gives us a nonroot user (named nonroot) and lets callers override run-as, uid, name, etc.

I think migrating more modules to this pattern will help a lot with standardization.

The reason some user are named/ID'ed something else is to retain compatibility with upstream tools (usually Helm) that expect or require those details. Not all cases need this, but we sometimes get in the bad habit of copying other images' configs -- this is something that will be easier when we share a consistent accts module, and config pattern more broadly.

Now we don't know which images need a custom name, and which don't. On the bright side, we have good basic tests that should tell us when something is wrong.

tl;dr: let's move more things to the config module pattern, and use tflib/accts anywhere we can. If we change something that shouldn't be changed, we'll rely on our tests to tell us.