fix(acl): incompatibilities with acl load #2867
Merged
Conversation
This file contains 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
kostasrim
force-pushed
the
fix_acl_compat
branch
from
64f13ec
to
c8bc6cd
Compare
romange
reviewed
Apr 9, 2024
src/server/acl/acl_family.cc
Outdated
| @@ -277,7 +277,8 @@ std::optional<facade::ErrorReply> AclFamily::LoadToRegistryFromFile(std::string_ | |||
| auto registry_with_wlock = registry_->GetRegistryWithWriteLock(); | |||
| auto& registry = registry_with_wlock.registry; | |||
| // TODO(see what redis is doing here) | |||
There was a problem hiding this comment.
is this TODO still relevant?
romange
reviewed
Apr 9, 2024
src/server/acl/acl_family.cc
Outdated
| @@ -308,14 +309,11 @@ void AclFamily::Load(CmdArgList args, ConnectionContext* cntx) { | |||
| return; | |||
| } | |||
|
|
|||
| const auto is_successfull = LoadToRegistryFromFile(acl_file, !cntx); | |||
| const auto has_failed = LoadToRegistryFromFile(acl_file, cntx); | |||
There was a problem hiding this comment.
i think it's less readable. you return the optional result and by reading from the code it's not clear. Lets rename it to load_error.
There was a problem hiding this comment.
btw, I am going to extend ErrorReply to support the happy case as well, I do not think optional chaining is justified in this case.
There was a problem hiding this comment.
Please use GenericError then!☝🏻 ErrorReply if for transfering replies, it WAS a capturing reply builder type originally
There was a problem hiding this comment.
I do not think optional chaining is justified in this case.
Given that I can pass it to ReplyBuilder and expect it so send a reply, I think it is. Otherwise, what should it send? Just an OK? Nothing?
if (auto err = Do(); err)
cntx->SendError(*err); // Will use appropriate internal method,
// with error_kind or op_status supportThere was a problem hiding this comment.
i am talking the usage here
There was a problem hiding this comment.
i renamed it to load_error and I changed the return type to GenericError
romange
approved these changes
Apr 9, 2024
szinn
pushed a commit
to szinn/k8s-homelab
that referenced
this pull request
Apr 16, 2024
…nfly ( v1.16.1 → v1.17.0 ) (#3473) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [docker.dragonflydb.io/dragonflydb/dragonfly](https://togithub.com/dragonflydb/dragonfly) | minor | `v1.16.1` -> `v1.17.0` | --- ### Release Notes <details> <summary>dragonflydb/dragonfly (docker.dragonflydb.io/dragonflydb/dragonfly)</summary> ### [`v1.17.0`](https://togithub.com/dragonflydb/dragonfly/releases/tag/v1.17.0) [Compare Source](https://togithub.com/dragonflydb/dragonfly/compare/v1.16.1...v1.17.0) ##### Dragonfly v1.17.0 Some prominent changes include: - Improved performance for MGET operations ([#​2453](https://togithub.com/dragonflydb/dragonfly/issues/2453)) - Fix argument parsing in json.objkeys ([#​2872](https://togithub.com/dragonflydb/dragonfly/issues/2872)) - Fix ipv6 support for replication ([#​2889](https://togithub.com/dragonflydb/dragonfly/issues/2889)) - Support serialisation of bloom filters - saving to and loading from snapshots ([#​2846](https://togithub.com/dragonflydb/dragonfly/issues/2846)) - Support of HLL PFADD ([#​2761](https://togithub.com/dragonflydb/dragonfly/issues/2761)) - Support bullmq workloads that do not have `{}` hashtags in their queue names ([#​2890](https://togithub.com/dragonflydb/dragonfly/issues/2890)) ##### What's Changed - fix: [#​2745](https://togithub.com/dragonflydb/dragonfly/issues/2745) don't start migration process again after apply the same the same config is applied by [@​BorysTheDev](https://togithub.com/BorysTheDev) in [dragonflydb/dragonfly#2822 - feat(transaction): Idempotent callbacks (immediate runs) by [@​dranikpg](https://togithub.com/dranikpg) in [dragonflydb/dragonfly#2453 - refactor(cluster): replace sync_id with node_id for slot migration [#​2835](https://togithub.com/dragonflydb/dragonfly/issues/2835) by [@​BorysTheDev](https://togithub.com/BorysTheDev) in [dragonflydb/dragonfly#2838 - feat(tiering): Simple OpManager by [@​dranikpg](https://togithub.com/dranikpg) in [dragonflydb/dragonfly#2781 - chore: implement path mutation for JsonFlat by [@​romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2805 - feat(cluster): add migration removing by config [#​2835](https://togithub.com/dragonflydb/dragonfly/issues/2835) by [@​BorysTheDev](https://togithub.com/BorysTheDev) in [dragonflydb/dragonfly#2844 - chore: expose direct API on Bloom objects by [@​romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2845 - chore: generalize CompactObject::AllocateMR by [@​kostasrim](https://togithub.com/kostasrim) in [dragonflydb/dragonfly#2847 - feat(tiering): Simplest small bins by [@​dranikpg](https://togithub.com/dranikpg) in [dragonflydb/dragonfly#2810 - refactor: clean cluster slot migration code by [@​BorysTheDev](https://togithub.com/BorysTheDev) in [dragonflydb/dragonfly#2848 - fix(tests): Fix numsub test by [@​dranikpg](https://togithub.com/dranikpg) in [dragonflydb/dragonfly#2852 - fix: healthcheck for docker containers by [@​romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2853 - fix: possible crash in tls code by [@​romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2854 - fix(server): Do not block admin-port commands by [@​chakaz](https://togithub.com/chakaz) in [dragonflydb/dragonfly#2842 - fix(pytest): make pytests fail if server crash on shutdown by [@​adiholden](https://togithub.com/adiholden) in [dragonflydb/dragonfly#2827 - feat(server): add prints on takeover timeout by [@​adiholden](https://togithub.com/adiholden) in [dragonflydb/dragonfly#2856 - fix(pytest): dont check process return code on kill by [@​adiholden](https://togithub.com/adiholden) in [dragonflydb/dragonfly#2862 - fix: authorize the http connection to call commands by [@​romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2863 - feat(cluster): Send number of keys for incoming and outgoing migrations. by [@​chakaz](https://togithub.com/chakaz) in [dragonflydb/dragonfly#2858 - feat(tiering): TieredStorageV2 by [@​dranikpg](https://togithub.com/dranikpg) in [dragonflydb/dragonfly#2849 - bug(server): set connection flags block/pause flag on all blocking commands by [@​adiholden](https://togithub.com/adiholden) in [dragonflydb/dragonfly#2816 - chore: serialize SBF by [@​romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2846 - fix: test_replicaof_reject_on_load crash on stop by [@​kostasrim](https://togithub.com/kostasrim) in [dragonflydb/dragonfly#2818 - feat(dbslice): Add self-laundering iterator in `DbSlice` by [@​chakaz](https://togithub.com/chakaz) in [dragonflydb/dragonfly#2815 - chore: License update by [@​romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2767 - fix(acl): incompatibilities with acl load by [@​kostasrim](https://togithub.com/kostasrim) in [dragonflydb/dragonfly#2867 - fix(json): make path optional in json.objkeys by [@​kostasrim](https://togithub.com/kostasrim) in [dragonflydb/dragonfly#2872 - fix: return wrong type errors for SET...GET command by [@​romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2874 - fix(redis replication): remove partial sync flow ,not supported yet by [@​adiholden](https://togithub.com/adiholden) in [dragonflydb/dragonfly#2865 - chore: limit traffic logger only to the main interface by [@​romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2877 - chore: relax repltakeover constraints to only exclude write commands by [@​kostasrim](https://togithub.com/kostasrim) in [dragonflydb/dragonfly#2873 - chore(replayer): Roll back to go1.18 by [@​dranikpg](https://togithub.com/dranikpg) in [dragonflydb/dragonfly#2881 - fix: brpoplpush single shard to wake up blocked transactions by [@​kostasrim](https://togithub.com/kostasrim) in [dragonflydb/dragonfly#2875 - chore: LockTable tracks fingerprints of keys by [@​romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2839 - chore: reject TLS handshake when our listener is plain TCP by [@​romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2882 - Add support for Sparse HLL PFADD by [@​azuredream](https://togithub.com/azuredream) in [dragonflydb/dragonfly#2761 - feat server: bring visibility to script errors by [@​adiholden](https://togithub.com/adiholden) in [dragonflydb/dragonfly#2879 - chore: clean up REPLTAKEOVER flow by [@​romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2887 - chore(tiering): Move files and move kb literal to common by [@​dranikpg](https://togithub.com/dranikpg) in [dragonflydb/dragonfly#2868 - chore(interpreter): Support object replies by [@​dranikpg](https://togithub.com/dranikpg) in [dragonflydb/dragonfly#2885 - fix(ci/helm): Stick to v0.73.0 version of prom operator by [@​Pothulapati](https://togithub.com/Pothulapati) in [dragonflydb/dragonfly#2893 - fix(acl): authentication with UDS socket by [@​kostasrim](https://togithub.com/kostasrim) in [dragonflydb/dragonfly#2895 - feat(cluster): add repeated ACK if an error is happened by [@​BorysTheDev](https://togithub.com/BorysTheDev) in [dragonflydb/dragonfly#2892 - chore(blocking): Remove faulty DCHECK by [@​dranikpg](https://togithub.com/dranikpg) in [dragonflydb/dragonfly#2898 - chore: add a clear link on how to build dragonfly from source by [@​romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2884 - feat(server): Allow configuration of hashtag extraction by [@​chakaz](https://togithub.com/chakaz) in [dragonflydb/dragonfly#2890 - fix: fix build under macos by [@​BorysTheDev](https://togithub.com/BorysTheDev) in [dragonflydb/dragonfly#2901 - fix(cluster_replication): replicate redis cluster node bug fix by [@​adiholden](https://togithub.com/adiholden) in [dragonflydb/dragonfly#2876 - fix(acl): skip http and add check on connection traversals by [@​kostasrim](https://togithub.com/kostasrim) in [dragonflydb/dragonfly#2883 - fix(zset): Better memory consumption calculation by [@​chakaz](https://togithub.com/chakaz) in [dragonflydb/dragonfly#2900 - fix: fix ld for num converting by [@​BorysTheDev](https://togithub.com/BorysTheDev) in [dragonflydb/dragonfly#2902 - chore: add help string for memory_fiberstack_vms_bytes by [@​romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2903 - fix(sanitizers): false positive fail on multi_test::Eval by [@​kostasrim](https://togithub.com/kostasrim) in [dragonflydb/dragonfly#2896 - chore: pull helio and add ipv6 replication test by [@​dranikpg](https://togithub.com/dranikpg) in [dragonflydb/dragonfly#2889 - chore: add ipv6 support for native linux release by [@​romange](https://togithub.com/romange) in [dragonflydb/dragonfly#2908 ##### Huge thanks to all the contributors! ❤️ **Full Changelog**: dragonflydb/dragonfly@v1.16.0...v1.17.0 </details> <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zMDEuNSIsInVwZGF0ZWRJblZlciI6IjM3LjMwMS41IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJyZW5vdmF0ZS9jb250YWluZXIiLCJ0eXBlL21pbm9yIl19--> Co-authored-by: repo-jeeves[bot] <106431701+repo-jeeves[bot]@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
addresses #2866 and #2837
OKbefore it kills the active connections