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

[ACC-585] Verify vault secrets workflow #14

Merged
merged 76 commits into from
May 1, 2024

Conversation

Conor-TS
Copy link
Contributor

Relevant ticket: https://thescore.atlassian.net/browse/ACC-585

Adding a shared workflow to verify that environment variables referenced in Elixir projects have values defined for all environments and edges in Vault.

The action takes inputs (example):

  • service (identity)
  • edges ("['us-core']")
  • path_suffixes ("['common','kafka-worker','oban']")
  • environments ("['staging','demo','uat','audit1','ps','production']")
  • ignored_keys (APP_VERSION)
  • vault_addr_prod (https://vault.prod.thescore.is)
  • vault_add_non_prod (https://vault.non-prod.thescore.is)

It will use those inputs to generate a matrix of jobs that pull secret names from Vault and store them as artifacts. Then we check per edge and environment that the retrieved keys include environment variables that are referenced in files changed in the relevant PR.

It uses a JS action to compare the referenced environment variables to the secret names retrieved from Vault. That JS action is tested and added to CI for this repo.

@Conor-TS Conor-TS requested a review from a team as a code owner April 29, 2024 17:19
@@ -0,0 +1,55 @@
const envVarsRegex = /System\.fetch_env!\("([^"]+)"\)/g;

Choose a reason for hiding this comment

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

should we also check for System.fetch_env (without the bang) and System.get_env?

Choose a reason for hiding this comment

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

also I think this would fail in a cases like

generate_env_key()
|> System.fetch_env()
# or
|> Enum.map(&System.fetch_env/1)

Copy link

@svistoi svistoi Apr 29, 2024

Choose a reason for hiding this comment

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

I was wondering, how far can we get with heuristic like "all caps, numbers, underscore, at least 2 in length"

So we can just get rid of all fetch_env elixir prefixes.

[A-Z_0-9]{2,}

@@ -1,4 +1,4 @@
const envVarsRegex = /System\.fetch_env!\("([^"]+)"\)/g;
const envVarsRegex = /[A-Z0-9_]{2,}/g;

Choose a reason for hiding this comment

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

testing this out a lot of unrelated stuff comes up
Screenshot 2024-04-30 at 10 20 24

it almost works if we surround with quotes, but there are some exceptions still
Screenshot 2024-04-30 at 10 21 39

it would work if we only target runtime.exs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah. I updated the regex to catch fetch_env, fetch_env!, and get_env - how's that?

Choose a reason for hiding this comment

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

there's no button to resolve the comment weird, but looks good

@martin-yang1234 martin-yang1234 merged commit 972e724 into master May 1, 2024
7 checks passed
@martin-yang1234 martin-yang1234 deleted the ACC-465-verify-vault-secrets-workflow branch May 1, 2024 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants