Write cmdlets: add typed body/query parameters for 56 New-*/Update-* cmdlets - #66
Open
juemerson-at-purestorage wants to merge 101 commits into
Conversation
…ire key Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Picks up the array-of-references projection resolver from the parent commit: attributesOnly 68 -> 65, typedUnresolved unchanged at 37, with exactly the three predicted entries moving to Typed -- New-PfbServer/DnsName, Update-PfbServer/DnsName and New-PfbNetworkInterface/AttachedServers. PATCH /servers and POST /network-interfaces promote from partial to high confidence and drop their phantom dns / attached_servers body gaps. POST /servers keeps its dns gap removed but stays partial: CreateDirectoryService is assigned from a local ($createDs), a separate unresolved shape that is deliberately out of scope here. Rebased onto the repo-relative-path fix and regenerated from scratch, so this diff no longer carries the ~174 lines of absolute-path churn the previous version of this commit did. The artifact diff is now only the semantic change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Canonical reference implementation for issue dmann000#31. Adds -AuthorizationModel, -Locked, -ManagementAccessPolicies, -OldPassword, -Password and -PublicKey in a new *Individual parameter set, with -Attributes moved into a parallel *Attributes set so the binder rejects a mixed invocation. The deprecated 'role' body field is deliberately not exposed (constraint 9). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds -Enabled in a new *Individual parameter set, with -Attributes moved into a parallel *Attributes set so the binder rejects a mixed invocation. PATCH /api-clients reuses the full ApiClient resource schema, in which every property except 'enabled' is readOnly, so 'enabled' is the only settable field. The deprecated 'max_role' field is deliberately not exposed (constraint 9). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
dmann000#31) Adds -AggregationStrategy, -Enabled, -Location, -PolicyName and -Rules in a new *Individual parameter set, with -Attributes moved into a parallel *Attributes set so the binder rejects a mixed invocation. Per constraint 8's item-schema test, 'location' is a scalar reference (_fixedReference) and gets a [string] name parameter, but 'rules' is an array of COMPOSITE objects -- ManagementAccessPolicyRuleInPolicy carries role/scope/index beyond {id, name, resource_type} and its own 'name' is readOnly -- so it is a [hashtable[]] passed straight through. Projecting it into @{ name = ... } would write a field the schema does not accept. No ValidateSet on -AggregationStrategy: the spec documents its two values in prose only, not as a schema enum (constraint 3). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds -Enabled, -Idp, -OidcIdpName and -Services in a new *Individual parameter
set, with -Attributes moved into a parallel *Attributes set so the binder rejects
a mixed invocation.
Per constraint 8's item-schema test, 'idp' is a COMPOSITE sub-object
(_oidcSsoPostIdp: provider_url plus CA-certificate references) with no 'name'
property, so it is a [hashtable] passed straight through rather than projected
into @{ name = ... }.
The body field 'name' is exposed as -OidcIdpName because -Name already selects
which provider to update. No ValidateSet on -Services (constraint 3).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds -ArrayUrl, -Binding, -Enabled, -Idp, -Management, -Saml2IdpName, -Services
and -Sp in a new *Individual parameter set, with -Attributes moved into a
parallel *Attributes set so the binder rejects a mixed invocation.
Per constraint 8's item-schema test, 'idp', 'management' and 'sp' are all
COMPOSITE sub-objects (_saml2SsoIdp, _saml2SsoManagement, _saml2SsoSp) with no
'name' property, so each is a [hashtable] passed straight through rather than
projected into @{ name = ... }.
The body field 'name' is exposed as -Saml2IdpName because -Name already selects
which provider to update. No ValidateSet on -Binding or -Services: the spec
documents their values in prose only, not as schema enums (constraint 3).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
POST /admins/api-tokens has no request body at all -- all three documented gaps (admin_ids, admin_names, timeout) are QUERY parameters, so this cmdlet gets no Individual/Attributes parameter-set split. BEHAVIOUR CHANGE: -Name and -Id previously sent the 'names' and 'ids' query keys, which this endpoint does not accept. The array silently ignored them, so -Name and -Id had no effect whatsoever. They now send 'admin_names' and 'admin_ids'. Callers that relied on the old no-op behaviour (i.e. that got a token for the authenticated user despite passing -Name) will now correctly target the named admin. Aliases -AdminNames/-AdminIds are added rather than new parameters, since two parameters writing one query key would itself be a bug. -Timeout is genuinely new and typed [long]: the spec declares it integer/int64, and [int] would overflow at ~24.8 days of milliseconds. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ew fixes C1, I1)
C1 (critical) -- guard every body parameter with ContainsKey, not truthiness.
The canonical template used 'if (\)' / 'if (\)',
which silently discards legitimate falsy values the caller explicitly supplied:
\False, 0, '' and @() all fail a truthiness test. Verified: -ManagementAccessPolicies @()
previously produced a body with no such key, so a list could never be cleared.
Constraint 2 only ever spoke about booleans, and this batch happens to contain no
integer parameter -- so the defect was invisible here but would have propagated to
49 copies. Downstream the field reference has 30 integer fields where 0 is
meaningful, including Update-PfbQosPolicy's max_total_ops_per_sec (0 = unlimited)
and New-PfbNfsExportRule's anonuid/anongid (0 = root).
Every body assignment across all six cmdlets now uses
\System.Management.Automation.PSBoundParametersDictionary.ContainsKey('X'). This does not affect constraint 7: the
resolver still resolves 32 of 33 parameters, the sole exception being the
already-known ManagementAccessPolicies array projection.
Adds falsy-value tests for an array and a string parameter on four cmdlets.
I1 (important) -- rename the 'rename this resource' body parameter to -NewName:
Update-PfbManagementAccessPolicy -PolicyName -> -NewName
Update-PfbOidcIdp -OidcIdpName -> -NewName
Update-PfbSaml2Idp -Saml2IdpName -> -NewName
Follows existing repo precedent (Update-PfbWorkload, Update-PfbDataEvictionPolicy,
Update-PfbPresetWorkload all expose -NewName for this same body field), so the 14
further endpoints with a 'name' body row in later batches have one spelling to copy.
The unrelated -PolicyName parameters on the association cmdlets (which map to the
policy_names QUERY key) are deliberately untouched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… review M-1, M-3)
M-1: the canonical guard comment in Update-PfbAdmin.ps1 said the ContainsKey rule
applies to 'body parameters', while the query-selector lines a few lines above use
truthiness -- an apparent contradiction for the nine agents about to copy this file.
Reworded to cover every VALUE-CARRYING parameter, body or query, and to name the real
exemption: a mandatory [string] selector is safe on truthiness only because the binder
rejects '' before any guard runs. Also notes the exemption is about mandatory-ness, not
about being a query parameter -- an optional query parameter still needs ContainsKey,
as -Timeout in New-PfbApiToken.ps1 demonstrates.
M-3: -Timeout is [long] and the only integer parameter in this batch, so the canonical
must demonstrate the explicit-zero case Constraint 2 now demands of every integer body
field in the nine remaining batches. Asserts 0 actually reaches queryParams['timeout'].
Mutation-verified: reverting the guard to 'if (\)' fails exactly this one test,
with the diagnostic showing QueryParams @{admin_names='ops-admin'} and no timeout key.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…mann000#31) Three Minor findings from the Task 1 independent audit, fixed in the files the remaining 49 cmdlets are told to copy. M3 Update-PfbApiClient help claimed "every property except enabled is read-only". max_role is readOnly:false -- it is deprecated (constraint 9), not read-only. Seven of nine properties are read-only. The two rules have different justifications and the conflated wording invites agents to generalise wrongly. M4 Tests/Update-PfbAdmin.Tests.ps1 was the weakest of the six test files while being the named reference. Adds a constraint-compliance Context asserting no ValidateSet on any of the six typed body parameters (PATCH /admins declares no enum on any body property) plus a parameter-presence assertion. Mutation-proved: adding [ValidateSet('a','b')] to -AuthorizationModel makes it fail. M6 New-PfbApiToken .PARAMETER Attributes said "additional token attributes" while the DESCRIPTION eleven lines above says the endpoint takes no request body. Five query-only New-Pfb* cmdlets copy this file's shape. Suite 714/8/2, up 7 passed from 707/8/2. Failure count unchanged; all eight are pre-existing stale acceptance pins.
…ann000#31) Adds all 16 settable CertificatePatch body fields plus the generate_new_key query parameter as typed, ContainsKey-guarded parameters in new ByNameIndividual/ ByIdIndividual parameter sets, mutually exclusive with -Attributes. The six read-only fields (issued_by, issued_to, realms, status, valid_from, valid_to) are excluded per the field reference. generate_new_key stays a bare parameter since it is a query field orthogonal to the body. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…icateGroup (dmann000#31) Adds -CertificateId/-CertificateGroupId (certificate_ids/certificate_group_ids query parameters) alongside the existing -CertificateName/-CertificateGroupName. This endpoint has no request body, so no body work was needed; the existing name-based wire keys were confirmed already correct and left untouched. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…mann000#31) Adds -Actions/-S3Prefixes (the only two BucketAuditFilterPost body fields) in new ByMemberNameIndividual/ByMemberIdIndividual sets, mutually exclusive with -Attributes. Also fixes a real wire-correctness bug found via the OpenAPI spec: the endpoint's query parameters are bucket_ids/bucket_names, not the member_ids/member_names this cmdlet previously sent (which do not exist on this endpoint at all), and the endpoint requires a 'names' query parameter on every PATCH that this cmdlet never sent -- new -FilterNames, defaulted from -MemberName for backward compatibility. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…00#31) ConvertTo-PfbQueryString sent [bool] values through .ToString(), producing True/False on the wire instead of the true/false the REST API expects. Found independently by Task 2 and Task 5 reviewers; propagated to every unmerged batch so each branch is self-consistent.
…dmann000#31) Inserting -CertificateId before -CertificateGroupName shifted the latter from position 1 to position 2, so a caller invoking the cmdlet positionally with two arguments (the pre-dmann000#31 calling convention) would silently bind its second argument to the new -CertificateId string parameter instead of -CertificateGroupName. Reordered so both pre-existing parameters keep their original absolute positions; the new -CertificateId/-CertificateGroupId parameters are appended after them, addressable only by name.
dmann000#31) Three cmdlets in this batch inserted new Id/selector parameters before pre-existing positional ones, shifting them out of their historical absolute position: - New-PfbNetworkInterfaceTlsPolicy: -PolicyName shifted by -MemberId - Update-PfbDns: -Domain/-Nameservers shifted by the new -Name/-Id query selectors - New-PfbNodeGroupNode: -MemberName shifted by -GroupId A caller using the pre-dmann000#31 positional calling convention would silently bind its argument to the wrong (new) parameter instead of the intended pre-existing one. Reordered so every pre-existing parameter keeps its original absolute position; new parameters are appended after them, addressable only by name.
…dmann000#31) Inserting -LocalFileSystemId before -MemberId shifted -MemberId from position 3 to position 4, so a caller invoking the cmdlet with the pre-dmann000#31 4-positional-argument convention would silently bind its 4th argument to the new -LocalFileSystemId string parameter instead of -MemberId. Reordered so -MemberId keeps its original absolute position; -LocalFileSystemId is appended after it, addressable only by name.
…ing @() coverage (dmann000#31 review) Update-PfbDirectoryServiceRole's -RoleIds/-RoleNames were declared bare per Constraint 17, but every parameter set on this cmdlet makes -Name or -Id mandatory, and the spec rejects role_ids/role_names combined with ids/names -- these two parameters were structurally unreachable in any valid invocation. Dropped per the controller's ruling (matches Constraint 9's deprecated-field precedent); replaced the now-invalid query-parameter tests with absence tests. Also added Constraint 2 @() coverage on Update-PfbActiveDirectory for the five array body fields that had none (encryption_types, fqdns, global_catalog_servers, kerberos_servers, service_principal_names) -- mutation-proved against a truthiness guard before landing.
…000#31 review) Update-PfbQosPolicy had a 0-value test for -MaxTotalOpsPerSec but not -MaxTotalBytesPerSec; added the matching test. Update-PfbStorageClassTieringPolicy's -ArchivalRules/-RetrievalRules are [hashtable[]] arrays, where Constraint 18's composite exemption does not apply (@() is falsy for an array, unlike @{} for a scalar hashtable) -- added @() tests for both. Code already used ContainsKey correctly; this was a test-coverage gap only. Mutation-proved both new tests against a truthiness guard before landing.
…iew) New-PfbFleetMember's -Members is [hashtable[]], where Constraint 18's composite exemption does not apply. Code already used ContainsKey correctly; this was a test-coverage gap only. Mutation-proved against a truthiness guard before landing.
…mann000#31 review) Reviewer found that the original fix's diagnosis was wrong about -MemberId: POST /policies/file-system-replica-links (and its GET/ DELETE siblings) declare member_ids as a real, spec-backed query parameter -- only member_names is genuinely absent. Verified independently against tools/specs/fb2.28.json. The prior fix removed both -MemberName and -MemberId as if both were equally invalid, silently dropping working functionality. Restored -MemberId (-> member_ids), keeping -MemberName removed (member_names is confirmed absent). Also fixed the positional-binding regression this reintroduced: -MemberId's restored declaration initially landed after the new -LocalFileSystemId, one position later than its original pre-dmann000#31 slot -- a caller using the old 4-positional- argument convention would have had their previously-working -MemberId value silently redirected to -LocalFileSystemId instead. Reordered so -MemberId keeps its original absolute position.
…mann000#31 review) Reviewer found that -MemberId alone (without -MemberName or an explicit -FilterNames) silently omitted the required 'names' query parameter entirely -- the elseif default only covers the -MemberName path. The array would have rejected the resulting request with a confusing 400 and no local indication of why. Added a local throw telling the caller to supply -FilterNames explicitly in that case, and a test proving it (the existing -MemberId test was vacuous about 'names' and has been fixed to supply -FilterNames so it still tests what it originally claimed to).
dmann000#31 whole-branch review I-1) Adding a ParameterSetName disables PowerShell's implicit positional binding for the ENTIRE function, not just the parameter that gained one. This cmdlet had no parameter sets before dmann000#31, so all of -ManagementAddress/-ReplicationAddress/-ConnectionKey/-Attributes were freely positional; after the Individual/Attributes split, none of them were, breaking any pre-dmann000#31 positional call. Restored via explicit Position on each set's own positional parameters.
…whole-branch review I-1) Same root cause as the New-PfbArrayConnection fix: this cmdlet had no parameter sets before dmann000#31, so -Domain/-Nameservers/-Attributes were freely positional; splitting into Individual/Attributes sets silently disabled ALL positional binding for the function. Restored via explicit Position on each set's own positional parameters.
…hole-branch review I-2) hold_names/member_names are not documented query parameters on POST /legal-holds/held-entities in any cached spec version (2.4-2.28) -- confirmed directly against the spec. Verified against the endpoint's description text that -Names/-FileSystemNames (both added by this task, real and spec-backed) already cover the same job these two were attempting, so removing them leaves no functional gap.
dmann000#31 whole-branch review I-3) The sibling cmdlet New-PfbFileSystemReplicaLinkPolicy kept -MemberName as a backward-compatible alias of its renamed equivalent parameter; this cmdlet had simply dropped -MemberName instead, giving two nearly identical wire-key fixes two different compatibility stories. Made consistent by adding the same alias here.
…nn000#31) Regenerates the API drift report and asserts none of the 56 in-scope write endpoints has dropped to partial confidence -- i.e. no future edit reintroduces a parser-untraceable body assignment (Constraint 7) without the suite catching it. Measured baseline: 44 in-scope write endpoints currently have an open parameterGaps entry, all 44 at high confidence, 0 partial (the other 12 in-scope cmdlets have zero remaining gaps at all, so they don't appear in the gaps list). Mutation-proved: casting Update-PfbAdmin's $body['locked'] = $Locked to [bool]$Locked breaks the AST trace and fails the guard naming 'PATCH /admins [Locked]' exactly as expected; reverted and confirmed green again.
… Task 12 step 3) In-scope (56 cmdlets) addable gaps: body 206 -> 3, query 83 -> 27. The 3 remaining body gaps are the deprecated fields Constraint 9 excludes on purpose (role/max_role/role on Update-PfbAdmin, Update-PfbApiClient, Update-PfbDirectoryServiceRole). Of the 27 remaining query gaps, 26 are context_names -- a systemic, cross-cutting gap tracked separately (Fusion context design, not in this issue's scope) -- and 1 is role_ids/role_names on Update-PfbDirectoryServiceRole, dropped under an explicit controller ruling (structurally unreachable given the cmdlet's other selectors). Overall: endpoints with parameter gaps 432 -> 420, addable body 606 -> 403, addable query 1004 -> 948, systemic gaps 312 -> 252.
…0#31, Task 12 step 2) Build-PfbApiDriftReport.ps1 requires PS7+ (#requires -Version 7.0), matching every other drift-tooling Describe block in this repo, which already skips itself the same way. The guard test lacked this skip, so it failed outright on Windows PowerShell 5.1 with a ScriptRequiresException rather than skipping gracefully. Found while running Task 12's PS 5.1 full-suite sweep.
…dmann000#31, Task 12 step 3) Closing real gaps on 56 cmdlets shifted every population-wide figure these tests pin. Two kinds of change: Pure re-pins (population drift, unrelated to this issue's correctness): context_names 253->254, allow_errors 109->110, names 306->308, ids 218->219, phantomFieldCount 34->40, high-confidence phantom-excluded 13->21 (duplicated in both Build-PfbApiDriftReport.Tests.ps1 and PfbApiDriftTools.Tests.ps1). Premise-flipped canaries, repurposed rather than blindly re-pinned: 10 of the 11 'Task 8 canary' (endpoint, field) pairs, and the generate_new_key spot-check, were regression canaries proving the drift tool's readOnly-detection didn't wrongly suppress a real gap -- issue dmann000#31 correctly closed all of them (Update-PfbTlsPolicy -NewName, Update-PfbHardwareConnector -PortSpeed, Update-PfbCertificate -GenerateNewKey, etc). Verified each directly against the regenerated report before flipping. Split into: one test asserting the still-open, correctly-deprecated max_role gap remains (Constraint 9), and one asserting the 10 fixed fields stay fixed (repurposed as a forward regression guard rather than left permanently red or deleted). All drift-tooling tests green: 166/166.
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
Closes #31. Gives 56
New-Pfb*/Update-Pfb*write cmdlets typed, discoverable, validatedparameters for the documented body/query fields they previously only exposed through a raw
-Attributeshashtable.-Attributesis retained on every cmdlet for backward compatibility,made mutually exclusive from the new typed parameters via PowerShell parameter sets.
This branch also carries
tools/resolve-array-projection-references(2 commits) as its base —that work was never separately merged to
main, so it rides along in this diff. It teaches theAST wire-name resolver the array-of-references projection shape so the drift report stops
reporting three already-implemented fields as gaps. Zero
Public/changes of its own.What's in this PR
FileSystem/Realm, Hardware/Monitoring, Misc, Network/Node, ObjectStore, Policy x2, Replication)
plus one canonical reference batch (6 Admin cmdlets), each independently implemented and code
reviewed.
56 cmdlets this closes.
Reports/PfbApiDriftReport.mdregenerated against the current spec cache.found — see the SDD ledger for the full per-cmdlet test log.
Deliberately out of scope
Filed as separate follow-up issues rather than silently dropped:
blast-radius risk on
Update-PfbArrayConnection.on 2026-07-26, the
Enabled/Nameparser-gap triage, an unpersistedDeprecatedBodyPropertiesflag, and several smaller items (
.EXAMPLEblock regressions, two test-coverage gaps, adispatch-idiom drift item, two unresolved live-testing puzzles, a stale branch to close, and
three older carried-forward audit items).
Test plan
-WhatIf) with no defects found🤖 Generated with Claude Code