Properly set configserver client SSL options in Environment.Builder

7 files changed, 37 insertions, 59 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/component/Environment.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/component/Environment.java
index c66d27f75c8..da7be1bff6d 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/component/Environment.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/component/Environment.java
@@ -90,21 +90,7 @@ public class Environment {
                 pathResolver,
                 getLogstashNodesFromEnvironment(),
                 coreDumpFeedEndpoint,
-
-                createKeyStoreOptions(
-                        configServerConfig.keyStoreConfig().path(),
-                        configServerConfig.keyStoreConfig().password().toCharArray(),
-                        configServerConfig.keyStoreConfig().type().name(),
-                        "BC"),
-                createKeyStoreOptions(
-                        configServerConfig.trustStoreConfig().path(),
-                        configServerConfig.trustStoreConfig().password().toCharArray(),
-                        configServerConfig.trustStoreConfig().type().name(),
-                        null),
-                createAthenzIdentity(
-                        configServerConfig.athenzDomain(),
-                        configServerConfig.serviceName()),
-             nodeType
+                nodeType
         );
     }
 
@@ -117,15 +103,26 @@ public class Environment {
                        PathResolver pathResolver,
                        List<String> logstashNodes,
                        Optional<String> feedEndpoint,
-                       Optional<KeyStoreOptions> keyStoreOptions,
-                       Optional<KeyStoreOptions> trustStoreOptions,
-                       Optional<AthenzIdentity> athenzIdentity,
                        NodeType nodeType) {
         this.configServerHostNames = configServerConfig.hosts();
         this.configServerURIs = createConfigServerUris(
                 configServerConfig.scheme(),
                 configServerConfig.hosts(),
                 configServerConfig.port());
+        this.keyStoreOptions = createKeyStoreOptions(
+                configServerConfig.keyStoreConfig().path(),
+                configServerConfig.keyStoreConfig().password().toCharArray(),
+                configServerConfig.keyStoreConfig().type().name(),
+                "BC");
+        this.trustStoreOptions = createKeyStoreOptions(
+                configServerConfig.trustStoreConfig().path(),
+                configServerConfig.trustStoreConfig().password().toCharArray(),
+                configServerConfig.trustStoreConfig().type().name(),
+                null);
+        this.athenzIdentity = createAthenzIdentity(
+                configServerConfig.athenzDomain(),
+                configServerConfig.serviceName());
+
         this.environment = environment;
         this.region = region;
         this.system = system;
@@ -134,9 +131,6 @@ public class Environment {
         this.pathResolver = pathResolver;
         this.logstashNodes = logstashNodes;
         this.feedEndpoint = feedEndpoint;
-        this.keyStoreOptions = keyStoreOptions;
-        this.trustStoreOptions = trustStoreOptions;
-        this.athenzIdentity = athenzIdentity;
         this.nodeType = nodeType;
     }
 
@@ -279,7 +273,7 @@ public class Environment {
     public NodeType getNodeType() { return nodeType; }
 
     public static class Builder {
-        ConfigServerConfig configServerConfig = new ConfigServerConfig(new ConfigServerConfig.Builder());
+        private ConfigServerConfig configServerConfig;
         private String environment;
         private String region;
         private String system;
@@ -288,9 +282,6 @@ public class Environment {
         private PathResolver pathResolver;
         private List<String> logstashNodes = Collections.emptyList();
         private Optional<String> feedEndpoint = Optional.empty();
-        private KeyStoreOptions keyStoreOptions;
-        private KeyStoreOptions trustStoreOptions;
-        private AthenzIdentity athenzIdentity;
         private NodeType nodeType = NodeType.tenant;
 
         public Builder configServerConfig(ConfigServerConfig configServerConfig) {
@@ -338,30 +329,17 @@ public class Environment {
             return this;
         }
 
-        public Builder keyStoreOptions(KeyStoreOptions keyStoreOptions) {
-            this.keyStoreOptions = keyStoreOptions;
-            return this;
-        }
-
-        public Builder trustStoreOptions(KeyStoreOptions trustStoreOptions) {
-            this.trustStoreOptions = trustStoreOptions;
-            return this;
-        }
-
-        public Builder athenzIdentity(AthenzIdentity athenzIdentity) {
-            this.athenzIdentity = athenzIdentity;
-            return this;
-        }
-
         public Builder nodeType(NodeType nodeType) {
             this.nodeType = nodeType;
             return this;
         }
 
         public Environment build() {
+            Objects.requireNonNull(configServerConfig, "configServerConfig cannot be null");
             Objects.requireNonNull(environment, "environment cannot be null");
             Objects.requireNonNull(region, "region cannot be null");
             Objects.requireNonNull(system, "system cannot be null");
+
             return new Environment(configServerConfig,
                                    environment,
                                    region,
@@ -371,9 +349,6 @@ public class Environment {
                                    Optional.ofNullable(pathResolver).orElseGet(PathResolver::new),
                                    logstashNodes,
                                    feedEndpoint,
-                                   Optional.ofNullable(keyStoreOptions),
-                                   Optional.ofNullable(trustStoreOptions),
-                                   Optional.ofNullable(athenzIdentity),
                                    nodeType);
         }
     }
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImplTest.java
index a07bd4e0e91..b0fc877bd88 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImplTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImplTest.java
@@ -9,6 +9,7 @@ import com.yahoo.vespa.hosted.dockerapi.Docker;
 import com.yahoo.vespa.hosted.dockerapi.DockerImage;
 import com.yahoo.vespa.hosted.dockerapi.ProcessResult;
 import com.yahoo.vespa.hosted.node.admin.component.Environment;
+import com.yahoo.vespa.hosted.node.admin.config.ConfigServerConfig;
 import com.yahoo.vespa.hosted.node.admin.task.util.network.IPAddressesMock;
 import org.junit.Test;
 import org.mockito.InOrder;
@@ -29,6 +30,7 @@ import static org.mockito.Mockito.when;
 
 public class DockerOperationsImplTest {
     private final Environment environment = new Environment.Builder()
+            .configServerConfig(new ConfigServerConfig(new ConfigServerConfig.Builder()))
             .region("us-east-1")
             .environment("prod")
             .system("main")
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java
index ba176305173..7923a58ec47 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java
@@ -6,6 +6,7 @@ import com.yahoo.metrics.simple.MetricReceiver;
 import com.yahoo.vespa.hosted.dockerapi.Docker;
 import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper;
 import com.yahoo.vespa.hosted.node.admin.ContainerNodeSpec;
+import com.yahoo.vespa.hosted.node.admin.config.ConfigServerConfig;
 import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations;
 import com.yahoo.vespa.hosted.node.admin.docker.DockerOperationsImpl;
 import com.yahoo.vespa.hosted.node.admin.maintenance.acl.AclMaintainer;
@@ -59,6 +60,7 @@ public class DockerTester implements AutoCloseable {
         }
 
         Environment environment = new Environment.Builder()
+                .configServerConfig(new ConfigServerConfig(new ConfigServerConfig.Builder()))
                 .inetAddressResolver(inetAddressResolver)
                 .region("us-east-1")
                 .environment("prod")
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/logging/FilebeatConfigProviderTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/logging/FilebeatConfigProviderTest.java
index 0c24abe8b69..9de88cbb8b3 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/logging/FilebeatConfigProviderTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/logging/FilebeatConfigProviderTest.java
@@ -4,9 +4,11 @@ package com.yahoo.vespa.hosted.node.admin.logging;
 import com.google.common.collect.ImmutableList;
 import com.yahoo.vespa.hosted.node.admin.ContainerNodeSpec;
 import com.yahoo.vespa.hosted.node.admin.component.Environment;
+import com.yahoo.vespa.hosted.node.admin.config.ConfigServerConfig;
 import com.yahoo.vespa.hosted.provision.Node;
 import org.junit.Test;
 
+import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 
@@ -19,7 +21,6 @@ import static org.junit.Assert.*;
  */
 public class FilebeatConfigProviderTest {
 
-
     private static final String tenant = "vespa";
     private static final String application = "music";
     private static final String instance = "default";
@@ -30,7 +31,7 @@ public class FilebeatConfigProviderTest {
 
     @Test
     public void it_replaces_all_fields_correctly() {
-        FilebeatConfigProvider filebeatConfigProvider = new FilebeatConfigProvider(getEnvironment());
+        FilebeatConfigProvider filebeatConfigProvider = new FilebeatConfigProvider(getEnvironment(logstashNodes));
 
         Optional<String> config = filebeatConfigProvider.getConfig(getNodeSpec(tenant, application, instance));
 
@@ -41,11 +42,7 @@ public class FilebeatConfigProviderTest {
 
     @Test
     public void it_does_not_generate_config_when_no_logstash_nodes() {
-        Environment env = new Environment.Builder()
-                .environment(environment)
-                .region(region)
-                .system(system)
-                .build();
+        Environment env = getEnvironment(Collections.emptyList());
 
         FilebeatConfigProvider filebeatConfigProvider = new FilebeatConfigProvider(env);
         Optional<String> config = filebeatConfigProvider.getConfig(getNodeSpec(tenant, application, instance));
@@ -54,7 +51,7 @@ public class FilebeatConfigProviderTest {
 
     @Test
     public void it_does_not_generate_config_for_nodes_wihout_owner() {
-        FilebeatConfigProvider filebeatConfigProvider = new FilebeatConfigProvider(getEnvironment());
+        FilebeatConfigProvider filebeatConfigProvider = new FilebeatConfigProvider(getEnvironment(logstashNodes));
         ContainerNodeSpec nodeSpec = new ContainerNodeSpec.Builder()
                 .nodeFlavor("flavor")
                 .nodeState(Node.State.active)
@@ -80,12 +77,7 @@ public class FilebeatConfigProviderTest {
 
     @Test
     public void it_does_not_add_double_quotes() {
-        Environment environment = new Environment.Builder()
-                .environment(FilebeatConfigProviderTest.environment)
-                .region(region)
-                .system(system)
-                .logstashNodes(ImmutableList.of("unquoted", "\"quoted\""))
-                .build();
+        Environment environment = getEnvironment(ImmutableList.of("unquoted", "\"quoted\""));
         FilebeatConfigProvider filebeatConfigProvider = new FilebeatConfigProvider(environment);
         Optional<String> config = filebeatConfigProvider.getConfig(getNodeSpec(tenant, application, instance));
         assertThat(config.get(), containsString("hosts: [\"unquoted\",\"quoted\"]"));
@@ -98,13 +90,14 @@ public class FilebeatConfigProviderTest {
     }
 
     private String getConfigString() {
-        FilebeatConfigProvider filebeatConfigProvider = new FilebeatConfigProvider(getEnvironment());
+        FilebeatConfigProvider filebeatConfigProvider = new FilebeatConfigProvider(getEnvironment(logstashNodes));
         ContainerNodeSpec nodeSpec = getNodeSpec(tenant, application, instance);
         return filebeatConfigProvider.getConfig(nodeSpec).orElseThrow(() -> new RuntimeException("Failed to get filebeat config"));
     }
 
-    private Environment getEnvironment() {
+    private Environment getEnvironment(List<String> logstashNodes) {
         return new Environment.Builder()
+                .configServerConfig(new ConfigServerConfig(new ConfigServerConfig.Builder()))
                 .environment(environment)
                 .region(region)
                 .system(system)
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java
index 9c21d7c92f2..cb7c22f1da9 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java
@@ -8,6 +8,7 @@ import com.yahoo.test.ManualClock;
 import com.yahoo.vespa.hosted.dockerapi.ContainerName;
 import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper;
 import com.yahoo.vespa.hosted.node.admin.ContainerNodeSpec;
+import com.yahoo.vespa.hosted.node.admin.config.ConfigServerConfig;
 import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations;
 import com.yahoo.vespa.hosted.node.admin.component.Environment;
 import com.yahoo.vespa.hosted.node.admin.component.PathResolver;
@@ -34,6 +35,7 @@ import static org.mockito.Mockito.when;
 public class StorageMaintainerTest {
     private final ManualClock clock = new ManualClock();
     private final Environment environment = new Environment.Builder()
+            .configServerConfig(new ConfigServerConfig(new ConfigServerConfig.Builder()))
             .region("us-east-1")
             .environment("prod")
             .system("main")
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java
index 8c6657c147a..6a93d4e3664 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java
@@ -12,6 +12,7 @@ import com.yahoo.vespa.hosted.dockerapi.Docker;
 import com.yahoo.vespa.hosted.dockerapi.DockerImage;
 import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper;
 import com.yahoo.vespa.hosted.node.admin.ContainerNodeSpec;
+import com.yahoo.vespa.hosted.node.admin.config.ConfigServerConfig;
 import com.yahoo.vespa.hosted.node.admin.containerdata.ConfigServerContainerData;
 import com.yahoo.vespa.hosted.node.admin.containerdata.ContainerData;
 import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations;
@@ -81,6 +82,7 @@ public class NodeAgentImplTest {
     private final PathResolver pathResolver = mock(PathResolver.class);
     private final ManualClock clock = new ManualClock();
     private final Environment environment = new Environment.Builder()
+            .configServerConfig(new ConfigServerConfig(new ConfigServerConfig.Builder()))
             .environment("dev")
             .region("us-east-1")
             .system("main")
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/util/EnvironmentTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/util/EnvironmentTest.java
index 4208388f2bd..80749e5d34b 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/util/EnvironmentTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/util/EnvironmentTest.java
@@ -5,6 +5,7 @@ import static com.yahoo.vespa.defaults.Defaults.getDefaults;
 import com.yahoo.vespa.hosted.dockerapi.ContainerName;
 import com.yahoo.vespa.hosted.node.admin.component.Environment;
 import com.yahoo.vespa.hosted.node.admin.component.PathResolver;
+import com.yahoo.vespa.hosted.node.admin.config.ConfigServerConfig;
 import org.junit.Test;
 
 import java.nio.file.Path;
@@ -17,6 +18,7 @@ import static org.junit.Assert.assertEquals;
  */
 public class EnvironmentTest {
     private final Environment environment = new Environment.Builder()
+            .configServerConfig(new ConfigServerConfig(new ConfigServerConfig.Builder()))
             .region("us-east-1")
             .environment("prod")
             .system("main")