Skip to content

Conversation

@sebimarkgraf
Copy link

Closely related to apple/container#305 I would like to override the used SSL TrustRoots via standard env variables.

This here would add this configuration and would give an entrypoint for an implementation of 305 to provide CLI flags or similar.

This has no tests yet, as this would require setting up something like a MITM proxy when testing against a registry.
As I am unfamiliar with the codebase, I would be willing to do this, but would require a first nudge on where to best implement this.

To actually use this, we would need to add the allowed env variables to the container system start command env filter.

}
}

func makeEnvironmentAwareTLSConfiguration() -> TLSConfiguration {
Copy link
Member

@dcantah dcantah Nov 12, 2025

Choose a reason for hiding this comment

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

I wonder if RegistryClient's constructor should just take in a TLSConfiguration and container just copies this logic you have here and supplies the TLSConfiguration we crafted from the envvars. An alternative would be to take in an enum that the cases could be .fromEnvironment, .custom(TLSConfiguration) and so on. I'm leaning towards the first (adding tlsConfiguration as a parameter) as it's the most flexible

Copy link
Author

Choose a reason for hiding this comment

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

Probably sensible to have the first approach:
Otherwise this enum would at some point repeat most of the options already covered in TLSConfiguration itself.
Also this could include a --skip-ca-verification cli flag that just allows skipping as a temporary workaround.

Also, if I move this up one level it could be easier to extract and merge it somewhere upstream in a more refined way.

Copy link
Author

Choose a reason for hiding this comment

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

But, this would mean that both container in LoginCommand and containerization in ImageStore need to instantiate a TLSConfig and hand it to the RegistryClient.

Copy link
Author

Choose a reason for hiding this comment

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

Reworked the RegistryClient to just take the TLSConfiguration.

Also moved the makeEnvironmentAwareTLSConfiguration as we already need it twice in containerization

Copy link
Member

Choose a reason for hiding this comment

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

Thanks!! Reviewing

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.

2 participants