From 261fbe5a29cabc114d6ff37806bce18e2a592341 Mon Sep 17 00:00:00 2001 From: Philippe Boneff Date: Wed, 8 Jan 2025 18:20:00 +0000 Subject: [PATCH 1/5] s/tesseraDeadline/httpDeadline --- cmd/gcp/main.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/gcp/main.go b/cmd/gcp/main.go index 09557078..27493ad6 100644 --- a/cmd/gcp/main.go +++ b/cmd/gcp/main.go @@ -52,7 +52,7 @@ var ( httpEndpoint = flag.String("http_endpoint", "localhost:6962", "Endpoint for HTTP (host:port).") metricsEndpoint = flag.String("metrics_endpoint", "", "Endpoint for serving metrics; if left empty, metrics will be visible on --http_endpoint.") - tesseraDeadline = flag.Duration("tessera_deadline", time.Second*10, "Deadline for Tessera requests.") + httpDeadline = flag.Duration("http_deadline", time.Second*10, "Deadline for HTTP requests.") maskInternalErrors = flag.Bool("mask_internal_errors", false, "Don't return error strings with Internal Server Error HTTP responses.") tracing = flag.Bool("tracing", false, "If true opencensus Stackdriver tracing will be enabled. See https://opencensus.io/.") tracingProjectID = flag.String("tracing_project_id", "", "project ID to pass to stackdriver. Can be empty for GCP, consult docs for other platforms.") @@ -104,7 +104,7 @@ func main() { // Register handlers for all the configured logs. opts := sctfe.InstanceOptions{ Validated: vCfg, - Deadline: *tesseraDeadline, + Deadline: *httpDeadline, MetricFactory: prometheus.MetricFactory{}, RequestLog: new(sctfe.DefaultRequestLog), MaskInternalErrors: *maskInternalErrors, From ecd2db1c729f23e143292a8070994b32ceaef3ac Mon Sep 17 00:00:00 2001 From: Philippe Boneff Date: Fri, 10 Jan 2025 14:33:27 +0000 Subject: [PATCH 2/5] add comments --- handlers.go | 2 +- instance.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/handlers.go b/handlers.go index 1356a8bd..617e7961 100644 --- a/handlers.go +++ b/handlers.go @@ -416,7 +416,7 @@ func getRoots(_ context.Context, li *logInfo, w http.ResponseWriter, _ *http.Req return http.StatusOK, nil } -// deadlineTime calculates the future time an RPC should expire based on our config +// deadlineTime calculates the future time a request should expire based on our config. func deadlineTime(li *logInfo) time.Time { return li.TimeSource.Now().Add(li.instanceOpts.Deadline) } diff --git a/instance.go b/instance.go index 9836269b..636a3108 100644 --- a/instance.go +++ b/instance.go @@ -35,7 +35,7 @@ type InstanceOptions struct { Validated *ValidatedLogConfig // CreateStorage instantiates a Tessera storage implementation with a signer option. CreateStorage func(context.Context, note.Signer) (*CTStorage, error) - // Deadline is a timeout for Tessera requests. + // Deadline is a timeout for HTTP requests. Deadline time.Duration // MetricFactory allows creating metrics. MetricFactory monitoring.MetricFactory From a0307dc35572793eaf1484e79b20d2e68c25d6f5 Mon Sep 17 00:00:00 2001 From: Philippe Boneff Date: Thu, 9 Jan 2025 12:42:48 +0000 Subject: [PATCH 3/5] s/LogOrigin/Origin/g --- handlers.go | 40 ++++++++++++++++++++-------------------- requestlog.go | 8 ++++---- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/handlers.go b/handlers.go index 617e7961..2d53e069 100644 --- a/handlers.go +++ b/handlers.go @@ -100,19 +100,19 @@ type AppHandler struct { // does additional common error and stats processing. func (a AppHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { var statusCode int - label0 := a.Info.LogOrigin + label0 := a.Info.Origin label1 := string(a.Name) reqsCounter.Inc(label0, label1) startTime := a.Info.TimeSource.Now() logCtx := a.Info.RequestLog.Start(r.Context()) - a.Info.RequestLog.LogOrigin(logCtx, a.Info.LogOrigin) + a.Info.RequestLog.Origin(logCtx, a.Info.Origin) defer func() { latency := a.Info.TimeSource.Now().Sub(startTime).Seconds() rspLatency.Observe(latency, label0, label1, strconv.Itoa(statusCode)) }() - klog.V(2).Infof("%s: request %v %q => %s", a.Info.LogOrigin, r.Method, r.URL, a.Name) + klog.V(2).Infof("%s: request %v %q => %s", a.Info.Origin, r.Method, r.URL, a.Name) if r.Method != a.Method { - klog.Warningf("%s: %s wrong HTTP method: %v", a.Info.LogOrigin, a.Name, r.Method) + klog.Warningf("%s: %s wrong HTTP method: %v", a.Info.Origin, a.Name, r.Method) a.Info.SendHTTPError(w, http.StatusMethodNotAllowed, fmt.Errorf("method not allowed: %s", r.Method)) a.Info.RequestLog.Status(logCtx, http.StatusMethodNotAllowed) return @@ -135,17 +135,17 @@ func (a AppHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { var err error statusCode, err = a.Handler(ctx, a.Info, w, r) a.Info.RequestLog.Status(ctx, statusCode) - klog.V(2).Infof("%s: %s <= st=%d", a.Info.LogOrigin, a.Name, statusCode) + klog.V(2).Infof("%s: %s <= st=%d", a.Info.Origin, a.Name, statusCode) rspsCounter.Inc(label0, label1, strconv.Itoa(statusCode)) if err != nil { - klog.Warningf("%s: %s handler error: %v", a.Info.LogOrigin, a.Name, err) + klog.Warningf("%s: %s handler error: %v", a.Info.Origin, a.Name, err) a.Info.SendHTTPError(w, statusCode, err) return } // Additional check, for consistency the handler must return an error for non-200 st if statusCode != http.StatusOK { - klog.Warningf("%s: %s handler non 200 without error: %d %v", a.Info.LogOrigin, a.Name, statusCode, err) + klog.Warningf("%s: %s handler non 200 without error: %d %v", a.Info.Origin, a.Name, statusCode, err) a.Info.SendHTTPError(w, http.StatusInternalServerError, fmt.Errorf("http handler misbehaved, st: %d", statusCode)) return } @@ -190,8 +190,8 @@ func NewCertValidationOpts(trustedRoots *x509util.PEMCertPool, currentTime time. // logInfo holds information for a specific log instance. type logInfo struct { - // LogOrigin identifies the log, as per https://c2sp.org/static-ct-api - LogOrigin string + // Origin identifies the log, as per https://c2sp.org/static-ct-api + Origin string // TimeSource is a TimeSource that can be injected for testing TimeSource TimeSource // RequestLog is a logger for various request / processing / response debug @@ -219,7 +219,7 @@ func newLogInfo( cfg := instanceOpts.Validated li := &logInfo{ - LogOrigin: cfg.Origin, + Origin: cfg.Origin, storage: storage, signer: signer, TimeSource: timeSource, @@ -297,7 +297,7 @@ func addChainInternal(ctx context.Context, li *logInfo, w http.ResponseWriter, r // Check the contents of the request and convert to slice of certificates. addChainReq, err := ParseBodyAsJSONChain(r) if err != nil { - return http.StatusBadRequest, fmt.Errorf("%s: failed to parse add-chain body: %s", li.LogOrigin, err) + return http.StatusBadRequest, fmt.Errorf("%s: failed to parse add-chain body: %s", li.Origin, err) } // Log the DERs now because they might not parse as valid X.509. for _, der := range addChainReq.Chain { @@ -319,7 +319,7 @@ func addChainInternal(ctx context.Context, li *logInfo, w http.ResponseWriter, r return http.StatusBadRequest, fmt.Errorf("failed to build MerkleTreeLeaf: %s", err) } - klog.V(2).Infof("%s: %s => storage.GetCertIndex", li.LogOrigin, method) + klog.V(2).Infof("%s: %s => storage.GetCertIndex", li.Origin, method) sctDedupInfo, isDup, err := li.storage.GetCertDedupInfo(ctx, chain[0]) idx := sctDedupInfo.Idx if err != nil { @@ -327,14 +327,14 @@ func addChainInternal(ctx context.Context, li *logInfo, w http.ResponseWriter, r } if isDup { - klog.V(3).Infof("%s: %s - found duplicate entry at index %d", li.LogOrigin, method, idx) + klog.V(3).Infof("%s: %s - found duplicate entry at index %d", li.Origin, method, idx) entry.Timestamp = sctDedupInfo.Timestamp } else { if err := li.storage.AddIssuerChain(ctx, chain[1:]); err != nil { return http.StatusInternalServerError, fmt.Errorf("failed to store issuer chain: %s", err) } - klog.V(2).Infof("%s: %s => storage.Add", li.LogOrigin, method) + klog.V(2).Infof("%s: %s => storage.Add", li.Origin, method) idx, err = li.storage.Add(ctx, entry)() if err != nil { if errors.Is(err, tessera.ErrPushback) { @@ -346,7 +346,7 @@ func addChainInternal(ctx context.Context, li *logInfo, w http.ResponseWriter, r // We store the index for this certificate in the deduplication storage immediately. // It might be stored again later, if a local deduplication storage is synced, potentially // with a smaller value. - klog.V(2).Infof("%s: %s => storage.AddCertIndex", li.LogOrigin, method) + klog.V(2).Infof("%s: %s => storage.AddCertIndex", li.Origin, method) err = li.storage.AddCertDedupInfo(ctx, chain[0], dedup.SCTDedupInfo{Idx: idx, Timestamp: entry.Timestamp}) // TODO: block log writes if deduplication breaks if err != nil { @@ -381,9 +381,9 @@ func addChainInternal(ctx context.Context, li *logInfo, w http.ResponseWriter, r // reason is logged and http status is already set return http.StatusInternalServerError, fmt.Errorf("failed to write response: %s", err) } - klog.V(3).Infof("%s: %s <= SCT", li.LogOrigin, method) + klog.V(3).Infof("%s: %s <= SCT", li.Origin, method) if sct.Timestamp == timeMillis { - lastSCTTimestamp.Set(float64(sct.Timestamp), li.LogOrigin) + lastSCTTimestamp.Set(float64(sct.Timestamp), li.Origin) } return http.StatusOK, nil @@ -409,7 +409,7 @@ func getRoots(_ context.Context, li *logInfo, w http.ResponseWriter, _ *http.Req enc := json.NewEncoder(w) err := enc.Encode(jsonMap) if err != nil { - klog.Warningf("%s: get_roots failed: %v", li.LogOrigin, err) + klog.Warningf("%s: get_roots failed: %v", li.Origin, err) return http.StatusInternalServerError, fmt.Errorf("get-roots failed with: %s", err) } @@ -440,9 +440,9 @@ func verifyAddChain(li *logInfo, req ct.AddChainRequest, expectingPrecert bool) // The type of the leaf must match the one the handler expects if isPrecert != expectingPrecert { if expectingPrecert { - klog.Warningf("%s: Cert (or precert with invalid CT ext) submitted as precert chain: %q", li.LogOrigin, req.Chain) + klog.Warningf("%s: Cert (or precert with invalid CT ext) submitted as precert chain: %q", li.Origin, req.Chain) } else { - klog.Warningf("%s: Precert (or cert with invalid CT ext) submitted as cert chain: %q", li.LogOrigin, req.Chain) + klog.Warningf("%s: Precert (or cert with invalid CT ext) submitted as cert chain: %q", li.Origin, req.Chain) } return nil, fmt.Errorf("cert / precert mismatch: %T", expectingPrecert) } diff --git a/requestlog.go b/requestlog.go index 4ef41d02..d337741b 100644 --- a/requestlog.go +++ b/requestlog.go @@ -38,8 +38,8 @@ type RequestLog interface { // The returned context should be used in all the following calls to // this API. This is normally arranged by the request handler code. Start(context.Context) context.Context - // LogOrigin will be called once per request to set the log prefix. - LogOrigin(context.Context, string) + // Origin will be called once per request to set the log prefix. + Origin(context.Context, string) // AddDERToChain will be called once for each certificate in a submitted // chain. It's called early in request processing so the supplied bytes // have not been checked for validity. Calls will be in order of the @@ -71,8 +71,8 @@ func (dlr *DefaultRequestLog) Start(ctx context.Context) context.Context { return ctx } -// LogOrigin logs the origin of the CT log that this request is for. -func (dlr *DefaultRequestLog) LogOrigin(_ context.Context, p string) { +// Origin logs the origin of the CT log that this request is for. +func (dlr *DefaultRequestLog) Origin(_ context.Context, p string) { klog.V(vLevel).Infof("RL: LogOrigin: %s", p) } From 80bfcfceb10c989e4e8d835a836d72f54a4ad7be Mon Sep 17 00:00:00 2001 From: Philippe Boneff Date: Thu, 9 Jan 2025 12:00:33 +0000 Subject: [PATCH 4/5] delete proto_gen.go --- proto_gen.go | 6 ------ 1 file changed, 6 deletions(-) delete mode 100644 proto_gen.go diff --git a/proto_gen.go b/proto_gen.go deleted file mode 100644 index 32cf8beb..00000000 --- a/proto_gen.go +++ /dev/null @@ -1,6 +0,0 @@ -// See the License for the specific language governing permissions and -// limitations under the License. - -package sctfe - -//go:generate sh -c "protoc -I=. -I$(go list -f '{{ .Dir }}' github.com/google/trillian) -I$(go list -f '{{ .Dir }}' github.com/transparency-dev/static-ct) --go_out=paths=source_relative:. configpb/config.proto" From 50b249cce8a83b35d8fd19c621ae1a7f9801a743 Mon Sep 17 00:00:00 2001 From: Philippe Boneff Date: Fri, 10 Jan 2025 15:06:50 +0000 Subject: [PATCH 5/5] add todo --- handlers.go | 1 + 1 file changed, 1 insertion(+) diff --git a/handlers.go b/handlers.go index 2d53e069..0ac0fdfc 100644 --- a/handlers.go +++ b/handlers.go @@ -111,6 +111,7 @@ func (a AppHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { rspLatency.Observe(latency, label0, label1, strconv.Itoa(statusCode)) }() klog.V(2).Infof("%s: request %v %q => %s", a.Info.Origin, r.Method, r.URL, a.Name) + // TODO(phboneff): add a.Method directly on the handler path and remove this test. if r.Method != a.Method { klog.Warningf("%s: %s wrong HTTP method: %v", a.Info.Origin, a.Name, r.Method) a.Info.SendHTTPError(w, http.StatusMethodNotAllowed, fmt.Errorf("method not allowed: %s", r.Method))