Skip to content

Update AuthorizationResolver to be hot-reload aware #2425

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
seantleonard merged 14 commits into from
Oct 31, 2024
Merged

Update AuthorizationResolver to be hot-reload aware #2425

seantleonard merged 14 commits into from
Oct 31, 2024

Conversation

@seantleonard
Copy link
Contributor

@seantleonard seantleonard commented Oct 14, 2024 •
edited
Loading

Why make this change?

  • Closes Hot-Reload Authorization rules #2324 which tracks making DAB's AuthorizationResolver hot-reload aware. Hot-reload aware just means that the service should update its internal state to align with the latest runtimeconfig changes.

What is this change?

  • Adds Event for DAB's HotReload event handler to signal changes to the runtime config. The AuthorizationResolver can only refresh once the SqlMetadataProvider has fetched updated database object metadata.
  • GraphQL is not yet supported (separate work item for schema eviction).
  • REST should work when Add Support for Hot Reload of DataSource Section of Config file. #2417 is merged which fixes bug with updating metadata providers after hot reload.

How was this tested?

  • Integration Tests -> setup test fixture with initial config specifying a Book entity with certain permissions. The test then hot-reloads a new config with a different entity and different permissions (different include/exclude column names) and validates that only the new entity is accessible and the expected fields are accessible. Validates that the old entity is not accessible and has "been forgotten" by DAB.

@seantleonard seantleonard marked this pull request as ready for review October 29, 2024 22:53
@seantleonard
Copy link
Contributor Author

seantleonard commented Oct 29, 2024

/azp run

Aniruddh25
Aniruddh25 reviewed Oct 29, 2024
View reviewed changes
Aniruddh25
Aniruddh25 reviewed Oct 29, 2024
View reviewed changes
Aniruddh25
Aniruddh25 reviewed Oct 29, 2024
View reviewed changes
Aniruddh25
Aniruddh25 approved these changes Oct 30, 2024
View reviewed changes
Copy link
Collaborator

@Aniruddh25 Aniruddh25 left a comment

Choose a reason for hiding this comment

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

Looks good, specially the TestFixture. Left a few questions. Thanks for pushing out these changes so quickly.

@seantleonard seantleonard added this to the 1.3 milestone Oct 30, 2024
abhishekkumams
abhishekkumams approved these changes Oct 30, 2024
View reviewed changes
Copy link
Contributor

@abhishekkumams abhishekkumams left a comment

Choose a reason for hiding this comment

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

LGTM

@abhishekkumams
Copy link
Contributor

abhishekkumams commented Oct 30, 2024

/azp run

@abhishekkumams
Copy link
Contributor

abhishekkumams commented Oct 30, 2024

/azp run

@abhishekkumams
adding the last commit back
466c0c1 
no change in pipeline. previous change was done to test pipelines behaviour.
@abhishekkumams
Copy link
Contributor

abhishekkumams commented Oct 30, 2024

/azp run

@abhishekkumams
Copy link
Contributor

abhishekkumams commented Oct 30, 2024

/azp run

@seantleonard seantleonard mentioned this pull request Oct 30, 2024
Merged
1 task
@seantleonard
Copy link
Contributor Author

seantleonard commented Oct 30, 2024

/azp run

@seantleonard seantleonard enabled auto-merge (squash) October 30, 2024 22:17
@seantleonard seantleonard merged commit 4f22ca8 into main Oct 31, 2024
1 check passed
@seantleonard seantleonard deleted the dev/sean/authZresolverHR branch October 31, 2024 00:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Aniruddh25 Aniruddh25 Aniruddh25 approved these changes

@aaronburtle aaronburtle aaronburtle approved these changes

@RubenCerna2079 RubenCerna2079 Awaiting requested review from RubenCerna2079 RubenCerna2079 is a code owner

@ravishetye ravishetye Awaiting requested review from ravishetye

@rohkhann rohkhann Awaiting requested review from rohkhann

@neeraj-sharma2592 neeraj-sharma2592 Awaiting requested review from neeraj-sharma2592 neeraj-sharma2592 is a code owner

@sourabh1007 sourabh1007 Awaiting requested review from sourabh1007 sourabh1007 is a code owner

@sezal98 sezal98 Awaiting requested review from sezal98

+1 more reviewer

@abhishekkumams abhishekkumams abhishekkumams approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

1.3

Development

Successfully merging this pull request may close these issues.

Hot-Reload Authorization rules

4 participants

@seantleonard @abhishekkumams @Aniruddh25 @aaronburtle