luxolus
/
open-consul
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

fix TestLeader_SecondaryCA_IntermediateRenew (#8702) 

* fix lessThanHalfTime
* get lock for CAProvider()
* make a var to relate both vars
* rename to getCAProviderWithLock
* move CertificateTimeDriftBuffer to agent/connect/ca
This commit is contained in:
Hans Hasselberg 2020-09-18 10:13:29 +02:00 committed by GitHub
parent 8e974bdf3c
commit c6fa758d6f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 30 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
agent/connect/ca/provider_consul.go
View File

@ -21,6 +21,13 @@ import (
"github.com/hashicorp/go-hclog"
)
const (
// NotBefore will be CertificateTimeDriftBuffer in the past to account for
// time drift between different servers.
CertificateTimeDriftBuffer = time.Minute
)
var ErrNotInitialized = errors.New("provider not initialized")
type ConsulProvider struct {
@ -474,7 +481,7 @@ func (c *ConsulProvider) SignIntermediate(csr *x509.CertificateRequest) (string,
// Sign the certificate valid from 1 minute in the past, this helps it be
// accepted right away even when nodes are not in close time sync across the
// cluster. A minute is more than enough for typical DC clock drift.
effectiveNow := time.Now().Add(-1 * time.Minute)
effectiveNow := time.Now().Add(-1 * CertificateTimeDriftBuffer)
template := x509.Certificate{
SerialNumber: sn,
Subject: csr.Subject,

2
agent/consul/leader_connect.go
View File

@ -696,7 +696,7 @@ func (s *Server) secondaryIntermediateCertRenewalWatch(ctx context.Context) erro
return fmt.Errorf("error parsing active intermediate cert: %v", err)
}
if lessThanHalfTimePassed(time.Now(), intermediateCert.NotBefore,
if lessThanHalfTimePassed(time.Now(), intermediateCert.NotBefore.Add(ca.CertificateTimeDriftBuffer),
intermediateCert.NotAfter) {
return nil
}

35
agent/consul/leader_connect_test.go
View File

@ -100,8 +100,8 @@ func TestLeader_SecondaryCA_Initialize(t *testing.T) {
err error
)
retry.Run(t, func(r *retry.R) {
_, caRoot = s1.getCAProvider()
secondaryProvider, _ = s2.getCAProvider()
_, caRoot = getCAProviderWithLock(s1)
secondaryProvider, _ = getCAProviderWithLock(s2)
intermediatePEM, err = secondaryProvider.ActiveIntermediate()
require.NoError(r, err)
@ -165,7 +165,7 @@ func TestLeader_SecondaryCA_Initialize(t *testing.T) {
func waitForActiveCARoot(t *testing.T, srv *Server, expect *structs.CARoot) {
retry.Run(t, func(r *retry.R) {
_, root := srv.getCAProvider()
_, root := getCAProviderWithLock(srv)
if root == nil {
r.Fatal("no root")
}
@ -175,6 +175,12 @@ func waitForActiveCARoot(t *testing.T, srv *Server, expect *structs.CARoot) {
})
}
func getCAProviderWithLock(s *Server) (ca.Provider, *structs.CARoot) {
s.caProviderReconfigurationLock.Lock()
defer s.caProviderReconfigurationLock.Unlock()
return s.getCAProvider()
}
func TestLeader_SecondaryCA_IntermediateRenew(t *testing.T) {
// no parallel execution because we change globals
origInterval := structs.IntermediateCertRenewInterval
@ -227,7 +233,8 @@ func TestLeader_SecondaryCA_IntermediateRenew(t *testing.T) {
testrpc.WaitForLeader(t, s2.RPC, "dc2")
// Get the original intermediate
secondaryProvider, _ := s2.getCAProvider()
// TODO: Wait for intermediate instead of wait for leader
secondaryProvider, _ := getCAProviderWithLock(s2)
intermediatePEM, err := secondaryProvider.ActiveIntermediate()
require.NoError(err)
cert, err := connect.ParseCert(intermediatePEM)
@ -253,7 +260,7 @@ func TestLeader_SecondaryCA_IntermediateRenew(t *testing.T) {
// however, defaultQueryTime will be configurable and we con lower it
// so that it returns for sure.
retry.Run(t, func(r *retry.R) {
secondaryProvider, _ := s2.getCAProvider()
secondaryProvider, _ = getCAProviderWithLock(s2)
intermediatePEM, err = secondaryProvider.ActiveIntermediate()
r.Check(err)
cert, err := connect.ParseCert(intermediatePEM)
@ -266,9 +273,9 @@ func TestLeader_SecondaryCA_IntermediateRenew(t *testing.T) {
})
require.NoError(err)
// Get the new root from dc1 and validate a chain of:
// Get the root from dc1 and validate a chain of:
// dc2 leaf -> dc2 intermediate -> dc1 root
_, caRoot := s1.getCAProvider()
_, caRoot := getCAProviderWithLock(s1)
// Have dc2 sign a leaf cert and make sure the chain is correct.
spiffeService := &connect.SpiffeIDService{
@ -329,7 +336,7 @@ func TestLeader_SecondaryCA_IntermediateRefresh(t *testing.T) {
testrpc.WaitForLeader(t, s2.RPC, "dc2")
// Get the original intermediate
secondaryProvider, _ := s2.getCAProvider()
secondaryProvider, _ := getCAProviderWithLock(s2)
oldIntermediatePEM, err := secondaryProvider.ActiveIntermediate()
require.NoError(err)
require.NotEmpty(oldIntermediatePEM)
@ -415,7 +422,7 @@ func TestLeader_SecondaryCA_IntermediateRefresh(t *testing.T) {
// Get the new root from dc1 and validate a chain of:
// dc2 leaf -> dc2 intermediate -> dc1 root
_, caRoot := s1.getCAProvider()
_, caRoot := getCAProviderWithLock(s1)
// Have dc2 sign a leaf cert and make sure the chain is correct.
spiffeService := &connect.SpiffeIDService{
@ -524,7 +531,7 @@ func TestLeader_SecondaryCA_FixSigningKeyID_via_IntermediateRefresh(t *testing.T
// the CA provider anyway.
retry.Run(t, func(r *retry.R) {
// verify that the root is now corrected
provider, activeRoot := s2.getCAProvider()
provider, activeRoot := getCAProviderWithLock(s2)
require.NotNil(r, provider)
require.NotNil(r, activeRoot)
@ -709,7 +716,7 @@ func TestLeader_SecondaryCA_UpgradeBeforePrimary(t *testing.T) {
// Wait for the secondary transition to happen and then verify the secondary DC
// has both roots present.
secondaryProvider, _ := s2.getCAProvider()
secondaryProvider, _ := getCAProviderWithLock(s2)
retry.Run(t, func(r *retry.R) {
state1 := s1.fsm.State()
_, roots1, err := state1.CARoots(nil)
@ -730,7 +737,7 @@ func TestLeader_SecondaryCA_UpgradeBeforePrimary(t *testing.T) {
require.NotEmpty(r, inter, "should have valid intermediate")
})
_, caRoot := s1.getCAProvider()
_, caRoot := getCAProviderWithLock(s1)
intermediatePEM, err := secondaryProvider.ActiveIntermediate()
require.NoError(t, err)
@ -1325,7 +1332,7 @@ func TestLeader_PersistIntermediateCAs(t *testing.T) {
}
// Get the active root before leader change.
_, root := s1.getCAProvider()
_, root := getCAProviderWithLock(s1)
require.Len(root.IntermediateCerts, 1)
// Force a leader change and make sure the root CA values are preserved.
@ -1344,7 +1351,7 @@ func TestLeader_PersistIntermediateCAs(t *testing.T) {
r.Fatal("no leader")
}
_, newLeaderRoot := leader.getCAProvider()
_, newLeaderRoot := getCAProviderWithLock(leader)
if !reflect.DeepEqual(newLeaderRoot, root) {
r.Fatalf("got %v, want %v", newLeaderRoot, root)
}