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

feat: Add option to config to set runtime socket address #831

Closed
wants to merge 5 commits into from

Conversation

pmengelbert
Copy link
Contributor

What this PR does / why we need it:

Which issue(s) this PR fixes (optional, using fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when the PR gets merged):
Fixes #647

Copy link
Member

Choose a reason for hiding this comment

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

why are we adding a new api version

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 wasn't sure if it was merited here or not. I don't mind adding the changes to v1alpha2 instead

Copy link
Member

Choose a reason for hiding this comment

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

that should be fine as long as we default if users don't specify. wdyt?

Copy link
Contributor Author

@pmengelbert pmengelbert Aug 15, 2023

Choose a reason for hiding this comment

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

v1alpha3 defaults to containerd and the default containerd socket address. If it's a version prior to v1alpha3 and something other than containerd is specified, then it'll use the default socket address for that runtime.

@pmengelbert pmengelbert force-pushed the feat/containerd.sock branch from bf42518 to bf83254 Compare August 15, 2023 13:49
This change will enable us to specify the runtime socket address. The
values do not propagate yet; this just introduces the necessary
backward-compatible struct.

Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
v1alpha1 -> unversioned
unversioned -> v1alpha1
v1alpha2 -> unversioned
unversioned -> v1alpha2

All of these needed ManagerConfig conversions because we are now
allowing for the specification of the `runtimeSocketAddress`, and not
just the name of the `runtime`.

Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
`runtimeConfig.manager.runtimeSocketAddress` can now be specified.
`runtimeConfig.manager.runtime` is now deprecated.

Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
Because trivy's behavior switches on the name of the runtime, it has to
be supplied at the time trivy runs.

To specify the address, there are 3 different environment variables that
trivy will check in order to find the socket, depending on the runtime.

In the case of the crio runtime, an exact address cannot be specified
(or rather, trivy won't understand it). Trivy will always look for the
podman address at $XDG_RUNTIME_DIR/podman/podman.sock . While the value
of XDG_RUNTIME_DIR can be changed, the exact socket address can't be.

Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
@pmengelbert pmengelbert force-pushed the feat/containerd.sock branch from bf83254 to cdeaefd Compare August 15, 2023 16:41
@pmengelbert
Copy link
Contributor Author

Superseded by #930

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.

Make containerd.sock configurable in your helm charts
2 participants