Skip to content

rm check_permission and check_relation - #730

Merged
aserto-bot merged 3 commits into
mainfrom
chore/rm-check-permission-relation
Jun 19, 2026
Merged

rm check_permission and check_relation#730
aserto-bot merged 3 commits into
mainfrom
chore/rm-check-permission-relation

Conversation

@gertd

@gertd gertd commented Jun 19, 2026

Copy link
Copy Markdown
Member

Removing the CheckPermission and CheckRelation gRPC and REST implementations, returning unimplemented errors, use the Check function instead.

Removing the ds.check_permission and ds.check_relation builtins, use the ds.check builtin instead.

When converting from check_relation simply change to use the check function, no futher changes are required!!

When converting from converting from check_permission, update to use the check function and update the permission parameter name into relation.

@coveralls

coveralls commented Jun 19, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 27824820047

Coverage increased (+0.1%) to 7.183%

Details

  • Coverage increased (+0.1%) from the base build.
  • Patch coverage: 8 uncovered changes across 1 file (0 of 8 lines covered, 0.0%).
  • 2 coverage regressions across 1 file.

Uncovered Changes

File Changed Covered %
internal/eds/pkg/directory/v3/reader.go 8 0 0.0%

Coverage Regressions

2 previously-covered lines in 1 file lost coverage.

File Lines Losing Coverage Coverage
internal/eds/pkg/directory/v3/reader.go 2 0.0%

Coverage Stats

Coverage Status
Relevant Lines: 10372
Covered Lines: 745
Line Coverage: 7.18%
Coverage Strength: 0.08 hits per line

💛 - Coveralls

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes the legacy CheckPermission / CheckRelation surfaces across Topaz (OPA builtins, runtime registration, and CLI test runner) and makes the corresponding Directory v3 Reader RPCs explicitly return Unimplemented, guiding callers to use Check instead.

Changes:

  • Removed ds.check_permission / ds.check_relation OPA builtins and their associated help tests.
  • Updated Topaz runtime builtin registration and CLI test runner to drop the removed check types.
  • Modified Directory v3 Reader CheckPermission / CheckRelation to return codes.Unimplemented errors.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
topazd/tests/builtin/builtin_test.go Removes help/format tests for the deleted ds.check_permission and ds.check_relation builtins.
topazd/authorizer/builtins/names.go Removes builtin name constants for check_permission / check_relation.
topazd/authorizer/builtins/ds/check_relation.go Deletes the obsolete ds.check_relation builtin implementation.
topazd/authorizer/builtins/ds/check_permission.go Deletes the obsolete ds.check_permission builtin implementation.
topazd/app/topaz/runtime_resolver.go Stops registering the removed builtins in the OPA runtime.
topaz/cmd/common/testrunner.go Removes CLI test-runner execution paths for legacy check types.
topaz/cmd/common/test.go Removes check-type enum/string mappings for check_permission / check_relation.
internal/eds/pkg/ds/check_relation.go Removes internal request wrapper/validation for CheckRelation.
internal/eds/pkg/ds/check_permission.go Removes internal request wrapper/validation for CheckPermission.
internal/eds/pkg/directory/v3/reader.go Makes CheckPermission / CheckRelation return Unimplemented; updates grpc metadata usage.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/eds/pkg/directory/v3/reader.go Outdated
Comment thread internal/eds/pkg/directory/v3/reader.go Outdated
@aserto-bot
aserto-bot merged commit 03d9713 into main Jun 19, 2026
6 checks passed
@aserto-bot
aserto-bot deleted the chore/rm-check-permission-relation branch June 19, 2026 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants