Skip to content

Commit 30a98da

Browse files
authored
fix: CredentialProviderConfig matchImages to support registries with port (#724)
**What problem does this PR solve?**: While debugging another issue where I launched a local registry in a container at `https://172.18.0.10:5000`, I noticed that Deployments with an image in that registry failed: ``` Warning Failed 7m14s (x4 over 8m47s) kubelet Failed to pull image "172.18.0.10:5000/library/nginx:latest": failed to pull and unpack image "172.18.0.10:5000/library/nginx:latest": failed to resolve reference "172.18.0.10:5000/library/nginx:latest": pull access denied, repository does not exist or may require authorization: authorization failed: no basic auth credentials ``` This is because of a bug in how `matchImages` in `CredentialProviderConfig` was generated, the globs don't match images with a port or a path. This PR fixes that by explicitly including the user provided registries in `matchImages`. **Which issue(s) this PR fixes**: Fixes # **How Has This Been Tested?**: <!-- Please describe the tests that you ran to verify your changes. Provide output from the tests and any manual steps needed to replicate the tests. --> * Updated unit tests **Special notes for your reviewer**: <!-- Use this to provide any additional information to the reviewers. This may include: - Best way to review the PR. - Where the author wants the most review attention on. - etc. -->
1 parent 2183f8f commit 30a98da

File tree

3 files changed

+80
-11
lines changed

3 files changed

+80
-11
lines changed

pkg/handlers/generic/mutation/imageregistries/credentials/credential_provider_config_files.go

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -100,17 +100,15 @@ func templateFilesForImageCredentialProviderConfigs(
100100
) ([]cabpkv1.File, error) {
101101
var files []cabpkv1.File
102102

103-
kubeletCredentialProviderConfigFile, err := templateKubeletCredentialProviderConfig()
103+
kubeletCredentialProviderConfigFile, err := templateKubeletCredentialProviderConfig(configs)
104104
if err != nil {
105105
return nil, err
106106
}
107107
if kubeletCredentialProviderConfigFile != nil {
108108
files = append(files, *kubeletCredentialProviderConfigFile)
109109
}
110110

111-
kubeletDynamicCredentialProviderConfigFile, err := templateDynamicCredentialProviderConfig(
112-
configs,
113-
)
111+
kubeletDynamicCredentialProviderConfigFile, err := templateDynamicCredentialProviderConfig(configs)
114112
if err != nil {
115113
return nil, err
116114
}
@@ -121,14 +119,31 @@ func templateFilesForImageCredentialProviderConfigs(
121119
return files, nil
122120
}
123121

124-
func templateKubeletCredentialProviderConfig() (*cabpkv1.File, error) {
122+
func templateKubeletCredentialProviderConfig(
123+
configs []providerConfig,
124+
) (*cabpkv1.File, error) {
125125
providerBinary, providerArgs, providerAPIVersion := kubeletCredentialProvider()
126126

127+
// In addition to the globs already defined in the template, also include the user provided registries.
128+
//
129+
// This is needed to match registries with a port and/or a URL path.
130+
// From https://kubernetes.io/docs/tasks/administer-cluster/kubelet-credential-provider/#configure-image-matching
131+
registryHosts := make([]string, 0, len(configs))
132+
for _, config := range configs {
133+
registryHostWithPath, err := config.registryHostWithPath()
134+
if err != nil {
135+
return nil, err
136+
}
137+
registryHosts = append(registryHosts, registryHostWithPath)
138+
}
139+
127140
templateInput := struct {
141+
RegistryHosts []string
128142
ProviderBinary string
129143
ProviderArgs []string
130144
ProviderAPIVersion string
131145
}{
146+
RegistryHosts: registryHosts,
132147
ProviderBinary: providerBinary,
133148
ProviderArgs: providerArgs,
134149
ProviderAPIVersion: providerAPIVersion,

pkg/handlers/generic/mutation/imageregistries/credentials/credential_provider_config_files_test.go

Lines changed: 52 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,16 @@ func Test_templateKubeletCredentialProviderConfig(t *testing.T) {
1515
t.Parallel()
1616

1717
tests := []struct {
18-
name string
19-
want *cabpkv1.File
20-
wantErr error
18+
name string
19+
credentials []providerConfig
20+
want *cabpkv1.File
21+
wantErr error
2122
}{
2223
{
2324
name: "ECR image registry",
25+
credentials: []providerConfig{
26+
{URL: "https://123456789.dkr.ecr.us-east-1.amazonaws.com"},
27+
},
2428
want: &cabpkv1.File{
2529
Path: "/etc/kubernetes/image-credential-provider-config.yaml",
2630
Owner: "",
@@ -36,6 +40,7 @@ providers:
3640
- -c
3741
- /etc/kubernetes/dynamic-credential-provider-config.yaml
3842
matchImages:
43+
- "123456789.dkr.ecr.us-east-1.amazonaws.com"
3944
- "*"
4045
- "*.*"
4146
- "*.*.*"
@@ -49,6 +54,45 @@ providers:
4954
},
5055
{
5156
name: "image registry with static config",
57+
credentials: []providerConfig{{
58+
URL: "https://myregistry.com:5000/myproject",
59+
Username: "myuser",
60+
Password: "mypassword",
61+
}},
62+
want: &cabpkv1.File{
63+
Path: "/etc/kubernetes/image-credential-provider-config.yaml",
64+
Owner: "",
65+
Permissions: "0600",
66+
Encoding: "",
67+
Append: false,
68+
Content: `apiVersion: kubelet.config.k8s.io/v1
69+
kind: CredentialProviderConfig
70+
providers:
71+
- name: dynamic-credential-provider
72+
args:
73+
- get-credentials
74+
- -c
75+
- /etc/kubernetes/dynamic-credential-provider-config.yaml
76+
matchImages:
77+
- "myregistry.com:5000/myproject"
78+
- "*"
79+
- "*.*"
80+
- "*.*.*"
81+
- "*.*.*.*"
82+
- "*.*.*.*.*"
83+
- "*.*.*.*.*.*"
84+
defaultCacheDuration: "0s"
85+
apiVersion: credentialprovider.kubelet.k8s.io/v1
86+
`,
87+
},
88+
},
89+
{
90+
name: "docker.io registry with static credentials",
91+
credentials: []providerConfig{{
92+
URL: "https://registry-1.docker.io",
93+
Username: "myuser",
94+
Password: "mypassword",
95+
}},
5296
want: &cabpkv1.File{
5397
Path: "/etc/kubernetes/image-credential-provider-config.yaml",
5498
Owner: "",
@@ -64,6 +108,8 @@ providers:
64108
- -c
65109
- /etc/kubernetes/dynamic-credential-provider-config.yaml
66110
matchImages:
111+
- "registry-1.docker.io"
112+
- "docker.io"
67113
- "*"
68114
- "*.*"
69115
- "*.*.*"
@@ -80,7 +126,7 @@ providers:
80126
tt := tests[idx]
81127
t.Run(tt.name, func(t *testing.T) {
82128
t.Parallel()
83-
file, err := templateKubeletCredentialProviderConfig()
129+
file, err := templateKubeletCredentialProviderConfig(tt.credentials)
84130
require.ErrorIs(t, err, tt.wantErr)
85131
assert.Equal(t, tt.want, file)
86132
})
@@ -127,7 +173,7 @@ credentialProviders:
127173
{
128174
name: "image registry with static credentials",
129175
credentials: []providerConfig{{
130-
URL: "https://myregistry.com",
176+
URL: "https://myregistry.com:5000/myproject",
131177
Username: "myuser",
132178
Password: "mypassword",
133179
}},
@@ -148,7 +194,7 @@ credentialProviders:
148194
args:
149195
- /etc/kubernetes/static-image-credentials.json
150196
matchImages:
151-
- "myregistry.com"
197+
- "myregistry.com:5000/myproject"
152198
defaultCacheDuration: "0s"
153199
apiVersion: credentialprovider.kubelet.k8s.io/v1
154200
`,

pkg/handlers/generic/mutation/imageregistries/credentials/templates/kubelet-image-credential-provider-config.yaml.gotmpl

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,14 @@ providers:
99
{{- end }}
1010
{{- end }}
1111
matchImages:
12+
{{- range .RegistryHosts}}
13+
{{- with . }}
14+
- {{ printf "%q" . }}
15+
{{- if eq . "registry-1.docker.io" }}
16+
- "docker.io"
17+
{{- end }}
18+
{{- end }}
19+
{{- end }}
1220
- "*"
1321
- "*.*"
1422
- "*.*.*"

0 commit comments

Comments
 (0)