From ec006269823db1c6a396a4fda7ba36277518893f Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 26 Sep 2022 21:27:54 -0700 Subject: [PATCH 001/105] Bump certifi from 2022.9.14 to 2022.9.24 in /perf-tests (#3099) Bumps [certifi](https://github.com/certifi/python-certifi) from 2022.9.14 to 2022.9.24. - [Release notes](https://github.com/certifi/python-certifi/releases) - [Commits](https://github.com/certifi/python-certifi/compare/2022.09.14...2022.09.24) --- updated-dependencies: - dependency-name: certifi dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- perf-tests/requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/perf-tests/requirements.txt b/perf-tests/requirements.txt index 37afa615e3..4ee6e27ef9 100644 --- a/perf-tests/requirements.txt +++ b/perf-tests/requirements.txt @@ -3,7 +3,7 @@ requests==2.28.1 kubernetes==24.2.0 pytest==7.1.3 cffi==1.15.1 -certifi==2022.9.14 +certifi==2022.9.24 urllib3==1.26.12 pytest-html==3.1.1 pytest-repeat==0.9.1 From bbe3ae97f1f58536ebb754da69418fe749a88e20 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 26 Sep 2022 21:28:13 -0700 Subject: [PATCH 002/105] Bump certifi from 2022.9.14 to 2022.9.24 in /tests (#3100) Bumps [certifi](https://github.com/certifi/python-certifi) from 2022.9.14 to 2022.9.24. - [Release notes](https://github.com/certifi/python-certifi/releases) - [Commits](https://github.com/certifi/python-certifi/compare/2022.09.14...2022.09.24) --- updated-dependencies: - dependency-name: certifi dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- tests/requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/requirements.txt b/tests/requirements.txt index e3265d3991..1fee2eab9f 100644 --- a/tests/requirements.txt +++ b/tests/requirements.txt @@ -5,7 +5,7 @@ kubernetes==24.2.0 pytest==7.1.3 cffi==1.15.1 pyOpenSSL==22.1.0 -certifi==2022.9.14 +certifi==2022.9.24 urllib3==1.26.12 pytest-html==3.1.1 pytest-profiling==1.7.0 From 296737da5808330bd1ae420f4c01595af22d24b4 Mon Sep 17 00:00:00 2001 From: Jakub Jarosz <99677300+jjngx@users.noreply.github.com> Date: Tue, 27 Sep 2022 08:41:40 +0100 Subject: [PATCH 003/105] Fix staticcheck linter issues (#3097) Co-authored-by: Venktesh Shivam Patel --- internal/configs/configurator.go | 80 ++++++++++++++--------------- internal/configs/parsing_helpers.go | 14 ++--- internal/k8s/validation_test.go | 12 ++--- 3 files changed, 53 insertions(+), 53 deletions(-) diff --git a/internal/configs/configurator.go b/internal/configs/configurator.go index 93514286f6..237f199004 100644 --- a/internal/configs/configurator.go +++ b/internal/configs/configurator.go @@ -255,11 +255,11 @@ func (cnf *Configurator) deleteIngressMetricsLabels(key string) { func (cnf *Configurator) AddOrUpdateIngress(ingEx *IngressEx) (Warnings, error) { 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) + return warnings, fmt.Errorf("error adding or updating ingress %v/%v: %w", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err) } if err := cnf.reload(nginx.ReloadForOtherUpdate); err != nil { - return warnings, fmt.Errorf("Error reloading NGINX for %v/%v: %w", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err) + return warnings, fmt.Errorf("error reloading NGINX for %v/%v: %w", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err) } return warnings, nil @@ -287,7 +287,7 @@ 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 warnings, fmt.Errorf("error generating Ingress Config %v: %w", name, err) } cnf.nginxManager.CreateConfig(name, content) @@ -302,11 +302,11 @@ func (cnf *Configurator) addOrUpdateIngress(ingEx *IngressEx) (Warnings, error) func (cnf *Configurator) AddOrUpdateMergeableIngress(mergeableIngs *MergeableIngresses) (Warnings, error) { 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) + return warnings, fmt.Errorf("error when adding or updating ingress %v/%v: %w", mergeableIngs.Master.Ingress.Namespace, mergeableIngs.Master.Ingress.Name, err) } if err := cnf.reload(nginx.ReloadForOtherUpdate); err != nil { - return warnings, fmt.Errorf("Error reloading NGINX for %v/%v: %w", mergeableIngs.Master.Ingress.Namespace, mergeableIngs.Master.Ingress.Name, err) + return warnings, fmt.Errorf("error reloading NGINX for %v/%v: %w", mergeableIngs.Master.Ingress.Namespace, mergeableIngs.Master.Ingress.Name, err) } return warnings, nil @@ -341,7 +341,7 @@ 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 warnings, fmt.Errorf("error generating Ingress Config %v: %w", name, err) } cnf.nginxManager.CreateConfig(name, content) @@ -441,11 +441,11 @@ func (cnf *Configurator) deleteVirtualServerMetricsLabels(key string) { func (cnf *Configurator) AddOrUpdateVirtualServer(virtualServerEx *VirtualServerEx) (Warnings, error) { 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) + return warnings, fmt.Errorf("error adding or updating VirtualServer %v/%v: %w", virtualServerEx.VirtualServer.Namespace, virtualServerEx.VirtualServer.Name, err) } if err := cnf.reload(nginx.ReloadForOtherUpdate); err != nil { - return warnings, fmt.Errorf("Error reloading NGINX for VirtualServer %v/%v: %w", virtualServerEx.VirtualServer.Namespace, virtualServerEx.VirtualServer.Name, err) + return warnings, fmt.Errorf("error reloading NGINX for VirtualServer %v/%v: %w", virtualServerEx.VirtualServer.Namespace, virtualServerEx.VirtualServer.Name, err) } return warnings, nil @@ -473,7 +473,7 @@ 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 warnings, fmt.Errorf("error generating VirtualServer config: %v: %w", name, err) } cnf.nginxManager.CreateConfig(name, content) @@ -498,7 +498,7 @@ func (cnf *Configurator) AddOrUpdateVirtualServers(virtualServerExes []*VirtualS } if err := cnf.reload(nginx.ReloadForOtherUpdate); err != nil { - return allWarnings, fmt.Errorf("Error when reloading NGINX when updating Policy: %w", err) + return allWarnings, fmt.Errorf("error when reloading NGINX when updating Policy: %w", err) } return allWarnings, nil @@ -630,7 +630,7 @@ func (cnf *Configurator) updateTLSPassthroughHostsConfig() error { content, err := cnf.templateExecutorV2.ExecuteTLSPassthroughHostsTemplate(cfg) if err != nil { - return fmt.Errorf("Error generating config for TLS Passthrough Unix Sockets map: %w", err) + return fmt.Errorf("error generating config for TLS Passthrough Unix Sockets map: %w", err) } cnf.nginxManager.CreateTLSPassthroughHostsConfig(content) @@ -673,7 +673,7 @@ func (cnf *Configurator) AddOrUpdateResources(resources ExtendedResources) (Warn for _, ingEx := range resources.IngressExes { warnings, err := cnf.addOrUpdateIngress(ingEx) if err != nil { - return nil, fmt.Errorf("Error adding or updating ingress %v/%v: %w", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err) + return nil, fmt.Errorf("error adding or updating ingress %v/%v: %w", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err) } allWarnings.Add(warnings) } @@ -681,7 +681,7 @@ func (cnf *Configurator) AddOrUpdateResources(resources ExtendedResources) (Warn for _, m := range resources.MergeableIngresses { warnings, err := cnf.addOrUpdateMergeableIngress(m) 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, fmt.Errorf("error adding or updating mergeableIngress %v/%v: %w", m.Master.Ingress.Namespace, m.Master.Ingress.Name, err) } allWarnings.Add(warnings) } @@ -689,7 +689,7 @@ func (cnf *Configurator) AddOrUpdateResources(resources ExtendedResources) (Warn for _, vsEx := range resources.VirtualServerExes { warnings, err := cnf.addOrUpdateVirtualServer(vsEx) if err != nil { - return nil, fmt.Errorf("Error adding or updating VirtualServer %v/%v: %w", vsEx.VirtualServer.Namespace, vsEx.VirtualServer.Name, err) + return nil, fmt.Errorf("error adding or updating VirtualServer %v/%v: %w", vsEx.VirtualServer.Namespace, vsEx.VirtualServer.Name, err) } allWarnings.Add(warnings) } @@ -703,7 +703,7 @@ func (cnf *Configurator) AddOrUpdateResources(resources ExtendedResources) (Warn } if err := cnf.reload(nginx.ReloadForOtherUpdate); err != nil { - return nil, fmt.Errorf("Error when reloading NGINX when updating resources: %w", err) + return nil, fmt.Errorf("error when reloading NGINX when updating resources: %w", err) } return allWarnings, nil } @@ -723,7 +723,7 @@ func (cnf *Configurator) AddOrUpdateSpecialTLSSecrets(secret *api_v1.Secret, sec } if err := cnf.reload(nginx.ReloadForOtherUpdate); err != nil { - return fmt.Errorf("Error when reloading NGINX when updating the special Secrets: %w", err) + return fmt.Errorf("error when reloading NGINX when updating the special Secrets: %w", err) } return nil @@ -762,7 +762,7 @@ func (cnf *Configurator) DeleteIngress(key string) error { } if err := cnf.reload(nginx.ReloadForOtherUpdate); err != nil { - return fmt.Errorf("Error when removing ingress %v: %w", key, err) + return fmt.Errorf("error when removing ingress %v: %w", key, err) } return nil @@ -779,7 +779,7 @@ func (cnf *Configurator) DeleteVirtualServer(key string) error { } if err := cnf.reload(nginx.ReloadForOtherUpdate); err != nil { - return fmt.Errorf("Error when removing VirtualServer %v: %w", key, err) + return fmt.Errorf("error when removing VirtualServer %v: %w", key, err) } return nil @@ -793,12 +793,12 @@ func (cnf *Configurator) DeleteTransportServer(key string) error { err := cnf.deleteTransportServer(key) if err != nil { - return fmt.Errorf("Error when removing TransportServer %v: %w", key, err) + return fmt.Errorf("error when removing TransportServer %v: %w", key, err) } err = cnf.reload(nginx.ReloadForOtherUpdate) if err != nil { - return fmt.Errorf("Error when removing TransportServer %v: %w", key, err) + return fmt.Errorf("error when removing TransportServer %v: %w", key, err) } return nil @@ -826,7 +826,7 @@ func (cnf *Configurator) UpdateEndpoints(ingExes []*IngressEx) error { // It is safe to ignore warnings here as no new warnings should appear when updating Endpoints for Ingresses _, 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) + return fmt.Errorf("error adding or updating ingress %v/%v: %w", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err) } if cnf.isPlus { @@ -844,7 +844,7 @@ func (cnf *Configurator) UpdateEndpoints(ingExes []*IngressEx) error { } if err := cnf.reload(nginx.ReloadForEndpointsUpdate); err != nil { - return fmt.Errorf("Error reloading NGINX when updating endpoints: %w", err) + return fmt.Errorf("error reloading NGINX when updating endpoints: %w", err) } return nil @@ -858,7 +858,7 @@ func (cnf *Configurator) UpdateEndpointsMergeableIngress(mergeableIngresses []*M // It is safe to ignore warnings here as no new warnings should appear when updating Endpoints for Ingresses _, 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) + return fmt.Errorf("error adding or updating mergeableIngress %v/%v: %w", mergeableIngresses[i].Master.Ingress.Namespace, mergeableIngresses[i].Master.Ingress.Name, err) } if cnf.isPlus { @@ -878,7 +878,7 @@ func (cnf *Configurator) UpdateEndpointsMergeableIngress(mergeableIngresses []*M } if err := cnf.reload(nginx.ReloadForEndpointsUpdate); err != nil { - return fmt.Errorf("Error reloading NGINX when updating endpoints for %v: %w", mergeableIngresses, err) + return fmt.Errorf("error reloading NGINX when updating endpoints for %v: %w", mergeableIngresses, err) } return nil @@ -892,7 +892,7 @@ func (cnf *Configurator) UpdateEndpointsForVirtualServers(virtualServerExes []*V // It is safe to ignore warnings here as no new warnings should appear when updating Endpoints for VirtualServers _, 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) + return fmt.Errorf("error adding or updating VirtualServer %v/%v: %w", vs.VirtualServer.Namespace, vs.VirtualServer.Name, err) } if cnf.isPlus { @@ -910,7 +910,7 @@ func (cnf *Configurator) UpdateEndpointsForVirtualServers(virtualServerExes []*V } if err := cnf.reload(nginx.ReloadForEndpointsUpdate); err != nil { - return fmt.Errorf("Error reloading NGINX when updating endpoints: %w", err) + return fmt.Errorf("error reloading NGINX when updating endpoints: %w", err) } return nil @@ -925,7 +925,7 @@ func (cnf *Configurator) updatePlusEndpointsForVirtualServer(virtualServerEx *Vi err := cnf.updateServersInPlus(upstream.Name, endpoints, serverCfg) if err != nil { - return fmt.Errorf("Couldn't update the endpoints for %v: %w", upstream.Name, err) + return fmt.Errorf("couldn't update the endpoints for %v: %w", upstream.Name, err) } } @@ -973,7 +973,7 @@ func (cnf *Configurator) updatePlusEndpointsForTransportServer(transportServerEx err := cnf.updateStreamServersInPlus(name, endpoints) if err != nil { - return fmt.Errorf("Couldn't update the endpoints for %v: %w", u.Name, err) + return fmt.Errorf("couldn't update the endpoints for %v: %w", u.Name, err) } } @@ -999,7 +999,7 @@ func (cnf *Configurator) updatePlusEndpoints(ingEx *IngressEx) error { name := getNameForUpstream(ingEx.Ingress, emptyHost, ingEx.Ingress.Spec.DefaultBackend) err := cnf.updateServersInPlus(name, endps, cfg) if err != nil { - return fmt.Errorf("Couldn't update the endpoints for %v: %w", name, err) + return fmt.Errorf("couldn't update the endpoints for %v: %w", name, err) } } } @@ -1021,7 +1021,7 @@ func (cnf *Configurator) updatePlusEndpoints(ingEx *IngressEx) error { name := getNameForUpstream(ingEx.Ingress, rule.Host, &path.Backend) err := cnf.updateServersInPlus(name, endps, cfg) if err != nil { - return fmt.Errorf("Couldn't update the endpoints for %v: %w", name, err) + return fmt.Errorf("couldn't update the endpoints for %v: %w", name, err) } } } @@ -1156,19 +1156,19 @@ func (cnf *Configurator) UpdateTransportServers(updatedTSExes []*TransportServer for _, tsEx := range updatedTSExes { _, 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) + return fmt.Errorf("error adding or updating TransportServer %v/%v: %w", tsEx.TransportServer.Namespace, tsEx.TransportServer.Name, err) } } for _, key := range deletedKeys { err := cnf.deleteTransportServer(key) if err != nil { - return fmt.Errorf("Error when removing TransportServer %v: %w", key, err) + return fmt.Errorf("error when removing TransportServer %v: %w", key, err) } } if err := cnf.reload(nginx.ReloadForOtherUpdate); err != nil { - return fmt.Errorf("Error when updating TransportServers: %w", err) + return fmt.Errorf("error when updating TransportServers: %w", err) } return nil @@ -1379,12 +1379,12 @@ type ResourceOperation func(resource *v1beta1.DosProtectedResource, ingExes []*I func (cnf *Configurator) AddOrUpdateAppProtectResource(resource *unstructured.Unstructured, ingExes []*IngressEx, mergeableIngresses []*MergeableIngresses, vsExes []*VirtualServerEx) (Warnings, error) { warnings, err := cnf.addOrUpdateIngressesAndVirtualServers(ingExes, mergeableIngresses, vsExes) if err != nil { - return warnings, fmt.Errorf("Error when updating %v %v/%v: %w", resource.GetKind(), resource.GetNamespace(), resource.GetName(), err) + return warnings, fmt.Errorf("error when updating %v %v/%v: %w", resource.GetKind(), resource.GetNamespace(), resource.GetName(), err) } err = cnf.reload(nginx.ReloadForOtherUpdate) if err != nil { - return warnings, fmt.Errorf("Error when reloading NGINX when updating %v %v/%v: %w", resource.GetKind(), resource.GetNamespace(), resource.GetName(), err) + return warnings, fmt.Errorf("error when reloading NGINX when updating %v %v/%v: %w", resource.GetKind(), resource.GetNamespace(), resource.GetName(), err) } return warnings, nil @@ -1411,7 +1411,7 @@ func (cnf *Configurator) addOrUpdateIngressesAndVirtualServers(ingExes []*Ingres for _, ingEx := range ingExes { 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) + return allWarnings, fmt.Errorf("error adding or updating ingress %v/%v: %w", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err) } allWarnings.Add(warnings) } @@ -1419,7 +1419,7 @@ func (cnf *Configurator) addOrUpdateIngressesAndVirtualServers(ingExes []*Ingres for _, m := range mergeableIngresses { 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) + return allWarnings, fmt.Errorf("error adding or updating mergeableIngress %v/%v: %w", m.Master.Ingress.Namespace, m.Master.Ingress.Name, err) } allWarnings.Add(warnings) } @@ -1427,7 +1427,7 @@ func (cnf *Configurator) addOrUpdateIngressesAndVirtualServers(ingExes []*Ingres for _, vs := range vsExes { 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) + return allWarnings, fmt.Errorf("error adding or updating VirtualServer %v/%v: %w", vs.VirtualServer.Namespace, vs.VirtualServer.Name, err) } allWarnings.Add(warnings) } @@ -1503,11 +1503,11 @@ func (cnf *Configurator) AddInternalRouteConfig() error { mainCfg := GenerateNginxMainConfig(cnf.staticCfgParams, cnf.cfgParams) mainCfgContent, err := cnf.templateExecutor.ExecuteMainConfigTemplate(mainCfg) if err != nil { - return fmt.Errorf("Error when writing main Config: %w", err) + return fmt.Errorf("error when writing main Config: %w", err) } cnf.nginxManager.CreateMainConfig(mainCfgContent) if err := cnf.reload(nginx.ReloadForOtherUpdate); err != nil { - return fmt.Errorf("Error when reloading nginx: %w", err) + return fmt.Errorf("error when reloading nginx: %w", err) } return nil } diff --git a/internal/configs/parsing_helpers.go b/internal/configs/parsing_helpers.go index 6ce4f44aae..3883056099 100644 --- a/internal/configs/parsing_helpers.go +++ b/internal/configs/parsing_helpers.go @@ -105,7 +105,7 @@ func ParseLBMethod(method string) (string, error) { return method, nil } - return "", fmt.Errorf("Invalid load balancing method: %q", method) + return "", fmt.Errorf("invalid load balancing method: %q", method) } var nginxLBValidInput = map[string]bool{ @@ -147,7 +147,7 @@ func ParseLBMethodForPlus(method string) (string, error) { return method, nil } - return "", fmt.Errorf("Invalid load balancing method: %q", method) + return "", fmt.Errorf("invalid load balancing method: %q", method) } func validateHashLBMethod(method string) (string, error) { @@ -222,7 +222,7 @@ func ParseOffset(s string) (string, error) { if offsetRegexp.MatchString(s) { return s, nil } - return "", errors.New("Invalid offset string") + return "", errors.New("invalid offset string") } // SizeFmt http://nginx.org/en/docs/syntax.html @@ -237,7 +237,7 @@ func ParseSize(s string) (string, error) { if sizeRegexp.MatchString(s) { return s, nil } - return "", errors.New("Invalid size string") + return "", errors.New("invalid size string") } // https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_buffers @@ -250,7 +250,7 @@ func ParseProxyBuffersSpec(s string) (string, error) { if proxyBuffersRegexp.MatchString(s) { return s, nil } - return "", errors.New("Invalid proxy buffers string") + return "", errors.New("invalid proxy buffers string") } // ParsePortList ensures that the string is a comma-separated list of port numbers @@ -269,11 +269,11 @@ func ParsePortList(s string) ([]int, error) { func parsePort(value string) (int, error) { port, err := strconv.ParseInt(value, 10, 16) if err != nil { - return 0, fmt.Errorf("Unable to parse port as integer: %w", err) + return 0, fmt.Errorf("unable to parse port as integer: %w", err) } if port <= 0 { - return 0, fmt.Errorf("Port number should be greater than zero: %q", port) + return 0, fmt.Errorf("port number should be greater than zero: %q", port) } return int(port), nil diff --git a/internal/k8s/validation_test.go b/internal/k8s/validation_test.go index 68d83d65ef..301c9e1e71 100644 --- a/internal/k8s/validation_test.go +++ b/internal/k8s/validation_test.go @@ -167,7 +167,7 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { appProtectDosEnabled: false, internalRoutesEnabled: false, expectedErrors: []string{ - `annotations.nginx.org/lb-method: Invalid value: "invalid_method": Invalid load balancing method: "invalid_method"`, + `annotations.nginx.org/lb-method: Invalid value: "invalid_method": invalid load balancing method: "invalid_method"`, `annotations.nginx.org/mergeable-ingress-type: Invalid value: "invalid": must be one of: 'master' or 'minion'`, }, msg: "invalid multiple annotations messages in alphabetical order", @@ -248,7 +248,7 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { appProtectDosEnabled: false, internalRoutesEnabled: false, expectedErrors: []string{ - `annotations.nginx.org/lb-method: Invalid value: "least_time header": Invalid load balancing method: "least_time header"`, + `annotations.nginx.org/lb-method: Invalid value: "least_time header": invalid load balancing method: "least_time header"`, }, msg: "invalid nginx.org/lb-method annotation, nginx plus only", }, @@ -262,7 +262,7 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { appProtectDosEnabled: false, internalRoutesEnabled: false, expectedErrors: []string{ - `annotations.nginx.org/lb-method: Invalid value: "least_time header;": Invalid load balancing method: "least_time header;"`, + `annotations.nginx.org/lb-method: Invalid value: "least_time header;": invalid load balancing method: "least_time header;"`, }, msg: "invalid nginx.org/lb-method annotation", }, @@ -276,7 +276,7 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { appProtectDosEnabled: false, internalRoutesEnabled: false, expectedErrors: []string{ - `annotations.nginx.org/lb-method: Invalid value: "{least_time header}": Invalid load balancing method: "{least_time header}"`, + `annotations.nginx.org/lb-method: Invalid value: "{least_time header}": invalid load balancing method: "{least_time header}"`, }, msg: "invalid nginx.org/lb-method annotation", }, @@ -290,7 +290,7 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { appProtectDosEnabled: false, internalRoutesEnabled: false, expectedErrors: []string{ - `annotations.nginx.org/lb-method: Invalid value: "$least_time header": Invalid load balancing method: "$least_time header"`, + `annotations.nginx.org/lb-method: Invalid value: "$least_time header": invalid load balancing method: "$least_time header"`, }, msg: "invalid nginx.org/lb-method annotation", }, @@ -304,7 +304,7 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { appProtectDosEnabled: false, internalRoutesEnabled: false, expectedErrors: []string{ - `annotations.nginx.org/lb-method: Invalid value: "invalid_method": Invalid load balancing method: "invalid_method"`, + `annotations.nginx.org/lb-method: Invalid value: "invalid_method": invalid load balancing method: "invalid_method"`, }, msg: "invalid nginx.org/lb-method annotation", }, From 5d9ccff82f847198ff4ad8d911fecb2a8ee85b38 Mon Sep 17 00:00:00 2001 From: Bryan Hendryx Date: Tue, 27 Sep 2022 03:35:53 -0700 Subject: [PATCH 004/105] Add RBAC for coordination.k8s.io/leases (#3101) Co-authored-by: Jakub Jarosz <99677300+jjngx@users.noreply.github.com> --- deployments/helm-chart/templates/rbac.yaml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/deployments/helm-chart/templates/rbac.yaml b/deployments/helm-chart/templates/rbac.yaml index 55329394e2..b70ede501e 100644 --- a/deployments/helm-chart/templates/rbac.yaml +++ b/deployments/helm-chart/templates/rbac.yaml @@ -74,6 +74,16 @@ rules: - create - patch - list +- apiGroups: + - coordination.k8s.io + resources: + - leases + verbs: + - get + - list + - watch + - update + - create - apiGroups: - networking.k8s.io resources: From fcbbca925fe73a330d7ef0fec46f751b9abff4be Mon Sep 17 00:00:00 2001 From: Jakub Jarosz <99677300+jjngx@users.noreply.github.com> Date: Tue, 27 Sep 2022 14:32:25 +0100 Subject: [PATCH 005/105] Eliminate dead code (#3104) --- internal/configs/annotations.go | 32 ++------ internal/configs/configmaps.go | 100 ++++++----------------- internal/configs/parsing_helpers.go | 7 +- internal/configs/parsing_helpers_test.go | 12 +-- 4 files changed, 40 insertions(+), 111 deletions(-) diff --git a/internal/configs/annotations.go b/internal/configs/annotations.go index f620183083..120e440f10 100644 --- a/internal/configs/annotations.go +++ b/internal/configs/annotations.go @@ -148,20 +148,12 @@ func parseAnnotations(ingEx *IngressEx, baseCfgParams *ConfigParams, isPlus bool } } - if serverSnippets, exists, err := GetMapKeyAsStringSlice(ingEx.Ingress.Annotations, "nginx.org/server-snippets", ingEx.Ingress, "\n"); exists { - if err != nil { - glog.Error(err) - } else { - cfgParams.ServerSnippets = serverSnippets - } + if serverSnippets, exists := GetMapKeyAsStringSlice(ingEx.Ingress.Annotations, "nginx.org/server-snippets", ingEx.Ingress, "\n"); exists { + cfgParams.ServerSnippets = serverSnippets } - if locationSnippets, exists, err := GetMapKeyAsStringSlice(ingEx.Ingress.Annotations, "nginx.org/location-snippets", ingEx.Ingress, "\n"); exists { - if err != nil { - glog.Error(err) - } else { - cfgParams.LocationSnippets = locationSnippets - } + if locationSnippets, exists := GetMapKeyAsStringSlice(ingEx.Ingress.Annotations, "nginx.org/location-snippets", ingEx.Ingress, "\n"); exists { + cfgParams.LocationSnippets = locationSnippets } if proxyConnectTimeout, exists := ingEx.Ingress.Annotations["nginx.org/proxy-connect-timeout"]; exists { @@ -188,20 +180,12 @@ func parseAnnotations(ingEx *IngressEx, baseCfgParams *ConfigParams, isPlus bool } } - if proxyHideHeaders, exists, err := GetMapKeyAsStringSlice(ingEx.Ingress.Annotations, "nginx.org/proxy-hide-headers", ingEx.Ingress, ","); exists { - if err != nil { - glog.Error(err) - } else { - cfgParams.ProxyHideHeaders = proxyHideHeaders - } + if proxyHideHeaders, exists := GetMapKeyAsStringSlice(ingEx.Ingress.Annotations, "nginx.org/proxy-hide-headers", ingEx.Ingress, ","); exists { + cfgParams.ProxyHideHeaders = proxyHideHeaders } - if proxyPassHeaders, exists, err := GetMapKeyAsStringSlice(ingEx.Ingress.Annotations, "nginx.org/proxy-pass-headers", ingEx.Ingress, ","); exists { - if err != nil { - glog.Error(err) - } else { - cfgParams.ProxyPassHeaders = proxyPassHeaders - } + if proxyPassHeaders, exists := GetMapKeyAsStringSlice(ingEx.Ingress.Annotations, "nginx.org/proxy-pass-headers", ingEx.Ingress, ","); exists { + cfgParams.ProxyPassHeaders = proxyPassHeaders } if clientMaxBodySize, exists := ingEx.Ingress.Annotations["nginx.org/client-max-body-size"]; exists { diff --git a/internal/configs/configmaps.go b/internal/configs/configmaps.go index 5250efd36c..9de5e00fa2 100644 --- a/internal/configs/configmaps.go +++ b/internal/configs/configmaps.go @@ -58,20 +58,12 @@ func ParseConfigMap(cfgm *v1.ConfigMap, nginxPlus bool, hasAppProtect bool, hasA cfgParams.ProxySendTimeout = proxySendTimeout } - if proxyHideHeaders, exists, err := GetMapKeyAsStringSlice(cfgm.Data, "proxy-hide-headers", cfgm, ","); exists { - if err != nil { - glog.Error(err) - } else { - cfgParams.ProxyHideHeaders = proxyHideHeaders - } + if proxyHideHeaders, exists := GetMapKeyAsStringSlice(cfgm.Data, "proxy-hide-headers", cfgm, ","); exists { + cfgParams.ProxyHideHeaders = proxyHideHeaders } - if proxyPassHeaders, exists, err := GetMapKeyAsStringSlice(cfgm.Data, "proxy-pass-headers", cfgm, ","); exists { - if err != nil { - glog.Error(err) - } else { - cfgParams.ProxyPassHeaders = proxyPassHeaders - } + if proxyPassHeaders, exists := GetMapKeyAsStringSlice(cfgm.Data, "proxy-pass-headers", cfgm, ","); exists { + cfgParams.ProxyPassHeaders = proxyPassHeaders } if clientMaxBodySize, exists := cfgm.Data["client-max-body-size"]; exists { @@ -170,12 +162,8 @@ func ParseConfigMap(cfgm *v1.ConfigMap, nginxPlus bool, hasAppProtect bool, hasA } } - if setRealIPFrom, exists, err := GetMapKeyAsStringSlice(cfgm.Data, "set-real-ip-from", cfgm, ","); exists { - if err != nil { - glog.Error(err) - } else { - cfgParams.SetRealIPFrom = setRealIPFrom - } + if setRealIPFrom, exists := GetMapKeyAsStringSlice(cfgm.Data, "set-real-ip-from", cfgm, ","); exists { + cfgParams.SetRealIPFrom = setRealIPFrom } if realIPRecursive, exists, err := GetMapKeyAsBool(cfgm.Data, "real-ip-recursive", cfgm); exists { @@ -219,12 +207,8 @@ func ParseConfigMap(cfgm *v1.ConfigMap, nginxPlus bool, hasAppProtect bool, hasA } } - if logFormat, exists, err := GetMapKeyAsStringSlice(cfgm.Data, "log-format", cfgm, "\n"); exists { - if err != nil { - glog.Error(err) - } else { - cfgParams.MainLogFormat = logFormat - } + if logFormat, exists := GetMapKeyAsStringSlice(cfgm.Data, "log-format", cfgm, "\n"); exists { + cfgParams.MainLogFormat = logFormat } if logFormatEscaping, exists := cfgm.Data["log-format-escaping"]; exists { @@ -234,12 +218,8 @@ func ParseConfigMap(cfgm *v1.ConfigMap, nginxPlus bool, hasAppProtect bool, hasA } } - if streamLogFormat, exists, err := GetMapKeyAsStringSlice(cfgm.Data, "stream-log-format", cfgm, "\n"); exists { - if err != nil { - glog.Error(err) - } else { - cfgParams.MainStreamLogFormat = streamLogFormat - } + if streamLogFormat, exists := GetMapKeyAsStringSlice(cfgm.Data, "stream-log-format", cfgm, "\n"); exists { + cfgParams.MainStreamLogFormat = streamLogFormat } if streamLogFormatEscaping, exists := cfgm.Data["stream-log-format-escaping"]; exists { @@ -281,36 +261,20 @@ func ParseConfigMap(cfgm *v1.ConfigMap, nginxPlus bool, hasAppProtect bool, hasA cfgParams.ProxyMaxTempFileSize = proxyMaxTempFileSize } - if mainMainSnippets, exists, err := GetMapKeyAsStringSlice(cfgm.Data, "main-snippets", cfgm, "\n"); exists { - if err != nil { - glog.Error(err) - } else { - cfgParams.MainMainSnippets = mainMainSnippets - } + if mainMainSnippets, exists := GetMapKeyAsStringSlice(cfgm.Data, "main-snippets", cfgm, "\n"); exists { + cfgParams.MainMainSnippets = mainMainSnippets } - if mainHTTPSnippets, exists, err := GetMapKeyAsStringSlice(cfgm.Data, "http-snippets", cfgm, "\n"); exists { - if err != nil { - glog.Error(err) - } else { - cfgParams.MainHTTPSnippets = mainHTTPSnippets - } + if mainHTTPSnippets, exists := GetMapKeyAsStringSlice(cfgm.Data, "http-snippets", cfgm, "\n"); exists { + cfgParams.MainHTTPSnippets = mainHTTPSnippets } - if locationSnippets, exists, err := GetMapKeyAsStringSlice(cfgm.Data, "location-snippets", cfgm, "\n"); exists { - if err != nil { - glog.Error(err) - } else { - cfgParams.LocationSnippets = locationSnippets - } + if locationSnippets, exists := GetMapKeyAsStringSlice(cfgm.Data, "location-snippets", cfgm, "\n"); exists { + cfgParams.LocationSnippets = locationSnippets } - if serverSnippets, exists, err := GetMapKeyAsStringSlice(cfgm.Data, "server-snippets", cfgm, "\n"); exists { - if err != nil { - glog.Error(err) - } else { - cfgParams.ServerSnippets = serverSnippets - } + if serverSnippets, exists := GetMapKeyAsStringSlice(cfgm.Data, "server-snippets", cfgm, "\n"); exists { + cfgParams.ServerSnippets = serverSnippets } if _, exists, err := GetMapKeyAsInt(cfgm.Data, "worker-processes", cfgm); exists { @@ -373,23 +337,15 @@ func ParseConfigMap(cfgm *v1.ConfigMap, nginxPlus bool, hasAppProtect bool, hasA cfgParams.VirtualServerTemplate = &virtualServerTemplate } - if mainStreamSnippets, exists, err := GetMapKeyAsStringSlice(cfgm.Data, "stream-snippets", cfgm, "\n"); exists { - if err != nil { - glog.Error(err) - } else { - cfgParams.MainStreamSnippets = mainStreamSnippets - } + if mainStreamSnippets, exists := GetMapKeyAsStringSlice(cfgm.Data, "stream-snippets", cfgm, "\n"); exists { + cfgParams.MainStreamSnippets = mainStreamSnippets } - if resolverAddresses, exists, err := GetMapKeyAsStringSlice(cfgm.Data, "resolver-addresses", cfgm, ","); exists { - if err != nil { - glog.Error(err) + if resolverAddresses, exists := GetMapKeyAsStringSlice(cfgm.Data, "resolver-addresses", cfgm, ","); exists { + if nginxPlus { + cfgParams.ResolverAddresses = resolverAddresses } else { - if nginxPlus { - cfgParams.ResolverAddresses = resolverAddresses - } else { - glog.Warning("ConfigMap key 'resolver-addresses' requires NGINX Plus") - } + glog.Warning("ConfigMap key 'resolver-addresses' requires NGINX Plus") } } @@ -521,12 +477,8 @@ func ParseConfigMap(cfgm *v1.ConfigMap, nginxPlus bool, hasAppProtect bool, hasA } if hasAppProtectDos { - if appProtectDosLogFormat, exists, err := GetMapKeyAsStringSlice(cfgm.Data, "app-protect-dos-log-format", cfgm, "\n"); exists { - if err != nil { - glog.Error(err) - } else { - cfgParams.MainAppProtectDosLogFormat = appProtectDosLogFormat - } + if appProtectDosLogFormat, exists := GetMapKeyAsStringSlice(cfgm.Data, "app-protect-dos-log-format", cfgm, "\n"); exists { + cfgParams.MainAppProtectDosLogFormat = appProtectDosLogFormat } if appProtectDosLogFormatEscaping, exists := cfgm.Data["app-protect-dos-log-format-escaping"]; exists { diff --git a/internal/configs/parsing_helpers.go b/internal/configs/parsing_helpers.go index 3883056099..831c30f034 100644 --- a/internal/configs/parsing_helpers.go +++ b/internal/configs/parsing_helpers.go @@ -79,13 +79,12 @@ func GetMapKeyAsUint64(m map[string]string, key string, context apiObject, nonZe } // GetMapKeyAsStringSlice tries to find and parse a key in the map as string slice splitting it on delimiter. -func GetMapKeyAsStringSlice(m map[string]string, key string, _ apiObject, delimiter string) ([]string, bool, error) { +func GetMapKeyAsStringSlice(m map[string]string, key string, _ apiObject, delimiter string) ([]string, bool) { if str, exists := m[key]; exists { slice := strings.Split(str, delimiter) - return slice, exists, nil + return slice, exists } - - return nil, false, nil + return nil, false } // ParseLBMethod parses method and matches it to a corresponding load balancing method in NGINX. An error is returned if method is not valid. diff --git a/internal/configs/parsing_helpers_test.go b/internal/configs/parsing_helpers_test.go index e406208686..ac5cdd78f9 100644 --- a/internal/configs/parsing_helpers_test.go +++ b/internal/configs/parsing_helpers_test.go @@ -229,10 +229,7 @@ func TestGetMapKeyAsStringSlice(t *testing.T) { "key": "1.String,2.String,3.String", } - slice, exists, err := GetMapKeyAsStringSlice(configMap.Data, "key", &configMap, ",") - if err != nil { - t.Errorf("Unexpected error: %v", err) - } + slice, exists := GetMapKeyAsStringSlice(configMap.Data, "key", &configMap, ",") if !exists { t.Errorf("The key 'key' must exist in the configMap") } @@ -253,10 +250,7 @@ func TestGetMapKeyAsStringSliceMultilineSnippets(t *testing.T) { }`, } - slice, exists, err := GetMapKeyAsStringSlice(configMap.Data, "server-snippets", &configMap, "\n") - if err != nil { - t.Errorf("Unexpected error: %v", err) - } + slice, exists := GetMapKeyAsStringSlice(configMap.Data, "server-snippets", &configMap, "\n") if !exists { t.Errorf("The key 'server-snippets' must exist in the configMap") } @@ -272,7 +266,7 @@ func TestGetMapKeyAsStringSliceNotFound(t *testing.T) { configMap := configMap configMap.Data = map[string]string{} - _, exists, _ := GetMapKeyAsStringSlice(configMap.Data, "key", &configMap, ",") + _, exists := GetMapKeyAsStringSlice(configMap.Data, "key", &configMap, ",") if exists { t.Errorf("The key 'key' must not exist in the configMap") } From 6b0a2c9dc84166fd77db1512d031e59db78694d6 Mon Sep 17 00:00:00 2001 From: Luca Comellini Date: Tue, 27 Sep 2022 17:34:37 -0700 Subject: [PATCH 006/105] Add `ARCH` in Makefile, make Dockerfile more compatible with podman (#3102) --- Makefile | 7 ++++--- build/Dockerfile | 10 +++++----- build/README.md | 2 +- .../installation/building-ingress-controller-image.md | 3 ++- 4 files changed, 12 insertions(+), 10 deletions(-) diff --git a/Makefile b/Makefile index 3b1a05a127..a81a910e8c 100644 --- a/Makefile +++ b/Makefile @@ -10,9 +10,10 @@ PREFIX = nginx/nginx-ingress## The name of the image. For example, nginx/nginx-i TAG = $(VERSION:v%=%)## The tag of the image. For example, 2.0.0 TARGET ?= local## The target of the build. Possible values: local, container and download override DOCKER_BUILD_OPTIONS += --build-arg IC_VERSION=$(VERSION) --build-arg GIT_COMMIT=$(GIT_COMMIT)## The options for the docker build command. For example, --pull. +ARCH ?= amd64## The architecture of the image or binary. For example: amd64, arm64, ppc64le, s390x. Not all architectures are supported for all targets. # final docker build command -DOCKER_CMD = docker build $(strip $(DOCKER_BUILD_OPTIONS)) --target $(strip $(TARGET)) -f build/Dockerfile -t $(strip $(PREFIX)):$(strip $(TAG)) . +DOCKER_CMD = docker build --platform linux/$(ARCH) $(strip $(DOCKER_BUILD_OPTIONS)) --target $(strip $(TARGET)) -f build/Dockerfile -t $(strip $(PREFIX)):$(strip $(TAG)) . export DOCKER_BUILDKIT = 1 @@ -71,7 +72,7 @@ build: ## Build Ingress Controller binary @docker -v || (code=$$?; printf "\033[0;31mError\033[0m: there was a problem with Docker\n"; exit $$code) ifeq (${TARGET},local) @go version || (code=$$?; printf "\033[0;31mError\033[0m: unable to build locally, try using the parameter TARGET=container or TARGET=download\n"; exit $$code) - CGO_ENABLED=0 GOOS=linux go build -trimpath -ldflags "-s -w -X main.version=${VERSION}" -o nginx-ingress github.com/nginxinc/kubernetes-ingress/cmd/nginx-ingress + CGO_ENABLED=0 GOOS=linux GOARCH=$(ARCH) go build -trimpath -ldflags "-s -w -X main.version=${VERSION}" -o nginx-ingress github.com/nginxinc/kubernetes-ingress/cmd/nginx-ingress else ifeq (${TARGET},download) @$(MAKE) download-binary-docker endif @@ -89,7 +90,7 @@ endif .PHONY: build-goreleaser build-goreleaser: ## Build Ingress Controller binary using GoReleaser @goreleaser -v || (code=$$?; printf "\033[0;31mError\033[0m: there was a problem with GoReleaser. Follow the docs to install it https://goreleaser.com/install\n"; exit $$code) - GOOS=linux GOPATH=$(shell go env GOPATH) goreleaser build --rm-dist --debug --snapshot --id kubernetes-ingress --single-target + GOOS=linux GOPATH=$(shell go env GOPATH) GOARCH=$(ARCH) goreleaser build --rm-dist --debug --snapshot --id kubernetes-ingress --single-target .PHONY: debian-image debian-image: build ## Create Docker image for Ingress Controller (Debian) diff --git a/build/Dockerfile b/build/Dockerfile index 76a0841c42..a842de0c1c 100644 --- a/build/Dockerfile +++ b/build/Dockerfile @@ -198,24 +198,24 @@ ARG TARGETPLATFORM ARG NAP_MODULES=none # copy oidc files on plus build -RUN --mount=target=/tmp [ -n "${BUILD_OS##*plus*}" ] && exit 0; mkdir -p etc/nginx/oidc/ && cp -a /tmp/internal/configs/oidc/* /etc/nginx/oidc/ +RUN --mount=type=bind,target=/tmp [ -n "${BUILD_OS##*plus*}" ] && exit 0; mkdir -p /etc/nginx/oidc/ && cp -a /tmp/internal/configs/oidc/* /etc/nginx/oidc/ # run only on nap waf build -RUN --mount=target=/tmp [ -n "${NAP_MODULES##*waf*}" ] && exit 0; mkdir -p /etc/nginx/waf/nac-policies /etc/nginx/waf/nac-logconfs /etc/nginx/waf/nac-usersigs /var/log/app_protect /opt/app_protect \ +RUN --mount=type=bind,target=/tmp [ -n "${NAP_MODULES##*waf*}" ] && exit 0; mkdir -p /etc/nginx/waf/nac-policies /etc/nginx/waf/nac-logconfs /etc/nginx/waf/nac-usersigs /var/log/app_protect /opt/app_protect \ && chown -R 101:0 /etc/app_protect /usr/share/ts /var/log/app_protect/ /opt/app_protect/ /var/log/nginx/ \ && touch /etc/nginx/waf/nac-usersigs/index.conf \ && cp -a /tmp/build/log-default.json /etc/nginx # run only on nap dos build -RUN --mount=target=/tmp [ -n "${NAP_MODULES##*dos*}" ] && exit 0; mkdir -p /root/app_protect_dos /etc/nginx/dos/policies /etc/nginx/dos/logconfs /shared/cores /var/log/adm /var/run/adm \ +RUN [ -n "${NAP_MODULES##*dos*}" ] && exit 0; mkdir -p /root/app_protect_dos /etc/nginx/dos/policies /etc/nginx/dos/logconfs /shared/cores /var/log/adm /var/run/adm \ && chmod 777 /shared/cores /var/log/adm /var/run/adm /etc/app_protect_dos -RUN --mount=target=/tmp mkdir -p /var/lib/nginx /etc/nginx/secrets /etc/nginx/stream-conf.d \ +RUN --mount=type=bind,target=/tmp mkdir -p /var/lib/nginx /etc/nginx/secrets /etc/nginx/stream-conf.d \ && setcap 'cap_net_bind_service=+ep' /usr/sbin/nginx 'cap_net_bind_service=+ep' /usr/sbin/nginx-debug \ && setcap -v 'cap_net_bind_service=+ep' /usr/sbin/nginx 'cap_net_bind_service=+ep' /usr/sbin/nginx-debug \ && [ -z "${BUILD_OS##*plus*}" ] && PLUS=-plus; cp -a /tmp/internal/configs/version1/nginx$PLUS.ingress.tmpl /tmp/internal/configs/version1/nginx$PLUS.tmpl \ /tmp/internal/configs/version2/nginx$PLUS.virtualserver.tmpl /tmp/internal/configs/version2/nginx$PLUS.transportserver.tmpl / \ - && chown -R 101:0 /etc/nginx /etc/nginx/secrets /var/cache/nginx /var/lib/nginx /*.tmpl \ + && chown -R 101:0 /etc/nginx /var/cache/nginx /var/lib/nginx /*.tmpl \ && rm -f /etc/nginx/conf.d/* /etc/apt/apt.conf.d/90pkgs-nginx /etc/apt/sources.list.d/nginx-plus.list # Uncomment the line below if you would like to add the default.pem to the image diff --git a/build/README.md b/build/README.md index 66c0fc4592..8bc31e68ab 100644 --- a/build/README.md +++ b/build/README.md @@ -1,3 +1,3 @@ # NGINX Ingress Controller -This doc is now available at https://docs.nginx.com/nginx-ingress-controller/installation/building-ingress-controller-image/ \ No newline at end of file +This doc is now available at https://docs.nginx.com/nginx-ingress-controller/installation/building-ingress-controller-image/ diff --git a/docs/content/installation/building-ingress-controller-image.md b/docs/content/installation/building-ingress-controller-image.md index 4edc188e78..2b1795b060 100644 --- a/docs/content/installation/building-ingress-controller-image.md +++ b/docs/content/installation/building-ingress-controller-image.md @@ -105,7 +105,8 @@ A few other useful targets: ### Makefile Variables The **Makefile** contains the following main variables for you to customize (either by changing the Makefile or by overriding the variables in the make command): +* **ARCH** -- the architecture of the image (and the binary). The default value is `amd64`. The most common architectures are `amd64` and `arm64`. Some other popular options are `arm`, `ppc64le` and `s390x`. * **PREFIX** -- the name of the image. The default is `nginx/nginx-ingress`. * **TAG** -- the tag added to the image. It's set to the version of the Ingress Controller by default. * **DOCKER_BUILD_OPTIONS** -- the [options](https://docs.docker.com/engine/reference/commandline/build/#options) for the `docker build` command. For example, `--pull`. -* **TARGET** -- By default, the Ingress Controller is compiled locally using a `local` golang environment. If you want to compile the Ingress Controller using your local golang environment, make sure that the Ingress Controller repo is in your `$GOPATH`. To compile the Ingress Controller using the Docker [golang](https://hub.docker.com/_/golang/) container, specify `TARGET=container`. If you checked out a tag or are on the latest commit on `main` you can specify `TARGET=download` to avoid compiling the binary. +* **TARGET** -- by default, the Ingress Controller is compiled locally using a `local` golang environment. If you want to compile the Ingress Controller using your local golang environment, make sure that the Ingress Controller repo is in your `$GOPATH`. To compile the Ingress Controller using the Docker [golang](https://hub.docker.com/_/golang/) container, specify `TARGET=container`. If you checked out a tag or are on the latest commit on `main` you can specify `TARGET=download` to avoid compiling the binary. From 88fb869a97b19eda9e4ed428101923b17e2f89b5 Mon Sep 17 00:00:00 2001 From: Dong Wang Date: Wed, 28 Sep 2022 11:40:07 +0800 Subject: [PATCH 007/105] Fix helm chart issue when set controller.strategy (#3106) --- deployments/helm-chart/templates/controller-daemonset.yaml | 2 +- deployments/helm-chart/templates/controller-deployment.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/deployments/helm-chart/templates/controller-daemonset.yaml b/deployments/helm-chart/templates/controller-daemonset.yaml index f097814afd..a318bfc0f6 100644 --- a/deployments/helm-chart/templates/controller-daemonset.yaml +++ b/deployments/helm-chart/templates/controller-daemonset.yaml @@ -216,7 +216,6 @@ spec: {{- if .Values.controller.initContainers }} initContainers: {{ toYaml .Values.controller.initContainers | nindent 8 }} {{- end }} -{{- end }} {{- if .Values.controller.strategy }} updateStrategy: {{ toYaml .Values.controller.strategy | indent 4 }} @@ -224,3 +223,4 @@ spec: {{- if .Values.controller.minReadySeconds }} minReadySeconds: {{ .Values.controller.minReadySeconds }} {{- end }} +{{- end }} diff --git a/deployments/helm-chart/templates/controller-deployment.yaml b/deployments/helm-chart/templates/controller-deployment.yaml index e8b20d4252..55b6f27f03 100644 --- a/deployments/helm-chart/templates/controller-deployment.yaml +++ b/deployments/helm-chart/templates/controller-deployment.yaml @@ -219,7 +219,6 @@ spec: {{- if .Values.controller.initContainers }} initContainers: {{ toYaml .Values.controller.initContainers | nindent 8 }} {{- end }} -{{- end }} {{- if .Values.controller.strategy }} strategy: {{ toYaml .Values.controller.strategy | indent 4 }} @@ -227,3 +226,4 @@ spec: {{- if .Values.controller.minReadySeconds }} minReadySeconds: {{ .Values.controller.minReadySeconds }} {{- end }} +{{- end }} From 3406737af4811a54fe13d5e833002b6e120d6f8a Mon Sep 17 00:00:00 2001 From: Haywood Shannon <5781935+haywoodsh@users.noreply.github.com> Date: Wed, 28 Sep 2022 14:41:27 +0100 Subject: [PATCH 008/105] Optimise path validation (#3094) * update path validation --- internal/k8s/validation.go | 84 ++++++++- internal/k8s/validation_test.go | 163 ++++++++++++++++++ .../validation/virtualserver_test.go | 6 + 3 files changed, 250 insertions(+), 3 deletions(-) diff --git a/internal/k8s/validation.go b/internal/k8s/validation.go index 4d51fce423..f5f6a5fb8e 100644 --- a/internal/k8s/validation.go +++ b/internal/k8s/validation.go @@ -72,18 +72,15 @@ const ( const ( commaDelimiter = "," annotationValueFmt = `([^"$\\]|\\[^$])*` - pathFmt = `/[^\s{};\\]*` jwtTokenValueFmt = "\\$" + annotationValueFmt ) const ( annotationValueFmtErrMsg = `a valid annotation value must have all '"' escaped and must not contain any '$' or end with an unescaped '\'` - pathErrMsg = "must start with / and must not include any whitespace character, `{`, `}` or `;`" jwtTokenValueFmtErrMsg = `a valid annotation value must start with '$', have all '"' escaped, and must not contain any '$' or end with an unescaped '\'` ) var ( - pathRegexp = regexp.MustCompile("^" + pathFmt + "$") validAnnotationValueRegex = regexp.MustCompile("^" + annotationValueFmt + "$") validJWTTokenAnnotationValueRegex = regexp.MustCompile("^" + jwtTokenValueFmt + "$") ) @@ -875,6 +872,13 @@ func validateBackend(backend *networking.IngressBackend, fieldPath *field.Path) return allErrs } +const ( + pathFmt = `/[^\s;]*` + pathErrMsg = "must start with / and must not include any whitespace character or `;`" +) + +var pathRegexp = regexp.MustCompile("^" + pathFmt + "$") + func validatePath(path string, fieldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} @@ -887,6 +891,80 @@ func validatePath(path string, fieldPath *field.Path) field.ErrorList { return append(allErrs, field.Invalid(fieldPath, path, msg)) } + allErrs = append(allErrs, validateRegexPath(path, fieldPath)...) + allErrs = append(allErrs, validateCurlyBraces(path, fieldPath)...) + allErrs = append(allErrs, validateIllegalKeywords(path, fieldPath)...) + + return allErrs +} + +func validateRegexPath(path string, fieldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + if _, err := regexp.Compile(path); err != nil { + return append(allErrs, field.Invalid(fieldPath, path, fmt.Sprintf("must be a valid regular expression: %v", err))) + } + + if err := ValidateEscapedString(path, "*.jpg", "^/images/image_*.png$"); err != nil { + return append(allErrs, field.Invalid(fieldPath, path, err.Error())) + } + + return allErrs +} + +const ( + curlyBracesFmt = `\{(.*?)\}` + alphabetFmt = `[A-Za-z]` + curlyBracesMsg = `must not include curly braces containing alphabetical characters` +) + +var ( + curlyBracesFmtRegexp = regexp.MustCompile(curlyBracesFmt) + alphabetFmtRegexp = regexp.MustCompile(alphabetFmt) +) + +func validateCurlyBraces(path string, fieldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + bracesContents := curlyBracesFmtRegexp.FindAllStringSubmatch(path, -1) + for _, v := range bracesContents { + if alphabetFmtRegexp.MatchString(v[1]) { + return append(allErrs, field.Invalid(fieldPath, path, curlyBracesMsg)) + } + } + return allErrs +} + +const ( + escapedStringsFmt = `([^"\\]|\\.)*` + escapedStringsErrMsg = `must have all '"' (double quotes) escaped and must not end with an unescaped '\' (backslash)` +) + +var escapedStringsFmtRegexp = regexp.MustCompile("^" + escapedStringsFmt + "$") + +// ValidateEscapedString validates an escaped string. +func ValidateEscapedString(body string, examples ...string) error { + if !escapedStringsFmtRegexp.MatchString(body) { + msg := validation.RegexError(escapedStringsErrMsg, escapedStringsFmt, examples...) + return fmt.Errorf(msg) + } + return nil +} + +const ( + illegalKeywordFmt = `/etc/|/root|/var|\\n|\\r` + illegalKeywordErrMsg = `must not contain invalid paths` +) + +var illegalKeywordFmtRegexp = regexp.MustCompile("^" + illegalKeywordFmt + "$") + +func validateIllegalKeywords(path string, fieldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + if illegalKeywordFmtRegexp.MatchString(path) { + return append(allErrs, field.Invalid(fieldPath, path, illegalKeywordErrMsg)) + } + return allErrs } diff --git a/internal/k8s/validation_test.go b/internal/k8s/validation_test.go index 301c9e1e71..1a383e028f 100644 --- a/internal/k8s/validation_test.go +++ b/internal/k8s/validation_test.go @@ -3335,3 +3335,166 @@ func TestGetSpecServices(t *testing.T) { } } } + +func TestValidateRegexPath(t *testing.T) { + t.Parallel() + tests := []struct { + regexPath string + msg string + }{ + { + regexPath: "/foo.*\\.jpg", + msg: "case sensitive regexp", + }, + { + regexPath: "/Bar.*\\.jpg", + msg: "case insensitive regexp", + }, + { + regexPath: `/f\"oo.*\\.jpg`, + msg: "regexp with escaped double quotes", + }, + { + regexPath: "/[0-9a-z]{4}[0-9]+", + msg: "regexp with curly braces", + }, + } + + for _, test := range tests { + allErrs := validateRegexPath(test.regexPath, field.NewPath("path")) + if len(allErrs) != 0 { + t.Errorf("validateRegexPath(%v) returned errors for valid input for the case of %v", test.regexPath, test.msg) + } + } +} + +func TestValidateRegexPathFails(t *testing.T) { + t.Parallel() + tests := []struct { + regexPath string + msg string + }{ + { + regexPath: "[{", + msg: "invalid regexp", + }, + { + regexPath: `/foo"`, + msg: "unescaped double quotes", + }, + { + regexPath: `"`, + msg: "empty regex", + }, + { + regexPath: `/foo\`, + msg: "ending in backslash", + }, + } + + for _, test := range tests { + allErrs := validateRegexPath(test.regexPath, field.NewPath("path")) + if len(allErrs) == 0 { + t.Errorf("validateRegexPath(%v) returned no errors for invalid input for the case of %v", test.regexPath, test.msg) + } + } +} + +func TestValidatePath(t *testing.T) { + t.Parallel() + + validPaths := []string{ + "/", + "/path", + "/a-1/_A/", + "/[A-Za-z]{6}/[a-z]{1,2}", + "/[0-9a-z]{4}[0-9]", + "/foo.*\\.jpg", + "/Bar.*\\.jpg", + `/f\"oo.*\\.jpg`, + "/[0-9a-z]{4}[0-9]+", + "/[a-z]{1,2}", + "/[A-Z]{6}", + "/[A-Z]{6}/[a-z]{1,2}", + "/path", + "/abc}{abc", + } + + for _, path := range validPaths { + allErrs := validatePath(path, field.NewPath("path")) + if len(allErrs) > 0 { + t.Errorf("validatePath(%q) returned errors %v for valid input", path, allErrs) + } + } + + invalidPaths := []string{ + "", + " /", + "/ ", + "/abc;", + `/path\`, + `/path\n`, + `/var/run/secrets`, + "/{autoindex on; root /var/run/secrets;}location /tea", + "/{root}", + } + + for _, path := range invalidPaths { + allErrs := validatePath(path, field.NewPath("path")) + if len(allErrs) == 0 { + t.Errorf("validatePath(%q) returned no errors for invalid input", path) + } + } +} + +func TestValidateCurlyBraces(t *testing.T) { + t.Parallel() + + validPaths := []string{ + "/[a-z]{1,2}", + "/[A-Z]{6}", + "/[A-Z]{6}/[a-z]{1,2}", + "/path", + "/abc}{abc", + } + + for _, path := range validPaths { + allErrs := validateCurlyBraces(path, field.NewPath("path")) + if len(allErrs) > 0 { + t.Errorf("validatePath(%q) returned errors %v for valid input", path, allErrs) + } + } + + invalidPaths := []string{ + "/[A-Z]{a}", + "/{abc}abc", + "/abc{a1}", + } + + for _, path := range invalidPaths { + allErrs := validateCurlyBraces(path, field.NewPath("path")) + if len(allErrs) == 0 { + t.Errorf("validateCurlyBraces(%q) returned no errors for invalid input", path) + } + } +} + +func TestValidateIllegalKeywords(t *testing.T) { + t.Parallel() + + invalidPaths := []string{ + "/root", + "/etc/nginx/secrets", + "/etc/passwd", + "/var/run/secrets", + `\n`, + `\r`, + } + + for _, path := range invalidPaths { + allErrs := validateIllegalKeywords(path, field.NewPath("path")) + if len(allErrs) == 0 { + t.Errorf("validateCurlyBraces(%q) returned no errors for invalid input", path) + } + } +} diff --git a/pkg/apis/configuration/validation/virtualserver_test.go b/pkg/apis/configuration/validation/virtualserver_test.go index de16fef91a..2280e48935 100644 --- a/pkg/apis/configuration/validation/virtualserver_test.go +++ b/pkg/apis/configuration/validation/virtualserver_test.go @@ -1465,6 +1465,10 @@ func TestValidateRegexPath(t *testing.T) { regexPath: `~ ^/f\"oo.*\\.jpg`, msg: "regexp with escaped double quotes", }, + { + regexPath: "~ [0-9a-z]{4}[0-9]+", + msg: "regexp with curly braces", + }, } for _, test := range tests { @@ -1526,6 +1530,8 @@ func TestValidateRoutePath(t *testing.T) { invalidPaths := []string{ "", "invalid", + // regex without preceding "~*" modifier + "^/foo.*\\.jpg", } for _, path := range invalidPaths { From f07a0a8b8f76d683a05b481004598dd3088c2f2c Mon Sep 17 00:00:00 2001 From: Jakub Jarosz <99677300+jjngx@users.noreply.github.com> Date: Thu, 29 Sep 2022 10:44:22 +0100 Subject: [PATCH 009/105] Fix staticcheck linter issues in k8s, nginx packages (#3107) * Fix staticcheck linter issues in k8s package * Fix linter issues in nginx package --- internal/k8s/controller.go | 42 +++++----- internal/k8s/controller_test.go | 141 ++++++++++++++++---------------- internal/k8s/handlers.go | 2 +- internal/k8s/handlers_test.go | 2 +- internal/k8s/status.go | 13 ++- internal/k8s/task_queue.go | 4 +- internal/nginx/manager.go | 4 +- internal/nginx/utils.go | 8 +- 8 files changed, 107 insertions(+), 109 deletions(-) diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index 75e2eec488..3f8e2f5358 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -2609,19 +2609,19 @@ func (lbc *LoadBalancerController) createIngressEx(ing *networking.Ingress, vali func (lbc *LoadBalancerController) getAppProtectLogConfAndDst(ing *networking.Ingress) ([]configs.AppProtectLog, error) { var apLogs []configs.AppProtectLog if _, exists := ing.Annotations[configs.AppProtectLogConfDstAnnotation]; !exists { - return apLogs, fmt.Errorf("Error: %v requires %v in %v", configs.AppProtectLogConfAnnotation, configs.AppProtectLogConfDstAnnotation, ing.Name) + return apLogs, fmt.Errorf("error: %v requires %v in %v", configs.AppProtectLogConfAnnotation, configs.AppProtectLogConfDstAnnotation, ing.Name) } logDsts := strings.Split(ing.Annotations[configs.AppProtectLogConfDstAnnotation], ",") logConfNsNs := appprotectcommon.ParseResourceReferenceAnnotationList(ing.Namespace, ing.Annotations[configs.AppProtectLogConfAnnotation]) if len(logDsts) != len(logConfNsNs) { - return apLogs, fmt.Errorf("Error Validating App Protect Destination and Config for Ingress %v: LogConf and LogDestination must have equal number of items", ing.Name) + return apLogs, fmt.Errorf("error Validating App Protect Destination and Config for Ingress %v: LogConf and LogDestination must have equal number of items", ing.Name) } for i, logConfNsN := range logConfNsNs { logConf, err := lbc.appProtectConfiguration.GetAppResource(appprotect.LogConfGVK.Kind, logConfNsN) if err != nil { - return apLogs, fmt.Errorf("Error retrieving App Protect Log Config for Ingress %v: %w", ing.Name, err) + return apLogs, fmt.Errorf("error retrieving App Protect Log Config for Ingress %v: %w", ing.Name, err) } apLogs = append(apLogs, configs.AppProtectLog{ LogConf: logConf, @@ -2637,7 +2637,7 @@ func (lbc *LoadBalancerController) getAppProtectPolicy(ing *networking.Ingress) apPolicy, err = lbc.appProtectConfiguration.GetAppResource(appprotect.PolicyGVK.Kind, polNsN) if err != nil { - return nil, fmt.Errorf("Error retrieving App Protect Policy for Ingress %v: %w", ing.Name, err) + return nil, fmt.Errorf("error retrieving App Protect Policy for Ingress %v: %w", ing.Name, err) } return apPolicy, nil @@ -2943,12 +2943,12 @@ func (lbc *LoadBalancerController) getPolicies(policies []conf_v1.PolicyReferenc } } if err != nil { - errors = append(errors, fmt.Errorf("Failed to get policy %s: %w", policyKey, err)) + errors = append(errors, fmt.Errorf("failed to get policy %s: %w", policyKey, err)) continue } if !exists { - errors = append(errors, fmt.Errorf("Policy %s doesn't exist", policyKey)) + errors = append(errors, fmt.Errorf("policy %s doesn't exist", policyKey)) continue } @@ -2961,7 +2961,7 @@ func (lbc *LoadBalancerController) getPolicies(policies []conf_v1.PolicyReferenc err = validation.ValidatePolicy(policy, lbc.isNginxPlus, lbc.enableOIDC, lbc.appProtectEnabled) if err != nil { - errors = append(errors, fmt.Errorf("Policy %s is invalid: %w", policyKey, err)) + errors = append(errors, fmt.Errorf("policy %s is invalid: %w", policyKey, err)) continue } @@ -3238,7 +3238,7 @@ func (lbc *LoadBalancerController) createTransportServerEx(transportServer *conf func (lbc *LoadBalancerController) getEndpointsForUpstream(namespace string, upstreamService string, upstreamPort uint16) (endps []podEndpoint, isExternal bool, err error) { svc, err := lbc.getServiceForUpstream(namespace, upstreamService, upstreamPort) if err != nil { - return nil, false, fmt.Errorf("Error getting service %v: %w", upstreamService, err) + return nil, false, fmt.Errorf("error getting service %v: %w", upstreamService, err) } backend := &networking.IngressBackend{ @@ -3252,7 +3252,7 @@ func (lbc *LoadBalancerController) getEndpointsForUpstream(namespace string, ups endps, isExternal, err = lbc.getEndpointsForIngressBackend(backend, svc) if err != nil { - return nil, false, fmt.Errorf("Error retrieving endpoints for the service %v: %w", upstreamService, err) + return nil, false, fmt.Errorf("error retrieving endpoints for the service %v: %w", upstreamService, err) } return endps, isExternal, err @@ -3261,7 +3261,7 @@ func (lbc *LoadBalancerController) getEndpointsForUpstream(namespace string, ups func (lbc *LoadBalancerController) getEndpointsForSubselector(namespace string, upstream conf_v1.Upstream) (endps []podEndpoint, err error) { svc, err := lbc.getServiceForUpstream(namespace, upstream.Service, upstream.Port) if err != nil { - return nil, fmt.Errorf("Error getting service %v: %w", upstream.Service, err) + return nil, fmt.Errorf("error getting service %v: %w", upstream.Service, err) } var targetPort int32 @@ -3270,19 +3270,19 @@ func (lbc *LoadBalancerController) getEndpointsForSubselector(namespace string, if port.Port == int32(upstream.Port) { targetPort, err = lbc.getTargetPort(port, svc) if err != nil { - return nil, fmt.Errorf("Error determining target port for port %v in service %v: %w", upstream.Port, svc.Name, err) + return nil, fmt.Errorf("error determining target port for port %v in service %v: %w", upstream.Port, svc.Name, err) } break } } if targetPort == 0 { - return nil, fmt.Errorf("No port %v in service %s", upstream.Port, svc.Name) + return nil, fmt.Errorf("no port %v in service %s", upstream.Port, svc.Name) } endps, err = lbc.getEndpointsForServiceWithSubselector(targetPort, upstream.Subselector, svc) if err != nil { - return nil, fmt.Errorf("Error retrieving endpoints for the service %v: %w", upstream.Service, err) + return nil, fmt.Errorf("error retrieving endpoints for the service %v: %w", upstream.Service, err) } return endps, err @@ -3297,7 +3297,7 @@ func (lbc *LoadBalancerController) getEndpointsForServiceWithSubselector(targetP } } if err != nil { - return nil, fmt.Errorf("Error getting pods in namespace %v that match the selector %v: %w", svc.Namespace, labels.Merge(svc.Spec.Selector, subselector), err) + return nil, fmt.Errorf("error getting pods in namespace %v that match the selector %v: %w", svc.Namespace, labels.Merge(svc.Spec.Selector, subselector), err) } var svcEps api_v1.Endpoints @@ -3432,7 +3432,7 @@ func (lbc *LoadBalancerController) getEndpointsForIngressBackend(backend *networ if err != nil { if svc.Spec.Type == api_v1.ServiceTypeExternalName { if !lbc.isNginxPlus { - return nil, false, fmt.Errorf("Type ExternalName Services feature is only available in NGINX Plus") + return nil, false, fmt.Errorf("type ExternalName Services feature is only available in NGINX Plus") } result = lbc.getExternalEndpointsForIngressBackend(backend, svc) return result, true, nil @@ -3457,14 +3457,14 @@ func (lbc *LoadBalancerController) getEndpointsForPort(endps api_v1.Endpoints, b if (backendPort.Name == "" && port.Port == backendPort.Number) || port.Name == backendPort.Name { targetPort, err = lbc.getTargetPort(port, svc) if err != nil { - return nil, fmt.Errorf("Error determining target port for port %v in Ingress: %w", backendPort, err) + return nil, fmt.Errorf("error determining target port for port %v in Ingress: %w", backendPort, err) } break } } if targetPort == 0 { - return nil, fmt.Errorf("No port %v in service %s", backendPort, svc.Name) + return nil, fmt.Errorf("no port %v in service %s", backendPort, svc.Name) } for _, subset := range endps.Subsets { @@ -3489,7 +3489,7 @@ func (lbc *LoadBalancerController) getEndpointsForPort(endps api_v1.Endpoints, b } } - return nil, fmt.Errorf("No endpoints for target port %v in service %s", targetPort, svc.Name) + return nil, fmt.Errorf("no endpoints for target port %v in service %s", targetPort, svc.Name) } func (lbc *LoadBalancerController) getPodOwnerTypeAndNameFromAddress(ns, name string) (parentType, parentName string) { @@ -3556,18 +3556,18 @@ func (lbc *LoadBalancerController) getTargetPort(svcPort api_v1.ServicePort, svc } } if err != nil { - return 0, fmt.Errorf("Error getting pod information: %w", err) + return 0, fmt.Errorf("error getting pod information: %w", err) } if len(pods) == 0 { - return 0, fmt.Errorf("No pods of service %s", svc.Name) + return 0, fmt.Errorf("no pods of service %s", svc.Name) } pod := pods[0] portNum, err := findPort(pod, svcPort) if err != nil { - return 0, fmt.Errorf("Error finding named port %v in pod %s: %w", svcPort, pod.Name, err) + return 0, fmt.Errorf("error finding named port %v in pod %s: %w", svcPort, pod.Name, err) } return portNum, nil diff --git a/internal/k8s/controller_test.go b/internal/k8s/controller_test.go index c13a289c7e..6034380aa1 100644 --- a/internal/k8s/controller_test.go +++ b/internal/k8s/controller_test.go @@ -19,7 +19,6 @@ import ( conf_v1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1" conf_v1alpha1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1alpha1" api_v1 "k8s.io/api/core/v1" - v1 "k8s.io/api/core/v1" networking "k8s.io/api/networking/v1" meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -221,43 +220,43 @@ func TestIngressClassForCustomResources(t *testing.T) { func TestComparePorts(t *testing.T) { scenarios := []struct { - sp v1.ServicePort - cp v1.ContainerPort + sp api_v1.ServicePort + cp api_v1.ContainerPort expected bool }{ { // match TargetPort.strval and Protocol - v1.ServicePort{ + api_v1.ServicePort{ TargetPort: intstr.FromString("name"), - Protocol: v1.ProtocolTCP, + Protocol: api_v1.ProtocolTCP, }, - v1.ContainerPort{ + api_v1.ContainerPort{ Name: "name", - Protocol: v1.ProtocolTCP, + Protocol: api_v1.ProtocolTCP, ContainerPort: 80, }, true, }, { // don't match Name and Protocol - v1.ServicePort{ + api_v1.ServicePort{ Name: "name", - Protocol: v1.ProtocolTCP, + Protocol: api_v1.ProtocolTCP, }, - v1.ContainerPort{ + api_v1.ContainerPort{ Name: "name", - Protocol: v1.ProtocolTCP, + Protocol: api_v1.ProtocolTCP, ContainerPort: 80, }, false, }, { // TargetPort intval mismatch, don't match by TargetPort.Name - v1.ServicePort{ + api_v1.ServicePort{ Name: "name", TargetPort: intstr.FromInt(80), }, - v1.ContainerPort{ + api_v1.ContainerPort{ Name: "name", ContainerPort: 81, }, @@ -265,23 +264,23 @@ func TestComparePorts(t *testing.T) { }, { // match by TargetPort intval - v1.ServicePort{ + api_v1.ServicePort{ TargetPort: intstr.IntOrString{ IntVal: 80, }, }, - v1.ContainerPort{ + api_v1.ContainerPort{ ContainerPort: 80, }, true, }, { // Fall back on ServicePort.Port if TargetPort is empty - v1.ServicePort{ + api_v1.ServicePort{ Name: "name", Port: 80, }, - v1.ContainerPort{ + api_v1.ContainerPort{ Name: "name", ContainerPort: 80, }, @@ -289,18 +288,18 @@ func TestComparePorts(t *testing.T) { }, { // TargetPort intval mismatch - v1.ServicePort{ + api_v1.ServicePort{ TargetPort: intstr.FromInt(80), }, - v1.ContainerPort{ + api_v1.ContainerPort{ ContainerPort: 81, }, false, }, { // don't match empty ports - v1.ServicePort{}, - v1.ContainerPort{}, + api_v1.ServicePort{}, + api_v1.ContainerPort{}, false, }, } @@ -313,14 +312,14 @@ func TestComparePorts(t *testing.T) { } func TestFindProbeForPods(t *testing.T) { - pods := []*v1.Pod{ + pods := []*api_v1.Pod{ { - Spec: v1.PodSpec{ - Containers: []v1.Container{ + Spec: api_v1.PodSpec{ + Containers: []api_v1.Container{ { - ReadinessProbe: &v1.Probe{ - ProbeHandler: v1.ProbeHandler{ - HTTPGet: &v1.HTTPGetAction{ + ReadinessProbe: &api_v1.Probe{ + ProbeHandler: api_v1.ProbeHandler{ + HTTPGet: &api_v1.HTTPGetAction{ Path: "/", Host: "asdf.com", Port: intstr.IntOrString{ @@ -330,11 +329,11 @@ func TestFindProbeForPods(t *testing.T) { }, PeriodSeconds: 42, }, - Ports: []v1.ContainerPort{ + Ports: []api_v1.ContainerPort{ { Name: "name", ContainerPort: 80, - Protocol: v1.ProtocolTCP, + Protocol: api_v1.ProtocolTCP, HostIP: "1.2.3.4", }, }, @@ -343,7 +342,7 @@ func TestFindProbeForPods(t *testing.T) { }, }, } - svcPort := v1.ServicePort{ + svcPort := api_v1.ServicePort{ TargetPort: intstr.FromInt(80), } probe := findProbeForPods(pods, &svcPort) @@ -351,25 +350,25 @@ func TestFindProbeForPods(t *testing.T) { t.Errorf("ServicePort.TargetPort as int match failed: %+v", probe) } - svcPort = v1.ServicePort{ + svcPort = api_v1.ServicePort{ TargetPort: intstr.FromString("name"), - Protocol: v1.ProtocolTCP, + Protocol: api_v1.ProtocolTCP, } probe = findProbeForPods(pods, &svcPort) if probe == nil || probe.PeriodSeconds != 42 { t.Errorf("ServicePort.TargetPort as string failed: %+v", probe) } - svcPort = v1.ServicePort{ + svcPort = api_v1.ServicePort{ TargetPort: intstr.FromInt(80), - Protocol: v1.ProtocolTCP, + Protocol: api_v1.ProtocolTCP, } probe = findProbeForPods(pods, &svcPort) if probe == nil || probe.PeriodSeconds != 42 { t.Errorf("ServicePort.TargetPort as int failed: %+v", probe) } - svcPort = v1.ServicePort{ + svcPort = api_v1.ServicePort{ Port: 80, } probe = findProbeForPods(pods, &svcPort) @@ -377,7 +376,7 @@ func TestFindProbeForPods(t *testing.T) { t.Errorf("ServicePort.Port should match if TargetPort is not set: %+v", probe) } - svcPort = v1.ServicePort{ + svcPort = api_v1.ServicePort{ TargetPort: intstr.FromString("wrong_name"), } probe = findProbeForPods(pods, &svcPort) @@ -385,7 +384,7 @@ func TestFindProbeForPods(t *testing.T) { t.Errorf("ServicePort.TargetPort should not have matched string: %+v", probe) } - svcPort = v1.ServicePort{ + svcPort = api_v1.ServicePort{ TargetPort: intstr.FromInt(22), } probe = findProbeForPods(pods, &svcPort) @@ -393,7 +392,7 @@ func TestFindProbeForPods(t *testing.T) { t.Errorf("ServicePort.TargetPort should not have matched int: %+v", probe) } - svcPort = v1.ServicePort{ + svcPort = api_v1.ServicePort{ Port: 22, } probe = findProbeForPods(pods, &svcPort) @@ -411,14 +410,14 @@ func TestGetServicePortForIngressPort(t *testing.T) { configurator: cnf, metricsCollector: collectors.NewControllerFakeCollector(), } - svc := v1.Service{ + svc := api_v1.Service{ TypeMeta: meta_v1.TypeMeta{}, ObjectMeta: meta_v1.ObjectMeta{ Name: "coffee-svc", Namespace: "default", }, - Spec: v1.ServiceSpec{ - Ports: []v1.ServicePort{ + Spec: api_v1.ServiceSpec{ + Ports: []api_v1.ServicePort{ { Name: "foo", Port: 80, @@ -426,7 +425,7 @@ func TestGetServicePortForIngressPort(t *testing.T) { }, }, }, - Status: v1.ServiceStatus{}, + Status: api_v1.ServiceStatus{}, } backendPort := networking.ServiceBackendPort{ Name: "foo", @@ -476,7 +475,7 @@ func TestGetEndpointsBySubselectedPods(t *testing.T) { tests := []struct { desc string targetPort int32 - svcEps v1.Endpoints + svcEps api_v1.Endpoints expectedEps []podEndpoint }{ { @@ -499,7 +498,7 @@ func TestGetEndpointsBySubselectedPods(t *testing.T) { }, } - pods := []*v1.Pod{ + pods := []*api_v1.Pod{ { ObjectMeta: meta_v1.ObjectMeta{ OwnerReferences: []meta_v1.OwnerReference{ @@ -510,22 +509,22 @@ func TestGetEndpointsBySubselectedPods(t *testing.T) { }, }, }, - Status: v1.PodStatus{ + Status: api_v1.PodStatus{ PodIP: "1.2.3.4", }, }, } - svcEps := v1.Endpoints{ - Subsets: []v1.EndpointSubset{ + svcEps := api_v1.Endpoints{ + Subsets: []api_v1.EndpointSubset{ { - Addresses: []v1.EndpointAddress{ + Addresses: []api_v1.EndpointAddress{ { IP: "1.2.3.4", Hostname: "asdf.com", }, }, - Ports: []v1.EndpointPort{ + Ports: []api_v1.EndpointPort{ { Port: 80, }, @@ -687,9 +686,9 @@ func TestGetPolicies(t *testing.T) { expectedPolicies := []*conf_v1.Policy{validPolicy} expectedErrors := []error{ - errors.New("Policy default/invalid-policy is invalid: spec: Invalid value: \"\": must specify exactly one of: `accessControl`, `rateLimit`, `ingressMTLS`, `egressMTLS`, `basicAuth`, `jwt`, `oidc`, `waf`"), - errors.New("Policy nginx-ingress/valid-policy doesn't exist"), - errors.New("Failed to get policy nginx-ingress/some-policy: GetByKey error"), + errors.New("policy default/invalid-policy is invalid: spec: Invalid value: \"\": must specify exactly one of: `accessControl`, `rateLimit`, `ingressMTLS`, `egressMTLS`, `basicAuth`, `jwt`, `oidc`, `waf`"), + errors.New("policy nginx-ingress/valid-policy doesn't exist"), + errors.New("failed to get policy nginx-ingress/some-policy: GetByKey error"), errors.New("referenced policy default/valid-policy-ingress-class has incorrect ingress class: test-class (controller ingress class: )"), } @@ -762,37 +761,37 @@ func TestGetPodOwnerTypeAndName(t *testing.T) { desc string expType string expName string - pod *v1.Pod + pod *api_v1.Pod }{ { desc: "deployment", expType: "deployment", expName: "deploy-name", - pod: &v1.Pod{ObjectMeta: createTestObjMeta("Deployment", "deploy-name", true)}, + pod: &api_v1.Pod{ObjectMeta: createTestObjMeta("Deployment", "deploy-name", true)}, }, { desc: "stateful set", expType: "statefulset", expName: "statefulset-name", - pod: &v1.Pod{ObjectMeta: createTestObjMeta("StatefulSet", "statefulset-name", true)}, + pod: &api_v1.Pod{ObjectMeta: createTestObjMeta("StatefulSet", "statefulset-name", true)}, }, { desc: "daemon set", expType: "daemonset", expName: "daemonset-name", - pod: &v1.Pod{ObjectMeta: createTestObjMeta("DaemonSet", "daemonset-name", true)}, + pod: &api_v1.Pod{ObjectMeta: createTestObjMeta("DaemonSet", "daemonset-name", true)}, }, { desc: "replica set with no pod hash", expType: "deployment", expName: "replicaset-name", - pod: &v1.Pod{ObjectMeta: createTestObjMeta("ReplicaSet", "replicaset-name", false)}, + pod: &api_v1.Pod{ObjectMeta: createTestObjMeta("ReplicaSet", "replicaset-name", false)}, }, { desc: "replica set with pod hash", expType: "deployment", expName: "replicaset-name", - pod: &v1.Pod{ + pod: &api_v1.Pod{ ObjectMeta: createTestObjMeta("ReplicaSet", "replicaset-name-67c6f7c5fd", true), }, }, @@ -800,7 +799,7 @@ func TestGetPodOwnerTypeAndName(t *testing.T) { desc: "nil controller should use default values", expType: "deployment", expName: "deploy-name", - pod: &v1.Pod{ + pod: &api_v1.Pod{ ObjectMeta: meta_v1.ObjectMeta{ OwnerReferences: []meta_v1.OwnerReference{ { @@ -1156,14 +1155,14 @@ func errorComparer(e1, e2 error) bool { func TestAddJWTSecrets(t *testing.T) { invalidErr := errors.New("invalid") - validJWKSecret := &v1.Secret{ + validJWKSecret := &api_v1.Secret{ ObjectMeta: meta_v1.ObjectMeta{ Name: "valid-jwk-secret", Namespace: "default", }, Type: secrets.SecretTypeJWK, } - invalidJWKSecret := &v1.Secret{ + invalidJWKSecret := &api_v1.Secret{ ObjectMeta: meta_v1.ObjectMeta{ Name: "invalid-jwk-secret", Namespace: "default", @@ -1280,14 +1279,14 @@ func TestAddJWTSecrets(t *testing.T) { func TestAddBasicSecrets(t *testing.T) { invalidErr := errors.New("invalid") - validBasicSecret := &v1.Secret{ + validBasicSecret := &api_v1.Secret{ ObjectMeta: meta_v1.ObjectMeta{ Name: "valid-basic-auth-secret", Namespace: "default", }, Type: secrets.SecretTypeJWK, } - invalidBasicSecret := &v1.Secret{ + invalidBasicSecret := &api_v1.Secret{ ObjectMeta: meta_v1.ObjectMeta{ Name: "invalid-basic-auth-secret", Namespace: "default", @@ -1404,14 +1403,14 @@ func TestAddBasicSecrets(t *testing.T) { func TestAddIngressMTLSSecret(t *testing.T) { invalidErr := errors.New("invalid") - validSecret := &v1.Secret{ + validSecret := &api_v1.Secret{ ObjectMeta: meta_v1.ObjectMeta{ Name: "valid-ingress-mtls-secret", Namespace: "default", }, Type: secrets.SecretTypeCA, } - invalidSecret := &v1.Secret{ + invalidSecret := &api_v1.Secret{ ObjectMeta: meta_v1.ObjectMeta{ Name: "invalid-ingress-mtls-secret", Namespace: "default", @@ -1526,28 +1525,28 @@ func TestAddIngressMTLSSecret(t *testing.T) { func TestAddEgressMTLSSecrets(t *testing.T) { invalidErr := errors.New("invalid") - validMTLSSecret := &v1.Secret{ + validMTLSSecret := &api_v1.Secret{ ObjectMeta: meta_v1.ObjectMeta{ Name: "valid-egress-mtls-secret", Namespace: "default", }, Type: api_v1.SecretTypeTLS, } - validTrustedSecret := &v1.Secret{ + validTrustedSecret := &api_v1.Secret{ ObjectMeta: meta_v1.ObjectMeta{ Name: "valid-egress-trusted-secret", Namespace: "default", }, Type: secrets.SecretTypeCA, } - invalidMTLSSecret := &v1.Secret{ + invalidMTLSSecret := &api_v1.Secret{ ObjectMeta: meta_v1.ObjectMeta{ Name: "invalid-egress-mtls-secret", Namespace: "default", }, Type: api_v1.SecretTypeTLS, } - invalidTrustedSecret := &v1.Secret{ + invalidTrustedSecret := &api_v1.Secret{ ObjectMeta: meta_v1.ObjectMeta{ Name: "invalid-egress-trusted-secret", Namespace: "default", @@ -1743,7 +1742,7 @@ func TestAddEgressMTLSSecrets(t *testing.T) { func TestAddOidcSecret(t *testing.T) { invalidErr := errors.New("invalid") - validSecret := &v1.Secret{ + validSecret := &api_v1.Secret{ ObjectMeta: meta_v1.ObjectMeta{ Name: "valid-oidc-secret", Namespace: "default", @@ -1753,7 +1752,7 @@ func TestAddOidcSecret(t *testing.T) { }, Type: secrets.SecretTypeOIDC, } - invalidSecret := &v1.Secret{ + invalidSecret := &api_v1.Secret{ ObjectMeta: meta_v1.ObjectMeta{ Name: "invalid-oidc-secret", Namespace: "default", diff --git a/internal/k8s/handlers.go b/internal/k8s/handlers.go index e520acf4b3..5cb40e271a 100644 --- a/internal/k8s/handlers.go +++ b/internal/k8s/handlers.go @@ -539,7 +539,7 @@ func areResourcesDifferent(oldresource, resource *unstructured.Unstructured) (bo return false, err } if !found { - return false, fmt.Errorf("Error, spec has unexpected format") + return false, fmt.Errorf("spec has unexpected format") } eq := reflect.DeepEqual(oldSpec, spec) if eq { diff --git a/internal/k8s/handlers_test.go b/internal/k8s/handlers_test.go index e2b880e706..eaed6eb2bc 100644 --- a/internal/k8s/handlers_test.go +++ b/internal/k8s/handlers_test.go @@ -204,7 +204,7 @@ func TestAreResourcesDifferent(t *testing.T) { Object: map[string]interface{}{}, }, expected: false, - expectErr: errors.New(`Error, spec has unexpected format`), + expectErr: errors.New(`spec has unexpected format`), msg: "new resource with missing spec", }, { diff --git a/internal/k8s/status.go b/internal/k8s/status.go index a8ab81c508..672c8aeeb8 100644 --- a/internal/k8s/status.go +++ b/internal/k8s/status.go @@ -10,7 +10,6 @@ import ( "github.com/golang/glog" conf_v1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1" - v1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1" conf_v1alpha1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1alpha1" k8s_nginx "github.com/nginxinc/kubernetes-ingress/pkg/client/clientset/versioned" api_v1 "k8s.io/api/core/v1" @@ -35,7 +34,7 @@ type statusUpdater struct { externalServicePorts string bigIPAddress string bigIPPorts string - externalEndpoints []v1.ExternalEndpoint + externalEndpoints []conf_v1.ExternalEndpoint status []api_v1.LoadBalancerIngress statusInitialized bool keyFunc func(obj interface{}) (string, error) @@ -522,7 +521,7 @@ func hasVsrStatusChanged(vsr *conf_v1.VirtualServerRoute, state string, reason s } // UpdateVirtualServerRouteStatusWithReferencedBy updates the status of a VirtualServerRoute, including the referencedBy field. -func (su *statusUpdater) UpdateVirtualServerRouteStatusWithReferencedBy(vsr *conf_v1.VirtualServerRoute, state string, reason string, message string, referencedBy []*v1.VirtualServer) error { +func (su *statusUpdater) UpdateVirtualServerRouteStatusWithReferencedBy(vsr *conf_v1.VirtualServerRoute, state string, reason string, message string, referencedBy []*conf_v1.VirtualServer) error { var referencedByString string if len(referencedBy) != 0 { vs := referencedBy[0] @@ -687,12 +686,12 @@ func (su *statusUpdater) generateExternalEndpointsFromStatus(status []api_v1.Loa return externalEndpoints } -func hasPolicyStatusChanged(pol *v1.Policy, state string, reason string, message string) bool { +func hasPolicyStatusChanged(pol *conf_v1.Policy, state string, reason string, message string) bool { return pol.Status.State != state || pol.Status.Reason != reason || pol.Status.Message != message } // UpdatePolicyStatus updates the status of a Policy. -func (su *statusUpdater) UpdatePolicyStatus(pol *v1.Policy, state string, reason string, message string) error { +func (su *statusUpdater) UpdatePolicyStatus(pol *conf_v1.Policy, state string, reason string, message string) error { // Get an up-to-date Policy from the Store var polLatest interface{} var exists bool @@ -717,7 +716,7 @@ func (su *statusUpdater) UpdatePolicyStatus(pol *v1.Policy, state string, reason return nil } - polCopy := polLatest.(*v1.Policy) + polCopy := polLatest.(*conf_v1.Policy) if !hasPolicyStatusChanged(polCopy, state, reason, message) { return nil @@ -736,7 +735,7 @@ func (su *statusUpdater) UpdatePolicyStatus(pol *v1.Policy, state string, reason return nil } -func (su *statusUpdater) retryUpdatePolicyStatus(polCopy *v1.Policy) error { +func (su *statusUpdater) retryUpdatePolicyStatus(polCopy *conf_v1.Policy) error { pol, err := su.confClient.K8sV1().Policies(polCopy.Namespace).Get(context.TODO(), polCopy.Name, metav1.GetOptions{}) if err != nil { return err diff --git a/internal/k8s/task_queue.go b/internal/k8s/task_queue.go index 5a77f4e718..021b415ed4 100644 --- a/internal/k8s/task_queue.go +++ b/internal/k8s/task_queue.go @@ -173,10 +173,10 @@ func newTask(key string, obj interface{}) (task, error) { } else if objectKind == appprotectdos.DosLogConfGVK.Kind { k = appProtectDosLogConf } else { - return task{}, fmt.Errorf("Unknown unstructured kind: %v", objectKind) + return task{}, fmt.Errorf("unknown unstructured kind: %v", objectKind) } default: - return task{}, fmt.Errorf("Unknown type: %v", t) + return task{}, fmt.Errorf("unknown type: %v", t) } return task{k, key}, nil diff --git a/internal/nginx/manager.go b/internal/nginx/manager.go index 93949f20fd..713d9053fd 100644 --- a/internal/nginx/manager.go +++ b/internal/nginx/manager.go @@ -235,7 +235,7 @@ func (lm *LocalManager) CreateDHParam(content string) (string, error) { err := createFileAndWrite(lm.dhparamFilename, []byte(content)) if err != nil { - return lm.dhparamFilename, fmt.Errorf("Failed to write dhparam file from %v: %w", lm.dhparamFilename, err) + return lm.dhparamFilename, fmt.Errorf("failed to write dhparam file from %v: %w", lm.dhparamFilename, err) } return lm.dhparamFilename, nil @@ -423,7 +423,7 @@ func (lm *LocalManager) CreateOpenTracingTracerConfig(content string) error { glog.V(3).Infof("Writing OpenTracing tracer config file to %v", jsonFileForOpenTracingTracer) err := createFileAndWrite(jsonFileForOpenTracingTracer, []byte(content)) if err != nil { - return fmt.Errorf("Failed to write config file: %w", err) + return fmt.Errorf("failed to write config file: %w", err) } return nil diff --git a/internal/nginx/utils.go b/internal/nginx/utils.go index 6cc70c742a..682db8ca15 100644 --- a/internal/nginx/utils.go +++ b/internal/nginx/utils.go @@ -22,12 +22,12 @@ func shellOut(cmd string) (err error) { err = command.Start() if err != nil { - return fmt.Errorf("Failed to execute %v, err: %w", cmd, err) + return fmt.Errorf("failed to execute %v, err: %w", cmd, err) } err = command.Wait() if err != nil { - return fmt.Errorf("Command %v stdout: %q\nstderr: %q\nfinished with error: %w", cmd, + return fmt.Errorf("command %v stdout: %q\nstderr: %q\nfinished with error: %w", cmd, stdout.String(), stderr.String(), err) } return nil @@ -36,14 +36,14 @@ func shellOut(cmd string) (err error) { func createFileAndWrite(name string, b []byte) error { w, err := os.Create(name) if err != nil { - return fmt.Errorf("Failed to open %v: %w", name, err) + return fmt.Errorf("failed to open %v: %w", name, err) } defer w.Close() _, err = w.Write(b) if err != nil { - return fmt.Errorf("Failed to write to %v: %w", name, err) + return fmt.Errorf("failed to write to %v: %w", name, err) } return nil From b14dc84fc723a11aee3b4b5d4d89525ffa5b7a97 Mon Sep 17 00:00:00 2001 From: pasmant <78279234+pasmant@users.noreply.github.com> Date: Thu, 29 Sep 2022 19:53:54 +0300 Subject: [PATCH 010/105] Apdoslogconf will warning case format is not splunk (#2991) --- .../appprotectdos.f5.com_apdoslogconfs.yaml | 2 - .../appprotectdos.f5.com_apdoslogconfs.yaml | 2 - docs/content/app-protect-dos/configuration.md | 12 +--- .../app-protect-dos/apdos-logconf.yaml | 3 - .../app-protect-dos/apdos-logconf.yaml | 3 - .../app_protect_dos_configuration.go | 19 +++++- .../app_protect_dos_configuration_test.go | 45 ++++++++++--- internal/k8s/controller.go | 13 ++++ pkg/apis/dos/validation/dos.go | 20 ++++-- pkg/apis/dos/validation/dos_test.go | 63 +++++++++++++------ tests/data/dos/dos-logconf.yaml | 3 - .../data/virtual-server-dos/dos-logconf.yaml | 3 - 12 files changed, 129 insertions(+), 59 deletions(-) diff --git a/deployments/common/crds/appprotectdos.f5.com_apdoslogconfs.yaml b/deployments/common/crds/appprotectdos.f5.com_apdoslogconfs.yaml index f6e0f7110e..e23e87184b 100644 --- a/deployments/common/crds/appprotectdos.f5.com_apdoslogconfs.yaml +++ b/deployments/common/crds/appprotectdos.f5.com_apdoslogconfs.yaml @@ -38,13 +38,11 @@ spec: - splunk - arcsight - user-defined - default: splunk type: string format_string: type: string max_message_size: pattern: ^([1-9]|[1-5][0-9]|6[0-4])k$ - default: 5k type: string type: object filter: diff --git a/deployments/helm-chart/crds/appprotectdos.f5.com_apdoslogconfs.yaml b/deployments/helm-chart/crds/appprotectdos.f5.com_apdoslogconfs.yaml index f6e0f7110e..e23e87184b 100644 --- a/deployments/helm-chart/crds/appprotectdos.f5.com_apdoslogconfs.yaml +++ b/deployments/helm-chart/crds/appprotectdos.f5.com_apdoslogconfs.yaml @@ -38,13 +38,11 @@ spec: - splunk - arcsight - user-defined - default: splunk type: string format_string: type: string max_message_size: pattern: ^([1-9]|[1-5][0-9]|6[0-4])k$ - default: 5k type: string type: object filter: diff --git a/docs/content/app-protect-dos/configuration.md b/docs/content/app-protect-dos/configuration.md index 39afa1a7ad..0e361ed9ec 100644 --- a/docs/content/app-protect-dos/configuration.md +++ b/docs/content/app-protect-dos/configuration.md @@ -112,12 +112,9 @@ For example, say you want to log state changing requests for your Ingress resour ```json { "filter": { - "request_type": "all" - }, - "content": { - "format": "default", - "max_request_size": "any", - "max_message_size": "64k" + "traffic-mitigation-stats": "all", + "bad-actors": "top 10", + "attack-signatures": "top 10" } } ``` @@ -130,9 +127,6 @@ kind: APDosLogConf metadata: name: doslogconf spec: - content: - format: splunk - max_message_size: 64k filter: traffic-mitigation-stats: all bad-actors: top 10 diff --git a/examples/custom-resources/app-protect-dos/apdos-logconf.yaml b/examples/custom-resources/app-protect-dos/apdos-logconf.yaml index 885a524647..688f3ea786 100644 --- a/examples/custom-resources/app-protect-dos/apdos-logconf.yaml +++ b/examples/custom-resources/app-protect-dos/apdos-logconf.yaml @@ -3,9 +3,6 @@ kind: APDosLogConf metadata: name: doslogconf spec: - content: - format: splunk - max_message_size: 64k filter: traffic-mitigation-stats: all bad-actors: top 10 diff --git a/examples/ingress-resources/app-protect-dos/apdos-logconf.yaml b/examples/ingress-resources/app-protect-dos/apdos-logconf.yaml index 885a524647..688f3ea786 100644 --- a/examples/ingress-resources/app-protect-dos/apdos-logconf.yaml +++ b/examples/ingress-resources/app-protect-dos/apdos-logconf.yaml @@ -3,9 +3,6 @@ kind: APDosLogConf metadata: name: doslogconf spec: - content: - format: splunk - max_message_size: 64k filter: traffic-mitigation-stats: all bad-actors: top 10 diff --git a/internal/k8s/appprotectdos/app_protect_dos_configuration.go b/internal/k8s/appprotectdos/app_protect_dos_configuration.go index f6aecfe1df..f01a671a97 100644 --- a/internal/k8s/appprotectdos/app_protect_dos_configuration.go +++ b/internal/k8s/appprotectdos/app_protect_dos_configuration.go @@ -114,11 +114,14 @@ func (ci *Configuration) AddOrUpdatePolicy(policyObj *unstructured.Unstructured) resNsName := appprotectcommon.GetNsName(policyObj) policy, err := createAppProtectDosPolicyEx(policyObj) ci.dosPolicies[resNsName] = policy + op := AddOrUpdate if err != nil { - changes = append(changes, Change{Op: Delete, Resource: policy}) + op = Delete problems = append(problems, Problem{Object: policyObj, Reason: "Rejected", Message: err.Error()}) } + changes = append(changes, Change{Op: op, Resource: policy}) + protectedResources := ci.GetDosProtectedThatReferencedDosPolicy(resNsName) for _, p := range protectedResources { proChanges, proProblems := ci.AddOrUpdateDosProtectedResource(p) @@ -134,11 +137,14 @@ func (ci *Configuration) AddOrUpdateLogConf(logConfObj *unstructured.Unstructure resNsName := appprotectcommon.GetNsName(logConfObj) logConf, err := createAppProtectDosLogConfEx(logConfObj) ci.dosLogConfs[resNsName] = logConf + op := AddOrUpdate if err != nil { - changes = append(changes, Change{Op: Delete, Resource: logConf}) + op = Delete problems = append(problems, Problem{Object: logConfObj, Reason: "Rejected", Message: err.Error()}) } + changes = append(changes, Change{Op: op, Resource: logConf}) + protectedResources := ci.GetDosProtectedThatReferencedDosLogConf(resNsName) for _, p := range protectedResources { proChanges, proProblems := ci.AddOrUpdateDosProtectedResource(p) @@ -357,7 +363,7 @@ func createAppProtectDosPolicyEx(policyObj *unstructured.Unstructured) (*DosPoli } func createAppProtectDosLogConfEx(dosLogConfObj *unstructured.Unstructured) (*DosLogConfEx, error) { - err := validation.ValidateAppProtectDosLogConf(dosLogConfObj) + warning, err := validation.ValidateAppProtectDosLogConf(dosLogConfObj) if err != nil { return &DosLogConfEx{ Obj: dosLogConfObj, @@ -365,6 +371,13 @@ func createAppProtectDosLogConfEx(dosLogConfObj *unstructured.Unstructured) (*Do ErrorMsg: fmt.Sprintf("failed to store ApDosLogconf: %v", err), }, err } + if warning != "" { + return &DosLogConfEx{ + Obj: dosLogConfObj, + IsValid: true, + ErrorMsg: warning, + }, nil + } return &DosLogConfEx{ Obj: dosLogConfObj, IsValid: true, diff --git a/internal/k8s/appprotectdos/app_protect_dos_configuration_test.go b/internal/k8s/appprotectdos/app_protect_dos_configuration_test.go index 23eb9df4ea..8b66934763 100644 --- a/internal/k8s/appprotectdos/app_protect_dos_configuration_test.go +++ b/internal/k8s/appprotectdos/app_protect_dos_configuration_test.go @@ -70,8 +70,7 @@ func TestCreateAppProtectDosLogConfEx(t *testing.T) { logConf: &unstructured.Unstructured{ Object: map[string]interface{}{ "spec": map[string]interface{}{ - "content": map[string]interface{}{}, - "filter": map[string]interface{}{}, + "filter": map[string]interface{}{}, }, }, }, @@ -86,10 +85,26 @@ func TestCreateAppProtectDosLogConfEx(t *testing.T) { logConf: &unstructured.Unstructured{ Object: map[string]interface{}{ "spec": map[string]interface{}{ - "content": map[string]interface{}{}, + "content": map[string]interface{}{ + "format": "splunk", + }, + "filter": map[string]interface{}{}, }, }, }, + expectedLogConfEx: &DosLogConfEx{ + IsValid: true, + ErrorMsg: "the Content field is not supported, defaulting to splunk format.", + }, + wantErr: false, + msg: "Valid DosLogConf with warning", + }, + { + logConf: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "spec": map[string]interface{}{}, + }, + }, expectedLogConfEx: &DosLogConfEx{ IsValid: false, ErrorMsg: "failed to store ApDosLogconf: error validating App Protect Dos Log Configuration : required field map[] not found", @@ -211,6 +226,11 @@ func TestAddOrUpdateDosPolicy(t *testing.T) { }, }, } + + basicPolicyResource := &DosPolicyEx{ + Obj: basicTestPolicy, + IsValid: true, + } invalidTestPolicy := &unstructured.Unstructured{ Object: map[string]interface{}{ "metadata": map[string]interface{}{ @@ -245,6 +265,10 @@ func TestAddOrUpdateDosPolicy(t *testing.T) { { policy: basicTestPolicy, expectedChanges: []Change{ + { + Resource: basicPolicyResource, + Op: AddOrUpdate, + }, { Resource: &DosProtectedResourceEx{ Obj: basicResource, @@ -298,20 +322,21 @@ func TestAddOrUpdateDosLogConf(t *testing.T) { "name": "testlogconf", }, "spec": map[string]interface{}{ - "content": map[string]interface{}{}, - "filter": map[string]interface{}{}, + "filter": map[string]interface{}{}, }, }, } + validLogConfResource := &DosLogConfEx{ + Obj: validLogConf, + IsValid: true, + } invalidLogConf := &unstructured.Unstructured{ Object: map[string]interface{}{ "metadata": map[string]interface{}{ "namespace": "testing", "name": "invalid-logconf", }, - "spec": map[string]interface{}{ - "content": map[string]interface{}{}, - }, + "spec": map[string]interface{}{}, }, } basicResource := &v1beta1.DosProtectedResource{ @@ -345,6 +370,10 @@ func TestAddOrUpdateDosLogConf(t *testing.T) { { logconf: validLogConf, expectedChanges: []Change{ + { + Resource: validLogConfResource, + Op: AddOrUpdate, + }, { Resource: &DosProtectedResourceEx{ Obj: basicResource, diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index 3f8e2f5358..cf5a9d4e5b 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -1506,6 +1506,19 @@ func (lbc *LoadBalancerController) processAppProtectDosChanges(changes []appprot lbc.updateResourcesStatusAndEvents(resources, warnings, err) msg := fmt.Sprintf("Configuration for %s/%s was added or updated", impl.Obj.Namespace, impl.Obj.Name) lbc.recorder.Event(impl.Obj, api_v1.EventTypeNormal, "AddedOrUpdated", msg) + case *appprotectdos.DosPolicyEx: + msg := "Configuration was added or updated" + lbc.recorder.Event(impl.Obj, api_v1.EventTypeNormal, "AddedOrUpdated", msg) + case *appprotectdos.DosLogConfEx: + eventType := api_v1.EventTypeNormal + eventTitle := "AddedOrUpdated" + msg := "Configuration was added or updated" + if impl.ErrorMsg != "" { + msg += fmt.Sprintf(" ; with warning(s): %s", impl.ErrorMsg) + eventTitle = "AddedOrUpdatedWithWarning" + eventType = api_v1.EventTypeWarning + } + lbc.recorder.Event(impl.Obj, eventType, eventTitle, msg) } } else if c.Op == appprotectdos.Delete { switch impl := c.Resource.(type) { diff --git a/pkg/apis/dos/validation/dos.go b/pkg/apis/dos/validation/dos.go index 55cb97555e..2cb51a244e 100644 --- a/pkg/apis/dos/validation/dos.go +++ b/pkg/apis/dos/validation/dos.go @@ -19,7 +19,6 @@ var appProtectDosPolicyRequiredFields = [][]string{ } var appProtectDosLogConfRequiredFields = [][]string{ - {"spec", "content"}, {"spec", "filter"}, } @@ -90,15 +89,28 @@ func validateResourceReference(ref string) error { return nil } +// checkAppProtectDosLogConfContentField check conetent field doesnt appear in dos log +func checkAppProtectDosLogConfContentField(obj *unstructured.Unstructured) string { + _, found, err := unstructured.NestedMap(obj.Object, "spec", "content") + if err == nil && found { + unstructured.RemoveNestedField(obj.Object, "spec", "content") + msg := "the Content field is not supported, defaulting to splunk format." + return msg + } + + return "" +} + // ValidateAppProtectDosLogConf validates LogConfiguration resource -func ValidateAppProtectDosLogConf(logConf *unstructured.Unstructured) error { +func ValidateAppProtectDosLogConf(logConf *unstructured.Unstructured) (string, error) { lcName := logConf.GetName() err := validation2.ValidateRequiredFields(logConf, appProtectDosLogConfRequiredFields) if err != nil { - return fmt.Errorf("error validating App Protect Dos Log Configuration %v: %w", lcName, err) + return "", fmt.Errorf("error validating App Protect Dos Log Configuration %v: %w", lcName, err) } + warning := checkAppProtectDosLogConfContentField(logConf) - return nil + return warning, nil } var ( diff --git a/pkg/apis/dos/validation/dos_test.go b/pkg/apis/dos/validation/dos_test.go index 11081f7633..fdb92c2661 100644 --- a/pkg/apis/dos/validation/dos_test.go +++ b/pkg/apis/dos/validation/dos_test.go @@ -240,66 +240,91 @@ func TestValidateAppProtectDosAccessLogDest(t *testing.T) { func TestValidateAppProtectDosLogConf(t *testing.T) { t.Parallel() tests := []struct { - logConf *unstructured.Unstructured - expectErr bool - msg string + logConf *unstructured.Unstructured + expectErr bool + expectWarn bool + msg string }{ { logConf: &unstructured.Unstructured{ Object: map[string]interface{}{ "spec": map[string]interface{}{ - "content": map[string]interface{}{}, - "filter": map[string]interface{}{}, + "filter": map[string]interface{}{}, }, }, }, - expectErr: false, - msg: "valid log conf", + expectErr: false, + expectWarn: false, + msg: "valid log conf", }, { logConf: &unstructured.Unstructured{ Object: map[string]interface{}{ - "spec": map[string]interface{}{ + "spec": map[string]interface{}{}, + }, + }, + expectErr: true, + expectWarn: false, + msg: "invalid log conf with no filter field", + }, + { + logConf: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "something": map[string]interface{}{ "filter": map[string]interface{}{}, }, }, }, - expectErr: true, - msg: "invalid log conf with no content field", + expectErr: true, + expectWarn: false, + msg: "invalid log conf with no spec field", }, { logConf: &unstructured.Unstructured{ Object: map[string]interface{}{ "spec": map[string]interface{}{ - "content": map[string]interface{}{}, + "content": map[string]interface{}{ + "format": "user-defined", + }, + "filter": map[string]interface{}{}, }, }, }, - expectErr: true, - msg: "invalid log conf with no filter field", + expectErr: false, + expectWarn: true, + msg: "Support only splunk format", }, { logConf: &unstructured.Unstructured{ Object: map[string]interface{}{ - "something": map[string]interface{}{ - "content": map[string]interface{}{}, - "filter": map[string]interface{}{}, + "spec": map[string]interface{}{ + "filter": map[string]interface{}{}, + "content": map[string]interface{}{ + "format": "user-defined", + }, }, }, }, - expectErr: true, - msg: "invalid log conf with no spec field", + expectErr: false, + expectWarn: true, + msg: "valid log conf with warning filter field", }, } for _, test := range tests { - err := ValidateAppProtectDosLogConf(test.logConf) + warn, err := ValidateAppProtectDosLogConf(test.logConf) if test.expectErr && err == nil { t.Errorf("validateAppProtectDosLogConf() returned no error for the case of %s", test.msg) } if !test.expectErr && err != nil { t.Errorf("validateAppProtectDosLogConf() returned unexpected error %v for the case of %s", err, test.msg) } + if test.expectWarn && warn == "" { + t.Errorf("validateAppProtectDosLogConf() returned no warning for the case of %s", test.msg) + } + if !test.expectWarn && warn != "" { + t.Errorf("validateAppProtectDosLogConf() returned unexpected warning: %s, for the case of %s", warn, test.msg) + } } } diff --git a/tests/data/dos/dos-logconf.yaml b/tests/data/dos/dos-logconf.yaml index 885a524647..688f3ea786 100644 --- a/tests/data/dos/dos-logconf.yaml +++ b/tests/data/dos/dos-logconf.yaml @@ -3,9 +3,6 @@ kind: APDosLogConf metadata: name: doslogconf spec: - content: - format: splunk - max_message_size: 64k filter: traffic-mitigation-stats: all bad-actors: top 10 diff --git a/tests/data/virtual-server-dos/dos-logconf.yaml b/tests/data/virtual-server-dos/dos-logconf.yaml index 885a524647..688f3ea786 100644 --- a/tests/data/virtual-server-dos/dos-logconf.yaml +++ b/tests/data/virtual-server-dos/dos-logconf.yaml @@ -3,9 +3,6 @@ kind: APDosLogConf metadata: name: doslogconf spec: - content: - format: splunk - max_message_size: 64k filter: traffic-mitigation-stats: all bad-actors: top 10 From 59ed7364a03f9d2e5910a566be85bed63f6530d7 Mon Sep 17 00:00:00 2001 From: Venktesh Shivam Patel Date: Fri, 30 Sep 2022 14:53:36 +0100 Subject: [PATCH 011/105] add canary header and update syslog yaml for AP tests (#3116) --- .../ap-waf/{syslog-1.yaml => syslog2.yaml} | 12 ++++++------ tests/suite/test_app_protect_integration.py | 5 ++--- tests/suite/test_app_protect_waf_policies.py | 19 +++++++++---------- .../test_virtual_server_focused_canary.py | 14 +++++++++----- 4 files changed, 26 insertions(+), 24 deletions(-) rename tests/data/ap-waf/{syslog-1.yaml => syslog2.yaml} (84%) diff --git a/tests/data/ap-waf/syslog-1.yaml b/tests/data/ap-waf/syslog2.yaml similarity index 84% rename from tests/data/ap-waf/syslog-1.yaml rename to tests/data/ap-waf/syslog2.yaml index 94ca7ff606..8939d36bba 100644 --- a/tests/data/ap-waf/syslog-1.yaml +++ b/tests/data/ap-waf/syslog2.yaml @@ -1,19 +1,19 @@ apiVersion: apps/v1 kind: Deployment metadata: - name: syslog-1 + name: syslog2 spec: replicas: 1 selector: matchLabels: - app: syslog-1 + app: syslog2 template: metadata: labels: - app: syslog-1 + app: syslog2 spec: containers: - - name: syslog + - name: syslog2 image: balabit/syslog-ng:3.38.1 ports: - containerPort: 514 @@ -30,11 +30,11 @@ spec: apiVersion: v1 kind: Service metadata: - name: syslog-svc-1 + name: syslog2-svc spec: ports: - port: 514 targetPort: 514 protocol: TCP selector: - app: syslog-1 + app: syslog2 diff --git a/tests/suite/test_app_protect_integration.py b/tests/suite/test_app_protect_integration.py index c76a415fde..2a96f86538 100644 --- a/tests/suite/test_app_protect_integration.py +++ b/tests/suite/test_app_protect_integration.py @@ -340,9 +340,6 @@ def test_ap_multi_sec_logs( syslog_dst = f"syslog-svc.{test_namespace}" syslog2_dst = f"syslog2-svc.{test_namespace}" - syslog_pod = get_pod_name_that_contains(kube_apis.v1, test_namespace, "syslog-") - syslog2_pod = get_pod_name_that_contains(kube_apis.v1, test_namespace, "syslog2") - with open(src_ing_yaml) as f: doc = yaml.safe_load(f) @@ -369,6 +366,8 @@ def test_ap_multi_sec_logs( print("----------------------- Send request ----------------------") response = requests.get(appprotect_setup.req_url + "/