Skip to content
This repository has been archived by the owner on Nov 30, 2022. It is now read-only.

Pass in Vault environment variables if they are present locally #1275

Merged
merged 2 commits into from
Sep 8, 2022

Conversation

galvana
Copy link
Collaborator

@galvana galvana commented Sep 8, 2022

Purpose

To reduce the amount of steps in setting up a developer's Vault integration for connector development

Changes

  • Mapping the Vault environment variables in docker-compose.yml so they don't need to be mapped manually after running nox -s dev -- shell

Checklist

  • Update CHANGELOG.md file
    • Merge in main so the most recent CHANGELOG.md file is being appended to
    • Add description within the Unreleased section in an appropriate category. Add a new category from the list at the top of the file if the needed one isn't already there.
  • The Run Unsafe PR Checks label has been applied, and checks have passed, if this PR touches any external services

@galvana galvana added the run unsafe ci checks Triggers running of unsafe CI checks label Sep 8, 2022
Copy link
Contributor

@eastandwestwind eastandwestwind left a comment

Choose a reason for hiding this comment

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

Looks good, but mind updating changelog?

Comment on lines 28 to 30
- VAULT_ADDR=${VAULT_ADDR}
- VAULT_NAMESPACE=${VAULT_NAMESPACE}
- VAULT_TOKEN=${VAULT_TOKEN}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you try testing locally with this change:

Suggested change
- VAULT_ADDR=${VAULT_ADDR}
- VAULT_NAMESPACE=${VAULT_NAMESPACE}
- VAULT_TOKEN=${VAULT_TOKEN}
- VAULT_ADDR
- VAULT_NAMESPACE
- VAULT_TOKEN

I don't believe the placeholder values should be necessary (for any of these ENV vars).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That worked 👍

@eastandwestwind eastandwestwind merged commit 8278db4 into main Sep 8, 2022
@eastandwestwind eastandwestwind deleted the vault-environment-variables branch September 8, 2022 17:05
sanders41 pushed a commit that referenced this pull request Sep 22, 2022
* Pass in Vault environment variables if they are present locally

* Changelog and minor cleanup
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
run unsafe ci checks Triggers running of unsafe CI checks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants