add validation for name and description for model model group and connector resources #3805

dhrubo-os · 2025-04-29T19:29:42Z

add validation for name and description for model model group and connector resources

Description

resolved #3639

Integ test will fail as code coverages is failing for previously merged PR.

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

Check List

New functionality includes testing.
New functionality has been documented.
API changes companion pull request created.
Commits are signed per the DCO using --signoff.
Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

ylwu-amzn · 2025-04-29T19:36:52Z

common/src/main/java/org/opensearch/ml/common/transport/connector/MLCreateConnectorRequest.java

+
+        if (modelName != null && !isSafeText(modelName)) {
+            exception = addValidationError(
+                "Model connector name can only contain letters, digits, spaces, underscores (_), hyphens (-), and dots (.)",


This error message is different with MLUpdateConnectorRequest.java one. Should we keep them consistent ?

"Model connector name can only contain letters, digits, spaces, underscores (_), hyphens (-), and dots (.). Max length: 1000 characters.",

Yes, in the next revision I kept it consistent.

ylwu-amzn · 2025-04-29T19:37:48Z

common/src/main/java/org/opensearch/ml/common/transport/connector/MLUpdateConnectorRequest.java

+
+            if (modelName != null && !isSafeText(modelName)) {
+                exception = addValidationError(
+                    "Model connector name can only contain letters, digits, spaces, underscores (_), hyphens (-), and dots (.). Max length: 1000 characters.",


Don't see where validate Max length: 1000 characters." , Is that built-in logic in isSafeText?

My bad, I removed that restriction from the regex. But then I forgot to remove that from the error message.

Initially I was thinking to provide 1000 characters restriction.

What do you think? Should we add such kind of restriction?

Ignore the comment, find that internal logic in isSafeText

ylwu-amzn · 2025-04-29T19:39:23Z

common/src/main/java/org/opensearch/ml/common/transport/model/MLUpdateModelRequest.java

+        String modelName = updateModelInput.getName();
+        String description = updateModelInput.getDescription();
+
+        if (modelName != null && !isSafeText(modelName)) {


Similar checking in multiple places ? Can we build some common util method ?

Yeah, I thought about that and felt too lazy to refactor 😄. But I fixed it in the second revision. Thanks for insisting on the highest standards!

codecov · 2025-04-30T19:55:34Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.00%. Comparing base (0cba0ed) to head (5dfb719).

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #3805      +/-   ##
============================================
+ Coverage     77.99%   78.00%   +0.01%     
- Complexity     7316     7323       +7     
============================================
  Files           655      655              
  Lines         33032    33057      +25     
  Branches       3706     3703       -3     
============================================
+ Hits          25764    25787      +23     
- Misses         5681     5684       +3     
+ Partials       1587     1586       -1

Flag	Coverage Δ
ml-commons	`78.00% <100.00%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…nector resources Signed-off-by: Dhrubo Saha <[email protected]>

…undnancy Signed-off-by: Dhrubo Saha <[email protected]>

Signed-off-by: Dhrubo Saha <[email protected]>

dhrubo-os requested review from b4sjoo, mingshl, jngz-es, model-collapse, rbhavna, ylwu-amzn, zane-neo, Zhangxunmt, austintlee, HenryL27 and xinyual as code owners April 29, 2025 19:29

dhrubo-os had a problem deploying to ml-commons-cicd-env April 29, 2025 19:31 — with GitHub Actions Error

dhrubo-os had a problem deploying to ml-commons-cicd-env April 29, 2025 19:31 — with GitHub Actions Failure

dhrubo-os had a problem deploying to ml-commons-cicd-env April 29, 2025 19:31 — with GitHub Actions Error

dhrubo-os had a problem deploying to ml-commons-cicd-env April 29, 2025 19:31 — with GitHub Actions Failure

ylwu-amzn reviewed Apr 29, 2025

View reviewed changes

dhrubo-os had a problem deploying to ml-commons-cicd-env April 29, 2025 20:45 — with GitHub Actions Error

dhrubo-os had a problem deploying to ml-commons-cicd-env April 29, 2025 20:45 — with GitHub Actions Failure

dhrubo-os had a problem deploying to ml-commons-cicd-env April 29, 2025 20:45 — with GitHub Actions Error

dhrubo-os had a problem deploying to ml-commons-cicd-env April 29, 2025 20:45 — with GitHub Actions Failure

dhrubo-os force-pushed the input_validation branch from 0326043 to d8eeede Compare April 29, 2025 20:49

dhrubo-os had a problem deploying to ml-commons-cicd-env April 29, 2025 20:50 — with GitHub Actions Failure

dhrubo-os had a problem deploying to ml-commons-cicd-env April 29, 2025 20:50 — with GitHub Actions Error

dhrubo-os had a problem deploying to ml-commons-cicd-env April 29, 2025 20:50 — with GitHub Actions Failure

dhrubo-os had a problem deploying to ml-commons-cicd-env April 29, 2025 23:25 — with GitHub Actions Failure

dhrubo-os had a problem deploying to ml-commons-cicd-env April 29, 2025 23:25 — with GitHub Actions Error

dhrubo-os had a problem deploying to ml-commons-cicd-env April 29, 2025 23:25 — with GitHub Actions Failure

dhrubo-os had a problem deploying to ml-commons-cicd-env April 30, 2025 14:10 — with GitHub Actions Failure

dhrubo-os had a problem deploying to ml-commons-cicd-env April 30, 2025 14:10 — with GitHub Actions Error

dhrubo-os force-pushed the input_validation branch from de8895d to ae85e9d Compare April 30, 2025 18:51

dhrubo-os temporarily deployed to ml-commons-cicd-env April 30, 2025 18:53 — with GitHub Actions Inactive

dhrubo-os temporarily deployed to ml-commons-cicd-env April 30, 2025 19:56 — with GitHub Actions Inactive

dhrubo-os added 4 commits April 30, 2025 16:39

add validation for name and description for model model group and con…

a6923be

…nector resources Signed-off-by: Dhrubo Saha <[email protected]>

addressing comment, added a common method in utils to remove code red…

989edf5

…undnancy Signed-off-by: Dhrubo Saha <[email protected]>

making more flexible the regex pattern

3f0f81a

Signed-off-by: Dhrubo Saha <[email protected]>

removing restriction of 1000 characters

460af00

Signed-off-by: Dhrubo Saha <[email protected]>

dhrubo-os force-pushed the input_validation branch from ae85e9d to 460af00 Compare April 30, 2025 20:42

dhrubo-os temporarily deployed to ml-commons-cicd-env April 30, 2025 20:43 — with GitHub Actions Inactive

dhrubo-os temporarily deployed to ml-commons-cicd-env April 30, 2025 21:47 — with GitHub Actions Inactive

Merge branch 'main' into input_validation

5dfb719

dhrubo-os temporarily deployed to ml-commons-cicd-env May 6, 2025 23:40 — with GitHub Actions Inactive

dhrubo-os had a problem deploying to ml-commons-cicd-env May 7, 2025 00:43 — with GitHub Actions Failure

dhrubo-os temporarily deployed to ml-commons-cicd-env May 7, 2025 00:43 — with GitHub Actions Inactive

dhrubo-os deployed to ml-commons-cicd-env May 7, 2025 03:12 — with GitHub Actions Active

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add validation for name and description for model model group and connector resources #3805

add validation for name and description for model model group and connector resources #3805

dhrubo-os commented Apr 29, 2025 •

edited

Loading

ylwu-amzn Apr 29, 2025 •

edited

Loading

dhrubo-os Apr 29, 2025

ylwu-amzn Apr 29, 2025

dhrubo-os Apr 29, 2025

ylwu-amzn Apr 29, 2025

ylwu-amzn Apr 29, 2025

dhrubo-os Apr 29, 2025

codecov bot commented Apr 30, 2025 •

edited

Loading

add validation for name and description for model model group and connector resources #3805

Are you sure you want to change the base?

add validation for name and description for model model group and connector resources #3805

Conversation

dhrubo-os commented Apr 29, 2025 • edited Loading

Description

Related Issues

Check List

ylwu-amzn Apr 29, 2025 • edited Loading

Choose a reason for hiding this comment

dhrubo-os Apr 29, 2025

Choose a reason for hiding this comment

ylwu-amzn Apr 29, 2025

Choose a reason for hiding this comment

dhrubo-os Apr 29, 2025

Choose a reason for hiding this comment

ylwu-amzn Apr 29, 2025

Choose a reason for hiding this comment

ylwu-amzn Apr 29, 2025

Choose a reason for hiding this comment

dhrubo-os Apr 29, 2025

Choose a reason for hiding this comment

codecov bot commented Apr 30, 2025 • edited Loading

Codecov Report

dhrubo-os commented Apr 29, 2025 •

edited

Loading

ylwu-amzn Apr 29, 2025 •

edited

Loading

codecov bot commented Apr 30, 2025 •

edited

Loading