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

Add options to pass redisPassword #216

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

AnthonyDurussel
Copy link

@AnthonyDurussel AnthonyDurussel commented Sep 15, 2021

Description

This is a

  • Bug Fix
  • Feature
  • Documentation
  • Other

Checklists

Commit style

  • Changes are on a branch with a descriptive name eg. fix/missing-queue, docs/setup-guide

  • Commits start with one of feat: fix: docs: chore: or similar

  • No excessive commits, eg: there should be no fix: commits for bugs that existed only on the PR branch (see guide-to-interactive-rebasing)

Protected files

The following files should not change unless they are directly a part of your change.

  • yarn.lock (unless package.json is also modified, then only the new/updated package should be changed here)

  • package.json (renovate bot should handle all routine updates)

  • package-lock.json (Should not exist as this project uses yarn)

  • tsconfig.json (only make it stricter, making it more lenient requires more discussion)

  • tslint.json (only make it stricter, making it more lenient requires more discussion)

@AnthonyDurussel
Copy link
Author

Hello,

I try to use your exporter with a Redis cluster that had a password with special characters.

Take as example : password = suds*$d@?2\

because of these characters, I was not able to connect the IoRedis. Even in using url encode.

So I develop a PR to allow passing Redis Password as an options

@autarchprinceps
Copy link

This can be added to the URL infront of the host, instead of requiring a separate env variable.

@AnthonyDurussel
Copy link
Author

But if you have any special characters in the password, you cannot add it to the URL. Even if you use url encode.

@@ -42,9 +44,11 @@ export class MetricCollector {
opts: MetricCollectorOptions,
registers: Registry[] = [globalRegister],
) {
const { logger, autoDiscover, redis, metricPrefix, ...bullOpts } = opts;
const { logger, autoDiscover, redis, redisPassword, metricPrefix, ...bullOpts } = opts;
logger.info('connection to redis ' + redis + ' with password ' + redisPassword);

Choose a reason for hiding this comment

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

it's a shame you log the password there... I had to make a fork just to remove it and re-publish

Copy link
Author

Choose a reason for hiding this comment

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

Used for debugging and forgot to remove it.

thanks @maxaggedon

@maxaggedon
Copy link

Other than special characters, having a separate environnement variable for password allows to retrieve it from a secret if you use this image in kubernetes

@AnthonyDurussel
Copy link
Author

hi @maxaggedon
I fixed the log the password. Can you merge this PR in the next release ?

@maxaggedon
Copy link

@AnthonyDurussel I reported a potential security issue because I saw it, but I don't have write access on this repo, sorry.

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

Successfully merging this pull request may close these issues.

3 participants