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

Support for vault namespaces #5520

Merged
merged 13 commits into from
Apr 6, 2019
Merged

Support for vault namespaces #5520

merged 13 commits into from
Apr 6, 2019

Conversation

cgbaker
Copy link
Contributor

@cgbaker cgbaker commented Apr 5, 2019

Updated Vault integration to add initial support for Vault namespaces.

Updates to:

  • Vault configuration stanza (server and client)
  • Vault client usage on server and client
  • consul-template configuration
  • numerous dependencies updates to support this
  • documentation

Note, the circonus-gometrics dependency in vendors was updated to work with the updated go-retryablehttp dependency. This is relatively low-risk for future updates, because it corrected a compile-error; if some future hypothetical update of circonus-gometrics doesn't compile, it will be pretty noticeable. Also, the change was a trivial function signature change:
https://github.com/hashicorp/nomad/pull/5520/files#r272587624

There's a question as to whether VAULT_NAMESPACE should be included on the default environment blacklist

Also, this apparently broke the vault e2e test in Travis; an endpoint used in the test suite changed in the Vault API; new versions of Vault are backwards compatible with old clients, but the new vault.api client is not compatible with old Vault servers. This endpoint isn't used in Nomad's Vault integration; it's only present as part of setup in the Vault e2e test suite. This has been modified to bypass the client in favor of a raw request against the previous API when testing against an older version of Vault. (I also added the latest versions of Vault to the test matrix.)

Chris Baker added 7 commits April 2, 2019 19:49
server/client: process `namespace` config, setting on the instantiated vault client
consul-template -> v0.20.0
consul/api -> v1.2.1
vault/api -> v1.0.3
go-retryablehttp -> v0.5.2
circonus-gometrics: modified local source for compat with go-retryablehttp
agent: added VAULT_NAMESPACE env-based configuration
…ation endpoint (old servers are not compatible with new client)
@cgbaker cgbaker requested a review from schmichael April 5, 2019 12:44
Copy link
Member

@schmichael schmichael left a comment

Choose a reason for hiding this comment

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

... circonus-gometrics ...

So the change are manually applied to our vendor? I think it'd be better to fork the repo to hashicorp, apply the update, use that in nomad, and submit a pr upstream. Looks like someone already reported this upstream, but there's no activity: circonus-labs/circonus-gometrics#94 (can be handled in a followup PR)

There's a question as to whether VAULT_NAMESPACE should be included on the default environment blacklist

Might this be necessary to include in task environments for services which use their Vault token to interact with Vault directly? (can be handled in a followup PR)

client/vaultclient/vaultclient_test.go Outdated Show resolved Hide resolved
command/agent/command.go Outdated Show resolved Hide resolved
nomad/structs/config/vault.go Outdated Show resolved Hide resolved
nomad/vault.go Outdated Show resolved Hide resolved
schmichael and others added 4 commits April 5, 2019 11:01
@cgbaker cgbaker merged commit c5f9c10 into 0.9.1-dev Apr 6, 2019
@cgbaker cgbaker deleted the f-nmd-1314-vault-namespaces branch April 6, 2019 01:01
@cgbaker
Copy link
Contributor Author

cgbaker commented Apr 12, 2019

There's a question as to whether VAULT_NAMESPACE should be included on the default environment blacklist

Might this be necessary to include in task environments for services which use their Vault token to interact with Vault directly? (can be handled in a followup PR)

Implemented in the following:
#5556

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants