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

backend: Prevent OIDC errors when not in use #2041

Merged
merged 1 commit into from
Jun 19, 2024
Merged

Conversation

skoeva
Copy link
Contributor

@skoeva skoeva commented Jun 14, 2024

This change prevents the spam of OIDC config errors in the logs when running the latest version of Headlamp without OIDC. Instead of logging an error when we check for the OIDC config, we simply return since we know we are not using OIDC auth.

Fixes: #1933

Testing

  • Build a custom headlamp image in minikube and point the kubernetes-headlamp.yaml to it
  • Apply the YAML (kubectl apply -f kubernetes-headlamp.yaml) and configure non-OIDC credentials
  • Run headlamp in-cluster and watch the logs for the new headlamp pod: kubectl logs -n kube-system <pod-name> -f
  • Open the app, click around, and ensure that the OIDC config errors do not appear

@skoeva skoeva linked an issue Jun 14, 2024 that may be closed by this pull request
@skoeva skoeva self-assigned this Jun 14, 2024
@skoeva skoeva marked this pull request as ready for review June 17, 2024 14:42
@skoeva skoeva requested review from knrt10 and yolossn June 17, 2024 14:44
@skoeva skoeva force-pushed the fix-oidc-config-spam branch from cf64c46 to dd564c0 Compare June 18, 2024 15:36
This change prevents the spam of OIDC config errors in the logs when
running the latest version of Headlamp without OIDC. Instead of logging
an error when we check for the OIDC config, we simply return since we
know we are not using OIDC auth.

Fixes: #1933

Signed-off-by: Evangelos Skopelitis <[email protected]>
@skoeva skoeva force-pushed the fix-oidc-config-spam branch from dd564c0 to ab2eb23 Compare June 18, 2024 15:37
@illume
Copy link
Collaborator

illume commented Jun 18, 2024

I think rather than disabling logging of the error... instead OIDCTokenRefreshMiddleware should not be used if oidc is not configured. If the config.oidcClientID is not set, don't add the OIDCTokenRefreshMiddleware.

What do you think?

@skoeva
Copy link
Contributor Author

skoeva commented Jun 18, 2024

I think rather than disabling logging of the error... instead OIDCTokenRefreshMiddleware should not be used if oidc is not configured. If the config.oidcClientID is not set, don't add the OIDCTokenRefreshMiddleware.

What do you think?

@illume the same check is performed on line 499, I think removing logging after 818 makes sense since it is assumed that OIDC auth is not being used by the comment on line 817. I'm not very familiar, but is there a reason to call OIDCTokenRefreshMiddleware when OIDC is not configured? line 818 assumes no config so that may need to be changed

@illume
Copy link
Collaborator

illume commented Jun 18, 2024

I'm not very familiar, but is there a reason to call OIDCTokenRefreshMiddleware when OIDC is not configured?

I don't think there's a reason to use OIDCTokenRefreshMiddleware when oidc is not configured. /cc @yolossn

@yolossn
Copy link
Contributor

yolossn commented Jun 19, 2024

I don't think there's a reason to use OIDCTokenRefreshMiddleware when oidc is not configured. /cc @yolossn

Since the middleware is for the cluster proxy endpoint we cannot conditionally add it, this is because there can be certain clusters with OIDC and other without and all of them connect to the cluster via the cluster proxy endpoint.

@illume
Copy link
Collaborator

illume commented Jun 19, 2024

Thanks @yolossn

Copy link
Collaborator

@illume illume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉🎈

thanks

@illume illume added bug Something isn't working backend Issues related to the backend labels Jun 19, 2024
@yolossn yolossn merged commit 6e49ed0 into main Jun 19, 2024
12 checks passed
@skoeva skoeva deleted the fix-oidc-config-spam branch June 19, 2024 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Issues related to the backend bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"failed to get oidc config" spam in logs
3 participants