Feature/dsc v3 native resources#1
Conversation
|
Warning Review limit reached
More reviews will be available in 49 minutes and 45 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughA new ChangesAzure Arc AgentConfiguration DSC Resource
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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
🧹 Nitpick comments (2)
dsc_resources/azure_arc_agent.dsc.resource.json (1)
117-122: ⚡ Quick winAdd enum constraint for configMode allowed values.
The schema permits any string or null for
configMode, but the documented contract restricts it to"monitor"or"full". Adding anenumconstraint provides fail-fast validation at the schema layer before the PowerShell normalization logic runs.🛡️ Proposed schema enhancement
"configMode": { "type": [ "string", "null" - ] + ], + "enum": [ + "monitor", + "full", + null + ] },🤖 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 `@dsc_resources/azure_arc_agent.dsc.resource.json` around lines 117 - 122, The configMode property in the schema currently accepts any string value or null, but it should be restricted to only the documented valid values "monitor" or "full". Add an enum constraint to the configMode field definition that explicitly lists the allowed string values "monitor" and "full", keeping null as an allowed type to maintain backward compatibility. This will enable schema-level validation before any PowerShell normalization logic executes.dsc_resources/azure_arc_agent.resource.ps1 (1)
10-12: 💤 Low valueConsider removing ValueFromPipeline or adding a process block.
The parameter uses
ValueFromPipeline = $truebut the script has noprocessblock. While this works for single JSON input from stdin in the DSC context, it's not best practice. Since the DSC manifest pipes$inputto the script and expects a single JSON string, theValueFromPipelineattribute may be unnecessary.Alternative approach
Remove the
ValueFromPipelineattribute since the script is invoked via stdin redirection rather than traditional pipeline binding:- [Parameter(Position = 1, ValueFromPipeline = $true)] + [Parameter(Position = 1)] [AllowEmptyString()] [string]$JsonInput🤖 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 `@dsc_resources/azure_arc_agent.resource.ps1` around lines 10 - 12, The $JsonInput parameter in the azure_arc_agent.resource.ps1 script has ValueFromPipeline set to true but the script does not contain a process block to handle pipeline input, which is not best practice. Since the DSC manifest pipes input via stdin redirection rather than traditional pipeline binding, remove the ValueFromPipeline = $true attribute from the $JsonInput parameter definition while keeping the Parameter, AllowEmptyString, and string type attributes intact.
🤖 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 `@docs/azure_arc_agent-operations-examples.md`:
- Line 1: In the markdown file, the main heading starts with "P#" which is a
typo that should be corrected to just "#". Locate the heading with the text
"Microsoft.Azure.Arc/AgentConfiguration operation examples" and remove the stray
"P" character from the beginning of that line so it starts with a single "#"
character as is standard for markdown h1 headings.
In `@dsc_resources/azure_arc_agent.resource.ps1`:
- Around line 28-35: Add comment-based help documentation to the
Find-AzcmAgentCommand function and all other functions mentioned in the review
(also applies to functions at lines 37-48, 50-101, 103-124, 126-142, 144-159,
161-182, 184-223, 225-237, 239-265, 267-284, 286-311, 313-325, and 327-362). For
each function, insert a comment block immediately before the function keyword
that includes .SYNOPSIS with a brief summary, .DESCRIPTION with at least 40
characters explaining what the function does, .INPUTS describing what data the
function accepts (if applicable), and .OUTPUTS describing what the function
returns. Ensure all parameter descriptions are included for functions that have
parameters.
- Line 100: Remove the unary comma operator from the return statement in the
function that returns $normalizedItems. Since $normalizedItems is already
constructed as an array using the @() syntax earlier in the function, simply
change the return statement from `return ,$normalizedItems` to `return
$normalizedItems` to comply with PowerShell guidelines.
In `@dsc_resources/tests/azure_arc_agent.tests.ps1`:
- Line 124: The It block descriptions in the test file do not follow the coding
guidelines which require descriptions to start with "Should" rather than action
verbs. Update all It block descriptions at lines 124, 136, and 142 to start with
"Should" instead of their current action verbs (such as "Get", etc.), and ensure
they do not contain the word "when". For example, change "Get returns the
current azcmagent configuration" to "Should return the current azcmagent
configuration".
- Around line 4-167: Reorganize the three test scenarios within the Describe
block into separate Context blocks. Create three new Context blocks with
descriptions starting with 'When': wrap the It block 'Get returns the current
azcmagent configuration' in a Context 'When performing Get operation', wrap the
It block 'Test returns false when the current state differs from the desired
defaults' in a Context 'When performing Test operation', and wrap the It block
'Set applies the required Azure Arc commands in order' in a Context 'When
performing Set operation'. Keep all BeforeAll, BeforeEach, and AfterAll blocks
at the Describe level unchanged, and move only the three It blocks into their
respective Context blocks.
---
Nitpick comments:
In `@dsc_resources/azure_arc_agent.dsc.resource.json`:
- Around line 117-122: The configMode property in the schema currently accepts
any string value or null, but it should be restricted to only the documented
valid values "monitor" or "full". Add an enum constraint to the configMode field
definition that explicitly lists the allowed string values "monitor" and "full",
keeping null as an allowed type to maintain backward compatibility. This will
enable schema-level validation before any PowerShell normalization logic
executes.
In `@dsc_resources/azure_arc_agent.resource.ps1`:
- Around line 10-12: The $JsonInput parameter in the
azure_arc_agent.resource.ps1 script has ValueFromPipeline set to true but the
script does not contain a process block to handle pipeline input, which is not
best practice. Since the DSC manifest pipes input via stdin redirection rather
than traditional pipeline binding, remove the ValueFromPipeline = $true
attribute from the $JsonInput parameter definition while keeping the Parameter,
AllowEmptyString, and string type attributes intact.
🪄 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: 58d25a25-9d33-47be-9120-6b73a25d1bd6
📒 Files selected for processing (9)
README.mddocs/azure_arc_agent-operations-examples.mddocs/azure_arc_agent-parameters.mddsc_config_example/arc-agent-export.dsc.yamldsc_config_example/arc-agent.dsc.yamldsc_resources/.project.data.jsondsc_resources/azure_arc_agent.dsc.resource.jsondsc_resources/azure_arc_agent.resource.ps1dsc_resources/tests/azure_arc_agent.tests.ps1
|
Hi @mgreenegit can you review and mearge ? |
1 participant
Add DSCv3 Azure Arc Resources and documentation example
This change is