Skip to content

fix: five wire-contract defects in the syslog, replication, quota, object-store and SMTP cmdlets - #108

Merged
juemerson-at-purestorage merged 11 commits into
dmann000:mainfrom
juemerson-at-purestorage:integration/p0-wire-fixes
Aug 12, 2026
Merged

fix: five wire-contract defects in the syslog, replication, quota, object-store and SMTP cmdlets#108
juemerson-at-purestorage merged 11 commits into
dmann000:mainfrom
juemerson-at-purestorage:integration/p0-wire-fixes

Conversation

@juemerson-at-purestorage

Copy link
Copy Markdown
Collaborator

Fixes #100, #101, #103, #87, and #80.

Five independent wire-contract defects, developed on separate branches and merged here as one PR so Reports/ is regenerated exactly once. Each fix keeps its own commit; the merges are --no-ff.

The common thread: every one of these cmdlets sent a query key the endpoint does not declare, or failed to send one it requires. Undeclared query parameters are silently ignored by the array rather than rejected, so all five produced plausible-looking results while doing the wrong thing. The published OpenAPI spec was treated as the authority throughout — no parameter was widened or narrowed on the basis of observed array behaviour.

#103Test-PfbSyslogServer tested every server regardless of -Name

/syslog-servers/test declares only X-Request-ID and continuation_token — no names, no ids, in any spec version from 2.0 through 2.28. The cmdlet sent both, so they were discarded and every call tested all configured servers.

-Name and -Id are removed along with their parameter sets. The endpoint offers no scoping and the cmdlet no longer pretends otherwise; the help now says so explicitly, with a Where-Object example for narrowing the returned rows client-side. This is a breaking signature change, and deliberately so — a script passing -Name today is already getting unscoped results, so a hard "parameter cannot be found" is strictly better than the current silence.

The other seven Test-Pfb* cmdlets were audited against their endpoints and are all correct. This was an isolated bug, not a family pattern.

#87Get-PfbFileSystemReplicaLinkTransfer sent a dead names key

GET /file-system-replica-links/transfer scopes by names_or_owner_names, not names. The cmdlet routed -Name through Add-PfbCommonQueryParams, which hardcodes the generic names/ids keys, so the filter never reached the wire.

Follows the pattern established by #91: -Name becomes -NameOrOwnerName with [Alias('Name')] for backward compatibility, the query key is set directly instead of through the generic helper, -Id gains pipeline-by-property-name binding, and a new imperative coercion guard catches a piped object stringified to @{name=...} before the request fires.

total_only is declared on this endpoint, so -TotalOnly stays here.

Scope correction worth noting: the issue names two cmdlets, but only one was broken. Remove-PfbFileSystemReplicaLink has never had a -Name parameter and never emitted a names key — all five keys it sends are declared on the DELETE. It is untouched.

#100 — group quota writes put the identity in the body

POST/PATCH/DELETE /quotas/groups identify the target through file_system_names, group_names and gids query parameters; the body carries only quota. The cmdlets were building the identity into the body, so the array never received a file-system identifier at all.

Identity now goes in the query for all three verbs. -GroupName and -GroupId are mutually exclusive parameter sets, and gids is sent as an integer per the schema.

#101New-PfbObjectStoreAccountExport could never create an export

Three independent defects, none of which had test coverage:

  • It sent names, which POST does not accept — the method identifies by member_names/member_ids and policy_names/policy_ids.
  • -Name was the only identifying parameter, and it was the wrong one.
  • The body was built as if ($Attributes) { $Attributes } else { @{} }, so the schema-required server reference was never sent unless the caller hand-rolled it.

The parameter surface is rebuilt to match the POST contract: -MemberName/-MemberId, -PolicyName/-PolicyId, -ServerName/-ServerId building the required server body reference, and -ExportEnabled as [Nullable[bool]] guarded by ContainsKey so $false survives. A call supplying no server identity throws before the request fires rather than sending a body the schema rejects.

Design note: this landed as four parameter sets plus two explicit pre-request throws for the policy and server exclusivity, rather than the eight sets a purely declarative encoding would need. Eight sets made the -? help unreadable for a marginal gain, and the throws give better messages than a failed set resolution.

Also here, both opportunistically:

#80 — consolidate the SMTP cmdlets onto /smtp-servers

Update-PfbSmtp PATCHed a REST 1.12 path that does not exist in 2.x, so it has never worked on a 2.x-negotiated connection — and there was no modern write cmdlet at all, leaving encryption_mode unsettable from the module.

Get-PfbSmtp and Update-PfbSmtp are deleted; Get-PfbSmtpServer stays; Update-PfbSmtpServer is new.

Deletion rather than aliasing loses nothing on the wire — /smtp-servers carries both GET and PATCH from REST 2.0, and its 2.0 writable body is exactly the 1.12 field set (relay_host, sender_domain), with encryption_mode added at 2.15. It is a strict superset from the first 2.x version onward.

Aliasing was rejected because the two endpoints disagree about what name means: /smtp returned the array name, /smtp-servers returns the resource name (management). Anyone reading (Get-PfbSmtp).name through an alias would silently get a different string. A removed cmdlet is an honest break; an alias would have been a quiet one.

No explicit version gate was added for -EncryptionMode. The capability map already records it at 2.15 and Invoke-PfbApiRequest asserts capability on every request, so putting the field in the body is sufficient — the throw fires automatically below 2.15. -ApiVersionOverride is kept as plumbing.

No ValidateSet on -EncryptionMode: the spec says valid values include starttls, which is open-ended, and a ValidateSet would impose a floor the spec does not set.

Tests

Six of the cmdlets touched here had zero test coverage, which is precisely why the bugs shipped. New coverage is part of the fix, not a follow-up.

New files: Test-PfbSyslogServer, Get-PfbFileSystemReplicaLinkTransfer, New-PfbQuotaGroup, QuotaGroup, New-PfbObjectStoreAccountExport, ObjectStoreAccountExport, Get-PfbSmtpServer, Update-PfbSmtpServer.

Each carries a permanent regression guard for the specific dead key it fixes, plus the cases that silently break under truthiness guards (-ExportEnabled:$false, -EncryptionMode '' to clear the value). The sub-2.15 encryption_mode gate is mock-only — no lab array is old enough to exercise it.

Run against the merged tree under both editions:

Scope pwsh 7 Windows PowerShell 5.1
The 9 test files above 94 passed, 0 failed 94 passed, 0 failed
Drift, field-map, manifest 202 passed, 0 failed 113 passed, 0 failed, 89 skipped

The 5.1 skips are the PS7-only tools/ tests.

Live verification

Verified against real hardware, not just mocks. FB-A (Purity//FB 4.8.2, REST 2.26) covered #87 and #103; FB-B (same version) covered #100, #101 and the #80 read path after FB-A became unavailable mid-run.

#87 — the transfer read returned 42 rows unfiltered and 42 with -Name 'fs-share' on main, confirming the key was ignored. On this branch the same snapshot filter returns 3, a bogus value is rejected with File system does not exist., and -TotalOnly still reports total_item_count: 42.

One thing observed and not explained: the owner form of the filter returns 43 rows against 42 unfiltered — stable across three passes, and above total_item_count. Reported as observed array-side behaviour of the owner form rather than as a regression; main cannot send names_or_owner_names at all, so there is no baseline to compare against.

#100 — the contrast is the evidence. Same call, same array:

Code Result
main HTTP 400 File system identifier is required.
this branch HTTP 400 Failed to create quota, names: pslivetest-grp cannot be mapped to ids using the configured directory server(s).

On main the request carried no file-system identifier at all. Here the array accepted file_system_names, moved on to the group, and quoted the exact group name back — the query keys were parsed and acted on. Both are failures and both are deliberate: the test array's directory service is disabled, so no group name can resolve to a GID there and a successful create was never available. -GroupId could not be exercised for an unrelated tooling limitation.

#101 — full lifecycle. main fails with Object store account export identifier is required.; this branch creates the export, and the response echoes member, policy and server exactly as sent. Read-back by name returns the one row, -ExportEnabled:$false comes back enabled: False, delete removes it, and a follow-up enumeration returns total_item_count: 0. All fixtures cleaned up, no residue.

#80Get-PfbSmtpServer verified live, returning name: management, which is the resource-name-vs-array-name difference described above, observed rather than inferred.

Update-PfbSmtpServer has no live coverage. The lab harness denies write verbs for the smtp-servers family, and that is a policy rail rather than an array condition, so no target changes the answer. The write path rests on mocked coverage alone. Flagging it rather than letting the batch read as fully live-verified.

Not in this PR

No version bump and no CHANGELOG entry — those are the maintainer's own call, always separate.

Reports/ is regenerated in a single commit at the end. The deltas track the fixes: uncovered endpoints 96 → 95 as /smtp-servers PATCH becomes covered, gids / member_ids / policy_names / file_system_ids each drop an endpoint as the writes start sending them, total_only's cmdlet count drops 39 → 38, and the two stale Update-PfbSmtp entries against the retired /smtp endpoint are gone. Generated by the tools, no hand edits.

Out of scope and left alone: context_names / allow_errors on any of these endpoints (in-flight Fusion work), the wider -TotalOnly sweep (#102), and the remote_names / remote_ids replication filters (#88).

One follow-up was filed while working through this batch: #106, for the same empty-body pattern that broke #101 appearing in New-PfbObjectStoreAccessPolicyRule and 22 other cmdlets.

juemerson-at-purestorage and others added 11 commits August 9, 2026 19:24
Test-PfbSyslogServer now reflects the API contract by testing all configured
syslog servers without sending unsupported names or ids query keys.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…n000#87)

Rename the transfer selector with a backward-compatible Name alias and reject piped object coercion before requests. Add regression coverage for declared query keys and common parameters.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…act (dmann000#101)

New-PfbObjectStoreAccountExport could never create an export. It sent a
`names` query key, which the POST does not accept, identified its target
with the wrong parameter, and never built the required `server` body
field. Rebuild the parameter surface against the published 2.28 spec:

- Drop -Name and the dead `names` query key.
- Add -MemberName/-MemberId and -PolicyName/-PolicyId, emitting
  member_names/member_ids and policy_names/policy_ids. The query hashtable
  is built directly rather than through Add-PfbCommonQueryParams, which
  hardcodes the generic `names`/`ids` keys POST rejects.
- Add -ServerName/-ServerId building the required `server` reference, never
  sending the readOnly `resource_type`. A call with no server identity
  throws before the request fires.
- Add -ExportEnabled as [Nullable[bool]], guarded by ContainsKey so an
  explicit $false reaches the body.
- Keep -Attributes as a mutually exclusive parameter set.

Also fix both Update-PfbObjectStoreAccountExport examples, which documented
a nonexistent `enabled` body field instead of `export_enabled`, and remove
the dead -TotalOnly switch from Get-PfbObjectStoreAccountExport (the GET
does not declare total_only).

Adds Tests/New-PfbObjectStoreAccountExport.Tests.ps1 and
Tests/ObjectStoreAccountExport.Tests.ps1; New-/Get-/Remove- had no coverage.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Update group-quota writes to use the published group_names/gids wire keys, quota-only bodies, and pipeline-capable parameter sets with focused cross-edition tests.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Update-PfbSmtp PATCHed the legacy REST 1.12 /smtp path, which does not exist
in any 2.x spec, so it has never worked on a 2.x-negotiated connection. There
was no modern write cmdlet, leaving encryption_mode unsettable.

/smtp-servers carries both GET and PATCH from REST 2.0 with the same writable
body (relay_host, sender_domain), so it is a strict superset of the 1.12
surface and deletion loses nothing.

- Delete Public/Monitoring/Get-PfbSmtp.ps1 and Update-PfbSmtp.ps1.
- Add Public/Monitoring/Update-PfbSmtpServer.ps1: PATCH smtp-servers with
  -RelayHost/-SenderDomain/-EncryptionMode, a mutually exclusive -Attributes
  set, and SupportsShouldProcess/ConfirmImpact=Medium. Every body field is
  guarded by $PSBoundParameters.ContainsKey, never truthiness, so the
  documented '' clear-the-value case for encryption_mode survives. No
  ValidateSet on -EncryptionMode; the spec enum is open-ended. No explicit
  version gate either -- the capability map records encryption_mode at 2.15
  and Assert-PfbApiCapability throws transparently below it.
- Rewrite the Get-PfbSmtpServer doc comment, which claimed to be an alias for
  the now-deleted Get-PfbSmtp, and note the name-field behaviour change.
- Update FunctionsToExport in the manifest.
- Add Tests/Update-PfbSmtpServer.Tests.ps1 and Tests/Get-PfbSmtpServer.Tests.ps1
  (net-new: the SMTP family had zero coverage) plus removal guards in
  Tests/RemovedCmdlets.Tests.ps1.

BREAKING CHANGE: Get-PfbSmtp and Update-PfbSmtp are removed. Get-PfbSmtpServer
returns the SMTP resource name (e.g. 'management') in .name, not the array name.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Regenerated once on the integration branch rather than per fix branch, so
the five merges stay conflict-free.

Build-PfbFieldCmdletMap and Build-PfbApiDriftReport, no hand edits. The
deltas track the five fixes: uncovered endpoints 96 -> 95 as /smtp-servers
PATCH is now covered by Update-PfbSmtpServer; gids, member_ids, policy_names
and file_system_ids each drop an endpoint as the quota-group and object-store
export writes start sending them; total_only's cmdlet count drops from 39 to
38 with the dead switch removed from Get-PfbObjectStoreAccountExport; and the
two stale Update-PfbSmtp entries against the retired /smtp endpoint are gone.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@juemerson-at-purestorage
juemerson-at-purestorage merged commit 1111920 into dmann000:main Aug 12, 2026
4 checks passed
juemerson-at-purestorage added a commit that referenced this pull request Aug 13, 2026
The endpoint-coverage invariant shipped with a deliberately self-retiring
allowlist of GET /smtp and PATCH /smtp, asserted by exact equality so that it
would fail in both directions: on a new map gap, and on the SMTP consolidation
landing.

#80 landed in PR #108 and the tripwire fired as designed. Get-PfbSmtp and
Update-PfbSmtp are gone, consolidated onto /smtp-servers, and the module now
carries no REST 1.x surface at all -- zero -ApiVersionOverride sites. The
expected set is therefore deleted rather than updated, per its own failure
message. Tests/RemovedCmdlets.Tests.ps1 (from #108) guards the removal itself.

Every endpoint literal under Public/ now resolves to a capability-map key with
no exceptions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant