/health 301 redirects issue - quarkus or MicroProfile spec issue?

[Max Rydahl Andersen]

Hey,

Adam Bien seemed to have bumped into the fix where we moved all non-app endpoints under /q for simpler security setup (i.e. just hide /q on incoming routes and you are fine).

Part of this we made MicroProfile endpoints return 301 redirects.

All should be good - but apparently there are some issues here.

1. the spec actually don't document 301 can happen - does that need clarification or is it assumed http clients will honour HTTP redirects ?

2. apparently there are implementations out there not honouring redirects. (stupid lazy defaults)

I assume we tested the 301 works - which setups did we test ?

should we get the spec clarified/updated?

/max
https://xam.dk/about

[Ken Finnigan]

We could look to modify the specs, but we may not get an agreement to do so.

As we're not rigid about MP TCK compatibility, do we worry about it?

--
You received this message because you are subscribed to the Google Groups "Quarkus Development mailing list" group.
To unsubscribe from this group and stop receiving emails from it, send an email to quarkus-dev...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/quarkus-dev/CCD4E370-0856-4900-8591-3104FE57459B%40redhat.com.

[Erin Schnabel]

Is this about 5he spec at all? Following redirects is at the scraper/scraping side, I believe?

While Adam is asking us not to drift from the MP Spec, I think we have been talking a lot about doing so. That said, we can see about updating the recommendation to allow for scoping.

Ken, was there an option to disable the ‘q’ namespace? I thought there was..

--

You received this message because you are subscribed to the Google Groups "Quarkus Development mailing list" group.
To unsubscribe from this group and stop receiving emails from it, send an email to quarkus-dev...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/quarkus-dev/CCD4E370-0856-4900-8591-3104FE57459B%40redhat.com.

--

Thanks,

Erin

 

------
Erin Schnabel <ebul...@redhat.com>

@ebullientworks

[Sergey Beryozkin]

Hi Max, Ken

IMHO this is not a spec level concern. If some browsers do not support redirects then there is a bigger problem - they won't work with any JAX-RS or indeed other endpoints doing the redirects.

The only thing that perhaps makes sense to revisit is the status - it should likely be 302.

`/q` is still there for the clients which are fine with `/q`.  `Moved Permanently` might have some side-effects to the way the browsers react to it. Perhaps I've missed the point of this 301 though...

Cheers, Sergey

To view this discussion on the web visit https://groups.google.com/d/msgid/quarkus-dev/CAKeeVe7Ci%3DgW0p-KQ6OYwuQYe%2BDqae2yVgeqo_VSEQ7sONz9Mg%40mail.gmail.com.

[Ken Finnigan]

It's somewhat spec related as some mandate what the URL endpoint for Health, for instance, should be available at and the TCK verifies it.

Agree we've talked about drifting from the spec where we see a benefit.

And yes, there is a way to customize /q, even setting it to / to remove it

To view this discussion on the web visit https://groups.google.com/d/msgid/quarkus-dev/CAJwjk9y0dc%2BC0h8pHBr6WrauPh1ozoF%3D_gV3oeLcZikRpTYLQA%40mail.gmail.com.

[Paul Carter-Brown]

We ran into this issue.

We use AWS ALB with target groups and health checks. The health check in AWS was pointing to /health and expected a 200 response. It does not follow a redirect - it just checks the response code. To fix we needed to set:

quarkus.http.redirect-to-non-application-root-path=false
quarkus.http.non-application-root-path=/

So this is not just about following redirect or not, more about a redirect not being a 200.

To view this discussion on the web visit https://groups.google.com/d/msgid/quarkus-dev/CAKeeVe6s6dx2u4KdDdbciF6pocwRhspgA%3DSRbLKUjpfF0A5W7g%40mail.gmail.com.

[Erin Schnabel]

Right -- that's what I thought.

This has nothing to do with browsers, and everything to do with automated use of /health and /metrics endpoints.

This is a no-win proposition: lots of people asking for endpoints to be scoped for administrative control, and lots of people that relied on the old endpoints. Maybe we shouldn't have switched the default to use /q, and instead made that an opt-in thing.

On Wed, Feb 10, 2021 at 9:18 AM Paul Carter-Brown <pa...@ukheshe.com> wrote:

We ran into this issue.

We use AWS ALB with target groups and health checks. The health check in AWS was pointing to /health and expected a 200 response. It does not follow a redirect - it just checks the response code. To fix we needed to set:

quarkus.http.redirect-to-non-application-root-path=false
quarkus.http.non-application-root-path=/

So this is not just about following redirect or not, more about a redirect not being a 200.

Ken -- is this the combination of options you would have expected? (e.g. are both required?)

[Max Rydahl Andersen]

> Hi Max, Ken
>
> IMHO this is not a spec level concern. If some browsers do not support
> redirects then there is a bigger problem - they won't work with any
> JAX-RS
> or indeed other endpoints doing the redirects.

this isn't about browsers :)

its that curl /health fails and so does default fetch using jaxrs or url
connection too.

IMO following HTTP spec should be assumed but reading the spec it
explicitly states status codes
and redirection is not there.

I'm fine keeping the behaviour but at least suggesting clarifying in
spec and at least
know if we checkedd kubernetes systems that actually does these checks
will honour 30x so
don't actually hurt users :)

>
> The only thing that perhaps makes sense to revisit is the status - it
> should likely be 302.
> `/q` is still there for the clients which are fine with `/q`. `Moved
> Permanently` might have some side-effects to the way the browsers
> react to
> it. Perhaps I've missed the point of this 301 though...
>
> Cheers, Sergey
>
> On Wed, Feb 10, 2021 at 1:03 PM Ken Finnigan <k...@kenfinnigan.me>
> wrote:
>
>> We could look to modify the specs, but we may not get an agreement to
>> do
>> so.
>>
>> As we're not rigid about MP TCK compatibility, do we worry about it?
>>
>> On Wed, Feb 10, 2021 at 4:41 AM Max Rydahl Andersen
>> <mand...@redhat.com>
>> wrote:
>>
>>> Hey,
>>>
>>> Adam Bien seemed to have bumped into

>>> <https://twitter.com/AdamBien/status/1359421483215446019> the fix

>>> where
>>> we moved all non-app endpoints under /q for simpler security setup
>>> (i.e.
>>> just hide /q on incoming routes and you are fine).
>>>
>>> Part of this we made MicroProfile endpoints return 301 redirects.
>>>
>>> All should be good - but apparently there are some issues here.
>>>

>>> 1.

>>>
>>> the spec actually don't document

>>> <https://download.eclipse.org/microprofile/microprofile-health-2.1/microprofile-health-spec.html#_appendix_a_rest_interfaces_specifications>

>>> 301 can happen - does that need clarification or is it assumed
>>> http clients
>>> will honour HTTP redirects ?

>>> 2.

>>>
>>> apparently there are implementations out there not honouring
>>> redirects. (stupid lazy defaults)
>>>
>>> I assume we tested the 301 works - which setups did we test ?
>>>
>>> should we get the spec clarified/updated?
>>>
>>> /max
>>> https://xam.dk/about
>>>
>>> --
>>> You received this message because you are subscribed to the Google
>>> Groups
>>> "Quarkus Development mailing list" group.
>>> To unsubscribe from this group and stop receiving emails from it,
>>> send an
>>> email to quarkus-dev...@googlegroups.com.
>>> To view this discussion on the web visit
>>> https://groups.google.com/d/msgid/quarkus-dev/CCD4E370-0856-4900-8591-3104FE57459B%40redhat.com

>>> <https://groups.google.com/d/msgid/quarkus-dev/CCD4E370-0856-4900-8591-3104FE57459B%40redhat.com?utm_medium=email&utm_source=footer>
>>> .

>>>
>> --
>> You received this message because you are subscribed to the Google
>> Groups
>> "Quarkus Development mailing list" group.
>> To unsubscribe from this group and stop receiving emails from it,
>> send an
>> email to quarkus-dev...@googlegroups.com.
>> To view this discussion on the web visit

>> https://groups.google.com/d/msgid/quarkus-dev/CAKeeVe7Ci%3DgW0p-KQ6OYwuQYe%2BDqae2yVgeqo_VSEQ7sONz9Mg%40mail.gmail.com
>> <https://groups.google.com/d/msgid/quarkus-dev/CAKeeVe7Ci%3DgW0p-KQ6OYwuQYe%2BDqae2yVgeqo_VSEQ7sONz9Mg%40mail.gmail.com?utm_medium=email&utm_source=footer>
>> .

>>
>
> --
> You received this message because you are subscribed to the Google
> Groups "Quarkus Development mailing list" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to quarkus-dev...@googlegroups.com.
> To view this discussion on the web visit

> https://groups.google.com/d/msgid/quarkus-dev/CAMsYBfX9R7i7y8-8Z98Nr73BExP1GmiG%3Dq6sYSMnFEC8z0bH7w%40mail.gmail.com.


/max
https://xam.dk/about

[Max Rydahl Andersen]

On 10 Feb 2021, at 16:19, Erin Schnabel wrote:

> Right -- that's what I thought.
>
> This has nothing to do with browsers, and everything to do with
> automated
> use of /health and /metrics endpoints.

yes. My ask was if spec should be updated and if we actually tested
these redirects works.

> This is a no-win proposition: lots of people asking for endpoints to
> be
> scoped for administrative control, and lots of people that relied on
> the
> old endpoints. Maybe we shouldn't have switched the default to use /q,
> and
> instead made that an opt-in thing.

If quarkus out of the box fails to get health checks in common systems
(Amazon being quite common)
then I think we need to reconsider this change for /health checks.
/max

> On Wed, Feb 10, 2021 at 9:18 AM Paul Carter-Brown <pa...@ukheshe.com>
> wrote:
>
>> We ran into this issue.
>>
>> We use AWS ALB with target groups and health checks. The health check
>> in
>> AWS was pointing to /health and expected a 200 response. It does not
>> follow
>> a redirect - it just checks the response code. To fix we needed to
>> set:
>> quarkus.http.redirect-to-non-application-root-path=false
>> quarkus.http.non-application-root-path=/
>>
>> So this is not just about following redirect or not, more about a
>> redirect
>> not being a 200.
>>
>>
>>
> Ken -- is this the combination of options you would have expected?
> (e.g.
> are both required?)
>

> --
> You received this message because you are subscribed to the Google
> Groups "Quarkus Development mailing list" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to quarkus-dev...@googlegroups.com.
> To view this discussion on the web visit

> https://groups.google.com/d/msgid/quarkus-dev/CAJwjk9w%3DMmEidEBN%3Dcax4-Wfzero3YoFYdJm2NGDVVmNWM3h%2BA%40mail.gmail.com.


/max
https://xam.dk/about

[Sergey Beryozkin]

Yeah, I've read about some consumers simply expecting 200...

Perhaps the other solution is to do the local forward with RequestDispatcher for servlet based endpoints and with Vert.x RequestContext - I believe one of the colleagues have tried the latter option (in a diff project)

Cheers, Sergey

[Max Rydahl Andersen]

Another downside of this change.

https://twitter.com/SileTwit/status/1359571073281499139

lots of redirects in access logs...

/max

/max
https://xam.dk/about

[Stuart Douglas]

IMHO we should just serve from both end points rather than redirect for 'automation focused endpoints' (i.e. endpoints that we expect will be generally accessed by something other than a browser).

Stuart

To view this discussion on the web visit https://groups.google.com/d/msgid/quarkus-dev/2E104C15-7862-4C55-96EE-408610326C08%40redhat.com.

[Max Rydahl Andersen]

On 10 Feb 2021, at 22:54, Stuart Douglas wrote:

> IMHO we should just serve from both end points rather than redirect
> for
> 'automation focused endpoints' (i.e. endpoints that we expect will be
> generally accessed by something other than a browser).

yeah, that is where I'm leaning.

I hadn't foreseen the access log noise this would give.

But what is the kicker is when googling for 30x redirects of healths and
seeing it affecting
amazon, gcp and some kubernetes distress being affected.

I could imagine an argument for health pings is that they are pinged so
often that they just
won't accept anything but status 200 for efficiency/simplicity.

/max

>> <https://groups.google.com/d/msgid/quarkus-dev/2E104C15-7862-4C55-96EE-408610326C08%40redhat.com?utm_medium=email&utm_source=footer>
>> .
>>


/max
https://xam.dk/about

[Max Rydahl Andersen]

On 10 Feb 2021, at 15:18, Paul Carter-Brown wrote:

> We ran into this issue.
>
> We use AWS ALB with target groups and health checks. The health check
> in
> AWS was pointing to /health and expected a 200 response. It does not
> follow
> a redirect - it just checks the response code. To fix we needed to
> set:
> quarkus.http.redirect-to-non-application-root-path=false
> quarkus.http.non-application-root-path=/

just checking but what prevented you from just changing the health check
in AWS to use /q/health ?

/max

>>>> <https://twitter.com/AdamBien/status/1359421483215446019> the fix

>>>> where
>>>> we moved all non-app endpoints under /q for simpler security setup
>>>> (i.e. just hide /q on incoming routes and you are fine).
>>>>
>>>> Part of this we made MicroProfile endpoints return 301 redirects.
>>>>
>>>> All should be good - but apparently there are some issues here.
>>>>

>>>> 1.

>>>>
>>>> the spec actually don't document

>>>> <https://download.eclipse.org/microprofile/microprofile-health-2.1/microprofile-health-spec.html#_appendix_a_rest_interfaces_specifications>

>>>> 301 can happen - does that need clarification or is it assumed
>>>> http clients
>>>> will honour HTTP redirects ?

>>>> 2.

>>>>
>>>> apparently there are implementations out there not honouring
>>>> redirects. (stupid lazy defaults)
>>>>
>>>> I assume we tested the 301 works - which setups did we test ?
>>>>
>>>> should we get the spec clarified/updated?
>>>>
>>>> /max
>>>> https://xam.dk/about
>>>>
>>>> --
>>>> You received this message because you are subscribed to the Google
>>>> Groups "Quarkus Development mailing list" group.
>>>> To unsubscribe from this group and stop receiving emails from it,
>>>> send
>>>> an email to quarkus-dev...@googlegroups.com.
>>>> To view this discussion on the web visit
>>>> https://groups.google.com/d/msgid/quarkus-dev/CCD4E370-0856-4900-8591-3104FE57459B%40redhat.com

>>>> <https://groups.google.com/d/msgid/quarkus-dev/CCD4E370-0856-4900-8591-3104FE57459B%40redhat.com?utm_medium=email&utm_source=footer>
>>>> .

>>>>
>>> --
>>> Thanks,
>>> Erin
>>>
>>> ------
>>> Erin Schnabel <ebul...@redhat.com>
>>> @ebullientworks
>>>
>>> --
>>> You received this message because you are subscribed to the Google
>>> Groups
>>> "Quarkus Development mailing list" group.
>>> To unsubscribe from this group and stop receiving emails from it,
>>> send an
>>> email to quarkus-dev...@googlegroups.com.
>>> To view this discussion on the web visit
>>> https://groups.google.com/d/msgid/quarkus-dev/CAJwjk9y0dc%2BC0h8pHBr6WrauPh1ozoF%3D_gV3oeLcZikRpTYLQA%40mail.gmail.com

>>> <https://groups.google.com/d/msgid/quarkus-dev/CAJwjk9y0dc%2BC0h8pHBr6WrauPh1ozoF%3D_gV3oeLcZikRpTYLQA%40mail.gmail.com?utm_medium=email&utm_source=footer>
>>> .

>>>
>> --
>> You received this message because you are subscribed to the Google
>> Groups
>> "Quarkus Development mailing list" group.
>> To unsubscribe from this group and stop receiving emails from it,
>> send an
>> email to quarkus-dev...@googlegroups.com.
>> To view this discussion on the web visit

>> https://groups.google.com/d/msgid/quarkus-dev/CAKeeVe6s6dx2u4KdDdbciF6pocwRhspgA%3DSRbLKUjpfF0A5W7g%40mail.gmail.com
>> <https://groups.google.com/d/msgid/quarkus-dev/CAKeeVe6s6dx2u4KdDdbciF6pocwRhspgA%3DSRbLKUjpfF0A5W7g%40mail.gmail.com?utm_medium=email&utm_source=footer>
>> .

>>
>
> --
> You received this message because you are subscribed to the Google
> Groups "Quarkus Development mailing list" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to quarkus-dev...@googlegroups.com.
> To view this discussion on the web visit

> https://groups.google.com/d/msgid/quarkus-dev/CAO8YPTSqHSbgtujzK7YTyEVrTNDgbs0MEJtHPTEYrUYeFwT3kA%40mail.gmail.com.


/max
https://xam.dk/about

[Paul Carter-Brown]

Hi Max

A few things played into this:

- we had /health already set up on the alb to allow external platforms to check health of our api. We cant just tell all partners to use a diff url 

- We do rolling deployments and changing to /q/health would cause all older service to now be seen as down causing a full outage

- We also have haproxy doing url rewriting for some services and hanging that would have involved reconfiguring haproxy and doing a deploy of that which again would cause downtime due to now seeing old pods as being down listening on /health

-We have multiple projects and aws accounts. Was muh easier to just change quarkus config and no other impact.

Could we have found a way to move to /q ... probably yes... but we just changed quarkus to act like it used to and all our problems went away.

Paul

[Max Rydahl Andersen]

Thanks for the details Paul!

The way I understand your comments then its an issue because you have an existing
deployment/setup which if Quarkus had used /q/health from the start that would have been fine -
or would you even then have moved it to /health because that is the convention you use everywhere
else anyway ?

/max

/max
https://xam.dk/about

[Paul Carter-Brown]

If it was nett-new I probably would just have adopted /q/health and all would have been fine.

[Phillip Krüger]

Maybe we should add a new 3rd config option, that allows devs to define the extensions that the /q namespace should apply on ? With the default being all we have now. But it allows devs to remove it for health, but keep it for OpenAPI and others as an example ? Because at the moment it's a all or nothing.

@Ken ?

[Max Rydahl Andersen]

On 11 Feb 2021, at 11:53, Phillip Krüger wrote:

Maybe we should add a new 3rd config option, that allows devs to define the
extensions that the /q namespace should apply on ? With the default being
all we have now. But it allows devs to remove it for health, but keep it
for OpenAPI and others as an example ? Because at the moment it's a all or
nothing.

There is now an issue on it at https://github.com/quarkusio/quarkus/issues/15030
lets continue there.

I think its important to remember that even though I MicroProfile spec is used as
reason to have them in place then that is really secondary. The issue is upgrades
of live systems will see their apps get killed as no /health end point and they
won't have any metrics to see what went wrong as /metrics is gone too.

that is the reality we need to deal with.

Fixing the spec issue is a sideeffect/secondary.

/max

To view this discussion on the web visit https://groups.google.com/d/msgid/quarkus-dev/29799563-14e1-4d5e-b314-ca8dc22f3890n%40googlegroups.com.

/max
https://xam.dk/about