5 Commits

Include renames

Author	SHA1	Message	Date
Nguyễn Lộc Phúcandchrislusf	eba2cb926d	fix(s3tables): refuse a create that a namespace delete can lose (#12239) ... * s3tables: refuse a create that a namespace delete can lose DeleteNamespace checks the namespace for children and then deletes its directory recursively in a second filer call. The filer locks each path on its own, so a table created under the namespace between the two is not seen by the check and is removed by the delete, while both requests answer 200. The mirror case loses the table too: a create that passed its namespace check before the delete finished re-created the namespace directory through the filer's parent creation and recorded a table in a namespace that no longer exists. Each side now writes before it reads. DeleteNamespace claims the namespace record with a fresh token before it scans, deletes only while the record still holds that claim, and lifts its own claim when it stops. CreateTable, RegisterTable and CreateView re-read the namespace after writing their record and back out when it is claimed or gone: they remove what they wrote and answer 409 or 404 instead of reporting an entry that does not survive, or 500 when the record could not be removed and may remain. An entry the scan missed was written after the claim, so its create sees the claim; a create and a delete can both fail, but no longer both succeed. A create is also refused up front while a claim is held, but only after its authorization check, so a caller that may not create there does not learn of the delete. A create writes its record marked "create" and lifts the mark once the namespace is confirmed open. Until then GetTable, GetView, UpdateTable and a second create treat the record as absent, so no request is told about a table or view that the first create may still back out. * s3tables: keep a create's unfinished record out of reads and listings * s3tables: refuse catalog writes on a record an operation claimed * s3tables: answer 404 when a create's write path loses the namespace * s3tables: confirm the rename destination's namespace like a create * s3tables: cover unfinished-record reads, claimed-record writes, rename claim * s3tables: lift a leftover claim when a no-token delete clears the record * s3tables: answer 409 when a claimed record turns away a write UpdateView, TagResource, and UntagResource surfaced ErrConcurrentUpdate as a 500, so an expected conflict looked like a server fault to the caller. Map it to Conflict the way UpdateTable and the policy handlers already do. * s3tables: name the lost source when a rename finds it deleted * s3tables: report an entry deleted before its update as not found The filer answers an UpdateEntry on an entry removed since the lookup with codes.NotFound. mutateEntryExtended returned that error unchanged instead of filer_pb.ErrNotFound, so a rename whose source was deleted mid-claim answered 500 instead of NoSuchTable or NoSuchView. MemFiler now answers a missing entry the same way, so the rename tests for tables and views take the real filer's path. * s3tables: unpublish the destination when a rename's source clear fails * s3tables: refuse to delete a record another operation claimed * s3tables: release catalog claims on an uncanceled cleanup context * s3tables: answer 404 when a racing delete already removed the table * s3tables: keep the rename destination marked until the source clears * s3tables: guard the rename rollback against a recreated source and an unfinished destination * s3tables: drop the unfinished destination only once the source is restored * s3tables: re-resolve the purge target on the record a delete claims * s3tables: cover a delete that meets a moved metadata location * s3tables: answer 409 when a create's record changed under it * s3tables: adopt a record still marked by its unfinished create * s3tables: skip unfinished creates in the namespace empty check * s3tables: delete only the record the caller was authorized on DeleteView and DeleteTable check permissions on the record they read first, then read the name again to claim it. A record that took the name in between, such as a table or another owner's view created after a concurrent delete, was claimed and removed on the first record's authorization, and DeleteView never compared the caller's version token with it. The second read now repeats the first one's checks: a record of the other kind or an unfinished create answers 404, and a token that no longer matches answers 409. A delete without a token also compares the record's version token with the authorized one and answers 409 when they differ. * s3tables: re-check the parent after a create lifts its mark A create that passed checkParentOpen and paused before lifting its mark could still confirm while a namespace delete held the claim: the delete scans the still-marked record, skips it, and removes it under a create that already answered success. confirmParent now re-reads the same parent record after the mark clears and backs the confirmed record out when the namespace moved on. * s3tables: mark the rename destination with a claim of its own claimCatalogEntry adopted every create-marked record, including a rename's destination: a create landing after the source cleared could replace the destination before the rename published it, and with the source name reused the restore could leave the table on neither name. The destination now marks with the rename's own claim, which readers still treat as unpublished but a create must refuse, and the commit re-checks the destination namespace after the record goes live. The source keeps its rename claim until the commit lands so a namespace delete scanning the cleared record still refuses. * s3tables: compare the parent's createdAt, not its editable metadata A namespace update rewrites ExtendedKeyMetadata in place, so a create or rename whose second parent read landed across an UpdateNamespace saw a changed blob and reported the still-live namespace as gone: creates backed their record out and answered 404, renames undid their destination. createdAt survives the edit but changes on a delete+recreate, so it still identifies the record. * s3tables: retry lifting the rename's claim off the cleared source The release's only failure mode is a transient filer error — nothing else can write the cleared record — yet a single loss left the claim behind: the old name stayed refused to creates and the namespace counted the record as a child. Retry the release before warning. * s3tables: page the namespace scan before deleting it A page of skippable entries — bare directories or unfinished creates — ended the scan with hasChildren false, so a table on a later page was removed with the namespace. Keep listing with StartFromFileName until a child shows or the listing ends. --------- Co-authored-by: chrislusf <chrislusf@users.noreply.github.com>	2026-10-30 03:18:52 -07:00
Chris LuandDevin	e7923c14d1	s3tables: honor resource policy Deny on remaining operations (#11825) ... * s3tables: honor a table policy Deny for maintenance operations authorizeMaintenanceTable evaluated only the bucket policy, so a Deny in the table policy never vetoed PutTableMaintenanceConfiguration, GetTableMaintenanceConfiguration or GetTableMaintenanceJobStatus even though they act on the table ARN. The stored table policy is now checked for a matching Deny first via deniedByPolicy, a veto helper that keeps the admin exemption, and resource and bucket tags are loaded into the policy context so tag-conditioned Denies evaluate. readBucketPolicy is renamed to readPolicy and reused for the table path. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3tables: honor the stored policy's Deny on policy management operations PutTablePolicy, GetTablePolicy and DeleteTablePolicy only evaluated the bucket policy, so a Deny in the table's own policy was ignored and a bucket-level Allow could strip or replace the policy guarding a table. The existing table policy is now loaded and a matching Deny vetoes the call. PutTableBucketPolicy likewise consults the existing bucket policy for a Deny before replacing it. Resource and bucket tags populate the policy context so tag-conditioned Denies evaluate, and policy writes assert the policy authorized against is still in force, returning a conflict when it changed mid-request. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3tables: apply a table policy Deny to tag operations TagResource, ListTagsForResource and UntagResource evaluated only the bucket policy; a Deny in the table's own policy is now honored when the target ARN is a table. ListTagsForResource and UntagResource also returned early when the resource carried no tags attribute, skipping the permission check entirely; the check now always runs. Policy evaluation uses the canonical resource ARN rebuilt from stored metadata, so a request ARN with a forged account or region cannot dodge the Deny. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3tables: regression tests for resource policy denies Cover the case where a bucket policy grants an identity s3tables:* while the table policy denies it: the policy, tag and maintenance operations must stay denied so the protection cannot be stripped. Also cover a bucket policy that denies PutTableBucketPolicy under the open fallback, and a tag-conditioned Deny, which vetoes only while the stored tag matches. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>	2026-10-19 03:59:18 -07:00
Chris Lu	8c55425d55	s3tables: tighten legacy OIDC subject re-anchoring (#11808) ... * sts: recognize issuer-scoped parent user ids ComputeParentUser output has a fixed shape (base64url SHA-256); let callers distinguish it from bare subject strings. * s3tables: keep legacy owner re-anchoring off local and scoped account ids Re-anchoring a stored bare subject under the single trusted issuer must only apply to strings that could actually be a legacy OIDC subject. A stored owner naming the reserved or a configured local account, or an already issuer-scoped id, can otherwise be claimed by any caller whose token asserts that string as sub. * s3tables: re-anchor legacy policy principals in map and array forms A stored bare subject can appear as {"AWS": sub} or inside a principal list, not only as a top-level string; re-anchoring only the string form left legacy Deny statements unable to match the caller. * s3tables: keep built-in legacy exclusions independent of the account checker A missing local-account predicate must not disable re-anchoring entirely: the reserved accounts, ARN-shaped values, and scoped-id shapes are never OIDC subjects regardless, while legacy Deny statements for plain subjects still need to resolve so a denied caller cannot fall through to an identity grant. * s3tables: exercise legacy Deny against an owner grant The previous case denied the caller regardless of whether the statement principal matched; make the caller the owner so a missed match shows. Cover the array Deny form alongside the AWS map.	2026-10-18 02:43:03 -07:00
Chris LuandDevin	446e6f3444	s3tables: scope OIDC-derived account IDs to the token issuer (#11774) ... * s3tables: scope OIDC-derived account IDs to the token issuer Table ownership keyed off the bare sub claim, so two identity providers issuing the same sub collided on one account - a caller could act on tables owned by a different provider's user. OIDC bearer tokens now carry the same issuer-namespaced (sub, iss) hash STS sessions already expose as aws:userid, and getAccountID prefers it over the raw claims. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3tables: keep legacy owners working and scrub client-subject claims - OIDC bearer tokens without a subject are rejected, and a token-supplied aws:userid claim is always overwritten with the verified (sub, iss) hash, so a forged claim can no longer stand in for the principal. - Owners written before issuer scoping hold a bare subject; permission checks now re-anchor that subject under the caller's issuer, so the same (sub, iss) pair keeps its ownership while scoped owners remain issuer-isolated. The token issuer is asserted into claims alongside sub and carried through PolicyContext. * s3tables: anchor legacy subjects to the configured issuer set * s3: enumerate trusted issuers from registered providers too * s3: fail closed when the OIDC provider store cannot be listed trustedOIDCIssuers kept the registered issuers when ListOIDCProviders failed, presenting a partial set as complete. A deployment trusting one registered issuer A plus a stored issuer B would, on a listing error, report only A — the single-issuer shape under which a legacy bare subject re-anchors — letting a caller from A claim a stored subject that belongs to B and gain owner access. With no provider store the registered providers are provably complete, so keep returning them; with a store, a failed listing means the real set is unknown, so return none. --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>	2026-10-16 19:48:32 -07:00
Chris Lu	eef6f3d1e6	s3tables: add the maintenance configuration APIs (#10773) ... * s3tables: add the maintenance configuration APIs Stores the configuration verbatim as the wire shape under a new s3tables.maintenance extended attribute, so Get hands back what Put took and no translation layer can drift from the AWS model. Nothing reads the configuration yet. Put merges a single type into the stored map so configuring compaction does not drop snapshot management, and asserts the attribute's prior value so two concurrent Puts cannot silently clobber each other. * iceberg: apply the maintenance configuration in the worker The worker now reads the per-table and per-bucket maintenance configuration written by the control plane, so the wildcard plugin config is a default rather than the only setting a table can have. Table properties still win by default, since a table declaring its own layout is what every engine honours and the compactor has to agree with whoever writes the files. Clearing table_properties_override makes the maintenance configuration authoritative instead. Status is not part of that contest: a disabled type drops its operations and no property can re-enable them, so the operator's kill switch always holds. Manifest and delete-file rewrites have no AWS equivalent and ride with compaction. Detection reads both attributes from entries it already lists. * s3tables: report maintenance job status The worker records the outcome of each run in its own extended attribute, separate from the configuration so operator and worker writes do not contend, and GetTableMaintenanceJobStatus reads it back. Only the types a run touched are written, so a partial run cannot erase what an earlier one recorded. The reader fills in the rest: Disabled when the configuration switched a type off, Not_Yet_Run otherwise. Status is advisory, so a lost race is logged rather than failing a job whose work already committed. * s3tables: route the maintenance APIs over REST The five actions were only reachable by X-Amz-Target dispatch, which the AWS CLI and SDK do not use for this service. They address the operations by path, so the APIs were unreachable from any official client. * s3tables: fix the table bucket ARN field name GetTableBucketMaintenanceConfiguration emitted tableBucketArn where the wire field is tableBucketARN, as every other response in this package already spells it. Official SDK deserializers ignore the unknown key, so the required field came back unset. * s3tables: carry the compaction strategy through to the worker IcebergCompactionSettings modelled only targetFileSizeMB, so a request naming a strategy was accepted and then dropped on the way to storage. The worker now maps binpack and sort onto its own rewrite strategy and lets auto defer to the worker configuration. z-order is rejected rather than accepted and quietly binpacked. * s3tables: report bucket-level maintenance status GetTableMaintenanceJobStatus read only the table's configuration, so unreferenced file removal — which is configured on the bucket — reported Not_Yet_Run or a stale success after an operator disabled it. The merge helper now lives in this package and the worker shares it. * iceberg: delete orphans only after the non-current window AWS marks a file non-current once it has been unreferenced for unreferencedDays, then deletes it a further nonCurrentDays later. The cutoff was taken from unreferencedDays alone, so a 3/10 configuration hard-deleted on day three and threw away the ten day recovery window. remove_orphans deletes in one step rather than marking, so the cutoff is now the sum of the two. * s3tables: assert every attribute when rewriting an entry UpdateEntry writes the whole entry back from the snapshot the caller read, and its precondition only covers the keys the caller names. Both maintenance writers named one key, so a job status write could revert a maintenance configuration an operator had just disabled, turning an advisory write into a silent re-enable. Both now assert the entry's full attribute set, including the target key when absent so a concurrent create also fails the precondition. * s3tables: assert absent attributes when rewriting an entry The precondition covered the attributes present when the writer read the entry, so an attribute created between that read and the write was absent from it. A first-time PutTableMaintenanceConfiguration disabling a type therefore lands, passes the per-key checks, and is then deleted by the stale whole-entry write. Every attribute this package stores is now asserted, absent ones included. The metadata commit and planning index writers rewrite the same entries and had the same exposure, so both use the shared snapshot too. * iceberg: implement the auto compaction strategy auto was accepted, stored and read back, but left the worker on its own default, so a sorted table configured as auto was compacted with binpack. AWS defines auto as sorting tables that declare a sort order and bin-packing the rest. That needs the table metadata, so the choice is made where the rewrite plan is resolved: an unsorted table falls back to binpack rather than failing the way an explicit sort request does. * s3tables: validate the maintenance setting ranges PUT accepted zero, negative and oversized values for every numeric setting. The worker then ignores a non-positive value and saturates an oversized one, so the configuration read back was not the one that ran. AWS bounds all five to 1..2147483647, which is now enforced. The fields are pointers so an explicit zero is distinguishable from an omitted one and can be rejected rather than silently ignored. * s3tables: give every entry writer the same compare-and-swap updateExtendedAttribute asserted the entry's attributes, but the helpers behind the metadata, policy and tag handlers still wrote the whole entry unconditionally. Any of them could land on a stale snapshot and delete a maintenance configuration an operator had just written. They all share one read-modify-write loop now, so the precondition and the bounded retry apply wherever an entry is rewritten. * s3tables: move the maintenance configuration with a renamed table RenameTable carried the metadata, version, policy and tags to the new name but left the maintenance configuration and job status behind. A table with snapshot management disabled came back enabled under its new name, and the stale configuration stayed on the old name where a table created there would inherit it. The decoupled-delete cleanup left the same two attributes behind. * s3tables: accept every AWS partition in ARNs The route regexes and the ARN patterns both hardcoded arn:aws, so valid aws-cn and aws-us-gov ARNs never reached a handler. The router now shares the partition-tolerant prefix with the parser, and a generated ARN uses the partition its region belongs to so it parses back. * s3tables: generate ARNs in the region's partition The handler's own ARN generators still formatted arn:aws directly rather than going through the partition-aware builder, so a China or GovCloud deployment routed the request but then returned a commercial ARN and matched IAM policies against it. The round-trip test missed this because parsing accepts any partition, so it now asserts the prefix the region implies. * s3tables: complete the ARN partition table aws-iso-e, aws-iso-f and aws-eusc were missing, so eu-isoe-*, us-isof-* and eusc-* regions fell through to the commercial partition. * s3tables: do not let a rename swallow a concurrent maintenance write Rename copied the source attributes early and cleared the source at the end, so a Put landing in between missed the copy to the destination and was then deleted by the cleanup. It succeeded and vanished. The cleanup now clears the source only while it still holds exactly what was copied, and returns a conflict otherwise. Put checks the catalog identity inside the same conditional mutation, so it also cannot write to a name that a rename or delete has already soft-deleted.	2026-08-16 10:36:59 -07:00