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 <[email protected]>
Azure · Jul 17, 2024 · 46b4276 · 46b4276
1 parent 18c0716
commit 46b4276
Show file tree

Hide file tree

Showing 5 changed files with 80 additions and 15 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
-}