From e69354269336784ff0d05ce35d26e3772f59d8d0 Mon Sep 17 00:00:00 2001 From: Roman Terentiev Date: Mon, 8 May 2017 17:15:59 +0300 Subject: [PATCH 1/2] Add ability to set JGit TransportConfigCallback --- .../EnvironmentRepositoryConfiguration.java | 5 ++ .../JGitEnvironmentRepository.java | 54 ++++++++++++------- .../MultipleJGitEnvironmentRepository.java | 3 ++ ...EnvironmentRepositoryIntegrationTests.java | 28 ++++++++++ .../JGitEnvironmentRepositoryTests.java | 26 +++++++++ ...ultipleJGitEnvironmentRepositoryTests.java | 24 +++++++++ 6 files changed, 122 insertions(+), 18 deletions(-) diff --git a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/config/EnvironmentRepositoryConfiguration.java b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/config/EnvironmentRepositoryConfiguration.java index 6714c352..d982064a 100644 --- a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/config/EnvironmentRepositoryConfiguration.java +++ b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/config/EnvironmentRepositoryConfiguration.java @@ -17,6 +17,7 @@ package org.springframework.cloud.config.server.config; import javax.servlet.http.HttpServletRequest; +import org.eclipse.jgit.api.TransportConfigCallback; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; @@ -57,9 +58,13 @@ public class EnvironmentRepositoryConfiguration { @Autowired private ConfigServerProperties server; + @Autowired(required = false) + private TransportConfigCallback transportConfigCallback; + @Bean public MultipleJGitEnvironmentRepository defaultEnvironmentRepository() { MultipleJGitEnvironmentRepository repository = new MultipleJGitEnvironmentRepository(this.environment); + repository.setTransportConfigCallback(this.transportConfigCallback); if (this.server.getDefaultLabel()!=null) { repository.setDefaultLabel(this.server.getDefaultLabel()); } 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 45b460b2..50db97f3 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 @@ -36,6 +36,7 @@ import org.eclipse.jgit.api.ResetCommand.ResetType; import org.eclipse.jgit.api.Status; import org.eclipse.jgit.api.StatusCommand; import org.eclipse.jgit.api.TransportCommand; +import org.eclipse.jgit.api.TransportConfigCallback; import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.api.errors.RefNotFoundException; import org.eclipse.jgit.lib.Ref; @@ -96,6 +97,11 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository */ private CredentialsProvider gitCredentialsProvider; + /** + * Transport configuration callback for JGit commands. + */ + private TransportConfigCallback transportConfigCallback; + /** * Flag to indicate that the repository should force pull. If true discard any local * changes and take from remote repository. @@ -122,6 +128,14 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository this.timeout = timeout; } + public TransportConfigCallback getTransportConfigCallback() { + return transportConfigCallback; + } + + public void setTransportConfigCallback(TransportConfigCallback transportConfigCallback) { + this.transportConfigCallback = transportConfigCallback; + } + public JGitFactory getGitFactory() { return this.gitFactory; } @@ -250,7 +264,7 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository } - public /*public for testing*/ boolean shouldPull(Git git) throws GitAPIException { + protected boolean shouldPull(Git git) throws GitAPIException { boolean shouldPull; Status gitStatus = git.status().call(); boolean isWorkingTreeClean = gitStatus.isClean(); @@ -292,14 +306,13 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository return isBranch(git, label) && !isLocalBranch(git, label); } - private FetchResult fetch(Git git, String label) { + protected FetchResult fetch(Git git, String label) { FetchCommand fetch = git.fetch(); fetch.setRemote("origin"); fetch.setTagOpt(TagOpt.FETCH_TAGS); - setTimeout(fetch); + configureCommand(fetch); try { - setCredentialsProvider(fetch); FetchResult result = fetch.call(); if(result.getTrackingRefUpdates() != null && result.getTrackingRefUpdates().size() > 0) { logger.info("Fetched for remote " + label + " and found " + result.getTrackingRefUpdates().size() @@ -396,8 +409,7 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository private Git cloneToBasedir() throws GitAPIException { CloneCommand clone = this.gitFactory.getCloneCommandByCloneRepository() .setURI(getUri()).setDirectory(getBasedir()); - setTimeout(clone); - setCredentialsProvider(clone); + configureCommand(clone); try { return clone.call(); } @@ -430,20 +442,26 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository } } - private void setCredentialsProvider(TransportCommand cmd) { - if (gitCredentialsProvider != null) { - cmd.setCredentialsProvider(gitCredentialsProvider); - } else if (hasText(getUsername())) { - cmd.setCredentialsProvider( - new UsernamePasswordCredentialsProvider(getUsername(), getPassword())); - } else if (hasText(getPassphrase())) { - cmd.setCredentialsProvider( - new PassphraseCredentialsProvider(getPassphrase())); - } + private void configureCommand(TransportCommand command) { + command.setTimeout(this.timeout); + command.setTransportConfigCallback(this.transportConfigCallback); + command.setCredentialsProvider(getCredentialsProvider()); } - private void setTimeout(TransportCommand pull) { - pull.setTimeout(this.timeout); + private CredentialsProvider getCredentialsProvider() { + if (this.gitCredentialsProvider != null) { + return this.gitCredentialsProvider; + } + + if (hasText(getUsername()) && hasText(getPassword())) { + return new UsernamePasswordCredentialsProvider(getUsername(), getPassword()); + } + + if (hasText(getPassphrase())) { + return new PassphraseCredentialsProvider(getPassphrase()); + } + + return null; } private boolean isClean(Git git) { 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 da55aec1..15fb3111 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 @@ -75,6 +75,9 @@ public class MultipleJGitEnvironmentRepository extends JGitEnvironmentRepository if (repo.getPattern() == null || repo.getPattern().length == 0) { repo.setPattern(new String[] { name }); } + if (repo.getTransportConfigCallback() == null) { + repo.setTransportConfigCallback(getTransportConfigCallback()); + } if (getTimeout() != 0 && repo.getTimeout() == 0) { repo.setTimeout(getTimeout()); } diff --git a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepositoryIntegrationTests.java b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepositoryIntegrationTests.java index 09d9dcf3..6f3d2653 100644 --- a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepositoryIntegrationTests.java +++ b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepositoryIntegrationTests.java @@ -30,6 +30,7 @@ import java.util.Arrays; import org.eclipse.jgit.api.CheckoutCommand; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.ResetCommand.ResetType; +import org.eclipse.jgit.api.TransportConfigCallback; import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; @@ -39,6 +40,7 @@ import org.hamcrest.Matchers; import org.junit.After; import org.junit.Before; import org.junit.Test; +import org.mockito.Mockito; import org.springframework.boot.autoconfigure.PropertyPlaceholderAutoConfiguration; import org.springframework.boot.builder.SpringApplicationBuilder; import org.springframework.boot.context.properties.EnableConfigurationProperties; @@ -47,6 +49,7 @@ import org.springframework.cloud.config.server.config.ConfigServerProperties; import org.springframework.cloud.config.server.config.EnvironmentRepositoryConfiguration; import org.springframework.cloud.config.server.test.ConfigServerTestUtils; import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; import org.springframework.util.ResourceUtils; @@ -56,6 +59,7 @@ import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; @@ -504,6 +508,18 @@ public class JGitEnvironmentRepositoryIntegrationTests { assertEquals(repository.isStrictHostKeyChecking(), strictHostKeyChecking); } + @Test + public void shouldSetTransportConfigCallback() throws IOException { + String uri = ConfigServerTestUtils.prepareLocalRepo(); + this.context = new SpringApplicationBuilder(TestConfigurationWithTransportConfigCallback.class) + .web(false) + .properties("spring.cloud.config.server.git.uri:" + uri) + .run(); + + JGitEnvironmentRepository repository = this.context.getBean(JGitEnvironmentRepository.class); + assertNotNull(repository.getTransportConfigCallback()); + } + @Configuration @EnableConfigurationProperties(ConfigServerProperties.class) @Import({ PropertyPlaceholderAutoConfiguration.class, @@ -511,4 +527,16 @@ public class JGitEnvironmentRepositoryIntegrationTests { protected static class TestConfiguration { } + @Configuration + @EnableConfigurationProperties(ConfigServerProperties.class) + @Import({ PropertyPlaceholderAutoConfiguration.class, + EnvironmentRepositoryConfiguration.class }) + protected static class TestConfigurationWithTransportConfigCallback { + + @Bean + public TransportConfigCallback transportConfigCallback() { + return Mockito.mock(TransportConfigCallback.class); + } + } + } 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 6e6e6914..f8b430cd 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 @@ -33,6 +33,7 @@ import org.eclipse.jgit.api.MergeCommand; import org.eclipse.jgit.api.ResetCommand; import org.eclipse.jgit.api.Status; import org.eclipse.jgit.api.StatusCommand; +import org.eclipse.jgit.api.TransportConfigCallback; import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.api.errors.InvalidRemoteException; import org.eclipse.jgit.api.errors.NotMergedException; @@ -727,6 +728,31 @@ public class JGitEnvironmentRepositoryTests { assertEquals("should call isDebugEnabled warn and debug", 3, numberOfInvocations); } + @Test + public void shouldSetTransportConfigCallbackOnCloneAndFetch() throws Exception { + Git mockGit = mock(Git.class); + FetchCommand fetchCommand = mock(FetchCommand.class); + when(mockGit.fetch()).thenReturn(fetchCommand); + when(fetchCommand.call()).thenReturn(mock(FetchResult.class)); + + CloneCommand mockCloneCommand = mock(CloneCommand.class); + when(mockCloneCommand.setURI(anyString())).thenReturn(mockCloneCommand); + when(mockCloneCommand.setDirectory(any(File.class))).thenReturn(mockCloneCommand); + + TransportConfigCallback configCallback = mock(TransportConfigCallback.class); + JGitEnvironmentRepository envRepository = new JGitEnvironmentRepository(this.environment); + envRepository.setGitFactory(new MockGitFactory(mockGit, mockCloneCommand)); + envRepository.setUri("http://somegitserver/somegitrepo"); + envRepository.setTransportConfigCallback(configCallback); + envRepository.setCloneOnStart(true); + + envRepository.afterPropertiesSet(); + verify(mockCloneCommand, times(1)).setTransportConfigCallback(configCallback); + + envRepository.fetch(mockGit, "master"); + verify(fetchCommand, times(1)).setTransportConfigCallback(configCallback); + } + class MockCloneCommand extends CloneCommand { private Git mockGit; diff --git a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/MultipleJGitEnvironmentRepositoryTests.java b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/MultipleJGitEnvironmentRepositoryTests.java index 9fd7d902..002fc235 100644 --- a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/MultipleJGitEnvironmentRepositoryTests.java +++ b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/MultipleJGitEnvironmentRepositoryTests.java @@ -20,6 +20,7 @@ import java.io.IOException; import java.util.HashMap; import java.util.Map; +import org.eclipse.jgit.api.TransportConfigCallback; import org.junit.Before; import org.junit.Test; @@ -33,6 +34,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; /** * @author Andy Chan (iceycake) @@ -169,6 +171,28 @@ public class MultipleJGitEnvironmentRepositoryTests { assertVersion(environment); } + @Test + public void shouldSetTransportConfigCallback() throws Exception { + TransportConfigCallback mockCallback1 = mock(TransportConfigCallback.class); + TransportConfigCallback mockCallback2 = mock(TransportConfigCallback.class); + + PatternMatchingJGitEnvironmentRepository repo1 = createRepository("test1", "*test1*", "test1Uri"); + + PatternMatchingJGitEnvironmentRepository repo2 = createRepository("test2", "*test2*", "test2Uri"); + repo2.setTransportConfigCallback(mockCallback2); + + Map repos = new HashMap<>(); + repos.put("test1", repo1); + repos.put("test2", repo2); + + this.repository.setRepos(repos); + this.repository.setTransportConfigCallback(mockCallback1); + this.repository.afterPropertiesSet(); + + assertEquals(repo1.getTransportConfigCallback(), mockCallback1); + assertEquals(repo2.getTransportConfigCallback(), mockCallback2); + } + private String getUri(String pattern) { String uri = null; From 7ee15a4f2a3b13353e264c4ba04bbb2087ce489a Mon Sep 17 00:00:00 2001 From: Roman Terentiev Date: Tue, 16 May 2017 12:27:57 +0300 Subject: [PATCH 2/2] Add null checks --- .../server/environment/JGitEnvironmentRepository.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) 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 50db97f3..1da9c00a 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 @@ -444,8 +444,13 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository private void configureCommand(TransportCommand command) { command.setTimeout(this.timeout); - command.setTransportConfigCallback(this.transportConfigCallback); - command.setCredentialsProvider(getCredentialsProvider()); + if (this.transportConfigCallback != null) { + command.setTransportConfigCallback(this.transportConfigCallback); + } + CredentialsProvider credentialsProvider = getCredentialsProvider(); + if (credentialsProvider != null) { + command.setCredentialsProvider(credentialsProvider); + } } private CredentialsProvider getCredentialsProvider() {