feat: fix ps rule error for waf aligned aks - avm/res/container-service/managed-cluster

[PixelRobots]

Description

Fixes issue with ps rule for WAF. For some reason it still complains about default, But I don't think the ps rule should be running on the default test as it is not required.

I am unsure how to change that. Happy to do so with some guidance.

Pipeline Reference

Pipeline

Type of Change

• Update to CI Environment or utilities (Non-module affecting changes)
• Azure Verified Module updates:
  • Bugfix containing backwards-compatible bug fixes, and I have NOT bumped the MAJOR or MINOR version in version.json:
    • Someone has opened a bug report issue, and I have included "Closes #{bug_report_issue_number}" in the PR description.
    • The bug was found by the module author, and no one has opened an issue to report it yet.
  • Feature update backwards compatible feature updates, and I have bumped the MINOR version in version.json.
  • Breaking changes and I have bumped the MAJOR version in version.json.
  • Update to documentation

Checklist

• I'm sure there are no other open Pull Requests for the same update/change
• I have run Set-AVMModule locally to generate the supporting module files.
• My corresponding pipelines / checks run clean and green without any errors or warnings

[AlexanderSehr]

Hey @PixelRobots,
some background regarding the defaults test - naturally, we want also the defaults of a module to be WAF-compliant. Either this can be done by providing a waf-compliant default in the module, or by setting it in the test. However, as some settings (e.g., private endpoints) require a user input (in this case a subnet), and because the defaults test should only contain required (or at most conditional) values, there is the option to disable the rule in the min-suppress.Rule.yaml file (with a comment as to why).

No before we go ahead and disable the rule, I have to ask the qeustion if it would make sense to provide a default in the module or not. The input does, afterall, not require a user value. Then again, we'd need to assume somebody would want to run their maintenance work on e.g., a Sunday. What's your take on this?

[eriqua]

Hey @PixelRobots,

Thanks a lot for this fix. You are right, PSRule reliability is enforced on both WAF-aligned and defaults test.

The reason is security by default.
Since defaults tests should only use required input parameters and rely on default values for the remaining inputs, enforcing PSRule on defaults tests helps us understand if by only using default values the module is secure.

To fix this blocker we have 2 alternatives:

• Make the corresponding parameter a required value, so that it can be used by defaults tests. If relevant, we can still allow disabling the additional feature. The idea behind that is to make it more difficult, rather than easier, to deploy a module not secure by default.
• If the PSRule check is involving external dependencies (like when asking for private endpoints) or in general if there is a good reason for that, this check can be skipped for the defaults test only. To do that, the rule name can be listed in this file, with a comment specifying the reason why that rule is not applied.
  I suggest discussing the best option with module owners.
  Cc @ilhaan @JPEasier

[PixelRobots]

As the maintenance windows can be fully customised for the end user needs I would suggest we skip this test. But will wait for the module owners to confirm what they feel is best.

[AlexanderSehr]

As the maintenance windows can be fully customised for the end user needs I would suggest we skip this test. But will wait for the module owners to confirm what they feel is best.

Sounds reasonable to me. There's not obvious 'right' window afterall.
@ilhaan & @JPEasier, curious to hear your thoughts 😏

[PixelRobots]

Any update on this?

[PixelRobots]

Just wondering if there is any update on this yet?

[ilhaan]

I agree with this and think we should skip this test. @JPEasier what do you think?

As the maintenance windows can be fully customised for the end user needs I would suggest we skip this test. But will wait for the module owners to confirm what they feel is best.

[PixelRobots]

Just checking in to see if a decision has been made on this one.

[eriqua]

Just checking in to see if a decision has been made on this one.

@PixelRobots let me bring this up with the core team. Since security is one of our priorities, I want to make sure that skipping the test for defaults test is acceptable, or if having it as a required parameter would fit better.

[AlexanderSehr]

I'd suggest to exclude the requirement from the default test via the min-suppresion rules, set the maintainence window in the WAF test and call it a day. Similar to the PSRule requirement for e.g., always deploying an MSI, I'd argue we can make this judgement call as the maintenanceWindows is so user-specific

[PixelRobots]

Would you like me to make the change and push to this pull request? Or is there another process to follow?

[ReneHezser]

@PixelRobots
We have added examples for Bicep parameter files to the Readme. This has been applied to all published modules but needs to be done for PRs as well. Can you please update your branch and run the Set-AVMModule utility as detailed here. It is required for the validation pipeline to succeed and the contribution to be published.

Please reach out if any support is needed.

[PixelRobots]

All updated and action ran again. all passing. Should be good for merging now.

[jtracey93]

related too Azure/Azure-Verified-Modules#1360

Core team agreement

[JPEasier]

@PixelRobots please see the two suggestions from @eriqua

Looks good so far :)

[PixelRobots]

Thanks I will try and take a look this weekend or first thing Monday.

[PixelRobots]

All updates should be completed now. Fingers crossed this is ready for merging.

[AlexanderSehr]

Off we go 💪