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

tls enforcement flaky tests #16543

Merged
merged 2 commits into from
Mar 17, 2023
Merged

tls enforcement flaky tests #16543

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a4cb204
tests: add WaitForLeaders helpers using must/wait timings
shoenig Mar 17, 2023
8e03dda
tests: start servers for mtls tests together
shoenig Mar 17, 2023
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
65 changes: 25 additions & 40 deletions nomad/rpc_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"testing"
"time"

"github.com/hashicorp/go-hclog"
"github.com/hashicorp/go-msgpack/codec"
"github.com/hashicorp/go-sockaddr"
msgpackrpc "github.com/hashicorp/net-rpc-msgpackrpc"
Expand Down Expand Up @@ -1346,9 +1347,9 @@ type tlsTestHelper struct {
nodeID int

mtlsServer1 *Server
mtlsServer1Cleanup func()
mtlsServerCleanup1 func()
mtlsServer2 *Server
mtlsServer2Cleanup func()
mtlsServerCleanup2 func()
nonVerifyServer *Server
nonVerifyServerCleanup func()

Expand All @@ -1367,57 +1368,41 @@ func newTLSTestHelper(t *testing.T) tlsTestHelper {

// Generate CA certificate and write it to disk.
h.caPEM, h.pk, err = tlsutil.GenerateCA(tlsutil.CAOpts{Days: 5, Domain: "nomad"})
require.NoError(t, err)
must.NoError(t, err)

err = os.WriteFile(filepath.Join(h.dir, "ca.pem"), []byte(h.caPEM), 0600)
require.NoError(t, err)
must.NoError(t, err)

// Generate servers and their certificate.
h.serverCert = h.newCert(t, "server.global.nomad")

h.mtlsServer1, h.mtlsServer1Cleanup = TestServer(t, func(c *Config) {
c.BootstrapExpect = 2
c.TLSConfig = &config.TLSConfig{
EnableRPC: true,
VerifyServerHostname: true,
CAFile: filepath.Join(h.dir, "ca.pem"),
CertFile: h.serverCert + ".pem",
KeyFile: h.serverCert + ".key",
}
})
h.mtlsServer2, h.mtlsServer2Cleanup = TestServer(t, func(c *Config) {
c.BootstrapExpect = 2
c.TLSConfig = &config.TLSConfig{
EnableRPC: true,
VerifyServerHostname: true,
CAFile: filepath.Join(h.dir, "ca.pem"),
CertFile: h.serverCert + ".pem",
KeyFile: h.serverCert + ".key",
}
})
TestJoin(t, h.mtlsServer1, h.mtlsServer2)
testutil.WaitForLeader(t, h.mtlsServer1.RPC)
testutil.WaitForLeader(t, h.mtlsServer2.RPC)
makeServer := func(bootstrapExpect int, verifyServerHostname bool) (*Server, func()) {
return TestServer(t, func(c *Config) {
c.Logger.SetLevel(hclog.Off)
c.BootstrapExpect = bootstrapExpect
c.TLSConfig = &config.TLSConfig{
EnableRPC: true,
VerifyServerHostname: verifyServerHostname,
CAFile: filepath.Join(h.dir, "ca.pem"),
CertFile: h.serverCert + ".pem",
KeyFile: h.serverCert + ".key",
}
})
}

h.nonVerifyServer, h.nonVerifyServerCleanup = TestServer(t, func(c *Config) {
c.TLSConfig = &config.TLSConfig{
EnableRPC: true,
VerifyServerHostname: false,
CAFile: filepath.Join(h.dir, "ca.pem"),
CertFile: h.serverCert + ".pem",
KeyFile: h.serverCert + ".key",
}
})
testutil.WaitForLeader(t, h.nonVerifyServer.RPC)
h.mtlsServer1, h.mtlsServerCleanup1 = makeServer(3, true)
h.mtlsServer2, h.mtlsServerCleanup2 = makeServer(3, true)
h.nonVerifyServer, h.nonVerifyServerCleanup = makeServer(3, false)

TestJoin(t, h.mtlsServer1, h.mtlsServer2, h.nonVerifyServer)
testutil.WaitForLeaders(t, h.mtlsServer1.RPC, h.mtlsServer2.RPC, h.nonVerifyServer.RPC)
Comment on lines +1397 to +1398
Copy link
Member Author

@shoenig shoenig Mar 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

instead of starting 1 & 2 with bootstrap = 2 then waiting for a leader then adding server 3 ... start all 3 together with bootstrap = 3 then wait for leader

return h
}

func (h tlsTestHelper) cleanup() {
h.mtlsServer1Cleanup()
h.mtlsServer2Cleanup()
h.mtlsServerCleanup1()
h.mtlsServerCleanup2()
h.nonVerifyServerCleanup()
os.RemoveAll(h.dir)
}

func (h tlsTestHelper) newCert(t *testing.T, name string) string {
Expand Down
13 changes: 6 additions & 7 deletions nomad/testing.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package nomad

import (
"errors"
"fmt"
"math/rand"
"net"
Expand All @@ -14,7 +15,7 @@ import (
"github.com/hashicorp/nomad/nomad/mock"
"github.com/hashicorp/nomad/nomad/structs"
"github.com/hashicorp/nomad/version"
"github.com/stretchr/testify/require"
"github.com/shoenig/test/must"
)

var (
Expand All @@ -38,7 +39,7 @@ func TestACLServer(t *testing.T, cb func(*Config)) (*Server, *structs.ACLToken,

func TestServer(t *testing.T, cb func(*Config)) (*Server, func()) {
s, c, err := TestServerErr(t, cb)
require.NoError(t, err, "failed to start test server")
must.NoError(t, err, must.Sprint("failed to start test server"))
return s, c
}

Expand Down Expand Up @@ -122,7 +123,7 @@ func TestServerErr(t *testing.T, cb func(*Config)) (*Server, func(), error) {
defer close(ch)

// Shutdown server
err := server.Shutdown()
err = server.Shutdown()
if err != nil {
ch <- fmt.Errorf("failed to shutdown server: %w", err)
}
Expand All @@ -137,9 +138,7 @@ func TestServerErr(t *testing.T, cb func(*Config)) (*Server, func(), error) {
t.Fatal("timed out while shutting down server")
}
}, nil
} else if i == 0 {
return nil, nil, err
} else {
} else if i > 0 {
if server != nil {
_ = server.Shutdown()
}
Expand All @@ -148,7 +147,7 @@ func TestServerErr(t *testing.T, cb func(*Config)) (*Server, func(), error) {
}
}

return nil, nil, nil
return nil, nil, errors.New("unable to acquire ports for test server")
}

func TestJoin(t *testing.T, servers ...*Server) {
Expand Down
20 changes: 20 additions & 0 deletions testutil/wait.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/hashicorp/nomad/nomad/structs"
"github.com/kr/pretty"
"github.com/shoenig/test/must"
"github.com/shoenig/test/wait"
)

type testFn func() (bool, error)
Expand Down Expand Up @@ -142,6 +143,25 @@ func WaitForLeader(t testing.TB, rpc rpcFn) {
})
}

// WaitForLeaders blocks until each serverRPC knows the leader.
func WaitForLeaders(t testing.TB, serverRPCs ...rpcFn) {
t.Helper()

for i := 0; i < len(serverRPCs); i++ {
ok := func() (bool, error) {
args := &structs.GenericRequest{}
var leader string
err := serverRPCs[i]("Status.Leader", args, &leader)
return leader != "", err
}
must.Wait(t, wait.InitialSuccess(
wait.TestFunc(ok),
wait.Timeout(10*time.Second),
wait.Gap(1*time.Second),
))
}
}

// WaitForClient blocks until the client can be found
func WaitForClient(t testing.TB, rpc rpcFn, nodeID string, region string) {
t.Helper()
Expand Down