diff --git a/windows-agent/internal/config/config.go b/windows-agent/internal/config/config.go index e76a72d7c..61eab7e45 100644 --- a/windows-agent/internal/config/config.go +++ b/windows-agent/internal/config/config.go @@ -4,16 +4,22 @@ package config import ( "context" + "crypto/sha512" "errors" "fmt" + "io/fs" + "os" + "path/filepath" "sync" "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/config/registry" "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/contracts" + "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/distros/database" "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/distros/task" log "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/grpc/logstreamer" "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/tasks" "github.com/ubuntu/decorate" + "golang.org/x/exp/slices" ) const ( @@ -415,3 +421,155 @@ func (c *Config) FetchMicrosoftStoreSubscription(ctx context.Context) (err error return nil } + +// UpdateRegistrySettings checks if any of the registry settings have changed since this function was last called. +// If so, new settings are pushed to the distros. +func (c *Config) UpdateRegistrySettings(ctx context.Context, cacheDir string, db *database.DistroDB) error { + type getTask = func(*Config, context.Context, string, *database.DistroDB) (task.Task, error) + + // Collect tasks for updated settings + var errs error + var taskList []task.Task + for _, f := range []getTask{(*Config).getTaskOnNewSubscription, (*Config).getTaskOnNewLandscape} { + task, err := f(c, ctx, cacheDir, db) + if err != nil { + errs = errors.Join(errs, err) + continue + } + if task != nil { + taskList = append(taskList, task) + } + } + + if errs != nil { + log.Warningf(ctx, "Could not obtain some updated registry settings: %v", errs) + } + + // Apply tasks for updated settings + errs = nil + for _, d := range db.GetAll() { + errs = errors.Join(errs, d.SubmitDeferredTasks(taskList...)) + } + + if errs != nil { + return fmt.Errorf("could not submit new task to certain distros: %v", errs) + } + + return nil +} + +// getTaskOnNewSubscription checks if the subscription has changed since the last time it was called. If so, the new subscription +// is returned in the form of a task. +func (c *Config) getTaskOnNewSubscription(ctx context.Context, cacheDir string, db *database.DistroDB) (task.Task, error) { + proToken, _, err := c.Subscription(ctx) + if err != nil { + return nil, fmt.Errorf("could not retrieve current subscription: %v", err) + } + + isNew, err := hasChanged(filepath.Join(cacheDir, "subscription.csum"), []byte(proToken)) + if err != nil { + log.Warningf(ctx, "could not update checksum for Ubuntu Pro subscription: %v", err) + } + + if !isNew { + return nil, nil + } + + log.Debug(ctx, "New Ubuntu Pro subscription settings detected in registry") + return tasks.ProAttachment{Token: proToken}, nil +} + +// getTaskOnNewLandscape checks if the Landscape settings has changed since the last time it was called. If so, the +// new Landscape settings are returned in the form of a task. +func (c *Config) getTaskOnNewLandscape(ctx context.Context, cacheDir string, db *database.DistroDB) (task.Task, error) { + landscapeConf, err := c.LandscapeClientConfig(ctx) + if err != nil { + return nil, fmt.Errorf("could not retrieve current landscape config: %v", err) + } + + landscapeUID, err := c.LandscapeAgentUID(ctx) + if err != nil { + return nil, fmt.Errorf("could not retrieve current landscape UID: %v", err) + } + + // We append them just so we can compute a combined checksum + serialized := fmt.Sprintf("%s%s", landscapeUID, landscapeConf) + + isNew, err := hasChanged(filepath.Join(cacheDir, "landscape.csum"), []byte(serialized)) + if err != nil { + log.Warningf(ctx, "could not update checksum for Landscape configuration: %v", err) + } + + if !isNew { + return nil, nil + } + + log.Debug(ctx, "New Landscape settings detected in registry") + + // We must not register to landscape if we have no Landscape UID + if landscapeConf != "" && landscapeUID == "" { + log.Debug(ctx, "Ignoring new landscape settings: no Landscape agent UID") + return nil, nil + } + + return tasks.LandscapeConfigure{Config: landscapeConf, HostagentUID: landscapeUID}, nil +} + +// hasChanged detects if the current value is different from the last time it was used. +// The return value is usable even if error is returned. +func hasChanged(cachePath string, newValue []byte) (new bool, err error) { + var newCheckSum []byte + if len(newValue) != 0 { + tmp := sha512.Sum512(newValue) + newCheckSum = tmp[:] + } + + defer decorateUpdateCache(&new, &err, cachePath, newCheckSum) + + oldChecksum, err := os.ReadFile(cachePath) + if errors.Is(err, fs.ErrNotExist) { + // File not found: there was no value before + oldChecksum = nil + } else if err != nil { + return true, fmt.Errorf("could not read old value: %v", err) + } + + if slices.Equal(oldChecksum, newCheckSum) { + return false, nil + } + + return true, nil +} + +// decorateUpdateCache acts depending on caller's return values (hence decorate). +// It stores the new checksum to the cachefile. Any errors are joined to *err. +func decorateUpdateCache(new *bool, err *error, cachePath string, newCheckSum []byte) { + writeCacheErr := func() error { + // If the value is empty, we remove the file. + // This preserves this function's idempotency. + if len(newCheckSum) == 0 { + err := os.Remove(cachePath) + if errors.Is(err, fs.ErrNotExist) { + return nil + } + if err != nil { + return fmt.Errorf("could not remove old checksum: %v", err) + } + return nil + } + + // Value is unchanged: don't write to file + if !*new { + return nil + } + + // Update to file + if err := os.WriteFile(cachePath, newCheckSum[:], 0600); err != nil { + return fmt.Errorf("could not write checksum to cache: %v", err) + } + + return nil + }() + + *err = errors.Join(*err, writeCacheErr) +} diff --git a/windows-agent/internal/config/config_test.go b/windows-agent/internal/config/config_test.go index be42e4775..7298d13cc 100644 --- a/windows-agent/internal/config/config_test.go +++ b/windows-agent/internal/config/config_test.go @@ -2,14 +2,23 @@ package config_test import ( "context" + "errors" + "io/fs" + "os" + "path/filepath" "testing" + "github.com/canonical/ubuntu-pro-for-windows/common/wsltestutils" config "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/config" "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/config/registry" + "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/distros/database" + "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/distros/distro" "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/distros/task" "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/tasks" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + wsl "github.com/ubuntu/gowsl" + wslmock "github.com/ubuntu/gowsl/mock" ) // registryState represents how much data is in the registry. @@ -432,6 +441,89 @@ func TestFetchMicrosoftStoreSubscription(t *testing.T) { } } +func TestUpdateRegistrySettings(t *testing.T) { + if wsl.MockAvailable() { + t.Parallel() + } + + testCases := map[string]struct { + valueToChange string + registryState registryState + breakTaskfile bool + + wantTasks []string + unwantedTasks []string + wantErr bool + }{ + "Success changing Pro token": {valueToChange: "ProTokenOrg", registryState: keyExists, wantTasks: []string{"tasks.ProAttachment"}, unwantedTasks: []string{"tasks.LandscapeConfigure"}}, + "Success changing Landscape without a UID": {valueToChange: "LandscapeClientConfig", registryState: keyExists, unwantedTasks: []string{"tasks.ProAttachment", "tasks.LandscapeConfigure"}}, + "Success changing Landscape with a UID": {valueToChange: "LandscapeClientConfig", registryState: landscapeAgentUIDHasValue, wantTasks: []string{"tasks.LandscapeConfigure"}, unwantedTasks: []string{"tasks.ProAttachment"}}, + "Success changing the Landscape UID": {valueToChange: "LandscapeAgentUID", registryState: keyExists, wantTasks: []string{"tasks.LandscapeConfigure"}, unwantedTasks: []string{"tasks.ProAttachment"}}, + + // Very implementation-detailed, but it's the only thing that actually triggers an error + "Error when the tasks cannot be submitted": {valueToChange: "ProTokenOrg", registryState: keyExists, breakTaskfile: true, wantErr: true}, + } + + for name, tc := range testCases { + tc := tc + t.Run(name, func(t *testing.T) { + ctx := context.Background() + if wsl.MockAvailable() { + t.Parallel() + ctx = wsl.WithMock(ctx, wslmock.New()) + } + + dir := t.TempDir() + + distroName, _ := wsltestutils.RegisterDistro(t, ctx, false) + taskFilePath := filepath.Join(dir, distroName+".tasks") + + db, err := database.New(ctx, dir, nil) + require.NoError(t, err, "Setup: could create empty database") + + _, err = db.GetDistroAndUpdateProperties(ctx, distroName, distro.Properties{}) + require.NoError(t, err, "Setup: could not add dummy distro to database") + + r := setUpMockRegistry(0, tc.registryState, false) + require.NoError(t, err, "Setup: could not create empty database") + + c := config.New(ctx, config.WithRegistry(r)) + + // Update value in registry + r.UbuntuProData[tc.valueToChange] = "NEW_VALUE!" + + if tc.breakTaskfile { + err := os.MkdirAll(taskFilePath, 0600) + require.NoError(t, err, "could not create directory to interfere with task file") + } + + err = c.UpdateRegistrySettings(ctx, dir, db) + if tc.wantErr { + require.Error(t, err, "UpdateRegistrySettings should return an error") + return + } + require.NoError(t, err, "UpdateRegistrySettings should return no error") + + out, err := readFileOrEmpty(taskFilePath) + require.NoError(t, err, "Could not read distro taskfile") + for _, task := range tc.wantTasks { + assert.Containsf(t, out, task, "Distro should have received a %s task", task) + } + for _, task := range tc.unwantedTasks { + assert.NotContainsf(t, out, task, "Distro should have received a %s task", task) + } + }) + } +} + +func readFileOrEmpty(path string) (string, error) { + out, err := os.ReadFile(path) + if errors.Is(err, fs.ErrNotExist) { + return "", nil + } + return string(out), err +} + // is defines equality between flags. It is convenience function to check if a registryState matches a certain state. func (state registryState) is(flag registryState) bool { return state&flag == flag diff --git a/windows-agent/internal/proservices/proservices.go b/windows-agent/internal/proservices/proservices.go index c9b2ec12b..ec79c3ec6 100644 --- a/windows-agent/internal/proservices/proservices.go +++ b/windows-agent/internal/proservices/proservices.go @@ -3,10 +3,7 @@ package proservices import ( "context" - "crypto/sha512" "errors" - "fmt" - "io/fs" "os" "path/filepath" @@ -19,8 +16,6 @@ import ( "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/proservices/landscape" "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/proservices/ui" "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/proservices/wslinstance" - "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/tasks" - "golang.org/x/exp/slices" "google.golang.org/grpc" ) @@ -95,18 +90,11 @@ func New(ctx context.Context, args ...Option) (s Manager, err error) { if err != nil { return s, err } - defer func() { - if err != nil { - db.Close(ctx) - } - }() + defer db.Close(ctx) - go func() { - err := updateSubscriptions(ctx, opts.cacheDir, conf, db) - if err != nil { - log.Warningf(ctx, "Could not update subscriptions: %v", err) - } - }() + if err := conf.UpdateRegistrySettings(ctx, opts.cacheDir, db); err != nil { + log.Warningf(ctx, "Could not update registry settings: %v", err) + } uiService := ui.New(ctx, conf, db) @@ -153,72 +141,3 @@ func (m Manager) RegisterGRPCServices(ctx context.Context) *grpc.Server { return grpcServer } - -// updateSubscriptions checks if the subscription has changed since the last time it was called. If so, the new subscription -// is pushed to all distros as a deferred task. -func updateSubscriptions(ctx context.Context, cacheDir string, conf *config.Config, db *database.DistroDB) error { - proToken, _, err := conf.Subscription(ctx) - if err != nil { - return fmt.Errorf("could not retrieve current subscription: %v", err) - } - - if !subscriptionIsNew(ctx, cacheDir, proToken) { - return nil - } - - task := tasks.ProAttachment{Token: proToken} - - for _, d := range db.GetAll() { - err = errors.Join(err, d.SubmitDeferredTasks(task)) - } - - if err != nil { - return fmt.Errorf("could not submit new token to certain distros: %v", err) - } - - return nil -} - -// subscriptionIsNew detects if the current subscription is different from the last time it was called. -func subscriptionIsNew(ctx context.Context, cacheDir string, newSubscription string) (new bool) { - cachePath := filepath.Join(cacheDir, "subscription.csum") - newCheckSum := sha512.Sum512([]byte(newSubscription)) - - // Update cache on exit - defer func() { - if newSubscription == "" { - // If there is no subscription, we remove the file. - // This preserves this function's idempotency. - err := os.Remove(cachePath) - if err != nil && !errors.Is(err, fs.ErrNotExist) { - log.Warningf(ctx, "Could not write new subscription to cache: %v", err) - } - return - } - - if !new { - return - } - - err := os.WriteFile(cachePath, newCheckSum[:], 0600) - if err != nil { - log.Warningf(ctx, "Could not write new subscription to cache: %v", err) - } - }() - - oldChecksum, err := os.ReadFile(cachePath) - if errors.Is(err, fs.ErrNotExist) { - // File not found: there was no subscription before - // (Lack of) subscription is new only if the new subscription non-empty. - return newSubscription != "" - } else if err != nil { - log.Warningf(ctx, "Could not read old subscription, assuming subscription is new. Error: %v", err) - return true - } - - if slices.Equal(oldChecksum, newCheckSum[:]) { - return false - } - - return true -} diff --git a/windows-agent/internal/proservices/proservices_test.go b/windows-agent/internal/proservices/proservices_test.go index a0c0b52c2..235c195a4 100644 --- a/windows-agent/internal/proservices/proservices_test.go +++ b/windows-agent/internal/proservices/proservices_test.go @@ -2,12 +2,9 @@ package proservices_test import ( "context" - "crypto/sha512" - "io/fs" "os" "path/filepath" "testing" - "time" "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/config/registry" "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/consts" @@ -26,13 +23,6 @@ func TestMain(m *testing.M) { func TestNew(t *testing.T) { t.Parallel() - type subscription = string - const ( - tokenA = "Token123" - tokenB = "TokenXYZ" - tokenNone = "" - ) - testCases := map[string]struct { configIsReadOnly bool @@ -40,24 +30,12 @@ func TestNew(t *testing.T) { breakMkDir bool breakNewDistroDB bool - newSubscription subscription - oldSubscription subscription - checksumIsEmpty bool - wantErr bool }{ "Success when the subscription stays empty": {}, "Success with a read-only registry": {configIsReadOnly: true}, "Success when the config cannot check if it is read-only": {breakConfig: true}, - "Success when the subscription stays the same": {oldSubscription: tokenA, newSubscription: tokenA}, - "Success when the subscription changes tokens": {oldSubscription: tokenA, newSubscription: tokenB}, - "Success when the subscription changes from empty": {oldSubscription: tokenNone, newSubscription: tokenA}, - "Success when the subscription changes to empty": {oldSubscription: tokenA, newSubscription: tokenNone}, - - "Success when the subscription was an empty file (subscribed)": {checksumIsEmpty: true, newSubscription: tokenA}, - "Success when the subscription was an empty file (non-subscribed)": {checksumIsEmpty: true, newSubscription: tokenNone}, - "Error when Manager cannot create its cache dir": {breakMkDir: true, wantErr: true}, "Error when database cannot create its dump file": {breakNewDistroDB: true, wantErr: true}, } @@ -69,22 +47,8 @@ func TestNew(t *testing.T) { ctx := context.Background() dir := t.TempDir() - subscriptionChecksumFile := filepath.Join(dir, "subscription.csum") - if tc.checksumIsEmpty { - err := os.WriteFile(subscriptionChecksumFile, []byte{}, 0600) - require.NoError(t, err, "Setup: could not write empty checksum file") - } else if tc.oldSubscription != "" { - oldChecksum := sha512.Sum512([]byte(tc.oldSubscription)) - err := os.WriteFile(subscriptionChecksumFile, oldChecksum[:], 0600) - require.NoError(t, err, "Setup: could not write subscription checksum file") - } - reg := registry.NewMock() reg.KeyExists = true - reg.UbuntuProData["ProTokenUser"] = tc.newSubscription - if tc.breakConfig { - reg.Errors = registry.MockErrOnCreateKey - } if tc.configIsReadOnly { reg.KeyExists = true @@ -113,19 +77,6 @@ func TestNew(t *testing.T) { return } require.NoError(t, err, "New should return no error") - - // Subscriptions are updated asyncronously - time.Sleep(time.Second) - - out, err := os.ReadFile(subscriptionChecksumFile) - if tc.newSubscription == "" { - require.ErrorIs(t, err, fs.ErrNotExist, "The subscription file should have been removed.") - return - } - require.NoError(t, err, "Could not read subscription checksum file: %v", err) - - newChecksum := sha512.Sum512([]byte(tc.newSubscription)) - require.Equal(t, out, newChecksum[:], "Checksum does no match the new subscription's") }) } } diff --git a/wsl-pro-service/go.sum b/wsl-pro-service/go.sum index 89ddee454..95bc88253 100644 --- a/wsl-pro-service/go.sum +++ b/wsl-pro-service/go.sum @@ -42,8 +42,6 @@ github.com/canonical/ubuntu-pro-for-windows/agentapi v0.0.0-20231023183501-458a1 github.com/canonical/ubuntu-pro-for-windows/agentapi v0.0.0-20231023183501-458a10db4c16/go.mod h1:yZFCC1gVaFNjIJ7Glk3FBiaCokcV/Jk5B6tFIlHBgd8= github.com/canonical/ubuntu-pro-for-windows/common v0.0.0-20231023183501-458a10db4c16 h1:u7afWNLgssNSEHKxldRbgCobFnGlILpECBeAPiWyb0c= github.com/canonical/ubuntu-pro-for-windows/common v0.0.0-20231023183501-458a10db4c16/go.mod h1:a/RSoEHIs9RmXxL/+68oDpXi8+d+TGptM/9N5ULw2Kw= -github.com/canonical/ubuntu-pro-for-windows/wslserviceapi v0.0.0-20231023183501-458a10db4c16 h1:B/iSer/eNknIEp9TnNYC56zdc6SLY6K7fXxkeFl15s8= -github.com/canonical/ubuntu-pro-for-windows/wslserviceapi v0.0.0-20231023183501-458a10db4c16/go.mod h1:gjEBJv2vc1VUqqrv1ZTDcNmd3TBBCPIlmBaR63YSX9Y= github.com/canonical/ubuntu-pro-for-windows/wslserviceapi v0.0.0-20231025153609-b3e7db95a9ce h1:zuJOIrd+T+J5UVEiCHT9lK7sYS4VDJ5ZxI81Yvdr6jY= github.com/canonical/ubuntu-pro-for-windows/wslserviceapi v0.0.0-20231025153609-b3e7db95a9ce/go.mod h1:gjEBJv2vc1VUqqrv1ZTDcNmd3TBBCPIlmBaR63YSX9Y= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=