Skip to content
New issue

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

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

Already on GitHub? # to your account

PS-9609: locate_secrets_* operations added to kmippp #18

Conversation

lukin-oleksiy
Copy link
Contributor

@lukin-oleksiy lukin-oleksiy commented Apr 7, 2025

    https://perconadev.atlassian.net/browse/PS-9609

op_locate_secrets_by_group() and op_all_secrets() operations added to kmippp library to be able to load audit log encryption passwords into component_keyring_kmip cache. Version bumped to 0.3.2

@lukin-oleksiy lukin-oleksiy force-pushed the PS-9609-add_locate_secret_data branch 2 times, most recently from 014db87 to c085c30 Compare April 7, 2025 10:33
@lukin-oleksiy lukin-oleksiy changed the title PS-9609: locate_secrets_by_group operation added to kmippp PS-9609: locate_secrets_* operations added to kmippp Apr 7, 2025
        https://perconadev.atlassian.net/browse/PS-9609

op_locate_secrets_by_group() and op_all_secrets()
operationos added to kmippp library
to be able to load audit log encryption passwords into
component_keyring_kmip cache. Version bumped to 0.3.2
@lukin-oleksiy lukin-oleksiy force-pushed the PS-9609-add_locate_secret_data branch from c085c30 to 5c2e392 Compare April 7, 2025 10:37
@lukin-oleksiy lukin-oleksiy requested a review from ktrushin April 7, 2025 10:39
Copy link
Contributor

@percona-ysorokin percona-ysorokin left a comment

Choose a reason for hiding this comment

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

LGTM

@lukin-oleksiy lukin-oleksiy merged commit 00496b5 into Percona-Lab:master Apr 8, 2025
Comment on lines +20 to +21
const std::string group = argv[6]!=nullptr? argv[6] : "TestGroup";
auto keys = ctx.op_locate_secrets_by_group (group);
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems these two lines have excessive indent of one space.


while (upto < all)
{
int result = kmip_bio_locate (bio_, a, 2, &locate_result, 16, upto);
Copy link
Contributor

Choose a reason for hiding this comment

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

I would use a named constant (e.g. attribute_count) instead of hardcoded 2.

ta.attribute_count = ARRAY_LENGTH (a);

int upto = 0;
int all = 1; // TMP
Copy link
Contributor

Choose a reason for hiding this comment

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

What does TMP mean?

Comment on lines +425 to +437
if (locate_result.located_items != 0)
{
all = locate_result.located_items; // shouldn't change after its != 1
}
else
{
// Dummy server sometimes returns 0 for located_items
all += locate_result.ids_size;
if (locate_result.ids_size == 0)
{
--all;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The logic with all, located_items and ids_size isn't crystal clear. Probably , a detailed comment would be helpful.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants