Skip to content

Ability to specify health endpoint not-sensitive w/o disabling security #5750

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

Closed
dwelch2344 opened this issue Apr 20, 2016 · 10 comments
Closed
Labels
status: declined A suggestion or change that we don't feel we should currently apply

Comments

@dwelch2344
Copy link

Currently, the only way to get the health endpoint to show the details of a downed service is to set endpoints.health.sensitive=false and management.security.enabled=false

The latter seems overly restrictive to me. We'd still like to keep the majority of our endpoints (env, trace, etc) secured, but it'd be nice to have health give more details.

Maybe having a third state like sensitive, insensitive, and detailed (for lack of a better word) where the health beans could determine how much detail to provide based on setting?

Open to any suggestions, just know I've run up to this many times and others have shared my thoughts. Also, happy to contribute to whatever is determined to be the best course :)

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Apr 20, 2016
@philwebb philwebb added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Apr 21, 2016
@philwebb philwebb added this to the 1.4.0.M3 milestone Apr 21, 2016
@philwebb philwebb modified the milestones: 1.4.0.RC1, 1.4.0.M3 May 17, 2016
@snicoll
Copy link
Member

snicoll commented Jun 17, 2016

This table in the documentation summarizes the current situation. Given that sensitive is not going to change from a boolean to some Enum what would you suggest @dwelch2344 ?

I am asking because while I agree it would be nice to have, I fail to see how we could implement that consistently without more headache...

@dsyer
Copy link
Member

dsyer commented Jun 17, 2016

Seems like we ran out of time to get any change into 1.4. There's always a workaround if you want to set up your own security for /health. E.g. a quick and dirty way to do it is to just add /health to the security.ignored.

@dwelch2344
Copy link
Author

@snicoll Perhaps an additional option? Even just exposing the name of the health checks that failed would be enough. Basically a way to say "DB is down, but Rabbit + Redis are still looking good." without having to worry about authentication.

What if endpoints.health.publish-downed or something and another attribute on the response of downed: [ 'rabbit', 'db'] or something of the like? Could default to false but still be available to those of us who are less concerned about exposing those details publicly.

@dsyer I tried that quick and dirty a while back and ran into issues, as some of the common checks throw NPEs w/o a user in the context (forget which – need to log an issue for that independently either way now that I think about it)

@snicoll
Copy link
Member

snicoll commented Jun 21, 2016

I don't think we should be that fine-grained as it'll add more confusion for something that is already hard to grasp for beginners. What we need is essentially an additional flag that states "just show everything". I would very much prefer that sensitive becomes an enum but that's not going to happen in 1.4

@dwelch2344
Copy link
Author

Fair enough. I agree that's the best solution. We can definitely wait on this for now.

I'm happy to contribute a PR to this, if you all want to weigh in on how this should go about. Would the enum route apply only to health?

@snicoll
Copy link
Member

snicoll commented Jun 25, 2016

I don't know at this point. How about giving that a try and report here? Thanks!

@mbhave
Copy link
Contributor

mbhave commented Jul 28, 2017

#9721 will remove the need for having to disable all management security to display health details to authenticated users.

@wilkinsona
Copy link
Member

Duplicate of #9721

@wilkinsona wilkinsona marked this as a duplicate of #9721 Jul 29, 2017
@wilkinsona wilkinsona added the status: declined A suggestion or change that we don't feel we should currently apply label Jul 29, 2017
@snicoll snicoll removed priority: normal type: enhancement A general enhancement labels Jul 30, 2017
@ionutincau
Copy link

ionutincau commented Sep 21, 2017

Did anyone solved this? #5750 and #9721 are both duplicated to each other and both were closed.

@wilkinsona
Copy link
Member

@ionutincau This has been solved in 2.0 M4 by #9721. We now have /application/health which provides full health details and /application/status which provides a rolled-up view only showing the overall status.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

No branches or pull requests

8 participants