Commit 91637164 authored by danielng's avatar danielng Committed by Commit Bot

crostini: Refactoring tests for port forwarding and extending coverage

Refactoring crostini port forwarding tests to be more readable and more
easily editable in the futre. Also adding new tests to cover interface
switching.

Tests: Running the implemented tests
Bug: 848127
Change-Id: I26fdbb65c92c2e9ea161fe20738644e12965fd5d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2214804Reviewed-by: default avatarNic Hollingum <hollingum@google.com>
Reviewed-by: default avatarFergus Dall <sidereal@google.com>
Commit-Queue: Daniel Ng <danielng@google.com>
Cr-Commit-Position: refs/heads/master@{#772109}
parent 03782603
...@@ -21,11 +21,9 @@ ...@@ -21,11 +21,9 @@
namespace crostini { namespace crostini {
// Currently, we are not supporting ethernet/mlan/usb port forwarding. // Currently, we are not supporting ethernet/mlan/usb port forwarding.
const char kDefaultInterfaceToForward[] = "all"; const char kDefaultInterfaceToForward[] = "wlan0";
const char kWlanInterface[] = "wlan0";
const char kPortNumberKey[] = "port_number"; const char kPortNumberKey[] = "port_number";
const char kPortProtocolKey[] = "protocol_type"; const char kPortProtocolKey[] = "protocol_type";
const char kPortInterfaceKey[] = "input_ifname";
const char kPortLabelKey[] = "label"; const char kPortLabelKey[] = "label";
const char kPortVmNameKey[] = "vm_name"; const char kPortVmNameKey[] = "vm_name";
const char kPortContainerNameKey[] = "container_name"; const char kPortContainerNameKey[] = "container_name";
...@@ -66,7 +64,7 @@ CrostiniPortForwarder* CrostiniPortForwarder::GetForProfile(Profile* profile) { ...@@ -66,7 +64,7 @@ CrostiniPortForwarder* CrostiniPortForwarder::GetForProfile(Profile* profile) {
CrostiniPortForwarder::CrostiniPortForwarder(Profile* profile) CrostiniPortForwarder::CrostiniPortForwarder(Profile* profile)
: profile_(profile) { : profile_(profile) {
current_interface_ = kWlanInterface; current_interface_ = kDefaultInterfaceToForward;
} }
CrostiniPortForwarder::~CrostiniPortForwarder() = default; CrostiniPortForwarder::~CrostiniPortForwarder() = default;
...@@ -81,13 +79,11 @@ bool CrostiniPortForwarder::MatchPortRuleDict(const base::Value& dict, ...@@ -81,13 +79,11 @@ bool CrostiniPortForwarder::MatchPortRuleDict(const base::Value& dict,
const PortRuleKey& key) { const PortRuleKey& key) {
base::Optional<int> port_number = dict.FindIntKey(kPortNumberKey); base::Optional<int> port_number = dict.FindIntKey(kPortNumberKey);
base::Optional<int> protocol_type = dict.FindIntKey(kPortProtocolKey); base::Optional<int> protocol_type = dict.FindIntKey(kPortProtocolKey);
const std::string* input_ifname = dict.FindStringKey(kPortInterfaceKey);
const std::string* vm_name = dict.FindStringKey(kPortVmNameKey); const std::string* vm_name = dict.FindStringKey(kPortVmNameKey);
const std::string* container_name = dict.FindStringKey(kPortContainerNameKey); const std::string* container_name = dict.FindStringKey(kPortContainerNameKey);
return (port_number && port_number.value() == key.port_number) && return (port_number && port_number.value() == key.port_number) &&
(protocol_type && (protocol_type &&
protocol_type.value() == static_cast<int>(key.protocol_type)) && protocol_type.value() == static_cast<int>(key.protocol_type)) &&
(input_ifname && *input_ifname == key.input_ifname) &&
(vm_name && *vm_name == key.container_id.vm_name) && (vm_name && *vm_name == key.container_id.vm_name) &&
(container_name && *container_name == key.container_id.container_name); (container_name && *container_name == key.container_id.container_name);
} }
...@@ -110,7 +106,6 @@ void CrostiniPortForwarder::AddNewPortPreference(const PortRuleKey& key, ...@@ -110,7 +106,6 @@ void CrostiniPortForwarder::AddNewPortPreference(const PortRuleKey& key,
new_port_metadata.SetIntKey(kPortNumberKey, key.port_number); new_port_metadata.SetIntKey(kPortNumberKey, key.port_number);
new_port_metadata.SetIntKey(kPortProtocolKey, new_port_metadata.SetIntKey(kPortProtocolKey,
static_cast<int>(key.protocol_type)); static_cast<int>(key.protocol_type));
new_port_metadata.SetStringKey(kPortInterfaceKey, kDefaultInterfaceToForward);
new_port_metadata.SetStringKey(kPortLabelKey, label); new_port_metadata.SetStringKey(kPortLabelKey, label);
new_port_metadata.SetStringKey(kPortVmNameKey, key.container_id.vm_name); new_port_metadata.SetStringKey(kPortVmNameKey, key.container_id.vm_name);
new_port_metadata.SetStringKey(kPortContainerNameKey, new_port_metadata.SetStringKey(kPortContainerNameKey,
...@@ -257,7 +252,6 @@ void CrostiniPortForwarder::AddPort(const ContainerId& container_id, ...@@ -257,7 +252,6 @@ void CrostiniPortForwarder::AddPort(const ContainerId& container_id,
PortRuleKey new_port_key = { PortRuleKey new_port_key = {
.port_number = port_number, .port_number = port_number,
.protocol_type = protocol_type, .protocol_type = protocol_type,
.input_ifname = kDefaultInterfaceToForward,
.container_id = container_id, .container_id = container_id,
}; };
...@@ -278,7 +272,6 @@ void CrostiniPortForwarder::ActivatePort(const ContainerId& container_id, ...@@ -278,7 +272,6 @@ void CrostiniPortForwarder::ActivatePort(const ContainerId& container_id,
PortRuleKey existing_port_key = { PortRuleKey existing_port_key = {
.port_number = port_number, .port_number = port_number,
.protocol_type = protocol_type, .protocol_type = protocol_type,
.input_ifname = kDefaultInterfaceToForward,
.container_id = container_id, .container_id = container_id,
}; };
...@@ -309,7 +302,6 @@ void CrostiniPortForwarder::DeactivatePort(const ContainerId& container_id, ...@@ -309,7 +302,6 @@ void CrostiniPortForwarder::DeactivatePort(const ContainerId& container_id,
PortRuleKey existing_port_key = { PortRuleKey existing_port_key = {
.port_number = port_number, .port_number = port_number,
.protocol_type = protocol_type, .protocol_type = protocol_type,
.input_ifname = kDefaultInterfaceToForward,
.container_id = container_id, .container_id = container_id,
}; };
...@@ -334,7 +326,6 @@ void CrostiniPortForwarder::RemovePort(const ContainerId& container_id, ...@@ -334,7 +326,6 @@ void CrostiniPortForwarder::RemovePort(const ContainerId& container_id,
PortRuleKey existing_port_key = { PortRuleKey existing_port_key = {
.port_number = port_number, .port_number = port_number,
.protocol_type = protocol_type, .protocol_type = protocol_type,
.input_ifname = kDefaultInterfaceToForward,
.container_id = container_id, .container_id = container_id,
}; };
...@@ -399,6 +390,8 @@ base::Optional<base::Value> CrostiniPortForwarder::ReadPortPreferenceForTesting( ...@@ -399,6 +390,8 @@ base::Optional<base::Value> CrostiniPortForwarder::ReadPortPreferenceForTesting(
void CrostiniPortForwarder::UpdateActivePortInterfaces() { void CrostiniPortForwarder::UpdateActivePortInterfaces() {
for (auto& port : forwarded_ports_) { for (auto& port : forwarded_ports_) {
// Note that this process erases the current lifeline attached to the port
// rule and implicitly causes the current port rule to be revoked.
TryActivatePort(port.first, port.first.container_id, base::DoNothing()); TryActivatePort(port.first, port.first.container_id, base::DoNothing());
} }
} }
......
...@@ -43,13 +43,11 @@ class CrostiniPortForwarder : public KeyedService { ...@@ -43,13 +43,11 @@ class CrostiniPortForwarder : public KeyedService {
struct PortRuleKey { struct PortRuleKey {
uint16_t port_number; uint16_t port_number;
Protocol protocol_type; Protocol protocol_type;
std::string input_ifname;
ContainerId container_id; ContainerId container_id;
bool operator==(const PortRuleKey& other) const { bool operator==(const PortRuleKey& other) const {
return port_number == other.port_number && return port_number == other.port_number &&
protocol_type == other.protocol_type && protocol_type == other.protocol_type;
input_ifname == other.input_ifname;
} }
}; };
...@@ -58,8 +56,7 @@ class CrostiniPortForwarder : public KeyedService { ...@@ -58,8 +56,7 @@ class CrostiniPortForwarder : public KeyedService {
std::size_t operator()(const PortRuleKey& k) const { std::size_t operator()(const PortRuleKey& k) const {
return ((std::hash<uint16_t>()(k.port_number) ^ return ((std::hash<uint16_t>()(k.port_number) ^
(std::hash<Protocol>()(k.protocol_type) << 1)) >> (std::hash<Protocol>()(k.protocol_type) << 1)) >>
1) ^ 1);
(std::hash<std::string>()(k.input_ifname) << 1);
} }
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment