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.

Sign up for GitHub

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

[WIP] Use GetCertificate for certificate lazy loading #13695

Closed
wants to merge 1 commit into from
Closed

[WIP] Use GetCertificate for certificate lazy loading #13695

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 15 additions & 12 deletions cmd/activator/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ func main() {
// Enable TLS client when queue-proxy-ca is specified.
// At this moment activator with TLS does not disable HTTP.
// See also https://github.com/knative/serving/issues/12808.
// Also, the current activator must be restarted when updating the secret of CA.
Copy link
Contributor

@skonto skonto Feb 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

(Thinking out loud) It seems that a cert pool uses a map underneath would it be possible to override the existing rootCA pool when the secret is updated? What are the options here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the comment.
Please let me try to solve only server cert part in this PR. Even only server certs part, it is not easy as some CI is failing and need to figure out it.

if tlsEnabled {
logger.Info("Internal Encryption is enabled")
caSecret, err := kubeClient.CoreV1().Secrets(system.Namespace()).Get(ctx, netcfg.ServingInternalCertName, metav1.GetOptions{})
Expand Down Expand Up @@ -300,20 +301,22 @@ func main() {
// At this moment activator with TLS does not disable HTTP.
// See also https://github.com/knative/serving/issues/12808.
if tlsEnabled {
secret, err := kubeClient.CoreV1().Secrets(system.Namespace()).Get(ctx, netcfg.ServingInternalCertName, metav1.GetOptions{})
if err != nil {
logger.Fatalw("failed to get secret", zap.Error(err))
}
cert, err := tls.X509KeyPair(secret.Data[certificates.CertName], secret.Data[certificates.PrivateKeyName])
if err != nil {
logger.Fatalw("failed to load certs", zap.Error(err))
}

// TODO: Implement the secret (certificate) rotation like knative.dev/pkg/webhook/certificates/.
// Also, the current activator must be restarted when updating the secret.
name, server := "https", pkgnet.NewServer(":"+strconv.Itoa(networking.BackendHTTPSPort), ah)
go func(name string, s *http.Server) {
s.TLSConfig = &tls.Config{Certificates: []tls.Certificate{cert}, MinVersion: tls.VersionTLS12}
s.TLSConfig = &tls.Config{
MinVersion: tls.VersionTLS12,
GetCertificate: func(*tls.ClientHelloInfo) (*tls.Certificate, error) {
secret, err := kubeClient.CoreV1().Secrets(system.Namespace()).Get(ctx, netcfg.ServingInternalCertName, metav1.GetOptions{})
if err != nil {
errCh <- fmt.Errorf("failed to get secret: %w", err)
}
cert, err := tls.X509KeyPair(secret.Data[certificates.CertName], secret.Data[certificates.PrivateKeyName])
if err != nil {
errCh <- fmt.Errorf("failed to load certs: %w", err)
}
return &cert, nil
},
}
// Don't forward ErrServerClosed as that indicates we're already shutting down.
if err := s.ListenAndServeTLS("", ""); err != nil && !errors.Is(err, http.ErrServerClosed) {
errCh <- fmt.Errorf("%s server failed: %w", name, err)
Expand Down