diff --git a/charts/nginx-ingress/README.md b/charts/nginx-ingress/README.md index b5ef71a6d8..a38ff2fc23 100644 --- a/charts/nginx-ingress/README.md +++ b/charts/nginx-ingress/README.md @@ -458,6 +458,7 @@ The following tables lists the configurable parameters of the NGINX Ingress Cont |`controller.defaultHTTPListenerPort` | Sets the port for the HTTP `default_server` listener. | 80 | |`controller.defaultHTTPSListenerPort` | Sets the port for the HTTPS `default_server` listener. | 443 | |`controller.readOnlyRootFilesystem` | Configure root filesystem as read-only and add volumes for temporary data. | false | +|`controller.enableSSLDynamicReload` | Enable lazy loading for SSL Certificates for NGINX Plus. | true | |`rbac.create` | Configures RBAC. | true | |`prometheus.create` | Expose NGINX or NGINX Plus metrics in the Prometheus format. | true | |`prometheus.port` | Configures the port to scrape the metrics. | 9113 | diff --git a/charts/nginx-ingress/templates/_helpers.tpl b/charts/nginx-ingress/templates/_helpers.tpl index 8008dc350f..2f5add833d 100644 --- a/charts/nginx-ingress/templates/_helpers.tpl +++ b/charts/nginx-ingress/templates/_helpers.tpl @@ -222,4 +222,5 @@ Build the args for the service binary. - -ready-status={{ .Values.controller.readyStatus.enable }} - -ready-status-port={{ .Values.controller.readyStatus.port }} - -enable-latency-metrics={{ .Values.controller.enableLatencyMetrics }} +- -ssl-dynamic-reload={{ .Values.controller.enableSSLDynamicReload }} {{- end -}} diff --git a/charts/nginx-ingress/values.schema.json b/charts/nginx-ingress/values.schema.json index c30938edb8..48b72ca22e 100644 --- a/charts/nginx-ingress/values.schema.json +++ b/charts/nginx-ingress/values.schema.json @@ -1359,6 +1359,14 @@ "examples": [ false ] + }, + "enableSSLDynamicReload": { + "type": "boolean", + "default": true, + "title": "Enable dynamic certificate reloads for NGINX Plus", + "examples": [ + true + ] } }, "examples": [ diff --git a/charts/nginx-ingress/values.yaml b/charts/nginx-ingress/values.yaml index e99dcd1250..0f41219cd5 100644 --- a/charts/nginx-ingress/values.yaml +++ b/charts/nginx-ingress/values.yaml @@ -463,6 +463,9 @@ controller: ## Configure root filesystem as read-only and add volumes for temporary data. readOnlyRootFilesystem: false + ## Enable dynamic reloading of certificates for NGINX Plus + enableSSLDynamicReload: true + rbac: ## Configures RBAC. create: true diff --git a/cmd/nginx-ingress/flags.go b/cmd/nginx-ingress/flags.go index 62c5ee4b9d..c8879b5297 100644 --- a/cmd/nginx-ingress/flags.go +++ b/cmd/nginx-ingress/flags.go @@ -15,6 +15,10 @@ import ( "k8s.io/apimachinery/pkg/util/validation" ) +const ( + dynamicSSLReloadParam = "ssl-dynamic-reload" +) + var ( healthStatus = flag.Bool("health-status", false, `Add a location based on the value of health-status-uri to the default server. The location responds with the 200 status code for any request. @@ -195,6 +199,8 @@ var ( defaultHTTPSListenerPort = flag.Int("default-https-listener-port", 443, "Sets a custom port for the HTTPS `default_server`. [1024 - 65535]") + enableDynamicSSLReload = flag.Bool(dynamicSSLReloadParam, true, "Enable reloading of SSL Certificates without restarting the NGINX process. Requires -nginx-plus") + startupCheckFn func() error ) @@ -269,6 +275,11 @@ func parseFlags() { if *ingressLink != "" && *externalService != "" { glog.Fatal("ingresslink and external-service cannot both be set") } + + if *enableDynamicSSLReload && !*nginxPlus { + glog.V(3).Infof("%s flag requires -nginx-plus and will not be enabled", dynamicSSLReloadParam) + *enableDynamicSSLReload = false + } } func initialChecks() { diff --git a/cmd/nginx-ingress/main.go b/cmd/nginx-ingress/main.go index bece95587c..40339c46e7 100644 --- a/cmd/nginx-ingress/main.go +++ b/cmd/nginx-ingress/main.go @@ -116,6 +116,8 @@ func main() { EnableOIDC: *enableOIDC, SSLRejectHandshake: sslRejectHandshake, EnableCertManager: *enableCertManager, + DynamicSSLReload: *enableDynamicSSLReload, + StaticSSLPath: nginxManager.GetSecretsDir(), } processNginxConfig(staticCfgParams, cfgParams, templateExecutor, nginxManager) @@ -132,17 +134,18 @@ func main() { plusCollector, syslogListener, latencyCollector := createPlusAndLatencyCollectors(registry, constLabels, kubeClient, plusClient, staticCfgParams.NginxServiceMesh) cnf := configs.NewConfigurator(configs.ConfiguratorParams{ - NginxManager: nginxManager, - StaticCfgParams: staticCfgParams, - Config: cfgParams, - TemplateExecutor: templateExecutor, - TemplateExecutorV2: templateExecutorV2, - LatencyCollector: latencyCollector, - LabelUpdater: plusCollector, - IsPlus: *nginxPlus, - IsWildcardEnabled: isWildcardEnabled, - IsPrometheusEnabled: *enablePrometheusMetrics, - IsLatencyMetricsEnabled: *enableLatencyMetrics, + NginxManager: nginxManager, + StaticCfgParams: staticCfgParams, + Config: cfgParams, + TemplateExecutor: templateExecutor, + TemplateExecutorV2: templateExecutorV2, + LatencyCollector: latencyCollector, + LabelUpdater: plusCollector, + IsPlus: *nginxPlus, + IsWildcardEnabled: isWildcardEnabled, + IsPrometheusEnabled: *enablePrometheusMetrics, + IsLatencyMetricsEnabled: *enableLatencyMetrics, + IsDynamicSSLReloadEnabled: *enableDynamicSSLReload, }) controllerNamespace := os.Getenv("POD_NAMESPACE") diff --git a/docs/content/configuration/global-configuration/command-line-arguments.md b/docs/content/configuration/global-configuration/command-line-arguments.md index d010b1095f..ff10f1830d 100644 --- a/docs/content/configuration/global-configuration/command-line-arguments.md +++ b/docs/content/configuration/global-configuration/command-line-arguments.md @@ -527,3 +527,11 @@ Sets the port for the HTTPS `default_server` listener. Default `443`. + +### -ssl-dynamic-reload + +Used to activate or deactivate lazy loading for SSL Certificates for NGINX Plus. + +The default value is `true` when using NGINX Plus. + + diff --git a/internal/configs/commonhelpers/common_template_helpers.go b/internal/configs/commonhelpers/common_template_helpers.go new file mode 100644 index 0000000000..b5727291e7 --- /dev/null +++ b/internal/configs/commonhelpers/common_template_helpers.go @@ -0,0 +1,15 @@ +// Package commonhelpers contains template helpers used in v1 and v2 +package commonhelpers + +import ( + "strings" +) + +// MakeSecretPath will return the path to the secret with the base secrets +// path replaced with the given variable +func MakeSecretPath(path, defaultPath, variable string, useVariable bool) string { + if useVariable { + return strings.Replace(path, defaultPath, variable, 1) + } + return path +} diff --git a/internal/configs/commonhelpers/common_template_helpers_test.go b/internal/configs/commonhelpers/common_template_helpers_test.go new file mode 100644 index 0000000000..3d6c06cea9 --- /dev/null +++ b/internal/configs/commonhelpers/common_template_helpers_test.go @@ -0,0 +1,63 @@ +package commonhelpers + +import ( + "bytes" + "html/template" + "testing" +) + +var helperFunctions = template.FuncMap{ + "makeSecretPath": MakeSecretPath, +} + +func TestMakeSecretPath(t *testing.T) { + t.Parallel() + + tmpl := newMakeSecretPathTemplate(t) + testCases := []struct { + Secret string + Path string + Variable string + Enabled bool + expected string + }{ + { + Secret: "/etc/nginx/secret/thing.crt", + Path: "/etc/nginx/secret", + Variable: "$secrets_path", + Enabled: true, + expected: "$secrets_path/thing.crt", + }, + { + Secret: "/etc/nginx/secret/thing.crt", + Path: "/etc/nginx/secret", + Variable: "$secrets_path", + Enabled: false, + expected: "/etc/nginx/secret/thing.crt", + }, + { + Secret: "/etc/nginx/secret/thing.crt", + expected: "/etc/nginx/secret/thing.crt", + }, + } + + for _, tc := range testCases { + var buf bytes.Buffer + err := tmpl.Execute(&buf, tc) + if err != nil { + t.Fatalf("Failed to execute the template %v", err) + } + if buf.String() != tc.expected { + t.Errorf("Template generated wrong config, got '%v' but expected '%v'.", buf.String(), tc.expected) + } + } +} + +func newMakeSecretPathTemplate(t *testing.T) *template.Template { + t.Helper() + tmpl, err := template.New("testTemplate").Funcs(helperFunctions).Parse(`{{makeSecretPath .Secret .Path .Variable .Enabled}}`) + if err != nil { + t.Fatalf("Failed to parse template: %v", err) + } + return tmpl +} diff --git a/internal/configs/config_params.go b/internal/configs/config_params.go index 3d6ee225a5..f76a944663 100644 --- a/internal/configs/config_params.go +++ b/internal/configs/config_params.go @@ -134,6 +134,8 @@ type StaticConfigParams struct { EnableOIDC bool SSLRejectHandshake bool EnableCertManager bool + DynamicSSLReload bool + StaticSSLPath string } // GlobalConfigParams holds global configuration parameters. For now, it only holds listeners. diff --git a/internal/configs/configmaps.go b/internal/configs/configmaps.go index 006f70c74c..0b653408f9 100644 --- a/internal/configs/configmaps.go +++ b/internal/configs/configmaps.go @@ -579,6 +579,8 @@ func GenerateNginxMainConfig(staticCfgParams *StaticConfigParams, config *Config InternalRouteServerName: staticCfgParams.InternalRouteServerName, LatencyMetrics: staticCfgParams.EnableLatencyMetrics, OIDC: staticCfgParams.EnableOIDC, + DynamicSSLReloadEnabled: staticCfgParams.DynamicSSLReload, + StaticSSLPath: staticCfgParams.StaticSSLPath, } return nginxCfg } diff --git a/internal/configs/configurator.go b/internal/configs/configurator.go index 007efd4808..4d03c03be9 100644 --- a/internal/configs/configurator.go +++ b/internal/configs/configurator.go @@ -109,40 +109,42 @@ type metricLabelsIndex struct { // This allows the Ingress Controller to incrementally build the NGINX configuration during the IC start and // then apply it at the end of the start. type Configurator struct { - nginxManager nginx.Manager - staticCfgParams *StaticConfigParams - cfgParams *ConfigParams - templateExecutor *version1.TemplateExecutor - templateExecutorV2 *version2.TemplateExecutor - ingresses map[string]*IngressEx - minions map[string]map[string]bool - virtualServers map[string]*VirtualServerEx - transportServers map[string]*TransportServerEx - tlsPassthroughPairs map[string]tlsPassthroughPair - isWildcardEnabled bool - isPlus bool - labelUpdater collector.LabelUpdater - metricLabelsIndex *metricLabelsIndex - isPrometheusEnabled bool - latencyCollector latCollector.LatencyCollector - isLatencyMetricsEnabled bool - isReloadsEnabled bool + nginxManager nginx.Manager + staticCfgParams *StaticConfigParams + cfgParams *ConfigParams + templateExecutor *version1.TemplateExecutor + templateExecutorV2 *version2.TemplateExecutor + ingresses map[string]*IngressEx + minions map[string]map[string]bool + virtualServers map[string]*VirtualServerEx + transportServers map[string]*TransportServerEx + tlsPassthroughPairs map[string]tlsPassthroughPair + isWildcardEnabled bool + isPlus bool + labelUpdater collector.LabelUpdater + metricLabelsIndex *metricLabelsIndex + isPrometheusEnabled bool + latencyCollector latCollector.LatencyCollector + isLatencyMetricsEnabled bool + isReloadsEnabled bool + isDynamicSSLReloadEnabled bool } // ConfiguratorParams is a collection of parameters used for the // NewConfigurator() function type ConfiguratorParams struct { - NginxManager nginx.Manager - StaticCfgParams *StaticConfigParams - Config *ConfigParams - TemplateExecutor *version1.TemplateExecutor - TemplateExecutorV2 *version2.TemplateExecutor - LabelUpdater collector.LabelUpdater - LatencyCollector latCollector.LatencyCollector - IsPlus bool - IsPrometheusEnabled bool - IsWildcardEnabled bool - IsLatencyMetricsEnabled bool + NginxManager nginx.Manager + StaticCfgParams *StaticConfigParams + Config *ConfigParams + TemplateExecutor *version1.TemplateExecutor + TemplateExecutorV2 *version2.TemplateExecutor + LabelUpdater collector.LabelUpdater + LatencyCollector latCollector.LatencyCollector + IsPlus bool + IsPrometheusEnabled bool + IsWildcardEnabled bool + IsLatencyMetricsEnabled bool + IsDynamicSSLReloadEnabled bool } // NewConfigurator creates a new Configurator. @@ -160,24 +162,25 @@ func NewConfigurator(p ConfiguratorParams) *Configurator { } cnf := Configurator{ - nginxManager: p.NginxManager, - staticCfgParams: p.StaticCfgParams, - cfgParams: p.Config, - ingresses: make(map[string]*IngressEx), - virtualServers: make(map[string]*VirtualServerEx), - transportServers: make(map[string]*TransportServerEx), - templateExecutor: p.TemplateExecutor, - templateExecutorV2: p.TemplateExecutorV2, - minions: make(map[string]map[string]bool), - tlsPassthroughPairs: make(map[string]tlsPassthroughPair), - isPlus: p.IsPlus, - isWildcardEnabled: p.IsWildcardEnabled, - labelUpdater: p.LabelUpdater, - metricLabelsIndex: metricLabelsIndex, - isPrometheusEnabled: p.IsPrometheusEnabled, - latencyCollector: p.LatencyCollector, - isLatencyMetricsEnabled: p.IsLatencyMetricsEnabled, - isReloadsEnabled: false, + nginxManager: p.NginxManager, + staticCfgParams: p.StaticCfgParams, + cfgParams: p.Config, + ingresses: make(map[string]*IngressEx), + virtualServers: make(map[string]*VirtualServerEx), + transportServers: make(map[string]*TransportServerEx), + templateExecutor: p.TemplateExecutor, + templateExecutorV2: p.TemplateExecutorV2, + minions: make(map[string]map[string]bool), + tlsPassthroughPairs: make(map[string]tlsPassthroughPair), + isPlus: p.IsPlus, + isWildcardEnabled: p.IsWildcardEnabled, + labelUpdater: p.LabelUpdater, + metricLabelsIndex: metricLabelsIndex, + isPrometheusEnabled: p.IsPrometheusEnabled, + latencyCollector: p.LatencyCollector, + isLatencyMetricsEnabled: p.IsLatencyMetricsEnabled, + isDynamicSSLReloadEnabled: p.IsDynamicSSLReloadEnabled, + isReloadsEnabled: false, } return &cnf } @@ -274,7 +277,7 @@ func (cnf *Configurator) deleteIngressMetricsLabels(key string) { // AddOrUpdateIngress adds or updates NGINX configuration for the Ingress resource. func (cnf *Configurator) AddOrUpdateIngress(ingEx *IngressEx) (Warnings, error) { - warnings, err := cnf.addOrUpdateIngress(ingEx) + _, warnings, err := cnf.addOrUpdateIngress(ingEx) if err != nil { return warnings, fmt.Errorf("error adding or updating ingress %v/%v: %w", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err) } @@ -357,7 +360,9 @@ func (cnf *Configurator) streamUpstreamsForTransportServer(ts *conf_v1.Transport return upstreamNames } -func (cnf *Configurator) addOrUpdateIngress(ingEx *IngressEx) (Warnings, error) { +// addOrUpdateIngress returns a bool that specifies if the underlying config +// file has changed, and any warnings or errors +func (cnf *Configurator) addOrUpdateIngress(ingEx *IngressEx) (bool, Warnings, error) { apResources := cnf.updateApResources(ingEx) cnf.updateDosResource(ingEx.DosEx) @@ -389,20 +394,20 @@ func (cnf *Configurator) addOrUpdateIngress(ingEx *IngressEx) (Warnings, error) name := objectMetaToFileName(&ingEx.Ingress.ObjectMeta) content, err := cnf.templateExecutor.ExecuteIngressConfigTemplate(&nginxCfg) if err != nil { - return warnings, fmt.Errorf("error generating Ingress Config %v: %w", name, err) + return false, warnings, fmt.Errorf("error generating Ingress Config %v: %w", name, err) } - cnf.nginxManager.CreateConfig(name, content) + configChanged := cnf.nginxManager.CreateConfig(name, content) cnf.ingresses[name] = ingEx if (cnf.isPlus && cnf.isPrometheusEnabled) || cnf.isLatencyMetricsEnabled { cnf.updateIngressMetricsLabels(ingEx, nginxCfg.Upstreams) } - return warnings, nil + return configChanged, warnings, nil } // AddOrUpdateMergeableIngress adds or updates NGINX configuration for the Ingress resources with Mergeable Types. func (cnf *Configurator) AddOrUpdateMergeableIngress(mergeableIngs *MergeableIngresses) (Warnings, error) { - warnings, err := cnf.addOrUpdateMergeableIngress(mergeableIngs) + _, warnings, err := cnf.addOrUpdateMergeableIngress(mergeableIngs) if err != nil { return warnings, fmt.Errorf("error when adding or updating ingress %v/%v: %w", mergeableIngs.Master.Ingress.Namespace, mergeableIngs.Master.Ingress.Name, err) } @@ -414,7 +419,7 @@ func (cnf *Configurator) AddOrUpdateMergeableIngress(mergeableIngs *MergeableIng return warnings, nil } -func (cnf *Configurator) addOrUpdateMergeableIngress(mergeableIngs *MergeableIngresses) (Warnings, error) { +func (cnf *Configurator) addOrUpdateMergeableIngress(mergeableIngs *MergeableIngresses) (bool, Warnings, error) { apResources := cnf.updateApResources(mergeableIngs.Master) cnf.updateDosResource(mergeableIngs.Master.DosEx) dosResource := getAppProtectDosResource(mergeableIngs.Master.DosEx) @@ -451,9 +456,9 @@ func (cnf *Configurator) addOrUpdateMergeableIngress(mergeableIngs *MergeableIng name := objectMetaToFileName(&mergeableIngs.Master.Ingress.ObjectMeta) content, err := cnf.templateExecutor.ExecuteIngressConfigTemplate(&nginxCfg) if err != nil { - return warnings, fmt.Errorf("error generating Ingress Config %v: %w", name, err) + return false, warnings, fmt.Errorf("error generating Ingress Config %v: %w", name, err) } - cnf.nginxManager.CreateConfig(name, content) + changed := cnf.nginxManager.CreateConfig(name, content) cnf.ingresses[name] = mergeableIngs.Master cnf.minions[name] = make(map[string]bool) @@ -465,7 +470,7 @@ func (cnf *Configurator) addOrUpdateMergeableIngress(mergeableIngs *MergeableIng cnf.updateIngressMetricsLabels(mergeableIngs.Master, nginxCfg.Upstreams) } - return warnings, nil + return changed, warnings, nil } func (cnf *Configurator) updateVirtualServerMetricsLabels(virtualServerEx *VirtualServerEx, upstreams []version2.Upstream) { @@ -549,7 +554,7 @@ func (cnf *Configurator) deleteVirtualServerMetricsLabels(key string) { // AddOrUpdateVirtualServer adds or updates NGINX configuration for the VirtualServer resource. func (cnf *Configurator) AddOrUpdateVirtualServer(virtualServerEx *VirtualServerEx) (Warnings, error) { - warnings, err := cnf.addOrUpdateVirtualServer(virtualServerEx) + _, warnings, err := cnf.addOrUpdateVirtualServer(virtualServerEx) if err != nil { return warnings, fmt.Errorf("error adding or updating VirtualServer %v/%v: %w", virtualServerEx.VirtualServer.Namespace, virtualServerEx.VirtualServer.Name, err) } @@ -565,7 +570,7 @@ func (cnf *Configurator) addOrUpdateOpenTracingTracerConfig(content string) erro return cnf.nginxManager.CreateOpenTracingTracerConfig(content) } -func (cnf *Configurator) addOrUpdateVirtualServer(virtualServerEx *VirtualServerEx) (Warnings, error) { +func (cnf *Configurator) addOrUpdateVirtualServer(virtualServerEx *VirtualServerEx) (bool, Warnings, error) { apResources := cnf.updateApResourcesForVs(virtualServerEx) dosResources := map[string]*appProtectDosResource{} for k, v := range virtualServerEx.DosProtectedEx { @@ -582,16 +587,16 @@ func (cnf *Configurator) addOrUpdateVirtualServer(virtualServerEx *VirtualServer vsCfg, warnings := vsc.GenerateVirtualServerConfig(virtualServerEx, apResources, dosResources) content, err := cnf.templateExecutorV2.ExecuteVirtualServerTemplate(&vsCfg) if err != nil { - return warnings, fmt.Errorf("error generating VirtualServer config: %v: %w", name, err) + return false, warnings, fmt.Errorf("error generating VirtualServer config: %v: %w", name, err) } - cnf.nginxManager.CreateConfig(name, content) + changed := cnf.nginxManager.CreateConfig(name, content) cnf.virtualServers[name] = virtualServerEx if (cnf.isPlus && cnf.isPrometheusEnabled) || cnf.isLatencyMetricsEnabled { cnf.updateVirtualServerMetricsLabels(virtualServerEx, vsCfg.Upstreams) } - return warnings, nil + return changed, warnings, nil } // AddOrUpdateVirtualServers adds or updates NGINX configuration for multiple VirtualServer resources. @@ -599,7 +604,7 @@ func (cnf *Configurator) AddOrUpdateVirtualServers(virtualServerExes []*VirtualS allWarnings := newWarnings() for _, vsEx := range virtualServerExes { - warnings, err := cnf.addOrUpdateVirtualServer(vsEx) + _, warnings, err := cnf.addOrUpdateVirtualServer(vsEx) if err != nil { return allWarnings, err } @@ -683,7 +688,7 @@ func (cnf *Configurator) deleteTransportServerMetricsLabels(key string) { // AddOrUpdateTransportServer adds or updates NGINX configuration for the TransportServer resource. // It is a responsibility of the caller to check that the TransportServer references an existing listener. func (cnf *Configurator) AddOrUpdateTransportServer(transportServerEx *TransportServerEx) (Warnings, error) { - warnings, err := cnf.addOrUpdateTransportServer(transportServerEx) + _, warnings, err := cnf.addOrUpdateTransportServer(transportServerEx) if err != nil { return nil, fmt.Errorf("error adding or updating TransportServer %v/%v: %w", transportServerEx.TransportServer.Namespace, transportServerEx.TransportServer.Name, err) } @@ -693,18 +698,25 @@ func (cnf *Configurator) AddOrUpdateTransportServer(transportServerEx *Transport return warnings, nil } -func (cnf *Configurator) addOrUpdateTransportServer(transportServerEx *TransportServerEx) (Warnings, error) { +func (cnf *Configurator) addOrUpdateTransportServer(transportServerEx *TransportServerEx) (bool, Warnings, error) { name := getFileNameForTransportServer(transportServerEx.TransportServer) - tsCfg, warnings := generateTransportServerConfig(transportServerEx, transportServerEx.ListenerPort, cnf.isPlus, cnf.IsResolverConfigured()) + tsCfg, warnings := generateTransportServerConfig(transportServerConfigParams{ + transportServerEx: transportServerEx, + listenerPort: transportServerEx.ListenerPort, + isPlus: cnf.isPlus, + isResolverConfigured: cnf.IsResolverConfigured(), + isDynamicReloadEnabled: cnf.staticCfgParams.DynamicSSLReload, + staticSSLPath: cnf.staticCfgParams.StaticSSLPath, + }) content, err := cnf.templateExecutorV2.ExecuteTransportServerTemplate(tsCfg) if err != nil { - return nil, fmt.Errorf("error generating TransportServer config %v: %w", name, err) + return false, nil, fmt.Errorf("error generating TransportServer config %v: %w", name, err) } if cnf.isPlus && cnf.isPrometheusEnabled { cnf.updateTransportServerMetricsLabels(transportServerEx, tsCfg.Upstreams) } - cnf.nginxManager.CreateStreamConfig(name, content) + changed := cnf.nginxManager.CreateStreamConfig(name, content) cnf.transportServers[name] = transportServerEx @@ -716,13 +728,13 @@ func (cnf *Configurator) addOrUpdateTransportServer(transportServerEx *Transport Host: transportServerEx.TransportServer.Spec.Host, UnixSocket: generateUnixSocket(transportServerEx), } - err := cnf.updateTLSPassthroughHostsConfig() + ptChanged, err := cnf.updateTLSPassthroughHostsConfig() if err != nil { - return nil, err + return false, nil, err } - return warnings, nil + return (changed || ptChanged), warnings, nil } - return warnings, nil + return changed, warnings, nil } // GetVirtualServerRoutesForVirtualServer returns the virtualServerRoutes that a virtualServer @@ -735,17 +747,15 @@ func (cnf *Configurator) GetVirtualServerRoutesForVirtualServer(key string) []*c return nil } -func (cnf *Configurator) updateTLSPassthroughHostsConfig() error { +func (cnf *Configurator) updateTLSPassthroughHostsConfig() (bool, error) { cfg := generateTLSPassthroughHostsConfig(cnf.tlsPassthroughPairs) content, err := cnf.templateExecutorV2.ExecuteTLSPassthroughHostsTemplate(cfg) if err != nil { - return fmt.Errorf("error generating config for TLS Passthrough Unix Sockets map: %w", err) + return false, fmt.Errorf("error generating config for TLS Passthrough Unix Sockets map: %w", err) } - cnf.nginxManager.CreateTLSPassthroughHostsConfig(content) - - return nil + return cnf.nginxManager.CreateTLSPassthroughHostsConfig(content), nil } func generateTLSPassthroughHostsConfig(tlsPassthroughPairs map[string]tlsPassthroughPair) *version2.TLSPassthroughHostsConfig { @@ -781,43 +791,62 @@ func (cnf *Configurator) addOrUpdateHtpasswdSecret(secret *api_v1.Secret) string } // AddOrUpdateResources adds or updates configuration for resources. -func (cnf *Configurator) AddOrUpdateResources(resources ExtendedResources) (Warnings, error) { +func (cnf *Configurator) AddOrUpdateResources(resources ExtendedResources, reloadIfUnchanged bool) (Warnings, error) { allWarnings := newWarnings() + configsChanged := false - for _, ingEx := range resources.IngressExes { - warnings, err := cnf.addOrUpdateIngress(ingEx) + updateResource := func(updateFunc func() (bool, Warnings, error), namespace, name string) error { + changed, warnings, err := updateFunc() if err != nil { - return nil, fmt.Errorf("error adding or updating ingress %v/%v: %w", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err) + return fmt.Errorf("error adding or updating resource %v/%v: %w", namespace, name, err) } allWarnings.Add(warnings) + if changed { + configsChanged = true + } + return nil + } + + for _, ingEx := range resources.IngressExes { + err := updateResource(func() (bool, Warnings, error) { + return cnf.addOrUpdateIngress(ingEx) + }, ingEx.Ingress.Namespace, ingEx.Ingress.Name) + if err != nil { + return nil, err + } } for _, m := range resources.MergeableIngresses { - warnings, err := cnf.addOrUpdateMergeableIngress(m) + err := updateResource(func() (bool, Warnings, error) { + return cnf.addOrUpdateMergeableIngress(m) + }, m.Master.Ingress.Namespace, m.Master.Ingress.Name) if err != nil { - return nil, fmt.Errorf("error adding or updating mergeableIngress %v/%v: %w", m.Master.Ingress.Namespace, m.Master.Ingress.Name, err) + return nil, err } - allWarnings.Add(warnings) } for _, vsEx := range resources.VirtualServerExes { - warnings, err := cnf.addOrUpdateVirtualServer(vsEx) + err := updateResource(func() (bool, Warnings, error) { + return cnf.addOrUpdateVirtualServer(vsEx) + }, vsEx.VirtualServer.Namespace, vsEx.VirtualServer.Name) if err != nil { - return nil, fmt.Errorf("error adding or updating VirtualServer %v/%v: %w", vsEx.VirtualServer.Namespace, vsEx.VirtualServer.Name, err) + return nil, err } - allWarnings.Add(warnings) } for _, tsEx := range resources.TransportServerExes { - warnings, err := cnf.addOrUpdateTransportServer(tsEx) + err := updateResource(func() (bool, Warnings, error) { + return cnf.addOrUpdateTransportServer(tsEx) + }, tsEx.TransportServer.Namespace, tsEx.TransportServer.Name) if err != nil { - return nil, fmt.Errorf("error adding or updating TransportServer %v/%v: %w", tsEx.TransportServer.Namespace, tsEx.TransportServer.Name, err) + return nil, err } - allWarnings.Add(warnings) } - if err := cnf.reload(nginx.ReloadForOtherUpdate); err != nil { - return nil, fmt.Errorf("error when reloading NGINX when updating resources: %w", err) + if configsChanged || reloadIfUnchanged { + if err := cnf.reload(nginx.ReloadForOtherUpdate); err != nil { + return nil, fmt.Errorf("error when reloading NGINX when updating resources: %w", err) + } } return allWarnings, nil } @@ -836,8 +865,12 @@ func (cnf *Configurator) AddOrUpdateSpecialTLSSecrets(secret *api_v1.Secret, sec cnf.nginxManager.CreateSecret(secretName, data, nginx.TLSSecretFileMode) } - if err := cnf.reload(nginx.ReloadForOtherUpdate); err != nil { - return fmt.Errorf("error when reloading NGINX when updating the special Secrets: %w", err) + if !cnf.DynamicSSLReloadEnabled() { + if err := cnf.reload(nginx.ReloadForOtherUpdate); err != nil { + return fmt.Errorf("error when reloading NGINX when updating the special Secrets: %w", err) + } + } else { + glog.V(3).Infof("Skipping reload for %d special Secrets", len(secretNames)) } return nil @@ -931,8 +964,8 @@ func (cnf *Configurator) deleteTransportServer(key string) error { // update TLS Passthrough Hosts config in case we have a TLS Passthrough TransportServer if _, exists := cnf.tlsPassthroughPairs[key]; exists { delete(cnf.tlsPassthroughPairs, key) - - return cnf.updateTLSPassthroughHostsConfig() + _, err := cnf.updateTLSPassthroughHostsConfig() + return err } return nil @@ -944,7 +977,7 @@ func (cnf *Configurator) UpdateEndpoints(ingExes []*IngressEx) error { for _, ingEx := range ingExes { // It is safe to ignore warnings here as no new warnings should appear when updating Endpoints for Ingresses - _, err := cnf.addOrUpdateIngress(ingEx) + _, _, err := cnf.addOrUpdateIngress(ingEx) if err != nil { return fmt.Errorf("error adding or updating ingress %v/%v: %w", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err) } @@ -976,7 +1009,7 @@ func (cnf *Configurator) UpdateEndpointsMergeableIngress(mergeableIngresses []*M for i := range mergeableIngresses { // It is safe to ignore warnings here as no new warnings should appear when updating Endpoints for Ingresses - _, err := cnf.addOrUpdateMergeableIngress(mergeableIngresses[i]) + _, _, err := cnf.addOrUpdateMergeableIngress(mergeableIngresses[i]) if err != nil { return fmt.Errorf("error adding or updating mergeableIngress %v/%v: %w", mergeableIngresses[i].Master.Ingress.Namespace, mergeableIngresses[i].Master.Ingress.Name, err) } @@ -1010,7 +1043,7 @@ func (cnf *Configurator) UpdateEndpointsForVirtualServers(virtualServerExes []*V for _, vs := range virtualServerExes { // It is safe to ignore warnings here as no new warnings should appear when updating Endpoints for VirtualServers - _, err := cnf.addOrUpdateVirtualServer(vs) + _, _, err := cnf.addOrUpdateVirtualServer(vs) if err != nil { return fmt.Errorf("error adding or updating VirtualServer %v/%v: %w", vs.VirtualServer.Namespace, vs.VirtualServer.Name, err) } @@ -1058,7 +1091,7 @@ func (cnf *Configurator) UpdateEndpointsForTransportServers(transportServerExes for _, tsEx := range transportServerExes { // Ignore warnings here as no new warnings should appear when updating Endpoints for TransportServers - _, err := cnf.addOrUpdateTransportServer(tsEx) + _, _, err := cnf.addOrUpdateTransportServer(tsEx) if err != nil { return fmt.Errorf("error adding or updating TransportServer %v/%v: %w", tsEx.TransportServer.Namespace, tsEx.TransportServer.Name, err) } @@ -1228,21 +1261,21 @@ func (cnf *Configurator) UpdateConfig(cfgParams *ConfigParams, resources Extende cnf.nginxManager.CreateMainConfig(mainCfgContent) for _, ingEx := range resources.IngressExes { - warnings, err := cnf.addOrUpdateIngress(ingEx) + _, warnings, err := cnf.addOrUpdateIngress(ingEx) if err != nil { return allWarnings, err } allWarnings.Add(warnings) } for _, mergeableIng := range resources.MergeableIngresses { - warnings, err := cnf.addOrUpdateMergeableIngress(mergeableIng) + _, warnings, err := cnf.addOrUpdateMergeableIngress(mergeableIng) if err != nil { return allWarnings, err } allWarnings.Add(warnings) } for _, vsEx := range resources.VirtualServerExes { - warnings, err := cnf.addOrUpdateVirtualServer(vsEx) + _, warnings, err := cnf.addOrUpdateVirtualServer(vsEx) if err != nil { return allWarnings, err } @@ -1250,7 +1283,7 @@ func (cnf *Configurator) UpdateConfig(cfgParams *ConfigParams, resources Extende } for _, tsEx := range resources.TransportServerExes { - warnings, err := cnf.addOrUpdateTransportServer(tsEx) + _, warnings, err := cnf.addOrUpdateTransportServer(tsEx) if err != nil { return allWarnings, err } @@ -1286,7 +1319,7 @@ func (cnf *Configurator) ReloadForBatchUpdates(batchReloadsEnabled bool) error { func (cnf *Configurator) UpdateVirtualServers(updatedVSExes []*VirtualServerEx, deletedKeys []string) []error { var errList []error for _, vsEx := range updatedVSExes { - _, err := cnf.addOrUpdateVirtualServer(vsEx) + _, _, err := cnf.addOrUpdateVirtualServer(vsEx) if err != nil { errList = append(errList, fmt.Errorf("error adding or updating VirtualServer %v/%v: %w", vsEx.VirtualServer.Namespace, vsEx.VirtualServer.Name, err)) } @@ -1310,7 +1343,7 @@ func (cnf *Configurator) UpdateVirtualServers(updatedVSExes []*VirtualServerEx, func (cnf *Configurator) UpdateTransportServers(updatedTSExes []*TransportServerEx, deletedKeys []string) []error { var errList []error for _, tsEx := range updatedTSExes { - _, err := cnf.addOrUpdateTransportServer(tsEx) + _, _, err := cnf.addOrUpdateTransportServer(tsEx) if err != nil { errList = append(errList, fmt.Errorf("error adding or updating TransportServer %v/%v: %w", tsEx.TransportServer.Namespace, tsEx.TransportServer.Name, err)) } @@ -1599,7 +1632,7 @@ func (cnf *Configurator) addOrUpdateIngressesAndVirtualServers(ingExes []*Ingres allWarnings := newWarnings() for _, ingEx := range ingExes { - warnings, err := cnf.addOrUpdateIngress(ingEx) + _, warnings, err := cnf.addOrUpdateIngress(ingEx) if err != nil { return allWarnings, fmt.Errorf("error adding or updating ingress %v/%v: %w", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err) } @@ -1607,7 +1640,7 @@ func (cnf *Configurator) addOrUpdateIngressesAndVirtualServers(ingExes []*Ingres } for _, m := range mergeableIngresses { - warnings, err := cnf.addOrUpdateMergeableIngress(m) + _, warnings, err := cnf.addOrUpdateMergeableIngress(m) if err != nil { return allWarnings, fmt.Errorf("error adding or updating mergeableIngress %v/%v: %w", m.Master.Ingress.Namespace, m.Master.Ingress.Name, err) } @@ -1615,7 +1648,7 @@ func (cnf *Configurator) addOrUpdateIngressesAndVirtualServers(ingExes []*Ingres } for _, vs := range vsExes { - warnings, err := cnf.addOrUpdateVirtualServer(vs) + _, warnings, err := cnf.addOrUpdateVirtualServer(vs) if err != nil { return allWarnings, fmt.Errorf("error adding or updating VirtualServer %v/%v: %w", vs.VirtualServer.Namespace, vs.VirtualServer.Name, err) } @@ -1727,3 +1760,8 @@ func (cnf *Configurator) AddOrUpdateSecret(secret *api_v1.Secret) string { func (cnf *Configurator) DeleteSecret(key string) { cnf.nginxManager.DeleteSecret(keyToFileName(key)) } + +// DynamicSSLReloadEnabled is used to check if dynamic reloading of SSL certificates is enabled +func (cnf *Configurator) DynamicSSLReloadEnabled() bool { + return cnf.isDynamicSSLReloadEnabled +} diff --git a/internal/configs/ingress.go b/internal/configs/ingress.go index c7eb24e203..799624a6fc 100644 --- a/internal/configs/ingress.go +++ b/internal/configs/ingress.go @@ -314,7 +314,9 @@ func generateNginxCfg(p NginxCfgParams) (version1.IngressNginxConfig, Warnings) Namespace: p.ingEx.Ingress.Namespace, Annotations: p.ingEx.Ingress.Annotations, }, - SpiffeClientCerts: p.staticParams.NginxServiceMesh && !cfgParams.SpiffeServerCerts, + SpiffeClientCerts: p.staticParams.NginxServiceMesh && !cfgParams.SpiffeServerCerts, + DynamicSSLReloadEnabled: p.staticParams.DynamicSSLReload, + StaticSSLPath: p.staticParams.StaticSSLPath, }, allWarnings } @@ -520,6 +522,9 @@ func createUpstream(ingEx *IngressEx, name string, backend *networking.IngressBa }) } if len(upsServers) > 0 { + sort.Slice(upsServers, func(i, j int) bool { + return upsServers[i].Address < upsServers[j].Address + }) ups.UpstreamServers = upsServers } } @@ -692,11 +697,13 @@ func generateNginxCfgForMergeableIngresses(p NginxCfgParams) (version1.IngressNg masterServer.Locations = locations return version1.IngressNginxConfig{ - Servers: []version1.Server{masterServer}, - Upstreams: upstreams, - Keepalive: keepalive, - Ingress: masterNginxCfg.Ingress, - SpiffeClientCerts: p.staticParams.NginxServiceMesh && !p.baseCfgParams.SpiffeServerCerts, + Servers: []version1.Server{masterServer}, + Upstreams: upstreams, + Keepalive: keepalive, + Ingress: masterNginxCfg.Ingress, + SpiffeClientCerts: p.staticParams.NginxServiceMesh && !p.baseCfgParams.SpiffeServerCerts, + DynamicSSLReloadEnabled: p.staticParams.DynamicSSLReload, + StaticSSLPath: p.staticParams.StaticSSLPath, }, warnings } diff --git a/internal/configs/transportserver.go b/internal/configs/transportserver.go index ff2976c77e..1996e6f037 100644 --- a/internal/configs/transportserver.go +++ b/internal/configs/transportserver.go @@ -2,6 +2,7 @@ package configs import ( "fmt" + "sort" "strings" api_v1 "k8s.io/api/core/v1" @@ -40,65 +41,74 @@ func newUpstreamNamerForTransportServer(transportServer *conf_v1.TransportServer } } +type transportServerConfigParams struct { + transportServerEx *TransportServerEx + listenerPort int + isPlus bool + isResolverConfigured bool + isDynamicReloadEnabled bool + staticSSLPath string +} + // generateTransportServerConfig generates a full configuration for a TransportServer. -func generateTransportServerConfig(transportServerEx *TransportServerEx, listenerPort int, isPlus bool, isResolverConfigured bool) (*version2.TransportServerConfig, Warnings) { +func generateTransportServerConfig(p transportServerConfigParams) (*version2.TransportServerConfig, Warnings) { warnings := newWarnings() - upstreamNamer := newUpstreamNamerForTransportServer(transportServerEx.TransportServer) + upstreamNamer := newUpstreamNamerForTransportServer(p.transportServerEx.TransportServer) - upstreams, w := generateStreamUpstreams(transportServerEx, upstreamNamer, isPlus, isResolverConfigured) + upstreams, w := generateStreamUpstreams(p.transportServerEx, upstreamNamer, p.isPlus, p.isResolverConfigured) warnings.Add(w) - healthCheck, match := generateTransportServerHealthCheck(transportServerEx.TransportServer.Spec.Action.Pass, - upstreamNamer.GetNameForUpstream(transportServerEx.TransportServer.Spec.Action.Pass), - transportServerEx.TransportServer.Spec.Upstreams) + healthCheck, match := generateTransportServerHealthCheck(p.transportServerEx.TransportServer.Spec.Action.Pass, + upstreamNamer.GetNameForUpstream(p.transportServerEx.TransportServer.Spec.Action.Pass), + p.transportServerEx.TransportServer.Spec.Upstreams) - sslConfig, w := generateSSLConfig(transportServerEx.TransportServer, transportServerEx.TransportServer.Spec.TLS, transportServerEx.TransportServer.Namespace, transportServerEx.SecretRefs) + sslConfig, w := generateSSLConfig(p.transportServerEx.TransportServer, p.transportServerEx.TransportServer.Spec.TLS, p.transportServerEx.TransportServer.Namespace, p.transportServerEx.SecretRefs) warnings.Add(w) var proxyRequests, proxyResponses *int var connectTimeout, nextUpstreamTimeout string var nextUpstream bool var nextUpstreamTries int - if transportServerEx.TransportServer.Spec.UpstreamParameters != nil { - proxyRequests = transportServerEx.TransportServer.Spec.UpstreamParameters.UDPRequests - proxyResponses = transportServerEx.TransportServer.Spec.UpstreamParameters.UDPResponses + if p.transportServerEx.TransportServer.Spec.UpstreamParameters != nil { + proxyRequests = p.transportServerEx.TransportServer.Spec.UpstreamParameters.UDPRequests + proxyResponses = p.transportServerEx.TransportServer.Spec.UpstreamParameters.UDPResponses - nextUpstream = transportServerEx.TransportServer.Spec.UpstreamParameters.NextUpstream + nextUpstream = p.transportServerEx.TransportServer.Spec.UpstreamParameters.NextUpstream if nextUpstream { - nextUpstreamTries = transportServerEx.TransportServer.Spec.UpstreamParameters.NextUpstreamTries - nextUpstreamTimeout = transportServerEx.TransportServer.Spec.UpstreamParameters.NextUpstreamTimeout + nextUpstreamTries = p.transportServerEx.TransportServer.Spec.UpstreamParameters.NextUpstreamTries + nextUpstreamTimeout = p.transportServerEx.TransportServer.Spec.UpstreamParameters.NextUpstreamTimeout } - connectTimeout = transportServerEx.TransportServer.Spec.UpstreamParameters.ConnectTimeout + connectTimeout = p.transportServerEx.TransportServer.Spec.UpstreamParameters.ConnectTimeout } var proxyTimeout string - if transportServerEx.TransportServer.Spec.SessionParameters != nil { - proxyTimeout = transportServerEx.TransportServer.Spec.SessionParameters.Timeout + if p.transportServerEx.TransportServer.Spec.SessionParameters != nil { + proxyTimeout = p.transportServerEx.TransportServer.Spec.SessionParameters.Timeout } - serverSnippets := generateSnippets(true, transportServerEx.TransportServer.Spec.ServerSnippets, []string{}) + serverSnippets := generateSnippets(true, p.transportServerEx.TransportServer.Spec.ServerSnippets, []string{}) - streamSnippets := generateSnippets(true, transportServerEx.TransportServer.Spec.StreamSnippets, []string{}) + streamSnippets := generateSnippets(true, p.transportServerEx.TransportServer.Spec.StreamSnippets, []string{}) - statusZone := transportServerEx.TransportServer.Spec.Listener.Name - if transportServerEx.TransportServer.Spec.Listener.Name == conf_v1.TLSPassthroughListenerName { - statusZone = transportServerEx.TransportServer.Spec.Host + statusZone := p.transportServerEx.TransportServer.Spec.Listener.Name + if p.transportServerEx.TransportServer.Spec.Listener.Name == conf_v1.TLSPassthroughListenerName { + statusZone = p.transportServerEx.TransportServer.Spec.Host } tsConfig := &version2.TransportServerConfig{ Server: version2.StreamServer{ - TLSPassthrough: transportServerEx.TransportServer.Spec.Listener.Name == conf_v1.TLSPassthroughListenerName, - UnixSocket: generateUnixSocket(transportServerEx), - Port: listenerPort, - UDP: transportServerEx.TransportServer.Spec.Listener.Protocol == "UDP", + TLSPassthrough: p.transportServerEx.TransportServer.Spec.Listener.Name == conf_v1.TLSPassthroughListenerName, + UnixSocket: generateUnixSocket(p.transportServerEx), + Port: p.listenerPort, + UDP: p.transportServerEx.TransportServer.Spec.Listener.Protocol == "UDP", StatusZone: statusZone, ProxyRequests: proxyRequests, ProxyResponses: proxyResponses, - ProxyPass: upstreamNamer.GetNameForUpstream(transportServerEx.TransportServer.Spec.Action.Pass), - Name: transportServerEx.TransportServer.Name, - Namespace: transportServerEx.TransportServer.Namespace, + ProxyPass: upstreamNamer.GetNameForUpstream(p.transportServerEx.TransportServer.Spec.Action.Pass), + Name: p.transportServerEx.TransportServer.Name, + Namespace: p.transportServerEx.TransportServer.Namespace, ProxyConnectTimeout: generateTimeWithDefault(connectTimeout, "60s"), ProxyTimeout: generateTimeWithDefault(proxyTimeout, "10m"), ProxyNextUpstream: nextUpstream, @@ -106,12 +116,14 @@ func generateTransportServerConfig(transportServerEx *TransportServerEx, listene ProxyNextUpstreamTries: nextUpstreamTries, HealthCheck: healthCheck, ServerSnippets: serverSnippets, - DisableIPV6: transportServerEx.DisableIPV6, + DisableIPV6: p.transportServerEx.DisableIPV6, SSL: sslConfig, }, - Match: match, - Upstreams: upstreams, - StreamSnippets: streamSnippets, + Match: match, + Upstreams: upstreams, + StreamSnippets: streamSnippets, + DynamicSSLReloadEnabled: p.isDynamicReloadEnabled, + StaticSSLPath: p.staticSSLPath, } return tsConfig, warnings } @@ -182,6 +194,9 @@ func generateStreamUpstreams(transportServerEx *TransportServerEx, upstreamNamer upstreams = append(upstreams, ups) } + sort.Slice(upstreams, func(i, j int) bool { + return upstreams[i].Name < upstreams[j].Name + }) return upstreams, warnings } @@ -283,6 +298,9 @@ func generateStreamUpstream(upstream conf_v1.TransportServerUpstream, upstreamNa }) } + sort.Slice(upsServers, func(i, j int) bool { + return upsServers[i].Address < upsServers[j].Address + }) return version2.StreamUpstream{ Name: name, Servers: upsServers, diff --git a/internal/configs/transportserver_test.go b/internal/configs/transportserver_test.go index 8845732814..ab16c221a7 100644 --- a/internal/configs/transportserver_test.go +++ b/internal/configs/transportserver_test.go @@ -142,9 +142,17 @@ func TestGenerateTransportServerConfigForTCPSnippets(t *testing.T) { SSL: &version2.StreamSSL{}, }, StreamSnippets: []string{"limit_conn_zone $binary_remote_addr zone=addr:10m;"}, + StaticSSLPath: "/etc/nginx/secret", } - result, warnings := generateTransportServerConfig(&transportServerEx, listenerPort, true, false) + result, warnings := generateTransportServerConfig(transportServerConfigParams{ + transportServerEx: &transportServerEx, + listenerPort: listenerPort, + isPlus: true, + isResolverConfigured: false, + isDynamicReloadEnabled: false, + staticSSLPath: "/etc/nginx/secret", + }) if len(warnings) != 0 { t.Errorf("want no warnings, got %v", warnings) } @@ -226,9 +234,17 @@ func TestGenerateTransportServerConfigForIPV6Disabled(t *testing.T) { SSL: &version2.StreamSSL{}, }, StreamSnippets: []string{}, + StaticSSLPath: "/etc/nginx/secret", } - result, warnings := generateTransportServerConfig(&transportServerEx, listenerPort, true, false) + result, warnings := generateTransportServerConfig(transportServerConfigParams{ + transportServerEx: &transportServerEx, + listenerPort: listenerPort, + isPlus: true, + isResolverConfigured: false, + isDynamicReloadEnabled: false, + staticSSLPath: "/etc/nginx/secret", + }) if len(warnings) != 0 { t.Errorf("want no warnings, got %v", warnings) } @@ -318,9 +334,17 @@ func TestGenerateTransportServerConfigForTCP(t *testing.T) { SSL: &version2.StreamSSL{}, }, StreamSnippets: []string{}, + StaticSSLPath: "/etc/nginx/secret", } - result, warnings := generateTransportServerConfig(&transportServerEx, listenerPort, true, false) + result, warnings := generateTransportServerConfig(transportServerConfigParams{ + transportServerEx: &transportServerEx, + listenerPort: listenerPort, + isPlus: true, + isResolverConfigured: false, + isDynamicReloadEnabled: false, + staticSSLPath: "/etc/nginx/secret", + }) if len(warnings) != 0 { t.Errorf("want no warnings, got %v", warnings) } @@ -413,9 +437,17 @@ func TestGenerateTransportServerConfigForTCPMaxConnections(t *testing.T) { SSL: &version2.StreamSSL{}, }, StreamSnippets: []string{}, + StaticSSLPath: "/etc/nginx/secret", } - result, warnings := generateTransportServerConfig(&transportServerEx, listenerPort, true, false) + result, warnings := generateTransportServerConfig(transportServerConfigParams{ + transportServerEx: &transportServerEx, + listenerPort: listenerPort, + isPlus: true, + isResolverConfigured: false, + isDynamicReloadEnabled: false, + staticSSLPath: "/etc/nginx/secret", + }) if len(warnings) != 0 { t.Errorf("want no warnings, got %v", warnings) } @@ -506,9 +538,17 @@ func TestGenerateTransportServerConfigForTLSPassthrough(t *testing.T) { }, DisableIPV6: false, StreamSnippets: []string{}, + StaticSSLPath: "/etc/nginx/secret", } - result, warnings := generateTransportServerConfig(&transportServerEx, listenerPort, true, false) + result, warnings := generateTransportServerConfig(transportServerConfigParams{ + transportServerEx: &transportServerEx, + listenerPort: listenerPort, + isPlus: true, + isResolverConfigured: false, + isDynamicReloadEnabled: false, + staticSSLPath: "/etc/nginx/secret", + }) if len(warnings) != 0 { t.Errorf("want no warnings, got %v", warnings) } @@ -604,9 +644,17 @@ func TestGenerateTransportServerConfigForUDP(t *testing.T) { SSL: &version2.StreamSSL{}, }, StreamSnippets: []string{}, + StaticSSLPath: "/etc/nginx/secret", } - result, warnings := generateTransportServerConfig(&transportServerEx, listenerPort, true, false) + result, warnings := generateTransportServerConfig(transportServerConfigParams{ + transportServerEx: &transportServerEx, + listenerPort: listenerPort, + isPlus: true, + isResolverConfigured: false, + isDynamicReloadEnabled: false, + staticSSLPath: "/etc/nginx/secret", + }) if len(warnings) != 0 { t.Errorf("want no warnings, got %v", warnings) } @@ -694,9 +742,17 @@ func TestGenerateTransportServerConfig_ProducesValidConfigOnValidInputForExterna SSL: &version2.StreamSSL{}, }, StreamSnippets: []string{}, + StaticSSLPath: "/etc/nginx/secret", } - result, warnings := generateTransportServerConfig(&transportServerEx, 2020, true, true) + result, warnings := generateTransportServerConfig(transportServerConfigParams{ + transportServerEx: &transportServerEx, + listenerPort: 2020, + isPlus: true, + isResolverConfigured: true, + isDynamicReloadEnabled: false, + staticSSLPath: "/etc/nginx/secret", + }) if len(warnings) != 0 { t.Errorf("want no warnings, got %v", warnings) } @@ -778,9 +834,17 @@ func TestGenerateTransportServerConfig_GeneratesWarningOnNotConfiguredResolver(t SSL: &version2.StreamSSL{}, }, StreamSnippets: []string{}, + StaticSSLPath: "/etc/nginx/secret", } - result, warnings := generateTransportServerConfig(&transportServerEx, 2020, true, false) + result, warnings := generateTransportServerConfig(transportServerConfigParams{ + transportServerEx: &transportServerEx, + listenerPort: 2020, + isPlus: true, + isResolverConfigured: false, + isDynamicReloadEnabled: false, + staticSSLPath: "/etc/nginx/secret", + }) if len(warnings) == 0 { t.Errorf("want warnings, got %v", warnings) } @@ -864,9 +928,17 @@ func TestGenerateTransportServerConfig_UsesNotExistignSocketOnNotPlusAndNoEndpoi SSL: &version2.StreamSSL{}, }, StreamSnippets: []string{}, + StaticSSLPath: "/etc/nginx/secret", } - result, warnings := generateTransportServerConfig(&transportServerEx, 2020, false, true) + result, warnings := generateTransportServerConfig(transportServerConfigParams{ + transportServerEx: &transportServerEx, + listenerPort: 2020, + isPlus: false, + isResolverConfigured: true, + isDynamicReloadEnabled: false, + staticSSLPath: "/etc/nginx/secret", + }) if len(warnings) != 0 { t.Errorf("want no warnings, got %v", warnings) } @@ -971,9 +1043,17 @@ func TestGenerateTransportServerConfigForTCPWithTLS(t *testing.T) { }, }, StreamSnippets: []string{}, + StaticSSLPath: "/etc/nginx/secret", } - result, warnings := generateTransportServerConfig(&transportServerEx, listenerPort, true, false) + result, warnings := generateTransportServerConfig(transportServerConfigParams{ + transportServerEx: &transportServerEx, + listenerPort: listenerPort, + isPlus: true, + isResolverConfigured: false, + isDynamicReloadEnabled: false, + staticSSLPath: "/etc/nginx/secret", + }) if len(warnings) != 0 { t.Errorf("want no warnings, got %v", warnings) } diff --git a/internal/configs/version1/config.go b/internal/configs/version1/config.go index eda5ffebbd..e1569a7bdb 100644 --- a/internal/configs/version1/config.go +++ b/internal/configs/version1/config.go @@ -10,11 +10,13 @@ type UpstreamLabels struct { // IngressNginxConfig describes an NGINX configuration. type IngressNginxConfig struct { - Upstreams []Upstream - Servers []Server - Keepalive string - Ingress Ingress - SpiffeClientCerts bool + Upstreams []Upstream + Servers []Server + Keepalive string + Ingress Ingress + SpiffeClientCerts bool + DynamicSSLReloadEnabled bool + StaticSSLPath string } // Ingress holds information about an Ingress resource. @@ -230,6 +232,8 @@ type MainConfig struct { InternalRouteServerName string LatencyMetrics bool OIDC bool + DynamicSSLReloadEnabled bool + StaticSSLPath string } // NewUpstreamWithDefaultServer creates an upstream with the default server. diff --git a/internal/configs/version1/nginx-plus.ingress.tmpl b/internal/configs/version1/nginx-plus.ingress.tmpl index 14343e65d0..bcf82ffb71 100644 --- a/internal/configs/version1/nginx-plus.ingress.tmpl +++ b/internal/configs/version1/nginx-plus.ingress.tmpl @@ -24,8 +24,8 @@ server { {{- if $server.SpiffeCerts}} listen 443 ssl; {{- if not $server.DisableIPV6}}listen [::]:443 ssl;{{end}} - ssl_certificate /etc/nginx/secrets/spiffe_cert.pem; - ssl_certificate_key /etc/nginx/secrets/spiffe_key.pem; + ssl_certificate {{ makeSecretPath "/etc/nginx/secrets/spiffe_cert.pem" $.StaticSSLPath "$secret_dir_path" $.DynamicSSLReloadEnabled }}; + ssl_certificate_key {{ makeSecretPath "/etc/nginx/secrets/spiffe_key.pem" $.StaticSSLPath "$secret_dir_path" $.DynamicSSLReloadEnabled }}; {{- else}} {{- if not $server.GRPCOnly}} {{- range $port := $server.Ports}} @@ -51,8 +51,8 @@ server { {{- if $server.SSLRejectHandshake}} ssl_reject_handshake on; {{- else}} - ssl_certificate {{$server.SSLCertificate}}; - ssl_certificate_key {{$server.SSLCertificateKey}}; + ssl_certificate {{ makeSecretPath $server.SSLCertificate $.StaticSSLPath "$secret_dir_path" $.DynamicSSLReloadEnabled }}; + ssl_certificate_key {{ makeSecretPath $server.SSLCertificateKey $.StaticSSLPath "$secret_dir_path" $.DynamicSSLReloadEnabled }}; {{- end}} {{- end}} {{- end}} @@ -235,8 +235,8 @@ server { grpc_buffer_size {{$location.ProxyBufferSize}}; {{- end}} {{- if $.SpiffeClientCerts}} - grpc_ssl_certificate /etc/nginx/secrets/spiffe_cert.pem; - grpc_ssl_certificate_key /etc/nginx/secrets/spiffe_key.pem; + grpc_ssl_certificate {{ makeSecretPath "/etc/nginx/secrets/spiffe_cert.pem" $.StaticSSLPath "$secret_dir_path" $.DynamicSSLReloadEnabled }}; + grpc_ssl_certificate_key {{ makeSecretPath "/etc/nginx/secrets/spiffe_key.pem" $.StaticSSLPath "$secret_dir_path" $.DynamicSSLReloadEnabled }}; grpc_ssl_trusted_certificate /etc/nginx/secrets/spiffe_rootca.pem; grpc_ssl_server_name on; grpc_ssl_verify on; @@ -296,8 +296,8 @@ server { proxy_max_temp_file_size {{$location.ProxyMaxTempFileSize}}; {{- end}} {{- if $.SpiffeClientCerts}} - proxy_ssl_certificate /etc/nginx/secrets/spiffe_cert.pem; - proxy_ssl_certificate_key /etc/nginx/secrets/spiffe_key.pem; + proxy_ssl_certificate {{ makeSecretPath "/etc/nginx/secrets/spiffe_cert.pem" $.StaticSSLPath "$secret_dir_path" $.DynamicSSLReloadEnabled }}; + proxy_ssl_certificate_key {{ makeSecretPath "/etc/nginx/secrets/spiffe_key.pem" $.StaticSSLPath "$secret_dir_path" $.DynamicSSLReloadEnabled }}; proxy_ssl_trusted_certificate /etc/nginx/secrets/spiffe_rootca.pem; proxy_ssl_server_name on; proxy_ssl_verify on; diff --git a/internal/configs/version1/nginx-plus.tmpl b/internal/configs/version1/nginx-plus.tmpl index 0675c97308..cb7cbd8706 100644 --- a/internal/configs/version1/nginx-plus.tmpl +++ b/internal/configs/version1/nginx-plus.tmpl @@ -27,7 +27,7 @@ load_module modules/ngx_fips_check_module.so; {{$value}}{{end}} {{- end}} -{{if .OIDC}} +{{- if .OIDC}} load_module modules/ngx_http_js_module.so; {{- end}} @@ -60,6 +60,12 @@ http { '' $sent_http_grpc_status; } + {{- if .DynamicSSLReloadEnabled }} + map $nginx_version $secret_dir_path { + default "{{ .StaticSSLPath }}"; + } + {{- end }} + {{- if .AppProtectDosLoadModule}} {{- if .AppProtectDosLogFormat}} log_format log_dos {{if .AppProtectDosLogFormatEscaping}}escape={{ .AppProtectDosLogFormatEscaping }} {{end}} @@ -172,8 +178,8 @@ http { {{- if .SSLRejectHandshake}} ssl_reject_handshake on; {{- else}} - ssl_certificate /etc/nginx/secrets/default; - ssl_certificate_key /etc/nginx/secrets/default; + ssl_certificate {{ makeSecretPath "/etc/nginx/secrets/default" .StaticSSLPath "$secret_dir_path" .DynamicSSLReloadEnabled }}; + ssl_certificate_key {{ makeSecretPath "/etc/nginx/secrets/default" .StaticSSLPath "$secret_dir_path" .DynamicSSLReloadEnabled }}; {{- end}} {{- range $setRealIPFrom := .SetRealIPFrom}} @@ -276,8 +282,8 @@ http { listen 443 ssl; {{if not .DisableIPV6}}listen [::]:443 ssl;{{end}} server_name {{.InternalRouteServerName}}; - ssl_certificate /etc/nginx/secrets/spiffe_cert.pem; - ssl_certificate_key /etc/nginx/secrets/spiffe_key.pem; + ssl_certificate {{ makeSecretPath "/etc/nginx/secrets/spiffe_cert.pem" .StaticSSLPath "$secret_dir_path" .DynamicSSLReloadEnabled }}; + ssl_certificate_key {{ makeSecretPath "/etc/nginx/secrets/spiffe_key.pem" .StaticSSLPath "$secret_dir_path" .DynamicSSLReloadEnabled }}; ssl_client_certificate /etc/nginx/secrets/spiffe_rootca.pem; ssl_verify_client on; ssl_verify_depth 25; @@ -310,6 +316,12 @@ stream { map_hash_max_size {{.MapHashMaxSize}}; {{if .MapHashBucketSize}}map_hash_bucket_size {{.MapHashBucketSize}};{{end}} + {{- if .DynamicSSLReloadEnabled }} + map $nginx_version $secret_dir_path { + default "{{ .StaticSSLPath }}"; + } + {{- end }} + {{- if .TLSPassthrough}} map $ssl_preread_server_name $dest_internal_passthrough { default unix:/var/lib/nginx/passthrough-https.sock; diff --git a/internal/configs/version1/template_executor.go b/internal/configs/version1/template_executor.go index d2eaae8304..0008b36a2e 100644 --- a/internal/configs/version1/template_executor.go +++ b/internal/configs/version1/template_executor.go @@ -15,7 +15,7 @@ type TemplateExecutor struct { // NewTemplateExecutor creates a TemplateExecutor. func NewTemplateExecutor(mainTemplatePath string, ingressTemplatePath string) (*TemplateExecutor, error) { // template name must be the base name of the template file https://golang.org/pkg/text/template/#Template.ParseFiles - nginxTemplate, err := template.New(path.Base(mainTemplatePath)).ParseFiles(mainTemplatePath) + nginxTemplate, err := template.New(path.Base(mainTemplatePath)).Funcs(helperFunctions).ParseFiles(mainTemplatePath) if err != nil { return nil, err } @@ -33,7 +33,7 @@ func NewTemplateExecutor(mainTemplatePath string, ingressTemplatePath string) (* // UpdateMainTemplate updates the main NGINX template. func (te *TemplateExecutor) UpdateMainTemplate(templateString *string) error { - newTemplate, err := template.New("nginxTemplate").Parse(*templateString) + newTemplate, err := template.New("nginxTemplate").Funcs(helperFunctions).Parse(*templateString) if err != nil { return err } diff --git a/internal/configs/version1/template_helper.go b/internal/configs/version1/template_helper.go index 4b18903bc5..b93d2e80bd 100644 --- a/internal/configs/version1/template_helper.go +++ b/internal/configs/version1/template_helper.go @@ -4,6 +4,8 @@ import ( "fmt" "strings" "text/template" + + "github.com/nginxinc/kubernetes-ingress/internal/configs/commonhelpers" ) func split(s string, delim string) []string { @@ -71,4 +73,5 @@ var helperFunctions = template.FuncMap{ "toLower": strings.ToLower, "toUpper": strings.ToUpper, "makeLocationPath": makeLocationPath, + "makeSecretPath": commonhelpers.MakeSecretPath, } diff --git a/internal/configs/version1/template_test.go b/internal/configs/version1/template_test.go index 2285b60ece..395dcd77a8 100644 --- a/internal/configs/version1/template_test.go +++ b/internal/configs/version1/template_test.go @@ -977,7 +977,7 @@ func newNGINXIngressTmpl(t *testing.T) *template.Template { func newNGINXPlusMainTmpl(t *testing.T) *template.Template { t.Helper() - tmpl, err := template.New("nginx-plus.tmpl").ParseFiles("nginx-plus.tmpl") + tmpl, err := template.New("nginx-plus.tmpl").Funcs(helperFunctions).ParseFiles("nginx-plus.tmpl") if err != nil { t.Fatal(err) } @@ -986,7 +986,7 @@ func newNGINXPlusMainTmpl(t *testing.T) *template.Template { func newNGINXMainTmpl(t *testing.T) *template.Template { t.Helper() - tmpl, err := template.New("nginx.tmpl").ParseFiles("nginx.tmpl") + tmpl, err := template.New("nginx.tmpl").Funcs(helperFunctions).ParseFiles("nginx.tmpl") if err != nil { t.Fatal(err) } diff --git a/internal/configs/version2/http.go b/internal/configs/version2/http.go index 12bbf85995..a7fbf3a6da 100644 --- a/internal/configs/version2/http.go +++ b/internal/configs/version2/http.go @@ -12,15 +12,17 @@ type UpstreamLabels struct { // VirtualServerConfig holds NGINX configuration for a VirtualServer. type VirtualServerConfig struct { - HTTPSnippets []string - LimitReqZones []LimitReqZone - Maps []Map - Server Server - SpiffeCerts bool - SpiffeClientCerts bool - SplitClients []SplitClient - StatusMatches []StatusMatch - Upstreams []Upstream + HTTPSnippets []string + LimitReqZones []LimitReqZone + Maps []Map + Server Server + SpiffeCerts bool + SpiffeClientCerts bool + SplitClients []SplitClient + StatusMatches []StatusMatch + Upstreams []Upstream + DynamicSSLReloadEnabled bool + StaticSSLPath string } // Upstream defines an upstream. diff --git a/internal/configs/version2/nginx-plus.transportserver.tmpl b/internal/configs/version2/nginx-plus.transportserver.tmpl index 4f45a91e5f..f9cf80b3c9 100644 --- a/internal/configs/version2/nginx-plus.transportserver.tmpl +++ b/internal/configs/version2/nginx-plus.transportserver.tmpl @@ -41,9 +41,9 @@ server { {{- end }} {{- if $ssl.Enabled }} - ssl_certificate {{ $ssl.Certificate }}; - ssl_certificate_key {{ $ssl.CertificateKey }}; - {{- end }} + ssl_certificate {{ makeSecretPath $ssl.Certificate $.StaticSSLPath "$secret_dir_path" $.DynamicSSLReloadEnabled }}; + ssl_certificate_key {{ makeSecretPath $ssl.CertificateKey $.StaticSSLPath "$secret_dir_path" $.DynamicSSLReloadEnabled }}; + {{- end }} {{- end }} status_zone {{ $s.StatusZone }}; diff --git a/internal/configs/version2/nginx-plus.virtualserver.tmpl b/internal/configs/version2/nginx-plus.virtualserver.tmpl index 5a7f512c92..e4207446e3 100644 --- a/internal/configs/version2/nginx-plus.virtualserver.tmpl +++ b/internal/configs/version2/nginx-plus.virtualserver.tmpl @@ -106,18 +106,18 @@ server { {{- if $ssl.RejectHandshake }} ssl_reject_handshake on; {{- else if $.SpiffeCerts }} - ssl_certificate /etc/nginx/secrets/spiffe_cert.pem; - ssl_certificate_key /etc/nginx/secrets/spiffe_key.pem; - {{- else }} - ssl_certificate {{ $ssl.Certificate }}; - ssl_certificate_key {{ $ssl.CertificateKey }}; + ssl_certificate {{ makeSecretPath "/etc/nginx/secrets/spiffe_cert.pem" $.StaticSSLPath "$secret_dir_path" $.DynamicSSLReloadEnabled }}; + ssl_certificate_key {{ makeSecretPath "/etc/nginx/secrets/spiffe_key.pem" $.StaticSSLPath "$secret_dir_path" $.DynamicSSLReloadEnabled }}; + {{- else }} + ssl_certificate {{ makeSecretPath $ssl.Certificate $.StaticSSLPath "$secret_dir_path" $.DynamicSSLReloadEnabled }}; + ssl_certificate_key {{ makeSecretPath $ssl.CertificateKey $.StaticSSLPath "$secret_dir_path" $.DynamicSSLReloadEnabled }}; {{- end }} {{- else }} {{- if $.SpiffeCerts }} listen 443 ssl; {{if not $s.DisableIPV6}}listen [::]:443 ssl;{{end}} - ssl_certificate /etc/nginx/secrets/spiffe_cert.pem; - ssl_certificate_key /etc/nginx/secrets/spiffe_key.pem; + ssl_certificate {{ makeSecretPath "/etc/nginx/secrets/spiffe_cert.pem" $.StaticSSLPath "$secret_dir_path" $.DynamicSSLReloadEnabled }}; + ssl_certificate_key {{ makeSecretPath "/etc/nginx/secrets/spiffe_key.pem" $.StaticSSLPath "$secret_dir_path" $.DynamicSSLReloadEnabled }}; {{- end }} {{- end }} @@ -216,8 +216,8 @@ server { {{- with $s.EgressMTLS }} {{- if .Certificate }} - proxy_ssl_certificate {{ .Certificate }}; - proxy_ssl_certificate_key {{ .CertificateKey }}; + proxy_ssl_certificate {{ makeSecretPath .Certificate $.StaticSSLPath "$secret_dir_path" $.DynamicSSLReloadEnabled }}; + proxy_ssl_certificate_key {{ makeSecretPath .CertificateKey $.StaticSSLPath "$secret_dir_path" $.DynamicSSLReloadEnabled }}; {{- end }} {{- if .TrustedCert }} proxy_ssl_trusted_certificate {{ .TrustedCert }}; @@ -403,8 +403,8 @@ server { {{- with $l.EgressMTLS }} {{- if .Certificate }} - {{ $proxyOrGRPC }}_ssl_certificate {{ .Certificate }}; - {{ $proxyOrGRPC }}_ssl_certificate_key {{ .CertificateKey }}; + {{ $proxyOrGRPC }}_ssl_certificate {{ makeSecretPath .Certificate $.StaticSSLPath "$secret_dir_path" $.DynamicSSLReloadEnabled }}; + {{ $proxyOrGRPC }}_ssl_certificate_key {{ makeSecretPath .CertificateKey $.StaticSSLPath "$secret_dir_path" $.DynamicSSLReloadEnabled }}; {{- end }} {{ if .TrustedCert }} {{ $proxyOrGRPC }}_ssl_trusted_certificate {{ .TrustedCert }}; @@ -577,8 +577,8 @@ server { add_header {{ $h.Name }} "{{ $h.Value }}" {{ if $h.Always }}always{{ end }}; {{- end }} {{- if $.SpiffeClientCerts }} - {{ $proxyOrGRPC }}_ssl_certificate /etc/nginx/secrets/spiffe_cert.pem; - {{ $proxyOrGRPC }}_ssl_certificate_key /etc/nginx/secrets/spiffe_key.pem; + {{ $proxyOrGRPC }}_ssl_certificate {{ makeSecretPath "/etc/nginx/secrets/spiffe_cert.pem" $.StaticSSLPath "$secret_dir_path" $.DynamicSSLReloadEnabled }}; + {{ $proxyOrGRPC }}_ssl_certificate_key {{ makeSecretPath "/etc/nginx/secrets/spiffe_key.pem" $.StaticSSLPath "$secret_dir_path" $.DynamicSSLReloadEnabled }}; {{ $proxyOrGRPC }}_ssl_trusted_certificate /etc/nginx/secrets/spiffe_rootca.pem; {{ $proxyOrGRPC }}_ssl_server_name on; {{ $proxyOrGRPC }}_ssl_verify on; diff --git a/internal/configs/version2/stream.go b/internal/configs/version2/stream.go index 86dc094c87..3b7d79f2c5 100644 --- a/internal/configs/version2/stream.go +++ b/internal/configs/version2/stream.go @@ -2,11 +2,13 @@ package version2 // TransportServerConfig holds NGINX configuration for a TransportServer. type TransportServerConfig struct { - Server StreamServer - Upstreams []StreamUpstream - StreamSnippets []string - Match *Match - DisableIPV6 bool + Server StreamServer + Upstreams []StreamUpstream + StreamSnippets []string + Match *Match + DisableIPV6 bool + DynamicSSLReloadEnabled bool + StaticSSLPath string } // StreamUpstream defines a stream upstream. diff --git a/internal/configs/version2/template_executor.go b/internal/configs/version2/template_executor.go index 473fb8840e..d7a4989ab6 100644 --- a/internal/configs/version2/template_executor.go +++ b/internal/configs/version2/template_executor.go @@ -29,7 +29,7 @@ func NewTemplateExecutor(virtualServerTemplatePath string, transportServerTempla return nil, err } - tsTemplate, err := template.New(path.Base(transportServerTemplatePath)).ParseFiles(transportServerTemplatePath) + tsTemplate, err := template.New(path.Base(transportServerTemplatePath)).Funcs(helperFunctions).ParseFiles(transportServerTemplatePath) if err != nil { return nil, err } diff --git a/internal/configs/version2/template_helper.go b/internal/configs/version2/template_helper.go index c09e26a9ce..997dd965e0 100644 --- a/internal/configs/version2/template_helper.go +++ b/internal/configs/version2/template_helper.go @@ -4,6 +4,8 @@ import ( "strconv" "strings" "text/template" + + "github.com/nginxinc/kubernetes-ingress/internal/configs/commonhelpers" ) type protocol int @@ -129,4 +131,5 @@ var helperFunctions = template.FuncMap{ "toUpper": strings.ToUpper, "makeHTTPListener": makeHTTPListener, "makeHTTPSListener": makeHTTPSListener, + "makeSecretPath": commonhelpers.MakeSecretPath, } diff --git a/internal/configs/version2/template_helper_test.go b/internal/configs/version2/template_helper_test.go index a96855e792..87890f702c 100644 --- a/internal/configs/version2/template_helper_test.go +++ b/internal/configs/version2/template_helper_test.go @@ -310,3 +310,51 @@ func newToUpperTemplate(t *testing.T) *template.Template { } return tmpl } + +func TestMakeSecretPath(t *testing.T) { + t.Parallel() + + tmpl := newMakeSecretPathTemplate(t) + testCases := []struct { + Secret string + Path string + Variable string + Enabled bool + expected string + }{ + { + Secret: "/etc/nginx/secret/thing.crt", + Path: "/etc/nginx/secret", + Variable: "$secrets_path", + Enabled: true, + expected: "$secrets_path/thing.crt", + }, + { + Secret: "/etc/nginx/secret/thing.crt", + Path: "/etc/nginx/secret", + Variable: "$secrets_path", + Enabled: false, + expected: "/etc/nginx/secret/thing.crt", + }, + } + + for _, tc := range testCases { + var buf bytes.Buffer + err := tmpl.Execute(&buf, tc) + if err != nil { + t.Fatalf("Failed to execute the template %v", err) + } + if buf.String() != tc.expected { + t.Errorf("Template generated wrong config, got '%v' but expected '%v'.", buf.String(), tc.expected) + } + } +} + +func newMakeSecretPathTemplate(t *testing.T) *template.Template { + t.Helper() + tmpl, err := template.New("testTemplate").Funcs(helperFunctions).Parse(`{{makeSecretPath .Secret .Path .Variable .Enabled}}`) + if err != nil { + t.Fatalf("Failed to parse template: %v", err) + } + return tmpl +} diff --git a/internal/configs/version2/templates_test.go b/internal/configs/version2/templates_test.go index 9e9968c6c0..97cfd7c45b 100644 --- a/internal/configs/version2/templates_test.go +++ b/internal/configs/version2/templates_test.go @@ -339,6 +339,16 @@ func TestTransportServerForNginx(t *testing.T) { t.Log(string(data)) } +func TestTransportServerWithSSL(t *testing.T) { + t.Parallel() + executor := newTmplExecutorNGINXPlus(t) + data, err := executor.ExecuteTransportServerTemplate(&transportServerCfgWithSSL) + if err != nil { + t.Errorf("Failed to execute template: %v", err) + } + t.Log(string(data)) +} + func TestTLSPassthroughHosts(t *testing.T) { t.Parallel() executor := newTmplExecutorNGINX(t) @@ -4220,4 +4230,51 @@ var ( }, }, } + + transportServerCfgWithSSL = TransportServerConfig{ + Upstreams: []StreamUpstream{ + { + Name: "udp-upstream", + Servers: []StreamUpstreamServer{ + { + Address: "10.0.0.20:5001", + }, + }, + }, + }, + Match: &Match{ + Name: "match_udp-upstream", + Send: `GET / HTTP/1.0\r\nHost: localhost\r\n\r\n`, + ExpectRegexModifier: "~*", + Expect: "200 OK", + }, + Server: StreamServer{ + Port: 1234, + UDP: true, + StatusZone: "udp-app", + ProxyRequests: createPointerFromInt(1), + ProxyResponses: createPointerFromInt(2), + ProxyPass: "udp-upstream", + ProxyTimeout: "10s", + ProxyConnectTimeout: "10s", + ProxyNextUpstream: true, + ProxyNextUpstreamTimeout: "10s", + ProxyNextUpstreamTries: 5, + HealthCheck: &StreamHealthCheck{ + Enabled: false, + Timeout: "5s", + Jitter: "0", + Port: 8080, + Interval: "5s", + Passes: 1, + Fails: 1, + Match: "match_udp-upstream", + }, + SSL: &StreamSSL{ + Enabled: true, + Certificate: "cafe-secret.pem", + CertificateKey: "cafe-secret.pem", + }, + }, + } ) diff --git a/internal/configs/virtualserver.go b/internal/configs/virtualserver.go index 95a59d6841..0c4c373319 100644 --- a/internal/configs/virtualserver.go +++ b/internal/configs/virtualserver.go @@ -3,6 +3,7 @@ package configs import ( "fmt" "net/url" + "sort" "strconv" "strings" @@ -226,17 +227,19 @@ func newHealthCheckWithDefaults(upstream conf_v1.Upstream, upstreamName string, // VirtualServerConfigurator generates a VirtualServer configuration type virtualServerConfigurator struct { - cfgParams *ConfigParams - isPlus bool - isWildcardEnabled bool - isResolverConfigured bool - isTLSPassthrough bool - enableSnippets bool - warnings Warnings - spiffeCerts bool - enableInternalRoutes bool - oidcPolCfg *oidcPolicyCfg - isIPV6Disabled bool + cfgParams *ConfigParams + isPlus bool + isWildcardEnabled bool + isResolverConfigured bool + isTLSPassthrough bool + enableSnippets bool + warnings Warnings + spiffeCerts bool + enableInternalRoutes bool + oidcPolCfg *oidcPolicyCfg + isIPV6Disabled bool + DynamicSSLReloadEnabled bool + StaticSSLPath string } type oidcPolicyCfg struct { @@ -267,17 +270,19 @@ func newVirtualServerConfigurator( isWildcardEnabled bool, ) *virtualServerConfigurator { return &virtualServerConfigurator{ - cfgParams: cfgParams, - isPlus: isPlus, - isWildcardEnabled: isWildcardEnabled, - isResolverConfigured: isResolverConfigured, - isTLSPassthrough: staticParams.TLSPassthrough, - enableSnippets: staticParams.EnableSnippets, - warnings: make(map[runtime.Object][]string), - spiffeCerts: staticParams.NginxServiceMesh, - enableInternalRoutes: staticParams.EnableInternalRoutes, - oidcPolCfg: &oidcPolicyCfg{}, - isIPV6Disabled: staticParams.DisableIPV6, + cfgParams: cfgParams, + isPlus: isPlus, + isWildcardEnabled: isWildcardEnabled, + isResolverConfigured: isResolverConfigured, + isTLSPassthrough: staticParams.TLSPassthrough, + enableSnippets: staticParams.EnableSnippets, + warnings: make(map[runtime.Object][]string), + spiffeCerts: staticParams.NginxServiceMesh, + enableInternalRoutes: staticParams.EnableInternalRoutes, + oidcPolCfg: &oidcPolicyCfg{}, + isIPV6Disabled: staticParams.DisableIPV6, + DynamicSSLReloadEnabled: staticParams.DynamicSSLReload, + StaticSSLPath: staticParams.StaticSSLPath, } } @@ -683,6 +688,10 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( vsc.cfgParams.ServerSnippets, ) + sort.Slice(upstreams, func(i, j int) bool { + return upstreams[i].Name < upstreams[j].Name + }) + vsCfg := version2.VirtualServerConfig{ Upstreams: upstreams, SplitClients: splitClients, @@ -729,8 +738,10 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( VSName: vsEx.VirtualServer.Name, DisableIPV6: vsc.isIPV6Disabled, }, - SpiffeCerts: enabledInternalRoutes, - SpiffeClientCerts: vsc.spiffeCerts && !enabledInternalRoutes, + SpiffeCerts: enabledInternalRoutes, + SpiffeClientCerts: vsc.spiffeCerts && !enabledInternalRoutes, + DynamicSSLReloadEnabled: vsc.DynamicSSLReloadEnabled, + StaticSSLPath: vsc.StaticSSLPath, } return vsCfg, vsc.warnings @@ -1414,6 +1425,9 @@ func (vsc *virtualServerConfigurator) generateUpstream( } upsServers = append(upsServers, s) } + sort.Slice(upsServers, func(i, j int) bool { + return upsServers[i].Address < upsServers[j].Address + }) lbMethod := generateLBMethod(upstream.LBMethod, vsc.cfgParams.LBMethod) diff --git a/internal/configs/virtualserver_test.go b/internal/configs/virtualserver_test.go index f72172ebc4..647d50470e 100644 --- a/internal/configs/virtualserver_test.go +++ b/internal/configs/virtualserver_test.go @@ -206,15 +206,15 @@ func TestGenerateVSConfig_GeneratesConfigWithGunzipOn(t *testing.T) { Upstreams: []version2.Upstream{ { UpstreamLabels: version2.UpstreamLabels{ - Service: "tea-svc", + Service: "coffee-svc", ResourceType: "virtualserver", ResourceName: "cafe", ResourceNamespace: "default", }, - Name: "vs_default_cafe_tea", + Name: "vs_default_cafe_coffee", Servers: []version2.UpstreamServer{ { - Address: "10.0.0.20:80", + Address: "10.0.0.40:80", }, }, Keepalive: 16, @@ -226,25 +226,25 @@ func TestGenerateVSConfig_GeneratesConfigWithGunzipOn(t *testing.T) { ResourceName: "cafe", ResourceNamespace: "default", }, - Name: "vs_default_cafe_tea-latest", + Name: "vs_default_cafe_tea", Servers: []version2.UpstreamServer{ { - Address: "10.0.0.30:80", + Address: "10.0.0.20:80", }, }, Keepalive: 16, }, { UpstreamLabels: version2.UpstreamLabels{ - Service: "coffee-svc", + Service: "tea-svc", ResourceType: "virtualserver", ResourceName: "cafe", ResourceNamespace: "default", }, - Name: "vs_default_cafe_coffee", + Name: "vs_default_cafe_tea-latest", Servers: []version2.UpstreamServer{ { - Address: "10.0.0.40:80", + Address: "10.0.0.30:80", }, }, Keepalive: 16, @@ -266,30 +266,30 @@ func TestGenerateVSConfig_GeneratesConfigWithGunzipOn(t *testing.T) { }, { UpstreamLabels: version2.UpstreamLabels{ - Service: "sub-tea-svc", + Service: "coffee-svc", ResourceType: "virtualserverroute", - ResourceName: "subtea", + ResourceName: "subcoffee", ResourceNamespace: "default", }, - Name: "vs_default_cafe_vsr_default_subtea_subtea", + Name: "vs_default_cafe_vsr_default_subcoffee_coffee", Servers: []version2.UpstreamServer{ { - Address: "10.0.0.50:80", + Address: "10.0.0.40:80", }, }, Keepalive: 16, }, { UpstreamLabels: version2.UpstreamLabels{ - Service: "coffee-svc", + Service: "sub-tea-svc", ResourceType: "virtualserverroute", - ResourceName: "subcoffee", + ResourceName: "subtea", ResourceNamespace: "default", }, - Name: "vs_default_cafe_vsr_default_subcoffee_coffee", + Name: "vs_default_cafe_vsr_default_subtea_subtea", Servers: []version2.UpstreamServer{ { - Address: "10.0.0.40:80", + Address: "10.0.0.50:80", }, }, Keepalive: 16, @@ -464,15 +464,15 @@ func TestGenerateVSConfig_GeneratesConfigWithGunzipOff(t *testing.T) { Upstreams: []version2.Upstream{ { UpstreamLabels: version2.UpstreamLabels{ - Service: "tea-svc", + Service: "coffee-svc", ResourceType: "virtualserver", ResourceName: "cafe", ResourceNamespace: "default", }, - Name: "vs_default_cafe_tea", + Name: "vs_default_cafe_coffee", Servers: []version2.UpstreamServer{ { - Address: "10.0.0.20:80", + Address: "10.0.0.40:80", }, }, Keepalive: 16, @@ -484,25 +484,25 @@ func TestGenerateVSConfig_GeneratesConfigWithGunzipOff(t *testing.T) { ResourceName: "cafe", ResourceNamespace: "default", }, - Name: "vs_default_cafe_tea-latest", + Name: "vs_default_cafe_tea", Servers: []version2.UpstreamServer{ { - Address: "10.0.0.30:80", + Address: "10.0.0.20:80", }, }, Keepalive: 16, }, { UpstreamLabels: version2.UpstreamLabels{ - Service: "coffee-svc", + Service: "tea-svc", ResourceType: "virtualserver", ResourceName: "cafe", ResourceNamespace: "default", }, - Name: "vs_default_cafe_coffee", + Name: "vs_default_cafe_tea-latest", Servers: []version2.UpstreamServer{ { - Address: "10.0.0.40:80", + Address: "10.0.0.30:80", }, }, Keepalive: 16, @@ -524,30 +524,30 @@ func TestGenerateVSConfig_GeneratesConfigWithGunzipOff(t *testing.T) { }, { UpstreamLabels: version2.UpstreamLabels{ - Service: "sub-tea-svc", + Service: "coffee-svc", ResourceType: "virtualserverroute", - ResourceName: "subtea", + ResourceName: "subcoffee", ResourceNamespace: "default", }, - Name: "vs_default_cafe_vsr_default_subtea_subtea", + Name: "vs_default_cafe_vsr_default_subcoffee_coffee", Servers: []version2.UpstreamServer{ { - Address: "10.0.0.50:80", + Address: "10.0.0.40:80", }, }, Keepalive: 16, }, { UpstreamLabels: version2.UpstreamLabels{ - Service: "coffee-svc", + Service: "sub-tea-svc", ResourceType: "virtualserverroute", - ResourceName: "subcoffee", + ResourceName: "subtea", ResourceNamespace: "default", }, - Name: "vs_default_cafe_vsr_default_subcoffee_coffee", + Name: "vs_default_cafe_vsr_default_subtea_subtea", Servers: []version2.UpstreamServer{ { - Address: "10.0.0.40:80", + Address: "10.0.0.50:80", }, }, Keepalive: 16, @@ -722,15 +722,15 @@ func TestGenerateVSConfig_GeneratesConfigWithNoGunzip(t *testing.T) { Upstreams: []version2.Upstream{ { UpstreamLabels: version2.UpstreamLabels{ - Service: "tea-svc", + Service: "coffee-svc", ResourceType: "virtualserver", ResourceName: "cafe", ResourceNamespace: "default", }, - Name: "vs_default_cafe_tea", + Name: "vs_default_cafe_coffee", Servers: []version2.UpstreamServer{ { - Address: "10.0.0.20:80", + Address: "10.0.0.40:80", }, }, Keepalive: 16, @@ -742,25 +742,25 @@ func TestGenerateVSConfig_GeneratesConfigWithNoGunzip(t *testing.T) { ResourceName: "cafe", ResourceNamespace: "default", }, - Name: "vs_default_cafe_tea-latest", + Name: "vs_default_cafe_tea", Servers: []version2.UpstreamServer{ { - Address: "10.0.0.30:80", + Address: "10.0.0.20:80", }, }, Keepalive: 16, }, { UpstreamLabels: version2.UpstreamLabels{ - Service: "coffee-svc", + Service: "tea-svc", ResourceType: "virtualserver", ResourceName: "cafe", ResourceNamespace: "default", }, - Name: "vs_default_cafe_coffee", + Name: "vs_default_cafe_tea-latest", Servers: []version2.UpstreamServer{ { - Address: "10.0.0.40:80", + Address: "10.0.0.30:80", }, }, Keepalive: 16, @@ -782,30 +782,30 @@ func TestGenerateVSConfig_GeneratesConfigWithNoGunzip(t *testing.T) { }, { UpstreamLabels: version2.UpstreamLabels{ - Service: "sub-tea-svc", + Service: "coffee-svc", ResourceType: "virtualserverroute", - ResourceName: "subtea", + ResourceName: "subcoffee", ResourceNamespace: "default", }, - Name: "vs_default_cafe_vsr_default_subtea_subtea", + Name: "vs_default_cafe_vsr_default_subcoffee_coffee", Servers: []version2.UpstreamServer{ { - Address: "10.0.0.50:80", + Address: "10.0.0.40:80", }, }, Keepalive: 16, }, { UpstreamLabels: version2.UpstreamLabels{ - Service: "coffee-svc", + Service: "sub-tea-svc", ResourceType: "virtualserverroute", - ResourceName: "subcoffee", + ResourceName: "subtea", ResourceNamespace: "default", }, - Name: "vs_default_cafe_vsr_default_subcoffee_coffee", + Name: "vs_default_cafe_vsr_default_subtea_subtea", Servers: []version2.UpstreamServer{ { - Address: "10.0.0.40:80", + Address: "10.0.0.50:80", }, }, Keepalive: 16, @@ -1178,15 +1178,15 @@ func TestGenerateVirtualServerConfig(t *testing.T) { Upstreams: []version2.Upstream{ { UpstreamLabels: version2.UpstreamLabels{ - Service: "tea-svc", + Service: "coffee-svc", ResourceType: "virtualserver", ResourceName: "cafe", ResourceNamespace: "default", }, - Name: "vs_default_cafe_tea", + Name: "vs_default_cafe_coffee", Servers: []version2.UpstreamServer{ { - Address: "10.0.0.20:80", + Address: "10.0.0.40:80", }, }, Keepalive: 16, @@ -1198,25 +1198,25 @@ func TestGenerateVirtualServerConfig(t *testing.T) { ResourceName: "cafe", ResourceNamespace: "default", }, - Name: "vs_default_cafe_tea-latest", + Name: "vs_default_cafe_tea", Servers: []version2.UpstreamServer{ { - Address: "10.0.0.30:80", + Address: "10.0.0.20:80", }, }, Keepalive: 16, }, { UpstreamLabels: version2.UpstreamLabels{ - Service: "coffee-svc", + Service: "tea-svc", ResourceType: "virtualserver", ResourceName: "cafe", ResourceNamespace: "default", }, - Name: "vs_default_cafe_coffee", + Name: "vs_default_cafe_tea-latest", Servers: []version2.UpstreamServer{ { - Address: "10.0.0.40:80", + Address: "10.0.0.30:80", }, }, Keepalive: 16, @@ -1238,30 +1238,30 @@ func TestGenerateVirtualServerConfig(t *testing.T) { }, { UpstreamLabels: version2.UpstreamLabels{ - Service: "sub-tea-svc", + Service: "coffee-svc", ResourceType: "virtualserverroute", - ResourceName: "subtea", + ResourceName: "subcoffee", ResourceNamespace: "default", }, - Name: "vs_default_cafe_vsr_default_subtea_subtea", + Name: "vs_default_cafe_vsr_default_subcoffee_coffee", Servers: []version2.UpstreamServer{ { - Address: "10.0.0.50:80", + Address: "10.0.0.40:80", }, }, Keepalive: 16, }, { UpstreamLabels: version2.UpstreamLabels{ - Service: "coffee-svc", + Service: "sub-tea-svc", ResourceType: "virtualserverroute", - ResourceName: "subcoffee", + ResourceName: "subtea", ResourceNamespace: "default", }, - Name: "vs_default_cafe_vsr_default_subcoffee_coffee", + Name: "vs_default_cafe_vsr_default_subtea_subtea", Servers: []version2.UpstreamServer{ { - Address: "10.0.0.40:80", + Address: "10.0.0.50:80", }, }, Keepalive: 16, @@ -1687,29 +1687,29 @@ func TestGenerateVirtualServerConfigIPV6Disabled(t *testing.T) { Upstreams: []version2.Upstream{ { UpstreamLabels: version2.UpstreamLabels{ - Service: "tea-svc", + Service: "coffee-svc", ResourceType: "virtualserver", ResourceName: "cafe", ResourceNamespace: "default", }, - Name: "vs_default_cafe_tea", + Name: "vs_default_cafe_coffee", Servers: []version2.UpstreamServer{ { - Address: "10.0.0.20:80", + Address: "10.0.0.40:80", }, }, }, { UpstreamLabels: version2.UpstreamLabels{ - Service: "coffee-svc", + Service: "tea-svc", ResourceType: "virtualserver", ResourceName: "cafe", ResourceNamespace: "default", }, - Name: "vs_default_cafe_coffee", + Name: "vs_default_cafe_tea", Servers: []version2.UpstreamServer{ { - Address: "10.0.0.40:80", + Address: "10.0.0.20:80", }, }, }, @@ -3974,30 +3974,30 @@ func TestGenerateVirtualServerConfigJWKSPolicy(t *testing.T) { Upstreams: []version2.Upstream{ { UpstreamLabels: version2.UpstreamLabels{ - Service: "tea-svc", + Service: "coffee-svc", ResourceType: "virtualserver", ResourceName: "cafe", ResourceNamespace: "default", }, - Name: "vs_default_cafe_tea", + Name: "vs_default_cafe_coffee", Servers: []version2.UpstreamServer{ { - Address: "10.0.0.20:80", + Address: "10.0.0.30:80", }, }, Keepalive: 16, }, { UpstreamLabels: version2.UpstreamLabels{ - Service: "coffee-svc", + Service: "tea-svc", ResourceType: "virtualserver", ResourceName: "cafe", ResourceNamespace: "default", }, - Name: "vs_default_cafe_coffee", + Name: "vs_default_cafe_tea", Servers: []version2.UpstreamServer{ { - Address: "10.0.0.30:80", + Address: "10.0.0.20:80", }, }, Keepalive: 16, diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index 7cd64a0b31..1e8184a5f5 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -927,7 +927,7 @@ func (lbc *LoadBalancerController) updateAllConfigs() { } gc := lbc.configuration.GetGlobalConfiguration() - if gc != nil { + if gc != nil && lbc.configMap != nil { key := getResourceKey(&lbc.configMap.ObjectMeta) lbc.recorder.Eventf(gc, eventType, eventTitle, fmt.Sprintf("GlobalConfiguration %s was updated %s", key, eventWarningMessage)) } @@ -2388,7 +2388,7 @@ func (lbc *LoadBalancerController) syncService(task task) { resourceExes := lbc.createExtendedResources(resources) - warnings, updateErr := lbc.configurator.AddOrUpdateResources(resourceExes) + warnings, updateErr := lbc.configurator.AddOrUpdateResources(resourceExes, true) lbc.updateResourcesStatusAndEvents(resources, warnings, updateErr) } @@ -2505,7 +2505,7 @@ func (lbc *LoadBalancerController) isSpecialSecret(secretName string) bool { func (lbc *LoadBalancerController) handleRegularSecretDeletion(resources []Resource) { resourceExes := lbc.createExtendedResources(resources) - warnings, addOrUpdateErr := lbc.configurator.AddOrUpdateResources(resourceExes) + warnings, addOrUpdateErr := lbc.configurator.AddOrUpdateResources(resourceExes, true) lbc.updateResourcesStatusAndEvents(resources, warnings, addOrUpdateErr) } @@ -2517,8 +2517,8 @@ func (lbc *LoadBalancerController) handleSecretUpdate(secret *api_v1.Secret, res var addOrUpdateErr error resourceExes := lbc.createExtendedResources(resources) - warnings, addOrUpdateErr = lbc.configurator.AddOrUpdateResources(resourceExes) + warnings, addOrUpdateErr = lbc.configurator.AddOrUpdateResources(resourceExes, !lbc.configurator.DynamicSSLReloadEnabled()) if addOrUpdateErr != nil { glog.Errorf("Error when updating Secret %v: %v", secretNsName, addOrUpdateErr) lbc.recorder.Eventf(secret, api_v1.EventTypeWarning, "UpdatedWithError", "%v was updated, but not applied: %v", secretNsName, addOrUpdateErr) diff --git a/internal/k8s/reference_checkers.go b/internal/k8s/reference_checkers.go index 237e06c4f4..c05af132e4 100644 --- a/internal/k8s/reference_checkers.go +++ b/internal/k8s/reference_checkers.go @@ -90,7 +90,15 @@ func (rc *secretReferenceChecker) IsReferencedByVirtualServerRoute(_ string, _ s return false } -func (rc *secretReferenceChecker) IsReferencedByTransportServer(_ string, _ string, _ *conf_v1.TransportServer) bool { +func (rc *secretReferenceChecker) IsReferencedByTransportServer(secretNamespace string, secretName string, ts *conf_v1.TransportServer) bool { + if ts.Namespace != secretNamespace { + return false + } + + if ts.Spec.TLS != nil && ts.Spec.TLS.Secret == secretName { + return true + } + return false } diff --git a/internal/k8s/reference_checkers_test.go b/internal/k8s/reference_checkers_test.go index ba2a4b2892..eec2988ef8 100644 --- a/internal/k8s/reference_checkers_test.go +++ b/internal/k8s/reference_checkers_test.go @@ -313,13 +313,68 @@ func TestSecretIsReferencedByVirtualServerRoute(t *testing.T) { func TestSecretIsReferencedByTransportServer(t *testing.T) { t.Parallel() - isPlus := false // doesn't matter for TransportServer - rc := newSecretReferenceChecker(isPlus) + tests := []struct { + ts *conf_v1.TransportServer + secretNamespace string + secretName string + expected bool + msg string + }{ + { + ts: &conf_v1.TransportServer{ + ObjectMeta: v1.ObjectMeta{ + Namespace: "default", + }, + Spec: conf_v1.TransportServerSpec{ + TLS: &conf_v1.TransportServerTLS{ + Secret: "test-secret", + }, + }, + }, + secretNamespace: "default", + secretName: "test-secret", + expected: true, + msg: "tls secret is referenced", + }, + { + ts: &conf_v1.TransportServer{ + ObjectMeta: v1.ObjectMeta{ + Namespace: "default", + }, + Spec: conf_v1.TransportServerSpec{ + TLS: &conf_v1.TransportServerTLS{ + Secret: "test-secret", + }, + }, + }, + secretNamespace: "other-namespace", + secretName: "test-secret", + expected: false, + msg: "tls secret is referenced but in another namespace", + }, + { + ts: &conf_v1.TransportServer{ + ObjectMeta: v1.ObjectMeta{ + Namespace: "default", + }, + Spec: conf_v1.TransportServerSpec{}, + }, + secretNamespace: "other-namespace", + secretName: "test-secret", + expected: false, + msg: "tls secret is not but in another namespace", + }, + } - // always returns false - result := rc.IsReferencedByTransportServer("", "", nil) - if result != false { - t.Error("IsReferencedByTransportServer() returned true but expected false") + for _, test := range tests { + isPlus := false // doesn't matter for TransportServer + rc := newSecretReferenceChecker(isPlus) + + // always returns false + result := rc.IsReferencedByTransportServer(test.secretNamespace, test.secretName, test.ts) + if result != test.expected { + t.Errorf("IsReferencedByTransportServer() returned %v but expected %v for the case of %s", result, test.expected, test.msg) + } } } diff --git a/internal/nginx/fake_manager.go b/internal/nginx/fake_manager.go index ad3da7505d..5533fb9180 100644 --- a/internal/nginx/fake_manager.go +++ b/internal/nginx/fake_manager.go @@ -26,15 +26,17 @@ func NewFakeManager(confPath string) *FakeManager { } // CreateMainConfig provides a fake implementation of CreateMainConfig. -func (*FakeManager) CreateMainConfig(content []byte) { +func (*FakeManager) CreateMainConfig(content []byte) bool { glog.V(3).Info("Writing main config") glog.V(3).Info(string(content)) + return true } // CreateConfig provides a fake implementation of CreateConfig. -func (*FakeManager) CreateConfig(name string, content []byte) { +func (*FakeManager) CreateConfig(name string, content []byte) bool { glog.V(3).Infof("Writing config %v", name) glog.V(3).Info(string(content)) + return true } // CreateAppProtectResourceFile provides a fake implementation of CreateAppProtectResourceFile @@ -59,9 +61,10 @@ func (*FakeManager) DeleteConfig(name string) { } // CreateStreamConfig provides a fake implementation of CreateStreamConfig. -func (*FakeManager) CreateStreamConfig(name string, content []byte) { +func (*FakeManager) CreateStreamConfig(name string, content []byte) bool { glog.V(3).Infof("Writing stream config %v", name) glog.V(3).Info(string(content)) + return true } // DeleteStreamConfig provides a fake implementation of DeleteStreamConfig. @@ -70,8 +73,9 @@ func (*FakeManager) DeleteStreamConfig(name string) { } // CreateTLSPassthroughHostsConfig provides a fake implementation of CreateTLSPassthroughHostsConfig. -func (*FakeManager) CreateTLSPassthroughHostsConfig(_ []byte) { +func (*FakeManager) CreateTLSPassthroughHostsConfig(_ []byte) bool { glog.V(3).Infof("Writing TLS Passthrough Hosts config file") + return false } // CreateSecret provides a fake implementation of CreateSecret. @@ -99,7 +103,7 @@ func (fm *FakeManager) CreateDHParam(_ string) (string, error) { // Version provides a fake implementation of Version. func (*FakeManager) Version() string { glog.V(3).Info("Printing nginx version") - return "fake version" + return "fake version plus" } // Start provides a fake implementation of Start. @@ -169,3 +173,8 @@ func (*FakeManager) AppProtectDosAgentQuit() { func (*FakeManager) AppProtectDosAgentStart(_ chan error, _ bool, _ int, _ int, _ int) { glog.V(3).Infof("Starting FakeAppProtectDosAgent") } + +// GetSecretsDir is a fake implementation +func (fm *FakeManager) GetSecretsDir() string { + return fm.secretsPath +} diff --git a/internal/nginx/manager.go b/internal/nginx/manager.go index 7f3348b1ca..b4ac67d110 100644 --- a/internal/nginx/manager.go +++ b/internal/nginx/manager.go @@ -7,6 +7,7 @@ import ( "os" "os/exec" "path" + "path/filepath" "strconv" "strings" "time" @@ -56,12 +57,12 @@ type ServerConfig struct { // The Manager interface updates NGINX configuration, starts, reloads and quits NGINX, // updates NGINX Plus upstream servers. type Manager interface { - CreateMainConfig(content []byte) - CreateConfig(name string, content []byte) + CreateMainConfig(content []byte) bool + CreateConfig(name string, content []byte) bool DeleteConfig(name string) - CreateStreamConfig(name string, content []byte) + CreateStreamConfig(name string, content []byte) bool DeleteStreamConfig(name string) - CreateTLSPassthroughHostsConfig(content []byte) + CreateTLSPassthroughHostsConfig(content []byte) bool CreateSecret(name string, content []byte, mode os.FileMode) string DeleteSecret(name string) CreateAppProtectResourceFile(name string, content []byte) @@ -83,6 +84,7 @@ type Manager interface { AppProtectPluginQuit() AppProtectDosAgentStart(apdaDone chan error, debug bool, maxDaemon int, maxWorkers int, memory int) AppProtectDosAgentQuit() + GetSecretsDir() string } // LocalManager updates NGINX configuration, starts, reloads and quits NGINX, @@ -133,29 +135,33 @@ func NewLocalManager(confPath string, debug bool, mc collectors.ManagerCollector } // CreateMainConfig creates the main NGINX configuration file. If the file already exists, it will be overridden. -func (lm *LocalManager) CreateMainConfig(content []byte) { +func (lm *LocalManager) CreateMainConfig(content []byte) bool { glog.V(3).Infof("Writing main config to %v", lm.mainConfFilename) glog.V(3).Infof(string(content)) + configChanged := configContentsChanged(lm.mainConfFilename, content) err := createFileAndWrite(lm.mainConfFilename, content) if err != nil { glog.Fatalf("Failed to write main config: %v", err) } + return configChanged } // CreateConfig creates a configuration file. If the file already exists, it will be overridden. -func (lm *LocalManager) CreateConfig(name string, content []byte) { - createConfig(lm.getFilenameForConfig(name), content) +func (lm *LocalManager) CreateConfig(name string, content []byte) bool { + return createConfig(lm.getFilenameForConfig(name), content) } -func createConfig(filename string, content []byte) { +func createConfig(filename string, content []byte) bool { glog.V(3).Infof("Writing config to %v", filename) glog.V(3).Info(string(content)) + configChanged := configContentsChanged(filename, content) err := createFileAndWrite(filename, content) if err != nil { glog.Fatalf("Failed to write config to %v: %v", filename, err) } + return configChanged } // DeleteConfig deletes the configuration file from the conf.d folder. @@ -177,8 +183,8 @@ func (lm *LocalManager) getFilenameForConfig(name string) string { // CreateStreamConfig creates a configuration file for stream module. // If the file already exists, it will be overridden. -func (lm *LocalManager) CreateStreamConfig(name string, content []byte) { - createConfig(lm.getFilenameForStreamConfig(name), content) +func (lm *LocalManager) CreateStreamConfig(name string, content []byte) bool { + return createConfig(lm.getFilenameForStreamConfig(name), content) } // DeleteStreamConfig deletes the configuration file from the stream-conf.d folder. @@ -193,9 +199,9 @@ func (lm *LocalManager) getFilenameForStreamConfig(name string) string { // CreateTLSPassthroughHostsConfig creates a configuration file with mapping between TLS Passthrough hosts and // the corresponding unix sockets. // If the file already exists, it will be overridden. -func (lm *LocalManager) CreateTLSPassthroughHostsConfig(content []byte) { +func (lm *LocalManager) CreateTLSPassthroughHostsConfig(content []byte) bool { glog.V(3).Infof("Writing TLS Passthrough Hosts config file to %v", lm.tlsPassthroughHostsFilename) - createConfig(lm.tlsPassthroughHostsFilename, content) + return createConfig(lm.tlsPassthroughHostsFilename, content) } // CreateSecret creates a secret file with the specified name, content and mode. If the file already exists, @@ -221,7 +227,7 @@ func (lm *LocalManager) DeleteSecret(name string) { } } -// GetFilenameForSecret constructs the filename for the secret. +// GetFilenameForSecret constructs the filename for the secret func (lm *LocalManager) GetFilenameForSecret(name string) string { return path.Join(lm.secretsPath, name) } @@ -552,3 +558,18 @@ func getBinaryFileName(debug bool) string { } return nginxBinaryPath } + +// GetSecretsDir allows the static config params to reference the secrets directory +func (lm *LocalManager) GetSecretsDir() string { + return lm.secretsPath +} + +func configContentsChanged(filename string, content []byte) bool { + filename = filepath.Clean(filename) + if currentContent, err := os.ReadFile(filename); err == nil { + if string(content) == string(currentContent) { + return false + } + } + return true +} diff --git a/tests/data/transport-server-tcp-load-balance/new-tls-secret.yaml b/tests/data/transport-server-tcp-load-balance/new-tls-secret.yaml new file mode 100644 index 0000000000..e975d21a3a --- /dev/null +++ b/tests/data/transport-server-tcp-load-balance/new-tls-secret.yaml @@ -0,0 +1,8 @@ +apiVersion: v1 +kind: Secret +metadata: + name: transport-server-tls-secret +type: kubernetes.io/tls +data: + tls.crt: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUQyakNDQXNLZ0F3SUJBZ0lKQU9BV0U3RWVwbmhyTUEwR0NTcUdTSWIzRFFFQkJRVUFNRkV4Q3pBSkJnTlYKQkFZVEFrZENNUmN3RlFZRFZRUUlFdzVEWVcxaWNtbGtaMlZ6YUdseVpURU9NQXdHQTFVRUNoTUZibWRwYm5neApHVEFYQmdOVkJBTVRFR05oWm1VdVpYaGhiWEJzWlM1amIyMHdIaGNOTVRjd056TXhNVEUxTmpVNFdoY05NVGd3Ck56TXhNVEUxTmpVNFdqQlJNUXN3Q1FZRFZRUUdFd0pIUWpFWE1CVUdBMVVFQ0JNT1EyRnRZbkpwWkdkbGMyaHAKY21VeERqQU1CZ05WQkFvVEJXNW5hVzU0TVJrd0Z3WURWUVFERXhCallXWmxMbVY0WVcxd2JHVXVZMjl0TUlJQgpJakFOQmdrcWhraUc5dzBCQVFFRkFBT0NBUThBTUlJQkNnS0NBUUVBODB0ZUlBTmM2M0dVQnU4U2hsZkh5OE1TCkVkdVE0am1CVm5BRzZ3TTZ5ZHdlbzhNS1MvZDFWSi9qWmRSMytnTVJMU0Y5d0xYU0hTaVBiK0VaOWcxc0RSUDgKWGhxaEhBcyszdnUva0Z5K1ZLYTdneWZwTGJGTkNsOS9vRStKZUxReWh3S2JwS1lrODg0ZDhKaGlhVFJvR05BRgp1S2RFTVRKc2ZHdTVOWGxYdkZxZ0lieWw3d3BCWGF6WXBjZUFoZUQ3azFMTlBNdnpaZ3llcWw0dk9qUDg4ck9vCkJ6Tm1seGdvODQ2VW5SN0c0L3NuZ0RFdVpPZXdYSWZlRVhXZDU1VTVzaFFtZWN6aExiaUducFZUNG9PWGhPWk8KYkd6ejNrY1RJOFlxOVc4eCswVkovaWZKQ0VYVitETzFEUXI2MEZDVTloSnQ3N2kwZ2NGWE9MNk5IYUVyVndJRApBUUFCbzRHME1JR3hNQjBHQTFVZERnUVdCQlJwSkg5b3RWYUtTM3dYV1IwdEZkK0hoQjRUYVRDQmdRWURWUjBqCkJIb3dlSUFVYVNSL2FMVldpa3Q4RjFrZExSWGZoNFFlRTJtaFZhUlRNRkV4Q3pBSkJnTlZCQVlUQWtkQ01SY3cKRlFZRFZRUUlFdzVEWVcxaWNtbGtaMlZ6YUdseVpURU9NQXdHQTFVRUNoTUZibWRwYm5neEdUQVhCZ05WQkFNVApFR05oWm1VdVpYaGhiWEJzWlM1amIyMkNDUURnRmhPeEhxWjRhekFNQmdOVkhSTUVCVEFEQVFIL01BMEdDU3FHClNJYjNEUUVCQlFVQUE0SUJBUUNsdEdaVE5SMFVVdVJqRVQxa1ZDdzB0QXY0YkswdFdUVDNoUVJkbmNkYjZRVGQKQkRUMEFPbjVTL1pSSytVZmJ2cllnSCsyRlZjZVE0Mzc1V2pNWGl4SUJ1QnprZlJMc01EMmZnMmlVc205NXcvUApZUFpmYW9vZjlMTVQ2V3BKYkl5N2VjWHpKZGN3aWJucnRkelc3VHljSE16ODlaZTEvR2xXRHQzZmp4YVlRTDhxCnRyWUFMWVFGU1NqZ2drdGRKeWkzRXdlem9jekV0dGFjSFYrb0xGT3F1c3EvMG83UDFabFJicXdHM3BqWUVHbUcKU3N3cVEzd25YMXpzamtBRFg2bkFCYkdaZHFaMWVZZVlIL0RTQm42ZFNGWjJaZEFRcWxIdTBJSE14aVdIa2poYwpoc1FwbUxNUFg4RTZ4SG5oT1dtbWRJVnAzbUlSdEEyQnhmYzROUWlQCi0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0K + tls.key: LS0tLS1CRUdJTiBSU0EgUFJJVkFURSBLRVktLS0tLQpNSUlFb3dJQkFBS0NBUUVBODB0ZUlBTmM2M0dVQnU4U2hsZkh5OE1TRWR1UTRqbUJWbkFHNndNNnlkd2VvOE1LClMvZDFWSi9qWmRSMytnTVJMU0Y5d0xYU0hTaVBiK0VaOWcxc0RSUDhYaHFoSEFzKzN2dS9rRnkrVkthN2d5ZnAKTGJGTkNsOS9vRStKZUxReWh3S2JwS1lrODg0ZDhKaGlhVFJvR05BRnVLZEVNVEpzZkd1NU5YbFh2RnFnSWJ5bAo3d3BCWGF6WXBjZUFoZUQ3azFMTlBNdnpaZ3llcWw0dk9qUDg4ck9vQnpObWx4Z284NDZVblI3RzQvc25nREV1ClpPZXdYSWZlRVhXZDU1VTVzaFFtZWN6aExiaUducFZUNG9PWGhPWk9iR3p6M2tjVEk4WXE5Vzh4KzBWSi9pZkoKQ0VYVitETzFEUXI2MEZDVTloSnQ3N2kwZ2NGWE9MNk5IYUVyVndJREFRQUJBb0lCQUhEeGxBaVloeEpsNzZvbwpZaGtydHZ6STJpS2dJMnBoOThFQTBMVlpFbm1UVGtZSHpVZm00UGtnSUppdFFlVTJkMHJVT1dTMUE0MjF2cURaCmh3dkt2MVp5NkwxbTcxUHRoSXBQcEdhSUozTjAwNmZYWjFCbTlyVFNFSldEVnZaSjhRcnNFd1VrZkJNU3BLT0UKbW1yc2dVYkRpMlJsZ2lxMGxkaE15ZlloRnJIQkdNYy9yWmh6ZkZucHNPSzhzRlArTTUvSVhkUjYyS21TemFrdgpLcE55TDZ5dmFUWTZ5dGwwRTd3NE1VREtGZ28wczhIdHdNdHdtWXE5aUFpdVhnOUhnOFp0VExIVnREWDlBVGRJCjliZVB0TUFDdWdEd3daZ2ppMEdTd2h0dkdmZFhleE1OaUhFWGVVaWpVMUVMWVFVWGZ3TUF0Y3hQbGF1dTZUU1YKNGZhc1lJRUNnWUVBL1pIbDBkdFgyQ25IZGJjdWExZ2xVMHo1RHZXTVMyVHFSaHNTM3JsaEZNeFJ6cGlkTHVLaApzS2xiblQyOElZNDRaUFBic1U4QVUvK0E5QWVPZmx6dG5xZ3RFemphVW4wTzJ2c2NLUUpNWXY3N0lzcStrV3BtCjZBNVIxSDdGbzk4SGRxSmRnZ3pSenVZeDRkOTYrWTk3SUJRV0psSVZ2MW56R1RGVWNPYVhlNzhDZ1lFQTlhQkMKb0hIdDRKTVlxRHJBZ1BIVnRpaWhoWDJpT3RsamxMUy9pVXVhenBSaUVhajdsZGZoTURGVG9xNkZId0ZCelErVwpCV09TNTlBQ1lYaVZKTS9nRHJRaVNWQ3QyRTFMa2dYTmVMQWJYaUJZSXcwNlQ4eDhsSmRTZzdid01OTU93SDBzClpMOUdLbm9zSWdqaS9MMDZaUndsOFBCU1hWVUF3VU9yV2RaakZta0NnWUI1bVRHZ3haTUdzbUpZYlJQeG5qK28KQnMyWkF0L1lkL2h3emlMcWMvTytTWTBoaWNZMjZhK29URThHeE1nblAxQ0QrUDF0dGZqdVR5VEQ0YXZQcFRpKwpVTi9zeStMR2svby93UlBzQnBJakZ5dlByM0pid2E2L3NiNUVMTmNTa3EyOWtuZE5HbUN5MjJrb2JFZEl6aW01ClpHaUt6K3BsN1BqTEtBRGFjM3BKZVFLQmdDRGlVY2sxTjRtblo5ZXQ5ZlBOYkxVMGYxdGwxSUJZZGxLRVdGaEQKUFBpSE9SSHdNNjU5OW5JRFNKVXhGRFZ3YjZUS2YyVTlUWCtuZzRvVklMS0srZzQ5NDVFNU1lMFJmQnFTbUUyZQpGaXZsM0tia3NIZmFncHRLSHd2dlEvemxaTVkwZStzSkNKWExRWGxWQXo2ZS91Qm1nbFhkZHNsMEJlUFo4V2pYCm9QQnhBb0dCQUlHQW1Oek1xNi8yS0NOQ3VmWkVDZGJ5NEMwVnVwTTJ4aHZkSmJ6Z2F0cFVxZ2d6LytzQlNBNVcKTzlXd21uQmFZdFBUcHZWdXFXUU9mYmF0RERmMlJab2lOUnBlMGQwdE9GeVFrZEh3UUVDdCtFYTZIMWpRYkh2MwoxVnFGeTBDbDFxYmZRektibzdaMmh4NnlXbElBaUJ6Yy8yeWt3cEhZbzdiMFo4K3ltOUtiCi0tLS0tRU5EIFJTQSBQUklWQVRFIEtFWS0tLS0tCg== diff --git a/tests/suite/fixtures/custom_resource_fixtures.py b/tests/suite/fixtures/custom_resource_fixtures.py index b78935d39b..67979f1067 100644 --- a/tests/suite/fixtures/custom_resource_fixtures.py +++ b/tests/suite/fixtures/custom_resource_fixtures.py @@ -110,13 +110,16 @@ class TransportServerSetup: namespace (str): """ - def __init__(self, name, namespace, ingress_pod_name, ic_namespace, public_endpoint: PublicEndpoint, resource): + def __init__( + self, name, namespace, ingress_pod_name, ic_namespace, public_endpoint: PublicEndpoint, resource, metrics_url + ): self.name = name self.namespace = namespace self.ingress_pod_name = ingress_pod_name self.ic_namespace = ic_namespace self.public_endpoint = public_endpoint self.resource = resource + self.metrics_url = metrics_url @pytest.fixture(scope="class") @@ -161,6 +164,8 @@ def fin(): ic_pod_name = get_first_pod_name(kube_apis.v1, ingress_controller_prerequisites.namespace) ic_namespace = ingress_controller_prerequisites.namespace + metrics_url = f"http://{ingress_controller_endpoint.public_ip}:{ingress_controller_endpoint.metrics_port}/metrics" + return TransportServerSetup( ts_resource["metadata"]["name"], test_namespace, @@ -168,6 +173,7 @@ def fin(): ic_namespace, ingress_controller_endpoint, ts_resource, + metrics_url, ) diff --git a/tests/suite/test_tls.py b/tests/suite/test_tls.py index 808005d35c..c4f5787f27 100644 --- a/tests/suite/test_tls.py +++ b/tests/suite/test_tls.py @@ -7,6 +7,7 @@ delete_items_from_yaml, delete_secret, ensure_connection_to_public_endpoint, + get_reload_count, is_secret_present, replace_secret, wait_before_test, @@ -43,12 +44,13 @@ def assert_gb_subject(endpoint, host): class TLSSetup: - def __init__(self, ingress_host, secret_name, secret_path, new_secret_path, invalid_secret_path): + def __init__(self, ingress_host, secret_name, secret_path, new_secret_path, invalid_secret_path, metrics_url): self.ingress_host = ingress_host self.secret_name = secret_name self.secret_path = secret_path self.new_secret_path = new_secret_path self.invalid_secret_path = invalid_secret_path + self.metrics_url = metrics_url @pytest.fixture(scope="class") @@ -63,6 +65,7 @@ def tls_setup( print("------------------------- Deploy TLS setup -----------------------------------") test_data_path = f"{TEST_DATA}/tls" + metrics_url = f"http://{ingress_controller_endpoint.public_ip}:{ingress_controller_endpoint.metrics_port}/metrics" ingress_path = f"{test_data_path}/{request.param}/ingress.yaml" create_ingress_from_yaml(kube_apis.networking_v1, test_namespace, ingress_path) @@ -90,11 +93,19 @@ def fin(): f"{test_data_path}/tls-secret.yaml", f"{test_data_path}/new-tls-secret.yaml", f"{test_data_path}/invalid-tls-secret.yaml", + metrics_url, ) @pytest.mark.ingresses -@pytest.mark.parametrize("tls_setup", ["standard", "mergeable"], indirect=True) +@pytest.mark.parametrize( + "ingress_controller, tls_setup", + [ + pytest.param({"extra_args": ["-enable-prometheus-metrics", "-ssl-dynamic-reload=false"]}, "standard"), + pytest.param({"extra_args": ["-enable-prometheus-metrics", "-ssl-dynamic-reload=false"]}, "mergeable"), + ], + indirect=True, +) class TestIngressTLS: def test_tls_termination(self, kube_apis, ingress_controller_endpoint, test_namespace, tls_setup): print("Step 1: no secret") @@ -127,7 +138,51 @@ def test_tls_termination(self, kube_apis, ingress_controller_endpoint, test_name wait_before_test(1) assert_us_subject(ingress_controller_endpoint, tls_setup.ingress_host) + # for OSS and and Plus with -ssl-dynamic-reload=false, we expect + # replacing a secret to trigger a reload + count_before_replace = get_reload_count(tls_setup.metrics_url) + print("Step 7: update secret and check") replace_secret(kube_apis.v1, tls_setup.secret_name, test_namespace, tls_setup.new_secret_path) wait_before_test(1) assert_gb_subject(ingress_controller_endpoint, tls_setup.ingress_host) + + count_after = get_reload_count(tls_setup.metrics_url) + reloads = count_after - count_before_replace + expected_reloads = 1 + assert reloads == expected_reloads, f"expected {expected_reloads} reloads, got {reloads}" + + +@pytest.mark.skip_for_nginx_oss +@pytest.mark.ingresses +@pytest.mark.parametrize( + "ingress_controller, tls_setup", + [ + pytest.param({"extra_args": ["-enable-prometheus-metrics"]}, "standard"), + pytest.param({"extra_args": ["-enable-prometheus-metrics"]}, "mergeable"), + ], + indirect=True, +) +class TestIngressTLSDynamicReloads: + def test_tls_termination(self, kube_apis, ingress_controller_endpoint, test_namespace, tls_setup): + print("Step 1: no secret") + assert_unrecognized_name_error(ingress_controller_endpoint, tls_setup.ingress_host) + + print("Step 2: deploy secret and check") + create_secret_from_yaml(kube_apis.v1, test_namespace, tls_setup.secret_path) + wait_before_test(1) + assert_us_subject(ingress_controller_endpoint, tls_setup.ingress_host) + + # for Plus with -ssl-dynamic-reload=true, we expect + # replacing a secret not to trigger a reload + count_before_replace = get_reload_count(tls_setup.metrics_url) + + print("Step 3: update secret and check") + replace_secret(kube_apis.v1, tls_setup.secret_name, test_namespace, tls_setup.new_secret_path) + wait_before_test(1) + assert_gb_subject(ingress_controller_endpoint, tls_setup.ingress_host) + + count_after = get_reload_count(tls_setup.metrics_url) + reloads = count_after - count_before_replace + expected_reloads = 0 + assert reloads == expected_reloads, f"expected {expected_reloads} reloads, got {reloads}" diff --git a/tests/suite/test_transport_server_tcp_load_balance.py b/tests/suite/test_transport_server_tcp_load_balance.py index 7dcd1871bb..12c1942fef 100644 --- a/tests/suite/test_transport_server_tcp_load_balance.py +++ b/tests/suite/test_transport_server_tcp_load_balance.py @@ -1,6 +1,7 @@ import re import socket import ssl +from datetime import datetime import pytest from settings import TEST_DATA @@ -8,7 +9,9 @@ from suite.utils.resources_utils import ( create_secret_from_yaml, delete_items_from_yaml, + get_reload_count, get_ts_nginx_template_conf, + replace_secret, scale_deployment, wait_before_test, ) @@ -26,6 +29,8 @@ "extra_args": [ "-global-configuration=nginx-ingress/nginx-configuration", "-enable-leader-election=false", + "-enable-prometheus-metrics", + "-ssl-dynamic-reload=false", ], }, {"example": "transport-server-tcp-load-balance"}, @@ -576,6 +581,7 @@ def test_secure_tcp_request_load_balanced( Sends requests to a TLS enabled load balanced TCP service. """ src_sec_yaml = f"{TEST_DATA}/transport-server-tcp-load-balance/tcp-tls-secret.yaml" + src_new_sec_yaml = f"{TEST_DATA}/transport-server-tcp-load-balance/new-tls-secret.yaml" create_secret_from_yaml(kube_apis.v1, transport_server_setup.namespace, src_sec_yaml) patch_src = f"{TEST_DATA}/transport-server-tcp-load-balance/transport-server-tls.yaml" patch_ts_from_yaml( @@ -617,5 +623,87 @@ def test_secure_tcp_request_load_balanced( endpoint = response.decode() print(f"Connected securely to: {endpoint}") + # for OSS and and Plus with -ssl-dynamic-reload=false, we expect + # replacing a secret to trigger a reload + count_before_replace = get_reload_count(transport_server_setup.metrics_url) + print(f"replacing: {sec_name} in {transport_server_setup.namespace}") + replace_secret(kube_apis.v1, sec_name, transport_server_setup.namespace, src_new_sec_yaml) + wait_before_test() + print(f"waited to {datetime.now().strftime('%m/%d/%Y, %H:%M:%S')}") + count_after = get_reload_count(transport_server_setup.metrics_url) + reloads = count_after - count_before_replace + expected_reloads = 1 + assert reloads == expected_reloads, f"expected {expected_reloads} reloads, got {reloads}" + self.restore_ts(kube_apis, transport_server_setup) delete_items_from_yaml(kube_apis, src_sec_yaml, transport_server_setup.namespace) + + +@pytest.mark.skip_for_nginx_oss +@pytest.mark.ts +@pytest.mark.skip_for_loadbalancer +@pytest.mark.parametrize( + "crd_ingress_controller, transport_server_setup", + [ + ( + { + "type": "complete", + "extra_args": [ + "-global-configuration=nginx-ingress/nginx-configuration", + "-enable-leader-election=false", + "-enable-prometheus-metrics", + "-v=3", + ], + }, + {"example": "transport-server-tcp-load-balance"}, + ) + ], + indirect=True, +) +class TestTransportServerTcpLoadBalanceDynamicReload: + def test_dynamic_reload( + self, kube_apis, crd_ingress_controller, transport_server_setup, ingress_controller_prerequisites + ): + """ + Updates a secret used by the transport server and verifies that NGINX is not reloaded. + """ + src_sec_yaml = f"{TEST_DATA}/transport-server-tcp-load-balance/tcp-tls-secret.yaml" + src_new_sec_yaml = f"{TEST_DATA}/transport-server-tcp-load-balance/new-tls-secret.yaml" + create_secret_from_yaml(kube_apis.v1, transport_server_setup.namespace, src_sec_yaml) + patch_src = f"{TEST_DATA}/transport-server-tcp-load-balance/transport-server-tls.yaml" + patch_ts_from_yaml( + kube_apis.custom_objects, + transport_server_setup.name, + patch_src, + transport_server_setup.namespace, + ) + wait_before_test() + + result_conf = get_ts_nginx_template_conf( + kube_apis.v1, + transport_server_setup.namespace, + transport_server_setup.name, + transport_server_setup.ingress_pod_name, + ingress_controller_prerequisites.namespace, + ) + + sec_name = get_secret_name_from_vs_or_ts_yaml(patch_src) + cert_name = f"{transport_server_setup.namespace}-{sec_name}" + + assert f"listen 3333 ssl;" in result_conf + assert f"ssl_certificate $secret_dir_path/{cert_name};" in result_conf + assert f"ssl_certificate_key $secret_dir_path/{cert_name};" in result_conf + + # for Plus with -ssl-dynamic-reload=true, we expect + # replacing a secret not to trigger a reload + count_before_replace = get_reload_count(transport_server_setup.metrics_url) + print(f"replacing: {sec_name} in {transport_server_setup.namespace}") + replace_secret(kube_apis.v1, sec_name, transport_server_setup.namespace, src_new_sec_yaml) + wait_before_test() + print(f"waited to {datetime.now().strftime('%m/%d/%Y, %H:%M:%S')}") + count_after = get_reload_count(transport_server_setup.metrics_url) + reloads = count_after - count_before_replace + expected_reloads = 0 + assert reloads == expected_reloads, f"expected {expected_reloads} reloads, got {reloads}" + + delete_items_from_yaml(kube_apis, src_sec_yaml, transport_server_setup.namespace) diff --git a/tests/suite/test_virtual_server_tls.py b/tests/suite/test_virtual_server_tls.py index e9d56a8041..5d052b03a6 100644 --- a/tests/suite/test_virtual_server_tls.py +++ b/tests/suite/test_virtual_server_tls.py @@ -4,6 +4,7 @@ from suite.utils.resources_utils import ( create_secret_from_yaml, delete_secret, + get_reload_count, is_secret_present, replace_secret, wait_before_test, @@ -76,7 +77,14 @@ def assert_gb_subject(virtual_server_setup): "crd_ingress_controller, virtual_server_setup", [ ( - {"type": "complete", "extra_args": [f"-enable-custom-resources"]}, + { + "type": "complete", + "extra_args": [ + f"-enable-custom-resources", + f"-enable-prometheus-metrics", + f"-ssl-dynamic-reload=false", + ], + }, {"example": "virtual-server-tls", "app_type": "simple"}, ) ], @@ -122,6 +130,10 @@ def test_tls_termination(self, kube_apis, crd_ingress_controller, virtual_server wait_before_test(1) assert_us_subject(virtual_server_setup) + # for OSS and and Plus with -ssl-dynamic-reload=false, we expect + # replacing a secret to trigger a reload + count_before_replace = get_reload_count(virtual_server_setup.metrics_url) + print("\nStep 7: update secret and check") replace_secret( kube_apis.v1, @@ -131,3 +143,59 @@ def test_tls_termination(self, kube_apis, crd_ingress_controller, virtual_server ) wait_before_test(1) assert_gb_subject(virtual_server_setup) + + count_after = get_reload_count(virtual_server_setup.metrics_url) + reloads = count_after - count_before_replace + expected_reloads = 1 + assert reloads == expected_reloads, f"expected {expected_reloads} reloads, got {reloads}" + + +@pytest.mark.skip_for_nginx_oss +@pytest.mark.vs +@pytest.mark.smoke +@pytest.mark.parametrize( + "crd_ingress_controller, virtual_server_setup", + [ + ( + { + "type": "complete", + "extra_args": [ + f"-enable-custom-resources", + f"-enable-prometheus-metrics", + ], + }, + {"example": "virtual-server-tls", "app_type": "simple"}, + ) + ], + indirect=True, +) +class TestVirtualServerTLSDynamicReloads: + def test_tls_termination(self, kube_apis, crd_ingress_controller, virtual_server_setup, clean_up): + print("\nStep 1: no secret") + assert_unrecognized_name_error(virtual_server_setup) + + print("\nStep 2: deploy secret and check") + secret_name = create_secret_from_yaml( + kube_apis.v1, virtual_server_setup.namespace, f"{TEST_DATA}/virtual-server-tls/tls-secret.yaml" + ) + wait_before_test(1) + assert_us_subject(virtual_server_setup) + + # for Plus with -ssl-dynamic-reload=true, we expect + # replacing a secret not to trigger a reload + count_before_replace = get_reload_count(virtual_server_setup.metrics_url) + + print("\nStep 3: update secret and check") + replace_secret( + kube_apis.v1, + secret_name, + virtual_server_setup.namespace, + f"{TEST_DATA}/virtual-server-tls/new-tls-secret.yaml", + ) + wait_before_test(1) + assert_gb_subject(virtual_server_setup) + + count_after = get_reload_count(virtual_server_setup.metrics_url) + reloads = count_after - count_before_replace + expected_reloads = 0 + assert reloads == expected_reloads, f"expected {expected_reloads} reloads, got {reloads}"