Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: disabling newly added kubelet serving certificate rotation #466

Closed
wants to merge 3 commits into from

Conversation

Bryce-Soghigian
Copy link
Collaborator

@Bryce-Soghigian Bryce-Soghigian commented Aug 23, 2024

Fixes #

Description
This PR in AgentBaker enables the kubelet flag --rotate-server-certificates kubelet flag. This flag will auto request and rotate the kubelet serving certificates by requesting new certs from the apiservers certificates.k8s.io apigroup when a certificate expiration approaches. This also requires the RotateKubeletServerCertificate feature gate to be enabled. The existing model just self signs the certificates.

In terms of the karpenter impact, we are just going to stick with self signed certificates for now since we don't have everything configured to leverage this functionality.

How was this change tested?

  • make presubmit

Does this change impact docs?

  • Yes, PR includes docs updates
  • Yes, issue opened: #
  • No

Release Note


@cameronmeissner
Copy link

we'll need to come up with a way Karpenter can determine when it's safe to enable this flag

@cameronmeissner
Copy link

will probably end up deploying the required plugin whenever karpenter is enabled to ensure this flag is safe to set, for now will keep as false until the new plugin as rolled out

@coveralls
Copy link

coveralls commented Aug 23, 2024

Pull Request Test Coverage Report for Build 10532706044

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 97.702%

Totals Coverage Status
Change from base Build 10517352172: 0.0%
Covered Lines: 36313
Relevant Lines: 37167

💛 - Coveralls

@cameronmeissner
Copy link

cameronmeissner commented Aug 23, 2024

one thing to also note, Karpenter will need to make sure to not pass --tls-cert-file and --tls-private-key-file as kubelet flags when this is enabled, and obviously pass --rotate-server-certificates

similar to: https://github.com/Azure/AgentBaker/blob/346c0bb4b43216f438734add970ab96157ed99b1/pkg/agent/baker.go#L295

Copy link
Collaborator Author

@Bryce-Soghigian Bryce-Soghigian left a comment

Choose a reason for hiding this comment

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

/test

README.md Outdated Show resolved Hide resolved
@tallaxes
Copy link
Collaborator

tallaxes commented Aug 23, 2024

Why do we need to do anything to preserve the current functionality? The support for ENABLE_KUBELET_SERVING_CERTIFICATE_ROTATION in AB/image is (correctly) added with backward compatibility: not having this flag in contract is the same as setting it to false.

@tallaxes tallaxes added area/bootstrap Issues or PRs related to bootstrap area/security Issues or PRs related to security labels Aug 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/bootstrap Issues or PRs related to bootstrap area/security Issues or PRs related to security
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants