Add support for pulling account assignment creation status #30
Conversation
| if err != nil { | ||
| var ae *awsSsoAdminTypes.AccessDeniedException | ||
| if errors.As(err, &ae) { | ||
| l.Info("aws-connector: access denied while attempting to check status. Assuming account assignment creation is complete.", zap.Error(err)) |
There was a problem hiding this comment.
I don't think we should assume it worked here. But we also don't want to have had it worked and we report that it didn't. Hmm. Maybe this is fine.
There was a problem hiding this comment.
This is to preserve the existing behavior and not break existing installs. Thoughts on another way?
There was a problem hiding this comment.
Well, also though imagine you make a request and we report it fails but in reality the person is actually provisioned. I think it's safer to assume it worked and be wrong than assume it didn't work and be wrong.
pkg/connector/account.go
Outdated
|
|
||
| attemptCount++ | ||
| attemptLogger.Debug("aws-connector: waiting for account assignment creation to complete, checking status...") | ||
| complete, err = o.checkCreateAccountAssignmentStatus(waitCtx, attemptLogger, createOut.AccountAssignmentCreationStatus) |
There was a problem hiding this comment.
Can we actually use complete to determine the grant status? If grant failed, completed is true, does the UI update properly that grant failed?
There was a problem hiding this comment.
if it fails, completed is true, but we also return an error indicating that it failed, so we'd return immediately.
When creating an account assignment(e.g. granting an sso account access to a permission set), it is actually an asynchronous process. This means that it isn't applied immediately, and the successful response from the create API request doesn't mean that provisioning was successful.
In order to validate the provisioning was successful, we must describe the status of the request, and wait for it to return successfully. It is possible for the request to fail for some reason after the fact, so we need to look out for that response, and return an error appropriately.
Unfortunately, this API requires an additional IAM permission that we don't request, so users will need to be update permissions for us to take advantage of this.
We should take a look and see if there are other async operations that we're doing, and generalize the wait loop if possible.