Improve ORCA implementation #726
Conversation
…ophe in the string, which would split the capture
…on and also include remediation action provided by ORCA
… regular expression
…nsistent between tests
…d (most likely due to missing license)
…y were not configured correct
…uild required columns
…be the goal of the test using "pass" string
…ed the functionality to: 1. Use pass text if unique between sub-tests 2. If pass text not unique, use pass text + area 3. If area text not unique, use test name 4. If test name not unique, fallback to use pass text regardless
…t like other Maester tests
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @tdcthosc, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
This pull request focuses on improving the ORCA (Office 365 Recommended Configuration Analyzer) implementation. According to the PR description, it includes various improvements and bug fixes to the Update-OrcaTests.ps1 script. The changes involve refining regular expressions for parsing test content, enhancing test result details, updating descriptions and remediation actions in markdown files, and updating the generation date in several PowerShell scripts. The goal is to enhance the accuracy and reliability of ORCA tests.
Highlights
- Regular Expression Improvements: The regular expressions used to extract information from the ORCA test files have been updated to be more robust and accurate. Specifically, the regex patterns for
name,passText,failrecommendation,control, andareahave been modified to capture the correct values (lines 147-151). - Enhanced Test Result Details: The script now includes more detailed information in the test results, including handling of
SkipInReportstatements (lines 241-243), checks for failed conditions (lines 246-251), and improved formatting of the result markdown (lines 256-278). - Markdown File Updates: The markdown files for the ORCA tests have been updated to include clearer descriptions, remediation actions, and related links (lines 297-305). The format of the markdown files has also been improved for better readability.
- Code Generation Date Updates: The generated PowerShell scripts now have the correct generation date, ensuring that the scripts are up-to-date (example lines 1, 2).
- Connector Handling: The Get-ORCACollection function now includes the
-SCC:$trueparameter (line 232).
Changelog
Click here to see the changelog
- build/orca/Update-OrcaTests.ps1
- Improved regular expressions for parsing test content (lines 147-151).
- Added logic to handle
SkipInReportstatements (lines 241-243). - Added checks for failed conditions and missing licenses (lines 246-251).
- Improved formatting of test result markdown (lines 256-278).
- Added logic to correlate passText with area, name, and pass values from other files (lines 158-176).
- Added logic to remove HTML tags from descriptions (line 293).
- Changed the way the function name is extracted from the file name (line 152).
- Added logic to ensure that passText and failrecommendation always end with a period (line 154, 180).
- powershell/internal/orca/Add-IsPresetValue.ps1
- Updated the generation date (line 1).
- powershell/internal/orca/Get-AnyPolicyState.ps1
- Updated the generation date (line 1).
- powershell/internal/orca/Get-ORCACollection.ps1
- Updated the generation date (line 1).
- powershell/internal/orca/Get-PolicyStateInt.ps1
- Updated the generation date (line 1).
- powershell/internal/orca/Get-PolicyStates.ps1
- Updated the generation date (line 1).
- powershell/internal/orca/check-ORCA100.ps1
- Updated the generation date (line 1).
- powershell/internal/orca/check-ORCA101.ps1
- Updated the generation date (line 1).
- powershell/internal/orca/check-ORCA102.ps1
- Updated the generation date (line 1).
- powershell/internal/orca/check-ORCA103.ps1
- Updated the generation date (line 1).
- powershell/internal/orca/check-ORCA104.ps1
- Updated the generation date (line 1).
- powershell/internal/orca/check-ORCA105.ps1
- Updated the generation date (line 1).
- powershell/internal/orca/check-ORCA106.ps1
- Updated the generation date (line 1).
- powershell/internal/orca/check-ORCA107.ps1
- Updated the generation date (line 1).
- powershell/internal/orca/check-ORCA108.ps1
- Updated the generation date (line 1).
- powershell/internal/orca/check-ORCA108_1.ps1
- Updated the generation date (line 1).
- powershell/internal/orca/check-ORCA109.ps1
- Updated the generation date (line 1).
- powershell/internal/orca/check-ORCA110.ps1
- Updated the generation date (line 1).
- powershell/internal/orca/check-ORCA111.ps1
- Updated the generation date (line 1).
- powershell/internal/orca/check-ORCA112.ps1
- Updated the generation date (line 1).
- powershell/internal/orca/check-ORCA113.ps1
- Updated the generation date (line 1).
- powershell/internal/orca/check-ORCA114.ps1
- Updated the generation date (line 1).
- powershell/internal/orca/check-ORCA115.ps1
- Updated the generation date (line 1).
- powershell/internal/orca/check-ORCA116.ps1
- Updated the generation date (line 1).
- powershell/internal/orca/check-ORCA118_1.ps1
- Updated the generation date (line 1).
- powershell/internal/orca/check-ORCA118_2.ps1
- Updated the generation date (line 1).
- powershell/internal/orca/check-ORCA118_3.ps1
- Updated the generation date (line 1).
- powershell/internal/orca/check-ORCA118_4.ps1
- Updated the generation date (line 1).
- powershell/internal/orca/check-ORCA119.ps1
- Updated the generation date (line 1).
- powershell/internal/orca/check-ORCA120_malware.ps1
- Updated the generation date (line 1).
- powershell/internal/orca/check-ORCA120_phish.ps1
- Updated the generation date (line 1).
- powershell/internal/orca/check-ORCA120_spam.ps1
- Updated the generation date (line 1).
- powershell/internal/orca/check-ORCA121.ps1
- Updated the generation date (line 1).
- powershell/internal/orca/check-ORCA123.ps1
- Updated the generation date (line 1).
- powershell/internal/orca/check-ORCA124.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA139.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA140.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA141.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA142.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA143.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA156.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA158.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA179.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA180.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA189.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA189_2.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA205.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA220.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA221.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA222.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA223.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA224.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA225.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA226.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA227.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA228.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA229.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA230.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA231.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA232.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA233.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA233_1.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA234.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA235.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA236.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA237.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA238.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA239.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA240.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA241.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA242.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA243.ps1
- Updated the generation date.
- powershell/internal/orca/check-ORCA244.ps1
- Updated the generation date.
- powershell/internal/orca/orcaClass.psm1
- Updated the generation date.
- tests/orca/Test-ORCA100.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA101.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA102.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA103.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA104.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA105.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA106.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA107.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA108.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA108_1.Tests.ps1
- Created a new test file.
- tests/orca/Test-ORCA109.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA110.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA111.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA112.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA113.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA114.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA115.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA116.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA118.Tests.ps1
- Removed the test file.
- tests/orca/Test-ORCA118_1.Tests.ps1
- Created a new test file.
- tests/orca/Test-ORCA118_2.Tests.ps1
- Created a new test file.
- tests/orca/Test-ORCA118_3.Tests.ps1
- Created a new test file.
- tests/orca/Test-ORCA118_4.Tests.ps1
- Created a new test file.
- tests/orca/Test-ORCA119.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA120.Tests.ps1
- Removed the test file.
- tests/orca/Test-ORCA120_malware.Tests.ps1
- Created a new test file.
- tests/orca/Test-ORCA120_phish.Tests.ps1
- Created a new test file.
- tests/orca/Test-ORCA120_spam.Tests.ps1
- Created a new test file.
- tests/orca/Test-ORCA121.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA123.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA124.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA139.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA140.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA141.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA142.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA143.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA156.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA158.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA179.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA180.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA189.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA189_2.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA205.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA220.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA221.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA222.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA223.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA224.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA225.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA226.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA227.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA228.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA229.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA230.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA231.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA232.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA233.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA233_1.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA234.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA235.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA236.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA237.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA238.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA239.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA240.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA241.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA242.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA243.Tests.ps1
- Updated the test description to match the synopsis.
- tests/orca/Test-ORCA244.Tests.ps1
- Updated the test description to match the synopsis.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Did you know?
PowerShell was initially named Monad and was created by Jeffrey Snover at Microsoft.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
The pull request introduces several improvements and bug fixes to the ORCA implementation, particularly in the Update-OrcaTests.ps1 script. The changes include enhanced regex patterns for data extraction, improved error handling, and better handling of edge cases. Overall, the changes seem to improve the robustness and maintainability of the ORCA tests.
Summary of Findings
- Regex Optimization: The regex patterns used to extract data from the content can be improved for better efficiency and readability. Consider using more specific patterns to avoid unnecessary backtracking.
- Code Duplication: There is some code duplication in handling the 'ExpandResults' condition. Consider refactoring this into a separate function to improve maintainability.
- Incomplete Error Handling: The try-catch block in the Test functions only rethrows the exception. More comprehensive error handling should be implemented to provide better diagnostics and prevent unexpected behavior.
- Descriptive messages in tests: The test descriptions in the It blocks of the tests should be more descriptive, providing more context to the reader.
Merge Readiness
The pull request is not yet ready for merging. There are several medium and high severity issues that need to be addressed. Specifically, the regex patterns should be optimized, code duplication should be refactored, and error handling should be improved. I am unable to approve this pull request, and recommend that another reviewer also approves this code before merging.
…l tests after resolving test-generation bug
…l tests after resolving test-generation bug
|
Should resolve some, if not all, issues mentioned in #639 |
build/orca/Update-OrcaTests.ps1
Outdated
| Write-Error "An error occurred during $($content.func): `$(`$_.Exception.Message)" | ||
| throw | ||
| } finally { | ||
| if(`$obj.SkipInReport) { |
There was a problem hiding this comment.
Can we completely skip generating any files for these checks instead of skipping here? For example, skip earlier around line 136. As in:
$skip = [regex]::Match($content.content,"this.SkipInReport\s*=\s*[`$]True",$option)
if ($skip.Success) {
continue
}There was a problem hiding this comment.
Ha, I forgot that I submitted this approach as #705.
There was a problem hiding this comment.
I think we should still include them in Maester, but simply set it as skipped already in the "It" statement of "tests" file. I will get to work on that. Thank you for the suggestion!
But just to clarify my understanding of how this implementation of ORCA has been made by @soulemike, all of the Exchange/SCC commands will always be run regardless and cached. Each individual test simply runs the class on the cached results.
There was a problem hiding this comment.
There is definitely a consideration of how much logic we add to this build process versus adding to the upstream module.
In this instance this type of skip logic would help our module handle the caching more efficiently, which is more around our modules implementation, so I don't see any concerns customizing that here assuming it is dynamic.
Improving this to allow grouping of commands within Get-OrcaCollection to filter what calls to the EXO/SCC module are actually made would be awesome. So in summary I think you both are heading down the right path. Happy to brainstorm if it helps.
There was a problem hiding this comment.
I've done consideration and I think we should keep the way I've done it here, so that we still see it in Maester report as skipped. By putting the skip logic where I've done it, we also still get the markdown file to fill the test details section.
However, one negligible detail is that I've move the "skip" out of "finally" logic for better readability.
|
Marked as draft, as the following is to do:
|
…mpliance is not connected. Only 1 test in ORCA rely on SCC. Add SCC boolean to upstream ORCA module check and skip this check if true and not connected to SCC.
|
Created PR in upstream ORCA module to work with commit d1ae7b4 |
First of all, hats off to @soulemike for the ORCA implementation!
Various improvements and bugfixes to the Update-OrcaTests