Full-stack: Make "Server url" validation conditions consistent across Fleet, update Web Address form validation and submission logic per Fleet best practices (frontend/docs/patterns.md)#27455
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #27455 +/- ##
==========================================
- Coverage 63.78% 63.76% -0.03%
==========================================
Files 1742 1746 +4
Lines 165562 165854 +292
Branches 4534 4536 +2
==========================================
+ Hits 105605 105756 +151
- Misses 51731 51836 +105
- Partials 8226 8262 +36
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
frontend/patterns.md)
|
Okay, so: PR currently has everywhere on front and backend following the same rules, and validation + submission logic on the Web Address org settings form up-to-date with Fleet's best practices Before these updates, there were different conditions enforced in different places. The conditions for validation are now, everywhere:
If we want to refine these conditions, we can. Significantly - this adds the requirement of a valid scheme to the Web Address org settings form, unless the URL begins with "localhost". An alternative is to not require a scheme more generally, and apply that to all 4 places outlined in this PR description. Another alternative is to only require a scheme. This could interfere with any users who have saved Fleet server URLs without schemes, which @sgress454 and I confirmed is, while not a great practice and pretty unlikely, a possible scenario. |
frontend/patterns.md)
sgress454
left a comment
There was a problem hiding this comment.
LGTM, may as well hold off merging until after meeting w/ product later today.
| protocols, | ||
| allowAnyLocalHost = false, | ||
| }: IValidUrl): boolean => { | ||
| if (allowAnyLocalHost && url.includes("localhost")) return true; |
There was a problem hiding this comment.
This still allows "localhost.com" etc., which would then be blocked by the back end. I don't think it's worth the effort to handle that here before we talk to the product team about this though.
- Front and backend validation
- Any users with now invalid Fleet server URLs, e.g. "localhost:8080" (no protocol)
now get an informative error whenever they try to change another setting
| @@ -1,3 +1,3 @@ | |||
| const INVALID_SERVER_URL_MESSAGE = `Fleet server URL must use “https” or “http”.`; | |||
| const INVALID_SERVER_URL_MESSAGE = `Fleet server address must be a valid “https” or “http” URL.`; | |||
There was a problem hiding this comment.
This is because the frontend actually does validate both the scheme and a valid hostname
For #27454
Consider Fleet web URL to be valid if it:
and
Setup flow UI URL validation:
Org settings UI URL validation:
Server URL validation:
Invalid Fleet server URL in DB error:
changes/,