Skip to content

🐙 octavia-cli: secret management #10885

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

Merged
merged 5 commits into from
Mar 9, 2022

Conversation

alafanechere
Copy link
Contributor

@alafanechere alafanechere commented Mar 6, 2022

What

We want secret management to avoid storing credentials as plain text.

How

  • Implement a custom YAML loader to expand env variables for node matching regex for string like ${MY_ENV_VAR}
  • Change the hashing logic:
    • Rename checksum to hash (because it's a hash and not a checksum...)
    • Compute the hash for a configuration after environment variable expansion. We now compute the hash on the deserialized yaml, not on the yaml file directly.

Recommended reading order

  1. octavia-cli/octavia_cli/apply/yaml_loaders.py
  2. octavia-cli/unit_tests/test_apply/test_yaml_loaders.py
  3. octavia-cli/octavia_cli/apply/resources.py
  4. octavia-cli/octavia_cli/templates/source_or_destination.yaml.j2
  5. octavia-cli/octavia_cli/generate/renderer.py

🚨 User Impact 🚨

Configuration can use ${MY_ENV_VAR} strings that will be expanded to the environment variable value at run time.

@alafanechere alafanechere temporarily deployed to more-secrets March 6, 2022 15:20 Inactive
@alafanechere alafanechere temporarily deployed to more-secrets March 6, 2022 15:20 Inactive
@alafanechere alafanechere temporarily deployed to more-secrets March 6, 2022 15:27 Inactive
@alafanechere alafanechere temporarily deployed to more-secrets March 6, 2022 15:27 Inactive
@alafanechere alafanechere temporarily deployed to more-secrets March 6, 2022 15:30 Inactive
@alafanechere alafanechere temporarily deployed to more-secrets March 6, 2022 15:30 Inactive
@alafanechere alafanechere force-pushed the augustin/octavia-cli/secret-management branch from 28ecc10 to 69e4eba Compare March 6, 2022 20:06
@alafanechere alafanechere temporarily deployed to more-secrets March 6, 2022 20:07 Inactive
@alafanechere alafanechere temporarily deployed to more-secrets March 6, 2022 20:07 Inactive
@alafanechere alafanechere added this to the octavia-cli-alpha milestone Mar 6, 2022
@alafanechere alafanechere marked this pull request as ready for review March 6, 2022 20:13
@alafanechere alafanechere temporarily deployed to more-secrets March 9, 2022 16:50 Inactive
@alafanechere alafanechere temporarily deployed to more-secrets March 9, 2022 16:51 Inactive
@alafanechere alafanechere temporarily deployed to more-secrets March 9, 2022 19:23 Inactive
@alafanechere alafanechere temporarily deployed to more-secrets March 9, 2022 19:23 Inactive
@alafanechere alafanechere temporarily deployed to more-secrets March 9, 2022 19:26 Inactive
@alafanechere alafanechere temporarily deployed to more-secrets March 9, 2022 19:26 Inactive
Copy link
Member

@marcosmarxm marcosmarxm left a comment

Choose a reason for hiding this comment

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

LGTM amazing @alafanechere

include_hash:
title: "Include hash"
description: "If true, include a hash with each data block."
include_checksum:
Copy link
Member

Choose a reason for hiding this comment

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

@alafanechere are you chancing to checksum?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here the original field name for this S3 destination connector was include_checksum. I mistakenly renamed it to include_hash while doing a brutal search and replace it when renaming other code occurrences of checksum to hash.

@alafanechere alafanechere merged commit ba4e86f into master Mar 9, 2022
@alafanechere alafanechere deleted the augustin/octavia-cli/secret-management branch March 9, 2022 20:35
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.

2 participants