From 279a4d539df69aee977771fb7310362301e4a607 Mon Sep 17 00:00:00 2001 From: Phil Porada Date: Mon, 13 Nov 2023 12:23:46 -0500 Subject: [PATCH] Read from durationpb instead of int64 durations (#7146) Switch to reading grpc duration values from the new durationpb protofbuf fields, completely ignoring the old int64 fields. Part 2 of 3 for https://github.com/letsencrypt/boulder/issues/7097 --- core/util.go | 5 +++++ core/util_test.go | 7 +++++++ sa/sa_test.go | 23 +++++++++++++++++++++-- sa/saro.go | 8 ++++---- 4 files changed, 37 insertions(+), 6 deletions(-) diff --git a/core/util.go b/core/util.go index d7fe0266895..6582b119c5b 100644 --- a/core/util.go +++ b/core/util.go @@ -25,6 +25,7 @@ import ( "time" "unicode" + "google.golang.org/protobuf/types/known/durationpb" "gopkg.in/go-jose/go-jose.v2" ) @@ -216,6 +217,10 @@ func IsAnyNilOrZero(vals ...interface{}) bool { if len(v) == 0 { return true } + case *durationpb.Duration: + if v == nil || v.AsDuration() == time.Duration(0) { + return true + } default: if reflect.ValueOf(v).IsZero() { return true diff --git a/core/util_test.go b/core/util_test.go index 211ee89ceeb..5f8f71f9e50 100644 --- a/core/util_test.go +++ b/core/util_test.go @@ -12,6 +12,7 @@ import ( "testing" "time" + "google.golang.org/protobuf/types/known/durationpb" "gopkg.in/go-jose/go-jose.v2" "github.com/letsencrypt/boulder/test" @@ -135,6 +136,12 @@ func TestIsAnyNilOrZero(t *testing.T) { test.Assert(t, IsAnyNilOrZero(1, ""), "Mixed values seen as non-zero") test.Assert(t, IsAnyNilOrZero("", 1), "Mixed values seen as non-zero") + + var d *durationpb.Duration + var zeroDuration time.Duration + test.Assert(t, IsAnyNilOrZero(d), "Pointer to uninitialized durationpb.Duration seen as non-zero") + test.Assert(t, IsAnyNilOrZero(durationpb.New(zeroDuration)), "*durationpb.Duration containing an zero value time.Duration is seen as non-zero") + test.Assert(t, !IsAnyNilOrZero(durationpb.New(666)), "A *durationpb.Duration with valid inner duration is seen as zero") } func TestUniqueLowerNames(t *testing.T) { diff --git a/sa/sa_test.go b/sa/sa_test.go index 65764591527..048430d34f8 100644 --- a/sa/sa_test.go +++ b/sa/sa_test.go @@ -918,8 +918,17 @@ func TestFQDNSets(t *testing.T) { test.AssertNotError(t, err, "Failed to add name set") test.AssertNotError(t, tx.Commit(), "Failed to commit transaction") - threeHours := time.Hour * 3 + // Invalid Window req := &sapb.CountFQDNSetsRequest{ + Domains: names, + WindowNS: 0, + Window: nil, + } + _, err = sa.CountFQDNSets(ctx, req) + test.AssertErrorIs(t, err, errIncompleteRequest) + + threeHours := time.Hour * 3 + req = &sapb.CountFQDNSetsRequest{ Domains: names, WindowNS: threeHours.Nanoseconds(), Window: durationpb.New(threeHours), @@ -977,8 +986,18 @@ func TestFQDNSetTimestampsForWindow(t *testing.T) { test.AssertNotError(t, err, "Failed to open transaction") names := []string{"a.example.com", "B.example.com"} - window := time.Hour * 3 + + // Invalid Window req := &sapb.CountFQDNSetsRequest{ + Domains: names, + WindowNS: 0, + Window: nil, + } + _, err = sa.FQDNSetTimestampsForWindow(ctx, req) + test.AssertErrorIs(t, err, errIncompleteRequest) + + window := time.Hour * 3 + req = &sapb.CountFQDNSetsRequest{ Domains: names, WindowNS: window.Nanoseconds(), Window: durationpb.New(window), diff --git a/sa/saro.go b/sa/saro.go index a74d356c8ec..4f73934a80b 100644 --- a/sa/saro.go +++ b/sa/saro.go @@ -507,7 +507,7 @@ func (ssa *SQLStorageAuthority) CountOrders(ctx context.Context, req *sapb.Count // CountFQDNSets counts the total number of issuances, for a set of domains, // that occurred during a given window of time. func (ssa *SQLStorageAuthorityRO) CountFQDNSets(ctx context.Context, req *sapb.CountFQDNSetsRequest) (*sapb.Count, error) { - if req.WindowNS == 0 || len(req.Domains) == 0 { + if core.IsAnyNilOrZero(req.Window) || len(req.Domains) == 0 { return nil, errIncompleteRequest } @@ -519,7 +519,7 @@ func (ssa *SQLStorageAuthorityRO) CountFQDNSets(ctx context.Context, req *sapb.C WHERE setHash = ? AND issued > ?`, core.HashNames(req.Domains), - ssa.clk.Now().Add(-time.Duration(req.WindowNS)), + ssa.clk.Now().Add(-req.Window.AsDuration()), ) return &sapb.Count{Count: count}, err } @@ -532,7 +532,7 @@ func (ssa *SQLStorageAuthority) CountFQDNSets(ctx context.Context, req *sapb.Cou // certificate, issued for a set of domains, during a given window of time, // starting from the most recent issuance. func (ssa *SQLStorageAuthorityRO) FQDNSetTimestampsForWindow(ctx context.Context, req *sapb.CountFQDNSetsRequest) (*sapb.Timestamps, error) { - if req.WindowNS == 0 || len(req.Domains) == 0 { + if core.IsAnyNilOrZero(req.Window) || len(req.Domains) == 0 { return nil, errIncompleteRequest } type row struct { @@ -547,7 +547,7 @@ func (ssa *SQLStorageAuthorityRO) FQDNSetTimestampsForWindow(ctx context.Context AND issued > ? ORDER BY issued DESC`, core.HashNames(req.Domains), - ssa.clk.Now().Add(-time.Duration(req.WindowNS)), + ssa.clk.Now().Add(-req.Window.AsDuration()), ) if err != nil { return nil, err