Uses label when searching secrets in [Spring]VaultEnvironmentReposito…

…ry (#2460) * Uses label when searching secrets in [Spring]VaultEnvironmentRepository Signed-off-by: kvmw <[email protected]> * Adds a feature flag to enable label in vault secret paths Signed-off-by: kvmw <[email protected]> * Makes default-label in [Spring]VaultEnvironmentRepository configurable Signed-off-by: kvmw <[email protected]> * When label flag is enabled, profile should always by included in vault key Signed-off-by: kvmw <[email protected]> * Updates Vault docs Signed-off-by: kvmw <[email protected]> * Switches to main as default label for vault Signed-off-by: kvmw <[email protected]> --------- Signed-off-by: kvmw <[email protected]>
spring-cloud · Oct 9, 2024 · 6914dc8 · 6914dc8
1 parent e464808
commit 6914dc8
Show file tree

Hide file tree

Showing 6 changed files with 505 additions and 91 deletions.
diff --git a/docs/modules/ROOT/pages/server/environment-repository/vault-backend.adoc b/docs/modules/ROOT/pages/server/environment-repository/vault-backend.adoc
@@ -59,6 +59,12 @@ The following table describes configurable Vault properties:
 |defaultKey
 |application
 
+|defaultLabel
+|main (Only used when `enableLabel` is set to `true`)
+
+|enableLabel
+|false
+
 |profileSeparator
 |,
 
@@ -158,6 +164,24 @@ Properties written to `secret/application` are available to <<_vault_server,all
 An application with the name, `myApp`, would have any properties written to `secret/myApp` and `secret/application` available to it.
 When `myApp` has the `dev` profile enabled, properties written to all of the above paths would be available to it, with properties in the first path in the list taking priority over the others.
 
+[[enabling-serach-by-label]]
+== Enabling Search by Label
+
+By default, Vault backend does not use the label when searching for secrets. You can change this by
+setting the `enableLabel` feature flag to `true` and, optionally, setting the `defaultLabel`.
+When `defaultLabel` is not provided `main` will be used.
+
+When `enableLabel` feature flag is on, the secrets in Vault should always have all three segments(application name, profile and label) in their paths.
+So the example in previous section, with enabled feature flag, would be like :
+
+[source,sh]
+----
+secret/myApp,dev,myLabel
+secret/myApp,default,myLabel       # default profile
+secret/application,dev,myLabel     # default application name
+secret/application,default,myLabel # default application name and default profile.
+----
+
 [[decrypting-vault-secrets]]
 == Decrypting Vault Secrets in Property Sources
 

diff --git a/spring-cloud-config-sample/src/test/java/sample/ConfigDataOrderingVaultIntegrationTests.java b/spring-cloud-config-sample/src/test/java/sample/ConfigDataOrderingVaultIntegrationTests.java
@@ -18,11 +18,11 @@
 
 import java.io.IOException;
 
-import org.json.JSONException;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 import org.testcontainers.containers.BindMode;
+import org.testcontainers.containers.Container.ExecResult;
 import org.testcontainers.junit.jupiter.Container;
 import org.testcontainers.junit.jupiter.Testcontainers;
 import org.testcontainers.utility.DockerImageName;
@@ -36,37 +36,37 @@
 import static org.assertj.core.api.Assertions.assertThat;
 
 /**
- * Integration test for https://github.com/spring-cloud/spring-cloud-config/issues/1997
- * The error only occurs if a profile specific config imports is used, otherwise
- * reordering does not take place. A profile specific config import is defined in
+ * Integration test for issue
+ * <a href="https://github.com/spring-cloud/spring-cloud-config/issues/1997">#1997</a> The
+ * error only occurs if a profile specific config imports is used, otherwise reordering
+ * does not take place. A profile specific config import is defined in
  * vaultordering/client-dev.yml
  */
 @Testcontainers
 public class ConfigDataOrderingVaultIntegrationTests {
 
 	private static final int configServerPort = TestSocketUtils.findAvailableTcpPort();
 
-	private static final int configClientPort = TestSocketUtils.findAvailableTcpPort();
-
 	private static ConfigurableApplicationContext client;
 
 	private static ConfigurableApplicationContext server;
 
 	@Container
-	public static VaultContainer vaultContainer = new VaultContainer<>(DockerImageName.parse("vault:1.13.3"))
+	public static VaultContainer<?> vaultContainer = new VaultContainer<>(DockerImageName.parse("vault:1.13.3"))
 		.withVaultToken("my-root-token")
 		.withClasspathResourceMapping("vaultordering/vault_test_policy.txt", "/tmp/vault_test_policy.txt",
 				BindMode.READ_ONLY);
 
 	@BeforeAll
-	public static void startConfigServer() throws IOException, InterruptedException, JSONException {
+	public static void startConfigServer() throws IOException, InterruptedException {
 		server = SpringApplication.run(TestConfigServerApplication.class,
 				"--spring.config.location=classpath:/vaultordering/", "--spring.config.name=server",
 				"--server.port=" + configServerPort,
 				"--spring.cloud.config.server.vault.port=" + vaultContainer.getFirstMappedPort());
 
-		execInVault("vault", "kv", "put", "secret/client-app,dev", "my.prop=vaultdev");
-		execInVault("vault", "kv", "put", "secret/client-app", "my.prop=vault");
+		execInVault("vault", "kv", "put", "secret/client-app,dev", "my.prop=value-in-dev");
+		execInVault("vault", "kv", "put", "secret/client-app,prod", "my.prop=value-in-prod");
+		execInVault("vault", "kv", "put", "secret/client-app", "my.prop=default-value");
 
 	}
 
@@ -81,22 +81,35 @@ public static void close() {
 	}
 
 	@Test
-	void propertyFromVaultIsUsed() {
-		client = SpringApplication.run(TestConfigServerApplication.class, "--server.port=" + configClientPort,
+	void profileSpecificPropertyFromVaultIsUsed() {
+		client = SpringApplication.run(TestConfigServerApplication.class,
+				"--server.port=" + TestSocketUtils.findAvailableTcpPort(),
 				"--spring.config.location=classpath:/vaultordering/", "--spring.config.name=client",
 				"--spring.profiles.active=dev", "--spring.application.name=client-app",
 				"--spring.cloud.config.enabled=true", "--spring.cloud.config.server.enabled=false",
 				"--config.server.port=" + configServerPort);
 
-		assertThat(client.getEnvironment().getProperty("my.prop")).isEqualTo("vaultdev");
+		assertThat(client.getEnvironment().getProperty("my.prop")).isEqualTo("value-in-dev");
+
+	}
+
+	@Test
+	void profileSpecificPropertyFromVaultIsUsedInCorrectOrder() {
+		client = SpringApplication.run(TestConfigServerApplication.class,
+				"--server.port=" + TestSocketUtils.findAvailableTcpPort(),
+				"--spring.config.location=classpath:/vaultordering/", "--spring.config.name=client",
+				"--spring.profiles.active=dev,prod", "--spring.application.name=client-app",
+				"--spring.cloud.config.enabled=true", "--spring.cloud.config.server.enabled=false",
+				"--config.server.port=" + configServerPort);
+
+		assertThat(client.getEnvironment().getProperty("my.prop")).isEqualTo("value-in-prod");
 
 	}
 
-	private static String execInVault(String... command) throws IOException, InterruptedException {
-		org.testcontainers.containers.Container.ExecResult execResult = vaultContainer.execInContainer(command);
+	private static void execInVault(String... command) throws IOException, InterruptedException {
+		ExecResult execResult = vaultContainer.execInContainer(command);
 		assertThat(execResult.getExitCode()).isZero();
 		assertThat(execResult.getStderr()).isEmpty();
-		return execResult.getStdout();
 	}
 
 }
diff --git a/...g/springframework/cloud/config/server/environment/AbstractVaultEnvironmentRepository.java b/...g/springframework/cloud/config/server/environment/AbstractVaultEnvironmentRepository.java
@@ -16,11 +16,12 @@
 
 package org.springframework.cloud.config.server.environment;
 
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
-import java.util.Properties;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import jakarta.servlet.http.HttpServletRequest;
 import jakarta.validation.constraints.NotEmpty;
@@ -33,6 +34,7 @@
 import org.springframework.cloud.config.environment.PropertySource;
 import org.springframework.core.Ordered;
 import org.springframework.core.io.ByteArrayResource;
+import org.springframework.util.ObjectUtils;
 import org.springframework.util.StringUtils;
 
 import static org.springframework.cloud.config.client.ConfigClientProperties.STATE_HEADER;
@@ -46,7 +48,9 @@
  */
 public abstract class AbstractVaultEnvironmentRepository implements EnvironmentRepository, Ordered {
 
-	private static Log log = LogFactory.getLog(AbstractVaultEnvironmentRepository.class);
+	private static final String DEFAULT_PROFILE = "default";
+
+	private static final Log log = LogFactory.getLog(AbstractVaultEnvironmentRepository.class);
 
 	// TODO: move to watchState:String on findOne?
 	protected final ObjectProvider<HttpServletRequest> request;
@@ -65,37 +69,51 @@ public abstract class AbstractVaultEnvironmentRepository implements EnvironmentR
 	@NotEmpty
 	protected String profileSeparator;
 
+	protected final boolean enableLabel;
+
+	protected final String defaultLabel;
+
 	protected int order;
 
 	public AbstractVaultEnvironmentRepository(ObjectProvider<HttpServletRequest> request, EnvironmentWatch watch,
 			VaultEnvironmentProperties properties) {
 		this.defaultKey = properties.getDefaultKey();
 		this.profileSeparator = properties.getProfileSeparator();
+		this.enableLabel = properties.isEnableLabel();
+		this.defaultLabel = properties.getDefaultLabel();
 		this.order = properties.getOrder();
 		this.request = request;
 		this.watch = watch;
 	}
 
 	@Override
 	public Environment findOne(String application, String profile, String label) {
-		String[] profiles = StringUtils.commaDelimitedListToStringArray(profile);
-		List<String> scrubbedProfiles = scrubProfiles(profiles);
-
-		List<String> keys = findKeys(application, scrubbedProfiles);
-
-		Environment environment = new Environment(application, profiles, label, null, getWatchState());
-
-		for (String key : keys) {
-			// read raw 'data' key from vault
-			String data = read(key);
-			if (data != null) {
-				// data is in json format of which, yaml is a superset, so parse
-				final YamlPropertiesFactoryBean yaml = new YamlPropertiesFactoryBean();
-				yaml.setResources(new ByteArrayResource(data.getBytes()));
-				Properties properties = yaml.getObject();
+		if (ObjectUtils.isEmpty(profile)) {
+			profile = DEFAULT_PROFILE;
+		}
+		if (ObjectUtils.isEmpty(label)) {
+			label = defaultLabel;
+		}
 
-				if (!properties.isEmpty()) {
-					environment.add(new PropertySource("vault:" + key, properties));
+		var environment = new Environment(application, split(profile), label, null, getWatchState());
+
+		var profiles = normalize(profile, DEFAULT_PROFILE);
+		var applications = normalize(application, this.defaultKey);
+
+		for (String prof : profiles) {
+			for (String app : applications) {
+				var key = vaultKey(app, prof, label);
+				// read raw 'data' key from vault
+				String data = read(key);
+				if (data != null) {
+					// data is in json format of which, yaml is a superset, so parse
+					var yaml = new YamlPropertiesFactoryBean();
+					yaml.setResources(new ByteArrayResource(data.getBytes()));
+					var properties = yaml.getObject();
+
+					if (properties != null && !properties.isEmpty()) {
+						environment.add(new PropertySource("vault:" + key, properties));
+					}
 				}
 			}
 		}
@@ -105,6 +123,22 @@ public Environment findOne(String application, String profile, String label) {
 
 	protected abstract String read(String key);
 
+	private String vaultKey(String application, String profile, String label) {
+		var key = application;
+		if (this.enableLabel) {
+			// always append profile to the key, if flag is enabled.
+			key += this.profileSeparator + profile;
+			// always append label to the key, if flag is enabled.
+			key += this.profileSeparator + label;
+		}
+		else if (!DEFAULT_PROFILE.equals(profile)) {
+			// default profile should not be included in the key, if flag is not enabled.
+			key += this.profileSeparator + profile;
+		}
+
+		return key;
+	}
+
 	private String getWatchState() {
 		HttpServletRequest servletRequest = this.request.getIfAvailable();
 		if (servletRequest != null) {
@@ -120,35 +154,22 @@ private String getWatchState() {
 		return null;
 	}
 
-	private List<String> findKeys(String application, List<String> profiles) {
-		List<String> keys = new ArrayList<>();
-
-		if (StringUtils.hasText(this.defaultKey) && !this.defaultKey.equals(application)) {
-			keys.add(this.defaultKey);
-			addProfiles(keys, this.defaultKey, profiles);
-		}
-
-		// application may have comma-separated list of names
-		String[] applications = StringUtils.commaDelimitedListToStringArray(application);
-		for (String app : applications) {
-			keys.add(app);
-			addProfiles(keys, app, profiles);
-		}
-
-		Collections.reverse(keys);
-		return keys;
-	}
-
-	private List<String> scrubProfiles(String[] profiles) {
-		List<String> scrubbedProfiles = new ArrayList<>(Arrays.asList(profiles));
-		scrubbedProfiles.remove("default");
-		return scrubbedProfiles;
+	/**
+	 * Splits the comma delimited items and returns the reversed distinct items with given
+	 * default item at the end.
+	 */
+	private List<String> normalize(String commaDelimitedItems, String defaultItem) {
+		var items = Stream.concat(Stream.of(defaultItem), Arrays.stream(split(commaDelimitedItems)))
+			.distinct()
+			.filter(Predicate.not(ObjectUtils::isEmpty))
+			.collect(Collectors.toList());
+
+		Collections.reverse(items);
+		return items;
 	}
 
-	private void addProfiles(List<String> contexts, String baseContext, List<String> profiles) {
-		for (String profile : profiles) {
-			contexts.add(baseContext + this.profileSeparator + profile);
-		}
+	private String[] split(String str) {
+		return StringUtils.commaDelimitedListToStringArray(str);
 	}
 
 	public void setDefaultKey(String defaultKey) {

diff --git a/.../java/org/springframework/cloud/config/server/environment/VaultEnvironmentProperties.java b/.../java/org/springframework/cloud/config/server/environment/VaultEnvironmentProperties.java
@@ -101,6 +101,17 @@ public class VaultEnvironmentProperties implements HttpEnvironmentRepositoryProp
 	 */
 	private String token;
 
+	/**
+	 * Flag to indicate that the repository should use 'label' as well as
+	 * 'application-name' and 'profile', for vault secrets. By default, the vault secrets
+	 * are expected to be in 'application-name,profile' path. When this flag enabled, they
+	 * are expected to be in `application-name,profile,label' path. To maintain
+	 * compatibility this flag is not enabled by default.
+	 */
+	private boolean enableLabel = false;
+
+	private String defaultLabel = "main";
+
 	private AppRoleProperties appRole = new AppRoleProperties();
 
 	private AwsEc2Properties awsEc2 = new AwsEc2Properties();
@@ -229,6 +240,22 @@ public void setToken(String token) {
 		this.token = token;
 	}
 
+	public boolean isEnableLabel() {
+		return enableLabel;
+	}
+
+	public void setEnableLabel(boolean enableLabel) {
+		this.enableLabel = enableLabel;
+	}
+
+	public String getDefaultLabel() {
+		return defaultLabel;
+	}
+
+	public void setDefaultLabel(String defaultLabel) {
+		this.defaultLabel = defaultLabel;
+	}
+
 	public AppRoleProperties getAppRole() {
 		return this.appRole;
 	}