fix: Connection timeout for git connect when the git repo is behind a firewall (#12819)

* Use non blocking IO for checking the repo status

* Cache the results

* Update tests for the util class method
This commit is contained in:
Anagh Hegde 2022-04-13 16:03:28 +05:30 committed by GitHub
parent 2abb7bb822
commit 5c17422956
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 81 additions and 63 deletions

View File

@ -1,13 +1,19 @@
package com.appsmith.server.helpers; package com.appsmith.server.helpers;
import com.appsmith.server.dtos.ResponseDTO;
import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithError;
import com.appsmith.server.exceptions.AppsmithException; import com.appsmith.server.exceptions.AppsmithException;
import org.eclipse.jgit.util.StringUtils; import org.eclipse.jgit.util.StringUtils;
import org.springframework.core.ParameterizedTypeReference;
import org.springframework.web.reactive.function.client.WebClient;
import reactor.core.publisher.Mono;
import reactor.netty.http.client.HttpClientRequest;
import java.io.IOException; import java.io.IOException;
import java.net.HttpURLConnection; import java.net.HttpURLConnection;
import java.net.URL; import java.net.URL;
import java.time.Duration;
import java.util.regex.Matcher; import java.util.regex.Matcher;
import java.util.regex.Pattern; import java.util.regex.Pattern;
@ -57,12 +63,23 @@ public class GitUtils {
* @return if the repo is public * @return if the repo is public
* @throws IOException exception thrown during openConnection * @throws IOException exception thrown during openConnection
*/ */
public static boolean isRepoPrivate(String remoteHttpsUrl) throws IOException { public static Mono<Boolean> isRepoPrivate(String remoteHttpsUrl) {
URL url = new URL(remoteHttpsUrl); return WebClient
HttpURLConnection huc = (HttpURLConnection) url.openConnection(); .create(remoteHttpsUrl)
int responseCode = huc.getResponseCode(); .get()
.httpRequest(httpRequest -> {
return !(HttpURLConnection.HTTP_OK == responseCode || HttpURLConnection.HTTP_ACCEPTED == responseCode); HttpClientRequest reactorRequest = httpRequest.getNativeRequest();
reactorRequest.responseTimeout(Duration.ofSeconds(2));
})
.exchange()
.flatMap(response -> {
if (response.statusCode().is2xxSuccessful()) {
return Mono.just(Boolean.FALSE);
} else {
return Mono.just(Boolean.TRUE);
}
})
.onErrorResume(throwable -> Mono.just(Boolean.TRUE));
} }
/** /**

View File

@ -394,20 +394,19 @@ public class GitServiceCEImpl implements GitServiceCE {
// Check if the repo is public for current application and if the user have changed the access after // Check if the repo is public for current application and if the user have changed the access after
// the connection // the connection
final Boolean isRepoPrivate = defaultGitMetadata.getIsRepoPrivate(); final Boolean isRepoPrivate = defaultGitMetadata.getIsRepoPrivate();
Mono<Application> applicationMono = Mono.just(defaultApplication); Mono<Application> applicationMono;
if (Boolean.FALSE.equals(isRepoPrivate)) { if (Boolean.FALSE.equals(isRepoPrivate)) {
try { applicationMono = GitUtils.isRepoPrivate(defaultGitMetadata.getBrowserSupportedRemoteUrl())
defaultGitMetadata.setIsRepoPrivate( .flatMap(isPrivate -> {
GitUtils.isRepoPrivate(defaultGitMetadata.getBrowserSupportedRemoteUrl()) defaultGitMetadata.setIsRepoPrivate(isPrivate);
); if (!isPrivate.equals(defaultGitMetadata.getIsRepoPrivate())) {
if (!isRepoPrivate.equals(defaultGitMetadata.getIsRepoPrivate())) { return applicationService.save(defaultApplication);
applicationMono = applicationService.save(defaultApplication); } else {
} else { return Mono.just(defaultApplication);
return applicationMono; }
} });
} catch (IOException e) { } else {
log.debug("Error while checking if the repo is private: ", e); return Mono.just(defaultApplication);
}
} }
// Check if the private repo count is less than the allowed repo count // Check if the private repo count is less than the allowed repo count
@ -643,17 +642,15 @@ public class GitServiceCEImpl implements GitServiceCE {
.switchIfEmpty(Mono.error(new AppsmithException(AppsmithError.INVALID_GIT_CONFIGURATION, GIT_PROFILE_ERROR))); .switchIfEmpty(Mono.error(new AppsmithException(AppsmithError.INVALID_GIT_CONFIGURATION, GIT_PROFILE_ERROR)));
final String browserSupportedUrl = GitUtils.convertSshUrlToBrowserSupportedUrl(gitConnectDTO.getRemoteUrl()); final String browserSupportedUrl = GitUtils.convertSshUrlToBrowserSupportedUrl(gitConnectDTO.getRemoteUrl());
boolean isRepoPrivateTemp = true;
try {
isRepoPrivateTemp = GitUtils.isRepoPrivate(browserSupportedUrl);
} catch (IOException e) {
log.debug("Error while checking if the repo is private: ", e);
}
final boolean isRepoPrivate = isRepoPrivateTemp; Mono<Boolean> isPrivateRepoMono = GitUtils.isRepoPrivate(browserSupportedUrl).cache();
Mono<Application> connectApplicationMono = profileMono Mono<Application> connectApplicationMono = profileMono
.then(getApplicationById(defaultApplicationId)) .then(getApplicationById(defaultApplicationId).zipWith(isPrivateRepoMono))
.flatMap(application -> { .flatMap(tuple -> {
Application application = tuple.getT1();
boolean isRepoPrivate = tuple.getT2();
// Check if the repo is public // Check if the repo is public
if(!isRepoPrivate) { if(!isRepoPrivate) {
return Mono.just(application); return Mono.just(application);
@ -733,8 +730,10 @@ public class GitServiceCEImpl implements GitServiceCE {
final String applicationId = application.getId(); final String applicationId = application.getId();
final String orgId = application.getOrganizationId(); final String orgId = application.getOrganizationId();
try { try {
return fileUtils.checkIfDirectoryIsEmpty(repoPath) return fileUtils.checkIfDirectoryIsEmpty(repoPath).zipWith(isPrivateRepoMono)
.flatMap(isEmpty -> { .flatMap(objects -> {
boolean isEmpty = objects.getT1();
boolean isRepoPrivate = objects.getT2();
if (!isEmpty) { if (!isEmpty) {
return addAnalyticsForGitOperation( return addAnalyticsForGitOperation(
AnalyticsEvents.GIT_CONNECT.getEventName(), AnalyticsEvents.GIT_CONNECT.getEventName(),
@ -1825,25 +1824,21 @@ public class GitServiceCEImpl implements GitServiceCE {
return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, "Invalid organization id")); return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, "Invalid organization id"));
} }
boolean isRepoPrivateTemp;
try {
isRepoPrivateTemp = GitUtils.isRepoPrivate(GitUtils.convertSshUrlToBrowserSupportedUrl(gitConnectDTO.getRemoteUrl()));
} catch (IOException e) {
log.error("Error while checking if the repo is private: ", e);
isRepoPrivateTemp = true;
}
final boolean isRepoPrivate = isRepoPrivateTemp;
final String repoName = GitUtils.getRepoName(gitConnectDTO.getRemoteUrl()); final String repoName = GitUtils.getRepoName(gitConnectDTO.getRemoteUrl());
Mono<Boolean> isPrivateRepoMono = GitUtils.isRepoPrivate(GitUtils.convertSshUrlToBrowserSupportedUrl(gitConnectDTO.getRemoteUrl())).cache();
Mono<ApplicationImportDTO> importedApplicationMono = getSSHKeyForCurrentUser() Mono<ApplicationImportDTO> importedApplicationMono = getSSHKeyForCurrentUser()
.zipWith(isPrivateRepoMono)
.switchIfEmpty(Mono.error(new AppsmithException(AppsmithError.INVALID_GIT_CONFIGURATION, .switchIfEmpty(Mono.error(new AppsmithException(AppsmithError.INVALID_GIT_CONFIGURATION,
"Unable to find git configuration for logged-in user. Please contact Appsmith team for support"))) "Unable to find git configuration for logged-in user. Please contact Appsmith team for support")))
//Check the limit for number of private repo //Check the limit for number of private repo
.flatMap(gitAuth -> { .flatMap(tuple -> {
// Check if the repo is public // Check if the repo is public
Application newApplication = new Application(); Application newApplication = new Application();
newApplication.setName(repoName); newApplication.setName(repoName);
newApplication.setOrganizationId(organizationId); newApplication.setOrganizationId(organizationId);
newApplication.setGitApplicationMetadata(new GitApplicationMetadata()); newApplication.setGitApplicationMetadata(new GitApplicationMetadata());
GitAuth gitAuth = tuple.getT1();
boolean isRepoPrivate = tuple.getT2();
Mono<Application> applicationMono = applicationPageService Mono<Application> applicationMono = applicationPageService
.createOrUpdateSuffixedApplication(newApplication, newApplication.getName(), 0); .createOrUpdateSuffixedApplication(newApplication, newApplication.getName(), 0);
if(!isRepoPrivate) { if(!isRepoPrivate) {
@ -1905,7 +1900,10 @@ public class GitServiceCEImpl implements GitServiceCE {
}); });
return defaultBranchMono return defaultBranchMono
.flatMap(defaultBranch -> { .zipWith(isPrivateRepoMono)
.flatMap(tuple2 -> {
String defaultBranch = tuple2.getT1();
boolean isRepoPrivate = tuple2.getT2();
GitApplicationMetadata gitApplicationMetadata = new GitApplicationMetadata(); GitApplicationMetadata gitApplicationMetadata = new GitApplicationMetadata();
gitApplicationMetadata.setGitAuth(gitAuth); gitApplicationMetadata.setGitAuth(gitAuth);
gitApplicationMetadata.setDefaultApplicationId(application.getId()); gitApplicationMetadata.setDefaultApplicationId(application.getId());
@ -2300,19 +2298,14 @@ public class GitServiceCEImpl implements GitServiceCE {
.flatMap(application -> { .flatMap(application -> {
GitApplicationMetadata gitData = application.getGitApplicationMetadata(); GitApplicationMetadata gitData = application.getGitApplicationMetadata();
final Boolean isRepoPrivate = gitData.getIsRepoPrivate(); final Boolean isRepoPrivate = gitData.getIsRepoPrivate();
try { return GitUtils.isRepoPrivate(application.getGitApplicationMetadata().getBrowserSupportedRemoteUrl())
// Check if user have altered the repo accessibility .flatMap(isPrivate -> {
gitData.setIsRepoPrivate( if (!isRepoPrivate.equals(gitData.getIsRepoPrivate())) {
GitUtils.isRepoPrivate(application.getGitApplicationMetadata().getBrowserSupportedRemoteUrl()) // Repo accessibility is changed
); return applicationService.save(application);
if (!isRepoPrivate.equals(gitData.getIsRepoPrivate())) { }
// Repo accessibility is changed return Mono.just(application);
return applicationService.save(application); });
}
} catch (IOException e) {
log.debug("Error while checking if the repo is private: ", e);
}
return Mono.just(application);
}) })
.then(applicationService.getGitConnectedApplicationsCountWithPrivateRepoByOrgId(organizationId)); .then(applicationService.getGitConnectedApplicationsCountWithPrivateRepoByOrgId(organizationId));
} }

View File

@ -4,6 +4,7 @@ import org.junit.Test;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.test.context.junit4.SpringRunner; import org.springframework.test.context.junit4.SpringRunner;
import reactor.test.StepVerifier;
import java.io.IOException; import java.io.IOException;
@ -28,15 +29,23 @@ public class GitUtilsTest {
.isEqualTo("https://example.test.net/user/test/tests/testRepo"); .isEqualTo("https://example.test.net/user/test/tests/testRepo");
} }
@Test @Test
public void isRepoPrivate() throws IOException { public void isRepoPrivate() {
assertThat(GitUtils.isRepoPrivate(GitUtils.convertSshUrlToBrowserSupportedUrl("git@example.com:test/testRepo.git")))
.isEqualTo(Boolean.TRUE); StepVerifier
assertThat(GitUtils.isRepoPrivate(GitUtils.convertSshUrlToBrowserSupportedUrl("git@example.com:test/testRepo.git"))) .create(GitUtils.isRepoPrivate(GitUtils.convertSshUrlToBrowserSupportedUrl("git@github.com:test/testRepo.git")))
.isEqualTo(Boolean.TRUE); .assertNext(isRepoPrivate -> assertThat(isRepoPrivate).isEqualTo(Boolean.TRUE))
assertThat(GitUtils.isRepoPrivate(GitUtils.convertSshUrlToBrowserSupportedUrl("git@example.org:test/testRepo.git"))) .verifyComplete();
.isEqualTo(Boolean.TRUE);
assertThat(GitUtils.isRepoPrivate(GitUtils.convertSshUrlToBrowserSupportedUrl("ssh://git@appsmith.com.git"))) StepVerifier
.isEqualTo(Boolean.FALSE); .create(GitUtils.isRepoPrivate(GitUtils.convertSshUrlToBrowserSupportedUrl("ssh://git@example.test.net:user/test/tests/testRepo.git")))
.assertNext(isRepoPrivate -> assertThat(isRepoPrivate).isEqualTo(Boolean.TRUE))
.verifyComplete();
StepVerifier
.create(GitUtils.isRepoPrivate(GitUtils.convertSshUrlToBrowserSupportedUrl("git@github.com:appsmithorg/appsmith.git")))
.assertNext(isRepoPrivate -> assertThat(isRepoPrivate).isEqualTo(Boolean.FALSE))
.verifyComplete();
} }
@Test @Test

View File

@ -731,7 +731,6 @@ public class GitServiceTest {
Application application1 = this.createApplicationConnectedToGit("private_repo_1", "master", limitPrivateRepoTestOrgId); Application application1 = this.createApplicationConnectedToGit("private_repo_1", "master", limitPrivateRepoTestOrgId);
this.createApplicationConnectedToGit("private_repo_2", "master", limitPrivateRepoTestOrgId); this.createApplicationConnectedToGit("private_repo_2", "master", limitPrivateRepoTestOrgId);
this.createApplicationConnectedToGit("private_repo_3", "master", limitPrivateRepoTestOrgId);
Application testApplication = new Application(); Application testApplication = new Application();
GitApplicationMetadata gitApplicationMetadata = new GitApplicationMetadata(); GitApplicationMetadata gitApplicationMetadata = new GitApplicationMetadata();