From 1152821221bd3eaa959981d8b750045b58466f5e Mon Sep 17 00:00:00 2001 From: ivasylyev Date: Thu, 2 Feb 2017 11:23:30 +0200 Subject: [PATCH] Add stacktrace logging --- .../CipherEnvironmentEncryptor.java | 9 +++-- .../encryption/EncryptionController.java | 26 +++++++-------- .../JGitEnvironmentRepository.java | 33 ++++++++++++------- .../MultipleJGitEnvironmentRepository.java | 4 +-- .../environment/NoSuchLabelException.java | 4 +++ .../environment/RepositoryException.java | 4 +++ .../SvnKitEnvironmentRepository.java | 10 ++++-- .../encryption/EncryptionControllerTests.java | 15 +++++++++ .../JGitEnvironmentRepositoryTests.java | 24 ++++++++++++++ 9 files changed, 95 insertions(+), 34 deletions(-) diff --git a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/encryption/CipherEnvironmentEncryptor.java b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/encryption/CipherEnvironmentEncryptor.java index a9f9a32f..04578abd 100644 --- a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/encryption/CipherEnvironmentEncryptor.java +++ b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/encryption/CipherEnvironmentEncryptor.java @@ -77,8 +77,13 @@ public class CipherEnvironmentEncryptor implements EnvironmentEncryptor { catch (Exception e) { value = ""; name = "invalid." + name; - logger.warn("Cannot decrypt key: " + key + " (" + e.getClass() - + ": " + e.getMessage() + ")"); + String message = "Cannot decrypt key: " + key + " (" + e.getClass() + + ": " + e.getMessage() + ")"; + if (logger.isDebugEnabled()) { + logger.debug(message, e); + } else if (logger.isWarnEnabled()) { + logger.warn(message); + } } map.put(name, value); } diff --git a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/encryption/EncryptionController.java b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/encryption/EncryptionController.java index 99825320..8e2162f2 100644 --- a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/encryption/EncryptionController.java +++ b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/encryption/EncryptionController.java @@ -125,19 +125,14 @@ public class EncryptionController { public String encrypt(@PathVariable String name, @PathVariable String profiles, @RequestBody String data, @RequestHeader("Content-Type") MediaType type) { checkEncryptorInstalled(name, profiles); - try { - String input = stripFormData(data, type, false); - Map keys = this.helper.getEncryptorKeys(name, profiles, - input); - String textToEncrypt = this.helper.stripPrefix(input); - String encrypted = this.helper.addPrefix(keys, - this.encryptor.locate(keys).encrypt(textToEncrypt)); - logger.info("Encrypted data"); - return encrypted; - } - catch (IllegalArgumentException e) { - throw new InvalidCipherException(); - } + String input = stripFormData(data, type, false); + Map keys = this.helper.getEncryptorKeys(name, profiles, + input); + String textToEncrypt = this.helper.stripPrefix(input); + String encrypted = this.helper.addPrefix(keys, + this.encryptor.locate(keys).encrypt(textToEncrypt)); + logger.info("Encrypted data"); + return encrypted; } @RequestMapping(value = "decrypt", method = RequestMethod.POST) @@ -161,7 +156,8 @@ public class EncryptionController { logger.info("Decrypted cipher data"); return decrypted; } - catch (IllegalArgumentException e) { + catch (IllegalArgumentException|IllegalStateException e) { + logger.error("Cannot decrypt key:" + name + ", value:" + data, e); throw new InvalidCipherException(); } } @@ -241,4 +237,4 @@ class KeyNotAvailableException extends RuntimeException { @SuppressWarnings("serial") class InvalidCipherException extends RuntimeException { -} +} \ No newline at end of file diff --git a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepository.java b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepository.java index cb544e7c..ae7564d3 100644 --- a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepository.java +++ b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepository.java @@ -197,7 +197,7 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository return git.getRepository().getRef("HEAD").getObjectId().getName(); } catch (RefNotFoundException e) { - throw new NoSuchLabelException("No such label: " + label); + throw new NoSuchLabelException("No such label: " + label, e); } catch (GitAPIException e) { throw new IllegalStateException("Cannot clone or checkout repository", e); @@ -302,14 +302,15 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository setCredentialsProvider(fetch); FetchResult result = fetch.call(); if(result.getTrackingRefUpdates() != null && result.getTrackingRefUpdates().size() > 0) { - this.logger.info("Fetched for remote " + label + " and found " + result.getTrackingRefUpdates().size() + logger.info("Fetched for remote " + label + " and found " + result.getTrackingRefUpdates().size() + " updates"); } return result; } catch (Exception ex) { - this.logger.warn("Could not fetch remote for " + label + " remote: " + git - .getRepository().getConfig().getString("remote", "origin", "url")); + String message = "Could not fetch remote for " + label + " remote: " + git + .getRepository().getConfig().getString("remote", "origin", "url"); + warn(message, ex); return null; } } @@ -325,8 +326,9 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository return result; } catch (Exception ex) { - this.logger.warn("Could not merge remote for " + label + " remote: " + git - .getRepository().getConfig().getString("remote", "origin", "url")); + String message = "Could not merge remote for " + label + " remote: " + git + .getRepository().getConfig().getString("remote", "origin", "url"); + warn(message, ex); return null; } } @@ -343,9 +345,10 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository return resetRef; } catch (Exception ex) { - this.logger.warn("Could not reset to remote for " + label + " (current ref=" + String message = "Could not reset to remote for " + label + " (current ref=" + ref + "), remote: " + git.getRepository().getConfig() - .getString("remote", "origin", "url")); + .getString("remote", "origin", "url"); + warn(message, ex); return null; } } @@ -449,10 +452,9 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository return status.call().isClean(); } catch (Exception e) { - this.logger - .warn("Could not execute status command on local repository. Cause: (" - + e.getClass().getSimpleName() + ") " + e.getMessage()); - + String message = "Could not execute status command on local repository. Cause: (" + + e.getClass().getSimpleName() + ") " + e.getMessage(); + warn(message, e); return false; } } @@ -486,6 +488,13 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository return false; } + protected void warn(String message, Exception ex) { + logger.warn(message); + if (logger.isDebugEnabled()) { + logger.debug("Stacktrace for: " + message, ex); + } + } + /** * Wraps the static method calls to {@link org.eclipse.jgit.api.Git} and * {@link org.eclipse.jgit.api.CloneCommand} allowing for easier unit testing. diff --git a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/MultipleJGitEnvironmentRepository.java b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/MultipleJGitEnvironmentRepository.java index 31a57708..83e335ae 100644 --- a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/MultipleJGitEnvironmentRepository.java +++ b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/MultipleJGitEnvironmentRepository.java @@ -119,7 +119,7 @@ public class MultipleJGitEnvironmentRepository extends JGitEnvironmentRepository if (logger.isDebugEnabled()) { this.logger.debug("Cannot retrieve resource locations from " + candidate.getUri() + ", cause: (" - + e.getClass().getSimpleName() + ") " + e.getMessage()); + + e.getClass().getSimpleName() + ") " + e.getMessage(), e); } continue; } @@ -154,7 +154,7 @@ public class MultipleJGitEnvironmentRepository extends JGitEnvironmentRepository if (logger.isDebugEnabled()) { this.logger.debug("Cannot load configuration from " + candidate.getUri() + ", cause: (" - + e.getClass().getSimpleName() + ") " + e.getMessage()); + + e.getClass().getSimpleName() + ") " + e.getMessage(), e); } continue; } diff --git a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/NoSuchLabelException.java b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/NoSuchLabelException.java index dcf3f8d2..e4939307 100644 --- a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/NoSuchLabelException.java +++ b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/NoSuchLabelException.java @@ -27,4 +27,8 @@ public class NoSuchLabelException extends RepositoryException { super(string); } + public NoSuchLabelException(String string, Exception e) { + super(string, e); + } + } diff --git a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/RepositoryException.java b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/RepositoryException.java index 71e2efa9..0803bc1e 100644 --- a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/RepositoryException.java +++ b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/RepositoryException.java @@ -27,4 +27,8 @@ public class RepositoryException extends RuntimeException { super(string); } + public RepositoryException(String message, Throwable cause) { + super(message, cause); + } + } diff --git a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/SvnKitEnvironmentRepository.java b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/SvnKitEnvironmentRepository.java index af19b126..30a7097f 100644 --- a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/SvnKitEnvironmentRepository.java +++ b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/SvnKitEnvironmentRepository.java @@ -146,9 +146,13 @@ public class SvnKitEnvironmentRepository extends AbstractScmEnvironmentRepositor return version.toString(); } catch (Exception e) { - this.logger.warn("Could not update remote for " + label + " (current local=" - + getWorkingDirectory().getPath() + "), remote: " + this.getUri() - + ")"); + String message = "Could not update remote for " + label + " (current local=" + + getWorkingDirectory().getPath() + "), remote: " + this.getUri() + ")"; + if (logger.isDebugEnabled()) { + logger.debug(message, e); + } else if (logger.isWarnEnabled()) { + logger.warn(message); + } } final SVNStatus status = SVNClientManager.newInstance().getStatusClient() diff --git a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/encryption/EncryptionControllerTests.java b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/encryption/EncryptionControllerTests.java index cc0c7797..2ff4e23c 100644 --- a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/encryption/EncryptionControllerTests.java +++ b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/encryption/EncryptionControllerTests.java @@ -55,6 +55,21 @@ public class EncryptionControllerTests { this.controller.decrypt("foo", MediaType.TEXT_PLAIN); } + @Test(expected = InvalidCipherException.class) + public void shouldThrowExceptionOnDecryptInvalidData() { + this.controller = new EncryptionController( + new SingleTextEncryptorLocator(new RsaSecretEncryptor())); + controller.decrypt("foo", MediaType.TEXT_PLAIN); + } + + @Test(expected = InvalidCipherException.class) + public void shouldThrowExceptionOnDecryptWrongKey() { + RsaSecretEncryptor encryptor = new RsaSecretEncryptor(); + this.controller = new EncryptionController( + new SingleTextEncryptorLocator(new RsaSecretEncryptor())); + controller.decrypt(encryptor.encrypt("foo"), MediaType.TEXT_PLAIN); + } + @Test public void sunnyDayRsaKey() { this.controller = new EncryptionController( diff --git a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepositoryTests.java b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepositoryTests.java index 0637eac4..6e6e6914 100644 --- a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepositoryTests.java +++ b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepositoryTests.java @@ -23,6 +23,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import org.apache.commons.logging.Log; import org.eclipse.jgit.api.CheckoutCommand; import org.eclipse.jgit.api.CloneCommand; import org.eclipse.jgit.api.FetchCommand; @@ -72,7 +73,9 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThat; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyString; +import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockingDetails; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -703,6 +706,27 @@ public class JGitEnvironmentRepositoryTests { } } + @Test + public void shouldPrintStacktraceIfDebugEnabled() throws Exception { + final Log mockLogger = mock(Log.class); + JGitEnvironmentRepository envRepository = new JGitEnvironmentRepository(this.environment){ + @Override + public void afterPropertiesSet() throws Exception { + this.logger = mockLogger; + } + }; + envRepository.afterPropertiesSet(); + when(mockLogger.isDebugEnabled()).thenReturn(true); + + envRepository.warn("", new RuntimeException()); + + verify(mockLogger).warn(eq("")); + verify(mockLogger).debug(eq("Stacktrace for: "), any(RuntimeException.class)); + + int numberOfInvocations = mockingDetails(mockLogger).getInvocations().size(); + assertEquals("should call isDebugEnabled warn and debug", 3, numberOfInvocations); + } + class MockCloneCommand extends CloneCommand { private Git mockGit;