You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@fluss.apache.org by "XuQianJin-Stars (via GitHub)" <gi...@apache.org> on 2026/03/10 11:02:18 UTC
[PR] [FLUSS-2686] Add COS filesystem support (fluss-fs-cos module) [fluss]
XuQianJin-Stars opened a new pull request, #2836:
URL: https://github.com/apache/fluss/pull/2836
### Purpose
Linked issue: close #2686
This PR adds support for Tencent Cloud Object Storage (COS) as a remote filesystem for Fluss. COS is one of the most widely used cloud storage services in China and this integration enables users to store Fluss remote data on COS, similar to the existing OSS, OBS, S3, and GCS filesystem integrations.
### Brief change log
- **`fluss-fs-cos/pom.xml`**: Add module configuration with dependencies on `hadoop-cos` (3.3.5), `cos_api` (5.6.139), and `fluss-fs-hadoop` for Hadoop filesystem abstraction.
- **`COSFileSystem`**: Extends `HadoopFileSystem` to wrap Hadoop's `CosNFileSystem`, with lazy-initialized security token provider support for obtaining temporary credentials.
- **`COSFileSystemPlugin`**: Implements `FileSystemPlugin` SPI for the `cosn` scheme. Handles Hadoop configuration translation from Fluss config (prefix `fs.cosn.*`), supports three credential modes: static secret key, custom credentials provider, and dynamic security token via `COSSecurityTokenReceiver`.
- **`COSSecurityTokenProvider`**: Generates temporary security tokens using Tencent Cloud STS (Security Token Service) based on existing `secretId`/`secretKey` configuration.
- **`COSSecurityTokenReceiver`**: Implements `SecurityTokenReceiver` SPI to receive and apply security tokens, configuring `DynamicTemporaryCOSCredentialsProvider` as the Hadoop credentials provider.
- **`DynamicTemporaryCOSCredentialsProvider`**: Implements COS SDK's `COSCredentialsProvider` interface, providing `BasicSessionCredentials` from tokens received via `COSSecurityTokenReceiver`.
- **`fluss-filesystems/pom.xml`**: Register `fluss-fs-cos` as a sub-module.
### Tests
- `COSFileSystemBehaviorITCase`: Integration test for basic COS filesystem behavior (create, read, write, delete) using static credentials (`secretId`/`secretKey`).
- `COSWithTokenFileSystemBehaviorITCase`: Integration test for COS filesystem behavior using dynamic security tokens obtained via STS.
- `COSWithTokenFileSystemBehaviorBaseITCase`: Base class for token-based filesystem tests, handling filesystem initialization with both static credentials and security tokens.
- `COSTestCredentials`: Test utility that reads COS credentials and endpoint from environment variables (`COSN_SECRET_ID`, `COSN_SECRET_KEY`, `COSN_ENDPOINT`, `COSN_BUCKET`).
> Note: These are IT (integration tests) that require actual COS credentials and bucket access. They are skipped automatically when credentials are not available via `Assumptions.assumeTrue`.
### API and Format
No existing API or storage format changes. This PR only adds a new filesystem plugin that registers via SPI (`META-INF/services/org.apache.fluss.fs.FileSystemPlugin`).
New configuration keys introduced (all following standard Hadoop COS conventions):
- `fs.cosn.endpoint` — COS region endpoint (e.g., `ap-guangzhou`)
- `fs.cosn.userinfo.secretId` — Tencent Cloud secret ID
- `fs.cosn.userinfo.secretKey` — Tencent Cloud secret key
- `fs.cosn.credentials.provider` — Custom credentials provider class
### Documentation
This PR introduces a new feature. Documentation for COS filesystem configuration should be added to the Fluss documentation site in a follow-up. Usage example:
```yaml
remote.data.dir: cosn://bucket-name/fluss-data
fs.cosn.endpoint: ap-guangzhou
fs.cosn.userinfo.secretId: <your-secret-id>
fs.cosn.userinfo.secretKey: <your-secret-key>
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLUSS-2686] Add COS filesystem support [fluss]
Posted by "leekeiabstraction (via GitHub)" <gi...@apache.org>.
leekeiabstraction merged PR #2836:
URL: https://github.com/apache/fluss/pull/2836
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLUSS-2686] Add COS filesystem support [fluss]
Posted by "leekeiabstraction (via GitHub)" <gi...@apache.org>.
leekeiabstraction commented on code in PR #2836:
URL: https://github.com/apache/fluss/pull/2836#discussion_r3105105844
##########
fluss-filesystems/fluss-fs-cos/pom.xml:
##########
@@ -0,0 +1,263 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ 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.
+-->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+ <parent>
+ <groupId>org.apache.fluss</groupId>
+ <artifactId>fluss-filesystems</artifactId>
+ <version>1.0-SNAPSHOT</version>
+ </parent>
+
+ <artifactId>fluss-fs-cos</artifactId>
+ <name>Fluss : FileSystems : COS FS</name>
+
+ <properties>
+ <fs.cosn.sdk.version>3.3.5</fs.cosn.sdk.version>
+ <fs.cos.api.version>5.6.139</fs.cos.api.version>
+ <fs.cos.sts.sdk.version>3.1.678</fs.cos.sts.sdk.version>
+ </properties>
+
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.fluss</groupId>
+ <artifactId>fluss-common</artifactId>
+ <version>${project.version}</version>
+ <scope>provided</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.apache.fluss</groupId>
+ <artifactId>fluss-fs-hadoop-shaded</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+
+ <dependency>
+ <groupId>org.apache.fluss</groupId>
+ <artifactId>fluss-fs-hadoop</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+
+ <dependency>
+ <groupId>org.apache.hadoop</groupId>
+ <artifactId>hadoop-cos</artifactId>
+ <version>${fs.cosn.sdk.version}</version>
Review Comment:
This could be better named as hadoop cos version. It is not the sdk version.
##########
fluss-filesystems/fluss-fs-cos/src/main/java/org/apache/fluss/fs/cos/token/COSSecurityTokenProvider.java:
##########
@@ -0,0 +1,128 @@
+/*
+ * 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.fluss.fs.cos.token;
+
+import org.apache.fluss.fs.token.Credentials;
+import org.apache.fluss.fs.token.CredentialsJsonSerde;
+import org.apache.fluss.fs.token.ObtainedSecurityToken;
+
+import com.tencentcloudapi.common.Credential;
+import com.tencentcloudapi.common.exception.TencentCloudSDKException;
+import com.tencentcloudapi.sts.v20180813.StsClient;
+import com.tencentcloudapi.sts.v20180813.models.GetFederationTokenRequest;
+import com.tencentcloudapi.sts.v20180813.models.GetFederationTokenResponse;
+import org.apache.hadoop.conf.Configuration;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.apache.fluss.fs.cos.COSFileSystemPlugin.ENDPOINT_KEY;
+import static org.apache.fluss.fs.cos.COSFileSystemPlugin.REGION;
+import static org.apache.fluss.fs.cos.COSFileSystemPlugin.SECRET_ID;
+import static org.apache.fluss.fs.cos.COSFileSystemPlugin.SECRET_KEY;
+import static org.apache.fluss.utils.Preconditions.checkNotNull;
+
+/**
+ * A provider to provide Tencent Cloud COS security token by calling STS GetFederationToken API to
+ * obtain temporary credentials.
+ */
+public class COSSecurityTokenProvider {
+
+ private static final Logger LOG = LoggerFactory.getLogger(COSSecurityTokenProvider.class);
+
+ /** Default federation token name. */
+ private static final String FEDERATION_TOKEN_NAME = "fluss-cos-federation";
+
+ /** Default duration seconds for temporary credentials, 1800s = 30min. */
+ private static final long DEFAULT_DURATION_SECONDS = 1800L;
Review Comment:
Curious on why picking lower default than other implementation like OSS/OBS
##########
fluss-filesystems/fluss-fs-cos/src/main/java/org/apache/fluss/fs/cos/COSFileSystemPlugin.java:
##########
@@ -0,0 +1,136 @@
+/*
+ * 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.fluss.fs.cos;
+
+import org.apache.fluss.annotation.VisibleForTesting;
+import org.apache.fluss.config.ConfigBuilder;
+import org.apache.fluss.config.Configuration;
+import org.apache.fluss.fs.FileSystem;
+import org.apache.fluss.fs.FileSystemPlugin;
+import org.apache.fluss.fs.cos.token.COSSecurityTokenReceiver;
+
+import org.apache.hadoop.fs.cosn.CosNFileSystem;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.net.URI;
+
+/** Simple factory for the Tencent Cloud COS file system. */
+public class COSFileSystemPlugin implements FileSystemPlugin {
+
+ private static final Logger LOG = LoggerFactory.getLogger(COSFileSystemPlugin.class);
+
+ public static final String SCHEME = "cosn";
+
+ /**
+ * In order to simplify, we make fluss cos configuration keys same with hadoop cos module. So,
+ * we add all configuration key with prefix `fs.cosn` in fluss conf to hadoop conf.
+ */
+ private static final String[] FLUSS_CONFIG_PREFIXES = {"fs.cosn."};
+
+ public static final String SECRET_ID = "fs.cosn.userinfo.secretId";
+ public static final String SECRET_KEY = "fs.cosn.userinfo.secretKey";
+ public static final String CREDENTIALS_PROVIDER = "fs.cosn.credentials.provider";
+
+ public static final String REGION = "fs.cosn.userinfo.region";
+
+ public static final String ENDPOINT_KEY = "fs.cosn.bucket.endpoint_suffix";
+
+ @Override
+ public String getScheme() {
+ return SCHEME;
+ }
+
+ @Override
+ public FileSystem create(URI fsUri, Configuration flussConfig) throws IOException {
+ org.apache.hadoop.conf.Configuration hadoopConfig = getHadoopConfiguration(flussConfig);
+
+ // set credential provider
+ if (hadoopConfig.get(SECRET_ID) == null) {
+ String credentialsProvider = hadoopConfig.get(CREDENTIALS_PROVIDER);
+ if (credentialsProvider != null) {
Review Comment:
Do we need to have IT for this branch?
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLUSS-2686] Add COS filesystem support [fluss]
Posted by "XuQianJin-Stars (via GitHub)" <gi...@apache.org>.
XuQianJin-Stars commented on PR #2836:
URL: https://github.com/apache/fluss/pull/2836#issuecomment-4514305453
> Thanks for adding COS support. I reviewed the implementation and found a few issues that should be addressed before merge.
>
> 1. **[major] STS tokens are too broadly scoped**
>
> `COSSecurityTokenProvider` currently uses this default policy:
>
> ```java
> "action": ["name/cos:*"],
> "resource": ["*"]
> ```
>
> That means the temporary credentials distributed to Fluss clients can access any COS resource allowed by the permanent server-side secret. This is broader than the Fluss remote storage path and could expose non-Fluss data if the permanent credential has wide permissions.
>
> Can we scope the token policy to the configured `remote.data.dir` bucket/prefix, or require an explicit user-provided token policy instead of defaulting to `resource: "*"`?
>
> 2. **[major] The shaded plugin currently contains two COS SDK variants**
>
> `hadoop-cos:3.3.5` brings in `com.qcloud:cos_api-bundle:5.6.69`, while this module also directly depends on `com.qcloud:cos_api:5.6.139`. During `package`, Maven Shade reports 902 overlapping `com.qcloud.cos.*` classes/resources between the two jars.
>
> That makes the actual runtime COS SDK version depend on shade ordering and can produce subtle incompatibilities. Please keep only one COS SDK variant, for example by excluding `cos_api-bundle` from `hadoop-cos` if the direct `cos_api` dependency is required, or by dropping the direct `cos_api` dependency if the bundled one is sufficient.
>
> 3. **[minor] COS documentation uses the wrong environment provider class/env vars**
>
> The docs show:
>
> ```yaml
> fs.cosn.credentials.provider: org.apache.hadoop.fs.cosn.auth.EnvironmentVariableCredentialProvider
> ```
>
> but the actual hadoop-cos class is `org.apache.hadoop.fs.cosn.auth.EnvironmentVariableCredentialsProvider` (plural `Credentials`). Also, the provider reads `COSN_SECRET_ID` and `COSN_SECRET_KEY`, while the docs say `COS_SECRET_ID` and `COS_SECRET_KEY`. Users following the docs would fail authentication.
>
> 4. **[minor] Token lifetime in docs does not match the code**
>
> The docs say the temporary credential is valid for 30 minutes, but `COSSecurityTokenProvider.DEFAULT_DURATION_SECONDS` is `3600L` (1 hour). Please align the docs or the code.
>
> 5. **[nit] `cos.md` has trailing whitespace**
>
> `git diff --check` reports trailing whitespace in `website/docs/maintenance/filesystems/cos.md` lines 14 and 17.
>
> Validation I ran:
>
> ```shell
> ./mvnw -pl fluss-filesystems/fluss-fs-cos -am -DskipTests compile
> ./mvnw -pl fluss-filesystems/fluss-fs-cos -am -DskipTests test-compile
> ./mvnw -pl fluss-filesystems/fluss-fs-cos -am -DskipTests package
> git diff --check
> ```
>
> The Maven commands passed; `package` emitted the duplicate COS SDK warnings mentioned above, and `git diff --check` failed on the trailing whitespace.
Thanks for the thorough review! The PR has been updated in `80c5435` to address all five points:
1. **STS policy scoping** — `COSSecurityTokenProvider` now derives a default policy from `remote.data.dir`, restricting `name/cos:*` to the target bucket (and prefix). Users can override it via `fs.cosn.security.token.policy`.
2. **Duplicate COS SDK** — excluded `com.qcloud:cos_api-bundle` from `hadoop-cos` so only the directly declared `cos_api` ends up on the classpath. `NOTICE` updated accordingly. The shade duplicate-class warnings are gone.
3. **Docs credential provider** — fixed to `EnvironmentVariableCredentialsProvider` and the correct env vars `COSN_SECRET_ID` / `COSN_SECRET_KEY`.
4. **Token lifetime** — docs now say 1 hour to match `DEFAULT_DURATION_SECONDS = 3600L`.
5. **Trailing whitespace** — cleaned up in `website/docs/maintenance/filesystems/cos.md`; `git diff --check` is clean.
PTAL when you have time.
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLUSS-2686] Add COS filesystem support [fluss]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #2836:
URL: https://github.com/apache/fluss/pull/2836#discussion_r2918821915
##########
fluss-filesystems/fluss-fs-cos/src/main/java/org/apache/fluss/fs/cos/token/COSSecurityTokenProvider.java:
##########
@@ -0,0 +1,63 @@
+/*
+ * 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.fluss.fs.cos.token;
+
+import org.apache.fluss.fs.token.Credentials;
+import org.apache.fluss.fs.token.CredentialsJsonSerde;
+import org.apache.fluss.fs.token.ObtainedSecurityToken;
+
+import org.apache.hadoop.conf.Configuration;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.apache.fluss.fs.cos.COSFileSystemPlugin.ENDPOINT_KEY;
+
+/** A provider to provide Tencent Cloud COS security token. */
+public class COSSecurityTokenProvider {
+
+ private static final String SECRET_ID = "fs.cosn.userinfo.secretId";
+ private static final String SECRET_KEY = "fs.cosn.userinfo.secretKey";
+
+ private final String endpoint;
+ private final String secretId;
+ private final String secretKey;
+
+ public COSSecurityTokenProvider(Configuration conf) {
+ endpoint = conf.get(ENDPOINT_KEY);
+ secretId = conf.get(SECRET_ID);
+ secretKey = conf.get(SECRET_KEY);
+ }
+
+ public ObtainedSecurityToken obtainSecurityToken(String scheme) {
+ // For COS, we directly use the configured secret id and secret key as the token.
+ // If STS temporary credentials are needed in the future, this can be extended
+ // to call Tencent Cloud STS API to get temporary credentials.
+ Map<String, String> additionInfo = new HashMap<>();
+ // we need to put endpoint as addition info
+ if (endpoint != null) {
+ additionInfo.put(ENDPOINT_KEY, endpoint);
+ }
+
+ Credentials credentials = new Credentials(secretId, secretKey, null);
+ byte[] tokenBytes = CredentialsJsonSerde.toJson(credentials);
+
+ // token does not expire when using static credentials
+ return new ObtainedSecurityToken(scheme, tokenBytes, Long.MAX_VALUE, additionInfo);
+ }
Review Comment:
This provider serializes `secretId/secretKey` with a `null` security token and sets `Long.MAX_VALUE` expiry, but the receiver converts tokens into `BasicSessionCredentials` (session-based) which generally expects a non-null session token. Either (a) actually obtain STS temporary credentials (accessKey/secretKey/sessionToken + real expiry) and serialize them, or (b) treat the propagated token as *static* credentials end-to-end (adjust receiver/provider to use non-session COS credentials) and rename/comment accordingly to avoid implying STS behavior.
##########
fluss-filesystems/fluss-fs-cos/src/main/java/org/apache/fluss/fs/cos/token/COSSecurityTokenReceiver.java:
##########
@@ -0,0 +1,108 @@
+/*
+ * 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.fluss.fs.cos.token;
+
+import org.apache.fluss.fs.cos.COSFileSystemPlugin;
+import org.apache.fluss.fs.token.CredentialsJsonSerde;
+import org.apache.fluss.fs.token.ObtainedSecurityToken;
+import org.apache.fluss.fs.token.SecurityTokenReceiver;
+
+import com.qcloud.cos.auth.BasicSessionCredentials;
+import com.qcloud.cos.auth.COSCredentials;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Map;
+
+import static org.apache.fluss.fs.cos.COSFileSystemPlugin.CREDENTIALS_PROVIDER;
+
+/** Security token receiver for Tencent Cloud COS filesystem. */
+public class COSSecurityTokenReceiver implements SecurityTokenReceiver {
+
+ private static final Logger LOG = LoggerFactory.getLogger(COSSecurityTokenReceiver.class);
+
+ static volatile COSCredentials credentials;
+ static volatile Map<String, String> additionInfos;
+
+ public static void updateHadoopConfig(org.apache.hadoop.conf.Configuration hadoopConfig) {
+ updateHadoopConfig(hadoopConfig, DynamicTemporaryCOSCredentialsProvider.NAME);
+ }
+
+ protected static void updateHadoopConfig(
+ org.apache.hadoop.conf.Configuration hadoopConfig, String credentialsProviderName) {
+ LOG.info("Updating Hadoop configuration");
+
+ String providers = hadoopConfig.get(CREDENTIALS_PROVIDER, "");
+
+ if (!providers.contains(credentialsProviderName)) {
+ if (providers.isEmpty()) {
+ LOG.debug("Setting provider");
+ providers = credentialsProviderName;
+ } else {
+ providers = credentialsProviderName + "," + providers;
+ LOG.debug("Prepending provider, new providers value: {}", providers);
+ }
+ hadoopConfig.set(CREDENTIALS_PROVIDER, providers);
+ } else {
+ LOG.debug("Provider already exists");
+ }
+
+ // then, set addition info
+ if (additionInfos == null) {
+ // if addition info is null, it also means we have not received any token,
+ throw new RuntimeException("Credentials is not ready.");
+ } else {
+ for (Map.Entry<String, String> entry : additionInfos.entrySet()) {
+ hadoopConfig.set(entry.getKey(), entry.getValue());
+ }
+ }
+
+ LOG.info("Updated Hadoop configuration successfully");
+ }
+
+ @Override
+ public String scheme() {
+ return COSFileSystemPlugin.SCHEME;
+ }
+
+ @Override
+ public void onNewTokensObtained(ObtainedSecurityToken token) {
+ LOG.info("Updating session credentials");
+
+ byte[] tokenBytes = token.getToken();
+
+ org.apache.fluss.fs.token.Credentials flussCredentials =
+ CredentialsJsonSerde.fromJson(tokenBytes);
+
+ // Create Credential from fluss credentials
+ credentials =
+ new BasicSessionCredentials(
+ flussCredentials.getAccessKeyId(),
+ flussCredentials.getSecretAccessKey(),
+ flussCredentials.getSecurityToken());
+ additionInfos = token.getAdditionInfos();
+
+ LOG.info(
+ "Session credentials updated successfully with access key: {}.",
+ credentials.getCOSAccessKeyId());
Review Comment:
Avoid logging credential identifiers (even access key IDs) at INFO level, as they are typically treated as sensitive and can leak via centralized logs. Consider removing the access key from the message, masking it (e.g., last 4 chars), and/or lowering the log level to DEBUG.
```suggestion
LOG.info("Session credentials updated successfully.");
```
##########
fluss-filesystems/fluss-fs-cos/src/main/java/org/apache/fluss/fs/cos/token/COSSecurityTokenReceiver.java:
##########
@@ -0,0 +1,108 @@
+/*
+ * 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.fluss.fs.cos.token;
+
+import org.apache.fluss.fs.cos.COSFileSystemPlugin;
+import org.apache.fluss.fs.token.CredentialsJsonSerde;
+import org.apache.fluss.fs.token.ObtainedSecurityToken;
+import org.apache.fluss.fs.token.SecurityTokenReceiver;
+
+import com.qcloud.cos.auth.BasicSessionCredentials;
+import com.qcloud.cos.auth.COSCredentials;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Map;
+
+import static org.apache.fluss.fs.cos.COSFileSystemPlugin.CREDENTIALS_PROVIDER;
+
+/** Security token receiver for Tencent Cloud COS filesystem. */
+public class COSSecurityTokenReceiver implements SecurityTokenReceiver {
+
+ private static final Logger LOG = LoggerFactory.getLogger(COSSecurityTokenReceiver.class);
+
+ static volatile COSCredentials credentials;
+ static volatile Map<String, String> additionInfos;
+
+ public static void updateHadoopConfig(org.apache.hadoop.conf.Configuration hadoopConfig) {
+ updateHadoopConfig(hadoopConfig, DynamicTemporaryCOSCredentialsProvider.NAME);
+ }
+
+ protected static void updateHadoopConfig(
+ org.apache.hadoop.conf.Configuration hadoopConfig, String credentialsProviderName) {
+ LOG.info("Updating Hadoop configuration");
+
+ String providers = hadoopConfig.get(CREDENTIALS_PROVIDER, "");
+
+ if (!providers.contains(credentialsProviderName)) {
+ if (providers.isEmpty()) {
+ LOG.debug("Setting provider");
+ providers = credentialsProviderName;
+ } else {
+ providers = credentialsProviderName + "," + providers;
+ LOG.debug("Prepending provider, new providers value: {}", providers);
+ }
+ hadoopConfig.set(CREDENTIALS_PROVIDER, providers);
+ } else {
+ LOG.debug("Provider already exists");
+ }
+
+ // then, set addition info
+ if (additionInfos == null) {
+ // if addition info is null, it also means we have not received any token,
+ throw new RuntimeException("Credentials is not ready.");
Review Comment:
The exception message is grammatically incorrect and not very actionable. Consider using a more specific message that indicates the real cause (e.g., token/credentials not yet received via `onNewTokensObtained`, so the receiver cannot update Hadoop config) and—if applicable—include what to configure/call first.
```suggestion
throw new RuntimeException(
"Cannot update Hadoop configuration: COS credentials and additional "
+ "information have not been received yet. Ensure "
+ "onNewTokensObtained(...) has been called with valid tokens "
+ "before invoking COSSecurityTokenReceiver.updateHadoopConfig().");
```
##########
fluss-filesystems/fluss-fs-cos/src/main/java/org/apache/fluss/fs/cos/token/COSSecurityTokenReceiver.java:
##########
@@ -0,0 +1,108 @@
+/*
+ * 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.fluss.fs.cos.token;
+
+import org.apache.fluss.fs.cos.COSFileSystemPlugin;
+import org.apache.fluss.fs.token.CredentialsJsonSerde;
+import org.apache.fluss.fs.token.ObtainedSecurityToken;
+import org.apache.fluss.fs.token.SecurityTokenReceiver;
+
+import com.qcloud.cos.auth.BasicSessionCredentials;
+import com.qcloud.cos.auth.COSCredentials;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Map;
+
+import static org.apache.fluss.fs.cos.COSFileSystemPlugin.CREDENTIALS_PROVIDER;
+
+/** Security token receiver for Tencent Cloud COS filesystem. */
+public class COSSecurityTokenReceiver implements SecurityTokenReceiver {
+
+ private static final Logger LOG = LoggerFactory.getLogger(COSSecurityTokenReceiver.class);
+
+ static volatile COSCredentials credentials;
+ static volatile Map<String, String> additionInfos;
+
+ public static void updateHadoopConfig(org.apache.hadoop.conf.Configuration hadoopConfig) {
+ updateHadoopConfig(hadoopConfig, DynamicTemporaryCOSCredentialsProvider.NAME);
+ }
+
+ protected static void updateHadoopConfig(
+ org.apache.hadoop.conf.Configuration hadoopConfig, String credentialsProviderName) {
+ LOG.info("Updating Hadoop configuration");
+
+ String providers = hadoopConfig.get(CREDENTIALS_PROVIDER, "");
+
+ if (!providers.contains(credentialsProviderName)) {
+ if (providers.isEmpty()) {
+ LOG.debug("Setting provider");
+ providers = credentialsProviderName;
+ } else {
+ providers = credentialsProviderName + "," + providers;
+ LOG.debug("Prepending provider, new providers value: {}", providers);
+ }
+ hadoopConfig.set(CREDENTIALS_PROVIDER, providers);
+ } else {
+ LOG.debug("Provider already exists");
+ }
Review Comment:
Using `String#contains` on a comma-separated provider list can yield false positives (e.g., partial class-name matches). Consider parsing the list as comma-separated tokens (trimmed) and checking for exact equality before prepending.
##########
fluss-filesystems/fluss-fs-cos/pom.xml:
##########
@@ -0,0 +1,254 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ 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.
+-->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+ <parent>
+ <groupId>org.apache.fluss</groupId>
+ <artifactId>fluss-filesystems</artifactId>
+ <version>0.10-SNAPSHOT</version>
+ </parent>
+
+ <artifactId>fluss-fs-cos</artifactId>
+ <name>Fluss : FileSystems : COS FS</name>
+
+ <properties>
+ <fs.cosn.sdk.version>3.3.5</fs.cosn.sdk.version>
+ <fs.cos.api.version>5.6.139</fs.cos.api.version>
+ </properties>
+
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.fluss</groupId>
+ <artifactId>fluss-common</artifactId>
+ <version>${project.version}</version>
+ <scope>provided</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.apache.fluss</groupId>
+ <artifactId>fluss-fs-hadoop-shaded</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+
+ <dependency>
+ <groupId>org.apache.fluss</groupId>
+ <artifactId>fluss-fs-hadoop</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+
+ <dependency>
+ <groupId>org.apache.hadoop</groupId>
+ <artifactId>hadoop-cos</artifactId>
+ <version>${fs.cosn.sdk.version}</version>
+ <exclusions>
+ <exclusion>
+ <groupId>com.qcloud</groupId>
+ <artifactId>cos_api</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.apache.hadoop</groupId>
+ <artifactId>hadoop-common</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>ch.qos.reload4j</groupId>
+ <artifactId>reload4j</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.slf4j</groupId>
+ <artifactId>slf4j-reload4j</artifactId>
+ </exclusion>
+ </exclusions>
+ </dependency>
+
+ <dependency>
+ <groupId>com.qcloud</groupId>
+ <artifactId>cos_api</artifactId>
+ <version>${fs.cos.api.version}</version>
+ <exclusions>
+ <exclusion>
+ <groupId>javax.xml.bind</groupId>
+ <artifactId>jaxb-api</artifactId>
+ </exclusion>
+ </exclusions>
+ </dependency>
+
+ <dependency>
+ <!-- Hadoop requires jaxb-api for javax.xml.bind.JAXBException -->
+ <groupId>javax.xml.bind</groupId>
+ <artifactId>jaxb-api</artifactId>
+ <version>${jaxb.api.version}</version>
+ <!-- packaged as an optional dependency that is only accessible on Java 11+ -->
+ <scope>provided</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.apache.fluss</groupId>
+ <artifactId>fluss-test-utils</artifactId>
+ </dependency>
+ <!-- for the behavior test suite -->
+ <dependency>
+ <groupId>org.apache.fluss</groupId>
+ <artifactId>fluss-common</artifactId>
+ <version>${project.version}</version>
+ <scope>test</scope>
+ <type>test-jar</type>
+ </dependency>
+ </dependencies>
+
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-jar-plugin</artifactId>
+ <executions>
+ <execution>
+ <goals>
+ <goal>jar</goal>
+ </goals>
+ </execution>
+ </executions>
+ <configuration>
+ <archive>
+ <manifestEntries>
+ <!-- jaxb-api is packaged as an optional dependency that is only accessible on Java 11 -->
+ <Multi-Release>true</Multi-Release>
+ </manifestEntries>
+ </archive>
+ </configuration>
+ </plugin>
Review Comment:
The same `maven-jar-plugin` is declared twice. This is easy to misread and can lead to configuration being overridden unexpectedly depending on Maven’s model merging. Consider merging the `jar` and `test-jar` executions into a single `maven-jar-plugin` declaration so the `Multi-Release` manifest configuration applies consistently.
##########
fluss-filesystems/fluss-fs-cos/pom.xml:
##########
@@ -0,0 +1,254 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ 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.
+-->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+ <parent>
+ <groupId>org.apache.fluss</groupId>
+ <artifactId>fluss-filesystems</artifactId>
+ <version>0.10-SNAPSHOT</version>
+ </parent>
+
+ <artifactId>fluss-fs-cos</artifactId>
+ <name>Fluss : FileSystems : COS FS</name>
+
+ <properties>
+ <fs.cosn.sdk.version>3.3.5</fs.cosn.sdk.version>
+ <fs.cos.api.version>5.6.139</fs.cos.api.version>
+ </properties>
+
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.fluss</groupId>
+ <artifactId>fluss-common</artifactId>
+ <version>${project.version}</version>
+ <scope>provided</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.apache.fluss</groupId>
+ <artifactId>fluss-fs-hadoop-shaded</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+
+ <dependency>
+ <groupId>org.apache.fluss</groupId>
+ <artifactId>fluss-fs-hadoop</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+
+ <dependency>
+ <groupId>org.apache.hadoop</groupId>
+ <artifactId>hadoop-cos</artifactId>
+ <version>${fs.cosn.sdk.version}</version>
+ <exclusions>
+ <exclusion>
+ <groupId>com.qcloud</groupId>
+ <artifactId>cos_api</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.apache.hadoop</groupId>
+ <artifactId>hadoop-common</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>ch.qos.reload4j</groupId>
+ <artifactId>reload4j</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.slf4j</groupId>
+ <artifactId>slf4j-reload4j</artifactId>
+ </exclusion>
+ </exclusions>
+ </dependency>
+
+ <dependency>
+ <groupId>com.qcloud</groupId>
+ <artifactId>cos_api</artifactId>
+ <version>${fs.cos.api.version}</version>
+ <exclusions>
+ <exclusion>
+ <groupId>javax.xml.bind</groupId>
+ <artifactId>jaxb-api</artifactId>
+ </exclusion>
+ </exclusions>
+ </dependency>
+
+ <dependency>
+ <!-- Hadoop requires jaxb-api for javax.xml.bind.JAXBException -->
+ <groupId>javax.xml.bind</groupId>
+ <artifactId>jaxb-api</artifactId>
+ <version>${jaxb.api.version}</version>
+ <!-- packaged as an optional dependency that is only accessible on Java 11+ -->
+ <scope>provided</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.apache.fluss</groupId>
+ <artifactId>fluss-test-utils</artifactId>
+ </dependency>
+ <!-- for the behavior test suite -->
+ <dependency>
+ <groupId>org.apache.fluss</groupId>
+ <artifactId>fluss-common</artifactId>
+ <version>${project.version}</version>
+ <scope>test</scope>
+ <type>test-jar</type>
+ </dependency>
+ </dependencies>
+
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-jar-plugin</artifactId>
+ <executions>
+ <execution>
+ <goals>
+ <goal>jar</goal>
+ </goals>
+ </execution>
+ </executions>
+ <configuration>
+ <archive>
+ <manifestEntries>
+ <!-- jaxb-api is packaged as an optional dependency that is only accessible on Java 11 -->
+ <Multi-Release>true</Multi-Release>
+ </manifestEntries>
+ </archive>
+ </configuration>
+ </plugin>
+
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-dependency-plugin</artifactId>
+ <executions>
+ <execution>
+ <id>copy-javax-jars</id>
+ <phase>process-resources</phase>
+ <goals>
+ <goal>copy</goal>
+ </goals>
+ </execution>
+ </executions>
+ <configuration>
+ <artifactItems>
+ <artifactItem>
+ <groupId>javax.xml.bind</groupId>
+ <artifactId>jaxb-api</artifactId>
+ <version>${jaxb.api.version}</version>
+ <type>jar</type>
+ <overWrite>true</overWrite>
+ </artifactItem>
+ </artifactItems>
+ <outputDirectory>${project.build.directory}/temporary</outputDirectory>
+ </configuration>
+ </plugin>
+
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-antrun-plugin</artifactId>
+ <executions>
+ <execution>
+ <id>unpack-javax-libraries</id>
+ <phase>process-resources</phase>
+ <goals>
+ <goal>run</goal>
+ </goals>
+ <configuration>
+ <target>
+ <echo message="unpacking javax jars"/>
+ <unzip dest="${project.build.directory}/classes/META-INF/versions/11">
+ <fileset dir="${project.build.directory}/temporary">
+ <include name="*"/>
+ </fileset>
+ </unzip>
+ </target>
+ </configuration>
+ </execution>
+ </executions>
+ </plugin>
+
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-shade-plugin</artifactId>
+ <executions>
+ <execution>
+ <id>shade-fluss</id>
+ <phase>package</phase>
+ <goals>
+ <goal>shade</goal>
+ </goals>
+ <configuration>
+ <artifactSet>
+ <includes>
+ <include>*:*</include>
+ </includes>
+ <excludes>
+ <exclude>javax.servlet:servlet-api</exclude>
+ <exclude>xmlenc:xmlenc</exclude>
+ </excludes>
+ </artifactSet>
+ <filters>
+ <filter>
+ <artifact>*</artifact>
+ <excludes>
+ <exclude>.gitkeep</exclude>
+ <exclude>mime.types</exclude>
+ <exclude>mozilla/**</exclude>
+ <exclude>LICENSE.txt</exclude>
+ <exclude>license/LICENSE*</exclude>
+ <exclude>okhttp3/internal/publicsuffix/NOTICE</exclude>
+ <exclude>NOTICE</exclude>
+ </excludes>
+ </filter>
+ <filter>
+ <artifact>org.apache.fluss:fluss-fs-hadoop</artifact>
+ <excludes>
+ <exclude>META-INF/**</exclude>
+ </excludes>
+ </filter>
+ </filters>
+ <relocations>
+ <relocation>
+ <pattern>org.apache.commons</pattern>
+ <shadedPattern>org.apache.fluss.shaded.org.apache.commons</shadedPattern>
+ </relocation>
+ </relocations>
+ </configuration>
+ </execution>
+ </executions>
+ </plugin>
+
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-jar-plugin</artifactId>
+ <executions>
+ <execution>
+ <goals>
+ <goal>test-jar</goal>
+ </goals>
+ </execution>
+ </executions>
+ </plugin>
Review Comment:
The same `maven-jar-plugin` is declared twice. This is easy to misread and can lead to configuration being overridden unexpectedly depending on Maven’s model merging. Consider merging the `jar` and `test-jar` executions into a single `maven-jar-plugin` declaration so the `Multi-Release` manifest configuration applies consistently.
##########
fluss-filesystems/fluss-fs-cos/src/main/java/org/apache/fluss/fs/cos/token/DynamicTemporaryCOSCredentialsProvider.java:
##########
@@ -0,0 +1,58 @@
+/*
+ * 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.fluss.fs.cos.token;
+
+import org.apache.fluss.annotation.Internal;
+
+import com.qcloud.cos.auth.BasicSessionCredentials;
+import com.qcloud.cos.auth.COSCredentials;
+import com.qcloud.cos.auth.COSCredentialsProvider;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Support dynamic session credentials for authenticating with Tencent Cloud COS. It'll get
+ * credentials from {@link COSSecurityTokenReceiver}. It implements COS native {@link
+ * COSCredentialsProvider} to work with CosNFileSystem.
+ */
+@Internal
+public class DynamicTemporaryCOSCredentialsProvider implements COSCredentialsProvider {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(DynamicTemporaryCOSCredentialsProvider.class);
+
+ public static final String NAME = DynamicTemporaryCOSCredentialsProvider.class.getName();
+
+ @Override
+ public COSCredentials getCredentials() {
+ COSCredentials credentials = COSSecurityTokenReceiver.getCredentials();
+ if (credentials == null) {
+ throw new RuntimeException("Credentials is not ready.");
+ }
+ LOG.debug("Providing session credentials");
+ return new BasicSessionCredentials(
+ credentials.getCOSAccessKeyId(),
+ credentials.getCOSSecretKey(),
+ ((BasicSessionCredentials) credentials).getSessionToken());
Review Comment:
This unconditionally casts `credentials` to `BasicSessionCredentials`, which will throw `ClassCastException` if the receiver ever stores a different `COSCredentials` implementation (e.g., static/basic credentials). Consider removing the cast by storing the session token separately, or branching with `instanceof` and returning the appropriate credential type based on what was received.
```suggestion
if (credentials instanceof BasicSessionCredentials) {
BasicSessionCredentials sessionCredentials = (BasicSessionCredentials) credentials;
LOG.debug("Providing session credentials");
return new BasicSessionCredentials(
sessionCredentials.getCOSAccessKeyId(),
sessionCredentials.getCOSSecretKey(),
sessionCredentials.getSessionToken());
} else {
LOG.debug("Providing non-session COS credentials");
return credentials;
}
```
##########
fluss-filesystems/fluss-fs-cos/src/main/java/org/apache/fluss/fs/cos/token/COSSecurityTokenReceiver.java:
##########
@@ -0,0 +1,108 @@
+/*
+ * 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.fluss.fs.cos.token;
+
+import org.apache.fluss.fs.cos.COSFileSystemPlugin;
+import org.apache.fluss.fs.token.CredentialsJsonSerde;
+import org.apache.fluss.fs.token.ObtainedSecurityToken;
+import org.apache.fluss.fs.token.SecurityTokenReceiver;
+
+import com.qcloud.cos.auth.BasicSessionCredentials;
+import com.qcloud.cos.auth.COSCredentials;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Map;
+
+import static org.apache.fluss.fs.cos.COSFileSystemPlugin.CREDENTIALS_PROVIDER;
+
+/** Security token receiver for Tencent Cloud COS filesystem. */
+public class COSSecurityTokenReceiver implements SecurityTokenReceiver {
+
+ private static final Logger LOG = LoggerFactory.getLogger(COSSecurityTokenReceiver.class);
+
+ static volatile COSCredentials credentials;
+ static volatile Map<String, String> additionInfos;
+
+ public static void updateHadoopConfig(org.apache.hadoop.conf.Configuration hadoopConfig) {
+ updateHadoopConfig(hadoopConfig, DynamicTemporaryCOSCredentialsProvider.NAME);
+ }
+
+ protected static void updateHadoopConfig(
+ org.apache.hadoop.conf.Configuration hadoopConfig, String credentialsProviderName) {
+ LOG.info("Updating Hadoop configuration");
+
+ String providers = hadoopConfig.get(CREDENTIALS_PROVIDER, "");
+
+ if (!providers.contains(credentialsProviderName)) {
+ if (providers.isEmpty()) {
+ LOG.debug("Setting provider");
+ providers = credentialsProviderName;
+ } else {
+ providers = credentialsProviderName + "," + providers;
+ LOG.debug("Prepending provider, new providers value: {}", providers);
+ }
+ hadoopConfig.set(CREDENTIALS_PROVIDER, providers);
+ } else {
+ LOG.debug("Provider already exists");
+ }
+
+ // then, set addition info
+ if (additionInfos == null) {
Review Comment:
The field name `additionInfos` is awkward/unclear; `additionalInfos` (or `additionalInfo` if singular) is more standard and improves readability throughout this class.
##########
fluss-filesystems/fluss-fs-cos/src/main/java/org/apache/fluss/fs/cos/token/COSSecurityTokenReceiver.java:
##########
@@ -0,0 +1,108 @@
+/*
+ * 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.fluss.fs.cos.token;
+
+import org.apache.fluss.fs.cos.COSFileSystemPlugin;
+import org.apache.fluss.fs.token.CredentialsJsonSerde;
+import org.apache.fluss.fs.token.ObtainedSecurityToken;
+import org.apache.fluss.fs.token.SecurityTokenReceiver;
+
+import com.qcloud.cos.auth.BasicSessionCredentials;
+import com.qcloud.cos.auth.COSCredentials;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Map;
+
+import static org.apache.fluss.fs.cos.COSFileSystemPlugin.CREDENTIALS_PROVIDER;
+
+/** Security token receiver for Tencent Cloud COS filesystem. */
+public class COSSecurityTokenReceiver implements SecurityTokenReceiver {
+
+ private static final Logger LOG = LoggerFactory.getLogger(COSSecurityTokenReceiver.class);
+
+ static volatile COSCredentials credentials;
+ static volatile Map<String, String> additionInfos;
+
+ public static void updateHadoopConfig(org.apache.hadoop.conf.Configuration hadoopConfig) {
+ updateHadoopConfig(hadoopConfig, DynamicTemporaryCOSCredentialsProvider.NAME);
+ }
+
+ protected static void updateHadoopConfig(
+ org.apache.hadoop.conf.Configuration hadoopConfig, String credentialsProviderName) {
+ LOG.info("Updating Hadoop configuration");
+
+ String providers = hadoopConfig.get(CREDENTIALS_PROVIDER, "");
+
+ if (!providers.contains(credentialsProviderName)) {
+ if (providers.isEmpty()) {
+ LOG.debug("Setting provider");
+ providers = credentialsProviderName;
+ } else {
+ providers = credentialsProviderName + "," + providers;
+ LOG.debug("Prepending provider, new providers value: {}", providers);
+ }
+ hadoopConfig.set(CREDENTIALS_PROVIDER, providers);
+ } else {
+ LOG.debug("Provider already exists");
+ }
+
+ // then, set addition info
+ if (additionInfos == null) {
+ // if addition info is null, it also means we have not received any token,
+ throw new RuntimeException("Credentials is not ready.");
+ } else {
+ for (Map.Entry<String, String> entry : additionInfos.entrySet()) {
Review Comment:
The field name `additionInfos` is awkward/unclear; `additionalInfos` (or `additionalInfo` if singular) is more standard and improves readability throughout this class.
##########
fluss-filesystems/fluss-fs-cos/src/main/java/org/apache/fluss/fs/cos/token/COSSecurityTokenReceiver.java:
##########
@@ -0,0 +1,108 @@
+/*
+ * 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.fluss.fs.cos.token;
+
+import org.apache.fluss.fs.cos.COSFileSystemPlugin;
+import org.apache.fluss.fs.token.CredentialsJsonSerde;
+import org.apache.fluss.fs.token.ObtainedSecurityToken;
+import org.apache.fluss.fs.token.SecurityTokenReceiver;
+
+import com.qcloud.cos.auth.BasicSessionCredentials;
+import com.qcloud.cos.auth.COSCredentials;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Map;
+
+import static org.apache.fluss.fs.cos.COSFileSystemPlugin.CREDENTIALS_PROVIDER;
+
+/** Security token receiver for Tencent Cloud COS filesystem. */
+public class COSSecurityTokenReceiver implements SecurityTokenReceiver {
+
+ private static final Logger LOG = LoggerFactory.getLogger(COSSecurityTokenReceiver.class);
+
+ static volatile COSCredentials credentials;
+ static volatile Map<String, String> additionInfos;
Review Comment:
The field name `additionInfos` is awkward/unclear; `additionalInfos` (or `additionalInfo` if singular) is more standard and improves readability throughout this class.
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLUSS-2686] Add COS filesystem support [fluss]
Posted by "XuQianJin-Stars (via GitHub)" <gi...@apache.org>.
XuQianJin-Stars commented on code in PR #2836:
URL: https://github.com/apache/fluss/pull/2836#discussion_r2970846657
##########
fluss-filesystems/fluss-fs-cos/src/test/java/org/apache/fluss/fs/cos/COSFileSystemBehaviorITCase.java:
##########
@@ -0,0 +1,63 @@
+/*
+ * 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.fluss.fs.cos;
+
+import org.apache.fluss.config.Configuration;
+import org.apache.fluss.fs.FileSystem;
+import org.apache.fluss.fs.FileSystemBehaviorTestSuite;
+import org.apache.fluss.fs.FsPath;
+
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+
+import java.util.UUID;
+
+/**
+ * An implementation of the {@link FileSystemBehaviorTestSuite} for the COS file system with Hadoop
+ * cos sdk.
+ */
+class COSFileSystemBehaviorITCase extends FileSystemBehaviorTestSuite {
Review Comment:
> Was the IT test run successful with creds set? Would be good to indicate in pr description
Yes, the IT tests have been run successfully with COS credentials configured. Will update the PR description to indicate this.
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLUSS-2686] Add COS filesystem support [fluss]
Posted by "luoyuxia (via GitHub)" <gi...@apache.org>.
luoyuxia commented on PR #2836:
URL: https://github.com/apache/fluss/pull/2836#issuecomment-4506996698
Thanks for adding COS support. I reviewed the implementation and found a few issues that should be addressed before merge.
1. **[major] STS tokens are too broadly scoped**
`COSSecurityTokenProvider` currently uses this default policy:
```java
"action": ["name/cos:*"],
"resource": ["*"]
```
That means the temporary credentials distributed to Fluss clients can access any COS resource allowed by the permanent server-side secret. This is broader than the Fluss remote storage path and could expose non-Fluss data if the permanent credential has wide permissions.
Can we scope the token policy to the configured `remote.data.dir` bucket/prefix, or require an explicit user-provided token policy instead of defaulting to `resource: "*"`?
2. **[major] The shaded plugin currently contains two COS SDK variants**
`hadoop-cos:3.3.5` brings in `com.qcloud:cos_api-bundle:5.6.69`, while this module also directly depends on `com.qcloud:cos_api:5.6.139`. During `package`, Maven Shade reports 902 overlapping `com.qcloud.cos.*` classes/resources between the two jars.
That makes the actual runtime COS SDK version depend on shade ordering and can produce subtle incompatibilities. Please keep only one COS SDK variant, for example by excluding `cos_api-bundle` from `hadoop-cos` if the direct `cos_api` dependency is required, or by dropping the direct `cos_api` dependency if the bundled one is sufficient.
3. **[minor] COS documentation uses the wrong environment provider class/env vars**
The docs show:
```yaml
fs.cosn.credentials.provider: org.apache.hadoop.fs.cosn.auth.EnvironmentVariableCredentialProvider
```
but the actual hadoop-cos class is `org.apache.hadoop.fs.cosn.auth.EnvironmentVariableCredentialsProvider` (plural `Credentials`). Also, the provider reads `COSN_SECRET_ID` and `COSN_SECRET_KEY`, while the docs say `COS_SECRET_ID` and `COS_SECRET_KEY`. Users following the docs would fail authentication.
4. **[minor] Token lifetime in docs does not match the code**
The docs say the temporary credential is valid for 30 minutes, but `COSSecurityTokenProvider.DEFAULT_DURATION_SECONDS` is `3600L` (1 hour). Please align the docs or the code.
5. **[nit] `cos.md` has trailing whitespace**
`git diff --check` reports trailing whitespace in `website/docs/maintenance/filesystems/cos.md` lines 14 and 17.
Validation I ran:
```bash
./mvnw -pl fluss-filesystems/fluss-fs-cos -am -DskipTests compile
./mvnw -pl fluss-filesystems/fluss-fs-cos -am -DskipTests test-compile
./mvnw -pl fluss-filesystems/fluss-fs-cos -am -DskipTests package
git diff --check
```
The Maven commands passed; `package` emitted the duplicate COS SDK warnings mentioned above, and `git diff --check` failed on the trailing whitespace.
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLUSS-2686] Add COS filesystem support [fluss]
Posted by "leekeiabstraction (via GitHub)" <gi...@apache.org>.
leekeiabstraction commented on code in PR #2836:
URL: https://github.com/apache/fluss/pull/2836#discussion_r3144225416
##########
fluss-filesystems/fluss-fs-cos/src/test/java/org/apache/fluss/fs/cos/COSTestCredentials.java:
##########
@@ -0,0 +1,119 @@
+/*
+ * 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.fluss.fs.cos;
+
+import javax.annotation.Nullable;
+
+import static org.junit.jupiter.api.Assumptions.assumeTrue;
+
+/** Access to credentials to access COS buckets during integration tests. */
+public class COSTestCredentials {
+ @Nullable private static final String ENDPOINT = System.getenv("COSN_ENDPOINT");
Review Comment:
Should this be endpoint suffix?
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLUSS-2686] Add COS filesystem support [fluss]
Posted by "XuQianJin-Stars (via GitHub)" <gi...@apache.org>.
XuQianJin-Stars commented on PR #2836:
URL: https://github.com/apache/fluss/pull/2836#issuecomment-4107562445
> Looks consistent to other filesystem support. Left comments on unrelated changes being included in this PR (looks like it crept back in), can approve once this is addresed.
>
> I think we should have someone else who has access to COS to test this as well as I do not have access to COS to manually test. cc @luoyuxia
I will supplement the test record for COS at a later time.
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLUSS-2686] Add COS filesystem support [fluss]
Posted by "XuQianJin-Stars (via GitHub)" <gi...@apache.org>.
XuQianJin-Stars commented on PR #2836:
URL: https://github.com/apache/fluss/pull/2836#issuecomment-4248842250
> TY for addressing PR comments. Left a couple more questions
Thanks for the review! I've addressed all three previous comments in the latest push:
1. STS Implementation: `COSSecurityTokenProvider` now calls the Tencent Cloud STS GetFederationToken API to obtain real temporary credentials (tmpSecretId, tmpSecretKey, sessionToken), consistent with the S3/OSS/OBS implementations. Added tencentcloud-sdk-java-sts:3.1.678 dependency.
2. REGION constant: Added public static final String REGION = "fs.cosn.userinfo.region" to `COSFileSystemPlugin`, and it's now used in `COSSecurityTokenProvider` for STS API calls and passed through `additionInfos`.
3. Endpoint key consistency: The code uses fs.cosn.bucket.endpoint_suffix which is the standard Hadoop-COS configuration key. Updated PR description to match.
Also updated the `COSSecurityTokenReceiver` to always use BasicSessionCredentials (since STS always returns temporary credentials with a session token), updated the NOTICE file, and added test coverage for the `REGION` config.
Please let me know about the additional questions — happy to address them!
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLUSS-2686] Add COS filesystem support [fluss]
Posted by "leekeiabstraction (via GitHub)" <gi...@apache.org>.
leekeiabstraction commented on code in PR #2836:
URL: https://github.com/apache/fluss/pull/2836#discussion_r3071306099
##########
fluss-filesystems/fluss-fs-cos/src/main/java/org/apache/fluss/fs/cos/COSFileSystemPlugin.java:
##########
@@ -0,0 +1,134 @@
+/*
+ * 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.fluss.fs.cos;
+
+import org.apache.fluss.annotation.VisibleForTesting;
+import org.apache.fluss.config.ConfigBuilder;
+import org.apache.fluss.config.Configuration;
+import org.apache.fluss.fs.FileSystem;
+import org.apache.fluss.fs.FileSystemPlugin;
+import org.apache.fluss.fs.cos.token.COSSecurityTokenReceiver;
+
+import org.apache.hadoop.fs.cosn.CosNFileSystem;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.net.URI;
+
+/** Simple factory for the Tencent Cloud COS file system. */
+public class COSFileSystemPlugin implements FileSystemPlugin {
+
+ private static final Logger LOG = LoggerFactory.getLogger(COSFileSystemPlugin.class);
+
+ public static final String SCHEME = "cosn";
+
+ /**
+ * In order to simplify, we make fluss cos configuration keys same with hadoop cos module. So,
+ * we add all configuration key with prefix `fs.cosn` in fluss conf to hadoop conf.
+ */
+ private static final String[] FLUSS_CONFIG_PREFIXES = {"fs.cosn."};
+
+ public static final String SECRET_ID = "fs.cosn.userinfo.secretId";
+ public static final String SECRET_KEY = "fs.cosn.userinfo.secretKey";
+ public static final String CREDENTIALS_PROVIDER = "fs.cosn.credentials.provider";
Review Comment:
Seems like region is a required property, but we are not defining it here?
Ref: https://hadoop.apache.org/docs/stable/hadoop-cos/cloud-storage/index.html
> fs.cosn.userinfo.region is an required property for Hadoop-COS. The reason is that Hadoop-COS must know the region of the using bucket in order to accurately construct a URL to access it.
##########
fluss-filesystems/fluss-fs-cos/src/main/java/org/apache/fluss/fs/cos/COSFileSystemPlugin.java:
##########
@@ -0,0 +1,134 @@
+/*
+ * 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.fluss.fs.cos;
+
+import org.apache.fluss.annotation.VisibleForTesting;
+import org.apache.fluss.config.ConfigBuilder;
+import org.apache.fluss.config.Configuration;
+import org.apache.fluss.fs.FileSystem;
+import org.apache.fluss.fs.FileSystemPlugin;
+import org.apache.fluss.fs.cos.token.COSSecurityTokenReceiver;
+
+import org.apache.hadoop.fs.cosn.CosNFileSystem;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.net.URI;
+
+/** Simple factory for the Tencent Cloud COS file system. */
+public class COSFileSystemPlugin implements FileSystemPlugin {
+
+ private static final Logger LOG = LoggerFactory.getLogger(COSFileSystemPlugin.class);
+
+ public static final String SCHEME = "cosn";
+
+ /**
+ * In order to simplify, we make fluss cos configuration keys same with hadoop cos module. So,
+ * we add all configuration key with prefix `fs.cosn` in fluss conf to hadoop conf.
+ */
+ private static final String[] FLUSS_CONFIG_PREFIXES = {"fs.cosn."};
+
+ public static final String SECRET_ID = "fs.cosn.userinfo.secretId";
+ public static final String SECRET_KEY = "fs.cosn.userinfo.secretKey";
+ public static final String CREDENTIALS_PROVIDER = "fs.cosn.credentials.provider";
+
+ public static final String ENDPOINT_KEY = "fs.cosn.bucket.endpoint_suffix";
Review Comment:
Just noticed that this is different to what's in the PR description `fs.cosn.endpoint`. Which is the configuration that we are exposing? `fs.cosn.endpoint` or `fs.cosn.bucket.endpoint_suffix`?
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLUSS-2686] Add COS filesystem support [fluss]
Posted by "leekeiabstraction (via GitHub)" <gi...@apache.org>.
leekeiabstraction commented on code in PR #2836:
URL: https://github.com/apache/fluss/pull/2836#discussion_r2972144070
##########
website/docs/_configs/_partial_config.mdx:
##########
@@ -78,7 +78,7 @@
| `client.lookup.batch-timeout` | `0 s` | Duration | The maximum time to wait for the lookup batch to full, if this timeout is reached, the lookup batch will be closed to send. |
| `client.lookup.max-retries` | `2147483647` | Integer | Setting a value greater than zero will cause the client to resend any lookup request that fails with a potentially transient error. |
| `client.scanner.remote-log.prefetch-num` | `4` | Integer | The number of remote log segments to keep in local temp file for LogScanner, which download from remote storage. The default setting is 4. |
-| `client.scanner.io.tmpdir` | `/var/folders/bp/v2l48kz51mx86d743qv0zhzh0000gn/T//fluss` | String | Local directory that is used by client for storing the data files (like kv snapshot, log segment files) to read temporarily |
+| `client.scanner.io.tmpdir` | `/var/folders/7r/lwdsh9ms4gn0fnxs8c6fcfpm0000gn/T//fluss` | String | Local directory that is used by client for storing the data files (like kv snapshot, log segment files) to read temporarily |
Review Comment:
Looks like unrelated changes crept back into this file
##########
website/docs/_configs/_partial_config.mdx:
##########
@@ -238,9 +238,13 @@
| Key | Default | Type | Description |
| :--- | :--- | :--- | :--- |
-| `remote.data.dir` | `none` | String | The directory used for storing the kv snapshot data files and remote log for log tiered storage in a Fluss supported filesystem. |
+| `remote.data.dir` | `none` | String | The directory used for storing the kv snapshot data files and remote log for log tiered storage in a Fluss supported filesystem. When upgrading to `remote.data.dirs`, please ensure this value is placed as the first entry in the new configuration.For new clusters, it is recommended to use `remote.data.dirs` instead. If `remote.data.dirs` is configured, this value will be ignored. |
+| `remote.data.dirs` | `[]` | ArrayList | A comma-separated list of directories in Fluss supported filesystems for storing the kv snapshot data files and remote log files of tables/partitions. If configured, when a new table or a new partition is created, one of the directories from this list will be selected according to the strategy specified by `remote.data.dirs.strategy` (`ROUND_ROBIN` by default). If not configured, the system uses `remote.data.dir` as the sole remote data directory for all data. |
+| `remote.data.dirs.strategy` | `ROUND_ROBIN` | RemoteDataDirStrategy | The strategy for selecting the remote data directory from `remote.data.dirs`. The candidate strategies are: [ROUND_ROBIN, WEIGHTED_ROUND_ROBIN], the default strategy is ROUND_ROBIN. ROUND_ROBIN: this strategy employs a round-robin approach to select one from the available remote directories. WEIGHTED_ROUND_ROBIN: this strategy selects one of the available remote directories based on the weights configured in `remote.data.dirs.weights`. |
+| `remote.data.dirs.weights` | `[]` | ArrayList | The weights of the remote data directories. This is a list of weights corresponding to the `remote.data.dirs` in the same order. When `remote.data.dirs.strategy` is set to `WEIGHTED_ROUND_ROBIN`, this must be configured, and its size must be equal to `remote.data.dirs`; otherwise, it will be ignored. |
Review Comment:
ditto
##########
website/docs/_configs/_partial_config.mdx:
##########
@@ -238,9 +238,13 @@
| Key | Default | Type | Description |
| :--- | :--- | :--- | :--- |
-| `remote.data.dir` | `none` | String | The directory used for storing the kv snapshot data files and remote log for log tiered storage in a Fluss supported filesystem. |
+| `remote.data.dir` | `none` | String | The directory used for storing the kv snapshot data files and remote log for log tiered storage in a Fluss supported filesystem. When upgrading to `remote.data.dirs`, please ensure this value is placed as the first entry in the new configuration.For new clusters, it is recommended to use `remote.data.dirs` instead. If `remote.data.dirs` is configured, this value will be ignored. |
+| `remote.data.dirs` | `[]` | ArrayList | A comma-separated list of directories in Fluss supported filesystems for storing the kv snapshot data files and remote log files of tables/partitions. If configured, when a new table or a new partition is created, one of the directories from this list will be selected according to the strategy specified by `remote.data.dirs.strategy` (`ROUND_ROBIN` by default). If not configured, the system uses `remote.data.dir` as the sole remote data directory for all data. |
+| `remote.data.dirs.strategy` | `ROUND_ROBIN` | RemoteDataDirStrategy | The strategy for selecting the remote data directory from `remote.data.dirs`. The candidate strategies are: [ROUND_ROBIN, WEIGHTED_ROUND_ROBIN], the default strategy is ROUND_ROBIN. ROUND_ROBIN: this strategy employs a round-robin approach to select one from the available remote directories. WEIGHTED_ROUND_ROBIN: this strategy selects one of the available remote directories based on the weights configured in `remote.data.dirs.weights`. |
+| `remote.data.dirs.weights` | `[]` | ArrayList | The weights of the remote data directories. This is a list of weights corresponding to the `remote.data.dirs` in the same order. When `remote.data.dirs.strategy` is set to `WEIGHTED_ROUND_ROBIN`, this must be configured, and its size must be equal to `remote.data.dirs`; otherwise, it will be ignored. |
| `remote.fs.write-buffer-size` | `4 kb` | MemorySize | The default size of the write buffer for writing the local files to remote file systems. |
| `remote.log.task-interval-duration` | `1 min` | Duration | Interval at which remote log manager runs the scheduled tasks like copy segments, clean up remote log segments, delete local log segments etc. If the value is set to 0, it means that the remote log storage is disabled. |
+| `remote.log.task-max-upload-segments` | `5` | Integer | The maximum number of log segments to upload to remote storage per tiering task execution. This limits the upload batch size to prevent overwhelming the remote storage when there is a large backlog of segments to upload. |
Review Comment:
ditto
##########
website/docs/_configs/_partial_config.mdx:
##########
@@ -284,7 +288,7 @@
| `table.auto-partition.enabled` | `false` | Boolean | Whether enable auto partition for the table. Disable by default. When auto partition is enabled, the partitions of the table will be created automatically. |
| `table.auto-partition.key` | `none` | String | This configuration defines the time-based partition key to be used for auto-partitioning when a table is partitioned with multiple keys. Auto-partitioning utilizes a time-based partition key to handle partitions automatically, including creating new ones and removing outdated ones, by comparing the time value of the partition with the current system time. In the case of a table using multiple partition keys (such as a composite partitioning strategy), this feature determines which key should serve as the primary time dimension for making auto-partitioning decisions.And If the table has only one partition key, this config is not necessary. Otherwise, it must be specified. |
| `table.auto-partition.time-unit` | `DAY` | AutoPartitionTimeUnit | The time granularity for auto created partitions. The default value is `DAY`. Valid values are `HOUR`, `DAY`, `MONTH`, `QUARTER`, `YEAR`. If the value is `HOUR`, the partition format for auto created is yyyyMMddHH. If the value is `DAY`, the partition format for auto created is yyyyMMdd. If the value is `MONTH`, the partition format for auto created is yyyyMM. If the value is `QUARTER`, the partition format for auto created is yyyyQ. If the value is `YEAR`, the partition format for auto created is yyyy. |
-| `table.auto-partition.time-zone` | `Europe/Paris` | String | The time zone for auto partitions, which is by default the same as the system time zone. |
+| `table.auto-partition.time-zone` | `Asia/Shanghai` | String | The time zone for auto partitions, which is by default the same as the system time zone. |
Review Comment:
ditto
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLUSS-2686] Add COS filesystem support [fluss]
Posted by "leekeiabstraction (via GitHub)" <gi...@apache.org>.
leekeiabstraction commented on code in PR #2836:
URL: https://github.com/apache/fluss/pull/2836#discussion_r2956390315
##########
fluss-filesystems/fluss-fs-cos/src/main/java/org/apache/fluss/fs/cos/token/COSSecurityTokenProvider.java:
##########
@@ -0,0 +1,63 @@
+/*
+ * 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.fluss.fs.cos.token;
+
+import org.apache.fluss.fs.token.Credentials;
+import org.apache.fluss.fs.token.CredentialsJsonSerde;
+import org.apache.fluss.fs.token.ObtainedSecurityToken;
+
+import org.apache.hadoop.conf.Configuration;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.apache.fluss.fs.cos.COSFileSystemPlugin.ENDPOINT_KEY;
+
+/** A provider to provide Tencent Cloud COS security token. */
+public class COSSecurityTokenProvider {
+
+ private static final String SECRET_ID = "fs.cosn.userinfo.secretId";
Review Comment:
This string constant seems to be declared in a few classes. Consider sharing single pub static final?
##########
fluss-filesystems/fluss-fs-cos/src/test/java/org/apache/fluss/fs/cos/COSFileSystemBehaviorITCase.java:
##########
@@ -0,0 +1,63 @@
+/*
+ * 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.fluss.fs.cos;
+
+import org.apache.fluss.config.Configuration;
+import org.apache.fluss.fs.FileSystem;
+import org.apache.fluss.fs.FileSystemBehaviorTestSuite;
+import org.apache.fluss.fs.FsPath;
+
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+
+import java.util.UUID;
+
+/**
+ * An implementation of the {@link FileSystemBehaviorTestSuite} for the COS file system with Hadoop
+ * cos sdk.
+ */
+class COSFileSystemBehaviorITCase extends FileSystemBehaviorTestSuite {
Review Comment:
Was the IT test run successful with creds set? Would be good to indicate in pr description
##########
fluss-filesystems/fluss-fs-cos/pom.xml:
##########
@@ -0,0 +1,250 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ 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.
+-->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+ <parent>
+ <groupId>org.apache.fluss</groupId>
+ <artifactId>fluss-filesystems</artifactId>
+ <version>0.10-SNAPSHOT</version>
+ </parent>
+
+ <artifactId>fluss-fs-cos</artifactId>
+ <name>Fluss : FileSystems : COS FS</name>
+
+ <properties>
+ <fs.cosn.sdk.version>3.3.5</fs.cosn.sdk.version>
+ <fs.cos.api.version>5.6.139</fs.cos.api.version>
+ </properties>
+
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.fluss</groupId>
+ <artifactId>fluss-common</artifactId>
+ <version>${project.version}</version>
+ <scope>provided</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.apache.fluss</groupId>
+ <artifactId>fluss-fs-hadoop-shaded</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+
+ <dependency>
+ <groupId>org.apache.fluss</groupId>
+ <artifactId>fluss-fs-hadoop</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+
+ <dependency>
+ <groupId>org.apache.hadoop</groupId>
+ <artifactId>hadoop-cos</artifactId>
+ <version>${fs.cosn.sdk.version}</version>
+ <exclusions>
+ <exclusion>
+ <groupId>com.qcloud</groupId>
+ <artifactId>cos_api</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.apache.hadoop</groupId>
+ <artifactId>hadoop-common</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>ch.qos.reload4j</groupId>
+ <artifactId>reload4j</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.slf4j</groupId>
+ <artifactId>slf4j-reload4j</artifactId>
+ </exclusion>
+ </exclusions>
+ </dependency>
+
+ <dependency>
+ <groupId>com.qcloud</groupId>
+ <artifactId>cos_api</artifactId>
+ <version>${fs.cos.api.version}</version>
+ <exclusions>
+ <exclusion>
+ <groupId>javax.xml.bind</groupId>
+ <artifactId>jaxb-api</artifactId>
+ </exclusion>
+ </exclusions>
+ </dependency>
+
+ <dependency>
+ <!-- Hadoop requires jaxb-api for javax.xml.bind.JAXBException -->
+ <groupId>javax.xml.bind</groupId>
+ <artifactId>jaxb-api</artifactId>
+ <version>${jaxb.api.version}</version>
+ <!-- packaged as an optional dependency that is only accessible on Java 11+ -->
+ <scope>provided</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.apache.fluss</groupId>
Review Comment:
Should this be in test scope?
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLUSS-2686] Add COS filesystem support [fluss]
Posted by "XuQianJin-Stars (via GitHub)" <gi...@apache.org>.
XuQianJin-Stars commented on PR #2836:
URL: https://github.com/apache/fluss/pull/2836#issuecomment-4233492784
hi @leekeiabstraction @luoyuxia Hi, i already updated the pr. Please help review when you got some time.
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLUSS-2686] Add COS filesystem support [fluss]
Posted by "XuQianJin-Stars (via GitHub)" <gi...@apache.org>.
XuQianJin-Stars commented on code in PR #2836:
URL: https://github.com/apache/fluss/pull/2836#discussion_r3107987050
##########
fluss-filesystems/fluss-fs-cos/src/main/java/org/apache/fluss/fs/cos/COSFileSystemPlugin.java:
##########
@@ -0,0 +1,136 @@
+/*
+ * 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.fluss.fs.cos;
+
+import org.apache.fluss.annotation.VisibleForTesting;
+import org.apache.fluss.config.ConfigBuilder;
+import org.apache.fluss.config.Configuration;
+import org.apache.fluss.fs.FileSystem;
+import org.apache.fluss.fs.FileSystemPlugin;
+import org.apache.fluss.fs.cos.token.COSSecurityTokenReceiver;
+
+import org.apache.hadoop.fs.cosn.CosNFileSystem;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.net.URI;
+
+/** Simple factory for the Tencent Cloud COS file system. */
+public class COSFileSystemPlugin implements FileSystemPlugin {
+
+ private static final Logger LOG = LoggerFactory.getLogger(COSFileSystemPlugin.class);
+
+ public static final String SCHEME = "cosn";
+
+ /**
+ * In order to simplify, we make fluss cos configuration keys same with hadoop cos module. So,
+ * we add all configuration key with prefix `fs.cosn` in fluss conf to hadoop conf.
+ */
+ private static final String[] FLUSS_CONFIG_PREFIXES = {"fs.cosn."};
+
+ public static final String SECRET_ID = "fs.cosn.userinfo.secretId";
+ public static final String SECRET_KEY = "fs.cosn.userinfo.secretKey";
+ public static final String CREDENTIALS_PROVIDER = "fs.cosn.credentials.provider";
+
+ public static final String REGION = "fs.cosn.userinfo.region";
+
+ public static final String ENDPOINT_KEY = "fs.cosn.bucket.endpoint_suffix";
+
+ @Override
+ public String getScheme() {
+ return SCHEME;
+ }
+
+ @Override
+ public FileSystem create(URI fsUri, Configuration flussConfig) throws IOException {
+ org.apache.hadoop.conf.Configuration hadoopConfig = getHadoopConfiguration(flussConfig);
+
+ // set credential provider
+ if (hadoopConfig.get(SECRET_ID) == null) {
+ String credentialsProvider = hadoopConfig.get(CREDENTIALS_PROVIDER);
+ if (credentialsProvider != null) {
Review Comment:
> Do we need to have IT for this branch?
Good catch — I've added `COSWithCredentialsProviderFileSystemBehaviorITCase` in the latest commit to cover this `CREDENTIALS_PROVIDER` branch.
It reuses `org.apache.hadoop.fs.cosn.auth.EnvironmentVariableCredentialsProvider` shipped with hadoop-cos, which reads `COSN_SECRET_ID / COSN_SECRET_KEY` from the environment — exactly the same variables already required by `COSTestCredentials`. So no extra CI configuration is needed: whenever the existing COS ITs are enabled, this new one runs too. PTAL.
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLUSS-2686] Add COS filesystem support [fluss]
Posted by "XuQianJin-Stars (via GitHub)" <gi...@apache.org>.
XuQianJin-Stars commented on code in PR #2836:
URL: https://github.com/apache/fluss/pull/2836#discussion_r2970846657
##########
fluss-filesystems/fluss-fs-cos/src/test/java/org/apache/fluss/fs/cos/COSFileSystemBehaviorITCase.java:
##########
@@ -0,0 +1,63 @@
+/*
+ * 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.fluss.fs.cos;
+
+import org.apache.fluss.config.Configuration;
+import org.apache.fluss.fs.FileSystem;
+import org.apache.fluss.fs.FileSystemBehaviorTestSuite;
+import org.apache.fluss.fs.FsPath;
+
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+
+import java.util.UUID;
+
+/**
+ * An implementation of the {@link FileSystemBehaviorTestSuite} for the COS file system with Hadoop
+ * cos sdk.
+ */
+class COSFileSystemBehaviorITCase extends FileSystemBehaviorTestSuite {
Review Comment:
> Was the IT test run successful with creds set? Would be good to indicate in pr description
Yes, the IT tests have been run successfully with COS credentials configured.
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLUSS-2686] Add COS filesystem support [fluss]
Posted by "XuQianJin-Stars (via GitHub)" <gi...@apache.org>.
XuQianJin-Stars commented on PR #2836:
URL: https://github.com/apache/fluss/pull/2836#issuecomment-4570605540
> +1
Thanks for the review! If there are no further concerns, let's go ahead and merge it.
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLUSS-2686] Add COS filesystem support [fluss]
Posted by "XuQianJin-Stars (via GitHub)" <gi...@apache.org>.
XuQianJin-Stars commented on code in PR #2836:
URL: https://github.com/apache/fluss/pull/2836#discussion_r2970845603
##########
fluss-filesystems/fluss-fs-cos/pom.xml:
##########
@@ -0,0 +1,250 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ 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.
+-->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+ <parent>
+ <groupId>org.apache.fluss</groupId>
+ <artifactId>fluss-filesystems</artifactId>
+ <version>0.10-SNAPSHOT</version>
+ </parent>
+
+ <artifactId>fluss-fs-cos</artifactId>
+ <name>Fluss : FileSystems : COS FS</name>
+
+ <properties>
+ <fs.cosn.sdk.version>3.3.5</fs.cosn.sdk.version>
+ <fs.cos.api.version>5.6.139</fs.cos.api.version>
+ </properties>
+
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.fluss</groupId>
+ <artifactId>fluss-common</artifactId>
+ <version>${project.version}</version>
+ <scope>provided</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.apache.fluss</groupId>
+ <artifactId>fluss-fs-hadoop-shaded</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+
+ <dependency>
+ <groupId>org.apache.fluss</groupId>
+ <artifactId>fluss-fs-hadoop</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+
+ <dependency>
+ <groupId>org.apache.hadoop</groupId>
+ <artifactId>hadoop-cos</artifactId>
+ <version>${fs.cosn.sdk.version}</version>
+ <exclusions>
+ <exclusion>
+ <groupId>com.qcloud</groupId>
+ <artifactId>cos_api</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.apache.hadoop</groupId>
+ <artifactId>hadoop-common</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>ch.qos.reload4j</groupId>
+ <artifactId>reload4j</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.slf4j</groupId>
+ <artifactId>slf4j-reload4j</artifactId>
+ </exclusion>
+ </exclusions>
+ </dependency>
+
+ <dependency>
+ <groupId>com.qcloud</groupId>
+ <artifactId>cos_api</artifactId>
+ <version>${fs.cos.api.version}</version>
+ <exclusions>
+ <exclusion>
+ <groupId>javax.xml.bind</groupId>
+ <artifactId>jaxb-api</artifactId>
+ </exclusion>
+ </exclusions>
+ </dependency>
+
+ <dependency>
+ <!-- Hadoop requires jaxb-api for javax.xml.bind.JAXBException -->
+ <groupId>javax.xml.bind</groupId>
+ <artifactId>jaxb-api</artifactId>
+ <version>${jaxb.api.version}</version>
+ <!-- packaged as an optional dependency that is only accessible on Java 11+ -->
+ <scope>provided</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.apache.fluss</groupId>
Review Comment:
> Should this be in test scope?
The test scope for fluss-test-utils is already defined in the parent pom's dependencyManagement section (line 336 in root pom.xml), so child modules inherit it automatically without needing to declare <scope>test</scope> explicitly. This is consistent with how other filesystem modules (oss, s3, gs, azure, obs, hdfs) declare this dependency.
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLUSS-2686] Add COS filesystem support [fluss]
Posted by "simonsssu (via GitHub)" <gi...@apache.org>.
simonsssu commented on PR #2836:
URL: https://github.com/apache/fluss/pull/2836#issuecomment-4497595687
@leekeiabstraction Hi lee, I have tested this manually.
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLUSS-2686] Add COS filesystem support [fluss]
Posted by "leekeiabstraction (via GitHub)" <gi...@apache.org>.
leekeiabstraction commented on code in PR #2836:
URL: https://github.com/apache/fluss/pull/2836#discussion_r3071631445
##########
fluss-filesystems/fluss-fs-cos/src/main/java/org/apache/fluss/fs/cos/token/COSSecurityTokenProvider.java:
##########
@@ -0,0 +1,62 @@
+/*
+ * 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.fluss.fs.cos.token;
+
+import org.apache.fluss.fs.token.Credentials;
+import org.apache.fluss.fs.token.CredentialsJsonSerde;
+import org.apache.fluss.fs.token.ObtainedSecurityToken;
+
+import org.apache.hadoop.conf.Configuration;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.apache.fluss.fs.cos.COSFileSystemPlugin.ENDPOINT_KEY;
+import static org.apache.fluss.fs.cos.COSFileSystemPlugin.SECRET_ID;
+import static org.apache.fluss.fs.cos.COSFileSystemPlugin.SECRET_KEY;
+
+/** A provider to provide Tencent Cloud COS security token. */
+public class COSSecurityTokenProvider {
+
+ private final String endpoint;
+ private final String secretId;
+ private final String secretKey;
+
+ public COSSecurityTokenProvider(Configuration conf) {
+ endpoint = conf.get(ENDPOINT_KEY);
+ secretId = conf.get(SECRET_ID);
+ secretKey = conf.get(SECRET_KEY);
+ }
+
+ public ObtainedSecurityToken obtainSecurityToken(String scheme) {
+ // For COS, we directly use the configured secret id and secret key as the token.
+ // If STS temporary credentials are needed in the future, this can be extended
Review Comment:
I think we need to update PR description to be consistent with behaviour here
> COSSecurityTokenProvider: Generates temporary security tokens using Tencent Cloud STS (Security Token Service) based on existing secretId/secretKey configuration.
Please also create issue to track implementation of using STS to generate token/
Actually, why not implement as part of this PR? I'd like to understand the reasoning.
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org