Merge branch 'tdanylchuk-feature/delete-untracked-branches'

This commit is contained in:
Spencer Gibb
2018-03-20 22:38:18 -04:00
6 changed files with 291 additions and 14 deletions

View File

@@ -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

View File

@@ -26,6 +26,7 @@ public class JGitEnvironmentProperties extends AbstractScmAccessorProperties {
private boolean cloneOnStart = false;
private boolean forcePull;
private int timeout = 5;
private boolean deleteUntrackedBranches = false;
public JGitEnvironmentProperties() {
super();
@@ -55,4 +56,12 @@ public class JGitEnvironmentProperties extends AbstractScmAccessorProperties {
public void setTimeout(int timeout) {
this.timeout = timeout;
}
public boolean isDeleteUntrackedBranches() {
return deleteUntrackedBranches;
}
public void setDeleteUntrackedBranches(boolean deleteUntrackedBranches) {
this.deleteUntrackedBranches = deleteUntrackedBranches;
}
}

View File

@@ -17,6 +17,9 @@ 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.HashSet;
import java.util.List;
import java.util.Set;
@@ -25,6 +28,7 @@ import com.jcraft.jsch.Session;
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;
@@ -46,8 +50,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;
@@ -56,8 +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 static org.springframework.util.StringUtils.hasText;
/**
@@ -74,6 +82,8 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository
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.
@@ -107,12 +117,18 @@ 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, JGitEnvironmentProperties properties) {
super(environment, properties);
this.cloneOnStart = properties.getCloneOnStart();
this.defaultLabel = properties.getDefaultLabel();
this.forcePull = properties.getForcePull();
this.timeout = properties.getTimeout();
this.deleteUntrackedBranches = properties.isDeleteUntrackedBranches();
}
public boolean isCloneOnStart() {
@@ -164,6 +180,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) {
@@ -193,9 +217,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
@@ -203,7 +229,7 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository
if (!isClean(git, label)) {
logger.warn("The local repository is dirty or ahead of origin. Resetting"
+ " it to origin/" + label + ".");
resetHard(git, label, "refs/remotes/origin/" + label);
resetHard(git, label, LOCAL_BRANCH_REF_PREFIX + label);
}
}
}
@@ -260,6 +286,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<String> deleteUntrackedLocalBranches(Collection<TrackingRefUpdate> trackingRefUpdates, Git git) {
if (CollectionUtils.isEmpty(trackingRefUpdates)) {
return Collections.emptyList();
}
Collection<String> 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<String> deleteBranches(Git git, Collection<String> 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<String> 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)) {
@@ -298,7 +373,7 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository
Set<String> 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")
@@ -318,6 +393,7 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository
FetchCommand fetch = git.fetch();
fetch.setRemote("origin");
fetch.setTagOpt(TagOpt.FETCH_TAGS);
fetch.setRemoveDeletedRefs(deleteUntrackedBranches);
configureCommand(fetch);
try {
@@ -497,9 +573,8 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository
private boolean isClean(Git git, String label) {
StatusCommand status = git.status();
try {
boolean isBranchAhead = false;
BranchTrackingStatus trackingStatus = BranchTrackingStatus.of(git.getRepository(), label);
isBranchAhead = trackingStatus != null && trackingStatus.getAheadCount() > 0;
boolean isBranchAhead = trackingStatus != null && trackingStatus.getAheadCount() > 0;
return status.call().isClean() && !isBranchAhead;
}
catch (Exception e) {

View File

@@ -26,6 +26,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
@@ -80,7 +83,13 @@ public class JGitConfigServerTestData {
return this.context;
}
public static JGitConfigServerTestData prepareClonedGitRepository(Class... sources) throws Exception {
public static JGitConfigServerTestData prepareClonedGitRepository(Class... sources)
throws Exception {
return prepareClonedGitRepository(Collections.emptySet(), sources);
}
public static JGitConfigServerTestData prepareClonedGitRepository(Collection<String> additionalProperties, Class... sources)
throws Exception {
//setup remote repository
String remoteUri = ConfigServerTestUtils.prepareLocalRepo();
File remoteRepoDir = ResourceUtils.getFile(remoteUri);
@@ -91,7 +100,7 @@ public class JGitConfigServerTestData {
File clonedRepoDir = new File("target/repos/cloned");
if(clonedRepoDir.exists()) {
FileSystemUtils.deleteRecursively(clonedRepoDir);
}else{
} else {
clonedRepoDir.mkdirs();
}
Git clonedGit = Git.cloneRepository()
@@ -102,8 +111,11 @@ public class JGitConfigServerTestData {
.call();
//setup our test spring application pointing to the local repo
Collection<String> properties = new ArrayList<>(additionalProperties);
properties.add("spring.cloud.config.server.git.uri:" + "file://" + clonedRepoDir.getAbsolutePath());
ConfigurableApplicationContext context = new SpringApplicationBuilder(sources).web(WebApplicationType.NONE)
.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(

View File

@@ -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;
@@ -510,6 +511,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();
testData.getRepository().findOne("bar", "staging", "branchToDelete");
assertNotNull(environment);
}
@Test(expected = NoSuchLabelException.class)
public void testShouldFailIfRemoteBranchWasDeleted() throws Exception {
JGitConfigServerTestData testData = JGitConfigServerTestData
.prepareClonedGitRepository(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 })

View File

@@ -19,22 +19,26 @@ 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;
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.NoMessageException;
import org.eclipse.jgit.api.errors.NotMergedException;
import org.eclipse.jgit.api.errors.TransportException;
import org.eclipse.jgit.lib.ObjectId;
@@ -44,6 +48,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;
@@ -59,13 +64,13 @@ import org.springframework.cloud.config.server.support.GitCredentialsProviderFac
import org.springframework.cloud.config.server.support.PassphraseCredentialsProvider;
import org.springframework.cloud.config.server.test.ConfigServerTestUtils;
import org.springframework.core.env.StandardEnvironment;
import static junit.framework.TestCase.assertTrue;
import static org.hamcrest.Matchers.is;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertThat;
import static org.mockito.AdditionalMatchers.aryEq;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyString;
import static org.mockito.Matchers.eq;
@@ -82,8 +87,7 @@ import static org.mockito.Mockito.when;
public class JGitEnvironmentRepositoryTests {
private StandardEnvironment environment = new StandardEnvironment();
private JGitEnvironmentRepository repository = new JGitEnvironmentRepository(this.environment,
new JGitEnvironmentProperties());
private JGitEnvironmentRepository repository;
private File basedir = new File("target/config");
@@ -93,6 +97,7 @@ public class JGitEnvironmentRepositoryTests {
@Before
public void init() throws Exception {
String uri = ConfigServerTestUtils.prepareLocalRepo();
this.repository = new JGitEnvironmentRepository(this.environment, new JGitEnvironmentProperties());
this.repository.setUri(uri);
if (this.basedir.exists()) {
FileUtils.delete(this.basedir, FileUtils.RECURSIVE | FileUtils.RETRY);
@@ -392,6 +397,8 @@ public class JGitEnvironmentRepositoryTests {
SearchPathLocator.Locations locations = this.repository.getLocations("bar", "staging", null);
assertEquals(locations.getVersion(), newObjectId.getName());
verify(git, times(0)).branchDelete();
}
@Test
@@ -448,6 +455,8 @@ public class JGitEnvironmentRepositoryTests {
SearchPathLocator.Locations locations = this.repository.getLocations("bar", "staging", "master");
assertEquals(locations.getVersion(),newObjectId.getName());
verify(git, times(0)).branchDelete();
}
@Test
@@ -516,6 +525,8 @@ public class JGitEnvironmentRepositoryTests {
SearchPathLocator.Locations locations = this.repository.getLocations("bar", "staging", "master");
assertEquals(locations.getVersion(), newObjectId.getName());
verify(git, times(0)).branchDelete();
}
@Test
@@ -731,6 +742,113 @@ 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));
JGitEnvironmentRepository envRepository = new JGitEnvironmentRepository(this.environment,
new JGitEnvironmentProperties());
envRepository.setGitFactory(new MockGitFactory(mockGit, mock(CloneCommand.class)));
envRepository.setUri("http://somegitserver/somegitrepo");
envRepository.setDeleteUntrackedBranches(true);
envRepository.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<TrackingRefUpdate> 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<Ref> 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
// git.getRepository().findRef("HEAD").getObjectId().getName();
Ref headRef = mock(Ref.class);
when(repository.findRef(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;