fix: fixing some scriptless bugs (#7698)

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Author: Devinwong

aks-node-controller/helpers/const.go

@@ -8,7 +8,7 @@ const (
 	NetworkPolicyAzure   = "azure"
 	NetworkPolicyCalico  = "calico"
 	LoadBalancerBasic    = "basic"
-	LoadBalancerStandard = "Standard"
+	LoadBalancerStandard = "standard"
 	VMSizeStandardDc2s   = "Standard_DC2s"
 	VMSizeStandardDc4s   = "Standard_DC4s"
 	DefaultLinuxUser     = "azureuser"

aks-node-controller/helpers/utils_test.go

@@ -23,14 +23,14 @@ func Test_getLoadBalancerSKU(t *testing.T) {
 		{
 			name: "LoadBalancerSKU Standard",
 			args: args{
-				sku: "Standard",
+				sku: "standard",
 			},
 			want: aksnodeconfigv1.LoadBalancerSku_LOAD_BALANCER_SKU_STANDARD,
 		},
 		{
 			name: "LoadBalancerSKU Basic",
 			args: args{
-				sku: "Basic",
+				sku: "basic",
 			},
 			want: aksnodeconfigv1.LoadBalancerSku_LOAD_BALANCER_SKU_BASIC,
 		},
@@ -299,3 +299,110 @@ func TestIsKubeletServingCertificateRotationEnabled(t *testing.T) {
 		})
 	}
 }
+
+func TestValidateAndSetLinuxKubeletFlags_RemovesDeprecatedFlags(t *testing.T) {
+	kubeletFlags := map[string]string{
+		"--dynamic-config-dir":           "/var/lib/kubelet",
+		"--non-masquerade-cidr":          "10.240.0.0/12",
+		"--cni-bin-dir":                  "/opt/cni/bin",
+		"--cni-cache-dir":                "/var/lib/cni",
+		"--cni-conf-dir":                 "/etc/cni/net.d",
+		"--docker-endpoint":              "npipe:////./pipe/docker_engine",
+		"--image-pull-progress-deadline": "30m",
+		"--network-plugin":               "cni",
+		"--network-plugin-mtu":           "1500",
+		"--feature-gates":                "",
+	}
+
+	ValidateAndSetLinuxKubeletFlags(kubeletFlags, newTestContainerService("1.27.3"), &datamodel.AgentPoolProfile{})
+
+	removedFlags := []string{
+		"--dynamic-config-dir",
+		"--non-masquerade-cidr",
+		"--cni-bin-dir",
+		"--cni-cache-dir",
+		"--cni-conf-dir",
+		"--docker-endpoint",
+		"--image-pull-progress-deadline",
+		"--network-plugin",
+		"--network-plugin-mtu",
+	}
+
+	for _, flag := range removedFlags {
+		if _, exists := kubeletFlags[flag]; exists {
+			t.Fatalf("expected flag %s to be removed", flag)
+		}
+	}
+}
+
+func TestValidateAndSetLinuxKubeletFlags_FeatureGatesByVersion(t *testing.T) {
+	testCases := []struct {
+		name                 string
+		version              string
+		initialFeatureGates  string
+		rotateServerCerts    bool
+		expectedFeatureGates map[string]bool
+	}{
+		{
+			name:                "removes dynamic gate when version >= 1.24",
+			version:             "1.26.0",
+			initialFeatureGates: "DynamicKubeletConfig=false,OtherFeature=true",
+			expectedFeatureGates: map[string]bool{
+				"OtherFeature": true,
+			},
+		},
+		{
+			name:                "adds dynamic and disable accelerator gates for 1.22",
+			version:             "1.22.6",
+			initialFeatureGates: "FooBar=true",
+			expectedFeatureGates: map[string]bool{
+				"FooBar":                         true,
+				"DynamicKubeletConfig":           false,
+				"DisableAcceleratorUsageMetrics": false,
+			},
+		},
+		{
+			name:                 "does not add dynamic gate before 1.11",
+			version:              "1.10.13",
+			initialFeatureGates:  "",
+			expectedFeatureGates: map[string]bool{},
+		},
+		{
+			name:                "adds rotate kubelet server certificate gate when enabled",
+			version:             "1.28.2",
+			initialFeatureGates: "",
+			rotateServerCerts:   true,
+			expectedFeatureGates: map[string]bool{
+				"RotateKubeletServerCertificate": true,
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			kubeletFlags := map[string]string{
+				"--feature-gates": tc.initialFeatureGates,
+			}
+			if tc.rotateServerCerts {
+				kubeletFlags["--rotate-server-certificates"] = "true"
+			}
+
+			ValidateAndSetLinuxKubeletFlags(kubeletFlags, newTestContainerService(tc.version), &datamodel.AgentPoolProfile{})
+
+			featureGateMap := strKeyValToMapBool(kubeletFlags["--feature-gates"], ",", "=")
+			if !reflect.DeepEqual(featureGateMap, tc.expectedFeatureGates) {
+				t.Fatalf("unexpected feature gates: got %v, want %v", featureGateMap, tc.expectedFeatureGates)
+			}
+		})
+	}
+}
+
+func newTestContainerService(version string) *datamodel.ContainerService {
+	return &datamodel.ContainerService{
+		Properties: &datamodel.Properties{
+			OrchestratorProfile: &datamodel.OrchestratorProfile{
+				OrchestratorVersion: version,
+			},
+		},
+	}
+}

aks-node-controller/parser/helper.go

@@ -596,13 +596,6 @@ func getDisableSSH(v *aksnodeconfigv1.Configuration) bool {
 	return !v.GetEnableSsh()
 }
 
-func getServicePrincipalFileContent(authConfig *aksnodeconfigv1.AuthConfig) string {
-	if authConfig.GetServicePrincipalSecret() == "" {
-		return ""
-	}
-	return base64.StdEncoding.EncodeToString([]byte(authConfig.GetServicePrincipalSecret()))
-}
-
 func getKubeletFlags(kubeletConfig *aksnodeconfigv1.KubeletConfig) string {
 	return createSortedKeyValuePairs(kubeletConfig.GetKubeletFlags(), " ")
 }

aks-node-controller/parser/parser.go

@@ -136,7 +136,7 @@ func getCSEEnv(config *aksnodeconfigv1.Configuration) map[string]string {
 		"DHCPV6_CONFIG_FILEPATH":                             getDHCPV6ConfigFilepath(),
 		"THP_ENABLED":                                        config.GetCustomLinuxOsConfig().GetTransparentHugepageSupport(),
 		"THP_DEFRAG":                                         config.GetCustomLinuxOsConfig().GetTransparentDefrag(),
-		"SERVICE_PRINCIPAL_FILE_CONTENT":                     getServicePrincipalFileContent(config.AuthConfig),
+		"SERVICE_PRINCIPAL_FILE_CONTENT":                     config.GetAuthConfig().GetServicePrincipalSecret(),
 		"KUBELET_CLIENT_CONTENT":                             config.GetKubeletConfig().GetKubeletClientKey(),
 		"KUBELET_CLIENT_CERT_CONTENT":                        config.GetKubeletConfig().GetKubeletClientCertContent(),
 		"KUBELET_CONFIG_FILE_ENABLED":                        fmt.Sprintf("%v", config.GetKubeletConfig().GetEnableKubeletConfigFile()),

aks-node-controller/parser/parser_test.go

@@ -334,6 +334,20 @@ oom_score = -999
 	}
 }
 
+func TestBuildCSECmd_SetsServicePrincipalFileContent(t *testing.T) {
+	secret := "super-secret-value"
+	cmd, err := BuildCSECmd(context.TODO(), &aksnodeconfigv1.Configuration{
+		AuthConfig: &aksnodeconfigv1.AuthConfig{ServicePrincipalSecret: secret},
+	})
+	require.NoError(t, err)
+
+	vars := environToMap(cmd.Env)
+
+	// The value should be exactly the secret, without additional base64 encoding.
+	// Actually the client which passes the secret to aks-node-controller should have base64 encoded it first.
+	assert.Equal(t, secret, vars["SERVICE_PRINCIPAL_FILE_CONTENT"])
+}
+
 func TestAKSNodeConfigCompatibilityFromJsonToCSECommand(t *testing.T) {
 	tests := []struct {
 		name      string

e2e/node_config.go

@@ -532,7 +532,7 @@ func baseTemplateLinux(t testing.TB, location string, k8sVersion string, arch st
 				},
 				ServicePrincipalProfile: &datamodel.ServicePrincipalProfile{
 					ClientID: "msi",
-					Secret:   "**msi**",
+					Secret:   base64.StdEncoding.EncodeToString([]byte("msi")),
 				},
 				CertificateProfile:  &datamodel.CertificateProfile{},
 				HostedMasterProfile: &datamodel.HostedMasterProfile{},
@@ -816,7 +816,7 @@ func baseTemplateWindows(t testing.TB, location string) *datamodel.NodeBootstrap
 				},
 				ServicePrincipalProfile: &datamodel.ServicePrincipalProfile{
 					ClientID: "msi",
-					Secret:   "**msi**",
+					Secret:   base64.StdEncoding.EncodeToString([]byte("msi")),
 				},
 				FeatureFlags: &datamodel.FeatureFlags{
 					EnableWinDSR: true,

e2e/validators.go

@@ -199,7 +199,7 @@ func ValidateLeakedSecrets(ctx context.Context, s *Scenario) {
 	for _, logFile := range []string{"/var/log/azure/cluster-provision.log", "/var/log/azure/aks-node-controller.log"} {
 		for _, secretValue := range secrets {
 			if secretValue != "" {
-				ValidateFileExcludesContent(ctx, s, logFile, secretValue)
+				ValidateFileExcludesExactContent(ctx, s, logFile, secretValue)
 			}
 		}
 	}
@@ -384,20 +384,65 @@ func fileHasContent(ctx context.Context, s *Scenario, fileName string, contents
 	}
 }
 
+func fileHasExactContent(ctx context.Context, s *Scenario, fileName string, contents string) bool {
+	s.T.Helper()
+	require.NotEmpty(s.T, contents, "Test setup failure: Can't validate that a file has contents with an empty string. Filename: %s", fileName)
+	encodedPattern := base64.StdEncoding.EncodeToString([]byte(contents))
+	if s.IsWindows() {
+		steps := []string{
+			"$ErrorActionPreference = \"Stop\"",
+			fmt.Sprintf("if ( -not ( Test-Path -Path %s ) ) { exit 2 }", fileName),
+			fmt.Sprintf("$pattern = [Text.Encoding]::UTF8.GetString([Convert]::FromBase64String('%s'))", encodedPattern),
+			fmt.Sprintf("$content = Get-Content -Path %s -Raw", fileName),
+			"$escaped = [regex]::Escape($pattern)",
+			"if ([regex]::Match($content, \"(?<!\\w)\" + $escaped + \"(?!\\w)\").Success) { exit 0 } else { exit 1 }",
+		}
+		execResult := execScriptOnVMForScenario(ctx, s, strings.Join(steps, "\n"))
+		return execResult.exitCode == "0"
+	} else {
+		steps := []string{
+			"set -ex",
+			fmt.Sprintf("if [ ! -f %s ]; then exit 2; fi", fileName),
+			fmt.Sprintf("pattern=$(printf '%%s' '%s' | base64 -d)", encodedPattern),
+			"escaped=$(printf '%s\n' \"$pattern\" | sed -e 's/[.\\[\\()*?^$+{}|]/\\\\&/g')",
+			"regex='(^|[^[:alnum:]_])'\"$escaped\"'([^[:alnum:]_]|$)'",
+			fmt.Sprintf("if sudo grep -Eq \"$regex\" %s; then exit 0; else exit 1; fi", fileName),
+		}
+		execResult := execScriptOnVMForScenario(ctx, s, strings.Join(steps, "\n"))
+		return execResult.exitCode == "0"
+	}
+}
+
+// ValidateFileHasContent passes the test if the specified file contains the specified contents.
+// The contents doesn't need to be surrounded by non-word characters.
+// E.g.: searching "bcd" in "abcdef" is a match, thus the validation passes.
 func ValidateFileHasContent(ctx context.Context, s *Scenario, fileName string, contents string) {
 	s.T.Helper()
 	if !fileHasContent(ctx, s, fileName, contents) {
 		s.T.Fatalf("expected file %s to have contents %q, but it does not", fileName, contents)
 	}
 }
 
+// ValidateFileExcludesContent fails the test if the specified file contains the specified contents.
+// The contents doesn't need to be surrounded by non-word characters.
+// E.g.: searching "bcd" in "abcdef" is a match, thus the validation fails.
 func ValidateFileExcludesContent(ctx context.Context, s *Scenario, fileName string, contents string) {
 	s.T.Helper()
 	if fileHasContent(ctx, s, fileName, contents) {
 		s.T.Fatalf("expected file %s to not have contents %q, but it does", fileName, contents)
 	}
 }
 
+// ValidateFileExcludesExactContent fails the test if the specified file contains the specified contents.
+// The contents needs to be surrounded by non-word characters.
+// E.g.: searching "bcd" in "abcdef" is not a match, thus the validation passes.
+func ValidateFileExcludesExactContent(ctx context.Context, s *Scenario, fileName string, contents string) {
+	s.T.Helper()
+	if fileHasExactContent(ctx, s, fileName, contents) {
+		s.T.Fatalf("expected file %s to not have exact contents %q, but it does", fileName, contents)
+	}
+}
+
 func ServiceCanRestartValidator(ctx context.Context, s *Scenario, serviceName string, restartTimeoutInSeconds int) {
 	s.T.Helper()
 	steps := []string{

e2e/vmss.go

@@ -548,8 +548,13 @@ func extractLogsFromVMLinux(ctx context.Context, s *Scenario, vm *ScenarioVM) er
 		"cluster-provision-cse-output.log": "sudo cat /var/log/azure/cluster-provision-cse-output.log",
 		"sysctl-out.log":                   "sudo sysctl -a",
 		"aks-node-controller.log":          "sudo cat /var/log/azure/aks-node-controller.log",
-		"syslog":                           "sudo cat /var/log/" + syslogHandle,
-		"journalctl":                       "sudo journalctl --boot=0 --no-pager",
+		"aks-node-controller-config.json":  "sudo cat /opt/azure/containers/aks-node-controller-config.json", // Only available in Scriptless.
+
+		// Only available in Scriptless. By default, e2e enables aks-node-controller-hack, so this is the actual config used. Only in e2e. Not used in production.
+		"aks-node-controller-config-hack.json": "sudo cat /opt/azure/containers/aks-node-controller-config-hack.json",
+		"syslog":                               "sudo cat /var/log/" + syslogHandle,
+		"journalctl":                           "sudo journalctl --boot=0 --no-pager",
+		"azure.json":                           "sudo cat /etc/kubernetes/azure.json",
 	}
 	if s.SecureTLSBootstrappingEnabled() {
 		commandList["secure-tls-bootstrap.log"] = "sudo cat /var/log/azure/aks/secure-tls-bootstrap.log"