diff --git a/.github/workflows/hotfix-generate.yml b/.github/workflows/hotfix-generate.yml index eb62f5baea6..40174f95cef 100644 --- a/.github/workflows/hotfix-generate.yml +++ b/.github/workflows/hotfix-generate.yml @@ -95,12 +95,22 @@ jobs: continue fi CHANGED=1 - CONTENT=$(base64 -w 0 "$FILE") # For a brand-new file the Contents API lookup 404s (no sha yet); the # step runs under `bash -e`, so guard the lookup and only pass -f sha # when the file already exists on the branch, otherwise the PUT must # omit sha entirely to create the file. SHA=$(gh api "repos/${{ github.repository }}/contents/${FILE}?ref=${{ github.head_ref }}" --jq '.sha' 2>/dev/null || true) + if [ ! -e "$FILE" ]; then + if [ -n "$SHA" ]; then + gh api "repos/${{ github.repository }}/contents/${FILE}" \ + -X DELETE \ + -f message="chore: auto-generate hotfix content for ${FILE}" \ + -f branch="${{ github.head_ref }}" \ + -f sha="$SHA" + fi + continue + fi + CONTENT=$(base64 -w 0 "$FILE") if [ -n "$SHA" ]; then gh api "repos/${{ github.repository }}/contents/${FILE}" \ -X PUT \ diff --git a/e2e/validators.go b/e2e/validators.go index 56f13327d89..abe8f2a3ab3 100644 --- a/e2e/validators.go +++ b/e2e/validators.go @@ -2749,15 +2749,14 @@ func ValidateNodeHasLabel(ctx context.Context, s *Scenario, labelKey, expectedVa // ValidateScriptlessCSECmd checks if the node has scriptless cmd correctly enabled func ValidateScriptlessCSECmd(ctx context.Context, s *Scenario) { nbc := s.Runtime.NBC - if nbc != nil && s.VHD.SupportsScriptless() && nbc.EnableScriptlessCSECmd && !nbc.EnableScriptlessNBCCSECmd { + if nbc != nil && s.VHD.SupportsScriptless() && nbc.EnableScriptlessCSECmd && !usesScriptlessNBCCSECmd(s) { ValidateFileExists(ctx, s, "/opt/azure/containers/scriptless-cse-overrides.txt") } } // ValidateScriptlessNBCCSECmd checks if the node has scriptless NBCCSECmd correctly enabled func ValidateScriptlessNBCCSECmd(ctx context.Context, s *Scenario) { - nbc := s.Runtime.NBC - if nbc != nil && nbc.EnableScriptlessNBCCSECmd && s.VHD.SupportsScriptless() { + if usesScriptlessNBCCSECmd(s) { fileNameToCheck := "/opt/azure/containers/aks-node-controller-nbc-cmd.sh" if enableScriptlessCompilation(s) { fileNameToCheck = "/opt/azure/containers/aks-node-controller-nbc-cmd-hack.sh" @@ -2770,7 +2769,7 @@ func ValidateScriptlessNBCCSECmd(ctx context.Context, s *Scenario) { // ValidateScriptlessPhase3 validates that there are not diffs between ANC generated cse cmd NBC cse cmd vars func ValidateScriptlessPhase3(ctx context.Context, s *Scenario) { s.T.Helper() - if s.Runtime.AKSNodeConfig != nil && s.Runtime.NBC.EnableScriptlessNBCCSECmd { + if s.Runtime.AKSNodeConfig != nil && usesScriptlessNBCCSECmd(s) { logFile := "/var/log/azure/aks-node-controller.log" if !fileHasContent(ctx, s, logFile, "env compare: no differences found between provision-config and nbc-cmd env vars") { // Grep for all env-compare diff markers to show what's different. diff --git a/e2e/vmss.go b/e2e/vmss.go index e39593ab1d7..80e918bdf20 100644 --- a/e2e/vmss.go +++ b/e2e/vmss.go @@ -488,8 +488,9 @@ func createVMSSModel(ctx context.Context, s *Scenario) armcompute.VirtualMachine if s.Runtime.AKSNodeConfig != nil { cse = nodeconfigutils.CSE aksNodeConfig := s.Runtime.AKSNodeConfig + scriptlessNBCCSECmdEnabled := usesScriptlessNBCCSECmd(s) - if s.Runtime.NBC.EnableScriptlessNBCCSECmd { + if scriptlessNBCCSECmdEnabled { cse = nodeBootstrapping.CSE } @@ -497,7 +498,7 @@ func createVMSSModel(ctx context.Context, s *Scenario) armcompute.VirtualMachine if config.Config.DisableScriptLessCompilation { var data string var err error - if s.Runtime.NBC.EnableScriptlessNBCCSECmd { + if scriptlessNBCCSECmdEnabled { return nodeBootstrapping.CustomData } if s.VHD.Flatcar { @@ -510,7 +511,7 @@ func createVMSSModel(ctx context.Context, s *Scenario) armcompute.VirtualMachine } binaryURL, err := CachedCompileAndUploadAKSNodeController(ctx, s.VHD.Arch) require.NoError(s.T, err, "failed to compile and upload aks-node-controller binary") - if s.Runtime.NBC.EnableScriptlessNBCCSECmd { + if scriptlessNBCCSECmdEnabled { customData := nodeBootstrapping.CustomData customData, err = CustomDataWithNBCCmdHack(s, customData, binaryURL) require.NoError(s.T, err, "failed to generate custom data with NBC cmd hack") @@ -534,7 +535,7 @@ func createVMSSModel(ctx context.Context, s *Scenario) armcompute.VirtualMachine customData, err = injectWriteFilesEntriesToCustomData(customData, s.Config.CustomDataWriteFiles) require.NoError(s.T, err, "failed to inject customData write_files entries") } - if s.Runtime.NBC.EnableScriptlessCSECmd && !s.Runtime.NBC.EnableScriptlessNBCCSECmd && s.VHD.SupportsScriptless() { + if s.Runtime.NBC.EnableScriptlessCSECmd && !usesScriptlessNBCCSECmd(s) && s.VHD.SupportsScriptless() { // Validate that the custom data doesn't contain any script content, // which indicates that the scriptless CSE is working as intended decodedCustomData, err := base64.StdEncoding.DecodeString(customData) @@ -591,8 +592,19 @@ func createVMSSModel(ctx context.Context, s *Scenario) armcompute.VirtualMachine return model } +func usesScriptlessNBCCSECmd(s *Scenario) bool { + if s == nil || s.Runtime == nil || s.Runtime.NBC == nil || s.VHD == nil { + return false + } + nbc := s.Runtime.NBC + return nbc.EnableScriptlessNBCCSECmd && + !nbc.PreProvisionOnly && + s.VHD.SupportsScriptless() && + (nbc.CustomCATrustConfig == nil || len(nbc.CustomCATrustConfig.CustomCATrustCerts) == 0) +} + func enableScriptlessCompilation(s *Scenario) bool { - return s.Runtime.NBC.EnableScriptlessNBCCSECmd && len(s.Config.CustomDataWriteFiles) <= 0 && !config.Config.DisableScriptLessCompilation && !s.Tags.NetworkIsolated && !s.Runtime.NBC.PreProvisionOnly + return usesScriptlessNBCCSECmd(s) && len(s.Config.CustomDataWriteFiles) <= 0 && !config.Config.DisableScriptLessCompilation && !s.Tags.NetworkIsolated } func CreateVMSSWithRetry(ctx context.Context, s *Scenario) (*ScenarioVM, error) { diff --git a/hotfix/hotfix_generate.py b/hotfix/hotfix_generate.py index b2658a531e8..5a303babd82 100644 --- a/hotfix/hotfix_generate.py +++ b/hotfix/hotfix_generate.py @@ -16,9 +16,9 @@ bumped using the same base-version + tag-collision algorithm as `version`. 3. Writes the resulting {"version", "scripts_version"} (omitting fields that don't - apply) to parts/linux/cloud-init/artifacts/aks-node-controller-hotfix.json, the - file pkg/agent/baker.go embeds directly into every scriptless customData render - (see hotfixJSONFile in pkg/agent/const.go). + apply) to parts/linux/cloud-init/artifacts/aks-node-controller-hotfix.json only + when there is an active hotfix. If no hotfix applies, the file is removed/left + absent so scriptless customData does not embed an empty hotfix artifact. Usage: python3 hotfix/hotfix_generate.py base_ref: git ref to diff against for changed-script/changed-code detection @@ -28,6 +28,7 @@ """ import json +import os import re import subprocess import sys @@ -167,22 +168,30 @@ def path_changed(base_ref, path): def write_hotfix_file(version, scripts_version): - """Write the resolved {version, scripts_version} to TARGET_FILE, the file - pkg/agent/baker.go embeds directly at customData-render time.""" + """Write the resolved {version, scripts_version} to TARGET_FILE when active. + + When no hotfix applies, remove TARGET_FILE if present. An empty JSON object is + still embedded as a real scriptless customData file, which changes payload + shape even though there is no hotfix for the wrapper to consume. + """ payload = {} if version: payload["version"] = version if scripts_version: payload["scripts_version"] = scripts_version - with open(TARGET_FILE, "w") as f: - json.dump(payload, f, indent=4) - f.write("\n") - if payload: + with open(TARGET_FILE, "w") as f: + json.dump(payload, f, indent=4) + f.write("\n") print(f"Wrote {payload} to {TARGET_FILE}", file=sys.stderr) - else: - print(f"No active hotfix; reset {TARGET_FILE} to {{}}", file=sys.stderr) + return + + try: + os.remove(TARGET_FILE) + print(f"No active hotfix; removed {TARGET_FILE}", file=sys.stderr) + except FileNotFoundError: + print(f"No active hotfix; {TARGET_FILE} already absent", file=sys.stderr) def detect_changed_varkeys(base_ref): diff --git a/pkg/agent/baker.go b/pkg/agent/baker.go index e0bb3bbc58c..6a96508a861 100644 --- a/pkg/agent/baker.go +++ b/pkg/agent/baker.go @@ -93,8 +93,13 @@ func (t *TemplateGenerator) getWindowsNodeBootstrappingPayload(config *datamodel } func (t *TemplateGenerator) getLinuxNodeBootstrappingPayload(config *datamodel.NodeBootstrappingConfiguration) string { - if config.EnableScriptlessNBCCSECmd && !config.PreProvisionOnly { - return t.getScriptlessNBCCustomData(config) + if config.EnableScriptlessNBCCSECmd { + if supportsScriptlessPhase2(config) { + return t.getScriptlessNBCCustomData(config) + } + // if we cannot enable scriptless phase2, we need to fallback to scriptless phase1 + config.EnableScriptlessNBCCSECmd = false + config.EnableScriptlessCSECmd = true } // this might seem strange that we're encoding the custom data to a JSON string and then extracting it, but without that serialisation and deserialisation @@ -163,6 +168,10 @@ func (t *TemplateGenerator) getScriptlessNBCCustomData(config *datamodel.NodeBoo return base64.StdEncoding.EncodeToString([]byte(customData)) } +func supportsScriptlessPhase2(config *datamodel.NodeBootstrappingConfiguration) bool { + return !config.PreProvisionOnly && !areCustomCATrustCertsPopulated(*config) +} + // renderEnabledFeatures serializes the feature toggle map into sorted KEY=VALUE lines for // enabled_features.sh. Keys are sorted so the output is deterministic (Go map iteration is // randomized) and filtered to valid shell identifiers - the same set the aks-node-controller @@ -440,7 +449,7 @@ func (t *TemplateGenerator) getNodeBootstrappingCmd(config *datamodel.NodeBootst if config.AgentPoolProfile.IsWindows() { return t.getWindowsNodeCSECommand(config) } - if config.EnableScriptlessNBCCSECmd && !config.PreProvisionOnly { + if config.EnableScriptlessNBCCSECmd && supportsScriptlessPhase2(config) { return "/opt/azure/containers/aks-node-controller provision-wait" } return t.getLinuxNodeCSECommand(config) @@ -1474,7 +1483,7 @@ func getContainerServiceFuncMap(config *datamodel.NodeBootstrappingConfiguration }, "GetPreProvisionOnly": func() bool { return config.PreProvisionOnly }, "GetCSETimeout": func() string { return datamodel.GetCSETimeout(config.CSETimeout) }, - "GetSkipWaAgentHold": func() bool { return config.EnableScriptlessNBCCSECmd }, + "GetSkipWaAgentHold": func() bool { return config.EnableScriptlessNBCCSECmd && supportsScriptlessPhase2(config) }, "BlockIptables": func() bool { return cs.Properties.OrchestratorProfile.KubernetesConfig.BlockIptables }, diff --git a/pkg/agent/baker_test.go b/pkg/agent/baker_test.go index 4141d642a76..959b0e2054c 100644 --- a/pkg/agent/baker_test.go +++ b/pkg/agent/baker_test.go @@ -153,6 +153,74 @@ var _ = Describe("Assert generated customData and cseCmd", func() { }) }) + Describe(".supportsScriptlessPhase2()", func() { + It("given PreProvisionOnly is false and an empty list of custom ca certs, it returns true", func() { + config.PreProvisionOnly = false + config.CustomCATrustConfig = &datamodel.CustomCATrustConfig{ + CustomCATrustCerts: []string{}, + } + Expect(supportsScriptlessPhase2(config)).To(BeTrue()) + }) + It("given PreProvisionOnly is false and custom ca certs are populated, it returns false", func() { + config.PreProvisionOnly = false + config.CustomCATrustConfig = &datamodel.CustomCATrustConfig{ + CustomCATrustCerts: []string{"mock cert value"}, + } + Expect(supportsScriptlessPhase2(config)).To(BeFalse()) + }) + It("given PreProvisionOnly is true and no CustomCATrustConfig, it returns false", func() { + config.PreProvisionOnly = true + Expect(supportsScriptlessPhase2(config)).To(BeFalse()) + }) + It("given PreProvisionOnly is true and custom ca certs are populated, it returns false", func() { + config.PreProvisionOnly = true + config.CustomCATrustConfig = &datamodel.CustomCATrustConfig{ + CustomCATrustCerts: []string{"mock cert value"}, + } + Expect(supportsScriptlessPhase2(config)).To(BeFalse()) + }) + }) + + Describe("GetSkipWaAgentHold template func", func() { + getSkipWaAgentHold := func() func() bool { + funcMap := getContainerServiceFuncMap(config) + fn, ok := funcMap["GetSkipWaAgentHold"].(func() bool) + Expect(ok).To(BeTrue()) + return fn + } + + It("returns false when scriptless NBC is disabled", func() { + config.PreProvisionOnly = false + config.EnableScriptlessNBCCSECmd = false + + Expect(getSkipWaAgentHold()()).To(BeFalse()) + }) + + It("returns true when scriptless NBC phase2 is supported", func() { + config.PreProvisionOnly = false + config.EnableScriptlessNBCCSECmd = true + + Expect(getSkipWaAgentHold()()).To(BeTrue()) + }) + + It("returns false when custom CA certs disable scriptless NBC phase2", func() { + config.PreProvisionOnly = false + config.EnableScriptlessNBCCSECmd = true + config.CustomCATrustConfig = &datamodel.CustomCATrustConfig{ + CustomCATrustCerts: []string{"mock cert value"}, + } + + Expect(getSkipWaAgentHold()()).To(BeFalse()) + }) + + It("returns false for pre-provisioning", func() { + config.PreProvisionOnly = true + config.EnableScriptlessNBCCSECmd = true + + Expect(getSkipWaAgentHold()()).To(BeFalse()) + }) + }) + Describe(".isMariner()", func() { It("given an empty string, that is not mariner", func() { Expect(isMariner("")).To(BeFalse()) @@ -1668,6 +1736,25 @@ var _ = Describe("getLinuxNodeBootstrappingPayload", func() { Expect(string(decodedPayload)).NotTo(ContainSubstring(enabledFeaturesFilepath)) }) + It("should disable scriptless NBC when falling back to scriptless CSE", func() { + templateGenerator := InitializeTemplateGenerator() + config := newConfig(false) + config.CustomCATrustConfig = &datamodel.CustomCATrustConfig{ + CustomCATrustCerts: []string{"mock cert value"}, + } + + payload := templateGenerator.getLinuxNodeBootstrappingPayload(config) + decodedPayload, err := base64.StdEncoding.DecodeString(payload) + Expect(err).NotTo(HaveOccurred()) + decompressedPayload, err := getGzipDecodedValue(decodedPayload) + Expect(err).NotTo(HaveOccurred()) + + Expect(config.EnableScriptlessNBCCSECmd).To(BeFalse()) + Expect(config.EnableScriptlessCSECmd).To(BeTrue()) + Expect(string(decompressedPayload)).To(ContainSubstring("/opt/azure/containers/scriptless-cse-overrides.txt")) + Expect(string(decompressedPayload)).NotTo(ContainSubstring(aksNbcCmdFilepath)) + }) + It("should drop enabled_features entries whose value contains a newline", func() { templateGenerator := InitializeTemplateGenerator() config := newConfig(false) @@ -1838,6 +1925,68 @@ var _ = Describe("getNodeBootstrappingCmd", func() { Expect(templateGenerator.getNodeBootstrappingCmd(config)).To(Equal(templateGenerator.getLinuxNodeCSECommand(config))) Expect(templateGenerator.getNodeBootstrappingCmd(config)).NotTo(Equal("/opt/azure/containers/aks-node-controller provision-wait")) }) + + newScriptlessCmdTestConfig := func() *datamodel.NodeBootstrappingConfiguration { + agentPoolProfile := &datamodel.AgentPoolProfile{ + Name: "nodepool1", + OSType: datamodel.Linux, + Distro: datamodel.AKSUbuntuContainerd2204Gen2, + } + return &datamodel.NodeBootstrappingConfiguration{ + ContainerService: &datamodel.ContainerService{ + Location: "eastus", + Properties: &datamodel.Properties{ + OrchestratorProfile: &datamodel.OrchestratorProfile{ + OrchestratorVersion: "1.29.0", + OrchestratorType: datamodel.Kubernetes, + KubernetesConfig: &datamodel.KubernetesConfig{ + ContainerRuntimeConfig: map[string]string{}, + }, + }, + HostedMasterProfile: &datamodel.HostedMasterProfile{ + FQDN: "test-cluster.hcp.eastus.azmk8s.io", + }, + AgentPoolProfiles: []*datamodel.AgentPoolProfile{agentPoolProfile}, + }, + }, + AgentPoolProfile: agentPoolProfile, + CloudSpecConfig: datamodel.AzurePublicCloudSpecForTest, + K8sComponents: &datamodel.K8sComponents{}, + KubeletConfig: map[string]string{}, + } + } + + It("should use the aks-node-controller provision-wait command when scriptless phase2 is supported", func() { + templateGenerator := InitializeTemplateGenerator() + config := newScriptlessCmdTestConfig() + config.EnableScriptlessNBCCSECmd = true + config.PreProvisionOnly = false + + Expect(templateGenerator.getNodeBootstrappingCmd(config)).To(Equal("/opt/azure/containers/aks-node-controller provision-wait")) + }) + + It("should use the regular linux CSE command when EnableScriptlessNBCCSECmd is false", func() { + templateGenerator := InitializeTemplateGenerator() + config := newScriptlessCmdTestConfig() + config.EnableScriptlessNBCCSECmd = false + config.PreProvisionOnly = false + + Expect(templateGenerator.getNodeBootstrappingCmd(config)).To(Equal(templateGenerator.getLinuxNodeCSECommand(config))) + Expect(templateGenerator.getNodeBootstrappingCmd(config)).NotTo(Equal("/opt/azure/containers/aks-node-controller provision-wait")) + }) + + It("should use the regular linux CSE command when custom ca trust certs are populated", func() { + templateGenerator := InitializeTemplateGenerator() + config := newScriptlessCmdTestConfig() + config.EnableScriptlessNBCCSECmd = true + config.PreProvisionOnly = false + config.CustomCATrustConfig = &datamodel.CustomCATrustConfig{ + CustomCATrustCerts: []string{"mock cert value"}, + } + + Expect(templateGenerator.getNodeBootstrappingCmd(config)).To(Equal(templateGenerator.getLinuxNodeCSECommand(config))) + Expect(templateGenerator.getNodeBootstrappingCmd(config)).NotTo(Equal("/opt/azure/containers/aks-node-controller provision-wait")) + }) }) var _ = Describe("cloudInitToButane", func() {