dimas-b commented on code in PR #3750: URL: https://github.com/apache/polaris/pull/3750#discussion_r3028936632
########## runtime/service/src/main/java/org/apache/polaris/service/catalog/AccessDelegationModeResolver.java: ########## @@ -0,0 +1,49 @@ +/* + * 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.service.catalog; + +import jakarta.annotation.Nonnull; +import jakarta.annotation.Nullable; +import java.util.EnumSet; +import java.util.Optional; +import org.apache.polaris.core.entity.CatalogEntity; + +/** + * Resolves the optimal {@link AccessDelegationMode} to use based on the client's requested modes + * and the catalog's capabilities. + */ +public interface AccessDelegationModeResolver { + + /** + * Resolves the optimal access delegation mode based on the requested modes and catalog + * capabilities. + * + * @param requestedModes The set of delegation modes requested by the client + * @param catalogEntity The catalog entity, used to determine storage configuration and + * capabilities + * @return The resolved access delegation mode, or empty if no suitable mode is available. + * @throws org.apache.iceberg.exceptions.ForbiddenException if the client requests a mode that is Review Comment: I'd prefer Polaris SPIs to not rely on Iceberg exceptions (narrower scope, easier maintenance). Also, Iceberg's `ForbiddenException` has javadoc like `HTTP 403 Forbidden - Failed authorization checks.`, which does not appear to fit this use case. Polaris Authorizers are not involved at all. I'd think a 400 response (Bad Request) is more appropriate because the caller ought to know the nature of the catalog for which it requests access delegation. That is to say, asking for credential vending from a federated catalog is not an authorization failure but a setup/configuration mistake. For example, `buildLoadTableResponseWithDelegationCredentials()` throws an `IllegalArgumentException`, when credential vending has been requested, but cannot be performed. I'd think we should throw an `IllegalArgumentException` in this case too. That said, do we have to declare a specific exception type here at all? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
