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.

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

Detect Vault 1.11+ import, update default issuer #15253

Merged
merged 14 commits into from
Nov 17, 2022
Merged

Detect Vault 1.11+ import, update default issuer #15253

Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
eeed745
Detect Vault 1.11+ import, update default issuer
cipherboy Nov 3, 2022
1fd1f89
More commentary
cipherboy Nov 3, 2022
067113d
Merge branch 'main' into fix-issuer-growing-list-maybe-from-vault
kisunji Nov 15, 2022
7074c98
Refactored setting default intermediate issuer
kisunji Nov 15, 2022
e117b64
Fix commit message
kisunji Nov 15, 2022
0bd2b7d
Merge branch 'main' into fix-issuer-growing-list-maybe-from-vault
kisunji Nov 15, 2022
313c025
Address PR feedback
kisunji Nov 16, 2022
9b8613f
Minor formatting cleanup
kisunji Nov 16, 2022
80ae5c5
Merge branch 'main' into fix-issuer-growing-list-maybe-from-vault
kisunji Nov 16, 2022
1ff1189
Merge branch 'main' into fix-issuer-growing-list-maybe-from-vault
kisunji Nov 16, 2022
7625c05
Add unit test
kisunji Nov 17, 2022
37e0216
Remove unnecessary line
kisunji Nov 17, 2022
7647728
Merge branch 'main' into fix-issuer-growing-list-maybe-from-vault
kisunji Nov 17, 2022
90124a8
Add changelog
kisunji Nov 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions agent/connect/ca/provider_vault_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -902,6 +902,29 @@ func TestVaultProvider_ReconfigureIntermediateTTL(t *testing.T) {
require.Equal(t, 333*3600, mountConfig.MaxLeaseTTL)
}

func TestVaultCAProvider_GenerateIntermediate(t *testing.T) {
Copy link
Contributor

@kisunji kisunji Nov 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Unsure if Vault tests will run so I'm pasting local findings here:

Vault 1.10.8 without changes (expected to pass in Vault <1.11)

=== RUN   TestVaultCAProvider_RenewIntermediate
[INFO] freeport: detected ephemeral port range of [49152, 65535]
[INFO] freeport: reducing max blocks from 30 to 26 to avoid the ephemeral port range
[DEBUG] freeport: Test "TestVaultCAProvider_RenewIntermediate" took ports [25001 25002]
[INFO] agent/connect/ca: testing with vault server version: 1.10.8
[DEBUG] freeport: Test "TestVaultCAProvider_RenewIntermediate" returned ports [25001 25002]
--- PASS: TestVaultCAProvider_RenewIntermediate (3.33s)
PASS
ok      github.com/hashicorp/consul/agent/connect/ca    3.681s

Vault 1.10.8 with changes (regression test)

=== RUN   TestVaultCAProvider_RenewIntermediate
[INFO] freeport: detected ephemeral port range of [49152, 65535]
[INFO] freeport: reducing max blocks from 30 to 26 to avoid the ephemeral port range
[DEBUG] freeport: Test "TestVaultCAProvider_RenewIntermediate" took ports [20501 20502]
[INFO] agent/connect/ca: testing with vault server version: 1.10.8
[DEBUG] freeport: Test "TestVaultCAProvider_RenewIntermediate" returned ports [20501 20502]
--- PASS: TestVaultCAProvider_RenewIntermediate (1.33s)
PASS
ok      github.com/hashicorp/consul/agent/connect/ca    1.687s

Vault 1.11.0 without changes

=== RUN   TestVaultCAProvider_RenewIntermediate
[INFO] freeport: detected ephemeral port range of [49152, 65535]
[INFO] freeport: reducing max blocks from 30 to 26 to avoid the ephemeral port range
[DEBUG] freeport: Test "TestVaultCAProvider_RenewIntermediate" took ports [32501 32502]
[INFO] agent/connect/ca: testing with vault server version: 1.11.0
    provider_vault_test.go:918: 
                Error Trace:    /Users/chriskim/code/consul/agent/connect/ca/provider_vault_test.go:918
                Error:          Should not be: "-----BEGIN CERTIFICATE-----\nMIICMDCCAdWgAwIBAgIUE/pOjHz3fDoy9Fgwbw4DYt+8CwAwCgYIKoZIzj0EAwIw\nMDEuMCwGA1UEAxMlcHJpLWhoZDgxdWFjLnZhdWx0LmNhLjExMTExMTExLmNvbnN1\nbDAeFw0yMjExMTcxNzQyMjBaFw0yMzExMTcxNzQyNTBaMDAxLjAsBgNVBAMTJXBy\naS0xanRyZDZ3Yi52YXVsdC5jYS4xMTExMTExMS5jb25zdWwwWTATBgcqhkjOPQIB\nBggqhkjOPQMBBwNCAATKxc/IyicWyhgazqFINH2LHxTPwV6/oyzJJL8ZUqie2VHX\nWQvuVm+SQ7YHT8Hv2/wd9Ji43OIqF/D2iZWX1F9Po4HMMIHJMA4GA1UdDwEB/wQE\nAwIBBjAPBgNVHRMBAf8EBTADAQH/MB0GA1UdDgQWBBT3BpRIx1gItM2dvzxCxk6K\nGZK+NTAfBgNVHSMEGDAWgBS4Klwhx8S2YHuEwR2pZSQM/S2jMTBmBgNVHREEXzBd\ngiVwcmktMWp0cmQ2d2IudmF1bHQuY2EuMTExMTExMTEuY29uc3VshjRzcGlmZmU6\nLy8xMTExMTExMS0yMjIyLTMzMzMtNDQ0NC01NTU1NTU1NTU1NTUuY29uc3VsMAoG\nCCqGSM49BAMCA0kAMEYCIQCFguARdz2ebI4Qz48tmuXp1/VgE94u+8pJK4wuMYAe\nZgIhAM//Dqv3ofZrmRtJbIx6VgjV15C9KqVOQUhwMlRcTalY\n-----END CERTIFICATE-----\n"
                Test:           TestVaultCAProvider_RenewIntermediate
[DEBUG] freeport: Test "TestVaultCAProvider_RenewIntermediate" returned ports [32501 32502]
--- FAIL: TestVaultCAProvider_RenewIntermediate (1.82s)
FAIL
FAIL    github.com/hashicorp/consul/agent/connect/ca    2.215s
FAIL

Vault 1.11.0 with changes

=== RUN   TestVaultCAProvider_RenewIntermediate
[INFO] freeport: detected ephemeral port range of [49152, 65535]
[INFO] freeport: reducing max blocks from 30 to 26 to avoid the ephemeral port range
[DEBUG] freeport: Test "TestVaultCAProvider_RenewIntermediate" took ports [29501 29502]
[INFO] agent/connect/ca: testing with vault server version: 1.11.0
[DEBUG] freeport: Test "TestVaultCAProvider_RenewIntermediate" returned ports [29501 29502]
--- PASS: TestVaultCAProvider_RenewIntermediate (1.30s)
PASS
ok      github.com/hashicorp/consul/agent/connect/ca    1.660s

Also tested with Vault 1.12.1 with same results as 1.11.0

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for including this detail :)


SkipIfVaultNotPresent(t)

provider, testVault := testVaultProviderWithConfig(t, true, nil)
_ = testVault

orig, err := provider.ActiveIntermediate()
require.NoError(t, err)

Copy link
Contributor

Choose a reason for hiding this comment

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

Wow you weren't joking this did turn out to be easier than we thought! Nice work!

// This test was created to ensure that our calls to Vault
// returns a new Intermediate certificate and further calls
// to ActiveIntermediate return the same new cert.
new, err := provider.GenerateIntermediate()
require.NoError(t, err)

newActive, err := provider.ActiveIntermediate()
require.NoError(t, err)

require.Equal(t, new, newActive)
require.NotEqual(t, orig, new)
}

func getIntermediateCertTTL(t *testing.T, caConf *structs.CAConfiguration) time.Duration {
t.Helper()

Expand Down