search

Purple-Devs / health_check

Simple health check of Rails app for use with uptime checking sites like newrelic and pingdom
MIT License
476 stars 125 forks source link

Remove default password for redis #108

Closed schm closed 1 year ago

schm commented 3 years ago

It seems, that there was an error when merging #103 to master in 6a4b72c3534483d311e373f64af6dcc6f9d870b5.

The current master 6cd2f91b522b7d4af16386f5a2e8f76300a8d6a2 (and the latest release) contains a default password for redis

https://github.com/ianheggie/health_check/blob/6cd2f91b522b7d4af16386f5a2e8f76300a8d6a2/lib/health_check.rb#L86-L87

while the original PR doesn't

https://github.com/ianheggie/health_check/pull/103/files#diff-1a44b837097f21eeaec4f487ab803a71df870ea355271e6e3838e81c8f614621R74-R75

This change makes our health checks fail (as we're not using password protection) with

[redis - ERR Client sent AUTH, but no password is set]

I've fixed that in this PR and replaced 'redis_password' with ENV['REDIS_PASSWORD'] as this is also documented in the README.

aedryan commented 3 years ago

Any way to get traction on this with the maintainer? I too am looking forward to a resolution here. Thanks for the fix!

aedryan commented 3 years ago

Any way to get traction on this with the maintainer? I too am looking forward to a resolution here. Thanks for the fix!

As a workaround to this, I found that including auth in the Redis URL and supplying this config helped us to workaround the issue.

config.redis_password = nil
schm commented 3 years ago

As a workaround to this, I found that including auth in the Redis URL and supplying this config helped us to workaround the issue.

config.redis_password = nil

We did exactly the same. I should have mentioned that in the PR description 🤦 . Thanks for adding this here. 💚

evolve2k commented 2 years ago

Tiny change fixes the issue. Looks good to merge.

marciondg commented 1 year ago

It happened to us in our app. It would be nice if it merged :pray: