Migrate remaining Get-Pfb* cmdlets to Add-PfbCommonQueryParams (#33) - #51
Open
juemerson-at-purestorage wants to merge 27 commits into
Open
Migrate remaining Get-Pfb* cmdlets to Add-PfbCommonQueryParams (#33)#51juemerson-at-purestorage wants to merge 27 commits into
juemerson-at-purestorage wants to merge 27 commits into
Conversation
Refactor the end{}/process{} query-param assembly in 12 cmdlets
(Get-PfbAlert, Get-PfbAlertWatcher, and 10 Public/Array/Get-Pfb*
cmdlets) to call the shared Add-PfbCommonQueryParams helper instead of
hand-rolled filter/sort/limit/names/ids lines. Non-generic
member_ids/member_names/policy_ids/policy_names lines in
Get-PfbArraySshCaPolicy are left untouched. Pure internal refactor;
no param()/help/pipeline-binding changes.
Refactors the end{}/body query-param assembly in Get-PfbAdmin,
Get-PfbAdminCache, Get-PfbApiClient, Get-PfbApiToken,
Get-PfbManagementAccessPolicy, Get-PfbOidcIdp, Get-PfbPublicKey,
Get-PfbSaml2Idp, and Get-PfbSession to route filter/sort/limit/names/ids
through the shared Add-PfbCommonQueryParams helper.
Get-PfbAdminManagementAccessPolicy, Get-PfbAdminSshCaPolicy,
Get-PfbManagementAccessPolicyAdmin,
Get-PfbManagementAccessPolicyDirectoryRole, and
Get-PfbManagementAccessPolicyMember have no generic names/ids query keys;
these now route only filter/limit through the helper (called with neither
-Names nor -Ids) and leave their member_*/policy_* lines untouched.
No public param() blocks, help text, or pipeline-binding attributes changed.
…rams (#33) Refactors the end{}/body query-param assembly of 12 Get-Pfb* cmdlets in Public/Bucket/ and Public/Certificate/ to route generic filter/sort/limit and names/ids keys through the shared Add-PfbCommonQueryParams helper. Four of these mix a generic names/ids key with non-generic member_*/policy_* keys (Get-PfbBucketAccessPolicy, Get-PfbBucketAccessPolicyRule, Get-PfbBucketCorsPolicy, Get-PfbBucketCorsPolicyRule); only the generic names/ids lines were routed through the helper, and the member_*/policy_* lines were left untouched. Cmdlets with no generic key at all (Get-PfbBucketAuditFilter, Get-PfbBucketAuditFilterAction, Get-PfbCertificateCertificateGroup) call the helper without -Names/-Ids. No param()/help/pipeline-binding changes.
…3-migrate-query-params
…3-migrate-query-params
Route the 15 Public/Misc Get-Pfb* cmdlets through the shared Add-PfbCommonQueryParams helper for filter/sort/limit and generic names/ids query params, per issue #33 Task 6. Non-generic wire keys (hold_*/member_* on Get-PfbLegalHoldEntity, bucket_names on Get-PfbLifecycleRule, file_system_names on Get-PfbUsageGroup/User) are left untouched, placed after the helper call. No param(), comment-based help, or pipeline-binding changes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Refactors the end{}/body query-param construction of 12 Get-Pfb* cmdlets
in Public/Hardware and Public/Monitoring onto the shared
Add-PfbCommonQueryParams helper, replacing hand-rolled filter/sort/limit
and names/ids building with the single centralized call. Pure internal
refactor: no param()/help/pipeline-binding changes.
Get-PfbHardwareTemperature is intentionally excluded (per Task 1 AST
audit): its -Limit is applied client-side only, after a post-fetch
filter, and is never sent to the server today. Migrating it would start
sending limit to the wire, changing behavior. Left completely untouched.
Refs #33
…Add-PfbCommonQueryParams (#33) Task 5 of Issue #33: route the hand-rolled filter/sort/limit and generic names/ids query-param assembly in 14 Get-Pfb* cmdlets through the shared Add-PfbCommonQueryParams helper. Non-generic member_*/ policy_*/group_* keys are left untouched per the AST-audited wire-key inventory; param blocks, help text, and begin/process accumulation logic are unchanged.
…3-migrate-query-params
…3-migrate-query-params
…3-migrate-query-params
…ryParams (#33) Refactors the end{} query-param assembly in 16 Get-Pfb* cmdlets across Public/Network, Public/Node, and Public/ObjectStore onto the shared Add-PfbCommonQueryParams helper (Task 8 of the Issue #33 plan). Pure internal refactor: param()/help/pipeline-binding/accumulation logic untouched; non-generic member_*/policy_*/group_* query keys on the 3 non-generic cmdlets are left as-is after the helper call. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Refactors the end{}/body query-param assembly for QosPolicyMember,
S3ExportPolicy/Rule, SmbClientPolicy/Rule, SmbSharePolicy/Rule,
SshCaPolicy/Admin/Array/Member, StorageClassTieringPolicy/Member,
TlsPolicy/Member, and WormPolicy/Member onto the shared
Add-PfbCommonQueryParams helper. Generic names/ids route through
-Names/-Ids; non-generic member_*/policy_* keys are left untouched.
No param()/help/pipeline-binding changes.
Refs #33
…3-migrate-query-params
) Task 9 of the #33 query-param centralization: refactor the end{}/process{} query-param assembly in 18 Get-Pfb* cmdlets under Public/Policy/ to use the shared Add-PfbCommonQueryParams helper for Filter/Sort/Limit and, where applicable, generic Names/Ids. Non-generic member_*/policy_* wire keys are left untouched. No param()/help/pipeline-binding changes.
…3-migrate-query-params
…3-migrate-query-params
…gServerSettings, Get-PfbDns (#33) Per Task 1's server-side audit, these three endpoints (smtp-servers, syslog-servers/settings, dns) support continuation_token/filter/limit/ offset/sort since REST 2.0 but the cmdlets exposed none of it. Wires the new params through the shared Add-PfbCommonQueryParams helper. No -TotalOnly added (no server-side support confirmed). The other 7 cmdlets audited in this task (Get-PfbRapidDataLocking, Get-PfbAsyncLogDownload, Get-PfbSmtp, Get-PfbSnmpAgentMib, Get-PfbPresetWorkload, Get-PfbQuotaSettings, Get-PfbWorkloadTag) were confirmed to have no server-side filter/sort/limit/total_only support and are left unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…3-migrate-query-params
…rams Task 11 of Issue #33: routes generic names/ids and hand-rolled filter/sort/limit assembly through the shared Add-PfbCommonQueryParams helper across Get-PfbArrayConnection(+Key/Path/PerformanceReplication), Get-PfbBucketReplicaLink, Get-PfbFileSystemReplicaLink(+Policy), Get-PfbFleet(+Key/Member), Get-PfbRemoteArray, Get-PfbTarget(+PerformanceReplication), and Get-PfbQuotaGroup/Get-PfbQuotaUser. Non-generic wire keys (local_*/remote_*/member_*/policy_*/fleet_names/file_system_names) are left untouched; the QuotaUser names-vs-file_system_names mutual exclusivity (live-confirmed FlashBlade API rejection) is preserved. Pure internal end{}/body refactor only -- no param()/help/pipeline changes. Refs #33
…3-migrate-query-params
…3-migrate-query-params
Task 1's audit found these endpoints support server-side filter/sort/ limit despite their cmdlets' docstrings calling them "singleton" resources: GET /admins/settings, /password-policies, /arrays, /arrays/eula, and /arrays/factory-reset-token. Adds -Filter/-Sort/-Limit (no -TotalOnly, per audit: none of these endpoints support total_only) to Get-PfbAdminSetting, Get-PfbPasswordPolicy, Get-PfbArray, Get-PfbArrayEula, and Get-PfbArrayFactoryResetToken, wired through the existing Add-PfbCommonQueryParams helper. Purely additive - no other behavior changed. Confirmed (no changes) the other 5 cmdlets in this task's scope - Get-PfbApiVersion, Get-PfbArrayPerformance, Get-PfbArraySpace, Get-PfbConnection, Get-PfbCredential - correctly have no REST-backed filter/sort/limit support to add.
…3-migrate-query-params
…d through Add-PfbCommonQueryParams These two cmdlets had manual names/ids parameter handling that should route through the shared helper function Add-PfbCommonQueryParams.ps1, consistent with the migration completed in Issue #32. No new parameters are added, and neither endpoint supports Filter/Sort/Limit/TotalOnly server-side. - Get-PfbAsyncLogDownload: Replace hand-rolled names/ids logic in end block - Get-PfbPresetWorkload: Replace hand-rolled names/ids logic in process block
…ment HardwareTemperature exclusion (#33) Normalizes the -Limit param contract across the 8 net-new cmdlets: drops ValidateRange(1, 10000) from Get-PfbSmtpServer, Get-PfbSyslogServerSettings, and Get-PfbDns so all 8 match the template convention and the repo-wide majority (only 20/191 Get-Pfb* cmdlets with -Limit use ValidateRange at all). Also documents in-tree why Get-PfbHardwareTemperature was intentionally NOT migrated onto Add-PfbCommonQueryParams: its -Limit is a client-side post-filter applied after temperature-presence filtering, and the helper would unconditionally start sending limit= server-side, changing results. This rationale previously only lived in a gitignored ledger and a commit message. Both findings from the whole-branch code review.
juemerson-at-purestorage
added a commit
to juemerson-at-purestorage/fb-powershell
that referenced
this pull request
Jul 25, 2026
…e objects Two more instances of the same failure mode PR dmann000#53 opened for -- the drift report going quieter, which reads as better -- found while scoping the "make the drift report actionable" work. 1. Null-vs-empty conflation in Get-PfbParameterCoverageGaps. The per-cmdlet inventory lookup is a Group-Object -AsHashTable, which returns $null for an absent key, and @($null) is a ONE-element array whose element is $null. The surface loop therefore ran once with $row = $null, read $null.Surface -ne 'Typed' as true, and discarded the endpoint into notVerified. A cmdlet contributes zero inventory rows when every parameter it declares is -Array/-Attributes (both inventory-exempt) -- the exact shape of the -Attributes-only write cmdlets -- and nothing contradicting full mapping means vacuously fully mapped, not unmappable. 36 endpoints leave notVerified, including PATCH /arrays, PATCH /password-policies and PATCH /support with 18/19/12 previously invisible missing fields. 2. Nested single-key reference objects, `$body['account'] = @{ name = $Account }`, are now credited to their OUTER key. The capability map records top-level body properties only, so `account.name` is not a field it can track and `account` is the field such a parameter actually covers. Two parameters resolving to the same outer key (by name, by id) is correct, not a collision to suppress. Resolves 13 (cmdlet, parameter) pairs across 9 cmdlets, and via the notVerified gate 6 more endpoints. New resolver runs strictly after both direct-assignment passes, so it can only turn an unresolved parameter Typed, never rename an already-resolved wire name (measured: 0 wire-name changes, 0 Typed -> non-Typed regressions). Combined, notVerified 101 -> 59 with 0 newly-notVerified, reported missing fields 1370 -> 1538 with 0 fields removed, and the residual 7-endpoint notVerified spread against PR dmann000#51's tip closes to 0. Measured, not asserted; report regenerated to scratch paths only -- committed Data/ and Reports/ artifacts unchanged. Live-array testing not applicable: tools/ and Tests/ only, no runtime surface, no Public//Private/ change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
juemerson-at-purestorage
added a commit
that referenced
this pull request
Jul 25, 2026
…and parser gaps (#53) * fix(tools): teach the parameter inventory two wire-name idioms it could not see tools/lib/PfbCmdletParamTools.ps1 resolved a parameter's REST wire name only from a literal `$body['k'] = $Param` / `$queryParams['k'] = $Param` index assignment in the cmdlet's own body. Two very common idioms were therefore invisible, and every parameter they cover was classified AttributesOnly/TypedUnresolved. Downstream, Get-PfbParameterCoverageGaps requires EVERY parameter on EVERY cmdlet calling an endpoint to be Surface='Typed', so one unresolved parameter pushed the whole endpoint into the notVerified bucket and out of gap analysis entirely. 1. The shared Private/Add-PfbCommonQueryParams.ps1 helper (issue #32/PR #49, extended to ~196 cmdlets by issue #33/PR #51) assigns -Filter/-Sort/-Limit/ -TotalOnly/-Names/-Ids on the caller's behalf, so migrated cmdlets contain no literal key at all. This made a pure mechanical refactor look like a 39% improvement: on PR #51's tree the headline missing-field count FELL 932 -> 571 while notVerified endpoints ROSE 194 -> 349. Added Get-PfbCommonQueryParamMap (a documented hardcoded mirror of the helper) plus Get-PfbCommonQueryParamHelperWireName. -Filter/-Sort/-Limit/-TotalOnly are matched by PARAMETER NAME (the helper reads them from the caller's $PSBoundParameters, and the call must actually forward it); -Names/-Ids by the variable passed to that argument, which the existing accumulator retry already traces from the parameter. Runs LAST so the non-generic keys those cmdlets deliberately kept as explicit lines (policy_names, file_system_names, ...) still win. 2. The hashtable-literal initializer `$queryParams = @{ 'names' = $Name }` -- the dominant shape across New-Pfb*/Remove-Pfb*/Update-Pfb* -- was only ever matched in its IndexExpressionAst form. Added Get-PfbHashtableLiteralWireNameForParameter, top-level pairs only: in `$body = @{ group = @{ name = $GroupName } }` the wire field is `group`, so crediting the inner `name` would mis-name the field and collide with every other sub-object's `name`. Also extracted the RHS shape test (Test-PfbWireValueIsParameter) and the parser's StatementAst unwrapping (Resolve-PfbSingleExpression) so both paths share one deliberately shape-exact matcher rather than duplicating it -- a pipeline transform is still refused rather than guessed at. Measured, generating the report from each tools/lib against each cmdlet tree (reporting only; no Public/, Private/, Data/ or Reports/ change): | gaps | notVerified | missing fields | uncovered baseline main | 254 | 194 | 932 | 117 baseline PR #51 | 132 | 349 | 571 | 117 fix 1 only main | 282 | 164 | 1019 | 117 fix 1 only PR #51 | 286 | 157 | 1029 | 117 both fixes main | 343 | 101 | 1370 | 117 both fixes PR #51 | 347 | 94 | 1380 | 117 The 155-endpoint main-vs-PR#51 spread closes to 7, zero endpoints newly become notVerified in any transition, and no already-resolved wire name changes. The residual 7 are cmdlets PR #51 gave query parameters to that had none on main (Get-PfbDns, Get-PfbAdminSetting, ...), which sidesteps a separate pre-existing bug in tools/lib/PfbApiDriftTools.ps1: `@($inventoryByCmdlet[$cmdletName])` yields @($null) for a cmdlet with zero inventory rows, wrongly marking its endpoint notVerified. That accounts for 37 of the remaining 101 and is left for its own change. Tests: 24 new cases in Tests/PfbCmdletParamTools.Tests.ps1 over synthetic Public/-shaped fixtures, including a mapping-table-vs-helper sync test that derives the contract from Private/Add-PfbCommonQueryParams.ps1's own AST so the hardcoded mirror cannot silently drift (mutation-verified: it fails when the helper gains a key). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(tools): stop discarding parameterless cmdlets and nested reference objects Two more instances of the same failure mode PR #53 opened for -- the drift report going quieter, which reads as better -- found while scoping the "make the drift report actionable" work. 1. Null-vs-empty conflation in Get-PfbParameterCoverageGaps. The per-cmdlet inventory lookup is a Group-Object -AsHashTable, which returns $null for an absent key, and @($null) is a ONE-element array whose element is $null. The surface loop therefore ran once with $row = $null, read $null.Surface -ne 'Typed' as true, and discarded the endpoint into notVerified. A cmdlet contributes zero inventory rows when every parameter it declares is -Array/-Attributes (both inventory-exempt) -- the exact shape of the -Attributes-only write cmdlets -- and nothing contradicting full mapping means vacuously fully mapped, not unmappable. 36 endpoints leave notVerified, including PATCH /arrays, PATCH /password-policies and PATCH /support with 18/19/12 previously invisible missing fields. 2. Nested single-key reference objects, `$body['account'] = @{ name = $Account }`, are now credited to their OUTER key. The capability map records top-level body properties only, so `account.name` is not a field it can track and `account` is the field such a parameter actually covers. Two parameters resolving to the same outer key (by name, by id) is correct, not a collision to suppress. Resolves 13 (cmdlet, parameter) pairs across 9 cmdlets, and via the notVerified gate 6 more endpoints. New resolver runs strictly after both direct-assignment passes, so it can only turn an unresolved parameter Typed, never rename an already-resolved wire name (measured: 0 wire-name changes, 0 Typed -> non-Typed regressions). Combined, notVerified 101 -> 59 with 0 newly-notVerified, reported missing fields 1370 -> 1538 with 0 fields removed, and the residual 7-endpoint notVerified spread against PR #51's tip closes to 0. Measured, not asserted; report regenerated to scratch paths only -- committed Data/ and Reports/ artifacts unchanged. Live-array testing not applicable: tools/ and Tests/ only, no runtime surface, no Public//Private/ change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Summary
Follow-up to #32 (PR #49): migrates the remaining 176 of 206
Get-Pfb*cmdlets onto the shared private helper
Add-PfbCommonQueryParams, givingthe whole module a uniform query-param surface (
-Filter/-Sort/-Limit/-TotalOnlyplus generic-Name/-Idaccumulation).filter/sort/limit/total_onlyand generic
names/idsquery-building replaced with the single helpercall. Non-generic wire keys (
member_names,policy_ids,file_system_names, etc.) are left untouched — several cmdlets mix ageneric key with non-generic ones in the same body; those were the
highest-risk edits and got extra scrutiny in review.
-Filter/-Sort/-Limitparams (never-TotalOnly) where an independent audit againstData/PfbCapabilityMap.jsonconfirmed server-side support that the cmdlet didn't previously expose —
even where the cmdlet's own docstring called it a "singleton."
correctly left param-less.
Get-PfbHardwareTemperatureapplies-Limitclient-side (post-fetch,after a temperature-presence filter) rather than sending it to the server.
Routing it through the shared helper would silently start sending
limitserver-side for the first time, changing results. This is documented
in-tree with a comment at the call site.
This is a pure refactor for the 156 + the exclusion: no
param(), parametersets, comment-based help, or pipeline-binding attributes changed on those
cmdlets — only the internal query-param-building body.
Net: 166 files changed, 308 insertions(+), 628 deletions(-).
Process
Executed as 14 independently-implemented-and-reviewed tasks (audit/scoping,
11 refactor batches, 2 net-new-param batches), plus one follow-up fix for two
cmdlets missed in the original batching, plus a final whole-branch review
(Opus 5) that found two Important issues — both fixed and re-reviewed clean:
-Limitvalidation was inconsistent across the 8 net-new cmdlets(3 had
[ValidateRange(1, 10000)], 5 didn't) — normalized to novalidation, matching the template and repo-wide majority.
Get-PfbHardwareTemperatureexclusion rationale only lived in agitignored ledger — now documented in-tree.
Test plan
pwsh 7.6.4, and WSL2 Ubuntu pwsh 7.6.4 (406/0/6 on pwsh 7, matching
pre-work baseline exactly)
documented exception + 10 correctly-untouched param-less cmdlets,
zero unexplained stragglers
(REST 2.26) — one representative cmdlet per batch, prioritizing the
highest-risk mixed/non-generic/net-new cases, all confirmed correct
via live request-URL inspection
.github/workflows/cross-platform-tests.yml)will run automatically now that this is pushed
🤖 Generated with Claude Code