Skip to content

Commit ffe1591

Browse files
mmorel-35mmorel-35
committed
chore(ca): chore: fix testifylint issues part 1
chore: fix testifylint issues in ca Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
1 parent 3e72cb6 commit ffe1591

5 files changed

Lines changed: 129 additions & 136 deletions

File tree

‎ca/certificates_test.go‎

Lines changed: 65 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -89,13 +89,13 @@ func TestCreateRootCASaveRootCA(t *testing.T) {
8989
paths := ca.NewConfigPaths(tempBaseDir)
9090

9191
rootCA, err := ca.CreateRootCA("rootCN")
92-
assert.NoError(t, err)
92+
require.NoError(t, err)
9393

9494
err = ca.SaveRootCA(rootCA, paths.RootCA)
95-
assert.NoError(t, err)
95+
require.NoError(t, err)
9696

9797
fi, err := os.Stat(paths.RootCA.Cert)
98-
assert.NoError(t, err)
98+
require.NoError(t, err)
9999
assert.False(t, groupWrite(fi.Mode()))
100100
assert.False(t, otherWrite(fi.Mode()))
101101

@@ -104,20 +104,20 @@ func TestCreateRootCASaveRootCA(t *testing.T) {
104104

105105
// ensure that the cert that was written is already normalized
106106
written, err := os.ReadFile(paths.RootCA.Cert)
107-
assert.NoError(t, err)
107+
require.NoError(t, err)
108108
assert.Equal(t, written, ca.NormalizePEMs(written))
109109
}
110110

111111
func TestCreateRootCAExpiry(t *testing.T) {
112112
rootCA, err := ca.CreateRootCA("rootCN")
113-
assert.NoError(t, err)
113+
require.NoError(t, err)
114114

115115
// Convert the certificate into an object to create a RootCA
116116
parsedCert, err := helpers.ParseCertificatePEM(rootCA.Certs)
117-
assert.NoError(t, err)
117+
require.NoError(t, err)
118118
duration, err := time.ParseDuration(ca.RootCAExpiration)
119-
assert.NoError(t, err)
120-
assert.True(t, time.Now().Add(duration).AddDate(0, -1, 0).Before(parsedCert.NotAfter))
119+
require.NoError(t, err)
120+
assert.Less(t, time.Now().Add(duration).AddDate(0, -1, 0), parsedCert.NotAfter)
121121
}
122122

123123
func TestGetLocalRootCA(t *testing.T) {
@@ -130,37 +130,37 @@ func TestGetLocalRootCA(t *testing.T) {
130130

131131
// Create the local Root CA to ensure that we can reload it correctly.
132132
rootCA, err := ca.CreateRootCA("rootCN")
133-
assert.NoError(t, err)
133+
require.NoError(t, err)
134134
s, err := rootCA.Signer()
135-
assert.NoError(t, err)
135+
require.NoError(t, err)
136136
err = ca.SaveRootCA(rootCA, paths.RootCA)
137-
assert.NoError(t, err)
137+
require.NoError(t, err)
138138

139139
// No private key here
140140
rootCA2, err := ca.GetLocalRootCA(paths.RootCA)
141-
assert.NoError(t, err)
141+
require.NoError(t, err)
142142
assert.Equal(t, rootCA.Certs, rootCA2.Certs)
143143
_, err = rootCA2.Signer()
144144
assert.Equal(t, err, ca.ErrNoValidSigner)
145145

146146
// write private key and assert we can load it and sign
147-
assert.NoError(t, os.WriteFile(paths.RootCA.Key, s.Key, os.FileMode(0o600)))
147+
require.NoError(t, os.WriteFile(paths.RootCA.Key, s.Key, os.FileMode(0o600)))
148148
rootCA3, err := ca.GetLocalRootCA(paths.RootCA)
149-
assert.NoError(t, err)
149+
require.NoError(t, err)
150150
assert.Equal(t, rootCA.Certs, rootCA3.Certs)
151151
_, err = rootCA3.Signer()
152-
assert.NoError(t, err)
152+
require.NoError(t, err)
153153

154154
// Try with a private key that does not match the CA cert public key.
155155
privKey, err := ecdsa.GenerateKey(elliptic.P256(), cryptorand.Reader)
156-
assert.NoError(t, err)
156+
require.NoError(t, err)
157157
privKeyBytes, err := x509.MarshalECPrivateKey(privKey)
158-
assert.NoError(t, err)
158+
require.NoError(t, err)
159159
privKeyPem := pem.EncodeToMemory(&pem.Block{
160160
Type: "EC PRIVATE KEY",
161161
Bytes: privKeyBytes,
162162
})
163-
assert.NoError(t, os.WriteFile(paths.RootCA.Key, privKeyPem, os.FileMode(0o600)))
163+
require.NoError(t, os.WriteFile(paths.RootCA.Key, privKeyPem, os.FileMode(0o600)))
164164

165165
_, err = ca.GetLocalRootCA(paths.RootCA)
166166
assert.EqualError(t, err, "certificate key mismatch")
@@ -198,21 +198,21 @@ some random garbage\n
198198

199199
func TestParseValidateAndSignCSR(t *testing.T) {
200200
rootCA, err := ca.CreateRootCA("rootCN")
201-
assert.NoError(t, err)
201+
require.NoError(t, err)
202202

203203
csr, _, err := ca.GenerateNewCSR()
204-
assert.NoError(t, err)
204+
require.NoError(t, err)
205205

206206
signedCert, err := rootCA.ParseValidateAndSignCSR(csr, "CN", "OU", "ORG")
207-
assert.NoError(t, err)
207+
require.NoError(t, err)
208208
assert.NotNil(t, signedCert)
209209

210210
assert.Len(t, checkLeafCert(t, signedCert, "rootCN", "CN", "OU", "ORG"), 1)
211211
}
212212

213213
func TestParseValidateAndSignMaliciousCSR(t *testing.T) {
214214
rootCA, err := ca.CreateRootCA("rootCN")
215-
assert.NoError(t, err)
215+
require.NoError(t, err)
216216

217217
req := &cfcsr.CertificateRequest{
218218
Names: []cfcsr.Name{
@@ -228,10 +228,10 @@ func TestParseValidateAndSignMaliciousCSR(t *testing.T) {
228228
}
229229

230230
csr, _, err := cfcsr.ParseRequest(req)
231-
assert.NoError(t, err)
231+
require.NoError(t, err)
232232

233233
signedCert, err := rootCA.ParseValidateAndSignCSR(csr, "CN", "OU", "ORG")
234-
assert.NoError(t, err)
234+
require.NoError(t, err)
235235
assert.NotNil(t, signedCert)
236236

237237
assert.Len(t, checkLeafCert(t, signedCert, "rootCN", "CN", "OU", "ORG"), 1)
@@ -274,7 +274,7 @@ func TestGetRemoteCA(t *testing.T) {
274274
if err == nil {
275275
return fmt.Errorf("testca's rootca hasn't updated yet")
276276
}
277-
require.Contains(t, err.Error(), "remote CA does not match fingerprint")
277+
require.ErrorContains(t, err, "remote CA does not match fingerprint")
278278
return nil
279279
}))
280280

@@ -284,7 +284,7 @@ func TestGetRemoteCA(t *testing.T) {
284284
downloadedRootCA, err = ca.GetRemoteCA(tc.Context, d, tc.ConnBroker)
285285
require.NoError(t, err)
286286
require.Equal(t, comboCertBundle, downloadedRootCA.Certs)
287-
require.Equal(t, 2, len(downloadedRootCA.Pool.Subjects()))
287+
require.Len(t, downloadedRootCA.Pool.Subjects(), 2)
288288

289289
for _, rootCA := range []ca.RootCA{tc.RootCA, otherRootCA} {
290290
krw := ca.NewKeyReadWriter(paths.Node, nil, nil)
@@ -510,38 +510,38 @@ func TestGetRemoteSignedCertificate(t *testing.T) {
510510

511511
// Create a new CSR to be signed
512512
csr, _, err := ca.GenerateNewCSR()
513-
assert.NoError(t, err)
513+
require.NoError(t, err)
514514

515515
certs, err := ca.GetRemoteSignedCertificate(tc.Context, csr, tc.RootCA.Pool,
516516
ca.CertificateRequestConfig{
517517
Token: tc.ManagerToken,
518518
ConnBroker: tc.ConnBroker,
519519
})
520-
assert.NoError(t, err)
520+
require.NoError(t, err)
521521
assert.NotNil(t, certs)
522522

523523
// Test the expiration for a manager certificate
524524
parsedCerts, err := helpers.ParseCertificatesPEM(certs)
525-
assert.NoError(t, err)
525+
require.NoError(t, err)
526526
assert.Len(t, parsedCerts, 1)
527-
assert.True(t, time.Now().Add(ca.DefaultNodeCertExpiration).AddDate(0, 0, -1).Before(parsedCerts[0].NotAfter))
528-
assert.True(t, time.Now().Add(ca.DefaultNodeCertExpiration).AddDate(0, 0, 1).After(parsedCerts[0].NotAfter))
529-
assert.Equal(t, parsedCerts[0].Subject.OrganizationalUnit[0], ca.ManagerRole)
527+
assert.Less(t, time.Now().Add(ca.DefaultNodeCertExpiration).AddDate(0, 0, -1), parsedCerts[0].NotAfter)
528+
assert.Greater(t, time.Now().Add(ca.DefaultNodeCertExpiration).AddDate(0, 0, 1), parsedCerts[0].NotAfter)
529+
assert.Equal(t, ca.ManagerRole, parsedCerts[0].Subject.OrganizationalUnit[0])
530530

531531
// Test the expiration for an worker certificate
532532
certs, err = ca.GetRemoteSignedCertificate(tc.Context, csr, tc.RootCA.Pool,
533533
ca.CertificateRequestConfig{
534534
Token: tc.WorkerToken,
535535
ConnBroker: tc.ConnBroker,
536536
})
537-
assert.NoError(t, err)
537+
require.NoError(t, err)
538538
assert.NotNil(t, certs)
539539
parsedCerts, err = helpers.ParseCertificatesPEM(certs)
540-
assert.NoError(t, err)
540+
require.NoError(t, err)
541541
assert.Len(t, parsedCerts, 1)
542-
assert.True(t, time.Now().Add(ca.DefaultNodeCertExpiration).AddDate(0, 0, -1).Before(parsedCerts[0].NotAfter))
543-
assert.True(t, time.Now().Add(ca.DefaultNodeCertExpiration).AddDate(0, 0, 1).After(parsedCerts[0].NotAfter))
544-
assert.Equal(t, parsedCerts[0].Subject.OrganizationalUnit[0], ca.WorkerRole)
542+
assert.Less(t, time.Now().Add(ca.DefaultNodeCertExpiration).AddDate(0, 0, -1), parsedCerts[0].NotAfter)
543+
assert.Greater(t, time.Now().Add(ca.DefaultNodeCertExpiration).AddDate(0, 0, 1), parsedCerts[0].NotAfter)
544+
assert.Equal(t, ca.WorkerRole, parsedCerts[0].Subject.OrganizationalUnit[0])
545545
}
546546

547547
func TestGetRemoteSignedCertificateNodeInfo(t *testing.T) {
@@ -550,14 +550,14 @@ func TestGetRemoteSignedCertificateNodeInfo(t *testing.T) {
550550

551551
// Create a new CSR to be signed
552552
csr, _, err := ca.GenerateNewCSR()
553-
assert.NoError(t, err)
553+
require.NoError(t, err)
554554

555555
cert, err := ca.GetRemoteSignedCertificate(tc.Context, csr, tc.RootCA.Pool,
556556
ca.CertificateRequestConfig{
557557
Token: tc.WorkerToken,
558558
ConnBroker: tc.ConnBroker,
559559
})
560-
assert.NoError(t, err)
560+
require.NoError(t, err)
561561
assert.NotNil(t, cert)
562562
}
563563

@@ -719,7 +719,7 @@ func TestGetRemoteSignedCertificateWithPending(t *testing.T) {
719719
case <-time.After(2500 * time.Millisecond):
720720
// good, it's still polling so we can proceed with the test
721721
}
722-
require.True(t, atomic.LoadInt64(&fakeCAServer.nodeStatusCalled) > 1, "expected NodeCertificateStatus to have been polled more than once")
722+
require.Greater(t, atomic.LoadInt64(&fakeCAServer.nodeStatusCalled), 1, "expected NodeCertificateStatus to have been polled more than once")
723723

724724
// Directly update the status of the store
725725
err = tc.MemoryStore.Update(func(tx store.Tx) error {
@@ -749,7 +749,7 @@ func TestGetRemoteSignedCertificateWithPending(t *testing.T) {
749749
select {
750750
case err = <-completed:
751751
s, _ := status.FromError(err)
752-
require.Equal(t, s.Code(), codes.DeadlineExceeded)
752+
require.Equal(t, codes.DeadlineExceeded, s.Code())
753753
case <-time.After(3 * time.Second):
754754
require.FailNow(t, "GetRemoteSignedCertificate should have been canceled after 1 second, and it has been 3")
755755
}
@@ -872,7 +872,7 @@ func TestGetRemoteSignedCertificateConnectionErrors(t *testing.T) {
872872
// wait for 5 seconds and ensure that GetRemoteSignedCertificate has returned with an error.
873873
select {
874874
case err = <-completed:
875-
require.Contains(t, err.Error(), "no more peers")
875+
require.ErrorContains(t, err, "no more peers")
876876
case <-time.After(5 * time.Second):
877877
require.FailNow(t, "GetRemoteSignedCertificate should errored after 5 seconds")
878878
}
@@ -916,68 +916,68 @@ func TestNewRootCABundle(t *testing.T) {
916916

917917
// make one rootCA
918918
firstRootCA, err := ca.CreateRootCA("rootCN1")
919-
assert.NoError(t, err)
919+
require.NoError(t, err)
920920

921921
// make a second root CA
922922
secondRootCA, err := ca.CreateRootCA("rootCN2")
923-
assert.NoError(t, err)
923+
require.NoError(t, err)
924924
s, err := firstRootCA.Signer()
925925
require.NoError(t, err)
926926

927927
// Overwrite the bytes of the second Root CA with the bundle, creating a valid 2 cert bundle
928928
bundle := append(firstRootCA.Certs, secondRootCA.Certs...)
929929
err = os.WriteFile(paths.RootCA.Cert, bundle, 0o644)
930-
assert.NoError(t, err)
930+
require.NoError(t, err)
931931

932932
newRootCA, err := ca.NewRootCA(bundle, firstRootCA.Certs, s.Key, ca.DefaultNodeCertExpiration, nil)
933-
assert.NoError(t, err)
933+
require.NoError(t, err)
934934
assert.Equal(t, bundle, newRootCA.Certs)
935-
assert.Equal(t, 2, len(newRootCA.Pool.Subjects()))
935+
assert.Len(t, newRootCA.Pool.Subjects(), 2)
936936

937937
// If I use newRootCA's IssueAndSaveNewCertificates to sign certs, I'll get the correct CA in the chain
938938
kw := ca.NewKeyReadWriter(paths.Node, nil, nil)
939939
_, _, err = newRootCA.IssueAndSaveNewCertificates(kw, "CN", "OU", "ORG")
940-
assert.NoError(t, err)
940+
require.NoError(t, err)
941941

942942
certBytes, err := os.ReadFile(paths.Node.Cert)
943-
assert.NoError(t, err)
943+
require.NoError(t, err)
944944
assert.Len(t, checkLeafCert(t, certBytes, "rootCN1", "CN", "OU", "ORG"), 1)
945945
}
946946

947947
func TestNewRootCANonDefaultExpiry(t *testing.T) {
948948
rootCA, err := ca.CreateRootCA("rootCN")
949-
assert.NoError(t, err)
949+
require.NoError(t, err)
950950
s, err := rootCA.Signer()
951951
require.NoError(t, err)
952952

953953
newRootCA, err := ca.NewRootCA(rootCA.Certs, rootCA.Certs, s.Key, 1*time.Hour, nil)
954-
assert.NoError(t, err)
954+
require.NoError(t, err)
955955

956956
// Create and sign a new CSR
957957
csr, _, err := ca.GenerateNewCSR()
958-
assert.NoError(t, err)
958+
require.NoError(t, err)
959959
cert, err := newRootCA.ParseValidateAndSignCSR(csr, "CN", ca.ManagerRole, "ORG")
960-
assert.NoError(t, err)
960+
require.NoError(t, err)
961961

962962
parsedCerts, err := helpers.ParseCertificatesPEM(cert)
963-
assert.NoError(t, err)
963+
require.NoError(t, err)
964964
assert.Len(t, parsedCerts, 1)
965-
assert.True(t, time.Now().Add(time.Minute*59).Before(parsedCerts[0].NotAfter))
966-
assert.True(t, time.Now().Add(time.Hour).Add(time.Minute).After(parsedCerts[0].NotAfter))
965+
assert.Less(t, time.Now().Add(time.Minute*59), parsedCerts[0].NotAfter)
966+
assert.Greater(t, time.Now().Add(time.Hour).Add(time.Minute), parsedCerts[0].NotAfter)
967967

968968
// Sign the same CSR again, this time with a 59 Minute expiration RootCA (under the 60 minute minimum).
969969
// This should use the default of 3 months
970970
newRootCA, err = ca.NewRootCA(rootCA.Certs, rootCA.Certs, s.Key, 59*time.Minute, nil)
971-
assert.NoError(t, err)
971+
require.NoError(t, err)
972972

973973
cert, err = newRootCA.ParseValidateAndSignCSR(csr, "CN", ca.ManagerRole, "ORG")
974-
assert.NoError(t, err)
974+
require.NoError(t, err)
975975

976976
parsedCerts, err = helpers.ParseCertificatesPEM(cert)
977-
assert.NoError(t, err)
977+
require.NoError(t, err)
978978
assert.Len(t, parsedCerts, 1)
979-
assert.True(t, time.Now().Add(ca.DefaultNodeCertExpiration).AddDate(0, 0, -1).Before(parsedCerts[0].NotAfter))
980-
assert.True(t, time.Now().Add(ca.DefaultNodeCertExpiration).AddDate(0, 0, 1).After(parsedCerts[0].NotAfter))
979+
assert.Less(t, time.Now().Add(ca.DefaultNodeCertExpiration).AddDate(0, 0, -1), parsedCerts[0].NotAfter)
980+
assert.Greater(t, time.Now().Add(ca.DefaultNodeCertExpiration).AddDate(0, 0, 1), parsedCerts[0].NotAfter)
981981
}
982982

983983
type invalidNewRootCATestCase struct {
@@ -1165,8 +1165,7 @@ func TestNewRootCAInvalidCertAndKeys(t *testing.T) {
11651165

11661166
for i, invalid := range invalids {
11671167
_, err := ca.NewRootCA(invalid.roots, invalid.cert, invalid.key, ca.DefaultNodeCertExpiration, invalid.intermediates)
1168-
require.Error(t, err, fmt.Sprintf("expected error containing: \"%s\", test case (%d)", invalid.errorStr, i))
1169-
require.Contains(t, err.Error(), invalid.errorStr, fmt.Sprintf("%d", i))
1168+
require.ErrorContains(t, err, invalid.errorStr, "%d", i)
11701169
}
11711170
}
11721171

@@ -1224,7 +1223,7 @@ func TestRootCAWithCrossSignedIntermediates(t *testing.T) {
12241223
require.NoError(t, err)
12251224
require.Len(t, parsedCerts, 2)
12261225
require.Len(t, chains, 1)
1227-
require.True(t, len(chains[0]) >= 2) // there are always at least 2 certs at minimum: the leaf and the root
1226+
require.GreaterOrEqual(t, len(chains[0]), 2) // there are always at least 2 certs at minimum: the leaf and the root
12281227
require.Equal(t, parsedCerts[0], chains[0][0])
12291228
require.Equal(t, parsedIntermediate.Raw, parsedCerts[1].Raw)
12301229

@@ -1380,8 +1379,7 @@ func TestValidateCertificateChain(t *testing.T) {
13801379
pool := x509.NewCertPool()
13811380
pool.AppendCertsFromPEM(invalid.root)
13821381
_, _, err := ca.ValidateCertChain(pool, invalid.cert, invalid.allowExpiry)
1383-
require.Error(t, err, invalid.errorStr)
1384-
require.Contains(t, err.Error(), invalid.errorStr)
1382+
require.ErrorContains(t, err, invalid.errorStr)
13851383
}
13861384

13871385
// these will default to using the root pool, so we don't have to specify the root pool
@@ -1409,7 +1407,7 @@ func TestValidateCertificateChain(t *testing.T) {
14091407
require.NotEmpty(t, chain)
14101408
for _, chain := range chains {
14111409
require.Equal(t, parsedCerts[0], chain[0]) // the leaf certs are equal
1412-
require.True(t, len(chain) >= 2)
1410+
require.GreaterOrEqual(t, len(chain), 2)
14131411
}
14141412
}
14151413
}

0 commit comments

Comments
 (0)