[Feature][Config] Support custom config keys for encrypt/decrypt #8739

remones · 2025-02-16T09:16:41Z

Purpose of this pull request

support custom config keys for encrypt/decrypt
related issue: #8511

Does this PR introduce any user-facing change?

Yes. config encryption/decryption now supports shade.options, allowing users to add custom parameters that intend to be encrypted/decrypted.

How was this patch tested?

Exist tests

Check list

If any new Jar binary package adding in your PR, please add License Notice according
New License Guide
If necessary, please update the documentation to describe the new feature. https://github.com/apache/seatunnel/tree/dev/docs
If you are contributing the connector code, please check that the following files are updated:
1. Update plugin-mapping.properties and add new connector information in it
2. Update the pom file of seatunnel-dist
3. Add ci label in label-scope-conf
4. Add e2e testcase in seatunnel-e2e
5. Update connector plugin_config
Update the release-note.

Hisoka-X · 2025-02-17T02:49:23Z

Please follow the guide to open github action on your fork repository. https://github.com/apache/seatunnel/pull/8739/checks?check_run_id=37293359572

Hisoka-X · 2025-02-18T04:07:50Z

...nel-core-starter/src/main/java/org/apache/seatunnel/core/starter/utils/ConfigShadeUtils.java

        return ConfigFactory.parseMap(configMap);
    }

+    public static List<String> getSensitiveOptions(Config config) {


Use set as return type?

ok, I'll revise it, and do you have any other suggestions or comments?

overall LGTM.

litiliu · 2025-02-18T09:31:36Z

docs/en/connector-v2/Config-Encryption-Decryption.md

   env {
     parallelism = 1
     shade.identifier = "base64"
+     shade.options = ["username", "password", "f1", "config1.f1",  "config2.list"]


is it better to rename to shade.keys?

@litiliu I don't think so, shade.options may be clearer in config context, especially with default values.

litiliu · 2025-02-18T09:36:44Z

...nel-core-starter/src/main/java/org/apache/seatunnel/core/starter/utils/ConfigShadeUtils.java

+                                        : ConfigFactory.empty(),
+                                SHADE_OPTIONS_OPTION,
+                                new ArrayList<>()));
+        sensitiveOptions.addAll(new ArrayList<>(Arrays.asList(DEFAULT_SENSITIVE_KEYWORDS)));


we can directly use sensitiveOptions.addAll(Arrays.asList(DEFAULT_SENSITIVE_KEYWORDS));

litiliu · 2025-02-18T09:50:24Z

...tunnel-core-starter/src/main/java/org/apache/seatunnel/core/starter/utils/ConfigBuilder.java

-                            if (Arrays.asList(DEFAULT_SENSITIVE_KEYWORDS)
-                                    .contains(key.toLowerCase())) {
-                                m.put(key, "******");
+                            if (sensitiveKeywords.contains(key.toLowerCase())) {


Why is the toLowerCase needed here? As in the doc, we didn't specify whether we can match with the ignore case.

We don't care keywords case. The old behavior already convert to lowercase https://github.com/apache/seatunnel/pull/8739/files#diff-f4fa1a9b065718cc135d232712063b7b3a9135933396dd2674253da754d95c86L131. So I think we should keep it. cc @remones

Hisoka-X

LGTM. cc @hailin0

docs/en/connector-v2/Config-Encryption-Decryption.md

remones · 2025-02-19T08:02:40Z

I noticed that every time I push a new commit, the GitHub Actions test workflow runs for over an hour, and some e2e test cases are not stable so it may fail sometimes. do you have any suggestions for this? cc @Hisoka-X @hailin0

hailin0 · 2025-02-19T08:49:58Z

I noticed that every time I push a new commit, the GitHub Actions test workflow runs for over an hour, and some e2e test cases are not stable so it may fail sometimes. do you have any suggestions for this? cc @Hisoka-X @hailin0

If you are interested, you can fix the unstable test case

…che#8739) Co-authored-by: wanqifei <[email protected]>

github-actions bot added document core SeaTunnel core module api labels Feb 16, 2025

remones mentioned this pull request Feb 16, 2025

[Feature][Config] Support custom config keys for encrypt/decrypt #8511

Closed

3 tasks

Hisoka-X added the First-time contributor First-time contributor label Feb 17, 2025

remones force-pushed the feature/custom-encrypt-options branch 5 times, most recently from 94129d4 to 48f123d Compare February 17, 2025 09:59

Hisoka-X reviewed Feb 18, 2025

View reviewed changes

remones force-pushed the feature/custom-encrypt-options branch from 48f123d to f352f20 Compare February 18, 2025 07:48

litiliu reviewed Feb 18, 2025

View reviewed changes

remones force-pushed the feature/custom-encrypt-options branch from f352f20 to fe8d1e3 Compare February 19, 2025 02:19

Hisoka-X linked an issue Feb 19, 2025 that may be closed by this pull request

[Feature][Config] Support custom config keys for encrypt/decrypt #8511

Closed

3 tasks

remones force-pushed the feature/custom-encrypt-options branch from fe8d1e3 to a30c818 Compare February 19, 2025 03:14

Hisoka-X previously approved these changes Feb 19, 2025

View reviewed changes

github-actions bot added approved reviewed labels Feb 19, 2025

hailin0 reviewed Feb 19, 2025

View reviewed changes

docs/en/connector-v2/Config-Encryption-Decryption.md Show resolved Hide resolved

[Feature][Config] Support custom config keys for encrypt/decrypt

ef251fb

remones dismissed Hisoka-X’s stale review via ef251fb February 19, 2025 07:53

remones force-pushed the feature/custom-encrypt-options branch from a30c818 to ef251fb Compare February 19, 2025 07:53

github-actions bot removed approved reviewed labels Feb 19, 2025

hailin0 approved these changes Feb 19, 2025

View reviewed changes

github-actions bot added approved reviewed labels Feb 19, 2025

Hisoka-X approved these changes Feb 20, 2025

View reviewed changes

Hisoka-X merged commit b16aac2 into apache:dev Feb 20, 2025
7 checks passed

remones deleted the feature/custom-encrypt-options branch February 20, 2025 02:05

zax1314157 pushed a commit to zax1314157/seatunnel that referenced this pull request Feb 24, 2025

[Feature][Config] Support custom config keys for encrypt/decrypt (apa…

6c85da0

…che#8739) Co-authored-by: wanqifei <[email protected]>

[Feature][Config] Support custom config keys for encrypt/decrypt #8739

[Feature][Config] Support custom config keys for encrypt/decrypt #8739

Uh oh!

Conversation

remones commented Feb 16, 2025

Purpose of this pull request

Does this PR introduce any user-facing change?

How was this patch tested?

Check list

Uh oh!

Hisoka-X commented Feb 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hisoka-X left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

remones commented Feb 19, 2025

Uh oh!

hailin0 commented Feb 19, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants