feat(ci): Enhance authorization bdd test coverage - #3298
Merged
elizabethhealy merged 59 commits intoJun 25, 2026
Conversation
…-rr-aav-bdd-namespaced-policy-tests
…-rr-aav-bdd-namespaced-policy-tests
…-rr-aav-bdd-namespaced-policy-tests
…-rr-aav-bdd-namespaced-policy-tests
Contributor
…-rr-aav-bdd-namespaced-policy-tests
…-1311-add-more-authorization-bdd-tests
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
Base automatically changed from
dspx-2753-rr-aav-bdd-namespaced-policy-tests
to
main
April 22, 2026 20:52
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
elizabethhealy
marked this pull request as ready for review
April 24, 2026 19:21
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@service/entityresolution/claims/v2/entity_resolution_test.go`:
- Around line 96-153: Add a third test (e.g.,
Test_ClaimsResolveEntityDirectEntitlementsMalformed) in
entity_resolution_test.go that builds a claims struct with malformed/partial
"direct_entitlements" (e.g., an entry missing "attribute_value_fqn" and another
with "actions" as a non-array), call claims.EntityResolution(...) with the
feature flag enabled (true), and assert the function safely ignores malformed
entries (no panic/error and resulting
entityRepresentations[0].GetDirectEntitlements() does not include invalid items
— expect empty or only valid entries) to pin down parser skip/error behavior;
reference claims.EntityResolution and the existing
Test_ClaimsResolveEntityDirectEntitlements/Test_ClaimsResolveEntityDirectEntitlementsDisabled
for structure.
In `@service/entityresolution/claims/v2/entity_resolution.go`:
- Around line 262-298: In parseDirectEntitlementActions, the case handling
[]string is effectively unreachable when inputs come from structpb
(Struct.AsMap/Value_ListValue yields []interface{}); either remove the []string
branch to avoid confusion or keep it but add a brief comment above the case
noting it is defensive for non-structpb inputs, so future readers understand why
it exists; reference the function name parseDirectEntitlementActions and the
[]string case to locate the change.
- Around line 38-49: Replace the stdlib log.Fatalf call in RegisterClaimsERS
with a panic using fmt.Sprintf so startup errors are raised consistently with
other registry-aware ERS constructors; when mapstructure.Decode returns an
error, keep the existing logger.Error line and then call
panic(fmt.Sprintf("Failed to decode claims entity resolution configuration: %v",
err)) so the service registry can catch and handle the panic (refer to
RegisterClaimsERS, mapstructure.Decode and logger.Error in this function).
In `@tests-bdd/cukes/steps_authorization.go`:
- Around line 493-545: In parseClaimsTable, avoid silently swallowing JSON parse
errors: when rawValue is non-empty and looks JSON-like (e.g., starts with '{' or
'[' or a digit) attempt json.Unmarshal and if it fails return that unmarshal
error instead of falling back to the raw string; only use parsed = rawValue as a
fallback for non-JSON-like tokens. Update the json.Unmarshal error handling in
parseClaimsTable to detect JSON-like rawValue and propagate the error for those
cases while preserving the existing string fallback for plain tokens.
- Around line 129-135: The local variable named "entity" shadows the imported
package "entity"; rename the local variable (e.g., to entityObj or
subjectEntity) where it's defined in the Entity literal so it no longer
conflicts with the package, update all subsequent uses (including
scenarioContext.RecordObject(referenceID, ...)) to the new name, and ensure
references to package-level identifiers like entity.Entity_CATEGORY_SUBJECT and
entity.Entity_Claims remain as package-qualified names.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b1a5b8cd-7ba8-49cb-b6f5-0e4baf2f576e
📒 Files selected for processing (11)
service/entityresolution/claims/v2/entity_resolution.goservice/entityresolution/claims/v2/entity_resolution_test.gotests-bdd/cukes/resources/platform.direct_entitlements.templatetests-bdd/cukes/steps_authorization.gotests-bdd/cukes/steps_obligations.gotests-bdd/cukes/steps_registeredresources.gotests-bdd/features/attribute-rules.featuretests-bdd/features/direct-entitlements.featuretests-bdd/features/multi-resource.featuretests-bdd/features/obligations.featuretests-bdd/features/registered-resources.feature
💤 Files with no reviewable changes (1)
- tests-bdd/cukes/steps_obligations.go
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
This reverts commit 5889055.
Contributor
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
|
Contributor
jakedoublev
approved these changes
Jun 25, 2026
This was referenced Jul 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed Changes
Checklist
Testing Instructions
Summary by CodeRabbit
New Features
Tests