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

feat: backup vault k8s secrets to s3 encrypted. #142

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

artsyjian
Copy link
Collaborator

@artsyjian artsyjian commented Feb 23, 2025

This repo is public.

The type of this PR is: Feat

This PR solves PHIRE-1618

Description

  • Add script to backup Vault K8S secrets to S3. The secrets are AES256 encrypted.
    foreman run --env .env.shared,.env python src/vault_backup/main.py staging --s3
    
  • Add a cronjob for the script for each env, to be run daily.
  • Rename bucket name for Vault snapshot script from "BACKUP" to "SNAPSHOT", in order to not collide with the new script. (Other changes are just formatting)

Shared env has been updated, with instruction on how to decrypt.

Before-merging tasks:

  • Generate encryption key, one per env.
  • Update Staging env
  • Update Production env

@artsyjian artsyjian requested a review from cavvia February 23, 2025 21:55
@artsyjian artsyjian self-assigned this Feb 23, 2025
@artsy-peril artsy-peril bot added the Jira Synced Indicates that Peril has connected this PR to Jira label Feb 23, 2025
Copy link

@cavvia cavvia left a comment

Choose a reason for hiding this comment

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

Thanks for this! The non-standard formatting is a blocker. I have some questions about the layout of the code and your cli opts as well.

Copy link
Contributor

@mc-jones mc-jones left a comment

Choose a reason for hiding this comment

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

@artsyjian
Copy link
Collaborator Author

let's hold this PR pending this ticket.

@artsyjian artsyjian changed the title feat: backup vault k8s secrets in ascii feat: backup vault k8s secrets to s3 encrypted. Mar 5, 2025
@artsyjian artsyjian requested a review from cavvia March 11, 2025 15:39
Comment on lines +4 to +8
def symmetric_encrypt(key, iv, message):
"""AES256 encrypt message and return ciphertext
params are all str
ciphertext returned is in bytes
"""
Copy link

Choose a reason for hiding this comment

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

Suggested change
def symmetric_encrypt(key, iv, message):
"""AES256 encrypt message and return ciphertext
params are all str
ciphertext returned is in bytes
"""
def symmetric_encrypt(key: str, iv: str, message: str) -> bytes:
"""AES256 encrypt a message.
Returns
-------
ciphertext: bytes
"""

Copy link

@cavvia cavvia Mar 12, 2025

Choose a reason for hiding this comment

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

Switches to using a typed function signature and use a standard docstring format (numpydoc in this case, which is commonly used in python and IDEs support).

def validate(artsy_env, vault_host, vault_port, s3, s3_bucket):
"""validate config obtained from env and command line"""
if not (vault_host and vault_port):
raise Exception(
Copy link

@cavvia cavvia Mar 12, 2025

Choose a reason for hiding this comment

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

This and the following exceptions could use the built-in ValueError instead. Use of generic Exception is generally to be avoided in favour of more specific errors.

Copy link

@cavvia cavvia left a comment

Choose a reason for hiding this comment

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

A couple of non-blocking suggestions, otherwise looks good from a python coding conventions perspective.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Jira Synced Indicates that Peril has connected this PR to Jira
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants