Skip to content

Conversation

@ellemouton
Copy link
Member

Last couple of prep commits in the actions store. Just to limit the scope of the PR that follows (#1072 )

Part of #917

For our kvdb firewalldb, we use an empty 4 byte array as the macaroon
identifier even if no macaroon was used to create the action. This is so
that we have some sort of "session ID" bucket to store these set of
actions under. For our SQL impl, however, this is not needed and we will
likely just use a nullable field for the macaroon ID. So in preparation
for this, we move the kvdb specific logic to the kvdb impl.
testTime1 should be before testTime2 otherwise it can lead to confusion.
Add a test covering a call to ListActions with group ID set and Reversed
set and assert that the actions are returned in the correct order. This
reveals a bug in the BoltDB implementation which was not making use of
the query.Reversed parameter when group ID is set. NOTE: it still wont
do pagination correctly (ie the pagination params still dont get used)
but we at least here can easily let it use the Reversed param.
@ellemouton ellemouton self-assigned this May 27, 2025
@ellemouton ellemouton added no-changelog This PR is does not require a release notes entry sql-ize labels May 27, 2025
Copy link
Contributor

@bitromortac bitromortac left a comment

Choose a reason for hiding this comment

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

LGTM ⚡

Copy link
Contributor

@ViktorT-11 ViktorT-11 left a comment

Choose a reason for hiding this comment

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

Nice, LGTM 🚀!

// macaroon ID. Otherwise, the last 4 bytes of the macaroon's root key
// ID are used.
var macaroonID [4]byte
var macaroonID fn.Option[[4]byte]
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for this fix 🙏!

@ellemouton ellemouton merged commit bd704c4 into master May 29, 2025
18 of 22 checks passed
@guggero guggero deleted the sql40 branch July 21, 2025 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog This PR is does not require a release notes entry sql-ize

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants