Skip to content

Commit

Permalink
Intercept ScmComunicationException on workspace start
Browse files Browse the repository at this point in the history
  • Loading branch information
vinokurig committed Aug 30, 2024
1 parent e04d787 commit 79ea899
Show file tree
Hide file tree
Showing 17 changed files with 80 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ public PersonalAccessToken fetchAndSave(Subject cheUser, String scmServerUrl)

@Override
public Optional<PersonalAccessToken> get(Subject cheUser, String scmServerUrl)
throws ScmConfigurationPersistenceException {
throws ScmConfigurationPersistenceException, ScmCommunicationException {
return doGetPersonalAccessTokens(cheUser, null, scmServerUrl).stream().findFirst();
}

Expand All @@ -174,13 +174,13 @@ public PersonalAccessToken get(String scmServerUrl)
@Override
public Optional<PersonalAccessToken> get(
Subject cheUser, String oAuthProviderName, @Nullable String scmServerUrl)
throws ScmConfigurationPersistenceException {
throws ScmConfigurationPersistenceException, ScmCommunicationException {
return doGetPersonalAccessTokens(cheUser, oAuthProviderName, scmServerUrl).stream().findFirst();
}

private List<PersonalAccessToken> doGetPersonalAccessTokens(
Subject cheUser, @Nullable String oAuthProviderName, @Nullable String scmServerUrl)
throws ScmConfigurationPersistenceException {
throws ScmConfigurationPersistenceException, ScmCommunicationException {
List<PersonalAccessToken> result = new ArrayList<>();
try {
LOG.debug(
Expand Down Expand Up @@ -358,7 +358,8 @@ public void forceRefreshPersonalAccessToken(String scmServerUrl)
}

private void removePreviousTokensIfPresent(Subject subject, String scmServerUrl)
throws ScmConfigurationPersistenceException, UnsatisfiedScmPreconditionException {
throws ScmConfigurationPersistenceException, UnsatisfiedScmPreconditionException,
ScmCommunicationException {
List<PersonalAccessToken> personalAccessTokens =
doGetPersonalAccessTokens(subject, null, scmServerUrl);
for (int i = 1; i < personalAccessTokens.size(); i++) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
import org.eclipse.che.api.workspace.server.spi.InfrastructureException;
import org.eclipse.che.api.workspace.server.spi.NamespaceResolutionContext;
import org.eclipse.che.workspace.infrastructure.kubernetes.CheServerKubernetesClientFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* This {@link NamespaceConfigurator} ensures that Secret {@link
Expand All @@ -45,6 +47,8 @@ public class CredentialsSecretConfigurator implements NamespaceConfigurator {
private static final String MERGED_GIT_CREDENTIALS_SECRET_NAME =
"devworkspace-merged-git-credentials";

private static final Logger LOG = LoggerFactory.getLogger(CredentialsSecretConfigurator.class);

@Inject
public CredentialsSecretConfigurator(
CheServerKubernetesClientFactory cheServerKubernetesClientFactory,
Expand Down Expand Up @@ -79,7 +83,7 @@ public void configure(NamespaceResolutionContext namespaceResolutionContext, Str
| ScmConfigurationPersistenceException
| UnsatisfiedScmPreconditionException
| ScmUnauthorizedException e) {
throw new RuntimeException(e);
LOG.error(e.getMessage(), e);
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,15 @@
import javax.inject.Singleton;
import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenFetcher;
import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenManager;
import org.eclipse.che.api.factory.server.scm.exception.ScmCommunicationException;
import org.eclipse.che.api.factory.server.scm.exception.ScmConfigurationPersistenceException;
import org.eclipse.che.api.workspace.server.spi.InfrastructureException;
import org.eclipse.che.api.workspace.server.spi.NamespaceResolutionContext;
import org.eclipse.che.commons.env.EnvironmentContext;
import org.eclipse.che.commons.subject.Subject;
import org.eclipse.che.workspace.infrastructure.kubernetes.CheServerKubernetesClientFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* Ensures that OAuth token that are represented by Kubernetes Secrets are valid.
Expand All @@ -44,6 +47,8 @@ public class OAuthTokenSecretsConfigurator implements NamespaceConfigurator {
"app.kubernetes.io/part-of", "che.eclipse.org",
"app.kubernetes.io/component", "scm-personal-access-token");

private static final Logger LOG = LoggerFactory.getLogger(OAuthTokenSecretsConfigurator.class);

@Inject
public OAuthTokenSecretsConfigurator(
CheServerKubernetesClientFactory cheServerKubernetesClientFactory,
Expand Down Expand Up @@ -74,8 +79,8 @@ public void configure(NamespaceResolutionContext namespaceResolutionContext, Str
Subject cheSubject = EnvironmentContext.getCurrent().getSubject();
personalAccessTokenManager.get(
cheSubject, s.getMetadata().getAnnotations().get(ANNOTATION_SCM_URL));
} catch (ScmConfigurationPersistenceException e) {
throw new RuntimeException(e);
} catch (ScmConfigurationPersistenceException | ScmCommunicationException e) {
LOG.error(e.getMessage(), e);
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import org.eclipse.che.api.core.util.LinksHelper;
import org.eclipse.che.api.factory.server.scm.PersonalAccessToken;
import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenManager;
import org.eclipse.che.api.factory.server.scm.exception.ScmCommunicationException;
import org.eclipse.che.api.factory.server.scm.exception.ScmConfigurationPersistenceException;
import org.eclipse.che.api.factory.server.scm.exception.UnsatisfiedScmPreconditionException;
import org.eclipse.che.commons.annotation.Nullable;
Expand Down Expand Up @@ -237,7 +238,7 @@ public OAuthToken getOrRefreshToken(String oauthProvider)
if (tokenOptional.isPresent()) {
return newDto(OAuthToken.class).withToken(tokenOptional.get().getToken());
}
} catch (ScmConfigurationPersistenceException e) {
} catch (ScmConfigurationPersistenceException | ScmCommunicationException e) {
throw new RuntimeException(e);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,8 @@ public Optional<Boolean> isValid(PersonalAccessToken personalAccessToken) {
}

@Override
public Optional<Pair<Boolean, String>> isValid(PersonalAccessTokenParams params) {
public Optional<Pair<Boolean, String>> isValid(PersonalAccessTokenParams params)
throws ScmCommunicationException {
if (!isValidScmServerUrl(params.getScmProviderUrl())) {
LOG.debug("not a valid url {} for current fetcher ", params.getScmProviderUrl());
return Optional.empty();
Expand All @@ -183,7 +184,7 @@ public Optional<Pair<Boolean, String>> isValid(PersonalAccessTokenParams params)
user = azureDevOpsApiClient.getUserWithPAT(params.getToken(), params.getOrganization());
}
return Optional.of(Pair.of(Boolean.TRUE, user.getEmailAddress()));
} catch (ScmItemNotFoundException | ScmCommunicationException | ScmBadRequestException e) {
} catch (ScmItemNotFoundException | ScmBadRequestException e) {
return Optional.empty();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,8 @@ public Optional<Boolean> isValid(PersonalAccessToken accessToken)
}

@Override
public Optional<Pair<Boolean, String>> isValid(PersonalAccessTokenParams params) {
public Optional<Pair<Boolean, String>> isValid(PersonalAccessTokenParams params)
throws ScmCommunicationException {
if (!bitbucketServerApiClient.isConnected(params.getScmProviderUrl())) {
// If BitBucket oAuth is not configured check the manually added user namespace token.
HttpBitbucketServerApiClient apiClient =
Expand All @@ -180,7 +181,7 @@ public Optional<Pair<Boolean, String>> isValid(PersonalAccessTokenParams params)
try {
BitbucketUser user = bitbucketServerApiClient.getUser(params.getToken());
return Optional.of(Pair.of(Boolean.TRUE, user.getName()));
} catch (ScmItemNotFoundException | ScmUnauthorizedException | ScmCommunicationException e) {
} catch (ScmItemNotFoundException | ScmUnauthorizedException e) {
return Optional.empty();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ private boolean isUserTokenPresent(String repositoryUrl) {
Optional<PersonalAccessToken> token =
personalAccessTokenManager.get(EnvironmentContext.getCurrent().getSubject(), serverUrl);
return token.isPresent() && token.get().getScmTokenName().equals(OAUTH_PROVIDER_NAME);
} catch (ScmConfigurationPersistenceException exception) {
} catch (ScmConfigurationPersistenceException | ScmCommunicationException exception) {
return false;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,8 @@ public Optional<Boolean> isValid(PersonalAccessToken personalAccessToken) {
}

@Override
public Optional<Pair<Boolean, String>> isValid(PersonalAccessTokenParams params) {
public Optional<Pair<Boolean, String>> isValid(PersonalAccessTokenParams params)
throws ScmCommunicationException {
if (!bitbucketApiClient.isConnected(params.getScmProviderUrl())) {
LOG.debug("not a valid url {} for current fetcher ", params.getScmProviderUrl());
return Optional.empty();
Expand All @@ -184,7 +185,7 @@ public Optional<Pair<Boolean, String>> isValid(PersonalAccessTokenParams params)
Pair.of(
isValidScope(Sets.newHashSet(pair.second)) ? Boolean.TRUE : Boolean.FALSE,
pair.first));
} catch (ScmItemNotFoundException | ScmCommunicationException | ScmBadRequestException e) {
} catch (ScmItemNotFoundException | ScmBadRequestException e) {
return Optional.empty();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,8 @@ public Optional<Boolean> isValid(PersonalAccessToken personalAccessToken) {
}

@Override
public Optional<Pair<Boolean, String>> isValid(PersonalAccessTokenParams params) {
public Optional<Pair<Boolean, String>> isValid(PersonalAccessTokenParams params)
throws ScmCommunicationException {
GithubApiClient apiClient;
if (githubApiClient.isConnected(params.getScmProviderUrl())) {
// The url from the token has the same url as the api client, no need to create a new one.
Expand All @@ -247,7 +248,7 @@ public Optional<Pair<Boolean, String>> isValid(PersonalAccessTokenParams params)
GithubUser user = apiClient.getUser(params.getToken());
return Optional.of(Pair.of(Boolean.TRUE, user.getLogin()));
}
} catch (ScmItemNotFoundException | ScmCommunicationException | ScmBadRequestException e) {
} catch (ScmItemNotFoundException | ScmBadRequestException e) {
return Optional.empty();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ private boolean isUserTokenPresent(String repositoryUrl) {
PersonalAccessToken accessToken = token.get();
return accessToken.getScmTokenName().equals(providerName);
}
} catch (ScmConfigurationPersistenceException exception) {
} catch (ScmConfigurationPersistenceException | ScmCommunicationException exception) {
return false;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,8 @@ public Optional<Boolean> isValid(PersonalAccessToken personalAccessToken) {
}

@Override
public Optional<Pair<Boolean, String>> isValid(PersonalAccessTokenParams params) {
public Optional<Pair<Boolean, String>> isValid(PersonalAccessTokenParams params)
throws ScmCommunicationException {
GitlabApiClient gitlabApiClient = getApiClient(params.getScmProviderUrl());
if (gitlabApiClient == null || !gitlabApiClient.isConnected(params.getScmProviderUrl())) {
if (OAUTH_PROVIDER_NAME.equals(params.getScmTokenName())) {
Expand All @@ -234,7 +235,7 @@ public Optional<Pair<Boolean, String>> isValid(PersonalAccessTokenParams params)
// latest GitLab version, we just perform check by accessing something from API.
// TODO: add PAT scope validation
return Optional.of(Pair.of(Boolean.TRUE, user.getUsername()));
} catch (ScmItemNotFoundException | ScmCommunicationException | ScmBadRequestException e) {
} catch (ScmItemNotFoundException | ScmBadRequestException e) {
return Optional.empty();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ private boolean isUserTokenPresent(String repositoryUrl) {
PersonalAccessToken accessToken = token.get();
return accessToken.getScmTokenName().equals(OAUTH_PROVIDER_NAME);
}
} catch (ScmConfigurationPersistenceException exception) {
} catch (ScmConfigurationPersistenceException | ScmCommunicationException exception) {
return false;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,15 @@
*/
package org.eclipse.che.api.factory.server;

import static com.google.common.base.Strings.isNullOrEmpty;
import static org.eclipse.che.dto.server.DtoFactory.newDto;

import java.util.Map;
import org.eclipse.che.api.core.ApiException;
import org.eclipse.che.api.core.BadRequestException;
import org.eclipse.che.api.core.ServerException;
import org.eclipse.che.api.core.UnauthorizedException;
import org.eclipse.che.api.core.rest.shared.dto.ExtendedError;
import org.eclipse.che.api.factory.server.scm.exception.ScmCommunicationException;
import org.eclipse.che.api.factory.server.scm.exception.ScmUnauthorizedException;
import org.eclipse.che.api.factory.server.scm.exception.UnknownScmProviderException;
Expand Down Expand Up @@ -48,6 +52,16 @@ public static ApiException toApiException(ScmUnauthorizedException scmUnauthoriz
+ scmUnauthorizedException.getMessage());
}

public static ApiException toApiException(ScmCommunicationException scmCommunicationException) {
ApiException apiException = getApiException(scmCommunicationException);
return (apiException != null)
? apiException
: new ServerException(
"Error occurred during SCM communication."
+ "Cause: "
+ scmCommunicationException.getMessage());
}

public static ApiException toApiException(
DevfileException devfileException, DevfileLocation location) {
ApiException cause = getApiException(devfileException.getCause());
Expand All @@ -70,15 +84,25 @@ private static ApiException getApiException(Throwable throwable) {
"oauth_version", scmCause.getOauthVersion(),
"oauth_provider", scmCause.getOauthProvider(),
"oauth_authentication_url", scmCause.getAuthenticateUrl()));
} else if (throwable instanceof UnknownScmProviderException) {
return new ServerException(
"Provided location is unknown or misconfigured on the server side. Error message: "
+ throwable.getMessage());
} else if (throwable instanceof ScmCommunicationException) {
return new ServerException(
"There is an error happened when communicate with SCM server. Error message: "
+ throwable.getMessage());
} else {
if (throwable instanceof UnknownScmProviderException) {
return new ServerException(
appendErrorMessage(
"Provided location is unknown or misconfigured on the server side",
throwable.getMessage()));
} else if (throwable instanceof ScmCommunicationException) {
return new ApiException(
newDto(ExtendedError.class)
.withMessage(
appendErrorMessage(
"Error occurred during SCM communication.", throwable.getMessage()))
.withErrorCode(404));
}
}
return null;
}

private static String appendErrorMessage(String message, String errorMessage) {
return message + (isNullOrEmpty(errorMessage) ? "" : " Error message: " + errorMessage);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -159,13 +159,14 @@ public void refreshToken(@Parameter(description = "Factory url") @QueryParam("ur
personalAccessTokenManager.getAndStore(scmServerUrl);
}
}
} catch (ScmCommunicationException
| ScmConfigurationPersistenceException
} catch (ScmConfigurationPersistenceException
| UnknownScmProviderException
| UnsatisfiedScmPreconditionException e) {
throw new ApiException(e);
} catch (ScmUnauthorizedException e) {
throw toApiException(e);
} catch (ScmCommunicationException e) {
throw toApiException(e);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ Optional<Boolean> isValid(PersonalAccessToken personalAccessToken)
* the token has expected scope of permissions, false if the token scopes does not match the
* expected ones. Empty optional if {@link PersonalAccessTokenFetcher} is not able to confirm
* or deny that token is valid.
* @throws ScmCommunicationException - problem occurred during communication with SCM server.
*/
Optional<Pair<Boolean, String>> isValid(PersonalAccessTokenParams params);
Optional<Pair<Boolean, String>> isValid(PersonalAccessTokenParams params)
throws ScmCommunicationException;
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,10 @@ PersonalAccessToken fetchAndSave(Subject cheUser, String scmServerUrl)
* @return personal access token
* @throws ScmConfigurationPersistenceException - problem occurred during communication with
* permanent storage.
* @throws ScmCommunicationException - problem occurred during communication with SCM server.
*/
Optional<PersonalAccessToken> get(Subject cheUser, String scmServerUrl)
throws ScmConfigurationPersistenceException;
throws ScmConfigurationPersistenceException, ScmCommunicationException;

/**
* Gets {@link PersonalAccessToken} from permanent storage.
Expand Down Expand Up @@ -79,10 +80,11 @@ PersonalAccessToken get(String scmServerUrl)
* @return personal access token
* @throws ScmConfigurationPersistenceException - problem occurred during communication with
* permanent storage.
* @throws ScmCommunicationException - problem occurred during communication with SCM server.
*/
Optional<PersonalAccessToken> get(
Subject cheUser, String oAuthProviderName, @Nullable String scmServerUrl)
throws ScmConfigurationPersistenceException;
throws ScmConfigurationPersistenceException, ScmCommunicationException;

/**
* Gets {@link PersonalAccessToken} from permanent storage. If the token is not found try to fetch
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public boolean isValid(PersonalAccessToken personalAccessToken)
* fetchers return an scm username, return it. Otherwise, return null.
*/
public Optional<String> getScmUsername(PersonalAccessTokenParams params)
throws UnknownScmProviderException {
throws UnknownScmProviderException, ScmCommunicationException {
for (PersonalAccessTokenFetcher fetcher : personalAccessTokenFetchers) {
Optional<Pair<Boolean, String>> isValid = fetcher.isValid(params);
if (isValid.isPresent() && isValid.get().first) {
Expand Down

0 comments on commit 79ea899

Please sign in to comment.