Move version state to event handler

Author: ciarams87

internal/mode/static/handler.go

@@ -45,6 +45,8 @@ type eventHandlerConfig struct {
 	logger logr.Logger
 	// controlConfigNSName is the NamespacedName of the NginxGateway config for this controller.
 	controlConfigNSName types.NamespacedName
+	// version is the current version number of the nginx config.
+	version int
 }
 
 // eventHandlerImpl implements EventHandler.
@@ -90,7 +92,11 @@ func (h *eventHandlerImpl) HandleEventBatch(ctx context.Context, batch events.Ev
 	}
 
 	var nginxReloadRes nginxReloadResult
-	if err := h.updateNginx(ctx, dataplane.BuildConfiguration(ctx, graph, h.cfg.serviceResolver)); err != nil {
+	h.cfg.version++
+	if err := h.updateNginx(
+		ctx,
+		dataplane.BuildConfiguration(ctx, graph, h.cfg.serviceResolver, h.cfg.version),
+	); err != nil {
 		h.cfg.logger.Error(err, "Failed to update NGINX configuration")
 		nginxReloadRes.error = err
 	} else {
@@ -100,7 +106,7 @@ func (h *eventHandlerImpl) HandleEventBatch(ctx context.Context, batch events.Ev
 	h.cfg.statusUpdater.Update(ctx, buildStatuses(graph, nginxReloadRes))
 }
 
-func (h *eventHandlerImpl) updateNginx(ctx context.Context, conf *dataplane.Configuration) error {
+func (h *eventHandlerImpl) updateNginx(ctx context.Context, conf dataplane.Configuration) error {
 	files := h.cfg.generator.Generate(conf)
 
 	if err := h.cfg.nginxFileMgr.ReplaceFiles(files); err != nil {

internal/mode/static/handler_test.go

@@ -45,7 +45,7 @@ var _ = Describe("eventHandler", func() {
 		Expect(fakeProcessor.ProcessCallCount()).Should(Equal(1))
 
 		Expect(fakeGenerator.GenerateCallCount()).Should(Equal(1))
-		Expect(fakeGenerator.GenerateArgsForCall(0)).Should(Equal(&expectedConf))
+		Expect(fakeGenerator.GenerateArgsForCall(0)).Should(Equal(expectedConf))
 
 		Expect(fakeNginxFileMgr.ReplaceFilesCallCount()).Should(Equal(1))
 		files := fakeNginxFileMgr.ReplaceFilesArgsForCall(0)
@@ -111,7 +111,7 @@ var _ = Describe("eventHandler", func() {
 				handler.HandleEventBatch(context.Background(), batch)
 
 				checkUpsertEventExpectations(e)
-				expectReconfig(dataplane.Configuration{}, fakeCfgFiles)
+				expectReconfig(dataplane.Configuration{Version: 1}, fakeCfgFiles)
 			})
 
 			It("should process Delete", func() {
@@ -124,7 +124,7 @@ var _ = Describe("eventHandler", func() {
 				handler.HandleEventBatch(context.Background(), batch)
 
 				checkDeleteEventExpectations(e)
-				expectReconfig(dataplane.Configuration{}, fakeCfgFiles)
+				expectReconfig(dataplane.Configuration{Version: 1}, fakeCfgFiles)
 			})
 		})
 

internal/mode/static/manager.go

@@ -212,7 +212,7 @@ func StartManager(cfg config.Config) error {
 	eventHandler := newEventHandlerImpl(eventHandlerConfig{
 		processor:           processor,
 		serviceResolver:     resolver.NewServiceResolverImpl(mgr.GetClient()),
-		generator:           &configGenerator,
+		generator:           configGenerator,
 		logger:              cfg.Logger.WithName("eventHandler"),
 		logLevelSetter:      logLevelSetter,
 		nginxFileMgr:        nginxFileMgr,

internal/mode/static/nginx/config/configfakes/fake_generator.go

@@ -32,7 +32,7 @@ var ConfigFolders = []string{httpFolder, secretsFolder}
 // This interface is used for testing purposes only.
 type Generator interface {
 	// Generate generates NGINX configuration files from internal representation.
-	Generate(configuration *dataplane.Configuration) []file.File
+	Generate(configuration dataplane.Configuration) []file.File
 }
 
 // GeneratorImpl is an implementation of Generator.
@@ -43,15 +43,11 @@ type Generator interface {
 //
 // It also expects that the main NGINX configuration file nginx.conf is located in configFolder and nginx.conf
 // includes (https://nginx.org/en/docs/ngx_core_module.html#include) the files from httpFolder.
-type GeneratorImpl struct {
-	configVersion int
-}
+type GeneratorImpl struct{}
 
 // NewGeneratorImpl creates a new GeneratorImpl.
 func NewGeneratorImpl() GeneratorImpl {
-	return GeneratorImpl{
-		configVersion: 0,
-	}
+	return GeneratorImpl{}
 }
 
 // executeFunc is a function that generates NGINX configuration from internal representation.
@@ -61,18 +57,16 @@ 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) []file.File {
-	g.configVersion++
-	conf.Version = g.configVersion
+func (g GeneratorImpl) Generate(conf dataplane.Configuration) []file.File {
 	files := make([]file.File, 0, len(conf.SSLKeyPairs)+1 /* http config */)
 
 	for id, pair := range conf.SSLKeyPairs {
 		files = append(files, generatePEM(id, pair.Cert, pair.Key))
 	}
 
-	files = append(files, generateHTTPConfig(*conf))
+	files = append(files, generateHTTPConfig(conf))
 
-	files = append(files, generateConfigVersion(g.configVersion))
+	files = append(files, generateConfigVersion(conf.Version))
 
 	return files
 }

internal/mode/static/nginx/config/generator.go

@@ -64,7 +64,7 @@ func TestGenerate(t *testing.T) {
 
 	generator := config.NewGeneratorImpl()
 
-	files := generator.Generate(&conf)
+	files := generator.Generate(conf)
 
 	g.Expect(files).To(HaveLen(3))
 

internal/mode/static/nginx/config/generator_test.go

@@ -16,13 +16,18 @@ import (
 const wildcardHostname = "~^"
 
 // BuildConfiguration builds the Configuration from the Graph.
-func BuildConfiguration(ctx context.Context, g *graph.Graph, resolver resolver.ServiceResolver) *Configuration {
+func BuildConfiguration(
+	ctx context.Context,
+	g *graph.Graph,
+	resolver resolver.ServiceResolver,
+	configVersion int,
+) Configuration {
 	if g.GatewayClass == nil || !g.GatewayClass.Valid {
-		return &Configuration{}
+		return Configuration{Version: configVersion}
 	}
 
 	if g.Gateway == nil {
-		return &Configuration{}
+		return Configuration{Version: configVersion}
 	}
 
 	upstreams := buildUpstreams(ctx, g.Gateway.Listeners, resolver)
@@ -36,9 +41,10 @@ func BuildConfiguration(ctx context.Context, g *graph.Graph, resolver resolver.S
 		Upstreams:     upstreams,
 		BackendGroups: backendGroups,
 		SSLKeyPairs:   keyPairs,
+		Version:       configVersion,
 	}
 
-	return &config
+	return config
 }
 
 // buildSSLKeyPairs builds the SSLKeyPairs from the Secrets. It will only include Secrets that are referenced by

internal/mode/static/state/dataplane/configuration.go

@@ -1488,13 +1488,14 @@ func TestBuildConfiguration(t *testing.T) {
 		t.Run(test.msg, func(t *testing.T) {
 			g := NewGomegaWithT(t)
 
-			result := BuildConfiguration(context.TODO(), test.graph, fakeResolver)
+			result := BuildConfiguration(context.TODO(), test.graph, fakeResolver, 1)
 
 			g.Expect(result.BackendGroups).To(ConsistOf(test.expConf.BackendGroups))
 			g.Expect(result.Upstreams).To(ConsistOf(test.expConf.Upstreams))
 			g.Expect(result.HTTPServers).To(ConsistOf(test.expConf.HTTPServers))
 			g.Expect(result.SSLServers).To(ConsistOf(test.expConf.SSLServers))
 			g.Expect(result.SSLKeyPairs).To(Equal(test.expConf.SSLKeyPairs))
+			g.Expect(result.Version).To(Equal(1))
 		})
 	}
 }