From 02b10b8788e7e118a538226a6e9816aaebc4c4ac Mon Sep 17 00:00:00 2001 From: Johannes Passing Date: Mon, 18 Dec 2023 07:36:12 +1100 Subject: [PATCH] b/316247020 Rename Result class (#223) Use AnnotatedResult as a less generic name for the class --- .../Result.java => AnnotatedResult.java} | 6 ++--- .../core/entitlements/RoleBinding.java | 1 + .../entitlements/RoleDiscoveryService.java | 11 +++----- .../TestRoleActivationService.java | 27 ++++++++++--------- .../jitaccess/web/rest/TestApiResource.java | 6 ++--- 5 files changed, 25 insertions(+), 26 deletions(-) rename sources/src/main/java/com/google/solutions/jitaccess/core/{entitlements/Result.java => AnnotatedResult.java} (90%) diff --git a/sources/src/main/java/com/google/solutions/jitaccess/core/entitlements/Result.java b/sources/src/main/java/com/google/solutions/jitaccess/core/AnnotatedResult.java similarity index 90% rename from sources/src/main/java/com/google/solutions/jitaccess/core/entitlements/Result.java rename to sources/src/main/java/com/google/solutions/jitaccess/core/AnnotatedResult.java index 08093305d..e8d862919 100644 --- a/sources/src/main/java/com/google/solutions/jitaccess/core/entitlements/Result.java +++ b/sources/src/main/java/com/google/solutions/jitaccess/core/AnnotatedResult.java @@ -19,7 +19,7 @@ // under the License. // -package com.google.solutions.jitaccess.core.entitlements; +package com.google.solutions.jitaccess.core; import com.google.common.base.Preconditions; @@ -29,7 +29,7 @@ /** * Result list of T with an optional set of warnings. */ -public class Result { +public class AnnotatedResult { /** * List of bindings. Might be incomplete if Warnings is non-empty. */ @@ -40,7 +40,7 @@ public class Result { */ private final Set warnings; - public Result(List roleBindings, Set warnings) { + public AnnotatedResult(List roleBindings, Set warnings) { Preconditions.checkNotNull(roleBindings); this.items = roleBindings; diff --git a/sources/src/main/java/com/google/solutions/jitaccess/core/entitlements/RoleBinding.java b/sources/src/main/java/com/google/solutions/jitaccess/core/entitlements/RoleBinding.java index 263b54293..532394a31 100644 --- a/sources/src/main/java/com/google/solutions/jitaccess/core/entitlements/RoleBinding.java +++ b/sources/src/main/java/com/google/solutions/jitaccess/core/entitlements/RoleBinding.java @@ -38,6 +38,7 @@ public record RoleBinding ( Preconditions.checkNotNull(fullResourceName, "fullResourceName"); Preconditions.checkNotNull(role, "role"); } + public RoleBinding(ProjectId project, String role) { this(project.getFullResourceName(), role); } diff --git a/sources/src/main/java/com/google/solutions/jitaccess/core/entitlements/RoleDiscoveryService.java b/sources/src/main/java/com/google/solutions/jitaccess/core/entitlements/RoleDiscoveryService.java index a80f53f63..4c4727cda 100644 --- a/sources/src/main/java/com/google/solutions/jitaccess/core/entitlements/RoleDiscoveryService.java +++ b/sources/src/main/java/com/google/solutions/jitaccess/core/entitlements/RoleDiscoveryService.java @@ -24,10 +24,7 @@ import com.google.api.services.cloudasset.v1.model.Expr; import com.google.api.services.cloudasset.v1.model.IamPolicyAnalysis; import com.google.common.base.Preconditions; -import com.google.solutions.jitaccess.core.AccessDeniedException; -import com.google.solutions.jitaccess.core.AccessException; -import com.google.solutions.jitaccess.core.ProjectId; -import com.google.solutions.jitaccess.core.UserId; +import com.google.solutions.jitaccess.core.*; import com.google.solutions.jitaccess.core.clients.AssetInventoryClient; import com.google.solutions.jitaccess.core.clients.ResourceManagerClient; import jakarta.enterprise.context.ApplicationScoped; @@ -161,7 +158,7 @@ public Set listAvailableProjects( /** * List eligible role bindings for the given user. */ - public Result listEligibleProjectRoles( + public AnnotatedResult listEligibleProjectRoles( UserId user, ProjectId projectId ) throws AccessException, IOException { @@ -177,7 +174,7 @@ public Result listEligibleProjectRoles( /** * List eligible role bindings for the given user. */ - public Result listEligibleProjectRoles( + public AnnotatedResult listEligibleProjectRoles( UserId user, ProjectId projectId, EnumSet statusesToInclude @@ -288,7 +285,7 @@ public Result listEligibleProjectRoles( .collect(Collectors.toList()); consolidatedRoles.addAll(activatedRoles); - return new Result<>( + return new AnnotatedResult<>( consolidatedRoles, Stream.ofNullable(analysisResult.getNonCriticalErrors()) .flatMap(Collection::stream) diff --git a/sources/src/test/java/com/google/solutions/jitaccess/core/entitlements/TestRoleActivationService.java b/sources/src/test/java/com/google/solutions/jitaccess/core/entitlements/TestRoleActivationService.java index 3d764ff63..7af75a9db 100644 --- a/sources/src/test/java/com/google/solutions/jitaccess/core/entitlements/TestRoleActivationService.java +++ b/sources/src/test/java/com/google/solutions/jitaccess/core/entitlements/TestRoleActivationService.java @@ -22,6 +22,7 @@ package com.google.solutions.jitaccess.core.entitlements; import com.google.solutions.jitaccess.core.AccessDeniedException; +import com.google.solutions.jitaccess.core.AnnotatedResult; import com.google.solutions.jitaccess.core.clients.ResourceManagerClient; import com.google.solutions.jitaccess.core.ProjectId; import com.google.solutions.jitaccess.core.UserId; @@ -87,7 +88,7 @@ public void whenCallerLacksRoleBinding_ThenActivateProjectRoleForSelfThrowsExcep var caller = SAMPLE_USER; when(discoveryService.listEligibleProjectRoles(eq(caller), eq(SAMPLE_PROJECT_ID), any())) - .thenReturn(new Result( + .thenReturn(new AnnotatedResult( List.of(new ProjectRole( new RoleBinding( SAMPLE_PROJECT_RESOURCE_1, @@ -167,7 +168,7 @@ public void whenCallerIsJitEligible_ThenActivateProjectRoleForSelfAddsBinding() var caller = SAMPLE_USER; when(discoveryService.listEligibleProjectRoles(eq(caller), eq(SAMPLE_PROJECT_ID), any())) - .thenReturn(new Result( + .thenReturn(new AnnotatedResult( List.of(new ProjectRole( new RoleBinding( SAMPLE_PROJECT_RESOURCE_1, @@ -280,7 +281,7 @@ public void whenRoleNotMpaEligibleForCaller_ThenActivateProjectRoleForPeerThrows var discoveryService = Mockito.mock(RoleDiscoveryService.class); when(discoveryService.listEligibleProjectRoles(eq(caller), eq(SAMPLE_PROJECT_ID), any())) - .thenReturn(new Result( + .thenReturn(new AnnotatedResult( List.of(new ProjectRole( roleBinding, ProjectRole.Status.ACTIVATED)), @@ -321,7 +322,7 @@ public void whenRoleIsJitEligibleForCaller_ThenActivateProjectRoleForPeerThrowsE var discoveryService = Mockito.mock(RoleDiscoveryService.class); when(discoveryService.listEligibleProjectRoles(eq(caller), eq(SAMPLE_PROJECT_ID), any())) - .thenReturn(new Result( + .thenReturn(new AnnotatedResult( List.of(new ProjectRole( roleBinding, ProjectRole.Status.ELIGIBLE_FOR_JIT)), @@ -362,13 +363,13 @@ public void whenRoleNotEligibleForPeer_ThenActivateProjectRoleForPeerThrowsExcep var discoveryService = Mockito.mock(RoleDiscoveryService.class); when(discoveryService.listEligibleProjectRoles(eq(caller), eq(SAMPLE_PROJECT_ID), any())) - .thenReturn(new Result( + .thenReturn(new AnnotatedResult( List.of(new ProjectRole( roleBinding, ProjectRole.Status.ELIGIBLE_FOR_MPA)), Set.of())); when(discoveryService.listEligibleProjectRoles(eq(peer), eq(SAMPLE_PROJECT_ID), any())) - .thenReturn(new Result( + .thenReturn(new AnnotatedResult( List.of(), Set.of())); @@ -413,7 +414,7 @@ public void whenPeerAndCallerEligible_ThenActivateProjectRoleAddsBinding() throw eq(EnumSet.of( ProjectRole.Status.ELIGIBLE_FOR_JIT, ProjectRole.Status.ELIGIBLE_FOR_MPA)))) - .thenReturn(new Result( + .thenReturn(new AnnotatedResult( List.of(new ProjectRole( roleBinding, ProjectRole.Status.ELIGIBLE_FOR_MPA)), @@ -424,7 +425,7 @@ public void whenPeerAndCallerEligible_ThenActivateProjectRoleAddsBinding() throw eq(EnumSet.of( ProjectRole.Status.ELIGIBLE_FOR_JIT, ProjectRole.Status.ELIGIBLE_FOR_MPA)))) - .thenReturn(new Result( + .thenReturn(new AnnotatedResult( List.of(new ProjectRole( roleBinding, ProjectRole.Status.ELIGIBLE_FOR_MPA)), @@ -484,13 +485,13 @@ public void whenRoleAlreadyActivated_ThenActivateProjectRoleAddsBinding() throws var resourceAdapter = Mockito.mock(ResourceManagerClient.class); var discoveryService = Mockito.mock(RoleDiscoveryService.class); when(discoveryService.listEligibleProjectRoles(eq(caller), eq(SAMPLE_PROJECT_ID), any())) - .thenReturn(new Result( + .thenReturn(new AnnotatedResult( List.of(new ProjectRole( roleBinding, ProjectRole.Status.ELIGIBLE_FOR_MPA)), Set.of())); when(discoveryService.listEligibleProjectRoles(eq(peer), eq(SAMPLE_PROJECT_ID), any())) - .thenReturn(new Result( + .thenReturn(new AnnotatedResult( List.of(new ProjectRole( roleBinding, ProjectRole.Status.ACTIVATED)), // Pretend someone else approved already @@ -658,7 +659,7 @@ public void whenRoleNotMpaEligibleForCaller_ThenCreateActivationRequestForPeerTh var discoveryService = Mockito.mock(RoleDiscoveryService.class); when(discoveryService.listEligibleProjectRoles(eq(caller), eq(SAMPLE_PROJECT_ID), any())) - .thenReturn(new Result( + .thenReturn(new AnnotatedResult( List.of(new ProjectRole( roleBinding, ProjectRole.Status.ACTIVATED)), @@ -693,7 +694,7 @@ public void whenRoleIsJitEligibleForCaller_ThenCreateActivationRequestForPeerThr var discoveryService = Mockito.mock(RoleDiscoveryService.class); when(discoveryService.listEligibleProjectRoles(eq(caller), eq(SAMPLE_PROJECT_ID), any())) - .thenReturn(new Result( + .thenReturn(new AnnotatedResult( List.of(new ProjectRole( roleBinding, ProjectRole.Status.ELIGIBLE_FOR_JIT)), @@ -728,7 +729,7 @@ public void whenCallerEligible_ThenCreateActivationRequestForPeerSucceeds() thro var discoveryService = Mockito.mock(RoleDiscoveryService.class); when(discoveryService.listEligibleProjectRoles(eq(caller), eq(SAMPLE_PROJECT_ID), any())) - .thenReturn(new Result( + .thenReturn(new AnnotatedResult( List.of(new ProjectRole( roleBinding, ProjectRole.Status.ELIGIBLE_FOR_MPA)), diff --git a/sources/src/test/java/com/google/solutions/jitaccess/web/rest/TestApiResource.java b/sources/src/test/java/com/google/solutions/jitaccess/web/rest/TestApiResource.java index 532a74220..e56d43fc6 100644 --- a/sources/src/test/java/com/google/solutions/jitaccess/web/rest/TestApiResource.java +++ b/sources/src/test/java/com/google/solutions/jitaccess/web/rest/TestApiResource.java @@ -35,7 +35,7 @@ import com.google.solutions.jitaccess.core.notifications.NotificationService; import com.google.solutions.jitaccess.core.entitlements.RoleActivationService; import com.google.solutions.jitaccess.core.entitlements.RoleDiscoveryService; -import com.google.solutions.jitaccess.core.entitlements.Result; +import com.google.solutions.jitaccess.core.AnnotatedResult; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -371,7 +371,7 @@ public void whenRoleDiscoveryReturnsNoRoles_ThenListRolesReturnsEmptyList() thro .listEligibleProjectRoles( eq(SAMPLE_USER), eq(new ProjectId("project-1")))) - .thenReturn(new Result<>( + .thenReturn(new AnnotatedResult<>( List.of(), Set.of("warning"))); @@ -401,7 +401,7 @@ public void whenRoleDiscoveryReturnsRoles_ThenListRolesReturnsList() throws Exce .listEligibleProjectRoles( eq(SAMPLE_USER), eq(new ProjectId("project-1")))) - .thenReturn(new Result<>( + .thenReturn(new AnnotatedResult<>( List.of(role1, role2), null));