[P1] Evaluate conditional PutObject against the logical object across pools

Problem and impact

Multi-pool PutObject holds the object lock but chooses a destination pool before evaluating public write preconditions. Its ordinary set-layer check reads only that pool. A stale If-Match can therefore replace the logical current object; If-None-Match: * can replace an object that exists in another pool even when no stale duplicate exists. A valid current ETag can also be rejected. This breaks conditional update and create-only guarantees.

PR #190 fixed CompleteMultipartUpload, not this path. Treat this as a separate P1 blocker for a general multi-pool release. The introduction commit and earliest affected published release are not yet established; do not infer that #190 introduced it.

Independent reproduction on current main

Six SigV4-signed PUT/GET handler scenarios ran on real two-pool fixture storage in an isolated Linux/arm64 Alpine 3.23 container, compiled with Go 1.27.1. The test changes reported allocation capacity only to deterministically place writes in pool 0; production routing, condition checking and object I/O are unchanged. Four defect cases fail and two controls pass. This is HTTP-handler/object-layer verification, not a production endpoint test.

Fixture and request	Expected	Observed
Old copy in pool 0, current in pool 1; If-Match=old ETag	412, current bytes unchanged	200; subsequent GET returns replacement bytes
Same fixture; If-Match=current ETag	200	412
Pool 0 empty, current object in pool 1; If-None-Match=*	412, current bytes unchanged	200; subsequent GET returns replacement bytes
Same fixture; If-Match=current ETag	200	404
Current object in pool 0; matching ETag	200	200 (control)
Current object in pool 0; stale ETag	412	412 (control)

Relevant code: pool dispatch, set-local condition, conditional completion fix. S3 conditional-write contract.

Implementation boundaries

Evaluate the appropriate object across pools under the existing object lock, fail closed on unverifiable metadata, and avoid repeating the accepted check against a destination-local copy. Distinguish public current-object conditions from version-addressed replication and internal PreserveETag duplicate detection. Preserve data movement, replica metadata reconciliation and SSE client-visible ETags. Reuse the current helpers instead of introducing a second lock or a general reconciliation framework.

Acceptance

Reproduce the four failures and retain the two controls as permanent tests.
Fix If-Match and If-None-Match for both stale-copy and empty-destination scenarios; failed writes preserve the current bytes.
Cover unversioned, versioned/suspended, delete-marker and missing-object states, unreadable other pools, and two concurrent create-only writes.
Cover version-addressed replication / PreserveETag, SSE client-visible ETags, and existing multipart/DELETE condition regressions without broadening their contracts.
Add the availability/compatibility change to release notes; determine affected-version bounds where evidence permits.
On the reviewed final SHA, pass focused, race, full cmd/internal and required CI checks; link the repair PR and evidence before closure.

Self-contained regression test (uses existing package cmd test helpers)

Save as cmd/conditional_put_review_test.go at the baseline and run go test ./cmd -run '^TestFollowupConditionalPutAcrossPoolsHTTP$' -count=1 -v. On macOS, the observed run cross-compiled the test binary for Linux/arm64 and ran it with tmpfs-backed fixture disks.

package cmd

import (
	"bytes"
	"context"
	"fmt"
	"net/http"
	"net/http/httptest"
	"testing"
	"time"

	xhttp "github.com/minio/minio/internal/http"
)

// Change allocation capacity only. All object metadata and data operations
// continue to use the real fixture disks.
type followupPutCapacityDisk struct {
	StorageAPI
	full bool
}

func (d followupPutCapacityDisk) DiskInfo(ctx context.Context, opts DiskInfoOptions) (DiskInfo, error) {
	info, err := d.StorageAPI.DiskInfo(ctx, opts)
	info.Total = 1 << 40
	info.Used = 0
	if d.full {
		info.Used = info.Total - (1 << 20)
	}
	info.Free = info.Total - info.Used
	return info, err
}

func TestFollowupConditionalPutAcrossPoolsHTTP(t *testing.T) {
	for _, tc := range []struct {
		name        string
		oldCopy     bool
		currentPool int
		condition   string
		wantStatus  int
	}{
		{"stale-etag-accepted", true, 1, "match-old", http.StatusPreconditionFailed},
		{"current-etag-rejected", true, 1, "match-current", http.StatusOK},
		{"create-only-overwrites-other-pool", false, 1, "none-match", http.StatusPreconditionFailed},
		{"current-etag-missing-in-write-pool", false, 1, "match-current", http.StatusOK},
		{"control-current-in-write-pool", true, 0, "match-current", http.StatusOK},
		{"control-stale-etag-rejected", true, 0, "match-old", http.StatusPreconditionFailed},
	} {
		t.Run(tc.name, func(t *testing.T) {
			z, _ := consistencyPools(t)
			bucket, router, err := initAPIHandlerTest(t.Context(), z, nil, MakeBucketOptions{})
			if err != nil {
				t.Fatal(err)
			}
			const object = "conditional-put-review"
			var old ObjectInfo
			if tc.oldCopy {
				old = putConsistencyObject(t, z, bucket, object, 1-tc.currentPool, "old", ObjectOptions{MTime: UTCNow().Add(-time.Hour)})
			}
			current := putConsistencyObject(t, z, bucket, object, tc.currentPool, "current", ObjectOptions{MTime: UTCNow().Add(-time.Minute)})
			visible, err := z.GetObjectInfo(t.Context(), bucket, object, ObjectOptions{})
			if err != nil || visible.ETag != current.ETag {
				t.Fatalf("invalid logical current object: %+v %v", visible, err)
			}
			for poolIdx, pool := range z.serverPools {
				set := pool.getHashedSet(object)
				previous := set.getDisks
				disks := append([]StorageAPI(nil), previous()...)
				for i, disk := range disks {
					disks[i] = followupPutCapacityDisk{StorageAPI: disk, full: poolIdx != 0}
				}
				set.getDisks = func() []StorageAPI { return disks }
				t.Cleanup(func() { set.getDisks = previous })
			}
			idx, err := z.getWritePoolIdx(t.Context(), bucket, object, 11, false)
			if err != nil || idx != 0 {
				t.Fatalf("allocation fixture must select pool 0: idx=%d err=%v", idx, err)
			}
			headers := make(map[string]string)
			switch tc.condition {
			case "match-old":
				headers[xhttp.IfMatch] = fmt.Sprintf("%q", old.ETag)
			case "match-current":
				headers[xhttp.IfMatch] = fmt.Sprintf("%q", current.ETag)
			case "none-match":
				headers[xhttp.IfNoneMatch] = "*"
			}
			body := []byte("replacement")
			req, err := newTestSignedRequestV4(http.MethodPut, getPutObjectURL("", bucket, object), int64(len(body)), bytes.NewReader(body), globalActiveCred.AccessKey, globalActiveCred.SecretKey, headers)
			if err != nil {
				t.Fatal(err)
			}
			rec := httptest.NewRecorder()
			router.ServeHTTP(rec, req)
			getReq, err := newTestSignedRequestV4(http.MethodGet, getGetObjectURL("", bucket, object), 0, nil, globalActiveCred.AccessKey, globalActiveCred.SecretKey, nil)
			if err != nil {
				t.Fatal(err)
			}
			getRec := httptest.NewRecorder()
			router.ServeHTTP(getRec, getReq)
			t.Logf("PUT status=%d want=%d; subsequent GET status=%d body=%q", rec.Code, tc.wantStatus, getRec.Code, getRec.Body.String())
			if rec.Code != tc.wantStatus {
				t.Errorf("conditional PUT status=%d want=%d; response=%s", rec.Code, tc.wantStatus, rec.Body.String())
			}
			wantBody := "current"
			if tc.wantStatus == http.StatusOK {
				wantBody = string(body)
			}
			if getRec.Code != http.StatusOK || getRec.Body.String() != wantBody {
				t.Errorf("conditional write changed or failed to change visible data: GET %d body=%q want=%q", getRec.Code, getRec.Body.String(), wantBody)
			}
		})
	}
}

Tracking boundary

Baseline: 40220bd836cbd066ca424fa4dc5dbb90057fb55a (main, PR #196), verified 2026-09-16. R1–R8 are closed within their original defect scopes. This issue tracks the follow-up below. R9 / #79 / #198 is explicitly deferred to a separate effort and is not a dependency of this workstream. Source merge, release artifacts and deployment acceptance must be reported separately.

Follow-up

Implementation priority: first. Release integration and final artifact acceptance: #203. Public notes: #202. The six-case failing/control test above is ready to promote into the repair PR.

A candidate implementation and regression suite have been prepared in a dedicated worktree. The required independent Claude Opus 5 Max review of the frozen source diff reported no production-code blockers, conditional on the remaining validation results.

The focused functional regressions passed, but race validation is not clean: it exposed interactions between test disk adapters and background IAM activity, including the new fixture. This must be resolved and the relevant checks rerun before submission. The execution session was interrupted before completing that work.

No fix commit or repair PR has been submitted, and this issue remains open. The existing candidate, review response and test logs are preserved; a changed final diff will need its corresponding review and validation evidence. #205 continues to describe conditional PUT as unresolved.

Confirmed as a real defect and fixed in #207 (open, CI green; not merged yet).

Reproduction and scope

All four defect scenarios in this issue reproduce on 40220bd836cbd066ca424fa4dc5dbb90057fb55a with the two controls passing, via SigV4 PUT/GET against real two-pool erasure storage with allocation-only capacity steering. The same six probes were recompiled against unmodified source from the published tag RELEASE.2026-09-03T13-18-01Z with identical results, so the released version is affected. Source history identifies capacity re-selection 83b2ad418 (2025-07-19) as the trigger for the empty-destination case; per the issue's caution this is not claimed as the earliest affected boundary for every duplicate/draining scenario, and #190 (multipart completion) neither introduced nor repaired this PUT path.

Fix

erasureServerPools.PutObject now evaluates ordinary public write conditions itself, under the object lock it already holds, before destination dispatch: it resolves the comparison object across all pools with objectPoolInfos (including draining pools, same ModTime/pool-index ordering as GET), treats a latest delete marker as absence, fails closed when any pool's metadata for the key cannot be verified, calls the existing handler callback once, and clears it on acceptance so the destination's local copy cannot re-judge the condition. Internal replica and data-movement callbacks (ReplicationRequest/ReplicaLockReconcile/DataMovement) are excluded and keep their addressed-version semantics; the write's VersionID/PreserveETag are untouched. 34 production lines; no API, lock, disk-format, or allocation change.

Validation

New permanent regressions: 7 failing top-level tests / 62 failing entries against baseline production source, all green after the fix, plus 5 controls that pass on both. 12 top-level tests / 105 entries total, zero skips: both pool orientations × unversioned/versioned/suspended; UUID/null/missing delete markers; specific and wildcard ETags; either-pool read faults (fail-closed); SSE-C/S3/KMS ETags; concurrent create/CAS; lock ordering vs DELETE/metadata; draining pools; ties; NoLock; destination versionId; replica duplicate suppression and availability.
Independent adversarial review of both the design and the frozen final diff (tree 90305806d, diff SHA-256 4045e218…) by Claude Code 2.1.270, claude-opus-5 --effort max: final verdict ACCEPT, no blockers. Its design-review blocking finding — the guard initially covered internal replica callbacks and would have reduced replication availability — was adopted before implementation. Nonblocking observations (e.g. stale duplicates are not auto-retired on the ordinary path; version-addressed placement semantics worth a dedicated test) are recorded for follow-up.
Local: focused suite, ./internal/..., build/vet/lint/generated/verifiers PASS; full ./cmd/ suite native run PASS (5,971 tests, ok 527.553s, zero failures). A Docker-isolated attempt of the same suite hung in TestCreateServerEndpoints on the orchestrated DNS wait (endpoint.go <-keepAliveTicker.C) — a container artifact of that isolation method, unrelated to this change; the plain VM and native runs pass.
CI on fix(pools): evaluate cross-pool PUT conditions against the current object #207: all checks green (Format/Build/Vet, Lint/Generated, Race-S3Select, Cross Compile, Test internal/, Test cmd/, DCO, VulnCheck).

Operator-visible behavior changes (for release notes)

A multi-pool ordinary conditional PUT now fails closed (503) when any pool's metadata for the key is unreadable, even if GET still succeeds from another pool. Restore readability/heal and retry; unconditional writes, single-pool conditions and replica traffic are unchanged.
With a destination versionId, ordinary public conditions now compare the current object; replication keeps addressed-version semantics.
A successful cross-pool conditional overwrite does not retire the superseded copy in the other pool (pre-existing duplicate behavior on this path); upgrade does not undo historical accepted overwrites.

Release-note integration, packaging, deployment acceptance and the merge decision remain with the maintainers, so this issue is intentionally left open until the fix is actually merged and released.

test: synchronize conditional PUT disk fixtures

Follow-up verification: PR #207 is now at 4620be394b523601285098be7b0fa3547dc27b8a, with all eight CI checks passing on that exact commit. The test-fixture race was repaired in the follow-up commit, and CI now explicitly runs the conditional PUT suite under the race detector. The follow-up changes only tests and CI; the original 34-line production fix is unchanged.

I also checked the actual Opus review records. The original production diff and the fixture/CI follow-up both received ACCEPT, no blockers from claude-opus-5 --effort max. The follow-up's frozen tree (d345640dbd080d22481daa6da6af9ef0f938933c) and diff SHA-256 (340248639e3308c83c2863cf8ae6ed5bf7a9961673238d22d5a2187090035193) match the committed change. The previously pending Linux race step passed, resolving the review's timing condition.

The bilingual operator notes are being coordinated through #202 and website PR #25, including fail-closed behavior when pool metadata is unreadable, public destination-version semantics and historical-state limits. #207 was subsequently merged at 9b4ae82a29cc2290fb5be7b551ec3d8cf7acdd99 on 2026-09-16. The source fix is now on main. Keeping the release boundary explicit: the published Server remains 20260903, and these checks do not establish a new release or production rollout.

fix(pools): evaluate cross-pool PUT conditions against the current ob…

Closing as completed for the source defect.

The conditional PUT repair in #207 is merged at 9b4ae82a29cc2290fb5be7b551ec3d8cf7acdd99. It evaluates ordinary public write conditions against the logical current object across pools under the existing lock, fails closed when the comparison state cannot be verified, and preserves the separate replication/data-movement paths.

The reviewed PR head 4620be394b523601285098be7b0fa3547dc27b8a passed all eight checks. The recorded validation includes the original failing/control cases, the expanded conditional PUT regressions and race coverage; the test-fixture race was repaired, and CI now explicitly runs this suite with the race detector. Release notes are merged through #208, with bilingual operator guidance integrated through website #25 and #30.

This closes the source-repair scope and supersedes the earlier plan to keep this issue open until publication. Final candidate artifacts and stack acceptance remain tracked in #203, with the final release/component identities in #202. The latest published Server is still RELEASE.2026-09-03T13-18-01Z, which is affected and does not contain this fix. Closure does not establish a new Server release, a production rollout, or recovery of historical overwrites.

Navigation Menu

Repository navigation

[P1] Evaluate conditional PutObject against the logical object across pools #199

Problem and impact

Independent reproduction on current main

Implementation boundaries

Acceptance

Tracking boundary

Follow-up

Reproduction and scope

Fix

Validation

Operator-visible behavior changes (for release notes)

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

[P1] Evaluate conditional PutObject against the logical object across pools #199

Description

Problem and impact

Independent reproduction on current main

Implementation boundaries

Acceptance

Tracking boundary

Follow-up

Activity

Vonng commented on Sep 15, 2026

Vonng commented on Sep 16, 2026

Reproduction and scope

Fix

Validation

Operator-visible behavior changes (for release notes)

Vonng commented on Sep 16, 2026

Vonng commented on Sep 16, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions