Skip to content

Conversation

@ianhundere
Copy link
Contributor

@ianhundere ianhundere commented Jul 30, 2025

closes #2118

Summary

currently, cert-maker will fail when using vault enterprise as per:
error initializing root KMS: failed to initialize HashiVault KMS: kms specification should be in the format hashivault://<key>

Release Note

  • Fixes cert-maker bug when using full vault key-name

Documentation

will update docs (e.g. include VAULT_NAMESPACE env var etc)

@ianhundere ianhundere force-pushed the fix/fixes-vault-key-name-conventions branch 2 times, most recently from ae6abdd to 43394e6 Compare July 30, 2025 22:53
@ianhundere ianhundere marked this pull request as ready for review July 30, 2025 22:53
@ianhundere ianhundere requested a review from a team as a code owner July 30, 2025 22:53
@ianhundere
Copy link
Contributor Author

@haydentherapper i saw the build tests failed, i'm assuming that's okay tho. 🤔

@ianhundere ianhundere force-pushed the fix/fixes-vault-key-name-conventions branch from f160e9b to b582a92 Compare July 31, 2025 13:34
@ianhundere ianhundere force-pushed the fix/fixes-vault-key-name-conventions branch from 681a873 to 12f01b4 Compare July 31, 2025 13:49
@codecov
Copy link

codecov bot commented Jul 31, 2025

Codecov Report

❌ Patch coverage is 41.66667% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 43.59%. Comparing base (cf238ac) to head (5bc2bfc).
⚠️ Report is 440 commits behind head on main.

Files with missing lines Patch % Lines
pkg/certmaker/certmaker.go 33.33% 3 Missing and 1 partial ⚠️
cmd/certificate_maker/certificate_maker.go 50.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #2117       +/-   ##
===========================================
- Coverage   57.93%   43.59%   -14.34%     
===========================================
  Files          50       71       +21     
  Lines        3119     5656     +2537     
===========================================
+ Hits         1807     2466      +659     
- Misses       1154     2965     +1811     
- Partials      158      225       +67     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@haydentherapper haydentherapper left a comment

Choose a reason for hiding this comment

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

Can you revert the protobuf file changes? I’ll guess that your local environment has a different version of the protobuf compiler installed.

@ianhundere ianhundere force-pushed the fix/fixes-vault-key-name-conventions branch from 12f01b4 to ce48af6 Compare July 31, 2025 16:00
Signed-off-by: ianhundere <[email protected]>
@ianhundere ianhundere force-pushed the fix/fixes-vault-key-name-conventions branch from ce48af6 to 5bc2bfc Compare July 31, 2025 16:01
@ianhundere
Copy link
Contributor Author

Can you revert the protobuf file changes? I’ll guess that your local environment has a different version of the protobuf compiler installed.

done / done, thanks for the quick approval.

@ianhundere ianhundere changed the title fix: vault now only expect the key name / sigstore library constructs… fix: vault for enterprise expects only the key name Jul 31, 2025
@haydentherapper haydentherapper merged commit 9320574 into sigstore:main Jul 31, 2025
13 checks passed
@haydentherapper
Copy link
Contributor

Thank you!

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.

fix: certmaker does not use the correct values for vault key names when using vault enterprise (must include namespace)

2 participants