diff --git a/.github/workflows/downstream-compatibility.yml b/.github/workflows/downstream-compatibility.yml index 837c929..18d3636 100644 --- a/.github/workflows/downstream-compatibility.yml +++ b/.github/workflows/downstream-compatibility.yml @@ -20,7 +20,6 @@ jobs: name: ${{ matrix.slug }} compatibility (advisory) runs-on: ubuntu-latest timeout-minutes: 60 - continue-on-error: true strategy: fail-fast: false matrix: @@ -190,7 +189,10 @@ jobs: bash -lc "$TEST_COMMAND" - name: Report compatibility finding - if: steps.evaluation.outputs.classification != 'compatible' + if: >- + always() && + steps.evaluation.outputs.classification != '' && + steps.evaluation.outputs.classification != 'compatible' env: CLASSIFICATION: ${{ steps.evaluation.outputs.classification }} run: | diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 44d18d6..dd2e408 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -24,7 +24,11 @@ Thank you for your interest in contributing to the FFE system test data reposito "defaultValue": "", "targetingKey": "", "attributes": { "": "" }, - "result": { "value": "", "reason": "STATIC|SPLIT|TARGETING_MATCH|DEFAULT|ERROR|DISABLED" } + "result": { + "value": "", + "reason": "STATIC|SPLIT|TARGETING_MATCH|DEFAULT|ERROR|DISABLED", + "errorCode": "" + } } ] ``` @@ -32,6 +36,12 @@ Thank you for your interest in contributing to the FFE system test data reposito 3. If your test case requires a new flag, add the flag definition to `ufc-config.json` 4. Keep the fixture SDK-neutral. Do not include SDK-specific fields such as `variant` or `flagMetadata`; downstream SDKs should derive those from `ufc-config.json` when they need them. +Malformed flags must be isolated during configuration ingestion. Exclude them +from the active evaluation map, retain only their rejected keys for the current +configuration, and return the caller default with `ERROR` / `PARSE_ERROR` when +those keys are evaluated. Reserve `ERROR` / `FLAG_NOT_FOUND` for keys absent from +both the active and rejected maps. Replace both maps atomically on refresh. + ### Modifying Flag Configuration When adding or modifying flags in `ufc-config.json`: diff --git a/README.md b/README.md index 04e4ba2..a6c7465 100644 --- a/README.md +++ b/README.md @@ -82,6 +82,7 @@ Each evaluation case uses a universal schema with the following fields: | `attributes` | object | Additional context attributes for targeting rules | | `result.value` | any | The expected evaluation result value | | `result.reason` | string | The expected OpenFeature reason: `STATIC`, `SPLIT`, `TARGETING_MATCH`, `DEFAULT`, `ERROR`, `DISABLED` | +| `result.errorCode` | string | Optional OpenFeature error code, such as `PARSE_ERROR` or `FLAG_NOT_FOUND` | Example: @@ -117,6 +118,12 @@ The validator deliberately does not fully schema-check allocation internals. Some fixtures contain malformed flag fields on purpose to verify that consumers reject only the affected flag. Run the same check locally with: +Consumers exclude malformed flags from the active evaluation map while retaining +their rejected keys for the current configuration. Evaluating a rejected key +returns the caller default with `ERROR` / `PARSE_ERROR`; a key absent from both +maps returns `ERROR` / `FLAG_NOT_FOUND`. Each configuration refresh replaces both +maps atomically so fixed or deleted flags do not leave stale rejection entries. + ```bash python3 ci/validate-fixtures.py ``` @@ -135,15 +142,17 @@ python3 ci/validate-fixtures.py | `test-case-flag-with-empty-string.json` | Flag with empty string in configuration | | `test-case-integer-flag.json` | Integer-typed flag evaluation | | `test-case-kill-switch-flag.json` | Kill switch (emergency off) flag | -| `test-case-invalid-shard-bounds-isolation.json` | Flags with shard bounds outside Rust/schema integer ranges are ignored without poisoning valid flags | -| `test-case-invalid-regex-isolation.json` | Flag with an invalid regular expression is ignored without poisoning valid flags | -| `test-case-malformed-flag-isolation.json` | Structurally malformed flag is ignored without poisoning valid flags | +| `test-case-invalid-condition-operands.json` | Flags with invalid configured operands for GT, ONE_OF, and IS_NULL are removed without poisoning valid flags | +| `test-case-invalid-shard-bounds-isolation.json` | Flags with shard bounds outside Rust/schema integer ranges are removed without poisoning valid flags | +| `test-case-invalid-regex-isolation.json` | A flag with an invalid regular expression is removed without poisoning the configuration | +| `test-case-malformed-flag-isolation.json` | A structurally malformed flag is removed without poisoning valid flags | +| `test-case-variant-type-mismatch.json` | A flag whose variant violates its declared type is removed without poisoning valid neighboring flags | | `test-case-microsecond-date-flag.json` | Flag with microsecond-precision date targeting | -| `test-case-missing-split-shards-isolation.json` | Flag with a split missing required `shards` is ignored without poisoning valid flags | +| `test-case-missing-split-shards-isolation.json` | A flag with a split missing required `shards` is removed without poisoning valid flags | | `test-case-new-user-onboarding-flag.json` | Multi-allocation onboarding flag with sharding | | `test-case-no-allocations-flag.json` | Flag with no allocations (returns default) | | `test-case-null-operator-flag.json` | Flag using IS_NULL operator | -| `test-case-null-shard-range-isolation.json` | Flag with a null shard range is ignored without poisoning valid flags | +| `test-case-null-shard-range-isolation.json` | A flag with a null shard range is removed without poisoning valid flags | | `test-case-null-targeting-key.json` | Evaluations with an explicit null targeting key | | `test-case-numeric-flag.json` | Numeric flag evaluation | | `test-case-numeric-one-of.json` | Numeric ONE_OF operator matching | @@ -154,7 +163,7 @@ python3 ci/validate-fixtures.py | `test-case-semver-validation-flag.json` | Rust-compatible SemVer parsing boundaries, invalid syntax, and invalid configured comparands | | `test-case-start-and-end-date-flag.json` | Flag with start/end date time bounds | | `test-case-unknown-fields-tolerance.json` | Unknown UFC object fields are ignored | -| `test-case-unknown-operator-isolation.json` | Flag with unknown operator is ignored without poisoning valid flags | +| `test-case-unknown-operator-isolation.json` | A flag with an unknown operator is removed without poisoning valid flags | | `test-flag-that-does-not-exist.json` | Non-existent flag returning the default value with `FLAG_NOT_FOUND` | | `test-json-config-flag.json` | JSON-typed flag returning object value | | `test-no-allocations-flag.json` | Another no-allocations variant | diff --git a/ci/run-downstream-conformance.sh b/ci/run-downstream-conformance.sh index 77e6078..92929fb 100755 --- a/ci/run-downstream-conformance.sh +++ b/ci/run-downstream-conformance.sh @@ -119,7 +119,7 @@ if [[ -n ${GITHUB_STEP_SUMMARY:-} ]]; then } >>"$GITHUB_STEP_SUMMARY" fi -# Consumer test failures are evidence, not a merge gate. Command/setup errors -# outside run_fixture_revision still fail this script so the workflow can expose -# broken orchestration; the job itself is explicitly allowed to fail. -exit 0 +# Reflect the proposed fixture result in the check conclusion. Branch protection +# decides whether this advisory check blocks merging; the check itself stays red +# until the downstream implementation accepts the proposed fixtures. +exit "$head_status" diff --git a/evaluation-cases/test-case-invalid-condition-operands.json b/evaluation-cases/test-case-invalid-condition-operands.json new file mode 100644 index 0000000..be22638 --- /dev/null +++ b/evaluation-cases/test-case-invalid-condition-operands.json @@ -0,0 +1,59 @@ +[ + { + "description": "GT requires a numeric configured operand. A flag with a string operand must return a parse error rather than falling through to the catch-all allocation.", + "attributes": { + "age": 42 + }, + "defaultValue": "default", + "flag": "invalid-gt-condition-flag", + "result": { + "errorCode": "PARSE_ERROR", + "reason": "ERROR", + "value": "default" + }, + "targetingKey": "invalid-gt", + "variationType": "STRING" + }, + { + "description": "ONE_OF requires an array configured operand. A flag with a scalar operand must return a parse error rather than falling through to the catch-all allocation.", + "attributes": { + "role": "admin" + }, + "defaultValue": "default", + "flag": "invalid-one-of-condition-flag", + "result": { + "errorCode": "PARSE_ERROR", + "reason": "ERROR", + "value": "default" + }, + "targetingKey": "invalid-one-of", + "variationType": "STRING" + }, + { + "description": "IS_NULL requires a boolean configured operand. A flag with a string operand must return a parse error rather than falling through to the catch-all allocation.", + "attributes": { + "deleted_at": null + }, + "defaultValue": "default", + "flag": "invalid-is-null-condition-flag", + "result": { + "errorCode": "PARSE_ERROR", + "reason": "ERROR", + "value": "default" + }, + "targetingKey": "invalid-is-null", + "variationType": "STRING" + }, + { + "description": "A valid flag in the same configuration must still evaluate after malformed condition operands are isolated.", + "attributes": {}, + "defaultValue": "default", + "flag": "valid-flag-after-invalid-config", + "result": { + "reason": "STATIC", + "value": "expected" + }, + "targetingKey": "valid-neighbor", + "variationType": "STRING" + } +] diff --git a/evaluation-cases/test-case-invalid-regex-isolation.json b/evaluation-cases/test-case-invalid-regex-isolation.json index 11ff466..3ad596c 100644 --- a/evaluation-cases/test-case-invalid-regex-isolation.json +++ b/evaluation-cases/test-case-invalid-regex-isolation.json @@ -1,13 +1,14 @@ [ { - "description": "The malformed flag uses the invalid regex *@datadoghq.com before a catch-all allocation. SDKs must ignore the whole flag and return the caller default.", + "description": "The malformed flag uses the invalid regex *@datadoghq.com before a catch-all allocation. SDKs must return a parse error without poisoning the full configuration.", "attributes": { "email": "harish@datadoghq.com" }, "defaultValue": "default", "flag": "invalid-regex-flag", "result": { - "reason": "DEFAULT", + "errorCode": "PARSE_ERROR", + "reason": "ERROR", "value": "default" }, "targetingKey": "harish", diff --git a/evaluation-cases/test-case-invalid-shard-bounds-isolation.json b/evaluation-cases/test-case-invalid-shard-bounds-isolation.json index f484238..ca8553f 100644 --- a/evaluation-cases/test-case-invalid-shard-bounds-isolation.json +++ b/evaluation-cases/test-case-invalid-shard-bounds-isolation.json @@ -1,23 +1,25 @@ [ { - "description": "The malformed flag has totalShards above the unsigned 32-bit range, so SDKs should ignore that flag and return the caller default.", + "description": "The malformed flag has totalShards above the unsigned 32-bit range. SDKs must return a parse error without poisoning the full configuration.", "attributes": {}, "defaultValue": "default", "flag": "overflow-total-shards-flag", "result": { - "reason": "DEFAULT", + "errorCode": "PARSE_ERROR", + "reason": "ERROR", "value": "default" }, "targetingKey": "alice", "variationType": "STRING" }, { - "description": "The malformed flag has a negative shard range start, so SDKs should ignore that flag and return the caller default.", + "description": "The malformed flag has a negative shard range start. SDKs must return a parse error without poisoning the full configuration.", "attributes": {}, "defaultValue": "default", "flag": "negative-shard-range-start-flag", "result": { - "reason": "DEFAULT", + "errorCode": "PARSE_ERROR", + "reason": "ERROR", "value": "default" }, "targetingKey": "alice", diff --git a/evaluation-cases/test-case-malformed-flag-isolation.json b/evaluation-cases/test-case-malformed-flag-isolation.json index dcfa7b1..4167235 100644 --- a/evaluation-cases/test-case-malformed-flag-isolation.json +++ b/evaluation-cases/test-case-malformed-flag-isolation.json @@ -1,11 +1,12 @@ [ { - "description": "The malformed flag has allocations encoded as a string instead of an array, so SDKs should ignore that flag and return the caller default.", + "description": "The malformed flag has allocations encoded as a string instead of an array. SDKs must return a parse error without poisoning the full configuration.", "attributes": {}, "defaultValue": "default", "flag": "malformed-allocations-flag", "result": { - "reason": "DEFAULT", + "errorCode": "PARSE_ERROR", + "reason": "ERROR", "value": "default" }, "targetingKey": "alice", diff --git a/evaluation-cases/test-case-missing-split-shards-isolation.json b/evaluation-cases/test-case-missing-split-shards-isolation.json index bf474c5..d6dfeb1 100644 --- a/evaluation-cases/test-case-missing-split-shards-isolation.json +++ b/evaluation-cases/test-case-missing-split-shards-isolation.json @@ -1,11 +1,12 @@ [ { - "description": "The malformed flag has a split that omits required shards, so SDKs should ignore that flag and return the caller default.", + "description": "The malformed flag has a split that omits required shards. SDKs must return a parse error without poisoning the full configuration.", "attributes": {}, "defaultValue": "default", "flag": "missing-split-shards-flag", "result": { - "reason": "DEFAULT", + "errorCode": "PARSE_ERROR", + "reason": "ERROR", "value": "default" }, "targetingKey": "alice", diff --git a/evaluation-cases/test-case-null-shard-range-isolation.json b/evaluation-cases/test-case-null-shard-range-isolation.json index ada4496..15f4af5 100644 --- a/evaluation-cases/test-case-null-shard-range-isolation.json +++ b/evaluation-cases/test-case-null-shard-range-isolation.json @@ -1,11 +1,12 @@ [ { - "description": "The malformed flag has a null shard range before a catch-all allocation. SDKs must ignore the whole flag and return the caller default.", + "description": "The malformed flag has a null shard range before a catch-all allocation. SDKs must return a parse error without poisoning the full configuration.", "attributes": {}, "defaultValue": "default", "flag": "null-shard-range-flag", "result": { - "reason": "DEFAULT", + "errorCode": "PARSE_ERROR", + "reason": "ERROR", "value": "default" }, "targetingKey": "alice", diff --git a/evaluation-cases/test-case-semver-validation-flag.json b/evaluation-cases/test-case-semver-validation-flag.json index 1ba5fff..9fa9062 100644 --- a/evaluation-cases/test-case-semver-validation-flag.json +++ b/evaluation-cases/test-case-semver-validation-flag.json @@ -112,13 +112,14 @@ "variationType": "STRING" }, { - "description": "An invalid configured SemVer comparand aborts flag evaluation with an error.", + "description": "A flag with an invalid configured SemVer comparand returns a parse error while the rest of the configuration remains usable.", "attributes": { "app_version": "1.2.3" }, "defaultValue": "unknown", "flag": "semver-invalid-comparand-test", "result": { + "errorCode": "PARSE_ERROR", "reason": "ERROR", "value": "unknown" }, diff --git a/evaluation-cases/test-case-unknown-operator-isolation.json b/evaluation-cases/test-case-unknown-operator-isolation.json index 23a3a68..c37fde4 100644 --- a/evaluation-cases/test-case-unknown-operator-isolation.json +++ b/evaluation-cases/test-case-unknown-operator-isolation.json @@ -1,13 +1,14 @@ [ { - "description": "The first allocation uses an unknown operator and the second is a catch-all trap; SDKs should ignore the whole flag and return the caller default.", + "description": "The first allocation uses an unknown operator and the second is a catch-all trap. SDKs must return a parse error without falling through to the catch-all allocation.", "attributes": { "country": "US" }, "defaultValue": "default", "flag": "operator-grease-flag", "result": { - "reason": "DEFAULT", + "errorCode": "PARSE_ERROR", + "reason": "ERROR", "value": "default" }, "targetingKey": "alice", diff --git a/evaluation-cases/test-case-variant-type-mismatch.json b/evaluation-cases/test-case-variant-type-mismatch.json new file mode 100644 index 0000000..ca3b775 --- /dev/null +++ b/evaluation-cases/test-case-variant-type-mismatch.json @@ -0,0 +1,27 @@ +[ + { + "description": "A flag whose variation violates its declared type returns a parse error while the rest of the configuration remains usable.", + "attributes": {}, + "defaultValue": 0, + "flag": "variant-type-mismatch-flag", + "result": { + "errorCode": "PARSE_ERROR", + "reason": "ERROR", + "value": 0 + }, + "targetingKey": "user-1", + "variationType": "INTEGER" + }, + { + "description": "A valid flag in the same configuration continues to evaluate when a malformed neighboring flag returns a parse error.", + "attributes": {}, + "defaultValue": "default", + "flag": "valid-flag-after-invalid-config", + "result": { + "reason": "STATIC", + "value": "expected" + }, + "targetingKey": "valid-neighbor", + "variationType": "STRING" + } +] diff --git a/ufc-config.json b/ufc-config.json index 69b0f3f..52fd31d 100644 --- a/ufc-config.json +++ b/ufc-config.json @@ -857,6 +857,7 @@ "key": "semver-invalid-comparand-test", "enabled": true, "variationType": "STRING", + "comment": "This flag deliberately uses an invalid configured SemVer comparand. SDKs must remove this flag without rejecting the full configuration.", "variations": { "matched": { "key": "matched", @@ -3684,7 +3685,7 @@ "key": "operator-grease-flag", "enabled": true, "variationType": "STRING", - "comment": "This flag deliberately uses an unknown condition operator. SDKs must ignore this flag rather than falling through to the catch-all allocation or rejecting the whole config.", + "comment": "This flag deliberately uses an unknown condition operator. SDKs must remove this flag rather than falling through to the catch-all allocation or rejecting the whole config.", "variations": { "trap": { "key": "trap", @@ -3734,7 +3735,7 @@ "key": "invalid-regex-flag", "enabled": true, "variationType": "STRING", - "comment": "This flag deliberately uses the invalid regex *@datadoghq.com. SDKs must ignore this flag rather than falling through to a later allocation or rejecting the whole config.", + "comment": "This flag deliberately uses the invalid regex *@datadoghq.com. SDKs must remove this flag rather than falling through to a later allocation or rejecting the whole config.", "variations": { "invalid-regex-match": { "key": "invalid-regex-match", @@ -3780,11 +3781,190 @@ } ] }, + "variant-type-mismatch-flag": { + "key": "variant-type-mismatch-flag", + "enabled": true, + "variationType": "INTEGER", + "comment": "This flag is intentionally invalid: one variation contains a string even though the flag declares INTEGER values. SDKs must remove this flag without rejecting the full configuration.", + "variations": { + "invalid": { + "key": "invalid", + "value": "not-an-integer" + }, + "valid": { + "key": "valid", + "value": 0 + } + }, + "allocations": [ + { + "key": "default-allocation", + "rules": [], + "splits": [ + { + "variationKey": "invalid", + "shards": [] + } + ], + "doLog": true + } + ] + }, + "invalid-gt-condition-flag": { + "key": "invalid-gt-condition-flag", + "enabled": true, + "variationType": "STRING", + "comment": "This flag deliberately gives GT a string operand. SDKs must remove this flag rather than falling through to the catch-all allocation.", + "variations": { + "trap": { + "key": "trap", + "value": "trap" + }, + "catch-all": { + "key": "catch-all", + "value": "catch-all" + } + }, + "allocations": [ + { + "key": "invalid-gt-allocation", + "rules": [ + { + "conditions": [ + { + "attribute": "age", + "operator": "GT", + "value": "not-a-number" + } + ] + } + ], + "splits": [ + { + "variationKey": "trap", + "shards": [] + } + ], + "doLog": true + }, + { + "key": "invalid-gt-catch-all", + "rules": [], + "splits": [ + { + "variationKey": "catch-all", + "shards": [] + } + ], + "doLog": true + } + ] + }, + "invalid-one-of-condition-flag": { + "key": "invalid-one-of-condition-flag", + "enabled": true, + "variationType": "STRING", + "comment": "This flag deliberately gives ONE_OF a scalar operand. SDKs must remove this flag rather than falling through to the catch-all allocation.", + "variations": { + "trap": { + "key": "trap", + "value": "trap" + }, + "catch-all": { + "key": "catch-all", + "value": "catch-all" + } + }, + "allocations": [ + { + "key": "invalid-one-of-allocation", + "rules": [ + { + "conditions": [ + { + "attribute": "role", + "operator": "ONE_OF", + "value": "admin" + } + ] + } + ], + "splits": [ + { + "variationKey": "trap", + "shards": [] + } + ], + "doLog": true + }, + { + "key": "invalid-one-of-catch-all", + "rules": [], + "splits": [ + { + "variationKey": "catch-all", + "shards": [] + } + ], + "doLog": true + } + ] + }, + "invalid-is-null-condition-flag": { + "key": "invalid-is-null-condition-flag", + "enabled": true, + "variationType": "STRING", + "comment": "This flag deliberately gives IS_NULL a non-boolean operand. SDKs must remove this flag rather than falling through to the catch-all allocation.", + "variations": { + "trap": { + "key": "trap", + "value": "trap" + }, + "catch-all": { + "key": "catch-all", + "value": "catch-all" + } + }, + "allocations": [ + { + "key": "invalid-is-null-allocation", + "rules": [ + { + "conditions": [ + { + "attribute": "deleted_at", + "operator": "IS_NULL", + "value": "not-a-boolean" + } + ] + } + ], + "splits": [ + { + "variationKey": "trap", + "shards": [] + } + ], + "doLog": true + }, + { + "key": "invalid-is-null-catch-all", + "rules": [], + "splits": [ + { + "variationKey": "catch-all", + "shards": [] + } + ], + "doLog": true + } + ] + }, "malformed-allocations-flag": { "key": "malformed-allocations-flag", "enabled": true, "variationType": "STRING", - "comment": "This flag is intentionally UFC-schema-invalid: allocations must be an array, but this fixture uses a string to verify per-flag parse error isolation.", + "comment": "This flag is intentionally UFC-schema-invalid: allocations must be an array. SDKs must remove this flag without rejecting the full configuration.", "variations": { "on": { "key": "on", @@ -3821,7 +4001,7 @@ "key": "overflow-total-shards-flag", "enabled": true, "variationType": "STRING", - "comment": "This flag is intentionally UFC-schema-invalid: split.shards[].totalShards must fit in an unsigned 32-bit integer. SDKs must ignore this flag rather than overflowing or poisoning the whole config.", + "comment": "This flag is intentionally UFC-schema-invalid: split.shards[].totalShards must fit in an unsigned 32-bit integer. SDKs must remove this flag rather than overflowing or poisoning the whole config.", "variations": { "trap": { "key": "trap", @@ -3857,7 +4037,7 @@ "key": "negative-shard-range-start-flag", "enabled": true, "variationType": "STRING", - "comment": "This flag is intentionally UFC-schema-invalid: split.shards[].ranges[].start must be an unsigned integer. SDKs must ignore this flag rather than treating a negative range as matching traffic.", + "comment": "This flag is intentionally UFC-schema-invalid: split.shards[].ranges[].start must be an unsigned integer. SDKs must remove this flag rather than treating a negative range as matching traffic.", "variations": { "trap": { "key": "trap", @@ -3893,7 +4073,7 @@ "key": "null-shard-range-flag", "enabled": true, "variationType": "STRING", - "comment": "This flag is intentionally UFC-schema-invalid: split.shards[].ranges[] contains null. SDKs must ignore this flag rather than panicking or poisoning the whole config.", + "comment": "This flag is intentionally UFC-schema-invalid: split.shards[].ranges[] contains null. SDKs must remove this flag rather than panicking or poisoning the whole config.", "variations": { "trap": { "key": "trap",