diff --git a/docs/src/main/asciidoc/spring-cloud-config.adoc b/docs/src/main/asciidoc/spring-cloud-config.adoc index b2c37688..b32ddbb3 100644 --- a/docs/src/main/asciidoc/spring-cloud-config.adoc +++ b/docs/src/main/asciidoc/spring-cloud-config.adoc @@ -529,6 +529,32 @@ spring: NOTE: The default value for `force-pull` property is `false`. +===== Deleting untracked branches in Git Repositories + +As Spring Cloud Config Server has a clone of the remote git repository +after check-outing branch to local repo (e.g fetching properties by label) it will keep this branch +forever or till the next server restart (which creates new local repo). +So there could be a case when remote branch is deleted but local copy of it is still available for fetching. +And if Spring Cloud Config Server client service starts with `--spring.cloud.config.label=deletedRemoteBranch,master` +it will fetch properties from `deletedRemoteBranch` local branch, but not from `master`. + +In order to keep local repository branches clean and up to remote - `deleteUntrackedBranches` property could be set. +It will make Spring Cloud Config Server *force* delete untracked branches from local repository. +Example: + +[source,yaml] +---- +spring: + cloud: + config: + server: + git: + uri: https://github.com/spring-cloud-samples/config-repo + deleteUntrackedBranches: true + +---- + +NOTE: The default value for `deleteUntrackedBranches` property is `false`. ==== Version Control Backend Filesystem Use 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 1e7c33c5..6db6dbda 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 @@ -20,6 +20,9 @@ import static org.springframework.util.StringUtils.hasText; import java.io.File; import java.io.IOException; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -27,6 +30,7 @@ import java.util.Set; import org.eclipse.jgit.api.CheckoutCommand; import org.eclipse.jgit.api.CloneCommand; import org.eclipse.jgit.api.CreateBranchCommand.SetupUpstreamMode; +import org.eclipse.jgit.api.DeleteBranchCommand; import org.eclipse.jgit.api.FetchCommand; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.ListBranchCommand; @@ -47,8 +51,10 @@ import org.eclipse.jgit.transport.CredentialsProvider; import org.eclipse.jgit.transport.FetchResult; import org.eclipse.jgit.transport.JschConfigSessionFactory; import org.eclipse.jgit.transport.OpenSshConfig.Host; +import org.eclipse.jgit.transport.ReceiveCommand; import org.eclipse.jgit.transport.SshSessionFactory; import org.eclipse.jgit.transport.TagOpt; +import org.eclipse.jgit.transport.TrackingRefUpdate; import org.eclipse.jgit.transport.UsernamePasswordCredentialsProvider; import org.eclipse.jgit.util.FileUtils; import org.springframework.beans.factory.InitializingBean; @@ -56,7 +62,10 @@ import org.springframework.cloud.config.server.support.PassphraseCredentialsProv import org.springframework.core.env.ConfigurableEnvironment; import org.springframework.core.io.UrlResource; import org.springframework.util.Assert; +import org.springframework.util.CollectionUtils; import org.springframework.util.StringUtils; +import static java.lang.String.format; +import static org.eclipse.jgit.transport.ReceiveCommand.Type.DELETE; import com.jcraft.jsch.Session; @@ -75,6 +84,8 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository private static final String DEFAULT_LABEL = "master"; private static final String FILE_URI_PREFIX = "file:"; + private static final String LOCAL_BRANCH_REF_PREFIX = "refs/remotes/origin/"; + /** * Timeout (in seconds) for obtaining HTTP or SSH connection (if applicable). Default * 5 seconds. @@ -108,6 +119,11 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository private boolean forcePull; private boolean initialized; + /** + * Flag to indicate that the branch should be deleted locally if it's origin tracked branch was removed. + */ + private boolean deleteUntrackedBranches; + public JGitEnvironmentRepository(ConfigurableEnvironment environment) { super(environment); } @@ -161,6 +177,14 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository this.forcePull = forcePull; } + public boolean isDeleteUntrackedBranches() { + return deleteUntrackedBranches; + } + + public void setDeleteUntrackedBranches(boolean deleteUntrackedBranches) { + this.deleteUntrackedBranches = deleteUntrackedBranches; + } + @Override public synchronized Locations getLocations(String application, String profile, String label) { @@ -190,9 +214,11 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository try { git = createGitClient(); if (shouldPull(git)) { - fetch(git, label); - // checkout after fetch so we can get any new branches, tags, - // ect. + FetchResult fetchStatus = fetch(git, label); + if(deleteUntrackedBranches) { + deleteUntrackedLocalBranches(fetchStatus.getTrackingRefUpdates(), git); + } + // checkout after fetch so we can get any new branches, tags, ect. checkout(git, label); if (isBranch(git, label)) { // merge results from fetch @@ -201,7 +227,7 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository logger.warn( "The local repository is dirty. Resetting it to origin/" + label + "."); - resetHard(git, label, "refs/remotes/origin/" + label); + resetHard(git, label, LOCAL_BRANCH_REF_PREFIX + label); } } } @@ -258,6 +284,55 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository } + /** + * Deletes local branches if corresponding remote branch was removed. + * + * @param trackingRefUpdates list of tracking ref updates + * @param git git instance + * @return list of deleted branches + */ + private Collection deleteUntrackedLocalBranches(Collection trackingRefUpdates, Git git) { + if (CollectionUtils.isEmpty(trackingRefUpdates)) { + return Collections.emptyList(); + } + + Collection branchesToDelete = new ArrayList<>(); + for (TrackingRefUpdate trackingRefUpdate : trackingRefUpdates) { + ReceiveCommand receiveCommand = trackingRefUpdate.asReceiveCommand(); + if (receiveCommand.getType() == DELETE) { + String localRefName = trackingRefUpdate.getLocalName(); + if (StringUtils.startsWithIgnoreCase(localRefName, LOCAL_BRANCH_REF_PREFIX)) { + String localBranchName = localRefName.substring(LOCAL_BRANCH_REF_PREFIX.length(), localRefName.length()); + branchesToDelete.add(localBranchName); + } + } + } + + if (CollectionUtils.isEmpty(branchesToDelete)) { + return Collections.emptyList(); + } + + try { + //make sure that deleted branch not a current one + checkout(git, defaultLabel); + return deleteBranches(git, branchesToDelete); + } catch (Exception ex) { + String message = format("Failed to delete %s branches.", branchesToDelete); + warn(message, ex); + return Collections.emptyList(); + } + } + + private List deleteBranches(Git git, Collection branchesToDelete) throws GitAPIException { + DeleteBranchCommand deleteBranchCommand = git.branchDelete() + .setBranchNames(branchesToDelete.toArray(new String[0])) + //local branch can contain data which is not merged to HEAD - force delete it anyway, since local copy should be R/O + .setForce(true); + List resultList = deleteBranchCommand.call(); + logger.info(format("Deleted %s branches from %s branches to delete.", resultList, branchesToDelete)); + return resultList; + } + private Ref checkout(Git git, String label) throws GitAPIException { CheckoutCommand checkout = git.checkout(); if (shouldTrack(git, label)) { @@ -296,7 +371,7 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository Set dirties = dirties(status.getAdded(), status.getChanged(), status.getRemoved(), status.getMissing(), status.getModified(), status.getConflicting(), status.getUntracked()); - this.logger.warn(String.format("Dirty files found: %s", dirties)); + this.logger.warn(format("Dirty files found: %s", dirties)); } @SuppressWarnings("unchecked") @@ -316,6 +391,7 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository FetchCommand fetch = git.fetch(); fetch.setRemote("origin"); fetch.setTagOpt(TagOpt.FETCH_TAGS); + fetch.setRemoveDeletedRefs(deleteUntrackedBranches); configureCommand(fetch); try { diff --git a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/JGitConfigServerTestData.java b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/JGitConfigServerTestData.java index dc4c06fd..fe7c674d 100644 --- a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/JGitConfigServerTestData.java +++ b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/JGitConfigServerTestData.java @@ -17,6 +17,7 @@ package org.springframework.cloud.config.server.environment; import org.eclipse.jgit.api.Git; + import org.springframework.boot.builder.SpringApplicationBuilder; import org.springframework.cloud.config.server.test.ConfigServerTestUtils; import org.springframework.context.ConfigurableApplicationContext; @@ -24,6 +25,9 @@ import org.springframework.util.FileSystemUtils; import org.springframework.util.ResourceUtils; import java.io.File; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; /** * Class that holds objects that can be used for testing @@ -78,7 +82,13 @@ public class JGitConfigServerTestData { return this.context; } - public static JGitConfigServerTestData prepareClonedGitRepository(Object... sources) throws Exception { + public static JGitConfigServerTestData prepareClonedGitRepository(Object... sources) + throws Exception { + return prepareClonedGitRepositoryWithProps(Collections.emptySet(), sources); + } + + public static JGitConfigServerTestData prepareClonedGitRepositoryWithProps(Collection additionalProperties, Object... sources) + throws Exception { //setup remote repository String remoteUri = ConfigServerTestUtils.prepareLocalRepo(); File remoteRepoDir = ResourceUtils.getFile(remoteUri); @@ -100,8 +110,11 @@ public class JGitConfigServerTestData { .call(); //setup our test spring application pointing to the local repo + Collection properties = new ArrayList<>(additionalProperties); + properties.add("spring.cloud.config.server.git.uri:" + "file://" + clonedRepoDir.getAbsolutePath()); ConfigurableApplicationContext context = new SpringApplicationBuilder(sources).web(false) - .properties("spring.cloud.config.server.git.uri:" + "file://" + clonedRepoDir.getAbsolutePath()).run(); + .properties(properties.toArray(new String[0])) + .run(); JGitEnvironmentRepository repository = context.getBean(JGitEnvironmentRepository.class); return new JGitConfigServerTestData( 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 1417c73b..6e23de52 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 @@ -26,6 +26,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.nio.file.StandardOpenOption; import java.util.Arrays; +import java.util.Collections; import org.eclipse.jgit.api.CheckoutCommand; import org.eclipse.jgit.api.Git; @@ -503,6 +504,42 @@ public class JGitEnvironmentRepositoryIntegrationTests { assertNotNull(repository.getTransportConfigCallback()); } + @Test + public void testShouldReturnEnvironmentFromLocalBranchInCaseRemoteDeleted() throws Exception { + JGitConfigServerTestData testData = JGitConfigServerTestData + .prepareClonedGitRepository(TestConfiguration.class); + + String branchToDelete = "branchToDelete"; + testData.getServerGit().getGit().branchCreate().setName(branchToDelete).call(); + + Environment environment = testData.getRepository().findOne("bar", "staging", "branchToDelete"); + assertNotNull(environment); + + testData.getServerGit().getGit().branchDelete().setBranchNames(branchToDelete).call(); + environment = testData.getRepository().findOne("bar", "staging", "branchToDelete"); + assertNotNull(environment); + } + + @Test(expected = NoSuchLabelException.class) + public void testShouldFailIfRemoteBranchWasDeleted() throws Exception { + JGitConfigServerTestData testData = JGitConfigServerTestData + .prepareClonedGitRepositoryWithProps(Collections.singleton("spring.cloud.config.server.git.deleteUntrackedBranches=true"), + TestConfiguration.class); + + String branchToDelete = "branchToDelete"; + testData.getServerGit().getGit().branchCreate().setName(branchToDelete).call(); + + //checkout and simulate regular flow + Environment environment = testData.getRepository().findOne("bar", "staging", "branchToDelete"); + assertNotNull(environment); + + //remove branch + testData.getServerGit().getGit().branchDelete().setBranchNames(branchToDelete).call(); + + //test + testData.getRepository().findOne("bar", "staging", "branchToDelete"); + } + @Configuration @EnableConfigurationProperties(ConfigServerProperties.class) @Import({ PropertyPlaceholderAutoConfiguration.class, EnvironmentRepositoryConfiguration.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 9d87cf99..2e6caf7d 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 @@ -19,16 +19,19 @@ package org.springframework.cloud.config.server.environment; import java.io.File; import java.io.IOException; import java.util.ArrayList; +import java.util.Collection; 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.DeleteBranchCommand; import org.eclipse.jgit.api.FetchCommand; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.ListBranchCommand; import org.eclipse.jgit.api.MergeCommand; +import org.eclipse.jgit.api.MergeResult; import org.eclipse.jgit.api.ResetCommand; import org.eclipse.jgit.api.Status; import org.eclipse.jgit.api.StatusCommand; @@ -44,6 +47,7 @@ import org.eclipse.jgit.lib.StoredConfig; import org.eclipse.jgit.transport.CredentialItem; import org.eclipse.jgit.transport.CredentialsProvider; import org.eclipse.jgit.transport.FetchResult; +import org.eclipse.jgit.transport.ReceiveCommand; import org.eclipse.jgit.transport.TrackingRefUpdate; import org.eclipse.jgit.transport.URIish; import org.eclipse.jgit.transport.UsernamePasswordCredentialsProvider; @@ -82,7 +86,7 @@ import static org.mockito.Mockito.when; public class JGitEnvironmentRepositoryTests { private StandardEnvironment environment = new StandardEnvironment(); - private JGitEnvironmentRepository repository = new JGitEnvironmentRepository(this.environment); + private JGitEnvironmentRepository repository; private File basedir = new File("target/config"); @@ -92,6 +96,7 @@ public class JGitEnvironmentRepositoryTests { @Before public void init() throws Exception { String uri = ConfigServerTestUtils.prepareLocalRepo(); + this.repository = new JGitEnvironmentRepository(this.environment); this.repository.setUri(uri); if (this.basedir.exists()) { FileUtils.delete(this.basedir, FileUtils.RECURSIVE | FileUtils.RETRY); @@ -385,6 +390,8 @@ public class JGitEnvironmentRepositoryTests { SearchPathLocator.Locations locations = this.repository.getLocations("bar", "staging", null); assertEquals(locations.getVersion(), newObjectId.getName()); + + verify(git, times(0)).branchDelete(); } @Test @@ -441,6 +448,8 @@ public class JGitEnvironmentRepositoryTests { SearchPathLocator.Locations locations = this.repository.getLocations("bar", "staging", "master"); assertEquals(locations.getVersion(),newObjectId.getName()); + + verify(git, times(0)).branchDelete(); } @Test @@ -509,6 +518,8 @@ public class JGitEnvironmentRepositoryTests { SearchPathLocator.Locations locations = this.repository.getLocations("bar", "staging", "master"); assertEquals(locations.getVersion(), newObjectId.getName()); + + verify(git, times(0)).branchDelete(); } @Test @@ -716,6 +727,110 @@ public class JGitEnvironmentRepositoryTests { verify(fetchCommand, times(1)).setTransportConfigCallback(configCallback); } + @Test + public void shouldSetRemoveBranchesFlagToFetchCommand() throws Exception { + Git mockGit = mock(Git.class); + FetchCommand fetchCommand = mock(FetchCommand.class); + + when(mockGit.fetch()).thenReturn(fetchCommand); + when(fetchCommand.call()).thenReturn(mock(FetchResult.class)); + + repository.setGitFactory(new MockGitFactory(mockGit, mock(CloneCommand.class))); + repository.setUri("http://somegitserver/somegitrepo"); + repository.setDeleteUntrackedBranches(true); + + repository.fetch(mockGit, "master"); + + verify(fetchCommand, times(1)).setRemoveDeletedRefs(true); + verify(fetchCommand, times(1)).call(); + } + + @Test + public void shouldHandleExceptionWhileRemovingBranches() throws Exception { + Git git = mock(Git.class); + CloneCommand cloneCommand = mock(CloneCommand.class); + MockGitFactory factory = new MockGitFactory(git, cloneCommand); + this.repository.setGitFactory(factory); + this.repository.setDeleteUntrackedBranches(true); + + // refresh()->shouldPull + StatusCommand statusCommand = mock(StatusCommand.class); + Status status = mock(Status.class); + when(git.status()).thenReturn(statusCommand); + Repository repository = mock(Repository.class); + when(git.getRepository()).thenReturn(repository); + StoredConfig storedConfig = mock(StoredConfig.class); + when(repository.getConfig()).thenReturn(storedConfig); + when(storedConfig.getString("remote", "origin", "url")).thenReturn("http://example/git"); + when(statusCommand.call()).thenReturn(status); + when(status.isClean()).thenReturn(true); + + // refresh()->fetch + FetchCommand fetchCommand = mock(FetchCommand.class); + FetchResult fetchResult = mock(FetchResult.class); + + TrackingRefUpdate trackingRefUpdate = mock(TrackingRefUpdate.class); + Collection trackingRefUpdates = Collections.singletonList(trackingRefUpdate); + + when(git.fetch()).thenReturn(fetchCommand); + when(fetchCommand.setRemote(anyString())).thenReturn(fetchCommand); + when(fetchCommand.call()).thenReturn(fetchResult); + when(fetchResult.getTrackingRefUpdates()).thenReturn(trackingRefUpdates); + + // refresh()->deleteBranch + ReceiveCommand receiveCommand = mock(ReceiveCommand.class); + when(trackingRefUpdate.asReceiveCommand()).thenReturn(receiveCommand); + when(receiveCommand.getType()).thenReturn(ReceiveCommand.Type.DELETE); + when(trackingRefUpdate.getLocalName()).thenReturn("refs/remotes/origin/feature/deletedBranchFromOrigin"); + + DeleteBranchCommand deleteBranchCommand = mock(DeleteBranchCommand.class); + when(git.branchDelete()).thenReturn(deleteBranchCommand); + when(deleteBranchCommand.setBranchNames(eq("feature/deletedBranchFromOrigin"))).thenReturn(deleteBranchCommand); + when(deleteBranchCommand.setForce(true)).thenReturn(deleteBranchCommand); + when(deleteBranchCommand.call()).thenThrow(new NotMergedException());// here + // is + // our + // exception + // we + // are + // testing + + // refresh()->checkout + CheckoutCommand checkoutCommand = mock(CheckoutCommand.class); + // refresh()->checkout->containsBranch + ListBranchCommand listBranchCommand = mock(ListBranchCommand.class); + when(git.checkout()).thenReturn(checkoutCommand); + when(git.branchList()).thenReturn(listBranchCommand); + List refs = new ArrayList<>(); + Ref ref = mock(Ref.class); + refs.add(ref); + when(ref.getName()).thenReturn("/master"); + when(listBranchCommand.call()).thenReturn(refs); + + // refresh()->merge + MergeResult mergeResult = mock(MergeResult.class); + MergeResult.MergeStatus mergeStatus = mock(MergeResult.MergeStatus.class); + MergeCommand mergeCommand = mock(MergeCommand.class); + when(git.merge()).thenReturn(mergeCommand); + when(mergeCommand.call()).thenReturn(mergeResult); + when(mergeResult.getMergeStatus()).thenReturn(mergeStatus); + when(mergeStatus.isSuccessful()).thenReturn(true); + + // refresh()->return + Ref headRef = mock(Ref.class); + when(repository.getRef(anyString())).thenReturn(headRef); + + ObjectId newObjectId = ObjectId.fromRaw(new int[]{1, 2, 3, 4, 5}); + when(headRef.getObjectId()).thenReturn(newObjectId); + + SearchPathLocator.Locations locations = this.repository.getLocations("bar", "staging", "master"); + assertEquals(locations.getVersion(), newObjectId.getName()); + + verify(deleteBranchCommand).setBranchNames(eq("feature/deletedBranchFromOrigin")); + verify(deleteBranchCommand).setForce(true); + verify(deleteBranchCommand).call(); + } + class MockCloneCommand extends CloneCommand { private Git mockGit;