Skip to content

[improvement](fe) Add connector property validators - #66153

Draft
CalvinKirs wants to merge 1 commit into
apache:masterfrom
CalvinKirs:security_path
Draft

[improvement](fe) Add connector property validators#66153
CalvinKirs wants to merge 1 commit into
apache:masterfrom
CalvinKirs:security_path

Conversation

@CalvinKirs

Copy link
Copy Markdown
Member

What problem does this PR solve?

Issue Number: None

Related PR: None

Problem Summary:

ConnectorProperty currently describes property names, aliases, required fields, supported fields, and sensitive values, while property-specific value checks have to be implemented separately by each connector.

This PR adds a reusable validation extension point to the connector property binding framework. A validator receives the target object, annotated field, matched property name, converted value, and the complete raw property map. Validation runs after type conversion and before field assignment. Validator instances are stateless and reused across bindings.

To add validation to a connector property, implement ConnectorPropertyValidator:

public final class ExampleValidator implements ConnectorPropertyValidator {
    @Override
    public void validate(Object target, Field field, String propertyName,
            Object value, Map<String, String> properties) {
        if (!isValid(value)) {
            throw new IllegalArgumentException("Invalid property " + propertyName);
        }
    }
}

Then declare it on the property:

@ConnectorProperty(
        names = {"example.property"},
        validator = ExampleValidator.class)
private String exampleProperty;

Properties without a validator retain the existing behavior through the default no-op validator.

The PR also provides NoPathTraversalValidator as the first reusable implementation. It rejects parent-directory components in path values and comma-separated resource lists. Hadoop, JFS, OSS-HDFS, and Hive XML resource properties now use this validator.

Release note

Add connector property validators and validate parent-directory components in Hadoop and Hive XML resource paths.

Check List (For Author)

  • Test

    • Regression test
    • Unit Test
      • ./run-fe-ut.sh --run org.apache.doris.foundation.property.ConnectorPropertiesUtilsTest,org.apache.doris.filesystem.hdfs.properties.HdfsPropertiesTest
    • Manual test
    • No need to test or manual test.
  • Behavior changed:

    • No.
    • Yes. Hadoop and Hive XML resource properties reject parent-directory components.
  • Does this need documentation?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

### What problem does this PR solve?

Issue Number: None

Related PR: None

Problem Summary: Connector properties only supported binding and type conversion, so path-valued properties had to duplicate validation or accepted parent-directory references. Add a reusable field-level validator extension point and apply a traversal validator to Hadoop and Hive XML resource properties before assignment.

### Release note

Reject parent-directory references in Hadoop and Hive XML connector resource paths.

### Check List (For Author)

- Test: Unit Test
    - ./run-fe-ut.sh --run org.apache.doris.foundation.property.ConnectorPropertiesUtilsTest,org.apache.doris.filesystem.hdfs.properties.HdfsPropertiesTest
- Behavior changed: Yes, Hadoop and Hive XML resource paths containing a parent-directory component are rejected.
- Does this need documentation: No
@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@CalvinKirs

Copy link
Copy Markdown
Member Author

run buildall

@CalvinKirs

Copy link
Copy Markdown
Member Author

/review

@github-actions

Copy link
Copy Markdown
Contributor

Codex automated review failed and did not complete.

Error: Your access token could not be refreshed because your refresh token was revoked. Please log out and sign in again.
Workflow run: https://github.com/apache/doris/actions/runs/30338296757

Please inspect the workflow logs and rerun the review after the underlying issue is resolved.

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-H: Total hot run time: 29500 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit ce449af3b85dfc38e365c80bc635f069c9b9e68a, data reload: false

------ Round 1 ----------------------------------
============================================
q1	17870	4138	4342	4138
q2	2126	335	206	206
q3	10261	1457	792	792
q4	4683	485	339	339
q5	7530	844	568	568
q6	183	174	137	137
q7	828	815	608	608
q8	9384	1509	1624	1509
q9	5568	4327	4300	4300
q10	6766	1769	1499	1499
q11	521	352	328	328
q12	739	596	461	461
q13	18130	3925	2730	2730
q14	270	271	243	243
q15	q16	791	778	714	714
q17	898	914	965	914
q18	6770	5690	5606	5606
q19	1303	1352	1118	1118
q20	812	667	556	556
q21	5791	2626	2431	2431
q22	438	352	303	303
Total cold run time: 101662 ms
Total hot run time: 29500 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	4390	4318	4325	4318
q2	284	312	217	217
q3	4618	4977	4395	4395
q4	2136	2163	1377	1377
q5	4411	4313	4303	4303
q6	230	177	127	127
q7	1712	2029	1825	1825
q8	2633	2217	2178	2178
q9	7963	8118	7770	7770
q10	4829	4655	4208	4208
q11	576	426	386	386
q12	744	770	553	553
q13	3394	3575	2933	2933
q14	291	307	291	291
q15	q16	691	719	641	641
q17	1345	1338	1465	1338
q18	7966	7430	7206	7206
q19	1188	1094	1093	1093
q20	2208	2211	1923	1923
q21	5257	4580	4468	4468
q22	525	470	393	393
Total cold run time: 57391 ms
Total hot run time: 51943 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 175926 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit ce449af3b85dfc38e365c80bc635f069c9b9e68a, data reload: false

query5	4326	628	493	493
query6	460	218	210	210
query7	4917	585	347	347
query8	334	191	169	169
query9	8812	4028	4052	4028
query10	456	354	310	310
query11	5838	2325	2122	2122
query12	158	105	101	101
query13	1277	596	443	443
query14	6242	5186	4881	4881
query14_1	4228	4220	4224	4220
query15	217	203	181	181
query16	1027	509	438	438
query17	958	749	554	554
query18	2421	452	337	337
query19	200	185	144	144
query20	108	106	110	106
query21	233	154	136	136
query22	13580	13501	13384	13384
query23	17311	16505	16081	16081
query23_1	16270	16167	16208	16167
query24	7486	1757	1271	1271
query24_1	1302	1266	1277	1266
query25	545	444	354	354
query26	1372	350	202	202
query27	2598	601	364	364
query28	4483	1945	1936	1936
query29	1106	607	469	469
query30	344	262	223	223
query31	1104	1088	969	969
query32	108	61	59	59
query33	527	312	245	245
query34	1152	1138	668	668
query35	760	801	660	660
query36	1034	1032	887	887
query37	154	101	91	91
query38	1868	1714	1658	1658
query39	870	888	842	842
query39_1	853	849	848	848
query40	250	159	138	138
query41	63	69	62	62
query42	100	93	91	91
query43	330	316	276	276
query44	1443	761	738	738
query45	190	201	173	173
query46	1058	1216	747	747
query47	2130	2114	2016	2016
query48	406	452	295	295
query49	569	413	321	321
query50	1085	435	323	323
query51	10767	10511	10182	10182
query52	87	89	74	74
query53	272	269	199	199
query54	271	225	210	210
query55	72	69	65	65
query56	311	308	306	306
query57	1317	1293	1193	1193
query58	273	253	252	252
query59	1615	1713	1477	1477
query60	312	261	242	242
query61	145	147	150	147
query62	545	494	426	426
query63	240	205	194	194
query64	2858	1075	856	856
query65	4724	4605	4659	4605
query66	1844	498	419	419
query67	29259	28564	29192	28564
query68	3144	1499	917	917
query69	428	301	266	266
query70	897	834	785	785
query71	368	324	323	323
query72	3044	2627	2484	2484
query73	831	810	429	429
query74	5101	4873	4731	4731
query75	2523	2477	2145	2145
query76	2359	1196	774	774
query77	355	380	275	275
query78	11962	11907	11395	11395
query79	1202	1081	726	726
query80	601	554	445	445
query81	452	335	285	285
query82	237	157	121	121
query83	316	320	291	291
query84	310	161	126	126
query85	903	602	506	506
query86	280	243	233	233
query87	1820	1835	1728	1728
query88	3716	2789	2729	2729
query89	411	370	335	335
query90	2156	201	188	188
query91	202	192	162	162
query92	64	60	53	53
query93	1623	1554	967	967
query94	547	354	338	338
query95	794	522	460	460
query96	1111	798	344	344
query97	2623	2619	2496	2496
query98	210	209	199	199
query99	1090	1097	975	975
Total cold run time: 261699 ms
Total hot run time: 175926 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
ClickBench: Total hot run time: 24.96 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit ce449af3b85dfc38e365c80bc635f069c9b9e68a, data reload: false

query1	0.00	0.03	0.00
query2	0.09	0.04	0.04
query3	0.26	0.15	0.13
query4	1.61	0.13	0.13
query5	0.24	0.22	0.22
query6	1.27	1.05	1.07
query7	0.04	0.01	0.01
query8	0.06	0.04	0.04
query9	0.40	0.31	0.31
query10	0.55	0.59	0.55
query11	0.20	0.13	0.14
query12	0.17	0.14	0.14
query13	0.46	0.48	0.48
query14	1.01	0.99	1.01
query15	0.63	0.59	0.60
query16	0.32	0.33	0.34
query17	1.09	1.08	1.07
query18	0.22	0.20	0.21
query19	1.96	2.02	1.95
query20	0.02	0.02	0.01
query21	15.43	0.22	0.14
query22	4.80	0.05	0.04
query23	16.12	0.31	0.12
query24	3.06	0.42	0.33
query25	0.11	0.05	0.05
query26	0.75	0.20	0.15
query27	0.05	0.04	0.03
query28	3.54	0.97	0.53
query29	12.49	4.10	3.25
query30	0.27	0.16	0.15
query31	2.77	0.60	0.32
query32	3.22	0.60	0.48
query33	3.20	3.15	3.19
query34	15.56	4.50	3.51
query35	3.55	3.54	3.56
query36	0.58	0.43	0.44
query37	0.09	0.07	0.07
query38	0.05	0.05	0.03
query39	0.03	0.02	0.03
query40	0.18	0.17	0.16
query41	0.09	0.04	0.03
query42	0.04	0.03	0.03
query43	0.04	0.04	0.03
Total cold run time: 96.62 s
Total hot run time: 24.96 s

@hello-stephen

Copy link
Copy Markdown
Contributor

FE UT Coverage Report

Increment line coverage 100% (0/0) 🎉
Increment coverage report
Complete coverage report

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants