Azure / PSRule.Rules.Azure

Rules to validate Azure resources and infrastructure as code (IaC) using PSRule.
https://azure.github.io/PSRule.Rules.Azure/
MIT License
383 stars 84 forks source link

feat(new): Added Azure.ServiceBus.GeoReplica #2989

Closed BenjaminEngeset closed 1 month ago

BenjaminEngeset commented 1 month ago

PR Summary

Working towards #2988

Added Azure.ServiceBus.GeoReplica.

PR Checklist

BenjaminEngeset commented 1 month ago

Hi @BernieWhite. I need some feedback from you on how to construct the rule body on this in the best way possible, also given the updates that will come so we don't have to do major refactor. The documentation states specifically Currently only a single secondary is supported.

Do you have any thoughts on how the rule body can look like? I have uploaded the draft design in this PR, but I feel it is to static and if the order is wrong in the array it will not work correctly. Maybe two contains methods can be used? If the operand is an array of strings, only one string must contain the specified string.

https://learn.microsoft.com/en-us/azure/service-bus-messaging/service-bus-geo-replication#using-bicep-template

BernieWhite commented 1 month ago

Hi @BernieWhite. I need some feedback from you on how to construct the rule body on this in the best way possible, also given the updates that will come so we don't have to do major refactor. The documentation states specifically Currently only a single secondary is supported.

Do you have any thoughts on how the rule body can look like? I have uploaded the draft design in this PR, but I feel it is to static and if the order is wrong in the array it will not work correctly. Maybe two contains methods can be used? If the operand is an array of strings, only one string must contain the specified string.

https://learn.microsoft.com/en-us/azure/service-bus-messaging/service-bus-geo-replication#using-bicep-template

Thanks @BenjaminEngeset I think the can primarily count how many entries are in properties.geoDataReplication.locations. i.e should be 2 or greater. The specific locations or roles and their order don't really matter and could easily change.

BenjaminEngeset commented 1 month ago

Ready for review @BernieWhite.

I noticed one thing that made me unsure about using YAML and that was that once the AllOf operator finds a fail, it will return and yield only that. So when the customer thinks that it is only the SKU that is the problem, next time it will yield the geo-replication configuration. I want both failures to be yielded. What do you think, is there anything we can do or should we move over to PowerShell?

BernieWhite commented 1 month ago

Ready for review @BernieWhite.

I noticed one thing that made me unsure about using YAML and that was that once the AllOf operator finds a fail, it will return and yield only that. So when the customer thinks that it is only the SKU that is the problem, next time it will yield the geo-replication configuration. I want both failures to be yielded. What do you think, is there anything we can do or should we move over to PowerShell?

That's fair. We should just improve that behaviour in PSRule. I'm fine for the YAML approach.

BenjaminEngeset commented 1 month ago

Thank you for the feedback. The requested changes have been implemented, @BernieWhite.