-
Notifications
You must be signed in to change notification settings - Fork 254
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Cleaning up and streamlining config files #1137
Conversation
The previous config files are referenced in the new-and-improved readme. Please read through the readme to make sure that the documentation is still accurate. ATM, you can find this on the 877 branch: |
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.
@amart241 The pre-approval section of the checklist is not complete and the ones that are checked, I would argue, are not complete since the PR title doesn't yet reflect a human-readable title (still the default) and the branch has not yet been rebased against main. Recommend completing the pre-approval checklist and then re-requesting reviews at that time.
631a5a0
to
e1a3d16
Compare
1c392a8
to
11c6991
Compare
@amart241 Any updates on this PR? It's been in "New" state with no updates for a month. |
Just got back this week from being sick and lots of trips. Working on this afternoon to get this resolved. |
11c6991
to
d0971bd
Compare
d0971bd
to
d9b425f
Compare
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.
Re-reviewed and recommending a number of misspelling clean ups and clarifications. See below for details.
354e2c3
to
bbb2314
Compare
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.
Looks good after updates.
2487f96
to
c6a9092
Compare
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.
Approving as the original issue was just to clean up the original config files.
However, I'm noticing how confusing our current documentation is and how the sample config file examples spread the currently available syntax for setting per product exclusions and omissions into multiple different files.
Created #1254 to create one unified file someone can look at to understand all the possible config file values they have to fill in order to pass certain policy checks for Dender and/or AAD and how to do policy omissions.
Co-authored-by: Addam Schroll <108814318+schrolla@users.noreply.github.com>
Co-authored-by: Addam Schroll <108814318+schrolla@users.noreply.github.com>
Co-authored-by: Addam Schroll <108814318+schrolla@users.noreply.github.com>
Co-authored-by: Addam Schroll <108814318+schrolla@users.noreply.github.com>
Co-authored-by: Addam Schroll <108814318+schrolla@users.noreply.github.com>
Co-authored-by: Addam Schroll <108814318+schrolla@users.noreply.github.com>
Co-authored-by: Addam Schroll <108814318+schrolla@users.noreply.github.com>
c6a9092
to
4ad05d6
Compare
@nanda-katikaneni This PR is ready for merge. |
Cleaning up and streamlining config files
🗣 Description
I have formatted the sample config files to be more streamlined. I have also added the other EntraID exclusions that are now available for AAD
💭 Motivation and context
The sample configs have been updated and not streamlined in a while and needed a refresh.
I removed extra references to other baselines in baseline specific sample files and added a more detailed and standardized description. This resolves #1075
🧪 Testing
Sample config files no code was changed
✅ Pre-approval checklist
✅ Pre-merge checklist
PR passed smoke test check.
Feature branch has been rebased against changes from parent branch, as needed
Use
Rebase branch
button below or use this reference to rebase from the command line.Resolved all merge conflicts on branch
Notified merge coordinator that PR is ready for merge via comment mention
✅ Post-merge checklist