Skip to content
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

fix: don't destory when authz'source unchanged #13398

Merged
merged 1 commit into from
Jul 3, 2024
Merged

fix: don't destory when authz'source unchanged #13398

merged 1 commit into from
Jul 3, 2024

Conversation

zhongwencool
Copy link
Contributor

@zhongwencool zhongwencool commented Jul 3, 2024 •
edited
Loading

Fixes https://emqx.atlassian.net/browse/EMQX-12630

#13385 (comment)

Keep in mind: we will fix as:

  1. Do not clear old ACL items when the built-in database of the authorization has not changed.
  2. Do not clear old ACL items when you update the settings of the built-in database of the authorization, such as enabling or disabling it.
  3. However, delete the old ACL items when you delete the sources of the built-in database of the authorization.

Release version: v/e5.7.2

Summary

PR Checklist

Please convert it to a draft if any of the following conditions are not met. Reviewers may skip over until all the items are checked:

  • Added tests for the changes
  • Added property-based tests for code which performs user input validation
  • Changed lines covered in coverage report
  • Change log has been added to changes/(ce|ee)/(feat|perf|fix|breaking)-<PR-id>.en.md files
  • For internal contributor: there is a jira ticket to track this change
  • Created PR to emqx-docs if documentation update is required, or link to a follow-up jira ticket
  • Schema changes are backward compatible

Checklist for CI (.github/workflows) changes

  • If changed package build workflow, pass this action (manual trigger)
  • Change log has been added to changes/ dir for user-facing artifacts update

@zhongwencool zhongwencool requested review from JimMoen and a team as code owners July 3, 2024 04:41
@zhongwencool zhongwencool force-pushed the authz-db-clear-table branch from e4eb673 to 112433d Compare July 3, 2024 05:15
@zmstone
Copy link
Member

zmstone commented Jul 3, 2024

we should perhaps never deleted data even if built-in authz is removed (same for authn).
For data purge, can expose an API/CLI to do so.

@zmstone
Copy link
Member

zmstone commented Jul 3, 2024

we should perhaps never deleted data even if built-in authz is removed (same for authn). For data purge, can expose an API/CLI to do so.

Discussed with Guowei, in 5.7.2, we can just fix it to survive enable/disable.
The new API/CLI can be added in a minor, 5.8 or 5.9

@zhongwencool zhongwencool merged commit 5f32170 into emqx:release-57 Jul 3, 2024
177 checks passed
@zhongwencool zhongwencool deleted the authz-db-clear-table branch July 3, 2024 07:29
@emqxqa
Copy link

emqxqa commented Jul 5, 2024

TestExecution

@BrewTestBot BrewTestBot mentioned this pull request Aug 7, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zmstone zmstone zmstone approved these changes

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

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@zhongwencool @zmstone @emqxqa