Skip to content

Commit

Permalink
sa: streamline use of dates in test (#7924)
Browse files Browse the repository at this point in the history 
Add mustTime and mustTimestamp, each of which parses a time in a simple
format and panics if it cannot be parsed.

Also, make the intent of each check in the GetRevokedCerts tests a
little clearer by starting with a basicRequest, and then defining
variations in terms of that request.

Fix the "different issuer" case in `TestGetRevokedCerts`, which was not
actually setting a different issuer.
  • Loading branch information
@jsha
jsha authored Jan 7, 2025
1 parent 673b93c commit f10f462
Showing 1 changed file with 63 additions and 79 deletions.
142 changes: 63 additions & 79 deletions sa/sa_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,18 @@ var (
}`
)

func mustTime(s string) time.Time {
t, err := time.Parse("2006-01-02 15:04", s)
if err != nil {
panic(fmt.Sprintf("parsing %q: %s", s, err))
}
return t.UTC()
}

func mustTimestamp(s string) *timestamppb.Timestamp {
return timestamppb.New(mustTime(s))
}

type fakeServerStream[T any] struct {
grpc.ServerStream
output chan<- *T
Expand Down Expand Up @@ -95,7 +107,7 @@ func initSA(t testing.TB) (*SQLStorageAuthority, clock.FakeClock, func()) {
}

fc := clock.NewFake()
fc.Set(time.Date(2015, 3, 4, 5, 0, 0, 0, time.UTC))
fc.Set(mustTime("2015-03-04 05:00"))

saro, err := NewSQLStorageAuthorityRO(dbMap, dbIncidentsMap, metrics.NoopRegisterer, 1, 0, fc, log)
if err != nil {
Expand All @@ -116,7 +128,7 @@ func createWorkingRegistration(t testing.TB, sa *SQLStorageAuthority) *corepb.Re
reg, err := sa.NewRegistration(context.Background(), &corepb.Registration{
Key: []byte(theKey),
Contact: []string{"mailto:foo@example.com"},
CreatedAt: timestamppb.New(time.Date(2003, 5, 10, 0, 0, 0, 0, time.UTC)),
CreatedAt: mustTimestamp("2003-05-10 00:00"),
Status: string(core.StatusValid),
})
if err != nil {
Expand Down Expand Up @@ -408,11 +420,11 @@ func TestAddPrecertificate(t *testing.T) {

// Add the cert as a precertificate
regID := reg.Id
issuedTime := time.Date(2018, 4, 1, 7, 0, 0, 0, time.UTC)
issuedTime := mustTimestamp("2018-04-01 07:00")
_, err := sa.AddPrecertificate(ctx, &sapb.AddCertificateRequest{
Der: testCert.Raw,
RegID: regID,
Issued: timestamppb.New(issuedTime),
Issued: issuedTime,
IssuerNameID: 1,
})
test.AssertNotError(t, err, "Couldn't add test cert")
Expand All @@ -435,7 +447,7 @@ func TestAddPrecertificate(t *testing.T) {
_, err = sa.AddCertificate(ctx, &sapb.AddCertificateRequest{
Der: testCert.Raw,
RegID: regID,
Issued: timestamppb.New(issuedTime),
Issued: issuedTime,
})
test.AssertNotError(t, err, "unexpected err adding final cert after precert")
}
Expand All @@ -448,11 +460,11 @@ func TestAddPrecertificateNoOCSP(t *testing.T) {
_, testCert := test.ThrowAwayCert(t, clk)

regID := reg.Id
issuedTime := time.Date(2018, 4, 1, 7, 0, 0, 0, time.UTC)
issuedTime := mustTimestamp("2018-04-01 07:00")
_, err := sa.AddPrecertificate(ctx, &sapb.AddCertificateRequest{
Der: testCert.Raw,
RegID: regID,
Issued: timestamppb.New(issuedTime),
Issued: issuedTime,
IssuerNameID: 1,
})
test.AssertNotError(t, err, "Couldn't add test cert")
Expand Down Expand Up @@ -496,11 +508,10 @@ func TestAddPrecertificateIncomplete(t *testing.T) {

// Add the cert as a precertificate
regID := reg.Id
issuedTime := time.Date(2018, 4, 1, 7, 0, 0, 0, time.UTC)
_, err := sa.AddPrecertificate(ctx, &sapb.AddCertificateRequest{
Der: testCert.Raw,
RegID: regID,
Issued: timestamppb.New(issuedTime),
Issued: mustTimestamp("2018-04-01 07:00"),
// Leaving out IssuerNameID
})

Expand Down Expand Up @@ -858,7 +869,7 @@ func (e *queryRecorder) QueryContext(ctx context.Context, query string, args ...
func TestAddIssuedNames(t *testing.T) {
serial := big.NewInt(1)
expectedSerial := "000000000000000000000000000000000001"
notBefore := time.Date(2018, 2, 14, 12, 0, 0, 0, time.UTC)
notBefore := mustTime("2018-02-14 12:00")
expectedNotBefore := notBefore.Truncate(24 * time.Hour)
placeholdersPerName := "(?,?,?,?)"
baseQuery := "INSERT INTO issuedNames (reversedName,serial,notBefore,renewal) VALUES"
Expand Down Expand Up @@ -2528,7 +2539,7 @@ func TestFinalizeAuthorization2(t *testing.T) {
sa, fc, cleanUp := initSA(t)
defer cleanUp()

fc.Set(time.Date(2021, 1, 1, 0, 0, 0, 0, time.UTC))
fc.Set(mustTime("2021-01-01 00:00"))

authzID := createPendingAuthorization(t, sa, "aaa", fc.Now().Add(time.Hour))
expires := fc.Now().Add(time.Hour * 2).UTC()
Expand Down Expand Up @@ -2600,7 +2611,7 @@ func TestRehydrateHostPort(t *testing.T) {
sa, fc, cleanUp := initSA(t)
defer cleanUp()

fc.Set(time.Date(2021, 1, 1, 0, 0, 0, 0, time.UTC))
fc.Set(mustTime("2021-01-01 00:00"))

expires := fc.Now().Add(time.Hour * 2).UTC()
attemptedAt := fc.Now()
Expand Down Expand Up @@ -3284,73 +3295,59 @@ func TestGetRevokedCerts(t *testing.T) {
return entriesReceived, err
}

// Asking for revoked certs now should return no results.
expiresAfter := time.Date(2023, time.March, 1, 0, 0, 0, 0, time.UTC)
expiresBefore := time.Date(2023, time.April, 1, 0, 0, 0, 0, time.UTC)
revokedBefore := time.Date(2023, time.April, 1, 0, 0, 0, 0, time.UTC)
count, err := countRevokedCerts(&sapb.GetRevokedCertsRequest{
// The basic request covers a time range that should include this certificate.
basicRequest := &sapb.GetRevokedCertsRequest{
IssuerNameID: 1,
ExpiresAfter: timestamppb.New(expiresAfter),
ExpiresBefore: timestamppb.New(expiresBefore),
RevokedBefore: timestamppb.New(revokedBefore),
})
ExpiresAfter: mustTimestamp("2023-03-01 00:00"),
ExpiresBefore: mustTimestamp("2023-04-01 00:00"),
RevokedBefore: mustTimestamp("2023-04-01 00:00"),
}
count, err := countRevokedCerts(basicRequest)
test.AssertNotError(t, err, "zero rows shouldn't result in error")
test.AssertEquals(t, count, 0)

// Revoke the certificate.
date := time.Date(2023, time.January, 1, 0, 0, 0, 0, time.UTC)
_, err = sa.RevokeCertificate(context.Background(), &sapb.RevokeCertificateRequest{
IssuerID: 1,
Serial: core.SerialToString(eeCert.SerialNumber),
Date: timestamppb.New(date),
Date: mustTimestamp("2023-01-01 00:00"),
Reason: 1,
Response: []byte{1, 2, 3},
})
test.AssertNotError(t, err, "failed to revoke test cert")

// Asking for revoked certs now should return one result.
count, err = countRevokedCerts(&sapb.GetRevokedCertsRequest{
IssuerNameID: 1,
ExpiresAfter: timestamppb.New(expiresAfter),
ExpiresBefore: timestamppb.New(expiresBefore),
RevokedBefore: timestamppb.New(revokedBefore),
})
count, err = countRevokedCerts(basicRequest)
test.AssertNotError(t, err, "normal usage shouldn't result in error")
test.AssertEquals(t, count, 1)

// Asking for revoked certs with an old RevokedBefore should return no results.
expiresAfter = time.Date(2023, time.March, 1, 0, 0, 0, 0, time.UTC)
expiresBefore = time.Date(2023, time.April, 1, 0, 0, 0, 0, time.UTC)
revokedBefore = time.Date(2020, time.March, 1, 0, 0, 0, 0, time.UTC)
count, err = countRevokedCerts(&sapb.GetRevokedCertsRequest{
IssuerNameID: 1,
ExpiresAfter: timestamppb.New(expiresAfter),
ExpiresBefore: timestamppb.New(expiresBefore),
RevokedBefore: timestamppb.New(revokedBefore),
ExpiresAfter: basicRequest.ExpiresAfter,
ExpiresBefore: basicRequest.ExpiresBefore,
RevokedBefore: mustTimestamp("2020-03-01 00:00"),
})
test.AssertNotError(t, err, "zero rows shouldn't result in error")
test.AssertEquals(t, count, 0)

// Asking for revoked certs in a time period that does not cover this cert's
// notAfter timestamp should return zero results.
expiresAfter = time.Date(2022, time.March, 1, 0, 0, 0, 0, time.UTC)
expiresBefore = time.Date(2022, time.April, 1, 0, 0, 0, 0, time.UTC)
revokedBefore = time.Date(2023, time.April, 1, 0, 0, 0, 0, time.UTC)
count, err = countRevokedCerts(&sapb.GetRevokedCertsRequest{
IssuerNameID: 1,
ExpiresAfter: timestamppb.New(expiresAfter),
ExpiresBefore: timestamppb.New(expiresBefore),
RevokedBefore: timestamppb.New(revokedBefore),
ExpiresAfter: mustTimestamp("2022-03-01 00:00"),
ExpiresBefore: mustTimestamp("2022-04-01 00:00"),
RevokedBefore: mustTimestamp("2023-04-01 00:00"),
})
test.AssertNotError(t, err, "zero rows shouldn't result in error")
test.AssertEquals(t, count, 0)

// Asking for revoked certs from a different issuer should return zero results.
count, err = countRevokedCerts(&sapb.GetRevokedCertsRequest{
IssuerNameID: 1,
ExpiresAfter: timestamppb.New(time.Date(2022, time.March, 1, 0, 0, 0, 0, time.UTC)),
ExpiresBefore: timestamppb.New(time.Date(2022, time.April, 1, 0, 0, 0, 0, time.UTC)),
RevokedBefore: timestamppb.New(time.Date(2023, time.April, 1, 0, 0, 0, 0, time.UTC)),
IssuerNameID: 5678,
ExpiresAfter: basicRequest.ExpiresAfter,
ExpiresBefore: basicRequest.ExpiresBefore,
RevokedBefore: basicRequest.RevokedBefore,
})
test.AssertNotError(t, err, "zero rows shouldn't result in error")
test.AssertEquals(t, count, 0)
Expand Down Expand Up @@ -3404,25 +3401,25 @@ func TestGetRevokedCertsByShard(t *testing.T) {
return entriesReceived, err
}

// Asking for revoked certs now should return no results.
expiresAfter := time.Date(2023, time.March, 1, 0, 0, 0, 0, time.UTC)
revokedBefore := time.Date(2023, time.April, 1, 0, 0, 0, 0, time.UTC)
count, err := countRevokedCerts(&sapb.GetRevokedCertsRequest{
// The basic request covers a time range and shard that should include this certificate.
basicRequest := &sapb.GetRevokedCertsRequest{
IssuerNameID: 1,
ShardIdx: 9,
ExpiresAfter: timestamppb.New(expiresAfter),
RevokedBefore: timestamppb.New(revokedBefore),
})
ExpiresAfter: mustTimestamp("2023-03-01 00:00"),
RevokedBefore: mustTimestamp("2023-04-01 00:00"),
}

// Nothing's been revoked yet. Count should be zero.
count, err := countRevokedCerts(basicRequest)
test.AssertNotError(t, err, "zero rows shouldn't result in error")
test.AssertEquals(t, count, 0)

// Revoke the certificate, providing the ShardIdx so it gets written into
// both the certificateStatus and revokedCertificates tables.
date := time.Date(2023, time.January, 1, 0, 0, 0, 0, time.UTC)
_, err = sa.RevokeCertificate(context.Background(), &sapb.RevokeCertificateRequest{
IssuerID: 1,
Serial: core.SerialToString(eeCert.SerialNumber),
Date: timestamppb.New(date),
Date: mustTimestamp("2023-01-01 00:00"),
Reason: 1,
Response: []byte{1, 2, 3},
ShardIdx: 9,
Expand All @@ -3437,49 +3434,36 @@ func TestGetRevokedCertsByShard(t *testing.T) {
test.AssertEquals(t, c.Int64, int64(1))

// Asking for revoked certs now should return one result.
expiresAfter = time.Date(2023, time.March, 1, 0, 0, 0, 0, time.UTC)
revokedBefore = time.Date(2023, time.April, 1, 0, 0, 0, 0, time.UTC)
count, err = countRevokedCerts(&sapb.GetRevokedCertsRequest{
IssuerNameID: 1,
ShardIdx: 9,
ExpiresAfter: timestamppb.New(expiresAfter),
RevokedBefore: timestamppb.New(revokedBefore),
})
count, err = countRevokedCerts(basicRequest)
test.AssertNotError(t, err, "normal usage shouldn't result in error")
test.AssertEquals(t, count, 1)

// Asking for revoked certs from a different issuer should return zero results.
expiresAfter = time.Date(2023, time.March, 1, 0, 0, 0, 0, time.UTC)
revokedBefore = time.Date(2023, time.April, 1, 0, 0, 0, 0, time.UTC)
count, err = countRevokedCerts(&sapb.GetRevokedCertsRequest{
IssuerNameID: 2,
ShardIdx: 9,
ExpiresAfter: timestamppb.New(expiresAfter),
RevokedBefore: timestamppb.New(revokedBefore),
IssuerNameID: 5678,
ShardIdx: basicRequest.ShardIdx,
ExpiresAfter: basicRequest.ExpiresAfter,
RevokedBefore: basicRequest.RevokedBefore,
})
test.AssertNotError(t, err, "zero rows shouldn't result in error")
test.AssertEquals(t, count, 0)

// Asking for revoked certs from a different shard should return zero results.
expiresAfter = time.Date(2023, time.March, 1, 0, 0, 0, 0, time.UTC)
revokedBefore = time.Date(2023, time.April, 1, 0, 0, 0, 0, time.UTC)
count, err = countRevokedCerts(&sapb.GetRevokedCertsRequest{
IssuerNameID: 1,
IssuerNameID: basicRequest.IssuerNameID,
ShardIdx: 8,
ExpiresAfter: timestamppb.New(expiresAfter),
RevokedBefore: timestamppb.New(revokedBefore),
ExpiresAfter: basicRequest.ExpiresAfter,
RevokedBefore: basicRequest.RevokedBefore,
})
test.AssertNotError(t, err, "zero rows shouldn't result in error")
test.AssertEquals(t, count, 0)

// Asking for revoked certs with an old RevokedBefore should return no results.
expiresAfter = time.Date(2023, time.March, 1, 0, 0, 0, 0, time.UTC)
revokedBefore = time.Date(2020, time.March, 1, 0, 0, 0, 0, time.UTC)
count, err = countRevokedCerts(&sapb.GetRevokedCertsRequest{
IssuerNameID: 1,
ShardIdx: 9,
ExpiresAfter: timestamppb.New(expiresAfter),
RevokedBefore: timestamppb.New(revokedBefore),
IssuerNameID: basicRequest.IssuerNameID,
ShardIdx: basicRequest.ShardIdx,
ExpiresAfter: basicRequest.ExpiresAfter,
RevokedBefore: mustTimestamp("2020-03-01 00:00"),
})
test.AssertNotError(t, err, "zero rows shouldn't result in error")
test.AssertEquals(t, count, 0)
Expand Down

0 comments on commit f10f462

Please sign in to comment.