diff --git a/sa/sa.go b/sa/sa.go index 59bc98a15..ec36d53b0 100644 --- a/sa/sa.go +++ b/sa/sa.go @@ -1112,20 +1112,27 @@ func (ssa *SQLStorageAuthority) leaseSpecificCRLShard(ctx context.Context, req * // rows are created if necessary when leased). It also sets the leasedUntil time // to be equal to thisUpdate, to indicate that the shard is no longer leased. func (ssa *SQLStorageAuthority) UpdateCRLShard(ctx context.Context, req *sapb.UpdateCRLShardRequest) (*emptypb.Empty, error) { - if core.IsAnyNilOrZero(req.IssuerNameID, req.ThisUpdate, req.NextUpdate) { + if core.IsAnyNilOrZero(req.IssuerNameID, req.ThisUpdate) { return nil, errIncompleteRequest } + // Only set the nextUpdate if it's actually present in the request message. + var nextUpdate *time.Time + if req.NextUpdate != nil { + nut := req.NextUpdate.AsTime() + nextUpdate = &nut + } + _, err := db.WithTransaction(ctx, ssa.dbMap, func(tx db.Executor) (interface{}, error) { res, err := tx.ExecContext(ctx, `UPDATE crlShards SET thisUpdate = ?, nextUpdate = ?, leasedUntil = ? WHERE issuerID = ? AND idx = ? - AND thisUpdate < ? + AND (thisUpdate is NULL OR thisUpdate < ?) LIMIT 1`, req.ThisUpdate.AsTime(), - req.NextUpdate.AsTime(), + nextUpdate, req.ThisUpdate.AsTime(), req.IssuerNameID, req.ShardIdx, diff --git a/sa/sa_test.go b/sa/sa_test.go index 0361ce6ed..9f5c8e678 100644 --- a/sa/sa_test.go +++ b/sa/sa_test.go @@ -3492,10 +3492,6 @@ func TestLeaseSpecificCRLShard(t *testing.T) { } func TestUpdateCRLShard(t *testing.T) { - if os.Getenv("BOULDER_CONFIG_DIR") != "test/config-next" { - t.Skip("Test requires crlShards database table") - } - sa, clk, cleanUp := initSA(t) defer cleanUp() @@ -3523,8 +3519,9 @@ func TestUpdateCRLShard(t *testing.T) { test.AssertNotError(t, err, "setting up test shards") thisUpdate := clk.Now().Truncate(time.Second).UTC() - var thisUpdateModel struct { - ThisUpdate time.Time `db:"thisUpdate"` + var crlModel struct { + ThisUpdate *time.Time + NextUpdate *time.Time } // Updating a leased shard should work. @@ -3541,13 +3538,11 @@ func TestUpdateCRLShard(t *testing.T) { err = sa.dbMap.SelectOne( ctx, - &thisUpdateModel, - `SELECT thisUpdate FROM crlShards WHERE issuerID = ? AND idx = ? LIMIT 1`, - 1, - 0, + &crlModel, + `SELECT thisUpdate FROM crlShards WHERE issuerID = 1 AND idx = 0 LIMIT 1`, ) test.AssertNotError(t, err, "getting updated thisUpdate timestamp") - test.Assert(t, thisUpdateModel.ThisUpdate.Equal(thisUpdate), "checking updated thisUpdate timestamp") + test.Assert(t, crlModel.ThisUpdate.Equal(thisUpdate), "checking updated thisUpdate timestamp") // Updating an unleased shard should work. _, err = sa.UpdateCRLShard( @@ -3563,13 +3558,30 @@ func TestUpdateCRLShard(t *testing.T) { err = sa.dbMap.SelectOne( ctx, - &thisUpdateModel, - `SELECT thisUpdate FROM crlShards WHERE issuerID = ? AND idx = ? LIMIT 1`, - 1, - 1, + &crlModel, + `SELECT thisUpdate FROM crlShards WHERE issuerID = 1 AND idx = 1 LIMIT 1`, ) test.AssertNotError(t, err, "getting updated thisUpdate timestamp") - test.Assert(t, thisUpdateModel.ThisUpdate.Equal(thisUpdate), "checking updated thisUpdate timestamp") + test.Assert(t, crlModel.ThisUpdate.Equal(thisUpdate), "checking updated thisUpdate timestamp") + + // Updating without supplying a NextUpdate should work. + _, err = sa.UpdateCRLShard( + context.Background(), + &sapb.UpdateCRLShardRequest{ + IssuerNameID: 1, + ShardIdx: 3, + ThisUpdate: timestamppb.New(thisUpdate.Add(time.Second)), + }, + ) + test.AssertNotError(t, err, "updating shard without NextUpdate") + + err = sa.dbMap.SelectOne( + ctx, + &crlModel, + `SELECT nextUpdate FROM crlShards WHERE issuerID = 1 AND idx = 3 LIMIT 1`, + ) + test.AssertNotError(t, err, "getting updated nextUpdate timestamp") + test.AssertBoxedNil(t, crlModel.NextUpdate, "checking updated nextUpdate timestamp") // Updating a shard to an earlier time should fail. _, err = sa.UpdateCRLShard( @@ -3593,5 +3605,5 @@ func TestUpdateCRLShard(t *testing.T) { NextUpdate: timestamppb.New(thisUpdate.Add(10 * 24 * time.Hour)), }, ) - test.AssertError(t, err, "updating shard to an earlier time") + test.AssertError(t, err, "updating an unknown shard") }