akuity / kargo

Application lifecycle orchestration
https://kargo.akuity.io/
Apache License 2.0
1.55k stars 133 forks source link

feat: implement conditional secret management #2517

Closed fykaa closed 2 weeks ago

fykaa commented 3 weeks ago

Fixes: https://github.com/akuity/kargo/issues/2214

Key changes include:

  1. Configuration updates:

    • EnableSecretManagement flag
    • modified ClusterRole and ConfigMap to manage secrets based on this flag
    • updated values.yaml and config.go to incorporate the new setting
  2. API enhancements:

    • updated GetConfig endpoint to include SecretManagementEnabled status
  3. Endpoint adjustments:

    • ensure endpoints for managing secrets return connect.CodeUnimplemented when secret management is disabled

cc: @krancour @rbreeze

netlify[bot] commented 3 weeks ago

Deploy Preview for docs-kargo-akuity-io ready!

Name Link
Latest commit 42254547cc8597185a8e66888d9dc3adda9f1f7c
Latest deploy log https://app.netlify.com/sites/docs-kargo-akuity-io/deploys/66e1c5350aa62d00084cc4bb
Deploy Preview https://deploy-preview-2517.kargo.akuity.io
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

krancour commented 3 weeks ago

It looks like a few unit tests that use the credential management endpoints may need an update. Their config needs secret management enabled.

codecov[bot] commented 3 weeks ago

Codecov Report

Attention: Patch coverage is 8.69565% with 21 lines in your changes missing coverage. Please review.

Project coverage is 48.26%. Comparing base (a64eb0e) to head (4225454). Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
internal/api/delete_credentials_v1alpha1.go 0.00% 5 Missing :warning:
internal/api/get_credentials_v1alpha1.go 0.00% 4 Missing and 1 partial :warning:
internal/api/list_credentials_v1alpha1.go 0.00% 5 Missing :warning:
internal/api/update_credentials_v1alpha1.go 0.00% 5 Missing :warning:
internal/api/config/config.go 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2517 +/- ## ========================================== - Coverage 48.32% 48.26% -0.06% ========================================== Files 254 254 Lines 18133 18155 +22 ========================================== + Hits 8762 8763 +1 - Misses 8889 8909 +20 - Partials 482 483 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

krancour commented 3 weeks ago

@rbreeze any chance we can get you to amend this PR with the bits to hide credential-management functionality when the config endpoint says its not enabled?

rbreeze commented 3 weeks ago

@fykaa @krancour Sure thing, just pushed the update 👍🏻

krancour commented 3 weeks ago

Added a tiny fix in 92f07be85881f3e7df6d34837893bf809536750a

krancour commented 3 weeks ago

Thank you @fykaa! This is great!

krancour commented 3 weeks ago

And @rbreeze, thanks to you as well!