Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix golang-ci linter errors #10128

Merged
merged 7 commits into from
Jul 3, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 2 additions & 7 deletions cmd/dataplane/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,10 @@ package main

import (
"fmt"
"math/rand" // #nosec
"net/http"
"os"
"time"

"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/collectors"
"net/http"
"os"

"k8s.io/klog/v2"

Expand All @@ -41,8 +38,6 @@ import (
func main() {
klog.InitFlags(nil)

rand.Seed(time.Now().UnixNano())
z1cheng marked this conversation as resolved.
Show resolved Hide resolved

fmt.Println(version.String())
var err error
showVersion, conf, err := ingressflags.ParseFlags()
Expand Down
3 changes: 0 additions & 3 deletions cmd/nginx/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package main
import (
"context"
"fmt"
"math/rand" // #nosec
"net/http"
"os"
"path/filepath"
Expand Down Expand Up @@ -54,8 +53,6 @@ import (
func main() {
klog.InitFlags(nil)

rand.Seed(time.Now().UnixNano())

fmt.Println(version.String())

showVersion, conf, err := ingressflags.ParseFlags()
Expand Down
4 changes: 3 additions & 1 deletion cmd/plugin/commands/certs/certs.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ func CreateCommand(flags *genericclioptions.ConfigFlags) *cobra.Command {
}

cmd.Flags().String("host", "", "Get the cert for this hostname")
cobra.MarkFlagRequired(cmd.Flags(), "host")
if err := cobra.MarkFlagRequired(cmd.Flags(), "host"); err != nil {
z1cheng marked this conversation as resolved.
Show resolved Hide resolved
fmt.Printf("error marking flag as required: %v", err)
}
pod = util.AddPodFlag(cmd)
deployment = util.AddDeploymentFlag(cmd)
selector = util.AddSelectorFlag(cmd)
Expand Down
6 changes: 5 additions & 1 deletion cmd/plugin/kubectl/kubectl.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,11 @@ func execToWriter(args []string, writer io.Writer) error {
return err
}

go io.Copy(writer, op)
go func() {
z1cheng marked this conversation as resolved.
Show resolved Hide resolved
if _, err := io.Copy(writer, op); err != nil {
fmt.Printf("Error copying output: %v\n", err)
}
}()
err = cmd.Run()
if err != nil {
return err
Expand Down
4 changes: 3 additions & 1 deletion images/fastcgi-helloserver/rootfs/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,7 @@ func main() {
if err != nil {
panic(err)
}
fcgi.Serve(l, nil)
if err := fcgi.Serve(l, nil); err != nil {
panic(err)
}
}
2 changes: 1 addition & 1 deletion images/kube-webhook-certgen/rootfs/pkg/k8s/k8s_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ package k8s
import (
"bytes"
"context"
"crypto/rand"
"errors"
"math/rand"
"testing"
"time"

Expand Down
14 changes: 9 additions & 5 deletions internal/admission/controller/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func TestHandleAdmission(t *testing.T) {
Checker: failTestChecker{t: t},
}

result, err := adm.HandleAdmission(&admissionv1.AdmissionReview{
_, err := adm.HandleAdmission(&admissionv1.AdmissionReview{
z1cheng marked this conversation as resolved.
Show resolved Hide resolved
Request: &admissionv1.AdmissionRequest{
Kind: v1.GroupVersionKind{Group: "", Version: "v1", Kind: "Pod"},
},
Expand All @@ -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{
Expand Down Expand Up @@ -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")
}
Expand All @@ -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")
}
Expand Down
4 changes: 3 additions & 1 deletion internal/admission/controller/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
28 changes: 22 additions & 6 deletions internal/ingress/controller/checker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -90,14 +93,25 @@ 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()
if err := cmd.Wait(); err != nil {
t.Errorf("unexpected error waiting for the process: %v", err)
}
}()

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)
Expand All @@ -109,7 +123,9 @@ func TestNginxCheck(t *testing.T) {
})

// pollute pid file
pidFile.Write([]byte("999999"))
if _, err := pidFile.Write([]byte("999999")); err != nil {
t.Errorf("unexpected error polluting the pid file: %v", err)
}
pidFile.Close()

t.Run("bad pid", func(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion internal/ingress/controller/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ const (

// Configuration represents the content of nginx.conf file
type Configuration struct {
defaults.Backend `json:",squash"`
rikatz marked this conversation as resolved.
Show resolved Hide resolved
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.
Expand Down
13 changes: 10 additions & 3 deletions internal/ingress/controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -167,6 +167,9 @@ func TestCheckIngress(t *testing.T) {
}
return nil
})
if err != nil {
t.Errorf("unexpected error: %v", err)
}
}()

err := file.CreateRequiredDirectories()
Expand All @@ -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{}
Expand Down
42 changes: 1 addition & 41 deletions internal/ingress/controller/nginx.go
z1cheng marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -248,8 +248,7 @@ type NGINXController struct {

store store.Storer

metricCollector metric.Collector
admissionCollector metric.Collector
metricCollector metric.Collector

validationWebhookServer *http.Server

Expand Down Expand Up @@ -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 {
Expand Down
20 changes: 15 additions & 5 deletions internal/ingress/controller/store/endpointslice_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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 {
Expand Down Expand Up @@ -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,
Expand All @@ -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)
Expand Down
29 changes: 22 additions & 7 deletions internal/ingress/controller/store/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -793,14 +796,26 @@ func New(
},
}

store.informers.Ingress.AddEventHandler(ingEventHandler)
if _, err := store.informers.Ingress.AddEventHandler(ingEventHandler); err != nil {
klog.Errorf("Error adding ingress event handler: %v", err)
}
if !icConfig.IgnoreIngressClass {
store.informers.IngressClass.AddEventHandler(ingressClassEventHandler)
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)
Expand Down
Loading