-
Notifications
You must be signed in to change notification settings - Fork 12
added purview (ipp) connection #18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@merill or @SamErde or @soulemike i use the preview and the stable version but both has this issues. Is there any fix that was applied in another part of maester and can used for it too? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds Purview (Information Protection and Purview Session) connection support to the Maester testing framework. The change introduces a new optional parameter to include Purview tests in test runs, similar to the existing Exchange Online integration.
- Added
IncludePurviewparameter to enable Purview connectivity - Integrated Purview connection logic using
Connect-IPPSSessionwith primary domain detection - Updated configuration files and documentation to support the new feature
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| script/Run-MaesterAction.ps1 | Added Purview connection logic and parameter handling |
| action.yml | Added include_purview input parameter definition |
| README.md | Updated documentation with new Purview parameter |
| .github/workflows/test-action.yml | Added include_purview configuration to test workflow |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
svrooij
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apart from this PR, (we need a new connection to something), which is great!
This is also related to maester365/maester#1043
I would like to see that tests that cannot execute report as failed (or skipped), but that the entire test run will be able to run.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
|
@l-gosling what does your last message mean? This cannot be executed without a user logging in? If that is the case should we even proceed in accepting this pr? |
|
@svrooij i am waiting for a response from fabian in maester365/maester#1173. The "issue" is that the permissions in EXO don't cover the needed permission. The security reader role had do be assigned (or another permission i am currently not found but sadly no api permissions). We can try it with URBAC, but this is also no API Permission. In the mentioned PR in the maester repo i suggest to have a validation of the needed permission for each test. If we have this. We can discuss if we want to auto connect to ipp when connecting to EXO because the permission checks avoid this errors. |
|
I still want to make this work. One thing that I noticed is that your new option is called |
added purview (ipp) connection using.
After connection to ipp the following tests will fail. The reason is on the right site.
<style> </style>