diff --git a/docs/quickstart/installing-thundernetes.md b/docs/quickstart/installing-thundernetes.md index 8f40faa7..7ceba964 100644 --- a/docs/quickstart/installing-thundernetes.md +++ b/docs/quickstart/installing-thundernetes.md @@ -80,7 +80,7 @@ There are two ways to generate a certificate. #### Using cert-manager to generate certificates -Since cert-manager is already installed in the cluster, it can be used to generate a certificate for mTLS authentication. This is the recommended approach. +Since cert-manager is already installed in the cluster, it can be used to generate a certificate for mTLS authentication. This is the recommended approach. When using cert-manager, certificate rotation is handled automatically — Thundernetes watches the mounted certificate files and reloads them when they change, so there is no need to restart the controller pod when certificates are renewed. First of all, you need to create the namespace `thundernetes-system`: @@ -155,6 +155,8 @@ kubectl apply --server-side -f https://raw.githubusercontent.com/PlayFab/thunder **Note:** The two installation files (operator.yaml and operator_with_security.yaml) are identical except for the API_SERVICE_SECURITY environment variable that is passed into the controller container. +> **Certificate rotation:** When TLS is enabled, Thundernetes automatically monitors the mounted TLS secret for changes and reloads the certificate without requiring a pod restart. This works with cert-manager automatic renewal or manual secret updates. The controller polls for certificate file changes every 30 seconds, so renewed certificates are picked up shortly after kubelet syncs the updated secret to the pod (typically within ~60 seconds total). + ### Next steps Check the [.NET sample](sample-dotnet.md) document to learn how to test your installation by using our fake .NET game server sample. \ No newline at end of file diff --git a/installfiles/operator.yaml b/installfiles/operator.yaml index 03b3e733..924c5c3d 100755 --- a/installfiles/operator.yaml +++ b/installfiles/operator.yaml @@ -17658,6 +17658,8 @@ spec: valueFrom: fieldRef: fieldPath: metadata.namespace + - name: TLS_CERT_DIR + value: /tmp/alloc-api-serving-certs image: ghcr.io/playfab/thundernetes-operator:0.6.0 imagePullPolicy: IfNotPresent livenessProbe: @@ -17690,6 +17692,9 @@ spec: securityContext: allowPrivilegeEscalation: false volumeMounts: + - mountPath: /tmp/alloc-api-serving-certs + name: alloc-api-cert + readOnly: true - mountPath: /tmp/k8s-webhook-server/serving-certs name: cert readOnly: true @@ -17700,6 +17705,11 @@ spec: serviceAccountName: thundernetes-controller-manager terminationGracePeriodSeconds: 10 volumes: + - name: alloc-api-cert + secret: + defaultMode: 420 + optional: true + secretName: tls-secret - name: cert secret: defaultMode: 420 diff --git a/installfiles/operator_with_monitoring.yaml b/installfiles/operator_with_monitoring.yaml index 1acf1fbd..c578ae17 100644 --- a/installfiles/operator_with_monitoring.yaml +++ b/installfiles/operator_with_monitoring.yaml @@ -17658,6 +17658,8 @@ spec: valueFrom: fieldRef: fieldPath: metadata.namespace + - name: TLS_CERT_DIR + value: /tmp/alloc-api-serving-certs image: ghcr.io/playfab/thundernetes-operator:0.6.0 imagePullPolicy: IfNotPresent livenessProbe: @@ -17690,6 +17692,9 @@ spec: securityContext: allowPrivilegeEscalation: false volumeMounts: + - mountPath: /tmp/alloc-api-serving-certs + name: alloc-api-cert + readOnly: true - mountPath: /tmp/k8s-webhook-server/serving-certs name: cert readOnly: true @@ -17700,6 +17705,11 @@ spec: serviceAccountName: thundernetes-controller-manager terminationGracePeriodSeconds: 10 volumes: + - name: alloc-api-cert + secret: + defaultMode: 420 + optional: true + secretName: tls-secret - name: cert secret: defaultMode: 420 diff --git a/installfiles/operator_with_security.yaml b/installfiles/operator_with_security.yaml index 5a2a2980..271d5572 100644 --- a/installfiles/operator_with_security.yaml +++ b/installfiles/operator_with_security.yaml @@ -17658,6 +17658,8 @@ spec: valueFrom: fieldRef: fieldPath: metadata.namespace + - name: TLS_CERT_DIR + value: /tmp/alloc-api-serving-certs image: ghcr.io/playfab/thundernetes-operator:0.6.0 imagePullPolicy: IfNotPresent livenessProbe: @@ -17690,6 +17692,9 @@ spec: securityContext: allowPrivilegeEscalation: false volumeMounts: + - mountPath: /tmp/alloc-api-serving-certs + name: alloc-api-cert + readOnly: true - mountPath: /tmp/k8s-webhook-server/serving-certs name: cert readOnly: true @@ -17700,6 +17705,11 @@ spec: serviceAccountName: thundernetes-controller-manager terminationGracePeriodSeconds: 10 volumes: + - name: alloc-api-cert + secret: + defaultMode: 420 + optional: true + secretName: tls-secret - name: cert secret: defaultMode: 420 diff --git a/installfiles/operator_with_security_and_monitoring.yaml b/installfiles/operator_with_security_and_monitoring.yaml index 5a2a2980..271d5572 100644 --- a/installfiles/operator_with_security_and_monitoring.yaml +++ b/installfiles/operator_with_security_and_monitoring.yaml @@ -17658,6 +17658,8 @@ spec: valueFrom: fieldRef: fieldPath: metadata.namespace + - name: TLS_CERT_DIR + value: /tmp/alloc-api-serving-certs image: ghcr.io/playfab/thundernetes-operator:0.6.0 imagePullPolicy: IfNotPresent livenessProbe: @@ -17690,6 +17692,9 @@ spec: securityContext: allowPrivilegeEscalation: false volumeMounts: + - mountPath: /tmp/alloc-api-serving-certs + name: alloc-api-cert + readOnly: true - mountPath: /tmp/k8s-webhook-server/serving-certs name: cert readOnly: true @@ -17700,6 +17705,11 @@ spec: serviceAccountName: thundernetes-controller-manager terminationGracePeriodSeconds: 10 volumes: + - name: alloc-api-cert + secret: + defaultMode: 420 + optional: true + secretName: tls-secret - name: cert secret: defaultMode: 420 diff --git a/pkg/operator/config/default/kustomization.yaml b/pkg/operator/config/default/kustomization.yaml index f8ac8dea..9c43b11a 100644 --- a/pkg/operator/config/default/kustomization.yaml +++ b/pkg/operator/config/default/kustomization.yaml @@ -38,6 +38,10 @@ patchesStrategicMerge: # crd/kustomization.yaml - manager_webhook_patch.yaml +# [TLS] Mount the tls-secret for the allocation API service cert rotation +# The secret is optional so the pod starts even without TLS configured +- manager_alloc_tls_patch.yaml + # [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'. # Uncomment 'CERTMANAGER' sections in crd/kustomization.yaml to enable the CA injection in the admission webhooks. # 'CERTMANAGER' needs to be enabled to use ca injection diff --git a/pkg/operator/config/default/manager_alloc_tls_patch.yaml b/pkg/operator/config/default/manager_alloc_tls_patch.yaml new file mode 100644 index 00000000..d93e43c4 --- /dev/null +++ b/pkg/operator/config/default/manager_alloc_tls_patch.yaml @@ -0,0 +1,20 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: controller-manager + namespace: system +spec: + template: + spec: + containers: + - name: manager + volumeMounts: + - mountPath: /tmp/alloc-api-serving-certs + name: alloc-api-cert + readOnly: true + volumes: + - name: alloc-api-cert + secret: + defaultMode: 420 + secretName: tls-secret + optional: true diff --git a/pkg/operator/config/manager/manager.yaml b/pkg/operator/config/manager/manager.yaml index bedd11e1..34bb860c 100644 --- a/pkg/operator/config/manager/manager.yaml +++ b/pkg/operator/config/manager/manager.yaml @@ -54,6 +54,8 @@ spec: valueFrom: fieldRef: fieldPath: metadata.namespace + - name: TLS_CERT_DIR + value: "/tmp/alloc-api-serving-certs" name: manager securityContext: allowPrivilegeEscalation: false diff --git a/pkg/operator/controllers/allocation_api_server.go b/pkg/operator/controllers/allocation_api_server.go index 97ea638d..dada3c3b 100644 --- a/pkg/operator/controllers/allocation_api_server.go +++ b/pkg/operator/controllers/allocation_api_server.go @@ -3,7 +3,6 @@ package controllers import ( "context" "crypto/tls" - "crypto/x509" "encoding/json" "errors" "fmt" @@ -38,10 +37,9 @@ const ( // so it can be added to our Manager type AllocationApiServer struct { Client client.Client - // CrtBytes is the PEM-encoded certificate - CrtBytes []byte - // KeyBytes is the PEM-encoded key - KeyBytes []byte + // certWatcher watches and reloads TLS certificates from disk for dynamic cert rotation. + // If nil, the server runs without TLS. + certWatcher *CertificateWatcher // gameServerQueue is a map of priority queues for game servers gameServerQueue *GameServersQueue // events is a buffered channel of GenericEvent @@ -51,10 +49,9 @@ type AllocationApiServer struct { listeningPort int32 } -func NewAllocationApiServer(crt, key []byte, cl client.Client, port int32) *AllocationApiServer { +func NewAllocationApiServer(certWatcher *CertificateWatcher, cl client.Client, port int32) *AllocationApiServer { return &AllocationApiServer{ - CrtBytes: crt, - KeyBytes: key, + certWatcher: certWatcher, Client: cl, events: make(chan event.GenericEvent, 100), logger: log.Log.WithName("allocation-api"), @@ -103,25 +100,19 @@ func (s *AllocationApiServer) Start(ctx context.Context) error { close(done) }() - if s.CrtBytes != nil && s.KeyBytes != nil { - s.logger.Info("starting TLS enabled allocation API service") - // Generate a key pair from your pem-encoded cert and key ([]byte). - cert, err := tls.X509KeyPair(s.CrtBytes, s.KeyBytes) - if err != nil { - return nil - } - caCertPool := x509.NewCertPool() - caCertPool.AppendCertsFromPEM(s.CrtBytes) - // Construct a tls.config + if s.certWatcher != nil { + s.logger.Info("starting TLS enabled allocation API service with dynamic certificate rotation") + // Use dynamic TLS configuration via CertificateWatcher callbacks + // This enables automatic certificate rotation without server restart tlsConfig := &tls.Config{ - Certificates: []tls.Certificate{cert}, - ClientCAs: caCertPool, - ClientAuth: tls.RequireAndVerifyClientCert, + GetCertificate: s.certWatcher.GetCertificate, + GetConfigForClient: s.certWatcher.GetConfigForClient, + ClientAuth: tls.RequireAndVerifyClientCert, } // Build a server: srv.TLSConfig = tlsConfig - // Finally: serve. + // Finally: serve. Empty strings because certs are provided via GetCertificate callback. if err := srv.ListenAndServeTLS("", ""); err != nil && err != http.ErrServerClosed { return err } diff --git a/pkg/operator/controllers/allocation_api_server_test.go b/pkg/operator/controllers/allocation_api_server_test.go index 47483fac..11177ed8 100644 --- a/pkg/operator/controllers/allocation_api_server_test.go +++ b/pkg/operator/controllers/allocation_api_server_test.go @@ -27,7 +27,7 @@ var _ = Describe("allocation API service input validation tests", func() { It("empty body should return error", func() { req := httptest.NewRequest(http.MethodPost, "/api/v1/allocate", nil) w := httptest.NewRecorder() - h := NewAllocationApiServer(nil, nil, nil, allocationApiSvcPort) + h := NewAllocationApiServer(nil, nil, allocationApiSvcPort) h.handleAllocationRequest(w, req) res := w.Result() defer res.Body.Close() @@ -38,7 +38,7 @@ var _ = Describe("allocation API service input validation tests", func() { It("GET method should return error", func() { req := httptest.NewRequest(http.MethodGet, "/api/v1/allocate", nil) w := httptest.NewRecorder() - h := NewAllocationApiServer(nil, nil, nil, allocationApiSvcPort) + h := NewAllocationApiServer(nil, nil, allocationApiSvcPort) h.handleAllocationRequest(w, req) res := w.Result() defer res.Body.Close() @@ -49,7 +49,7 @@ var _ = Describe("allocation API service input validation tests", func() { It("bad body should return error", func() { req := httptest.NewRequest(http.MethodPost, "/api/v1/allocate", bytes.NewBufferString("{\"foo\":\"bar\"}")) w := httptest.NewRecorder() - h := NewAllocationApiServer(nil, nil, nil, allocationApiSvcPort) + h := NewAllocationApiServer(nil, nil, allocationApiSvcPort) h.handleAllocationRequest(w, req) res := w.Result() defer res.Body.Close() @@ -60,7 +60,7 @@ var _ = Describe("allocation API service input validation tests", func() { It("buildID should be a GUID", func() { req := httptest.NewRequest(http.MethodPost, "/api/v1/allocate", bytes.NewBufferString("{\"buildID\":\"NOT_A_GUID\",\"sessionID\":\"9bb3bbb2-5031-42fd-8982-5a3f76ef2c8a\"}")) w := httptest.NewRecorder() - h := NewAllocationApiServer(nil, nil, nil, allocationApiSvcPort) + h := NewAllocationApiServer(nil, nil, allocationApiSvcPort) h.handleAllocationRequest(w, req) res := w.Result() defer res.Body.Close() @@ -71,7 +71,7 @@ var _ = Describe("allocation API service input validation tests", func() { It("should return NotFound on an empty list", func() { req := httptest.NewRequest(http.MethodPost, "/api/v1/allocate", bytes.NewBufferString("{\"sessionID\":\"9bb3bbb2-5031-42fd-8982-5a3f76ef2c8a\",\"buildID\":\"9bb3bbb2-5031-42fd-8982-5a3f76ef2c8a\"}")) w := httptest.NewRecorder() - h := NewAllocationApiServer(nil, nil, testNewSimpleK8sClient(), allocationApiSvcPort) + h := NewAllocationApiServer(nil, testNewSimpleK8sClient(), allocationApiSvcPort) h.handleAllocationRequest(w, req) res := w.Result() defer res.Body.Close() @@ -85,7 +85,7 @@ var _ = Describe("allocation API service input validation tests", func() { Expect(err).ToNot(HaveOccurred()) req := httptest.NewRequest(http.MethodPost, "/api/v1/allocate", bytes.NewBufferString(fmt.Sprintf("{\"sessionID\":\"%s\",\"buildID\":\"%s\"}", sessionID1, buildID1))) w := httptest.NewRecorder() - h := NewAllocationApiServer(nil, nil, client, allocationApiSvcPort) + h := NewAllocationApiServer(nil, client, allocationApiSvcPort) h.handleAllocationRequest(w, req) res := w.Result() defer res.Body.Close() @@ -105,7 +105,7 @@ var _ = Describe("allocation API service input validation tests", func() { Expect(err).ToNot(HaveOccurred()) req := httptest.NewRequest(http.MethodPost, "/api/v1/allocate", bytes.NewBufferString(fmt.Sprintf("{\"sessionID\":\"%s\",\"buildID\":\"%s\"}", sessionID1, buildID1))) w := httptest.NewRecorder() - h := NewAllocationApiServer(nil, nil, client, allocationApiSvcPort) + h := NewAllocationApiServer(nil, client, allocationApiSvcPort) h.gameServerQueue = NewGameServersQueue() h.gameServerQueue.PushToQueue(&GameServerForQueue{ Name: gsName, diff --git a/pkg/operator/controllers/cert_watcher.go b/pkg/operator/controllers/cert_watcher.go new file mode 100644 index 00000000..b7a23a9e --- /dev/null +++ b/pkg/operator/controllers/cert_watcher.go @@ -0,0 +1,152 @@ +package controllers + +import ( + "context" + "crypto/tls" + "crypto/x509" + "fmt" + "os" + "sync" + "time" + + "github.com/go-logr/logr" + "sigs.k8s.io/controller-runtime/pkg/log" +) + +const ( + // defaultCertPollInterval is how often the CertificateWatcher checks for cert file changes + defaultCertPollInterval = 30 * time.Second +) + +// CertificateWatcher watches certificate files on disk and reloads them when they change. +// It is designed to work with Kubernetes Secret volume mounts, where kubelet atomically +// rotates the files via symlink swaps. +// CertificateWatcher implements the manager.Runnable interface so it can be added to the controller manager. +type CertificateWatcher struct { + mu sync.RWMutex + currentCert *tls.Certificate + caCertPool *x509.CertPool + certPath string + keyPath string + pollInterval time.Duration + logger logr.Logger +} + +// NewCertificateWatcher creates a new CertificateWatcher that monitors the given cert and key files. +func NewCertificateWatcher(certPath, keyPath string) *CertificateWatcher { + return &CertificateWatcher{ + certPath: certPath, + keyPath: keyPath, + pollInterval: defaultCertPollInterval, + logger: log.Log.WithName("cert-watcher"), + } +} + +// SetPollInterval sets the polling interval for file change detection. +// Must be called before Start(). +func (cw *CertificateWatcher) SetPollInterval(d time.Duration) { + cw.pollInterval = d +} + +// LoadCertificate reads and parses the certificate and key from disk. +// It is safe to call concurrently. +func (cw *CertificateWatcher) LoadCertificate() error { + certPEM, err := os.ReadFile(cw.certPath) + if err != nil { + return fmt.Errorf("reading certificate file %s: %w", cw.certPath, err) + } + keyPEM, err := os.ReadFile(cw.keyPath) + if err != nil { + return fmt.Errorf("reading key file %s: %w", cw.keyPath, err) + } + + cert, err := tls.X509KeyPair(certPEM, keyPEM) + if err != nil { + return fmt.Errorf("parsing TLS key pair: %w", err) + } + + caCertPool := x509.NewCertPool() + caCertPool.AppendCertsFromPEM(certPEM) + + cw.mu.Lock() + cw.currentCert = &cert + cw.caCertPool = caCertPool + cw.mu.Unlock() + + cw.logger.Info("loaded TLS certificate", "certPath", cw.certPath, "keyPath", cw.keyPath) + return nil +} + +// GetCertificate returns the current TLS certificate. +// It is intended to be used as the tls.Config.GetCertificate callback. +func (cw *CertificateWatcher) GetCertificate(_ *tls.ClientHelloInfo) (*tls.Certificate, error) { + cw.mu.RLock() + defer cw.mu.RUnlock() + if cw.currentCert == nil { + return nil, fmt.Errorf("no certificate loaded") + } + return cw.currentCert, nil +} + +// GetConfigForClient returns a tls.Config with the current CA cert pool for client certificate verification. +// It is intended to be used as the tls.Config.GetConfigForClient callback. +func (cw *CertificateWatcher) GetConfigForClient(_ *tls.ClientHelloInfo) (*tls.Config, error) { + cw.mu.RLock() + defer cw.mu.RUnlock() + if cw.caCertPool == nil { + return nil, fmt.Errorf("no CA certificate pool loaded") + } + return &tls.Config{ + ClientCAs: cw.caCertPool, + ClientAuth: tls.RequireAndVerifyClientCert, + GetCertificate: cw.GetCertificate, + GetConfigForClient: nil, // avoid recursive call + }, nil +} + +// Start implements the manager.Runnable interface. +// It polls the cert files for changes and reloads them when modified. +func (cw *CertificateWatcher) Start(ctx context.Context) error { + cw.logger.Info("starting certificate watcher", "certPath", cw.certPath, "keyPath", cw.keyPath, "pollInterval", cw.pollInterval) + + ticker := time.NewTicker(cw.pollInterval) + defer ticker.Stop() + + var lastCertModTime, lastKeyModTime time.Time + + // Initialize modification times + if info, err := os.Stat(cw.certPath); err == nil { + lastCertModTime = info.ModTime() + } + if info, err := os.Stat(cw.keyPath); err == nil { + lastKeyModTime = info.ModTime() + } + + for { + select { + case <-ctx.Done(): + cw.logger.Info("stopping certificate watcher") + return nil + case <-ticker.C: + changed := false + if info, err := os.Stat(cw.certPath); err == nil { + if !info.ModTime().Equal(lastCertModTime) { + lastCertModTime = info.ModTime() + changed = true + } + } + if info, err := os.Stat(cw.keyPath); err == nil { + if !info.ModTime().Equal(lastKeyModTime) { + lastKeyModTime = info.ModTime() + changed = true + } + } + if changed { + cw.logger.Info("certificate file change detected, reloading") + if err := cw.LoadCertificate(); err != nil { + cw.logger.Error(err, "failed to reload certificate, keeping previous certificate") + } + } + } + } +} diff --git a/pkg/operator/controllers/cert_watcher_test.go b/pkg/operator/controllers/cert_watcher_test.go new file mode 100644 index 00000000..13d47d76 --- /dev/null +++ b/pkg/operator/controllers/cert_watcher_test.go @@ -0,0 +1,311 @@ +package controllers + +import ( + "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "math/big" + "os" + "path/filepath" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// generateTestCert creates a self-signed certificate and returns PEM-encoded cert and key bytes. +func generateTestCert(t *testing.T, cn string) ([]byte, []byte) { + t.Helper() + + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + + template := x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: cn}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(24 * time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth}, + IsCA: true, + BasicConstraintsValid: true, + } + + certDER, err := x509.CreateCertificate(rand.Reader, &template, &template, &key.PublicKey, key) + require.NoError(t, err) + + certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certDER}) + + keyDER, err := x509.MarshalECPrivateKey(key) + require.NoError(t, err) + keyPEM := pem.EncodeToMemory(&pem.Block{Type: "EC PRIVATE KEY", Bytes: keyDER}) + + return certPEM, keyPEM +} + +// writeCertFiles writes cert and key PEM bytes to the specified directory. +func writeCertFiles(t *testing.T, dir string, certPEM, keyPEM []byte) { + t.Helper() + require.NoError(t, os.WriteFile(filepath.Join(dir, "tls.crt"), certPEM, 0600)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "tls.key"), keyPEM, 0600)) +} + +func TestCertificateWatcher_InitialLoad(t *testing.T) { + dir := t.TempDir() + certPEM, keyPEM := generateTestCert(t, "initial-load-test") + writeCertFiles(t, dir, certPEM, keyPEM) + + cw := NewCertificateWatcher( + filepath.Join(dir, "tls.crt"), + filepath.Join(dir, "tls.key"), + ) + + err := cw.LoadCertificate() + require.NoError(t, err) + + // Verify GetCertificate returns a valid cert + cert, err := cw.GetCertificate(nil) + require.NoError(t, err) + assert.NotNil(t, cert) + + // Verify GetConfigForClient returns a valid config + cfg, err := cw.GetConfigForClient(nil) + require.NoError(t, err) + assert.NotNil(t, cfg) + assert.Equal(t, tls.RequireAndVerifyClientCert, cfg.ClientAuth) + assert.NotNil(t, cfg.ClientCAs) +} + +func TestCertificateWatcher_GetCertificateBeforeLoad(t *testing.T) { + cw := NewCertificateWatcher("/nonexistent/tls.crt", "/nonexistent/tls.key") + + // GetCertificate should return an error if no cert has been loaded + cert, err := cw.GetCertificate(nil) + assert.Error(t, err) + assert.Nil(t, cert) + + // GetConfigForClient should also return an error + cfg, err := cw.GetConfigForClient(nil) + assert.Error(t, err) + assert.Nil(t, cfg) +} + +func TestCertificateWatcher_MissingFiles(t *testing.T) { + cw := NewCertificateWatcher("/nonexistent/tls.crt", "/nonexistent/tls.key") + + err := cw.LoadCertificate() + assert.Error(t, err) + assert.Contains(t, err.Error(), "reading certificate file") +} + +func TestCertificateWatcher_MissingKeyFile(t *testing.T) { + dir := t.TempDir() + certPEM, _ := generateTestCert(t, "missing-key-test") + require.NoError(t, os.WriteFile(filepath.Join(dir, "tls.crt"), certPEM, 0600)) + + cw := NewCertificateWatcher( + filepath.Join(dir, "tls.crt"), + filepath.Join(dir, "tls.key"), // does not exist + ) + + err := cw.LoadCertificate() + assert.Error(t, err) + assert.Contains(t, err.Error(), "reading key file") +} + +func TestCertificateWatcher_InvalidCertContent(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "tls.crt"), []byte("not a cert"), 0600)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "tls.key"), []byte("not a key"), 0600)) + + cw := NewCertificateWatcher( + filepath.Join(dir, "tls.crt"), + filepath.Join(dir, "tls.key"), + ) + + err := cw.LoadCertificate() + assert.Error(t, err) + assert.Contains(t, err.Error(), "parsing TLS key pair") +} + +func TestCertificateWatcher_Reload(t *testing.T) { + dir := t.TempDir() + + // Write initial cert + certPEM1, keyPEM1 := generateTestCert(t, "cert-v1") + writeCertFiles(t, dir, certPEM1, keyPEM1) + + cw := NewCertificateWatcher( + filepath.Join(dir, "tls.crt"), + filepath.Join(dir, "tls.key"), + ) + + err := cw.LoadCertificate() + require.NoError(t, err) + + cert1, err := cw.GetCertificate(nil) + require.NoError(t, err) + + // Write a new cert (different CN) + certPEM2, keyPEM2 := generateTestCert(t, "cert-v2") + writeCertFiles(t, dir, certPEM2, keyPEM2) + + // Reload + err = cw.LoadCertificate() + require.NoError(t, err) + + cert2, err := cw.GetCertificate(nil) + require.NoError(t, err) + + // The two certs should be different + assert.NotEqual(t, cert1.Certificate, cert2.Certificate) +} + +func TestCertificateWatcher_StartDetectsFileChange(t *testing.T) { + dir := t.TempDir() + + // Write initial cert + certPEM1, keyPEM1 := generateTestCert(t, "start-test-v1") + writeCertFiles(t, dir, certPEM1, keyPEM1) + + cw := NewCertificateWatcher( + filepath.Join(dir, "tls.crt"), + filepath.Join(dir, "tls.key"), + ) + cw.SetPollInterval(50 * time.Millisecond) // fast polling for tests + + err := cw.LoadCertificate() + require.NoError(t, err) + + cert1, err := cw.GetCertificate(nil) + require.NoError(t, err) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + // Start the watcher in a goroutine + go func() { + _ = cw.Start(ctx) + }() + + // Wait a bit, then write a new cert + time.Sleep(100 * time.Millisecond) + certPEM2, keyPEM2 := generateTestCert(t, "start-test-v2") + writeCertFiles(t, dir, certPEM2, keyPEM2) + + // Wait for the watcher to detect the change + assert.Eventually(t, func() bool { + cert2, err := cw.GetCertificate(nil) + if err != nil { + return false + } + return !certsEqual(cert1, cert2) + }, 2*time.Second, 50*time.Millisecond, "watcher should detect cert file change and reload") + + cancel() +} + +func TestCertificateWatcher_ConcurrentAccess(t *testing.T) { + dir := t.TempDir() + certPEM, keyPEM := generateTestCert(t, "concurrent-test") + writeCertFiles(t, dir, certPEM, keyPEM) + + cw := NewCertificateWatcher( + filepath.Join(dir, "tls.crt"), + filepath.Join(dir, "tls.key"), + ) + + err := cw.LoadCertificate() + require.NoError(t, err) + + var wg sync.WaitGroup + const concurrency = 50 + + // Multiple readers + for i := 0; i < concurrency; i++ { + wg.Add(1) + go func() { + defer wg.Done() + for j := 0; j < 100; j++ { + cert, err := cw.GetCertificate(nil) + assert.NoError(t, err) + assert.NotNil(t, cert) + + cfg, err := cw.GetConfigForClient(nil) + assert.NoError(t, err) + assert.NotNil(t, cfg) + } + }() + } + + // Concurrent reloaders + for i := 0; i < 5; i++ { + wg.Add(1) + go func() { + defer wg.Done() + for j := 0; j < 20; j++ { + err := cw.LoadCertificate() + assert.NoError(t, err) + } + }() + } + + wg.Wait() +} + +func TestCertificateWatcher_ReloadKeepsPreviousOnError(t *testing.T) { + dir := t.TempDir() + + // Write valid cert + certPEM, keyPEM := generateTestCert(t, "keep-previous-test") + writeCertFiles(t, dir, certPEM, keyPEM) + + cw := NewCertificateWatcher( + filepath.Join(dir, "tls.crt"), + filepath.Join(dir, "tls.key"), + ) + + err := cw.LoadCertificate() + require.NoError(t, err) + + cert1, err := cw.GetCertificate(nil) + require.NoError(t, err) + + // Now write invalid cert content + require.NoError(t, os.WriteFile(filepath.Join(dir, "tls.crt"), []byte("invalid"), 0600)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "tls.key"), []byte("invalid"), 0600)) + + // LoadCertificate should fail + err = cw.LoadCertificate() + assert.Error(t, err) + + // Previous cert should still be available + cert2, err := cw.GetCertificate(nil) + require.NoError(t, err) + assert.Equal(t, cert1.Certificate, cert2.Certificate) +} + +// certsEqual checks if two TLS certificates have the same raw certificate data. +func certsEqual(a, b *tls.Certificate) bool { + if len(a.Certificate) != len(b.Certificate) { + return false + } + for i := range a.Certificate { + if len(a.Certificate[i]) != len(b.Certificate[i]) { + return false + } + for j := range a.Certificate[i] { + if a.Certificate[i][j] != b.Certificate[i][j] { + return false + } + } + } + return true +} diff --git a/pkg/operator/controllers/config.go b/pkg/operator/controllers/config.go index 2431e3eb..26842596 100644 --- a/pkg/operator/controllers/config.go +++ b/pkg/operator/controllers/config.go @@ -8,6 +8,7 @@ type Config struct { TlsSecretNamespace string `env:"TLS_SECRET_NAMESPACE" envDefault:"thundernetes-system"` TlsCertificateName string `env:"TLS_CERTIFICATE_FILENAME" envDefault:"tls.crt"` TlsPrivateKeyFilename string `env:"TLS_PRIVATE_KEY_FILENAME" envDefault:"tls.key"` + TlsCertDir string `env:"TLS_CERT_DIR" envDefault:"/tmp/alloc-api-serving-certs"` PortRegistryExclusivelyGameServerNodes bool `env:"PORT_REGISTRY_EXCLUSIVELY_GAME_SERVER_NODES" envDefault:"false"` LogLevel string `env:"LOG_LEVEL" envDefault:"info"` MinPort int32 `env:"MIN_PORT" envDefault:"10000"` diff --git a/pkg/operator/controllers/port_registry_test.go b/pkg/operator/controllers/port_registry_test.go index 03b762b9..b81bb79a 100644 --- a/pkg/operator/controllers/port_registry_test.go +++ b/pkg/operator/controllers/port_registry_test.go @@ -433,7 +433,8 @@ var _ = Describe("Random port registration on port registry with two thousand po var gameServerName string for { // make sure we don't register the same GameServer twice gameServerName = generateName(prefix) - if _, ok := gameServerNamesAndPorts.Load(gameServerName); !ok { + // use LoadOrStore to atomically claim the name, avoiding TOCTOU races + if _, loaded := gameServerNamesAndPorts.LoadOrStore(gameServerName, int32(0)); !loaded { break } } @@ -494,7 +495,14 @@ var _ = Describe("Random port registration on port registry with two thousand po defer wg.Done() n := rand.Intn(200) + 50 // n will be between 50 and 250 time.Sleep(time.Duration(n) * time.Millisecond) - gameServerName := generateName(prefix) + var gameServerName string + for { // make sure we don't register the same GameServer twice + gameServerName = generateName(prefix) + // use LoadOrStore to atomically claim the name, avoiding TOCTOU races + if _, loaded := gameServerNamesAndPorts.LoadOrStore(gameServerName, int32(0)); !loaded { + break + } + } ports, err := portRegistry.GetNewPorts(testnamespace, gameServerName, 1) Expect(err).ToNot(HaveOccurred()) Expect(len(ports)).To(Equal(1)) diff --git a/pkg/operator/controllers/suite_test.go b/pkg/operator/controllers/suite_test.go index 8f83feb4..3860976e 100644 --- a/pkg/operator/controllers/suite_test.go +++ b/pkg/operator/controllers/suite_test.go @@ -126,7 +126,7 @@ var _ = BeforeSuite(func() { Expect(err).ToNot(HaveOccurred()) // allocation api service is a controller, so add it to the manager - testAllocationApiServer = NewAllocationApiServer(nil, nil, k8sManager.GetClient(), allocationApiSvcPort) + testAllocationApiServer = NewAllocationApiServer(nil, k8sManager.GetClient(), allocationApiSvcPort) err = testAllocationApiServer.SetupWithManager(k8sManager) Expect(err).ToNot(HaveOccurred()) diff --git a/pkg/operator/main.go b/pkg/operator/main.go index 269dee26..b44a3380 100644 --- a/pkg/operator/main.go +++ b/pkg/operator/main.go @@ -22,6 +22,7 @@ import ( "flag" "log" "os" + "path/filepath" "time" // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) @@ -31,7 +32,6 @@ import ( _ "go.uber.org/automaxprocs" "go.uber.org/zap/zapcore" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" _ "k8s.io/client-go/plugin/pkg/client/auth" @@ -105,13 +105,29 @@ func main() { setupLog.Error(err, "unable to start manager") os.Exit(1) } - // initialize a live API client, used for the PortRegistry and fetching the mTLS secret + // initialize a live API client, used for the PortRegistry k8sClient := mgr.GetAPIReader() - // get public and private key, if enabled - crt, key := getCrtKeyIfTlsEnabled(k8sClient, cfg) + + // initialize the certificate watcher for the allocation API service, if TLS is enabled + var certWatcher *controllers.CertificateWatcher + if cfg.ApiServiceSecurity == "usetls" { + certPath := filepath.Join(cfg.TlsCertDir, cfg.TlsCertificateName) + keyPath := filepath.Join(cfg.TlsCertDir, cfg.TlsPrivateKeyFilename) + certWatcher = controllers.NewCertificateWatcher(certPath, keyPath) + // load the initial certificate to fail fast if the cert files are missing or invalid + if err := certWatcher.LoadCertificate(); err != nil { + setupLog.Error(err, "unable to load initial TLS certificate for allocation API") + os.Exit(1) + } + // add the cert watcher as a runnable so it polls for cert changes alongside the manager + if err := mgr.Add(certWatcher); err != nil { + setupLog.Error(err, "unable to add certificate watcher to manager") + os.Exit(1) + } + } // initialize the allocation API service, which is also a controller. So we add it to the manager - aas := controllers.NewAllocationApiServer(crt, key, mgr.GetClient(), int32(allocationApiSvcPort)) + aas := controllers.NewAllocationApiServer(certWatcher, mgr.GetClient(), int32(allocationApiSvcPort)) if err = aas.SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create HTTP allocation API Server", "Allocation API Server", "HTTP Allocation API Server") os.Exit(1) @@ -211,20 +227,6 @@ func initializePortRegistry(k8sClient client.Reader, crClient client.Client, set return portRegistry, nil } -// getTlsSecret returns the TLS secret from the given namespace -// used in the allocation API service -func getTlsSecret(k8sClient client.Reader, cfg *controllers.Config) ([]byte, []byte, error) { - var secret corev1.Secret - err := k8sClient.Get(context.Background(), types.NamespacedName{ - Name: cfg.TlsSecretName, - Namespace: cfg.TlsSecretNamespace, - }, &secret) - if err != nil { - return nil, nil, err - } - return []byte(secret.Data[cfg.TlsCertificateName]), []byte(secret.Data[cfg.TlsPrivateKeyFilename]), nil -} - // validateMinMaxPort validates minimum and maximum ports func validateMinMaxPort(cfg *controllers.Config) (int32, int32, error) { if cfg.MinPort >= cfg.MaxPort { @@ -253,19 +255,3 @@ func getLogLevel(logLevel string) zapcore.LevelEnabler { return zapcore.InfoLevel } } - -// getCrtKeyIfTlsEnabled returns public and private key components for securing the allocation API service with mTLS -// for this to happen, user has to set "API_SERVICE_SECURITY" env as "usetls" and set the env "TLS_SECRET_NAMESPACE" with the namespace -// that contains the Kubernetes Secret with the cert -// if any of the mentioned conditions are not set, method returns nil -func getCrtKeyIfTlsEnabled(c client.Reader, cfg *controllers.Config) ([]byte, []byte) { - if cfg.ApiServiceSecurity == "usetls" { - crt, key, err := getTlsSecret(c, cfg) - if err != nil { - setupLog.Error(err, "unable to get TLS secret") - os.Exit(1) - } - return crt, key - } - return nil, nil -}