-
Notifications
You must be signed in to change notification settings - Fork 330
Initial MVP implementation of Catalog Federation to remote Iceberg REST Catalogs #1305
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
eric-maynard
merged 20 commits into
apache:main
from
dennishuo:dhuo-catalog-federation-mvp-clean
Apr 18, 2025
Merged
Changes from all commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
16d33fd
Initial prototype of catalog federation just passing special properti…
dennishuo 3b1aa16
Defined internal representation classes for connection config
XJDKC 1d1650b
Construct and initialize federated iceberg catalog based on connectio…
XJDKC 205ef1e
Apply the same spec renames to the internal ConnectionConfiguration r…
dennishuo c8484e0
Manually pick @XJDKC fixes for integration tests and omittign secrets…
dennishuo acb5bec
Fix internal connection structs with updated naming from spec PR
dennishuo 300553f
Push CreateCatalogRequest down to PolarisAdminService::createCatalog …
dennishuo 4ad235e
Add new interface UserSecretsManager along with a default implementation
dennishuo cc17ae5
Wire in UserSecretsManager to createCatalog and federated Iceberg API…
dennishuo 6700fb2
Since we already use commons-codec DigestUtils.sha256Hex, use that fo…
dennishuo 10b2966
Rename the persistence-objects corresponding to API model objects wit…
dennishuo fd7ae59
Use UserSecretsManagerFactory to Produce the UserSecretsManager (#1)
XJDKC cbe362d
Gate all federation logic behind a new FeatureConfiguration - ENABLE_…
dennishuo 3e85244
Also rename some variables and method names to be consistent with pri…
dennishuo 9be6ee3
Merge branch 'main' of github.com:dennishuo/polaris into dhuo-catalog…
dennishuo 7d5c9e1
Merge branch 'main' of github.com:dennishuo/polaris into dhuo-catalog…
dennishuo 31a7333
Merge branch 'main' of github.com:dennishuo/polaris into dhuo-catalog…
dennishuo 03af9f5
Change ConnectionType and AuthenticationType to be stored as int code…
dennishuo 544ce82
Add javadoc comment to IcebergCatalogPropertiesProvider
dennishuo beaa34e
Add some constraints on the expected format of the URN in UserSecretR…
dennishuo 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
88 changes: 88 additions & 0 deletions
88
...is-core/src/main/java/org/apache/polaris/core/connection/AuthenticationParametersDpo.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,88 @@ | ||
| /* | ||
| * 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.polaris.core.connection; | ||
|
|
||
| import com.fasterxml.jackson.annotation.JsonProperty; | ||
| import com.fasterxml.jackson.annotation.JsonSubTypes; | ||
| import com.fasterxml.jackson.annotation.JsonTypeInfo; | ||
| import java.util.Map; | ||
| import org.apache.polaris.core.admin.model.AuthenticationParameters; | ||
| import org.apache.polaris.core.admin.model.BearerAuthenticationParameters; | ||
| import org.apache.polaris.core.admin.model.OAuthClientCredentialsParameters; | ||
| import org.apache.polaris.core.secrets.UserSecretReference; | ||
|
|
||
| /** | ||
| * The internal persistence-object counterpart to AuthenticationParameters defined in the API model. | ||
| * Important: JsonSubTypes must be kept in sync with {@link AuthenticationType}. | ||
| */ | ||
| @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "authenticationTypeCode", visible = true) | ||
| @JsonSubTypes({ | ||
| @JsonSubTypes.Type(value = OAuthClientCredentialsParametersDpo.class, name = "1"), | ||
| @JsonSubTypes.Type(value = BearerAuthenticationParametersDpo.class, name = "2"), | ||
| }) | ||
| public abstract class AuthenticationParametersDpo implements IcebergCatalogPropertiesProvider { | ||
|
|
||
| public static final String INLINE_CLIENT_SECRET_REFERENCE_KEY = "inlineClientSecretReference"; | ||
| public static final String INLINE_BEARER_TOKEN_REFERENCE_KEY = "inlineBearerTokenReference"; | ||
|
|
||
| @JsonProperty(value = "authenticationTypeCode") | ||
| private final int authenticationTypeCode; | ||
|
|
||
| public AuthenticationParametersDpo( | ||
| @JsonProperty(value = "authenticationTypeCode", required = true) int authenticationTypeCode) { | ||
| this.authenticationTypeCode = authenticationTypeCode; | ||
| } | ||
|
|
||
| public int getAuthenticationTypeCode() { | ||
| return authenticationTypeCode; | ||
| } | ||
|
|
||
| public abstract AuthenticationParameters asAuthenticationParametersModel(); | ||
|
|
||
| public static AuthenticationParametersDpo fromAuthenticationParametersModelWithSecrets( | ||
| AuthenticationParameters authenticationParameters, | ||
| Map<String, UserSecretReference> secretReferences) { | ||
| final AuthenticationParametersDpo config; | ||
| switch (authenticationParameters.getAuthenticationType()) { | ||
| case OAUTH: | ||
| OAuthClientCredentialsParameters oauthClientCredentialsModel = | ||
| (OAuthClientCredentialsParameters) authenticationParameters; | ||
| config = | ||
| new OAuthClientCredentialsParametersDpo( | ||
| AuthenticationType.OAUTH.getCode(), | ||
| oauthClientCredentialsModel.getTokenUri(), | ||
| oauthClientCredentialsModel.getClientId(), | ||
| secretReferences.get(INLINE_CLIENT_SECRET_REFERENCE_KEY), | ||
| oauthClientCredentialsModel.getScopes()); | ||
| break; | ||
| case BEARER: | ||
| BearerAuthenticationParameters bearerAuthenticationParametersModel = | ||
| (BearerAuthenticationParameters) authenticationParameters; | ||
| config = | ||
| new BearerAuthenticationParametersDpo( | ||
| AuthenticationType.BEARER.getCode(), | ||
| secretReferences.get(INLINE_BEARER_TOKEN_REFERENCE_KEY)); | ||
| break; | ||
| default: | ||
| throw new IllegalStateException( | ||
| "Unsupported authentication type: " + authenticationParameters.getAuthenticationType()); | ||
| } | ||
| return config; | ||
| } | ||
| } |
42 changes: 42 additions & 0 deletions
42
polaris-core/src/main/java/org/apache/polaris/core/connection/AuthenticationType.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,42 @@ | ||
| /* | ||
| * 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.polaris.core.connection; | ||
|
|
||
| /** | ||
| * The internal persistence-object counterpart to AuthenticationParameters.AuthenticationTypeEnum | ||
| * defined in the API model. We define integer type codes in this enum for better compatibility | ||
| * within persisted data in case the names of enum types are ever changed in place. | ||
| * | ||
| * <p>Important: Codes must be kept in-sync with JsonSubTypes annotated within {@link | ||
| * AuthenticationParametersDpo}. | ||
| */ | ||
| public enum AuthenticationType { | ||
| OAUTH(1), | ||
| BEARER(2); | ||
|
|
||
| private final int code; | ||
|
|
||
| AuthenticationType(int code) { | ||
| this.code = code; | ||
| } | ||
|
|
||
| public int getCode() { | ||
| return this.code; | ||
| } | ||
| } | ||
73 changes: 73 additions & 0 deletions
73
...e/src/main/java/org/apache/polaris/core/connection/BearerAuthenticationParametersDpo.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,73 @@ | ||
| /* | ||
| * 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.polaris.core.connection; | ||
|
|
||
| import com.fasterxml.jackson.annotation.JsonProperty; | ||
| import com.google.common.base.MoreObjects; | ||
| import jakarta.annotation.Nonnull; | ||
| import java.util.Map; | ||
| import org.apache.iceberg.rest.auth.OAuth2Properties; | ||
| import org.apache.polaris.core.admin.model.AuthenticationParameters; | ||
| import org.apache.polaris.core.admin.model.BearerAuthenticationParameters; | ||
| import org.apache.polaris.core.secrets.UserSecretReference; | ||
| import org.apache.polaris.core.secrets.UserSecretsManager; | ||
|
|
||
| /** | ||
| * The internal persistence-object counterpart to BearerAuthenticationParameters defined in the API | ||
| * model. | ||
| */ | ||
| public class BearerAuthenticationParametersDpo extends AuthenticationParametersDpo { | ||
|
|
||
| @JsonProperty(value = "bearerTokenReference") | ||
| private final UserSecretReference bearerTokenReference; | ||
|
|
||
| public BearerAuthenticationParametersDpo( | ||
| @JsonProperty(value = "authenticationTypeCode", required = true) int authenticationTypeCode, | ||
| @JsonProperty(value = "bearerTokenReference", required = true) @Nonnull | ||
| UserSecretReference bearerTokenReference) { | ||
| super(authenticationTypeCode); | ||
| this.bearerTokenReference = bearerTokenReference; | ||
| } | ||
|
|
||
| public @Nonnull UserSecretReference getBearerTokenReference() { | ||
| return bearerTokenReference; | ||
| } | ||
|
|
||
| @Override | ||
| public @Nonnull Map<String, String> asIcebergCatalogProperties( | ||
| UserSecretsManager secretsManager) { | ||
| String bearerToken = secretsManager.readSecret(getBearerTokenReference()); | ||
| return Map.of(OAuth2Properties.TOKEN, bearerToken); | ||
| } | ||
|
|
||
| @Override | ||
| public AuthenticationParameters asAuthenticationParametersModel() { | ||
| return BearerAuthenticationParameters.builder() | ||
| .setAuthenticationType(AuthenticationParameters.AuthenticationTypeEnum.BEARER) | ||
| .build(); | ||
| } | ||
|
|
||
| @Override | ||
| public String toString() { | ||
| return MoreObjects.toStringHelper(this) | ||
| .add("authenticationTypeCode", getAuthenticationTypeCode()) | ||
| .add("bearerTokenReference", getBearerTokenReference()) | ||
| .toString(); | ||
| } | ||
| } |
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.
For the sake of wire/serialized compatibility perhaps we should specify the int values here like we do for some other enums
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.
Done. Note that in existing places where we use
intcodes, we explicitly store the codes first-class in the entity model's top-level fields, whereas here we're using the enum to match the pattern ofJsonSubTypehierarchy, so we also depend on annotation magic. This means the update to support the int values here also meansAuthenticationParametersDpowill have kind of ugly annotations that need to be manually kept in sync:Of course, we already had to manually keep it in sync before, it just happened to be with strings that were a little more accessible (
OAUTH, BEARER) since they matched the primary enum name.In any case, I agree int codes will be more future-proof for our persisted data, even if it means it'll be a bit harder to debug at a glance.