DevFW-CICD/ingress-nginx-helm
6
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge pull request #10227 from k8s-infra-cherrypick-robot/cherry-pick-10128-to-release-1.8

Browse source 
[release-1.8] Fix golang-ci linter errors
This commit is contained in:
James Strong 2023-07-20 13:52:54 -04:00 • committed by GitHub
commit 443dfbe096
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
37 changed files with 206 additions and 162 deletions
Download patch file Download diff file Expand all files Collapse all files

14
internal/admission/controller/main_test.go
View file

@ -67,7 +67,7 @@ func TestHandleAdmission(t *testing.T) {
Checker: failTestChecker{t: t},
}
result, err := adm.HandleAdmission(&admissionv1.AdmissionReview{
_, err := adm.HandleAdmission(&admissionv1.AdmissionReview{
Request: &admissionv1.AdmissionRequest{
Kind: v1.GroupVersionKind{Group: "", Version: "v1", Kind: "Pod"},
},
@ -76,12 +76,12 @@ func TestHandleAdmission(t *testing.T) {
t.Fatalf("with a non ingress resource, the check should not pass")
}
result, err = adm.HandleAdmission(nil)
_, err = adm.HandleAdmission(nil)
if err == nil {
t.Fatalf("with a nil AdmissionReview request, the check should not pass")
}
result, err = adm.HandleAdmission(&admissionv1.AdmissionReview{
result, err := adm.HandleAdmission(&admissionv1.AdmissionReview{
Request: &admissionv1.AdmissionRequest{
Kind: v1.GroupVersionKind{Group: networking.GroupName, Version: "v1", Kind: "Ingress"},
Object: runtime.RawExtension{
@ -114,7 +114,9 @@ func TestHandleAdmission(t *testing.T) {
err: fmt.Errorf("this is a test error"),
}
adm.HandleAdmission(review)
if _, err := adm.HandleAdmission(review); err != nil {
t.Errorf("unexpected error: %v", err)
}
if review.Response.Allowed {
t.Fatalf("when the checker returns an error, the request should not be allowed")
}
@ -124,7 +126,9 @@ func TestHandleAdmission(t *testing.T) {
err: nil,
}
adm.HandleAdmission(review)
if _, err := adm.HandleAdmission(review); err != nil {
t.Errorf("unexpected error: %v", err)
}
if !review.Response.Allowed {
t.Fatalf("when the checker returns no error, the request should be allowed")
}

4
internal/admission/controller/server.go
View file

@ -31,7 +31,9 @@ var (
)
func init() {
admissionv1.AddToScheme(scheme)
if err := admissionv1.AddToScheme(scheme); err != nil {
klog.ErrorS(err, "Failed to add scheme")
}
}
// AdmissionController checks if an object

24
internal/ingress/controller/checker_test.go
View file

@ -76,7 +76,10 @@ func TestNginxCheck(t *testing.T) {
})
// create pid file
os.MkdirAll("/tmp/nginx", file.ReadWriteByUser)
if err := os.MkdirAll("/tmp/nginx", file.ReadWriteByUser); err != nil {
t.Errorf("unexpected error creating pid file: %v", err)
}
pidFile, err := os.Create(nginx.PID)
if err != nil {
t.Fatalf("unexpected error: %v", err)
@ -90,14 +93,23 @@ func TestNginxCheck(t *testing.T) {
// start dummy process to use the PID
cmd := exec.Command("sleep", "3600")
cmd.Start()
if err := cmd.Start(); err != nil {
t.Errorf("unexpected error: %v", err)
}
pid := cmd.Process.Pid
defer cmd.Process.Kill()
defer func() {
if err := cmd.Process.Kill(); err != nil {
t.Errorf("unexpected error killing the process: %v", err)
}
}()
go func() {
cmd.Wait()
cmd.Wait() //nolint:errcheck
}()
pidFile.Write([]byte(fmt.Sprintf("%v", pid)))
if _, err := pidFile.Write([]byte(fmt.Sprintf("%v", pid))); err != nil {
t.Errorf("unexpected error writing the pid file: %v", err)
}
pidFile.Close()
healthz.InstallPathHandler(mux, tt.healthzPath, n)
@ -109,7 +121,7 @@ func TestNginxCheck(t *testing.T) {
})
// pollute pid file
pidFile.Write([]byte("999999"))
pidFile.Write([]byte("999999")) //nolint:errcheck
pidFile.Close()
t.Run("bad pid", func(t *testing.T) {

2
internal/ingress/controller/config/config.go
View file

@ -91,7 +91,7 @@ const (
// Configuration represents the content of nginx.conf file
type Configuration struct {
defaults.Backend `json:",squash"`
defaults.Backend `json:",squash"` //nolint:staticcheck
// AllowSnippetAnnotations enable users to add their own snippets via ingress annotation.
// If disabled, only snippets added via ConfigMap are added to ingress.

13
internal/ingress/controller/controller_test.go
View file

@ -158,7 +158,7 @@ func (fakeTemplate) Write(conf ngx_config.TemplateConfig) ([]byte, error) {
func TestCheckIngress(t *testing.T) {
defer func() {
filepath.Walk(os.TempDir(), func(path string, info os.FileInfo, err error) error {
err := filepath.Walk(os.TempDir(), func(path string, info os.FileInfo, err error) error {
if info.IsDir() && os.TempDir() != path {
return filepath.SkipDir
}
@ -167,6 +167,9 @@ func TestCheckIngress(t *testing.T) {
}
return nil
})
if err != nil {
t.Errorf("unexpected error: %v", err)
}
}()
err := file.CreateRequiredDirectories()
@ -176,9 +179,13 @@ func TestCheckIngress(t *testing.T) {
// Ensure no panic with wrong arguments
var nginx *NGINXController
nginx.CheckIngress(nil)
if err := nginx.CheckIngress(nil); err != nil {
t.Errorf("unexpected error: %v", err)
}
nginx = newNGINXController(t)
nginx.CheckIngress(nil)
if err := nginx.CheckIngress(nil); err != nil {
t.Errorf("unexpected error: %v", err)
}
nginx.metricCollector = metric.DummyCollector{}
nginx.t = fakeTemplate{}

42
internal/ingress/controller/nginx.go
View file

@ -248,8 +248,7 @@ type NGINXController struct {
store store.Storer
metricCollector metric.Collector
admissionCollector metric.Collector
metricCollector metric.Collector
validationWebhookServer *http.Server
@ -799,45 +798,6 @@ func (n *NGINXController) setupSSLProxy() {
}()
}
// Helper function to clear Certificates from the ingress configuration since they should be ignored when
// checking if the new configuration changes can be applied dynamically if dynamic certificates is on
func clearCertificates(config *ingress.Configuration) {
var clearedServers []*ingress.Server
for _, server := range config.Servers {
copyOfServer := *server
copyOfServer.SSLCert = nil
clearedServers = append(clearedServers, &copyOfServer)
}
config.Servers = clearedServers
}
// Helper function to clear endpoints from the ingress configuration since they should be ignored when
// checking if the new configuration changes can be applied dynamically.
func clearL4serviceEndpoints(config *ingress.Configuration) {
var clearedTCPL4Services []ingress.L4Service
var clearedUDPL4Services []ingress.L4Service
for _, service := range config.TCPEndpoints {
copyofService := ingress.L4Service{
Port: service.Port,
Backend: service.Backend,
Endpoints: []ingress.Endpoint{},
Service: nil,
}
clearedTCPL4Services = append(clearedTCPL4Services, copyofService)
}
for _, service := range config.UDPEndpoints {
copyofService := ingress.L4Service{
Port: service.Port,
Backend: service.Backend,
Endpoints: []ingress.Endpoint{},
Service: nil,
}
clearedUDPL4Services = append(clearedUDPL4Services, copyofService)
}
config.TCPEndpoints = clearedTCPL4Services
config.UDPEndpoints = clearedUDPL4Services
}
// configureDynamically encodes new Backends in JSON format and POSTs the
// payload to an internal HTTP endpoint handled by Lua.
func (n *NGINXController) configureDynamically(pcfg *ingress.Configuration) error {

20
internal/ingress/controller/store/endpointslice_test.go
View file

@ -59,7 +59,9 @@ func TestEndpointSliceLister(t *testing.T) {
},
},
}
el.Add(endpointSlice)
if err := el.Add(endpointSlice); err != nil {
t.Errorf("unexpected error %v", err)
}
endpointSlice = &discoveryv1.EndpointSlice{
ObjectMeta: metav1.ObjectMeta{
Namespace: "namespace",
@ -69,7 +71,9 @@ func TestEndpointSliceLister(t *testing.T) {
},
},
}
el.Add(endpointSlice)
if err := el.Add(endpointSlice); err != nil {
t.Errorf("unexpected error %v", err)
}
endpointSlice = &discoveryv1.EndpointSlice{
ObjectMeta: metav1.ObjectMeta{
Namespace: "namespace",
@ -79,7 +83,9 @@ func TestEndpointSliceLister(t *testing.T) {
},
},
}
el.Add(endpointSlice)
if err := el.Add(endpointSlice); err != nil {
t.Errorf("unexpected error %v", err)
}
eps, err := el.MatchByKey(key)
if err != nil {
@ -108,7 +114,9 @@ func TestEndpointSliceLister(t *testing.T) {
},
},
}
el.Add(endpointSlice)
if err := el.Add(endpointSlice); err != nil {
t.Errorf("unexpected error %v", err)
}
endpointSlice2 := &discoveryv1.EndpointSlice{
ObjectMeta: metav1.ObjectMeta{
Namespace: ns2,
@ -118,7 +126,9 @@ func TestEndpointSliceLister(t *testing.T) {
},
},
}
el.Add(endpointSlice2)
if err := el.Add(endpointSlice2); err != nil {
t.Errorf("unexpected error %v", err)
}
eps, err := el.MatchByKey(key)
if err != nil {
t.Errorf("unexpeted error %v", err)

31
internal/ingress/controller/store/store.go
View file

@ -404,7 +404,10 @@ func New(
return
}
store.listers.IngressWithAnnotation.Delete(ing)
if err := store.listers.IngressWithAnnotation.Delete(ing); err != nil {
klog.ErrorS(err, "Error while deleting ingress from store", "ingress", klog.KObj(ing))
return
}
key := k8s.MetaNamespaceKey(ing)
store.secretIngressMap.Delete(key)
@ -793,14 +796,26 @@ func New(
},
}
store.informers.Ingress.AddEventHandler(ingEventHandler)
if !icConfig.IgnoreIngressClass {
store.informers.IngressClass.AddEventHandler(ingressClassEventHandler)
if _, err := store.informers.Ingress.AddEventHandler(ingEventHandler); err != nil {
klog.Errorf("Error adding ingress event handler: %v", err)
}
if !icConfig.IgnoreIngressClass {
if _, err := store.informers.IngressClass.AddEventHandler(ingressClassEventHandler); err != nil {
klog.Errorf("Error adding ingress class event handler: %v", err)
}
}
if _, err := store.informers.EndpointSlice.AddEventHandler(epsEventHandler); err != nil {
klog.Errorf("Error adding endpoint slice event handler: %v", err)
}
if _, err := store.informers.Secret.AddEventHandler(secrEventHandler); err != nil {
klog.Errorf("Error adding secret event handler: %v", err)
}
if _, err := store.informers.ConfigMap.AddEventHandler(cmEventHandler); err != nil {
klog.Errorf("Error adding configmap event handler: %v", err)
}
if _, err := store.informers.Service.AddEventHandler(serviceHandler); err != nil {
klog.Errorf("Error adding service event handler: %v", err)
}
store.informers.EndpointSlice.AddEventHandler(epsEventHandler)
store.informers.Secret.AddEventHandler(secrEventHandler)
store.informers.ConfigMap.AddEventHandler(cmEventHandler)
store.informers.Service.AddEventHandler(serviceHandler)
// do not wait for informers to read the configmap configuration
ns, name, _ := k8s.ParseNameNS(configmap)

36
internal/ingress/controller/store/store_test.go
View file

@ -92,7 +92,7 @@ func TestStore(t *testing.T) {
emptySelector, _ := labels.Parse("")
defer te.Stop()
defer te.Stop() //nolint:errcheck
clientSet, err := kubernetes.NewForConfig(cfg)
if err != nil {
@ -1377,14 +1377,18 @@ func TestUpdateSecretIngressMap(t *testing.T) {
Namespace: "testns",
},
}
s.listers.Ingress.Add(ingTpl)
if err := s.listers.Ingress.Add(ingTpl); err != nil {
t.Errorf("error adding the Ingress template: %v", err)
}
t.Run("with TLS secret", func(t *testing.T) {
ing := ingTpl.DeepCopy()
ing.Spec = networking.IngressSpec{
TLS: []networking.IngressTLS{{SecretName: "tls"}},
}
s.listers.Ingress.Update(ing)
if err := s.listers.Ingress.Update(ing); err != nil {
t.Errorf("error updating the Ingress: %v", err)
}
s.updateSecretIngressMap(ing)
if l := s.secretIngressMap.Len(); !(l == 1 && s.secretIngressMap.Has("testns/tls")) {
@ -1397,7 +1401,9 @@ func TestUpdateSecretIngressMap(t *testing.T) {
ing.ObjectMeta.SetAnnotations(map[string]string{
parser.GetAnnotationWithPrefix("auth-secret"): "auth",
})
s.listers.Ingress.Update(ing)
if err := s.listers.Ingress.Update(ing); err != nil {
t.Errorf("error updating the Ingress: %v", err)
}
s.updateSecretIngressMap(ing)
if l := s.secretIngressMap.Len(); !(l == 1 && s.secretIngressMap.Has("testns/auth")) {
@ -1410,7 +1416,9 @@ func TestUpdateSecretIngressMap(t *testing.T) {
ing.ObjectMeta.SetAnnotations(map[string]string{
parser.GetAnnotationWithPrefix("auth-secret"): "otherns/auth",
})
s.listers.Ingress.Update(ing)
if err := s.listers.Ingress.Update(ing); err != nil {
t.Errorf("error updating the Ingress: %v", err)
}
s.updateSecretIngressMap(ing)
if l := s.secretIngressMap.Len(); !(l == 1 && s.secretIngressMap.Has("otherns/auth")) {
@ -1423,7 +1431,9 @@ func TestUpdateSecretIngressMap(t *testing.T) {
ing.ObjectMeta.SetAnnotations(map[string]string{
parser.GetAnnotationWithPrefix("auth-secret"): "ns/name/garbage",
})
s.listers.Ingress.Update(ing)
if err := s.listers.Ingress.Update(ing); err != nil {
t.Errorf("error updating the Ingress: %v", err)
}
s.updateSecretIngressMap(ing)
if l := s.secretIngressMap.Len(); l != 0 {
@ -1457,7 +1467,9 @@ func TestListIngresses(t *testing.T) {
},
},
}
s.listers.IngressWithAnnotation.Add(ingressToIgnore)
if err := s.listers.IngressWithAnnotation.Add(ingressToIgnore); err != nil {
t.Errorf("error adding the Ingress: %v", err)
}
ingressWithoutPath := &ingress.Ingress{
Ingress: networking.Ingress{
@ -1492,8 +1504,9 @@ func TestListIngresses(t *testing.T) {
},
},
}
s.listers.IngressWithAnnotation.Add(ingressWithoutPath)
if err := s.listers.IngressWithAnnotation.Add(ingressWithoutPath); err != nil {
t.Errorf("error adding the Ingress: %v", err)
}
ingressWithNginxClassAnnotation := &ingress.Ingress{
Ingress: networking.Ingress{
ObjectMeta: metav1.ObjectMeta{
@ -1531,8 +1544,9 @@ func TestListIngresses(t *testing.T) {
},
},
}
s.listers.IngressWithAnnotation.Add(ingressWithNginxClassAnnotation)
if err := s.listers.IngressWithAnnotation.Add(ingressWithNginxClassAnnotation); err != nil {
t.Errorf("error adding the Ingress: %v", err)
}
ingresses := s.ListIngresses()
if s := len(ingresses); s != 3 {

15
internal/ingress/controller/template/template.go
View file

@ -18,13 +18,14 @@ package template
import (
"bytes"
"crypto/rand"
"crypto/sha1" // #nosec
"encoding/base64"
"encoding/hex"
"encoding/json"
"fmt"
"io"
"math/rand" // #nosec
"math/big"
"net"
"net/url"
"os"
@ -34,7 +35,6 @@ import (
"strconv"
"strings"
text_template "text/template"
"time"
networkingv1 "k8s.io/api/networking/v1"
"k8s.io/apimachinery/pkg/util/sets"
@ -1187,14 +1187,15 @@ func buildAuthSignURLLocation(location, authSignURL string) string {
var letters = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ")
func init() {
rand.Seed(time.Now().UnixNano())
}
func randomString() string {
b := make([]rune, 32)
for i := range b {
b[i] = letters[rand.Intn(len(letters))] // #nosec
idx, err := rand.Int(rand.Reader, big.NewInt(int64(len(letters))))
if err != nil {
klog.Errorf("unexpected error generating random index: %v", err)
return ""
}
b[i] = letters[idx.Int64()]
}
return string(b)

4
internal/ingress/controller/template/template_test.go
View file

@ -767,7 +767,9 @@ func BenchmarkTemplateWithData(b *testing.B) {
}
for i := 0; i < b.N; i++ {
ngxTpl.Write(dat)
if _, err := ngxTpl.Write(dat); err != nil {
b.Errorf("unexpected error writing template: %v", err)
}
}
}

4
internal/ingress/metric/collectors/process_test.go
View file

@ -48,7 +48,7 @@ func TestProcessCollector(t *testing.T) {
done := make(chan struct{})
go func() {
cmd.Wait()
cmd.Wait() //nolint:errcheck
status := cmd.ProcessState.Sys().(syscall.WaitStatus)
if status.Signaled() {
t.Logf("Signal: %v", status.Signal())
@ -69,7 +69,7 @@ func TestProcessCollector(t *testing.T) {
defer func() {
cm.Stop()
cmd.Process.Kill()
cmd.Process.Kill() //nolint:errcheck
<-done
close(done)
}()

4
internal/ingress/metric/collectors/socket_test.go
View file

@ -58,7 +58,9 @@ func TestNewUDPLogListener(t *testing.T) {
}()
conn, _ := net.Dial("unix", tmpFile)
conn.Write([]byte("message"))
if _, err := conn.Write([]byte("message")); err != nil {
t.Errorf("unexpected error writing to unix socket: %v", err)
}
conn.Close()
time.Sleep(1 * time.Millisecond)

5
internal/ingress/status/status.go
View file

@ -95,10 +95,13 @@ func (s statusSync) Run(stopCh chan struct{}) {
// when this instance is the leader we need to enqueue
// an item to trigger the update of the Ingress status.
wait.PollUntil(time.Duration(UpdateInterval)*time.Second, func() (bool, error) {
err := wait.PollUntil(time.Duration(UpdateInterval)*time.Second, func() (bool, error) {
s.syncQueue.EnqueueTask(task.GetDummyObject("sync status"))
return false, nil
}, stopCh)
if err != nil {
klog.ErrorS(err, "error running poll")
}
}
// Shutdown stops the sync. In case the instance is the leader it will remove the current IP

4
internal/ingress/status/status_test.go
View file

@ -325,7 +325,9 @@ func TestStatusActions(t *testing.T) {
// wait for the election
time.Sleep(100 * time.Millisecond)
// execute sync
fk.sync("just-test")
if err := fk.sync("just-test"); err != nil {
t.Errorf("unexpected error: %v", err)
}
// PublishService is empty, so the running address is: ["11.0.0.2"]
// after updated, the ingress's ip should only be "11.0.0.2"
newIPs := []networking.IngressLoadBalancerIngress{{

5
internal/net/dns/dns_test.go
View file

@ -40,13 +40,16 @@ func TestGetDNSServers(t *testing.T) {
defer f.Close()
defer os.Remove(f.Name())
os.WriteFile(f.Name(), []byte(`
err = os.WriteFile(f.Name(), []byte(`
# comment
; comment
nameserver 2001:4860:4860::8844
nameserver 2001:4860:4860::8888
nameserver 8.8.8.8
`), file.ReadWriteByUser)
if err != nil {
t.Errorf("unexpected error: %v", err)
}
defResolvConf = f.Name()
s, err = GetSystemNameServers()

2
internal/net/ssl/ssl.go
View file

@ -228,7 +228,7 @@ func ConfigureCRL(name string, crl []byte, sslCert *ingress.SSLCert) error {
return fmt.Errorf("CRL file %v contains invalid data, and must be created only with PEM formatted certificates", name)
}
_, err := x509.ParseCRL(pemCRLBlock.Bytes)
_, err := x509.ParseRevocationList(pemCRLBlock.Bytes)
if err != nil {
return fmt.Errorf(err.Error())
}

8
internal/net/ssl/ssl_test.go
View file

@ -397,7 +397,9 @@ func newFakeCertificate(t *testing.T) ([]byte, string, string) {
t.Errorf("failed to write test key: %v", err)
}
certFile.Write(cert)
if _, err := certFile.Write(cert); err != nil {
t.Errorf("failed to write cert: %v", err)
}
defer certFile.Close()
keyFile, err := os.CreateTemp("", "key-")
@ -405,7 +407,9 @@ func newFakeCertificate(t *testing.T) ([]byte, string, string) {
t.Errorf("failed to write test key: %v", err)
}
keyFile.Write(key)
if _, err := keyFile.Write(key); err != nil {
t.Errorf("failed to write key: %v", err)
}
defer keyFile.Close()
return cert, certFile.Name(), keyFile.Name()