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

Proposal/WIP: adding support for external cacerts.txt file #539

Closed

Conversation

roblg
Copy link

@roblg roblg commented Oct 10, 2018

Note: Work based on (and depends on) google/containerregistry#89

Addresses #273

Wanted to optimistically start the ball rolling on a possible implementation to take advantage of google/containerregistry#89 providing a --cacert flag. I don't believe this is a complete implementation.

Proposal: manage cacerts.txt like any other external dependency: in the WORKSPACE.

Non-working example for illustration:

# WORKSPACE
http_file(
    name='cacerts',
    urls=['https://example.com/cacerts.txt'],
    sha256='<sha256 here>',
)

container_pull(
    name = "favorite",
    registry = "private.myregistry.com",
    repository = "favorites/container",
    digest = "sha256:<digest>",
    cacerts = "@cacerts//file:downloaded",
)

I was able to make this work and resolve the issue described in #273 using this approach. (Due to container_pull dependency on a prebuilt puller artifact from google/containerregistry, in order to really make this work, you also have to separately bazel build //:puller.par in containerregistry and add it to your workspace as puller.)

I'm open to alternative suggestions as well, this just seemed like a reasonable and reasonably quick route to get from the changes proposed to containerregistry and me having a working build :)

Feedback welcome!

@nlopezgi
Copy link
Contributor

Thanks for sending this PR. Will look at it in detail next week (in a conference all this week), in the meantime, can you check out #531 and let us know if these two issues are related (seems to me) and we can have a conversation with @pcj to figure out how to address these use cases? Thanks!

@roblg
Copy link
Author

roblg commented Oct 11, 2018

@nlopezgi no worries; no rush on my end.

I checked out #531, and I'll want that functionality too, eventually :) It feels related, but separate from the issue I'm trying to tackle here. This PR is specifically about being able to override the (incomplete) cacerts.txt file that's bundled with httplib2 down in the depths of containerregistry with one that includes our root CA (or, in others' cases, their internal/private CA).

@nlopezgi
Copy link
Contributor

thanks for the clarifications. This does seem like it would be a useful feature. Is the cacerts something you envision setting for each container_pull rule or is this more of a global setting that you would want all your rules_docker rules to run with. If the latter, then I dont think this is the best approach. We need to create a toolchain rule for docker and this looks like a piece of information that could be stored there. In any case, you probably do need the upstream changes in containerregistry first. We plan on working on a toolchain rule for rules_docker soon, so I'll keep you posted on that.

@roblg
Copy link
Author

roblg commented Oct 15, 2018

Is the cacerts something you envision setting for each container_pull rule or is this more of a global setting that you would want all your rules_docker rules to run with.

Definitely would prefer a single, global setting. I wasn't sure how to achieve that with my bazel knowledge, but the toolchain thing sounds promising if it provides that. I'll go read about toolchains.

Going to close this since it seems like the fix will end up being radically different. Thanks for reviewing!

@roblg roblg closed this Oct 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants