From b033eb1846e4fc341cfad3e46729a0506acc8341 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Tue, 24 Oct 2023 09:33:15 +0200 Subject: [PATCH 01/18] Config now holds the Landscape host-agent UID --- windows-agent/internal/config/config.go | 49 +++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/windows-agent/internal/config/config.go b/windows-agent/internal/config/config.go index 3c36e451f..2e53b0e5d 100644 --- a/windows-agent/internal/config/config.go +++ b/windows-agent/internal/config/config.go @@ -26,6 +26,9 @@ const ( fieldLandscapeAgentURL = "LandscapeAgentURL" defaultLandscapeAgentURL = "" + + fieldLandscapeAgentUID = "LandscapeAgentUID" + defaultLandscapeAgentUID = "" ) // fieldsProToken contains the fields in the registry where each source will store its token. @@ -60,6 +63,7 @@ type Config struct { type configData struct { landscapeClientConfig string landscapeAgentURL string + landscapeAgentUID string } // SubscriptionSource indicates the method the subscription was acquired. @@ -227,6 +231,41 @@ func (c *Config) LandscapeClientConfig(ctx context.Context) (string, error) { return c.data.landscapeClientConfig, 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(ctx); err != nil { + return "", fmt.Errorf("could not load: %v", err) + } + + return c.data.landscapeAgentUID, nil +} + +// SetLandscapeAgentUID overrides the Landscape agent UID. +func (c *Config) SetLandscapeAgentUID(ctx context.Context, uid string) error { + c.mu.Lock() + defer c.mu.Unlock() + + // Load before dumping to avoid overriding recent changes to registry + if err := c.load(ctx); err != nil { + return err + } + + old := c.data.landscapeAgentUID + c.data.landscapeAgentUID = uid + + if err := c.dump(); err != nil { + log.Errorf(ctx, "Could not update landscape agent UID in registry, UID will be ignored: %v", err) + c.data.landscapeAgentUID = old + return err + } + + return nil +} + func (c *Config) load(ctx context.Context) (err error) { defer decorate.OnError(&err, "could not load data for Config") @@ -253,6 +292,7 @@ func (c *Config) loadRegistry(ctx context.Context) (proTokens map[SubscriptionSo log.Debug(ctx, "Registry key does not exist, using default values") data.landscapeAgentURL = defaultLandscapeAgentURL data.landscapeClientConfig = defaultLandscapeClientConfig + data.landscapeAgentUID = defaultLandscapeAgentUID return proTokens, data, nil } if err != nil { @@ -288,6 +328,11 @@ func (c *Config) loadRegistry(ctx context.Context) (proTokens map[SubscriptionSo return proTokens, data, err } + data.landscapeAgentUID, err = c.readValue(ctx, k, fieldLandscapeAgentUID, defaultLandscapeAgentUID) + if err != nil { + return proTokens, data, err + } + return proTokens, data, nil } @@ -328,6 +373,10 @@ func (c *Config) dump() (err error) { return fmt.Errorf("could not write into registry key: %v", err) } + if err := c.registry.WriteValue(k, fieldLandscapeAgentUID, c.data.landscapeAgentUID); err != nil { + return fmt.Errorf("could not write into registry key: %v", err) + } + return nil } From 7ca542e17b4723ad029658109601934080fd925c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Tue, 24 Oct 2023 09:33:15 +0200 Subject: [PATCH 02/18] Config now holds the Landscape host-agent UID --- windows-agent/internal/config/config.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/windows-agent/internal/config/config.go b/windows-agent/internal/config/config.go index 2e53b0e5d..5681371cf 100644 --- a/windows-agent/internal/config/config.go +++ b/windows-agent/internal/config/config.go @@ -377,6 +377,10 @@ func (c *Config) dump() (err error) { return fmt.Errorf("could not write into registry key: %v", err) } + if err := c.registry.WriteValue(k, fieldLandscapeAgentUID, c.data.landscapeAgentUID); err != nil { + return fmt.Errorf("could not write into registry key: %v", err) + } + return nil } From b5d5bfecd57bd507cb1a0a08260d6710671ec6ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Tue, 24 Oct 2023 09:35:02 +0200 Subject: [PATCH 03/18] Refactor config.ProvisioningTasks It was reading the registry twice, with two mutex lock-unlocks. If we now add the need to read the hostagent UID, that would be a third time. So we simplify it to more basic operations. --- windows-agent/internal/config/config.go | 37 +++++++++++++++---------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/windows-agent/internal/config/config.go b/windows-agent/internal/config/config.go index 5681371cf..e34ed4ae7 100644 --- a/windows-agent/internal/config/config.go +++ b/windows-agent/internal/config/config.go @@ -134,6 +134,12 @@ func (c *Config) Subscription(ctx context.Context) (token string, source Subscri return "", SubscriptionNone, fmt.Errorf("could not load: %v", err) } + token, source = c.subscription() + return token, source, nil +} + +// subscription is the unsafe version of Subscription. It returns the ProToken and the method it was acquired with (if any). +func (c *Config) subscription() (token string, source SubscriptionSource) { for src := subscriptionMaxPriority - 1; src > SubscriptionNone; src-- { token, ok := c.proTokens[src] if !ok { @@ -143,10 +149,10 @@ func (c *Config) Subscription(ctx context.Context) (token string, source Subscri continue } - return token, src, nil + return token, src } - return "", SubscriptionNone, nil + return "", SubscriptionNone } // IsReadOnly returns whether the registry can be written to. @@ -166,22 +172,25 @@ func (c *Config) IsReadOnly() (b bool, err error) { // ProvisioningTasks returns a slice of all tasks to be submitted upon first contact with a distro. func (c *Config) ProvisioningTasks(ctx context.Context, distroName string) ([]task.Task, error) { - token, _, err := c.Subscription(ctx) - if err != nil { - return nil, err - } + var taskList []task.Task - taskList := []task.Task{ - tasks.ProAttachment{Token: token}, - } + // Refresh data from registry + c.mu.Lock() + defer c.mu.Unlock() - if conf, err := c.LandscapeClientConfig(ctx); err != nil { - log.Errorf(ctx, "Could not generate provisioning task LandscapeConfigure: %v", err) - } else { - landscape := tasks.LandscapeConfigure{Config: conf} - taskList = append(taskList, landscape) + if err := c.load(ctx); err != nil { + return nil, fmt.Errorf("could not load: %v", err) } + // Ubuntu Pro attachment + proToken, _ := c.subscription() + taskList = append(taskList, tasks.ProAttachment{Token: proToken}) + + // Landcape registration + taskList = append(taskList, tasks.LandscapeConfigure{ + Config: c.data.landscapeClientConfig, + }) + return taskList, nil } From f2776d81ea95590f0dd5277140ba0239511c3c04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Tue, 24 Oct 2023 09:37:59 +0200 Subject: [PATCH 04/18] WSL Service API now supports Landscape hostagent UID --- wslserviceapi/wslserviceapi.proto | 1 + 1 file changed, 1 insertion(+) diff --git a/wslserviceapi/wslserviceapi.proto b/wslserviceapi/wslserviceapi.proto index 4b854ceaa..a36687800 100644 --- a/wslserviceapi/wslserviceapi.proto +++ b/wslserviceapi/wslserviceapi.proto @@ -18,6 +18,7 @@ message ProAttachInfo { message LandscapeConfig { // Empty configuration is interpreted as "landscape-config --disable" string configuration = 1; + string hostagentUID = 2; } message Empty {} From 8fb116a6df90a40cc8ad235efa82351867a2fa57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Tue, 24 Oct 2023 09:38:32 +0200 Subject: [PATCH 05/18] Update gRPC generated code --- wslserviceapi/wslserviceapi.pb.go | 54 +++++++++++++++----------- wslserviceapi/wslserviceapi_grpc.pb.go | 2 +- 2 files changed, 33 insertions(+), 23 deletions(-) diff --git a/wslserviceapi/wslserviceapi.pb.go b/wslserviceapi/wslserviceapi.pb.go index a1bd7401e..f8852823a 100644 --- a/wslserviceapi/wslserviceapi.pb.go +++ b/wslserviceapi/wslserviceapi.pb.go @@ -1,7 +1,7 @@ // Code generated by protoc-gen-go. DO NOT EDIT. // versions: -// protoc-gen-go v1.31.0 -// protoc v3.21.12 +// protoc-gen-go v1.28.1 +// protoc v4.24.3 // source: wslserviceapi.proto package wslserviceapi @@ -75,6 +75,7 @@ type LandscapeConfig struct { // Empty configuration is interpreted as "landscape-config --disable" Configuration string `protobuf:"bytes,1,opt,name=configuration,proto3" json:"configuration,omitempty"` + HostagentUID string `protobuf:"bytes,2,opt,name=hostagentUID,proto3" json:"hostagentUID,omitempty"` } func (x *LandscapeConfig) Reset() { @@ -116,6 +117,13 @@ func (x *LandscapeConfig) GetConfiguration() string { return "" } +func (x *LandscapeConfig) GetHostagentUID() string { + if x != nil { + return x.HostagentUID + } + return "" +} + type Empty struct { state protoimpl.MessageState sizeCache protoimpl.SizeCache @@ -161,29 +169,31 @@ var file_wslserviceapi_proto_rawDesc = []byte{ 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x12, 0x0d, 0x77, 0x73, 0x6c, 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x61, 0x70, 0x69, 0x22, 0x25, 0x0a, 0x0d, 0x50, 0x72, 0x6f, 0x41, 0x74, 0x74, 0x61, 0x63, 0x68, 0x49, 0x6e, 0x66, 0x6f, 0x12, 0x14, 0x0a, 0x05, 0x74, 0x6f, 0x6b, 0x65, 0x6e, 0x18, 0x01, - 0x20, 0x01, 0x28, 0x09, 0x52, 0x05, 0x74, 0x6f, 0x6b, 0x65, 0x6e, 0x22, 0x37, 0x0a, 0x0f, 0x4c, + 0x20, 0x01, 0x28, 0x09, 0x52, 0x05, 0x74, 0x6f, 0x6b, 0x65, 0x6e, 0x22, 0x5b, 0x0a, 0x0f, 0x4c, 0x61, 0x6e, 0x64, 0x73, 0x63, 0x61, 0x70, 0x65, 0x43, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x12, 0x24, 0x0a, 0x0d, 0x63, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x75, 0x72, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x0d, 0x63, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x75, 0x72, 0x61, - 0x74, 0x69, 0x6f, 0x6e, 0x22, 0x07, 0x0a, 0x05, 0x45, 0x6d, 0x70, 0x74, 0x79, 0x32, 0xd2, 0x01, - 0x0a, 0x03, 0x57, 0x53, 0x4c, 0x12, 0x45, 0x0a, 0x0d, 0x41, 0x70, 0x70, 0x6c, 0x79, 0x50, 0x72, - 0x6f, 0x54, 0x6f, 0x6b, 0x65, 0x6e, 0x12, 0x1c, 0x2e, 0x77, 0x73, 0x6c, 0x73, 0x65, 0x72, 0x76, - 0x69, 0x63, 0x65, 0x61, 0x70, 0x69, 0x2e, 0x50, 0x72, 0x6f, 0x41, 0x74, 0x74, 0x61, 0x63, 0x68, - 0x49, 0x6e, 0x66, 0x6f, 0x1a, 0x14, 0x2e, 0x77, 0x73, 0x6c, 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, - 0x65, 0x61, 0x70, 0x69, 0x2e, 0x45, 0x6d, 0x70, 0x74, 0x79, 0x22, 0x00, 0x12, 0x34, 0x0a, 0x04, - 0x50, 0x69, 0x6e, 0x67, 0x12, 0x14, 0x2e, 0x77, 0x73, 0x6c, 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, - 0x65, 0x61, 0x70, 0x69, 0x2e, 0x45, 0x6d, 0x70, 0x74, 0x79, 0x1a, 0x14, 0x2e, 0x77, 0x73, 0x6c, - 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x61, 0x70, 0x69, 0x2e, 0x45, 0x6d, 0x70, 0x74, 0x79, - 0x22, 0x00, 0x12, 0x4e, 0x0a, 0x14, 0x41, 0x70, 0x70, 0x6c, 0x79, 0x4c, 0x61, 0x6e, 0x64, 0x73, - 0x63, 0x61, 0x70, 0x65, 0x43, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x12, 0x1e, 0x2e, 0x77, 0x73, 0x6c, - 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x61, 0x70, 0x69, 0x2e, 0x4c, 0x61, 0x6e, 0x64, 0x73, - 0x63, 0x61, 0x70, 0x65, 0x43, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x1a, 0x14, 0x2e, 0x77, 0x73, 0x6c, - 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x61, 0x70, 0x69, 0x2e, 0x45, 0x6d, 0x70, 0x74, 0x79, - 0x22, 0x00, 0x42, 0x3b, 0x5a, 0x39, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, - 0x2f, 0x63, 0x61, 0x6e, 0x6f, 0x6e, 0x69, 0x63, 0x61, 0x6c, 0x2f, 0x75, 0x62, 0x75, 0x6e, 0x74, - 0x75, 0x2d, 0x70, 0x72, 0x6f, 0x2d, 0x66, 0x6f, 0x72, 0x2d, 0x77, 0x69, 0x6e, 0x64, 0x6f, 0x77, - 0x73, 0x2f, 0x77, 0x73, 0x6c, 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x61, 0x70, 0x69, 0x62, - 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, + 0x74, 0x69, 0x6f, 0x6e, 0x12, 0x22, 0x0a, 0x0c, 0x68, 0x6f, 0x73, 0x74, 0x61, 0x67, 0x65, 0x6e, + 0x74, 0x55, 0x49, 0x44, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, 0x0c, 0x68, 0x6f, 0x73, 0x74, + 0x61, 0x67, 0x65, 0x6e, 0x74, 0x55, 0x49, 0x44, 0x22, 0x07, 0x0a, 0x05, 0x45, 0x6d, 0x70, 0x74, + 0x79, 0x32, 0xd2, 0x01, 0x0a, 0x03, 0x57, 0x53, 0x4c, 0x12, 0x45, 0x0a, 0x0d, 0x41, 0x70, 0x70, + 0x6c, 0x79, 0x50, 0x72, 0x6f, 0x54, 0x6f, 0x6b, 0x65, 0x6e, 0x12, 0x1c, 0x2e, 0x77, 0x73, 0x6c, + 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x61, 0x70, 0x69, 0x2e, 0x50, 0x72, 0x6f, 0x41, 0x74, + 0x74, 0x61, 0x63, 0x68, 0x49, 0x6e, 0x66, 0x6f, 0x1a, 0x14, 0x2e, 0x77, 0x73, 0x6c, 0x73, 0x65, + 0x72, 0x76, 0x69, 0x63, 0x65, 0x61, 0x70, 0x69, 0x2e, 0x45, 0x6d, 0x70, 0x74, 0x79, 0x22, 0x00, + 0x12, 0x34, 0x0a, 0x04, 0x50, 0x69, 0x6e, 0x67, 0x12, 0x14, 0x2e, 0x77, 0x73, 0x6c, 0x73, 0x65, + 0x72, 0x76, 0x69, 0x63, 0x65, 0x61, 0x70, 0x69, 0x2e, 0x45, 0x6d, 0x70, 0x74, 0x79, 0x1a, 0x14, + 0x2e, 0x77, 0x73, 0x6c, 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x61, 0x70, 0x69, 0x2e, 0x45, + 0x6d, 0x70, 0x74, 0x79, 0x22, 0x00, 0x12, 0x4e, 0x0a, 0x14, 0x41, 0x70, 0x70, 0x6c, 0x79, 0x4c, + 0x61, 0x6e, 0x64, 0x73, 0x63, 0x61, 0x70, 0x65, 0x43, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x12, 0x1e, + 0x2e, 0x77, 0x73, 0x6c, 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x61, 0x70, 0x69, 0x2e, 0x4c, + 0x61, 0x6e, 0x64, 0x73, 0x63, 0x61, 0x70, 0x65, 0x43, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x1a, 0x14, + 0x2e, 0x77, 0x73, 0x6c, 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x61, 0x70, 0x69, 0x2e, 0x45, + 0x6d, 0x70, 0x74, 0x79, 0x22, 0x00, 0x42, 0x3b, 0x5a, 0x39, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, + 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x63, 0x61, 0x6e, 0x6f, 0x6e, 0x69, 0x63, 0x61, 0x6c, 0x2f, 0x75, + 0x62, 0x75, 0x6e, 0x74, 0x75, 0x2d, 0x70, 0x72, 0x6f, 0x2d, 0x66, 0x6f, 0x72, 0x2d, 0x77, 0x69, + 0x6e, 0x64, 0x6f, 0x77, 0x73, 0x2f, 0x77, 0x73, 0x6c, 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, + 0x61, 0x70, 0x69, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, } var ( diff --git a/wslserviceapi/wslserviceapi_grpc.pb.go b/wslserviceapi/wslserviceapi_grpc.pb.go index 5f948f7ab..74dc17ab9 100644 --- a/wslserviceapi/wslserviceapi_grpc.pb.go +++ b/wslserviceapi/wslserviceapi_grpc.pb.go @@ -1,7 +1,7 @@ // Code generated by protoc-gen-go-grpc. DO NOT EDIT. // versions: // - protoc-gen-go-grpc v1.3.0 -// - protoc v3.21.12 +// - protoc v4.24.3 // source: wslserviceapi.proto package wslserviceapi From 83f64cd02bf12dc3c0a3401e52330be3cf9352a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Tue, 24 Oct 2023 09:39:33 +0200 Subject: [PATCH 06/18] LandscapeConfigure provisioning task now also sends hostagentUID --- windows-agent/internal/config/config.go | 1 + windows-agent/internal/tasks/landscape_configure.go | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/windows-agent/internal/config/config.go b/windows-agent/internal/config/config.go index e34ed4ae7..5b5aaac6e 100644 --- a/windows-agent/internal/config/config.go +++ b/windows-agent/internal/config/config.go @@ -189,6 +189,7 @@ func (c *Config) ProvisioningTasks(ctx context.Context, distroName string) ([]ta // Landcape registration taskList = append(taskList, tasks.LandscapeConfigure{ Config: c.data.landscapeClientConfig, + HostagentUID: c.data.landscapeAgentUID, }) return taskList, nil diff --git a/windows-agent/internal/tasks/landscape_configure.go b/windows-agent/internal/tasks/landscape_configure.go index 9f5104e93..38313245c 100644 --- a/windows-agent/internal/tasks/landscape_configure.go +++ b/windows-agent/internal/tasks/landscape_configure.go @@ -15,14 +15,15 @@ func init() { // - to register: send the config to register with. // - to disable: send an empty config. type LandscapeConfigure struct { - Config string + Config string + HostagentUID string } // Execute sends the config to the target WSL-Pro-Service so that the distro can be // registered in Landscape. func (t LandscapeConfigure) Execute(ctx context.Context, client wslserviceapi.WSLClient) error { // First value is a dummy message, we ignore it. We only care about success/failure. - _, err := client.ApplyLandscapeConfig(ctx, &wslserviceapi.LandscapeConfig{Configuration: t.Config}) + _, err := client.ApplyLandscapeConfig(ctx, &wslserviceapi.LandscapeConfig{Configuration: t.Config, HostagentUID: t.HostagentUID}) if err != nil { return task.NeedsRetryError{SourceErr: err} } From 10dd221c83c6edc4ec7725e08a5285f4c97846db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Tue, 24 Oct 2023 10:09:31 +0200 Subject: [PATCH 07/18] Landscape service no longer holds the hostagent UID This responsability has been shifted to the config. --- .../proservices/landscape/export_test.go | 6 -- .../proservices/landscape/landscape.go | 94 ++++--------------- .../proservices/landscape/receive_commands.go | 8 +- .../landscape/send_hostagentinfo.go | 7 +- .../internal/proservices/proservices.go | 2 +- 5 files changed, 31 insertions(+), 86 deletions(-) diff --git a/windows-agent/internal/proservices/landscape/export_test.go b/windows-agent/internal/proservices/landscape/export_test.go index 57544b5d4..732b382af 100644 --- a/windows-agent/internal/proservices/landscape/export_test.go +++ b/windows-agent/internal/proservices/landscape/export_test.go @@ -6,9 +6,3 @@ func WithHostname(hostname string) Option { o.hostname = hostname } } - -func (c *Client) UID() string { - return c.getUID() -} - -const CacheFileBase = cacheFileBase diff --git a/windows-agent/internal/proservices/landscape/landscape.go b/windows-agent/internal/proservices/landscape/landscape.go index 87315d8eb..6cb46d058 100644 --- a/windows-agent/internal/proservices/landscape/landscape.go +++ b/windows-agent/internal/proservices/landscape/landscape.go @@ -7,12 +7,9 @@ import ( "crypto/x509" "errors" "fmt" - "io/fs" "os" - "path/filepath" "strings" "sync" - "sync/atomic" "time" landscapeapi "github.com/canonical/landscape-hostagent-api" @@ -39,10 +36,6 @@ type Client struct { // Cached hostname hostname string - // Client UID and where it is stored - uid atomic.Value - cacheFile string - // Connection conn *connection connMu sync.RWMutex @@ -59,13 +52,15 @@ type connection struct { receivingCommands sync.WaitGroup } -const cacheFileBase = "landscape.conf" - // Config is a configuration provider for ProToken and the Landscape URL. type Config interface { LandscapeClientConfig(context.Context) (string, error) LandscapeAgentURL(context.Context) (string, error) + Subscription(context.Context) (string, config.SubscriptionSource, error) + + LandscapeAgentUID(context.Context) (string, error) + SetLandscapeAgentUID(context.Context, string) error } type options struct { @@ -76,7 +71,7 @@ type options struct { type Option = func(*options) // NewClient creates a new Client for the Landscape service. -func NewClient(conf Config, db *database.DistroDB, cacheDir string, args ...Option) (*Client, error) { +func NewClient(conf Config, db *database.DistroDB, args ...Option) (*Client, error) { var opts options for _, f := range args { @@ -92,15 +87,10 @@ func NewClient(conf Config, db *database.DistroDB, cacheDir string, args ...Opti } c := &Client{ - conf: conf, - db: db, - hostname: opts.hostname, - cacheFile: filepath.Join(cacheDir, cacheFileBase), - stopped: make(chan struct{}), - } - - if err := c.load(); err != nil { - return nil, err + conf: conf, + db: db, + hostname: opts.hostname, + stopped: make(chan struct{}), } return c, nil @@ -233,7 +223,7 @@ func (c *Client) connect(ctx context.Context, address string) (conn *connection, } }() - if err := c.handshake(conn); err != nil { + if err := c.handshake(ctx, conn); err != nil { conn.disconnect() return nil, err } @@ -249,14 +239,16 @@ func (c *Client) connect(ctx context.Context, address string) (conn *connection, // If this is the first connection ever, the server will respond by assigning // the host a UID. This Recv is handled by receiveCommands, but handshake // waits until the UID is received before returning. -func (c *Client) handshake(conn *connection) error { +func (c *Client) handshake(ctx context.Context, conn *connection) error { // Send first message if err := c.sendUpdatedInfo(conn); err != nil { return err } // Not the first contact between client and server: done! - if c.getUID() != "" { + if uid, err := c.conf.LandscapeAgentUID(ctx); err != nil { + return err + } else if uid != "" { return nil } @@ -271,12 +263,15 @@ func (c *Client) handshake(conn *connection) error { select { case <-ctx.Done(): conn.disconnect() - c.setUID("") // Avoid races where the UID arrives just after cancelling the context - return fmt.Errorf("Landscape server did not respond with a client UID") + // Avoid races where the UID arrives just after cancelling the context + err := c.conf.SetLandscapeAgentUID(ctx, "") + return errors.Join(err, fmt.Errorf("Landscape server did not respond with a client UID")) case <-ticker.C: } - if c.getUID() != "" { + if uid, err := c.conf.LandscapeAgentUID(ctx); err != nil { + return err + } else if uid != "" { // UID received: success. break } @@ -297,10 +292,6 @@ func (c *Client) Stop(ctx context.Context) { c.conn.disconnect() c.conn = nil } - - if err := c.dump(); err != nil { - log.Errorf(ctx, "Landscape client: %v", err) - } }) } @@ -344,51 +335,6 @@ func (conn *connection) connected() bool { return true } -// load reads persistent Landscape data from disk. -func (c *Client) load() error { - out, err := os.ReadFile(c.cacheFile) - if errors.Is(err, fs.ErrNotExist) { - // No file: New client - c.setUID("") - return nil - } - - if err != nil { - // Something is wrong with the file - return fmt.Errorf("could not read landscape config file: %v", err) - } - - // First contact done in previous session - c.setUID(string(out)) - return nil -} - -// dump stores persistent Landscape data to disk. -func (c *Client) dump() error { - tmpFile := fmt.Sprintf("%s.tmp", c.cacheFile) - - if err := os.WriteFile(tmpFile, []byte(c.getUID()), 0600); err != nil { - return fmt.Errorf("could not store Landscape data to temporary file: %v", err) - } - - if err := os.Rename(tmpFile, c.cacheFile); err != nil { - return fmt.Errorf("could not move Landscape data from tmp to file: %v", err) - } - - return nil -} - -// getUID is syntax sugar to read the UID. -func (c *Client) getUID() string { - //nolint:forcetypeassert // We know it is going to be a string - return c.uid.Load().(string) -} - -// setUID is syntax sugar to set the UID. -func (c *Client) setUID(s string) { - c.uid.Store(s) -} - // transportCredentials reads the Landscape client config to check if a SSL public key is specified. // // If this credential is not specified, an insecure credential is returned. diff --git a/windows-agent/internal/proservices/landscape/receive_commands.go b/windows-agent/internal/proservices/landscape/receive_commands.go index 45ed352aa..953e924ed 100644 --- a/windows-agent/internal/proservices/landscape/receive_commands.go +++ b/windows-agent/internal/proservices/landscape/receive_commands.go @@ -81,13 +81,13 @@ func commandString(command *landscapeapi.Command) string { } func (c *Client) cmdAssignHost(ctx context.Context, cmd *landscapeapi.Command_AssignHost) error { - if c.getUID() != "" { + if uid, err := c.conf.LandscapeAgentUID(ctx); err != nil { + log.Warningf(ctx, "Possibly overriding current landscape client UID: could not read current Landscape UID: %v", err) + } else if uid != "" { log.Warning(ctx, "Overriding current landscape client UID") } - c.setUID(cmd.GetUid()) - - if err := c.dump(); err != nil { + if err := c.conf.SetLandscapeAgentUID(ctx, cmd.GetUid()); err != nil { return err } diff --git a/windows-agent/internal/proservices/landscape/send_hostagentinfo.go b/windows-agent/internal/proservices/landscape/send_hostagentinfo.go index d0aa162de..288da95d0 100644 --- a/windows-agent/internal/proservices/landscape/send_hostagentinfo.go +++ b/windows-agent/internal/proservices/landscape/send_hostagentinfo.go @@ -69,9 +69,14 @@ func (c *Client) newHostAgentInfo(ctx context.Context) (info *landscapeapi.HostA instances = append(instances, instanceInfo) } + uid, err := c.conf.LandscapeAgentUID(ctx) + if err != nil { + return info, err + } + info = &landscapeapi.HostAgentInfo{ Token: token, - Uid: c.getUID(), + Uid: uid, Hostname: c.hostname, Instances: instances, } diff --git a/windows-agent/internal/proservices/proservices.go b/windows-agent/internal/proservices/proservices.go index a1e64122c..c9b2ec12b 100644 --- a/windows-agent/internal/proservices/proservices.go +++ b/windows-agent/internal/proservices/proservices.go @@ -110,7 +110,7 @@ func New(ctx context.Context, args ...Option) (s Manager, err error) { uiService := ui.New(ctx, conf, db) - landscape, err := landscape.NewClient(conf, db, opts.cacheDir) + landscape, err := landscape.NewClient(conf, db) if err != nil { return s, err } From ab1f31ce1c853a404348fb00b2292a19c1347cc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Tue, 24 Oct 2023 10:10:51 +0200 Subject: [PATCH 08/18] Update Landscape service tests There are now fewer, but different failure modes. --- .../proservices/landscape/landscape_test.go | 127 +++++++----------- 1 file changed, 51 insertions(+), 76 deletions(-) diff --git a/windows-agent/internal/proservices/landscape/landscape_test.go b/windows-agent/internal/proservices/landscape/landscape_test.go index 6ba7f5d35..73d6960f4 100644 --- a/windows-agent/internal/proservices/landscape/landscape_test.go +++ b/windows-agent/internal/proservices/landscape/landscape_test.go @@ -39,68 +39,21 @@ func TestMain(m *testing.M) { defer os.Exit(exit) } -func TestNew(t *testing.T) { +func TestConnect(t *testing.T) { if wsl.MockAvailable() { t.Parallel() } testCases := map[string]struct { - uid string - loadErr bool - - wantErr bool - }{ - "Success loading a new UID": {}, - "Success without loading a UID": {uid: "123"}, - - "Error reading UID": {loadErr: 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() - } - - dir := t.TempDir() - if tc.loadErr { - err := os.MkdirAll(filepath.Join(dir, landscape.CacheFileBase), 0700) - require.NoError(t, err, "Setup: could not create directory to interfere with the Landscape client") - } else if tc.uid != "" { - err := os.WriteFile(filepath.Join(dir, landscape.CacheFileBase), []byte(tc.uid), 0600) - require.NoError(t, err, "Setup: could not write Landscape client config file") - } + precancelContext bool + serverNotAvailable bool - conf := &mockConfig{} + landscapeUIDReadErr bool + landscapeUIDWriteErr bool - db, err := database.New(ctx, t.TempDir(), conf) - require.NoError(t, err, "Setup: database New should not return an error") - - c, err := landscape.NewClient(conf, db, dir) - if tc.wantErr { - require.Error(t, err, "Unexpected success in NewClient") - return - } - require.NoError(t, err, "NewClient should return no error") - - require.Equal(t, tc.uid, c.UID(), "Unexpected value for Landscape Client UID") - }) - } -} + landscapeURLErr bool + tokenErr bool -func TestConnect(t *testing.T) { - if wsl.MockAvailable() { - t.Parallel() - } - - testCases := map[string]struct { - precancelContext bool - serverNotAvailable bool - landscapeURLErr bool - tokenErr bool requireCertificate bool breakLandscapeClientConfig bool @@ -117,6 +70,8 @@ func TestConnect(t *testing.T) { "Error when the context is cancelled before Connected": {precancelContext: true, wantErr: true}, "Error when the landscape URL cannot be retrieved": {landscapeURLErr: true, wantErr: true}, + "Error when the landscape UID cannot be retrieved": {landscapeUIDReadErr: true, wantErr: true}, + "Error when the landscape UID cannot be stored": {landscapeUIDWriteErr: true, wantErr: true}, "Error when the server cannot be reached": {serverNotAvailable: true, wantErr: true}, "Error when the first-contact SendUpdatedInfo fails": {tokenErr: true, wantErr: true}, "Error when the config cannot be accessed": {breakLandscapeClientConfig: true, wantErr: true}, @@ -147,8 +102,14 @@ func TestConnect(t *testing.T) { // We trigger an earlier error by erroring out on LandscapeAgentURL landscapeURLErr: tc.landscapeURLErr, + // We trigger errors trying to read or write to/from the registry + landscapeUIDErr: tc.landscapeUIDReadErr, + setLandscapeUIDErr: tc.landscapeUIDWriteErr, + // We trigger an error when deciding to use a certificate or not landscapeConfigErr: tc.breakLandscapeClientConfig, + + landscapeAgentUID: tc.uid, } out, err := os.ReadFile(filepath.Join(golden.TestFixturePath(t), "landscape.conf")) @@ -166,12 +127,6 @@ func TestConnect(t *testing.T) { defer server.Stop() } - dir := t.TempDir() - if tc.uid != "" { - err := os.WriteFile(filepath.Join(dir, landscape.CacheFileBase), []byte(tc.uid), 0600) - require.NoError(t, err, "Setup: could not write Landscape client config file") - } - db, err := database.New(ctx, t.TempDir(), conf) require.NoError(t, err, "Setup: database New should not return an error") @@ -179,7 +134,7 @@ func TestConnect(t *testing.T) { _, err = db.GetDistroAndUpdateProperties(ctx, distroName, distro.Properties{}) require.NoError(t, err, "Setup: GetDistroAndUpdateProperties should return no errors") - client, err := landscape.NewClient(conf, db, dir) + client, err := landscape.NewClient(conf, db) require.NoError(t, err, "Setup: NewClient should return no errrors") ctx, cancel := context.WithCancel(ctx) @@ -209,16 +164,11 @@ func TestConnect(t *testing.T) { require.False(t, client.Connected(), "Connected should have returned false after disconnecting") - confFile := filepath.Join(dir, landscape.CacheFileBase) - require.FileExists(t, confFile, "Landscape config file should be created after disconnecting") - out, err = os.ReadFile(confFile) - require.NoError(t, err, "Could not read landscape config file") - wantUID := tc.uid if tc.uid == "" { wantUID = "ServerAssignedUID" } - requireHasPrefix(t, wantUID, string(out), "Landscape config should contain the Landscape Client UID") + requireHasPrefix(t, wantUID, conf.landscapeAgentUID, "Landscape client UID was not set properly") server.Stop() lis.Close() @@ -293,7 +243,7 @@ func TestSendUpdatedInfo(t *testing.T) { const hostname = "HOSTNAME" - client, err := landscape.NewClient(conf, db, t.TempDir(), landscape.WithHostname(hostname)) + client, err := landscape.NewClient(conf, db, landscape.WithHostname(hostname)) require.NoError(t, err, "Landscape NewClient should not return an error") if tc.distroIsRunning { @@ -445,7 +395,7 @@ func TestAutoReconnection(t *testing.T) { const hostname = "HOSTNAME" - client, err := landscape.NewClient(conf, db, t.TempDir(), landscape.WithHostname(hostname)) + client, err := landscape.NewClient(conf, db, landscape.WithHostname(hostname)) require.NoError(t, err, "Landscape NewClient should not return an error") defer client.Stop(ctx) @@ -593,7 +543,7 @@ func TestReceiveCommands(t *testing.T) { require.NoError(t, d.LockAwake(), "Setup: could not lock distro awake") } - client, err := landscape.NewClient(conf, db, t.TempDir(), landscape.WithHostname("HOSTNAME")) + client, err := landscape.NewClient(conf, db, landscape.WithHostname("HOSTNAME")) require.NoError(t, err, "Landscape NewClient should not return an error") err = client.Connect(ctx) @@ -601,12 +551,12 @@ func TestReceiveCommands(t *testing.T) { defer client.Stop(ctx) require.Eventually(t, func() bool { - return client.Connected() && client.UID() != "" && service.IsConnected(client.UID()) + return client.Connected() && conf.landscapeAgentUID != "" && service.IsConnected(conf.landscapeAgentUID) }, 10*time.Second, 100*time.Millisecond, "Landscape server and client never made a connection") enableMockErrors() - err = service.SendCommand(ctx, client.UID(), command) + err = service.SendCommand(ctx, conf.landscapeAgentUID, command) require.NoError(t, err, "SendCommand should return no error") // Allow some time for the message to be sent, received, and executed. @@ -631,7 +581,7 @@ func TestReceiveCommands(t *testing.T) { client.Stop(ctx) disableMockErrors() - requireCommandResult(t, ctx, tc.command, d, client, !tc.wantFailure) + requireCommandResult(t, ctx, tc.command, d, conf, !tc.wantFailure) }) } } @@ -691,16 +641,16 @@ func commandSetup(t *testing.T, ctx context.Context, command command, distro *di // did not complete. // //nolint:revive // testing.T goes before context -func requireCommandResult(t *testing.T, ctx context.Context, command command, distro *distro.Distro, client *landscape.Client, wantSuccess bool) { +func requireCommandResult(t *testing.T, ctx context.Context, command command, distro *distro.Distro, conf *mockConfig, wantSuccess bool) { t.Helper() // Ensuring a connection was made - require.NotEmpty(t, client.UID(), "Landscape client should have had a UID assigned by the server") + require.NotEmpty(t, conf.landscapeAgentUID, "Landscape client should have had a UID assigned by the server") switch command { case cmdAssignHost: if wantSuccess { - require.Equal(t, "HostUID123", client.UID(), "Landscape client should have overridden the initial UID sent by the server") + require.Equal(t, "HostUID123", conf.landscapeAgentUID, "Landscape client should have overridden the initial UID sent by the server") } else { require.Fail(t, "Test not implemented") } @@ -839,10 +789,13 @@ type mockConfig struct { proToken string landscapeAgentURL string landscapeClientConfig string + landscapeAgentUID string proTokenErr bool landscapeURLErr bool landscapeConfigErr bool + landscapeUIDErr bool + setLandscapeUIDErr bool mu sync.Mutex } @@ -880,3 +833,25 @@ func (m *mockConfig) LandscapeAgentURL(ctx context.Context) (string, error) { } return m.landscapeAgentURL, nil } + +func (m *mockConfig) LandscapeAgentUID(ctx context.Context) (string, error) { + m.mu.Lock() + defer m.mu.Unlock() + + if m.landscapeUIDErr { + return "", errors.New("Mock error") + } + return m.landscapeAgentUID, nil +} + +func (m *mockConfig) SetLandscapeAgentUID(ctx context.Context, uid string) error { + m.mu.Lock() + defer m.mu.Unlock() + + if m.setLandscapeUIDErr { + return errors.New("Mock error") + } + + m.landscapeAgentUID = uid + return nil +} From 9a54158e2d2fe28e55d4c7f75ca80221c97d384c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Tue, 24 Oct 2023 10:39:13 +0200 Subject: [PATCH 09/18] WSL-Pro-Service: write host-agent UID into landscape.conf --- wsl-pro-service/internal/system/landscape.go | 34 +++++++++---------- .../internal/system/system_test.go | 2 +- .../wslinstanceservice/wslinstanceservice.go | 4 ++- .../wslinstanceservice_test.go | 2 +- 4 files changed, 22 insertions(+), 20 deletions(-) diff --git a/wsl-pro-service/internal/system/landscape.go b/wsl-pro-service/internal/system/landscape.go index cf02bb8b5..6589d5331 100644 --- a/wsl-pro-service/internal/system/landscape.go +++ b/wsl-pro-service/internal/system/landscape.go @@ -19,11 +19,11 @@ const ( ) // LandscapeEnable registers the current distro to Landscape with the specified config. -func (s *System) LandscapeEnable(ctx context.Context, landscapeConfig string) (err error) { +func (s *System) LandscapeEnable(ctx context.Context, landscapeConfig string, hostagentUID string) (err error) { // Decorating here to avoid stuttering the URL (url package prints it as well) defer decorate.OnError(&err, "could not register to landscape") - if landscapeConfig, err = modifyConfig(ctx, s, landscapeConfig); err != nil { + if landscapeConfig, err = modifyConfig(ctx, s, landscapeConfig, hostagentUID); err != nil { return err } @@ -76,7 +76,7 @@ func (s *System) writeConfig(landscapeConfig string) (err error) { } // modifyConfig overrides parameters in the configuration to adapt them to the current distro. -func modifyConfig(ctx context.Context, s *System, landscapeConfig string) (string, error) { +func modifyConfig(ctx context.Context, s *System, landscapeConfig string, hostagentUID string) (string, error) { if landscapeConfig == "" { return "", nil } @@ -87,7 +87,15 @@ func modifyConfig(ctx context.Context, s *System, landscapeConfig string) (strin return "", fmt.Errorf("could not parse config: %v", err) } - if err := overrideComputerTitle(ctx, s, data); err != nil { + distroName, err := s.wslDistroName(ctx) + if err != nil { + return "", err + } + if err := overrideKey(ctx, data, "client", "computer_title", distroName); err != nil { + return "", err + } + + if err := overrideKey(ctx, data, "client", "hostagent_uid", hostagentUID); err != nil { return "", err } @@ -103,16 +111,8 @@ func modifyConfig(ctx context.Context, s *System, landscapeConfig string) (strin return w.String(), nil } -// overrideComputerTitle overrides the computer_title field in the Landscape config. -func overrideComputerTitle(ctx context.Context, s *System, data *ini.File) error { - const section = "client" - const key = "computer_title" - - distroName, err := s.wslDistroName(ctx) - if err != nil { - return err - } - +// overrideKey sets a key to a particular value. +func overrideKey(ctx context.Context, data *ini.File, section, key, value string) error { sec, err := data.GetSection(section) if err != nil { if sec, err = data.NewSection(section); err != nil { @@ -121,12 +121,12 @@ func overrideComputerTitle(ctx context.Context, s *System, data *ini.File) error } if sec.HasKey(key) { - log.Infof(ctx, "Landscape config contains key %q. Its value will be overridden with %s", key, distroName) + log.Infof(ctx, "Landscape config contains key %q. Its value will be overridden with %s", key, value) sec.DeleteKey(key) } - if _, err := sec.NewKey(key, distroName); err != nil { - return fmt.Errorf("could not create %q key", key) + if _, err := sec.NewKey(key, value); err != nil { + return fmt.Errorf("could not create key %q: %v", key, err) } return nil diff --git a/wsl-pro-service/internal/system/system_test.go b/wsl-pro-service/internal/system/system_test.go index 2de3b2cf5..592758adc 100644 --- a/wsl-pro-service/internal/system/system_test.go +++ b/wsl-pro-service/internal/system/system_test.go @@ -389,7 +389,7 @@ func TestLandscapeEnable(t *testing.T) { config, err := os.ReadFile(filepath.Join(golden.TestFixturePath(t), "landscape.conf")) require.NoError(t, err, "Setup: could not load fixture") - err = s.LandscapeEnable(ctx, string(config)) + err = s.LandscapeEnable(ctx, string(config), "landscapeUID1234") if tc.wantErr { require.Error(t, err, "LandscapeEnable should have returned an error") return diff --git a/wsl-pro-service/internal/wslinstanceservice/wslinstanceservice.go b/wsl-pro-service/internal/wslinstanceservice/wslinstanceservice.go index 123c47068..9173a67da 100644 --- a/wsl-pro-service/internal/wslinstanceservice/wslinstanceservice.go +++ b/wsl-pro-service/internal/wslinstanceservice/wslinstanceservice.go @@ -107,8 +107,10 @@ func (s *Service) ApplyLandscapeConfig(ctx context.Context, msg *wslserviceapi.L return &wslserviceapi.Empty{}, nil } + uid := msg.GetHostagentUID() + log.Infof(ctx, "ApplyLandscapeConfig: Received config: registering") - if err := s.system.LandscapeEnable(ctx, conf); err != nil { + if err := s.system.LandscapeEnable(ctx, conf, uid); err != nil { return nil, err } diff --git a/wsl-pro-service/internal/wslinstanceservice/wslinstanceservice_test.go b/wsl-pro-service/internal/wslinstanceservice/wslinstanceservice_test.go index 92c0e9334..36c642b6d 100644 --- a/wsl-pro-service/internal/wslinstanceservice/wslinstanceservice_test.go +++ b/wsl-pro-service/internal/wslinstanceservice/wslinstanceservice_test.go @@ -172,7 +172,7 @@ func TestApplyLandscapeConfig(t *testing.T) { config = "[hello]\nworld: true" } - empty, err := wslClient.ApplyLandscapeConfig(ctx, &wslserviceapi.LandscapeConfig{Configuration: config}) + empty, err := wslClient.ApplyLandscapeConfig(ctx, &wslserviceapi.LandscapeConfig{Configuration: config, HostagentUID: "landscapeHostagent1234"}) if tc.wantErr { require.Error(t, err, "ApplyLandscapeConfig call should return an error") return From e85641d4f5e1ce33a57d0aecd91ba77f06486548 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Tue, 24 Oct 2023 10:39:48 +0200 Subject: [PATCH 10/18] Update golden files --- .../internal/system/testdata/TestLandscapeEnable/golden/success | 1 + .../success_despite_failing_to_override_the_ssl_certficate_path | 1 + .../TestLandscapeEnable/golden/success_overriding_computer_title | 1 + .../golden/success_overriding_the_ssl_certficate_path | 1 + .../testdata/TestApplyLandscapeConfig/golden/success_enabling | 1 + 5 files changed, 5 insertions(+) diff --git a/wsl-pro-service/internal/system/testdata/TestLandscapeEnable/golden/success b/wsl-pro-service/internal/system/testdata/TestLandscapeEnable/golden/success index 1a9966055..a16b4d924 100644 --- a/wsl-pro-service/internal/system/testdata/TestLandscapeEnable/golden/success +++ b/wsl-pro-service/internal/system/testdata/TestLandscapeEnable/golden/success @@ -1,3 +1,4 @@ [client] hello = world computer_title = TEST_DISTRO +hostagent_uid = landscapeUID1234 diff --git a/wsl-pro-service/internal/system/testdata/TestLandscapeEnable/golden/success_despite_failing_to_override_the_ssl_certficate_path b/wsl-pro-service/internal/system/testdata/TestLandscapeEnable/golden/success_despite_failing_to_override_the_ssl_certficate_path index 3ee8ae0a7..37c31b7c7 100644 --- a/wsl-pro-service/internal/system/testdata/TestLandscapeEnable/golden/success_despite_failing_to_override_the_ssl_certficate_path +++ b/wsl-pro-service/internal/system/testdata/TestLandscapeEnable/golden/success_despite_failing_to_override_the_ssl_certficate_path @@ -2,3 +2,4 @@ hello = world ssl_public_key = D:\Users\TestUser\certificate computer_title = TEST_DISTRO +hostagent_uid = landscapeUID1234 diff --git a/wsl-pro-service/internal/system/testdata/TestLandscapeEnable/golden/success_overriding_computer_title b/wsl-pro-service/internal/system/testdata/TestLandscapeEnable/golden/success_overriding_computer_title index 1a9966055..a16b4d924 100644 --- a/wsl-pro-service/internal/system/testdata/TestLandscapeEnable/golden/success_overriding_computer_title +++ b/wsl-pro-service/internal/system/testdata/TestLandscapeEnable/golden/success_overriding_computer_title @@ -1,3 +1,4 @@ [client] hello = world computer_title = TEST_DISTRO +hostagent_uid = landscapeUID1234 diff --git a/wsl-pro-service/internal/system/testdata/TestLandscapeEnable/golden/success_overriding_the_ssl_certficate_path b/wsl-pro-service/internal/system/testdata/TestLandscapeEnable/golden/success_overriding_the_ssl_certficate_path index e9563aa4f..20c374f00 100644 --- a/wsl-pro-service/internal/system/testdata/TestLandscapeEnable/golden/success_overriding_the_ssl_certficate_path +++ b/wsl-pro-service/internal/system/testdata/TestLandscapeEnable/golden/success_overriding_the_ssl_certficate_path @@ -2,3 +2,4 @@ hello = world ssl_public_key = ${FILESYSTEM_ROOT}/mnt/d/Users/TestUser/certificate computer_title = TEST_DISTRO +hostagent_uid = landscapeUID1234 diff --git a/wsl-pro-service/internal/wslinstanceservice/testdata/TestApplyLandscapeConfig/golden/success_enabling b/wsl-pro-service/internal/wslinstanceservice/testdata/TestApplyLandscapeConfig/golden/success_enabling index e0016e6c5..57e29c0bb 100644 --- a/wsl-pro-service/internal/wslinstanceservice/testdata/TestApplyLandscapeConfig/golden/success_enabling +++ b/wsl-pro-service/internal/wslinstanceservice/testdata/TestApplyLandscapeConfig/golden/success_enabling @@ -3,3 +3,4 @@ world = true [client] computer_title = TEST_DISTRO +hostagent_uid = landscapeHostagent1234 From fc85d6fcceddcd80f18ec952c045ee4acada6eed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Tue, 24 Oct 2023 10:41:38 +0200 Subject: [PATCH 11/18] Only send landscape config task if we've made contact already Otherwise, the Landcape back-end cannot match the WSL distros to this any agent. --- windows-agent/internal/config/config.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/windows-agent/internal/config/config.go b/windows-agent/internal/config/config.go index 5b5aaac6e..fd6e0f35e 100644 --- a/windows-agent/internal/config/config.go +++ b/windows-agent/internal/config/config.go @@ -186,11 +186,16 @@ func (c *Config) ProvisioningTasks(ctx context.Context, distroName string) ([]ta proToken, _ := c.subscription() taskList = append(taskList, tasks.ProAttachment{Token: proToken}) - // Landcape registration - taskList = append(taskList, tasks.LandscapeConfigure{ - Config: c.data.landscapeClientConfig, - HostagentUID: c.data.landscapeAgentUID, - }) + if c.data.landscapeClientConfig == "" { + // Landscape unregistration: always + taskList = append(taskList, tasks.LandscapeConfigure{}) + } else if c.data.landscapeAgentUID != "" { + // Landcape registration: only when we have a UID assigned + taskList = append(taskList, tasks.LandscapeConfigure{ + Config: c.data.landscapeClientConfig, + HostagentUID: c.data.landscapeAgentUID, + }) + } return taskList, nil } From b1d40ae1a966f0ecfc1f9a7aac4a844b1a2f67cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Tue, 24 Oct 2023 11:44:48 +0200 Subject: [PATCH 12/18] Config test: de-duplicate getter tests Done in anticiption of adding a third test for the LandscapeAgentUID getter. --- windows-agent/internal/config/config_test.go | 90 ++++++++------------ 1 file changed, 35 insertions(+), 55 deletions(-) diff --git a/windows-agent/internal/config/config_test.go b/windows-agent/internal/config/config_test.go index 74358bfe7..f7cfe4aa1 100644 --- a/windows-agent/internal/config/config_test.go +++ b/windows-agent/internal/config/config_test.go @@ -84,9 +84,15 @@ func TestSubscription(t *testing.T) { } } -//nolint:dupl // This and the following test are very similar, but there is no clean way to merge them -func TestLandscapeAgentURL(t *testing.T) { - t.Parallel() +type testConfigGetterSettings struct { + getter func(*config.Config, context.Context) (string, error) + getterName string + registryHasValue registryState + want string +} + +func testConfigGetter(t *testing.T, s testConfigGetterSettings) { + t.Helper() testCases := map[string]struct { mockErrors uint32 @@ -94,12 +100,12 @@ func TestLandscapeAgentURL(t *testing.T) { wantError bool }{ - "Success": {registryState: landscapeAgentURLHasValue}, + "Success": {registryState: s.registryHasValue}, "Success when the key does not exist": {registryState: untouched}, "Success when the value does not exist": {registryState: keyExists}, - "Error when the registry key cannot be opened": {registryState: landscapeAgentURLHasValue, mockErrors: registry.MockErrOnOpenKey, wantError: true}, - "Error when the registry key cannot be read from": {registryState: landscapeAgentURLHasValue, mockErrors: registry.MockErrReadValue, wantError: true}, + "Error when the registry key cannot be opened": {registryState: s.registryHasValue, mockErrors: registry.MockErrOnOpenKey, wantError: true}, + "Error when the registry key cannot be read from": {registryState: s.registryHasValue, mockErrors: registry.MockErrReadValue, wantError: true}, } for name, tc := range testCases { @@ -111,71 +117,45 @@ func TestLandscapeAgentURL(t *testing.T) { r := setUpMockRegistry(tc.mockErrors, tc.registryState, false) conf := config.New(ctx, config.WithRegistry(r)) - landscapeURL, err := conf.LandscapeAgentURL(ctx) + v, err := s.getter(conf, ctx) if tc.wantError { - require.Error(t, err, "LandscapeAgentURL should return an error") + require.Error(t, err, "%s should return an error", s.getterName) return } - require.NoError(t, err, "LandscapeAgentURL should return no error") + require.NoError(t, err, "%s should return no error", s.getterName) // Test default values - if !tc.registryState.is(landscapeAgentURLHasValue) { - require.Equal(t, "", landscapeURL, "Unexpected Landscape URL default value") + if !tc.registryState.is(s.registryHasValue) { + require.Emptyf(t, v, "Unexpected value when %s is not set in registry", s.getterName) return } // Test non-default values - assert.Equal(t, "www.example.com/registry-example", landscapeURL, "Unexpected Landscape URL value") - assert.Zero(t, r.OpenKeyCount.Load(), "Leaking keys after ProToken") + assert.Equalf(t, s.want, v, "%s returned an unexpected value", s.getterName) + assert.Zerof(t, r.OpenKeyCount.Load(), "Call to %s leaks registry keys", s.getterName) }) } } -//nolint:dupl // This and the previous test are very similar, but there is no clean way to merge them -func TestLandscapeClientConfig(t *testing.T) { +func TestLandscapeAgentURL(t *testing.T) { t.Parallel() + testConfigGetter(t, testConfigGetterSettings{ + getter: (*config.Config).LandscapeAgentURL, + getterName: "LandscapeAgentURL", + registryHasValue: landscapeAgentURLHasValue, + want: "www.example.com/registry-example", + }) +} - testCases := map[string]struct { - mockErrors uint32 - registryState registryState - - wantError bool - }{ - "Success": {registryState: landscapeClientConfigHasValue}, - "Success when the key does not exist": {registryState: untouched}, - "Success when the value does not exist": {registryState: keyExists}, - - "Error when the registry key cannot be opened": {registryState: landscapeClientConfigHasValue, mockErrors: registry.MockErrOnOpenKey, wantError: true}, - "Error when the registry key cannot be read from": {registryState: landscapeClientConfigHasValue, mockErrors: registry.MockErrReadValue, wantError: true}, - } - - for name, tc := range testCases { - tc := tc - t.Run(name, func(t *testing.T) { - t.Parallel() - ctx := context.Background() - - r := setUpMockRegistry(tc.mockErrors, tc.registryState, false) - conf := config.New(ctx, config.WithRegistry(r)) - - landscapeURL, err := conf.LandscapeClientConfig(ctx) - if tc.wantError { - require.Error(t, err, "LandscapeClientConfig should return an error") - return - } - require.NoError(t, err, "LandscapeClientConfig should return no error") - - // Test default values - if !tc.registryState.is(landscapeClientConfigHasValue) { - require.Equal(t, "", landscapeURL, "Unexpected Landscape config default value") - return - } +func TestLandscapeClientConfig(t *testing.T) { + t.Parallel() - // Test non-default values - assert.Equal(t, "[client]\nuser=JohnDoe", landscapeURL, "Unexpected Landscape URL value") - assert.Zero(t, r.OpenKeyCount.Load(), "Leaking keys after ProToken") - }) - } + testConfigGetter(t, testConfigGetterSettings{ + getter: (*config.Config).LandscapeClientConfig, + getterName: "LandscapeClientConfig", + registryHasValue: landscapeClientConfigHasValue, + want: "[client]\nuser=JohnDoe", + }) } func TestProvisioningTasks(t *testing.T) { From 08b7a0d4add5fd7169b569b1877c4042cadd5b28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Tue, 24 Oct 2023 12:11:25 +0200 Subject: [PATCH 13/18] Config: implement tests for Landscape UID getter and setter --- windows-agent/internal/config/config_test.go | 109 +++++++++++++++++-- 1 file changed, 99 insertions(+), 10 deletions(-) diff --git a/windows-agent/internal/config/config_test.go b/windows-agent/internal/config/config_test.go index f7cfe4aa1..72a6e8f9b 100644 --- a/windows-agent/internal/config/config_test.go +++ b/windows-agent/internal/config/config_test.go @@ -24,12 +24,14 @@ const ( storeTokenExists // Key exists, microsoft store token field exists landscapeAgentURLExists // Key exists, landscape agent URL field exists landscapeClientConfigExists // Key exists, landscape client config field exists + landscapeAgentUIDExists // Key exists, landscape agent UID field exists orgTokenHasValue = orgTokenExists | 1<<16 // Key exists, organization token field exists and is not empty userTokenHasValue = userTokenExists | 1<<17 // Key exists, user token field exists and is not empty storeTokenHasValue = storeTokenExists | 1<<18 // Key exists, microsoft store token field exists and is not empty landscapeAgentURLHasValue = landscapeAgentURLExists | 1<<19 // Key exists, landscape agent URL field exists and is not empty landscapeClientConfigHasValue = landscapeClientConfigExists | 1<<20 // Key exists, landscape client config field exists and is not empty + landscapeAgentUIDHasValue = landscapeAgentUIDExists | 1<<21 // Key exists, landscape agent UID field exists and is not empty ) func TestSubscription(t *testing.T) { @@ -158,6 +160,17 @@ func TestLandscapeClientConfig(t *testing.T) { }) } +func TestLandscapeAgentUID(t *testing.T) { + t.Parallel() + + testConfigGetter(t, testConfigGetterSettings{ + getter: (*config.Config).LandscapeAgentUID, + getterName: "LandscapeAgentUID", + registryHasValue: landscapeAgentUIDHasValue, + want: "landscapeUID1234", + }) +} + func TestProvisioningTasks(t *testing.T) { t.Parallel() @@ -165,13 +178,16 @@ func TestProvisioningTasks(t *testing.T) { mockErrors uint32 registryState registryState - want string - wantError bool + want string + wantNoLandscape bool + wantError bool }{ - "Success when the key does not exist": {registryState: untouched}, - "Success when the pro token field does not exist": {registryState: keyExists}, - "Success when the pro token exists but is empty": {registryState: userTokenExists}, - "Success with a user token": {registryState: userTokenHasValue, want: "user_token"}, + "Success when the key does not exist": {registryState: untouched}, + "Success when the pro token field does not exist": {registryState: keyExists}, + "Success when the pro token exists but is empty": {registryState: userTokenExists}, + "Success with a user token": {registryState: userTokenHasValue, want: "user_token"}, + "Success when there is Landscape config, but no UID": {registryState: landscapeClientConfigHasValue, wantNoLandscape: true}, + "Success when there is Landscape config and UID": {registryState: landscapeClientConfigHasValue | landscapeAgentUIDHasValue}, "Error when the registry key cannot be opened": {registryState: userTokenExists, mockErrors: registry.MockErrOnOpenKey, wantError: true}, "Error when the registry key cannot be read from": {registryState: userTokenExists, mockErrors: registry.MockErrReadValue, wantError: true}, @@ -186,17 +202,25 @@ func TestProvisioningTasks(t *testing.T) { r := setUpMockRegistry(tc.mockErrors, tc.registryState, false) conf := config.New(ctx, config.WithRegistry(r)) - pt, err := conf.ProvisioningTasks(ctx, "UBUNTU") + gotTasks, err := conf.ProvisioningTasks(ctx, "UBUNTU") if tc.wantError { require.Error(t, err, "ProvisioningTasks should return an error") return } require.NoError(t, err, "ProvisioningTasks should return no error") - require.ElementsMatch(t, pt, []task.Task{ + wantTasks := []task.Task{ tasks.ProAttachment{Token: tc.want}, - tasks.LandscapeConfigure{Config: ""}, - }, "Unexpected contents returned by ProvisioningTasks") + } + + if !tc.wantNoLandscape { + wantTasks = append(wantTasks, tasks.LandscapeConfigure{ + Config: r.UbuntuProData["LandscapeClientConfig"], + HostagentUID: r.UbuntuProData["LandscapeAgentUID"], + }) + } + + require.ElementsMatch(t, wantTasks, gotTasks, "Unexpected contents returned by ProvisioningTasks") }) } } @@ -260,6 +284,64 @@ func TestSetSubscription(t *testing.T) { } } +func TestSetLandscapeAgentUID(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + mockErrors uint32 + registryState registryState + accessIsReadOnly bool + setEmptyUID bool + + want string + wantError bool + wantErrorType error + }{ + "Success": {registryState: landscapeAgentUIDHasValue, want: "new_uid"}, + "Success unsetting the UID": {registryState: landscapeAgentUIDHasValue, setEmptyUID: true, want: ""}, + "Success when the key does not exist": {registryState: untouched, want: "new_uid"}, + "Success when the pro token field does not exist": {registryState: keyExists, want: "new_uid"}, + + "Error when the registry key cannot be written on due to lack of permission": {registryState: landscapeAgentUIDHasValue, accessIsReadOnly: true, want: "landscapeUID1234", wantError: true, wantErrorType: registry.ErrAccessDenied}, + "Error when the registry key cannot be opened": {registryState: landscapeAgentUIDHasValue, mockErrors: registry.MockErrOnCreateKey, want: "landscapeUID1234", wantError: true, wantErrorType: registry.ErrMock}, + "Error when the registry key cannot be written on": {registryState: landscapeAgentUIDHasValue, mockErrors: registry.MockErrOnWriteValue, want: "landscapeUID1234", wantError: true, wantErrorType: registry.ErrMock}, + "Error when the registry key cannot be read": {registryState: landscapeAgentUIDHasValue, mockErrors: registry.MockErrOnOpenKey, want: "landscapeUID1234", wantError: true, wantErrorType: registry.ErrMock}, + } + + for name, tc := range testCases { + tc := tc + t.Run(name, func(t *testing.T) { + t.Parallel() + ctx := context.Background() + + r := setUpMockRegistry(tc.mockErrors, tc.registryState, tc.accessIsReadOnly) + conf := config.New(ctx, config.WithRegistry(r)) + + uid := "new_uid" + if tc.setEmptyUID { + uid = "" + } + + err := conf.SetLandscapeAgentUID(ctx, uid) + if tc.wantError { + require.Error(t, err, "SetLandscapeAgentUID should return an error") + if tc.wantErrorType != nil { + require.ErrorIs(t, err, tc.wantErrorType, "SetLandscapeAgentUID returned an error of unexpected type") + } + } else { + require.NoError(t, err, "SetLandscapeAgentUID should return no error") + } + + // Disable errors so we can retrieve the UID + r.Errors = 0 + uid, err = conf.LandscapeAgentUID(ctx) + require.NoError(t, err, "LandscapeAgentUID should return no error") + + require.Equal(t, tc.want, uid, "LandscapeAgentUID returned an unexpected value for the token") + }) + } +} + func TestIsReadOnly(t *testing.T) { t.Parallel() @@ -401,5 +483,12 @@ func setUpMockRegistry(mockErrors uint32, state registryState, readOnly bool) *r r.UbuntuProData["LandscapeClientConfig"] = "[client]\nuser=JohnDoe" } + if state.is(landscapeAgentUIDExists) { + r.UbuntuProData["LandscapeAgentUID"] = "" + } + if state.is(landscapeAgentUIDHasValue) { + r.UbuntuProData["LandscapeAgentUID"] = "landscapeUID1234" + } + return r } From c8b3916bf106b2f56d5a3311af05a5758a1ea6eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Tue, 24 Oct 2023 13:00:47 +0200 Subject: [PATCH 14/18] Config: get rid of default registry values All default values are empty strings, and the linter was also complaining about readValue always receiving the same value for its 'defaultValue' argument. Hence, I simplified the code so that there are no default values, or rather the default values are always "". We can bring them back if we ever need one of the defaults to be non-empty. --- windows-agent/internal/config/config.go | 30 +++++++++---------------- 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/windows-agent/internal/config/config.go b/windows-agent/internal/config/config.go index fd6e0f35e..e76a72d7c 100644 --- a/windows-agent/internal/config/config.go +++ b/windows-agent/internal/config/config.go @@ -19,16 +19,9 @@ import ( const ( registryPath = `Software\Canonical\UbuntuPro` - defaultToken = "" - - fieldLandscapeClientConfig = "LandscapeClientConfig" - defaultLandscapeClientConfig = "" - - fieldLandscapeAgentURL = "LandscapeAgentURL" - defaultLandscapeAgentURL = "" - - fieldLandscapeAgentUID = "LandscapeAgentUID" - defaultLandscapeAgentUID = "" + fieldLandscapeClientConfig = "LandscapeClientConfig" + fieldLandscapeAgentURL = "LandscapeAgentURL" + fieldLandscapeAgentUID = "LandscapeAgentUID" ) // fieldsProToken contains the fields in the registry where each source will store its token. @@ -305,9 +298,6 @@ func (c *Config) loadRegistry(ctx context.Context) (proTokens map[SubscriptionSo k, err := c.registry.HKCUOpenKey(registryPath, registry.READ) if errors.Is(err, registry.ErrKeyNotExist) { log.Debug(ctx, "Registry key does not exist, using default values") - data.landscapeAgentURL = defaultLandscapeAgentURL - data.landscapeClientConfig = defaultLandscapeClientConfig - data.landscapeAgentUID = defaultLandscapeAgentUID return proTokens, data, nil } if err != nil { @@ -316,7 +306,7 @@ func (c *Config) loadRegistry(ctx context.Context) (proTokens map[SubscriptionSo defer c.registry.CloseKey(k) for source, field := range fieldsProToken { - proToken, e := c.readValue(ctx, k, field, defaultToken) + proToken, e := c.readValue(ctx, k, field) if e != nil { err = errors.Join(err, fmt.Errorf("could not read %q: %v", field, e)) continue @@ -333,17 +323,17 @@ func (c *Config) loadRegistry(ctx context.Context) (proTokens map[SubscriptionSo return nil, data, err } - data.landscapeAgentURL, err = c.readValue(ctx, k, fieldLandscapeAgentURL, defaultLandscapeAgentURL) + data.landscapeAgentURL, err = c.readValue(ctx, k, fieldLandscapeAgentURL) if err != nil { return proTokens, data, err } - data.landscapeClientConfig, err = c.readValue(ctx, k, fieldLandscapeClientConfig, defaultLandscapeClientConfig) + data.landscapeClientConfig, err = c.readValue(ctx, k, fieldLandscapeClientConfig) if err != nil { return proTokens, data, err } - data.landscapeAgentUID, err = c.readValue(ctx, k, fieldLandscapeAgentUID, defaultLandscapeAgentUID) + data.landscapeAgentUID, err = c.readValue(ctx, k, fieldLandscapeAgentUID) if err != nil { return proTokens, data, err } @@ -351,11 +341,11 @@ func (c *Config) loadRegistry(ctx context.Context) (proTokens map[SubscriptionSo return proTokens, data, nil } -func (c *Config) readValue(ctx context.Context, key uintptr, field string, defaultValue string) (string, error) { +func (c *Config) readValue(ctx context.Context, key uintptr, field string) (string, error) { value, err := c.registry.ReadValue(key, field) if errors.Is(err, registry.ErrFieldNotExist) { - log.Debugf(ctx, "Registry value %q does not exist, defaulting to %q", field, defaultValue) - return defaultValue, nil + log.Debugf(ctx, "Registry value %q does not exist, defaulting to empty", field) + return "", nil } if err != nil { return "", err From 963073de8b2e322bf471709312b3000f8a54e391 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Wed, 25 Oct 2023 15:59:17 +0200 Subject: [PATCH 15/18] Reordered meta-test in config_tests.go --- windows-agent/internal/config/config_test.go | 64 ++++++++++---------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/windows-agent/internal/config/config_test.go b/windows-agent/internal/config/config_test.go index 72a6e8f9b..6c757636b 100644 --- a/windows-agent/internal/config/config_test.go +++ b/windows-agent/internal/config/config_test.go @@ -86,6 +86,38 @@ func TestSubscription(t *testing.T) { } } +func TestLandscapeAgentURL(t *testing.T) { + t.Parallel() + testConfigGetter(t, testConfigGetterSettings{ + getter: (*config.Config).LandscapeAgentURL, + getterName: "LandscapeAgentURL", + registryHasValue: landscapeAgentURLHasValue, + want: "www.example.com/registry-example", + }) +} + +func TestLandscapeClientConfig(t *testing.T) { + t.Parallel() + + testConfigGetter(t, testConfigGetterSettings{ + getter: (*config.Config).LandscapeClientConfig, + getterName: "LandscapeClientConfig", + registryHasValue: landscapeClientConfigHasValue, + want: "[client]\nuser=JohnDoe", + }) +} + +func TestLandscapeAgentUID(t *testing.T) { + t.Parallel() + + testConfigGetter(t, testConfigGetterSettings{ + getter: (*config.Config).LandscapeAgentUID, + getterName: "LandscapeAgentUID", + registryHasValue: landscapeAgentUIDHasValue, + want: "landscapeUID1234", + }) +} + type testConfigGetterSettings struct { getter func(*config.Config, context.Context) (string, error) getterName string @@ -139,38 +171,6 @@ func testConfigGetter(t *testing.T, s testConfigGetterSettings) { } } -func TestLandscapeAgentURL(t *testing.T) { - t.Parallel() - testConfigGetter(t, testConfigGetterSettings{ - getter: (*config.Config).LandscapeAgentURL, - getterName: "LandscapeAgentURL", - registryHasValue: landscapeAgentURLHasValue, - want: "www.example.com/registry-example", - }) -} - -func TestLandscapeClientConfig(t *testing.T) { - t.Parallel() - - testConfigGetter(t, testConfigGetterSettings{ - getter: (*config.Config).LandscapeClientConfig, - getterName: "LandscapeClientConfig", - registryHasValue: landscapeClientConfigHasValue, - want: "[client]\nuser=JohnDoe", - }) -} - -func TestLandscapeAgentUID(t *testing.T) { - t.Parallel() - - testConfigGetter(t, testConfigGetterSettings{ - getter: (*config.Config).LandscapeAgentUID, - getterName: "LandscapeAgentUID", - registryHasValue: landscapeAgentUIDHasValue, - want: "landscapeUID1234", - }) -} - func TestProvisioningTasks(t *testing.T) { t.Parallel() From 0adac6906a69bbaef28c36427317d48fb163e225 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Wed, 25 Oct 2023 16:02:12 +0200 Subject: [PATCH 16/18] Remove redundant t.Helper in config meta-test --- windows-agent/internal/config/config_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/windows-agent/internal/config/config_test.go b/windows-agent/internal/config/config_test.go index 6c757636b..37612492b 100644 --- a/windows-agent/internal/config/config_test.go +++ b/windows-agent/internal/config/config_test.go @@ -125,9 +125,8 @@ type testConfigGetterSettings struct { want string } +//nolint:thelper // This is the test itself, not a helper. Besides, a t.Helper() here would not affect the subtests. func testConfigGetter(t *testing.T, s testConfigGetterSettings) { - t.Helper() - testCases := map[string]struct { mockErrors uint32 registryState registryState From bc4ba7e3e0e0f18529c8c8f1ab2b2d4013826fe3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Wed, 25 Oct 2023 16:03:58 +0200 Subject: [PATCH 17/18] Rename test parameter to emptyUID --- windows-agent/internal/config/config_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/windows-agent/internal/config/config_test.go b/windows-agent/internal/config/config_test.go index 37612492b..323942254 100644 --- a/windows-agent/internal/config/config_test.go +++ b/windows-agent/internal/config/config_test.go @@ -231,14 +231,14 @@ func TestSetSubscription(t *testing.T) { mockErrors uint32 registryState registryState accessIsReadOnly bool - setEmptyToken bool + emptyToken bool want string wantError bool wantErrorType error }{ "Success": {registryState: userTokenHasValue, want: "new_token"}, - "Success disabling a subscription": {registryState: userTokenHasValue, setEmptyToken: true, want: ""}, + "Success disabling a subscription": {registryState: userTokenHasValue, emptyToken: true, want: ""}, "Success when the key does not exist": {registryState: untouched, want: "new_token"}, "Success when the pro token field does not exist": {registryState: keyExists, want: "new_token"}, "Success when there is a store token active": {registryState: storeTokenHasValue, want: "store_token"}, @@ -259,7 +259,7 @@ func TestSetSubscription(t *testing.T) { conf := config.New(ctx, config.WithRegistry(r)) token := "new_token" - if tc.setEmptyToken { + if tc.emptyToken { token = "" } @@ -290,14 +290,14 @@ func TestSetLandscapeAgentUID(t *testing.T) { mockErrors uint32 registryState registryState accessIsReadOnly bool - setEmptyUID bool + emptyUID bool want string wantError bool wantErrorType error }{ "Success": {registryState: landscapeAgentUIDHasValue, want: "new_uid"}, - "Success unsetting the UID": {registryState: landscapeAgentUIDHasValue, setEmptyUID: true, want: ""}, + "Success unsetting the UID": {registryState: landscapeAgentUIDHasValue, emptyUID: true, want: ""}, "Success when the key does not exist": {registryState: untouched, want: "new_uid"}, "Success when the pro token field does not exist": {registryState: keyExists, want: "new_uid"}, @@ -317,7 +317,7 @@ func TestSetLandscapeAgentUID(t *testing.T) { conf := config.New(ctx, config.WithRegistry(r)) uid := "new_uid" - if tc.setEmptyUID { + if tc.emptyUID { uid = "" } From 0eb44028fd5f4b08183ce00f057ba9a2d49abe17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Wed, 25 Oct 2023 16:05:12 +0200 Subject: [PATCH 18/18] Improved variable naming in config test Good old want/got :) --- windows-agent/internal/config/config_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/windows-agent/internal/config/config_test.go b/windows-agent/internal/config/config_test.go index 323942254..be42e4775 100644 --- a/windows-agent/internal/config/config_test.go +++ b/windows-agent/internal/config/config_test.go @@ -275,10 +275,10 @@ func TestSetSubscription(t *testing.T) { // Disable errors so we can retrieve the token r.Errors = 0 - token, _, err = conf.Subscription(ctx) + got, _, err := conf.Subscription(ctx) require.NoError(t, err, "ProToken should return no error") - require.Equal(t, tc.want, token, "ProToken returned an unexpected value for the token") + require.Equal(t, tc.want, got, "ProToken returned an unexpected value for the token") }) } } @@ -333,10 +333,10 @@ func TestSetLandscapeAgentUID(t *testing.T) { // Disable errors so we can retrieve the UID r.Errors = 0 - uid, err = conf.LandscapeAgentUID(ctx) + got, err := conf.LandscapeAgentUID(ctx) require.NoError(t, err, "LandscapeAgentUID should return no error") - require.Equal(t, tc.want, uid, "LandscapeAgentUID returned an unexpected value for the token") + require.Equal(t, tc.want, got, "LandscapeAgentUID returned an unexpected value for the token") }) } }