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
Prevent orphan ACl without service user being created #2442
Conversation
…cl, Also if acl is created on server, then continue and ensure the service user is also created Signed-off-by: Aindriu Lavelle <aindriu.lavelle@aiven.io>
cluster-api/src/main/java/io/aiven/klaw/clusterapi/services/AivenApiService.java
Dismissed
Show dismissed
Hide dismissed
cluster-api/src/main/java/io/aiven/klaw/clusterapi/services/AivenApiService.java
Dismissed
Show dismissed
Hide dismissed
@@ -55,14 +55,6 @@ public ResponseEntity<ClusterStatus> getApiStatus() { | |||
return new ResponseEntity<>(ClusterStatus.ONLINE, HttpStatus.OK); | |||
} | |||
|
|||
// @RequestMapping(value = "/reloadTruststore/{protocol}/{clusterName}", method = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice one
cluster-api/src/main/java/io/aiven/klaw/clusterapi/services/AivenApiService.java
Show resolved
Hide resolved
extractAcl(clusterAclRequest, aclResponse.getBody()); | ||
if (aivenAclStructOptional.isEmpty()) { | ||
aclResponse = restTemplate.postForEntity(uri, request, AivenAclResponse.class); | ||
aivenAclStructOptional = extractAcl(clusterAclRequest, aclResponse.getBody()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also here a null check ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added a null check into extractAcl. which returns an empty optional.
@@ -133,7 +143,7 @@ private void handleAclCreationResponse( | |||
projectName, | |||
serviceName, | |||
clusterAclRequest.getTopicName()); | |||
resultMap.put("result", "Failure in adding acls" + response.getBody()); | |||
resultMap.put("result", "Failure in adding acls" + response.getBody().getMessage()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also here a null check ?
@@ -145,12 +142,12 @@ public void createAclsServiceAccountExists() throws Exception { | |||
+ "/acl"; | |||
|
|||
AivenAclResponse aivenAclResponse = utilMethods.getAivenAclResponse(); | |||
String aivenAclResponseString = OBJECT_MAPPER.writeValueAsString(aivenAclResponse); | |||
// String aivenAclResponseString = OBJECT_MAPPER.writeValueAsString(aivenAclResponse); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delete this ?
…null pointers Signed-off-by: Aindriu Lavelle <aindriu.lavelle@aiven.io>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Add check if acl already exists in Klaw before allowing creation of acl, Also if acl is created on server, then continue and ensure the service user is also created
Linked issue
Resolves: #2412
What kind of change does this PR introduce?
What is the current behavior?
When creating an ACL, klaw created the ACL and then creates the related service users.
If the service user limit is hit the acl is left but the service user is not.
Re-approving the request does not create the service user and the acl needs to be manually removed before re-approving the ACL.
What is the new behavior?
When creating an ACL, klaw created the ACL and then creates the related service users.
If the service user limit is hit the acl is left but the service user is not.
Re-approving the request does creates the service user.
We also check if an ACL already exists in Klaw before allowing a new Klaw ACL Request to be created
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)
main
branch have been pulledpnpm lint
has been run successfullyA follow up PR is required to ensure that if a delete request is created for an acl that was deleted on the cluster that it passes.
This means sync functionality should be used when things get out of sync but requests won't be left failing if they have already been removed.