Skip to content
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

Do not create/update ServiceMonitor without verifying BasicAuth secrets #228

Closed
wants to merge 11 commits into from

Conversation

kabicin
Copy link
Contributor

@kabicin kabicin commented Jul 19, 2022

This change does not allow creation or the updating of the Service Monitor if BasicAuth secrets are specified in the WSLA instance but do not actually exist.

@kabicin kabicin requested a review from arturdzm July 19, 2022 17:58
@kabicin
Copy link
Contributor Author

kabicin commented Oct 20, 2022

This fix can also be delivered (or re-delivered) into RCO once application-stacks/runtime-component-operator#414 (Share controller code further) is merged, and then this WLO PR can update its controller code to call RCO utils. That way, the basicAuth fix can be applied to RCO and all extending operators rather than just WLO alone.

@arturdzm
Copy link
Contributor

arturdzm commented Apr 4, 2023

Looks good to me for basicAuth. However, we might want to expand this check for other secret types. There are few supported by Prometheus (oauth2, authorization, bearerTokenSecret)
Also this code can be better placed in RCO

- Validate Prometheus monitoring Secrets and ConfigMaps by breaking out of the reconcile loop if they do not exist.
- Prevent validation of Secrets/ConfigMaps if the KeySelector's Optional parameter is set to true.
@leochr
Copy link
Member

leochr commented Jun 6, 2023

@arturdzm please review the updates when you get a chance. Thanks

@kabicin kabicin removed the request for review from arturdzm October 25, 2023 16:27
@kabicin
Copy link
Contributor Author

kabicin commented Oct 25, 2023

Closing, because this change is replaced by PR #554

@kabicin kabicin closed this Oct 25, 2023
@kabicin kabicin deleted the bug-160 branch October 25, 2023 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants