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

[TBD] Remove BPM feature flag #1716

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ public class FeatureConfiguration {
private boolean azureControlPlaneEnabled;
private boolean alpha1Enabled;
private boolean tpsEnabled;
private boolean bpmGcpEnabled;
private boolean temporaryGrantEnabled;
private WsmResourceStateRule stateRule;

Expand Down Expand Up @@ -54,14 +53,6 @@ public void setTpsEnabled(boolean tpsEnabled) {
this.tpsEnabled = tpsEnabled;
}

public boolean isBpmGcpEnabled() {
return bpmGcpEnabled;
}

public void setBpmGcpEnabled(boolean bpmGcpEnabled) {
this.bpmGcpEnabled = bpmGcpEnabled;
}

public boolean isTemporaryGrantEnabled() {
return temporaryGrantEnabled;
}
Expand Down Expand Up @@ -107,7 +98,6 @@ public void logFeatures() {
logger.info("Feature: azure-enabled: {}", isAzureControlPlaneEnabled());
logger.info("Feature: alpha1-enabled: {}", isAlpha1Enabled());
logger.info("Feature: tps-enabled: {}", isTpsEnabled());
logger.info("Feature: bpm-gcp-enabled: {}", isBpmGcpEnabled());
logger.info("Feature: temporary-grant-enabled: {}", isTemporaryGrantEnabled());
logger.info("Feature: state-rule: {}", getStateRule());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -501,9 +501,7 @@ public ResponseEntity<ApiCreateCloudContextResult> createCloudContext(
.orElseThrow(() -> MissingSpendProfileException.forWorkspace(workspace.workspaceId()));

// Make sure the caller is authorized to use the spend profile
SpendProfile spendProfile =
spendProfileService.authorizeLinking(
spendProfileId, features.isBpmGcpEnabled(), userRequest);
SpendProfile spendProfile = spendProfileService.authorizeLinking(spendProfileId, userRequest);

workspaceService.createCloudContext(
workspace, cloudPlatform, spendProfile, jobId, userRequest, resultPath);
Expand Down Expand Up @@ -630,9 +628,7 @@ public ResponseEntity<ApiCloneWorkspaceResult> cloneWorkspace(
.orElse(workspaceSpendProfileId);
SpendProfile spendProfile = null;
if (spendProfileId != null) {
spendProfile =
spendProfileService.authorizeLinking(
spendProfileId, features.isBpmGcpEnabled(), petRequest);
spendProfile = spendProfileService.authorizeLinking(spendProfileId, petRequest);
}

// Accept a target workspace id if one is provided. This allows Rawls to specify an
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,7 @@ public static WorkspaceStage getStageFromApiStage(ApiWorkspaceStageModel apiWork
if (apiSpendProfile == null) {
return null;
}
return spendProfileService.authorizeLinking(
new SpendProfileId(apiSpendProfile), features.isBpmGcpEnabled(), userRequest);
return spendProfileService.authorizeLinking(new SpendProfileId(apiSpendProfile), userRequest);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,9 @@ public SpendProfileService(
*/
@WithSpan
public SpendProfile authorizeLinking(
SpendProfileId spendProfileId, boolean bpmEnabled, AuthenticatedUserRequest userRequest) {
SpendProfileId spendProfileId, AuthenticatedUserRequest userRequest) {

SpendProfile spend = null;
SpendProfile spend;
if (spendProfiles.containsKey(spendProfileId)) {
if (!Rethrow.onInterrupted(
() ->
Expand All @@ -96,20 +96,9 @@ public SpendProfile authorizeLinking(
throw SpendUnauthorizedException.linkUnauthorized(spendProfileId);
}
spend = spendProfiles.get(spendProfileId);
} else if (bpmEnabled) {
} else {
// profiles returned from BPM means we are auth'ed
spend = getSpendProfileFromBpm(userRequest, spendProfileId);
} else {
if (!Rethrow.onInterrupted(
() ->
samService.isAuthorized(
userRequest,
SamConstants.SamResource.SPEND_PROFILE,
spendProfileId.getId(),
SamConstants.SamSpendProfileAction.LINK),
"isAuthorized")) {
throw SpendUnauthorizedException.linkUnauthorized(spendProfileId);
}
}

if (spend == null) {
Expand Down
2 changes: 0 additions & 2 deletions service/src/main/resources/application.yml
Original file line number Diff line number Diff line change
Expand Up @@ -285,8 +285,6 @@ feature:
alpha1-enabled: false
# tps-enabled - Controls whether Terra Policy Service is called. It is always built into WSM
tps-enabled: false
# bpm-enabled-gcp - Controls whether spend profile checks are made for GCP workspaces
bpm-gcp-enabled: false
# temporary-grant-enabled - Controls whether temporary direct ACL grants are made on creates
temporary-grant-enabled: false
# state-rule - see WsmResourceStateRule - default to original state rule
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,7 @@ protected void createLandingZone() {
protected SpendProfileId initSpendProfileMock() {
Mockito.when(
mockSpendProfileService()
.authorizeLinking(
Mockito.eq(azureTestUtils.getSpendProfileId()),
Mockito.eq(true),
Mockito.any()))
.authorizeLinking(Mockito.eq(azureTestUtils.getSpendProfileId()), Mockito.any()))
.thenReturn(
new SpendProfile(
azureTestUtils.getSpendProfileId(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public Workspace createWorkspaceWithGcpContext(AuthenticatedUserRequest userRequ
// make the authorize request.
SpendProfile spendProfile =
spendProfileService.authorizeLinking(
workspace.getSpendProfileId().orElseThrow(), features.isBpmGcpEnabled(), userRequest);
workspace.getSpendProfileId().orElseThrow(), userRequest);

String gcpContextJobId = UUID.randomUUID().toString();
workspaceService.createCloudContext(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public void testAuthorizingLinkingOfAnAzureProfile(MockServer mockServer) {
var spendProfileId = new SpendProfileId(dummyAzureProfileId);
var service = new SpendProfileService(samService, config, OpenTelemetry.noop());

service.authorizeLinking(spendProfileId, true, userRequest);
service.authorizeLinking(spendProfileId, userRequest);
}

@Test
Expand All @@ -121,7 +121,7 @@ public void testAuthorizingLinkingOfGCPProfile(MockServer mockServer) {
var spendProfileId = new SpendProfileId(dummyGCPProfileId);
var service = new SpendProfileService(samService, config, OpenTelemetry.noop());

service.authorizeLinking(spendProfileId, true, userRequest);
service.authorizeLinking(spendProfileId, userRequest);
}

@Test
Expand All @@ -139,6 +139,6 @@ public void testAuthorizingLinkingOfAnNonexistantProfile(MockServer mockServer)
var service = new SpendProfileService(samService, config, OpenTelemetry.noop());
assertThrows(
SpendUnauthorizedException.class,
() -> service.authorizeLinking(spendProfileId, true, userRequest));
() -> service.authorizeLinking(spendProfileId, userRequest));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,7 @@ public void cleanUp() {
@DisabledIf("bpmUnavailable")
void authorizeLinkingSuccess() {
var linkedProfile =
spendProfileService.authorizeLinking(
profile.id(), true, userAccessUtils.thirdUserAuthRequest());
spendProfileService.authorizeLinking(profile.id(), userAccessUtils.thirdUserAuthRequest());
assertEquals(linkedProfile.billingAccountId(), profile.billingAccountId());
assertEquals(linkedProfile.id(), profile.id());
}
Expand All @@ -74,7 +73,7 @@ void authorizeLinkingFailure() {
SpendUnauthorizedException.class,
() ->
spendProfileService.authorizeLinking(
profile.id(), true, userAccessUtils.defaultUserAuthRequest()));
profile.id(), userAccessUtils.defaultUserAuthRequest()));
}

@Test
Expand All @@ -86,7 +85,6 @@ void authorizeLinkingUnknownId() {
() ->
spendProfileService.authorizeLinking(
new SpendProfileId(UUID.randomUUID().toString()),
true,
userAccessUtils.thirdUserAuthRequest()));
assert (ex.getStatusCode() == HttpStatus.FORBIDDEN);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ void authorizeLinkingSuccess() {
new SpendProfileService(
samService, ImmutableList.of(profile), spendProfileConfiguration, OpenTelemetry.noop());

assertEquals(
profile, service.authorizeLinking(id, false, userAccessUtils.defaultUserAuthRequest()));
assertEquals(profile, service.authorizeLinking(id, userAccessUtils.defaultUserAuthRequest()));
}

@Test
Expand All @@ -54,7 +53,7 @@ void authorizeLinkingSamUnauthorizedThrowsUnauthorized() {

assertThrows(
SpendUnauthorizedException.class,
() -> service.authorizeLinking(id, false, userAccessUtils.secondUserAuthRequest()));
() -> service.authorizeLinking(id, userAccessUtils.secondUserAuthRequest()));
}

@Test
Expand All @@ -67,7 +66,7 @@ void authorizeLinkingUnknownIdThrowsUnauthorized() {
SpendUnauthorizedException.class,
() ->
service.authorizeLinking(
new SpendProfileId("bar"), false, userAccessUtils.defaultUserAuthRequest()));
new SpendProfileId("bar"), userAccessUtils.defaultUserAuthRequest()));
}

@Test
Expand All @@ -79,6 +78,6 @@ void parseSpendProfileConfiguration() {
SpendProfile.buildGcpSpendProfile(
spendUtils.defaultSpendId(), spendUtils.defaultBillingAccountId()),
service.authorizeLinking(
spendUtils.defaultSpendId(), false, userAccessUtils.defaultUserAuthRequest()));
spendUtils.defaultSpendId(), userAccessUtils.defaultUserAuthRequest()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,7 @@ void removeUserFromWorkspaceFlightDoUndo() throws Exception {
AuthenticatedUserRequest userRequest = userAccessUtils.defaultUser().getAuthenticatedRequest();
String makeContextJobId = UUID.randomUUID().toString();
SpendProfile spendProfile =
spendProfileService.authorizeLinking(
DEFAULT_SPEND_PROFILE_ID, features.isBpmGcpEnabled(), userRequest);
spendProfileService.authorizeLinking(DEFAULT_SPEND_PROFILE_ID, userRequest);

workspaceService.createCloudContext(
workspace, CloudPlatform.GCP, spendProfile, makeContextJobId, userRequest, null);
Expand Down
Loading