some nitpicks in fabric8 secrets implementation (#857)

* started work

* more changes

* checkstyle

* add labels to equals/hashCode

* dot, not comma

* some refactor

* checkstyle
This commit is contained in:
erabii
2021-08-27 11:55:11 -04:00
committed by GitHub
parent 536b532c53
commit 6a189335ea
6 changed files with 60 additions and 46 deletions

View File

@@ -137,7 +137,7 @@ class KubernetesClientEventBasedSecretsChangeDetectorTests {
KubernetesMockEnvironment environment = new KubernetesMockEnvironment(
mock(KubernetesClientSecretsPropertySource.class)).withProperty("db-password", "p455w0rd");
KubernetesClientSecretsPropertySourceLocator locator = mock(KubernetesClientSecretsPropertySourceLocator.class);
when(locator.locate(environment)).thenReturn(new MockPropertySource().withProperty("db-password", "p455w0rd2"));
when(locator.locate(environment)).thenAnswer(ignoreMe -> new MockPropertySource().withProperty("db-password", "p455w0rd2"));
ConfigReloadProperties properties = new ConfigReloadProperties();
properties.setMonitoringSecrets(true);
KubernetesNamespaceProvider kubernetesNamespaceProvider = mock(KubernetesNamespaceProvider.class);

View File

@@ -17,10 +17,9 @@
package org.springframework.cloud.kubernetes.commons.config;
import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.stream.Collectors;
import org.springframework.boot.context.properties.ConfigurationProperties;
@@ -37,11 +36,11 @@ public class SecretsConfigProperties extends AbstractConfigProperties {
private boolean enableApi = false;
private Map<String, String> labels = new HashMap<>();
private Map<String, String> labels = Collections.emptyMap();
private List<String> paths = new LinkedList<>();
private List<String> paths = Collections.emptyList();
private List<Source> sources = new LinkedList<>();
private List<Source> sources = Collections.emptyList();
public boolean isEnableApi() {
return this.enableApi;
@@ -81,9 +80,9 @@ public class SecretsConfigProperties extends AbstractConfigProperties {
}
/**
* @return A list of Source to use If the user has not specified any Source
* @return A list of Source to use. If the user has not specified any Source
* properties, then a single Source is constructed based on the supplied name and
* namespace
* namespace.
*
* These are the actual name/namespace pairs that are used to create a
* SecretsPropertySource
@@ -114,7 +113,7 @@ public class SecretsConfigProperties extends AbstractConfigProperties {
/**
* The labels of the Secret to find.
*/
private Map<String, String> labels = new HashMap<>();
private Map<String, String> labels = Collections.emptyMap();
public Source() {
}
@@ -190,6 +189,29 @@ public class SecretsConfigProperties extends AbstractConfigProperties {
return labels;
}
@Override
public String toString() {
return "{ secret name : '" + name + "', namespace : '" + namespace + "'";
}
@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}
SecretsConfigProperties.NormalizedSource other = (SecretsConfigProperties.NormalizedSource) o;
return Objects.equals(this.name, other.name) && Objects.equals(this.namespace, other.namespace)
&& Objects.equals(this.labels, other.labels);
}
@Override
public int hashCode() {
return Objects.hash(name, namespace, labels);
}
}
}

View File

@@ -36,14 +36,12 @@ public class SecretsPropertySource extends MapPropertySource {
}
protected static String getSourceName(String name, String namespace) {
return new StringBuilder().append(PREFIX).append(Constants.PROPERTY_SOURCE_NAME_SEPARATOR).append(name)
.append(Constants.PROPERTY_SOURCE_NAME_SEPARATOR).append(namespace).toString();
return PREFIX + Constants.PROPERTY_SOURCE_NAME_SEPARATOR + name +
Constants.PROPERTY_SOURCE_NAME_SEPARATOR + namespace;
}
protected static void putAll(Map<String, String> data, Map<String, Object> result) {
if (data != null) {
data.forEach((k, v) -> result.put(k, new String(Base64.getDecoder().decode(v)).trim()));
}
data.forEach((k, v) -> result.put(k, new String(Base64.getDecoder().decode(v)).trim()));
}
@Override

View File

@@ -23,6 +23,7 @@ import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Collections;
import java.util.EnumSet;
import java.util.HashSet;
import java.util.List;
import java.util.Objects;
import java.util.Set;
@@ -60,17 +61,19 @@ public abstract class SecretsPropertySourceLocator implements PropertySourceLoca
}
@Override
public PropertySource locate(Environment environment) {
public PropertySource<?> locate(Environment environment) {
if (environment instanceof ConfigurableEnvironment) {
ConfigurableEnvironment env = (ConfigurableEnvironment) environment;
List<SecretsConfigProperties.NormalizedSource> sources = this.properties.determineSources();
Set<SecretsConfigProperties.NormalizedSource> uniqueSources = new HashSet<>(sources);
LOG.debug("Secrets normalized sources : " + sources);
CompositePropertySource composite = new CompositePropertySource("composite-secrets");
// read for secrets mount
putPathConfig(composite);
if (this.properties.isEnableApi()) {
sources.forEach(s -> composite.addPropertySource(getKubernetesPropertySourceForSingleSecret(env, s)));
uniqueSources.forEach(s -> composite.addPropertySource(getMapPropertySourceForSingleSecret(env, s)));
}
return composite;
@@ -78,7 +81,7 @@ public abstract class SecretsPropertySourceLocator implements PropertySourceLoca
return null;
}
private MapPropertySource getKubernetesPropertySourceForSingleSecret(ConfigurableEnvironment environment,
private MapPropertySource getMapPropertySourceForSingleSecret(ConfigurableEnvironment environment,
SecretsConfigProperties.NormalizedSource normalizedSource) {
String configurationTarget = this.properties.getConfigurationTarget();

View File

@@ -25,7 +25,6 @@ import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.springframework.cloud.kubernetes.commons.config.SecretsPropertySource;
import org.springframework.core.env.Environment;
import org.springframework.util.StringUtils;
/**
@@ -38,51 +37,43 @@ public class Fabric8SecretsPropertySource extends SecretsPropertySource {
private static final Log LOG = LogFactory.getLog(Fabric8SecretsPropertySource.class);
private static final String PREFIX = "secrets";
public Fabric8SecretsPropertySource(KubernetesClient client, Environment env, String name, String namespace,
public Fabric8SecretsPropertySource(KubernetesClient client, String name, String namespace,
Map<String, String> labels) {
super(getSourceName(name, namespace), getSourceData(client, env, name, namespace, labels));
super(getSourceName(name, namespace), getSourceData(client, name, namespace, labels));
}
private static Map<String, Object> getSourceData(KubernetesClient client, Environment env, String name,
String namespace, Map<String, String> labels) {
private static Map<String, Object> getSourceData(KubernetesClient client, String name, String namespace,
Map<String, String> labels) {
Map<String, Object> result = new HashMap<>();
String namespaceToUse = StringUtils.hasLength(namespace) ? namespace : client.getNamespace();
try {
// Read for secrets api (named)
Secret secret;
if (StringUtils.isEmpty(namespace)) {
secret = client.secrets().withName(name).get();
Secret secret = client.secrets().inNamespace(namespaceToUse).withName(name).get();
// the API is documented that it might return null
if (secret == null) {
LOG.warn("secret with name : " + name + " in namespace : " + namespaceToUse + " not found");
}
else {
secret = client.secrets().inNamespace(namespace).withName(name).get();
putDataFromSecret(secret, result, namespaceToUse);
}
putAll(secret, result);
// Read for secrets api (label)
if (!labels.isEmpty()) {
if (StringUtils.isEmpty(namespace)) {
client.secrets().withLabels(labels).list().getItems().forEach(s -> putAll(s, result));
}
else {
client.secrets().inNamespace(namespace).withLabels(labels).list().getItems()
.forEach(s -> putAll(s, result));
}
}
client.secrets().inNamespace(namespaceToUse).withLabels(labels).list().getItems()
.forEach(s -> putDataFromSecret(s, result, namespaceToUse));
}
catch (Exception e) {
LOG.warn("Can't read secret with name: [" + name + "] or labels [" + labels + "] in namespace:[" + namespace
LOG.warn("Can't read secret with name: [" + name + "] or labels [" + labels + "] in namespace: [" + namespaceToUse
+ "] (cause: " + e.getMessage() + "). Ignoring");
}
return result;
}
private static void putAll(Secret secret, Map<String, Object> result) {
if (secret != null) {
putAll(secret.getData(), result);
}
private static void putDataFromSecret(Secret secret, Map<String, Object> result, String namespace) {
LOG.debug("reading secret with name : " + secret.getMetadata().getName() + " in namespace : " + namespace);
putAll(secret.getData(), result);
}
}

View File

@@ -46,7 +46,7 @@ public class Fabric8SecretsPropertySourceLocator extends SecretsPropertySourceLo
@Override
protected MapPropertySource getPropertySource(ConfigurableEnvironment environment,
SecretsConfigProperties.NormalizedSource normalizedSource, String configurationTarget) {
return new Fabric8SecretsPropertySource(this.client, environment,
return new Fabric8SecretsPropertySource(this.client,
getApplicationName(environment, normalizedSource.getName(), configurationTarget), Fabric8ConfigUtils
.getApplicationNamespace(this.client, normalizedSource.getNamespace(), configurationTarget),
normalizedSource.getLabels());