From 46db0245f1d298d9ac8ffc2cf60f90899fe66537 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Tue, 27 Jan 2026 15:33:45 -0500 Subject: [PATCH 1/6] feat: add ETag/caching support in OFREP Signed-off-by: Todd Baert --- .../service/flag-evaluation/ofrep/handler.go | 66 ++++++- .../flag-evaluation/ofrep/handler_test.go | 166 +++++++++++++++++- 2 files changed, 226 insertions(+), 6 deletions(-) diff --git a/flagd/pkg/service/flag-evaluation/ofrep/handler.go b/flagd/pkg/service/flag-evaluation/ofrep/handler.go index cbc8d435b..19700f01f 100644 --- a/flagd/pkg/service/flag-evaluation/ofrep/handler.go +++ b/flagd/pkg/service/flag-evaluation/ofrep/handler.go @@ -2,6 +2,8 @@ package ofrep import ( "context" + "crypto/sha256" + "encoding/hex" "encoding/json" "fmt" "net/http" @@ -22,9 +24,13 @@ import ( ) const ( - key = "key" - singleEvaluation = "/ofrep/v1/evaluate/flags/{key}" - bulkEvaluation = "/ofrep/v1/evaluate/{path:flags\\/|flags}" + key = "key" + singleEvaluation = "/ofrep/v1/evaluate/flags/{key}" + bulkEvaluation = "/ofrep/v1/evaluate/{path:flags\\/|flags}" + headerETag = "ETag" + headerIfNoneMatch = "If-None-Match" + headerContentType = "Content-Type" + contentTypeJSON = "application/json" ) type handler struct { @@ -128,7 +134,44 @@ func (h *handler) HandleBulkEvaluation(w http.ResponseWriter, r *http.Request) { fmt.Sprintf("Bulk evaluation failed. Tracking ID: %s", requestID)) h.writeJSONToResponse(http.StatusInternalServerError, res, w) } else { - h.writeJSONToResponse(http.StatusOK, ofrep.BulkEvaluationResponseFrom(evaluations, metadata), w) + response := ofrep.BulkEvaluationResponseFrom(evaluations, metadata) + h.writeBulkEvaluationResponse(w, r, response) + } +} + +// writes the bulk evaluation response with ETag support +func (h *handler) writeBulkEvaluationResponse(w http.ResponseWriter, r *http.Request, response ofrep.BulkEvaluationResponse) { + // calculate ETag based on response body + eTag, err := calculateETag(response) + if err != nil { + h.Logger.Warn(fmt.Sprintf("error calculating ETag: %v", err)) + w.WriteHeader(http.StatusInternalServerError) + return + } + + // check If-None-Match header for cache validation + ifNoneMatch := r.Header.Get(headerIfNoneMatch) + if ifNoneMatch == eTag { + // ETag matches, return 304 Not Modified + w.Header().Add(headerETag, eTag) + w.WriteHeader(http.StatusNotModified) + return + } + + // ETag doesn't match or missing, return the response with the new ETag + marshal, err := json.Marshal(response) + if err != nil { + h.Logger.Warn(fmt.Sprintf("error marshalling the response: %v", err)) + w.WriteHeader(http.StatusInternalServerError) + return + } + + w.Header().Add(headerContentType, contentTypeJSON) + w.Header().Add(headerETag, eTag) + w.WriteHeader(http.StatusOK) + _, err = w.Write(marshal) + if err != nil { + h.Logger.Warn(fmt.Sprintf("error while writing response: %v", err)) } } @@ -142,7 +185,7 @@ func (h *handler) writeJSONToResponse(status int, payload interface{}, w http.Re return } - w.Header().Add("Content-Type", "application/json") + w.Header().Add(headerContentType, contentTypeJSON) w.WriteHeader(status) _, err = w.Write(marshal) if err != nil { @@ -150,6 +193,19 @@ func (h *handler) writeJSONToResponse(status int, payload interface{}, w http.Re } } +// calculateETag generates an ETag from the bulk evaluation response +func calculateETag(response ofrep.BulkEvaluationResponse) (string, error) { + // Marshal the response to JSON + data, err := json.Marshal(response) + if err != nil { + return "", fmt.Errorf("failed to marshal response for ETag calculation: %w", err) + } + + // Calculate SHA256 hash of the JSON response + hash := sha256.Sum256(data) + return fmt.Sprintf("\"%s\"", hex.EncodeToString(hash[:])), nil +} + func extractOfrepRequest(req *http.Request) (ofrep.Request, error) { request := ofrep.Request{} err := json.NewDecoder(req.Body).Decode(&request) diff --git a/flagd/pkg/service/flag-evaluation/ofrep/handler_test.go b/flagd/pkg/service/flag-evaluation/ofrep/handler_test.go index 2ef114d41..db2d9ea88 100644 --- a/flagd/pkg/service/flag-evaluation/ofrep/handler_test.go +++ b/flagd/pkg/service/flag-evaluation/ofrep/handler_test.go @@ -281,7 +281,7 @@ func TestWriteJSONResponse(t *testing.T) { var rsp evaluator.AnyValue err = json.Unmarshal(b, &rsp) if err != nil { - t.Errorf("error unmarshelling body: %v", err) + t.Errorf("error unmarshaling body: %v", err) } if !reflect.DeepEqual(test.payload, rsp) { @@ -290,3 +290,167 @@ func TestWriteJSONResponse(t *testing.T) { }) } } + +func TestWriteBulkEvaluationResponse_ETag(t *testing.T) { + log := logger.NewLogger(nil, false) + h := handler{Logger: log} + + // test response + response := ofrep.BulkEvaluationResponse{ + Flags: []interface{}{ + ofrep.EvaluationSuccess{ + Key: "test-flag", + Value: true, + Reason: model.StaticReason, + Variant: "on", + }, + }, + Metadata: model.Metadata{}, + } + + tests := []struct { + name string + eTagGenerator func() (string, error) // Optional function to generate the ETag + expectedStatus int + expectedHasETag bool + expectedHasBody bool + }{ + { + name: "no If-None-Match header returns 200 with body and ETag", + eTagGenerator: nil, // Will use no ETag in request + expectedStatus: http.StatusOK, + expectedHasETag: true, + expectedHasBody: true, + }, + { + name: "matching If-None-Match header returns 304 Not Modified", + eTagGenerator: func() (string, error) { + return calculateETag(response) + }, + expectedStatus: http.StatusNotModified, + expectedHasETag: true, + expectedHasBody: false, + }, + { + name: "non-matching If-None-Match header returns 200 with body and ETag", + eTagGenerator: func() (string, error) { + return "\"some-invalid-etag-lmao\"", nil + }, + expectedStatus: http.StatusOK, + expectedHasETag: true, + expectedHasBody: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + // Calculate the ETag for this specific test case + var ifNoneMatchHeader string + if test.eTagGenerator != nil { + eTag, err := test.eTagGenerator() + if err != nil { + t.Fatalf("error generating ETag: %v", err) + } + ifNoneMatchHeader = eTag + } + + request := httptest.NewRequest(http.MethodPost, "/ofrep/v1/evaluate/flags", bytes.NewReader([]byte{})) + if ifNoneMatchHeader != "" { + request.Header.Set("If-None-Match", ifNoneMatchHeader) + } + + recorder := httptest.NewRecorder() + h.writeBulkEvaluationResponse(recorder, request, response) + + if test.expectedStatus != recorder.Code { + t.Errorf("expected status code %d, but got %d", test.expectedStatus, recorder.Code) + } + + eTagHeader := recorder.Header().Get("ETag") + if test.expectedHasETag && eTagHeader == "" { + t.Error("expected ETag header to be present, but it was missing") + } + if !test.expectedHasETag && eTagHeader != "" { + t.Error("expected ETag header to be absent, but it was present") + } + + body := recorder.Body.String() + if test.expectedHasBody && body == "" { + t.Error("expected response body, but got empty body") + } + if !test.expectedHasBody && body != "" { + t.Errorf("expected no response body, but got: %s", body) + } + + // Verify ETag behavior: if matching ETag in request, should get 304 + if ifNoneMatchHeader != "" && eTagHeader != "" && ifNoneMatchHeader == eTagHeader { + if test.expectedStatus != http.StatusNotModified { + t.Errorf("expected 304 status when ETag matches, but got %d", test.expectedStatus) + } + } + + // For 200 responses, verify Content-Type header is present + if test.expectedStatus == http.StatusOK && recorder.Header().Get("Content-Type") != "application/json" { + t.Error("expected Content-Type header to be application/json, but it was missing") + } + }) + } +} + +func TestCalculateETag(t *testing.T) { + tests := []struct { + name string + response ofrep.BulkEvaluationResponse + expectedErrNil bool + }{ + { + name: "valid response produces ETag", + response: ofrep.BulkEvaluationResponse{ + Flags: []interface{}{ + ofrep.EvaluationSuccess{ + Key: "test-flag", + Value: true, + Reason: model.StaticReason, + Variant: "on", + }, + }, + Metadata: model.Metadata{}, + }, + expectedErrNil: true, + }, + { + name: "empty response produces ETag", + response: ofrep.BulkEvaluationResponse{ + Flags: []interface{}{}, + Metadata: model.Metadata{}, + }, + expectedErrNil: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + eTag, err := calculateETag(test.response) + + if test.expectedErrNil && err != nil { + t.Errorf("expected no error, but got: %v", err) + } + if !test.expectedErrNil && err == nil { + t.Error("expected error, but got none") + } + + if test.expectedErrNil { + // Verify ETag format: quoted hex string + if len(eTag) < 2 || eTag[0] != '"' || eTag[len(eTag)-1] != '"' { + t.Errorf("expected ETag to be quoted, but got: %s", eTag) + } + + // Calculate again to ensure deterministic output + eTag2, _ := calculateETag(test.response) + if eTag != eTag2 { + t.Errorf("expected deterministic ETag, but got different values: %s vs %s", eTag, eTag2) + } + } + }) + } +} From d6b50a5ac66db2fe74ae9a44309150827b1b8593 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Tue, 27 Jan 2026 15:47:42 -0500 Subject: [PATCH 2/6] fixup: gemini recommendations for avoiding more marshalling Signed-off-by: Todd Baert --- .../service/flag-evaluation/ofrep/handler.go | 30 ++++++++----------- .../flag-evaluation/ofrep/handler_test.go | 17 ++++++++--- 2 files changed, 25 insertions(+), 22 deletions(-) diff --git a/flagd/pkg/service/flag-evaluation/ofrep/handler.go b/flagd/pkg/service/flag-evaluation/ofrep/handler.go index 19700f01f..0352e3bff 100644 --- a/flagd/pkg/service/flag-evaluation/ofrep/handler.go +++ b/flagd/pkg/service/flag-evaluation/ofrep/handler.go @@ -21,6 +21,7 @@ import ( "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/trace" + "go.uber.org/zap" ) const ( @@ -141,10 +142,10 @@ func (h *handler) HandleBulkEvaluation(w http.ResponseWriter, r *http.Request) { // writes the bulk evaluation response with ETag support func (h *handler) writeBulkEvaluationResponse(w http.ResponseWriter, r *http.Request, response ofrep.BulkEvaluationResponse) { - // calculate ETag based on response body - eTag, err := calculateETag(response) + // calculate ETag and marshal response in one operation + eTag, body, err := calculateETag(response) if err != nil { - h.Logger.Warn(fmt.Sprintf("error calculating ETag: %v", err)) + h.Logger.Warn("error calculating ETag", zap.Error(err)) w.WriteHeader(http.StatusInternalServerError) return } @@ -159,19 +160,12 @@ func (h *handler) writeBulkEvaluationResponse(w http.ResponseWriter, r *http.Req } // ETag doesn't match or missing, return the response with the new ETag - marshal, err := json.Marshal(response) - if err != nil { - h.Logger.Warn(fmt.Sprintf("error marshalling the response: %v", err)) - w.WriteHeader(http.StatusInternalServerError) - return - } - w.Header().Add(headerContentType, contentTypeJSON) w.Header().Add(headerETag, eTag) w.WriteHeader(http.StatusOK) - _, err = w.Write(marshal) + _, err = w.Write(body) if err != nil { - h.Logger.Warn(fmt.Sprintf("error while writing response: %v", err)) + h.Logger.Warn("error while writing response", zap.Error(err)) } } @@ -180,7 +174,7 @@ func (h *handler) writeJSONToResponse(status int, payload interface{}, w http.Re marshal, err := json.Marshal(payload) if err != nil { // always a 500 - h.Logger.Warn(fmt.Sprintf("error marshelling the response: %v", err)) + h.Logger.Warn("error marshalling the response", zap.Error(err)) w.WriteHeader(http.StatusInternalServerError) return } @@ -189,21 +183,21 @@ func (h *handler) writeJSONToResponse(status int, payload interface{}, w http.Re w.WriteHeader(status) _, err = w.Write(marshal) if err != nil { - h.Logger.Warn(fmt.Sprintf("error while writing response: %v", err)) + h.Logger.Warn("error while writing response", zap.Error(err)) } } // calculateETag generates an ETag from the bulk evaluation response -func calculateETag(response ofrep.BulkEvaluationResponse) (string, error) { - // Marshal the response to JSON +func calculateETag(response ofrep.BulkEvaluationResponse) (string, []byte, error) { + // marshal the response to JSON data, err := json.Marshal(response) if err != nil { - return "", fmt.Errorf("failed to marshal response for ETag calculation: %w", err) + return "", nil, fmt.Errorf("failed to marshal response for ETag calculation: %w", err) } // Calculate SHA256 hash of the JSON response hash := sha256.Sum256(data) - return fmt.Sprintf("\"%s\"", hex.EncodeToString(hash[:])), nil + return fmt.Sprintf("\"%s\"", hex.EncodeToString(hash[:])), data, nil } func extractOfrepRequest(req *http.Request) (ofrep.Request, error) { diff --git a/flagd/pkg/service/flag-evaluation/ofrep/handler_test.go b/flagd/pkg/service/flag-evaluation/ofrep/handler_test.go index db2d9ea88..4154dcdf9 100644 --- a/flagd/pkg/service/flag-evaluation/ofrep/handler_test.go +++ b/flagd/pkg/service/flag-evaluation/ofrep/handler_test.go @@ -325,7 +325,8 @@ func TestWriteBulkEvaluationResponse_ETag(t *testing.T) { { name: "matching If-None-Match header returns 304 Not Modified", eTagGenerator: func() (string, error) { - return calculateETag(response) + eTag, _, err := calculateETag(response) + return eTag, err }, expectedStatus: http.StatusNotModified, expectedHasETag: true, @@ -430,7 +431,7 @@ func TestCalculateETag(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - eTag, err := calculateETag(test.response) + eTag, body, err := calculateETag(test.response) if test.expectedErrNil && err != nil { t.Errorf("expected no error, but got: %v", err) @@ -445,11 +446,19 @@ func TestCalculateETag(t *testing.T) { t.Errorf("expected ETag to be quoted, but got: %s", eTag) } - // Calculate again to ensure deterministic output - eTag2, _ := calculateETag(test.response) + // verify body is not empty + if len(body) == 0 { + t.Error("expected marshaled body, but got empty bytes") + } + + // calculate again to ensure deterministic output + eTag2, body2, _ := calculateETag(test.response) if eTag != eTag2 { t.Errorf("expected deterministic ETag, but got different values: %s vs %s", eTag, eTag2) } + if !bytes.Equal(body, body2) { + t.Errorf("expected deterministic body, but got different values") + } } }) } From bfa8a1787e3bb44bedf4111c05daae64566a634f Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Wed, 28 Jan 2026 07:24:59 -0500 Subject: [PATCH 3/6] Update flagd/pkg/service/flag-evaluation/ofrep/handler.go Co-authored-by: Roman Dmytrenko Signed-off-by: Todd Baert --- flagd/pkg/service/flag-evaluation/ofrep/handler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flagd/pkg/service/flag-evaluation/ofrep/handler.go b/flagd/pkg/service/flag-evaluation/ofrep/handler.go index 0352e3bff..6efc79372 100644 --- a/flagd/pkg/service/flag-evaluation/ofrep/handler.go +++ b/flagd/pkg/service/flag-evaluation/ofrep/handler.go @@ -197,7 +197,7 @@ func calculateETag(response ofrep.BulkEvaluationResponse) (string, []byte, error // Calculate SHA256 hash of the JSON response hash := sha256.Sum256(data) - return fmt.Sprintf("\"%s\"", hex.EncodeToString(hash[:])), data, nil + return fmt.Sprintf("\"%x\"", hash), data, nil } func extractOfrepRequest(req *http.Request) (ofrep.Request, error) { From f1b017f5339fc2d6f248f3ffc535d87b0d9e235c Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Wed, 28 Jan 2026 07:39:00 -0500 Subject: [PATCH 4/6] fixup: unused import Signed-off-by: Todd Baert --- flagd/pkg/service/flag-evaluation/ofrep/handler.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/flagd/pkg/service/flag-evaluation/ofrep/handler.go b/flagd/pkg/service/flag-evaluation/ofrep/handler.go index 6efc79372..6b5b2fb67 100644 --- a/flagd/pkg/service/flag-evaluation/ofrep/handler.go +++ b/flagd/pkg/service/flag-evaluation/ofrep/handler.go @@ -3,7 +3,6 @@ package ofrep import ( "context" "crypto/sha256" - "encoding/hex" "encoding/json" "fmt" "net/http" @@ -195,7 +194,6 @@ func calculateETag(response ofrep.BulkEvaluationResponse) (string, []byte, error return "", nil, fmt.Errorf("failed to marshal response for ETag calculation: %w", err) } - // Calculate SHA256 hash of the JSON response hash := sha256.Sum256(data) return fmt.Sprintf("\"%x\"", hash), data, nil } From 567fcbb8947cf1de75c9b55a65ccdff35416f2ea Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Wed, 28 Jan 2026 08:51:50 -0500 Subject: [PATCH 5/6] fixup: sha1, and docs Signed-off-by: Todd Baert --- docs/reference/flagd-ofrep.md | 9 +++++++++ flagd/pkg/service/flag-evaluation/ofrep/handler.go | 4 ++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/docs/reference/flagd-ofrep.md b/docs/reference/flagd-ofrep.md index f28ac3b85..3c89b0731 100644 --- a/docs/reference/flagd-ofrep.md +++ b/docs/reference/flagd-ofrep.md @@ -23,4 +23,13 @@ To evaluate all flags currently configured at flagd, use OFREP bulk evaluation r curl -X POST 'http://localhost:8016/ofrep/v1/evaluate/flags' ``` +## HTTP Caching + +The bulk evaluation endpoint supports HTTP caching via ETags (SHA1-based). Clients can use the `If-None-Match` header with a previously received `ETag` to validate cached responses. When the response hasn't changed, the server returns `304 Not Modified` without a body, reducing bandwidth. + +```shell +curl -X POST 'http://localhost:8016/ofrep/v1/evaluate/flags' \ + -H 'If-None-Match: "a1b2c3d4e5f6..."' +``` + See the [cheat sheet](./cheat-sheet.md#ofrep-api-http) for more OFREP examples including context-sensitive evaluation and selectors. diff --git a/flagd/pkg/service/flag-evaluation/ofrep/handler.go b/flagd/pkg/service/flag-evaluation/ofrep/handler.go index 6b5b2fb67..58ed3712e 100644 --- a/flagd/pkg/service/flag-evaluation/ofrep/handler.go +++ b/flagd/pkg/service/flag-evaluation/ofrep/handler.go @@ -2,7 +2,7 @@ package ofrep import ( "context" - "crypto/sha256" + "crypto/sha1" "encoding/json" "fmt" "net/http" @@ -194,7 +194,7 @@ func calculateETag(response ofrep.BulkEvaluationResponse) (string, []byte, error return "", nil, fmt.Errorf("failed to marshal response for ETag calculation: %w", err) } - hash := sha256.Sum256(data) + hash := sha1.Sum(data) return fmt.Sprintf("\"%x\"", hash), data, nil } From 73f07a4765ca18c711a7afbf960f62d70e559fa8 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Thu, 29 Jan 2026 12:48:31 -0500 Subject: [PATCH 6/6] fixup: use md5 Signed-off-by: Todd Baert --- docs/reference/flagd-ofrep.md | 2 +- flagd/pkg/service/flag-evaluation/ofrep/handler.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/reference/flagd-ofrep.md b/docs/reference/flagd-ofrep.md index 3c89b0731..fc516165a 100644 --- a/docs/reference/flagd-ofrep.md +++ b/docs/reference/flagd-ofrep.md @@ -25,7 +25,7 @@ curl -X POST 'http://localhost:8016/ofrep/v1/evaluate/flags' ## HTTP Caching -The bulk evaluation endpoint supports HTTP caching via ETags (SHA1-based). Clients can use the `If-None-Match` header with a previously received `ETag` to validate cached responses. When the response hasn't changed, the server returns `304 Not Modified` without a body, reducing bandwidth. +The bulk evaluation endpoint supports HTTP caching via ETags (MD5-based). Clients can use the `If-None-Match` header with a previously received `ETag` to validate cached responses. When the response hasn't changed, the server returns `304 Not Modified` without a body, reducing bandwidth. ```shell curl -X POST 'http://localhost:8016/ofrep/v1/evaluate/flags' \ diff --git a/flagd/pkg/service/flag-evaluation/ofrep/handler.go b/flagd/pkg/service/flag-evaluation/ofrep/handler.go index 58ed3712e..9fb56d0f3 100644 --- a/flagd/pkg/service/flag-evaluation/ofrep/handler.go +++ b/flagd/pkg/service/flag-evaluation/ofrep/handler.go @@ -2,7 +2,7 @@ package ofrep import ( "context" - "crypto/sha1" + "crypto/md5" "encoding/json" "fmt" "net/http" @@ -194,7 +194,7 @@ func calculateETag(response ofrep.BulkEvaluationResponse) (string, []byte, error return "", nil, fmt.Errorf("failed to marshal response for ETag calculation: %w", err) } - hash := sha1.Sum(data) + hash := md5.Sum(data) return fmt.Sprintf("\"%x\"", hash), data, nil }