Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(retrofit2): replace retrofit client with retrofit2 client #1195

Merged
merged 2 commits into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions fiat-api/fiat-api.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,8 @@ dependencies {
implementation "io.spinnaker.kork:kork-telemetry"
implementation "io.spinnaker.kork:kork-web"
implementation "io.spinnaker.kork:kork-retrofit"
implementation "com.squareup.retrofit:retrofit"
implementation "com.squareup.retrofit:converter-jackson"
implementation "com.jakewharton.retrofit:retrofit1-okhttp3-client"
implementation "com.squareup.retrofit2:retrofit"
implementation "com.squareup.retrofit2:converter-jackson"
implementation "org.apache.commons:commons-lang3"

compileOnly "javax.servlet:javax.servlet-api"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,11 @@

import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.jakewharton.retrofit.Ok3Client;
import com.netflix.spinnaker.config.DefaultServiceEndpoint;
import com.netflix.spinnaker.config.ErrorConfiguration;
import com.netflix.spinnaker.config.okhttp3.OkHttpClientProvider;
import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerRetrofitErrorHandler;
import com.netflix.spinnaker.config.OkHttp3ClientConfiguration;
import com.netflix.spinnaker.kork.retrofit.ErrorHandlingExecutorCallAdapterFactory;
import com.netflix.spinnaker.kork.web.exceptions.ExceptionMessageDecorator;
import com.netflix.spinnaker.okhttp.SpinnakerRequestInterceptor;
import com.netflix.spinnaker.retrofit.Slf4jRetrofitLogger;
import lombok.Setter;
import lombok.val;
import okhttp3.OkHttpClient;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
import org.springframework.boot.context.properties.EnableConfigurationProperties;
Expand All @@ -46,9 +39,8 @@
import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter;
import org.springframework.security.web.authentication.AnonymousAuthenticationFilter;
import org.springframework.security.web.authentication.AuthenticationConverter;
import retrofit.Endpoints;
import retrofit.RestAdapter;
import retrofit.converter.JacksonConverter;
import retrofit2.Retrofit;
import retrofit2.converter.jackson.JacksonConverterFactory;

@Import(ErrorConfiguration.class)
@EnableWebSecurity
Expand All @@ -58,33 +50,21 @@
@ComponentScan("com.netflix.spinnaker.fiat.shared")
public class FiatAuthenticationConfig {

@Autowired(required = false)
@Setter
private RestAdapter.LogLevel retrofitLogLevel = RestAdapter.LogLevel.BASIC;
dbyron-sf marked this conversation as resolved.
Show resolved Hide resolved

@Bean
@ConditionalOnMissingBean(FiatService.class) // Allows for override
public FiatService fiatService(
FiatClientConfigurationProperties fiatConfigurationProperties,
SpinnakerRequestInterceptor interceptor,
OkHttpClientProvider okHttpClientProvider) {
OkHttp3ClientConfiguration okHttpClientConfig) {
// New role providers break deserialization if this is not enabled.
val objectMapper = new ObjectMapper();
objectMapper.enable(DeserializationFeature.READ_UNKNOWN_ENUM_VALUES_AS_NULL);
objectMapper.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);

OkHttpClient okHttpClient =
okHttpClientProvider.getClient(
new DefaultServiceEndpoint("fiat", fiatConfigurationProperties.getBaseUrl()));

return new RestAdapter.Builder()
.setEndpoint(Endpoints.newFixedEndpoint(fiatConfigurationProperties.getBaseUrl()))
.setRequestInterceptor(interceptor)
.setClient(new Ok3Client(okHttpClient))
.setConverter(new JacksonConverter(objectMapper))
.setErrorHandler(SpinnakerRetrofitErrorHandler.getInstance())
.setLogLevel(retrofitLogLevel)
.setLog(new Slf4jRetrofitLogger(FiatService.class))
return new Retrofit.Builder()
.baseUrl(fiatConfigurationProperties.getBaseUrl())
.client(okHttpClientConfig.createForRetrofit2().build())
.addCallAdapterFactory(ErrorHandlingExecutorCallAdapterFactory.getInstance())
.addConverterFactory(JacksonConverterFactory.create(objectMapper))
.build()
.create(FiatService.class);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.netflix.spinnaker.fiat.model.resources.Account;
import com.netflix.spinnaker.fiat.model.resources.Authorizable;
import com.netflix.spinnaker.fiat.model.resources.ResourceType;
import com.netflix.spinnaker.kork.retrofit.Retrofit2SyncCall;
import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException;
import com.netflix.spinnaker.kork.telemetry.caffeine.CaffeineStatsCounter;
import com.netflix.spinnaker.security.AccessControlled;
Expand Down Expand Up @@ -185,7 +186,8 @@ public boolean canCreate(String resourceType, Object resource) {
"determine whether " + username + " can create resource " + resource,
() -> {
try {
fiatService.canCreate(username, resourceType, resource);
Retrofit2SyncCall.execute(
fiatService.canCreate(username, resourceType, resource));
return true;
} catch (SpinnakerHttpException e) {
if (e.getResponseCode() == HttpStatus.NOT_FOUND.value()) {
Expand Down Expand Up @@ -312,7 +314,9 @@ public UserPermission.View getPermission(String username) {
try {
return retryHandler.retry(
"getUserPermission for " + loadUserName,
() -> fiatService.getUserPermission(loadUserName));
() ->
Retrofit2SyncCall.execute(
fiatService.getUserPermission(loadUserName)));
} catch (Exception e) {
if (!fiatStatus.isLegacyFallbackEnabled()) {
throw e;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@
import com.netflix.spinnaker.fiat.model.UserPermission;
import java.util.Collection;
import java.util.List;
import retrofit.client.Response;
import retrofit.http.Body;
import retrofit.http.DELETE;
import retrofit.http.GET;
import retrofit.http.POST;
import retrofit.http.PUT;
import retrofit.http.Path;
import retrofit2.Call;
import retrofit2.http.Body;
import retrofit2.http.DELETE;
import retrofit2.http.GET;
import retrofit2.http.POST;
import retrofit2.http.PUT;
import retrofit2.http.Path;

public interface FiatService {

Expand All @@ -34,7 +34,7 @@ public interface FiatService {
* @return The full UserPermission of the user.
*/
@GET("/authorize/{userId}")
UserPermission.View getUserPermission(@Path("userId") String userId);
Call<UserPermission.View> getUserPermission(@Path("userId") String userId);

/**
* @param userId The username of the user
Expand All @@ -44,7 +44,7 @@ public interface FiatService {
* @return True if the user has access to the specified resource.
*/
@GET("/authorize/{userId}/{resourceType}/{resourceName}/{authorization}")
Response hasAuthorization(
Call<Void> hasAuthorization(
@Path("userId") String userId,
@Path("resourceType") String resourceType,
@Path("resourceName") String resourceName,
Expand All @@ -59,7 +59,7 @@ Response hasAuthorization(
* @param resource The resource to check
*/
@POST("/authorize/{userId}/{resourceType}/create")
Response canCreate(
Call<Void> canCreate(
@Path("userId") String userId,
@Path("resourceType") String resourceType,
@Body Object resource);
Expand All @@ -70,7 +70,7 @@ Response canCreate(
* @return The number of non-anonymous users synced.
*/
@POST("/roles/sync")
long sync();
Call<Long> sync();

/**
* Use to update a subset of users. An empty list will update the anonymous/unrestricted user.
Expand All @@ -79,7 +79,7 @@ Response canCreate(
* @return The number of non-anonymous users synced.
*/
@POST("/roles/sync")
long sync(@Body List<String> roles);
Call<Long> sync(@Body List<String> roles);

/**
* Use to update a service account. As opposed to `sync`, this will not trigger a full sync for
Expand All @@ -90,17 +90,15 @@ Response canCreate(
* @return The number of non-anonymous users synced.
*/
@POST("/roles/sync/serviceAccount/{serviceAccountId}")
long syncServiceAccount(
Call<Long> syncServiceAccount(
@Path("serviceAccountId") String serviceAccountId, @Body List<String> roles);

/**
* @param userId The user being logged in
* @param ignored ignored.
* @return ignored.
*/
@POST("/roles/{userId}")
Response loginUser(
@Path("userId") String userId, @Body String ignored /* retrofit requires this */);
Call<Void> loginUser(@Path("userId") String userId);

/**
* Used specifically for logins that contain the users roles/groups.
Expand All @@ -110,12 +108,12 @@ Response loginUser(
* @return ignored.
*/
@PUT("/roles/{userId}")
Response loginWithRoles(@Path("userId") String userId, @Body Collection<String> roles);
Call<Void> loginWithRoles(@Path("userId") String userId, @Body Collection<String> roles);

/**
* @param userId The user being logged out
* @return ignored.
*/
@DELETE("/roles/{userId}")
Response logoutUser(@Path("userId") String userId);
Call<Void> logoutUser(@Path("userId") String userId);
}
Loading
Loading