pass environment variables securily in config

8 files changed, 49 insertions, 28 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/AbstractService.java b/config-model/src/main/java/com/yahoo/vespa/model/AbstractService.java
index 6777e2fb741..1548f6ea728 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/AbstractService.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/AbstractService.java
@@ -409,11 +409,17 @@ public abstract class AbstractService extends AbstractConfigProducer<AbstractCon
         int pos = nameAndValue.indexOf('=');
         environmentVariables.put(nameAndValue.substring(0, pos), nameAndValue.substring(pos+1));
     }
+
     public void addEnvironmentVariable(String name, Object value) {
         environmentVariables.put(name, value);
     }
 
-    public String getEnv() {
+    @Override
+    public Map<String, Object> getEnvVars() {
+        return Map.copyOf(environmentVariables);
+    }
+
+    public String getEnvStringForTesting() {
         return environmentVariables.entrySet().stream().map(e -> e.getKey() + '=' + toEnvValue(e.getValue())).collect(Collectors.joining(" "));
     }
 
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/ConfigSentinel.java b/config-model/src/main/java/com/yahoo/vespa/model/ConfigSentinel.java
index 217f6cff778..d3fad2d94d1 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/ConfigSentinel.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/ConfigSentinel.java
@@ -99,6 +99,9 @@ public class ConfigSentinel extends AbstractService implements SentinelConfig.Pr
         serviceBuilder.name(s.getServiceName());
         serviceBuilder.id(s.getConfigId());
         serviceBuilder.affinity(getServiceAffinity(s));
+        for (var entry : s.getEnvVars().entrySet()) {
+            serviceBuilder.environ(b -> b.varname(entry.getKey()).varvalue(entry.getValue().toString()));
+        }
         setPreShutdownCommand(serviceBuilder, s);
         return serviceBuilder;
     }
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/Service.java b/config-model/src/main/java/com/yahoo/vespa/model/Service.java
index 3849a57db6f..87fd8078c3f 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/Service.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/Service.java
@@ -4,6 +4,7 @@ package com.yahoo.vespa.model;
 import com.yahoo.config.model.api.ServiceInfo;
 
 import java.util.HashMap;
+import java.util.Map;
 import java.util.Optional;
 
 /**
@@ -21,6 +22,9 @@ public interface Service extends ConfigProducer, NetworkPortRequestor {
      */
     String getStartupCommand();
 
+    // environment variables specific for this service:
+    Map<String, Object> getEnvVars();
+
     /**
      * Services that wish that a command should be run before shutdown
      * should return the command here. The command will be executed
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/search/SearchNode.java b/config-model/src/main/java/com/yahoo/vespa/model/search/SearchNode.java
index 3d4e0bd22d4..40e57e2f962 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/search/SearchNode.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/search/SearchNode.java
@@ -109,10 +109,10 @@ public class SearchNode extends AbstractService implements
         SearchNode node = new SearchNode(parent, name, distributionKey, nodeSpec, clusterName, serviceLayerService, flushOnShutdown,
                               tuning, resourceLimits, isHostedVespa, fractionOfMemoryReserved);
         if (featureFlags.loadCodeAsHugePages()) {
-            node.addEnvironmentVariable("VESPA_LOAD_CODE_AS_HUGEPAGES", "true");
+            node.addEnvironmentVariable("VESPA_LOAD_CODE_AS_HUGEPAGES", true);
         }
-        if ( featureFlags.sharedStringRepoNoReclaim()) {
-            node.addEnvironmentVariable("VESPA_SHARED_STRING_REPO_NO_RECLAIM", "true");
+        if (featureFlags.sharedStringRepoNoReclaim()) {
+            node.addEnvironmentVariable("VESPA_SHARED_STRING_REPO_NO_RECLAIM", true);
         }
         return node;
     }
@@ -240,7 +240,7 @@ public class SearchNode extends AbstractService implements
 
     @Override
     public String getStartupCommand() {
-        String startup = getEnv() + " exec $ROOT/sbin/vespa-proton " + "--identity " + getConfigId();
+        String startup = "exec $ROOT/sbin/vespa-proton --identity " + getConfigId();
         if (serviceLayerService != null) {
             startup = startup + " --serviceidentity " + serviceLayerService.getConfigId();
         }
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/builder/xml/dom/ContentBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/builder/xml/dom/ContentBuilderTest.java
index 0848b8becb9..6ac4dbd17e3 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/builder/xml/dom/ContentBuilderTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/builder/xml/dom/ContentBuilderTest.java
@@ -307,8 +307,8 @@ public class ContentBuilderTest extends DomBuilderTest {
         assertEquals(200000, b.getRootGroup().getMmapNoCoreLimit().get().longValue());
 
         assertEquals(2, s.getSearchNodes().size());
-        assertTrue(s.getSearchNodes().get(0).getEnv().contains("VESPA_MMAP_NOCORE_LIMIT=200000"));
-        assertTrue(s.getSearchNodes().get(1).getEnv().contains("VESPA_MMAP_NOCORE_LIMIT=200000"));
+        assertTrue(s.getSearchNodes().get(0).getEnvStringForTesting().contains("VESPA_MMAP_NOCORE_LIMIT=200000"));
+        assertTrue(s.getSearchNodes().get(1).getEnvStringForTesting().contains("VESPA_MMAP_NOCORE_LIMIT=200000"));
     }
 
     @Test
@@ -334,12 +334,12 @@ public class ContentBuilderTest extends DomBuilderTest {
         AbstractService node = s.getSearchNodes().get(0);
         node.addEnvironmentVariable("MY_ENV_1", 7);
         node.addEnvironmentVariable("MY_ENV_2", "7 8");
-        assertTrue(node.getEnv().contains("MY_ENV_1=7"));
-        assertTrue(node.getEnv().contains("MY_ENV_2=\"7 8\""));
+        assertTrue(node.getEnvStringForTesting().contains("MY_ENV_1=7"));
+        assertTrue(node.getEnvStringForTesting().contains("MY_ENV_2=\"7 8\""));
         node.addEnvironmentVariable("MY_PARSED_ENV_1=7");
         node.addEnvironmentVariable("MY_PARSED_ENV_2=7 8");
-        assertTrue(node.getEnv().contains("MY_PARSED_ENV_1=\"7\""));
-        assertTrue(node.getEnv().contains("MY_PARSED_ENV_2=\"7 8\""));
+        assertTrue(node.getEnvStringForTesting().contains("MY_PARSED_ENV_1=\"7\""));
+        assertTrue(node.getEnvStringForTesting().contains("MY_PARSED_ENV_2=\"7 8\""));
     }
 
     @Test
@@ -363,8 +363,8 @@ public class ContentBuilderTest extends DomBuilderTest {
 
         assertEquals(1, s.getSearchNodes().size());
         AbstractService node = s.getSearchNodes().get(0);
-        assertTrue(node.getEnv().contains("MY_1_ENV=\"xyz abc\""));
-        assertTrue(node.getEnv().contains("MY_2_ENV=\"2\""));
+        assertTrue(node.getEnvStringForTesting().contains("MY_1_ENV=\"xyz abc\""));
+        assertTrue(node.getEnvStringForTesting().contains("MY_2_ENV=\"2\""));
     }
 
     @Test
@@ -390,8 +390,8 @@ public class ContentBuilderTest extends DomBuilderTest {
         assertTrue(b.getRootGroup().getCoreOnOOM().get());
 
         assertEquals(2, s.getSearchNodes().size());
-        assertFalse(s.getSearchNodes().get(0).getEnv().contains("VESPA_SILENCE_CORE_ON_OOM=true"));
-        assertFalse(s.getSearchNodes().get(1).getEnv().contains("VESPA_SILENCE_CORE_ON_OOM=true"));
+        assertFalse(s.getSearchNodes().get(0).getEnvStringForTesting().contains("VESPA_SILENCE_CORE_ON_OOM=true"));
+        assertFalse(s.getSearchNodes().get(1).getEnvStringForTesting().contains("VESPA_SILENCE_CORE_ON_OOM=true"));
     }
 
     @Test
@@ -414,8 +414,8 @@ public class ContentBuilderTest extends DomBuilderTest {
         assertFalse(b.getRootGroup().getCoreOnOOM().isPresent());
 
         assertEquals(2, s.getSearchNodes().size());
-        assertTrue(s.getSearchNodes().get(0).getEnv().contains("VESPA_SILENCE_CORE_ON_OOM=true"));
-        assertTrue(s.getSearchNodes().get(1).getEnv().contains("VESPA_SILENCE_CORE_ON_OOM=true"));
+        assertTrue(s.getSearchNodes().get(0).getEnvStringForTesting().contains("VESPA_SILENCE_CORE_ON_OOM=true"));
+        assertTrue(s.getSearchNodes().get(1).getEnvStringForTesting().contains("VESPA_SILENCE_CORE_ON_OOM=true"));
     }
 
     @Test
@@ -438,8 +438,8 @@ public class ContentBuilderTest extends DomBuilderTest {
         assertFalse(b.getRootGroup().getMmapNoCoreLimit().isPresent());
 
         assertEquals(2, s.getSearchNodes().size());
-        assertTrue(s.getSearchNodes().get(0).getEnv().contains("VESPA_MMAP_NOCORE_LIMIT=200000"));
-        assertFalse(s.getSearchNodes().get(1).getEnv().contains("VESPA_MMAP_NOCORE_LIMIT="));
+        assertTrue(s.getSearchNodes().get(0).getEnvStringForTesting().contains("VESPA_MMAP_NOCORE_LIMIT=200000"));
+        assertFalse(s.getSearchNodes().get(1).getEnvStringForTesting().contains("VESPA_MMAP_NOCORE_LIMIT="));
     }
 
     @Test
@@ -462,8 +462,8 @@ public class ContentBuilderTest extends DomBuilderTest {
         assertFalse(b.getRootGroup().getCoreOnOOM().isPresent());
 
         assertEquals(2, s.getSearchNodes().size());
-        assertFalse(s.getSearchNodes().get(0).getEnv().contains("VESPA_SILENCE_CORE_ON_OOM=true"));
-        assertTrue(s.getSearchNodes().get(1).getEnv().contains("VESPA_SILENCE_CORE_ON_OOM=true"));
+        assertFalse(s.getSearchNodes().get(0).getEnvStringForTesting().contains("VESPA_SILENCE_CORE_ON_OOM=true"));
+        assertTrue(s.getSearchNodes().get(1).getEnvStringForTesting().contains("VESPA_SILENCE_CORE_ON_OOM=true"));
     }
 
     @Test
@@ -496,7 +496,7 @@ public class ContentBuilderTest extends DomBuilderTest {
 
         assertEquals(4, s.getSearchNodes().size());
         for (SearchNode n : s.getSearchNodes()) {
-            assertEquals("OMP_NUM_THREADS=1 VESPA_SILENCE_CORE_ON_OOM=true VESPA_USE_NO_VESPAMALLOC=\"proton\" VESPA_USE_VESPAMALLOC=\"storaged\" VESPA_USE_VESPAMALLOC_D=\"distributord\" VESPA_USE_VESPAMALLOC_DST=\"all\"", n.getEnv());
+            assertEquals("OMP_NUM_THREADS=1 VESPA_SILENCE_CORE_ON_OOM=true VESPA_USE_NO_VESPAMALLOC=\"proton\" VESPA_USE_VESPAMALLOC=\"storaged\" VESPA_USE_VESPAMALLOC_D=\"distributord\" VESPA_USE_VESPAMALLOC_DST=\"all\"", n.getEnvStringForTesting());
         }
     }
 
@@ -525,10 +525,10 @@ public class ContentBuilderTest extends DomBuilderTest {
         assertFalse(b.getRootGroup().getVespaMallocDebugStackTrace().isPresent());
 
         assertEquals(4, s.getSearchNodes().size());
-        assertEquals("OMP_NUM_THREADS=1 VESPA_SILENCE_CORE_ON_OOM=true VESPA_USE_NO_VESPAMALLOC=\"proton\"", s.getSearchNodes().get(0).getEnv());
-        assertEquals("OMP_NUM_THREADS=1 VESPA_SILENCE_CORE_ON_OOM=true VESPA_USE_VESPAMALLOC_D=\"distributord\"", s.getSearchNodes().get(1).getEnv());
-        assertEquals("OMP_NUM_THREADS=1 VESPA_SILENCE_CORE_ON_OOM=true VESPA_USE_VESPAMALLOC_DST=\"all\"", s.getSearchNodes().get(2).getEnv());
-        assertEquals("OMP_NUM_THREADS=1 VESPA_SILENCE_CORE_ON_OOM=true VESPA_USE_VESPAMALLOC=\"storaged\"", s.getSearchNodes().get(3).getEnv());
+        assertEquals("OMP_NUM_THREADS=1 VESPA_SILENCE_CORE_ON_OOM=true VESPA_USE_NO_VESPAMALLOC=\"proton\"", s.getSearchNodes().get(0).getEnvStringForTesting());
+        assertEquals("OMP_NUM_THREADS=1 VESPA_SILENCE_CORE_ON_OOM=true VESPA_USE_VESPAMALLOC_D=\"distributord\"", s.getSearchNodes().get(1).getEnvStringForTesting());
+        assertEquals("OMP_NUM_THREADS=1 VESPA_SILENCE_CORE_ON_OOM=true VESPA_USE_VESPAMALLOC_DST=\"all\"", s.getSearchNodes().get(2).getEnvStringForTesting());
+        assertEquals("OMP_NUM_THREADS=1 VESPA_SILENCE_CORE_ON_OOM=true VESPA_USE_VESPAMALLOC=\"storaged\"", s.getSearchNodes().get(3).getEnvStringForTesting());
     }
 
     @Test
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/search/test/SearchNodeTest.java b/config-model/src/test/java/com/yahoo/vespa/model/search/test/SearchNodeTest.java
index 7ed27018d5f..f60441bac03 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/search/test/SearchNodeTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/search/test/SearchNodeTest.java
@@ -95,7 +95,7 @@ public class SearchNodeTest {
         SearchNode node = createSearchNode(root, "mynode2", 4, new NodeSpec(7, 5), true, false, new TestProperties().loadCodeAsHugePages(hugePages));
         node.setHostResource(new HostResource(new Host(node, "mynbode2")));
         node.initService(root.getDeployState());
-        assertEquals(hugePages, node.getStartupCommand().contains("VESPA_LOAD_CODE_AS_HUGEPAGES="));
+        assertEquals(hugePages, node.getEnvVars().get("VESPA_LOAD_CODE_AS_HUGEPAGES") != null);
     }
 
     @Test
@@ -109,7 +109,7 @@ public class SearchNodeTest {
         SearchNode node = createSearchNode(root, "mynode2", 4, new NodeSpec(7, 5), true, false, new TestProperties().sharedStringRepoNoReclaim(sharedStringRepoNoReclaim));
         node.setHostResource(new HostResource(new Host(node, "mynbode2")));
         node.initService(root.getDeployState());
-        assertEquals(sharedStringRepoNoReclaim, node.getStartupCommand().contains("VESPA_SHARED_STRING_REPO_NO_RECLAIM="));
+        assertEquals(sharedStringRepoNoReclaim, node.getEnvVars().get("VESPA_SHARED_STRING_REPO_NO_RECLAIM") != null);
     }
 
     @Test
diff --git a/configd/src/apps/sentinel/service.cpp b/configd/src/apps/sentinel/service.cpp
index 16ca132c29d..e9225325e27 100644
--- a/configd/src/apps/sentinel/service.cpp
+++ b/configd/src/apps/sentinel/service.cpp
@@ -326,6 +326,10 @@ Service::runChild()
         fcntl(n, F_SETFD, FD_CLOEXEC);
     }
 
+    for (const auto &envvar : _config->environ) {
+        setenv(envvar.varname.c_str(), envvar.varvalue.c_str(), 1);
+    }
+
     // Set up environment
     setenv("VESPA_SERVICE_NAME", _config->name.c_str(), 1);
     setenv("VESPA_CONFIG_ID", _config->id.c_str(), 1);
diff --git a/configdefinitions/src/vespa/sentinel.def b/configdefinitions/src/vespa/sentinel.def
index 7bd4a000055..f28c43c77d4 100644
--- a/configdefinitions/src/vespa/sentinel.def
+++ b/configdefinitions/src/vespa/sentinel.def
@@ -39,6 +39,10 @@ ignoreRequestedStackSizes bool default=false restart
 ## stdout and stderr connected to sentinel via pipe on startup.
 service[].command string
 
+## Extra environment variables that will be exposed for this service
+service[].environ[].varname string
+service[].environ[].varvalue string
+
 ## The command to run before stopping service. The same properties as for
 ## startup command holds.
 service[].preShutdownCommand string default=""