Skip to content

Commit

Permalink
Move version state to event handler
Browse files Browse the repository at this point in the history
  • Loading branch information
ciarams87 committed Sep 11, 2023
1 parent ac09dcd commit d7b261d
Show file tree
Hide file tree
Showing 8 changed files with 37 additions and 30 deletions.
10 changes: 8 additions & 2 deletions internal/mode/static/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions internal/mode/static/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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() {
Expand All @@ -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)
})
})

Expand Down
2 changes: 1 addition & 1 deletion internal/mode/static/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
12 changes: 6 additions & 6 deletions internal/mode/static/nginx/config/configfakes/fake_generator.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 6 additions & 12 deletions internal/mode/static/nginx/config/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand All @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion internal/mode/static/nginx/config/generator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand Down
14 changes: 10 additions & 4 deletions internal/mode/static/state/dataplane/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion internal/mode/static/state/dataplane/configuration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
})
}
}
Expand Down

0 comments on commit d7b261d

Please sign in to comment.