From 14e66ed5ad5cc3993040930c30cf50884bfdcc2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Fri, 27 Oct 2023 18:13:29 +0200 Subject: [PATCH] Remove config runtime setter: we only need it at compile-time --- windows-agent/internal/config/config.go | 67 +++++++++---------- .../internal/config/config_source.go | 28 -------- windows-agent/internal/config/config_test.go | 4 +- windows-agent/internal/proservices/ui/ui.go | 4 +- .../internal/proservices/ui/ui_test.go | 13 +++- 5 files changed, 44 insertions(+), 72 deletions(-) diff --git a/windows-agent/internal/config/config.go b/windows-agent/internal/config/config.go index 6eafff401..2ac5e056d 100644 --- a/windows-agent/internal/config/config.go +++ b/windows-agent/internal/config/config.go @@ -140,28 +140,6 @@ func (c *Config) ProvisioningTasks(ctx context.Context, distroName string) ([]ta return taskList, nil } -// SetSubscription overwrites the value of the pro token and the method with which it has been acquired. -func (c *Config) SetSubscription(ctx context.Context, proToken string, source Source) error { - c.mu.Lock() - defer c.mu.Unlock() - - // Load before dumping to avoid overriding recent changes to registry - if err := c.load(); err != nil { - return err - } - - old := c.subscription.Get(source) - c.subscription.Set(source, proToken) - - if err := c.dump(); err != nil { - log.Errorf(ctx, "Could not update subscription, token will be ignored: %v", err) - c.subscription.Set(source, old) - return err - } - - return nil -} - // LandscapeClientConfig returns the value of the landscape server URL and // the method it was acquired with (if any). func (c *Config) LandscapeClientConfig(ctx context.Context) (string, Source, error) { @@ -176,21 +154,23 @@ func (c *Config) LandscapeClientConfig(ctx context.Context) (string, Source, err return conf, src, nil } -// LandscapeAgentUID returns the UID assigned to this agent by the Landscape server. -// An empty string is returned if no UID has been assigned. -func (c *Config) LandscapeAgentUID(ctx context.Context) (string, error) { - c.mu.Lock() - defer c.mu.Unlock() - - if err := c.load(); err != nil { - return "", fmt.Errorf("could not load: %v", err) - } +// SetSubscription overwrites the value of the user-provided Ubuntu Pro token. +func (c *Config) SetUserSubscription(ctx context.Context, proToken string) error { + return c.set(ctx, &c.subscription.User, proToken) +} - return c.landscape.UID, nil +// setStoreSubscription overwrites the value of the store-provided Ubuntu Pro token. +func (c *Config) setStoreSubscription(ctx context.Context, proToken string) error { + return c.set(ctx, &c.subscription.Store, proToken) } // SetLandscapeAgentUID overrides the Landscape agent UID. func (c *Config) SetLandscapeAgentUID(ctx context.Context, uid string) error { + return c.set(ctx, &c.landscape.UID, uid) +} + +// set is a generic method to safely modify the config. +func (c *Config) set(ctx context.Context, field *string, value string) error { c.mu.Lock() defer c.mu.Unlock() @@ -199,18 +179,31 @@ func (c *Config) SetLandscapeAgentUID(ctx context.Context, uid string) error { return err } - old := c.landscape.UID - c.landscape.UID = uid + old := *field + *field = value if err := c.dump(); err != nil { - log.Errorf(ctx, "Could not update landscape settings, UID will be ignored: %v", err) - c.landscape.UID = old + log.Errorf(ctx, "Could not update settings: %v", err) + *field = old return err } return nil } +// LandscapeAgentUID returns the UID assigned to this agent by the Landscape server. +// An empty string is returned if no UID has been assigned. +func (c *Config) LandscapeAgentUID(ctx context.Context) (string, error) { + c.mu.Lock() + defer c.mu.Unlock() + + if err := c.load(); err != nil { + return "", fmt.Errorf("could not load: %v", err) + } + + return c.landscape.UID, nil +} + // FetchMicrosoftStoreSubscription contacts Ubuntu Pro's contract server and the Microsoft Store // to check if the user has an active subscription that provides a pro token. If so, that token is used. func (c *Config) FetchMicrosoftStoreSubscription(ctx context.Context) (err error) { @@ -231,7 +224,7 @@ func (c *Config) FetchMicrosoftStoreSubscription(ctx context.Context) (err error return fmt.Errorf("could not get ProToken from Microsoft Store: %v", err) } - if err := c.SetSubscription(ctx, proToken, SourceMicrosoftStore); err != nil { + if err := c.setStoreSubscription(ctx, proToken); err != nil { return err } diff --git a/windows-agent/internal/config/config_source.go b/windows-agent/internal/config/config_source.go index 260648568..17225474f 100644 --- a/windows-agent/internal/config/config_source.go +++ b/windows-agent/internal/config/config_source.go @@ -1,7 +1,5 @@ package config -import "fmt" - // Source indicates the method a configuration parameter was acquired. type Source int @@ -43,32 +41,6 @@ func (s subscription) resolve() (string, Source) { return "", SourceNone } -func (s *subscription) Set(src Source, proToken string) { - ptr := s.src(src) - *ptr = proToken -} - -func (s subscription) Get(src Source) string { - return *s.src(src) -} - -// src is a helper to avoid duplicating the mapping in Get and Set. -func (s *subscription) src(src Source) *string { - switch src { - case SourceNone: - // TODO: Panic? Warning? - return new(string) - case SourceUser: - return &s.User - case SourceRegistry: - return &s.Organization - case SourceMicrosoftStore: - return &s.Store - } - - panic(fmt.Sprintf("Unknown enum value for SubscriptionSource: %d", src)) -} - type landscapeConf struct { UserConfig string `yaml:"config"` OrgConfig string `yaml:"-"` diff --git a/windows-agent/internal/config/config_test.go b/windows-agent/internal/config/config_test.go index 8a5e92471..b093693b3 100644 --- a/windows-agent/internal/config/config_test.go +++ b/windows-agent/internal/config/config_test.go @@ -263,7 +263,7 @@ func TestProvisioningTasks(t *testing.T) { } } -func TestSetSubscription(t *testing.T) { +func TestSetUserSubscription(t *testing.T) { t.Parallel() testCases := map[string]struct { @@ -297,7 +297,7 @@ func TestSetSubscription(t *testing.T) { token = "" } - err := conf.SetSubscription(ctx, token, config.SourceUser) + err := conf.SetUserSubscription(ctx, token) if tc.wantError { require.Error(t, err, "SetSubscription should return an error") return diff --git a/windows-agent/internal/proservices/ui/ui.go b/windows-agent/internal/proservices/ui/ui.go index e300ac2a6..7a032f7a1 100644 --- a/windows-agent/internal/proservices/ui/ui.go +++ b/windows-agent/internal/proservices/ui/ui.go @@ -16,7 +16,7 @@ import ( // Config is a provider for the subcription configuration. type Config interface { - SetSubscription(ctx context.Context, token string, source config.Source) error + SetUserSubscription(ctx context.Context, token string) error IsReadOnly() (bool, error) Subscription(context.Context) (string, config.Source, error) FetchMicrosoftStoreSubscription(context.Context) error @@ -45,7 +45,7 @@ func (s *Service) ApplyProToken(ctx context.Context, info *agentapi.ProAttachInf token := info.GetToken() log.Debugf(ctx, "Received token %s", common.Obfuscate(token)) - err := s.config.SetSubscription(ctx, token, config.SourceUser) + err := s.config.SetUserSubscription(ctx, token) if err != nil { return nil, err } diff --git a/windows-agent/internal/proservices/ui/ui_test.go b/windows-agent/internal/proservices/ui/ui_test.go index 66a7109af..7f591006d 100644 --- a/windows-agent/internal/proservices/ui/ui_test.go +++ b/windows-agent/internal/proservices/ui/ui_test.go @@ -224,12 +224,12 @@ type mockConfig struct { source config.Source // stores the configured subscription source. } -func (m *mockConfig) SetSubscription(ctx context.Context, token string, source config.Source) error { +func (m *mockConfig) SetUserSubscription(ctx context.Context, token string) error { if m.setSubscriptionErr { return errors.New("SetSubscription error") } m.token = token - m.source = source + m.source = config.SourceUser return nil } func (m mockConfig) IsReadOnly() (bool, error) { @@ -259,5 +259,12 @@ func (m *mockConfig) FetchMicrosoftStoreSubscription(ctx context.Context) error return errors.New("Already subscribed") } - return m.SetSubscription(ctx, "MS", config.SourceMicrosoftStore) + if m.setSubscriptionErr { + return errors.New("SetSubscription error") + } + + m.token = "MS" + m.source = config.SourceMicrosoftStore + + return nil }