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

Pods logs at the default log level should make it easier to debug Supervisor ingress, SNI, and TLS certificate problems #1393

Closed
cfryanr opened this issue Jan 26, 2023 · 1 comment
Assignees
Labels
enhancement New feature or request estimate/S Estimated effort/complexity/risk is small state/accepted All done!

Comments

@cfryanr
Copy link
Member

cfryanr commented Jan 26, 2023

Is your feature request related to a problem? Please describe.

When the Supervisor's configuration has bad TLS certs, missing TLS certs, bad or missing incoming SNI information on requests, or other problems related to driving https traffic into the Supervisor pods, then the pod logs should contain helpful hints, even when at the default log level.

Describe the solution you'd like

Today, many of the helpful log statements are at the debug or trace level. Add more statements at the default level, or change some of the existing statements to be visible at the debug level. Make sure that at the default level there is not too much logging, and no sharing of sensitive information like request query params, etc.

Using only the default log level, an admin user should be able to tell:

  • Are the default and per-FederationDomain certificates being loaded without error?
  • On each incoming request, which certificate is being used, and why? Show the request path in so you can tell with which request the log statement corresponds.

Describe alternatives you've considered

None.

Are you considering submitting a PR for this feature?

  • How will this project improvement be tested?
  • How does this change the current architecture?
  • How will this change be backwards compatible?
  • How will this feature be documented?

This could be documented in https://pinniped.dev/docs/howto/configure-supervisor/, which could have a new troubleshooting section. It could also mention that when using an Ingress, make sure that the Ingress will pass the SNI information through to the Supervisor.

Additional context

None.

@pinniped-ci-bot pinniped-ci-bot added enhancement New feature or request priority/backlog Prioritized for an upcoming iteration labels Jan 26, 2023
@cfryanr
Copy link
Member Author

cfryanr commented Feb 9, 2023

This is already needed with a single FederationDomain, but note that this is also closely related to #1406, because making this easier to debug will be even more needed with multiple FederationDomains.

@pinniped-ci-bot pinniped-ci-bot added the estimate/S Estimated effort/complexity/risk is small label Sep 7, 2023
@pinniped-ci-bot pinniped-ci-bot added the state/started Someone is working on it currently label Sep 7, 2023
@pinniped-ci-bot pinniped-ci-bot added state/finished Code finished but not yet delivered and removed state/started Someone is working on it currently labels Sep 7, 2023
@pinniped-ci-bot pinniped-ci-bot added state/delivered Ready for manual acceptance review and removed state/finished Code finished but not yet delivered labels Sep 14, 2023
@pinniped-ci-bot pinniped-ci-bot added state/accepted All done! and removed priority/backlog Prioritized for an upcoming iteration state/delivered Ready for manual acceptance review labels Sep 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request estimate/S Estimated effort/complexity/risk is small state/accepted All done!
Projects
None yet
Development

No branches or pull requests

2 participants