Watch for Secrets (draft)

internal/events/handler.go

@@ -5,17 +5,12 @@ import (
 	"fmt"
 
 	"github.com/go-logr/logr"
-	apiv1 "k8s.io/api/core/v1"
-	discoveryV1 "k8s.io/api/discovery/v1"
-	"sigs.k8s.io/gateway-api/apis/v1beta1"
-
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/config"
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/file"
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/runtime"
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/state"
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/state/dataplane"
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/state/resolver"
-	"github.com/nginxinc/nginx-kubernetes-gateway/internal/state/secrets"
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/status"
 )
 
@@ -32,10 +27,6 @@ type EventHandler interface {
 type EventHandlerConfig struct {
 	// Processor is the state ChangeProcessor.
 	Processor state.ChangeProcessor
-	// SecretStore is the state SecretStore.
-	SecretStore secrets.SecretStore
-	// SecretMemoryManager is the state SecretMemoryManager.
-	SecretMemoryManager secrets.SecretDiskMemoryManager
 	// ServiceResolver resolves Services to Endpoints.
 	ServiceResolver resolver.ServiceResolver
 	// Generator is the nginx config Generator.
@@ -69,9 +60,9 @@ func (h *EventHandlerImpl) HandleEventBatch(ctx context.Context, batch EventBatc
 	for _, event := range batch {
 		switch e := event.(type) {
 		case *UpsertEvent:
-			h.propagateUpsert(e)
+			h.cfg.Processor.CaptureUpsertChange(e.Resource)
 		case *DeleteEvent:
-			h.propagateDelete(e)
+			h.cfg.Processor.CaptureDeleteChange(e.Type, e.NamespacedName)
 		default:
 			panic(fmt.Errorf("unknown event type %T", e))
 		}
@@ -96,70 +87,12 @@ func (h *EventHandlerImpl) HandleEventBatch(ctx context.Context, batch EventBatc
 }
 
 func (h *EventHandlerImpl) updateNginx(ctx context.Context, conf dataplane.Configuration) error {
-	// Write all secrets (nuke and pave).
-	// This will remove all secrets in the secrets directory before writing the requested secrets.
-	// FIXME(kate-osborn): We may want to rethink this approach in the future and write and remove secrets individually.
-	// https://github.com/nginxinc/nginx-kubernetes-gateway/issues/561
-	err := h.cfg.SecretMemoryManager.WriteAllRequestedSecrets()
-	if err != nil {
-		return err
-	}
-
-	cfg := h.cfg.Generator.Generate(conf)
+	files := h.cfg.Generator.Generate(conf)
 
-	// For now, we keep all http servers and upstreams in one config file.
-	// We might rethink that. For example, we can write each server to its file
-	// or group servers in some way.
-	err = h.cfg.NginxFileMgr.WriteHTTPConfig("http", cfg)
+	err := h.cfg.NginxFileMgr.ReplaceFiles(files)
 	if err != nil {
 		return err
 	}
 
 	return h.cfg.NginxRuntimeMgr.Reload(ctx)
 }
-
-func (h *EventHandlerImpl) propagateUpsert(e *UpsertEvent) {
-	switch r := e.Resource.(type) {
-	case *v1beta1.GatewayClass:
-		h.cfg.Processor.CaptureUpsertChange(r)
-	case *v1beta1.Gateway:
-		h.cfg.Processor.CaptureUpsertChange(r)
-	case *v1beta1.HTTPRoute:
-		h.cfg.Processor.CaptureUpsertChange(r)
-	case *apiv1.Service:
-		h.cfg.Processor.CaptureUpsertChange(r)
-	case *apiv1.Namespace:
-		h.cfg.Processor.CaptureUpsertChange(r)
-	case *apiv1.Secret:
-		// FIXME(kate-osborn): need to handle certificate rotation
-		// https://github.com/nginxinc/nginx-kubernetes-gateway/issues/553
-		h.cfg.SecretStore.Upsert(r)
-	case *discoveryV1.EndpointSlice:
-		h.cfg.Processor.CaptureUpsertChange(r)
-	default:
-		panic(fmt.Errorf("unknown resource type %T", e.Resource))
-	}
-}
-
-func (h *EventHandlerImpl) propagateDelete(e *DeleteEvent) {
-	switch e.Type.(type) {
-	case *v1beta1.GatewayClass:
-		h.cfg.Processor.CaptureDeleteChange(e.Type, e.NamespacedName)
-	case *v1beta1.Gateway:
-		h.cfg.Processor.CaptureDeleteChange(e.Type, e.NamespacedName)
-	case *v1beta1.HTTPRoute:
-		h.cfg.Processor.CaptureDeleteChange(e.Type, e.NamespacedName)
-	case *apiv1.Service:
-		h.cfg.Processor.CaptureDeleteChange(e.Type, e.NamespacedName)
-	case *apiv1.Namespace:
-		h.cfg.Processor.CaptureDeleteChange(e.Type, e.NamespacedName)
-	case *apiv1.Secret:
-		// FIXME(kate-osborn): make sure that affected servers are updated
-		// https://github.com/nginxinc/nginx-kubernetes-gateway/issues/553
-		h.cfg.SecretStore.Delete(e.NamespacedName)
-	case *discoveryV1.EndpointSlice:
-		h.cfg.Processor.CaptureDeleteChange(e.Type, e.NamespacedName)
-	default:
-		panic(fmt.Errorf("unknown resource type %T", e.Type))
-	}
-}

internal/manager/manager.go

@@ -29,7 +29,6 @@ import (
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/state"
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/state/relationship"
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/state/resolver"
-	"github.com/nginxinc/nginx-kubernetes-gateway/internal/state/secrets"
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/state/validation"
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/status"
 )
@@ -129,16 +128,12 @@ func Start(cfg config.Config) error {
 		}
 	}
 
-	secretStore := secrets.NewSecretStore()
-	secretMemoryMgr := secrets.NewSecretDiskMemoryManager(secretsFolder, secretStore)
-
 	recorderName := fmt.Sprintf("nginx-kubernetes-gateway-%s", cfg.GatewayClassName)
 	recorder := mgr.GetEventRecorderFor(recorderName)
 
 	processor := state.NewChangeProcessorImpl(state.ChangeProcessorConfig{
 		GatewayCtlrName:      cfg.GatewayCtlrName,
 		GatewayClassName:     cfg.GatewayClassName,
-		SecretMemoryManager:  secretMemoryMgr,
 		RelationshipCapturer: relationship.NewCapturerImpl(),
 		Logger:               cfg.Logger.WithName("changeProcessor"),
 		Validators: validation.Validators{
@@ -162,15 +157,13 @@ func Start(cfg config.Config) error {
 	})
 
 	eventHandler := events.NewEventHandlerImpl(events.EventHandlerConfig{
-		Processor:           processor,
-		SecretStore:         secretStore,
-		SecretMemoryManager: secretMemoryMgr,
-		ServiceResolver:     resolver.NewServiceResolverImpl(mgr.GetClient()),
-		Generator:           configGenerator,
-		Logger:              cfg.Logger.WithName("eventHandler"),
-		NginxFileMgr:        nginxFileMgr,
-		NginxRuntimeMgr:     nginxRuntimeMgr,
-		StatusUpdater:       statusUpdater,
+		Processor:       processor,
+		ServiceResolver: resolver.NewServiceResolverImpl(mgr.GetClient()),
+		Generator:       configGenerator,
+		Logger:          cfg.Logger.WithName("eventHandler"),
+		NginxFileMgr:    nginxFileMgr,
+		NginxRuntimeMgr: nginxRuntimeMgr,
+		StatusUpdater:   statusUpdater,
 	})
 
 	objects, objectLists := prepareFirstEventBatchPreparerArgs(cfg.GatewayClassName, cfg.GatewayNsName)

internal/nginx/config/generator.go

@@ -1,6 +1,7 @@
 package config
 
 import (
+	"github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/file"
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/state/dataplane"
 )
 
@@ -10,7 +11,7 @@ import (
 // This interface is used for testing purposes only.
 type Generator interface {
 	// Generate generates NGINX configuration from internal representation.
-	Generate(configuration dataplane.Configuration) []byte
+	Generate(configuration dataplane.Configuration) []file.File
 }
 
 // GeneratorImpl is an implementation of Generator.
@@ -28,13 +29,39 @@ type executeFunc func(configuration dataplane.Configuration) []byte
 // It is the responsibility of the caller to validate the configuration before calling this function.
 // In case of invalid configuration, NGINX will fail to reload or could be configured with malicious configuration.
 // To validate, use the validators from the validation package.
-func (g GeneratorImpl) Generate(conf dataplane.Configuration) []byte {
+func (g GeneratorImpl) Generate(conf dataplane.Configuration) []file.File {
+	files := make([]file.File, 0, len(conf.TLSCerts)+1)
+
+	// certs
+	for id, cert := range conf.TLSCerts {
+		contents := make([]byte, 0, len(cert.Cert)+len(cert.Key)+1)
+		contents = append(contents, cert.Cert...)
+		contents = append(contents, '\n')
+		contents = append(contents, cert.Key...)
+
+		files = append(files, file.File{
+			Content:     contents,
+			Path:        generateTLSCertPath(id),
+			Permissions: 0600,
+		})
+	}
+
 	var generated []byte
 	for _, execute := range getExecuteFuncs() {
 		generated = append(generated, execute(conf)...)
 	}
 
-	return generated
+	files = append(files, file.File{
+		Content:     generated,
+		Path:        "/etc/nginx/conf.d/http.conf",
+		Permissions: 0644,
+	})
+
+	return files
+}
+
+func generateTLSCertPath(id dataplane.TLSCertID) string {
+	return "/etc/nginx/secrets/" + string(id) + ".pem"
 }
 
 func getExecuteFuncs() []executeFunc {

internal/nginx/config/servers.go

@@ -48,8 +48,8 @@ func createSSLServer(virtualServer dataplane.VirtualServer) http.Server {
 	return http.Server{
 		ServerName: virtualServer.Hostname,
 		SSL: &http.SSL{
-			Certificate:    virtualServer.SSL.CertificatePath,
-			CertificateKey: virtualServer.SSL.CertificatePath,
+			Certificate:    generateTLSCertPath(virtualServer.SSL.TLSCertID),
+			CertificateKey: generateTLSCertPath(virtualServer.SSL.TLSCertID),
 		},
 		Locations: createLocations(virtualServer.PathRules, 443),
 	}

internal/nginx/file/manager.go

@@ -3,47 +3,62 @@ package file
 import (
 	"fmt"
 	"os"
-	"path/filepath"
 )
 
 const confdFolder = "/etc/nginx/conf.d"
 
 //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . Manager
 
+type File struct {
+	Path        string
+	Content     []byte
+	Permissions os.FileMode
+}
+
 // Manager manages NGINX configuration files.
 type Manager interface {
-	// WriteHTTPConfig writes the http config on the file system.
-	// The name distinguishes this config among all other configs. For that, it must be unique.
-	// Note that name is not the name of the corresponding configuration file.
-	WriteHTTPConfig(name string, cfg []byte) error
+	// ReplaceFiles replaces the files on the file system with the given files.
+	ReplaceFiles(files []File) error
 }
 
 // ManagerImpl is an implementation of Manager.
-type ManagerImpl struct{}
+type ManagerImpl struct {
+	lastWrittenPaths []string
+}
 
 // NewManagerImpl creates a new NewManagerImpl.
 func NewManagerImpl() *ManagerImpl {
 	return &ManagerImpl{}
 }
 
-func (m *ManagerImpl) WriteHTTPConfig(name string, cfg []byte) error {
-	path := getPathForConfig(name)
-
-	file, err := os.Create(path)
-	if err != nil {
-		return fmt.Errorf("failed to create server config %s: %w", path, err)
+func (m *ManagerImpl) ReplaceFiles(files []File) error {
+	for _, path := range m.lastWrittenPaths {
+		err := os.Remove(path)
+		if err != nil {
+			return fmt.Errorf("failed to delete file %s: %w", path, err)
+		}
 	}
 
-	defer file.Close()
+	// In some cases, NGINX reads files in runtime, like JWT secrets
+	// In that case, removal will lead to errors.
+	// However, we don't have such files yet. so not a problem
+
+	m.lastWrittenPaths = make([]string, 0, len(files))
 
-	_, err = file.Write(cfg)
-	if err != nil {
-		return fmt.Errorf("failed to write server config %s: %w", path, err)
+	for _, file := range files {
+		f, err := os.Create(file.Path)
+		if err != nil {
+			return fmt.Errorf("failed to create server config %s: %w", file.Path, err)
+		}
+		defer f.Close()
+
+		_, err = f.Write(file.Content)
+		if err != nil {
+			return fmt.Errorf("failed to write server config %s: %w", file.Path, err)
+		}
+
+		m.lastWrittenPaths = append(m.lastWrittenPaths, file.Path)
 	}
 
 	return nil
 }
-
-func getPathForConfig(name string) string {
-	return filepath.Join(confdFolder, name+".conf")
-}