Skip to content

[fuchsia_ctl] Fix IPv6 URL formatting for amber_ctl #46

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
godofredoc merged 3 commits into from
Nov 19, 2019
Merged

[fuchsia_ctl] Fix IPv6 URL formatting for amber_ctl #46

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
94987e2
[fuchsia_ctl] Fix IPv6 URL formatting for amber_ctl
ianloic Nov 19, 2019
1685e4e
[fuchsia_ctl] Discover local address over ssh.
ianloic Nov 19, 2019
4de6a2c
[fuchsia_ctl] remove author from pubspec.yaml
ianloic Nov 19, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/fuchsia_ctl/bin/main.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ Future<OperationResult> test(
command: <String>[
'amberctl',
'add_src',
'-f', 'http://$localIp:${server.serverPort}/config.json', //
'-f', server.sourceUrl(localIp), //
'-n', uuid,
],
);
Expand Down
11 changes: 11 additions & 0 deletions packages/fuchsia_ctl/lib/src/package_server.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,4 +121,15 @@ class PackageServer {
return OperationResult.error(
'The "pm" executable exited with non-zero exit code.');
}

/// Returns the source URL for the given local address.
///
/// Calling this before calling [serveRepo] will result in a [StateError].
String sourceUrl(String localIp) {
if (localIp.contains(':')) {

@godofredoc godofredoc Nov 19, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did we validated this fixes the problem locally?

@ianloic ianloic Nov 19, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope. And actually, I think it's wrong...

@dnfield dnfield Nov 19, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe try https://api.dartlang.org/stable/2.6.1/dart-io/InternetAddress-class.html and get the type from it?

@ianloic ianloic Nov 19, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried that but InternetAddress fails to parse IPv6 link local addresses. Additionally we need the link local address from the perspective of the Fuchsia device, not the host. I'm working on an update to this PR that actually does that.

@godofredoc godofredoc Nov 19, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I ran locally " amberctl add_src" I just added the ipv6 + interface name from the host which is connected to the devices network and it worked. The device was able to access the amber server.

Devices in the lab are connected to a similar network setup " a nuc(host) with two interfaces one for internet access/lab network access and another one for internal nat network. The device is connected to the nat network. I think eno2 should work

@ianloic ianloic Nov 19, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may work but I think it's incorrect. With the second commit in this PR I've made the logic match that in fx add-update-source.

return 'http://[${localIp.replaceAll('%', '%25')}]:$serverPort/config.json';
} else {
return 'http://$localIp:$serverPort/config.json';
}
}
}
4 changes: 4 additions & 0 deletions packages/fuchsia_ctl/test/package_server_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,10 @@ void main() {
]);
expect(server.serverPort, randomPort);

expect(server.sourceUrl('192.168.42.42'),
'http://192.168.42.42:$randomPort/config.json');
expect(server.sourceUrl('fe80::f64d:30ff:fe6b:25d6%br0'),
'http://[fe80::f64d:30ff:fe6b:25d6%25br0]:$randomPort/config.json');
final OperationResult result = await server.close();

expect(result.success, true);
Expand Down