From d5a82b37eb147a68ffd08fc8ec800edc92da9f9c Mon Sep 17 00:00:00 2001 From: Andrey Smirnov Date: Fri, 24 Dec 2021 14:40:19 +0300 Subject: [PATCH] feat: remove `ApplyDynamicConfig` This means that Talos no longer tries to change the machine configuration on the fly. Certificate SANs are already handled automatically by COSI controllers, so the only remaining part is the platform's hostname which was written back to machine configuration. In the follow-up PRs we'll provide better interface for platforms to provide network-related data. Fixes #4597 Signed-off-by: Andrey Smirnov --- .../server/v1alpha1/v1alpha1_server.go | 5 - .../v1alpha1/platform/openstack/openstack.go | 1 - .../v1alpha1/v1alpha1_sequencer_tasks.go | 9 -- pkg/machinery/config/provider.go | 2 - .../types/v1alpha1/v1alpha1_provider.go | 100 ------------------ .../types/v1alpha1/v1alpha1_provider_test.go | 90 ---------------- 6 files changed, 207 deletions(-) diff --git a/internal/app/machined/internal/server/v1alpha1/v1alpha1_server.go b/internal/app/machined/internal/server/v1alpha1/v1alpha1_server.go index 65f920427..b09b7a1d4 100644 --- a/internal/app/machined/internal/server/v1alpha1/v1alpha1_server.go +++ b/internal/app/machined/internal/server/v1alpha1/v1alpha1_server.go @@ -133,11 +133,6 @@ func (s *Server) ApplyConfiguration(ctx context.Context, in *machine.ApplyConfig return nil, err } - err = cfgProvider.ApplyDynamicConfig(ctx, s.Controller.Runtime().State().Platform()) - if err != nil { - return nil, err - } - // --immediate if in.Immediate { if err = s.Controller.Runtime().CanApplyImmediate(cfgProvider); err != nil { diff --git a/internal/app/machined/pkg/runtime/v1alpha1/platform/openstack/openstack.go b/internal/app/machined/pkg/runtime/v1alpha1/platform/openstack/openstack.go index ac55a9476..e567a1a86 100644 --- a/internal/app/machined/pkg/runtime/v1alpha1/platform/openstack/openstack.go +++ b/internal/app/machined/pkg/runtime/v1alpha1/platform/openstack/openstack.go @@ -292,7 +292,6 @@ func (o *Openstack) Hostname(ctx context.Context) (hostname []byte, err error) { download.WithErrorOnEmptyResponse(errors.ErrNoHostname)) if err != nil { // Platform cannot support this endpoint, or return timeout. - // ApplyDynamicConfig can crash in this situation. log.Printf("failed to fetch hostname, ignored: %s", err) return nil, nil diff --git a/internal/app/machined/pkg/runtime/v1alpha1/v1alpha1_sequencer_tasks.go b/internal/app/machined/pkg/runtime/v1alpha1/v1alpha1_sequencer_tasks.go index 87c34f554..6012451f6 100644 --- a/internal/app/machined/pkg/runtime/v1alpha1/v1alpha1_sequencer_tasks.go +++ b/internal/app/machined/pkg/runtime/v1alpha1/v1alpha1_sequencer_tasks.go @@ -471,15 +471,6 @@ func LoadConfig(seq runtime.Sequence, data interface{}) (runtime.TaskExecutionFu // SaveConfig represents the SaveConfig task. func SaveConfig(seq runtime.Sequence, data interface{}) (runtime.TaskExecutionFunc, string) { return func(ctx context.Context, logger *log.Logger, r runtime.Runtime) (err error) { - if err = r.Config().ApplyDynamicConfig(ctx, r.State().Platform()); err != nil { - return err - } - - err = r.State().V1Alpha2().SetConfig(r.Config()) - if err != nil { - return err - } - var b []byte b, err = r.Config().Bytes() diff --git a/pkg/machinery/config/provider.go b/pkg/machinery/config/provider.go index 909ac99c6..1d62bfeea 100644 --- a/pkg/machinery/config/provider.go +++ b/pkg/machinery/config/provider.go @@ -5,7 +5,6 @@ package config import ( - "context" "crypto/tls" "net" "net/url" @@ -28,7 +27,6 @@ type Provider interface { Cluster() ClusterConfig // Validate checks configuration and returns warnings and fatal errors (as multierror). Validate(RuntimeMode, ...ValidationOption) ([]string, error) - ApplyDynamicConfig(context.Context, DynamicConfigProvider) error String(encoderOptions ...encoder.Option) (string, error) Bytes(encoderOptions ...encoder.Option) ([]byte, error) } diff --git a/pkg/machinery/config/types/v1alpha1/v1alpha1_provider.go b/pkg/machinery/config/types/v1alpha1/v1alpha1_provider.go index c35af9b23..3ba579c43 100644 --- a/pkg/machinery/config/types/v1alpha1/v1alpha1_provider.go +++ b/pkg/machinery/config/types/v1alpha1/v1alpha1_provider.go @@ -5,12 +5,9 @@ package v1alpha1 import ( - "context" "crypto/tls" stdx509 "crypto/x509" "fmt" - "log" - "net" "os" "strings" "time" @@ -78,79 +75,6 @@ func (c *Config) Bytes(options ...encoder.Option) ([]byte, error) { return encoder.NewEncoder(c, options...).Encode() } -// ApplyDynamicConfig implements the config.Provider interface. -//nolint:gocyclo -func (c *Config) ApplyDynamicConfig(ctx context.Context, dynamicProvider config.DynamicConfigProvider) error { - ctx, cancel := context.WithTimeout(ctx, 30*time.Second) - defer cancel() - - if c.MachineConfig == nil { - c.MachineConfig = &MachineConfig{} - } - - hostname, err := dynamicProvider.Hostname(ctx) - if err != nil { - return err - } - - if hostname != nil { - if c.MachineConfig.MachineNetwork == nil { - c.MachineConfig.MachineNetwork = &NetworkConfig{} - } - - c.MachineConfig.MachineNetwork.NetworkHostname = string(hostname) - } - - addrs, err := dynamicProvider.ExternalIPs(ctx) - if err != nil { - // TODO: use passed logger instead of the global one - log.Printf("certificates will be created without external IPs: %v", err) - } - - if c.MachineConfig.MachineNetwork != nil { - addrs = append(addrs, addressesFromMachineNetworkConfig(c.MachineConfig.MachineNetwork)...) - } - - existingSANs := map[string]bool{} - for _, addr := range c.MachineConfig.MachineCertSANs { - existingSANs[addr] = true - } - - sans := make([]string, 0, len(addrs)) - for i, addr := range addrs { - sans = append(sans, addr.String()) - - if existingSANs[sans[i]] { - continue - } - - c.MachineConfig.MachineCertSANs = append(c.MachineConfig.MachineCertSANs, sans[i]) - } - - if c.ClusterConfig == nil { - c.ClusterConfig = &ClusterConfig{} - } - - if c.ClusterConfig.APIServerConfig == nil { - c.ClusterConfig.APIServerConfig = &APIServerConfig{} - } - - existingCertSANs := map[string]bool{} - for _, certSAN := range c.ClusterConfig.APIServerConfig.CertSANs { - existingCertSANs[certSAN] = true - } - - for _, certSAN := range sans { - if existingCertSANs[certSAN] { - continue - } - - c.ClusterConfig.APIServerConfig.CertSANs = append(c.ClusterConfig.APIServerConfig.CertSANs, certSAN) - } - - return nil -} - // Install implements the config.Provider interface. func (m *MachineConfig) Install() config.Install { if m.MachineInstall == nil { @@ -1278,27 +1202,3 @@ func (v VolumeMountConfig) ReadOnly() bool { func (u *UdevConfig) Rules() []string { return u.UdevRules } - -func addressesFromMachineNetworkConfig(nc *NetworkConfig) []net.IP { - var addresses []net.IP - - for _, networkConfig := range nc.NetworkInterfaces { - if networkConfig.VIPConfig() != nil { - sharedIP := net.ParseIP(networkConfig.VIPConfig().IP()) - if sharedIP != nil { - addresses = append(addresses, sharedIP) - } - - for _, vlan := range networkConfig.Vlans() { - if vlan.VIPConfig() != nil { - sharedIP := net.ParseIP(vlan.VIPConfig().IP()) - if sharedIP != nil { - addresses = append(addresses, sharedIP) - } - } - } - } - } - - return addresses -} diff --git a/pkg/machinery/config/types/v1alpha1/v1alpha1_provider_test.go b/pkg/machinery/config/types/v1alpha1/v1alpha1_provider_test.go index 3206eccfb..f6cb16d3a 100644 --- a/pkg/machinery/config/types/v1alpha1/v1alpha1_provider_test.go +++ b/pkg/machinery/config/types/v1alpha1/v1alpha1_provider_test.go @@ -5,104 +5,14 @@ package v1alpha1_test import ( - "context" - "net" "testing" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "github.com/talos-systems/talos/pkg/machinery/config" "github.com/talos-systems/talos/pkg/machinery/config/types/v1alpha1" - "github.com/talos-systems/talos/pkg/machinery/config/types/v1alpha1/bundle" - "github.com/talos-systems/talos/pkg/machinery/constants" ) -type dynamicConfigProvider struct { - externalIPs []net.IP -} - -// Hostname implements DynamicConfigProvider. -func (cp *dynamicConfigProvider) Hostname(ctx context.Context) ([]byte, error) { - return []byte("talos-default-master-1"), nil -} - -// ExternalIPs implements DynamicConfigProvider. -func (cp *dynamicConfigProvider) ExternalIPs(ctx context.Context) ([]net.IP, error) { - return cp.externalIPs, nil -} - -// TestApplyDynamicConfig check ApplyDynamicConfig works and is idempotent. -func TestApplyDynamicConfig(t *testing.T) { - b, err := bundle.NewConfigBundle( - bundle.WithInputOptions( - &bundle.InputOptions{ - ClusterName: "talos-default", - Endpoint: "10.5.0.1", - KubeVersion: constants.DefaultKubernetesVersion, - }, - ), - ) - require.NoError(t, err) - - config := b.ControlPlane() - - ctx := context.Background() - - provider := &dynamicConfigProvider{ - externalIPs: []net.IP{ - net.ParseIP("10.2.0.3"), - net.ParseIP("10.10.1.2"), - }, - } - - err = config.ApplyDynamicConfig(ctx, provider) - require.NoError(t, err) - - c, ok := config.(*v1alpha1.Config) - - require.True(t, ok) - - require.Equal(t, "talos-default-master-1", c.Machine().Network().Hostname()) - require.Equal(t, []string{"10.2.0.3", "10.10.1.2"}, c.MachineConfig.CertSANs()) - - provider.externalIPs = []net.IP{ - net.ParseIP("10.2.0.3"), - net.ParseIP("10.10.1.2"), - } - - provider = &dynamicConfigProvider{ - externalIPs: []net.IP{ - net.ParseIP("10.2.0.3"), - net.ParseIP("10.10.1.2"), - net.ParseIP("10.10.1.3"), - }, - } - - err = config.ApplyDynamicConfig(ctx, provider) - require.NoError(t, err) - require.Equal(t, []string{"10.2.0.3", "10.10.1.2", "10.10.1.3"}, c.MachineConfig.CertSANs()) - require.Equal(t, []string{"10.2.0.3", "10.10.1.2", "10.10.1.3"}, c.ClusterConfig.CertSANs()) - - c.MachineConfig.MachineNetwork.NetworkInterfaces = append(c.MachineConfig.MachineNetwork.NetworkInterfaces, &v1alpha1.Device{ - DeviceVIPConfig: &v1alpha1.DeviceVIPConfig{ - SharedIP: "192.168.88.77", - }, - DeviceVlans: []*v1alpha1.Vlan{ - { - VlanID: 100, - VlanVIP: &v1alpha1.DeviceVIPConfig{ - SharedIP: "192.168.88.66", - }, - }, - }, - }) - - err = config.ApplyDynamicConfig(ctx, provider) - require.NoError(t, err) - require.Equal(t, []string{"10.2.0.3", "10.10.1.2", "10.10.1.3", "192.168.88.77", "192.168.88.66"}, c.MachineConfig.CertSANs()) -} - func TestInterfaces(t *testing.T) { t.Parallel()