-
Notifications
You must be signed in to change notification settings - Fork 23
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
V3.1.5 #89
V3.1.5 #89
Conversation
…Duration and relayState
…or nested OU support
…or nested OU support
Fails to self-mutate between current version in master and the new version because, in the OrgMain account, on updating the CF stack It's really a pain when the self-mutation fails :( |
The recent fixes (current as of commit id 2958f9d) get rid of all the errors I’ve been seeing. However, when updating an existing permission set that had no inline policy and no managed policies, when I try to add AWS managed policies in the S3 permission set, it doesn’t provide any error over SNS, but it doesn’t update. Here is my permission set data:
The previously existing permission set in AWS SSO has zero managed policies and no inline policy. Also, even after the latest commit (2958f9d), I still get rate limit exceeded when pushing to S3 many (new) permission sets, permission set updates, new links data, or links data updates. I have to sleep for about 9 seconds between S3 PutObject calls to keep it from getting rate limited. For clarification, I’m not getting rate limited by S3; it’s the Lambda functions within SSO Extensions that get rate limited by AWS SSO, far as I can tell. |
Hi @allquixotic , I had re-factored the queueing design (now taking into consideration that Lambda-SQS trigger does not really abide by concurrency limits you set up for the lambda) such that using both a batch size of 1, along with message group ID's ranging between 0 and 9 (take the last digit of the account) and the queue being FIFO, this should theoretically stay within the 10 live calls threshold at any given time. Rather than hyptohesising this further, we will be setting up load test environment with approximately ~50 accounts and validate /fix the behaviour. Regarding the permission set scenario you described, using the same commit ID, I first created a PS with empty inline policy and managed policies array, and then updated it to reflect this:
|
…reated prior to state machine; Tune retry and jitter back-off parameters to handle heavier loads
…nd then state machine; tune retry and jitter delay params for handling throttling scenarios in heavy load
...
I tried it again, and it still does not attach the managed policies. It doesn't throw any error over SNS either. Is there some specific function I should look for logs? |
@allquixotic , when you are provisioning the changed permission set, can you see what's happening from logs in the log group Thank you, |
We have tested the Nested OU and account assignment to orgs with over 40 accounts and that is all working. An issue that did arise was doing account assignment at the root level with 126 accounts. This fails in the step function with the error Root mapping is only mapping to five accounts, no error in lambda or no error email. When the step function was checked the error above was noticed. Mapping file - root%all%aws-aes-engg-sso-ps%aws-aes-engg-sso%GROUP%ssofile step input for failing step below. I replaced the sensitive details
|
…ugh FIFO+messae group ID restriction
Hi @jjleigh , We're unable to reproduce the root issue that you are seeing with the step functions. We've tried the below use cases in 2 of our dev environments and are unable to reproduce the issue. @jmejco had done a stellar job of setting up an AWS org with ~ 60 accounts , so that we could stress test the solution and document processing teams for average loads. These tests were done as part of this validation
As this is specific to your environment, could you please validate the following:
|
Hi @allquixotic , we are in the process of doing more stress tests on the solution, but so far we've validated ~ 300 account assignment create/delete operations and ensured that the throttle tuning works with 0 failures. If it's feasible for you , could you try the same bulk load flow you were trying earlier with the latest commit ID please? Only if that's feasible for you. Thank you, |
We tried this and got a message saying: Cannot specify --no-paginate along with pagination arguements: --page-size so we tried without the page size flag and got the same error from the step function that I shared above. |
The correct argument to provide the |
Hi, with the latest commits, I uploaded one item to However, I still can reproduce two issues that may be outside the scope of this PR:
|
There's also a race condition, where, while waiting for accounts to be deprovisioned (which can take minutes if you have dozens of accounts provisioned for the same permission set), trying to delete the permission set won't work. So to do something like clear the links_data and permission_set data in S3 for a given permission set, you first have to delete the links_data, wait, then once it's deprovisioned, delete the permission_set file. |
Hi @allquixotic ,
Thank you, |
I'm OK with most of what you wrote and agree, but I tried something completely new: First, I deleted my permission set (after removing all the account links -- I had to do 9 of them by hand, because for some reason, deleting all the links data relative to this permset didn't delete all the account links, and none of them were established by hand) Once confirming the permission set did not exist at all in AWS SSO, I re-uploaded the .json into permission_sets, and the account links into links_data. The link was Then, confusingly, I neither received an SNS error notification about this, nor did the permission set get created. So, something is failing at a low level with this particular permission set, so that it just won't create it, and won't tell me why it can't via the usual error email. Here is the permission set verbatim, except that
I'll note that SSO Extensions does create and provision many other permission sets successfully, but this specific one just doesn't work for some reason. I'll check the log groups you suggested earlier when I have a chance. |
Thank you for the updates @allquixotic . When you do get a chance, please have a look at those log files and see what's happening with this specific permission set. If the issue persists, please can you create a new issue so that this could be tracked separately. For now, the PR would be approved and merged in the next few days as all the target stories for the PR are now handled. Do let us know if you still see issues that are in the scope of this PR i.e. nested OU support (both for provisioning & self sustaining flows), handle throttling for any organizations load, updated permission set schema definitions to align with standard AWS SSO admin API. Thank you, |
Thanks, I agree, this PR seems to resolve the issues it claims to. No need to hold the PR for other unrelated issues. Just reporting the results of my testing. Tell you what; to make it easier to forget about the messages in this PR comments, I'll open a new issue with my findings about that bad permission set that isn't being created or updated. |
Issue #, if available: Fixes #88, Fixes #87, Fixes #86, Fixes #82
Description of changes:
NotAction
,NotResource
,NotCondition
keys as well as allow actions/resources elements to be specified as either as single string (or) array of strings. With these changes, the permission set object aligns completely to the schema spec supported by AWS SSO admin API for a permission set object, except forsessionDurationInMinutes
. The solution still expects this to be a string instead of a number as it's necessary for ISO 86401 conversions.60 minutes
when unset. So, the permission set with an empty session duration set by the user is created in SSO with a value of60 minutes
waiter
blocks within the pagination process when the solution discovers the target list of accounts that fall under aroot, ou_id, or account_tag
scope , to avoid getting throttled.SupportNestedOU
, the solution would automatically discover all the accounts under the specified ou_id , and traverses the complete tree including any child OU's. Similarly, when this behaviour is enabled, in case of an account moving from one OU to another OU, the solution would also traverse the tree upwards until root to discover the complete list of parent OU's and compute the list of account assignments that are to be added/removed.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.