diff --git a/phase/configure_k0s.go b/phase/configure_k0s.go index ee913c63c..12a024fd9 100644 --- a/phase/configure_k0s.go +++ b/phase/configure_k0s.go @@ -95,50 +95,25 @@ func (p *ConfigureK0s) Prepare(config *v1beta1.Cluster) error { // do nothing - base k0s config does not contain any existing SANs } - // populate SANs with all controller addresses for i, c := range p.Config.Spec.Hosts.Controllers() { if c.Reset { continue } - if !slices.Contains(sans, c.Address()) { - sans = append(sans, c.Address()) - log.Debugf("added controller %d address %s to spec.api.sans", i+1, c.Address()) - } - if c.PrivateAddress != "" && !slices.Contains(sans, c.PrivateAddress) { - sans = append(sans, c.PrivateAddress) - log.Debugf("added controller %d private address %s to spec.api.sans", i+1, c.PrivateAddress) - } - } - // assign populated sans to the base config - p.newBaseConfig.DigMapping("spec", "api")["sans"] = sans + // keep dedicated SANs list per controller, + // to avoid k0s config regeneration and k0scontroller restarts + // when adding new CP node to the cluster + log.Debugf("building spec.api.sans for controller %d (%s)", i+1, c.Address()) - for _, h := range p.Config.Spec.Hosts.Controllers() { - if h.Reset { - continue - } + // store sans to config + controllerBaseConfig := p.newBaseConfig.Dup() + controllerBaseConfig.DigMapping("spec", "api")["sans"] = hostsSans(sans, c) - cfgNew, err := p.configFor(h) - if err != nil { - return fmt.Errorf("failed to build k0s config for %s: %w", h, err) - } - tempConfigPath, err := h.FS().CreateTemp("", "") + cfgNew, err := p.configFor(c, controllerBaseConfig) if err != nil { - return fmt.Errorf("failed to create temporary file for config: %w", err) + return fmt.Errorf("failed to build k0s config for %s: %w", c, err) } - defer func() { - if err := h.Sudo().FS().Remove(tempConfigPath); err != nil { - log.Warnf("%s: failed to delete temporary file %s: %s", h, tempConfigPath, err) - } - }() - - if err := h.Sudo().FS().WriteFile(tempConfigPath, []byte(cfgNew), 0o600); err != nil { - return err - } - - // Prepare has no ctx; the Prepare interface takes only *v1beta1.Cluster. - // TODO: thread a real ctx if the Prepare interface ever takes a context. - if err := p.validateConfig(context.Background(), h, tempConfigPath); err != nil { + if err := p.validateConfigWithTempFile(c, cfgNew); err != nil { return err } @@ -147,7 +122,7 @@ func (p *ConfigureK0s) Prepare(config *v1beta1.Cluster) error { if err := yaml.Unmarshal([]byte(cfgNew), &cfgA); err != nil { return fmt.Errorf("failed to unmarshal new config: %w", err) } - if err := yaml.Unmarshal([]byte(h.Metadata.K0sExistingConfig), &cfgB); err != nil { + if err := yaml.Unmarshal([]byte(c.Metadata.K0sExistingConfig), &cfgB); err != nil { return fmt.Errorf("failed to unmarshal existing config: %w", err) } cfgAString, err := yaml.Marshal(cfgA) @@ -160,13 +135,13 @@ func (p *ConfigureK0s) Prepare(config *v1beta1.Cluster) error { } if bytes.Equal(cfgAString, cfgBString) { - log.Debugf("%s: configuration will not change", h) + log.Debugf("%s: configuration will not change", c) continue } - log.Debugf("%s: configuration will change", h) - h.Metadata.K0sNewConfig = cfgNew - p.hosts = append(p.hosts, h) + log.Debugf("%s: configuration will change", c) + c.Metadata.K0sNewConfig = cfgNew + p.hosts = append(p.hosts, c) } return nil @@ -228,6 +203,20 @@ func (p *ConfigureK0s) Run(ctx context.Context) error { return p.parallelDo(ctx, controllers, p.configureK0s) } +// hostsSans returns a dedicated copy of the base sans extended with the +// host's own addresses. +func hostsSans(sans []string, host *cluster.Host) []string { + out := make([]string, len(sans)) + copy(out, sans) + if !slices.Contains(out, host.Address()) { + out = append(out, host.Address()) + } + if host.PrivateAddress != "" && !slices.Contains(out, host.PrivateAddress) { + out = append(out, host.PrivateAddress) + } + return out +} + func requiresIPv6NodeLocalAPIAddress(cfg dig.Mapping) bool { if cfg == nil { return false @@ -241,6 +230,28 @@ func requiresIPv6NodeLocalAPIAddress(cfg dig.Mapping) bool { return false } +// validateConfigWithTempFile writes cfg to a temporary file on the host, +// runs k0s config validation against it and removes the file afterwards. +func (p *ConfigureK0s) validateConfigWithTempFile(c *cluster.Host, cfg string) error { + tempConfigPath, err := c.FS().CreateTemp("", "") + if err != nil { + return fmt.Errorf("failed to create temporary file for config: %w", err) + } + defer func() { + if err := c.Sudo().FS().Remove(tempConfigPath); err != nil { + log.Warnf("%s: failed to delete temporary file %s: %s", c, tempConfigPath, err) + } + }() + + if err := c.Sudo().FS().WriteFile(tempConfigPath, []byte(cfg), 0o600); err != nil { + return err + } + + // Prepare has no ctx; the Prepare interface takes only *v1beta1.Cluster. + // TODO: thread a real ctx if the Prepare interface ever takes a context. + return p.validateConfig(context.Background(), c, tempConfigPath) +} + func (p *ConfigureK0s) validateConfig(ctx context.Context, h *cluster.Host, configPath string) error { log.Infof("%s: validating configuration", h) @@ -335,19 +346,19 @@ func (p *ConfigureK0s) configureK0s(ctx context.Context, h *cluster.Host) error return nil } -func (p *ConfigureK0s) configFor(h *cluster.Host) (string, error) { +func (p *ConfigureK0s) configFor(h *cluster.Host, hostBaseConfig dig.Mapping) (string, error) { var cfg dig.Mapping if p.Config.Spec.K0s.DynamicConfig { if h == p.leader && h.Metadata.K0sRunningVersion == nil { log.Debugf("%s: leader will get a full config on initialize ", h) - cfg = p.newBaseConfig.Dup() + cfg = hostBaseConfig } else { log.Debugf("%s: using a stripped down config for dynamic config", h) cfg = p.Config.Spec.K0s.NodeConfig() } } else { - cfg = p.newBaseConfig.Dup() + cfg = hostBaseConfig } var addr string diff --git a/phase/configure_k0s_test.go b/phase/configure_k0s_test.go index fcff40af5..ead97c8e2 100644 --- a/phase/configure_k0s_test.go +++ b/phase/configure_k0s_test.go @@ -8,11 +8,21 @@ import ( "github.com/k0sproject/k0sctl/configurer/linux" "github.com/k0sproject/k0sctl/pkg/apis/k0sctl.k0sproject.io/v1beta1" "github.com/k0sproject/k0sctl/pkg/apis/k0sctl.k0sproject.io/v1beta1/cluster" + "github.com/k0sproject/rig/v2" + "github.com/k0sproject/rig/v2/protocol/ssh" "github.com/k0sproject/version" "github.com/stretchr/testify/require" "gopkg.in/yaml.v2" ) +func testController(address, privateAddress string) *cluster.Host { + return &cluster.Host{ + Role: "controller", + PrivateAddress: privateAddress, + CompositeConfig: rig.CompositeConfig{SSH: &ssh.Config{Address: address}}, + } +} + func TestBuildConfigValidateCommandAddsFeatureGates(t *testing.T) { cfg := &v1beta1.Cluster{ Spec: &cluster.Spec{ @@ -45,7 +55,7 @@ func TestConfigForSetsAPIAddressWhenIPv6NodeLocalLBEnabled(t *testing.T) { p := &ConfigureK0s{GenericPhase: GenericPhase{Config: clusterConfig}, newBaseConfig: base} h := &cluster.Host{PrivateAddress: "fc00::101"} - config, err := p.configFor(h) + config, err := p.configFor(h, base.Dup()) require.NoError(t, err) require.Equal(t, "fc00::101", apiAddressFromConfig(t, config)) } @@ -65,24 +75,101 @@ func TestConfigForLeavesAPIAddressWhenIPv6NodeLocalLBDisabled(t *testing.T) { p := &ConfigureK0s{GenericPhase: GenericPhase{Config: clusterConfig}, newBaseConfig: base} h := &cluster.Host{PrivateAddress: "fc00::102"} - config, err := p.configFor(h) + config, err := p.configFor(h, base.Dup()) require.NoError(t, err) require.Empty(t, apiAddressFromConfig(t, config)) } +func TestControllerSansAppendsOwnAddresses(t *testing.T) { + base := []string{"lb.example.com"} + + sans := hostsSans(base, testController("10.0.0.1", "172.16.0.1")) + require.Equal(t, []string{"lb.example.com", "10.0.0.1", "172.16.0.1"}, sans) + + sans = hostsSans(base, testController("10.0.0.2", "")) + require.Equal(t, []string{"lb.example.com", "10.0.0.2"}, sans) +} + +func TestControllerSansDoesNotMutateBase(t *testing.T) { + // build base via append so the slice has spare capacity, which would + // expose backing array sharing between the per-controller copies + var base []string + base = append(base, "lb.example.com") + + sansA := hostsSans(base, testController("10.0.0.1", "172.16.0.1")) + sansB := hostsSans(base, testController("10.0.0.2", "172.16.0.2")) + + require.Equal(t, []string{"lb.example.com"}, base) + require.Equal(t, []string{"lb.example.com", "10.0.0.1", "172.16.0.1"}, sansA) + require.Equal(t, []string{"lb.example.com", "10.0.0.2", "172.16.0.2"}, sansB) +} + +func TestControllerSansDedup(t *testing.T) { + base := []string{"10.0.0.1", "172.16.0.1"} + sans := hostsSans(base, testController("10.0.0.1", "172.16.0.1")) + require.Equal(t, base, sans) + + sans = hostsSans(nil, testController("10.0.0.1", "10.0.0.1")) + require.Equal(t, []string{"10.0.0.1"}, sans) +} + +func TestConfigForUsesPerHostSans(t *testing.T) { + newBaseConfig := dig.Mapping{ + "spec": dig.Mapping{ + "api": dig.Mapping{ + "sans": []string{"lb.example.com"}, + }, + }, + } + + clusterConfig := &v1beta1.Cluster{Spec: &cluster.Spec{K0s: &cluster.K0s{}}} + p := &ConfigureK0s{GenericPhase: GenericPhase{Config: clusterConfig}, newBaseConfig: newBaseConfig} + + hosts := []*cluster.Host{ + testController("10.0.0.1", "172.16.0.1"), + testController("10.0.0.2", "172.16.0.2"), + } + + base := []string{"lb.example.com"} + for i, h := range hosts { + hostBaseConfig := p.newBaseConfig.Dup() + hostBaseConfig.DigMapping("spec", "api")["sans"] = hostsSans(base, h) + + config, err := p.configFor(h, hostBaseConfig) + require.NoError(t, err) + + expected := []string{"lb.example.com", h.Address(), h.PrivateAddress} + require.Equal(t, expected, sansFromConfig(t, config), "host %d", i+1) + } + + // shared base config must remain untouched + require.Equal(t, []string{"lb.example.com"}, newBaseConfig.Dig("spec", "api", "sans")) +} + type apiSpec struct { Spec struct { API struct { - Address string `yaml:"address"` + Address string `yaml:"address"` + Sans []string `yaml:"sans"` } `yaml:"api"` } `yaml:"spec"` } -func apiAddressFromConfig(t *testing.T, cfg string) string { +func parseAPISpec(t *testing.T, cfg string) apiSpec { t.Helper() parts := strings.SplitN(cfg, "\n", 2) require.Len(t, parts, 2) var parsed apiSpec require.NoError(t, yaml.Unmarshal([]byte(parts[1]), &parsed)) - return parsed.Spec.API.Address + return parsed +} + +func apiAddressFromConfig(t *testing.T, cfg string) string { + t.Helper() + return parseAPISpec(t, cfg).Spec.API.Address +} + +func sansFromConfig(t *testing.T, cfg string) []string { + t.Helper() + return parseAPISpec(t, cfg).Spec.API.Sans }