Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes logging for Parameter Store and Secrets Manager integrations #229

Merged
merged 4 commits into from
Feb 9, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@
import com.amazonaws.services.simplesystemsmanagement.model.GetParametersByPathRequest;
import com.amazonaws.services.simplesystemsmanagement.model.GetParametersByPathResult;
import com.amazonaws.services.simplesystemsmanagement.model.Parameter;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;

import org.springframework.core.env.EnumerablePropertySource;

Expand All @@ -39,7 +39,7 @@
*/
public class AwsParamStorePropertySource extends EnumerablePropertySource<AWSSimpleSystemsManagement> {

private static final Logger LOGGER = LoggerFactory.getLogger(AwsParamStorePropertySource.class);
private static Log LOG = LogFactory.getLog(AwsParamStorePropertySource.class);

private final String context;

Expand Down Expand Up @@ -71,7 +71,7 @@ private void getParameters(GetParametersByPathRequest paramsRequest) {
GetParametersByPathResult paramsResult = this.source.getParametersByPath(paramsRequest);
for (Parameter parameter : paramsResult.getParameters()) {
String key = parameter.getName().replace(this.context, "").replace('/', '.');
LOGGER.debug("Populating property retrieved from AWS Parameter Store: {}", key);
LOG.debug("Populating property retrieved from AWS Parameter Store: " + key);
this.properties.put(key, parameter.getValue());
}
if (paramsResult.getNextToken() != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@
import java.util.Set;

import com.amazonaws.services.simplesystemsmanagement.AWSSimpleSystemsManagement;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;

import org.springframework.cloud.bootstrap.config.PropertySourceLocator;
import org.springframework.core.env.CompositePropertySource;
Expand Down Expand Up @@ -54,8 +52,6 @@ public class AwsParamStorePropertySourceLocator implements PropertySourceLocator

private final Set<String> contexts = new LinkedHashSet<>();

private Log logger = LogFactory.getLog(getClass());

public AwsParamStorePropertySourceLocator(AWSSimpleSystemsManagement ssmClient,
AwsParamStoreProperties properties) {
this.ssmClient = ssmClient;
Expand All @@ -74,7 +70,7 @@ public PropertySource<?> locate(Environment environment) {

ConfigurableEnvironment env = (ConfigurableEnvironment) environment;

AwsParamStorePropertySources sources = new AwsParamStorePropertySources(this.properties, this.logger);
AwsParamStorePropertySources sources = new AwsParamStorePropertySources(this.properties);

List<String> profiles = Arrays.asList(env.getActiveProfiles());
List<String> contexts = sources.getAutomaticContexts(profiles);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import com.amazonaws.services.simplesystemsmanagement.AWSSimpleSystemsManagement;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;

import org.springframework.util.StringUtils;

Expand All @@ -34,13 +35,12 @@
*/
public class AwsParamStorePropertySources {

private final AwsParamStoreProperties properties;
private static Log LOG = LogFactory.getLog(AwsParamStorePropertySources.class);

private final Log log;
private final AwsParamStoreProperties properties;

public AwsParamStorePropertySources(AwsParamStoreProperties properties, Log log) {
public AwsParamStorePropertySources(AwsParamStoreProperties properties) {
this.properties = properties;
this.log = log;
}

/**
Expand Down Expand Up @@ -95,7 +95,7 @@ private void addProfiles(List<String> contexts, String baseContext, List<String>
*/
public AwsParamStorePropertySource createPropertySource(String context, boolean optional,
AWSSimpleSystemsManagement client) {
log.info("Loading property from AWS Parameter Store with name: " + context + ", optional: " + optional);
LOG.info("Loading property from AWS Parameter Store with name: " + context + ", optional: " + optional);
try {
AwsParamStorePropertySource propertySource = new AwsParamStorePropertySource(context, client);
propertySource.init();
Expand All @@ -107,7 +107,7 @@ public AwsParamStorePropertySource createPropertySource(String context, boolean
throw new AwsParameterPropertySourceNotFoundException(e);
}
else {
log.warn("Unable to load AWS parameter from " + context + ". " + e.getMessage());
LOG.warn("Unable to load AWS parameter from " + context + ". " + e.getMessage());
}
}
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,10 @@
import java.util.Collections;
import java.util.List;

import org.apache.commons.logging.Log;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;

/**
* Unit test for {@link AwsParamStorePropertySourceLocator}.
Expand All @@ -33,8 +31,6 @@
*/
class AwsParamStorePropertySourcesTest {

private final Log logMock = mock(Log.class);

private AwsParamStoreProperties properties;

@BeforeEach
Expand All @@ -46,7 +42,7 @@ void setUp() {

@Test
void getAutomaticContextsWithSingleProfile() {
AwsParamStorePropertySources propertySource = new AwsParamStorePropertySources(properties, logMock);
AwsParamStorePropertySources propertySource = new AwsParamStorePropertySources(properties);

List<String> contexts = propertySource.getAutomaticContexts(Collections.singletonList("production"));

Expand All @@ -57,7 +53,7 @@ void getAutomaticContextsWithSingleProfile() {

@Test
void getAutomaticContextsWithoutProfile() {
AwsParamStorePropertySources propertySource = new AwsParamStorePropertySources(properties, logMock);
AwsParamStorePropertySources propertySource = new AwsParamStorePropertySources(properties);

List<String> contexts = propertySource.getAutomaticContexts(Collections.emptyList());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@
<artifactId>spring-boot-starter</artifactId>
</dependency>

<dependency>
<groupId>org.springframework.cloud</groupId>
<artifactId>spring-cloud-starter-bootstrap</artifactId>
maciejwalkowiak marked this conversation as resolved.
Show resolved Hide resolved
<version>3.0.0</version>
</dependency>

<dependency>
<groupId>io.awspring.cloud</groupId>
<artifactId>spring-cloud-starter-aws-parameter-store-config</artifactId>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# importing single parameter
spring.config.import=aws-parameterstore:/config/spring/
#spring.config.import=aws-parameterstore:/config/spring/
logging.level.io.awspring.cloud=debug
maciejwalkowiak marked this conversation as resolved.
Show resolved Hide resolved

# importing multiple parameters
# spring.config.import: aws-parameterstore:/config/spring;/config/common
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export class InfrastructureStack extends cdk.Stack {
generateSecretString: {
secretStringTemplate: '{}',
generateStringKey: 'password',
excludeCharacters: '#$' // Spring tries to interpolate any ${} and #{}
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;

import org.springframework.core.env.EnumerablePropertySource;

Expand All @@ -40,6 +42,8 @@
*/
public class AwsSecretsManagerPropertySource extends EnumerablePropertySource<AWSSecretsManager> {

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

private final ObjectMapper jsonMapper = new ObjectMapper();

private final String context;
Expand Down Expand Up @@ -79,6 +83,7 @@ private void readSecretValue(GetSecretValueRequest secretValueRequest) {
});

for (Map.Entry<String, Object> secretEntry : secretMap.entrySet()) {
LOG.debug("Populating property retrieved from AWS Secrets Manager: " + secretEntry.getKey());
properties.put(secretEntry.getKey(), secretEntry.getValue());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@
import java.util.Set;

import com.amazonaws.services.secretsmanager.AWSSecretsManager;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;

import org.springframework.cloud.bootstrap.config.PropertySourceLocator;
import org.springframework.core.env.CompositePropertySource;
Expand Down Expand Up @@ -57,8 +55,6 @@ public class AwsSecretsManagerPropertySourceLocator implements PropertySourceLoc

private final Set<String> contexts = new LinkedHashSet<>();

private Log logger = LogFactory.getLog(getClass());

public AwsSecretsManagerPropertySourceLocator(String propertySourceName, AWSSecretsManager smClient,
AwsSecretsManagerProperties properties) {
this.propertySourceName = propertySourceName;
Expand All @@ -82,7 +78,7 @@ public PropertySource<?> locate(Environment environment) {

ConfigurableEnvironment env = (ConfigurableEnvironment) environment;

AwsSecretsManagerPropertySources sources = new AwsSecretsManagerPropertySources(properties, logger);
AwsSecretsManagerPropertySources sources = new AwsSecretsManagerPropertySources(properties);

List<String> profiles = Arrays.asList(env.getActiveProfiles());
List<String> contexts = sources.getAutomaticContexts(profiles);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import com.amazonaws.services.secretsmanager.AWSSecretsManager;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;

import org.springframework.util.StringUtils;

Expand All @@ -34,13 +35,12 @@
*/
public class AwsSecretsManagerPropertySources {

private final AwsSecretsManagerProperties properties;
private static Log LOG = LogFactory.getLog(AwsSecretsManagerPropertySources.class);

private final Log log;
private final AwsSecretsManagerProperties properties;

public AwsSecretsManagerPropertySources(AwsSecretsManagerProperties properties, Log log) {
public AwsSecretsManagerPropertySources(AwsSecretsManagerProperties properties) {
this.properties = properties;
this.log = log;
}

public List<String> getAutomaticContexts(List<String> profiles) {
Expand Down Expand Up @@ -83,7 +83,7 @@ private void addProfiles(List<String> contexts, String baseContext, List<String>
*/
public AwsSecretsManagerPropertySource createPropertySource(String context, boolean optional,
AWSSecretsManager client) {
log.info("Loading secrets from AWS Secret Manager secret with name: " + context + ", optional: " + optional);
LOG.info("Loading secrets from AWS Secret Manager secret with name: " + context + ", optional: " + optional);
try {
AwsSecretsManagerPropertySource propertySource = new AwsSecretsManagerPropertySource(context, client);
propertySource.init();
Expand All @@ -95,7 +95,7 @@ public AwsSecretsManagerPropertySource createPropertySource(String context, bool
throw new AwsSecretsManagerPropertySourceNotFoundException(e);
}
else {
log.warn("Unable to load AWS secret from " + context + ". " + e.getMessage());
LOG.warn("Unable to load AWS secret from " + context + ". " + e.getMessage());
}
}
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,10 @@
import java.util.Collections;
import java.util.List;

import org.apache.commons.logging.Log;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;

/**
* Unit test for {@link AwsSecretsManagerPropertySources}.
Expand All @@ -33,8 +31,6 @@
*/
class AwsSecretsManagerPropertySourcesTests {

private final Log logMock = mock(Log.class);

private AwsSecretsManagerProperties properties;

@BeforeEach
Expand All @@ -46,7 +42,7 @@ void setUp() {

@Test
void getAutomaticContextsWithSingleProfile() {
AwsSecretsManagerPropertySources propertySource = new AwsSecretsManagerPropertySources(properties, logMock);
AwsSecretsManagerPropertySources propertySource = new AwsSecretsManagerPropertySources(properties);

List<String> contexts = propertySource.getAutomaticContexts(Collections.singletonList("production"));

Expand All @@ -57,7 +53,7 @@ void getAutomaticContextsWithSingleProfile() {

@Test
void getAutomaticContextsWithoutProfile() {
AwsSecretsManagerPropertySources propertySource = new AwsSecretsManagerPropertySources(properties, logMock);
AwsSecretsManagerPropertySources propertySource = new AwsSecretsManagerPropertySources(properties);

List<String> contexts = propertySource.getAutomaticContexts(Collections.emptyList());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,37 @@

package io.awspring.cloud.autoconfigure.paramstore;

import java.lang.reflect.Field;
import java.lang.reflect.Modifier;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;

import com.amazonaws.services.simplesystemsmanagement.AWSSimpleSystemsManagement;
import io.awspring.cloud.paramstore.AwsParamStorePropertySource;
import io.awspring.cloud.paramstore.AwsParamStorePropertySources;
import org.apache.commons.logging.Log;

import org.springframework.boot.context.config.ConfigData;
import org.springframework.boot.context.config.ConfigDataLoader;
import org.springframework.boot.context.config.ConfigDataLoaderContext;
import org.springframework.boot.context.config.ConfigDataResourceNotFoundException;
import org.springframework.boot.logging.DeferredLogFactory;
import org.springframework.util.ReflectionUtils;

/**
* @author Edd煤 Mel茅ndez
* @since 2.3.0
*/
public class AwsParamStoreConfigDataLoader implements ConfigDataLoader<AwsParamStoreConfigDataResource> {

private final DeferredLogFactory logFactory;

public AwsParamStoreConfigDataLoader(DeferredLogFactory logFactory) {
this.logFactory = logFactory;
reconfigureLoggers(logFactory);
}

@Override
public ConfigData load(ConfigDataLoaderContext context, AwsParamStoreConfigDataResource resource) {
try {
Expand All @@ -50,4 +65,24 @@ public ConfigData load(ConfigDataLoaderContext context, AwsParamStoreConfigDataR
}
}

private void reconfigureLoggers(DeferredLogFactory logFactory) {
// loggers in these classes must be static non-final
List<Class<?>> loggers = Arrays.asList(AwsParamStorePropertySource.class, AwsParamStorePropertySources.class);

loggers.forEach(it -> reconfigureLogger(it, logFactory));
}

static void reconfigureLogger(Class<?> type, DeferredLogFactory logFactory) {
ReflectionUtils.doWithFields(type, field -> {

field.setAccessible(true);
field.set(null, logFactory.getLog(type));

}, AwsParamStoreConfigDataLoader::isUpdateableLogField);
}

private static boolean isUpdateableLogField(Field field) {
return !Modifier.isFinal(field.getModifiers()) && field.getType().isAssignableFrom(Log.class);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@
import com.amazonaws.services.simplesystemsmanagement.AWSSimpleSystemsManagement;
import io.awspring.cloud.paramstore.AwsParamStoreProperties;
import io.awspring.cloud.paramstore.AwsParamStorePropertySources;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;

import org.springframework.boot.BootstrapContext;
import org.springframework.boot.BootstrapRegistry;
Expand Down Expand Up @@ -53,8 +51,6 @@ public class AwsParamStoreConfigDataLocationResolver
*/
public static final String PREFIX = "aws-parameterstore:";

private final Log log = LogFactory.getLog(AwsParamStoreConfigDataLocationResolver.class);

@Override
public boolean isResolvable(ConfigDataLocationResolverContext context, ConfigDataLocation location) {
if (!location.hasPrefix(PREFIX)) {
Expand All @@ -80,7 +76,7 @@ public List<AwsParamStoreConfigDataResource> resolveProfileSpecific(

AwsParamStoreProperties properties = loadConfigProperties(resolverContext.getBinder());

AwsParamStorePropertySources sources = new AwsParamStorePropertySources(properties, log);
AwsParamStorePropertySources sources = new AwsParamStorePropertySources(properties);

List<String> contexts = location.getValue().equals(PREFIX)
? sources.getAutomaticContexts(profiles.getAccepted())
Expand Down