fix: unable to pull images from private ECR #67
Conversation
- this started happening since v0.6.0 release of warm metal driver
| @@ -127,7 +127,7 @@ func (s keyringStore) GetDockerKeyring(ctx context.Context, secretData map[strin | |||
| return credentialprovider.UnionDockerKeyring{preferredKeyring, daemonKeyring}, nil | |||
| } | |||
|
|
|||
| return daemonKeyring, err | |||
| return credentialprovider.UnionDockerKeyring{daemonKeyring, credentialprovider.NewDockerKeyring()}, err | |||
There was a problem hiding this comment.
NewDockerKeyring enables authentication with cloudproviders like AWS, Azure and GCP. This was present in v0.5.1 but was removed in v0.6.0 (maybe we missed adding NewDockerKeyring in v0.6.0?).
You can check the diff between v0.5.1 and v0.6.0 here: v0.5.1...v0.6.0#diff-ded1b59e8d55d0b1494a34a4d36b7d3711e552252fcca2a223177373ef8f0f5eL48 (search for NewDockerKeyring)
There was a problem hiding this comment.
NewDockerKeyring is important because it creates providersDockerKeyring
// NewDockerKeyring creates a DockerKeyring to use for resolving credentials,
// which draws from the set of registered credential providers.
func NewDockerKeyring() DockerKeyring {
keyring := &providersDockerKeyring{
Providers: make([]DockerConfigProvider, 0),
}providersDockerKeyring supports RegisterCredentialProvider (note that providers is a package/global variable)
// RegisterCredentialProvider is called by provider implementations on
// initialization to register themselves, like so:
//
// func init() {
// RegisterCredentialProvider("name", &myProvider{...})
// }
func RegisterCredentialProvider(name string, provider DockerConfigProvider) {
providersMutex.Lock()
defer providersMutex.Unlock()
_, found := providers[name]
if found {
klog.Fatalf("Credential provider %q was registered twice", name)
}
klog.V(4).Infof("Registered credential provider %q", name)
providers[name] = provider
}RegisterCredentialProvider is called by imports in secrets/cache.go
File: pkg/secret/cache.go
17: // register credential providers
18: _ "k8s.io/kubernetes/pkg/credentialprovider/aws"
19: _ "k8s.io/kubernetes/pkg/credentialprovider/azure"
20: _ "k8s.io/kubernetes/pkg/credentialprovider/gcp"If you check the code for initialisation of "k8s.io/kubernetes/pkg/credentialprovider/aws", it calls RegisterCredentialProvider in init()
// init registers a credential provider for each registryURLTemplate and creates
// an ECR token getter factory with a new cache to store token getters
func init() {
credentialprovider.RegisterCredentialProvider("amazon-ecr",
newECRProvider(&ecrTokenGetterFactory{cache: make(map[string]tokenGetter)},
ec2ValidationImpl,
))
}During lookup, providersDockerKeyring's Lookup function is called (line 33)
https://github.com/warm-metal/csi-driver-image/blob/28f7e80fb73dc617b707741b719731febf02d13c/pkg/remoteimage/pull.go#L30-L34
providersDockerKeyring's Lookup function calls Provide function to get credentials from the AWS provider
File: kubernetes@v1.25.2/pkg/credentialprovider/keyring.go
263: // Lookup implements the DockerKeyring method for fetching credentials
264: // based on image name.
265: func (dk *providersDockerKeyring) Lookup(image string) ([]AuthConfig, bool) {
266: keyring := &BasicDockerKeyring{}
267:
268: for _, p := range dk.Providers {
269: keyring.Add(p.Provide(image))
270: }
271:
272: return keyring.Lookup(image)
273: }Basically, warm metal CSI driver is able to fetch images from private ECR registry because:
NewDockerRegistry -> uses providersDockerKeyring -> imports like k8s.io/kubernetes/pkg/credentialprovider/aws call providersDockerKeyring's RegisterCredentialProvider function to register -> warm metal calls providersDockerKeyring to get docker config -> providersDockerKeyring calls k8s.io/kubernetes/pkg/credentialprovider/aws package's Provide function which gets private ECR credentials from AWS
There was a problem hiding this comment.
Sorry, my explanation is complicated.
There was a problem hiding this comment.
Note that I have made the change in GetDockerKeyring function of keyringStore because it is called while pulling image
https://github.com/warm-metal/csi-driver-image/blob/5a3391102513c547576bc6dcf59663951c431e1f/cmd/plugin/node_server.go#L99
And it is used both with cache enabled and without cache enabled.
with cache enabled (line 216)
without cache enabled
vadasambar
changed the title
vadasambar
changed the title
|
I tried testing the code in this PR but the warm metal CSI driver pod goes in
I think this is because of #57 I created a new branch (on my local machine) by cherry-picking the commit (from this PR) to |
|
@kitt1987 would it be possible to backport this change to v0.6.0 and v0.6.1 so that these releases have the bug fix as well. 🙏 cc: @mugdha-adhav I have basically tested the PR with v0.6.1 and it works. We need to test it once with |
I can cherry-pick this fix to v0.6.1, then file a new release v0.6.2. Does it work for you? |
The latest version introduced a new Deployment to support dynamic provisioning. It can only be installed through Helm currently. The install utility can be removed from the next release. |
Yep, that would be great. Note: We are currently working on testing the changes on master branch. Once we are done, we'll send the PR out for review. |
Like @mugdha-adhav said, that works great! and thank you 🙏 ❤️ ! |
kitt1987
changed the title
…ndys shit Signed-off-by: Mriyam Tamuli <mbtamuli@gmail.com>
Fixes #64