New resource: wiz_saml_group_mapping #216
Conversation
# Conflicts: # internal/acceptance/common.go # internal/acceptance/provider_test.go
|
Hey, thanks for your comments. I think it's ready for merging now 😁 |
|
I will make time in the coming days to review, thanks for all the contributions so far! |
…pdateSAMLGroupMappingInput to resource impl
|
I left a few comments and a question/consideration around the schema of the resource itself. Also once ready for another review, please update the PR comments with the latest acc test run results |
|
Hi @jschoombee, |
|
Output of latest acceptance test: |
|
Thanks, think its good for merge. One last question around the acc tests, I notice quite a few 429 errors (rate limiting back off); is this happening in general for other acc tests also? |
|
I think that is happening because it gets checked first if the mapping exists which involves a lot of queries because it has to go through all the data until it eventually finds the one or comes to the end. |
Maybe a pagination issue? Your acc test is just comparing one mapping against the saml idp's slice of mappings, which may be compute heavy but shouldn't cause a 429 which says that there are just too many API requests going on and for an acceptance test that shouldn't happen. How many mappings are present for the saml idp you are targeting? For an idp with 1000s of group mappings in eu1 region I'm testing with, there's no validation of I think this needs to be tested out a bit more and possibly raised with Wiz. Query that is returning ±2300 results and should be returning an error due to query samlIdentityProviderGroupMappings ($id: ID!, $first: Int! $after: String){
samlIdentityProviderGroupMappings (
id: $id,
first: $first
after: $after
) {
pageInfo {
hasNextPage
endCursor
}
nodes {
providerGroupId
role {
id
}
projects {
id
}
}
}
}
###
{
"first": 1000000000,
"id": "company-sso"
}
The behaviour now is that it allows to create a resource for an IDP with multiple mappings, could be useful for say grouping by department.. This means that for every resource, its another round trip to get mappings right? In this case it might make sense to implement a data source for |
I am testing with an idp that has about 1900 group mappings. You are right, the |
So it needs a bit of attention then before merge, shall I release your other changes in the interim? Would you be open to continue working on this? |
Hey, yes it would be nice if you could release the other changes in the mean time. |
Would you mind submitting a PR to fix goreleaser (as I can't approve my own PRs and only a member of this repo), you can reference the open pull request in the repo? Once done then I can release your changes. In terms of this PR I think it would be good to first get clarity from the vendor around the pagination issue, as we don't want to see rate limits being hit in simple acceptance tests. |
Sure, I created a PR for that: #224. |
Suggest to log a support case with them for the same, it doesn't make sense for me to be the proxy for this. |
|
Hi @jschoombee, |
|
Hey @jschoombee, |
This new resources allows the mapping of a saml provider group to a role on project(s) or on global scope without the need for the saml identity provider resource.
Output of acceptance test: