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

Initial support for wan fed using Vault as secrets backend #1016

Merged
merged 1 commit into from
Feb 17, 2022

Conversation

ishustava
Copy link
Contributor

@ishustava ishustava commented Feb 3, 2022

Changes proposed in this PR:

This is initial support for the Consul WAN federation which includes only gossip and TLS credentials. Other credentials will be added in a separate PR.

  • Modify mesh gateway deployment to use Consul server's CA when it's stored in Vault.
  • Add acceptance test for WAN federation with Vault
  • Refactor common functions that are now shared between vault tests as well as other tests

How I've tested this PR:
acceptance tests

How I expect reviewers to test this PR:
👀

Checklist:

  • Tests added
  • CHANGELOG entry added (will be added as one entry in the last PR in the wan federation with vault series)

    HashiCorp engineers only, community PRs should not add a changelog entry.
    Entries should use present tense (e.g. Add support for...)

@ishustava ishustava force-pushed the ishustava/vault-wan-fed branch 6 times, most recently from bf10149 to 1c03ab2 Compare February 9, 2022 00:22
@ishustava ishustava marked this pull request as ready for review February 9, 2022 00:26
@ishustava ishustava requested review from a team, jmurret and kschoche and removed request for a team February 9, 2022 00:27
@ishustava ishustava force-pushed the ishustava/vault-wan-fed branch from 1c03ab2 to b342d8c Compare February 9, 2022 15:42
"policies": fmt.Sprintf("consul-gossip,connect-ca,consul-server-%s", datacenter),
"ttl": "24h",
}
_, err = vaultClient.Logical().Write(fmt.Sprintf("auth/%s/role/consul-server", authPath), params)
Copy link
Contributor

Choose a reason for hiding this comment

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

No changes needed but I am thinking ahead to the documentation and wondering what you think about us suggesting that the server roles also have DC appended to help the user visually? Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤔 we could do that

Copy link
Contributor

@kschoche kschoche left a comment

Choose a reason for hiding this comment

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

This looks great! I love the cleanup and refactor of the setup stages for Vault, this is exactly how I'd envisioned it coming together once we started adding new test files!

I've left just a couple non-blocking comments!
Great work!

@ishustava ishustava force-pushed the ishustava/vault-wan-fed branch from b342d8c to 64cb04f Compare February 9, 2022 19:22
Copy link
Contributor

@thisisnotashwin thisisnotashwin left a comment

Choose a reason for hiding this comment

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

this is very impressive!! have a few questions but they are not blockers

acceptance/framework/k8s/helpers.go Show resolved Hide resolved
Comment on lines +20 to +21
// connectCAPolicy allows Consul to bootstrap all certificates for the service mesh in Vault.
// Adapted from https://www.consul.io/docs/connect/ca/vault#consul-managed-pki-paths.
Copy link
Contributor

Choose a reason for hiding this comment

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

🙏

@ishustava ishustava force-pushed the ishustava/vault-wan-fed branch from 64cb04f to d1c08cd Compare February 10, 2022 15:56
@ishustava ishustava merged commit 3ad09f7 into main Feb 17, 2022
@ishustava ishustava deleted the ishustava/vault-wan-fed branch February 17, 2022 16:20
geobeau pushed a commit to geobeau/consul-k8s that referenced this pull request May 20, 2022
Fork PRs don't have access to the enterprise license secret.
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.

3 participants