Repository navigation
[#12566] feat(core): Dispatch events for catalog connection tests #12585
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
7cd4a06
fix: feat: dispatch events for catalog connection tests
VedantMadane 27b68ce
fix(core): sort TestConnection imports for spotlessJavaCheck
VedantMadane 0f5282e
fix(core): complete testConnection event lifecycle and audit mapping
VedantMadane 8cf927a
chore: retrigger CI
VedantMadane 9e5fd72
[#12566] fix(core): Protect connection test events and complete coverage
mchades e9e6c26
[#12566] fix(core): Align connection test events with catalog convent…
mchades 21f7714
[#12566] fix(server): Return 403 for connection test vetoes
mchades f54d3be
Merge branch 'main' into fix/issue-12566
mchades 7273238
Merge branch 'main' into fix/issue-12566
yuqi1129 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
91 changes: 91 additions & 0 deletions
91
core/src/main/java/org/apache/gravitino/listener/api/event/TestConnectionEvent.java
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one | ||
| * or more contributor license agreements. See the NOTICE file | ||
| * distributed with this work for additional information | ||
| * regarding copyright ownership. The ASF licenses this file | ||
| * to you under the Apache License, Version 2.0 (the | ||
| * "License"); you may not use this file except in compliance | ||
| * with the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, | ||
| * software distributed under the License is distributed on an | ||
| * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| * KIND, either express or implied. See the License for the | ||
| * specific language governing permissions and limitations | ||
| * under the License. | ||
| */ | ||
|
|
||
| package org.apache.gravitino.listener.api.event; | ||
|
|
||
| import javax.annotation.Nullable; | ||
| import org.apache.gravitino.CatalogChange; | ||
| import org.apache.gravitino.NameIdentifier; | ||
| import org.apache.gravitino.annotation.DeveloperApi; | ||
| import org.apache.gravitino.listener.api.info.CatalogInfo; | ||
|
|
||
| /** | ||
| * Represents an event that is triggered upon a successful catalog connection test. | ||
| * | ||
| * <p>Request parameters are available without redaction, consistent with other catalog events. | ||
| * Listeners are responsible for redacting sensitive values before forwarding or logging them. | ||
| */ | ||
| @DeveloperApi | ||
| public final class TestConnectionEvent extends CatalogEvent { | ||
| @Nullable private final CatalogInfo catalogInfo; | ||
| @Nullable private final CatalogChange[] catalogChanges; | ||
|
|
||
| /** | ||
| * Constructs an instance of {@code TestConnectionEvent}. | ||
| * | ||
| * @param user The username of the individual who initiated the connection test. | ||
| * @param identifier The identifier of the catalog whose connection was tested. | ||
| */ | ||
| public TestConnectionEvent(String user, NameIdentifier identifier) { | ||
| this(user, identifier, null, null); | ||
| } | ||
|
|
||
| /** | ||
| * Constructs a success event with the parameters supplied for the connection test. | ||
| * | ||
| * @param user The username of the individual who initiated the connection test. | ||
| * @param identifier The identifier of the catalog whose connection was tested. | ||
| * @param catalogInfo The parameters of a pre-create test, or null for an existing catalog. | ||
| * @param catalogChanges The proposed changes, or null when no changes were supplied. | ||
| */ | ||
| public TestConnectionEvent( | ||
| String user, | ||
| NameIdentifier identifier, | ||
| @Nullable CatalogInfo catalogInfo, | ||
| @Nullable CatalogChange[] catalogChanges) { | ||
| super(user, identifier); | ||
| this.catalogInfo = catalogInfo; | ||
| this.catalogChanges = catalogChanges == null ? null : catalogChanges.clone(); | ||
| } | ||
|
|
||
| /** | ||
| * Returns the catalog parameters supplied for a pre-create connection test. | ||
| * | ||
| * @return The catalog parameters, or null for an existing-catalog test. | ||
| */ | ||
| @Nullable | ||
| public CatalogInfo catalogInfo() { | ||
| return catalogInfo; | ||
| } | ||
|
|
||
| /** | ||
| * Returns the proposed changes supplied for an existing-catalog connection test. | ||
| * | ||
| * @return The proposed changes, or null when no changes were supplied. | ||
| */ | ||
| @Nullable | ||
| public CatalogChange[] catalogChanges() { | ||
| return catalogChanges; | ||
| } | ||
|
|
||
| @Override | ||
| public OperationType operationType() { | ||
| return OperationType.TEST_CONNECTION_CATALOG; | ||
| } | ||
| } |
95 changes: 95 additions & 0 deletions
95
core/src/main/java/org/apache/gravitino/listener/api/event/TestConnectionFailureEvent.java
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,95 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one | ||
| * or more contributor license agreements. See the NOTICE file | ||
| * distributed with this work for additional information | ||
| * regarding copyright ownership. The ASF licenses this file | ||
| * to you under the Apache License, Version 2.0 (the | ||
| * "License"); you may not use this file except in compliance | ||
| * with the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, | ||
| * software distributed under the License is distributed on an | ||
| * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| * KIND, either express or implied. See the License for the | ||
| * specific language governing permissions and limitations | ||
| * under the License. | ||
| */ | ||
|
|
||
| package org.apache.gravitino.listener.api.event; | ||
|
|
||
| import javax.annotation.Nullable; | ||
| import org.apache.gravitino.CatalogChange; | ||
| import org.apache.gravitino.NameIdentifier; | ||
| import org.apache.gravitino.annotation.DeveloperApi; | ||
| import org.apache.gravitino.listener.api.info.CatalogInfo; | ||
|
|
||
| /** | ||
| * Represents an event that is triggered when a catalog connection test fails. | ||
| * | ||
| * <p>Request parameters and the original exception are available without redaction, consistent with | ||
| * other catalog events. Listeners are responsible for redacting sensitive values before forwarding | ||
| * or logging them. | ||
| */ | ||
| @DeveloperApi | ||
| public final class TestConnectionFailureEvent extends CatalogFailureEvent { | ||
| @Nullable private final CatalogInfo catalogInfo; | ||
| @Nullable private final CatalogChange[] catalogChanges; | ||
|
|
||
| /** | ||
| * Constructs an instance of {@code TestConnectionFailureEvent}. | ||
| * | ||
| * @param user The username of the individual who initiated the connection test. | ||
| * @param identifier The identifier of the catalog whose connection test failed. | ||
| * @param exception The original exception encountered during the connection test. | ||
| */ | ||
| public TestConnectionFailureEvent(String user, NameIdentifier identifier, Exception exception) { | ||
| this(user, identifier, exception, null, null); | ||
| } | ||
|
|
||
| /** | ||
| * Constructs a failure event with the parameters supplied for the connection test. | ||
| * | ||
| * @param user The username of the individual who initiated the connection test. | ||
| * @param identifier The identifier of the catalog whose connection test failed. | ||
| * @param exception The original exception encountered during the connection test. | ||
| * @param catalogInfo The parameters of a pre-create test, or null for an existing catalog. | ||
| * @param catalogChanges The proposed changes, or null when no changes were supplied. | ||
| */ | ||
| public TestConnectionFailureEvent( | ||
| String user, | ||
| NameIdentifier identifier, | ||
| Exception exception, | ||
| @Nullable CatalogInfo catalogInfo, | ||
| @Nullable CatalogChange[] catalogChanges) { | ||
| super(user, identifier, exception); | ||
| this.catalogInfo = catalogInfo; | ||
| this.catalogChanges = catalogChanges == null ? null : catalogChanges.clone(); | ||
| } | ||
|
|
||
| /** | ||
| * Returns the catalog parameters supplied for a pre-create connection test. | ||
| * | ||
| * @return The catalog parameters, or null for an existing-catalog test. | ||
| */ | ||
| @Nullable | ||
| public CatalogInfo catalogInfo() { | ||
| return catalogInfo; | ||
| } | ||
|
|
||
| /** | ||
| * Returns the proposed changes supplied for an existing-catalog connection test. | ||
| * | ||
| * @return The proposed changes, or null when no changes were supplied. | ||
| */ | ||
| @Nullable | ||
| public CatalogChange[] catalogChanges() { | ||
| return catalogChanges; | ||
| } | ||
|
|
||
| @Override | ||
| public OperationType operationType() { | ||
| return OperationType.TEST_CONNECTION_CATALOG; | ||
| } | ||
| } |
91 changes: 91 additions & 0 deletions
91
core/src/main/java/org/apache/gravitino/listener/api/event/TestConnectionPreEvent.java
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one | ||
| * or more contributor license agreements. See the NOTICE file | ||
| * distributed with this work for additional information | ||
| * regarding copyright ownership. The ASF licenses this file | ||
| * to you under the Apache License, Version 2.0 (the | ||
| * "License"); you may not use this file except in compliance | ||
| * with the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, | ||
| * software distributed under the License is distributed on an | ||
| * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| * KIND, either express or implied. See the License for the | ||
| * specific language governing permissions and limitations | ||
| * under the License. | ||
| */ | ||
|
|
||
| package org.apache.gravitino.listener.api.event; | ||
|
|
||
| import javax.annotation.Nullable; | ||
| import org.apache.gravitino.CatalogChange; | ||
| import org.apache.gravitino.NameIdentifier; | ||
| import org.apache.gravitino.annotation.DeveloperApi; | ||
| import org.apache.gravitino.listener.api.info.CatalogInfo; | ||
|
|
||
| /** | ||
| * Represents an event that is generated before a catalog connection test. | ||
| * | ||
| * <p>Request parameters are available without redaction, consistent with other catalog events. | ||
| * Listeners are responsible for redacting sensitive values before forwarding or logging them. | ||
| */ | ||
| @DeveloperApi | ||
| public final class TestConnectionPreEvent extends CatalogPreEvent { | ||
| @Nullable private final CatalogInfo catalogInfo; | ||
| @Nullable private final CatalogChange[] catalogChanges; | ||
|
|
||
| /** | ||
| * Constructs an instance of {@code TestConnectionPreEvent}. | ||
| * | ||
| * @param user The username of the individual who initiated the connection test. | ||
| * @param identifier The identifier of the catalog whose connection is being tested. | ||
| */ | ||
| public TestConnectionPreEvent(String user, NameIdentifier identifier) { | ||
| this(user, identifier, null, null); | ||
| } | ||
|
|
||
| /** | ||
| * Constructs a pre-event with the parameters supplied for the connection test. | ||
| * | ||
| * @param user The username of the individual who initiated the connection test. | ||
| * @param identifier The identifier of the catalog whose connection is being tested. | ||
| * @param catalogInfo The parameters of a pre-create test, or null for an existing catalog. | ||
| * @param catalogChanges The proposed changes, or null when no changes were supplied. | ||
| */ | ||
| public TestConnectionPreEvent( | ||
| String user, | ||
| NameIdentifier identifier, | ||
| @Nullable CatalogInfo catalogInfo, | ||
| @Nullable CatalogChange[] catalogChanges) { | ||
| super(user, identifier); | ||
| this.catalogInfo = catalogInfo; | ||
| this.catalogChanges = catalogChanges == null ? null : catalogChanges.clone(); | ||
| } | ||
|
|
||
| /** | ||
| * Returns the catalog parameters supplied for a pre-create connection test. | ||
| * | ||
| * @return The catalog parameters, or null for an existing-catalog test. | ||
| */ | ||
| @Nullable | ||
| public CatalogInfo catalogInfo() { | ||
| return catalogInfo; | ||
| } | ||
|
|
||
| /** | ||
| * Returns the proposed changes supplied for an existing-catalog connection test. | ||
| * | ||
| * @return The proposed changes, or null when no changes were supplied. | ||
| */ | ||
| @Nullable | ||
| public CatalogChange[] catalogChanges() { | ||
| return catalogChanges; | ||
| } | ||
|
|
||
| @Override | ||
| public OperationType operationType() { | ||
| return OperationType.TEST_CONNECTION_CATALOG; | ||
| } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pre-event veto works in
CatalogEventDispatcher, but both test-connection REST methods (CatalogOperations.java:226and:273) pass the thrownForbiddenExceptiontohandleTestConnectionException(ExceptionHandlers.java:245), which has no forbidden branch and returns HTTP 500 viaUtils.internalError. Other catalog handlers map this toUtils.forbidden(403). Please handleForbiddenExceptionthere and add a REST test for both endpoints so a synchronous pre-listener veto stays a 403 and does not run the connection test.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mapped
ForbiddenExceptionto HTTP 403 in the shared handler. Added REST tests for both endpoints, covering stored configuration, proposed changes, and empty changes. They verify that a synchronous pre-listener veto never reaches the delegate and preserves the existing stack-trace behavior.