Skip to content

Conversation

@dereknola
Copy link
Member

@dereknola dereknola commented Mar 27, 2025

Proposed Changes

  • Adds a new server configuration flag: secrets-encryption-provider with valuse of aescbc (default and original) and secretbox.
  • Supports migration between key types. Only allowed with the newer k3s secrets-encrypt rotate-keys command. Not supported on the older (eventually will be deprecated) prepare rotate and reencrypt subcommands.

Types of Changes

New Feature

Verification

Manually tested through various configurations:
NEW CLUSTER:

  • Add secrets-encryption: true and secrets-encryption-provider: secretbox to the config.yaml
  • Start the cluster
  • Check k3s secrets-encrypt status you will see a new XSalsa20-POLY1305 type as the active key

MIGRATING CLUSTER

  • Add secrets-encryption-provider: secretbox to the config.yaml. secrets-encryption should already be there
  • Restart all servers
  • Run k3s secrets-encrypt rotate-keys on a single server
  • Restart all servers
  • All servers should now have a new XSalsa20-POLY1305 type as the active key

Testing

Covered by new testlet in docker tests

Linked Issues

#12222

User-Facing Change

Users can now configure secrets encryption to use `secretbox` provider by setting the `secrets-encryption-provider` flag.

Further Comments

@dereknola dereknola requested a review from a team as a code owner March 27, 2025 17:50
Copy link
Member

@brandond brandond left a comment

Choose a reason for hiding this comment

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

We're allowing users to select the provider, not the key type. Lets use the same language as the upstream docs to avoid confusion.

@codecov
Copy link

codecov bot commented Mar 28, 2025

Codecov Report

Attention: Patch coverage is 3.50877% with 220 lines in your changes missing coverage. Please review.

Project coverage is 44.72%. Comparing base (d45006b) to head (2003460).
Report is 11 commits behind head on master.

Files with missing lines Patch % Lines
pkg/server/handlers/secrets-encrypt.go 0.00% 104 Missing ⚠️
pkg/secretsencrypt/config.go 0.00% 70 Missing ⚠️
pkg/daemons/control/deps/deps.go 0.00% 41 Missing ⚠️
pkg/cluster/bootstrap.go 0.00% 3 Missing ⚠️
pkg/cli/secretsencrypt/secrets_encrypt.go 66.66% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #12021      +/-   ##
==========================================
- Coverage   45.89%   44.72%   -1.18%     
==========================================
  Files         188      188              
  Lines       19068    19190     +122     
==========================================
- Hits         8752     8582     -170     
- Misses       9032     9361     +329     
+ Partials     1284     1247      -37     
Flag Coverage Δ
e2etests 35.63% <1.75%> (+1.03%) ⬆️
inttests 35.20% <3.50%> (-0.21%) ⬇️
unittests 16.74% <0.00%> (-0.11%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

brandond
brandond previously approved these changes Mar 28, 2025
- Add testlet for new provider switch
- Handle migration between providers
- Add exception for criticalcontrolargs
Signed-off-by: Derek Nola <derek.nola@suse.com>
Signed-off-by: Derek Nola <derek.nola@suse.com>
Signed-off-by: Derek Nola <derek.nola@suse.com>
Signed-off-by: Derek Nola <derek.nola@suse.com>
return err
}
if control.EncryptProvider == secretsencrypt.SecretBoxProvider {
return fmt.Errorf("rotate does not support secretbox key type, use rotate-keys instead")
Copy link
Member

Choose a reason for hiding this comment

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

is this something we need to cover in the docs?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah this will be mentioned when this gets added to docs.

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.

3 participants