Autopudate#174
Conversation
When multiple roles grant the same exact target or wildcard, or if multiple targets generate the same fact (e.g. granted_target_all_facts), the biscuit builder would fail with 'fact already exists'. This intercepts facts during minting and deduplicates them using their string representation before adding them to the token builder.
The peaceiris/actions-gh-pages action is getting stuck in the 'deployment_queued' state due to GitHub's internal gh-pages branch syncing issues. This transitions the workflow to use the native actions/upload-pages-artifact and actions/deploy-pages flow, which deploys the artifact directly to Pages without relying on the intermediate gh-pages branch.
…rors Since the 'github-pages' environment enforces branch protection rules (e.g., only 'main' can deploy), attaching the environment to a single job causes PR builds to fail immediately when trying to evaluate the job. This commit splits the workflow into 'build' and 'deploy' jobs. The 'build' job runs on PRs to verify that Hugo compiles successfully, while the 'deploy' job is strictly skipped for PRs and only runs on 'main'.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a centralized deduplication mechanism for adding authority facts to Biscuit tokens in cmd/sam-hub/biscuit.go by wrapping builder.AddAuthorityFact in a helper function addFact that tracks added facts using a map. This prevents errors when duplicate facts are added, which is verified by a new unit test. The reviewer suggested removing a now-redundant local seen map in translateClaimsToFacts to simplify the code and avoid unnecessary allocations.
| } | ||
| seen[g] = true | ||
| if err := builder.AddAuthorityFact(biscuit.Fact{Predicate: biscuit.Predicate{ | ||
| if err := addFact(biscuit.Fact{Predicate: biscuit.Predicate{ |
There was a problem hiding this comment.
With the introduction of the centralized deduplication logic in addFact (which uses the addedFacts map globally), the local seen map used in translateClaimsToFacts for api.FactGroup is now redundant.
You can simplify this block and avoid the extra map allocation by removing the seen map entirely:
case api.FactGroup:
groups := toStringSlice(val)
for _, g := range groups {
if err := addFact(biscuit.Fact{Predicate: biscuit.Predicate{
Name: factName,
IDs: []biscuit.Term{biscuit.String(g)},
}}); err != nil {
return fmt.Errorf("failed to add %s fact: %w", factName, err)
}
}1 participant
No description provided.