Skip to content

Commit

Permalink
fix(core): fix the issues caused due to the use of SpinnakerRetrofitE…
Browse files Browse the repository at this point in the history
…rrorHandler in building FiatService
  • Loading branch information
kirangodishala committed Oct 21, 2024
1 parent efcfe05 commit b7e6a31
Show file tree
Hide file tree
Showing 5 changed files with 371 additions and 181 deletions.
1 change: 1 addition & 0 deletions gate-core/gate-core.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ dependencies {
implementation "io.spinnaker.fiat:fiat-core:$fiatVersion"

implementation "io.spinnaker.kork:kork-core"
implementation "io.spinnaker.kork:kork-retrofit"
implementation "io.spinnaker.kork:kork-web"
implementation "io.spinnaker.kork:kork-security"
implementation "com.netflix.spectator:spectator-api"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
import static retrofit.RetrofitError.Kind.HTTP;

import com.netflix.spinnaker.kork.exceptions.SpinnakerException;
import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException;
import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException;
import java.util.Collection;
import retrofit.RetrofitError;

Expand All @@ -36,6 +38,14 @@ private UpstreamBadRequest(RetrofitError cause) {
error = cause.getBody();
}

private UpstreamBadRequest(SpinnakerHttpException cause) {
super(cause.getMessage(), cause);
this.setRetryable(false);
status = cause.getResponseCode();
url = cause.getUrl();
error = cause.getResponseBody();
}

public int getStatus() {
return status;
}
Expand Down Expand Up @@ -66,4 +76,13 @@ public static RuntimeException classifyError(
return error;
}
}

public static RuntimeException classifyError(SpinnakerServerException error) {
if (error instanceof SpinnakerHttpException
&& ((SpinnakerHttpException) error).getResponseCode() < INTERNAL_SERVER_ERROR.value()) {
return new UpstreamBadRequest((SpinnakerHttpException) error);
} else {
return error;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@
import com.netflix.spinnaker.kork.core.RetrySupport;
import com.netflix.spinnaker.kork.exceptions.SpinnakerException;
import com.netflix.spinnaker.kork.exceptions.SystemException;
import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException;
import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerNetworkException;
import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException;
import com.netflix.spinnaker.security.AuthenticatedRequest;
import com.netflix.spinnaker.security.User;
import java.time.Duration;
Expand Down Expand Up @@ -83,7 +86,7 @@ public void login(final String userId) {
permissionEvaluator.invalidatePermission(userId);
return null;
});
} catch (RetrofitError e) {
} catch (SpinnakerServerException e) {
throw UpstreamBadRequest.classifyError(e);
}
}
Expand All @@ -98,7 +101,7 @@ public void loginWithRoles(final String userId, final Collection<String> roles)
permissionEvaluator.invalidatePermission(userId);
return null;
});
} catch (RetrofitError e) {
} catch (SpinnakerServerException e) {
throw UpstreamBadRequest.classifyError(e);
}
}
Expand All @@ -109,7 +112,7 @@ public void logout(String userId) {
try {
getFiatServiceForLogin().logoutUser(userId);
permissionEvaluator.invalidatePermission(userId);
} catch (RetrofitError e) {
} catch (SpinnakerServerException e) {
throw UpstreamBadRequest.classifyError(e);
}
}
Expand All @@ -119,7 +122,7 @@ public void sync() {
if (fiatStatus.isEnabled()) {
try {
getFiatServiceForLogin().sync(List.of());
} catch (RetrofitError e) {
} catch (SpinnakerServerException e) {
throw UpstreamBadRequest.classifyError(e);
}
}
Expand All @@ -133,22 +136,27 @@ public Set<Role.View> getRoles(String userId) {
var permission = permissionEvaluator.getPermission(userId);
var roles = permission != null ? permission.getRoles() : null;
return roles != null ? roles : Set.of();
} catch (RetrofitError e) {
} catch (SpinnakerServerException e) {
throw UpstreamBadRequest.classifyError(e);
}
}

List<UserPermission.View> lookupServiceAccounts(String userId) {
try {
return extendedFiatService.getUserServiceAccounts(userId);
} catch (RetrofitError re) {
var response = re.getResponse();
if (response != null && response.getStatus() == HttpStatus.NOT_FOUND.value()) {
} catch (SpinnakerNetworkException e) {
throw new SystemException("getUserServiceAccounts failed", e).setRetryable(true);
} catch (SpinnakerHttpException e) {
if (e.getResponseCode() == 404) {
return List.of();
}
boolean shouldRetry =
response == null || HttpStatus.valueOf(response.getStatus()).is5xxServerError();
throw new SystemException("getUserServiceAccounts failed", re).setRetryable(shouldRetry);
if (HttpStatus.valueOf(e.getResponseCode()).is5xxServerError()) {
throw new SystemException("getUserServiceAccounts failed", e).setRetryable(true);
}
throw UpstreamBadRequest.classifyError(e);
} catch (SpinnakerServerException e) {
e.setRetryable(false);
throw UpstreamBadRequest.classifyError(e);
}
}

Expand Down Expand Up @@ -217,8 +225,8 @@ public List<String> getServiceAccounts(@SpinnakerUser User user) {
return permission.getServiceAccounts().stream()
.map(ServiceAccount.View::getName)
.collect(Collectors.toList());
} catch (RetrofitError re) {
throw UpstreamBadRequest.classifyError(re);
} catch (SpinnakerServerException e) {
throw UpstreamBadRequest.classifyError(e);
}
}

Expand Down

This file was deleted.

Loading

0 comments on commit b7e6a31

Please sign in to comment.