diff --git a/.github/workflows/prbuild.yaml b/.github/workflows/prbuild.yaml index c156b2a931a..bfbc9401334 100644 --- a/.github/workflows/prbuild.yaml +++ b/.github/workflows/prbuild.yaml @@ -27,10 +27,10 @@ jobs: with: persist-credentials: false - name: golangci-lint - uses: golangci/golangci-lint-action@55c2c1448f86e01eaae002a5a3a9624417608d84 # v6.5.2 + uses: golangci/golangci-lint-action@1481404843c368bc19ca9406f87d6e0fc97bdcfd # v7.0.0 with: - version: v1.64.5 - args: --build-tags=e2e,conformance,gcp,oidc,none --out-format=colored-line-number + version: v2.0.2 + args: --build-tags=e2e,conformance,gcp,oidc,none codespell: name: Codespell runs-on: ubuntu-latest diff --git a/.golangci.yml b/.golangci.yml index 28e4143e019..3079a8055da 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,118 +1,137 @@ -run: - timeout: 10m - +version: "2" linters: enable: - - bodyclose - - gofumpt - - goimports - - gci - - importas - - revive - - gosec - - misspell - - unconvert - - unparam - - goheader - - gocritic - - forbidigo - - testifylint - - ginkgolinter - -linters-settings: - misspell: - ignore-words: - - clas - - cancelled - locale: US - unparam: - check-exported: false - goheader: - template-path: ./hack/license-template.txt - forbidigo: - forbid: - - http.DefaultTransport - revive: + - bodyclose + - forbidigo + - ginkgolinter + - gocritic + - goheader + - gosec + - importas + - misspell + - revive + - testifylint + - unconvert + - unparam + settings: + forbidigo: + forbid: + - pattern: http.DefaultTransport + ginkgolinter: + forbid-focus-container: true + goheader: + template-path: ./hack/license-template.txt + importas: + alias: + - pkg: github.com/projectcontour/contour/apis/projectcontour/(v\w+) + alias: contour_${1} + - pkg: sigs.k8s.io/gateway-api/apis/(v\w+) + alias: gatewayapi_${1} + - pkg: k8s.io.*/apis?/(\w+)/(v\w+) + alias: ${1}_${2} + - pkg: github.com/envoyproxy/go-control-plane/envoy/config/(\w+)/(v\w+) + alias: envoy_config_${1}_${2} + - pkg: github.com/envoyproxy/go-control-plane/envoy/service/(\w+)/(v\w+) + alias: envoy_service_${1}_${2} + - pkg: github.com/envoyproxy/go-control-plane/envoy/extensions/filters/(\w+)/(\w+)/(v\w+) + alias: envoy_filter_${1}_${2}_${3} + - pkg: github.com/envoyproxy/go-control-plane/envoy/extensions/transport_sockets/(\w+)/(v\w+) + alias: envoy_transport_socket_${1}_${2} + - pkg: github.com/envoyproxy/go-control-plane/envoy/extensions/compression/(\w+)/(\w+)/(v\w+) + alias: envoy_compression_${1}_${2}_${3} + - pkg: github.com/envoyproxy/go-control-plane/envoy/extensions/access_loggers/(\w+)/(v\w+) + alias: envoy_access_logger_${1}_${2} + - pkg: github.com/envoyproxy/go-control-plane/envoy/extensions/formatter/(\w+)/(v\w+) + alias: envoy_formatter_${1}_${2} + - pkg: github.com/envoyproxy/go-control-plane/envoy/extensions/upstreams/(\w+)/(v\w+) + alias: envoy_upstream_${1}_${2} + - pkg: github.com/envoyproxy/go-control-plane/envoy/type/(v\w+) + alias: envoy_type_${1} + - pkg: github.com/envoyproxy/go-control-plane/envoy/type/matcher/(v\w+) + alias: envoy_matcher_${1} + no-unaliased: true + misspell: + locale: US + ignore-rules: + - clas + - cancelled + revive: + rules: + - name: blank-imports + - name: context-as-argument + - name: context-keys-type + - name: dot-imports + - name: empty-block + - name: error-naming + - name: error-return + - name: error-strings + - name: errorf + - name: exported + - name: increment-decrement + - name: indent-error-flow + - name: package-comments + - name: range + - name: receiver-naming + - name: redefines-builtin-id + - name: superfluous-else + - name: time-naming + - name: unexported-return + - name: unreachable-code + - name: unused-parameter + - name: use-any + - name: var-declaration + - name: var-naming + testifylint: + enable-all: true + unparam: + check-exported: false + exclusions: + generated: lax + presets: + - comments + - common-false-positives + - legacy + - std-error-handling rules: - - name: blank-imports - - name: context-as-argument - - name: context-keys-type - - name: dot-imports - - name: empty-block - - name: error-naming - - name: error-return - - name: error-strings - - name: errorf - - name: exported - - name: increment-decrement - - name: indent-error-flow - - name: package-comments - - name: range - - name: receiver-naming - - name: redefines-builtin-id - - name: superfluous-else - - name: time-naming - - name: unexported-return - - name: unreachable-code - - name: unused-parameter - - name: use-any - - name: var-declaration - - name: var-naming - testifylint: - enable-all: true - ginkgolinter: - forbid-focus-container: true - gofumpt: - extra-rules: true - gci: - custom-order: true - sections: - - standard - - default - - prefix(github.com/projectcontour/contour) - importas: - no-unaliased: true - alias: - - pkg: github.com/projectcontour/contour/apis/projectcontour/(v\w+) - alias: contour_${1} - - pkg: sigs.k8s.io/gateway-api/apis/(v\w+) - alias: gatewayapi_${1} - - pkg: k8s.io.*/apis?/(\w+)/(v\w+) - alias: ${1}_${2} - - pkg: github.com/envoyproxy/go-control-plane/envoy/config/(\w+)/(v\w+) - alias: envoy_config_${1}_${2} - - pkg: github.com/envoyproxy/go-control-plane/envoy/service/(\w+)/(v\w+) - alias: envoy_service_${1}_${2} - - pkg: github.com/envoyproxy/go-control-plane/envoy/extensions/filters/(\w+)/(\w+)/(v\w+) - alias: envoy_filter_${1}_${2}_${3} - - pkg: github.com/envoyproxy/go-control-plane/envoy/extensions/transport_sockets/(\w+)/(v\w+) - alias: envoy_transport_socket_${1}_${2} - - pkg: github.com/envoyproxy/go-control-plane/envoy/extensions/compression/(\w+)/(\w+)/(v\w+) - alias: envoy_compression_${1}_${2}_${3} - - pkg: github.com/envoyproxy/go-control-plane/envoy/extensions/access_loggers/(\w+)/(v\w+) - alias: envoy_access_logger_${1}_${2} - - pkg: github.com/envoyproxy/go-control-plane/envoy/extensions/formatter/(\w+)/(v\w+) - alias: envoy_formatter_${1}_${2} - - pkg: github.com/envoyproxy/go-control-plane/envoy/extensions/upstreams/(\w+)/(v\w+) - alias: envoy_upstream_${1}_${2} - - pkg: github.com/envoyproxy/go-control-plane/envoy/type/(v\w+) - alias: envoy_type_${1} - - pkg: github.com/envoyproxy/go-control-plane/envoy/type/matcher/(v\w+) - alias: envoy_matcher_${1} - + - linters: + - unparam + text: always receives + - linters: + - bodyclose + path: test/e2e + - linters: + - revive + path: test/e2e + text: should not use dot imports + - linters: + - testifylint + path: test/e2e + text: require must only be used in the goroutine running the test function + paths: + - third_party$ + - builtin$ + - examples$ issues: max-issues-per-linter: 0 max-same-issues: 0 - exclude-rules: - - linters: ["unparam"] - text: "always receives" - - path: zz_generated - linters: ["goimports"] - - path: test/e2e - linters: ["bodyclose"] - - path: test/e2e - linters: ["revive"] - text: "should not use dot imports" - - path: test/e2e - linters: ["testifylint"] - text: "require must only be used in the goroutine running the test function" +formatters: + enable: + - gci + - gofumpt + - goimports + settings: + gci: + sections: + - standard + - default + - prefix(github.com/projectcontour/contour) + custom-order: true + gofumpt: + extra-rules: true + exclusions: + generated: lax + paths: + - third_party$ + - builtin$ + - examples$ + - zz_generated diff --git a/cmd/contour/cli.go b/cmd/contour/cli.go index 66281679882..68ce39e8eaf 100644 --- a/cmd/contour/cli.go +++ b/cmd/contour/cli.go @@ -73,7 +73,7 @@ func (c *Client) dial() *grpc.ClientConn { switch { case c.CAFile != "" || c.ClientCert != "" || c.ClientKey != "": // If one of the three TLS commands is not empty, they all must be not empty - if !(c.CAFile != "" && c.ClientCert != "" && c.ClientKey != "") { + if c.CAFile == "" || c.ClientCert == "" || c.ClientKey == "" { kingpin.Fatalf("you must supply all three TLS parameters - --cafile, --cert-file, --key-file, or none of them") } // Load the client certificates from disk diff --git a/cmd/contour/contour_test.go b/cmd/contour/contour_test.go index ae4bf8bacbb..63286ef3bf0 100644 --- a/cmd/contour/contour_test.go +++ b/cmd/contour/contour_test.go @@ -25,7 +25,7 @@ import ( func assertOptionFlagsAreSorted(t *testing.T, cmd *kingpin.CmdClause) { var flags []string - for _, v := range cmd.Model().FlagGroupModel.Flags { + for _, v := range cmd.Model().Flags { flags = append(flags, v.Name) } assert.Truef(t, sort.StringsAreSorted(flags), "the flags for subcommand %q aren't sorted: %v", cmd.Model().Name, flags) diff --git a/cmd/contour/serve.go b/cmd/contour/serve.go index e6efeb73670..16a00bdd283 100644 --- a/cmd/contour/serve.go +++ b/cmd/contour/serve.go @@ -924,7 +924,7 @@ func (s *Server) setupMetrics(metricsConfig contour_v1alpha1.MetricsConfig, heal ServeMux: http.ServeMux{}, } - metricsvc.ServeMux.Handle("/metrics", metrics.Handler(registry)) + metricsvc.Handle("/metrics", metrics.Handler(registry)) if metricsConfig.TLS != nil { metricsvc.Cert = metricsConfig.TLS.CertFile @@ -934,8 +934,8 @@ func (s *Server) setupMetrics(metricsConfig contour_v1alpha1.MetricsConfig, heal if healthConfig.Address == metricsConfig.Address && healthConfig.Port == metricsConfig.Port { h := health.Handler(s.coreClient) - metricsvc.ServeMux.Handle("/health", h) - metricsvc.ServeMux.Handle("/healthz", h) + metricsvc.Handle("/health", h) + metricsvc.Handle("/healthz", h) } return s.mgr.Add(metricsvc) @@ -952,8 +952,8 @@ func (s *Server) setupHealth(healthConfig contour_v1alpha1.HealthConfig, } h := health.Handler(s.coreClient) - healthsvc.ServeMux.Handle("/health", h) - healthsvc.ServeMux.Handle("/healthz", h) + healthsvc.Handle("/health", h) + healthsvc.Handle("/healthz", h) return s.mgr.Add(healthsvc) } diff --git a/cmd/contour/serve_test.go b/cmd/contour/serve_test.go index f7846e5dffb..1cf5439a337 100644 --- a/cmd/contour/serve_test.go +++ b/cmd/contour/serve_test.go @@ -112,15 +112,15 @@ func TestGetDAGBuilder(t *testing.T) { commonAssertions(t, got) httpProxyProcessor := mustGetHTTPProxyProcessor(t, got) - assert.EqualValues(t, policy.RequestHeadersPolicy.Set, httpProxyProcessor.RequestHeadersPolicy.Set) + assert.Equal(t, policy.RequestHeadersPolicy.Set, httpProxyProcessor.RequestHeadersPolicy.Set) assert.ElementsMatch(t, policy.RequestHeadersPolicy.Remove, httpProxyProcessor.RequestHeadersPolicy.Remove) - assert.EqualValues(t, policy.ResponseHeadersPolicy.Set, httpProxyProcessor.ResponseHeadersPolicy.Set) + assert.Equal(t, policy.ResponseHeadersPolicy.Set, httpProxyProcessor.ResponseHeadersPolicy.Set) assert.ElementsMatch(t, policy.ResponseHeadersPolicy.Remove, httpProxyProcessor.ResponseHeadersPolicy.Remove) ingressProcessor := mustGetIngressProcessor(t, got) - assert.EqualValues(t, map[string]string(nil), ingressProcessor.RequestHeadersPolicy.Set) + assert.Equal(t, map[string]string(nil), ingressProcessor.RequestHeadersPolicy.Set) assert.ElementsMatch(t, map[string]string(nil), ingressProcessor.RequestHeadersPolicy.Remove) - assert.EqualValues(t, map[string]string(nil), ingressProcessor.ResponseHeadersPolicy.Set) + assert.Equal(t, map[string]string(nil), ingressProcessor.ResponseHeadersPolicy.Set) assert.ElementsMatch(t, map[string]string(nil), ingressProcessor.ResponseHeadersPolicy.Remove) }) @@ -140,13 +140,13 @@ func TestGetDAGBuilder(t *testing.T) { }) iProcessor := mustGetIngressProcessor(t, got) - assert.EqualValues(t, iProcessor.GlobalCircuitBreakerDefaults, &g) + assert.Equal(t, iProcessor.GlobalCircuitBreakerDefaults, &g) hProcessor := mustGetHTTPProxyProcessor(t, got) - assert.EqualValues(t, hProcessor.GlobalCircuitBreakerDefaults, &g) + assert.Equal(t, hProcessor.GlobalCircuitBreakerDefaults, &g) gProcessor := mustGetGatewayAPIProcessor(t, got) - assert.EqualValues(t, gProcessor.GlobalCircuitBreakerDefaults, &g) + assert.Equal(t, gProcessor.GlobalCircuitBreakerDefaults, &g) }) t.Run("request and response headers policy specified for ingress", func(t *testing.T) { @@ -179,9 +179,9 @@ func TestGetDAGBuilder(t *testing.T) { commonAssertions(t, got) ingressProcessor := mustGetIngressProcessor(t, got) - assert.EqualValues(t, policy.RequestHeadersPolicy.Set, ingressProcessor.RequestHeadersPolicy.Set) + assert.Equal(t, policy.RequestHeadersPolicy.Set, ingressProcessor.RequestHeadersPolicy.Set) assert.ElementsMatch(t, policy.RequestHeadersPolicy.Remove, ingressProcessor.RequestHeadersPolicy.Remove) - assert.EqualValues(t, policy.ResponseHeadersPolicy.Set, ingressProcessor.ResponseHeadersPolicy.Set) + assert.Equal(t, policy.ResponseHeadersPolicy.Set, ingressProcessor.ResponseHeadersPolicy.Set) assert.ElementsMatch(t, policy.ResponseHeadersPolicy.Remove, ingressProcessor.ResponseHeadersPolicy.Remove) }) @@ -197,7 +197,7 @@ func TestGetDAGBuilder(t *testing.T) { ingressClassNames: ingressClassNames, }) commonAssertions(t, got) - assert.EqualValues(t, ingressClassNames, got.Source.IngressClassNames) + assert.Equal(t, ingressClassNames, got.Source.IngressClassNames) }) t.Run("multiple comma-separated ingress classes specified", func(t *testing.T) { @@ -212,7 +212,7 @@ func TestGetDAGBuilder(t *testing.T) { ingressClassNames: ingressClassNames, }) commonAssertions(t, got) - assert.EqualValues(t, ingressClassNames, got.Source.IngressClassNames) + assert.Equal(t, ingressClassNames, got.Source.IngressClassNames) }) // TODO(3453): test additional properties of the DAG builder (processor fields, cache fields, Gateway tests (requires a client fake)) diff --git a/cmd/contour/servecontext.go b/cmd/contour/servecontext.go index 59db754c3c2..8c4ee27d73b 100644 --- a/cmd/contour/servecontext.go +++ b/cmd/contour/servecontext.go @@ -234,8 +234,8 @@ func verifyTLSFlags(contourXDSTLS *contour_v1alpha1.TLS) error { if contourXDSTLS.CAFile == "" && contourXDSTLS.CertFile == "" && contourXDSTLS.KeyFile == "" { return errors.New("no TLS parameters and --insecure not supplied. You must supply one or the other") } - // If one of the three TLS commands is not empty, they all must be not empty - if !(contourXDSTLS.CAFile != "" && contourXDSTLS.CertFile != "" && contourXDSTLS.KeyFile != "") { + // If one of the three TLS flags is not empty, they all must be not empty + if contourXDSTLS.CAFile == "" || contourXDSTLS.CertFile == "" || contourXDSTLS.KeyFile == "" { return errors.New("you must supply all three TLS parameters - --contour-cafile, --contour-cert-file, --contour-key-file, or none of them") } diff --git a/hack/golangci-lint b/hack/golangci-lint index 9ec9af0d1f3..f6a8588e89b 100755 --- a/hack/golangci-lint +++ b/hack/golangci-lint @@ -1,3 +1,3 @@ #! /usr/bin/env bash -go run github.com/golangci/golangci-lint/cmd/golangci-lint@v1.64.5 "$@" +go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.0.2 "$@" diff --git a/internal/annotation/annotations.go b/internal/annotation/annotations.go index ad38cd0f352..b43d8c87372 100644 --- a/internal/annotation/annotations.go +++ b/internal/annotation/annotations.go @@ -145,7 +145,7 @@ func ParseUpstreamProtocols(m map[string]string) map[string]string { // HTTPAllowed returns true unless the kubernetes.io/ingress.allow-http annotation is // present and set to false. func HTTPAllowed(i *networking_v1.Ingress) bool { - return !(i.Annotations["kubernetes.io/ingress.allow-http"] == "false") + return i.Annotations["kubernetes.io/ingress.allow-http"] != "false" } // TLSRequired returns true if the ingress.kubernetes.io/force-ssl-redirect annotation is diff --git a/internal/dag/accessors.go b/internal/dag/accessors.go index c3b78f71bc2..fc69c2e94b9 100644 --- a/internal/dag/accessors.go +++ b/internal/dag/accessors.go @@ -250,7 +250,7 @@ func (d *DAG) GetClusters() []*Cluster { } if vhost.TCPProxy != nil { - res = append(res, vhost.TCPProxy.Clusters...) + res = append(res, vhost.Clusters...) } } } diff --git a/internal/dag/cache.go b/internal/dag/cache.go index 28ea8350f7b..a6d7fd02b5b 100644 --- a/internal/dag/cache.go +++ b/internal/dag/cache.go @@ -446,7 +446,7 @@ func (kc *KubernetesCache) serviceTriggersRebuild(service *core_v1.Service) bool } for _, rule := range ingress.Spec.Rules { - http := rule.IngressRuleValue.HTTP + http := rule.HTTP if http == nil { continue } @@ -655,7 +655,7 @@ func (kc *KubernetesCache) routeTriggersRebuild(parentRefs []gatewayapi_v1.Paren // then delegation check is performed. func (kc *KubernetesCache) LookupTLSSecret(name types.NamespacedName, targetNamespace string) (*Secret, error) { if !kc.delegationPermitted(name, targetNamespace) { - return nil, NewDelegationNotPermittedError(fmt.Errorf("Certificate delegation not permitted")) + return nil, NewDelegationNotPermittedError(fmt.Errorf("certificate delegation not permitted")) } return kc.LookupTLSSecretInsecure(name) } @@ -665,7 +665,7 @@ func (kc *KubernetesCache) LookupTLSSecret(name types.NamespacedName, targetName // then delegation check is performed. func (kc *KubernetesCache) LookupCASecret(name types.NamespacedName, targetNamespace string) (*Secret, error) { if !kc.delegationPermitted(name, targetNamespace) { - return nil, NewDelegationNotPermittedError(fmt.Errorf("Certificate delegation not permitted")) + return nil, NewDelegationNotPermittedError(fmt.Errorf("certificate delegation not permitted")) } sec, ok := kc.secrets[name] @@ -713,7 +713,7 @@ func (kc *KubernetesCache) LookupCAConfigMap(name types.NamespacedName) (*Secret // then delegation check is performed. func (kc *KubernetesCache) LookupCRLSecret(name types.NamespacedName, targetNamespace string) (*Secret, error) { if !kc.delegationPermitted(name, targetNamespace) { - return nil, NewDelegationNotPermittedError(fmt.Errorf("Certificate delegation not permitted")) + return nil, NewDelegationNotPermittedError(fmt.Errorf("certificate delegation not permitted")) } sec, ok := kc.secrets[name] @@ -881,9 +881,9 @@ func (kc *KubernetesCache) LookupBackendTLSPolicyByTargetRef(targetRef gatewayap sectionNameMatches := tr.SectionName != nil && targetRef.SectionName != nil && *tr.SectionName == *targetRef.SectionName - if tr.LocalPolicyTargetReference.Group == targetRef.Group && - tr.LocalPolicyTargetReference.Kind == targetRef.Kind && - tr.LocalPolicyTargetReference.Name == targetRef.Name { + if tr.Group == targetRef.Group && + tr.Kind == targetRef.Kind && + tr.Name == targetRef.Name { if sectionNameMatches { return v, true } diff --git a/internal/dag/cache_test.go b/internal/dag/cache_test.go index f2baf2974c9..d95e7822564 100644 --- a/internal/dag/cache_test.go +++ b/internal/dag/cache_test.go @@ -2489,7 +2489,7 @@ func TestSecretTriggersRebuild(t *testing.T) { }, } if secretNamespace != "" { - i.ObjectMeta.Annotations = map[string]string{ + i.Annotations = map[string]string{ "projectcontour.io/tls-cert-namespace": secretNamespace, } } diff --git a/internal/dag/gatewayapi_processor.go b/internal/dag/gatewayapi_processor.go index 8deeb2271ba..acbba86b917 100644 --- a/internal/dag/gatewayapi_processor.go +++ b/internal/dag/gatewayapi_processor.go @@ -1872,11 +1872,11 @@ func (p *GatewayAPIProcessor) validateBackendObjectRef( routeKind string, routeNamespace string, ) (*Service, *meta_v1.Condition) { - if !(backendObjectRef.Group == nil || *backendObjectRef.Group == "") { + if backendObjectRef.Group != nil && *backendObjectRef.Group != "" { return nil, ptr.To(resolvedRefsFalse(gatewayapi_v1.RouteReasonInvalidKind, fmt.Sprintf("%s.Group must be \"\"", field))) } - if !(backendObjectRef.Kind != nil && *backendObjectRef.Kind == "Service") { + if backendObjectRef.Kind == nil || *backendObjectRef.Kind != "Service" { return nil, ptr.To(resolvedRefsFalse(gatewayapi_v1.RouteReasonInvalidKind, fmt.Sprintf("%s.Kind must be 'Service'", field))) } @@ -2428,9 +2428,10 @@ func setDefaultServiceProtocol(service *Service, protocolType gatewayapi_v1.Prot // For GRPCRoute, if the protocol is not set on the Service via annotation, // we should assume a protocol that matches what listener the route was attached to if isBlank(service.Protocol) { - if protocolType == gatewayapi_v1.HTTPProtocolType { + switch protocolType { + case gatewayapi_v1.HTTPProtocolType: service.Protocol = "h2c" - } else if protocolType == gatewayapi_v1.HTTPSProtocolType { + case gatewayapi_v1.HTTPSProtocolType: service.Protocol = "h2" } } diff --git a/internal/dag/ingress_processor.go b/internal/dag/ingress_processor.go index 63639e22712..4b2d26b9507 100644 --- a/internal/dag/ingress_processor.go +++ b/internal/dag/ingress_processor.go @@ -393,9 +393,9 @@ func stringOrDefault(s, def string) string { // In the case that the IngressRule contains no valid HTTPIngressPaths, a // nil slice is returned. func httppaths(rule networking_v1.IngressRule) []networking_v1.HTTPIngressPath { - if rule.IngressRuleValue.HTTP == nil { + if rule.HTTP == nil { // rule.IngressRuleValue.HTTP value is optional. return nil } - return rule.IngressRuleValue.HTTP.Paths + return rule.HTTP.Paths } diff --git a/internal/dag/status_test.go b/internal/dag/status_test.go index a4a9b867dad..1e4ddd33c54 100644 --- a/internal/dag/status_test.go +++ b/internal/dag/status_test.go @@ -4942,7 +4942,7 @@ func TestDAGStatus(t *testing.T) { Name: fallbackCertificate.Name, Namespace: fallbackCertificate.Namespace, }: fixture.NewValidCondition(). - WithError(contour_v1.ConditionTypeTLSError, "DelegationNotPermitted", `Spec.VirtualHost.TLS CA Secret "delegated/delegated" is invalid: Certificate delegation not permitted`), + WithError(contour_v1.ConditionTypeTLSError, "DelegationNotPermitted", `Spec.VirtualHost.TLS CA Secret "delegated/delegated" is invalid: certificate delegation not permitted`), }, }) @@ -5024,7 +5024,7 @@ func TestDAGStatus(t *testing.T) { Name: fallbackCertificate.Name, Namespace: fallbackCertificate.Namespace, }: fixture.NewValidCondition(). - WithError(contour_v1.ConditionTypeTLSError, "DelegationNotPermitted", `Spec.VirtualHost.TLS CRL Secret "delegated/delegated" is invalid: Certificate delegation not permitted`), + WithError(contour_v1.ConditionTypeTLSError, "DelegationNotPermitted", `Spec.VirtualHost.TLS CRL Secret "delegated/delegated" is invalid: certificate delegation not permitted`), }, }) diff --git a/internal/debug/dot.go b/internal/debug/dot.go index 95bd3a8db24..ed1697b1d75 100644 --- a/internal/debug/dot.go +++ b/internal/debug/dot.go @@ -117,7 +117,7 @@ func collectDag(b DagBuilder) (nodeCollection, edgeCollection) { edges[pair{vhost, vhost.TCPProxy}] = true nodes[vhost.TCPProxy] = true - for _, cluster := range vhost.TCPProxy.Clusters { + for _, cluster := range vhost.Clusters { edges[pair{vhost.TCPProxy, cluster}] = true nodes[cluster] = true @@ -161,7 +161,7 @@ func printNodes(nodes nodeCollection, w io.Writer) { case *dag.VirtualHost: fmt.Fprintf(w, `"%p" [shape=record, label="{http://%s}"]`+"\n", node, html.EscapeString(node.Name)) case *dag.SecureVirtualHost: - fmt.Fprintf(w, `"%p" [shape=record, label="{https://%s}"]`+"\n", node, html.EscapeString(node.VirtualHost.Name)) + fmt.Fprintf(w, `"%p" [shape=record, label="{https://%s}"]`+"\n", node, html.EscapeString(node.Name)) case *dag.Route: fmt.Fprintf(w, `"%p" [shape=record, label="{%s}"]`+"\n", node, html.EscapeString(node.PathMatchCondition.String())) case *dag.Cluster: diff --git a/internal/debug/dot_test.go b/internal/debug/dot_test.go index e7d961b5ae5..08db17c80f5 100644 --- a/internal/debug/dot_test.go +++ b/internal/debug/dot_test.go @@ -84,8 +84,8 @@ func TestWriteDotLineCount(t *testing.T) { labeledLineCount++ } } - require.EqualValues(t, 21, lineCount) - require.EqualValues(t, 9, labeledLineCount) + require.Equal(t, 21, lineCount) + require.Equal(t, 9, labeledLineCount) } func getTestListeners() []*dag.Listener { diff --git a/internal/envoy/cluster.go b/internal/envoy/cluster.go index 60afe0d8e29..1a6b96ba32d 100644 --- a/internal/envoy/cluster.go +++ b/internal/envoy/cluster.go @@ -48,7 +48,7 @@ func Clustername(cluster *dag.Cluster) string { } if uv := cluster.UpstreamValidation; uv != nil { if len(uv.CACertificates) > 0 { - buf += uv.CACertificates[0].Object.ObjectMeta.Name + buf += uv.CACertificates[0].Object.Name } if len(uv.SubjectNames) > 0 { buf += uv.SubjectNames[0] diff --git a/internal/envoy/v3/accesslog.go b/internal/envoy/v3/accesslog.go index 2684d371c0a..4c7660b1087 100644 --- a/internal/envoy/v3/accesslog.go +++ b/internal/envoy/v3/accesslog.go @@ -33,9 +33,10 @@ func FileAccessLogEnvoy(path, format string, extensions []string, level contour_ } var filter *envoy_config_accesslog_v3.AccessLogFilter - if level == contour_v1alpha1.LogLevelError { + switch level { + case contour_v1alpha1.LogLevelError: filter = filterOnlyErrors(300) // We want to log resp status >= 300 - } else if level == contour_v1alpha1.LogLevelCritical { + case contour_v1alpha1.LogLevelCritical: filter = filterOnlyErrors(500) // We want to log resp status >= 500 } // Nil by default to defer to Envoy's default log format. @@ -76,9 +77,10 @@ func FileAccessLogJSON(path string, fields contour_v1alpha1.AccessLogJSONFields, } var filter *envoy_config_accesslog_v3.AccessLogFilter - if level == contour_v1alpha1.LogLevelError { + switch level { + case contour_v1alpha1.LogLevelError: filter = filterOnlyErrors(300) // We want to log resp status >= 300 - } else if level == contour_v1alpha1.LogLevelCritical { + case contour_v1alpha1.LogLevelCritical: filter = filterOnlyErrors(500) // We want to log resp status >= 500 } diff --git a/internal/envoy/v3/accesslog_test.go b/internal/envoy/v3/accesslog_test.go index 2a0819b4705..07f8caad6c2 100644 --- a/internal/envoy/v3/accesslog_test.go +++ b/internal/envoy/v3/accesslog_test.go @@ -284,6 +284,51 @@ func TestAccessLogLevel(t *testing.T) { }} protobuf.ExpectEqual(t, want, got) + got = FileAccessLogJSON("/dev/stdout", nil, nil, contour_v1alpha1.LogLevelCritical) + want = []*envoy_config_accesslog_v3.AccessLog{{ + Name: wellknown.FileAccessLog, + ConfigType: &envoy_config_accesslog_v3.AccessLog_TypedConfig{ + TypedConfig: protobuf.MustMarshalAny(&envoy_access_logger_file_v3.FileAccessLog{ + Path: "/dev/stdout", + AccessLogFormat: &envoy_access_logger_file_v3.FileAccessLog_LogFormat{ + LogFormat: &envoy_config_core_v3.SubstitutionFormatString{ + OmitEmptyValues: true, + Format: &envoy_config_core_v3.SubstitutionFormatString_JsonFormat{ + JsonFormat: &structpb.Struct{ + Fields: map[string]*structpb.Value{}, + }, + }, + }, + }, + }), + }, + Filter: &envoy_config_accesslog_v3.AccessLogFilter{ + FilterSpecifier: &envoy_config_accesslog_v3.AccessLogFilter_OrFilter{ + OrFilter: &envoy_config_accesslog_v3.OrFilter{ + Filters: []*envoy_config_accesslog_v3.AccessLogFilter{ + { + FilterSpecifier: &envoy_config_accesslog_v3.AccessLogFilter_StatusCodeFilter{ + StatusCodeFilter: &envoy_config_accesslog_v3.StatusCodeFilter{ + Comparison: &envoy_config_accesslog_v3.ComparisonFilter{ + Op: envoy_config_accesslog_v3.ComparisonFilter_GE, + Value: &envoy_config_core_v3.RuntimeUInt32{ + DefaultValue: 500, + RuntimeKey: "contour.accesslog.filter.status_code", + }, + }, + }, + }, + }, + { + FilterSpecifier: &envoy_config_accesslog_v3.AccessLogFilter_ResponseFlagFilter{}, + }, + }, + }, + }, + }, + }} + protobuf.ExpectEqual(t, want, got) + // Log level disabled should return nil. assert.Nil(t, FileAccessLogJSON("/dev/stdout", nil, nil, contour_v1alpha1.LogLevelDisabled)) } diff --git a/internal/featuretests/v3/featuretests.go b/internal/featuretests/v3/featuretests.go index b9903ef5787..6afc3de3e7a 100644 --- a/internal/featuretests/v3/featuretests.go +++ b/internal/featuretests/v3/featuretests.go @@ -49,7 +49,6 @@ import ( "github.com/projectcontour/contour/internal/contour" "github.com/projectcontour/contour/internal/dag" envoy_v3 "github.com/projectcontour/contour/internal/envoy/v3" - v3 "github.com/projectcontour/contour/internal/envoy/v3" "github.com/projectcontour/contour/internal/fixture" "github.com/projectcontour/contour/internal/k8s" "github.com/projectcontour/contour/internal/metrics" @@ -93,7 +92,7 @@ func setup(t *testing.T, opts ...any) (ResourceEventHandlerWrapper, *Contour, fu } } - envoyGen := v3.NewEnvoyGen(envoy_v3.EnvoyGenOpt{ + envoyGen := envoy_v3.NewEnvoyGen(envoy_v3.EnvoyGenOpt{ XDSClusterName: envoy_v3.DefaultXDSClusterName, }) @@ -321,49 +320,6 @@ type StatusResult struct { Have *contour_v1.HTTPProxyStatus } -// Equals asserts that the status result is not an error and matches -// the wanted status exactly. -func (s *StatusResult) Equals(want contour_v1.HTTPProxyStatus) *Contour { - s.T.Helper() - - // We should never get an error fetching the status for an - // object, so make it fatal if we do. - if s.Err != nil { - s.T.Fatal(s.Err.Error()) - } - - assert.Equal(s.T, want, *s.Have) - return s.Contour -} - -// Like asserts that the status result is not an error and matches -// non-empty fields in the wanted status. -func (s *StatusResult) Like(want contour_v1.HTTPProxyStatus) *Contour { - s.T.Helper() - - // We should never get an error fetching the status for an - // object, so make it fatal if we do. - if s.Err != nil { - s.T.Fatal(s.Err.Error()) - } - - if len(want.CurrentStatus) > 0 { - assert.Equal(s.T, - contour_v1.HTTPProxyStatus{CurrentStatus: want.CurrentStatus}, - contour_v1.HTTPProxyStatus{CurrentStatus: s.Have.CurrentStatus}, - ) - } - - if len(want.Description) > 0 { - assert.Equal(s.T, - contour_v1.HTTPProxyStatus{Description: want.Description}, - contour_v1.HTTPProxyStatus{Description: s.Have.Description}, - ) - } - - return s.Contour -} - // HasError asserts that there is an error on the Valid Condition in the proxy // that matches the given values. func (s *StatusResult) HasError(condType, reason, message string) *Contour { @@ -374,7 +330,7 @@ func (s *StatusResult) HasError(condType, reason, message string) *Contour { subCond, ok := validCond.GetError(condType) if !ok { - s.T.Fatalf("Did not find error %s", condType) + s.Fatalf("Did not find error %s", condType) } assert.Equal(s.T, reason, subCond.Reason) assert.Equal(s.T, message, subCond.Message) @@ -384,7 +340,7 @@ func (s *StatusResult) HasError(condType, reason, message string) *Contour { // IsValid asserts that the proxy's CurrentStatus field is equal to "valid". func (s *StatusResult) IsValid() *Contour { - s.T.Helper() + s.Helper() assert.Equal(s.T, status.ProxyStatusValid, status.ProxyStatus(s.Have.CurrentStatus)) @@ -393,7 +349,7 @@ func (s *StatusResult) IsValid() *Contour { // IsInvalid asserts that the proxy's CurrentStatus field is equal to "invalid". func (s *StatusResult) IsInvalid() *Contour { - s.T.Helper() + s.Helper() assert.Equal(s.T, status.ProxyStatusInvalid, status.ProxyStatus(s.Have.CurrentStatus)) @@ -422,7 +378,7 @@ func (c *Contour) Status(obj any) *StatusResult { // NoStatus asserts that the given object did not get any status set. func (c *Contour) NoStatus(obj any) *Contour { if _, err := c.statusUpdateCache.GetStatus(obj); err == nil { - c.T.Errorf("found cached object status, wanted no status") + c.Errorf("found cached object status, wanted no status") } return c @@ -493,7 +449,7 @@ func (r *Response) Equals(want *envoy_service_discovery_v3.DiscoveryResponse) *C sort.Slice(want.Resources, func(i, j int) bool { return string(want.Resources[i].Value) < string(want.Resources[j].Value) }) sort.Slice(r.Resources, func(i, j int) bool { return string(r.Resources[i].Value) < string(r.Resources[j].Value) }) - protobuf.RequireEqual(r.T, want.Resources, r.DiscoveryResponse.Resources) + protobuf.RequireEqual(r.T, want.Resources, r.Resources) return r.Contour } diff --git a/internal/featuretests/v3/ingressclass_test.go b/internal/featuretests/v3/ingressclass_test.go index b26758d73eb..4515ba7ee86 100644 --- a/internal/featuretests/v3/ingressclass_test.go +++ b/internal/featuretests/v3/ingressclass_test.go @@ -505,7 +505,7 @@ func TestIngressClassAnnotationUpdate(t *testing.T) { } // With the configured ingress class, a virtual show should be added. - vhost.ObjectMeta.Annotations = map[string]string{ + vhost.Annotations = map[string]string{ "kubernetes.io/ingress.class": "contour", } @@ -528,7 +528,7 @@ func TestIngressClassAnnotationUpdate(t *testing.T) { // Updating to the non-configured ingress class should remove the // vhost. orig := vhost.DeepCopy() - vhost.ObjectMeta.Annotations = map[string]string{ + vhost.Annotations = map[string]string{ "kubernetes.io/ingress.class": "not-contour", } diff --git a/internal/fixture/httpproxy.go b/internal/fixture/httpproxy.go index 4f392e4de20..2e861450604 100644 --- a/internal/fixture/httpproxy.go +++ b/internal/fixture/httpproxy.go @@ -26,8 +26,8 @@ func NewProxy(name string) *ProxyBuilder { ObjectMeta: ObjectMeta(name), } - b.ObjectMeta.Annotations = map[string]string{} - b.ObjectMeta.Labels = map[string]string{} + b.Annotations = map[string]string{} + b.Labels = map[string]string{} return b } @@ -47,13 +47,7 @@ func (b *ProxyBuilder) ensureTLS() { // Annotate adds the given values as metadata annotations. func (b *ProxyBuilder) Annotate(k, v string) *ProxyBuilder { - b.ObjectMeta.Annotations[k] = v - return b -} - -// Label adds the given values as metadata labels. -func (b *ProxyBuilder) Label(k, v string) *ProxyBuilder { - b.ObjectMeta.Labels[k] = v + b.Annotations[k] = v return b } diff --git a/internal/fixture/service.go b/internal/fixture/service.go index 225dd3c371b..be13217d398 100644 --- a/internal/fixture/service.go +++ b/internal/fixture/service.go @@ -31,7 +31,7 @@ func NewService(name string) *ServiceBuilder { // Annotate adds the given values as metadata annotations. func (s *ServiceBuilder) Annotate(k, v string) *ServiceBuilder { - s.ObjectMeta.Annotations[k] = v + s.Annotations[k] = v return s } diff --git a/internal/httpsvc/http_test.go b/internal/httpsvc/http_test.go index 89aa70eb2d5..7ce95887f8c 100644 --- a/internal/httpsvc/http_test.go +++ b/internal/httpsvc/http_test.go @@ -39,7 +39,7 @@ func TestHTTPService(t *testing.T) { Port: 8001, FieldLogger: fixture.NewTestLogger(t), } - svc.ServeMux.HandleFunc("/test", func(w http.ResponseWriter, _ *http.Request) { + svc.HandleFunc("/test", func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusOK) }) @@ -118,7 +118,7 @@ func TestHTTPSService(t *testing.T) { err = contourCertBeforeRotation.WritePEM(svc.Cert, svc.Key) checkFatalErr(t, err) - svc.ServeMux.HandleFunc("/test", func(w http.ResponseWriter, _ *http.Request) { + svc.HandleFunc("/test", func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusOK) }) ctx, cancel := context.WithCancel(context.Background()) diff --git a/internal/provisioner/controller/gateway_test.go b/internal/provisioner/controller/gateway_test.go index fc5383f8631..f1ce742a316 100644 --- a/internal/provisioner/controller/gateway_test.go +++ b/internal/provisioner/controller/gateway_test.go @@ -680,7 +680,7 @@ func TestGatewayReconcile(t *testing.T) { require.NotNil(t, deploy.Spec.Replicas) assert.EqualValues(t, 4, *deploy.Spec.Replicas) require.NotNil(t, deploy.Spec.Strategy) - assert.EqualValues(t, apps_v1.RecreateDeploymentStrategyType, deploy.Spec.Strategy.Type) + assert.Equal(t, apps_v1.RecreateDeploymentStrategyType, deploy.Spec.Strategy.Type) }, }, "If ContourDeployment.Spec.Contour.NodePlacement is not specified, the Contour deployment has no node selector or tolerations set": { @@ -1051,7 +1051,7 @@ func TestGatewayReconcile(t *testing.T) { assert.EqualValues(t, 6, *deploy.Spec.Replicas) assert.NotNil(t, deploy.Spec.Strategy) - assert.EqualValues(t, apps_v1.RecreateDeploymentStrategyType, deploy.Spec.Strategy.Type) + assert.Equal(t, apps_v1.RecreateDeploymentStrategyType, deploy.Spec.Strategy.Type) // Verify that a daemonset has *not* been created ds := &apps_v1.DaemonSet{ @@ -1098,7 +1098,7 @@ func TestGatewayReconcile(t *testing.T) { }, } require.NoError(t, r.client.Get(context.Background(), keyFor(ds), ds)) - assert.Contains(t, ds.Spec.Template.ObjectMeta.Annotations, "key") + assert.Contains(t, ds.Spec.Template.Annotations, "key") }, }, @@ -1262,7 +1262,7 @@ func TestGatewayReconcile(t *testing.T) { } require.NoError(t, r.client.Get(context.Background(), keyFor(deploy), deploy)) - assert.Contains(t, deploy.Spec.Template.ObjectMeta.Annotations, "key") + assert.Contains(t, deploy.Spec.Template.Annotations, "key") }, }, @@ -1303,7 +1303,7 @@ func TestGatewayReconcile(t *testing.T) { }, } require.NoError(t, r.client.Get(context.Background(), keyFor(ds), ds)) - assert.EqualValues(t, apps_v1.OnDeleteDaemonSetStrategyType, ds.Spec.UpdateStrategy.Type) + assert.Equal(t, apps_v1.OnDeleteDaemonSetStrategyType, ds.Spec.UpdateStrategy.Type) // Verify that a deployment has *not* been created deployment := &apps_v1.Deployment{ diff --git a/internal/provisioner/equality/equality_test.go b/internal/provisioner/equality/equality_test.go index 856121b3a8c..a6ba0641b7b 100644 --- a/internal/provisioner/equality/equality_test.go +++ b/internal/provisioner/equality/equality_test.go @@ -202,7 +202,7 @@ func TestDeploymentConfigChanged(t *testing.T) { { description: "if probe values are set to default values", mutate: func(deployment *apps_v1.Deployment) { - deployment.Spec.Template.Spec.Containers[0].LivenessProbe.ProbeHandler.HTTPGet.Scheme = "HTTP" + deployment.Spec.Template.Spec.Containers[0].LivenessProbe.HTTPGet.Scheme = "HTTP" deployment.Spec.Template.Spec.Containers[0].LivenessProbe.TimeoutSeconds = int32(1) deployment.Spec.Template.Spec.Containers[0].LivenessProbe.PeriodSeconds = int32(10) deployment.Spec.Template.Spec.Containers[0].LivenessProbe.SuccessThreshold = int32(1) diff --git a/internal/provisioner/objects/dataplane/dataplane_test.go b/internal/provisioner/objects/dataplane/dataplane_test.go index f15e0e43aa3..226d208096c 100644 --- a/internal/provisioner/objects/dataplane/dataplane_test.go +++ b/internal/provisioner/objects/dataplane/dataplane_test.go @@ -108,7 +108,7 @@ func checkDaemonSetHasLabels(t *testing.T, ds *apps_v1.DaemonSet, expected map[s func checkDaemonSetHasPodAnnotations(t *testing.T, ds *apps_v1.DaemonSet, expected map[string]string) { t.Helper() - if apiequality.Semantic.DeepEqual(ds.Spec.Template.ObjectMeta.Annotations, expected) { + if apiequality.Semantic.DeepEqual(ds.Spec.Template.Annotations, expected) { return } @@ -179,7 +179,7 @@ func checkDaemonSetHasVolume(t *testing.T, ds *apps_v1.DaemonSet, vol core_v1.Vo } } - if !(hasVol && hasVolMount) { + if !hasVol || !hasVolMount { t.Errorf("daemonset has not found volume or volumeMount") } } diff --git a/internal/provisioner/objects/service/service.go b/internal/provisioner/objects/service/service.go index bd4ed821dc3..08530c8e337 100644 --- a/internal/provisioner/objects/service/service.go +++ b/internal/provisioner/objects/service/service.go @@ -234,18 +234,20 @@ func DesiredEnvoyService(contour *model.Contour) *core_v1.Service { // Add the Subnet annotation if specified by provider parameters. if subnetNeeded(&contour.Spec) { - if providerParams.Type == model.AzureLoadBalancerProvider { + switch providerParams.Type { + case model.AzureLoadBalancerProvider: svc.Annotations[azureLBSubnetAnnotation] = *providerParams.Azure.Subnet - } else if providerParams.Type == model.GCPLoadBalancerProvider { + case model.GCPLoadBalancerProvider: svc.Annotations[gcpLBSubnetAnnotation] = *providerParams.GCP.Subnet } } // Add LoadBalancerIP parameter if specified by provider parameters. if loadBalancerAddressNeeded(&contour.Spec) { - if providerParams.Type == model.AzureLoadBalancerProvider { + switch providerParams.Type { + case model.AzureLoadBalancerProvider: svc.Spec.LoadBalancerIP = *providerParams.Azure.Address - } else if providerParams.Type == model.GCPLoadBalancerProvider { + case model.GCPLoadBalancerProvider: svc.Spec.LoadBalancerIP = *providerParams.GCP.Address } } diff --git a/internal/status/backendtlspolicyconditions_test.go b/internal/status/backendtlspolicyconditions_test.go index 6c5b922a995..193668819db 100644 --- a/internal/status/backendtlspolicyconditions_test.go +++ b/internal/status/backendtlspolicyconditions_test.go @@ -44,9 +44,9 @@ func TestBackendTLSPolicyAddCondition(t *testing.T) { got := backendTLSPolicyUpdate.ConditionsForAncestorRef(ancestorRef)[0] assert.EqualValues(t, gatewayapi_v1alpha2.PolicyConditionAccepted, got.Type) - assert.EqualValues(t, meta_v1.ConditionTrue, got.Status) + assert.Equal(t, meta_v1.ConditionTrue, got.Status) assert.EqualValues(t, gatewayapi_v1alpha2.PolicyReasonAccepted, got.Reason) - assert.EqualValues(t, "Valid BackendTLSPolicy", got.Message) + assert.Equal(t, "Valid BackendTLSPolicy", got.Message) assert.EqualValues(t, 7, got.ObservedGeneration) } diff --git a/internal/status/routeconditions_test.go b/internal/status/routeconditions_test.go index 65784d0393f..06fd07edf7e 100644 --- a/internal/status/routeconditions_test.go +++ b/internal/status/routeconditions_test.go @@ -42,9 +42,9 @@ func TestHTTPRouteAddCondition(t *testing.T) { got := httpRouteUpdate.ConditionsForParentRef(parentRef)[0] assert.EqualValues(t, gatewayapi_v1.RouteConditionAccepted, got.Type) - assert.EqualValues(t, meta_v1.ConditionTrue, got.Status) - assert.EqualValues(t, "Valid", got.Reason) - assert.EqualValues(t, "Valid HTTPRoute", got.Message) + assert.Equal(t, meta_v1.ConditionTrue, got.Status) + assert.Equal(t, "Valid", got.Reason) + assert.Equal(t, "Valid HTTPRoute", got.Message) assert.EqualValues(t, 7, got.ObservedGeneration) } diff --git a/internal/xdscache/v3/listener.go b/internal/xdscache/v3/listener.go index 9fc3ad94963..a0e62004d80 100644 --- a/internal/xdscache/v3/listener.go +++ b/internal/xdscache/v3/listener.go @@ -511,7 +511,7 @@ func (c *ListenerCache) OnChange(root *dag.DAG) { alpnProtos...) } - listeners[listener.Name].FilterChains = append(listeners[listener.Name].FilterChains, envoy_v3.FilterChainTLS(vh.VirtualHost.Name, downstreamTLS, filters)) + listeners[listener.Name].FilterChains = append(listeners[listener.Name].FilterChains, envoy_v3.FilterChainTLS(vh.Name, downstreamTLS, filters)) // If this VirtualHost has enabled the fallback certificate then set a default // FilterChain which will allow routes with this vhost to accept non-SNI TLS requests. @@ -601,11 +601,11 @@ func httpGlobalExternalAuthConfig(config *GlobalExternalAuthConfig) *envoy_filte return envoy_v3.FilterExternalAuthz(&dag.ExternalAuthorization{ AuthorizationService: &dag.ExtensionCluster{ - Name: dag.ExtensionClusterName(config.ExtensionServiceConfig.ExtensionService), - SNI: config.ExtensionServiceConfig.SNI, + Name: dag.ExtensionClusterName(config.ExtensionService), + SNI: config.SNI, }, AuthorizationFailOpen: config.FailOpen, - AuthorizationResponseTimeout: config.ExtensionServiceConfig.Timeout, + AuthorizationResponseTimeout: config.Timeout, AuthorizationServerWithRequestBody: config.WithRequestBody, }) } @@ -616,10 +616,10 @@ func envoyGlobalRateLimitConfig(config *RateLimitConfig) *envoy_v3.GlobalRateLim } return &envoy_v3.GlobalRateLimitConfig{ - ExtensionService: config.ExtensionServiceConfig.ExtensionService, - SNI: config.ExtensionServiceConfig.SNI, + ExtensionService: config.ExtensionService, + SNI: config.SNI, FailOpen: config.FailOpen, - Timeout: config.ExtensionServiceConfig.Timeout, + Timeout: config.Timeout, Domain: config.Domain, EnableXRateLimitHeaders: config.EnableXRateLimitHeaders, EnableResourceExhaustedCode: config.EnableResourceExhaustedCode, @@ -632,10 +632,10 @@ func envoyTracingConfig(config *TracingConfig) *envoy_v3.EnvoyTracingConfig { } return &envoy_v3.EnvoyTracingConfig{ - ExtensionService: config.ExtensionServiceConfig.ExtensionService, + ExtensionService: config.ExtensionService, ServiceName: config.ServiceName, - SNI: config.ExtensionServiceConfig.SNI, - Timeout: config.ExtensionServiceConfig.Timeout, + SNI: config.SNI, + Timeout: config.Timeout, OverallSampling: config.OverallSampling, MaxPathTagLength: config.MaxPathTagLength, CustomTags: envoyTracingConfigCustomTag(config.CustomTags), diff --git a/internal/xdscache/v3/route.go b/internal/xdscache/v3/route.go index 3e0b21b629a..c5a147ec96e 100644 --- a/internal/xdscache/v3/route.go +++ b/internal/xdscache/v3/route.go @@ -102,7 +102,7 @@ func (c *RouteCache) OnChange(root *dag.DAG) { } // Add secure vhost route config if not already present. - routeConfigName := httpsRouteConfigName(dagListener, vhost.VirtualHost.Name) + routeConfigName := httpsRouteConfigName(dagListener, vhost.Name) if _, ok := routeConfigs[routeConfigName]; !ok { routeConfigs[routeConfigName] = envoy_v3.RouteConfiguration(routeConfigName) diff --git a/test/e2e/bench/bench_test.go b/test/e2e/bench/bench_test.go index 052914962c4..adec8813fc0 100644 --- a/test/e2e/bench/bench_test.go +++ b/test/e2e/bench/bench_test.go @@ -351,7 +351,7 @@ func drawScatterPlot(durations []time.Duration) { s, err := plotter.NewScatter(points) require.NoError(f.T(), err) - s.GlyphStyle.Color = color.RGBA{R: 9, G: 87, B: 245, A: 1} + s.Color = color.RGBA{R: 9, G: 87, B: 245, A: 1} p.Add(s) diff --git a/test/e2e/certs.go b/test/e2e/certs.go index a4d413a6b02..8ea08358462 100644 --- a/test/e2e/certs.go +++ b/test/e2e/certs.go @@ -27,7 +27,6 @@ import ( "github.com/stretchr/testify/require" core_v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" - api_errors "k8s.io/apimachinery/pkg/api/errors" meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -57,7 +56,7 @@ func (c *Certs) CreateSelfSignedCert(ns, name, secretName, dnsName string) func( } if err := c.client.Create(context.TODO(), issuer); err != nil && !errors.IsAlreadyExists(err) { - require.FailNow(c.t, "error creating Issuer: %v", err) + require.FailNowf(c.t, "failed creating Issuer", "error: %s", err) } cert := &certmanagerv1.Certificate{ @@ -122,7 +121,7 @@ func (c *Certs) ensureSelfSignedIssuer(ns string) *certmanagerv1.Issuer { } if err := c.client.Get(context.TODO(), client.ObjectKeyFromObject(issuer), issuer); err != nil { - if api_errors.IsNotFound(err) { + if errors.IsNotFound(err) { require.NoError(c.t, c.client.Create(context.TODO(), issuer)) } else { require.NoError(c.t, err) diff --git a/test/e2e/deployment.go b/test/e2e/deployment.go index 65611fabe67..1bd83c6c56f 100644 --- a/test/e2e/deployment.go +++ b/test/e2e/deployment.go @@ -718,7 +718,7 @@ func (d *Deployment) StopLocalContour(contourCmd *gexec.Session, configFile stri // a minute should be more than enough to avoid them. logs := contourCmd.Terminate().Wait(time.Minute).Err.Contents() if strings.Contains(string(logs), "DATA RACE") { - return errors.New("Detected data race, see log output above to diagnose") + return errors.New("detected data race, see log output above to diagnose") } return nil } diff --git a/test/e2e/framework.go b/test/e2e/framework.go index b41bf59b02b..497e996a0c3 100644 --- a/test/e2e/framework.go +++ b/test/e2e/framework.go @@ -553,8 +553,8 @@ func HTTPProxyErrors(proxy *contour_v1.HTTPProxy) string { // list contains a condition of type "Valid" and status "False". func DetailedConditionInvalid(conditions []contour_v1.DetailedCondition) bool { for _, c := range conditions { - if c.Condition.Type == "Valid" { - return c.Condition.Status == "False" + if c.Type == "Valid" { + return c.Status == "False" } } return false diff --git a/test/e2e/gateway/tcproute_test.go b/test/e2e/gateway/tcproute_test.go index 51dae2821de..4c92df7da77 100644 --- a/test/e2e/gateway/tcproute_test.go +++ b/test/e2e/gateway/tcproute_test.go @@ -69,8 +69,8 @@ func testTCPRoute(namespace string, gateway types.NamespacedName) { // Envoy is expected to add the "server: envoy" and // "x-envoy-upstream-service-time" HTTP headers when // proxying HTTP; this ensures we are proxying TCP only. - assert.Equal(t, "", res.Headers.Get("server")) - assert.Equal(t, "", res.Headers.Get("x-envoy-upstream-service-time")) + assert.Empty(t, res.Headers.Get("server")) + assert.Empty(t, res.Headers.Get("x-envoy-upstream-service-time")) // Delete route and wait for config to no longer be present so this // test doesn't pollute others. This route effectively matches all diff --git a/test/e2e/httpproxy/internal_redirect_test.go b/test/e2e/httpproxy/internal_redirect_test.go index a020dab9068..916da216ee4 100644 --- a/test/e2e/httpproxy/internal_redirect_test.go +++ b/test/e2e/httpproxy/internal_redirect_test.go @@ -111,7 +111,7 @@ func doInternalRedirectTest(namespace string, proxy *contour_v1.HTTPProxy, t Gin }, Spec: core_v1.ServiceSpec{ Type: core_v1.ServiceTypeExternalName, - ExternalName: f.Deployment.EnvoyService.ObjectMeta.Name + "." + f.Deployment.EnvoyService.ObjectMeta.Namespace, + ExternalName: f.Deployment.EnvoyService.Name + "." + f.Deployment.EnvoyService.Namespace, Ports: []core_v1.ServicePort{ { Name: "http", diff --git a/test/e2e/ingress/headers_policy_test.go b/test/e2e/ingress/headers_policy_test.go index 6820ce8ca8f..6967a43d396 100644 --- a/test/e2e/ingress/headers_policy_test.go +++ b/test/e2e/ingress/headers_policy_test.go @@ -93,8 +93,8 @@ func testGlobalHeadersPolicy(applyToIngress bool) e2e.NamespacedTestBody { assert.Equal(t, "foo", f.GetEchoResponseBody(res.Body).RequestHeaders.Get("X-Contour-GlobalRequestHeader")) assert.Equal(t, "bar", res.Headers.Get("X-Contour-GlobalResponseHeader")) } else { - assert.Equal(t, "", f.GetEchoResponseBody(res.Body).RequestHeaders.Get("X-Contour-GlobalRequestHeader")) - assert.Equal(t, "", res.Headers.Get("X-Contour-GlobalResponseHeader")) + assert.Empty(t, f.GetEchoResponseBody(res.Body).RequestHeaders.Get("X-Contour-GlobalRequestHeader")) + assert.Empty(t, res.Headers.Get("X-Contour-GlobalResponseHeader")) } }) } diff --git a/test/e2e/provisioner/provisioner_test.go b/test/e2e/provisioner/provisioner_test.go index 8c29f04a834..dba410892b3 100644 --- a/test/e2e/provisioner/provisioner_test.go +++ b/test/e2e/provisioner/provisioner_test.go @@ -381,7 +381,7 @@ var _ = Describe("Gateway provisioner", func() { } require.True(f.T(), f.CreateGatewayAndWaitFor(gateway, func(gw *gatewayapi_v1.Gateway) bool { - if !(e2e.GatewayProgrammed(gw) && e2e.GatewayHasAddress(gw)) { + if !e2e.GatewayProgrammed(gw) || !e2e.GatewayHasAddress(gw) { return false } @@ -502,8 +502,8 @@ var _ = Describe("Gateway provisioner", func() { // Envoy is expected to add the "server: envoy" and // "x-envoy-upstream-service-time" HTTP headers when // proxying HTTP; this ensures we are proxying TCP only. - assert.Equal(f.T(), "", res.Headers.Get("server")) - assert.Equal(f.T(), "", res.Headers.Get("x-envoy-upstream-service-time")) + assert.Empty(f.T(), res.Headers.Get("server")) + assert.Empty(f.T(), res.Headers.Get("x-envoy-upstream-service-time")) } }) }) diff --git a/test/e2e/upgrade/upgrade_test.go b/test/e2e/upgrade/upgrade_test.go index 3ab5f57b58a..4e4bf17038c 100644 --- a/test/e2e/upgrade/upgrade_test.go +++ b/test/e2e/upgrade/upgrade_test.go @@ -130,7 +130,7 @@ var _ = Describe("When upgrading", func() { poller.Stop() totalRequests, successfulRequests := poller.Results() f.T().Logf("Total requests: %d, successful requests: %d\n", totalRequests, successfulRequests) - require.Positive(f.T(), totalRequests) + require.NotZero(f.T(), totalRequests) successPercentage := 100 * float64(successfulRequests) / float64(totalRequests) require.Greaterf(f.T(), successPercentage, float64(90.0), "success rate of %.2f%% less than 90%%", successPercentage) }) @@ -292,7 +292,7 @@ var _ = Describe("When upgrading", func() { poller.Stop() totalRequests, successfulRequests := poller.Results() f.T().Logf("Total requests: %d, successful requests: %d\n", totalRequests, successfulRequests) - require.Positive(f.T(), totalRequests) + require.NotZero(f.T(), totalRequests) successPercentage := 100 * float64(successfulRequests) / float64(totalRequests) // Success threshold is somewhat arbitrary but less than the standalone // Contour upgrade threshold because the Gateway provisioner does not