Added composite resource 'AADSyncRuleCounts'#11
Open
raandree wants to merge 1 commit into
Open
Conversation
WalkthroughThis PR adds a new composite DSC resource ChangesAADSyncRuleCounts Composite Resource
🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.memory-bank/activeContext.md:
- Around line 54-55: The file ends without a trailing newline; add exactly one
newline character at EOF so the file terminates with a single newline (MD047).
Open the file containing the lines "- No additional composite resources
planned." and "- No restructuring of the build pipeline." and ensure there is
one and only one trailing newline character after the final line.
- Around line 47-50: Replace the ordered-list numbering to use repeated "1."
prefixes to satisfy markdownlint MD029: change the second list item that starts
"Cut a release with the `Unreleased` entry..." so both items use "1." (i.e.,
make the two lines beginning "Wait for the `AADConnectDsc` PR..." and "Cut a
release with the `Unreleased` entry..." use "1." as their list marker).
In `@docs/AADSyncRuleCounts.md`:
- Line 140: The file AADSyncRuleCounts.md is missing a final newline (MD047);
open AADSyncRuleCounts.md and add a single trailing newline character at the end
of the file so the file ends with exactly one final newline, then save and
commit the change.
In `@examples/README.md`:
- Line 29: The long sentence describing the report-only `AADSyncRuleCounts`
composite resource exceeds the 80-character markdown line-length limit; edit the
README sentence that starts with "Demonstrates the report-only
`AADSyncRuleCounts` composite resource..." and rewrap it at word boundaries into
multiple lines under 80 characters (e.g., break after phrases like "composite
resource", "for bulk drift detection", and "per connector and across all
connectors") so the prose conforms to the markdown wrap guideline.
In `@source/DSCResources/AADSyncRuleCounts/AADSyncRuleCounts.schema.psm1`:
- Around line 12-27: The loop builds an execution name ($executionName) from
each item's ConnectorName which can collide for duplicate scopes; modify the
code that iterates $Items (the foreach) to compute the normalized scope value
(same logic as $scope) for every $item, track seen execution names (or
normalized scopes) and if a duplicate is detected, throw/terminate with a clear
error before calling Get-DscSplattedResource -ExecutionName; ensure the check
references the same normalization logic used for $executionName so duplicates
(including '' vs '*') are caught and reported early.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c9d2c478-432b-41ae-81de-120f99162c8f
📒 Files selected for processing (13)
.memory-bank/activeContext.md.memory-bank/progress.md.memory-bank/promptHistory.md.memory-bank/systemPatterns.mdCHANGELOG.mdREADME.mddocs/AADSyncRuleCounts.mdexamples/6-AADSyncRuleCounts.ps1examples/README.mdsource/DSCResources/AADSyncRuleCounts/AADSyncRuleCounts.psd1source/DSCResources/AADSyncRuleCounts/AADSyncRuleCounts.schema.psm1source/DscConfig.AADConnect.psd1tests/Unit/DSCResources/Assets/Config/AADSyncRuleCounts.yml
Comment on lines
+47
to
+50
| 1. Wait for the `AADConnectDsc` PR (`feature/AadsyncrulecountResource`) to be | ||
| merged and a new version published, so CI can resolve `AADConnectDsc` from | ||
| the gallery instead of relying on the local 0.6.0 copy. | ||
| 2. Cut a release with the `Unreleased` entry promoted to a numbered version. |
There was a problem hiding this comment.
Fix ordered-list prefix style to satisfy markdownlint.
The second item uses 2. while your configured MD029 style expects repeated 1. prefixes.
Suggested patch
1. Wait for the `AADConnectDsc` PR (`feature/AadsyncrulecountResource`) to be
merged and a new version published, so CI can resolve `AADConnectDsc` from
the gallery instead of relying on the local 0.6.0 copy.
-2. Cut a release with the `Unreleased` entry promoted to a numbered version.
+1. Cut a release with the `Unreleased` entry promoted to a numbered version.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| 1. Wait for the `AADConnectDsc` PR (`feature/AadsyncrulecountResource`) to be | |
| merged and a new version published, so CI can resolve `AADConnectDsc` from | |
| the gallery instead of relying on the local 0.6.0 copy. | |
| 2. Cut a release with the `Unreleased` entry promoted to a numbered version. | |
| 1. Wait for the `AADConnectDsc` PR (`feature/AadsyncrulecountResource`) to be | |
| merged and a new version published, so CI can resolve `AADConnectDsc` from | |
| the gallery instead of relying on the local 0.6.0 copy. | |
| 1. Cut a release with the `Unreleased` entry promoted to a numbered version. |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 50-50: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/1/1
(MD029, ol-prefix)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.memory-bank/activeContext.md around lines 47 - 50, Replace the ordered-list
numbering to use repeated "1." prefixes to satisfy markdownlint MD029: change
the second list item that starts "Cut a release with the `Unreleased` entry..."
so both items use "1." (i.e., make the two lines beginning "Wait for the
`AADConnectDsc` PR..." and "Cut a release with the `Unreleased` entry..." use
"1." as their list marker).
Comment on lines
+54
to
+55
| - No additional composite resources planned. | ||
| - No restructuring of the build pipeline. No newline at end of file |
There was a problem hiding this comment.
Add a single trailing newline at EOF.
MD047 indicates the file should end with exactly one newline character.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 55-55: Files should end with a single newline character
(MD047, single-trailing-newline)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.memory-bank/activeContext.md around lines 54 - 55, The file ends without a
trailing newline; add exactly one newline character at EOF so the file
terminates with a single newline (MD047). Open the file containing the lines "-
No additional composite resources planned." and "- No restructuring of the build
pipeline." and ensure there is one and only one trailing newline character after
the final line.
| - The companion `AADSyncRuleCount` resource ships with `AADConnectDsc` | ||
| starting with the version that introduces report-only count drift detection. | ||
| If your installed `AADConnectDsc` predates that version, compilation will | ||
| fail because the underlying resource is not present. No newline at end of file |
There was a problem hiding this comment.
Add a single trailing newline at end of file.
Markdown lint indicates missing final newline (MD047).
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 140-140: Files should end with a single newline character
(MD047, single-trailing-newline)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/AADSyncRuleCounts.md` at line 140, The file AADSyncRuleCounts.md is
missing a final newline (MD047); open AADSyncRuleCounts.md and add a single
trailing newline character at the end of the file so the file ends with exactly
one final newline, then save and commit the change.
|
|
||
| ### [6-AADSyncRuleCounts.ps1](6-AADSyncRuleCounts.ps1) | ||
|
|
||
| Demonstrates the report-only `AADSyncRuleCounts` composite resource for bulk drift detection of expected sync-rule counts per connector and across all connectors. |
There was a problem hiding this comment.
Wrap the long sentence to satisfy markdown line-length rules.
Line 29 exceeds the 80-character wrap limit for markdown prose.
Suggested fix
-Demonstrates the report-only `AADSyncRuleCounts` composite resource for bulk drift detection of expected sync-rule counts per connector and across all connectors.
+Demonstrates the report-only `AADSyncRuleCounts` composite resource for
+bulk drift detection of expected sync-rule counts per connector and across
+all connectors.As per coding guidelines, "Wrap lines at word boundaries when over 80 characters (except tables/code blocks)".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Demonstrates the report-only `AADSyncRuleCounts` composite resource for bulk drift detection of expected sync-rule counts per connector and across all connectors. | |
| Demonstrates the report-only `AADSyncRuleCounts` composite resource for | |
| bulk drift detection of expected sync-rule counts per connector and across | |
| all connectors. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/README.md` at line 29, The long sentence describing the report-only
`AADSyncRuleCounts` composite resource exceeds the 80-character markdown
line-length limit; edit the README sentence that starts with "Demonstrates the
report-only `AADSyncRuleCounts` composite resource..." and rewrap it at word
boundaries into multiple lines under 80 characters (e.g., break after phrases
like "composite resource", "for bulk drift detection", and "per connector and
across all connectors") so the prose conforms to the markdown wrap guideline.
Comment on lines
+12
to
+27
| foreach ($item in $Items) | ||
| { | ||
| # ConnectorName is the key. Empty string or '*' means "all connectors". | ||
| # Translate that to a safe, descriptive token for the execution name so | ||
| # individual array items remain uniquely identifiable in compiled MOF. | ||
| $scope = if ([string]::IsNullOrEmpty($item.ConnectorName) -or $item.ConnectorName -eq '*') | ||
| { | ||
| 'AllConnectors' | ||
| } | ||
| else | ||
| { | ||
| $item.ConnectorName | ||
| } | ||
|
|
||
| $executionName = ("AADSyncRuleCount__$scope") -replace '[\s(){}/\\:-]', '_' | ||
| (Get-DscSplattedResource -ResourceName AADSyncRuleCount -ExecutionName $executionName -Properties $item -NoInvoke).Invoke($item) |
There was a problem hiding this comment.
Execution-name collisions are possible for duplicate connector scopes.
If two items resolve to the same scope (e.g., duplicate ConnectorName, or '' and '*' together), they produce the same ExecutionName, which can fail DSC compilation due to duplicate resource instance names.
Suggested fix (fail fast on duplicates)
+ $seenExecutionNames = @{}
foreach ($item in $Items)
{
# ConnectorName is the key. Empty string or '*' means "all connectors".
# Translate that to a safe, descriptive token for the execution name so
# individual array items remain uniquely identifiable in compiled MOF.
$scope = if ([string]::IsNullOrEmpty($item.ConnectorName) -or $item.ConnectorName -eq '*')
{
'AllConnectors'
}
else
{
$item.ConnectorName
}
$executionName = ("AADSyncRuleCount__$scope") -replace '[\s(){}/\\:-]', '_'
+ if ($seenExecutionNames.ContainsKey($executionName))
+ {
+ throw "Duplicate AADSyncRuleCounts item for scope '$scope'. Ensure each scope appears only once."
+ }
+ $seenExecutionNames[$executionName] = $true
+
(Get-DscSplattedResource -ResourceName AADSyncRuleCount -ExecutionName $executionName -Properties $item -NoInvoke).Invoke($item)
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@source/DSCResources/AADSyncRuleCounts/AADSyncRuleCounts.schema.psm1` around
lines 12 - 27, The loop builds an execution name ($executionName) from each
item's ConnectorName which can collide for duplicate scopes; modify the code
that iterates $Items (the foreach) to compute the normalized scope value (same
logic as $scope) for every $item, track seen execution names (or normalized
scopes) and if a duplicate is detected, throw/terminate with a clear error
before calling Get-DscSplattedResource -ExecutionName; ensure the check
references the same normalization logic used for $executionName so duplicates
(including '' vs '*') are caught and reported early.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Pull Request (PR) description
Adds a new
AADSyncRuleCountscomposite resource that wraps the report-onlyAADSyncRuleCountDSC resource introduced inAADConnectDsc0.6.0. The composite takes an array of{ ConnectorName, RuleCount }hashtables and emits one underlyingAADSyncRuleCountinstance per item; empty /'*'connector values are mapped to the literal tokenAllConnectorsso execution names stay unique and valid.Also fixes a pre-existing build break: DscConfig.AADConnect.psd1 was missing
DscResourcesToExport, which madeGet-DscResource -Module DscConfig.AADConnectreturn zero composite resources in PowerShell 7 and silently broke DscResources.Tests.ps1.Added
AADSyncRuleCountswrapping the report-onlyAADSyncRuleCountresource fromAADConnectDsc(per-connector or all-connectors drift detection).Fixed
DscResourcesToExportentry to DscConfig.AADConnect.psd1 soGet-DscResource -Module DscConfig.AADConnectreturns the composite resources and the auto-discovery test runs.Task list
pwsh -NoProfile -Command '.\Build.ps1 -Tasks build,test'→ 8/8 tests passed, exit 0; requiresAADConnectDsc≥ 0.6.0).# … #block in the schema)This change is