Skip to content

Commit

Permalink
clean-up for issue 1592 (#1782)
Browse files Browse the repository at this point in the history
* first, no tests

Signed-off-by: wind57 <[email protected]>

* still broken

Signed-off-by: wind57 <[email protected]>

* still broken

Signed-off-by: wind57 <[email protected]>

* still broken

Signed-off-by: wind57 <[email protected]>

* checkstyle

Signed-off-by: wind57 <[email protected]>

* still broken

Signed-off-by: wind57 <[email protected]>

* still broken

Signed-off-by: wind57 <[email protected]>

* still broken

Signed-off-by: wind57 <[email protected]>

* add more debug logs

Signed-off-by: wind57 <[email protected]>

* rename

Signed-off-by: wind57 <[email protected]>

* rename

Signed-off-by: wind57 <[email protected]>

* add debug statement

Signed-off-by: wind57 <[email protected]>

* more simplifications

Signed-off-by: wind57 <[email protected]>

* correction

Signed-off-by: wind57 <[email protected]>

* corect debug statement

Signed-off-by: wind57 <[email protected]>

* disable configmap

Signed-off-by: wind57 <[email protected]>

* test

Signed-off-by: wind57 <[email protected]>

* cleanup

Signed-off-by: wind57 <[email protected]>

* cleanup

Signed-off-by: wind57 <[email protected]>

* cleanup

Signed-off-by: wind57 <[email protected]>

* fix

Signed-off-by: wind57 <[email protected]>

* add log statement

Signed-off-by: wind57 <[email protected]>

---------

Signed-off-by: wind57 <[email protected]>
  • Loading branch information
wind57 authored Nov 6, 2024
1 parent 952d954 commit aad8cd0
Show file tree
Hide file tree
Showing 10 changed files with 55 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ private static List<StrippedSourceContainer> strippedConfigMaps(CoreV1Api coreV1
private static List<StrippedSourceContainer> strippedSecrets(CoreV1Api coreV1Api, String namespace) {
List<StrippedSourceContainer> strippedSecrets = KubernetesClientSecretsCache.byNamespace(coreV1Api, namespace);
if (strippedSecrets.isEmpty()) {
LOG.debug("No configmaps in namespace '" + namespace + "'");
LOG.debug("No secrets in namespace '" + namespace + "'");
}
return strippedSecrets;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,11 @@ public PropertySource<?> locate(Environment environment) {
if (this.properties.enableApi()) {
Set<NormalizedSource> sources = new LinkedHashSet<>(this.properties.determineSources(environment));
LOG.debug("Config Map normalized sources : " + sources);
sources.forEach(s -> composite.addFirstPropertySource(getMapPropertySource(s, env)));
sources.forEach(s -> {
MapPropertySource propertySource = getMapPropertySource(s, env);
LOG.debug("Adding config map property source " + propertySource.getName());
composite.addFirstPropertySource(propertySource);
});
}

addPropertySourcesFromPaths(environment, composite);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ public static MultipleSourcesContainer processNamedData(List<StrippedSourceConta
sourceNames.forEach(sourceName -> {
StrippedSourceContainer stripped = hashByName.get(sourceName);
if (stripped != null) {
LOG.debug("Found source with name : '" + sourceName + " in namespace: '" + namespace + "'");
LOG.debug("Found source with name : '" + sourceName + "' in namespace: '" + namespace + "'");
foundSourceNames.add(sourceName);
// see if data is a single yaml/properties file and if it needs decoding
Map<String, String> rawData = stripped.data();
Expand All @@ -229,6 +229,9 @@ public static MultipleSourcesContainer processNamedData(List<StrippedSourceConta
environment, includeDefaultProfileData));
}
}
else {
LOG.warn("sourceName : " + sourceName + " was requested, but not found in namespace : " + namespace);
}
});

return new MultipleSourcesContainer(foundSourceNames, data);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@ public final SourceData compute(Map<String, String> labels, ConfigUtils.Prefix p
data = dataSupplier(labels, profiles);

// need this check because when there is no data, the name of the property
// source
// is using provided labels,
// source is using provided labels,
// unlike when the data is present: when we use secret names
if (data.names().isEmpty()) {
String names = labels.keySet()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@
package org.springframework.cloud.kubernetes.commons.config;

import java.util.LinkedHashSet;
import java.util.Map;
import java.util.stream.Collectors;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;

import static org.springframework.cloud.kubernetes.commons.config.ConfigUtils.onException;
import static org.springframework.cloud.kubernetes.commons.config.Constants.PROPERTY_SOURCE_NAME_SEPARATOR;

Expand All @@ -31,6 +33,8 @@
*/
public abstract class NamedSourceData {

private static final Log LOG = LogFactory.getLog(NamedSourceData.class);

public final SourceData compute(String sourceName, ConfigUtils.Prefix prefix, String target, boolean profileSources,
boolean failFast, String namespace, String[] activeProfiles) {

Expand All @@ -51,7 +55,9 @@ public final SourceData compute(String sourceName, ConfigUtils.Prefix prefix, St
data = dataSupplier(sourceNames);

if (data.names().isEmpty()) {
return new SourceData(ConfigUtils.sourceName(target, sourceName, namespace), Map.of());
String emptySourceName = ConfigUtils.sourceName(target, sourceName, namespace);
LOG.debug("Will return empty source with name : " + emptySourceName);
return SourceData.emptyRecord(emptySourceName);
}

if (prefix != ConfigUtils.Prefix.DEFAULT) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import org.springframework.core.env.CompositePropertySource;
import org.springframework.core.env.ConfigurableEnvironment;
import org.springframework.core.env.Environment;
import org.springframework.core.env.MapPropertySource;
import org.springframework.core.env.PropertySource;

/**
Expand Down Expand Up @@ -87,8 +88,11 @@ public PropertySource<?> locate(Environment environment) {
putPathConfig(composite);

if (this.properties.enableApi()) {
uniqueSources
.forEach(s -> composite.addPropertySource(getSecretsPropertySourceForSingleSecret(env, s)));
uniqueSources.forEach(s -> {
MapPropertySource propertySource = getSecretsPropertySourceForSingleSecret(env, s);
LOG.debug("Adding secret property source " + propertySource.getName());
composite.addFirstPropertySource(propertySource);
});
}

cache.discardAll();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

package org.springframework.cloud.kubernetes.commons.config;

import java.util.Collections;
import java.util.Map;

/**
Expand All @@ -25,10 +24,10 @@
*
* @author wind57
*/
public final record SourceData(String sourceName, Map<String, Object> sourceData) {
public record SourceData(String sourceName, Map<String, Object> sourceData) {

public static SourceData emptyRecord(String sourceName) {
return new SourceData(sourceName, Collections.emptyMap());
return new SourceData(sourceName, Map.of());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public class SourceDataEntriesProcessor extends MapPropertySource {

private static final Log LOG = LogFactory.getLog(SourceDataEntriesProcessor.class);

private static Predicate<String> ENDS_IN_EXTENSION = x -> x.endsWith(".yml") || x.endsWith(".yaml")
private static final Predicate<String> ENDS_IN_EXTENSION = x -> x.endsWith(".yml") || x.endsWith(".yaml")
|| x.endsWith(".properties");

public SourceDataEntriesProcessor(SourceData sourceData) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,25 +141,25 @@ else if (propertySource instanceof CompositePropertySource source) {
return result;
}

static boolean changed(List<? extends MapPropertySource> left, List<? extends MapPropertySource> right) {
if (left.size() != right.size()) {
static boolean changed(List<? extends MapPropertySource> k8sSources, List<? extends MapPropertySource> appSources) {
if (k8sSources.size() != appSources.size()) {
if (LOG.isDebugEnabled()) {
LOG.debug("left size: " + left.size());
left.forEach(item -> LOG.debug(item.toString()));
LOG.debug("k8s property sources size: " + k8sSources.size());
k8sSources.forEach(item -> LOG.debug(item.toString()));

LOG.debug("right size: " + right.size());
right.forEach(item -> LOG.debug(item.toString()));
LOG.debug("app property sources size size: " + appSources.size());
appSources.forEach(item -> LOG.debug(item.toString()));
}
LOG.warn(() -> "The current number of ConfigMap PropertySources does not match "
LOG.warn(() -> "The current number of PropertySources does not match "
+ "the ones loaded from Kubernetes - No reload will take place");
return false;
}

for (int i = 0; i < left.size(); i++) {
MapPropertySource leftPropertySource = left.get(i);
MapPropertySource rightPropertySource = right.get(i);
if (changed(leftPropertySource, rightPropertySource)) {
LOG.debug(() -> "found change in : " + leftPropertySource);
for (int i = 0; i < k8sSources.size(); i++) {
MapPropertySource k8sSource = k8sSources.get(i);
MapPropertySource appSource = appSources.get(i);
if (changed(k8sSource, appSource)) {
LOG.debug(() -> "found change in : " + k8sSource);
return true;
}
}
Expand All @@ -169,20 +169,20 @@ static boolean changed(List<? extends MapPropertySource> left, List<? extends Ma

/**
* Determines if two property sources are different.
* @param left left map property sources
* @param right right map property sources
* @param k8sSource left map property sources
* @param appSource right map property sources
* @return {@code true} if source has changed
*/
static boolean changed(MapPropertySource left, MapPropertySource right) {
if (left == right) {
static boolean changed(MapPropertySource k8sSource, MapPropertySource appSource) {
if (k8sSource == appSource) {
return false;
}
if (left == null || right == null) {
if (k8sSource == null || appSource == null) {
return true;
}
Map<String, Object> leftMap = left.getSource();
Map<String, Object> rightMap = right.getSource();
return !Objects.equals(leftMap, rightMap);
Map<String, Object> k8sMap = k8sSource.getSource();
Map<String, Object> appMap = appSource.getSource();
return !Objects.equals(k8sMap, appMap);
}

}
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
logging:
level:
root: DEBUG
org:
springframework:
cloud:
kubernetes: debug

spring:
application:
Expand All @@ -18,3 +21,6 @@ spring:
secrets:
enabled: true
enable-api: true

config:
enabled: false

0 comments on commit aad8cd0

Please sign in to comment.