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

Introduced new soa key for rotating database passwords #3655

Conversation

RobbySingh
Copy link
Contributor

@RobbySingh RobbySingh commented Jul 19, 2023

https://fluffy.yelpcorp.com/i/67hDPrWrG31CtcZRZf0RB17HqJg8XqXZ.html

Pasting in fluffy for security reasons

(updated Aug 9, 2023 @ 1:46pm)

- database_credentials is a top level key. The next key will be a datastore. This could be MySQL or Cassandra or anything else.
  Each data store (dstore) contains a name, eg {mysql: ['cred1', 'cred2']}.
  These credentials point to a vault path under /dstore/mysql/{cred1,cred2}
  These passwords are then mapped to the paasta service as a volume, where each file corresponds to a Vault entry
@RobbySingh RobbySingh requested a review from flopex July 19, 2023 16:57
@RobbySingh RobbySingh changed the title Introduced new soa key for rotating database passwords Introduced new soa key for rotating database passwords [WIP] Jul 19, 2023
@RobbySingh RobbySingh changed the title Introduced new soa key for rotating database passwords [WIP] Introduced new soa key for rotating database passwords Jul 19, 2023
paasta_tools/kubernetes/bin/paasta_secrets_sync.py Outdated Show resolved Hide resolved
paasta_tools/secret_providers/vault.py Outdated Show resolved Hide resolved
paasta_tools/kubernetes_tools.py Outdated Show resolved Hide resolved
paasta_tools/kubernetes_tools.py Outdated Show resolved Hide resolved
paasta_tools/kubernetes_tools.py Outdated Show resolved Hide resolved
paasta_tools/secret_providers/vault.py Outdated Show resolved Hide resolved
paasta_tools/secret_providers/vault.py Outdated Show resolved Hide resolved
paasta_tools/kubernetes/bin/paasta_secrets_sync.py Outdated Show resolved Hide resolved
paasta_tools/kubernetes/bin/paasta_secrets_sync.py Outdated Show resolved Hide resolved
- Used a context manager for temporarily setting vault credentials
- Several python magic upgrades
- Added sample return types for data
- Changed default parameters to required for dstore/creds
- Few refactors
@RobbySingh
Copy link
Contributor Author

I ran a fresh make setup-kubernetes-job with the vault credentials set in etc_paasta_playground.vault.json under database_credentials_vault_env_overrides (just vault_addr_override and vault_token_override) and confirmed that the env worked as expected

Copy link
Member

@nemacysts nemacysts left a comment

Choose a reason for hiding this comment

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

i still have to formulate some thoughts/suggestions re: tests - but for now i've left some other comments so that you're not blocked waiting for me to review

paasta_tools/kubernetes/bin/paasta_secrets_sync.py Outdated Show resolved Hide resolved
paasta_tools/kubernetes/bin/paasta_secrets_sync.py Outdated Show resolved Hide resolved
paasta_tools/kubernetes_tools.py Outdated Show resolved Hide resolved
paasta_tools/kubernetes_tools.py Outdated Show resolved Hide resolved
paasta_tools/secret_providers/vault.py Outdated Show resolved Hide resolved
paasta_tools/kubernetes_tools.py Outdated Show resolved Hide resolved
tests/kubernetes/bin/test_paasta_secrets_sync.py Outdated Show resolved Hide resolved
tests/test_kubernetes_tools.py Outdated Show resolved Hide resolved
paasta_tools/kubernetes/bin/paasta_secrets_sync.py Outdated Show resolved Hide resolved
- Changed database and dstore -> datastore for clarity
- set_env always resets the environ later, so it should instead be renamed to reflect this
paasta_tools/kubernetes_tools.py Outdated Show resolved Hide resolved
paasta_tools/secret_providers/vault.py Show resolved Hide resolved
paasta_tools/utils.py Outdated Show resolved Hide resolved
- Although we didn't specify cassandra here (since we don't know yet), we add additional_properties = True to suffice
paasta_tools/kubernetes_tools.py Outdated Show resolved Hide resolved
paasta_tools/secret_providers/vault.py Outdated Show resolved Hide resolved
@RobbySingh RobbySingh requested a review from tub August 9, 2023 16:28
paasta_tools/cli/schemas/kubernetes_schema.json Outdated Show resolved Hide resolved
paasta_tools/kubernetes_tools.py Outdated Show resolved Hide resolved
paasta_tools/kubernetes_tools.py Outdated Show resolved Hide resolved
paasta_tools/kubernetes_tools.py Outdated Show resolved Hide resolved
paasta_tools/kubernetes_tools.py Outdated Show resolved Hide resolved
paasta_tools/kubernetes_tools.py Show resolved Hide resolved
paasta_tools/kubernetes/bin/paasta_secrets_sync.py Outdated Show resolved Hide resolved
- Corrected 'help' for --secret-type call
@RobbySingh RobbySingh requested a review from mattmb August 10, 2023 20:26
@nemacysts nemacysts merged commit 8c62b00 into master Aug 17, 2023
6 checks passed
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.

5 participants