[feat]:Refactor KumaSDconfig#7379
[feat]:Refactor KumaSDconfig#7379Dharma-09 wants to merge 13 commits intoprometheus-operator:mainfrom
Conversation
|
I have closed Pull Request #7241 due to rebase conflicts. To ensure a clean integration, I have opened a new pull request with the necessary changes. |
mviswanathsai
left a comment
There was a problem hiding this comment.
Thanks for the effort! a few comments, but otherwise seems good to me.
|
Also, the You would also need to add the comment above the |
|
@mviswanathsai Thanks for reviewing it. |
|
@Dharma-09 Can you rebase, if everything looks good we may be able to merge this before release tomorrow |
| func (rs *ResourceSelector) validateKumaSDConfigs(ctx context.Context, sc *monitoringv1alpha1.ScrapeConfig) error { | ||
| for i, config := range sc.Spec.KumaSDConfigs { | ||
| if config.ClientID != nil && rs.version.LT(semver.MustParse("2.50.0")) { | ||
| return fmt.Errorf("field `clientID` is only supported for Prometheus version >= 2.50.0") |
There was a problem hiding this comment.
can we add the [%d] prefix like for other errors?
| return fmt.Errorf("[%d]: invalid scheme '%s'. Only 'http' and 'https' are supported", i, parsedURL.Scheme) | ||
| } | ||
|
|
||
| if len(parsedURL.Scheme) == 0 || len(parsedURL.Host) == 0 { |
There was a problem hiding this comment.
If I read the code properly, scheme can't be empty at this stage. Nit but in other places, we prefer to test against the empty string rather than the string length.
| if len(parsedURL.Scheme) == 0 || len(parsedURL.Host) == 0 { | |
| if parsedURL.Host == "" { |
| @@ -923,6 +923,8 @@ type KumaSDConfig struct { | |||
| // +required | |||
There was a problem hiding this comment.
would it make sense to use the URL type which would validate that the value starts by https?://?
|
@Dharma-09 Can you rebase and address code review comments? |
|
superseded by #7966. |
Description
This pull request refactors the validation logic for the KumaSDConfig struct in the Prometheus Operator. The changes include improving the handling of configuration fields such as server, ClientID, TLSConfig , ProxyURL,AllowRedirect.
Fixes
#7203
Type of change
What type of changes does your code introduce to the Prometheus operator? Put an
xin the box that apply.CHANGE(fix or feature that would cause existing functionality to not work as expected)FEATURE(non-breaking change which adds functionality)BUGFIX(non-breaking change which fixes an issue)ENHANCEMENT(non-breaking change which improves existing functionality)NONE(if none of the other choices apply. Example, tooling, build system, CI, docs, etc.)Verification
Please check the Prometheus-Operator testing guidelines for recommendations about automated tests.
Changelog entry
Please put a one-line changelog entry below. This will be copied to the changelog file during the release process.