diff --git a/gui/packages/ubuntupro/lib/l10n/app_en.arb b/gui/packages/ubuntupro/lib/l10n/app_en.arb index f722bbfc9..eefe9c88b 100644 --- a/gui/packages/ubuntupro/lib/l10n/app_en.arb +++ b/gui/packages/ubuntupro/lib/l10n/app_en.arb @@ -53,7 +53,7 @@ "purchaseStatusServer":"Something went wrong with Microsoft Store. Please try again later.", "purchaseStatusUnknown": "Unknown error when trying to purchase the subscription. Consider restarting this app.", - "landscapeHeading": "You can manage your Ubuntu instances remotely with {landscapeLink}.\n\nConfigure the connection to Landscape:", + "landscapeHeading": "Configure the connection to {landscapeLink} to manage your Ubuntu WSL instances remotely.", "@landscapeHeading": { "placeholders": { "landscapeLink": { @@ -61,9 +61,9 @@ } } }, - "landscapeQuickSetupSaas": "Quick Setup (SaaS)", + "landscapeQuickSetupSaas": "Landscape SaaS configuration", "landscapeQuickSetupSaasHint": "Register with landscape.canonical.com", - "landscapeQuickSetupSelfHosted": "Quick Setup (Self-hosted)", + "landscapeQuickSetupSelfHosted": "Self-hosted Landscape configuration", "landscapeQuickSetupSelfHostedHint": "Register with your own Landscape server", "landscapeFQDNLabel": "Landscape server address", "landscapeFQDNError": "Invalid URI. Format should be a hostname or IP address.", diff --git a/gui/packages/ubuntupro/lib/pages/landscape/landscape_page.dart b/gui/packages/ubuntupro/lib/pages/landscape/landscape_page.dart index 03f6c80b3..0a89e36df 100644 --- a/gui/packages/ubuntupro/lib/pages/landscape/landscape_page.dart +++ b/gui/packages/ubuntupro/lib/pages/landscape/landscape_page.dart @@ -141,7 +141,7 @@ class LandscapeConfigForm extends StatelessWidget { final lang = AppLocalizations.of(context); return Padding( - padding: const EdgeInsets.symmetric(vertical: _kHeight), + padding: const EdgeInsets.symmetric(vertical: 2 * _kHeight), // The FocusTraversalGroup is necessary to keep the tab navigation order wed expect: // We ping-pong between the radio buttons and the form fields that belong to the selected radio button, // by assigning odd NumericFocusOrder() values to the radio buttons (on the left) and even values to the form fields, @@ -158,17 +158,17 @@ class LandscapeConfigForm extends StatelessWidget { Flexible( child: Column( children: [ - FocusTraversalOrder( - order: const NumericFocusOrder(0), - child: _ConfigTypeRadio( - value: LandscapeConfigType.saas, - title: lang.landscapeQuickSetupSaas, - subtitle: lang.landscapeQuickSetupSaasHint, - groupValue: model.configType, - onChanged: - model.isSaaSSupported ? model.setConfigType : null, + if (model.isSaaSSupported) + FocusTraversalOrder( + order: const NumericFocusOrder(0), + child: _ConfigTypeRadio( + value: LandscapeConfigType.saas, + title: lang.landscapeQuickSetupSaas, + subtitle: lang.landscapeQuickSetupSaasHint, + groupValue: model.configType, + onChanged: model.setConfigType, + ), ), - ), FocusTraversalOrder( order: const NumericFocusOrder(2), child: _ConfigTypeRadio( diff --git a/gui/packages/ubuntupro/lib/pages/subscribe_now/subscribe_now_widgets.dart b/gui/packages/ubuntupro/lib/pages/subscribe_now/subscribe_now_widgets.dart index 5587db278..685e8e1bc 100644 --- a/gui/packages/ubuntupro/lib/pages/subscribe_now/subscribe_now_widgets.dart +++ b/gui/packages/ubuntupro/lib/pages/subscribe_now/subscribe_now_widgets.dart @@ -1,5 +1,6 @@ import 'package:dart_either/dart_either.dart'; import 'package:flutter/material.dart'; +import 'package:flutter/services.dart'; import 'package:flutter_gen/gen_l10n/app_localizations.dart'; import 'package:yaru/yaru.dart'; import '../../core/either_value_notifier.dart'; @@ -78,6 +79,10 @@ class _ProTokenInputFieldState extends State { children: [ Expanded( child: TextField( + inputFormatters: [ + // This ignores all sorts of (Unicode) whitespaces (not only at the ends). + FilteringTextInputFormatter.deny(RegExp(r'\s')), + ], autofocus: false, controller: _controller, decoration: InputDecoration( diff --git a/gui/packages/ubuntupro/pubspec.lock b/gui/packages/ubuntupro/pubspec.lock index 2573d6fa3..88355b0d0 100644 --- a/gui/packages/ubuntupro/pubspec.lock +++ b/gui/packages/ubuntupro/pubspec.lock @@ -254,10 +254,10 @@ packages: dependency: "direct main" description: name: file_picker - sha256: "1375f8685ca6f0412effecc2db834757e9d0e3e055468053e563794b0755cdcd" + sha256: "167bb619cdddaa10ef2907609feb8a79c16dfa479d3afaf960f8e223f754bf12" url: "https://pub.dev" source: hosted - version: "8.1.1" + version: "8.1.2" fixnum: dependency: transitive description: diff --git a/gui/packages/ubuntupro/pubspec.yaml b/gui/packages/ubuntupro/pubspec.yaml index 94020ce2c..1e08eb0c8 100644 --- a/gui/packages/ubuntupro/pubspec.yaml +++ b/gui/packages/ubuntupro/pubspec.yaml @@ -37,7 +37,7 @@ dependencies: base_x: ^2.0.1 crypto: ^3.0.5 dart_either: ^1.0.0 - file_picker: ^8.1.1 + file_picker: ^8.1.2 flutter: sdk: flutter flutter_localizations: diff --git a/gui/packages/ubuntupro/test/pages/subcribe_now/subscribe_now_widgets_test.dart b/gui/packages/ubuntupro/test/pages/subcribe_now/subscribe_now_widgets_test.dart index 355c258ea..15930cf3d 100644 --- a/gui/packages/ubuntupro/test/pages/subcribe_now/subscribe_now_widgets_test.dart +++ b/gui/packages/ubuntupro/test/pages/subcribe_now/subscribe_now_widgets_test.dart @@ -137,6 +137,24 @@ void main() { final input = tester.firstWidget(inputField); expect(input.decoration!.errorText, isNull); + final button = + tester.firstWidget(find.byType(ElevatedButton)); + expect(button.enabled, isTrue); + }); + testWidgets('good token with spaces', (tester) async { + await tester.pumpWidget(theApp); + final inputField = find.byType(TextField); + + await tester.enterText( + inputField, + // good token plus a bunch of types of white spaces. + ' ${tks.good} \u{00A0}\u{2000}\u{2002}\u{202F}\u{205F}\u{3000} ', + ); + await tester.pump(); + + final input = tester.firstWidget(inputField); + expect(input.decoration!.errorText, isNull); + final button = tester.firstWidget(find.byType(ElevatedButton)); expect(button.enabled, isTrue); diff --git a/windows-agent/internal/cloudinit/cloudinit.go b/windows-agent/internal/cloudinit/cloudinit.go index b1fb3ee17..c4b5971bc 100644 --- a/windows-agent/internal/cloudinit/cloudinit.go +++ b/windows-agent/internal/cloudinit/cloudinit.go @@ -37,6 +37,12 @@ func New(ctx context.Context, conf Config, publicDir string) (CloudInit, error) conf: conf, } + // c.writeAgentData() is no longer guaranteed to create the cloud-init directory, + // so let's check now if we have permission to do so. + if err := os.MkdirAll(c.dataDir, 0700); err != nil { + return CloudInit{}, fmt.Errorf("could not create cloud-init directory: %v", err) + } + if err := c.writeAgentData(); err != nil { return CloudInit{}, err } @@ -60,6 +66,11 @@ func (c CloudInit) writeAgentData() (err error) { return err } + // Nothing to write, we don't want an empty agent.yaml confusing the real cloud-init. + if cloudInit == nil { + return removeFileInDir(c.dataDir, "agent.yaml") + } + err = writeFileInDir(c.dataDir, "agent.yaml", cloudInit) if err != nil { return err @@ -78,6 +89,15 @@ func (c CloudInit) WriteDistroData(distroName string, cloudInit string) error { return nil } +// removeFileInDir attempts to remove the file 'dir/file' if it exists. Missing file is not an error. +func removeFileInDir(dir, file string) error { + err := os.Remove(filepath.Join(dir, file)) + if err != nil && !os.IsNotExist(err) { + return err + } + return nil +} + // writeFileInDir: // 1. Creates the directory if it did not exist. // 2. Creates the file using the temp-then-move pattern. This avoids read/write races. @@ -121,12 +141,6 @@ func (c CloudInit) RemoveDistroData(distroName string) (err error) { } func marshalConfig(conf Config) ([]byte, error) { - w := &bytes.Buffer{} - - if _, err := fmt.Fprintln(w, "#cloud-config\n# This file was generated automatically and must not be edited"); err != nil { - return nil, fmt.Errorf("could not write #cloud-config stenza and warning message: %v", err) - } - contents := make(map[string]interface{}) if err := ubuntuProModule(conf, contents); err != nil { @@ -137,11 +151,22 @@ func marshalConfig(conf Config) ([]byte, error) { return nil, err } + // If there is no config to write, then let's not write an empty object with comments to avoid confusing cloud-init. + if len(contents) == 0 { + return nil, nil + } + out, err := yaml.Marshal(contents) if err != nil { return nil, fmt.Errorf("could not Marshal user data as a YAML: %v", err) } + w := &bytes.Buffer{} + + if _, err := fmt.Fprintln(w, "#cloud-config\n# This file was generated automatically and must not be edited"); err != nil { + return nil, fmt.Errorf("could not write #cloud-config stenza and warning message: %v", err) + } + if _, err := w.Write(out); err != nil { return nil, fmt.Errorf("could not write config body: %v", err) } diff --git a/windows-agent/internal/cloudinit/cloudinit_test.go b/windows-agent/internal/cloudinit/cloudinit_test.go index 06875dc47..d632dd4c9 100644 --- a/windows-agent/internal/cloudinit/cloudinit_test.go +++ b/windows-agent/internal/cloudinit/cloudinit_test.go @@ -19,9 +19,13 @@ func TestNew(t *testing.T) { testCases := map[string]struct { breakWriteAgentData bool - wantErr bool + emptyConfig bool + + wantErr bool + wantNoAgentYaml bool }{ "Success": {}, + "No file if there is no config to write into": {emptyConfig: true, wantNoAgentYaml: true}, "Error when cloud-init agent file cannot be written": {breakWriteAgentData: true, wantErr: true}, } @@ -33,7 +37,13 @@ func TestNew(t *testing.T) { publicDir := t.TempDir() + proToken := "test token" + if tc.emptyConfig { + proToken = "" + } + conf := &mockConfig{ + proToken: proToken, subcriptionErr: tc.breakWriteAgentData, } @@ -47,6 +57,10 @@ func TestNew(t *testing.T) { // We don't assert on specifics, as they are tested in WriteAgentData tests. path := filepath.Join(publicDir, ".cloud-init", "agent.yaml") + if tc.wantNoAgentYaml { + require.NoFileExists(t, path, "there should be no agent data file if there is no config to write into") + return + } require.FileExists(t, path, "agent data file was not created when updating the config") }) } @@ -87,9 +101,12 @@ hostagent_uid = landscapeUID1234 badLandscape bool // Break writing to file - breakDir bool - breakTempFile bool - breakFile bool + breakDir bool + breakTempFile bool + breakFile bool + breakRemovingFile bool + + wantAgentYamlAsDir bool }{ "Success": {}, "Without hostagent UID": {skipHostAgentUID: true}, @@ -98,6 +115,7 @@ hostagent_uid = landscapeUID1234 "Without Landscape [client] section": {landscapeNoClientSection: true}, "With empty contents": {skipProToken: true, skipLandscapeConf: true}, + "Error to remove existing agent.yaml": {skipProToken: true, skipLandscapeConf: true, breakRemovingFile: true, wantAgentYamlAsDir: true}, "Error obtaining pro token": {breakSubscription: true}, "Error obtaining Landscape config": {breakLandscape: true}, "Error with erroneous Landscape config": {badLandscape: true}, @@ -164,6 +182,11 @@ hostagent_uid = landscapeUID1234 require.NoError(t, os.WriteFile(dir, nil, 0600), "Setup: could not create file to mess with cloud-init directory") } + if tc.breakRemovingFile { + _ = os.RemoveAll(path) + require.NoError(t, os.MkdirAll(path, 0750), "Creating the directory that breaks removing agent.yaml should not fail") + require.NoError(t, os.WriteFile(filepath.Join(path, "child.txt"), nil, 0600), "Setup: could not create file to mess with agent.yaml") + } ci.Update(ctx) // Assert that the file was updated (success case) or that the old one remains (error case) @@ -172,6 +195,17 @@ hostagent_uid = landscapeUID1234 return } + if tc.wantAgentYamlAsDir { + require.DirExists(t, path, "There should be a directory instead of agent.yaml") + return + } + + golden := testutils.Path(t) + if _, err = os.Stat(golden); err != nil && os.IsNotExist(err) { + // golden file doesn't exist + require.NoFileExists(t, path, "There should not be cloud-init agent file without useful contents") + return + } got, err := os.ReadFile(path) require.NoError(t, err, "There should be no error reading the cloud-init agent file") @@ -237,7 +271,7 @@ data: require.NoError(t, err, "Setup: cloud-init New should return no errors") if !tc.noOldData { - require.NoError(t, os.MkdirAll(filepath.Dir(path), 0600), "Setup: could not write old distro data directory") + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0700), "Setup: could not write old distro data directory") require.NoError(t, os.WriteFile(path, []byte(oldCloudInit), 0600), "Setup: could not write old distro data") } diff --git a/windows-agent/internal/cloudinit/testdata/TestUpdate/golden/with_empty_contents b/windows-agent/internal/cloudinit/testdata/TestUpdate/golden/with_empty_contents deleted file mode 100644 index f72d75c57..000000000 --- a/windows-agent/internal/cloudinit/testdata/TestUpdate/golden/with_empty_contents +++ /dev/null @@ -1,3 +0,0 @@ -#cloud-config -# This file was generated automatically and must not be edited -{} diff --git a/windows-agent/internal/proservices/proservices_test.go b/windows-agent/internal/proservices/proservices_test.go index 9f524ce10..b00d877cf 100644 --- a/windows-agent/internal/proservices/proservices_test.go +++ b/windows-agent/internal/proservices/proservices_test.go @@ -92,12 +92,16 @@ func TestNew(t *testing.T) { } require.NoError(t, err, "New should return no error") - // Those lines are just to trigger the registry watcher callbacks, they don't write anything to the agent.yaml file by the time we check, that's why goldens are empty. err = reg.WriteValue(k, "LandscapeConfig", "[client]\nuser=JohnDoe", true) require.NoError(t, err, "Setup: could not write LandscapeConfig to the registry mock") err = reg.WriteValue(k, "UbuntuProToken", "test-token", false) require.NoError(t, err, "Setup: could not write UbuntuProToken to the registry mock") + agentYamlPath := filepath.Join(publicDir, ".cloud-init", "agent.yaml") + + // Wait for the agent.yaml to be written + require.Eventually(t, checkFileExists(agentYamlPath), 5*time.Second, 200*time.Millisecond, "agent.yaml file should have been created with registry data") + got, err := os.ReadFile(filepath.Join(publicDir, ".cloud-init", "agent.yaml")) require.NoError(t, err, "Setup: could not read agent.yaml file post test completion") want := testutils.LoadWithUpdateFromGolden(t, string(got)) @@ -106,6 +110,13 @@ func TestNew(t *testing.T) { } } +func checkFileExists(path string) func() bool { + return func() bool { + s, err := os.Stat(path) + return err == nil && !s.IsDir() + } +} + func TestRegisterGRPCServices(t *testing.T) { t.Parallel() diff --git a/windows-agent/internal/proservices/testdata/TestNew/golden/when_the_config_cannot_check_if_it_is_read-only b/windows-agent/internal/proservices/testdata/TestNew/golden/when_the_config_cannot_check_if_it_is_read-only index f72d75c57..3c977d136 100644 --- a/windows-agent/internal/proservices/testdata/TestNew/golden/when_the_config_cannot_check_if_it_is_read-only +++ b/windows-agent/internal/proservices/testdata/TestNew/golden/when_the_config_cannot_check_if_it_is_read-only @@ -1,3 +1,8 @@ #cloud-config # This file was generated automatically and must not be edited -{} +landscape: + client: + tags: wsl + user: JohnDoe +ubuntu_pro: + token: test-token diff --git a/windows-agent/internal/proservices/testdata/TestNew/golden/when_the_subscription_stays_empty b/windows-agent/internal/proservices/testdata/TestNew/golden/when_the_subscription_stays_empty index f72d75c57..3c977d136 100644 --- a/windows-agent/internal/proservices/testdata/TestNew/golden/when_the_subscription_stays_empty +++ b/windows-agent/internal/proservices/testdata/TestNew/golden/when_the_subscription_stays_empty @@ -1,3 +1,8 @@ #cloud-config # This file was generated automatically and must not be edited -{} +landscape: + client: + tags: wsl + user: JohnDoe +ubuntu_pro: + token: test-token