fix: invalidate cache when S3 object deleted (404), don't serve stale#13
Merged
Conversation
handleDownload treated every HeadObject error as "S3 unreachable" and served the stale cached copy. A deleted object returns a 404, which was misclassified, so deleted-on-S3 objects were served from cache forever even when S3 was reachable (CAPI BUG-02 stale cloud-init user-data ISO). Add s3client.ErrNotFound sentinel; HeadObject now wraps 404 responses (types.NotFound/NoSuchKey, smithy APIError, or HTTP 404) with it. handleDownload branches on it: purge the cache via Remove (clears the immutable flag PVE sets on ISOs) and return 404, failing closed instead of serving stale identity-bearing bytes. Genuine transport errors still serve stale cache for graceful degradation. Co-Authored-By: Claude Opus 4.8 (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
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.
Issue #12
Problem
proxs3dserved a stale cached copy of an object that had been deleted from S3, indefinitely, even when S3 was fully reachable.HeadObject(internal/s3client/client.go) wrapped every error in a genericfmt.Errorf, so a404 NotFoundwas indistinguishable from a transport failure.handleDownload(internal/api/api.go) then treated anyHeadObjecterror as "S3 unreachable" and fell into its serve-stale-cache fallback:So once an object was deleted on S3, the cached copy was returned on every subsequent
/v1/downloadand never purged. (Object replacement with a new ETag was already handled byIsStalewhile S3 is reachable — only the deletion/404 path leaked.)This is the proxs3-side amplifier behind stale cloud-init
user-data-<vmid>.isopayloads: a replacement VM reusing a VMID could boot with a previous worker's nocloud identity. It's both an availability and a secret-hygiene problem (bootstrap ISOs carry join tokens / credentials).Fix
internal/s3client/client.go— addErrNotFoundsentinel andisNotFound()(detectstypes.NotFound/NoSuchKey, smithyAPIErrorcodes, and HTTP 404 for S3-compatible backends like MinIO).HeadObjectwraps 404 responses withErrNotFound.internal/api/api.go—handleDownloadbranches onerrors.Is(err, s3client.ErrNotFound): purge the cache and return 404, failing closed instead of serving stale identity-bearing bytes. Usescache.Remove(notInvalidate) so thechattr +iflag PVE sets on ISOs is cleared and the file is actually deleted. Genuine transport errors keep serving stale cache (graceful degradation preserved for immutable ISOs/templates).internal/api/handler_test.go—TestHandleDownload_ObjectDeletedOnS3_RemovesCache(cached entry + 404 → 404 response + cache purged); existingTestHandleDownload_S3Unreachable_ServeStaleCachekept green.Behavior change
Verification
go build ./...,go vet ./...— cleango test ./...— all pass except the pre-existingTestFileInUse_OpenFile(darwin-onlyfileInUsedetection, fails on a clean tree too — unrelated)