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.

Sign up for GitHub

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

Jump to bottom

Issue transfer ownership of service user #2393

Merged
merged 6 commits into from
Apr 4, 2024
Merged

Issue transfer ownership of service user #2393

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8c5d3f4
Change ownership of service users
aindriu-aiven Apr 3, 2024
4b0539f
Allow service accounts to be transfered between teams
aindriu-aiven Apr 3, 2024
2e5ffc4
Additon of unit tests to check for various scenarios of transfer of o…
aindriu-aiven Apr 4, 2024
8fc5963
Update to only transfer ownership when the last acl is transfered fro…
aindriu-aiven Apr 4, 2024
7cf795f
Address PR comment move if statement up a level
aindriu-aiven Apr 4, 2024
433e2e2
Merge branch 'main' into issue-transfer-ownership-of-service-user
aindriu-aiven Apr 4, 2024
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
2 changes: 2 additions & 0 deletions core/src/main/java/io/aiven/klaw/helpers/HandleDbRequests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,8 @@ boolean validateIfConsumerGroupUsedByAnotherTeam(
boolean existsSpecificAclRequest(
String topicName, String requestStatus, String env, int tenantId, int associatedAclId);

boolean existsAclSslInTeam(int teamId, int tenantId, String aclSsl);

boolean existsSchemaRequest(String topicName, String requestStatus, String env, int tenantId);

boolean existsSchemaRequest(
Expand Down
5 changes: 5 additions & 0 deletions core/src/main/java/io/aiven/klaw/helpers/db/rdbms/HandleDbRequestsJdbc.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,11 @@ public boolean existsSpecificAclRequest(
topicName, requestStatus, env, tenantId, associatedAclId);
}

@Override
public boolean existsAclSslInTeam(int teamId, int tenantId, String aclSsl) {
return jdbcSelectHelper.existsAclSslInTeam(teamId, tenantId, aclSsl);
}

@Override
public boolean existsSchemaRequest(
String topicName, String requestStatus, String env, int tenantId) {
Expand Down
4 changes: 4 additions & 0 deletions core/src/main/java/io/aiven/klaw/helpers/db/rdbms/SelectDataJdbc.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@ public boolean existsAclRequest(
tenantId, env, requestStatus, topicName);
}

public boolean existsAclSslInTeam(int teamId, int tenantId, String aclSsl) {
return aclRepo.existsAclSslInTeamInTenant(teamId, tenantId, aclSsl);
}

public boolean existsSpecificAclRequest(
String topicName, String requestStatus, String env, int tenantId, int associatedAclId) {
log.debug("associatedAclId = {}", associatedAclId);
Expand Down
6 changes: 6 additions & 0 deletions core/src/main/java/io/aiven/klaw/repository/AclRepo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@ boolean existsByTeamIdNotAndTenantIdAndConsumergroup(
boolean existsByEnvironmentAndTenantId(
@Param("envId") String envId, @Param("tenantId") Integer tenantId);

@Query(
value =
"select count(*) > 0 from kwacls where teamid = :teamId and tenantid = :tenantId and aclssl = :aclSsl",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could be better to use a jpa instead of native here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah thats what I wanted to do but we have a limitation with JPA.
Unfortunately acl_ssl in the aclRequest prevents us from using the JPA, I can refactor this in 2.10.0 was my plan though.
(The underscore in the variable name is not supported to be specific )

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we try this way, or may be in future

@column(name = "municipal_id", nullable = false)
private Integer municipalId; // <-- field was renamed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes this is the way to fix it.
in AclRequests
@column(name = "aclssl")
private String acl_ssl; -> private String aclSsl;

I was going to leave it till a future PR but I can include in this one and then use JPA

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this rename leads to several changes. Lets try in 2.10

nativeQuery = true)
boolean existsAclSslInTeamInTenant(Integer teamId, Integer tenantId, String aclSsl);

@Query(
value = "select count(*) from kwacls where env = :envId and tenantid = :tenantId",
nativeQuery = true)
Expand Down
64 changes: 64 additions & 0 deletions core/src/main/java/io/aiven/klaw/service/AclControllerService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -744,6 +744,16 @@ private ApiResponse approveClaimAcl(

acl.get().setTeamId(aclReq.getRequestingteam());
manageDatabase.getHandleDbRequests().updateAcl(acl.get());
if (Objects.equals(RequestOperationType.CLAIM.value, aclReq.getRequestOperationType())) {
if (!manageDatabase
.getHandleDbRequests()
.existsAclSslInTeam(aclReq.getTeamId(), aclReq.getTenantId(), aclReq.getAcl_ssl())) {
// Team has no other Acls left with service user so transfer ownership to the next
// team.
// Transfer Service Account ownership
transferServiceUserOwnership(aclReq);
}
}
}
aclReq.setApprovals(KlawResourceUtils.approvalsToAclApprovalsList(approvals));
String status =
Expand All @@ -770,11 +780,43 @@ private ApiResponse emailAndReturnClaimUpdate(
dbHandle,
notifyUserType,
commonUtilsService.getLoginUrl());

return ApiResultStatus.SUCCESS.value.equals(updateAclReqStatus)
? ApiResponse.ok(updateAclReqStatus)
: ApiResponse.notOk(updateAclReqStatus);
}

private void transferServiceUserOwnership(AclRequests aclReq) {
removeServiceAccountOnTransferOfOwnership(aclReq, aclReq.getTenantId());
Optional<Team> optionalTeam =
manageDatabase.getTeamObjForTenant(aclReq.getTenantId()).stream()
.filter(team -> Objects.equals(team.getTeamId(), aclReq.getRequestingteam()))
.findFirst();
optionalTeam.ifPresent(
team -> {
if (Objects.equals(RequestOperationType.CLAIM.value, aclReq.getRequestOperationType())) {
ServiceAccounts serviceAccounts = team.getServiceAccounts();
if (serviceAccounts != null
&& serviceAccounts.getServiceAccountsList() != null
&& !serviceAccounts.getServiceAccountsList().isEmpty()) {
// add to your team
serviceAccounts.getServiceAccountsList().add(aclReq.getAcl_ssl());
} else {
serviceAccounts = new ServiceAccounts();
serviceAccounts.setNumberOfAllowedAccounts(allowedServiceAccountsPerTeam);
serviceAccounts.setServiceAccountsList(new HashSet<>());
serviceAccounts.getServiceAccountsList().add(aclReq.getAcl_ssl());
team.setServiceAccounts(serviceAccounts);
}

// Update team with service account
manageDatabase.getHandleDbRequests().updateTeam(team);
commonUtilsService.updateMetadata(
aclReq.getTenantId(), EntityType.TEAM, MetadataOperationType.UPDATE, null);
}
});
}

private void saveToTopicHistory(String userDetails, int tenantId, AclRequests aclReq) {
String remarksAcl =
RequestEntityType.ACL.name()
Expand Down Expand Up @@ -905,13 +947,35 @@ private void updateServiceAccountsForTeam(AclRequests aclRequest, int tenantId)
serviceAccounts.getServiceAccountsList().add(aclRequest.getAcl_ssl());
optionalTeam.get().setServiceAccounts(serviceAccounts);
}

// Update team with service account
manageDatabase.getHandleDbRequests().updateTeam(optionalTeam.get());
commonUtilsService.updateMetadata(
tenantId, EntityType.TEAM, MetadataOperationType.UPDATE, null);
}
}

private void removeServiceAccountOnTransferOfOwnership(AclRequests aclRequest, int tenantId) {
if (Objects.equals(RequestOperationType.CLAIM.value, aclRequest.getRequestOperationType())) {
// remove the service account from the other team as they no longer have any acls using that
// service user left.
Optional<Team> origTeam =
manageDatabase.getTeamObjForTenant(tenantId).stream()
.filter(team -> Objects.equals(team.getTeamId(), aclRequest.getTeamId()))
.findFirst();
origTeam.ifPresent(
team -> {
ServiceAccounts origServiceAccounts = team.getServiceAccounts();
if (origServiceAccounts != null
&& origServiceAccounts.getServiceAccountsList() != null
&& origServiceAccounts.getServiceAccountsList().size() > 0) {
origServiceAccounts.getServiceAccountsList().remove(aclRequest.getAcl_ssl());
}
manageDatabase.getHandleDbRequests().updateTeam(team);
});
}
}

private ResponseEntity<ApiResponse> invokeClusterApiAclRequest(int tenantId, AclRequests aclReq)
throws KlawException {
ResponseEntity<ApiResponse> response = null;
Expand Down
27 changes: 27 additions & 0 deletions core/src/test/java/io/aiven/klaw/helpers/db/rdbms/SelectDataJdbcTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,4 +353,31 @@ public static Stream<Arguments> doesAclExist() {
Arguments.of(101, "TEST", "Topic3", 19292, false),
Arguments.of(101, "TEST", "Topic4", 120202, false));
}

@ParameterizedTest
@MethodSource
public void doesAclSslExistInTeam(
int teamId, int tenantId, String aclSsl, boolean expectedResult) {

when(aclRepo.existsAclSslInTeamInTenant(teamId, tenantId, aclSsl)).thenReturn(expectedResult);

assertThat(selectData.existsAclSslInTeam(teamId, tenantId, aclSsl)).isEqualTo(expectedResult);

verify(aclRepo, times(1)).existsAclSslInTeamInTenant(teamId, tenantId, aclSsl);
}

public static Stream<Arguments> doesAclSslExistInTeam() {

return Stream.of(
Arguments.of(1002, 101, "Alice", true),
Arguments.of(1004, 101, "White-Queen", true),
Arguments.of(1002, 101, "Red-Queen", true),
Arguments.of(1004, 101, "Tarrant", true),
Arguments.of(1009, 101, "Hamish", false),
Arguments.of(1, 101, "Hatter", true),
Arguments.of(21, 101, "Caterpillar", false),
Arguments.of(1006, 101, "Cheshire", false),
Arguments.of(1009, 101, "Knave", false),
Arguments.of(1009, 101, "White-Rabbit", false));
}
}
Loading