From 268236a159bd289b6734a0bb9022ed4963150a47 Mon Sep 17 00:00:00 2001 From: spinnakerbot Date: Tue, 10 Dec 2024 14:29:24 -0500 Subject: [PATCH] chore(dependencies): Autobump fiatVersion (#1519) * chore(dependencies): Autobump fiatVersion * refactor(retrofit2): refactor the code to align with the retrofit2 upgrade of fiat-api * refactor(retrofit2): use retrofit-mock library instead of mocking Call. --------- Co-authored-by: root Co-authored-by: kirangodishala --- front50-core/front50-core.gradle | 1 + .../front50/ApplicationPermissionsService.java | 3 ++- .../spinnaker/front50/ServiceAccountsService.java | 11 ++++++++--- .../front50/ApplicationPermissionsServiceSpec.groovy | 3 ++- .../front50/ServiceAccountsServiceSpec.groovy | 11 ++++++++--- front50-web/front50-web.gradle | 1 + .../controllers/v2/ApplicationsController.java | 3 ++- gradle.properties | 2 +- 8 files changed, 25 insertions(+), 10 deletions(-) diff --git a/front50-core/front50-core.gradle b/front50-core/front50-core.gradle index 00919eb28..a196e2ee4 100644 --- a/front50-core/front50-core.gradle +++ b/front50-core/front50-core.gradle @@ -47,5 +47,6 @@ dependencies { // in front50-test to front50-common, but front50-test seems like a better place. testImplementation project(":front50-test") testImplementation "io.spinnaker.kork:kork-sql-test" + testImplementation "com.squareup.retrofit2:retrofit-mock" } diff --git a/front50-core/src/main/java/com/netflix/spinnaker/front50/ApplicationPermissionsService.java b/front50-core/src/main/java/com/netflix/spinnaker/front50/ApplicationPermissionsService.java index cc396ab8e..4f717fc25 100644 --- a/front50-core/src/main/java/com/netflix/spinnaker/front50/ApplicationPermissionsService.java +++ b/front50-core/src/main/java/com/netflix/spinnaker/front50/ApplicationPermissionsService.java @@ -25,6 +25,7 @@ import com.netflix.spinnaker.front50.model.application.ApplicationDAO; import com.netflix.spinnaker.front50.model.application.ApplicationPermissionDAO; import com.netflix.spinnaker.kork.exceptions.SystemException; +import com.netflix.spinnaker.kork.retrofit.Retrofit2SyncCall; import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException; import com.netflix.spinnaker.kork.web.exceptions.NotFoundException; import java.util.AbstractMap.SimpleEntry; @@ -182,7 +183,7 @@ private void syncUsers(Permission newPermission, Permission oldPermission) { if (fiatConfigurationProperties.getRoleSync().isEnabled()) { try { - fiatService.get().sync(new ArrayList<>(roles)); + Retrofit2SyncCall.execute(fiatService.get().sync(new ArrayList<>(roles))); } catch (SpinnakerServerException e) { log.warn("Error syncing users", e); } diff --git a/front50-core/src/main/java/com/netflix/spinnaker/front50/ServiceAccountsService.java b/front50-core/src/main/java/com/netflix/spinnaker/front50/ServiceAccountsService.java index 4fdb57cfe..97dfc38b6 100644 --- a/front50-core/src/main/java/com/netflix/spinnaker/front50/ServiceAccountsService.java +++ b/front50-core/src/main/java/com/netflix/spinnaker/front50/ServiceAccountsService.java @@ -23,6 +23,7 @@ import com.netflix.spinnaker.front50.config.annotations.ConditionalOnAnyProviderExceptRedisIsEnabled; import com.netflix.spinnaker.front50.model.serviceaccount.ServiceAccount; import com.netflix.spinnaker.front50.model.serviceaccount.ServiceAccountDAO; +import com.netflix.spinnaker.kork.retrofit.Retrofit2SyncCall; import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException; import com.netflix.spinnaker.kork.web.exceptions.NotFoundException; import java.util.Collection; @@ -86,7 +87,8 @@ public void deleteServiceAccounts(Collection serviceAccountsToDe sa -> { try { serviceAccountDAO.delete(sa.getId()); - fiatService.ifPresent(service -> service.logoutUser(sa.getId())); + fiatService.ifPresent( + service -> Retrofit2SyncCall.execute(service.logoutUser(sa.getId()))); } catch (Exception e) { log.warn("Could not delete service account user {}", sa.getId(), e); } @@ -129,7 +131,7 @@ private void syncUsers(Collection serviceAccounts) { .flatMap(Collection::stream) .distinct() .collect(Collectors.toList()); - fiatService.get().sync(rolesToSync); + Retrofit2SyncCall.execute(fiatService.get().sync(rolesToSync)); log.debug("Synced users with roles: {}", rolesToSync); // Invalidate the current user's permissions in the local cache Authentication auth = SecurityContextHolder.getContext().getAuthentication(); @@ -149,7 +151,10 @@ private void syncServiceAccount(ServiceAccount serviceAccount) { return; } try { - fiatService.get().syncServiceAccount(serviceAccount.getId(), serviceAccount.getMemberOf()); + Retrofit2SyncCall.execute( + fiatService + .get() + .syncServiceAccount(serviceAccount.getId(), serviceAccount.getMemberOf())); log.debug( "Synced service account {} with roles: {}", serviceAccount.getId(), diff --git a/front50-core/src/test/groovy/com/netflix/spinnaker/front50/ApplicationPermissionsServiceSpec.groovy b/front50-core/src/test/groovy/com/netflix/spinnaker/front50/ApplicationPermissionsServiceSpec.groovy index e35bfea1f..9961bb48b 100644 --- a/front50-core/src/test/groovy/com/netflix/spinnaker/front50/ApplicationPermissionsServiceSpec.groovy +++ b/front50-core/src/test/groovy/com/netflix/spinnaker/front50/ApplicationPermissionsServiceSpec.groovy @@ -23,6 +23,7 @@ import com.netflix.spinnaker.front50.config.FiatConfigurationProperties import com.netflix.spinnaker.front50.model.application.Application import com.netflix.spinnaker.front50.model.application.ApplicationDAO import com.netflix.spinnaker.front50.model.application.ApplicationPermissionDAO +import retrofit2.mock.Calls import spock.lang.Specification import spock.lang.Unroll @@ -44,7 +45,7 @@ class ApplicationPermissionsServiceSpec extends Specification { subject.createApplicationPermission(permission) then: - 1 * fiatService.sync(expectedSyncedRoles) + 1 * fiatService.sync(expectedSyncedRoles) >> Calls.response(null) where: permission | expectedSyncedRoles diff --git a/front50-core/src/test/groovy/com/netflix/spinnaker/front50/ServiceAccountsServiceSpec.groovy b/front50-core/src/test/groovy/com/netflix/spinnaker/front50/ServiceAccountsServiceSpec.groovy index 043b9ebe0..2c4b83596 100644 --- a/front50-core/src/test/groovy/com/netflix/spinnaker/front50/ServiceAccountsServiceSpec.groovy +++ b/front50-core/src/test/groovy/com/netflix/spinnaker/front50/ServiceAccountsServiceSpec.groovy @@ -26,6 +26,7 @@ import com.netflix.spinnaker.front50.model.serviceaccount.ServiceAccountDAO import org.springframework.security.core.Authentication import org.springframework.security.core.context.SecurityContext import org.springframework.security.core.context.SecurityContextHolder +import retrofit2.mock.Calls import spock.lang.Specification import spock.lang.Subject @@ -67,13 +68,14 @@ class ServiceAccountsServiceSpec extends Specification { } SecurityContextHolder.setContext(securityContext) fiatConfigurationProperties.isDisableRoleSyncWhenSavingServiceAccounts() >> false + when: serviceAccountDAO.create(serviceAccount.id, serviceAccount) >> serviceAccount service.createServiceAccount(serviceAccount) then: 1 * fiatPermissionsEvaluator.invalidatePermission(_) - 1 * fiatService.sync(["test-role"]) + 1 * fiatService.sync(["test-role"]) >> Calls.response(null) } def "deleting multiple service account should call sync once"() { @@ -92,13 +94,14 @@ class ServiceAccountsServiceSpec extends Specification { ] )] fiatConfigurationProperties.isDisableRoleSyncWhenSavingServiceAccounts() >> false + when: service.deleteServiceAccounts(serviceAccounts) then: 1 * serviceAccountDAO.delete("test-svc-acct-1") 1 * serviceAccountDAO.delete("test-svc-acct-2") - 1 * fiatService.sync(['test-role-1', 'test-role-2']) + 1 * fiatService.sync(['test-role-1', 'test-role-2']) >> Calls.response(null) } def "unknown managed service accounts should not throw exception"() { @@ -118,6 +121,8 @@ class ServiceAccountsServiceSpec extends Specification { 1 * serviceAccountDAO.findById(test1ServiceAccount.id) >> test1ServiceAccount 1 * serviceAccountDAO.findById(test2ServiceAccount.id) >> { throw new NotFoundException(test2ServiceAccount.id) } 1 * serviceAccountDAO.delete(test1ServiceAccount.id) + 1 * fiatService.logoutUser(_) >> Calls.response(null) + 1 * fiatService.sync(_) >> Calls.response(1L) 0 * serviceAccountDAO.delete(test2ServiceAccount.id) } @@ -144,7 +149,7 @@ class ServiceAccountsServiceSpec extends Specification { then: 1 * fiatPermissionsEvaluator.invalidatePermission(_) - 1 * fiatService.syncServiceAccount("test-svc-acct", ["test-role"]) + 1 * fiatService.syncServiceAccount("test-svc-acct", ["test-role"]) >> Calls.response(1L) 0 * fiatService.sync(["test-role"]) } } diff --git a/front50-web/front50-web.gradle b/front50-web/front50-web.gradle index 95ac9b175..88917cd11 100644 --- a/front50-web/front50-web.gradle +++ b/front50-web/front50-web.gradle @@ -32,6 +32,7 @@ dependencies { implementation "io.spinnaker.fiat:fiat-core:$fiatVersion" implementation "io.spinnaker.kork:kork-artifacts" implementation "io.spinnaker.kork:kork-config" + implementation "io.spinnaker.kork:kork-retrofit" implementation "io.spinnaker.kork:kork-web" implementation "io.spinnaker.kork:kork-exceptions" implementation "com.squareup.retrofit:converter-jackson" diff --git a/front50-web/src/main/java/com/netflix/spinnaker/front50/controllers/v2/ApplicationsController.java b/front50-web/src/main/java/com/netflix/spinnaker/front50/controllers/v2/ApplicationsController.java index 7c8879b17..19fd5305f 100644 --- a/front50-web/src/main/java/com/netflix/spinnaker/front50/controllers/v2/ApplicationsController.java +++ b/front50-web/src/main/java/com/netflix/spinnaker/front50/controllers/v2/ApplicationsController.java @@ -12,6 +12,7 @@ import com.netflix.spinnaker.front50.model.application.ApplicationDAO; import com.netflix.spinnaker.front50.model.application.ApplicationPermissionDAO; import com.netflix.spinnaker.front50.model.application.ApplicationService; +import com.netflix.spinnaker.kork.retrofit.Retrofit2SyncCall; import com.netflix.spinnaker.kork.web.exceptions.NotFoundException; import io.swagger.v3.oas.annotations.Operation; import io.swagger.v3.oas.annotations.tags.Tag; @@ -125,7 +126,7 @@ public Application create(@RequestBody final Application app) { && fiatConfigurationProperties.getRoleSync().isEnabled() && fiatService.isPresent()) { try { - fiatService.get().sync(); + Retrofit2SyncCall.execute(fiatService.get().sync()); } catch (Exception e) { log.warn("failed to trigger fiat permission sync", e); } diff --git a/gradle.properties b/gradle.properties index 8d8e92489..8339eb949 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,4 +1,4 @@ -fiatVersion=1.52.0 +fiatVersion=1.53.0 includeProviders=azure,gcs,oracle,redis,s3,swift,sql korkVersion=7.247.0 org.gradle.parallel=true