14144 s3 endpoint field in cluster configuration missing input validation #14557
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
June 23, 2025 16:22
|
Note that the issue is currently targeting 2.13, and 2.12 priorities need to come first |
richard-cox
requested changes
Jun 26, 2025
| }; | ||
| }, | ||
|
|
||
| computed: { | ||
| ...mapGetters({ features: 'features/get' }), | ||
|
|
||
| s3ConfigComponent() { |
There was a problem hiding this comment.
this logic should be encapsulated inside the S3Config component and it's valid state can be emited out
- logic specific to inside a component stays inside the component
- the interface between parent and child components is clear, rather than parent reaching into child and using internal properties
| @@ -12,6 +16,7 @@ export default { | |||
| Checkbox, | |||
| SelectOrCreateAuthSecret, | |||
| }, | |||
| mixins: [CreateEditView, FormValidation], | |||
There was a problem hiding this comment.
the CreateEditView mixin is intended for root resources create/edit/view components rather than a sub component
| }; | ||
| </script> | ||
|
|
||
| <template> | ||
| <div> | ||
| <SelectOrCreateAuthSecret | ||
| v-model:value="config.cloudCredentialName" | ||
| :mode="mode" | ||
| :disable="isView" |
There was a problem hiding this comment.
:disable="isView" on this and below shouldn't be needed with :mode="mode"
shell/utils/validators/setting.js
Outdated
| return false; // Or throw new Error('Input is not a string'); | ||
| } | ||
|
|
||
| return value.toLowerCase().startsWith('https://') || value.toLowerCase().startsWith('http://'); |
There was a problem hiding this comment.
generally it's better to reduce duplication
const lowerCaseValue = value.toLowerCase();
return lowerCaseValue.startsWith('https://') || lowerCaseValue.startsWith('http://');
|
Hi @richard-cox |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
The core requirement was to prevent users from entering "http://" or "https://" at the beginning of the S3 endpoint URL in a form, and consequently, disable the main "Save" button of the cluster creation/edit form. Additionally, a tooltip should appear next to the endpoint field to inform the user about this specific error.
Fixes #14144
Occurred changes and/or fixed issues
Check if value typed on endpoint form is http:// or https://, in case of positive, return an error and disable the save button.
The save button was kept enable in view mode, the fix was to disable it in case the mode is view.
Technical notes summary
Areas or cases that should be tested
Rancher new cluster`s provisioning with etcd/s3 endpoint information
Browser: Opera 119.0.5497.88
Steps to reproduce:
Scenario 1)
Scenario 2)
Screenshot/Video
Screencast.From.2025-06-23.14-06-54.webm
Checklist