feat: allow empty evaluation context for flags that don't require bucketing

cmd/relayproxy/controller/all_flags_test.go

@@ -65,15 +65,17 @@ func Test_all_flag_Handler_DefaultMode(t *testing.T) {
 			},
 		},
 		{
-			name: "No user key in payload",
+			name: "No user key in payload - should try to evaluate flags individually",
+			// the API should return targeting key missing for all flags that require bucketing
+			// and it should also perform all the static evaluations for non-bucketing flags
 			args: args{
 				bodyFile:            "../testdata/controller/all_flags/no_user_key_request.json",
 				configFlagsLocation: configFlagsLocation,
 			},
 			want: want{
-				handlerErr: true,
-				errorMsg:   "empty key for evaluation context, impossible to retrieve flags",
-				errorCode:  http.StatusBadRequest,
+				handlerErr: false,
+				httpCode:   http.StatusOK,
+				bodyFile:   "../testdata/controller/all_flags/no_user_key_response_updated.json",
 			},
 		},
 		{
@@ -194,15 +196,17 @@ func Test_all_flag_Handler_FlagsetMode(t *testing.T) {
 			},
 		},
 		{
-			name: "No user key in payload",
+			name: "No user key in payload - should now pass and evaluate flags individually",
+			// the API should return targeting key missing for all flags that require bucketing
+			// and it should also perform all the static evaluations for non-bucketing flags
 			args: args{
 				bodyFile:            "../testdata/controller/all_flags/no_user_key_request.json",
 				configFlagsLocation: configFlagsLocation,
 			},
 			want: want{
-				handlerErr: true,
-				errorMsg:   "empty key for evaluation context, impossible to retrieve flags",
-				errorCode:  http.StatusBadRequest,
+				handlerErr: false, // No longer fails at API validation level
+				httpCode:   http.StatusOK,
+				bodyFile:   "../testdata/controller/all_flags/no_user_key_response_updated.json",
 			},
 		},
 		{

cmd/relayproxy/controller/flag_eval_test.go

@@ -2,7 +2,6 @@ package controller_test
 
 import (
 	"io"
-	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
 	"os"
@@ -126,9 +125,9 @@ func Test_flag_eval_Handler(t *testing.T) {
 				bodyFile: "../testdata/controller/flag_eval/no_user_key_request.json",
 			},
 			want: want{
-				handlerErr: true,
-				errorMsg:   "empty key for evaluation context, impossible to retrieve flags",
-				errorCode:  http.StatusBadRequest,
+				handlerErr: false,
+				bodyFile:   "../testdata/controller/flag_eval/no_user_key_response.json",
+				httpCode:   http.StatusOK,
 			},
 		},
 		{
@@ -169,7 +168,7 @@ func Test_flag_eval_Handler(t *testing.T) {
 			// read wantBody request file
 			var bodyReq io.Reader
 			if tt.args.bodyFile != "" {
-				bodyReqContent, err := ioutil.ReadFile(tt.args.bodyFile)
+				bodyReqContent, err := os.ReadFile(tt.args.bodyFile)
 				assert.NoError(t, err, "request wantBody file missing %s", tt.args.bodyFile)
 				bodyReq = strings.NewReader(string(bodyReqContent))
 			}

cmd/relayproxy/controller/utils.go

@@ -19,21 +19,6 @@ func assertRequest(u *model.AllFlagRequest) *echo.HTTPError {
 			"assertRequest: impossible to find user in request",
 		)
 	}
-
-	if u.EvaluationContext != nil {
-		return assertContextKey(u.EvaluationContext.Key)
-	}
-	return assertContextKey(u.User.Key) // nolint: staticcheck
-}
-
-// assertContextKey is checking that the user key is valid, if not an echo.HTTPError is return.
-func assertContextKey(key string) *echo.HTTPError {
-	if len(key) == 0 {
-		return &echo.HTTPError{
-			Code:    http.StatusBadRequest,
-			Message: "empty key for evaluation context, impossible to retrieve flags",
-		}
-	}
 	return nil
 }
 

cmd/relayproxy/controller/utils_test.go

@@ -41,13 +41,14 @@ func Test_assertRequest(t *testing.T) {
 		},
 		{
 			name: "user with User and EvaluationContext, empty key for evaluation context",
+			// in this case, since we have a targetingKey set for the evaluation context we take the one from the
+			// evaluation context key not from the user key.
+			// In that case the targetingKey is empty, but this is allowed by the core evaluation logic.
 			req: &model.AllFlagRequest{
 				User:              &model.UserRequest{Key: "my-key"},
 				EvaluationContext: &model.EvaluationContextRequest{Key: ""},
 			},
-			wantErr: echo.NewHTTPError(
-				http.StatusBadRequest,
-				"empty key for evaluation context, impossible to retrieve flags"),
+			wantErr: nil,
 		},
 		{
 			name: "invalid user but valid evaluation context should pass",

cmd/relayproxy/ofrep/evaluate.go

@@ -70,17 +70,11 @@ func (h *EvaluateCtrl) Evaluate(c echo.Context) error {
 			Key:                      flagKey,
 		})
 	}
-	evalCtx, err := evaluationContextFromOFREPRequest(reqBody.Context)
+	evalCtx, err := evaluationContextFromOFREPRequest(reqBody)
 	if err != nil {
 		return c.JSON(
 			http.StatusBadRequest,
-			model.OFREPEvaluateResponseError{
-				OFREPCommonResponseError: model.OFREPCommonResponseError{
-					ErrorCode:    flag.ErrorCodeInvalidContext,
-					ErrorDetails: err.Error(),
-				},
-				Key: flagKey,
-			})
+			err)
 	}
 
 	tracer := otel.GetTracerProvider().Tracer(config.OtelTracerName)
@@ -167,7 +161,7 @@ func (h *EvaluateCtrl) BulkEvaluate(c echo.Context) error {
 	if err := assertOFREPEvaluateRequest(request); err != nil {
 		return c.JSON(http.StatusBadRequest, err)
 	}
-	evalCtx, err := evaluationContextFromOFREPRequest(request.Context)
+	evalCtx, err := evaluationContextFromOFREPRequest(request)
 	if err != nil {
 		return c.JSON(
 			http.StatusBadRequest,
@@ -227,19 +221,36 @@ func (h *EvaluateCtrl) BulkEvaluate(c echo.Context) error {
 func assertOFREPEvaluateRequest(
 	ofrepEvalReq *model.OFREPEvalFlagRequest,
 ) *model.OFREPCommonResponseError {
-	if ofrepEvalReq.Context == nil || ofrepEvalReq.Context["targetingKey"] == "" {
-		return NewOFREPCommonError(flag.ErrorCodeTargetingKeyMissing,
-			"GO Feature Flag MUST have a targeting key in the request.")
+	if ofrepEvalReq.Context == nil {
+		return NewOFREPCommonError(flag.ErrorCodeInvalidContext,
+			"GO Feature Flag requires an evaluation context in the request.")
 	}
+
+	// An empty context object is allowed since the evaluation context is optional.
+	// If the context does not have any targetingKey, this is fine since the core
+	// evaluation logic will handle if it is required or not.
+
 	return nil
 }
 
-func evaluationContextFromOFREPRequest(ctx map[string]any) (ffcontext.Context, error) {
-	if targetingKey, ok := ctx["targetingKey"].(string); ok {
-		evalCtx := utils.ConvertEvaluationCtxFromRequest(targetingKey, ctx)
-		return evalCtx, nil
+func evaluationContextFromOFREPRequest(req *model.OFREPEvalFlagRequest) (ffcontext.Context, error) {
+	if req == nil || req.Context == nil {
+		return ffcontext.EvaluationContext{}, NewOFREPCommonError(
+			flag.ErrorCodeInvalidContext,
+			"GO Feature Flag has received an invalid context.")
 	}
-	return ffcontext.EvaluationContext{}, NewOFREPCommonError(
-		flag.ErrorCodeTargetingKeyMissing,
-		"GO Feature Flag has received no targetingKey or a none string value that is not a string.")
+
+	ctx := req.Context
+
+	// targetingKey is optional, it is only required if the flag needs bucketing and
+	// the check is done in the core evaluation logic.
+	// If we don't have a targetingKey, we return an empty string.
+	targetingKey := ""
+	if key, ok := ctx["targetingKey"].(string); ok {
+		targetingKey = key
+	}
+
+	// Create evaluation context (empty targeting key is allowed)
+	evalCtx := utils.ConvertEvaluationCtxFromRequest(targetingKey, ctx)
+	return evalCtx, nil
 }

cmd/relayproxy/ofrep/evaluate_test.go

@@ -72,6 +72,17 @@ func Test_Bulk_Evaluation(t *testing.T) {
 				bodyFile: "../testdata/ofrep/responses/invalid_context.json",
 			},
 		},
+		{
+			name: "Empty body",
+			args: args{
+				bodyFile:            "../testdata/ofrep/empty_body.json",
+				configFlagsLocation: configFlagsLocation,
+			},
+			want: want{
+				httpCode: http.StatusBadRequest,
+				bodyFile: "../testdata/ofrep/responses/empty_body.json",
+			},
+		},
 		{
 			name: "Nil context",
 			args: args{
@@ -85,12 +96,14 @@ func Test_Bulk_Evaluation(t *testing.T) {
 		},
 		{
 			name: "No Targeting Key in context",
+			// in this case we don't have a targetingKey, so we will evaluate the flags individually
+			// if the flag requires bucketing, we will return a targeting key missing error
 			args: args{
 				bodyFile:            "../testdata/ofrep/no_targeting_key_context.json",
 				configFlagsLocation: configFlagsLocation,
 			},
 			want: want{
-				httpCode: http.StatusBadRequest,
+				httpCode: http.StatusOK,
 				bodyFile: "../testdata/ofrep/responses/no_targeting_key_context.json",
 			},
 		},
@@ -209,15 +222,39 @@ func Test_Evaluate(t *testing.T) {
 			},
 		},
 		{
-			name: "No Targeting Key in context",
+			name: "No Targeting Key for bucketing-required flag - should return 400 from core evaluation",
 			args: args{
 				bodyFile:            "../testdata/ofrep/no_targeting_key_context.json",
 				configFlagsLocation: configFlagsLocation,
-				flagKey:             "number-flag",
+				flagKey:             "number-flag", // This flag has percentage rules, requires bucketing
 			},
 			want: want{
 				httpCode: http.StatusBadRequest,
-				bodyFile: "../testdata/ofrep/responses/no_targeting_key_context_with_key.json",
+				bodyFile: "../testdata/ofrep/responses/no_targeting_key_bucketing_flag.json",
+			},
+		},
+		{
+			name: "No Targeting Key for non-bucketing flag - should succeed",
+			args: args{
+				bodyFile:            "../testdata/ofrep/no_targeting_key_context.json",
+				configFlagsLocation: configFlagsLocation,
+				flagKey:             "targeting-key-rule", // This flag has no percentages, doesn't require bucketing
+			},
+			want: want{
+				httpCode: http.StatusOK,
+				bodyFile: "../testdata/ofrep/responses/no_targeting_key_static_flag.json",
+			},
+		},
+		{
+			name: "Percentage-based rule in flag without targeting key should return 400 error",
+			args: args{
+				bodyFile:            "../testdata/ofrep/no_targeting_key_context.json",
+				configFlagsLocation: configFlagsLocation,
+				flagKey:             "flag-only-for-admin", // This flag has percentage rules, requires bucketing
+			},
+			want: want{
+				httpCode: http.StatusBadRequest, // Core evaluation returns 400 for missing targeting key
+				bodyFile: "../testdata/ofrep/responses/percentage_flag_no_key_error.json",
 			},
 		},
 		{

cmd/relayproxy/testdata/controller/all_flags/no_user_key_response_updated.json

@@ -0,0 +1,85 @@
+{
+  "flags": {
+    "array-flag": {
+      "value": null,
+      "timestamp": 1652273630,
+      "variationType": "",
+      "trackEvents": true,
+      "errorCode": "TARGETING_KEY_MISSING",
+      "errorDetails": "Error: Empty targeting key",
+      "reason": "ERROR"
+    },
+    "disable-flag": {
+      "value": null,
+      "timestamp": 1652273630,
+      "variationType": "",
+      "trackEvents": true,
+      "errorCode": "TARGETING_KEY_MISSING",
+      "errorDetails": "Error: Empty targeting key",
+      "reason": "ERROR"
+    },
+    "flag-only-for-admin": {
+      "value": null,
+      "timestamp": 1652273630,
+      "variationType": "",
+      "trackEvents": true,
+      "errorCode": "TARGETING_KEY_MISSING",
+      "errorDetails": "Error: Empty targeting key",
+      "reason": "ERROR"
+    },
+    "new-admin-access": {
+      "value": null,
+      "timestamp": 1652273630,
+      "variationType": "",
+      "trackEvents": true,
+      "errorCode": "TARGETING_KEY_MISSING",
+      "errorDetails": "Error: Empty targeting key",
+      "reason": "ERROR"
+    },
+    "number-flag": {
+      "value": null,
+      "timestamp": 1652273630,
+      "variationType": "",
+      "trackEvents": true,
+      "errorCode": "TARGETING_KEY_MISSING",
+      "errorDetails": "Error: Empty targeting key",
+      "reason": "ERROR"
+    },
+    "targeting-key-rule": {
+      "value": false,
+      "timestamp": 1652273630,
+      "variationType": "false_var",
+      "trackEvents": true,
+      "errorCode": "",
+      "reason": "DEFAULT"
+    },
+    "test-flag-rule-apply": {
+      "value": null,
+      "timestamp": 1652273630,
+      "variationType": "",
+      "trackEvents": true,
+      "errorCode": "TARGETING_KEY_MISSING",
+      "errorDetails": "Error: Empty targeting key",
+      "reason": "ERROR"
+    },
+    "test-flag-rule-apply-false": {
+      "value": null,
+      "timestamp": 1652273630,
+      "variationType": "",
+      "trackEvents": true,
+      "errorCode": "TARGETING_KEY_MISSING",
+      "errorDetails": "Error: Empty targeting key",
+      "reason": "ERROR"
+    },
+    "test-flag-rule-not-apply": {
+      "value": null,
+      "timestamp": 1652273630,
+      "variationType": "",
+      "trackEvents": true,
+      "errorCode": "TARGETING_KEY_MISSING",
+      "errorDetails": "Error: Empty targeting key",
+      "reason": "ERROR"
+    }
+  },
+  "valid": false
+}

cmd/relayproxy/testdata/controller/flag_eval/disable_flag_request.json

@@ -1,11 +1,10 @@
 {
-  "user": {
+  "evaluationContext": {
     "key": "random-key",
-    "anonymous": false,
     "custom": {
       "custom1": "value1",
       "custom2": "value2"
     }
   },
   "defaultValue": "mydefaultFlagValue"
-}
+}

cmd/relayproxy/testdata/controller/flag_eval/flag_not_exist_request.json

@@ -1,11 +1,10 @@
 {
-  "user": {
+  "evaluationContext": {
     "key": "random-key",
-    "anonymous": false,
     "custom": {
       "custom1": "value1",
       "custom2": "value2"
     }
   },
   "defaultValue": "not exist flag value"
-}
+}

cmd/relayproxy/testdata/controller/flag_eval/invalid_json_request.json

@@ -1,11 +1,9 @@
 {
-  "user": {
+  "evaluationContext": {
     "key": "random-key",
-    "anonymous": false,
     "custom": {
       "custom1": "value1",
       "custom2": "value2"
     }
   },
-  "defaultValue": "mydefaultFlagValue"
-
+  "defaultValue": "mydefaultFlagValue"