CASS-1608 derive public ip, toggle ingressing public ips exclusively (#…

…946)
Netflix · Jun 1, 2021 · 05fccf2 · 05fccf2
1 parent ee2308a
commit 05fccf2
Show file tree

Hide file tree

Showing 8 changed files with 151 additions and 2 deletions.
diff --git a/priam/src/main/java/com/netflix/priam/aws/AWSIPConverter.java b/priam/src/main/java/com/netflix/priam/aws/AWSIPConverter.java
@@ -0,0 +1,58 @@
+package com.netflix.priam.aws;
+
+import com.google.common.base.Splitter;
+import com.netflix.priam.identity.PriamInstance;
+import com.netflix.priam.identity.config.InstanceInfo;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.util.List;
+import java.util.Optional;
+import java.util.regex.Pattern;
+import javax.inject.Inject;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class AWSIPConverter implements IPConverter {
+    private static final Logger logger = LoggerFactory.getLogger(AWSIPConverter.class);
+    private static final Pattern IP_PART = Pattern.compile("^[0-9]{1,3}$");
+
+    private final InstanceInfo myInstance;
+
+    @Inject
+    public AWSIPConverter(InstanceInfo myInstance) {
+        this.myInstance = myInstance;
+    }
+
+    @Override
+    public Optional<String> getPublicIP(PriamInstance instance) {
+        if (instance.getInstanceId().equals(myInstance.getInstanceId())) {
+            return Optional.ofNullable(myInstance.getHostIP());
+        }
+        Optional<String> ip = extractIPFromHostnameString(instance.getHostName());
+        return !instance.getDC().equals(myInstance.getRegion()) && !ip.isPresent()
+                ? deriveIPFromHostname(instance.getHostName())
+                : ip;
+    }
+
+    private Optional<String> extractIPFromHostnameString(String hostname) {
+        if (hostname.contains(".")) {
+            String ip = hostname.substring(4, hostname.indexOf('.')).replace('-', '.');
+            List<String> parts = Splitter.on(".").splitToList(ip);
+            return parts.size() == 4 && parts.stream().allMatch(p -> IP_PART.matcher(p).matches())
+                    ? Optional.of(ip)
+                    : Optional.empty();
+        }
+        return Optional.empty();
+    }
+
+    private Optional<String> deriveIPFromHostname(String hostname) {
+        String ip = null;
+        try {
+            ip = InetAddress.getByName(hostname).getHostAddress();
+            logger.info("Derived IP for " + hostname + ": " + ip);
+        } catch (UnknownHostException e) {
+            logger.warn("Could not resolve [" + hostname + "]");
+        }
+        return Optional.ofNullable(ip);
+    }
+}
diff --git a/priam/src/main/java/com/netflix/priam/aws/IPConverter.java b/priam/src/main/java/com/netflix/priam/aws/IPConverter.java
@@ -0,0 +1,11 @@
+package com.netflix.priam.aws;
+
+import com.google.inject.ImplementedBy;
+import com.netflix.priam.identity.PriamInstance;
+import java.util.Optional;
+
+/** Derives public ip from hostname. */
+@ImplementedBy(AWSIPConverter.class)
+public interface IPConverter {
+    Optional<String> getPublicIP(PriamInstance instance);
+}
diff --git a/priam/src/main/java/com/netflix/priam/aws/UpdateSecuritySettings.java b/priam/src/main/java/com/netflix/priam/aws/UpdateSecuritySettings.java
@@ -25,9 +25,12 @@
 import com.netflix.priam.identity.IMembership;
 import com.netflix.priam.identity.IPriamInstanceFactory;
 import com.netflix.priam.identity.InstanceIdentity;
+import com.netflix.priam.identity.PriamInstance;
 import com.netflix.priam.scheduler.SimpleTimer;
 import com.netflix.priam.scheduler.Task;
 import com.netflix.priam.scheduler.TaskTimer;
+import java.util.Objects;
+import java.util.Optional;
 import java.util.Random;
 import java.util.Set;
 import java.util.stream.Collectors;
@@ -53,15 +56,20 @@ public class UpdateSecuritySettings extends Task {
     private static final Random ran = new Random();
     private final IMembership membership;
     private final IPriamInstanceFactory factory;
+    private final IPConverter ipConverter;
 
     @Inject
     // Note: do not parameterized the generic type variable to an implementation as it confuses
     // Guice in the binding.
     public UpdateSecuritySettings(
-            IConfiguration config, IMembership membership, IPriamInstanceFactory factory) {
+            IConfiguration config,
+            IMembership membership,
+            IPriamInstanceFactory factory,
+            IPConverter ipConverter) {
         super(config);
         this.membership = membership;
         this.factory = factory;
+        this.ipConverter = ipConverter;
     }
 
     /**
@@ -75,7 +83,9 @@ public void execute() {
         Set<String> desiredAcl =
                 factory.getAllIds(config.getAppName())
                         .stream()
-                        .map(i -> i.getHostIP() + "/32")
+                        .map(i -> getIngressRule(i).orElse(null))
+                        .filter(Objects::nonNull)
+                        .map(ip -> ip + "/32")
                         .collect(Collectors.toSet());
         if (!config.skipDeletingOthersIngressRules()) {
             Set<String> aclToRemove = Sets.difference(currentAcl, desiredAcl);
@@ -99,6 +109,12 @@ public void execute() {
         }
     }
 
+    private Optional<String> getIngressRule(PriamInstance instance) {
+        return config.skipIngressUnlessIPIsPublic()
+                ? ipConverter.getPublicIP(instance)
+                : Optional.of(instance.getHostIP());
+    }
+
     public static TaskTimer getTimer(InstanceIdentity id) {
         SimpleTimer return_;
         if (id.isSeed()) {

diff --git a/priam/src/main/java/com/netflix/priam/config/IConfiguration.java b/priam/src/main/java/com/netflix/priam/config/IConfiguration.java
@@ -1132,6 +1132,14 @@ default BackupsToCompress getBackupsToCompress() {
         return BackupsToCompress.ALL;
     }
 
+    /*
+     * @return true if Priam should skip ingress on an IP address from the token database unless it
+     *     can confirm that it is public
+     */
+    default boolean skipIngressUnlessIPIsPublic() {
+        return false;
+    }
+
     /**
      * Escape hatch for getting any arbitrary property by key This is useful so we don't have to
      * keep adding methods to this interface for every single configuration option ever. Also

diff --git a/priam/src/test/java/com/netflix/priam/TestModule.java b/priam/src/test/java/com/netflix/priam/TestModule.java
@@ -20,6 +20,8 @@
 import com.google.common.collect.ImmutableList;
 import com.google.inject.AbstractModule;
 import com.google.inject.Scopes;
+import com.netflix.priam.aws.FakeIPConverter;
+import com.netflix.priam.aws.IPConverter;
 import com.netflix.priam.backup.FakeCredentials;
 import com.netflix.priam.backup.IBackupFileSystem;
 import com.netflix.priam.backup.NullBackupFileSystem;
@@ -63,5 +65,6 @@ protected void configure() {
         bind(IBackupFileSystem.class).to(NullBackupFileSystem.class);
         bind(Sleeper.class).to(FakeSleeper.class);
         bind(Registry.class).toInstance(new DefaultRegistry());
+        bind(IPConverter.class).toInstance(new FakeIPConverter());
     }
 }
diff --git a/priam/src/test/java/com/netflix/priam/aws/FakeIPConverter.java b/priam/src/test/java/com/netflix/priam/aws/FakeIPConverter.java
@@ -0,0 +1,19 @@
+package com.netflix.priam.aws;
+
+import com.netflix.priam.identity.PriamInstance;
+import groovy.lang.Singleton;
+import java.util.Optional;
+
+@Singleton
+public class FakeIPConverter implements IPConverter {
+    private boolean shouldFail;
+
+    public void setShouldFail(boolean shouldFail) {
+        this.shouldFail = shouldFail;
+    }
+
+    @Override
+    public Optional<String> getPublicIP(PriamInstance instance) {
+        return shouldFail ? Optional.empty() : Optional.of("1.1.1.1");
+    }
+}
diff --git a/priam/src/test/java/com/netflix/priam/aws/TestUpdateSecuritySettings.java b/priam/src/test/java/com/netflix/priam/aws/TestUpdateSecuritySettings.java
@@ -20,6 +20,7 @@ public class TestUpdateSecuritySettings {
     private IMembership membership;
     private IPriamInstanceFactory factory;
     private FakeConfiguration config;
+    private FakeIPConverter ipConverter;
 
     @Before
     public void setUp() {
@@ -28,6 +29,7 @@ public void setUp() {
         membership = injector.getInstance(IMembership.class);
         updateSecuritySettings = injector.getInstance(UpdateSecuritySettings.class);
         config = (FakeConfiguration) injector.getInstance(IConfiguration.class);
+        ipConverter = (FakeIPConverter) injector.getInstance(IPConverter.class);
     }
 
     @Test
@@ -82,6 +84,28 @@ public void dontUpdateOthersIngress() {
         Truth.assertThat(membership.listACL(PORT, PORT)).isEmpty();
     }
 
+    @Test
+    public void dontUpdatePrivateIngress() {
+        addToFactory(1, "2.2.2.2");
+        ipConverter.setShouldFail(false);
+        config.setSkipIngressUnlessIPIsPublic(true);
+        Truth.assertThat(membership.listACL(PORT, PORT)).isEmpty();
+        updateSecuritySettings.execute();
+        // The private IP produced by our FakeIPConverter
+        Truth.assertThat(membership.listACL(PORT, PORT)).contains("1.1.1.1/32");
+        Truth.assertThat(membership.listACL(PORT, PORT)).doesNotContain("2.2.2.2/32");
+    }
+
+    @Test
+    public void dontUpdateUnknownIngress() {
+        addToFactory(1, "1.1.1.1");
+        ipConverter.setShouldFail(true);
+        config.setSkipIngressUnlessIPIsPublic(true);
+        Truth.assertThat(membership.listACL(PORT, PORT)).isEmpty();
+        updateSecuritySettings.execute();
+        Truth.assertThat(membership.listACL(PORT, PORT)).isEmpty();
+    }
+
     private void addToFactory(int id, String ip) {
         factory.create("myApp", id, "i-1", "hostname", ip, "us-east-1a", null /* volumes */, "123");
     }

diff --git a/priam/src/test/java/com/netflix/priam/config/FakeConfiguration.java b/priam/src/test/java/com/netflix/priam/config/FakeConfiguration.java
@@ -38,6 +38,7 @@ public class FakeConfiguration implements IConfiguration {
     private boolean checkThriftIsListening;
     private boolean skipDeletingOthersIngressRules;
     private boolean skipUpdatingOthersIngressRules;
+    private boolean skipIngressUnlessIPIsPublic;
 
     public final Map<String, String> fakeProperties = new HashMap<>();
 
@@ -269,4 +270,13 @@ public BackupsToCompress getBackupsToCompress() {
         return (BackupsToCompress)
                 fakeConfig.getOrDefault("Priam.backupsToCompress", BackupsToCompress.ALL);
     }
+
+    @Override
+    public boolean skipIngressUnlessIPIsPublic() {
+        return this.skipIngressUnlessIPIsPublic;
+    }
+
+    public void setSkipIngressUnlessIPIsPublic(boolean skipIngressUnlessIPIsPublic) {
+        this.skipIngressUnlessIPIsPublic = skipIngressUnlessIPIsPublic;
+    }
 }