cri: pass pull image credentials to snapshotter

[bergwolf]

Pass them via annotations so that a snapshotter can fetch image contents
with proper credentials. It is needed for remote snapshotters like
estargz and nydus. While it is possible for the snapshotters to parse
credentials on their own, but it several drawbacks:

1. snapshotter needs to access containerd config file
2. snapshotter needs to handle different credential sources that are
   already handled by kubelet/containerd
3. snapshotter needs additional kubernetes privileges to access
   kubernetes secrets APIs

By passing image pulling credentials to snapshotters, we can avoid the
above drawbacks. The behavior is still toggled by the current
disable_snapshot_annotations option and is disabled by default.

Fixes: #5105

/cc @ktock @ruiwen-zhao @alakesh

[k8s-ci-robot]

@bergwolf: GitHub didn't allow me to request PR reviews from the following users: ruiwen-zhao, alakesh.

Note that only containerd members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

Pass them via annotations so that a snapshotter can fetch image contents
with proper credentials. It is needed for remote snapshotters like
estargz and nydus. While it is possible for the snapshotters to parse
credentials on their own, but it several drawbacks:

1. snapshotter needs to access containerd config file
2. snapshotter needs to handle different credential sources that are
   already handled by kubelet/containerd
3. snapshotter needs additional kubernetes privileges to access
   kubernetes secrets APIs

By passing image pulling credentials to snapshotters, we can avoid the
above drawbacks. The behavior is still toggled by the current
disable_snapshot_annotations option and is disabled by default.

Fixes: #5105

/cc @ktock @ruiwen-zhao @alakesh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[k8s-ci-robot]

Hi @bergwolf. Thanks for your PR.

I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[theopenlab-ci]

Build succeeded.

• containerd-build-arm64 : SUCCESS in 14m 27s (non-voting)
• containerd-test-arm64 : SUCCESS in 5m 58s (non-voting)
• containerd-integration-test-arm64 : FAILURE in 24m 45s (non-voting)

[kzys]

It is a scary path to take to be honest. Have we put credentials in labels before? Wouldn't it leak credentials to logs?

[bergwolf]

@kzys I don't think we've done it before. You are right that we need to audit the code to make sure we never print snapshot annotations in any logs.

[cpuguy83]

We would be storing creds in plaint text on disk, I think?
I don't like this solution.

[ruiwen-zhao]

I guess my major concerns are

1. By adding the cred to labels, which are stored in bolt db, we are essentially storing the creds on disk.
2. People might not be aware that the labels contain sensitive info and might be logging or mishandling them (we might be doing it today I am not sure)

I guess a more preferred way is what is proposed here: #5105

[k8s-ci-robot]

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.