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

Add Client::with_valid_until for client cert expiry #1707

Merged
merged 3 commits into from
Mar 10, 2025

Conversation

goenning
Copy link
Contributor

fixes #1675 as a follow up from #1676

This PR implements Option 3 as described in #1676 without any breaking change

I decided to make a separate so they can be compared

Signed-off-by: goenning <me@goenning.net>
Signed-off-by: goenning <me@goenning.net>
Copy link

codecov bot commented Feb 27, 2025

Codecov Report

Attention: Patch coverage is 50.00000% with 11 lines in your changes missing coverage. Please review.

Project coverage is 76.1%. Comparing base (b27cbcd) to head (a99263f).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
kube-client/src/client/auth/mod.rs 0.0% 4 Missing ⚠️
kube-client/src/client/config_ext.rs 57.2% 3 Missing ⚠️
kube-client/src/client/builder.rs 75.0% 2 Missing ⚠️
kube-client/src/client/mod.rs 33.4% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            main   #1707     +/-   ##
=======================================
- Coverage   76.2%   76.1%   -0.0%     
=======================================
  Files         84      84             
  Lines       7847    7858     +11     
=======================================
+ Hits        5972    5977      +5     
- Misses      1875    1881      +6     
Files with missing lines Coverage Δ
kube-client/src/client/builder.rs 64.3% <75.0%> (+0.2%) ⬆️
kube-client/src/client/mod.rs 75.6% <33.4%> (-0.6%) ⬇️
kube-client/src/client/config_ext.rs 50.0% <57.2%> (ø)
kube-client/src/client/auth/mod.rs 48.9% <0.0%> (-0.4%) ⬇️
🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@clux clux left a comment

Choose a reason for hiding this comment

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

yeah, i like this! thank you!

@clux clux added the changelog-add changelog added category for prs label Mar 4, 2025
@clux clux added this to the 0.99.0 milestone Mar 4, 2025
Comment on lines +299 to +308
pub(crate) fn exec_identity_pem(&self) -> (Option<Vec<u8>>, Option<DateTime<Utc>>) {
match Auth::try_from(&self.auth_info) {
Ok(Auth::Certificate(client_certificate_data, client_key_data)) => {
Ok(Auth::Certificate(client_certificate_data, client_key_data, expiratiom)) => {
const NEW_LINE: u8 = b'\n';

let mut buffer = client_key_data.expose_secret().as_bytes().to_vec();
buffer.push(NEW_LINE);
buffer.extend_from_slice(client_certificate_data.as_bytes());
buffer.push(NEW_LINE);
Some(buffer)
(Some(buffer), expiratiom)
Copy link
Member

@clux clux Mar 7, 2025

Choose a reason for hiding this comment

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

I was trying to think of a way to lock this down with some tests, because that's the only thing that makes me a little tentative about merging this immediately.

Maybe this is a good fn to put a unit test around?

Copy link
Member

Choose a reason for hiding this comment

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

Eh, this isn't a big deal. I'll merge it in.

@clux clux enabled auto-merge (squash) March 10, 2025 12:24
@clux clux merged commit 55e865c into kube-rs:main Mar 10, 2025
15 of 16 checks passed
@clux clux changed the title add valid_until to Client Add Client::with_valid_until for client cert expiry Mar 11, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
changelog-add changelog added category for prs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Kube ignores expirationTimestamp when exec returns a client certificate
2 participants