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.

Sign up for GitHub

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

Jump to bottom

Add support for Enterprise License to be configured in Vault #1032

Merged

jmurret merged 21 commits into main from jm/vault-ent-license

Feb 18, 2022

Member

jmurret commented Feb 14, 2022 •

edited

Loading

Changes proposed in this PR:
For server-statefulset, client-agent-snapshot-deployment, and client-daemonset:

when vault is enabled as secrets backend, allow default behavior of retrieving kubernetes secret for enterprise license
when vault is enabled as secrets backend
- use vault injectors to get retrieve the license from vault and write it to disk
- use the license from disk to write the location of the vault injected license to the environment variable for the license. (server-statefulset and client-daemonset use CONSUL_LICENSE_PATH and client-agent-snapshot-deployment uses ENTERPRISE_LICENSE).
- do not create a volume or a volumeMount for the license since it is not coming from kubernetes.

How I've tested this PR:

unit tests
acceptance tests

How I expect reviewers to test this PR:

review code and test changes

Checklist:

Tests added
CHANGELOG entry added
Acceptance tests added
Docs PR to describe how to enable this behavior

Member Author

jmurret commented Feb 14, 2022

I'm most curious to talk through the VolumeMounts, how I should assume they will work, and whether those need to be modified.

Member Author

jmurret commented Feb 14, 2022

I've also found these failure validations for gossipEncryption key that I also implemented for enterpriseLicense in this PR.

This has caused these two tests to fail. So, I amwondering:

should I modify the old tests? modify the added logic? just forgo this logic in the PR?
should these failure validations also be in client-daemonset and client-snapshot-agent-deployment? (likewise client-daemonset does not have the validations for gossip key).

jmurret commented

View reviewed changes

charts/consul/templates/_helpers.tpl Show resolved Hide resolved

jmurret commented

View reviewed changes

charts/consul/test/unit/enterprise-license-job.bats Show resolved Hide resolved

jmurret commented

View reviewed changes

charts/consul/test/unit/enterprise-license-podsecuritypolicy.bats Show resolved Hide resolved

jmurret commented

View reviewed changes

charts/consul/test/unit/enterprise-license-role.bats Show resolved Hide resolved

jmurret commented

View reviewed changes

charts/consul/test/unit/enterprise-license-rolebinding.bats Show resolved Hide resolved

jmurret commented

View reviewed changes

charts/consul/test/unit/enterprise-license-serviceaccount.bats Show resolved Hide resolved

jmurret commented

View reviewed changes

charts/consul/test/unit/server-acl-init-job.bats Show resolved Hide resolved

jmurret force-pushed the jm/vault-ent-license branch from 29628f3 to a48d018 Compare

February 17, 2022 00:50

jmurret commented

View reviewed changes

acceptance/tests/vault/vault_test.go Outdated Show resolved Hide resolved

jmurret marked this pull request as ready for review

February 17, 2022 02:43

jmurret requested review from a team, kschoche and ishustava and removed request for a team

February 17, 2022 15:27

jmurret mentioned this pull request

Adding documentation to store Enterprise License in Vault hashicorp/consul#12375

Merged

kschoche reviewed

View reviewed changes

acceptance/tests/vault/vault_test.go Outdated Show resolved Hide resolved

jmurret force-pushed the jm/vault-ent-license branch from 57eeab2 to 3516257 Compare

February 17, 2022 16:59

ishustava reviewed

View reviewed changes

Contributor

ishustava left a comment

Looking pretty good, John! I left some comments in-line.

acceptance/tests/vault/vault_test.go Outdated Show resolved Hide resolved

acceptance/tests/vault/vault_test.go Outdated Show resolved Hide resolved

charts/consul/templates/client-daemonset.yaml Show resolved Hide resolved

charts/consul/templates/server-statefulset.yaml Show resolved Hide resolved

charts/consul/templates/server-statefulset.yaml Outdated Show resolved Hide resolved

jmurret force-pushed the jm/vault-ent-license branch 2 times, most recently from 82f9f48 to 6977124 Compare

February 17, 2022 21:14

jmurret added 8 commits

February 17, 2022 17:42


          Validate that users must set either both or neither enterpriseLicense…

81e5f4d

….secretName and secretKey


          Add ability to configure enterprise license tocome from vault

be31beb


          Add ability to configure enterprise license for client snapshot agent…

7c6ce90

… to come from vault


          Add ability to configure enterprise license for client daemonset to c…

8a244b8

…ome from vault


          Updating the comments in help for enterprise license to mention it ca…

6305ad5

…n also come from Vault.


          Update Changelog for Enterprise License on Vault

6cf183f


          Removing redundant tests in server-acl-init-job that test for existan…

f56aeb3

…ce of either both or neither enterprise license secretKey and secretName.


          Do not mount volume or volume mounts for license when using vault

9b6ad49

jmurret added 10 commits

February 17, 2022 17:42


          Removing redundant broken tests related to entperiseLicense not havin…

d2b845b

…g either secretKey or secretName when the other is supplied


          Adding test for that both global.enterpriseLicense.secretName and sec…

3112bb3

…retKey are provided when one of them is provided.


          Adding acceptance test for Enterprise License on vault

4e0dcac


          Fixing acceptance test for Enterprise License on vault

895849e


          Fixing ENTERPRISE_LICENSE setting enterprise-license-job for Enterpri…

d977a0b

…se License on vault


          Fixing formatting

6d86504


          Fixing unit tests

b702665


          Changing enterprise license logic in vault acceptance test to be cond…

888495b

…itional based on -enable-enterprise


          Making helm values for ent license conditional in vault accpentance t…

d7fca34

…est. Adding failure logic to client-daemonset.yaml if secretName or secretKey is missing if one of them is already supplied.


          checking on only enterpriseLicense.secretName (without secretKey) sin…

f729ed4

…ce chart will fail when both are not supplied.

jmurret force-pushed the jm/vault-ent-license branch from 0d765e2 to 6d483ee Compare

February 18, 2022 00:47


          Fixing broken test by allowing test suite config to be passed in so t…

cb6c3f8

…hat we can conditionally add consul-enterprise licence policy to the Vault Auth Roles.

jmurret force-pushed the jm/vault-ent-license branch from 6d483ee to cb6c3f8 Compare

February 18, 2022 00:50


          Adding license config in vault to TestVault_WANFederationViaGateways

0d43d9b

jmurret requested review from kschoche and ishustava

February 18, 2022 02:36

ishustava reviewed

View reviewed changes

Contributor

ishustava left a comment

A couple more comments about tests, but otherwise looks good!

acceptance/tests/vault/helpers.go Outdated Show resolved Hide resolved

charts/consul/test/unit/server-statefulset.bats Show resolved Hide resolved

jmurret requested a review from ishustava

February 18, 2022 18:59


          Adding failure tests to client-statefulset when only one of global.en…

172401f

…terpriseLicense.secretKey or secretName is supplied.

jmurret force-pushed the jm/vault-ent-license branch from d8562cb to 172401f Compare

February 18, 2022 19:03

ishustava approved these changes

View reviewed changes

Contributor

ishustava left a comment

jmurret merged commit ae206c9 into main

jmurret deleted the jm/vault-ent-license branch

February 18, 2022 19:58

jmurret added the vault label

geobeau pushed a commit to geobeau/consul-k8s that referenced this pull request


          update the wording for the consul sidecar to reflect current behaviour (

132f081

hashicorp#1032)

* update the wording for the consul sidecar to reflect current behaviour

Co-authored-by: Nitya Dhanushkodi <nitya@hashicorp.com>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels