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

Health Check Endpoints Return 200 Despite Invalid Credentials #741

Open
2 of 3 tasks
poojareddy0328 opened this issue Dec 4, 2024 · 3 comments
Open
2 of 3 tasks

Comments

@poojareddy0328
Copy link

poojareddy0328 commented Dec 4, 2024

Pre issue-raising checklist

I have already (please mark the applicable with an x):

  • Upgraded to the latest Pact Broker OR
  • Checked the CHANGELOG to see if the issue I am about to raise has been fixed
  • Created an executable example that demonstrates the issue using either a:
    • Dockerfile
    • Git repository with a Travis or Appveyor (or similar) build

Software versions

Pactbroker docker image version 2.122.0-pactbroker2.112.0
Database: PostgreSQL (AWS RDS)
Deployment Platform: Openshift/Kubernates

Expected behavior

When the database credentials goes invalid, the /diagnostic/status/heartbeat and /diagnostic/status/dependencies endpoints should return an appropriate error status (e.g., 500) to indicate that the application is not healthy.

Actual behavior

When the database credentials goes invalid, the application throws 500 Internal Server Error for normal operations. the /diagnostic/status/heartbeat and /diagnostic/status/dependencies endpoints will return 200 OK, indicating the application is healthy, even thought it is unable to function properly due to invalid credentials.

Steps to reproduce

  1. Configure the Pact Broker with database credentials that rotate or become invalid.
  2. Wait the credentials to expire or become invalid.
  3. Access the pactbroker application.
    - Observe the application gives 500 Internal Server Error
  4. Access the /diagnostic/status/heartbeat and /diagnostic/status/dependencies endpoint
    - Observe that these endpoints return 200 OK despite 500 Internal Server Error from application.
@YOU54F
Copy link
Member

YOU54F commented Dec 6, 2024

Your expected behaviour isn't quite correct at least for /diagnostic/status/heartbeat

Let's take a look at the docs.

https://docs.pact.io/pact_broker/configuration/features#healthcheckheartbeat-url
https://docs.pact.io/pact_broker/configuration/settings#enable_diagnostic_endpoints

/diagnostic/status/heartbeat is not designed to access your database at all.

diagnostic/status/dependencies should check that the database is available, and connect to it.

b848ce3#diff-ae75b0267097077500d2c3597f335ce847f1cf76f10712dec9ad1321a6fb6a59R61

What do you get in the response from the /diagnostic/status/dependencies endpoint, when your creds have expired

@poojareddy0328
Copy link
Author

The response from the /diagnostic/status/dependencies endpoint returns true with 200 status and the pactbroker application throws 500 Internal Server Error
{"database":{"ok":true},"

@YOU54F
Copy link
Member

YOU54F commented Dec 6, 2024

I would be expecting it to hit this block

returning a tuple of false, report with a report containing the credential connection error

            report = {
              "ok" => false,
              "error" => {
                "message" => "#{e.class} - #{e.message}"
              }
            }

The first value of the tuple controls the response status

It calls sequels valid_connection method

https://www.rubydoc.info/gems/sequel/4.38.0/Sequel%2FDatabase:valid_connection%3F

I can't see whether that will check the credentials or just availability of the database endpoint. I assume it in the latter, which is why you aren't getting the error you expect.

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

No branches or pull requests

2 participants