sundapeng commented on code in PR #10178: URL: https://github.com/apache/paimon/pull/10178#discussion_r4249940879
########## paimon-filesystems/paimon-oss-impl/src/main/java/org/apache/paimon/oss/RESTTokenCredentialsProvider.java: ########## @@ -0,0 +1,112 @@ +/* + * 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.paimon.oss; + +import org.apache.paimon.options.Options; +import org.apache.paimon.rest.RESTToken; +import org.apache.paimon.rest.RESTTokenRefresher; + +import com.aliyun.oss.common.auth.Credentials; +import com.aliyun.oss.common.auth.CredentialsProvider; +import com.aliyun.oss.common.auth.DefaultCredentials; +import org.apache.hadoop.conf.Configuration; + +import java.net.URI; +import java.util.Locale; +import java.util.Map; + +import static org.apache.paimon.utils.Preconditions.checkArgument; + +/** + * An OSS {@link CredentialsProvider} that refreshes the data token of a REST catalog table by + * itself and is asked on every request, so streams that are already open also pick up refreshed + * tokens. + */ +public class RESTTokenCredentialsProvider implements CredentialsProvider { + + /** Prefix under which {@link OSSFileIO} passes the catalog options to this provider. */ + static final String CATALOG_OPTIONS_PREFIX = "fs.oss.rest-token.catalog."; + + private static final String ACCESS_KEY_ID = "fs.oss.accessKeyId"; + private static final String ACCESS_KEY_SECRET = "fs.oss.accessKeySecret"; + private static final String SECURITY_TOKEN = "fs.oss.securityToken"; + + private final RESTTokenRefresher refresher; + + // The last token and the credentials built from it, swapped together. + private volatile Resolved last; + + public RESTTokenCredentialsProvider(URI uri, Configuration conf) { + Options options = new Options(conf.getPropsWithPrefix(CATALOG_OPTIONS_PREFIX)); + checkArgument( + RESTTokenRefresher.isConfigured(options), + "No REST catalog table configured under '%s'.", + CATALOG_OPTIONS_PREFIX); + this.refresher = RESTTokenRefresher.fromOptions(options); + } + + @Override + public void setCredentials(Credentials credentials) { + throw new UnsupportedOperationException( + "Credentials come from the REST catalog and cannot be set."); + } + + @Override + public Credentials getCredentials() { + RESTToken token = refresher.token(); Review Comment: Good catch, the token that signs and the wrapper's token can diverge now. In 8e7c0207d, `OSSBlobPresigner` lets `RESTTokenCredentialsProvider` do the signing: it takes a token with at least the requested validity left, reloading one when the current token expires earlier, hands that token's credentials to the SDK for this signing call only, and checks the token's remaining lifetime again after signing. If even the reloaded token expires before the URL, presigning fails. The wrapper's checks stay for delegates that sign with the token they were built with. `RESTTokenFileIOOnOSSTest` now rotates to a shorter-lived token during materialization: a 10-minute URL starts with ak-1 at 61 minutes left, the clock moves 31 minutes while copying, and the provider reloads ak-2 with 5 minutes left. With a longer-lived ak-3 in the catalog the URL is signed with ak-3, and with only ak-2 presigning fails. Without the fix, the first case returns a URL signed with ak-2, as in your probe. It uses the real SDK signing and local REST HTTP, with the storage calls stubbed. Separately, I opened #10522 for the eviction leak we discussed earlier. -- 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]
