Copilot commented on code in PR #54523: URL: https://github.com/apache/doris/pull/54523#discussion_r2265113404
########## fe/fe-core/src/main/java/org/apache/doris/datasource/property/metastore/PaimonRestMetaStoreProperties.java: ########## @@ -0,0 +1,111 @@ +// 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.doris.datasource.property.metastore; + +import org.apache.doris.datasource.paimon.PaimonExternalCatalog; +import org.apache.doris.datasource.property.ConnectorProperty; +import org.apache.doris.datasource.property.ParamRules; +import org.apache.doris.datasource.property.storage.StorageProperties; + +import lombok.Getter; +import org.apache.paimon.catalog.Catalog; +import org.apache.paimon.catalog.CatalogContext; +import org.apache.paimon.catalog.CatalogFactory; + +import java.util.List; +import java.util.Map; + +public class PaimonRestMetaStoreProperties extends AbstractPaimonProperties { + + private static final String PAIMON_REST_PROPERTY_PREFIX = "paimon.rest."; + + @ConnectorProperty(names = {"paimon.rest.uri", "uri"}, + description = "The uri of the Paimon rest catalog service.") + private String paimonRestUri = ""; + + @Getter + @ConnectorProperty( + names = {"paimon.rest.token.provider"}, + description = "the token provider for Paimon REST metastore, e.g., 'dlf' for Aliyun DLF." + ) + protected String tokenProvider = ""; + + // The following properties are specific to DLF rest catalog + @ConnectorProperty( + names = {"paimon.rest.dlf.access-key-id"}, + required = false, + description = "The access key ID for DLF, required when using DLF as token provider." + ) + protected String paimonRestDlfAccessKey = ""; + + @ConnectorProperty( + names = {"paimon.rest.dlf.access-key-secret"}, + required = false, + description = "The secret key secret for DLF, required when using DLF as token provider." + ) + protected String paimonRestDlfSecretKey = ""; + + protected PaimonRestMetaStoreProperties(Map<String, String> props) { + super(props); + } + + @Override + public void initNormalizeAndCheckProps() { + super.initNormalizeAndCheckProps(); + buildRules().validate(); + } + + @Override + public String getPaimonCatalogType() { + return PaimonExternalCatalog.PAIMON_REST; + } + + @Override + public Catalog initializeCatalog(String catalogName, List<StorageProperties> storagePropertiesList) { + buildCatalogOptions(storagePropertiesList); + CatalogContext catalogContext = CatalogContext.create(catalogOptions); + return CatalogFactory.createCatalog(catalogContext); + } + + @Override + protected void appendCustomCatalogOptions() { + catalogOptions.set("uri", paimonRestUri); + for (Map.Entry<String, String> entry : origProps.entrySet()) { + if (entry.getKey().startsWith(PAIMON_REST_PROPERTY_PREFIX)) { + String key = entry.getKey().substring(PAIMON_REST_PROPERTY_PREFIX.length()); + catalogOptions.set(key, entry.getValue()); + } + } + } + + @Override + protected String getMetastoreType() { + return "rest"; + } + + private ParamRules buildRules() { + ParamRules rules = new ParamRules(); + // Check for dlf rest catalog + rules.requireIf(tokenProvider, "dlf", Review Comment: The string literal "dlf" is used in the validation logic but should be defined as a constant to improve maintainability and avoid potential typos. ```suggestion rules.requireIf(tokenProvider, TOKEN_PROVIDER_DLF, ``` ########## fe/fe-core/src/main/java/org/apache/doris/datasource/credentials/CredentialUtils.java: ########## @@ -0,0 +1,21 @@ +package org.apache.doris.datasource.credentials; Review Comment: Missing license header in CredentialUtils.java. All source files should include the Apache License header. ########## fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonVendedCredentialsProvider.java: ########## @@ -0,0 +1,100 @@ +// 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.doris.datasource.paimon; + +import org.apache.doris.datasource.property.metastore.MetastoreProperties; +import org.apache.doris.datasource.property.metastore.PaimonRestMetaStoreProperties; +import org.apache.doris.datasource.property.storage.StorageProperties; +import org.apache.doris.datasource.property.storage.StorageProperties.Type; + +import com.google.common.collect.Maps; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.paimon.rest.RESTToken; +import org.apache.paimon.rest.RESTTokenFileIO; +import org.apache.paimon.table.Table; + +import java.util.Map; + +public class PaimonVendedCredentialsProvider { + private static final Logger LOG = LogManager.getLogger(PaimonVendedCredentialsProvider.class); + + private static final PaimonOssCredentialExtractor ossExtractor = new PaimonOssCredentialExtractor(); + + /** + * Extract vended credentials from Paimon Table and convert to backend properties. + * + * @param table the Iceberg table + * @return Map of backend properties with credentials + */ + public static Map<String, String> extractVendedCredentialsFromTable(String tokenProvider, Table table) { + if (table == null || table.fileIO() == null) { + return Maps.newHashMap(); + } + + if (!(table.fileIO() instanceof RESTTokenFileIO)) { + if (LOG.isDebugEnabled()) { + LOG.debug("File IO of table {} is not RESTTokenFileIO, cannot extract vended credentials: {}", + table.name(), table.fileIO().getClass().getName()); + } + return Maps.newHashMap(); + } + + RESTTokenFileIO restTokenFileIO = (RESTTokenFileIO) table.fileIO(); + RESTToken restToken = restTokenFileIO.validToken(); + Map<String, String> tokens = restToken.token(); + if ("dlf".equalsIgnoreCase(tokenProvider)) { + return ossExtractor.extractCredentials(tokens); + } else { + if (LOG.isDebugEnabled()) { + LOG.debug("Unsupported token provider: {} for table {}, cannot extract vended credentials", + tokenProvider, table.name()); + } + return Maps.newHashMap(); + } + } + + /** + * Get backend location properties for Paimon catalog with optional vended credentials support. + * This method extracts the duplicate logic from PaimonScanNode Review Comment: The comment mentions 'extracts the duplicate logic from PaimonScanNode' but this doesn't accurately describe what the method does. It should describe the method's purpose of providing backend location properties with vended credentials support. ```suggestion * Provides backend location properties for a Paimon catalog, including support for vended credentials * if available from the provided Paimon table and metastore properties. ``` ########## fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonVendedCredentialsProvider.java: ########## @@ -0,0 +1,100 @@ +// 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.doris.datasource.paimon; + +import org.apache.doris.datasource.property.metastore.MetastoreProperties; +import org.apache.doris.datasource.property.metastore.PaimonRestMetaStoreProperties; +import org.apache.doris.datasource.property.storage.StorageProperties; +import org.apache.doris.datasource.property.storage.StorageProperties.Type; + +import com.google.common.collect.Maps; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.paimon.rest.RESTToken; +import org.apache.paimon.rest.RESTTokenFileIO; +import org.apache.paimon.table.Table; + +import java.util.Map; + +public class PaimonVendedCredentialsProvider { + private static final Logger LOG = LogManager.getLogger(PaimonVendedCredentialsProvider.class); + + private static final PaimonOssCredentialExtractor ossExtractor = new PaimonOssCredentialExtractor(); + + /** + * Extract vended credentials from Paimon Table and convert to backend properties. + * + * @param table the Iceberg table Review Comment: The javadoc comment incorrectly refers to 'Iceberg table' instead of 'Paimon table'. This should be corrected for accuracy. ```suggestion * @param table the Paimon table ``` -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
