fix(workloads): send a JSON array when a single tag is supplied, and fold Set-PfbWorkloadTag onto the shared request path - #81
Merged
juemerson-at-purestorage merged 4 commits intoAug 2, 2026
Conversation
PUT /workloads/tags/batch declares its request body as a top-level array with
minItems 1, and Set-PfbWorkloadTag's [ValidateCount(1, 30)] explicitly permits
one tag. But the body was built by piping $Tags into ConvertTo-Json, and the
pipeline unrolls a collection: a one-element [hashtable[]] arrived at
ConvertTo-Json as a bare hashtable and serialised to a JSON object.
1 tag via pipeline : {"key":"k","value":"v"} <- rejected by the array schema
2 tags via pipeline : [{"key":"k",...},{...}]
1 tag -InputObject : [{"key":"k","value":"v"}]
So every single-tag call was broken on the wire while every multi-tag call
worked. -InputObject serialises the collection as a whole and fixes it; the
multi-tag output is byte-identical either way.
Also removes the comment above that line. It described a $Raw/dummy-key design
for smuggling an array through Invoke-PfbApiRequest's hashtable-only -Body --
an approach that was never built, and whose absence is what left the
hand-rolled Invoke-RestMethod call below it in place.
Adds Tests/Set-PfbWorkloadTag.Tests.ps1; the cmdlet had none. The assertions
sit on the serialised body handed to Invoke-RestMethod rather than on the local
ConvertTo-Json call, so they keep testing the real wire shape after the cmdlet
is later refolded onto the shared request path. Verified to fail (single-tag
case only, both PowerShell editions) with this fix reverted.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Prerequisite for letting an array body through Invoke-PfbApiRequest, and ordered ahead of it deliberately: Invoke-PfbApiRequest forwards its own -Body straight into this function, so widening the caller first fails at bind time with "Cannot convert System.Collections.Hashtable[] to System.Collections .Hashtable". The plan for dmann000#77 had these two commits the other way round; that order does not build. -Body becomes [System.Collections.ICollection]. Not IEnumerable, which looks narrower but is worse -- System.String implements it, so PowerShell silently converts -Body 42 into the string "42" rather than rejecting it. Not a ValidateScript either: PowerShell rejects an explicit $null argument against one even under [AllowNull()], and every bodyless call reaches this function as -Body $null. ICollection admits Hashtable, Hashtable[], Object[], OrderedDictionary and List<T> while rejecting strings, numbers, booleans and PSCustomObjects exactly as [hashtable] did, and accepts $null. Confirmed identical under PowerShell 5.1 and 7. The element fields of an array body are deliberately NOT validated, which re-scopes what the plan recommended. The check it asked for first: does the capability map record per-element fields for these endpoints? It does not, and cannot today. tools/Build-PfbCapabilityMap.ps1 fills bodyProperties from Get-PfbSchemaPropertyNames, whose schema walk resolves $ref and allOf but never descends through an array schema's "items" -- there is no "items" handling in tools/lib/PfbSpecTools.ps1 at all. All three array-bodied endpoints in the 2.26 spec carry "bodyProperties": {} in Data/PfbCapabilityMap.json as a result: PUT /workloads/tags/batch bodyProperties {} (items -> $ref TagBatch) POST /nodes/batch bodyProperties {} POST /resource-accesses/batch bodyProperties {} So the recommended union-of-element-fields check would compare every field against an empty map and could never fire. Implementing it would have produced a gate that only looks like one, and the honest alternative to inventing a new map representation is to skip. Endpoint minVersion and query-parameter checks still run for array-bodied calls, which is gating those endpoints previously had none of at all. The loop is guarded on IDictionary rather than on an array test, so it picks up per-element data for free if the map ever learns to record it. The hashtable path is unchanged. The guard moves from `if ($Body)` to `if ($Body -is [IDictionary])`, which differs only for an empty hashtable -- and that case iterates zero keys and raises zero violations either way. New tests live in their own Describe with their own synthetic map so the existing hashtable-path expectations, which are the regression gate for this widening, are untouched: the file's diff is 118 insertions and 0 deletions. They cover an accepted array body, minVersion and query-parameter gating still firing for one, an array element field NOT being checked against a bodyProperties entry the same hashtable body IS checked against, an empty array, an explicit $null, and bind-time rejection of int/string/PSCustomObject bodies. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three endpoints in the REST spec declare a top-level JSON array as their
request body rather than an object -- PUT /workloads/tags/batch,
POST /nodes/batch and POST /resource-accesses/batch. None could be expressed
through the shared request path, which is why Set-PfbWorkloadTag hand-rolls its
own Invoke-RestMethod call.
Two changes:
- -Body widens from [hashtable] to [System.Collections.ICollection], matching
Assert-PfbApiCapability's parameter (widened in the preceding commit, which
had to land first because this function forwards -Body straight into it).
The type note lives there.
- Body serialisation switches from the pipeline to -InputObject. The pipeline
unrolls a collection, so a one-element array body would arrive at
ConvertTo-Json as its bare element and serialise to a JSON object instead of
a one-element array -- the same defect fixed in Set-PfbWorkloadTag two
commits ago would have been reintroduced here the moment that cmdlet was
folded onto this path. A hashtable is never unrolled by the pipeline, so
every existing caller's output is byte-identical.
No existing caller changes shape. All 153 -Attributes parameters in Public/ are
declared [hashtable], every $body local is initialised to @{}, $Attributes or
$null, and the two remaining call sites pass @{} and a hashtable. The verb
whitelist at the serialisation guard is untouched, so GET and DELETE still drop
a body.
Tests: an array body serialising as a top-level array, a single-element array
staying an array, a hashtable body still serialising as an object (the
regression gate for the -InputObject switch), an explicit $null -Body still
binding and sending nothing, and bind-time rejection of int/string/
PSCustomObject bodies.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The cmdlet hand-rolled its own Invoke-RestMethod call because the shared path could not carry a top-level array body. It can now, so the bypass goes away and Set-PfbWorkloadTag stops being the last write cmdlet in the module exempt from the shared request path (Set-PfbPresetWorkload was the other, folded back in dmann000#76). Four user-visible consequences: - Capability gating now runs. A call against an array below REST 2.23 throws locally instead of being sent and rejected on the wire. Covered by a test that also asserts no network call is made. - Errors now come from ConvertTo-PfbApiError rather than a raw Invoke-RestMethod exception, matching every other write cmdlet. - OAuth2/Certificate sessions work. The hand-rolled block set x-auth-token unconditionally and never consulted $Array.BearerToken, so the cmdlet was broken for certificate-authenticated connections. - 401/403 reconnect-and-retry now applies. PUT on this endpoint is a batch tag upsert, which is idempotent, so a replayed request is safe. Body serialisation moves to the shared path at -Depth 10, up from the local -Depth 5. This deletes the line the first commit on this branch fixed, so the single-tag guarantee now rests on Invoke-PfbApiRequest's ConvertTo-Json -InputObject instead. That is why the wire-shape tests were written against Invoke-RestMethod rather than against the local ConvertTo-Json: they are unchanged by this commit and still assert, end to end, that one tag reaches the wire as a one-element JSON array. The first commit remains independently revertable and worth keeping on its own -- it fixes the defect whether or not this refactor lands. Assert-PfbConnection and the ShouldProcess gate are unchanged, and ShouldProcess still wraps the call rather than the parameter construction. New tests: verb and endpoint, no direct Invoke-RestMethod escaping the cmdlet, -Tags passed through unmodified and still a collection, a single tag arriving as a one-element collection rather than being unwrapped by parameter binding, resource_names/resource_ids in QueryParams, -WhatIf making no call, and the sub-2.23 capability throw. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
juemerson-at-purestorage
added a commit
to juemerson-at-purestorage/fb-powershell
that referenced
this pull request
Aug 2, 2026
Rev 3 as first drafted modelled the .arrays suffix as a FanOut boolean on each context entry, and offered Get-PfbTopologyGroupMember | Set-PfbContext as reaching "the same outcome as <group>.arrays". Both are wrong. The field becomes an enum-valued Form (Object | AllArrays). A boolean can express exactly one alternative to the default, and the suffix vocabulary is already known to be open -- an all-sub-groups equivalent and realm-as-context would each need their own flag, and two booleans on one entry can encode a meaningless both-set state. Retrofitting the enum later is a breaking change to a published parameter, since -AllArrays and -Form cannot coexist cleanly. "Fan-out" is retired as a name for .arrays and reserved for a possible future client-side serial loop over N arrays, which differs in request count, failure semantics and mutability. The switch is -AllArrays. A terminology note in the Background separates the three senses; the non-goal already used the reserved one. The member-pipeline idiom is withdrawn as the whole-group form. A topology group's members may themselves be groups, so the pipeline yields group names where array names are required; .arrays is transitive over the sub-tree and is re-resolved server-side per request rather than snapshotting membership. The cmdlet stays correct for scoping to specific members, which is a different request. -AllArrays resolves membership before storing the context, via GET /topology-groups/arrays (2.26) or GET /fleets (2.17) -- an asymmetry forced by there being no /fleets/arrays. This is a bounded exception to the no-hidden-network-calls preference and settles Open Question 2: the rejected alternative was inferring Kind from topology on every context set, unbidden; this confirms a name the caller explicitly supplied, once. Without it a mistyped group name is stored and then silently misdirects every later call. Adds an ownership boundary under Phasing. Topology-group object management stays with issue dmann000#38; membership resolution is this design's; and dmann000#38 carries a stated contract that Get-PfbTopologyGroup emit a top-level Name that binds by property name -- without it the documented pipeline silently no-ops in exactly the way Ergonomics describes for Get-PfbFleetMember. All seven topology-group endpoints declare context_names and none declares allow_errors, so they are uniformly single-context under section 8 and need nothing further. Section 1 records Set-PfbWorkloadTag as closed pending merge (dmann000#77 / PR dmann000#81), and corrects its open question: /workloads/tags/batch is array-scoped, the exact inverse of /presets/workload, so it does not repeat the fleet-scoped failure -- but it declares context_names at 2.23 and still needs injection. Appendix A gains the CLI-versus-REST parity table for .arrays across seven cases, Appendix B's .arrays row records that the CLI man page documents the form the OpenAPI spec omits, and Appendix E carries the revision record. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
juemerson-at-purestorage
added a commit
that referenced
this pull request
Aug 3, 2026
…m order (#86) * fix(tools): emit report records in canonical order, not filesystem order Get-PfbCmdletParameterInventory and Get-PfbModuleCalledEndpoints both walked Public//Private/ with an unsorted recursive Get-ChildItem and let that walk become the emit order of their records. Regenerating Reports/ on a Linux CI runner instead of a Windows workstation therefore produced a 10,218-line diff across PfbFieldCmdletMap.json/.md and PfbApiDriftReport.json/.md with zero semantic change -- all 2015 entries moved, not one changed content, and the two files were even identical in byte length. That is what makes the update-api-capability-map auto-PR unreviewable. Sorted at emit rather than only on the file list: FullName carries platform-specific separators, so sorting the walk alone is the fragile fix. The walk is sorted too, but only as belt-and-braces for intermediate debugging output. Every sort pins -Culture '' (invariant) so the runner locale cannot reintroduce the divergence. Also replaces Select-Object -Unique with Sort-Object -Unique at the intra-row cmdlet list in Get-PfbParameterCoverageGaps (-Unique preserves input order and does not sort -- the observed `Get-PfbArray, Test-PfbConnection` flip), and sorts the Group-Object groups themselves, whose order is first-appearance in the input and so was also file-walk-derived. The regression tests deliberately do not regenerate twice on one machine -- that is the assertion Tests/Build-PfbApiDriftReport.Tests.ps1 already makes, and enumeration order is stable within a filesystem, so it can never fail. Instead two fixture trees hold the same cmdlets with the cmdlet-to-filename mapping swapped, which reproduces the divergence on any single platform and needs no tools/specs (so it will not silently skip in a fresh clone -- see #63). Refs #85 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(reports): regenerate under the canonical emit order Do NOT review this line by line. Two of the four files are a pure re-sort; the review that matters is the canonical comparison, reproduced below. Reports/PfbFieldCmdletMap.json and .md -- pure re-sort, zero content change: order-insensitive-equal vs committed: true entries 2015 -> 2015 multiset-equal 2015/2015 repositioned attributesOnly 68 -> 68 multiset-equal 65/68 repositioned typedUnresolved 39 -> 39 multiset-equal 35/39 repositioned markdown: identical line multiset, 161 lines both sides Generating the same artifact with HEAD~1's tools/lib and with this branch, from identical inputs, gives byte-length-identical files (586,461 both) that are order-insensitive-equal -- so the sort provably reorders and never rewrites. Reports/PfbApiDriftReport.json came out raw-identical between those two runs: on NTFS the unsorted walk already happened to yield the canonical order, which is precisely why this defect was invisible from a Windows workstation. Reports/PfbApiDriftReport.json and .md -- a real, expected content refresh, NOT part of the re-sort. The committed copies predate PRs #78/#81: uncoveredEndpoints 98 -> 96 PUT /presets/workload, PUT /workloads/tags/batch parameterGaps 436 -> 438 the same two, now as gap rows systemicGaps 252 -> 252 context_names 269 -> 270; 13 body-property names +1 each, all from Set-PfbPresetWorkload Those PRs routed Set-PfbPresetWorkload/Set-PfbWorkloadTag through Invoke-PfbApiRequest, which is the only thing the AST resolver can see -- so the endpoints leave the uncovered list and Set-PfbPresetWorkload immediately reappears carrying 14 missing body properties. "Uncovered -> covered" here means visible to the scanner, not finished; see #85 and the #45/#44 caveat. Per #85, the two artifacts that must NOT move were regenerated and hashed to confirm they did not: Data/PfbCapabilityMap.json (632 endpoints, 29 versions, -MaxVersion 2.28) and Reports/PfbValueEnumMap.json are both byte-identical to their committed copies. Tests/Build-PfbApiDriftReport.Tests.ps1's "no serialization-only divergence" invariant was already failing on main with 19 differences, unchanged by the sort, and passes again now the report is current. Refs #85 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <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
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.
Fixes #77.
Set-PfbWorkloadTagwas the last write cmdlet hand-rolling its ownInvoke-RestMethod, and it serialised its body with$Tags | ConvertTo-Json. The pipeline unrolls a collection, so a single tag arrived atConvertTo-Jsonas a bare hashtable and went over the wire as a JSON object where the endpoint requires an array.The array names the defect precisely. Against FB-A (Purity//FB 4.8.2, REST 2.26), one tag, before this change:
Two tags never hit it — the pipeline emits a real array at N≥2 — which is why this survived.
Commits
e356a5ffix(workloads): send a JSON array when a single tag is supplied41dab21feat(core): accept array request bodies in Assert-PfbApiCapability9f527ddfeat(core): allow array request bodies through Invoke-PfbApiRequest39bd09ffix(workloads): route Set-PfbWorkloadTag through Invoke-PfbApiRequeste356a5fis the minimal standalone fix and is independently revertable — verified by bothgit revertat its own tip andgit cherry-pickontomain. The remaining three fold the cmdlet onto the shared request path, which is what the endpoint needed anyway and what makes the cmdlet reachable by any future central query-parameter injection.Design decision: array bodies are not field-validated
Assert-PfbApiCapabilityiterated$Body.Keys, which an array does not have. The obvious fix is to validate the union of element field names — but the capability map records nothing to validate against.PUT /workloads/tags/batchcarries"bodyProperties": {}, as do the only other three array-bodied endpoints in the spec (POST /nodes/batch,POST /resource-accesses/batch,POST /fleets/members/batch).Root cause:
Build-PfbCapabilityMap.ps1fillsbodyPropertiesviaGet-PfbSchemaPropertyNames, whose schema walk resolves$refandallOfbut never descends through an array schema'sitems. Filed as #82; tracked with the other pending capability-map changes in #84.So a union check would compare every field against an empty map and could never fire — a gate that only looks like one. The loop is now guarded on
-is [IDictionary], which picks up per-element data for free if the map ever learns to record it. EndpointminVersionand query-parameter gating do still run for array bodies, which is gating these endpoints previously had none of.Confirmed against the live array that this loses nothing today: a bogus element field is rejected on the wire with
Unrecognized field "bogus_field".Notes on the implementation
-Bodyis typed[System.Collections.ICollection].IEnumerablelooks narrower but is worse —System.Stringimplements it, so-Body 42would silently bind as the string"42"instead of being rejected.ValidateScriptis unusable because PowerShell rejects an explicit$nullargument against one even under[AllowNull()], and several callers pass-Body $null.ICollectionadmits Hashtable / Hashtable[] / Object[] / OrderedDictionary / List and$null, and rejects int, string, bool and PSCustomObject — matching[hashtable]'s rejections exactly. Verified identical under 5.1 and 7.The shared serialisation line changed to
ConvertTo-Json -InputObject $Body. The previous($Body | ConvertTo-Json)would have reintroduced the exact single-element defect inside the shared path the moment the cmdlet was folded onto it. Hashtable output is byte-identical, and there is a test pinning that.Verification
Scoped Pester, 12 files (this change's tests plus every file exercising the real request path), both editions:
The 24 WinPS 5.1 skips are pre-existing
-Skip:($PSVersionTable.PSVersion.Major -lt 7)guards.Tests/Assert-PfbApiCapability.Tests.ps1is +118/-0 andTests/Invoke-PfbApiRequest.Tests.ps1is +99/-0 — no existing expectation was modified to accommodate this change.One pre-existing pwsh-7 failure in
Tests/Build-PfbApiDriftReport.Tests.ps1("Nothing vanishes" vs the committed report on disk) reproduces identically on untouchedmainand is excluded from the counts above.Live-verified against FB-A (REST 2.26):
mainwithcode 3deserialize error; reachescode 6herecode 6before and after — no regression400 (); nowFlashBlade API error: Workload does not exist.The error-shape improvement is a side effect of folding onto the shared path: the old raw
Invoke-RestMethoddiscarded the response body, so every failure surfaced as a bare 400. The cmdlet also inherits the 401/403 reconnect-and-retry path, which is safe here since this PUT is idempotent.Not in scope
PUT /workloads/tags/batchdeclarescontext_names(2.23), so this cmdlet will need fleet-context handling — that belongs to the Fusion work in #72, and this PR is the prerequisite that makes the cmdlet reachable by it. Live testing separately confirmed the endpoint is array-scoped: it accepts array names and<fleet>.arrays, and rejects a bare fleet name.No version bump or CHANGELOG edit, per the maintainer's call on those.