Skip to content

Commit 38bf0e4

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

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
@@ -82,13 +82,13 @@ func TestCreateRootCASaveRootCA(t *testing.T) {
8282
paths := ca.NewConfigPaths(tempBaseDir)
8383

8484
rootCA, err := ca.CreateRootCA("rootCN")
85-
assert.NoError(t, err)
85+
require.NoError(t, err)
8686

8787
err = ca.SaveRootCA(rootCA, paths.RootCA)
88-
assert.NoError(t, err)
88+
require.NoError(t, err)
8989

9090
perms, err := permbits.Stat(paths.RootCA.Cert)
91-
assert.NoError(t, err)
91+
require.NoError(t, err)
9292
assert.False(t, perms.GroupWrite())
9393
assert.False(t, perms.OtherWrite())
9494

@@ -97,20 +97,20 @@ func TestCreateRootCASaveRootCA(t *testing.T) {
9797

9898
// ensure that the cert that was written is already normalized
9999
written, err := os.ReadFile(paths.RootCA.Cert)
100-
assert.NoError(t, err)
100+
require.NoError(t, err)
101101
assert.Equal(t, written, ca.NormalizePEMs(written))
102102
}
103103

104104
func TestCreateRootCAExpiry(t *testing.T) {
105105
rootCA, err := ca.CreateRootCA("rootCN")
106-
assert.NoError(t, err)
106+
require.NoError(t, err)
107107

108108
// Convert the certificate into an object to create a RootCA
109109
parsedCert, err := helpers.ParseCertificatePEM(rootCA.Certs)
110-
assert.NoError(t, err)
110+
require.NoError(t, err)
111111
duration, err := time.ParseDuration(ca.RootCAExpiration)
112-
assert.NoError(t, err)
113-
assert.True(t, time.Now().Add(duration).AddDate(0, -1, 0).Before(parsedCert.NotAfter))
112+
require.NoError(t, err)
113+
assert.Less(t, time.Now().Add(duration).AddDate(0, -1, 0), parsedCert.NotAfter)
114114
}
115115

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

124124
// Create the local Root CA to ensure that we can reload it correctly.
125125
rootCA, err := ca.CreateRootCA("rootCN")
126-
assert.NoError(t, err)
126+
require.NoError(t, err)
127127
s, err := rootCA.Signer()
128-
assert.NoError(t, err)
128+
require.NoError(t, err)
129129
err = ca.SaveRootCA(rootCA, paths.RootCA)
130-
assert.NoError(t, err)
130+
require.NoError(t, err)
131131

132132
// No private key here
133133
rootCA2, err := ca.GetLocalRootCA(paths.RootCA)
134-
assert.NoError(t, err)
134+
require.NoError(t, err)
135135
assert.Equal(t, rootCA.Certs, rootCA2.Certs)
136136
_, err = rootCA2.Signer()
137137
assert.Equal(t, err, ca.ErrNoValidSigner)
138138

139139
// write private key and assert we can load it and sign
140-
assert.NoError(t, os.WriteFile(paths.RootCA.Key, s.Key, os.FileMode(0o600)))
140+
require.NoError(t, os.WriteFile(paths.RootCA.Key, s.Key, os.FileMode(0o600)))
141141
rootCA3, err := ca.GetLocalRootCA(paths.RootCA)
142-
assert.NoError(t, err)
142+
require.NoError(t, err)
143143
assert.Equal(t, rootCA.Certs, rootCA3.Certs)
144144
_, err = rootCA3.Signer()
145-
assert.NoError(t, err)
145+
require.NoError(t, err)
146146

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

158158
_, err = ca.GetLocalRootCA(paths.RootCA)
159159
assert.EqualError(t, err, "certificate key mismatch")
@@ -191,21 +191,21 @@ some random garbage\n
191191

192192
func TestParseValidateAndSignCSR(t *testing.T) {
193193
rootCA, err := ca.CreateRootCA("rootCN")
194-
assert.NoError(t, err)
194+
require.NoError(t, err)
195195

196196
csr, _, err := ca.GenerateNewCSR()
197-
assert.NoError(t, err)
197+
require.NoError(t, err)
198198

199199
signedCert, err := rootCA.ParseValidateAndSignCSR(csr, "CN", "OU", "ORG")
200-
assert.NoError(t, err)
200+
require.NoError(t, err)
201201
assert.NotNil(t, signedCert)
202202

203203
assert.Len(t, checkLeafCert(t, signedCert, "rootCN", "CN", "OU", "ORG"), 1)
204204
}
205205

206206
func TestParseValidateAndSignMaliciousCSR(t *testing.T) {
207207
rootCA, err := ca.CreateRootCA("rootCN")
208-
assert.NoError(t, err)
208+
require.NoError(t, err)
209209

210210
req := &cfcsr.CertificateRequest{
211211
Names: []cfcsr.Name{
@@ -221,10 +221,10 @@ func TestParseValidateAndSignMaliciousCSR(t *testing.T) {
221221
}
222222

223223
csr, _, err := cfcsr.ParseRequest(req)
224-
assert.NoError(t, err)
224+
require.NoError(t, err)
225225

226226
signedCert, err := rootCA.ParseValidateAndSignCSR(csr, "CN", "OU", "ORG")
227-
assert.NoError(t, err)
227+
require.NoError(t, err)
228228
assert.NotNil(t, signedCert)
229229

230230
assert.Len(t, checkLeafCert(t, signedCert, "rootCN", "CN", "OU", "ORG"), 1)
@@ -267,7 +267,7 @@ func TestGetRemoteCA(t *testing.T) {
267267
if err == nil {
268268
return fmt.Errorf("testca's rootca hasn't updated yet")
269269
}
270-
require.Contains(t, err.Error(), "remote CA does not match fingerprint")
270+
require.ErrorContains(t, err, "remote CA does not match fingerprint")
271271
return nil
272272
}))
273273

@@ -277,7 +277,7 @@ func TestGetRemoteCA(t *testing.T) {
277277
downloadedRootCA, err = ca.GetRemoteCA(tc.Context, d, tc.ConnBroker)
278278
require.NoError(t, err)
279279
require.Equal(t, comboCertBundle, downloadedRootCA.Certs)
280-
require.Equal(t, 2, len(downloadedRootCA.Pool.Subjects()))
280+
require.Len(t, downloadedRootCA.Pool.Subjects(), 2)
281281

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

504504
// Create a new CSR to be signed
505505
csr, _, err := ca.GenerateNewCSR()
506-
assert.NoError(t, err)
506+
require.NoError(t, err)
507507

508508
certs, err := ca.GetRemoteSignedCertificate(tc.Context, csr, tc.RootCA.Pool,
509509
ca.CertificateRequestConfig{
510510
Token: tc.ManagerToken,
511511
ConnBroker: tc.ConnBroker,
512512
})
513-
assert.NoError(t, err)
513+
require.NoError(t, err)
514514
assert.NotNil(t, certs)
515515

516516
// Test the expiration for a manager certificate
517517
parsedCerts, err := helpers.ParseCertificatesPEM(certs)
518-
assert.NoError(t, err)
518+
require.NoError(t, err)
519519
assert.Len(t, parsedCerts, 1)
520-
assert.True(t, time.Now().Add(ca.DefaultNodeCertExpiration).AddDate(0, 0, -1).Before(parsedCerts[0].NotAfter))
521-
assert.True(t, time.Now().Add(ca.DefaultNodeCertExpiration).AddDate(0, 0, 1).After(parsedCerts[0].NotAfter))
522-
assert.Equal(t, parsedCerts[0].Subject.OrganizationalUnit[0], ca.ManagerRole)
520+
assert.Less(t, time.Now().Add(ca.DefaultNodeCertExpiration).AddDate(0, 0, -1), parsedCerts[0].NotAfter)
521+
assert.Greater(t, time.Now().Add(ca.DefaultNodeCertExpiration).AddDate(0, 0, 1), parsedCerts[0].NotAfter)
522+
assert.Equal(t, ca.ManagerRole, parsedCerts[0].Subject.OrganizationalUnit[0])
523523

524524
// Test the expiration for an worker certificate
525525
certs, err = ca.GetRemoteSignedCertificate(tc.Context, csr, tc.RootCA.Pool,
526526
ca.CertificateRequestConfig{
527527
Token: tc.WorkerToken,
528528
ConnBroker: tc.ConnBroker,
529529
})
530-
assert.NoError(t, err)
530+
require.NoError(t, err)
531531
assert.NotNil(t, certs)
532532
parsedCerts, err = helpers.ParseCertificatesPEM(certs)
533-
assert.NoError(t, err)
533+
require.NoError(t, err)
534534
assert.Len(t, parsedCerts, 1)
535-
assert.True(t, time.Now().Add(ca.DefaultNodeCertExpiration).AddDate(0, 0, -1).Before(parsedCerts[0].NotAfter))
536-
assert.True(t, time.Now().Add(ca.DefaultNodeCertExpiration).AddDate(0, 0, 1).After(parsedCerts[0].NotAfter))
537-
assert.Equal(t, parsedCerts[0].Subject.OrganizationalUnit[0], ca.WorkerRole)
535+
assert.Less(t, time.Now().Add(ca.DefaultNodeCertExpiration).AddDate(0, 0, -1), parsedCerts[0].NotAfter)
536+
assert.Greater(t, time.Now().Add(ca.DefaultNodeCertExpiration).AddDate(0, 0, 1), parsedCerts[0].NotAfter)
537+
assert.Equal(t, ca.WorkerRole, parsedCerts[0].Subject.OrganizationalUnit[0])
538538
}
539539

540540
func TestGetRemoteSignedCertificateNodeInfo(t *testing.T) {
@@ -543,14 +543,14 @@ func TestGetRemoteSignedCertificateNodeInfo(t *testing.T) {
543543

544544
// Create a new CSR to be signed
545545
csr, _, err := ca.GenerateNewCSR()
546-
assert.NoError(t, err)
546+
require.NoError(t, err)
547547

548548
cert, err := ca.GetRemoteSignedCertificate(tc.Context, csr, tc.RootCA.Pool,
549549
ca.CertificateRequestConfig{
550550
Token: tc.WorkerToken,
551551
ConnBroker: tc.ConnBroker,
552552
})
553-
assert.NoError(t, err)
553+
require.NoError(t, err)
554554
assert.NotNil(t, cert)
555555
}
556556

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

717717
// Directly update the status of the store
718718
err = tc.MemoryStore.Update(func(tx store.Tx) error {
@@ -742,7 +742,7 @@ func TestGetRemoteSignedCertificateWithPending(t *testing.T) {
742742
select {
743743
case err = <-completed:
744744
s, _ := status.FromError(err)
745-
require.Equal(t, s.Code(), codes.DeadlineExceeded)
745+
require.Equal(t, codes.DeadlineExceeded, s.Code())
746746
case <-time.After(3 * time.Second):
747747
require.FailNow(t, "GetRemoteSignedCertificate should have been canceled after 1 second, and it has been 3")
748748
}
@@ -865,7 +865,7 @@ func TestGetRemoteSignedCertificateConnectionErrors(t *testing.T) {
865865
// wait for 5 seconds and ensure that GetRemoteSignedCertificate has returned with an error.
866866
select {
867867
case err = <-completed:
868-
require.Contains(t, err.Error(), "no more peers")
868+
require.ErrorContains(t, err, "no more peers")
869869
case <-time.After(5 * time.Second):
870870
require.FailNow(t, "GetRemoteSignedCertificate should errored after 5 seconds")
871871
}
@@ -909,68 +909,68 @@ func TestNewRootCABundle(t *testing.T) {
909909

910910
// make one rootCA
911911
firstRootCA, err := ca.CreateRootCA("rootCN1")
912-
assert.NoError(t, err)
912+
require.NoError(t, err)
913913

914914
// make a second root CA
915915
secondRootCA, err := ca.CreateRootCA("rootCN2")
916-
assert.NoError(t, err)
916+
require.NoError(t, err)
917917
s, err := firstRootCA.Signer()
918918
require.NoError(t, err)
919919

920920
// Overwrite the bytes of the second Root CA with the bundle, creating a valid 2 cert bundle
921921
bundle := append(firstRootCA.Certs, secondRootCA.Certs...)
922922
err = os.WriteFile(paths.RootCA.Cert, bundle, 0o644)
923-
assert.NoError(t, err)
923+
require.NoError(t, err)
924924

925925
newRootCA, err := ca.NewRootCA(bundle, firstRootCA.Certs, s.Key, ca.DefaultNodeCertExpiration, nil)
926-
assert.NoError(t, err)
926+
require.NoError(t, err)
927927
assert.Equal(t, bundle, newRootCA.Certs)
928-
assert.Equal(t, 2, len(newRootCA.Pool.Subjects()))
928+
assert.Len(t, newRootCA.Pool.Subjects(), 2)
929929

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

935935
certBytes, err := os.ReadFile(paths.Node.Cert)
936-
assert.NoError(t, err)
936+
require.NoError(t, err)
937937
assert.Len(t, checkLeafCert(t, certBytes, "rootCN1", "CN", "OU", "ORG"), 1)
938938
}
939939

940940
func TestNewRootCANonDefaultExpiry(t *testing.T) {
941941
rootCA, err := ca.CreateRootCA("rootCN")
942-
assert.NoError(t, err)
942+
require.NoError(t, err)
943943
s, err := rootCA.Signer()
944944
require.NoError(t, err)
945945

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

949949
// Create and sign a new CSR
950950
csr, _, err := ca.GenerateNewCSR()
951-
assert.NoError(t, err)
951+
require.NoError(t, err)
952952
cert, err := newRootCA.ParseValidateAndSignCSR(csr, "CN", ca.ManagerRole, "ORG")
953-
assert.NoError(t, err)
953+
require.NoError(t, err)
954954

955955
parsedCerts, err := helpers.ParseCertificatesPEM(cert)
956-
assert.NoError(t, err)
956+
require.NoError(t, err)
957957
assert.Len(t, parsedCerts, 1)
958-
assert.True(t, time.Now().Add(time.Minute*59).Before(parsedCerts[0].NotAfter))
959-
assert.True(t, time.Now().Add(time.Hour).Add(time.Minute).After(parsedCerts[0].NotAfter))
958+
assert.Less(t, time.Now().Add(time.Minute*59), parsedCerts[0].NotAfter)
959+
assert.Greater(t, time.Now().Add(time.Hour).Add(time.Minute), parsedCerts[0].NotAfter)
960960

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

966966
cert, err = newRootCA.ParseValidateAndSignCSR(csr, "CN", ca.ManagerRole, "ORG")
967-
assert.NoError(t, err)
967+
require.NoError(t, err)
968968

969969
parsedCerts, err = helpers.ParseCertificatesPEM(cert)
970-
assert.NoError(t, err)
970+
require.NoError(t, err)
971971
assert.Len(t, parsedCerts, 1)
972-
assert.True(t, time.Now().Add(ca.DefaultNodeCertExpiration).AddDate(0, 0, -1).Before(parsedCerts[0].NotAfter))
973-
assert.True(t, time.Now().Add(ca.DefaultNodeCertExpiration).AddDate(0, 0, 1).After(parsedCerts[0].NotAfter))
972+
assert.Less(t, time.Now().Add(ca.DefaultNodeCertExpiration).AddDate(0, 0, -1), parsedCerts[0].NotAfter)
973+
assert.Greater(t, time.Now().Add(ca.DefaultNodeCertExpiration).AddDate(0, 0, 1), parsedCerts[0].NotAfter)
974974
}
975975

976976
type invalidNewRootCATestCase struct {
@@ -1158,8 +1158,7 @@ func TestNewRootCAInvalidCertAndKeys(t *testing.T) {
11581158

11591159
for i, invalid := range invalids {
11601160
_, err := ca.NewRootCA(invalid.roots, invalid.cert, invalid.key, ca.DefaultNodeCertExpiration, invalid.intermediates)
1161-
require.Error(t, err, fmt.Sprintf("expected error containing: \"%s\", test case (%d)", invalid.errorStr, i))
1162-
require.Contains(t, err.Error(), invalid.errorStr, fmt.Sprintf("%d", i))
1161+
require.ErrorContains(t, err, invalid.errorStr, "%d", i)
11631162
}
11641163
}
11651164

@@ -1217,7 +1216,7 @@ func TestRootCAWithCrossSignedIntermediates(t *testing.T) {
12171216
require.NoError(t, err)
12181217
require.Len(t, parsedCerts, 2)
12191218
require.Len(t, chains, 1)
1220-
require.True(t, len(chains[0]) >= 2) // there are always at least 2 certs at minimum: the leaf and the root
1219+
require.GreaterOrEqual(t, len(chains[0]), 2) // there are always at least 2 certs at minimum: the leaf and the root
12211220
require.Equal(t, parsedCerts[0], chains[0][0])
12221221
require.Equal(t, parsedIntermediate.Raw, parsedCerts[1].Raw)
12231222

@@ -1373,8 +1372,7 @@ func TestValidateCertificateChain(t *testing.T) {
13731372
pool := x509.NewCertPool()
13741373
pool.AppendCertsFromPEM(invalid.root)
13751374
_, _, err := ca.ValidateCertChain(pool, invalid.cert, invalid.allowExpiry)
1376-
require.Error(t, err, invalid.errorStr)
1377-
require.Contains(t, err.Error(), invalid.errorStr)
1375+
require.ErrorContains(t, err, invalid.errorStr)
13781376
}
13791377

13801378
// these will default to using the root pool, so we don't have to specify the root pool
@@ -1402,7 +1400,7 @@ func TestValidateCertificateChain(t *testing.T) {
14021400
require.NotEmpty(t, chain)
14031401
for _, chain := range chains {
14041402
require.Equal(t, parsedCerts[0], chain[0]) // the leaf certs are equal
1405-
require.True(t, len(chain) >= 2)
1403+
require.GreaterOrEqual(t, len(chain), 2)
14061404
}
14071405
}
14081406
}

0 commit comments

Comments
 (0)