Issue transfer ownership of service user #2393
Conversation
Signed-off-by: Aindriu Lavelle <aindriu.lavelle@aiven.io>
Signed-off-by: Aindriu Lavelle <aindriu.lavelle@aiven.io>
| @@ -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", | |||
There was a problem hiding this comment.
could be better to use a jpa instead of native here
There was a problem hiding this comment.
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 )
There was a problem hiding this comment.
Can we try this way, or may be in future
@column(name = "municipal_id", nullable = false)
private Integer municipalId; // <-- field was renamed
There was a problem hiding this comment.
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
There was a problem hiding this comment.
But this rename leads to several changes. Lets try in 2.10
…wnership of a service account. Signed-off-by: Aindriu Lavelle <aindriu.lavelle@aiven.io>
…m the team Signed-off-by: Aindriu Lavelle <aindriu.lavelle@aiven.io>
| // 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())) { | ||
| if (manageDatabase |
There was a problem hiding this comment.
how about moving this condition to a higher level ? So transfer (remove/add) only for this condition.
if (!manageDatabase
.getHandleDbRequests()
.existsAclSslInTeam(
aclRequest.getTeamId(), aclRequest.getTenantId(), aclRequest.getAcl_ssl())) {
// Team still has other Acls left with service user so do not remove from here.
transferServiceUserOwnership(aclReq);
}
Signed-off-by: Aindriu Lavelle <aindriu.lavelle@aiven.io>
When claiming an ACL transfer the ownership of the service user to the new account.
If the existing team no longer has any acls with that service user remove it from their service user accounts.
What kind of change does this PR introduce?
What is the current behavior?
Describe the state of the application before this PR. Illustrations appreciated (videos, gifs, screenshots).
What is the new behavior?
Describe the state of the application after this PR. Illustrations appreciated (videos, gifs, screenshots).
Other information
Additional changes, explanations of the approach taken, unresolved issues, necessary follow ups, etc.
Requirements (all must be checked before review)
mainbranch have been pulledpnpm linthas been run successfully