Skip to content

Drop /status endpoint #11113

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
philwebb opened this issue Nov 22, 2017 · 13 comments
Closed

Drop /status endpoint #11113

philwebb opened this issue Nov 22, 2017 · 13 comments
Assignees
Labels
type: task A general task
Milestone

Comments

@philwebb
Copy link
Member

philwebb commented Nov 22, 2017

We've discussed the /health and /status endpoints quite a bit and we were planning to switch them. After some more thought I'm thinking a simple property to enabled/disable the full details on /health might be enough.

The status and health endpoint are so similar in code and concept that I think having them both might actually be confusing. I think offering a single endpoint with a switch might be enough for most users.

If a user is running behind a firewall they can set the property to expose more details. If they are not running behind a firewall they can either stick with hidden details, or configure securirty.

The only use-case we've not covered very well is someone getting an alert from an insecure /health and then wanting to get more details about which specific aspect caused the problem. That seems like a fairly edge case so I'm tempted to see of people complain first.

It's pretty easy for us to re-introduce a new endpoint later. It's much harder for us to take an existing one away.

@philwebb philwebb added this to the 2.0.0.M7 milestone Nov 22, 2017
@pkostrzewa
Copy link
Contributor

@philwebb could I try to switch these endpoints?

@philwebb philwebb self-assigned this Nov 22, 2017
@philwebb philwebb changed the title Switch /health and /status Drop /status endpoint Nov 22, 2017
@philwebb
Copy link
Member Author

@pkostrzewa Thanks for the offer, but I've already got this one in hand.

philwebb added a commit that referenced this issue Nov 23, 2017
@snicoll
Copy link
Member

snicoll commented Nov 23, 2017

That seems like a fairly edge case so I'm tempted to see of people complain first.

This is a feature of 1.5.x that our users are using (given the number of complains we got that the configuration part was confusing). I don't think dropping this is a good idea.

@snicoll snicoll added the for: team-attention An issue we'd like other members of the team to review label Nov 23, 2017
snicoll added a commit that referenced this issue Nov 23, 2017
@bclozel
Copy link
Member

bclozel commented Nov 27, 2017

Agree with @snicoll, I quite liked the separate endpoint as it made it easier to think about security and addressing two different use cases: automated health checks and incident response.

Now about the edge case side of this, I'd argue the other way around; unless you're deploying your app in an on-premise infrastructure and you can configure actuator on a separate port - any Spring Boot app serving public traffic would miss the health details.

Comparing this with our other issue (re: renaming the health endpoint), I think it's easier to reconfigure the path your health check client sends requests to (like AWS ELB health checks) than configuring it to send authenticated requests.

@philwebb
Copy link
Member Author

I quite liked the separate endpoint as it made it easier to think about security and addressing two different use cases: automated health checks and incident response.

I guess I'm still not convinced that hitting a secured version of /health to get the additional information is something that actually happens during incident response. I don't know, perhaps it is, I'm hoping that this change will cause people that care to shout.

If you think about a monitoring system like Pingdom, the chances are that you want the full details payload to be returned. You want some history to be stored in Pingdom so that if the status changes back to healthy you still know what caused the problem.

I really don't know which way to jump with this one. I still think having both /health and /status feels wrong.

@snicoll
Copy link
Member

snicoll commented Nov 28, 2017

We have another problem to deal with unfortunately. This change broke the CF integration as we don't show details by default anymore and I think we should as we deploy the endpoint in a separate space so that an administrator has full access regardless of how the app is configured.

(I've just pushed an app with master and I don't get any detail anymore).

@snicoll snicoll reopened this Nov 28, 2017
@dbacinski
Copy link

Separate endpoints for /health and /status is a good idea, one can be used with authentication by humans to audit environment status with details that can contain sensitive data and second one can be used by monitoring tools and stay public.

@philwebb
Copy link
Member Author

@dbacinski Do you currently rely on the existing Spring Boot 1.5 behavior where if you are authenticated you see more details?

philwebb added a commit that referenced this issue Nov 29, 2017
Align reactive and non-reactive web extensions and update `showDetails`
so that it only applies to web exposure.

See gh-11113
See gh-11192
@philwebb
Copy link
Member Author

I'm going to close this one again for now. I'm not 100% convinced that removing /status is the right call, but I'm sure it's going to be easier to add it later than take it away. The CF issue has been fixed in #11192 and I've added a new enhancement request to see if a /ping endpoint would be a worthwhile addition (#11191).

@dbacinski
Copy link

dbacinski commented Nov 29, 2017

I am using Spring Boot 2 M6 status and health enpoints. So what is a final result of this ticket? status is removed and only health with full data is available?

@wilkinsona
Copy link
Member

status is removed and health is available with or without details depending on the value of the management.endpoint.health.show-details configuration property.

@snicoll snicoll removed the for: team-attention An issue we'd like other members of the team to review label Nov 29, 2017
@HaVonTe1
Copy link

HaVonTe1 commented Jan 30, 2018

Hi. I was encouraged by @snicoll to describe my case here.
I have a lot of headache with the change behaviour of the 'health' endpoint.

We use docker swarm and sensu here to monitor our services. Both executing curl requests on /health and evaluating the response code. So no health indicator details necessary.
But as a developer I hardly need the details when something goes wrong and docker swarm is constantly killing my containers because status: down. Without the details I have no clue whats wrong.

So we secured actuator with basic auth. Un-authorized access from sensu / docker results in a handy http response code. Authorized requests from a developer showed the details. Perfect.

But know with Spring Boot 2 this feature is gone. My Admins refuse to make different curl requests separating SB1.5 und SB2.0 services.

Such a 'status' endpoint would solve my issue completely.

update
Showing the details with unauthorized access is sadly not an option. Our admins and security guys are concerned with exposing details like free disk space, version numbers of the rabbitmq, registered services on eureka and so on. I wont be able to argue about that.

update2
To make it more clear. It is sadly not an option to modify the healthcheck commands of docker and sensu to pass proper credentials. The devops refuse to do so. I know: a very poor argument.

@snicoll snicoll added for: team-attention An issue we'd like other members of the team to review and removed for: team-attention An issue we'd like other members of the team to review labels Jan 30, 2018
@snicoll
Copy link
Member

snicoll commented Jan 31, 2018

@HaVonTe1 Thanks for the feedback, we discussed about this during our team meeting today and we believe we should at least reconsider some form of support. I've created #11869

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: task A general task
Projects
None yet
Development

No branches or pull requests

7 participants