Add credential manager for external secret providers #63

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.

Sign up for GitHub

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

Jump to bottom

Closed

alberefe wants to merge 32 commits into chaoss:main from alberefe:feature/credential_manager

Contributor

alberefe commented Sep 5, 2025

Implement unified credential management system that supports retrieving secrets from Bitwarden, HashiCorp Vault, and AWS Secrets Manager.

Key features:

Factory pattern
Command-line interface
Python API
Full test coverage

The system allows secure credential management without hardcoding sensitive information in configuration files.

Updated README.md to include instructions.

jjmerchante requested changes

View reviewed changes

Contributor

jjmerchante left a comment

Overall, it looks good to me as it includes useful functionality, but it might need some changes that I detail below.

grimoirelab_toolkit/credential_manager/hc_manager.py Outdated Show resolved Hide resolved

grimoirelab_toolkit/credential_manager/bw_manager.py Outdated Show resolved Hide resolved

grimoirelab_toolkit/credential_manager/bw_manager.py Outdated Show resolved Hide resolved

grimoirelab_toolkit/credential_manager/hc_manager.py Outdated Show resolved Hide resolved

grimoirelab_toolkit/credential_manager/aws_manager.py Outdated Show resolved Hide resolved

grimoirelab_toolkit/credential_manager/bw_manager.py Outdated Show resolved Hide resolved

README.md Show resolved Hide resolved

Contributor Author

alberefe commented Sep 12, 2025

I'm on it, will update when I fix those.

Contributor Author

alberefe commented Sep 15, 2025

All the changes are up!

sduenas requested changes

View reviewed changes

Member

sduenas left a comment

I made some general comments to your PR. I think this is a nice functionality to have but there's room for improvement. Please address my comments and fix the flake8 errors reported too.

grimoirelab_toolkit/credential_manager/aws_manager.py Outdated Show resolved Hide resolved

grimoirelab_toolkit/credential_manager/aws_manager.py Outdated Show resolved Hide resolved

grimoirelab_toolkit/credential_manager/aws_manager.py Outdated Show resolved Hide resolved

grimoirelab_toolkit/credential_manager/aws_manager.py Outdated Show resolved Hide resolved

grimoirelab_toolkit/credential_manager/aws_manager.py Outdated Show resolved Hide resolved

grimoirelab_toolkit/credential_manager/bw_manager.py Show resolved Hide resolved

tests/test_hc_manager.py Outdated Show resolved Hide resolved

grimoirelab_toolkit/credential_manager/secrets_manager_factory.py Outdated Show resolved Hide resolved

Contributor Author

alberefe commented Oct 5, 2025

All changes should be up and running. I'll be waiting for any other corrections! :)

alberefe force-pushed the feature/credential_manager branch from 08b2f0f to f3e0133 Compare

October 7, 2025 15:36

alberefe added 24 commits

October 7, 2025 17:40


          Add credential manager for external secret providers

d9308a3

Implement unified credential management system that supports retrieving
secrets from Bitwarden, HashiCorp Vault, and AWS Secrets Manager.

Key features:
- Factory pattern
- Command-line interface
- Python API
- Full test coverage

The system allows secure credential management without hardcoding
sensitive information in configuration files.

Signed-off-by: [Your Name] <[your-email]> This is a combination of 2 commits.

Added credential_manager tool and tests.

Updated README.md to include credential_manager instructions,.

Signed-off-by: Alberto Ferrer Sánchez <alberefe@tutanota.com>


          Updated logging bw_manager.py

c1e088c

Signed-off-by: Alberto Ferrer Sánchez <alberefe@tutanota.com>


          Updated logging hc_manager.py

4c5ba5d

Signed-off-by: Alberto Ferrer Sánchez <alberefe@tutanota.com>


          Bitwarden now has to be installed and in path for the program to use it.

df4bc5e

Signed-off-by: Alberto Ferrer Sánchez <alberefe@tutanota.com>


          hc_manager.py raises exception if authentication fails

Signed-off-by: Alberto Ferrer Sánchez <alberefe@tutanota.com>


          It does not return an empty string anymore.

ec22cf6

Signed-off-by: Alberto Ferrer Sánchez <alberefe@tutanota.com>


          commented line that prints credentials retrieved

3f13072

Signed-off-by: Alberto Ferrer Sánchez <alberefe@tutanota.com>


          added return to main()

36fcbdd

Signed-off-by: Alberto Ferrer Sánchez <alberefe@tutanota.com>


          Unified logger.

19d5525

Signed-off-by: Alberto Ferrer Sánchez <alberefe@tutanota.com>


          fixed bw snap path

6e130ce

Signed-off-by: Alberto Ferrer Sánchez <alberefe@tutanota.com>


          fixed bw snap path

936817a

Signed-off-by: Alberto Ferrer Sánchez <alberefe@tutanota.com>


          fixed debug lvl argument

94e59d3

Signed-off-by: Alberto Ferrer Sánchez <alberefe@tutanota.com>


          Renamed datetime to datetime_toolkit so it does not cause import prob…

165ebf0

…lems.

Signed-off-by: Alberto Ferrer Sánchez <alberefe@tutanota.com>


          Added dependencies

db69efc

Signed-off-by: Alberto Ferrer Sánchez <alberefe@tutanota.com>


          Fixed logger naming

754627d

Signed-off-by: Alberto Ferrer Sánchez <alberefe@tutanota.com>


          Fixed exception complexity.

4e6b1b5

Signed-off-by: Alberto Ferrer Sánchez <alberefe@tutanota.com>


          Updated the docstring format to follow reStructuredText like the rest…

557cb89

… of the project.

Signed-off-by: Alberto Ferrer Sánchez <alberefe@tutanota.com>


          Added W504 to ignored like in Perceval.

98c6eb7

Signed-off-by: Alberto Ferrer Sánchez <alberefe@tutanota.com>


          Fixed flake8 errors.

71e5d1f

Signed-off-by: Alberto Ferrer Sánchez <alberefe@tutanota.com>


          Now raises error instead of returning an empty string.

855200f

Also updated docstring to reflect the change.

Signed-off-by: Alberto Ferrer Sánchez <alberefe@tutanota.com>


          Created new exceptions.py file to hold the custom exceptions of the p…

b20d422

…rogram.

Signed-off-by: Alberto Ferrer Sánchez <alberefe@tutanota.com>


          New exceptions in aws_manager

410376c

Signed-off-by: Alberto Ferrer Sánchez <alberefe@tutanota.com>


          New exceptions in bw_manager

864c51a

Signed-off-by: Alberto Ferrer Sánchez <alberefe@tutanota.com>


          new exceptions in hc_manager

14894d7

Signed-off-by: Alberto Ferrer Sánchez <alberefe@tutanota.com>

alberefe added 6 commits

October 7, 2025 17:41


          Added imports for new exceptions

88c48b9

Signed-off-by: Alberto Ferrer Sánchez <alberefe@tutanota.com>


          Fixed docstring

80982ac

Signed-off-by: Alberto Ferrer Sánchez <alberefe@tutanota.com>


          Fixed prompting andnow it's done in credential_manager.py

55df93a

Also fixed some docstring formatting.

Signed-off-by: Alberto Ferrer Sánchez <alberefe@tutanota.com>


          Fixed imports.

648720b

Signed-off-by: Alberto Ferrer Sánchez <alberefe@tutanota.com>


          Fixed typo

632121a

Signed-off-by: Alberto Ferrer Sánchez <alberefe@tutanota.com>


          Changed pytest to unittest to match the rest of the project.

d8cb01f

Signed-off-by: Alberto Ferrer Sánchez <alberefe@tutanota.com>

alberefe force-pushed the feature/credential_manager branch from 7d1b8a5 to d8cb01f Compare

October 7, 2025 15:41


          Merge branch 'main' into feature/credential_manager

78b0a9e

Contributor Author

alberefe commented Oct 7, 2025

I force-pushed stuff following the instructions that I found in DCO from the signoff problems. I am not sure this was the correct thing to do and would like some guidance / opinions on the matter and hope I'm not breaking anything.

alberefe requested review from jjmerchante and sduenas

October 8, 2025 11:25


          Merge branch 'main' into feature/credential_manager

d642003

sduenas requested changes

View reviewed changes

Member

sduenas left a comment

Can you squash your commits and create the commits only needed for this feature? It will be easier to review them.

grimoirelab_toolkit/credential_manager/bw_manager.py Show resolved Hide resolved

grimoirelab_toolkit/credential_manager/secrets_manager_factory.py Outdated Show resolved Hide resolved

grimoirelab_toolkit/credential_manager/__init__.py Show resolved Hide resolved

grimoirelab_toolkit/credential_manager/credential_manager.py Show resolved Hide resolved

grimoirelab_toolkit/credential_manager/exceptions.py Show resolved Hide resolved

sduenas reviewed

View reviewed changes

Member

sduenas left a comment

Why don't you open a new PR, with just the basic functionality, probably just the code needed to make it work with Bitwarden, and after we merge it, you add the functionality in different PRs for Amazon and Hashicorp Vault?

This way it would be easier to test and review and you can just push the necessary commits for it. I suggest you should rebase the commits, squashing them and keeping only the relevant code.

Please read this: https://github.com/chaoss/grimoirelab/blob/main/CONTRIBUTING_WITH_CODE.md#contributing-with-code

Contributor Author

alberefe commented Oct 16, 2025

Okay, I'll do a new PR. Just fixed the bw_manager.py and will open a new PR with the changes like you say. Will reread that document, sorry for not following all the things, first time doing something like this and it's being a little overwhelming.

Will follow your instructions after reading the document.

alberefe closed this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet