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

OCPBUGS-25662: Add Image Credential Provider flags for Kubelet on AWS #4103

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 28 additions & 0 deletions pkg/controller/template/render.go
Expand Up @@ -326,6 +326,7 @@ func renderTemplate(config RenderConfig, path string, b []byte) ([]byte, error)
funcs["skip"] = skipMissing
funcs["cloudProvider"] = cloudProvider
funcs["cloudConfigFlag"] = cloudConfigFlag
funcs["credentialProviderConfigFlag"] = credentialProviderConfigFlag
funcs["onPremPlatformAPIServerInternalIP"] = onPremPlatformAPIServerInternalIP
funcs["onPremPlatformAPIServerInternalIPs"] = onPremPlatformAPIServerInternalIPs
funcs["onPremPlatformIngressIP"] = onPremPlatformIngressIP
Expand Down Expand Up @@ -433,6 +434,33 @@ func cloudConfigFlag(cfg RenderConfig) interface{} {
}
}

// Process the {{credentialProviderConfigFlag .}}
// On supported platforms, this returns the `--image-credential-provider` flags for Kubelet.
// This will point to the bin dir containing the binaries and the appropriate config for
// the platform.
func credentialProviderConfigFlag(cfg RenderConfig) interface{} {
if cfg.Infra == nil {
cfg.Infra = &configv1.Infrastructure{
Status: configv1.InfrastructureStatus{},
}
}

if cfg.Infra.Status.PlatformStatus == nil {
cfg.Infra.Status.PlatformStatus = &configv1.PlatformStatus{
Type: "",
}
}

credentialProviderBinDirFlag := "--image-credential-provider-bin-dir=/usr/libexec/kubelet-image-credential-provider-plugins"
credentialProviderConfigFlag := "--image-credential-provider-config=/etc/kubernetes/credential-providers/"
switch cfg.Infra.Status.PlatformStatus.Type {
case configv1.AWSPlatformType:
return fmt.Sprintf("%s %s%s", credentialProviderBinDirFlag, credentialProviderConfigFlag, "ecr-credential-provider.yaml")
Copy link
Member

Choose a reason for hiding this comment

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

question (non-blocking): Would it make more sense to make ecr-credential-provider.yaml part of credentialProviderConfigFlag variable? In other words:

credentialProviderConfigFlag := "--image-credential-provider-config=/etc/kubernetes/credential-providers/ecr-credential-provider.yaml"

Then the return statement would simplify to:

case configv1.AWSPlatformType:
	return fmt.Sprintf("%s %s", credentialProviderBinDirFlag, credentialProviderConfigFlag)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the future, this switch will support more platforms.We will want those variables separate so they can be combined in different ways to get the correct flags for the different platforms, I'm expecting the config file name itself to be different per platform

Copy link
Member

Choose a reason for hiding this comment

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

Got it, thanks for the clarification!

default:
return ""
}
}

func onPremPlatformShortName(cfg RenderConfig) interface{} {
if cfg.Infra.Status.PlatformStatus != nil {
switch cfg.Infra.Status.PlatformStatus.Type {
Expand Down
63 changes: 63 additions & 0 deletions pkg/controller/template/render_test.go
Expand Up @@ -298,6 +298,69 @@ func TestCloudConfigFlag(t *testing.T) {
}
}

func TestCredentialProviderConfigFlag(t *testing.T) {
dummyTemplate := []byte(`{{credentialProviderConfigFlag .}}`)

testCases := []struct {
platform configv1.PlatformType
res string
}{
{
platform: configv1.AWSPlatformType,
res: "--image-credential-provider-bin-dir=/usr/libexec/kubelet-image-credential-provider-plugins --image-credential-provider-config=/etc/kubernetes/credential-providers/ecr-credential-provider.yaml",
},
{
platform: configv1.AzurePlatformType,
res: "",
},
{
platform: configv1.GCPPlatformType,
res: "",
},
{
platform: configv1.VSpherePlatformType,
res: "",
},
{
platform: configv1.OpenStackPlatformType,
res: "",
},
{
platform: configv1.BareMetalPlatformType,
res: "",
},
}

for idx, c := range testCases {
name := fmt.Sprintf("case #%d: %s", idx, c.platform)
t.Run(name, func(t *testing.T) {
config := &mcfgv1.ControllerConfig{
Spec: mcfgv1.ControllerConfigSpec{
Infra: &configv1.Infrastructure{
Status: configv1.InfrastructureStatus{
Platform: c.platform,
PlatformStatus: &configv1.PlatformStatus{
Type: c.platform,
},
},
},
},
}

fgAccess := featuregates.NewHardcodedFeatureGateAccess(nil, nil)

got, err := renderTemplate(RenderConfig{&config.Spec, `{"dummy":"dummy"}`, `{"dummy":"dummy"}`, fgAccess, nil}, name, dummyTemplate)
if err != nil {
t.Fatalf("expected nil error %v", err)
}

if string(got) != c.res {
t.Fatalf("mismatch got: %s want: %s", got, c.res)
}
})
}
}

func TestSkipMissing(t *testing.T) {
dummyTemplate := `{{skip "%s"}}`

Expand Down
@@ -0,0 +1,16 @@
mode: 0644
path: "/etc/kubernetes/credential-providers/ecr-credential-provider.yaml"
contents:
inline: |
apiVersion: kubelet.config.k8s.io/v1
kind: CredentialProviderConfig
providers:
- name: ecr-credential-provider
matchImages:
- "*.dkr.ecr.*.amazonaws.com"
- "*.dkr.ecr.*.amazonaws.com.cn"
- "*.dkr.ecr-fips.*.amazonaws.com"
- "*.dkr.ecr.us-iso-east-1.c2s.ic.gov"
- "*.dkr.ecr.us-isob-east-1.sc2s.sgov.gov"
defaultCacheDuration: "12h"
apiVersion: credentialprovider.kubelet.k8s.io/v1
Expand Up @@ -37,6 +37,7 @@ contents: |
--cloud-provider={{cloudProvider .}} \
--volume-plugin-dir=/etc/kubernetes/kubelet-plugins/volume/exec \
{{cloudConfigFlag . }} \
{{credentialProviderConfigFlag . }} \
--hostname-override=${KUBELET_NODE_NAME} \
--provider-id=${KUBELET_PROVIDERID} \
--register-with-taints=node-role.kubernetes.io/master=:NoSchedule \
Expand Down
Expand Up @@ -37,6 +37,7 @@ contents: |
--volume-plugin-dir=/etc/kubernetes/kubelet-plugins/volume/exec \
--cloud-provider={{cloudProvider .}} \
{{cloudConfigFlag . }} \
{{credentialProviderConfigFlag . }} \
--hostname-override=${KUBELET_NODE_NAME} \
--provider-id=${KUBELET_PROVIDERID} \
--pod-infra-container-image={{.Images.infraImageKey}} \
Expand Down