hashicorp / terraform-provider-google

Terraform Provider for Google Cloud Platform
https://registry.terraform.io/providers/hashicorp/google/latest/docs
Mozilla Public License 2.0
2.36k stars 1.75k forks source link

Update generated IAM acceptance tests to include conditions, or omit IAM conditions from missing test detector #18215

Open SarahFrench opened 6 months ago

SarahFrench commented 6 months ago

What kind of contribution is this issue about?

MMv1-based resource

Details

When new resources are added to the provider and associated IAM resources are added at the same time there are missing test detector messages like this: https://github.com/GoogleCloudPlatform/magic-modules/pull/10757#issuecomment-2123420422

The PR author doesn't have power to update the IAM resource tests because they are generated, and the PR reviewer needs to know context about IAM resource generation.

I imagine that generated IAM resource tests don't include conditions because they aren't available for all IAM resources, which is fair enough! If that is the case it would be good to come up with a way to either stop the missing test detector flagging condition arguments in IAM resources, or enable conditions to be conditionally included in generated IAM acc tests.

References

No response

SarahFrench commented 6 months ago

See also this list: https://github.com/GoogleCloudPlatform/magic-modules/pull/10757#issuecomment-2124753717

melinath commented 5 months ago

We should be able to detect whether a resource supports IAM conditions at generation time using the presence / absence of iam_conditions_request_type. For example, see:

melinath commented 5 months ago

Weirdly we still add the conditions field to the IAM resources regardless - we just don't document them.

melinath commented 5 months ago

Unfortunately the missing test detector does not run at generation time, and I don't think we have a great way to introspect whether conditions are supported. So the best option really might be to just ignore the conditions field in the missing test detector, since we know they're generated if supported.

Alternatively, the "real" fix would be to not support the fields unless conditions are supported - but that would be a breaking change and a much larger one as well.

github-actions[bot] commented 4 months ago

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

melinath commented 5 days ago

@trodge it looks like the omission isn't working - https://github.com/GoogleCloudPlatform/magic-modules/pull/12181#issuecomment-2475029873