-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[VTAdmin] Output warning log if cluster ID has invalid characters #16034
Conversation
Signed-off-by: Frances Thai <notfelineit@gmail.com>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16034 +/- ##
==========================================
- Coverage 68.40% 68.25% -0.16%
==========================================
Files 1556 1541 -15
Lines 195121 197170 +2049
==========================================
+ Hits 133479 134569 +1090
- Misses 61642 62601 +959 ☔ View full report in Codecov by Sentry. |
This PR is being marked as stale because it has been open for 30 days with no activity. To rectify, you may do any of the following:
If no action is taken within 7 days, this PR will be closed. |
This PR was closed because it has been stale for 7 days with no activity. |
Description
In VTAdmin, Cluster IDs are used as part of the gRPC resolver name. As such, they need to follow a specific URI scheme, as specified in https://datatracker.ietf.org/doc/html/rfc3986#section-3.1
Namely, Cluster IDs can only have alphanumeric characters, a
+
,.
, or-
and nothing else.The first step of Cluster ID validation is simply outputting a warning when bad characters are detected. VTAdmin simply will not work if someone passes an invalid Cluster ID (current behavior). This PR adds that warning.
Related Issue(s)
First part of #15721
Checklist
Deployment Notes