Skip to content

Commit

Permalink
fix(gui): Landscape config serialization (#855)
Browse files Browse the repository at this point in the history
The Landscape ping service doesn't support https by default.

Additionally, wslpath can fool wsl-pro-service, when the path passed to
the command line is an empty path it outputs the current working
directory absolute path. Due systemd, cwd is `/`.


![image](https://github.com/user-attachments/assets/6d8cb161-cd06-48ea-9782-d7e478056fdd)

We should have wsl-pro-service checking for empty paths before invoking
wslpath for added correctness (users could well assemble a config file
by hand having `ssl_public_key = `), but it also makes sense to fix it
here, as we're aware of which fields are optional in the UI. But,
instead of treating this (or any path) field as special, I'm being more
generic, **filtering out any line in the INI configuration output that
could have empty values**, i.e., we won't output lines with this shape :
`key =`.

More test assertions were added validating those assumptions.
  • Loading branch information
CarlosNihelton authored Aug 2, 2024
2 parents 69f592c + ab57440 commit 3a85a9f
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 9 deletions.
24 changes: 17 additions & 7 deletions gui/packages/ubuntupro/lib/pages/landscape/landscape_model.dart
Original file line number Diff line number Diff line change
Expand Up @@ -174,16 +174,20 @@ class LandscapeSaasConfig extends LandscapeConfig {
if (!isComplete) return null;
final uri = Uri.https(landscapeSaas);

final registrationKeyLine =
registrationKey.isEmpty ? '' : 'registration_key = $registrationKey';

return '''
[host]
url = ${uri.replace(port: 6554).authority}
[client]
account_name = $accountName
registration_key = $registrationKey
url = ${uri.replace(path: '/message-system')}
ping_url = ${uri.replace(path: '/ping')}
ping_url = ${uri.replace(scheme: 'http').replace(path: '/ping')}
log_level = info
''';
$registrationKeyLine
'''
.trimRight();
}
}

Expand Down Expand Up @@ -262,18 +266,24 @@ class LandscapeSelfHostedConfig extends LandscapeConfig {
@override
String? config() {
if (!isComplete) return null;

final sslKeyLine = sslKeyPath.isEmpty ? '' : 'ssl_public_key = $sslKeyPath';
final registrationKeyLine =
registrationKey.isEmpty ? '' : 'registration_key = $registrationKey';

final uri = Uri.parse(_fqdn);
return '''
[host]
url = ${uri.replace(port: 6554).authority}
[client]
account_name = $standalone
registration_key = $registrationKey
url = ${uri.replace(path: '/message-system')}
ping_url = ${uri.replace(path: '/ping')}
ping_url = ${uri.replace(scheme: 'http').replace(path: '/ping')}
log_level = info
ssl_public_key = $sslKeyPath
''';
$sslKeyLine
$registrationKeyLine
'''
.trimRight();
}
}

Expand Down
8 changes: 8 additions & 0 deletions gui/packages/ubuntupro/pubspec.lock
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,14 @@ packages:
url: "https://pub.dev"
source: hosted
version: "4.2.0"
ini:
dependency: "direct dev"
description:
name: ini
sha256: "12a76c53591ffdf86d1265be3f986888a6dfeb34a85957774bc65912d989a173"
url: "https://pub.dev"
source: hosted
version: "2.1.0"
integration_test:
dependency: "direct dev"
description: flutter
Expand Down
1 change: 1 addition & 0 deletions gui/packages/ubuntupro/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ dev_dependencies:
flutter_lints: ^4.0.0
flutter_test:
sdk: flutter
ini: ^2.1.0
integration_test:
sdk: flutter
nested: ^1.0.0
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import 'package:flutter_test/flutter_test.dart';
import 'package:ini/ini.dart';

import 'package:ubuntupro/pages/landscape/landscape_model.dart';

Expand Down Expand Up @@ -30,7 +31,11 @@ void main() {
c.accountName = tc.account;
expect(c.accountNameError, tc.wantError);
expect(c.isComplete, tc.wantComplete);
expect(c.config(), tc.wantConfig);
final raw = c.config();
expect(raw, tc.wantConfig);
if (raw != null) {
expectINI(raw);
}
});
}
});
Expand Down Expand Up @@ -114,7 +119,11 @@ void main() {
expect(c.fqdnError, tc.wantFqdnError);
expect(c.fileError, tc.wantFileError);
expect(c.isComplete, tc.wantComplete);
expect(c.config(), tc.wantConfig);
final raw = c.config();
expect(raw, tc.wantConfig);
if (raw != null) {
expectINI(raw);
}
});
}
});
Expand Down Expand Up @@ -177,6 +186,25 @@ void main() {
});
}

void expectINI(String raw) {
final config = Config.fromStrings(raw.split('\n'));
expectNoEmptyValuesInINI(config);
expectUrlSchemes(config);
}

void expectNoEmptyValuesInINI(Config config) {
for (final o in config.items('client')!) {
expect(o[1], isNotEmpty);
}
}

void expectUrlSchemes(Config config) {
final url = Uri.parse(config.get('client', 'url')!);
expect(url.scheme, 'https');
final ping = Uri.parse(config.get('client', 'ping_url')!);
expect(ping.scheme, 'http');
}

const saasURL = 'https://landscape.canonical.com';
const customConf = './test/testdata/landscape/custom.conf';
const notFoundPath = './test/testdata/landscape/notfound.txt';

0 comments on commit 3a85a9f

Please sign in to comment.