From 34bdc87a117ff56bda7c8b112a3d1847feb96968 Mon Sep 17 00:00:00 2001 From: Sylvain Boily <4981802+djsly@users.noreply.github.com> Date: Sun, 7 Jun 2026 11:57:23 -0400 Subject: [PATCH] Skip gRPC DNS lookup when APISERVER_IP env var is set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When STLS bootstrap runs on a node with a broken DNS path (CoreDNS not ready, systemd-resolved race, custom DNS misconfig), the gRPC built-in dns:/// resolver re-resolves the apiserver FQDN on every retry and the client loops forever emitting 'name resolver error: produced zero addresses' (reference incident VMId b409a057-44a0-4b06-a6de-2e24e59a90a5, 53+ hours of retries on Jun 5-6). This change adds a pre-resolved IP escape hatch: * New Config field APIServerIP (json: apiServerIp). * applyDefaults() falls back to the APISERVER_IP environment variable when the json field is empty. Invalid IP literals are cleared silently so a bad value never fails bootstrap. * getServiceClient() now delegates target selection to a new getDialParams() helper. When APIServerIP is set we dial via 'passthrough:///:443', set tls.Config.ServerName to the FQDN so SAN validation continues to match, and pass WithAuthority(:443) so the :authority header is unchanged from the FQDN path. * When APIServerIP is empty the historical FQDN-only dial is preserved verbatim — full backward compatibility with old AgentBaker CSE that does not yet set the env var. Unit tests cover the new env-var pickup, json-precedence, IPv6 acceptance, and silent-clear-on-invalid behavior, plus assertions on the getDialParams output for FQDN-only, IPv4, and IPv6 inputs. Companion AgentBaker change (cse_config.sh) writes APISERVER_IP into /etc/default/secure-tls-bootstrap from IMDS tags / getent. Tracking: AB#38327357 (parent feature AB#34681743) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- client/internal/bootstrap/config.go | 28 +++++++++ client/internal/bootstrap/config_test.go | 77 ++++++++++++++++++++++++ client/internal/bootstrap/grpc.go | 34 ++++++++++- client/internal/bootstrap/grpc_test.go | 44 ++++++++++++++ 4 files changed, 180 insertions(+), 3 deletions(-) diff --git a/client/internal/bootstrap/config.go b/client/internal/bootstrap/config.go index 5479ac9..7a5ed8f 100644 --- a/client/internal/bootstrap/config.go +++ b/client/internal/bootstrap/config.go @@ -6,6 +6,7 @@ package bootstrap import ( "encoding/json" "fmt" + "net" "os" "time" @@ -13,6 +14,14 @@ import ( "go.uber.org/zap" ) +// apiServerIPEnvVar is the name of the environment variable the AgentBaker CSE +// scripts (parts/linux/cloud-init/artifacts/cse_config.sh) drop into +// /etc/default/secure-tls-bootstrap with the pre-resolved apiserver IP. When +// set, the gRPC client dials this IP literal directly and never invokes the +// built-in dns:/// resolver, which avoids infinite "produced zero addresses" +// retries when node DNS is unhealthy at bootstrap time. +const apiServerIPEnvVar = "APISERVER_IP" + const ( defaultTLSMinVersion = "1.3" defaultValidateKubeconfigTimeout = 15 * time.Second @@ -27,6 +36,12 @@ type Config struct { CloudProviderConfig *cloud.ProviderConfig CloudProviderConfigPath string `json:"cloudProviderConfigPath"` APIServerFQDN string `json:"apiServerFqdn"` + // APIServerIP, when non-empty, must be a valid IPv4 or IPv6 literal. It is + // populated either via the json config file or, by default, by reading the + // APISERVER_IP environment variable in applyDefaults. When set, the gRPC + // client dials this IP directly via the passthrough resolver and skips + // DNS resolution entirely. See AB#38327357. + APIServerIP string `json:"apiServerIp"` UserAssignedIdentityID string `json:"userAssignedIdentityId"` NextProto string `json:"nextProto"` AADResource string `json:"aadResource"` @@ -67,6 +82,7 @@ func (c *Config) ToZapFields() []zap.Field { return []zap.Field{ zap.String("cloudProviderConfigPath", c.CloudProviderConfigPath), zap.String("apiServerFqdn", c.APIServerFQDN), + zap.String("apiServerIp", c.APIServerIP), zap.String("userAssignedIdentityId", c.UserAssignedIdentityID), zap.String("nextProto", c.NextProto), zap.String("aadResource", c.AADResource), @@ -86,6 +102,18 @@ func (c *Config) ToZapFields() []zap.Field { } func (c *Config) applyDefaults() { + // If no APIServerIP was supplied through the json config, fall back to the + // APISERVER_IP environment variable. Reject anything that does not parse + // as a valid IPv4 or IPv6 literal — the gRPC client treats this field as + // optional, so silently clearing on bad input is preferable to failing + // the whole bootstrap. The downstream consumer then falls back to dialing + // by FQDN, which preserves the pre-AB#38327357 behavior. + if c.APIServerIP == "" { + c.APIServerIP = os.Getenv(apiServerIPEnvVar) + } + if c.APIServerIP != "" && net.ParseIP(c.APIServerIP) == nil { + c.APIServerIP = "" + } if c.TLSMinVersion == "" { c.TLSMinVersion = defaultTLSMinVersion } diff --git a/client/internal/bootstrap/config_test.go b/client/internal/bootstrap/config_test.go index 069d513..b6d79e5 100644 --- a/client/internal/bootstrap/config_test.go +++ b/client/internal/bootstrap/config_test.go @@ -304,6 +304,7 @@ func TestToZapFields(t *testing.T) { config := &Config{ CloudProviderConfigPath: "path/to/azure.json", APIServerFQDN: "fqdn", + APIServerIP: "10.0.0.1", UserAssignedIdentityID: "clientId", NextProto: "alpn", AADResource: "appID", @@ -323,6 +324,7 @@ func TestToZapFields(t *testing.T) { expectedFields := []zap.Field{ zap.String("cloudProviderConfigPath", "path/to/azure.json"), zap.String("apiServerFqdn", "fqdn"), + zap.String("apiServerIp", "10.0.0.1"), zap.String("userAssignedIdentityId", "clientId"), zap.String("nextProto", "alpn"), zap.String("aadResource", "appID"), @@ -341,3 +343,78 @@ func TestToZapFields(t *testing.T) { } assert.Equal(t, expectedFields, config.ToZapFields()) } + +// TestApplyDefaultsAPIServerIP verifies the APISERVER_IP fallback path added +// for AB#38327357. +func TestApplyDefaultsAPIServerIP(t *testing.T) { + cases := []struct { + name string + envValue string + preset string + expectIP string + setEnvVar bool + }{ + { + name: "env var ipv4 picked up when field empty", + envValue: "10.0.0.1", + preset: "", + expectIP: "10.0.0.1", + setEnvVar: true, + }, + { + name: "env var ipv6 picked up when field empty", + envValue: "2001:db8::1", + preset: "", + expectIP: "2001:db8::1", + setEnvVar: true, + }, + { + name: "json field takes precedence over env var", + envValue: "10.0.0.99", + preset: "10.0.0.1", + expectIP: "10.0.0.1", + setEnvVar: true, + }, + { + name: "invalid env var literal cleared silently", + envValue: "not-an-ip", + preset: "", + expectIP: "", + setEnvVar: true, + }, + { + name: "invalid json field cleared silently", + envValue: "", + preset: "256.300.0.1", + expectIP: "", + setEnvVar: false, + }, + { + name: "no env var, no preset leaves empty", + envValue: "", + preset: "", + expectIP: "", + setEnvVar: false, + }, + { + name: "ipv4 in brackets is rejected (not a literal)", + envValue: "[10.0.0.1]", + preset: "", + expectIP: "", + setEnvVar: true, + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + if c.setEnvVar { + t.Setenv(apiServerIPEnvVar, c.envValue) + } else { + t.Setenv(apiServerIPEnvVar, "") + } + cfg := &Config{APIServerIP: c.preset} + cfg.applyDefaults() + assert.Equal(t, c.expectIP, cfg.APIServerIP) + }) + } +} + diff --git a/client/internal/bootstrap/grpc.go b/client/internal/bootstrap/grpc.go index dc611e5..9581ef7 100644 --- a/client/internal/bootstrap/grpc.go +++ b/client/internal/bootstrap/grpc.go @@ -9,6 +9,7 @@ import ( "crypto/x509" "fmt" "math" + "net" "os" "time" @@ -36,6 +37,24 @@ type closeFunc func() error // getServiceClientFunc returns a new SecureTLSBootstrapServiceClient over a gRPC connection, fake implementations given in unit tests. type getServiceClientFunc func(token string, cfg *Config) (v1.SecureTLSBootstrapServiceClient, closeFunc, error) +// getDialParams returns the gRPC NewClient target string and any extra dial +// options needed for that target. When APIServerIP is set, it is treated as an +// IPv4/IPv6 literal (already validated in Config.applyDefaults) and used with +// the passthrough resolver so gRPC never invokes the built-in dns:/// resolver. +// In that case we also pass WithAuthority so the gRPC :authority header still +// carries the apiserver FQDN — TLS SAN validation continues to match the FQDN +// via tlsConfig.ServerName (set by the caller). See AB#38327357. +// +// When APIServerIP is empty, this preserves the historical FQDN-only dial. +func getDialParams(cfg *Config) (target string, extraOpts []grpc.DialOption) { + if cfg.APIServerIP != "" { + hostPort := net.JoinHostPort(cfg.APIServerIP, "443") + authority := net.JoinHostPort(cfg.APIServerFQDN, "443") + return "passthrough:///" + hostPort, []grpc.DialOption{grpc.WithAuthority(authority)} + } + return fmt.Sprintf("%s:443", cfg.APIServerFQDN), nil +} + func getServiceClient(token string, cfg *Config) (v1.SecureTLSBootstrapServiceClient, closeFunc, error) { clusterCAData, err := os.ReadFile(cfg.ClusterCAFilePath) if err != nil { @@ -46,6 +65,12 @@ func getServiceClient(token string, cfg *Config) (v1.SecureTLSBootstrapServiceCl if err != nil { return nil, nil, fmt.Errorf("failed to get TLS config: %w", err) } + // When dialing by IP (APISERVER_IP set), force TLS SAN validation against + // the apiserver FQDN, not the IP literal. The apiserver cert does not list + // the IP as a SAN. + if cfg.APIServerIP != "" { + tlsConfig.ServerName = cfg.APIServerFQDN + } // override max delay to 3s (default is 120s) - this ensures the gRPC subchannel // re-attempts a real TCP+TLS connection at least every 3s, which aligns with @@ -55,8 +80,8 @@ func getServiceClient(token string, cfg *Config) (v1.SecureTLSBootstrapServiceCl grpcConnectionBackoffConfig := backoff.DefaultConfig grpcConnectionBackoffConfig.MaxDelay = 3 * time.Second - conn, err := grpc.NewClient( - fmt.Sprintf("%s:443", cfg.APIServerFQDN), + dialTarget, extraDialOpts := getDialParams(cfg) + dialOpts := []grpc.DialOption{ grpc.WithUserAgent(internalhttp.GetUserAgent()), grpc.WithTransportCredentials(credentials.NewTLS(tlsConfig)), grpc.WithPerRPCCredentials(oauth.TokenSource{ @@ -85,7 +110,10 @@ func getServiceClient(token string, cfg *Config) (v1.SecureTLSBootstrapServiceCl // enough to have the client bypass any proxies when communicating with the cluster's apiserver. // see: https://github.com/grpc/grpc-go/issues/3401 for more details. grpc.WithNoProxy(), - ) + } + dialOpts = append(dialOpts, extraDialOpts...) + + conn, err := grpc.NewClient(dialTarget, dialOpts...) if err != nil { return nil, nil, fmt.Errorf("failed to dial client connection with context: %w", err) } diff --git a/client/internal/bootstrap/grpc_test.go b/client/internal/bootstrap/grpc_test.go index c0cb268..7433fdf 100644 --- a/client/internal/bootstrap/grpc_test.go +++ b/client/internal/bootstrap/grpc_test.go @@ -234,3 +234,47 @@ func TestWithLastGRPCRetryErrorIfDeadlineExceeded(t *testing.T) { }) } } + +// TestGetDialParams covers the IP-vs-FQDN dial target selection added for +// AB#38327357 — when APIServerIP is set we must use the gRPC passthrough +// resolver so the built-in dns:/// resolver is never invoked. +func TestGetDialParams(t *testing.T) { + cases := []struct { + name string + cfg *Config + expectTarget string + expectExtraOpts int + }{ + { + name: "fqdn only — historical dial preserved", + cfg: &Config{APIServerFQDN: "my-aks.hcp.eastus.azmk8s.io"}, + expectTarget: "my-aks.hcp.eastus.azmk8s.io:443", + expectExtraOpts: 0, + }, + { + name: "ipv4 set — passthrough target plus WithAuthority", + cfg: &Config{ + APIServerFQDN: "my-aks.hcp.eastus.azmk8s.io", + APIServerIP: "10.0.0.1", + }, + expectTarget: "passthrough:///10.0.0.1:443", + expectExtraOpts: 1, + }, + { + name: "ipv6 set — bracketed host:port", + cfg: &Config{ + APIServerFQDN: "my-aks.hcp.eastus.azmk8s.io", + APIServerIP: "2001:db8::1", + }, + expectTarget: "passthrough:///[2001:db8::1]:443", + expectExtraOpts: 1, + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + target, extraOpts := getDialParams(c.cfg) + assert.Equal(t, c.expectTarget, target) + assert.Len(t, extraOpts, c.expectExtraOpts) + }) + } +}