Azure / PSRule.Rules.Azure-quickstart

Sample code you can use to quickly start using PSRule for Azure.
https://azure.github.io/PSRule.Rules.Azure/
MIT License
32 stars 23 forks source link

Unsure of results and behaviour when using this repo #43

Closed kamfaima closed 10 months ago

kamfaima commented 10 months ago

I've just cloned this repo into my Azure DevOps org, added a pipeline based on .pipelines/azure-analyze.yaml, made one small change (see screenshot below) to generate a fail, and ran the pipeline.

CleanShot 2023-11-15 at 14 43 49

The pipeline runs, but fails—is it meant to fail given I've literally just cloned the repo and made a small modification? In short, I'm unsure of what the expected output should be. I'm not sure if everything is working as expected or there is indeed something broken within this repo or the pipeline has changed its behaviour.

I've included the screenshots of the pipeline job so I'd appreciate it if you/I/we can work through it together.

So the below screenshot shows an error against AZR-000355 which I'm expecting given the modification I made. That's fine. I'm guessing the next line, "One or more assertions failed" is also fine since that is true. CleanShot 2023-11-15 at 14 46 09@2x

This next screenshot shows the warnings.

The next screenshot shows the Extensions tab. This looks fine to me. CleanShot 2023-11-15 at 14 46 26@2x

Finally, there are no artifacts produced (under the Scans tab) as the pipeline never got to that task. CleanShot 2023-11-15 at 15 04 08@2x

From what I've seen in the YT videos and my expectations given the minimal modifications, the job should complete with a status of Warning as opposed to Failed so that one can view the prettyfied output under Scans?

Edit: ok so I just re-read some of the documentation and have configured the pipeline to continue on error. Now I can see the Scans output which is great. But if this is set to true just so I can get some pretty outputs, then wouldn't it be more suited to permanently leave this to true?

Thank you and I appreciate your time!

BernieWhite commented 10 months ago

Thanks for the feedback @kamfaima. In relation to your questions:

  1. The comments in the bicep file are intended to allow you to see the pipeline is a failing and passing state (should pass by default). The change you have mentioned by changing the Key Vault defaultAction from Deny to Allow opens the firewall of the Key Vault to accept traffic from any network location. So it is designed to fail. Similar to the other comment indicating specific changes.
  2. "I'm not sure why that first warning (refs/heads/main has not been processed) is there?" - This is a good point, we should turn this off for the repository, as it can be confusing in the case you have mentioned. This is a PSRule default, indicating that something did not have any matching rules associated with it. However specifically for refs/heads/main is it related to processing of the Git branch which can be rules associated with it for checking code ownership and the like. But it is not hugely relevant for this use case. To disable this warning set unprocessedObject: Ignore in ps-rule.yaml see: https://microsoft.github.io/PSRule/v2/concepts/PSRule/en-US/about_PSRule_Options/#executionunprocessedobject
  3. "Finally, there are no artifacts produced (under the Scans tab) as the pipeline never got to that task." thanks for spotting this. This is an issue we should address in the repo.

The pipeline should be updated to include the condition: succeededOrFailed() property on the artifact upload task CodeAnalysisLogs.

Alternatively continue on error will work however generally the desired outcome would be to break the pipeline to prevent merging a PR or prevent a release.


Fixes to ADO pipeline:


Hopefully that answers your questions.

kamfaima commented 10 months ago

Thanks for the feedback @kamfaima. In relation to your questions:

1. The comments in the bicep file are intended to allow you to see the pipeline is a failing and passing state (should pass by default). The change you have mentioned by changing the Key Vault `defaultAction` from `Deny` to `Allow` opens the firewall of the Key Vault to accept traffic from any network location. So it is designed to fail. Similar to the other comment indicating specific changes.

2. **"I'm not sure why that first warning (refs/heads/main has not been processed) is there?"** - This is a good point, we should turn this off for the repository, as it can be confusing in the case you have mentioned. This is a PSRule default, indicating that something did not have any matching rules associated with it. However specifically for `refs/heads/main` is it related to processing of the Git branch which can be rules associated with it for checking code ownership and the like. But it is not hugely relevant for this use case. To disable this warning set `unprocessedObject: Ignore` in `ps-rule.yaml` see: https://microsoft.github.io/PSRule/v2/concepts/PSRule/en-US/about_PSRule_Options/#executionunprocessedobject

3. **"Finally, there are no artifacts produced (under the Scans tab) as the pipeline never got to that task."** thanks for spotting this. This is an issue we should address in the repo.

The pipeline should be updated to include the condition: succeededOrFailed() property on the artifact upload task CodeAnalysisLogs.

Alternatively continue on error will work however generally the desired outcome would be to break the pipeline to prevent merging a PR or prevent a release.

Fixes to ADO pipeline:

* [x]  Disable not processed warning in `ps-rule.yaml`.

* [x]  Update pipeline to include `condition: succeededOrFailed()`.

Hopefully that answers your questions.

Hi @BernieWhite thanks for taking the time to answer my questions! For now, yes you've answered my questions however I do have more (apologies in advance for my obtuseness!) but I'll ask them in the actual PSRule for Azure GitHub page so it's more relevant to the subject at hand.