New Functionality - DataSource okta_app_* `limit` value

exitcode0 commented 8 months ago

Community Note

Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request
If you are interested in working on this issue or have submitted a pull request, please leave a comment

Terraform Version

Terraform v1.5.7
on darwin_arm64
+ provider registry.terraform.io/hashicorp/aws v5.17.0
+ provider registry.terraform.io/okta/okta v4.5.0

Affected Resource(s)

data_source_okta_app

Expected Behavior

We should throw an explicit error if okta_app would return an non-deterministic result

Actual Behavior

We set the limit to 1 on all data_source_okta_app resources Here When we get the results back, we only check if len(appList) < 1 We should also be checking if we get multiple results as well len(appList) > 1 to avoid unexpected behavior

Important Factoids

This is essentially the same bug as #1783, just on a different data_source

I'm not sure if it would be possible to write a test that returns more/less objects to ensure data sources behave as expected, If this is possible it could identify all instances of this bug

I'm curious if Okta team wants to keep the both singular & plural data sources or if there is a want to combine these? I think it might make sense to standardize on plural data sources and expose a limit parameter that the user can use (if they need it) I'm not familiar with provider development, but I'm curious if this response length validation is a problem that should be solved by the provider, or if users should be left to mitigate the risk that their vague API query returns a response of unexpected length? 🤷

References

1783
1528

duytiennguyen-okta commented 8 months ago

OKTA internal reference https://oktainc.atlassian.net/browse/OKTA-682422

duytiennguyen-okta commented 7 months ago

There is a caveat here. According to PR #1115 and issue #1111, the app data source is using the q parameter to filter apps by label using the Okta Apps API. In the API's documentation for the List Apps request, the q parameter doesn't check for equality, but rather the startsWith operator. That's why we have allow this to exist. There is already log that multiple app is found

okta / terraform-provider-okta