From 46b42767d207a981f158507a018ddd1b0e051185 Mon Sep 17 00:00:00 2001 From: Bryce Soghigian <49734722+Bryce-Soghigian@users.noreply.github.com> Date: Wed, 17 Jul 2024 11:11:31 -0700 Subject: [PATCH] fix(bootstrap): OOT credential provider support in 1.30+ (#429) * test: 1.30.0 * fix: credential provider URL * test: associated tests * chore: remove unused function * fix: minor unrelated log fix * chore: undo AKS version choice --------- Co-authored-by: tallaxes <18728999+tallaxes@users.noreply.github.com> --- .../imagefamily/bootstrap/aksbootstrap.go | 28 +++++++++-- .../bootstrap/aksbootstrap_test.go | 50 +++++++++++++++++++ pkg/providers/instance/instance.go | 2 +- pkg/providers/instancetype/suite_test.go | 11 ++-- pkg/utils/utils.go | 4 -- 5 files changed, 80 insertions(+), 15 deletions(-) diff --git a/pkg/providers/imagefamily/bootstrap/aksbootstrap.go b/pkg/providers/imagefamily/bootstrap/aksbootstrap.go index f4e9b2260..c064a8f7b 100644 --- a/pkg/providers/imagefamily/bootstrap/aksbootstrap.go +++ b/pkg/providers/imagefamily/bootstrap/aksbootstrap.go @@ -420,6 +420,28 @@ func kubeBinaryURL(kubernetesVersion, cpuArch string) string { return fmt.Sprintf("%s/kubernetes/v%s/binaries/kubernetes-node-linux-%s.tar.gz", globalAKSMirror, kubernetesVersion, cpuArch) } +// CredentialProviderURL returns the URL for OOT credential provider, +// or an empty string if OOT provider is not to be used +func CredentialProviderURL(kubernetesVersion, arch string) string { + minorVersion := semver.MustParse(kubernetesVersion).Minor + if minorVersion < 30 { // use from 1.30; 1.29 supports it too, but we have not fully tested it with Karpenter + return "" + } + + // credential provider has its own release outside of k8s version, and there'll be one credential provider binary for each k8s release, + // as credential provider release goes with cloud-provider-azure, not every credential provider release will be picked up unless + // there are CVE or bug fixes. + credentialProviderVersion := "1.29.2" + switch minorVersion { + case 29: + credentialProviderVersion = "1.29.2" + case 30: + credentialProviderVersion = "1.30.0" + } + + return fmt.Sprintf("%s/cloud-provider-azure/v%s/binaries/azure-acr-credential-provider-linux-%s-v%s.tar.gz", globalAKSMirror, credentialProviderVersion, arch, credentialProviderVersion) +} + func (a AKS) applyOptions(nbv *NodeBootstrapVariables) { nbv.KubeCACrt = *a.CABundle nbv.APIServerName = a.APIServerName @@ -464,9 +486,9 @@ func (a AKS) applyOptions(nbv *NodeBootstrapVariables) { }), ",") // Assign Per K8s version kubelet flags - minorVersion := semver.MustParse(a.KubernetesVersion).Minor - if utils.UseOOTCredential(minorVersion) { - nbv.CredentialProviderDownloadURL = fmt.Sprintf("https://acs-mirror.azureedge.net/cloud-provider-azure/%s/binaries/azure-acr-credential-provider-linux-amd64-v%s.tar.gz", nbv.KubernetesVersion, nbv.KubernetesVersion) + credentialProviderURL := CredentialProviderURL(a.KubernetesVersion, a.Arch) + if credentialProviderURL != "" { // use OOT credential provider + nbv.CredentialProviderDownloadURL = credentialProviderURL kubeletFlagsBase["--image-credential-provider-config"] = "/var/lib/kubelet/credential-provider-config.yaml" kubeletFlagsBase["--image-credential-provider-bin-dir"] = "/var/lib/kubelet/credential-provider" } else { // Versions Less than 1.30 diff --git a/pkg/providers/imagefamily/bootstrap/aksbootstrap_test.go b/pkg/providers/imagefamily/bootstrap/aksbootstrap_test.go index 3c501e8fa..ec0cb423f 100644 --- a/pkg/providers/imagefamily/bootstrap/aksbootstrap_test.go +++ b/pkg/providers/imagefamily/bootstrap/aksbootstrap_test.go @@ -58,3 +58,53 @@ func TestKubeBinaryURL(t *testing.T) { }) } } + +func TestGetCredentialProviderURL(t *testing.T) { + tests := []struct { + version string + arch string + url string + }{ + { + version: "1.30.2", + arch: "amd64", + url: fmt.Sprintf("%s/cloud-provider-azure/v1.30.0/binaries/azure-acr-credential-provider-linux-amd64-v1.30.0.tar.gz", globalAKSMirror), + }, + { + version: "1.30.0", + arch: "amd64", + url: fmt.Sprintf("%s/cloud-provider-azure/v1.30.0/binaries/azure-acr-credential-provider-linux-amd64-v1.30.0.tar.gz", globalAKSMirror), + }, + { + version: "1.30.0", + arch: "arm64", + url: fmt.Sprintf("%s/cloud-provider-azure/v1.30.0/binaries/azure-acr-credential-provider-linux-arm64-v1.30.0.tar.gz", globalAKSMirror), + }, + { + version: "1.29.2", + arch: "amd64", + url: "", + }, + { + version: "1.29.0", + arch: "amd64", + url: "", + }, + { + version: "1.29.0", + arch: "arm64", + url: "", + }, + { + version: "1.28.7", + arch: "amd64", + url: "", + }, + } + for _, tt := range tests { + url := CredentialProviderURL(tt.version, tt.arch) + if url != tt.url { + t.Errorf("for version %s expected %s, got %s", tt.version, tt.url, url) + } + } +} diff --git a/pkg/providers/instance/instance.go b/pkg/providers/instance/instance.go index dd1fdc74d..4c2ea355f 100644 --- a/pkg/providers/instance/instance.go +++ b/pkg/providers/instance/instance.go @@ -178,7 +178,7 @@ func (p *Provider) List(ctx context.Context) ([]*armcompute.VirtualMachine, erro } func (p *Provider) Delete(ctx context.Context, resourceName string) error { - logging.FromContext(ctx).Debugf("Deleting virtual machine %s and associated resources") + logging.FromContext(ctx).Debugf("Deleting virtual machine %s and associated resources", resourceName) return p.cleanupAzureResources(ctx, resourceName) } diff --git a/pkg/providers/instancetype/suite_test.go b/pkg/providers/instancetype/suite_test.go index 66d2879db..6e03d42de 100644 --- a/pkg/providers/instancetype/suite_test.go +++ b/pkg/providers/instancetype/suite_test.go @@ -27,7 +27,6 @@ import ( "testing" "time" - "github.com/blang/semver/v4" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/samber/lo" @@ -53,6 +52,7 @@ import ( sdkerrors "github.com/Azure/azure-sdk-for-go-extensions/pkg/errors" "github.com/Azure/azure-sdk-for-go/sdk/azcore" "github.com/Azure/karpenter-provider-azure/pkg/providers/imagefamily" + "github.com/Azure/karpenter-provider-azure/pkg/providers/imagefamily/bootstrap" "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute" "github.com/Azure/karpenter-provider-azure/pkg/apis" @@ -1112,15 +1112,12 @@ var _ = Describe("InstanceType Provider", func() { // NOTE: env.Version may differ from the version we get for the apiserver k8sVersion, err := azureEnv.ImageProvider.KubeServerVersion(ctx) Expect(err).To(BeNil()) - parsed := semver.MustParse(k8sVersion) - if utils.UseOOTCredential(parsed.Minor) { + crendetialProviderURL := bootstrap.CredentialProviderURL(k8sVersion, "amd64") + if crendetialProviderURL != "" { Expect(kubeletFlags).ToNot(ContainSubstring("--azure-container-registry-config")) Expect(kubeletFlags).To(ContainSubstring("--image-credential-provider-config=/var/lib/kubelet/credential-provider-config.yaml")) Expect(kubeletFlags).To(ContainSubstring("--image-credential-provider-bin-dir=/var/lib/kubelet/credential-provider")) - Expect(decodedString).To(ContainSubstring( - fmt.Sprintf("https://acs-mirror.azureedge.net/cloud-provider-azure/%s/binaries/azure-acr-credential-provider-linux-amd64-v%s.tar.gz", parsed.String(), parsed.String()), - )) - + Expect(decodedString).To(ContainSubstring(crendetialProviderURL)) } else { Expect(kubeletFlags).To(ContainSubstring("--azure-container-registry-config")) Expect(kubeletFlags).ToNot(ContainSubstring("--image-credential-provider-config")) diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index edb84c7fc..bd6ea31de 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -58,7 +58,3 @@ func MkVMID(resourceGroupName string, vmName string) string { const idFormat = "/subscriptions/subscriptionID/resourceGroups/%s/providers/Microsoft.Compute/virtualMachines/%s" return fmt.Sprintf(idFormat, resourceGroupName, vmName) } - -func UseOOTCredential(minorK8sVersion uint64) bool { - return minorK8sVersion >= 30 -}