Skip to content

Conversation

jaggederest
Copy link
Contributor

#468

Completely raw, but testing Works On My Machine(tm)

I suspect this will need a bunch of cleanup and rework but it's worth looking at to see the scale of the project.

@jaggederest jaggederest requested a review from levkk September 18, 2025 05:18
jaggederest and others added 2 commits September 17, 2025 23:00
* A few fixes

* downgrade logs

* save
@levkk
Copy link
Collaborator

levkk commented Sep 18, 2025

Could we also move the client gssapi stuff in its own module like the other auth algos

- name: Install CMake 3.31
run: |
sudo apt update && sudo apt install mold -y
sudo apt update && sudo apt install -y mold clang pkg-config protobuf-compiler
Copy link
Collaborator

Choose a reason for hiding this comment

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

We have protobuf! We are finally complete

principal.replace(['@', '.', '/'], "_"),
std::process::id()
);
std::env::set_var("KRB5CCNAME", format!("FILE:{}", cache_file));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Doesn't look like access to this function is synchronized. set_var isn't thread-safe & if we have multiple creds (e.g., for multiple databases), this will race I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, there's a lot of issues like that here. Especially with the env variables but the KDC is kind of a synchronization point so we'll need to use ScopedEnv or something.

std::env::set_var("KRB5CCNAME", format!("FILE:{}", cache_file));

// Acquire credentials using the keytab
Cred::acquire(
Copy link
Collaborator

Choose a reason for hiding this comment

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

This calls into gssapi right? How expensive is this call? We might want to offload this to spawn_blocking to avoid blocking Tokio.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's pretty heavy. There's a tension here, because Cred::acquire is blocking and times out if there's a problem (great api design), but shelling out to kinit or other command line tools is probably even more unreliable. in the v2 branch I'm trying to get either an async wrapper around it or a quicker failure through a proper timer in a different thread but it's tricky.

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

Successfully merging this pull request may close these issues.

2 participants