Skip to content

Validate table-notification location before auto-creating parent namespaces - #5541

Open
iprithv wants to merge 1 commit into
apache:mainfrom
iprithv:fix/notification-validate-location-before-namespace-create
Open

iprithv wants to merge 1 commit into
apache:mainfrom
iprithv:fix/notification-validate-location-before-namespace-create

Conversation

@iprithv

@iprithv iprithv commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

CREATE/UPDATE table notifications call createNonExistingNamespaces to auto-create any missing parent namespaces before validating the proposed metadata location. A notification whose location is outside the catalog's allowed locations therefore persisted the new namespaces first and only then failed validation, leaving them orphaned. It also let a principal holding only SEND_NOTIFICATIONS create namespaces as a side effect of a request that ultimately fails.

This validates the location before creating namespaces. Because the location check needs the storage configuration of the parent hierarchy — which may not exist yet — it resolves the deepest already-existing ancestor (down to the catalog root) and validates against it, mirroring what the VALIDATE notification path already does. Auto-created namespaces inherit their storage configuration from that same ancestor, so behavior is unchanged for valid notifications. The shared walk-up logic is extracted into resolveDeepestExistingStorageEntity and reused by both paths; the now-unused 2-arg validateLocationForTableLike overload is removed.

…spaces

CREATE/UPDATE table notifications auto-create any missing parent
namespaces via createNonExistingNamespaces before validating the
proposed metadata location. A notification carrying a location outside
the catalog's allowed locations therefore persisted the new namespaces
first and only then failed validation, leaving them orphaned. It also
let a principal holding only SEND_NOTIFICATIONS create namespaces as a
side effect of a request that ultimately fails.

Validate the location before creating namespaces. Because the location
check needs the storage configuration of the parent hierarchy - which
may not exist yet - resolve the deepest already-existing ancestor
(down to the catalog root) and validate against it, mirroring what the
VALIDATE notification path already does. Auto-created namespaces inherit
their storage configuration from that same ancestor, so the decision is
unchanged for valid notifications. The shared walk-up logic is extracted
into resolveDeepestExistingStorageEntity and reused by both paths, and
the now-unused two-argument validateLocationForTableLike overload is
removed.

Adds a regression test asserting that a notification with a disallowed
location creates no namespaces.

@vigneshio vigneshio left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.. Thanks @iprithv

@dimas-b dimas-b left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 Thanks for the fix, @iprithv !

Note: there's a merge conflict.

@github-project-automation github-project-automation Bot moved this from PRs In Progress to Ready to merge in Basic Kanban Board Sep 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants