Skip to content
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.

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

Feat network steps #173

Open
wants to merge 24 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 16 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -194,11 +194,6 @@
<version>${auto.service.version}</version>
<scope>provided</scope>
</dependency>
<dependency>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was this dependency removed?

<groupId>com.google.inject</groupId>
<artifactId>guice</artifactId>
<version>${guice.version}</version>
</dependency>
<dependency>
<groupId>org.immutables</groupId>
<artifactId>value</artifactId>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/

package com.aws.greengrass.testing.features;

import com.aws.greengrass.testing.platform.Platform;
import io.cucumber.guice.ScenarioScoped;
import io.cucumber.java.After;
import io.cucumber.java.en.When;

import java.io.IOException;
import javax.inject.Inject;

@ScenarioScoped
public class ConnectivitySteps {
private final Platform platform;
private boolean offline = false;

@Inject
@SuppressWarnings("MissingJavadocMethod")
public ConnectivitySteps(Platform platform) {
this.platform = platform;
}

/**
* Blocks the traffic port (IP ports) on ports 443,8888,8889 and when the connectivity parameter is "offline" and.
jaiszeen marked this conversation as resolved.
Show resolved Hide resolved
* re-enables traffic on the ports when it is "online".
*
* @param connectivity desired connectivity status ("offline", "online")
* @throws IOException {throws IOException}
* @throws InterruptedException {throws IInterruptedException}
* @throws UnsupportedOperationException {throws UnsupportedOperationException}
*/
@When("the device network connectivity is {word}")
public void setDeviceNetwork(final String connectivity) throws IOException, InterruptedException {
switch (connectivity) {
Copy link
Member

Choose a reason for hiding this comment

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

the device network connectivity is OFFLINE is considered as a wrong input here. Do we want that?

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe so, there are only 2 branches

case "offline":
platform.getNetworkUtils().disconnectNetwork();
break;
case "online":
platform.getNetworkUtils().recoverNetwork();
break;
default:
throw new UnsupportedOperationException("Connectivity " + connectivity + " is not supported");
}

offline = connectivity.equalsIgnoreCase("offline");
Copy link
Member

Choose a reason for hiding this comment

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

In general, this can throw null pointer exception if connectivity is null . Just reverse the check here - "offline".equalsIgnoreCase(connectivity). Also, this can be moved to L40 where you can directly update offline to true

Copy link
Contributor

Choose a reason for hiding this comment

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

Great call out. I like the reversing of order to avoid the NPE. I would suggest not moving it to L40. There are scenarios where the client might go offline, then online and we want to flip the flag to avoid triggering the recover step on teardown

}

@After
private void teardown() throws IOException, InterruptedException {
if (offline) {
platform.getNetworkUtils().recoverNetwork();
}
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -397,4 +397,5 @@ void checkADeploymentReachesCompleted(GreengrassV2Lifecycle ggv2, String deploym
Thread.currentThread().interrupt();
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,15 @@
<version>2.13.0</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>software.amazon.awssdk</groupId>
<artifactId>utils</artifactId>
</dependency>
<dependency>
<groupId>com.google.inject</groupId>
Copy link
Contributor

Choose a reason for hiding this comment

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

This dependency is not needed here

<artifactId>guice</artifactId>
<version>${guice.version}</version>
</dependency>
</dependencies>

</project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/

package com.aws.greengrass.testing.platform;

import java.io.IOException;
import java.io.UnsupportedEncodingException;
import java.util.ArrayList;
import java.util.List;

public abstract class NetworkUtils {
protected static final String[] MQTT_PORTS = {"8883", "443"};
// 8888 and 8889 are used by the squid proxy which runs on a remote DUT
// and need to disable access to test offline proxy scenarios
protected static final String[] NETWORK_PORTS = {"443", "8888", "8889"};
protected static final int[] GG_UPSTREAM_PORTS = {8883, 8443, 443};
protected static final int SSH_PORT = 22;
protected final List<Integer> blockedPorts = new ArrayList<>();

public abstract void disconnectNetwork() throws InterruptedException, IOException;

public abstract void recoverNetwork() throws InterruptedException, IOException;

}
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,10 @@ public interface Platform {
Commands commands();

PlatformFiles files();

default NetworkUtils getNetworkUtils() {
throw new UnsupportedOperationException("Not yet implemented");
}

}

Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,24 @@
import com.aws.greengrass.testing.api.device.Device;
import com.aws.greengrass.testing.api.model.PillboxContext;
import com.aws.greengrass.testing.platform.AbstractPlatform;
import com.aws.greengrass.testing.platform.NetworkUtils;

public class LinuxPlatform extends AbstractPlatform {

private final NetworkUtilsLinux networkUtilsLinux;

public LinuxPlatform(final Device device, final PillboxContext pillboxContext) {
super(device, pillboxContext);
networkUtilsLinux = new NetworkUtilsLinux();
}

@Override
public LinuxCommands commands() {
return new LinuxCommands(device, pillboxContext);
}

@Override
public NetworkUtils getNetworkUtils() {
return this.networkUtilsLinux;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/

package com.aws.greengrass.testing.platform.linux;

import com.aws.greengrass.testing.platform.NetworkUtils;
import software.amazon.awssdk.utils.IoUtils;

import java.io.IOException;
import java.net.NetworkInterface;
import java.nio.charset.StandardCharsets;
import java.util.Collections;
import java.util.Enumeration;
import java.util.List;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.stream.Collectors;
import java.util.stream.Stream;


public class NetworkUtilsLinux extends NetworkUtils {
private static final String ENABLE_OPTION = "--insert";
private static final String DISABLE_OPTION = "--delete";
private static final String APPEND_OPTION = "-A";
private static final String IPTABLE_COMMAND_BLOCK_INGRESS_STR =
"sudo iptables %s INPUT -p tcp --sport %s -j REJECT";
private static final String IPTABLE_COMMAND_STR = "sudo iptables %s OUTPUT -p tcp --dport %s -j REJECT && "
+ "sudo iptables %s INPUT -p tcp --sport %s -j REJECT";
private static final String IPTABLES_DROP_DPORT_EXTERNAL_ONLY_COMMAND_STR =
"sudo iptables %s INPUT -p tcp -s localhost --dport %s -j ACCEPT && "
+
"sudo iptables %s INPUT -p tcp --dport %s -j DROP && "
+
"sudo iptables %s OUTPUT -p tcp -d localhost --dport %s -j ACCEPT && "
+
"sudo iptables %s OUTPUT -p tcp --dport %s -j DROP";
private static final String IPTABLE_SAFELIST_COMMAND_STR
= "sudo iptables %s OUTPUT -p tcp -d %s --dport %d -j ACCEPT && "
+
"sudo iptables %s INPUT -p tcp -s %s --sport %d -j ACCEPT";
private static final String GET_IPTABLES_RULES = "sudo iptables -S";

// The string we are looking for to verify that there is an iptables rule to reject a port
// We only need to look for sport because sport only gets created if dport is successful
private static final String IPTABLES_RULE = "-m tcp --sport %s -j REJECT";

private static final AtomicBoolean bandwidthSetup = new AtomicBoolean(false);


private void modifyMqttConnection(String action) throws IOException, InterruptedException {
for (String port : MQTT_PORTS) {
new ProcessBuilder().command(
"sh", "-c", String.format(IPTABLES_DROP_DPORT_EXTERNAL_ONLY_COMMAND_STR,
action, port, action, port, action, port, action, port)
).start().waitFor(2, TimeUnit.SECONDS);
}
}

jaiszeen marked this conversation as resolved.
Show resolved Hide resolved
private void filterPortOnInterface(String iface, int port) throws IOException, InterruptedException {
// Filtering SSH traffic impacts test execution, so we explicitly disallow it
if (port == SSH_PORT) {
return;
}
List<String> filterSourcePortCommand = Stream.of("sudo", "tc", "filter", "add", "dev",
iface, "parent", "1:", "protocol", "ip", "prio", "1", "u32", "match",
"ip", "sport", Integer.toString(port), "0xffff", "flowid", "1:2").collect(Collectors.toList());
executeCommand(filterSourcePortCommand);

List<String> filterDestPortCommand = Stream.of("sudo", "tc", "filter", "add", "dev", iface,
"parent", "1:", "protocol", "ip", "prio", "1", "u32", "match",
"ip", "dport", Integer.toString(port), "0xffff", "flowid", "1:2").collect(Collectors.toList());
executeCommand(filterDestPortCommand);
}

private void deleteRootNetemQdiscOnInterface() throws InterruptedException, IOException {
Enumeration<NetworkInterface> nets = NetworkInterface.getNetworkInterfaces();
for (NetworkInterface netint : Collections.list(nets)) {
if (netint.isPointToPoint() || netint.isLoopback()) {
continue;
}
executeCommand(Stream.of("sudo", "tc", "qdisc", "del", "dev", netint.getName(), "root")
.collect(Collectors.toList()));
}
}

private void createRootNetemQdiscOnInterface(String iface, int netemRateKbps)
throws InterruptedException, IOException {
// TODO: Add support for setting packet loss and delay
int netemDelayMs = 750;
List<String> addQdiscCommand = Stream.of("sudo", "tc", "qdisc", "add", "dev", iface, "root", "handle",
"1:", "prio", "bands", "2", "priomap", "0", "0", "0", "0", "0", "0", "0", "0", "0", "0", "0",
"0", "0", "0", "0", "0").collect(Collectors.toList());
executeCommand(addQdiscCommand);

List<String> netemCommand =
Stream.of("sudo", "tc", "qdisc", "add", "dev", iface, "parent", "1:2", "netem", "delay",
String.format("%dms", netemDelayMs), "rate", String.format("%dkbit", netemRateKbps))
.collect(Collectors.toList());
executeCommand(netemCommand);
}

private String executeCommand(List<String> command) throws IOException, InterruptedException {
Process proc = new ProcessBuilder().command(command).start();
proc.waitFor(2, TimeUnit.SECONDS);
if (proc.exitValue() != 0) {
throw new IOException("CLI command " + command + " failed with error "
+ new String(IoUtils.toByteArray(proc.getErrorStream()), StandardCharsets.UTF_8));
}
return new String(IoUtils.toByteArray(proc.getInputStream()), StandardCharsets.UTF_8);
}

@Override
public void disconnectNetwork() throws InterruptedException, IOException {
interfacepolicy(IPTABLE_COMMAND_STR, ENABLE_OPTION, "connection-loss", NETWORK_PORTS);
}

@Override
public void recoverNetwork() throws InterruptedException, IOException {
interfacepolicy(IPTABLE_COMMAND_STR, DISABLE_OPTION, "connection-recover", NETWORK_PORTS);

if (bandwidthSetup.get()) {
Nelsonochoam marked this conversation as resolved.
Show resolved Hide resolved
deleteRootNetemQdiscOnInterface();
bandwidthSetup.set(false);
}
}

private void interfacepolicy(String iptableCommandString, String option, String eventName, String... ports)
throws InterruptedException, IOException {
for (String port : ports) {
new ProcessBuilder().command("sh", "-c", String.format(iptableCommandString, option, port, option, port))
.start().waitFor(2, TimeUnit.SECONDS);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@
<dependency>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
<version>31.1-jre</version>
<version>23.0</version>
Copy link
Member

Choose a reason for hiding this comment

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

Why is this updated?

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

Copy link
Author

Choose a reason for hiding this comment

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

resolved

Copy link
Contributor

Choose a reason for hiding this comment

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

Mmm I don't know why this shows up as changed but we have to make sure this has the previous value. We don't want to update this dependencies if it is not needed.

</dependency>
<dependency>
<groupId>org.immutables</groupId>
Expand Down