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

Fix ACL check on health endpoint #17424

Merged
merged 3 commits into from
May 24, 2023
Merged

Conversation

hashi-derek
Copy link
Member

Prior to this change, the service health API would not explicitly return an error whenever a token with invalid permissions was given, and it would instead return empty results.

With this change, a "Permission denied" error is returned whenever data is queried. This is done to better support the agent cache, which performs a fetch backoff sleep whenever ACL errors are encountered.

Affected endpoints are: /v1/health/connect/ and /v1/health/ingress/.

Prior to this change, the service health API would not explicitly return an
error whenever a token with invalid permissions was given, and it would instead
return empty results.  With this change, a "Permission denied" error is returned
whenever data is queried. This is done to better support the agent cache, which
performs a fetch backoff sleep whenever ACL errors are encountered.  Affected
endpoints are: `/v1/health/connect/` and `/v1/health/ingress/`.
@hashi-derek hashi-derek marked this pull request as ready for review May 22, 2023 18:07
@hashi-derek hashi-derek requested a review from a team as a code owner May 22, 2023 18:07
Copy link
Contributor

@boruszak boruszak left a comment

Choose a reason for hiding this comment

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

LGTM!

I made a few suggestions, but feel free to make additional adjustments as needed. Let me know if you have any questions or need any additional review!

Approving on behalf of consul-docs.

.changelog/17424.txt Outdated Show resolved Hide resolved
website/content/docs/upgrading/upgrade-specific.mdx Outdated Show resolved Hide resolved
website/content/docs/upgrading/upgrade-specific.mdx Outdated Show resolved Hide resolved
website/content/docs/upgrading/upgrade-specific.mdx Outdated Show resolved Hide resolved
Co-authored-by: Jeff Boruszak <104028618+boruszak@users.noreply.github.com>
@hashi-derek hashi-derek merged commit a90c9ce into main May 24, 2023
108 checks passed
@hashi-derek hashi-derek deleted the derekm/NET-3881/health-check-err branch May 24, 2023 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants