From d0be1f5bd84463f5920cc3489ac3c8fa37def90f Mon Sep 17 00:00:00 2001 From: Abigailliang Date: Fri, 23 Oct 2026 14:05:16 -0700 Subject: [PATCH 1/5] feat: initialize Azure clouds natively in ANC Run native cloud certificate and RepoDepot initialization before the existing provisioning scripts for --provision-config. Add native periodic certificate refresh while retaining shell initialization for traditional CSE, NBC, and older VHDs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: be499bac-1283-498e-b5b1-59d8c91047a4 --- aks-node-controller/README.md | 42 ++ aks-node-controller/app.go | 26 +- aks-node-controller/app_test.go | 1 + aks-node-controller/cloud_certificates.go | 252 ++++++++ aks-node-controller/cloud_init.go | 406 +++++++++++++ aks-node-controller/cloud_init_test.go | 556 ++++++++++++++++++ aks-node-controller/cloud_provision_test.go | 158 +++++ aks-node-controller/cloud_repositories.go | 261 ++++++++ .../cloud_repositories_test.go | 163 +++++ aks-node-controller/embeddednodecustomdata.go | 3 +- aks-node-controller/parser/consts.go | 1 - aks-node-controller/parser/helper.go | 9 +- aks-node-controller/parser/parser.go | 3 +- aks-node-controller/parser/parser_test.go | 10 + .../parser/templates/cse_cmd.sh.gtpl | 3 - .../AKSUbuntu2204+China/generatedCSECommand | 2 +- .../generatedCSECommand | 2 +- .../generatedCSECommand | 2 +- .../generatedCSECommand | 2 +- .../generatedCSECommand | 2 +- .../generatedCSECommand | 2 +- .../generatedCSECommand | 2 +- .../generatedCSECommand | 2 +- .../generatedCSECommand | 2 +- .../generatedCSECommand | 2 +- .../generatedCSECommand | 2 +- .../generatedCSECommand | 2 +- .../generatedCSECommand | 2 +- 28 files changed, 1892 insertions(+), 28 deletions(-) create mode 100644 aks-node-controller/cloud_certificates.go create mode 100644 aks-node-controller/cloud_init.go create mode 100644 aks-node-controller/cloud_init_test.go create mode 100644 aks-node-controller/cloud_provision_test.go create mode 100644 aks-node-controller/cloud_repositories.go create mode 100644 aks-node-controller/cloud_repositories_test.go diff --git a/aks-node-controller/README.md b/aks-node-controller/README.md index f1b960a2698..86e07dca3c0 100644 --- a/aks-node-controller/README.md +++ b/aks-node-controller/README.md @@ -54,6 +54,48 @@ the provisioning config in CustomData; the automatic size fallback uses the same config-only payload. Leaving the new flag false preserves legacy, Phase 2, and Phase 2.5 behavior. +### Native Azure cloud initialization + +When provisioning with `--provision-config` alone, ANC initializes Azure cloud +certificates and RepoDepot in Go before starting the provisioning scripts. This path +does not execute `init-aks-cloud.sh`. NBC takes precedence when `--nbc-cmd` is +supplied, so Phase 2, Phase 2.5, and traditional CSE retain their existing shell +initialization. The shell artifact is still delivered for those paths and older +VHD binaries; this change does not remove it from AgentBaker CustomData. + +Regions starting with `ussec` or `usnat` use the legacy certificate endpoint. +Other regions check WireServer's root-certificate opt-in before downloading root +and intermediate certificates. WireServer failures, malformed responses, invalid +certificates, trust-store errors, and refresh scheduling failures stop native +provisioning rather than falling back to the distribution's default trust. +Raw certificate responses are never logged; `AKS_CLOUD_LOG_CERTIFICATES=true` +enables diagnostics containing re-encoded public certificates only. + +Eligible nodes require a nonempty region and register a daily 19:00 certificate +refresh using the running ANC binary, including when provisioning uses an ANC +hotfix. Ubuntu, Mariner, and Azure Linux use cron; Flatcar and Azure Container +Linux use the existing `azure-ca-refresh.service` and timer. Existing shell +refresh entries are replaced. +The refresh command takes the location explicitly and does not configure +repositories or reinstall its schedule: + +```sh +/opt/azure/containers/aks-node-controller refresh-cloud-certificates --location westus2 +``` + +RepoDepot initialization is independent of certificate opt-in and runs only when +a repository endpoint is configured. Ubuntu sources and Microsoft keyrings, +Mariner mirrors, and Azure Linux repositories retain their existing roles; +immutable Flatcar/ACL images do not configure package repositories. Cache refreshes +retry up to ten times and report exhausted failures. Native initialization honors +the configured CSE timeout, with 30-second HTTP requests and two-minute command +timeouts. Periodic certificate refresh has a 15-minute budget. `provision --dry-run` +does not fetch certificates, change files, or install refresh schedules. + +Native initialization and periodic refresh emit guest-agent events through +`EventLogger` using `CloudInitialization`, `CloudRepositoryInitialization`, and +`CloudCertificateRefresh` task suffixes, alongside the existing ANC logs. + ### Validating and normalizing AKSNodeConfig Before serializing a typed config, producers such as RP should call: diff --git a/aks-node-controller/app.go b/aks-node-controller/app.go index 9284a6245d6..67b26128100 100644 --- a/aks-node-controller/app.go +++ b/aks-node-controller/app.go @@ -37,7 +37,8 @@ func isExpectedDiffCSEVar(key string) bool { "SUBNET", "ROUTE_TABLE", "VM_TYPE", - "PROXY_VARS": + "PROXY_VARS", + "INIT_AKS_CLOUD_FILEPATH": return true } return false @@ -102,6 +103,9 @@ type App struct { // grpcDialContext overrides how the gRPC LPS client dials, letting tests point the client at // an in-process (bufconn) server. When nil, the real TLS dial to the apiserver front is used. grpcDialContext func(ctx context.Context, target string) (net.Conn, error) + // Cloud initialization overrides keep provisioning tests independent of the host. + initializeCloud func(context.Context, cloudInitConfig) error + refreshCloudCertificates func(context.Context, string) error } // provision.json values are emitted as strings by the shell jq invocation. @@ -181,6 +185,19 @@ func (a *App) Run(ctx context.Context, args []string) int { return nil }, }, + { + Name: "refresh-cloud-certificates", + Usage: "Refresh Azure cloud CA certificates without configuring repositories", + Flags: []cli.Flag{ + &cli.StringFlag{Name: "location", Required: true, Usage: "Azure region used to select the certificate endpoint"}, + }, + Action: func(ctx context.Context, cmd *cli.Command) error { + if cmd.Args().Len() != 0 { + return fmt.Errorf("unexpected refresh-cloud-certificates arguments: %s", strings.Join(cmd.Args().Slice(), " ")) + } + return a.runCloudCertificateRefresh(ctx, cmd.String("location")) + }, + }, { Name: "download-hotfix", Usage: "Download the requested hotfix binary", @@ -685,6 +702,9 @@ func (a *App) Provision(ctx context.Context, flags ProvisionFlags) (*ProvisionRe cmd, err = buildCmdFromNBCCmd(ctx, flags.NBCCmd) } else if flags.ProvisionConfig != "" && flags.NBCCmd == "" { cmd, err = buildCmdFromProvisionConfig(ctx, flags.ProvisionConfig, a.getGPUComponentsFilePath()) + if err == nil { + err = a.initializeAKSCloud(ctx, envSliceToMap(cmd.Env)) + } } if err != nil { provisionResult.ExitCode = strconv.Itoa(240) @@ -770,6 +790,10 @@ func (a *App) runProvision(ctx context.Context, flags ProvisionFlags, dryRun boo if dryRun { a.cmdRun = cmdRunnerDryRun + a.initializeCloud = func(_ context.Context, config cloudInitConfig) error { + slog.Info("dry-run: initialize Azure cloud before provisioning", "location", config.Location) + return nil + } } provisionResult, provisionErr = a.Provision(ctx, flags) diff --git a/aks-node-controller/app_test.go b/aks-node-controller/app_test.go index 827d9d772ed..6bf7cd54943 100644 --- a/aks-node-controller/app_test.go +++ b/aks-node-controller/app_test.go @@ -190,6 +190,7 @@ func NewTestApp(t *testing.T, cfg TestAppConfig) *TestApp { eventLogger: eventLogger, gpuComponentsFilePath: writeTestGPUComponentsFile(t), readyReportMarkerPath: filepath.Join(t.TempDir(), "experimental_skip_ready_report"), + initializeCloud: func(context.Context, cloudInitConfig) error { return nil }, }, } } diff --git a/aks-node-controller/cloud_certificates.go b/aks-node-controller/cloud_certificates.go new file mode 100644 index 00000000000..e0980c918df --- /dev/null +++ b/aks-node-controller/cloud_certificates.go @@ -0,0 +1,252 @@ +package main + +import ( + "bytes" + "context" + "crypto/x509" + "encoding/json" + "encoding/pem" + "errors" + "fmt" + "log/slog" + "maps" + "net/url" + "os" + "path/filepath" + "sort" + "strings" + "unicode" +) + +func (c *cloudInitializer) refresh(ctx context.Context, platform cloudPlatform, location string) (bool, error) { + legacy := strings.HasPrefix(location, "ussec") || strings.HasPrefix(location, "usnat") + slog.Info("selecting Azure cloud certificate endpoint", "legacy", legacy, "location", location) + if !legacy { + body, err := c.requestCertificates(ctx, "/acms/isOptedInForRootCerts") + if err != nil { + return false, fmt.Errorf("check Azure cloud root certificate opt-in: %w", err) + } + var response struct { + OptedIn *bool `json:"IsOptedInForRootCerts"` + } + if err := json.Unmarshal(body, &response); err != nil { + return false, fmt.Errorf("decode Azure cloud root certificate opt-in: %w", err) + } + if response.OptedIn == nil { + return false, errors.New("cloud root certificate opt-in response is missing IsOptedInForRootCerts") + } + if !*response.OptedIn { + slog.Info("Azure cloud root certificates are not opted in, skipping certificate installation") + return false, nil + } + } + var certificates map[string][]byte + var err error + if legacy { + certificates, err = c.legacyCertificates(ctx) + } else { + certificates, err = c.operationCertificates(ctx) + } + if err != nil { + return false, err + } + if err := c.installCertificates(ctx, platform, certificates); err != nil { + return false, fmt.Errorf("install Azure cloud CA certificates: %w", err) + } + slog.Info("installed Azure cloud CA certificates", "count", len(certificates)) + return true, nil +} + +func certificatePackagePath(kind, extension string) string { + return "/machine?" + url.Values{ + "comp": {"acmspackage"}, + "type": {kind}, + "ext": {extension}, + }.Encode() +} + +func validCloudCertificateName(name string) bool { + return name != "" && name != "." && name != ".." && filepath.Base(name) == name && + !strings.Contains(name, `\`) && strings.IndexFunc(name, unicode.IsControl) == -1 +} + +func (c *cloudInitializer) legacyCertificates(ctx context.Context) (map[string][]byte, error) { + body, err := c.requestCertificates(ctx, certificatePackagePath("cacertificates", "json")) + if err != nil { + return nil, fmt.Errorf("retrieve legacy Azure cloud certificates: %w", err) + } + var response struct { + Certificates []struct { + Name string + Body string `json:"CertBody"` + } + } + if err := json.Unmarshal(body, &response); err != nil { + return nil, fmt.Errorf("decode legacy Azure cloud certificates: %w", err) + } + certificates := make(map[string][]byte) + for _, cert := range response.Certificates { + if !validCloudCertificateName(cert.Name) { + slog.Warn("rejecting invalid Azure cloud certificate filename") + continue + } + name := strings.ReplaceAll(cert.Name, ".cer", ".crt") + data, err := publicCloudCertificates([]byte(cert.Body)) + if err != nil { + return nil, fmt.Errorf("decode legacy Azure cloud certificate %s: %w", name, err) + } + if strings.HasSuffix(name, ".crt") { + certificates[name] = data + } + } + if len(certificates) == 0 { + return nil, errors.New("legacy Azure cloud response contains no installable CA certificates") + } + return certificates, nil +} + +func (c *cloudInitializer) operationCertificates(ctx context.Context) (map[string][]byte, error) { + certificates := make(map[string][]byte) + for _, kind := range []string{"operationrequestsroot", "operationrequestsintermediate"} { + group, err := c.operationCertificateGroup(ctx, kind) + if err != nil { + return nil, err + } + maps.Copy(certificates, group) + } + return certificates, nil +} + +func (c *cloudInitializer) operationCertificateGroup(ctx context.Context, kind string) (map[string][]byte, error) { + body, err := c.requestCertificates(ctx, certificatePackagePath(kind, "json")) + if err != nil { + return nil, fmt.Errorf("retrieve Azure cloud %s: %w", kind, err) + } + var response struct { + Operations []struct { + // ResouceFileName is the spelling used by the WireServer protocol. + Name string `json:"ResouceFileName"` + } `json:"OperationsInfo"` + } + if err := json.Unmarshal(body, &response); err != nil { + return nil, fmt.Errorf("decode Azure cloud %s: %w", kind, err) + } + certificates := make(map[string][]byte) + for _, operation := range response.Operations { + if !validCloudCertificateName(operation.Name) { + slog.Warn("rejecting invalid Azure cloud certificate filename") + continue + } + data, err := c.operationCertificate(ctx, operation.Name) + if err != nil { + return nil, err + } + if strings.HasSuffix(operation.Name, ".crt") { + certificates[operation.Name] = data + } + } + if len(certificates) == 0 { + return nil, fmt.Errorf("cloud %s contains no installable CA certificates", kind) + } + return certificates, nil +} + +func (c *cloudInitializer) operationCertificate(ctx context.Context, filename string) ([]byte, error) { + extension := filepath.Ext(filename) + if extension == "" { + return nil, errors.New("cloud certificate filename is missing an extension") + } + name := strings.TrimSuffix(filename, extension) + body, err := c.requestCertificates(ctx, certificatePackagePath(name, strings.TrimPrefix(extension, "."))) + if err != nil { + return nil, fmt.Errorf("retrieve Azure cloud certificate %s: %w", filename, err) + } + data, err := publicCloudCertificates(body) + if err != nil { + return nil, fmt.Errorf("decode Azure cloud certificate %s: %w", filename, err) + } + return data, nil +} + +func publicCloudCertificates(data []byte) ([]byte, error) { + var result bytes.Buffer + for { + block, rest := pem.Decode(data) + if block == nil { + break + } + data = rest + if block.Type != "CERTIFICATE" { + continue + } + if _, err := x509.ParseCertificate(block.Bytes); err != nil { + return nil, fmt.Errorf("invalid CA certificate: %w", err) + } + if err := pem.Encode(&result, &pem.Block{Type: "CERTIFICATE", Bytes: block.Bytes}); err != nil { + return nil, err + } + } + if result.Len() == 0 { + return nil, errors.New("response contains no PEM certificates") + } + return result.Bytes(), nil +} + +func (c *cloudInitializer) installCertificates(ctx context.Context, platform cloudPlatform, certificates map[string][]byte) error { + destination, update := "/usr/local/share/ca-certificates", "update-ca-certificates" + switch platform.ID { + case osReleaseIDAzureLinux, osReleaseIDMariner, osReleaseIDAzureContainerLinux: + destination, update = "/etc/pki/ca-trust/source/anchors", "update-ca-trust" + case osReleaseIDFlatcar: + destination = "/etc/ssl/certs" + } + names := make([]string, 0, len(certificates)) + for name := range certificates { + names = append(names, name) + } + sort.Strings(names) + for _, name := range names { + data := certificates[name] + if err := c.writeFile("/root/AzureCACertificates/"+name, data); err != nil { + return err + } + targetName := name + if platform.ID == osReleaseIDFlatcar { + targetName = strings.TrimSuffix(name, ".crt") + ".pem" + } + if err := c.writeFile(filepath.Join(destination, targetName), data); err != nil { + return err + } + if os.Getenv("AKS_CLOUD_LOG_CERTIFICATES") == "true" { + // Only re-encoded public certificates reach this opt-in diagnostic. + slog.Info("Azure cloud public CA certificate", "name", name, "certificate", string(data)) + } + } + if _, err := c.command(ctx, nil, update); err != nil { + return err + } + if platform.ID == osReleaseIDUbuntu { + return c.copyOpenSSLBundle() + } + return nil +} + +func (c *cloudInitializer) copyOpenSSLBundle() error { + source, target := c.path("/etc/ssl/certs/ca-certificates.crt"), c.path("/usr/lib/ssl/cert.pem") + sourceInfo, err := os.Stat(source) + if err != nil { + return err + } + targetInfo, err := os.Stat(target) + if err == nil && os.SameFile(sourceInfo, targetInfo) { + return nil + } + if err != nil && !errors.Is(err, os.ErrNotExist) { + return err + } + data, err := os.ReadFile(source) + if err != nil { + return err + } + return c.writeFile("/usr/lib/ssl/cert.pem", data) +} diff --git a/aks-node-controller/cloud_init.go b/aks-node-controller/cloud_init.go new file mode 100644 index 00000000000..2867f3af586 --- /dev/null +++ b/aks-node-controller/cloud_init.go @@ -0,0 +1,406 @@ +package main + +import ( + "bytes" + "context" + "errors" + "fmt" + "io" + "log/slog" + "net/http" + "os" + "os/exec" + "path/filepath" + "strconv" + "strings" + "time" + "unicode" + + "github.com/Azure/agentbaker/aks-node-controller/common" + "github.com/Azure/agentbaker/aks-node-controller/helpers" +) + +const ( + cloudRequestAttempts = 10 + cloudResponseLimit = 8 << 20 + cloudCommandTimeout = 2 * time.Minute + cloudRefreshTimeout = 15 * time.Minute + cloudRefreshService = `[Unit] +Description=Refresh Azure Custom Cloud CA certificates +After=network-online.target +Wants=network-online.target + +[Service] +Type=oneshot +ExecStart=%s +` + cloudRefreshTimer = `[Unit] +Description=Daily refresh of Azure Custom Cloud CA certificates + +[Timer] +OnCalendar=19:00 +Persistent=true +RandomizedDelaySec=300 + +[Install] +WantedBy=timers.target +` +) + +type cloudInitConfig struct { + Location string + RepoDepotEndpoint string + Timeout time.Duration +} + +type cloudPlatform struct { + ID, Version, Codename string +} + +type cloudInitializer struct { + run func(*exec.Cmd) error + wait func(context.Context, time.Duration) error + wireClient *http.Client + repoClient *http.Client + wireEndpoint string + eventLogger *helpers.EventLogger + rootDir string + osReleasePath string + executable string +} + +func newCloudInitializer(a *App) *cloudInitializer { + transport := common.NewBaseTransport(common.HTTPTransportOptions{ + DialTimeout: 10 * time.Second, + TLSHandshakeTimeout: 10 * time.Second, + ResponseHeaderTimeout: 30 * time.Second, + }) + repoTransport := transport.Clone() + repoTransport.Proxy = http.ProxyFromEnvironment + return &cloudInitializer{ + run: a.cmdRun, + wait: waitCloudRetry, + wireEndpoint: wireServerEndpoint, + wireClient: &http.Client{ + Transport: transport, + Timeout: 30 * time.Second, + // WireServer responses must not redirect to another endpoint. + CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse }, + }, + repoClient: &http.Client{Transport: repoTransport, Timeout: 30 * time.Second}, + eventLogger: a.eventLogger, + osReleasePath: a.osReleasePath, + } +} + +func (c *cloudInitializer) close() { + c.wireClient.CloseIdleConnections() + c.repoClient.CloseIdleConnections() +} + +func (a *App) initializeAKSCloud(ctx context.Context, env map[string]string) error { + timeout, err := time.ParseDuration(env["CSE_TIMEOUT"] + "s") + if err != nil || timeout <= 0 { + return fmt.Errorf("invalid cloud initialization timeout %q", env["CSE_TIMEOUT"]) + } + config := cloudInitConfig{ + Location: env["LOCATION"], + RepoDepotEndpoint: env["REPO_DEPOT_ENDPOINT"], + Timeout: timeout, + } + if a.initializeCloud != nil { + return a.initializeCloud(ctx, config) + } + initializer := newCloudInitializer(a) + defer initializer.close() + return initializer.initialize(ctx, config) +} + +func (a *App) runCloudCertificateRefresh(ctx context.Context, location string) error { + if strings.TrimSpace(location) == "" { + return errors.New("cloud certificate refresh requires a location") + } + var err error + if a.refreshCloudCertificates != nil { + err = a.refreshCloudCertificates(ctx, location) + } else { + initializer := newCloudInitializer(a) + defer initializer.close() + err = initializer.refreshCertificates(ctx, location) + } + if err != nil { + slog.Error("Azure cloud certificate refresh failed", "error", err) + } + return err +} + +func (c *cloudInitializer) initialize(ctx context.Context, config cloudInitConfig) error { + initCtx, cancel := context.WithTimeout(ctx, config.Timeout) + defer cancel() + return c.withEvent("CloudInitialization", func() error { + location, err := normalizeCloudLocation(config.Location) + if err != nil { + return err + } + platform, err := c.platform() + if err != nil { + return err + } + eligible, err := c.refresh(initCtx, platform, location) + if err != nil { + return err + } + if eligible { + if err := c.installRefreshSchedule(initCtx, platform, location); err != nil { + return fmt.Errorf("schedule Azure cloud certificate refresh: %w", err) + } + } + return c.withEvent("CloudRepositoryInitialization", func() error { + return c.initializeRepositories(initCtx, platform, config.RepoDepotEndpoint) + }) + }) +} + +func (c *cloudInitializer) refreshCertificates(ctx context.Context, location string) error { + refreshCtx, cancel := context.WithTimeout(ctx, cloudRefreshTimeout) + defer cancel() + return c.withEvent("CloudCertificateRefresh", func() error { + normalizedLocation, err := normalizeCloudLocation(location) + if err != nil { + return err + } + platform, err := c.platform() + if err != nil { + return err + } + _, err = c.refresh(refreshCtx, platform, normalizedLocation) + return err + }) +} + +func normalizeCloudLocation(location string) (string, error) { + location = strings.ToLower(strings.Map(func(r rune) rune { + if unicode.IsSpace(r) { + return -1 + } + return r + }, location)) + if strings.IndexFunc(location, func(r rune) bool { + return !strings.ContainsRune("abcdefghijklmnopqrstuvwxyz0123456789-", r) + }) != -1 { + return "", errors.New("cloud location contains unsupported characters") + } + if location == "" { + slog.Warn("Azure cloud location is empty, using rcv1p certificate endpoints") + } + return location, nil +} + +func (c *cloudInitializer) platform() (cloudPlatform, error) { + path := c.osReleasePath + if path == "" { + path = c.path(defaultOSReleasePath) + } + data, err := os.ReadFile(path) + if err != nil { + return cloudPlatform{}, fmt.Errorf("read cloud initialization OS release: %w", err) + } + values := parseNodeCustomDataOSRelease(data) + id := strings.ToLower(values["ID"]) + switch { + case strings.Contains(values["NAME"], "Ubuntu"), id == osReleaseIDUbuntu: + id = osReleaseIDUbuntu + case strings.Contains(id, osReleaseIDFlatcar): + id = osReleaseIDFlatcar + case id == osReleaseIDAzureContainerLinux, + id == osReleaseIDAzureLinux && values["VARIANT_ID"] == osReleaseIDAzureContainerLinux: + id = osReleaseIDAzureContainerLinux + case strings.Contains(values["NAME"], "Mariner"), id == osReleaseIDMariner: + id = osReleaseIDMariner + case strings.Contains(values["NAME"], "Microsoft Azure Linux"), id == osReleaseIDAzureLinux: + id = osReleaseIDAzureLinux + default: + return cloudPlatform{}, fmt.Errorf("unsupported cloud initialization OS %q", id) + } + return cloudPlatform{ID: id, Version: values["VERSION_ID"], Codename: values["VERSION_CODENAME"]}, nil +} + +func (c *cloudInitializer) path(path string) string { + if c.rootDir == "" { + return path + } + return filepath.Join(c.rootDir, strings.TrimPrefix(path, "/")) +} + +func (c *cloudInitializer) writeFile(path string, data []byte) error { + return writeCloudFile(c.path(path), data) +} + +func writeCloudFile(path string, data []byte) error { + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + return err + } + root, err := os.OpenRoot(filepath.Dir(path)) + if err != nil { + return err + } + defer root.Close() + // #nosec G306 -- Public CAs, package keys, and repository metadata must be readable by unprivileged clients. + return root.WriteFile(filepath.Base(path), data, 0o644) +} + +func (c *cloudInitializer) withEvent(task string, action func() error) error { + start := time.Now() + err := action() + level, message := helpers.EventLevelInformational, "Completed" + if err != nil { + level, message = helpers.EventLevelError, err.Error() + slog.Error("Azure cloud operation failed", "task", task, "error", err) + } + if c.eventLogger != nil { + c.eventLogger.LogEvent(task, message, level, start, time.Now()) + } + return err +} + +func (c *cloudInitializer) command(ctx context.Context, input []byte, name string, args ...string) ([]byte, error) { + if err := ctx.Err(); err != nil { + return nil, err + } + ctx, cancel := context.WithTimeout(ctx, cloudCommandTimeout) + defer cancel() + cmd := exec.CommandContext(ctx, name, args...) + cmd.Env = append(os.Environ(), "LC_ALL=C") + cmd.Stdin = bytes.NewReader(input) + var stdout, stderr bytes.Buffer + cmd.Stdout, cmd.Stderr = &stdout, &stderr + run := c.run + if run == nil { + run = cmdRunner + } + if err := run(cmd); err != nil { + return stdout.Bytes(), fmt.Errorf("%s: %w: %s", name, err, strings.TrimSpace(stderr.String())) + } + if name == "dnf" { + // DNF can report unavailable repositories on stderr despite exiting zero. + _, _ = stdout.Write(stderr.Bytes()) + } + return stdout.Bytes(), nil +} + +func waitCloudRetry(ctx context.Context, delay time.Duration) error { + timer := time.NewTimer(delay) + defer timer.Stop() + select { + case <-ctx.Done(): + return ctx.Err() + case <-timer.C: + return nil + } +} + +func readCloudResponse(resp *http.Response) ([]byte, error) { + defer resp.Body.Close() + body, err := io.ReadAll(io.LimitReader(resp.Body, cloudResponseLimit+1)) + if err != nil { + return nil, err + } + if len(body) > cloudResponseLimit { + return nil, errors.New("cloud response exceeds size limit") + } + return body, nil +} + +func (c *cloudInitializer) requestCertificates(ctx context.Context, path string) ([]byte, error) { + attempt := 0 + delay, nextDelay := 3*time.Second, time.Duration(0) + return common.RetryFetch(ctx, cloudRequestAttempts, nil, func(ctx context.Context) ([]byte, error) { + if attempt > 0 { + if err := c.wait(ctx, nextDelay); err != nil { + return nil, err + } + } + attempt++ + req, err := http.NewRequestWithContext(ctx, http.MethodGet, c.wireEndpoint+path, nil) + if err != nil { + return nil, err + } + nextDelay = delay + resp, err := c.wireClient.Do(req) + if err != nil { + return nil, err + } + body, err := readCloudResponse(resp) + if err != nil { + return nil, err + } + if resp.StatusCode >= 200 && resp.StatusCode < 300 { + return body, nil + } + if resp.StatusCode == http.StatusForbidden && bytes.Contains(body, []byte("RequestRateLimitExceeded")) { + delay *= 2 + } + return nil, fmt.Errorf("WireServer certificate request %s returned HTTP %d", path, resp.StatusCode) + }) +} + +func (c *cloudInitializer) installRefreshSchedule(ctx context.Context, platform cloudPlatform, location string) error { + if location == "" { + return errors.New("cloud certificate refresh requires a location") + } + binary := c.executable + if binary == "" { + var err error + binary, err = os.Executable() + if err != nil { + return err + } + } + if !filepath.IsAbs(binary) || strings.ContainsAny(binary, "\"\\%\r\n") { + return errors.New("invalid executable path for Azure cloud certificate refresh") + } + // Use the running binary, not the VHD binary: an ANC hotfix can add this + // command on images whose baked-in binary does not support it yet. + command := strconv.Quote(binary) + " refresh-cloud-certificates --location " + strconv.Quote(location) + if platform.ID == osReleaseIDFlatcar || platform.ID == osReleaseIDAzureContainerLinux { + return c.installSystemdRefresh(ctx, command) + } + return c.installCronRefresh(ctx, command) +} + +func (c *cloudInitializer) installSystemdRefresh(ctx context.Context, command string) error { + if err := c.writeFile("/etc/systemd/system/azure-ca-refresh.service", []byte(fmt.Sprintf(cloudRefreshService, command))); err != nil { + return err + } + if err := c.writeFile("/etc/systemd/system/azure-ca-refresh.timer", []byte(cloudRefreshTimer)); err != nil { + return err + } + if _, err := c.command(ctx, nil, "systemctl", "daemon-reload"); err != nil { + return err + } + _, err := c.command(ctx, nil, "systemctl", "enable", "--now", "azure-ca-refresh.timer") + return err +} + +func (c *cloudInitializer) installCronRefresh(ctx context.Context, command string) error { + existing, err := c.command(ctx, nil, "crontab", "-l") + if err != nil { + var exitErr interface{ ExitCode() int } + if !errors.As(err, &exitErr) || exitErr.ExitCode() != 1 || !strings.Contains(err.Error(), "no crontab for") { + return err + } + existing = nil + } + var lines []string + for _, line := range strings.Split(string(existing), "\n") { + if strings.TrimSpace(line) == "" || + strings.Contains(line, "init-aks-cloud.sh") && strings.Contains(line, "ca-refresh") || + strings.Contains(line, "refresh-cloud-certificates") && strings.Contains(line, "aks-node-controller") { + continue + } + lines = append(lines, line) + } + lines = append(lines, "0 19 * * * "+command) + _, err = c.command(ctx, []byte(strings.Join(lines, "\n")+"\n"), "crontab", "-") + return err +} diff --git a/aks-node-controller/cloud_init_test.go b/aks-node-controller/cloud_init_test.go new file mode 100644 index 00000000000..a37720d9aa1 --- /dev/null +++ b/aks-node-controller/cloud_init_test.go @@ -0,0 +1,556 @@ +package main + +import ( + "context" + "crypto/ed25519" + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" + "encoding/json" + "encoding/pem" + "errors" + "fmt" + "io" + "math/big" + "net/http" + "net/http/httptest" + "os" + "os/exec" + "path/filepath" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/Azure/agentbaker/aks-node-controller/helpers" + "github.com/stretchr/testify/require" +) + +const cloudTestUbuntu = "ID=ubuntu\nNAME=Ubuntu\nVERSION_ID=22.04\nVERSION_CODENAME=jammy\n" + +func newTestCloudInitializer(t *testing.T, release string) *cloudInitializer { + t.Helper() + c := newCloudInitializer(&App{ + cmdRun: func(cmd *exec.Cmd) error { + if filepath.Base(cmd.Path) == "gpg" { + _, _ = io.WriteString(cmd.Stdout, "dearmored-key") + } + return nil + }, + eventLogger: helpers.NewEventLogger(t.TempDir()), + }) + c.rootDir = t.TempDir() + c.executable = hotfixBinaryPath + c.wait = func(ctx context.Context, _ time.Duration) error { return ctx.Err() } + require.NoError(t, c.writeFile(defaultOSReleasePath, []byte(release))) + require.NoError(t, c.writeFile("/etc/ssl/certs/ca-certificates.crt", []byte("system-ca-bundle"))) + t.Cleanup(c.close) + return c +} + +func cloudTestCertificate(t *testing.T) []byte { + t.Helper() + public, private, err := ed25519.GenerateKey(rand.Reader) + require.NoError(t, err) + template := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "ANC cloud initialization test CA"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + IsCA: true, + BasicConstraintsValid: true, + KeyUsage: x509.KeyUsageCertSign, + } + der, err := x509.CreateCertificate(rand.Reader, template, template, public, private) + require.NoError(t, err) + return pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}) +} + +func serveCloudTestResponses(t *testing.T, c *cloudInitializer, cert []byte, optIn string) <-chan string { + t.Helper() + requests := make(chan string, 100) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests <- r.URL.Path + "?" + r.URL.RawQuery + switch kind := r.URL.Query().Get("type"); { + case r.URL.Path == "/acms/isOptedInForRootCerts": + _, _ = io.WriteString(w, optIn) + case kind == "cacertificates": + _ = json.NewEncoder(w).Encode(map[string]any{"Certificates": []map[string]string{ + {"Name": "root.cer", "CertBody": strings.ReplaceAll(string(cert), "\n", "\r\n")}, + }}) + case kind == "operationrequestsroot", kind == "operationrequestsintermediate": + name := "root.crt" + if kind == "operationrequestsintermediate" { + name = "intermediate.crt" + } + _ = json.NewEncoder(w).Encode(map[string]any{"OperationsInfo": []map[string]string{{"ResouceFileName": name}}}) + case kind == "root", kind == "intermediate": + _, _ = w.Write(cert) + case strings.HasPrefix(r.URL.Path, "/keys/"): + _, _ = io.WriteString(w, "armored-public-package-key") + case strings.HasPrefix(r.URL.Path, "/microsoft/ubuntu/"): + _, _ = io.WriteString(w, "repository available") + default: + t.Errorf("unexpected cloud request: %s", r.URL) + w.WriteHeader(http.StatusNotFound) + } + })) + t.Cleanup(server.Close) + c.wireEndpoint = server.URL + c.wireClient, c.repoClient = server.Client(), server.Client() + return requests +} + +func TestCloudFileWrites(t *testing.T) { + t.Run("create and replace", func(t *testing.T) { + path := filepath.Join(t.TempDir(), "certificates", "root.crt") + require.NoError(t, writeCloudFile(path, []byte("original"))) + require.NoError(t, writeCloudFile(path, []byte("replacement"))) + data, err := os.ReadFile(path) + require.NoError(t, err) + require.Equal(t, "replacement", string(data)) + }) + t.Run("reject symlinks outside the destination directory", func(t *testing.T) { + directory := t.TempDir() + outside := filepath.Join(t.TempDir(), "outside.crt") + require.NoError(t, os.WriteFile(outside, []byte("original"), 0o600)) + path := filepath.Join(directory, "root.crt") + require.NoError(t, os.Symlink(outside, path)) + require.Error(t, writeCloudFile(path, []byte("replacement"))) + data, err := os.ReadFile(outside) + require.NoError(t, err) + require.Equal(t, "original", string(data)) + }) +} + +func TestCloudPlatform(t *testing.T) { + for _, tc := range []struct { + name, release, want string + }{ + {"Ubuntu", cloudTestUbuntu, osReleaseIDUbuntu}, + {"Mariner", "ID=mariner\nNAME=CBL-Mariner\nVERSION_ID=2.0\n", "mariner"}, + {"AzureLinux", "ID=azurelinux\nNAME=\"Microsoft Azure Linux\"\nVERSION_ID=3.0\n", osReleaseIDAzureLinux}, + {"OSGuard", "ID=azurelinux\nVARIANT_ID=osguard\nVERSION_ID=3.0\n", osReleaseIDAzureLinux}, + {"Flatcar", "ID=flatcar\n", osReleaseIDFlatcar}, + {"ACL", "ID=azurecontainerlinux\n", osReleaseIDAzureContainerLinux}, + {"ACL variant", "ID=azurelinux\nVARIANT_ID=azurecontainerlinux\n", osReleaseIDAzureContainerLinux}, + {"unsupported", "ID=debian\n", ""}, + {"missing ID", "", ""}, + } { + t.Run(tc.name, func(t *testing.T) { + c := newTestCloudInitializer(t, tc.release) + platform, err := c.platform() + if tc.want == "" { + require.Error(t, err) + } else { + require.NoError(t, err) + require.Equal(t, tc.want, platform.ID) + } + }) + } +} + +func TestNormalizeCloudLocation(t *testing.T) { + location, err := normalizeCloudLocation(" US Sec East \t\n") + require.NoError(t, err) + require.Equal(t, "usseceast", location) + for _, value := range []string{`westus";echo bad`, "westus%bad", "westus\\bad"} { + _, err = normalizeCloudLocation(value) + require.Error(t, err) + } +} + +func TestCloudInitializationCertificateModes(t *testing.T) { + cert := cloudTestCertificate(t) + for _, osCase := range []struct{ name, release, trustDir, update string }{ + {"Ubuntu", cloudTestUbuntu, "/usr/local/share/ca-certificates", "update-ca-certificates"}, + {"Mariner", "ID=mariner\n", "/etc/pki/ca-trust/source/anchors", "update-ca-trust"}, + {"AzureLinux", "ID=azurelinux\n", "/etc/pki/ca-trust/source/anchors", "update-ca-trust"}, + {"Flatcar", "ID=flatcar\n", "/etc/ssl/certs", "update-ca-certificates"}, + {"ACL", "ID=azurecontainerlinux\n", "/etc/pki/ca-trust/source/anchors", "update-ca-trust"}, + } { + for _, mode := range []struct { + name, location, optIn string + eligible bool + }{ + {"not opted in", "westus2", `{"IsOptedInForRootCerts":false}`, false}, + {"rcv1p", "westus2", `{"IsOptedInForRootCerts":true}`, true}, + {"legacy", " US NAT East ", "not used", true}, + } { + t.Run(osCase.name+"/"+mode.name, func(t *testing.T) { + c := newTestCloudInitializer(t, osCase.release) + requests := serveCloudTestResponses(t, c, cert, mode.optIn) + var commands []string + run := c.run + c.run = func(cmd *exec.Cmd) error { + commands = append(commands, strings.Join(cmd.Args, " ")) + return run(cmd) + } + err := c.initialize(t.Context(), cloudInitConfig{Location: mode.location, Timeout: time.Minute}) + require.NoError(t, err) + if !mode.eligible { + require.Empty(t, commands) + require.Len(t, requests, 1) + return + } + require.Equal(t, osCase.update, commands[0]) + filename := "root.crt" + if osCase.name == "Flatcar" { + filename = "root.pem" + } + got, err := os.ReadFile(c.path(filepath.Join(osCase.trustDir, filename))) + require.NoError(t, err) + require.Equal(t, cert, got) + assertCloudSchedule(t, c, osCase.name, commands) + if mode.name == "legacy" { + require.Len(t, requests, 1) + } else { + require.Len(t, requests, 5) + } + }) + } + } +} + +func assertCloudSchedule(t *testing.T, c *cloudInitializer, osName string, commands []string) { + t.Helper() + if osName != "Flatcar" && osName != "ACL" { + require.Contains(t, commands, "crontab -") + return + } + require.Contains(t, commands, "systemctl enable --now azure-ca-refresh.timer") + service, err := os.ReadFile(c.path("/etc/systemd/system/azure-ca-refresh.service")) + require.NoError(t, err) + require.Contains(t, string(service), hotfixBinaryPath) + require.Contains(t, string(service), "refresh-cloud-certificates") + require.NotContains(t, string(service), "init-aks-cloud.sh") +} + +func TestCloudCertificateOptInFailsClosed(t *testing.T) { + for _, response := range []string{"not json", `{}`, `null`, `{"IsOptedInForRootCerts":null}`, `{"IsOptedInForRootCerts":"false"}`} { + t.Run(response, func(t *testing.T) { + c := newTestCloudInitializer(t, cloudTestUbuntu) + serveCloudTestResponses(t, c, nil, response) + c.run = func(*exec.Cmd) error { + t.Fatal("must not update trust or schedules after an invalid opt-in response") + return nil + } + require.Error(t, c.initialize(t.Context(), cloudInitConfig{Location: "westus2", Timeout: time.Minute})) + events := c.eventLogger.Events() + require.NotEmpty(t, events) + require.Equal(t, string(helpers.EventLevelError), events[len(events)-1].EventLevel) + }) + } +} + +func TestCloudCertificateRequestRetryAndCancellation(t *testing.T) { + for _, tc := range []struct { + name string + status int + body string + failures int32 + wantWaits []time.Duration + }{ + {"transient", http.StatusServiceUnavailable, "do-not-log-body", 2, []time.Duration{3 * time.Second, 3 * time.Second}}, + {"rate limited", http.StatusForbidden, "RequestRateLimitExceeded", 2, []time.Duration{3 * time.Second, 6 * time.Second}}, + {"exhausted", http.StatusServiceUnavailable, "do-not-log-body", cloudRequestAttempts, nil}, + } { + t.Run(tc.name, func(t *testing.T) { + c := newTestCloudInitializer(t, cloudTestUbuntu) + var attempts atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + if attempts.Add(1) <= tc.failures { + w.WriteHeader(tc.status) + _, _ = io.WriteString(w, tc.body) + return + } + _, _ = io.WriteString(w, "success") + })) + defer server.Close() + c.wireEndpoint = server.URL + var waits []time.Duration + c.wait = func(_ context.Context, delay time.Duration) error { + waits = append(waits, delay) + return nil + } + body, err := c.requestCertificates(t.Context(), "/test") + if tc.failures == cloudRequestAttempts { + require.Error(t, err) + require.NotContains(t, err.Error(), tc.body) + require.EqualValues(t, cloudRequestAttempts, attempts.Load()) + require.Len(t, waits, cloudRequestAttempts-1) + } else { + require.NoError(t, err) + require.Equal(t, "success", string(body)) + require.Equal(t, tc.wantWaits, waits) + } + }) + } + t.Run("cancellation during backoff", func(t *testing.T) { + c := newTestCloudInitializer(t, cloudTestUbuntu) + var attempts atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + attempts.Add(1) + w.WriteHeader(http.StatusServiceUnavailable) + })) + defer server.Close() + c.wireEndpoint = server.URL + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + c.wait = func(ctx context.Context, _ time.Duration) error { + cancel() + return ctx.Err() + } + _, err := c.requestCertificates(ctx, "/test") + require.ErrorIs(t, err, context.Canceled) + require.EqualValues(t, 1, attempts.Load()) + }) +} + +func TestCloudCertificateRequestsDoNotFollowRedirects(t *testing.T) { + c := newTestCloudInitializer(t, cloudTestUbuntu) + var redirected atomic.Int32 + target := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { redirected.Add(1) })) + defer target.Close() + source := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, target.URL, http.StatusFound) + })) + defer source.Close() + c.wireEndpoint = source.URL + _, err := c.requestCertificates(t.Context(), "/test") + require.ErrorContains(t, err, "HTTP 302") + require.Zero(t, redirected.Load()) +} + +func TestCloudInitializationHonorsConfiguredTimeout(t *testing.T) { + c := newTestCloudInitializer(t, cloudTestUbuntu) + server := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + <-r.Context().Done() + })) + defer server.Close() + c.wireEndpoint = server.URL + start := time.Now() + err := c.initialize(t.Context(), cloudInitConfig{Location: "westus2", Timeout: 25 * time.Millisecond}) + require.ErrorIs(t, err, context.DeadlineExceeded) + require.Less(t, time.Since(start), time.Second) +} + +func TestCloudCertificateResourceNamesAreURLEncoded(t *testing.T) { + c := newTestCloudInitializer(t, "ID=azurelinux\n") + cert := cloudTestCertificate(t) + var fetched atomic.Bool + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch kind := r.URL.Query().Get("type"); { + case r.URL.Path == "/acms/isOptedInForRootCerts": + _, _ = io.WriteString(w, `{"IsOptedInForRootCerts":true}`) + case kind == "operationrequestsroot": + _, _ = io.WriteString(w, `{"OperationsInfo":[{"ResouceFileName":"root+ca.crt"}]}`) + case kind == "operationrequestsintermediate": + _, _ = io.WriteString(w, `{"OperationsInfo":[{"ResouceFileName":"intermediate.crt"}]}`) + case kind == "root+ca", kind == "intermediate": + fetched.Store(true) + _, _ = w.Write(cert) + default: + t.Errorf("incorrectly encoded certificate resource: %s", r.URL) + w.WriteHeader(http.StatusBadRequest) + } + })) + defer server.Close() + c.wireEndpoint = server.URL + require.NoError(t, c.refreshCertificates(t.Context(), "westus2")) + require.True(t, fetched.Load()) + installed, err := os.ReadFile(c.path("/etc/pki/ca-trust/source/anchors/root+ca.crt")) + require.NoError(t, err) + require.Equal(t, cert, installed) +} + +func TestCloudCertificateResponseLimits(t *testing.T) { + _, err := readCloudResponse(&http.Response{ + Body: io.NopCloser(strings.NewReader(strings.Repeat("x", cloudResponseLimit+1))), + }) + require.ErrorContains(t, err, "size limit") +} + +func TestCloudCertificateNamesAndPublicContent(t *testing.T) { + for _, name := range []string{"", ".", "..", "../root.crt", "/root.crt", `..\root.crt`, "root\x00.crt", "root\n.crt"} { + require.False(t, validCloudCertificateName(name), name) + } + require.True(t, validCloudCertificateName("root+ca.crt")) + cert := cloudTestCertificate(t) + private := pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: []byte("not-a-real-private-key")}) + got, err := publicCloudCertificates(append(private, cert...)) + require.NoError(t, err) + require.Equal(t, cert, got) + for _, bad := range [][]byte{nil, private, pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: []byte("invalid")})} { + _, err = publicCloudCertificates(bad) + require.Error(t, err) + } +} + +func TestCloudCertificateLoggingIsOptInAndPublicOnly(t *testing.T) { + for _, enabled := range []string{"false", "true"} { + t.Run(enabled, func(t *testing.T) { + t.Setenv("AKS_CLOUD_LOG_CERTIFICATES", enabled) + logs := installLogCapturer(t) + c := newTestCloudInitializer(t, "ID=azurelinux\n") + cert := cloudTestCertificate(t) + secret := pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: []byte("not-a-real-private-key")}) + serveCloudTestResponses(t, c, append(secret, cert...), `{"IsOptedInForRootCerts":true}`) + require.NoError(t, c.refreshCertificates(t.Context(), "westus2")) + found := false + for _, record := range logs.getRecords() { + for _, value := range record.Attrs { + require.NotContains(t, value, "PRIVATE KEY") + if strings.Contains(value, "BEGIN CERTIFICATE") { + found = true + } + } + } + require.Equal(t, enabled == "true", found) + got, err := os.ReadFile(c.path("/etc/pki/ca-trust/source/anchors/root.crt")) + require.NoError(t, err) + require.Equal(t, cert, got) + }) + } +} + +func TestCloudCertificateInvalidOperationsDoNotInstall(t *testing.T) { + for _, response := range []string{ + `{}`, `{"OperationsInfo":[]}`, `{"OperationsInfo":[{"ResouceFileName":"../escape.crt"}]}`, + `{"OperationsInfo":[{"ResouceFileName":"/escape.crt"}]}`, `{"OperationsInfo":[{"ResouceFileName":"missing-extension"}]}`, + } { + t.Run(response, func(t *testing.T) { + c := newTestCloudInitializer(t, "ID=azurelinux\n") + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/acms/isOptedInForRootCerts" { + _, _ = io.WriteString(w, `{"IsOptedInForRootCerts":true}`) + } else { + _, _ = io.WriteString(w, response) + } + })) + defer server.Close() + c.wireEndpoint = server.URL + c.run = func(*exec.Cmd) error { + t.Fatal("invalid operations must not install certificates") + return nil + } + require.Error(t, c.refreshCertificates(t.Context(), "westus2")) + _, err := os.Stat(c.path("/etc/pki/ca-trust/source/anchors")) + require.ErrorIs(t, err, os.ErrNotExist) + }) + } +} + +func TestCloudCertificateUbuntuBundleSymlink(t *testing.T) { + c := newTestCloudInitializer(t, cloudTestUbuntu) + require.NoError(t, os.MkdirAll(c.path("/usr/lib/ssl"), 0o755)) + require.NoError(t, os.Symlink(c.path("/etc/ssl/certs/ca-certificates.crt"), c.path("/usr/lib/ssl/cert.pem"))) + require.NoError(t, c.copyOpenSSLBundle()) + info, err := os.Lstat(c.path("/usr/lib/ssl/cert.pem")) + require.NoError(t, err) + require.NotZero(t, info.Mode()&os.ModeSymlink) +} + +func TestCloudRefreshScheduleReplacesLegacyCron(t *testing.T) { + c := newTestCloudInitializer(t, cloudTestUbuntu) + cron := "5 2 * * * /usr/bin/keep-me\n" + + "0 19 * * * \"/opt/azure/containers/init-aks-cloud.sh\" ca-refresh \"usnateast\"\n" + c.run = func(cmd *exec.Cmd) error { + switch strings.Join(cmd.Args, " ") { + case "crontab -l": + _, _ = io.WriteString(cmd.Stdout, cron) + case "crontab -": + data, err := io.ReadAll(cmd.Stdin) + require.NoError(t, err) + cron = string(data) + default: + t.Fatalf("unexpected scheduling command: %s", cmd) + } + return nil + } + for range 2 { + require.NoError(t, c.installRefreshSchedule(t.Context(), cloudPlatform{ID: osReleaseIDUbuntu}, "usnateast")) + } + require.Contains(t, cron, "5 2 * * * /usr/bin/keep-me") + require.NotContains(t, cron, "init-aks-cloud.sh") + require.Equal(t, 1, strings.Count(cron, "refresh-cloud-certificates")) + require.Contains(t, cron, fmt.Sprintf(`0 19 * * * "%s" refresh-cloud-certificates --location "usnateast"`, hotfixBinaryPath)) +} + +func TestCloudRefreshScheduleCrontabErrors(t *testing.T) { + for _, tc := range []struct { + name, stderr string + wantError bool + }{ + {"no existing crontab", "no crontab for root", false}, + {"permission denied", "permission denied", true}, + } { + t.Run(tc.name, func(t *testing.T) { + c := newTestCloudInitializer(t, cloudTestUbuntu) + writes := 0 + c.run = func(cmd *exec.Cmd) error { + if cmd.Args[1] == "-l" { + _, _ = io.WriteString(cmd.Stderr, tc.stderr) + return &testExitError{Code: 1} + } + writes++ + return nil + } + err := c.installRefreshSchedule(t.Context(), cloudPlatform{ID: osReleaseIDUbuntu}, "westus2") + if tc.wantError { + require.Error(t, err) + require.Zero(t, writes) + } else { + require.NoError(t, err) + require.Equal(t, 1, writes) + } + }) + } +} + +func TestCloudInitializationStopsOnTrustAndScheduleFailures(t *testing.T) { + for _, tc := range []struct { + name, location, failedCommand, wantError string + }{ + {"trust store", "westus2", "update-ca-trust", "host operation failed"}, + {"crontab", "westus2", "crontab", "host operation failed"}, + {"missing location", " \t", "", "requires a location"}, + } { + t.Run(tc.name, func(t *testing.T) { + c := newTestCloudInitializer(t, "ID=azurelinux\n") + serveCloudTestResponses(t, c, cloudTestCertificate(t), `{"IsOptedInForRootCerts":true}`) + c.run = func(cmd *exec.Cmd) error { + if cmd.Args[0] == tc.failedCommand { + return errors.New("host operation failed") + } + require.NotEqual(t, "dnf", cmd.Args[0]) + if tc.failedCommand == "" { + require.NotEqual(t, "crontab", cmd.Args[0]) + } + return nil + } + err := c.initialize(t.Context(), cloudInitConfig{ + Location: tc.location, RepoDepotEndpoint: "https://repo.example/ubuntu", Timeout: time.Minute, + }) + require.ErrorContains(t, err, tc.wantError) + }) + } +} + +func TestCloudPeriodicRefreshDoesNotInitializeRepositoriesOrSchedules(t *testing.T) { + c := newTestCloudInitializer(t, cloudTestUbuntu) + requests := serveCloudTestResponses(t, c, cloudTestCertificate(t), `{"IsOptedInForRootCerts":true}`) + var commands []string + c.run = func(cmd *exec.Cmd) error { + commands = append(commands, strings.Join(cmd.Args, " ")) + return nil + } + require.NoError(t, c.refreshCertificates(t.Context(), "westus2")) + require.Equal(t, []string{"update-ca-certificates"}, commands) + require.Len(t, requests, 5) + _, err := os.Stat(c.path(defaultAptSourcesDir)) + require.ErrorIs(t, err, os.ErrNotExist) + events := c.eventLogger.Events() + require.Len(t, events, 1) + require.Equal(t, "AKS.AKSNodeController.CloudCertificateRefresh", events[0].TaskName) +} diff --git a/aks-node-controller/cloud_provision_test.go b/aks-node-controller/cloud_provision_test.go new file mode 100644 index 00000000000..db16756978a --- /dev/null +++ b/aks-node-controller/cloud_provision_test.go @@ -0,0 +1,158 @@ +package main + +import ( + "context" + "errors" + "os" + "os/exec" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func writeCloudProvisionConfig(t *testing.T) string { + t.Helper() + path := filepath.Join(t.TempDir(), "config.json") + require.NoError(t, os.WriteFile(path, []byte(`{`+requiredProvisionFields+`"version":"v1"}`), 0o600)) + return path +} + +func TestNativeCloudInitializationPrecedesProvisioning(t *testing.T) { + var order []string + tt := NewTestApp(t, TestAppConfig{RunFunc: func(*exec.Cmd) error { + order = append(order, "provision") + return nil + }}) + tt.App.initializeCloud = func(_ context.Context, config cloudInitConfig) error { + order = append(order, "initialize") + require.Equal(t, "westus2", config.Location) + require.Equal(t, 15*time.Minute, config.Timeout) + return nil + } + _, err := tt.App.Provision(t.Context(), ProvisionFlags{ProvisionConfig: writeCloudProvisionConfig(t)}) + require.NoError(t, err) + require.Equal(t, []string{"initialize", "provision"}, order) +} + +func TestNativeCloudInitializationInstallsCertificatesBeforeProvisioning(t *testing.T) { + c := newTestCloudInitializer(t, cloudTestUbuntu) + cert := cloudTestCertificate(t) + serveCloudTestResponses(t, c, cert, `{"IsOptedInForRootCerts":true}`) + called := false + tt := NewTestApp(t, TestAppConfig{RunFunc: func(*exec.Cmd) error { + called = true + installed, err := os.ReadFile(c.path("/usr/local/share/ca-certificates/root.crt")) + require.NoError(t, err) + require.Equal(t, cert, installed) + return nil + }}) + tt.App.initializeCloud = c.initialize + _, err := tt.App.Provision(t.Context(), ProvisionFlags{ProvisionConfig: writeCloudProvisionConfig(t)}) + require.NoError(t, err) + require.True(t, called) +} + +func TestNativeCloudInitializationFailureReportsWithoutRunningScripts(t *testing.T) { + tt := NewTestApp(t, TestAppConfig{RunFunc: func(*exec.Cmd) error { + t.Fatal("provisioning must not run after cloud initialization fails") + return nil + }}) + tt.App.initializeCloud = func(context.Context, cloudInitConfig) error { return errors.New("cloud initialization failed") } + require.NoError(t, os.WriteFile(tt.App.readyReportMarkerPath, nil, 0o600)) + reported := false + tt.App.goalStateReporter = func(_ context.Context, failed bool, description string) error { + reported = true + require.True(t, failed) + require.Contains(t, description, "cloud initialization failed") + return nil + } + result, err := tt.App.runProvision(t.Context(), ProvisionFlags{ProvisionConfig: writeCloudProvisionConfig(t)}, false) + require.ErrorContains(t, err, "cloud initialization failed") + require.Equal(t, "240", result.ExitCode) + require.Equal(t, err.Error(), result.Error) + require.True(t, reported) +} + +func TestNativeCloudInitializationIsNotUsedForNBC(t *testing.T) { + for _, compare := range []bool{false, true} { + t.Run(map[bool]string{false: "phase2", true: "phase2.5"}[compare], func(t *testing.T) { + tt := NewTestApp(t, TestAppConfig{}) + tt.App.initializeCloud = func(context.Context, cloudInitConfig) error { + t.Fatal("NBC must retain shell initialization") + return nil + } + path := filepath.Join(t.TempDir(), "nbc-cmd.sh") + require.NoError(t, os.WriteFile(path, []byte("#!/bin/bash\ntrue\n"), 0o600)) + flags := ProvisionFlags{NBCCmd: path} + if compare { + flags.ProvisionConfig = writeCloudProvisionConfig(t) + } + _, err := tt.App.Provision(t.Context(), flags) + require.NoError(t, err) + }) + } +} + +func TestNativeCloudInitializationDryRunHasNoSideEffects(t *testing.T) { + tt := NewTestApp(t, TestAppConfig{RunFunc: func(*exec.Cmd) error { + t.Fatal("dry-run must not run commands") + return nil + }}) + tt.App.initializeCloud = func(context.Context, cloudInitConfig) error { + t.Fatal("dry-run must not fetch certificates or modify the host") + return nil + } + _, err := tt.App.runProvision(t.Context(), ProvisionFlags{ProvisionConfig: writeCloudProvisionConfig(t)}, true) + require.NoError(t, err) +} + +func TestNativeCloudInitializationConfigEnvironment(t *testing.T) { + tt := NewTestApp(t, TestAppConfig{}) + tt.App.initializeCloud = func(_ context.Context, config cloudInitConfig) error { + require.Equal(t, cloudInitConfig{ + Location: "usnateast", RepoDepotEndpoint: "https://repo.example/ubuntu", Timeout: 30 * time.Minute, + }, config) + return nil + } + require.NoError(t, tt.App.initializeAKSCloud(t.Context(), map[string]string{ + "LOCATION": "usnateast", "REPO_DEPOT_ENDPOINT": "https://repo.example/ubuntu", "CSE_TIMEOUT": "1800", + })) + for _, timeout := range []string{"", "bad", "0", "-1"} { + require.ErrorContains(t, tt.App.initializeAKSCloud(t.Context(), map[string]string{"CSE_TIMEOUT": timeout}), "invalid cloud initialization timeout") + } +} + +func TestCloudCertificateRefreshCommand(t *testing.T) { + for _, tc := range []struct { + name string + args []string + err error + wantCall bool + wantError bool + }{ + {"valid", []string{"--location", "usnateast"}, nil, true, false}, + {"refresh failure", []string{"--location", "usnateast"}, errors.New("refresh failed"), true, true}, + {"missing location", nil, nil, false, true}, + {"empty location", []string{"--location", " "}, nil, false, true}, + {"unexpected argument", []string{"--location", "usnateast", "extra"}, nil, false, true}, + } { + t.Run(tc.name, func(t *testing.T) { + tt := NewTestApp(t, TestAppConfig{}) + called := false + tt.App.refreshCloudCertificates = func(_ context.Context, location string) error { + called = true + require.Equal(t, "usnateast", location) + return tc.err + } + code := tt.App.Run(t.Context(), append([]string{"aks-node-controller", "refresh-cloud-certificates"}, tc.args...)) + require.Equal(t, tc.wantCall, called) + require.Equal(t, tc.wantError, code != 0) + }) + } +} + +func TestCloudInitializationExpectedNBCEnvironmentDifference(t *testing.T) { + require.Empty(t, diffEnvMaps(nil, map[string]string{"INIT_AKS_CLOUD_FILEPATH": "/opt/azure/containers/init-aks-cloud.sh"})) +} diff --git a/aks-node-controller/cloud_repositories.go b/aks-node-controller/cloud_repositories.go new file mode 100644 index 00000000000..e71a0affce6 --- /dev/null +++ b/aks-node-controller/cloud_repositories.go @@ -0,0 +1,261 @@ +package main + +import ( + "context" + "errors" + "fmt" + "log/slog" + "net/http" + "net/url" + "os" + "path/filepath" + "regexp" + "runtime" + "strings" + "time" + + "github.com/Azure/agentbaker/aks-node-controller/common" +) + +const ( + cloudRepoSchemeHTTP = "http" + cloudRepoSchemeHTTPS = "https" + ubuntuCloudSource = ` +Types: deb +URIs: %s/ubuntu +Suites: %s %s-updates %s-backports %s-security +Components: main universe restricted multiverse +Signed-By: /usr/share/keyrings/ubuntu-archive-keyring.gpg +` + microsoftCloudSource = ` +Types: deb +URIs: %s +Suites: %s +Components: main +Architectures: %s +Signed-By: %s +` + azureLinuxCloudRepo = `[azurelinux-official-%s] +name=Azure Linux Official %s $releasever $basearch +baseurl=%s/azurelinux/$releasever/prod/%s/$basearch +gpgkey=file:///etc/pki/rpm-gpg/MICROSOFT-RPM-GPG-KEY +gpgcheck=1 +repo_gpgcheck=1 +enabled=1 +skip_if_unavailable=True +sslverify=1 +` +) + +func (c *cloudInitializer) initializeRepositories(ctx context.Context, platform cloudPlatform, endpoint string) error { + if endpoint == "" { + slog.Info("RepoDepot endpoint is empty, skipping repository initialization") + return nil + } + endpoint = strings.TrimSuffix(strings.TrimRight(endpoint, "/"), "/ubuntu") + parsed, err := url.Parse(endpoint) + if err != nil || parsed.Host == "" || parsed.User != nil || parsed.RawQuery != "" || parsed.Fragment != "" || + (parsed.Scheme != cloudRepoSchemeHTTP && parsed.Scheme != cloudRepoSchemeHTTPS) || strings.ContainsAny(endpoint, "\r\n\t ") { + return errors.New("invalid RepoDepot endpoint") + } + switch platform.ID { + case osReleaseIDUbuntu: + if err := c.initializeUbuntuRepositories(ctx, platform, endpoint); err != nil { + return err + } + return c.updateRepositoryCache(ctx, "apt-get", "update") + case osReleaseIDMariner: + if err := c.initializeMarinerRepositories(endpoint); err != nil { + return err + } + return c.updateRepositoryCache(ctx, "dnf", "makecache", "-y") + case osReleaseIDAzureLinux: + if err := c.initializeAzureLinuxRepositories(endpoint); err != nil { + return err + } + return c.updateRepositoryCache(ctx, "dnf", "makecache", "-y") + default: + slog.Info("RepoDepot initialization is not needed on immutable OS", "OS", platform.ID) + return nil + } +} + +func (c *cloudInitializer) repositoryGET(ctx context.Context, endpoint string) ([]byte, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil) + if err != nil { + return nil, err + } + resp, err := c.repoClient.Do(req) + if err != nil { + return nil, err + } + body, err := readCloudResponse(resp) + if err != nil { + return nil, err + } + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + return nil, fmt.Errorf("RepoDepot request returned HTTP %d", resp.StatusCode) + } + return body, nil +} + +func (c *cloudInitializer) initializeUbuntuRepositories(ctx context.Context, platform cloudPlatform, endpoint string) error { + if platform.Version == "" || platform.Codename == "" || + strings.ContainsAny(platform.Version+platform.Codename, "\r\n\t ") { + return errors.New("missing valid Ubuntu VERSION_ID or VERSION_CODENAME") + } + if err := c.copyOpenSSLBundle(); err != nil { + return fmt.Errorf("copy Ubuntu CA bundle: %w", err) + } + if err := c.backupUbuntuSources(); err != nil { + return err + } + mainSource := fmt.Sprintf(ubuntuCloudSource, + endpoint, platform.Codename, platform.Codename, platform.Codename, platform.Codename) + if err := c.writeFile(defaultAptSourcesDir+"/ubuntu.sources", []byte(mainSource)); err != nil { + return err + } + microsoftSource := endpoint + "/microsoft/ubuntu/" + platform.Version + "/prod" + if _, err := c.repositoryGET(ctx, microsoftSource); err != nil { + return fmt.Errorf("check Ubuntu Microsoft repository: %w", err) + } + keyPaths, err := c.ubuntuRepositoryKeyrings(ctx, endpoint) + if err != nil { + return err + } + microsoftConfig := fmt.Sprintf(microsoftCloudSource, + microsoftSource, platform.Codename, runtime.GOARCH, strings.Join(keyPaths, " ")) + for _, name := range []string{"microsoft-prod", "microsoft-prod-testing"} { + if err := c.writeFile(defaultAptSourcesDir+"/"+name+".sources", []byte(microsoftConfig)); err != nil { + return err + } + } + return nil +} + +func (c *cloudInitializer) backupUbuntuSources() error { + sourcesDir, backupDir := c.path(defaultAptSourcesDir), c.path("/etc/apt/backup") + if err := os.MkdirAll(sourcesDir, 0o755); err != nil { + return err + } + if err := os.MkdirAll(backupDir, 0o755); err != nil { + return err + } + sources, err := filepath.Glob(filepath.Join(sourcesDir, "*")) + if err != nil { + return err + } + sources = append(sources, c.path("/etc/apt/sources.list")) + for _, source := range sources { + info, err := os.Stat(source) + if errors.Is(err, os.ErrNotExist) { + continue + } + if err != nil { + return err + } + if info.Mode().IsRegular() { + if err := os.Rename(source, filepath.Join(backupDir, filepath.Base(source))); err != nil { + return err + } + } + } + return nil +} + +func (c *cloudInitializer) ubuntuRepositoryKeyrings(ctx context.Context, endpoint string) ([]string, error) { + var keyPaths []string + for _, key := range []string{"microsoft.asc", "msopentech.asc"} { + body, err := c.repositoryGET(ctx, endpoint+"/keys/"+key) + if err != nil { + return nil, fmt.Errorf("download RepoDepot key %s: %w", key, err) + } + keyring, err := c.command(ctx, body, "gpg", "--dearmor") + if err != nil { + return nil, err + } + if len(keyring) == 0 { + return nil, fmt.Errorf("RepoDepot key %s produced an empty keyring", key) + } + path := "/etc/apt/keyrings/" + key + ".gpg" + if err := c.writeFile(path, keyring); err != nil { + return nil, err + } + keyPaths = append(keyPaths, path) + } + return keyPaths, nil +} + +func (c *cloudInitializer) initializeMarinerRepositories(endpoint string) error { + source, err := os.ReadFile(c.path(defaultYumReposDir + "/mariner-extras.repo")) + if err != nil { + return err + } + for _, repo := range []struct{ id, title string }{ + {"extended", "Extended"}, {"nvidia", "Nvidia"}, {"cloud-native", "Cloud-Native"}, + } { + content := strings.ReplaceAll(string(source), "extras", repo.id) + content = strings.ReplaceAll(content, "Extras", repo.title) + if writeErr := c.writeFile(defaultYumReposDir+"/mariner-"+repo.id+".repo", []byte(content)); writeErr != nil { + return writeErr + } + } + paths, err := filepath.Glob(c.path(defaultYumReposDir + "/*.repo")) + if err != nil { + return err + } + for _, path := range paths { + content, err := os.ReadFile(path) + if err != nil { + return err + } + content = []byte(strings.ReplaceAll(string(content), "https://packages.microsoft.com", endpoint+"/mariner/packages.microsoft.com")) + if err := writeCloudFile(path, content); err != nil { + return err + } + } + return nil +} + +func (c *cloudInitializer) initializeAzureLinuxRepositories(endpoint string) error { + paths, err := filepath.Glob(c.path(defaultYumReposDir + "/azurelinux*")) + if err != nil { + return err + } + for _, path := range paths { + if err := os.Remove(path); err != nil { + return err + } + } + for _, repo := range []string{"amd", "base", "cloud-native", "extended", "ms-non-oss", "ms-oss", "nvidia"} { + content := fmt.Sprintf(azureLinuxCloudRepo, + repo, repo, endpoint, repo) + if err := c.writeFile(defaultYumReposDir+"/azurelinux-"+repo+".repo", []byte(content)); err != nil { + return err + } + } + return nil +} + +func (c *cloudInitializer) updateRepositoryCache(ctx context.Context, name string, args ...string) error { + attempt := 0 + dnfError := regexp.MustCompile(`(?m)^([WE]:.*)|([eE]rr.*)$`) + _, err := common.RetryFetch(ctx, cloudRequestAttempts, nil, func(ctx context.Context) (struct{}, error) { + if attempt > 0 { + if err := c.wait(ctx, 5*time.Second); err != nil { + return struct{}{}, err + } + } + attempt++ + output, err := c.command(ctx, nil, name, args...) + slog.Info("refreshing RepoDepot package cache", "command", name, "attempt", attempt, "output", string(output)) + if err == nil && name == "dnf" && dnfError.Match(output) { + err = errors.New("dnf reported repository metadata errors") + } + return struct{}{}, err + }) + if err != nil { + return fmt.Errorf("refresh RepoDepot package cache: %w", err) + } + return nil +} diff --git a/aks-node-controller/cloud_repositories_test.go b/aks-node-controller/cloud_repositories_test.go new file mode 100644 index 00000000000..7ec5f3ed299 --- /dev/null +++ b/aks-node-controller/cloud_repositories_test.go @@ -0,0 +1,163 @@ +package main + +import ( + "context" + "errors" + "io" + "os" + "os/exec" + "runtime" + "strconv" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func TestCloudUbuntuRepositoryInitializationIndependentOfOptIn(t *testing.T) { + c := newTestCloudInitializer(t, cloudTestUbuntu) + serveCloudTestResponses(t, c, nil, `{"IsOptedInForRootCerts":false}`) + require.NoError(t, c.writeFile("/etc/apt/sources.list", []byte("original ubuntu sources"))) + require.NoError(t, c.writeFile(defaultAptSourcesDir+"/original.sources", []byte("original extra sources"))) + var commands []string + run := c.run + c.run = func(cmd *exec.Cmd) error { + commands = append(commands, strings.Join(cmd.Args, " ")) + return run(cmd) + } + require.NoError(t, c.initialize(t.Context(), cloudInitConfig{ + Location: "westus2", RepoDepotEndpoint: c.wireEndpoint + "/ubuntu/", Timeout: time.Minute, + })) + require.Equal(t, []string{"gpg --dearmor", "gpg --dearmor", "apt-get update"}, commands) + backup, err := os.ReadFile(c.path("/etc/apt/backup/sources.list")) + require.NoError(t, err) + require.Equal(t, "original ubuntu sources", string(backup)) + backup, err = os.ReadFile(c.path("/etc/apt/backup/original.sources")) + require.NoError(t, err) + require.Equal(t, "original extra sources", string(backup)) + source, err := os.ReadFile(c.path(defaultAptSourcesDir + "/ubuntu.sources")) + require.NoError(t, err) + require.Contains(t, string(source), "URIs: "+c.wireEndpoint+"/ubuntu\n") + require.Contains(t, string(source), "Suites: jammy jammy-updates jammy-backports jammy-security") + for _, name := range []string{"microsoft-prod", "microsoft-prod-testing"} { + source, err := os.ReadFile(c.path(defaultAptSourcesDir + "/" + name + ".sources")) + require.NoError(t, err) + require.Contains(t, string(source), c.wireEndpoint+"/microsoft/ubuntu/22.04/prod") + require.Contains(t, string(source), "Architectures: "+runtime.GOARCH) + require.Contains(t, string(source), "Signed-By: /etc/apt/keyrings/microsoft.asc.gpg /etc/apt/keyrings/msopentech.asc.gpg") + } + for _, name := range []string{"microsoft.asc", "msopentech.asc"} { + key, err := os.ReadFile(c.path("/etc/apt/keyrings/" + name + ".gpg")) + require.NoError(t, err) + require.Equal(t, "dearmored-key", string(key)) + } +} + +func TestCloudRPMRepositoryInitializationIndependentOfOptIn(t *testing.T) { + for _, id := range []string{"mariner", osReleaseIDAzureLinux} { + t.Run(id, func(t *testing.T) { + c := newTestCloudInitializer(t, "ID="+id+"\n") + serveCloudTestResponses(t, c, nil, `{"IsOptedInForRootCerts":false}`) + require.NoError(t, c.writeFile(defaultYumReposDir+"/keep.repo", []byte("keep this repository"))) + require.NoError(t, c.writeFile(defaultYumReposDir+"/azurelinux-old.repo", []byte("obsolete"))) + require.NoError(t, c.writeFile(defaultYumReposDir+"/mariner-extras.repo", + []byte("[mariner-extras]\nname=Mariner Extras\nbaseurl=https://packages.microsoft.com/cbl-mariner/$releasever/extras/$basearch\n"))) + var commands []string + c.run = func(cmd *exec.Cmd) error { + commands = append(commands, strings.Join(cmd.Args, " ")) + return nil + } + require.NoError(t, c.initialize(t.Context(), cloudInitConfig{ + Location: "westus2", RepoDepotEndpoint: "https://repo.example/ubuntu", Timeout: time.Minute, + })) + require.Equal(t, []string{"dnf makecache -y"}, commands) + if id == "mariner" { + for _, repo := range []string{"extras", "extended", "nvidia", "cloud-native"} { + source, err := os.ReadFile(c.path(defaultYumReposDir + "/mariner-" + repo + ".repo")) + require.NoError(t, err) + require.Contains(t, string(source), "https://repo.example/mariner/packages.microsoft.com/") + require.Contains(t, string(source), "/"+repo+"/") + } + } else { + _, err := os.Stat(c.path(defaultYumReposDir + "/azurelinux-old.repo")) + require.ErrorIs(t, err, os.ErrNotExist) + for _, repo := range []string{"amd", "base", "cloud-native", "extended", "ms-non-oss", "ms-oss", "nvidia"} { + source, err := os.ReadFile(c.path(defaultYumReposDir + "/azurelinux-" + repo + ".repo")) + require.NoError(t, err) + require.Contains(t, string(source), "https://repo.example/azurelinux/$releasever/prod/"+repo+"/$basearch") + require.Contains(t, string(source), "gpgcheck=1\nrepo_gpgcheck=1") + } + } + keep, err := os.ReadFile(c.path(defaultYumReposDir + "/keep.repo")) + require.NoError(t, err) + require.Equal(t, "keep this repository", string(keep)) + }) + } +} + +func TestCloudRepositoriesSkipEmptyEndpointAndImmutableOS(t *testing.T) { + for _, tc := range []struct{ id, endpoint string }{ + {osReleaseIDUbuntu, ""}, {"mariner", ""}, {osReleaseIDAzureLinux, ""}, + {osReleaseIDFlatcar, "https://repo.example"}, {osReleaseIDAzureContainerLinux, "https://repo.example"}, + } { + t.Run(tc.id+"/"+tc.endpoint, func(t *testing.T) { + c := newTestCloudInitializer(t, "ID="+tc.id+"\n") + c.run = func(*exec.Cmd) error { + t.Fatal("no repository commands expected") + return nil + } + require.NoError(t, c.initializeRepositories(t.Context(), cloudPlatform{ID: tc.id}, tc.endpoint)) + }) + } +} + +func TestCloudRepositoryEndpointValidation(t *testing.T) { + c := newTestCloudInitializer(t, cloudTestUbuntu) + for _, endpoint := range []string{ + "not-a-url", "file:///etc/apt", "https://repo.example\nSigned-By: bad", "https://user:password@repo.example", + "https://repo.example?query=1", "https://repo.example/#fragment", + } { + require.ErrorContains(t, c.initializeRepositories(t.Context(), cloudPlatform{ID: osReleaseIDUbuntu}, endpoint), "invalid RepoDepot endpoint") + } +} + +func TestCloudRepositoryCacheRetriesAndFailsExplicitly(t *testing.T) { + for _, name := range []string{"apt-get", "dnf"} { + for _, recoverAfter := range []int{2, cloudRequestAttempts + 1} { + t.Run(name+"/"+strconv.Itoa(recoverAfter), func(t *testing.T) { + c := newTestCloudInitializer(t, cloudTestUbuntu) + attempts := 0 + c.run = func(cmd *exec.Cmd) error { + attempts++ + if attempts < recoverAfter { + if name == "dnf" { + _, _ = io.WriteString(cmd.Stderr, "Error: repository metadata is unavailable\n") + return nil + } + return errors.New("cache refresh failed") + } + return nil + } + err := c.updateRepositoryCache(t.Context(), name, "update") + if recoverAfter > cloudRequestAttempts { + require.ErrorContains(t, err, "refresh RepoDepot package cache") + require.Equal(t, cloudRequestAttempts, attempts) + } else { + require.NoError(t, err) + require.Equal(t, recoverAfter, attempts) + } + }) + } + } + t.Run("canceled", func(t *testing.T) { + c := newTestCloudInitializer(t, cloudTestUbuntu) + c.run = func(*exec.Cmd) error { + t.Fatal("canceled context must not execute a command") + return nil + } + ctx, cancel := context.WithCancel(t.Context()) + cancel() + require.ErrorIs(t, c.updateRepositoryCache(ctx, "apt-get", "update"), context.Canceled) + }) +} diff --git a/aks-node-controller/embeddednodecustomdata.go b/aks-node-controller/embeddednodecustomdata.go index 67615d54fcb..1f1ee70c0e5 100644 --- a/aks-node-controller/embeddednodecustomdata.go +++ b/aks-node-controller/embeddednodecustomdata.go @@ -20,6 +20,7 @@ const ( osReleaseIDAzureLinux = "azurelinux" osReleaseIDFlatcar = "flatcar" osReleaseIDUbuntu = "ubuntu" + osReleaseIDMariner = "mariner" osVariantIDOSGuard = "osguard" ) @@ -27,7 +28,7 @@ type nodeCustomDataPlatform string const ( nodeCustomDataPlatformUbuntu nodeCustomDataPlatform = "ubuntu" - nodeCustomDataPlatformMariner nodeCustomDataPlatform = "mariner" + nodeCustomDataPlatformMariner nodeCustomDataPlatform = osReleaseIDMariner nodeCustomDataPlatformUnsupported nodeCustomDataPlatform = "unsupported" ) diff --git a/aks-node-controller/parser/consts.go b/aks-node-controller/parser/consts.go index c52803b7189..bd6d97036cb 100644 --- a/aks-node-controller/parser/consts.go +++ b/aks-node-controller/parser/consts.go @@ -32,7 +32,6 @@ const ( customSearchDomainsCSEScriptFilepath = "/opt/azure/containers/setup-custom-search-domains.sh" dhcpV6ServiceCSEScriptFilepath = "/etc/systemd/system/dhcpv6.service" dhcpV6ConfigCSEScriptFilepath = "/opt/azure/containers/enable-dhcpv6.sh" - initAKSCloudFilepath = "/opt/azure/containers/init-aks-cloud.sh" ) const ( diff --git a/aks-node-controller/parser/helper.go b/aks-node-controller/parser/helper.go index 1008e668509..07eb4d765ad 100644 --- a/aks-node-controller/parser/helper.go +++ b/aks-node-controller/parser/helper.go @@ -76,9 +76,8 @@ var ( func getFuncMap() template.FuncMap { return template.FuncMap{ - "getInitAKSCloudFilepath": getInitAKSCloudFilepath, - "getIsAksCustomCloud": getIsAksCustomCloud, - "getCloudLocation": getCloudLocation, + "getIsAksCustomCloud": getIsAksCustomCloud, + "getCloudLocation": getCloudLocation, } } @@ -868,10 +867,6 @@ func getHasKubeletDiskType(kubeletConfig *aksnodeconfigv1.KubeletConfig) bool { return kubeletConfig.GetKubeletDiskType() == aksnodeconfigv1.KubeletDisk_KUBELET_DISK_TEMP_DISK } -func getInitAKSCloudFilepath() string { - return initAKSCloudFilepath -} - func getGPUNeedsFabricManager(vmSize string) bool { return gpu.GPUNeedsFabricManager(vmSize) } diff --git a/aks-node-controller/parser/parser.go b/aks-node-controller/parser/parser.go index 7f83919d1da..cd89bdfd4ce 100644 --- a/aks-node-controller/parser/parser.go +++ b/aks-node-controller/parser/parser.go @@ -223,7 +223,6 @@ func getCSEEnv(ctx context.Context, config *aksnodeconfigv1.Configuration, gpuCo "NETWORK_ISOLATED_CLUSTER_TEST_MODE": "false", // temp: needs to be added to config "STANDARD_SECONDARY_NIC_COUNT": fmt.Sprintf("%d", config.GetNetworkConfig().GetStandardSecondaryNicCount()), "ENABLE_MANAGED_GPU_DRA": fmt.Sprintf("%v", config.GetGpuConfig().GetEnableManagedGpuDra()), - "INIT_AKS_CLOUD_FILEPATH": getInitAKSCloudFilepath(), "REPO_DEPOT_ENDPOINT": getRepoDepotEndpoint(config), } @@ -323,7 +322,7 @@ func BuildCSECmd(ctx context.Context, config *aksnodeconfigv1.Configuration, gpu return nil, fmt.Errorf("failed to execute the template: %w", err) } // Convert to one-liner - triggerBootstrapScript = strings.ReplaceAll(triggerBootstrapScript, "\n", " ") + triggerBootstrapScript = strings.TrimSpace(strings.ReplaceAll(triggerBootstrapScript, "\n", " ")) cmd := exec.CommandContext(ctx, "/bin/bash", "-c", triggerBootstrapScript) env := mapToEnviron(getCSEEnv(ctx, config, gpuConfig)) cmd.Env = append(os.Environ(), env...) // append existing environment variables diff --git a/aks-node-controller/parser/parser_test.go b/aks-node-controller/parser/parser_test.go index fc6109fc6ab..56c5d896ee0 100644 --- a/aks-node-controller/parser/parser_test.go +++ b/aks-node-controller/parser/parser_test.go @@ -394,6 +394,16 @@ oom_score = -999 } } +func TestBuildCSECmdDoesNotExecuteCloudInitializationScript(t *testing.T) { + cmd, err := BuildCSECmd(t.Context(), &aksnodeconfigv1.Configuration{}, nil) + require.NoError(t, err) + require.NotContains(t, cmd.String(), "INIT_AKS_CLOUD_FILEPATH") + require.NotContains(t, cmd.String(), "init-aks-cloud.sh") + require.NotContains(t, environToMap(cmd.Env), "INIT_AKS_CLOUD_FILEPATH") + require.Contains(t, cmd.String(), "/opt/azure/containers/provision_start.sh") + require.Equal(t, strings.TrimSpace(cmd.Args[2]), cmd.Args[2]) +} + func TestBuildCSECmd_MCRRepositoryBase(t *testing.T) { tests := []struct { name string diff --git a/aks-node-controller/parser/templates/cse_cmd.sh.gtpl b/aks-node-controller/parser/templates/cse_cmd.sh.gtpl index 344ca7a7713..411ed80f475 100644 --- a/aks-node-controller/parser/templates/cse_cmd.sh.gtpl +++ b/aks-node-controller/parser/templates/cse_cmd.sh.gtpl @@ -1,5 +1,2 @@ echo $(date),$(hostname) > ${PROVISION_OUTPUT}; -if [ -f "${INIT_AKS_CLOUD_FILEPATH}" ]; then - "${INIT_AKS_CLOUD_FILEPATH}" >> /var/log/azure/cluster-provision.log 2>&1 || exit $?; -fi; /usr/bin/nohup /bin/bash -c "/bin/bash /opt/azure/containers/provision_start.sh" diff --git a/aks-node-controller/parser/testdata/AKSUbuntu2204+China/generatedCSECommand b/aks-node-controller/parser/testdata/AKSUbuntu2204+China/generatedCSECommand index 85480046d68..1e7a6f30401 100644 --- a/aks-node-controller/parser/testdata/AKSUbuntu2204+China/generatedCSECommand +++ b/aks-node-controller/parser/testdata/AKSUbuntu2204+China/generatedCSECommand @@ -1 +1 @@ -/bin/bash -c echo $(date),$(hostname) > ${PROVISION_OUTPUT}; if [ -f "${INIT_AKS_CLOUD_FILEPATH}" ]; then "${INIT_AKS_CLOUD_FILEPATH}" >> /var/log/azure/cluster-provision.log 2>&1 || exit $?; fi; /usr/bin/nohup /bin/bash -c "/bin/bash /opt/azure/containers/provision_start.sh" \ No newline at end of file +/bin/bash -c echo $(date),$(hostname) > ${PROVISION_OUTPUT}; /usr/bin/nohup /bin/bash -c "/bin/bash /opt/azure/containers/provision_start.sh" \ No newline at end of file diff --git a/aks-node-controller/parser/testdata/AKSUbuntu2204+CloudProviderOverrides/generatedCSECommand b/aks-node-controller/parser/testdata/AKSUbuntu2204+CloudProviderOverrides/generatedCSECommand index 85480046d68..1e7a6f30401 100644 --- a/aks-node-controller/parser/testdata/AKSUbuntu2204+CloudProviderOverrides/generatedCSECommand +++ b/aks-node-controller/parser/testdata/AKSUbuntu2204+CloudProviderOverrides/generatedCSECommand @@ -1 +1 @@ -/bin/bash -c echo $(date),$(hostname) > ${PROVISION_OUTPUT}; if [ -f "${INIT_AKS_CLOUD_FILEPATH}" ]; then "${INIT_AKS_CLOUD_FILEPATH}" >> /var/log/azure/cluster-provision.log 2>&1 || exit $?; fi; /usr/bin/nohup /bin/bash -c "/bin/bash /opt/azure/containers/provision_start.sh" \ No newline at end of file +/bin/bash -c echo $(date),$(hostname) > ${PROVISION_OUTPUT}; /usr/bin/nohup /bin/bash -c "/bin/bash /opt/azure/containers/provision_start.sh" \ No newline at end of file diff --git a/aks-node-controller/parser/testdata/AKSUbuntu2204+Containerd+MIG/generatedCSECommand b/aks-node-controller/parser/testdata/AKSUbuntu2204+Containerd+MIG/generatedCSECommand index 85480046d68..1e7a6f30401 100644 --- a/aks-node-controller/parser/testdata/AKSUbuntu2204+Containerd+MIG/generatedCSECommand +++ b/aks-node-controller/parser/testdata/AKSUbuntu2204+Containerd+MIG/generatedCSECommand @@ -1 +1 @@ -/bin/bash -c echo $(date),$(hostname) > ${PROVISION_OUTPUT}; if [ -f "${INIT_AKS_CLOUD_FILEPATH}" ]; then "${INIT_AKS_CLOUD_FILEPATH}" >> /var/log/azure/cluster-provision.log 2>&1 || exit $?; fi; /usr/bin/nohup /bin/bash -c "/bin/bash /opt/azure/containers/provision_start.sh" \ No newline at end of file +/bin/bash -c echo $(date),$(hostname) > ${PROVISION_OUTPUT}; /usr/bin/nohup /bin/bash -c "/bin/bash /opt/azure/containers/provision_start.sh" \ No newline at end of file diff --git a/aks-node-controller/parser/testdata/AKSUbuntu2204+CustomCloud/generatedCSECommand b/aks-node-controller/parser/testdata/AKSUbuntu2204+CustomCloud/generatedCSECommand index 85480046d68..1e7a6f30401 100644 --- a/aks-node-controller/parser/testdata/AKSUbuntu2204+CustomCloud/generatedCSECommand +++ b/aks-node-controller/parser/testdata/AKSUbuntu2204+CustomCloud/generatedCSECommand @@ -1 +1 @@ -/bin/bash -c echo $(date),$(hostname) > ${PROVISION_OUTPUT}; if [ -f "${INIT_AKS_CLOUD_FILEPATH}" ]; then "${INIT_AKS_CLOUD_FILEPATH}" >> /var/log/azure/cluster-provision.log 2>&1 || exit $?; fi; /usr/bin/nohup /bin/bash -c "/bin/bash /opt/azure/containers/provision_start.sh" \ No newline at end of file +/bin/bash -c echo $(date),$(hostname) > ${PROVISION_OUTPUT}; /usr/bin/nohup /bin/bash -c "/bin/bash /opt/azure/containers/provision_start.sh" \ No newline at end of file diff --git a/aks-node-controller/parser/testdata/AKSUbuntu2204+CustomOSConfig/generatedCSECommand b/aks-node-controller/parser/testdata/AKSUbuntu2204+CustomOSConfig/generatedCSECommand index 85480046d68..1e7a6f30401 100644 --- a/aks-node-controller/parser/testdata/AKSUbuntu2204+CustomOSConfig/generatedCSECommand +++ b/aks-node-controller/parser/testdata/AKSUbuntu2204+CustomOSConfig/generatedCSECommand @@ -1 +1 @@ -/bin/bash -c echo $(date),$(hostname) > ${PROVISION_OUTPUT}; if [ -f "${INIT_AKS_CLOUD_FILEPATH}" ]; then "${INIT_AKS_CLOUD_FILEPATH}" >> /var/log/azure/cluster-provision.log 2>&1 || exit $?; fi; /usr/bin/nohup /bin/bash -c "/bin/bash /opt/azure/containers/provision_start.sh" \ No newline at end of file +/bin/bash -c echo $(date),$(hostname) > ${PROVISION_OUTPUT}; /usr/bin/nohup /bin/bash -c "/bin/bash /opt/azure/containers/provision_start.sh" \ No newline at end of file diff --git a/aks-node-controller/parser/testdata/AKSUbuntu2204+DefaultPubkeyAuth/generatedCSECommand b/aks-node-controller/parser/testdata/AKSUbuntu2204+DefaultPubkeyAuth/generatedCSECommand index 85480046d68..1e7a6f30401 100644 --- a/aks-node-controller/parser/testdata/AKSUbuntu2204+DefaultPubkeyAuth/generatedCSECommand +++ b/aks-node-controller/parser/testdata/AKSUbuntu2204+DefaultPubkeyAuth/generatedCSECommand @@ -1 +1 @@ -/bin/bash -c echo $(date),$(hostname) > ${PROVISION_OUTPUT}; if [ -f "${INIT_AKS_CLOUD_FILEPATH}" ]; then "${INIT_AKS_CLOUD_FILEPATH}" >> /var/log/azure/cluster-provision.log 2>&1 || exit $?; fi; /usr/bin/nohup /bin/bash -c "/bin/bash /opt/azure/containers/provision_start.sh" \ No newline at end of file +/bin/bash -c echo $(date),$(hostname) > ${PROVISION_OUTPUT}; /usr/bin/nohup /bin/bash -c "/bin/bash /opt/azure/containers/provision_start.sh" \ No newline at end of file diff --git a/aks-node-controller/parser/testdata/AKSUbuntu2204+DisablePubkeyAuth/generatedCSECommand b/aks-node-controller/parser/testdata/AKSUbuntu2204+DisablePubkeyAuth/generatedCSECommand index 85480046d68..1e7a6f30401 100644 --- a/aks-node-controller/parser/testdata/AKSUbuntu2204+DisablePubkeyAuth/generatedCSECommand +++ b/aks-node-controller/parser/testdata/AKSUbuntu2204+DisablePubkeyAuth/generatedCSECommand @@ -1 +1 @@ -/bin/bash -c echo $(date),$(hostname) > ${PROVISION_OUTPUT}; if [ -f "${INIT_AKS_CLOUD_FILEPATH}" ]; then "${INIT_AKS_CLOUD_FILEPATH}" >> /var/log/azure/cluster-provision.log 2>&1 || exit $?; fi; /usr/bin/nohup /bin/bash -c "/bin/bash /opt/azure/containers/provision_start.sh" \ No newline at end of file +/bin/bash -c echo $(date),$(hostname) > ${PROVISION_OUTPUT}; /usr/bin/nohup /bin/bash -c "/bin/bash /opt/azure/containers/provision_start.sh" \ No newline at end of file diff --git a/aks-node-controller/parser/testdata/AKSUbuntu2204+EnablePubkeyAuth/generatedCSECommand b/aks-node-controller/parser/testdata/AKSUbuntu2204+EnablePubkeyAuth/generatedCSECommand index 85480046d68..1e7a6f30401 100644 --- a/aks-node-controller/parser/testdata/AKSUbuntu2204+EnablePubkeyAuth/generatedCSECommand +++ b/aks-node-controller/parser/testdata/AKSUbuntu2204+EnablePubkeyAuth/generatedCSECommand @@ -1 +1 @@ -/bin/bash -c echo $(date),$(hostname) > ${PROVISION_OUTPUT}; if [ -f "${INIT_AKS_CLOUD_FILEPATH}" ]; then "${INIT_AKS_CLOUD_FILEPATH}" >> /var/log/azure/cluster-provision.log 2>&1 || exit $?; fi; /usr/bin/nohup /bin/bash -c "/bin/bash /opt/azure/containers/provision_start.sh" \ No newline at end of file +/bin/bash -c echo $(date),$(hostname) > ${PROVISION_OUTPUT}; /usr/bin/nohup /bin/bash -c "/bin/bash /opt/azure/containers/provision_start.sh" \ No newline at end of file diff --git a/aks-node-controller/parser/testdata/AKSUbuntu2204+LocalDNS+HostsPlugin/generatedCSECommand b/aks-node-controller/parser/testdata/AKSUbuntu2204+LocalDNS+HostsPlugin/generatedCSECommand index 85480046d68..1e7a6f30401 100644 --- a/aks-node-controller/parser/testdata/AKSUbuntu2204+LocalDNS+HostsPlugin/generatedCSECommand +++ b/aks-node-controller/parser/testdata/AKSUbuntu2204+LocalDNS+HostsPlugin/generatedCSECommand @@ -1 +1 @@ -/bin/bash -c echo $(date),$(hostname) > ${PROVISION_OUTPUT}; if [ -f "${INIT_AKS_CLOUD_FILEPATH}" ]; then "${INIT_AKS_CLOUD_FILEPATH}" >> /var/log/azure/cluster-provision.log 2>&1 || exit $?; fi; /usr/bin/nohup /bin/bash -c "/bin/bash /opt/azure/containers/provision_start.sh" \ No newline at end of file +/bin/bash -c echo $(date),$(hostname) > ${PROVISION_OUTPUT}; /usr/bin/nohup /bin/bash -c "/bin/bash /opt/azure/containers/provision_start.sh" \ No newline at end of file diff --git a/aks-node-controller/parser/testdata/AKSUbuntu2204+LocalDNS/generatedCSECommand b/aks-node-controller/parser/testdata/AKSUbuntu2204+LocalDNS/generatedCSECommand index 85480046d68..1e7a6f30401 100644 --- a/aks-node-controller/parser/testdata/AKSUbuntu2204+LocalDNS/generatedCSECommand +++ b/aks-node-controller/parser/testdata/AKSUbuntu2204+LocalDNS/generatedCSECommand @@ -1 +1 @@ -/bin/bash -c echo $(date),$(hostname) > ${PROVISION_OUTPUT}; if [ -f "${INIT_AKS_CLOUD_FILEPATH}" ]; then "${INIT_AKS_CLOUD_FILEPATH}" >> /var/log/azure/cluster-provision.log 2>&1 || exit $?; fi; /usr/bin/nohup /bin/bash -c "/bin/bash /opt/azure/containers/provision_start.sh" \ No newline at end of file +/bin/bash -c echo $(date),$(hostname) > ${PROVISION_OUTPUT}; /usr/bin/nohup /bin/bash -c "/bin/bash /opt/azure/containers/provision_start.sh" \ No newline at end of file diff --git a/aks-node-controller/parser/testdata/AKSUbuntu2204+SSHStatusOn/generatedCSECommand b/aks-node-controller/parser/testdata/AKSUbuntu2204+SSHStatusOn/generatedCSECommand index 85480046d68..1e7a6f30401 100644 --- a/aks-node-controller/parser/testdata/AKSUbuntu2204+SSHStatusOn/generatedCSECommand +++ b/aks-node-controller/parser/testdata/AKSUbuntu2204+SSHStatusOn/generatedCSECommand @@ -1 +1 @@ -/bin/bash -c echo $(date),$(hostname) > ${PROVISION_OUTPUT}; if [ -f "${INIT_AKS_CLOUD_FILEPATH}" ]; then "${INIT_AKS_CLOUD_FILEPATH}" >> /var/log/azure/cluster-provision.log 2>&1 || exit $?; fi; /usr/bin/nohup /bin/bash -c "/bin/bash /opt/azure/containers/provision_start.sh" \ No newline at end of file +/bin/bash -c echo $(date),$(hostname) > ${PROVISION_OUTPUT}; /usr/bin/nohup /bin/bash -c "/bin/bash /opt/azure/containers/provision_start.sh" \ No newline at end of file diff --git a/aks-node-controller/parser/testdata/AzureLinuxv2+Kata+DisableUnattendedUpgrades=false/generatedCSECommand b/aks-node-controller/parser/testdata/AzureLinuxv2+Kata+DisableUnattendedUpgrades=false/generatedCSECommand index 85480046d68..1e7a6f30401 100644 --- a/aks-node-controller/parser/testdata/AzureLinuxv2+Kata+DisableUnattendedUpgrades=false/generatedCSECommand +++ b/aks-node-controller/parser/testdata/AzureLinuxv2+Kata+DisableUnattendedUpgrades=false/generatedCSECommand @@ -1 +1 @@ -/bin/bash -c echo $(date),$(hostname) > ${PROVISION_OUTPUT}; if [ -f "${INIT_AKS_CLOUD_FILEPATH}" ]; then "${INIT_AKS_CLOUD_FILEPATH}" >> /var/log/azure/cluster-provision.log 2>&1 || exit $?; fi; /usr/bin/nohup /bin/bash -c "/bin/bash /opt/azure/containers/provision_start.sh" \ No newline at end of file +/bin/bash -c echo $(date),$(hostname) > ${PROVISION_OUTPUT}; /usr/bin/nohup /bin/bash -c "/bin/bash /opt/azure/containers/provision_start.sh" \ No newline at end of file diff --git a/aks-node-controller/parser/testdata/Compatibility+EmptyConfig/generatedCSECommand b/aks-node-controller/parser/testdata/Compatibility+EmptyConfig/generatedCSECommand index 85480046d68..1e7a6f30401 100644 --- a/aks-node-controller/parser/testdata/Compatibility+EmptyConfig/generatedCSECommand +++ b/aks-node-controller/parser/testdata/Compatibility+EmptyConfig/generatedCSECommand @@ -1 +1 @@ -/bin/bash -c echo $(date),$(hostname) > ${PROVISION_OUTPUT}; if [ -f "${INIT_AKS_CLOUD_FILEPATH}" ]; then "${INIT_AKS_CLOUD_FILEPATH}" >> /var/log/azure/cluster-provision.log 2>&1 || exit $?; fi; /usr/bin/nohup /bin/bash -c "/bin/bash /opt/azure/containers/provision_start.sh" \ No newline at end of file +/bin/bash -c echo $(date),$(hostname) > ${PROVISION_OUTPUT}; /usr/bin/nohup /bin/bash -c "/bin/bash /opt/azure/containers/provision_start.sh" \ No newline at end of file From facce1ab6f8930c8ea804ee2887916371e08c9da Mon Sep 17 00:00:00 2001 From: Abigailliang Date: Fri, 23 Oct 2026 18:22:11 -0700 Subject: [PATCH 2/5] feat: enhance Azure cloud certificate handling and logging --- aks-node-controller/README.md | 20 +- aks-node-controller/cloud_certificates.go | 146 ++++++++++---- aks-node-controller/cloud_init_test.go | 233 +++++++++++++++++++++- 3 files changed, 340 insertions(+), 59 deletions(-) diff --git a/aks-node-controller/README.md b/aks-node-controller/README.md index 86e07dca3c0..d0a5fafb370 100644 --- a/aks-node-controller/README.md +++ b/aks-node-controller/README.md @@ -65,11 +65,21 @@ VHD binaries; this change does not remove it from AgentBaker CustomData. Regions starting with `ussec` or `usnat` use the legacy certificate endpoint. Other regions check WireServer's root-certificate opt-in before downloading root -and intermediate certificates. WireServer failures, malformed responses, invalid -certificates, trust-store errors, and refresh scheduling failures stop native -provisioning rather than falling back to the distribution's default trust. -Raw certificate responses are never logged; `AKS_CLOUD_LOG_CERTIFICATES=true` -enables diagnostics containing re-encoded public certificates only. +and intermediate certificates. Certificate handling retains the shell behavior: +only a JSON boolean `true` enables opt-in; malformed or non-true opt-in responses +skip installation, while failed or empty opt-in requests stop initialization. +Individual operation-certificate download failures are logged and skipped; the +remaining `.crt` files are installed. Missing operation lists and an absence of +installable certificates fail the opted-in path. Legacy certificate retrieval +errors still fail, but legacy trust-store installation errors are logged without +blocking repository initialization or refresh scheduling. + +Downloaded payloads are passed to the system trust-store tools without additional +PEM/X.509 validation. Refresh clears visible files in the certificate staging +directory, not existing certificates in the system trust store. Raw certificate +responses are never logged; `AKS_CLOUD_LOG_CERTIFICATES=true` enables diagnostics +containing re-encoded public certificates only. Diagnostic parsing errors do not +block installation. Eligible nodes require a nonempty region and register a daily 19:00 certificate refresh using the running ANC binary, including when provisioning uses an ANC diff --git a/aks-node-controller/cloud_certificates.go b/aks-node-controller/cloud_certificates.go index e0980c918df..92dd69030d7 100644 --- a/aks-node-controller/cloud_certificates.go +++ b/aks-node-controller/cloud_certificates.go @@ -15,10 +15,10 @@ import ( "path/filepath" "sort" "strings" - "unicode" ) func (c *cloudInitializer) refresh(ctx context.Context, platform cloudPlatform, location string) (bool, error) { + c.clearCertificateCache() legacy := strings.HasPrefix(location, "ussec") || strings.HasPrefix(location, "usnat") slog.Info("selecting Azure cloud certificate endpoint", "legacy", legacy, "location", location) if !legacy { @@ -26,16 +26,15 @@ func (c *cloudInitializer) refresh(ctx context.Context, platform cloudPlatform, if err != nil { return false, fmt.Errorf("check Azure cloud root certificate opt-in: %w", err) } - var response struct { - OptedIn *bool `json:"IsOptedInForRootCerts"` + if len(bytes.TrimRight(body, "\n")) == 0 { + return false, errors.New("Azure cloud root certificate opt-in response is empty") } + var response map[string]json.RawMessage if err := json.Unmarshal(body, &response); err != nil { - return false, fmt.Errorf("decode Azure cloud root certificate opt-in: %w", err) - } - if response.OptedIn == nil { - return false, errors.New("cloud root certificate opt-in response is missing IsOptedInForRootCerts") + slog.Warn("could not decode Azure cloud root certificate opt-in, skipping certificate installation", "error", err) + return false, nil } - if !*response.OptedIn { + if !bytes.Equal(bytes.TrimSpace(response["IsOptedInForRootCerts"]), []byte("true")) { slog.Info("Azure cloud root certificates are not opted in, skipping certificate installation") return false, nil } @@ -51,12 +50,45 @@ func (c *cloudInitializer) refresh(ctx context.Context, platform cloudPlatform, return false, err } if err := c.installCertificates(ctx, platform, certificates); err != nil { + if ctx.Err() != nil { + return false, ctx.Err() + } + if legacy { + slog.Warn("legacy Azure cloud CA certificate installation failed, continuing cloud initialization", "error", err) + return true, nil + } return false, fmt.Errorf("install Azure cloud CA certificates: %w", err) } - slog.Info("installed Azure cloud CA certificates", "count", len(certificates)) + slog.Info("installed Azure cloud CA certificates") return true, nil } +func (c *cloudInitializer) clearCertificateCache() { + directory := c.path("/root/AzureCACertificates") + if err := os.MkdirAll(directory, 0o755); err != nil { + slog.Warn("could not create Azure cloud certificate cache", "error", err) + return + } + entries, err := os.ReadDir(directory) + if err != nil { + slog.Warn("could not read Azure cloud certificate cache", "error", err) + return + } + for _, entry := range entries { + // Match the shell's non-recursive cleanup, which excludes hidden files. + if strings.HasPrefix(entry.Name(), ".") { + continue + } + if entry.IsDir() { + slog.Warn("leaving directory in Azure cloud certificate cache", "name", entry.Name()) + continue + } + if err := os.Remove(filepath.Join(directory, entry.Name())); err != nil { + slog.Warn("could not remove cached Azure cloud certificate", "name", entry.Name(), "error", err) + } + } +} + func certificatePackagePath(kind, extension string) string { return "/machine?" + url.Values{ "comp": {"acmspackage"}, @@ -67,7 +99,7 @@ func certificatePackagePath(kind, extension string) string { func validCloudCertificateName(name string) bool { return name != "" && name != "." && name != ".." && filepath.Base(name) == name && - !strings.Contains(name, `\`) && strings.IndexFunc(name, unicode.IsControl) == -1 + !strings.ContainsRune(name, 0) } func (c *cloudInitializer) legacyCertificates(ctx context.Context) (map[string][]byte, error) { @@ -77,30 +109,37 @@ func (c *cloudInitializer) legacyCertificates(ctx context.Context) (map[string][ } var response struct { Certificates []struct { - Name string - Body string `json:"CertBody"` + Name json.RawMessage + Body json.RawMessage `json:"CertBody"` } } if err := json.Unmarshal(body, &response); err != nil { return nil, fmt.Errorf("decode legacy Azure cloud certificates: %w", err) } + if response.Certificates == nil { + return nil, errors.New("legacy Azure cloud response is missing a Certificates array") + } certificates := make(map[string][]byte) for _, cert := range response.Certificates { - if !validCloudCertificateName(cert.Name) { + var name string + if err := json.Unmarshal(cert.Name, &name); err != nil || !validCloudCertificateName(name) { slog.Warn("rejecting invalid Azure cloud certificate filename") continue } - name := strings.ReplaceAll(cert.Name, ".cer", ".crt") - data, err := publicCloudCertificates([]byte(cert.Body)) - if err != nil { - return nil, fmt.Errorf("decode legacy Azure cloud certificate %s: %w", name, err) + var certBody *string + if err := json.Unmarshal(cert.Body, &certBody); err != nil { + return nil, fmt.Errorf("decode legacy Azure cloud certificate body %s: %w", name, err) } - if strings.HasSuffix(name, ".crt") { - certificates[name] = data + if certBody == nil { + return nil, fmt.Errorf("legacy Azure cloud certificate %s is missing CertBody", name) } - } - if len(certificates) == 0 { - return nil, errors.New("legacy Azure cloud response contains no installable CA certificates") + // Shell command substitution trims trailing LFs before CRLF normalization. + data := strings.ReplaceAll(strings.TrimRight(*certBody, "\n"), "\r\n", "\n") + "\n" + name = strings.ReplaceAll(name, ".cer", ".crt") + if err := c.writeFile("/root/AzureCACertificates/"+name, []byte(data)); err != nil { + return nil, fmt.Errorf("cache legacy Azure cloud certificate %s: %w", name, err) + } + certificates[name] = []byte(data) } return certificates, nil } @@ -125,47 +164,59 @@ func (c *cloudInitializer) operationCertificateGroup(ctx context.Context, kind s var response struct { Operations []struct { // ResouceFileName is the spelling used by the WireServer protocol. - Name string `json:"ResouceFileName"` + Name json.RawMessage `json:"ResouceFileName"` } `json:"OperationsInfo"` } if err := json.Unmarshal(body, &response); err != nil { return nil, fmt.Errorf("decode Azure cloud %s: %w", kind, err) } certificates := make(map[string][]byte) + found := false for _, operation := range response.Operations { - if !validCloudCertificateName(operation.Name) { + var name string + if err := json.Unmarshal(operation.Name, &name); err != nil || name == "" { + slog.Warn("ignoring missing or non-string Azure cloud certificate filename") + continue + } + found = true + if !validCloudCertificateName(name) { slog.Warn("rejecting invalid Azure cloud certificate filename") continue } - data, err := c.operationCertificate(ctx, operation.Name) + data, err := c.operationCertificate(ctx, name) if err != nil { - return nil, err + if ctx.Err() != nil { + return nil, ctx.Err() + } + slog.Warn("could not retrieve Azure cloud certificate, continuing with remaining certificates", "name", name, "error", err) + continue } - if strings.HasSuffix(operation.Name, ".crt") { - certificates[operation.Name] = data + if err := c.writeFile("/root/AzureCACertificates/"+name, data); err != nil { + return nil, fmt.Errorf("cache Azure cloud certificate %s: %w", name, err) } + certificates[name] = data } - if len(certificates) == 0 { - return nil, fmt.Errorf("cloud %s contains no installable CA certificates", kind) + if !found { + return nil, fmt.Errorf("cloud %s contains no certificate filenames", kind) } return certificates, nil } func (c *cloudInitializer) operationCertificate(ctx context.Context, filename string) ([]byte, error) { - extension := filepath.Ext(filename) - if extension == "" { - return nil, errors.New("cloud certificate filename is missing an extension") + name, extension := filename, filename + if suffix := filepath.Ext(filename); suffix != "" { + name = strings.TrimSuffix(filename, suffix) + extension = strings.TrimPrefix(suffix, ".") } - name := strings.TrimSuffix(filename, extension) - body, err := c.requestCertificates(ctx, certificatePackagePath(name, strings.TrimPrefix(extension, "."))) + body, err := c.requestCertificates(ctx, certificatePackagePath(name, extension)) if err != nil { return nil, fmt.Errorf("retrieve Azure cloud certificate %s: %w", filename, err) } - data, err := publicCloudCertificates(body) - if err != nil { - return nil, fmt.Errorf("decode Azure cloud certificate %s: %w", filename, err) + body = bytes.TrimRight(body, "\n") + if len(body) == 0 { + return nil, fmt.Errorf("Azure cloud certificate %s response is empty", filename) } - return data, nil + return append(body, '\n'), nil } func publicCloudCertificates(data []byte) ([]byte, error) { @@ -205,10 +256,11 @@ func (c *cloudInitializer) installCertificates(ctx context.Context, platform clo names = append(names, name) } sort.Strings(names) + installed := 0 for _, name := range names { data := certificates[name] - if err := c.writeFile("/root/AzureCACertificates/"+name, data); err != nil { - return err + if !strings.HasSuffix(name, ".crt") { + continue } targetName := name if platform.ID == osReleaseIDFlatcar { @@ -217,11 +269,19 @@ func (c *cloudInitializer) installCertificates(ctx context.Context, platform clo if err := c.writeFile(filepath.Join(destination, targetName), data); err != nil { return err } + installed++ if os.Getenv("AKS_CLOUD_LOG_CERTIFICATES") == "true" { - // Only re-encoded public certificates reach this opt-in diagnostic. - slog.Info("Azure cloud public CA certificate", "name", name, "certificate", string(data)) + public, err := publicCloudCertificates(data) + if err != nil { + slog.Warn("could not decode public Azure cloud CA certificates for logging", "name", name, "error", err) + continue + } + slog.Info("Azure cloud public CA certificate", "name", name, "certificate", string(public)) } } + if installed == 0 { + return errors.New("no *.crt files in Azure cloud certificate cache to install") + } if _, err := c.command(ctx, nil, update); err != nil { return err } diff --git a/aks-node-controller/cloud_init_test.go b/aks-node-controller/cloud_init_test.go index a37720d9aa1..9080518c6f2 100644 --- a/aks-node-controller/cloud_init_test.go +++ b/aks-node-controller/cloud_init_test.go @@ -200,7 +200,11 @@ func TestCloudInitializationCertificateModes(t *testing.T) { } got, err := os.ReadFile(c.path(filepath.Join(osCase.trustDir, filename))) require.NoError(t, err) - require.Equal(t, cert, got) + want := cert + if mode.name == "legacy" { + want = []byte(strings.TrimSuffix(string(cert), "\n") + "\r\n") + } + require.Equal(t, want, got) assertCloudSchedule(t, c, osCase.name, commands) if mode.name == "legacy" { require.Len(t, requests, 1) @@ -226,23 +230,227 @@ func assertCloudSchedule(t *testing.T, c *cloudInitializer, osName string, comma require.NotContains(t, string(service), "init-aks-cloud.sh") } -func TestCloudCertificateOptInFailsClosed(t *testing.T) { - for _, response := range []string{"not json", `{}`, `null`, `{"IsOptedInForRootCerts":null}`, `{"IsOptedInForRootCerts":"false"}`} { +func TestCloudCertificateOptInNonTrueResponsesSkip(t *testing.T) { + for _, response := range []string{ + "not json", " ", `{}`, `null`, `[]`, `{"IsOptedInForRootCerts":false}`, + `{"IsOptedInForRootCerts":null}`, `{"IsOptedInForRootCerts":"false"}`, + `{"IsOptedInForRootCerts":"true"}`, `{"IsOptedInForRootCerts":1}`, + `{"isOptedInForRootCerts":true}`, + } { t.Run(response, func(t *testing.T) { c := newTestCloudInitializer(t, cloudTestUbuntu) - serveCloudTestResponses(t, c, nil, response) + requests := serveCloudTestResponses(t, c, nil, response) c.run = func(*exec.Cmd) error { - t.Fatal("must not update trust or schedules after an invalid opt-in response") + t.Fatal("must not update trust or schedules without explicit opt-in") return nil } - require.Error(t, c.initialize(t.Context(), cloudInitConfig{Location: "westus2", Timeout: time.Minute})) + require.NoError(t, c.initialize(t.Context(), cloudInitConfig{Location: "westus2", Timeout: time.Minute})) + require.Len(t, requests, 1) events := c.eventLogger.Events() require.NotEmpty(t, events) - require.Equal(t, string(helpers.EventLevelError), events[len(events)-1].EventLevel) + require.Equal(t, string(helpers.EventLevelInformational), events[len(events)-1].EventLevel) + }) + } +} + +func TestCloudCertificateOptInEmptyResponsesFail(t *testing.T) { + for _, response := range []string{"", "\n", "\n\n"} { + t.Run(fmt.Sprintf("%q", response), func(t *testing.T) { + c := newTestCloudInitializer(t, cloudTestUbuntu) + serveCloudTestResponses(t, c, nil, response) + require.ErrorContains(t, c.refreshCertificates(t.Context(), "westus2"), "response is empty") + }) + } +} + +func TestCloudCertificateRefreshClearsOnlyVisibleCachedFiles(t *testing.T) { + c := newTestCloudInitializer(t, cloudTestUbuntu) + serveCloudTestResponses(t, c, nil, `{"IsOptedInForRootCerts":false}`) + require.NoError(t, c.writeFile("/root/AzureCACertificates/stale.crt", []byte("stale"))) + for _, path := range []string{ + "/root/AzureCACertificates/.keep", + "/root/AzureCACertificates/nested/keep.crt", + "/usr/local/share/ca-certificates/existing.crt", + } { + require.NoError(t, c.writeFile(path, []byte("keep"))) + } + require.NoError(t, c.refreshCertificates(t.Context(), "westus2")) + _, err := os.Stat(c.path("/root/AzureCACertificates/stale.crt")) + require.ErrorIs(t, err, os.ErrNotExist) + for _, path := range []string{ + "/root/AzureCACertificates/.keep", + "/root/AzureCACertificates/nested/keep.crt", + "/usr/local/share/ca-certificates/existing.crt", + } { + data, err := os.ReadFile(c.path(path)) + require.NoError(t, err) + require.Equal(t, "keep", string(data)) + } +} + +func TestCloudCertificatePayloadsArePassedToTrustStoreTools(t *testing.T) { + for _, tc := range []struct{ name, location, want string }{ + {"legacy", "usnateast", "not a PEM certificate\r\n"}, + {"rcv1p", "westus2", "not a PEM certificate\n"}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Setenv("AKS_CLOUD_LOG_CERTIFICATES", "true") + logs := installLogCapturer(t) + c := newTestCloudInitializer(t, cloudTestUbuntu) + serveCloudTestResponses(t, c, []byte("not a PEM certificate\n"), `{"IsOptedInForRootCerts":true}`) + var commands []string + c.run = func(cmd *exec.Cmd) error { + commands = append(commands, cmd.Args[0]) + return nil + } + require.NoError(t, c.refreshCertificates(t.Context(), tc.location)) + got, err := os.ReadFile(c.path("/usr/local/share/ca-certificates/root.crt")) + require.NoError(t, err) + require.Equal(t, tc.want, string(got)) + require.Equal(t, []string{"update-ca-certificates"}, commands) + for _, record := range logs.getRecords() { + for _, value := range record.Attrs { + require.NotContains(t, value, "not a PEM certificate") + } + } }) } } +func TestCloudCertificateOperationDownloadFailures(t *testing.T) { + for _, tc := range []struct { + name string + roots []string + intermediates []string + wantInstalled []string + wantError bool + }{ + {"partial group", []string{"missing.crt", "root.crt"}, []string{"intermediate.crt"}, []string{"root.crt", "intermediate.crt"}, false}, + {"empty root group after failures", []string{"missing.crt"}, []string{"intermediate.crt"}, []string{"intermediate.crt"}, false}, + {"empty certificate response", []string{"empty.crt"}, []string{"intermediate.crt"}, []string{"intermediate.crt"}, false}, + {"non-crt root", []string{"root.cer"}, []string{"intermediate.crt"}, []string{"intermediate.crt"}, false}, + {"all downloads fail", []string{"missing.crt"}, []string{"missing.crt"}, nil, true}, + {"no crt files", []string{"root.cer"}, []string{"intermediate.cer"}, nil, true}, + } { + t.Run(tc.name, func(t *testing.T) { + c := newTestCloudInitializer(t, "ID=azurelinux\n") + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch kind := r.URL.Query().Get("type"); { + case r.URL.Path == "/acms/isOptedInForRootCerts": + _, _ = io.WriteString(w, `{"IsOptedInForRootCerts":true}`) + case kind == "operationrequestsroot", kind == "operationrequestsintermediate": + names := tc.roots + if kind == "operationrequestsintermediate" { + names = tc.intermediates + } + operations := make([]map[string]string, 0, len(names)) + for _, name := range names { + operations = append(operations, map[string]string{"ResouceFileName": name}) + } + _ = json.NewEncoder(w).Encode(map[string]any{"OperationsInfo": operations}) + case kind == "missing": + w.WriteHeader(http.StatusServiceUnavailable) + case kind == "empty": + _, _ = io.WriteString(w, "\n\n") + default: + _, _ = io.WriteString(w, "certificate payload\n\n") + } + })) + defer server.Close() + c.wireEndpoint = server.URL + var commands []string + c.run = func(cmd *exec.Cmd) error { + commands = append(commands, cmd.Args[0]) + return nil + } + err := c.refreshCertificates(t.Context(), "westus2") + if tc.wantError { + require.ErrorContains(t, err, "no *.crt files") + require.Empty(t, commands) + return + } + require.NoError(t, err) + require.Equal(t, []string{"update-ca-trust"}, commands) + for _, name := range tc.wantInstalled { + data, err := os.ReadFile(c.path("/etc/pki/ca-trust/source/anchors/" + name)) + require.NoError(t, err) + require.Equal(t, "certificate payload\n", string(data)) + } + for _, name := range []string{"missing.crt", "empty.crt", "root.cer"} { + _, err = os.Stat(c.path("/etc/pki/ca-trust/source/anchors/" + name)) + require.ErrorIs(t, err, os.ErrNotExist) + } + }) + } +} + +func TestCloudLegacyCertificateResponseBehavior(t *testing.T) { + for _, tc := range []struct { + name, response string + wantError bool + }{ + {"empty array", `{"Certificates":[]}`, false}, + {"non-crt file", `{"Certificates":[{"Name":"root.txt","CertBody":"payload"}]}`, false}, + {"empty body", `{"Certificates":[{"Name":"root.cer","CertBody":""}]}`, false}, + {"trust update failure", `{"Certificates":[{"Name":"root.cer","CertBody":"certificate payload"}]}`, false}, + {"invalid name", `{"Certificates":[{"Name":"../root.crt"}]}`, false}, + {"non-string name", `{"Certificates":[{"Name":123}]}`, false}, + {"missing array", `{}`, true}, + {"null array", `{"Certificates":null}`, true}, + {"missing body", `{"Certificates":[{"Name":"root.cer"}]}`, true}, + {"null body", `{"Certificates":[{"Name":"root.cer","CertBody":null}]}`, true}, + } { + t.Run(tc.name, func(t *testing.T) { + c := newTestCloudInitializer(t, "ID=azurelinux\n") + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = io.WriteString(w, tc.response) + })) + defer server.Close() + c.wireEndpoint = server.URL + var commands []string + c.run = func(cmd *exec.Cmd) error { + commands = append(commands, cmd.Args[0]) + if cmd.Args[0] == "update-ca-trust" { + return errors.New("trust store update failed") + } + return nil + } + err := c.initialize(t.Context(), cloudInitConfig{Location: "usnateast", Timeout: time.Minute}) + if tc.wantError { + require.Error(t, err) + require.Empty(t, commands) + } else { + require.NoError(t, err) + require.Contains(t, commands, "crontab") + } + }) + } +} + +func TestCloudLegacyCertificateInstallationDoesNotIgnoreCancellation(t *testing.T) { + c := newTestCloudInitializer(t, "ID=azurelinux\n") + serveCloudTestResponses(t, c, cloudTestCertificate(t), "not used") + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + c.run = func(*exec.Cmd) error { + cancel() + return ctx.Err() + } + require.ErrorIs(t, c.refreshCertificates(ctx, "usnateast"), context.Canceled) +} + +func TestCloudLegacyCertificateCacheWriteFailureStopsInitialization(t *testing.T) { + c := newTestCloudInitializer(t, "ID=azurelinux\n") + serveCloudTestResponses(t, c, cloudTestCertificate(t), "not used") + require.NoError(t, c.writeFile("/root/AzureCACertificates/root.crt/keep", []byte("keep"))) + c.run = func(*exec.Cmd) error { + t.Fatal("must not install certificates or schedules after a cache write failure") + return nil + } + err := c.initialize(t.Context(), cloudInitConfig{Location: "usnateast", Timeout: time.Minute}) + require.ErrorContains(t, err, "cache legacy Azure cloud certificate") +} + func TestCloudCertificateRequestRetryAndCancellation(t *testing.T) { for _, tc := range []struct { name string @@ -372,10 +580,12 @@ func TestCloudCertificateResponseLimits(t *testing.T) { } func TestCloudCertificateNamesAndPublicContent(t *testing.T) { - for _, name := range []string{"", ".", "..", "../root.crt", "/root.crt", `..\root.crt`, "root\x00.crt", "root\n.crt"} { + for _, name := range []string{"", ".", "..", "../root.crt", "/root.crt", "root\x00.crt"} { require.False(t, validCloudCertificateName(name), name) } - require.True(t, validCloudCertificateName("root+ca.crt")) + for _, name := range []string{"root+ca.crt", `..\root.crt`, "root\n.crt"} { + require.True(t, validCloudCertificateName(name), name) + } cert := cloudTestCertificate(t) private := pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: []byte("not-a-real-private-key")}) got, err := publicCloudCertificates(append(private, cert...)) @@ -395,7 +605,8 @@ func TestCloudCertificateLoggingIsOptInAndPublicOnly(t *testing.T) { c := newTestCloudInitializer(t, "ID=azurelinux\n") cert := cloudTestCertificate(t) secret := pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: []byte("not-a-real-private-key")}) - serveCloudTestResponses(t, c, append(secret, cert...), `{"IsOptedInForRootCerts":true}`) + payload := append(secret, cert...) + serveCloudTestResponses(t, c, payload, `{"IsOptedInForRootCerts":true}`) require.NoError(t, c.refreshCertificates(t.Context(), "westus2")) found := false for _, record := range logs.getRecords() { @@ -409,7 +620,7 @@ func TestCloudCertificateLoggingIsOptInAndPublicOnly(t *testing.T) { require.Equal(t, enabled == "true", found) got, err := os.ReadFile(c.path("/etc/pki/ca-trust/source/anchors/root.crt")) require.NoError(t, err) - require.Equal(t, cert, got) + require.Equal(t, payload, got) }) } } From d68e228f9c43df9cb727243b1b659e077eff1180 Mon Sep 17 00:00:00 2001 From: Abigailliang Date: Fri, 23 Oct 2026 18:45:25 -0700 Subject: [PATCH 3/5] chore: keep cloud certificate compatibility checks lint-clean Preserve the certificate compatibility fixes already present in the PR. Extract the operation-download test server helper, remove shadowing and slice-append warnings, and normalize empty-response errors without changing their return paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: be499bac-1283-498e-b5b1-59d8c91047a4 --- aks-node-controller/cloud_certificates.go | 4 +- aks-node-controller/cloud_init_test.go | 61 ++++++++++++----------- 2 files changed, 35 insertions(+), 30 deletions(-) diff --git a/aks-node-controller/cloud_certificates.go b/aks-node-controller/cloud_certificates.go index 92dd69030d7..8cdf2d0b5a3 100644 --- a/aks-node-controller/cloud_certificates.go +++ b/aks-node-controller/cloud_certificates.go @@ -27,7 +27,7 @@ func (c *cloudInitializer) refresh(ctx context.Context, platform cloudPlatform, return false, fmt.Errorf("check Azure cloud root certificate opt-in: %w", err) } if len(bytes.TrimRight(body, "\n")) == 0 { - return false, errors.New("Azure cloud root certificate opt-in response is empty") + return false, errors.New("cloud root certificate opt-in response is empty") } var response map[string]json.RawMessage if err := json.Unmarshal(body, &response); err != nil { @@ -214,7 +214,7 @@ func (c *cloudInitializer) operationCertificate(ctx context.Context, filename st } body = bytes.TrimRight(body, "\n") if len(body) == 0 { - return nil, fmt.Errorf("Azure cloud certificate %s response is empty", filename) + return nil, fmt.Errorf("cloud certificate %s response is empty", filename) } return append(body, '\n'), nil } diff --git a/aks-node-controller/cloud_init_test.go b/aks-node-controller/cloud_init_test.go index 9080518c6f2..21693a159db 100644 --- a/aks-node-controller/cloud_init_test.go +++ b/aks-node-controller/cloud_init_test.go @@ -101,6 +101,34 @@ func serveCloudTestResponses(t *testing.T, c *cloudInitializer, cert []byte, opt return requests } +func serveCloudOperationTestResponses(t *testing.T, c *cloudInitializer, roots, intermediates []string) { + t.Helper() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch kind := r.URL.Query().Get("type"); { + case r.URL.Path == "/acms/isOptedInForRootCerts": + _, _ = io.WriteString(w, `{"IsOptedInForRootCerts":true}`) + case kind == "operationrequestsroot", kind == "operationrequestsintermediate": + names := roots + if kind == "operationrequestsintermediate" { + names = intermediates + } + operations := make([]map[string]string, 0, len(names)) + for _, name := range names { + operations = append(operations, map[string]string{"ResouceFileName": name}) + } + _ = json.NewEncoder(w).Encode(map[string]any{"OperationsInfo": operations}) + case kind == "missing": + w.WriteHeader(http.StatusServiceUnavailable) + case kind == "empty": + _, _ = io.WriteString(w, "\n\n") + default: + _, _ = io.WriteString(w, "certificate payload\n\n") + } + })) + t.Cleanup(server.Close) + c.wireEndpoint = server.URL +} + func TestCloudFileWrites(t *testing.T) { t.Run("create and replace", func(t *testing.T) { path := filepath.Join(t.TempDir(), "certificates", "root.crt") @@ -334,30 +362,7 @@ func TestCloudCertificateOperationDownloadFailures(t *testing.T) { } { t.Run(tc.name, func(t *testing.T) { c := newTestCloudInitializer(t, "ID=azurelinux\n") - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch kind := r.URL.Query().Get("type"); { - case r.URL.Path == "/acms/isOptedInForRootCerts": - _, _ = io.WriteString(w, `{"IsOptedInForRootCerts":true}`) - case kind == "operationrequestsroot", kind == "operationrequestsintermediate": - names := tc.roots - if kind == "operationrequestsintermediate" { - names = tc.intermediates - } - operations := make([]map[string]string, 0, len(names)) - for _, name := range names { - operations = append(operations, map[string]string{"ResouceFileName": name}) - } - _ = json.NewEncoder(w).Encode(map[string]any{"OperationsInfo": operations}) - case kind == "missing": - w.WriteHeader(http.StatusServiceUnavailable) - case kind == "empty": - _, _ = io.WriteString(w, "\n\n") - default: - _, _ = io.WriteString(w, "certificate payload\n\n") - } - })) - defer server.Close() - c.wireEndpoint = server.URL + serveCloudOperationTestResponses(t, c, tc.roots, tc.intermediates) var commands []string c.run = func(cmd *exec.Cmd) error { commands = append(commands, cmd.Args[0]) @@ -372,8 +377,8 @@ func TestCloudCertificateOperationDownloadFailures(t *testing.T) { require.NoError(t, err) require.Equal(t, []string{"update-ca-trust"}, commands) for _, name := range tc.wantInstalled { - data, err := os.ReadFile(c.path("/etc/pki/ca-trust/source/anchors/" + name)) - require.NoError(t, err) + data, readErr := os.ReadFile(c.path("/etc/pki/ca-trust/source/anchors/" + name)) + require.NoError(t, readErr) require.Equal(t, "certificate payload\n", string(data)) } for _, name := range []string{"missing.crt", "empty.crt", "root.cer"} { @@ -604,8 +609,8 @@ func TestCloudCertificateLoggingIsOptInAndPublicOnly(t *testing.T) { logs := installLogCapturer(t) c := newTestCloudInitializer(t, "ID=azurelinux\n") cert := cloudTestCertificate(t) - secret := pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: []byte("not-a-real-private-key")}) - payload := append(secret, cert...) + payload := pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: []byte("not-a-real-private-key")}) + payload = append(payload, cert...) serveCloudTestResponses(t, c, payload, `{"IsOptedInForRootCerts":true}`) require.NoError(t, c.refreshCertificates(t.Context(), "westus2")) found := false From 9967df4c564dd40ce5ddbb2c77e1b339c8ad0abf Mon Sep 17 00:00:00 2001 From: Abigailliang Date: Fri, 23 Oct 2026 19:02:21 -0700 Subject: [PATCH 4/5] fix: update certificate cache clearing logic to remove hidden files and improve test clarity --- aks-node-controller/README.md | 4 ++-- aks-node-controller/cloud_certificates.go | 9 +------ aks-node-controller/cloud_init_test.go | 29 ++++++++--------------- 3 files changed, 13 insertions(+), 29 deletions(-) diff --git a/aks-node-controller/README.md b/aks-node-controller/README.md index d0a5fafb370..dd6a4bb4ff9 100644 --- a/aks-node-controller/README.md +++ b/aks-node-controller/README.md @@ -75,8 +75,8 @@ errors still fail, but legacy trust-store installation errors are logged without blocking repository initialization or refresh scheduling. Downloaded payloads are passed to the system trust-store tools without additional -PEM/X.509 validation. Refresh clears visible files in the certificate staging -directory, not existing certificates in the system trust store. Raw certificate +PEM/X.509 validation. Refresh clears all files, including hidden files, in the flat +certificate staging directory, not existing certificates in the system trust store. Raw certificate responses are never logged; `AKS_CLOUD_LOG_CERTIFICATES=true` enables diagnostics containing re-encoded public certificates only. Diagnostic parsing errors do not block installation. diff --git a/aks-node-controller/cloud_certificates.go b/aks-node-controller/cloud_certificates.go index 8cdf2d0b5a3..8b0cb8bfa52 100644 --- a/aks-node-controller/cloud_certificates.go +++ b/aks-node-controller/cloud_certificates.go @@ -74,15 +74,8 @@ func (c *cloudInitializer) clearCertificateCache() { slog.Warn("could not read Azure cloud certificate cache", "error", err) return } + // Certificate downloads are written directly here as files, without subdirectories. for _, entry := range entries { - // Match the shell's non-recursive cleanup, which excludes hidden files. - if strings.HasPrefix(entry.Name(), ".") { - continue - } - if entry.IsDir() { - slog.Warn("leaving directory in Azure cloud certificate cache", "name", entry.Name()) - continue - } if err := os.Remove(filepath.Join(directory, entry.Name())); err != nil { slog.Warn("could not remove cached Azure cloud certificate", "name", entry.Name(), "error", err) } diff --git a/aks-node-controller/cloud_init_test.go b/aks-node-controller/cloud_init_test.go index 21693a159db..d6292220de0 100644 --- a/aks-node-controller/cloud_init_test.go +++ b/aks-node-controller/cloud_init_test.go @@ -291,29 +291,20 @@ func TestCloudCertificateOptInEmptyResponsesFail(t *testing.T) { } } -func TestCloudCertificateRefreshClearsOnlyVisibleCachedFiles(t *testing.T) { +func TestCloudCertificateRefreshClearsCachedFiles(t *testing.T) { c := newTestCloudInitializer(t, cloudTestUbuntu) serveCloudTestResponses(t, c, nil, `{"IsOptedInForRootCerts":false}`) - require.NoError(t, c.writeFile("/root/AzureCACertificates/stale.crt", []byte("stale"))) - for _, path := range []string{ - "/root/AzureCACertificates/.keep", - "/root/AzureCACertificates/nested/keep.crt", - "/usr/local/share/ca-certificates/existing.crt", - } { - require.NoError(t, c.writeFile(path, []byte("keep"))) + for _, name := range []string{"stale.crt", ".stale.crt", "certificate-without-extension"} { + require.NoError(t, c.writeFile("/root/AzureCACertificates/"+name, []byte("stale"))) } + require.NoError(t, c.writeFile("/usr/local/share/ca-certificates/existing.crt", []byte("keep"))) require.NoError(t, c.refreshCertificates(t.Context(), "westus2")) - _, err := os.Stat(c.path("/root/AzureCACertificates/stale.crt")) - require.ErrorIs(t, err, os.ErrNotExist) - for _, path := range []string{ - "/root/AzureCACertificates/.keep", - "/root/AzureCACertificates/nested/keep.crt", - "/usr/local/share/ca-certificates/existing.crt", - } { - data, err := os.ReadFile(c.path(path)) - require.NoError(t, err) - require.Equal(t, "keep", string(data)) - } + entries, err := os.ReadDir(c.path("/root/AzureCACertificates")) + require.NoError(t, err) + require.Empty(t, entries) + data, err := os.ReadFile(c.path("/usr/local/share/ca-certificates/existing.crt")) + require.NoError(t, err) + require.Equal(t, "keep", string(data)) } func TestCloudCertificatePayloadsArePassedToTrustStoreTools(t *testing.T) { From ba9a4aacc7cb73b492d008f6bd205d21d3987114 Mon Sep 17 00:00:00 2001 From: Abigailliang Date: Mon, 26 Oct 2026 14:12:47 -0700 Subject: [PATCH 5/5] fix: preserve best-effort cloud initialization failures Keep cron read/install failures and exhausted RepoDepot cache refreshes from blocking native provisioning, matching the shell. Retain failure logging and repository error events without swallowing cancellation, repository setup errors, or systemd failures. Cover successful provisioning exit codes and preserved fatal boundaries. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: be499bac-1283-498e-b5b1-59d8c91047a4 --- aks-node-controller/README.md | 8 +- aks-node-controller/cloud_init.go | 24 +++- aks-node-controller/cloud_init_test.go | 71 ++++++++++-- aks-node-controller/cloud_provision_test.go | 107 ++++++++++++++++++ aks-node-controller/cloud_repositories.go | 7 +- .../cloud_repositories_test.go | 18 ++- 6 files changed, 219 insertions(+), 16 deletions(-) diff --git a/aks-node-controller/README.md b/aks-node-controller/README.md index dd6a4bb4ff9..c5f463a2a27 100644 --- a/aks-node-controller/README.md +++ b/aks-node-controller/README.md @@ -86,6 +86,9 @@ refresh using the running ANC binary, including when provisioning uses an ANC hotfix. Ubuntu, Mariner, and Azure Linux use cron; Flatcar and Azure Container Linux use the existing `azure-ca-refresh.service` and timer. Existing shell refresh entries are replaced. +As in the shell, an unreadable crontab is treated as empty. Unexpected read errors +and cron installation failures are logged without stopping initialization; +context cancellation and systemd refresh failures still stop initialization. The refresh command takes the location explicitly and does not configure repositories or reinstall its schedule: @@ -97,7 +100,10 @@ RepoDepot initialization is independent of certificate opt-in and runs only when a repository endpoint is configured. Ubuntu sources and Microsoft keyrings, Mariner mirrors, and Azure Linux repositories retain their existing roles; immutable Flatcar/ACL images do not configure package repositories. Cache refreshes -retry up to ten times and report exhausted failures. Native initialization honors +retry up to ten times and report exhausted failures in logs and repository events +without blocking provisioning, matching the shell's best-effort handling. The +cache operation still reports failure; repository configuration errors and +context cancellation still stop initialization. Native initialization honors the configured CSE timeout, with 30-second HTTP requests and two-minute command timeouts. Periodic certificate refresh has a 15-minute budget. `provision --dry-run` does not fetch certificates, change files, or install refresh schedules. diff --git a/aks-node-controller/cloud_init.go b/aks-node-controller/cloud_init.go index 2867f3af586..581eb2d5c7e 100644 --- a/aks-node-controller/cloud_init.go +++ b/aks-node-controller/cloud_init.go @@ -155,9 +155,18 @@ func (c *cloudInitializer) initialize(ctx context.Context, config cloudInitConfi return fmt.Errorf("schedule Azure cloud certificate refresh: %w", err) } } - return c.withEvent("CloudRepositoryInitialization", func() error { + repositoryErr := c.withEvent("CloudRepositoryInitialization", func() error { return c.initializeRepositories(initCtx, platform, config.RepoDepotEndpoint) }) + if errors.Is(repositoryErr, errCloudRepositoryCacheRefresh) { + if contextErr := initCtx.Err(); contextErr != nil { + return contextErr + } + // The shell reports cache-refresh failures without propagating them to CSE. + slog.Warn("RepoDepot package cache refresh failed, continuing cloud initialization", "error", repositoryErr) + return nil + } + return repositoryErr }) } @@ -385,9 +394,12 @@ func (c *cloudInitializer) installSystemdRefresh(ctx context.Context, command st func (c *cloudInitializer) installCronRefresh(ctx context.Context, command string) error { existing, err := c.command(ctx, nil, "crontab", "-l") if err != nil { + if contextErr := ctx.Err(); contextErr != nil { + return contextErr + } var exitErr interface{ ExitCode() int } if !errors.As(err, &exitErr) || exitErr.ExitCode() != 1 || !strings.Contains(err.Error(), "no crontab for") { - return err + slog.Warn("could not read existing crontab, continuing with an empty crontab", "error", err) } existing = nil } @@ -402,5 +414,11 @@ func (c *cloudInitializer) installCronRefresh(ctx context.Context, command strin } lines = append(lines, "0 19 * * * "+command) _, err = c.command(ctx, []byte(strings.Join(lines, "\n")+"\n"), "crontab", "-") - return err + if err != nil { + if contextErr := ctx.Err(); contextErr != nil { + return contextErr + } + slog.Warn("could not install Azure cloud certificate refresh cron job, continuing cloud initialization", "error", err) + } + return nil } diff --git a/aks-node-controller/cloud_init_test.go b/aks-node-controller/cloud_init_test.go index d6292220de0..45be627ceef 100644 --- a/aks-node-controller/cloud_init_test.go +++ b/aks-node-controller/cloud_init_test.go @@ -687,10 +687,9 @@ func TestCloudRefreshScheduleReplacesLegacyCron(t *testing.T) { func TestCloudRefreshScheduleCrontabErrors(t *testing.T) { for _, tc := range []struct { name, stderr string - wantError bool }{ - {"no existing crontab", "no crontab for root", false}, - {"permission denied", "permission denied", true}, + {"no existing crontab", "no crontab for root"}, + {"permission denied", "permission denied"}, } { t.Run(tc.name, func(t *testing.T) { c := newTestCloudInitializer(t, cloudTestUbuntu) @@ -704,13 +703,66 @@ func TestCloudRefreshScheduleCrontabErrors(t *testing.T) { return nil } err := c.installRefreshSchedule(t.Context(), cloudPlatform{ID: osReleaseIDUbuntu}, "westus2") - if tc.wantError { - require.Error(t, err) - require.Zero(t, writes) - } else { - require.NoError(t, err) - require.Equal(t, 1, writes) + require.NoError(t, err) + require.Equal(t, 1, writes) + }) + } +} + +func TestCloudCronInstallationFailureIsLogged(t *testing.T) { + c := newTestCloudInitializer(t, cloudTestUbuntu) + logs := installLogCapturer(t) + c.run = func(cmd *exec.Cmd) error { + if cmd.Args[1] == "-" { + return errors.New("cron installation failed") + } + return nil + } + require.NoError(t, c.installRefreshSchedule(t.Context(), cloudPlatform{ID: osReleaseIDUbuntu}, "westus2")) + require.Contains(t, fmt.Sprint(logs.getRecords()), "cron installation failed") +} + +func TestCloudCronCancellationIsNotIgnored(t *testing.T) { + for _, canceledArg := range []string{"-l", "-"} { + t.Run(canceledArg, func(t *testing.T) { + c := newTestCloudInitializer(t, cloudTestUbuntu) + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + c.run = func(cmd *exec.Cmd) error { + if cmd.Args[1] == canceledArg { + cancel() + return ctx.Err() + } + return nil + } + require.ErrorIs(t, c.installRefreshSchedule(ctx, cloudPlatform{ID: osReleaseIDUbuntu}, "westus2"), context.Canceled) + }) + } +} + +func TestCloudCronDeadlineIsNotIgnored(t *testing.T) { + c := newTestCloudInitializer(t, cloudTestUbuntu) + ctx, cancel := context.WithDeadline(t.Context(), time.Now().Add(-time.Second)) + defer cancel() + c.run = func(*exec.Cmd) error { + t.Fatal("an expired context must not execute crontab") + return nil + } + require.ErrorIs(t, c.installRefreshSchedule(ctx, cloudPlatform{ID: osReleaseIDUbuntu}, "westus2"), context.DeadlineExceeded) +} + +func TestCloudSystemdRefreshFailuresRemainFatal(t *testing.T) { + for _, failedArgs := range []string{"daemon-reload", "enable --now azure-ca-refresh.timer"} { + t.Run(failedArgs, func(t *testing.T) { + c := newTestCloudInitializer(t, "ID=flatcar\n") + c.run = func(cmd *exec.Cmd) error { + if strings.Join(cmd.Args[1:], " ") == failedArgs { + return errors.New("systemd operation failed") + } + return nil } + require.ErrorContains(t, c.installRefreshSchedule(t.Context(), cloudPlatform{ID: osReleaseIDFlatcar}, "westus2"), + "systemd operation failed") }) } } @@ -720,7 +772,6 @@ func TestCloudInitializationStopsOnTrustAndScheduleFailures(t *testing.T) { name, location, failedCommand, wantError string }{ {"trust store", "westus2", "update-ca-trust", "host operation failed"}, - {"crontab", "westus2", "crontab", "host operation failed"}, {"missing location", " \t", "", "requires a location"}, } { t.Run(tc.name, func(t *testing.T) { diff --git a/aks-node-controller/cloud_provision_test.go b/aks-node-controller/cloud_provision_test.go index db16756978a..1923cf81223 100644 --- a/aks-node-controller/cloud_provision_test.go +++ b/aks-node-controller/cloud_provision_test.go @@ -19,6 +19,113 @@ func writeCloudProvisionConfig(t *testing.T) string { return path } +func requireCloudInitializationAllowsProvisioning(t *testing.T, c *cloudInitializer) { + t.Helper() + called := false + tt := NewTestApp(t, TestAppConfig{RunFunc: func(cmd *exec.Cmd) error { + called = true + require.Contains(t, cmd.String(), "/opt/azure/containers/provision_start.sh") + path, lookupErr := exec.LookPath("true") + require.NoError(t, lookupErr) + // Execute a harmless stub so Provision observes a real zero exit status. + cmd.Path, cmd.Args = path, []string{path} + return cmd.Run() + }}) + tt.App.initializeCloud = func(ctx context.Context, config cloudInitConfig) error { + config.RepoDepotEndpoint = c.wireEndpoint + return c.initialize(ctx, config) + } + result, err := tt.App.Provision(t.Context(), ProvisionFlags{ProvisionConfig: writeCloudProvisionConfig(t)}) + require.NoError(t, err) + require.True(t, called) + require.Equal(t, "0", result.ExitCode) +} + +func TestNativeCloudCronFailuresDoNotBlockProvisioning(t *testing.T) { + for _, failedArg := range []string{"-l", "-"} { + t.Run(failedArg, func(t *testing.T) { + c := newTestCloudInitializer(t, cloudTestUbuntu) + serveCloudTestResponses(t, c, cloudTestCertificate(t), `{"IsOptedInForRootCerts":true}`) + run := c.run + writes, cacheUpdates := 0, 0 + c.run = func(cmd *exec.Cmd) error { + if cmd.Args[0] == "crontab" { + if cmd.Args[1] == "-" { + writes++ + } + if cmd.Args[1] == failedArg { + return errors.New("crontab operation failed") + } + } + if cmd.Args[0] == "apt-get" { + cacheUpdates++ + } + return run(cmd) + } + requireCloudInitializationAllowsProvisioning(t, c) + require.Equal(t, 1, writes) + require.Equal(t, 1, cacheUpdates) + }) + } +} + +func TestNativeCloudCacheFailuresDoNotBlockProvisioning(t *testing.T) { + for _, tc := range []struct{ id, release, command string }{ + {osReleaseIDUbuntu, cloudTestUbuntu, "apt-get"}, + {osReleaseIDMariner, "ID=mariner\n", "dnf"}, + {osReleaseIDAzureLinux, "ID=azurelinux\n", "dnf"}, + } { + t.Run(tc.id, func(t *testing.T) { + c := newTestCloudInitializer(t, tc.release) + serveCloudTestResponses(t, c, cloudTestCertificate(t), `{"IsOptedInForRootCerts":true}`) + if tc.id == osReleaseIDMariner { + require.NoError(t, c.writeFile(defaultYumReposDir+"/mariner-extras.repo", + []byte("[mariner-extras]\nbaseurl=https://packages.microsoft.com/extras\n"))) + } + run := c.run + attempts := 0 + c.run = func(cmd *exec.Cmd) error { + if cmd.Args[0] == tc.command { + attempts++ + return errors.New("cache refresh failed") + } + return run(cmd) + } + requireCloudInitializationAllowsProvisioning(t, c) + require.Equal(t, cloudRequestAttempts, attempts) + levels := make(map[string]string) + for _, event := range c.eventLogger.Events() { + levels[event.TaskName] = event.EventLevel + } + require.Equal(t, "Error", levels["AKS.AKSNodeController.CloudRepositoryInitialization"]) + require.Equal(t, "Informational", levels["AKS.AKSNodeController.CloudInitialization"]) + }) + } +} + +func TestNativeCloudCacheCancellationStillBlocksProvisioning(t *testing.T) { + c := newTestCloudInitializer(t, "ID=azurelinux\n") + serveCloudTestResponses(t, c, nil, `{"IsOptedInForRootCerts":false}`) + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + c.run = func(cmd *exec.Cmd) error { + require.Equal(t, "dnf", cmd.Args[0]) + cancel() + return ctx.Err() + } + tt := NewTestApp(t, TestAppConfig{RunFunc: func(*exec.Cmd) error { + t.Fatal("provisioning must not run after initialization is canceled") + return nil + }}) + tt.App.initializeCloud = func(ctx context.Context, config cloudInitConfig) error { + config.RepoDepotEndpoint = c.wireEndpoint + return c.initialize(ctx, config) + } + result, err := tt.App.Provision(ctx, ProvisionFlags{ProvisionConfig: writeCloudProvisionConfig(t)}) + require.ErrorIs(t, err, context.Canceled) + require.Equal(t, "240", result.ExitCode) +} + func TestNativeCloudInitializationPrecedesProvisioning(t *testing.T) { var order []string tt := NewTestApp(t, TestAppConfig{RunFunc: func(*exec.Cmd) error { diff --git a/aks-node-controller/cloud_repositories.go b/aks-node-controller/cloud_repositories.go index e71a0affce6..6a1512e6afb 100644 --- a/aks-node-controller/cloud_repositories.go +++ b/aks-node-controller/cloud_repositories.go @@ -47,6 +47,8 @@ sslverify=1 ` ) +var errCloudRepositoryCacheRefresh = errors.New("refresh RepoDepot package cache") + func (c *cloudInitializer) initializeRepositories(ctx context.Context, platform cloudPlatform, endpoint string) error { if endpoint == "" { slog.Info("RepoDepot endpoint is empty, skipping repository initialization") @@ -255,7 +257,10 @@ func (c *cloudInitializer) updateRepositoryCache(ctx context.Context, name strin return struct{}{}, err }) if err != nil { - return fmt.Errorf("refresh RepoDepot package cache: %w", err) + if contextErr := ctx.Err(); contextErr != nil { + return fmt.Errorf("refresh RepoDepot package cache: %w", contextErr) + } + return fmt.Errorf("%w: %w", errCloudRepositoryCacheRefresh, err) } return nil } diff --git a/aks-node-controller/cloud_repositories_test.go b/aks-node-controller/cloud_repositories_test.go index 7ec5f3ed299..61e17a5f5a4 100644 --- a/aks-node-controller/cloud_repositories_test.go +++ b/aks-node-controller/cloud_repositories_test.go @@ -122,6 +122,19 @@ func TestCloudRepositoryEndpointValidation(t *testing.T) { } } +func TestCloudRepositorySetupFailureStillStopsInitialization(t *testing.T) { + c := newTestCloudInitializer(t, cloudTestUbuntu) + serveCloudTestResponses(t, c, nil, `{"IsOptedInForRootCerts":false}`) + c.run = func(*exec.Cmd) error { + t.Fatal("an invalid endpoint must not reach package cache refresh") + return nil + } + err := c.initialize(t.Context(), cloudInitConfig{ + Location: "westus2", RepoDepotEndpoint: "not-a-url", Timeout: time.Minute, + }) + require.ErrorContains(t, err, "invalid RepoDepot endpoint") +} + func TestCloudRepositoryCacheRetriesAndFailsExplicitly(t *testing.T) { for _, name := range []string{"apt-get", "dnf"} { for _, recoverAfter := range []int{2, cloudRequestAttempts + 1} { @@ -142,6 +155,7 @@ func TestCloudRepositoryCacheRetriesAndFailsExplicitly(t *testing.T) { err := c.updateRepositoryCache(t.Context(), name, "update") if recoverAfter > cloudRequestAttempts { require.ErrorContains(t, err, "refresh RepoDepot package cache") + require.ErrorIs(t, err, errCloudRepositoryCacheRefresh) require.Equal(t, cloudRequestAttempts, attempts) } else { require.NoError(t, err) @@ -158,6 +172,8 @@ func TestCloudRepositoryCacheRetriesAndFailsExplicitly(t *testing.T) { } ctx, cancel := context.WithCancel(t.Context()) cancel() - require.ErrorIs(t, c.updateRepositoryCache(ctx, "apt-get", "update"), context.Canceled) + err := c.updateRepositoryCache(ctx, "apt-get", "update") + require.ErrorIs(t, err, context.Canceled) + require.NotErrorIs(t, err, errCloudRepositoryCacheRefresh) }) }