feat(new): Added Azure.EventHub.Firewall

[BenjaminEngeset]

PR Summary

Fixes #2701

Added Azure.EventHub.Firewall.

PR Checklist

• PR has a meaningful title
• Summarized changes
• Change is not breaking
• This PR is ready to merge and is not Work in Progress
• Rule changes
  • Unit tests created/ updated
  • Rule documentation created/ updated
  • Link to a filed issue
  • Change log has been updated with change under unreleased section
• Other code changes
  • Unit tests created/ updated
  • Link to a filed issue
  • Change log has been updated with change under unreleased section

[BenjaminEngeset]

Hi @BernieWhite. I want some feedback from you on this one, what do you think? The AnyOf method used can give customers some output that can be confusing, but after reading the rule documentation I suppose it will make sense?

I can also go for another design with an if else statement using $PSRule.TargetType, but will end up with some "duplicated" code by doing so for each block.

[BernieWhite]

@BenjaminEngeset Nice work so far. This one does have one additional complexity that you may not have encountered with previous rules, that I hadn't thought about in the initial issue.

There is an order of precedence on which will take effect for the final resource.

ARM take the approach of deploy the parent resource first Microsoft.EventHub/namespaces then the sub-resource Microsoft.EventHub/namespaces/networkRuleSets.

The impact of this is: If the sub-resource is set it will override the configuration on the parent resource, because it is deployed last and modifies the state of the parent.

Use Azure.SQL.AAD rule as a guide for the implementation: https://github.com/benjaminengeset/psrule.rules.azure/blob/daf85d5b183913461c6ef4d575e8fe93aae727a6/src/PSRule.Rules.Azure/rules/Azure.SQL.Rule.ps1#L58-L80

i.e.

• If the resource type is Microsoft.EventHub/namespaces/networkRuleSets check properties.publicNetworkAccess or properties.defaultAction. Using AnyOf is fine.
• If the resource type is Microsoft.EventHub/namespaces check for sub-resources an loop over any found performing a check properties.publicNetworkAccess or properties.defaultAction. Using AnyOf is fine.
• Fall back to check of the parent resource properties properties.publicNetworkAccess. Using AnyOf is fine.

[BernieWhite]

Some additional suggestions for docs.

[BenjaminEngeset]

@BenjaminEngeset Nice work so far. This one does have one additional complexity that you may not have encountered with previous rules, that I hadn't thought about in the initial issue.

There is an order of precedence on which will take effect for the final resource.

ARM take the approach of deploy the parent resource first Microsoft.EventHub/namespaces then the sub-resource Microsoft.EventHub/namespaces/networkRuleSets.

The impact of this is: If the sub-resource is set it will override the configuration on the parent resource, because it is deployed last and modifies the state of the parent.

Use Azure.SQL.AAD rule as a guide for the implementation: https://github.com/benjaminengeset/psrule.rules.azure/blob/daf85d5b183913461c6ef4d575e8fe93aae727a6/src/PSRule.Rules.Azure/rules/Azure.SQL.Rule.ps1#L58-L80

i.e.

• If the resource type is Microsoft.EventHub/namespaces/networkRuleSets check properties.publicNetworkAccess or properties.defaultAction. Using AnyOf is fine.
• If the resource type is Microsoft.EventHub/namespaces check for sub-resources an loop over any found performing a check properties.publicNetworkAccess or properties.defaultAction. Using AnyOf is fine.
• Fall back to check of the parent resource properties properties.publicNetworkAccess. Using AnyOf is fine.

Makes sense. I was aware of this, but didn't expect us to take that into consideration, but now that we are aligned I'll adjust accordingly. Great feedback. The $Assert.AnyOf() method doesn't work so well for me.

Writing it with AnyOf{} instead works. What is this, a global function?

Found it here:

PSRule.Rules.Azure/src/PSRule.Rules.Azure/rules/Azure.Template.Rule.ps1

Line 131 in b65fb70

AnyOf {

[BernieWhite]

@BenjaminEngeset Nice work so far. This one does have one additional complexity that you may not have encountered with previous rules, that I hadn't thought about in the initial issue.
There is an order of precedence on which will take effect for the final resource.
ARM take the approach of deploy the parent resource first Microsoft.EventHub/namespaces then the sub-resource Microsoft.EventHub/namespaces/networkRuleSets.
The impact of this is: If the sub-resource is set it will override the configuration on the parent resource, because it is deployed last and modifies the state of the parent.
Use Azure.SQL.AAD rule as a guide for the implementation: https://github.com/benjaminengeset/psrule.rules.azure/blob/daf85d5b183913461c6ef4d575e8fe93aae727a6/src/PSRule.Rules.Azure/rules/Azure.SQL.Rule.ps1#L58-L80
i.e.

• If the resource type is Microsoft.EventHub/namespaces/networkRuleSets check properties.publicNetworkAccess or properties.defaultAction. Using AnyOf is fine.
• If the resource type is Microsoft.EventHub/namespaces check for sub-resources an loop over any found performing a check properties.publicNetworkAccess or properties.defaultAction. Using AnyOf is fine.
• Fall back to check of the parent resource properties properties.publicNetworkAccess. Using AnyOf is fine.

Makes sense. I was aware of this, but didn't expect us to take that into consideration, but now that we are aligned I'll adjust accordingly. Great feedback. The $Assert.AnyOf() method doesn't work so well for me.

Writing it with AnyOf{} instead works. What is this, a global function?

Found it here:

PSRule.Rules.Azure/src/PSRule.Rules.Azure/rules/Azure.Template.Rule.ps1

Line 131 in b65fb70

AnyOf {

$Assert.AnyOf would require , between because it is an array of results. See: https://microsoft.github.io/PSRule/v2/concepts/PSRule/en-US/about_PSRule_Assert/#aggregating-assertion-methods

Or use the { } if that works better for you. Either is fine.

[BenjaminEngeset]

Should be ready for another review @BernieWhite. I don't undestand why $ruleresult[2].Reason and up to $ruleresult[4].Reason is empty. Any idea why the property has no value on these objects?

[BernieWhite]

Should be ready for another review @BernieWhite. I don't undestand why $ruleresult[2].Reason and up to $ruleresult[4].Reason is empty. Any idea why the property has no value on these objects?

I did a bit of investigating and this seems to be a bug with $Assert.AnyOf, it is currently dropping any aggregated reasons. Using AnyOf produces the correct result.

[BernieWhite]

Thanks @BenjaminEngeset. All good to merge.