Added Azure.APIM.PolicyBase

[BenjaminEngeset]

PR Summary

Fixes #2072

Added Azure.APIM.PolicyBase rule.

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 need some feedback and suggestions if needed. Correct design?

Currently this should find if the sections has a base property. It does not check if its added before or after an statement, but that does not really matter that much, if an customer have added it after an statement it is probably done for an reason. You can place the base element before or after any policy element in a section.

Global policies is not needed, so excluded, any idea how we can solve that within the function to not break the other cors rule?

[BernieWhite]

Hi @BernieWhite

I need some feedback and suggestions if needed. Correct design?

Currently this should find if the sections has a base property. It does not check if its added before or after an statement, but that does not really matter that much, if an customer have added it after an statement it is probably done for an reason. You can place the base element before or after any policy element in a section.

Global policies is not needed, so excluded, any idea how we can solve that within the function to not break the other cors rule?

I think for this rule ordering doesn't matter. We want to make sure that base is referenced in each element for all non-global polices.

However it is a good idea for a new rule to limit which properties can be used above base such as cors and set-variable, maybe a configurable list.

Does that help?

[BenjaminEngeset]

Hi @BernieWhite
I need some feedback and suggestions if needed. Correct design?
Currently this should find if the sections has a base property. It does not check if its added before or after an statement, but that does not really matter that much, if an customer have added it after an statement it is probably done for an reason. You can place the base element before or after any policy element in a section.
Global policies is not needed, so excluded, any idea how we can solve that within the function to not break the other cors rule?

I think for this rule ordering doesn't matter. We want to make sure that base is referenced in each element for all non-global polices.

However it is a good idea for a new rule to limit which properties can be used above base such as cors and set-variable, maybe a configurable list.

Does that help?

Yes. What do you think about the rule logic provided in this PR?

Something that should be done otherwise?

I also have to find a way to exclude globals here that may be configured within the parent or create a new helper function. Help?

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

Line 341 in 2f265e2

$policies = $PSRule.GetPath($TargetObject, '..resources[[email protected] == ''Microsoft.ApiManagement/service/policies'' || @.type == ''Microsoft.ApiManagement/service/apis/resolvers/policies'' || @.type == ''Microsoft.ApiManagement/service/products/policies'' || @.type == ''Microsoft.ApiManagement/service/apis/policies'' || @.type == ''Microsoft.ApiManagement/service/apis/operations/policies'']')

[BernieWhite]

@BenjaminEngeset The design is fine. We just need to exclude the global (all APIs) policy.

The policy will be type Microsoft.ApiManagement/service/policies with name policy. We should be able to add this to the pre-condition within GetAPIMPolicyNode. Put it here:

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

Line 345 in 2f265e2

if ($_.properties.format -in 'rawxml', 'xml' -and $_.properties.value) {

[BenjaminEngeset]

@BernieWhite Gotcha I think, please check latest update. But doesn't this cause Azure.APIM.CORSPolicy to not work against global policies since we're using the same helper function there?

[BernieWhite]

Yes you will need to add something like a switch to allow this to work correctly and not break the other rule.

[BenjaminEngeset]

@BernieWhite I'm having some issues and debugging has not given me the faulty code yet.

I am getting failed test results where I expect it to pass. Probably the rule is configured wrong somewhere.

   TargetName: api-policy-C

RuleName                            Outcome    Recommendation
--------                            -------    --------------
Azure.APIM.PolicyBase               Fail       Base element for any policy element in a section should be configured.

PS C:\Users\benjamine\PSRule.Rules.Azure\PSRule.Rules.Azure> $ruleResult[5].detail

Reason
------
{Path base: Does not exist., Path base: Does not exist., Path base: Does not exist., Path base: Does not exist.}

When testing with the policy value outside PSRule, I am able to see that all sections contain the base member.

PSRule.Rules.Azure/tests/PSRule.Rules.Azure.Tests/Resources.APIM.json

Line 3090 in ddbbba9

"value": "<policies><inbound><base /><ip-filter action=\"allow\"><address-range from=\"51.175.196.188\" to=\"51.175.196.188\" /></ip-filter></inbound><backend><base /></backend><outbound><base /></outbound><on-error><base /></on-error></policies>",

[BenjaminEngeset]

@BernieWhite Thanks for the catch!

I'm still struggling with one particular test case, where not all sections has a base element, only certain sections.

This is a pass, any idea why? I cannot see that adding an all of method should make any impact.

PSRule.Rules.Azure/tests/PSRule.Rules.Azure.Tests/Resources.APIM.json

Line 3102 in b76be98

"value": "<policies><inbound><ip-filter action=\"allow\"><address-range from=\"51.175.196.187\" to=\"51.175.196.187\" /></ip-filter></inbound><backend><base /></backend><outbound><base /></outbound><on-error><base /></on-error></policies>",

[BernieWhite]

@BenjaminEngeset I can't find an issue with your code. Looks like a PSRule bug handling XML nodes. I've updated the rule slightly which works around the issue, but need to do some further investigation and release a fix. I'll log the issue on the PSRule repo.

[BenjaminEngeset]

@BenjaminEngeset I can't find an issue with your code. Looks like a PSRule bug handling XML nodes. I've updated the rule slightly which works around the issue, but need to do some further investigation and release a fix. I'll log the issue on the PSRule repo.

Great, thanks!

[BenjaminEngeset]

@BernieWhite Ready for review, let me know what you think.

[BernieWhite]

Great work @BenjaminEngeset! Just a few documentation tweaks and we should be good to merge.

[BenjaminEngeset]

Hi @BernieWhite, thanks for the great feedback! Adjusted accordingly, what do you think?

[BernieWhite]

Thanks @BenjaminEngeset. All good to merge.