diff --git a/.golangci.yml b/.golangci.yml index b238df381e82..f52f9f10d6ab 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -116,10 +116,12 @@ linters: - gosimple - interfacer - lll + - maligned - misspell - staticcheck - structcheck - ineffassign + - typecheck - unconvert - unparam - unused @@ -128,7 +130,6 @@ linters: disable: - gocritic - prealloc - - typecheck - megacheck - nakedret - scopelint @@ -138,7 +139,6 @@ linters: # start here to enable back all gometalinter linters - errcheck - goconst - - maligned disable-all: false presets: - bugs diff --git a/galley/pkg/crd/validation/webhook_test.go b/galley/pkg/crd/validation/webhook_test.go index 24677d5dd9f2..e9dd76f3d7c2 100644 --- a/galley/pkg/crd/validation/webhook_test.go +++ b/galley/pkg/crd/validation/webhook_test.go @@ -537,9 +537,9 @@ func TestServe(t *testing.T) { name string body []byte contentType string - wantAllowed bool wantStatusCode int - allowedResponse bool // + wantAllowed bool + allowedResponse bool }{ { name: "valid", diff --git a/galley/pkg/server/args.go b/galley/pkg/server/args.go index f74eac9f7992..d71430c42ace 100644 --- a/galley/pkg/server/args.go +++ b/galley/pkg/server/args.go @@ -43,27 +43,18 @@ type Args struct { // Address to use for Galley's gRPC API. APIAddress string - // Enables gRPC-level tracing - EnableGRPCTracing bool - // Maximum size of individual received gRPC messages MaxReceivedMessageSize uint // Maximum number of outstanding RPCs per connection MaxConcurrentStreams uint - // Insecure gRPC service is used for the MCP server. CertificateFile and KeyFile is ignored. - Insecure bool - // The credential options to use for MCP. CredentialOptions *creds.Options // The introspection options to use IntrospectionOptions *ctrlz.Options - // Enable galley server mode - EnableServer bool - // AccessListFile is the YAML file that specifies ids of the allowed mTLS peers. AccessListFile string @@ -79,11 +70,6 @@ type Args struct { // DNS Domain suffix to use while constructing Ingress based resources. DomainSuffix string - // DisableResourceReadyCheck disables the CRD readiness check. This - // allows Galley to start when not all supported CRD are - // registered with the kube-apiserver. - DisableResourceReadyCheck bool - // SinkAddress should be set to the address of a MCP Resource // Sink service that Galley will dial out to. Leaving empty disables // sink. @@ -96,6 +82,20 @@ type Args struct { // SinkMeta list of key=values to attach as gRPC stream metadata to // outgoing Sink connections. SinkMeta []string + + // Enables gRPC-level tracing + EnableGRPCTracing bool + + // Insecure gRPC service is used for the MCP server. CertificateFile and KeyFile is ignored. + Insecure bool + + // Enable galley server mode + EnableServer bool + + // DisableResourceReadyCheck disables the CRD readiness check. This + // allows Galley to start when not all supported CRD are + // registered with the kube-apiserver. + DisableResourceReadyCheck bool } // DefaultArgs allocates an Args struct initialized with Mixer's default configuration. diff --git a/galley/pkg/server/callout_test.go b/galley/pkg/server/callout_test.go index 8684d4fbce02..19ceaea88b05 100644 --- a/galley/pkg/server/callout_test.go +++ b/galley/pkg/server/callout_test.go @@ -36,19 +36,19 @@ func TestCallout(t *testing.T) { if len(co.metadata) != 2 || co.metadata[0] != "foo" || co.metadata[1] != "bar" { t.Errorf("Callout meta not set: %v", co.metadata) } - co, err = newCallout("foo", "NONE", []string{"foo"}, &source.Options{}) + _, err = newCallout("foo", "NONE", []string{"foo"}, &source.Options{}) if err == nil { t.Error("did not error with malformed metadata, no =") } - co, err = newCallout("foo", "NONE", []string{"foo="}, &source.Options{}) + _, err = newCallout("foo", "NONE", []string{"foo="}, &source.Options{}) if err == nil { t.Error("did not error with malformed metadata, no value") } - co, err = newCallout("foo", "NONE", []string{"=foo"}, &source.Options{}) + _, err = newCallout("foo", "NONE", []string{"=foo"}, &source.Options{}) if err == nil { t.Error("did not error with malformed metadata, no key") } - co, err = newCallout("foo", "NONE", []string{"="}, &source.Options{}) + _, err = newCallout("foo", "NONE", []string{"="}, &source.Options{}) if err == nil { t.Error("did not error with malformed metadata, no key or value") } diff --git a/galley/pkg/server/server.go b/galley/pkg/server/server.go index 96889cc530a4..90a3f760ffa9 100644 --- a/galley/pkg/server/server.go +++ b/galley/pkg/server/server.go @@ -157,8 +157,7 @@ func newServer(a *Args, p patchTable) (*Server, error) { grpcOptions = append(grpcOptions, grpc.MaxRecvMsgSize(int(a.MaxReceivedMessageSize))) s.stopCh = make(chan struct{}) - var checker server.AuthChecker - checker = server.NewAllowAllChecker() + var checker server.AuthChecker = server.NewAllowAllChecker() if !a.Insecure { checker, err = watchAccessList(s.stopCh, a.AccessListFile) if err != nil { diff --git a/galley/pkg/source/fs/source.go b/galley/pkg/source/fs/source.go index f573d0939b24..4917a84a8048 100644 --- a/galley/pkg/source/fs/source.go +++ b/galley/pkg/source/fs/source.go @@ -90,7 +90,7 @@ func (s *source) readFiles(root string) map[fileResourceKey]*fileResource { return err } result := s.readFile(path, info, true) - if result != nil && len(result) != 0 { + if len(result) != 0 { for _, r := range result { results[r.newKey()] = r } @@ -303,32 +303,25 @@ func (s *source) Start(handler resource.EventHandler) error { fi, err := os.Stat(ev.Name) if err != nil { log.Scope.Warnf("error occurs for watching %s", ev.Name) - } else { - if fi.Mode().IsDir() { - log.Scope.Debugf("add watcher for new folder %s", ev.Name) - _ = s.watcher.Watch(ev.Name) - } else { - newData := s.readFile(ev.Name, fi, true) - if newData != nil && len(newData) != 0 { - s.processAddOrUpdate(ev.Name, newData) - } - } + } else if fi.Mode().IsDir() { + log.Scope.Debugf("add watcher for new folder %s", ev.Name) + _ = s.watcher.Watch(ev.Name) + } else if newData := s.readFile(ev.Name, fi, true); len(newData) != 0 { + s.processAddOrUpdate(ev.Name, newData) } } else if ev.IsModify() { fi, err := os.Stat(ev.Name) if err != nil { log.Scope.Warnf("error occurs for watching %s", ev.Name) + } else if fi.Mode().IsDir() { + _ = s.watcher.RemoveWatch(ev.Name) } else { - if fi.Mode().IsDir() { - _ = s.watcher.RemoveWatch(ev.Name) + newData := s.readFile(ev.Name, fi, false) + if len(newData) != 0 { + s.processPartialDelete(ev.Name, newData) + s.processAddOrUpdate(ev.Name, newData) } else { - newData := s.readFile(ev.Name, fi, false) - if newData != nil && len(newData) != 0 { - s.processPartialDelete(ev.Name, newData) - s.processAddOrUpdate(ev.Name, newData) - } else { - s.processDelete(ev.Name) - } + s.processDelete(ev.Name) } } } diff --git a/galley/pkg/source/kube/dynamic/converter/converter_test.go b/galley/pkg/source/kube/dynamic/converter/converter_test.go index e96ec0e2c5d2..c931466103fd 100644 --- a/galley/pkg/source/kube/dynamic/converter/converter_test.go +++ b/galley/pkg/source/kube/dynamic/converter/converter_test.go @@ -527,8 +527,8 @@ func TestKubeIngressResource(t *testing.T) { func TestShouldProcessIngress(t *testing.T) { istio := model.DefaultMeshConfig().IngressClass cases := []struct { - ingressMode meshcfg.MeshConfig_IngressControllerMode ingressClass string + ingressMode meshcfg.MeshConfig_IngressControllerMode shouldProcess bool }{ {ingressMode: meshcfg.MeshConfig_DEFAULT, ingressClass: "nginx", shouldProcess: false}, diff --git a/galley/pkg/source/kube/schema/check/check_test.go b/galley/pkg/source/kube/schema/check/check_test.go index 59ff62050c23..990c8430eff4 100644 --- a/galley/pkg/source/kube/schema/check/check_test.go +++ b/galley/pkg/source/kube/schema/check/check_test.go @@ -119,7 +119,6 @@ func TestFindSupportedResourceSchemas(t *testing.T) { cases := []struct { name string missing map[int]bool - want map[int]bool }{ { name: "all present", diff --git a/istioctl/cmd/istioctl/handlers_test.go b/istioctl/cmd/istioctl/handlers_test.go index 3d4585ad1159..624144647b68 100644 --- a/istioctl/cmd/istioctl/handlers_test.go +++ b/istioctl/cmd/istioctl/handlers_test.go @@ -15,7 +15,6 @@ package main import ( - "fmt" "strings" "testing" ) @@ -47,7 +46,7 @@ func TestInferPodInfo(t *testing.T) { }, } for _, tt := range tests { - t.Run(fmt.Sprintf("%s", strings.Split(tt.proxyName, ".")[0]), func(t *testing.T) { + t.Run(strings.Split(tt.proxyName, ".")[0], func(t *testing.T) { gotPodName, gotNamespace := inferPodInfo(tt.proxyName, tt.namespace) if gotPodName != tt.wantPodName || gotNamespace != tt.wantNamespace { t.Errorf("unexpected podName and namespace: wanted %v %v got %v %v", tt.wantPodName, tt.wantNamespace, gotPodName, gotNamespace) diff --git a/istioctl/pkg/util/configdump/cluster_test.go b/istioctl/pkg/util/configdump/cluster_test.go index 10295fc79c23..e58ef1a80220 100644 --- a/istioctl/pkg/util/configdump/cluster_test.go +++ b/istioctl/pkg/util/configdump/cluster_test.go @@ -34,10 +34,10 @@ func setupWrapper(t *testing.T) *Wrapper { func TestWrapper_GetClusterConfigDump(t *testing.T) { tests := []struct { name string - noConfigs bool - noCluster bool wantVersion string wantStatic, wantDynamic int + noConfigs bool + noCluster bool wantErr bool }{ { @@ -89,9 +89,9 @@ func TestWrapper_GetClusterConfigDump(t *testing.T) { func TestWrapper_GetDynamicClusterDump(t *testing.T) { tests := []struct { name string + wantStatic, wantDynamic int noCluster bool stripVersion, wantVersion, wantLast bool - wantStatic, wantDynamic int wantErr bool }{ { diff --git a/istioctl/pkg/util/configdump/listener_test.go b/istioctl/pkg/util/configdump/listener_test.go index fefb8b1601c3..73bd830dccdd 100644 --- a/istioctl/pkg/util/configdump/listener_test.go +++ b/istioctl/pkg/util/configdump/listener_test.go @@ -23,10 +23,10 @@ import ( func TestWrapper_GetListenerConfigDump(t *testing.T) { tests := []struct { name string - noConfigs bool - noListener bool wantVersion string wantStatic, wantDynamic int + noConfigs bool + noListener bool wantErr bool }{ { @@ -79,9 +79,9 @@ func TestWrapper_GetListenerConfigDump(t *testing.T) { func TestWrapper_GetDynamicListenerDump(t *testing.T) { tests := []struct { name string + wantStatic, wantDynamic int noListener bool stripVersion, wantVersion, wantLast bool - wantStatic, wantDynamic int wantErr bool }{ { diff --git a/istioctl/pkg/util/configdump/route_test.go b/istioctl/pkg/util/configdump/route_test.go index 7b4ebb87cf63..2d508d7c135e 100644 --- a/istioctl/pkg/util/configdump/route_test.go +++ b/istioctl/pkg/util/configdump/route_test.go @@ -23,9 +23,9 @@ import ( func TestWrapper_GetRouteConfigDump(t *testing.T) { tests := []struct { name string + wantStatic, wantDynamic int noConfigs bool noRoute bool - wantStatic, wantDynamic int wantErr bool }{ { @@ -74,9 +74,9 @@ func TestWrapper_GetRouteConfigDump(t *testing.T) { func TestWrapper_GetDynamicRouteDump(t *testing.T) { tests := []struct { name string + wantStatic, wantDynamic int noRoute bool stripVersion, wantVersion, wantLast bool - wantStatic, wantDynamic int wantErr bool }{ { diff --git a/istioctl/pkg/validate/validate_test.go b/istioctl/pkg/validate/validate_test.go index 038070924afa..0592dcf86530 100644 --- a/istioctl/pkg/validate/validate_test.go +++ b/istioctl/pkg/validate/validate_test.go @@ -202,8 +202,6 @@ func TestValidateCommand(t *testing.T) { cases := []struct { name string args []string - stdin *bytes.Buffer - in []string wantError bool }{ { diff --git a/istioctl/pkg/writer/compare/cluster_test.go b/istioctl/pkg/writer/compare/cluster_test.go index 5e4f685b9056..bfde322e0c71 100644 --- a/istioctl/pkg/writer/compare/cluster_test.go +++ b/istioctl/pkg/writer/compare/cluster_test.go @@ -28,7 +28,6 @@ func TestComparator_ClusterDiff(t *testing.T) { envoy []byte pilot map[string][]byte wantClusterDump bool - wantMatch bool wantDiff string }{ { diff --git a/istioctl/pkg/writer/compare/listener_test.go b/istioctl/pkg/writer/compare/listener_test.go index 6f170b48f562..72f0f3d03042 100644 --- a/istioctl/pkg/writer/compare/listener_test.go +++ b/istioctl/pkg/writer/compare/listener_test.go @@ -27,9 +27,8 @@ func TestComparator_ListenerDiff(t *testing.T) { name string envoy []byte pilot map[string][]byte - wantListenerDump bool - wantMatch bool wantDiff string + wantListenerDump bool }{ { name: "prints a diff", diff --git a/istioctl/pkg/writer/compare/route_test.go b/istioctl/pkg/writer/compare/route_test.go index 27811c6e0394..ce15ec0796fe 100644 --- a/istioctl/pkg/writer/compare/route_test.go +++ b/istioctl/pkg/writer/compare/route_test.go @@ -28,9 +28,8 @@ func TestComparator_RouteDiff(t *testing.T) { name string envoy []byte pilot map[string][]byte - wantRouteDump bool - wantMatch bool wantDiff string + wantRouteDump bool }{ { name: "prints a diff", diff --git a/mixer/adapter/fluentd/fluentd_test.go b/mixer/adapter/fluentd/fluentd_test.go index b3d4e81365f3..be61e86b5f4f 100644 --- a/mixer/adapter/fluentd/fluentd_test.go +++ b/mixer/adapter/fluentd/fluentd_test.go @@ -232,14 +232,14 @@ func TestHandleLogEntry(t *testing.T) { cases := []struct { name string instances []*logentry.Instance - failWrites bool expected int - intDur bool maxBytes int64 + failWrites bool + intDur bool }{ { - "Basic Logs", - []*logentry.Instance{ + name: "Basic Logs", + instances: []*logentry.Instance{ { Name: "Foo", Severity: "WARNING", @@ -255,14 +255,12 @@ func TestHandleLogEntry(t *testing.T) { }, }, }, - false, - 2, - false, - 11, + expected: 2, + maxBytes: 11, }, { - "Complex Log", - []*logentry.Instance{ + name: "Complex Log", + instances: []*logentry.Instance{ { Name: "Foo", Severity: "WARNING", @@ -280,14 +278,12 @@ func TestHandleLogEntry(t *testing.T) { }, }, }, - false, - 1, - false, - 11, + expected: 1, + maxBytes: 11, }, { - "Integer Duration", - []*logentry.Instance{ + name: "Integer Duration", + instances: []*logentry.Instance{ { Name: "Foo", Severity: "WARNING", @@ -297,14 +293,13 @@ func TestHandleLogEntry(t *testing.T) { }, }, }, - false, - 1, - true, - 11, + expected: 1, + intDur: true, + maxBytes: 11, }, { - "Too Large Log", - []*logentry.Instance{ + name: "Too Large Log", + instances: []*logentry.Instance{ { Name: "Foo", Severity: "WARNING", @@ -324,10 +319,7 @@ func TestHandleLogEntry(t *testing.T) { }, }, }, - false, - 0, - false, - 2, + maxBytes: 2, }, } @@ -455,7 +447,7 @@ func (l *mockFluentd) EncodeData(tag string, ts time.Time, msg interface{}) ([]b func (l *mockFluentd) PostRawData(data []byte) { l.bytes.Write(data) l.mutex.Lock() - l.Batches = l.Batches + 1 + l.Batches++ l.mutex.Unlock() } diff --git a/mixer/adapter/prometheus/prometheus_test.go b/mixer/adapter/prometheus/prometheus_test.go index e34547ee2d83..e34d84b5ef4d 100644 --- a/mixer/adapter/prometheus/prometheus_test.go +++ b/mixer/adapter/prometheus/prometheus_test.go @@ -26,7 +26,6 @@ import ( "time" "github.com/prometheus/client_golang/prometheus" - "github.com/prometheus/client_golang/prometheus/promhttp" dto "github.com/prometheus/client_model/go" "istio.io/istio/mixer/adapter/prometheus/config" @@ -446,21 +445,20 @@ func TestProm_HandleMetrics(t *testing.T) { t.Errorf("HandleMetric() could not find metric with name %s:", adapterVal.Name) continue } - c := ci.c m := new(dto.Metric) - switch c.(type) { + switch c := ci.c.(type) { case *prometheus.CounterVec: - if err := c.(*prometheus.CounterVec).With(promLabels(adapterVal.Dimensions)).Write(m); err != nil { + if err := c.With(promLabels(adapterVal.Dimensions)).Write(m); err != nil { t.Errorf("Error writing metric value to proto: %v", err) continue } case *prometheus.GaugeVec: - if err := c.(*prometheus.GaugeVec).With(promLabels(adapterVal.Dimensions)).Write(m); err != nil { + if err := c.With(promLabels(adapterVal.Dimensions)).Write(m); err != nil { t.Errorf("Error writing metric value to proto: %v", err) continue } case *prometheus.HistogramVec: - if err := c.(*prometheus.HistogramVec).With(promLabels(adapterVal.Dimensions)).(prometheus.Metric).Write(m); err != nil { + if err := c.With(promLabels(adapterVal.Dimensions)).(prometheus.Metric).Write(m); err != nil { t.Errorf("Error writing metric value to proto: %v", err) continue } @@ -683,8 +681,7 @@ func expPolicy(expiry, check time.Duration) *config.Params_MetricsExpirationPoli func TestPromLogger_Println(t *testing.T) { testEnvLogger := test.NewEnv(t) - var underTest promhttp.Logger - underTest = &promLogger{logger: testEnvLogger} + underTest := &promLogger{logger: testEnvLogger} underTest.Println("Istio Service Mesh", 94.5, false) diff --git a/mixer/adapter/signalfx/signalfx.go b/mixer/adapter/signalfx/signalfx.go index 0eaa8751c2d8..8e7b50a65848 100644 --- a/mixer/adapter/signalfx/signalfx.go +++ b/mixer/adapter/signalfx/signalfx.go @@ -169,7 +169,7 @@ func (b *builder) Validate() (ce *adapter.ConfigErrors) { ce = ce.Appendf("tracing_sample_probability", "must be between 0.0 and 1.0 inclusive") } - if b.config.TracingBufferSize <= 0 { + if b.config.TracingBufferSize == 0 { ce = ce.Appendf("tracing_buffer_size", "must be greater than 0") } diff --git a/mixer/adapter/solarwinds/internal/appoptics/batching_test.go b/mixer/adapter/solarwinds/internal/appoptics/batching_test.go index 221b70a78c31..a46a74c3cded 100644 --- a/mixer/adapter/solarwinds/internal/appoptics/batching_test.go +++ b/mixer/adapter/solarwinds/internal/appoptics/batching_test.go @@ -101,9 +101,9 @@ func (s *MockServiceAccessor) MeasurementsService() MeasurementsCommunicator { func TestPersistBatches(t *testing.T) { tests := []struct { name string - expectedCount int32 response *http.Response error error + expectedCount int32 sendOnStopChan bool }{ { diff --git a/mixer/adapter/stackdriver/contextgraph/contextgraph_test.go b/mixer/adapter/stackdriver/contextgraph/contextgraph_test.go index 2f08c87b1786..1b9a49612a76 100644 --- a/mixer/adapter/stackdriver/contextgraph/contextgraph_test.go +++ b/mixer/adapter/stackdriver/contextgraph/contextgraph_test.go @@ -106,7 +106,7 @@ func TestBuild(t *testing.T) { mEnv := env.NewEnv(t) - han, err := b.Build(nil, mEnv) + han, err := b.Build(context.TODO(), mEnv) h := han.(*handler) if err != nil { t.Errorf("Build returned unexpected err: %v", err) @@ -162,7 +162,7 @@ func TestHandleEdge(t *testing.T) { Timestamp: time.Unix(1337, 0), }, } - if err := h.HandleEdge(nil, i); err != nil { + if err := h.HandleEdge(context.TODO(), i); err != nil { t.Errorf("HandleEdge returned unexpected err: %v", err) } ta := <-h.traffics diff --git a/mixer/adapter/stackdriver/log/log.go b/mixer/adapter/stackdriver/log/log.go index 795b59b55937..09c8e9c315b9 100644 --- a/mixer/adapter/stackdriver/log/log.go +++ b/mixer/adapter/stackdriver/log/log.go @@ -245,8 +245,7 @@ func toReq(mapping *config.Params_LogInfo_HttpRequestMapping, variables map[stri if variables[mapping.Method] != nil { method = variables[mapping.Method].(string) } - var httpHeaders http.Header - httpHeaders = make(http.Header) + httpHeaders := make(http.Header) if variables[mapping.UserAgent] != nil { httpHeaders.Add("User-Agent", variables[mapping.UserAgent].(string)) } diff --git a/mixer/adapter/stackdriver/metric/bufferedClient.go b/mixer/adapter/stackdriver/metric/bufferedClient.go index f3df3a2422a9..d00b73cfd13a 100644 --- a/mixer/adapter/stackdriver/metric/bufferedClient.go +++ b/mixer/adapter/stackdriver/metric/bufferedClient.go @@ -182,7 +182,7 @@ func (b *buffered) Close() error { // handleError extract out timeseries that fails to create from response status. // If no sepecific timeseries listed in error response, retry all time series in batch. func handleError(err error, tsSent []*monitoringpb.TimeSeries) []*monitoringpb.TimeSeries { - errorTS := make([]*monitoringpb.TimeSeries, 0, 0) + errorTS := make([]*monitoringpb.TimeSeries, 0) retryAll := true s, ok := status.FromError(err) if !ok { @@ -199,9 +199,7 @@ func handleError(err error, tsSent []*monitoringpb.TimeSeries) []*monitoringpb.T } } if isRetryable(status.Code(err)) && retryAll { - for _, ts := range tsSent { - errorTS = append(errorTS, ts) - } + errorTS = append(errorTS, tsSent...) } return errorTS } diff --git a/mixer/cmd/mixc/cmd/util.go b/mixer/cmd/mixc/cmd/util.go index e9fac80d844d..df7e3aa82da9 100644 --- a/mixer/cmd/mixc/cmd/util.go +++ b/mixer/cmd/mixc/cmd/util.go @@ -213,7 +213,7 @@ func parseAttributes(rootArgs *rootArgs) (*mixerpb.CompressedAttributes, []strin var attrs mixerpb.CompressedAttributes b.ToProto(&attrs, nil, 0) - dw := make([]string, len(gb), len(gb)) + dw := make([]string, len(gb)) for k, v := range gb { dw[v] = k } @@ -248,7 +248,7 @@ func decodeStatus(status rpc.Status) string { return result } -// nolint:deadcode +/* func dumpAttributes(printf, fatalf shared.FormatFn, attrs *mixerpb.CompressedAttributes) { if attrs == nil { return @@ -269,16 +269,17 @@ func dumpAttributes(printf, fatalf shared.FormatFn, attrs *mixerpb.CompressedAtt buf := bytes.Buffer{} tw := tabwriter.NewWriter(&buf, 0, 0, 2, ' ', 0) - fmt.Fprint(tw, " Attribute\tType\tValue\n") + _, _ = fmt.Fprint(tw, " Attribute\tType\tValue\n") for _, name := range names { v, _ := b.Get(name) - fmt.Fprintf(tw, " %s\t%T\t%v\n", name, v, v) + _, _ = fmt.Fprintf(tw, " %s\t%T\t%v\n", name, v, v) } _ = tw.Flush() printf("%s", buf.String()) } +*/ func dumpReferencedAttributes(printf shared.FormatFn, attrs *mixerpb.ReferencedAttributes) { vals := make([]string, 0, len(attrs.AttributeMatches)) diff --git a/mixer/pkg/api/grpcServer.go b/mixer/pkg/api/grpcServer.go index 68271d22ee6e..3ef6f8fa2be9 100644 --- a/mixer/pkg/api/grpcServer.go +++ b/mixer/pkg/api/grpcServer.go @@ -98,7 +98,7 @@ func (s *grpcServer) Check(ctx context.Context, req *mixerpb.CheckRequest) (*mix Code: value.StatusCode, Message: value.StatusMessage, }, - ValidDuration: value.Expiration.Sub(time.Now()), + ValidDuration: time.Until(value.Expiration), ValidUseCount: value.ValidUseCount, ReferencedAttributes: &value.ReferencedAttributes, RouteDirective: value.RouteDirective, @@ -276,14 +276,12 @@ func (s *grpcServer) Report(ctx context.Context, req *mixerpb.ReportRequest) (*m protoBag = attribute.GetProtoBag(&req.Attributes[i], s.globalDict, s.globalWordList) accumBag = attribute.GetMutableBag(protoBag) reportBag = attribute.GetMutableBag(accumBag) - } else { - if err := accumBag.UpdateBagFromProto(&req.Attributes[i], s.globalWordList); err != nil { - err = fmt.Errorf("request could not be processed due to invalid attributes: %v", err) - span.LogFields(otlog.String("error", err.Error())) - span.Finish() - errors = multierror.Append(errors, err) - break - } + } else if err := accumBag.UpdateBagFromProto(&req.Attributes[i], s.globalWordList); err != nil { + err = fmt.Errorf("request could not be processed due to invalid attributes: %v", err) + span.LogFields(otlog.String("error", err.Error())) + span.Finish() + errors = multierror.Append(errors, err) + break } if err := dispatchSingleReport(newctx, s.dispatcher, reporter, accumBag, reportBag); err != nil { span.LogFields(otlog.String("error", err.Error())) diff --git a/mixer/pkg/checkcache/cache.go b/mixer/pkg/checkcache/cache.go index 4da1430f4350..11b54bc48325 100644 --- a/mixer/pkg/checkcache/cache.go +++ b/mixer/pkg/checkcache/cache.go @@ -53,15 +53,15 @@ type Cache struct { // Value holds the data that the check cache stores. type Value struct { - // StatusCode for the Check operation - StatusCode int32 - // StatusMessage for the Check operation StatusMessage string // Expiration is the point at which this cache value becomes stale and shouldn't be used Expiration time.Time + // StatusCode for the Check operation + StatusCode int32 + // ValidUseCount for the Check operation ValidUseCount int32 diff --git a/mixer/pkg/checkcache/keyShape.go b/mixer/pkg/checkcache/keyShape.go index a8b1c8e40af4..f53f7ac74f0d 100644 --- a/mixer/pkg/checkcache/keyShape.go +++ b/mixer/pkg/checkcache/keyShape.go @@ -160,7 +160,7 @@ func (ks keyShape) checkPresentAttrs(attrs attribute.Bag) bool { // This function assumes that the bag has previously been deemed compatible via isCompatible. func (ks keyShape) makeKey(attrs attribute.Bag) string { buf := pool.GetBuffer() - b := make([]byte, 8, 8) + b := make([]byte, 8) for i := 0; i < len(ks.presentAttrs); i++ { name := ks.presentAttrs[i].name diff --git a/mixer/pkg/checkcache/keyShape_test.go b/mixer/pkg/checkcache/keyShape_test.go index 391da7c12dfc..67f919d9d883 100644 --- a/mixer/pkg/checkcache/keyShape_test.go +++ b/mixer/pkg/checkcache/keyShape_test.go @@ -31,7 +31,7 @@ type attrCase struct { } func TestKeyShape(t *testing.T) { - allKeys := make(map[string]struct{}, 0) + allKeys := make(map[string]struct{}) globalWords := attribute.GlobalList() cases := []struct { diff --git a/mixer/pkg/config/store/fsstore_test.go b/mixer/pkg/config/store/fsstore_test.go index e9db2f7da389..c69130ecb317 100644 --- a/mixer/pkg/config/store/fsstore_test.go +++ b/mixer/pkg/config/store/fsstore_test.go @@ -299,15 +299,6 @@ func TestFSStore2MissingRoot(t *testing.T) { func TestFSStore2Robust(t *testing.T) { const ns = "testing" - const tmpl = ` -kind: %s -apiVersion: config.istio.io/v1alpha2 -metadata: - namespace: testing - name: %s -spec: - %s -` for _, c := range []struct { title string prepare func(fsroot string) error diff --git a/mixer/pkg/config/store/listener_test.go b/mixer/pkg/config/store/listener_test.go index 1c6d39a21158..d7dd3211c8f3 100644 --- a/mixer/pkg/config/store/listener_test.go +++ b/mixer/pkg/config/store/listener_test.go @@ -119,19 +119,15 @@ func TestWatchChanges(t *testing.T) { } type mockStore struct { - // Init method related fields - initCalled bool - initKinds map[string]proto.Message - initErrorToReturn error - - // Watch method related fields - watchCalled bool + initKinds map[string]proto.Message + initErrorToReturn error watchChannelToReturn chan Event watchErrorToReturn error + listResultToReturn map[Key]*Resource - // List method related fields - listCalled bool - listResultToReturn map[Key]*Resource + initCalled bool + watchCalled bool + listCalled bool } var _ Store = &mockStore{} diff --git a/mixer/pkg/mockapi/server.go b/mixer/pkg/mockapi/server.go index ef2d85136f57..5fccd19b0a47 100644 --- a/mixer/pkg/mockapi/server.go +++ b/mixer/pkg/mockapi/server.go @@ -53,23 +53,23 @@ type AttributesServer struct { // GlobalDict controls the known global dictionary for attribute processing. GlobalDict map[string]int32 - // GenerateGRPCError instructs the server whether or not to fail-fast with - // an error that will manifest as a GRPC error. - GenerateGRPCError bool - // Handler is what the server will call to simulate passing attribute bags // and method args within the Mixer server. It allows tests to gain access // to the attribute handling pipeline within Mixer and to set the response // details. Handler AttributesHandler - // CheckGlobalDict indicates whether to check if proxy global dictionary - // is ahead of the one in mixer. - checkGlobalDict bool - // CheckMetadata indicates whether to check for presence of gRPC metadata for // forwarded attributes. checkMetadata func(*mixerpb.Attributes) error + + // GenerateGRPCError instructs the server whether or not to fail-fast with + // an error that will manifest as a GRPC error. + GenerateGRPCError bool + + // CheckGlobalDict indicates whether to check if proxy global dictionary + // is ahead of the one in mixer. + checkGlobalDict bool } // NewAttributesServer creates an AttributesServer. All channels are set to diff --git a/mixer/pkg/perf/client.go b/mixer/pkg/perf/client.go index 28315370c539..345b159a2a94 100644 --- a/mixer/pkg/perf/client.go +++ b/mixer/pkg/perf/client.go @@ -64,15 +64,15 @@ func (c *client) run(iterations int) (err error) { for i := 0; i < iterations; i++ { for _, r := range c.requests { - switch r.(type) { + switch r := r.(type) { case *istio_mixer_v1.ReportRequest: - _, e := c.mixer.Report(context.Background(), r.(*istio_mixer_v1.ReportRequest)) + _, e := c.mixer.Report(context.Background(), r) if e != nil && err == nil { err = e } case *istio_mixer_v1.CheckRequest: - _, e := c.mixer.Check(context.Background(), r.(*istio_mixer_v1.CheckRequest)) + _, e := c.mixer.Check(context.Background(), r) if e != nil && err == nil { err = e } diff --git a/mixer/pkg/protobuf/yaml/dynamic/auth.go b/mixer/pkg/protobuf/yaml/dynamic/auth.go index 9038fe2163ef..fb4ed9b1085d 100644 --- a/mixer/pkg/protobuf/yaml/dynamic/auth.go +++ b/mixer/pkg/protobuf/yaml/dynamic/auth.go @@ -93,10 +93,7 @@ func getWhitelistSAN(cert []byte) []*url.URL { if err != nil { return whitelistSAN } - for _, uri := range c.URIs { - whitelistSAN = append(whitelistSAN, uri) - } - return whitelistSAN + return append(whitelistSAN, c.URIs...) } var bypassVerificationVar = env.RegisterBoolVar("BYPASS_OOP_MTLS_SAN_VERIFICATION", false, "") @@ -271,7 +268,6 @@ func buildTLSDialOption(tlsCfg *policypb.Tls, skipVerify bool) ([]grpc.DialOptio func (a *authHelper) getAuthOpt() (opts []grpc.DialOption, err error) { // TODO(bianpengyuan) pass in grpc connection context so that oauth client would share the same context. if a.authCfg == nil { - opts = append(opts) return []grpc.DialOption{grpc.WithInsecure()}, nil } switch t := a.authCfg.AuthType.(type) { diff --git a/mixer/pkg/protobuf/yaml/encoder.go b/mixer/pkg/protobuf/yaml/encoder.go index 48ae755d0c37..3754e113b0e0 100644 --- a/mixer/pkg/protobuf/yaml/encoder.go +++ b/mixer/pkg/protobuf/yaml/encoder.go @@ -51,7 +51,7 @@ func (e *Encoder) EncodeBytes(data map[string]interface{}, msgName string, skipU } original := buf.Bytes() - result := make([]byte, len(original), len(original)) + result := make([]byte, len(original)) copy(result, original) return result, nil } diff --git a/mixer/pkg/runtime/dispatcher/dispatcher_test.go b/mixer/pkg/runtime/dispatcher/dispatcher_test.go index e6c1fe3054ee..9a682852c9ea 100644 --- a/mixer/pkg/runtime/dispatcher/dispatcher_test.go +++ b/mixer/pkg/runtime/dispatcher/dispatcher_test.go @@ -56,9 +56,6 @@ var tests = []struct { // attributes to use. If left empty, a default bag will be used. attr map[string]interface{} - // the variety of the operation to apply. - variety tpb.TemplateVariety - // quota method arguments to pass qma *QuotaMethodArgs @@ -75,6 +72,9 @@ var tests = []struct { // expected adapter/template log. log string + // the variety of the operation to apply. + variety tpb.TemplateVariety + // print out the full log for this test. Useful for debugging. fullLog bool }{ diff --git a/mixer/pkg/runtime/runtime_test.go b/mixer/pkg/runtime/runtime_test.go index f20352a684a2..647d8757aaaa 100644 --- a/mixer/pkg/runtime/runtime_test.go +++ b/mixer/pkg/runtime/runtime_test.go @@ -301,19 +301,15 @@ func TestRuntime_InFlightRequestsDuringConfigChange(t *testing.T) { } type mockStore struct { - // Init method related fields - initCalled bool - initKinds map[string]proto.Message - initErrorToReturn error - - // Watch method related fields - watchCalled bool + initKinds map[string]proto.Message + initErrorToReturn error watchChannelToReturn chan store.Event watchErrorToReturn error + listResultToReturn map[store.Key]*store.Resource - // List method related fields - listCalled bool - listResultToReturn map[store.Key]*store.Resource + initCalled bool + watchCalled bool + listCalled bool } var _ store.Store = &mockStore{} diff --git a/mixer/pkg/server/args.go b/mixer/pkg/server/args.go index ca4afd5d7302..61dab15c7f4e 100644 --- a/mixer/pkg/server/args.go +++ b/mixer/pkg/server/args.go @@ -80,15 +80,18 @@ type Args struct { // The introspection options to use IntrospectionOptions *ctrlz.Options - // Port to use for Mixer's gRPC API - APIPort uint16 - // Address to use for Mixer's gRPC API. This setting supercedes the API port setting. APIAddress string + // Port to use for Mixer's gRPC API + APIPort uint16 + // Port to use for exposing mixer self-monitoring information MonitoringPort uint16 + // Maximum number of entries in the check cache + NumCheckCacheEntries int32 + // Enable profiling via web interface host:port/debug/pprof EnableProfiling bool @@ -98,9 +101,6 @@ type Args struct { // If true, each request to Mixer will be executed in a single go routine (useful for debugging) SingleThreaded bool - // Maximum number of entries in the check cache - NumCheckCacheEntries int32 - // Whether or not to establish watches for adapter-specific CRDs UseAdapterCRDs bool @@ -131,11 +131,11 @@ func DefaultArgs() *Args { } func (a *Args) validate() error { - if a.MaxMessageSize <= 0 { + if a.MaxMessageSize == 0 { return fmt.Errorf("max message size must be > 0, got %d", a.MaxMessageSize) } - if a.MaxConcurrentStreams <= 0 { + if a.MaxConcurrentStreams == 0 { return fmt.Errorf("max concurrent streams must be > 0, got %d", a.MaxConcurrentStreams) } diff --git a/mixer/template/sample/template.gen_test.go b/mixer/template/sample/template.gen_test.go index c76d1ba907d2..29ef047cd16e 100644 --- a/mixer/template/sample/template.gen_test.go +++ b/mixer/template/sample/template.gen_test.go @@ -135,13 +135,6 @@ func (h *fakeQuotaHandler) SetQuotaTypes(t map[string]*sample_quota.Type) { func (h *fakeQuotaHandler) Validate() *adapter.ConfigErrors { return nil } func (h *fakeQuotaHandler) SetAdapterConfig(cfg adapter.Config) {} -type fakeBag struct{} - -func (f fakeBag) Get(name string) (value interface{}, found bool) { return nil, false } -func (f fakeBag) Names() []string { return []string{} } -func (f fakeBag) Done() {} -func (f fakeBag) String() string { return "" } - func TestGeneratedFields(t *testing.T) { for _, tst := range []struct { tmpl string @@ -1053,17 +1046,6 @@ attribute_bindings: } } -func InterfaceSlice(slice interface{}) []interface{} { - s := reflect.ValueOf(slice) - - ret := make([]interface{}, s.Len()) - for i := 0; i < s.Len(); i++ { - ret[i] = s.Index(i).Interface() - } - - return ret -} - // nolint: unparam func fillProto(cfg string, o interface{}) error { //data []byte, m map[string]interface{}, err error diff --git a/mixer/test/client/env/http_server.go b/mixer/test/client/env/http_server.go index 274937b0d7a2..ad8c0fdf538c 100644 --- a/mixer/test/client/env/http_server.go +++ b/mixer/test/client/env/http_server.go @@ -120,9 +120,7 @@ func (s *HTTPServer) handle(w http.ResponseWriter, r *http.Request) { reqHeaders[":authority"] = []string{r.Host} reqHeaders[":path"] = []string{r.URL.String()} for name, headers := range r.Header { - for _, h := range headers { - reqHeaders[name] = append(reqHeaders[name], h) - } + reqHeaders[name] = append(reqHeaders[name], headers...) } s.mu.Lock() diff --git a/mixer/test/spybackend/args.go b/mixer/test/spybackend/args.go index 309dc570986f..eb4ff19a228b 100644 --- a/mixer/test/spybackend/args.go +++ b/mixer/test/spybackend/args.go @@ -87,14 +87,14 @@ type ( HandleSampleApaSleep time.Duration // Auth - RequireTLS bool - RequireMTls bool HeaderKey string HeaderToken string CredsPath string KeyPath string CertPath string InsecureSkipVerification bool + RequireTLS bool + RequireMTls bool } // Requests record captured requests by the spy diff --git a/mixer/tools/adapterlinter/main.go b/mixer/tools/adapterlinter/main.go index aff470fbed18..ba0c275ab5ed 100644 --- a/mixer/tools/adapterlinter/main.go +++ b/mixer/tools/adapterlinter/main.go @@ -60,23 +60,21 @@ func main() { } func getReport(args []string) reports { - var reports reports + var reps reports if len(args) == 0 { - reports = doDir(".") + reps = doDir(".") } else { - reports = doAllDirs(args) + reps = doAllDirs(args) } - return reports + return reps } func doAllDirs(args []string) reports { - reports := make(reports, 0) + reps := make(reports, 0) for _, name := range args { - for _, r := range doDir(name) { - reports = append(reports, r) - } + reps = append(reps, doDir(name)...) } - return reports + return reps } func notests(info os.FileInfo) bool { diff --git a/mixer/tools/adapterlinter/main_test.go b/mixer/tools/adapterlinter/main_test.go index 351464e3e2dd..3c7c54d8e9e2 100644 --- a/mixer/tools/adapterlinter/main_test.go +++ b/mixer/tools/adapterlinter/main_test.go @@ -21,11 +21,11 @@ import ( ) func TestDoAllDirs(t *testing.T) { - reports := doAllDirs([]string{"testdata/bad"}) + reps := doAllDirs([]string{"testdata/bad"}) - got := make([]string, len(reports)) - for i := range reports { - got[i] = reports[i].msg + got := make([]string, len(reps)) + for i := range reps { + got[i] = reps[i].msg } sort.Strings(got) diff --git a/mixer/tools/mixgen/cmd/template_test.go b/mixer/tools/mixgen/cmd/template_test.go index ab20ab2a6784..b16c139f0800 100644 --- a/mixer/tools/mixgen/cmd/template_test.go +++ b/mixer/tools/mixgen/cmd/template_test.go @@ -50,9 +50,7 @@ spec: }, } { t.Run(fmt.Sprintf("%d", i), func(tt *testing.T) { - var args []string - - args = []string{"template", "-d", td.templateFile, "-n", "myTemplateResourceName", "--namespace", "mynamespace"} + args := []string{"template", "-d", td.templateFile, "-n", "myTemplateResourceName", "--namespace", "mynamespace"} gotCfg := "" root := GetRootCmd(args, diff --git a/pilot/cmd/pilot-agent/status/server.go b/pilot/cmd/pilot-agent/status/server.go index 8ec2ee9cd45d..c4c95dfdea15 100644 --- a/pilot/cmd/pilot-agent/status/server.go +++ b/pilot/cmd/pilot-agent/status/server.go @@ -67,11 +67,11 @@ type Config struct { // Server provides an endpoint for handling status probes. type Server struct { - statusPort uint16 ready *ready.Probe mutex sync.RWMutex - lastProbeSuccessful bool appKubeProbers KubeAppProbers + statusPort uint16 + lastProbeSuccessful bool } // NewServer creates a new status server. diff --git a/pilot/cmd/pilot-agent/status/server_test.go b/pilot/cmd/pilot-agent/status/server_test.go index ffe3b273022b..51ffa652b408 100644 --- a/pilot/cmd/pilot-agent/status/server_test.go +++ b/pilot/cmd/pilot-agent/status/server_test.go @@ -121,7 +121,6 @@ func TestAppProbe(t *testing.T) { testCases := []struct { probePath string statusCode int - err string }{ { probePath: fmt.Sprintf(":%v/bad-path-should-be-disallowed", statusPort), diff --git a/pilot/pkg/config/kube/ingress/conversion_test.go b/pilot/pkg/config/kube/ingress/conversion_test.go index 7eb3ea4f1207..a586c302bc54 100644 --- a/pilot/pkg/config/kube/ingress/conversion_test.go +++ b/pilot/pkg/config/kube/ingress/conversion_test.go @@ -181,8 +181,8 @@ func TestEncoding(t *testing.T) { func TestIngressClass(t *testing.T) { istio := model.DefaultMeshConfig().IngressClass cases := []struct { - ingressMode meshconfig.MeshConfig_IngressControllerMode ingressClass string + ingressMode meshconfig.MeshConfig_IngressControllerMode shouldProcess bool }{ {ingressMode: meshconfig.MeshConfig_DEFAULT, ingressClass: "nginx", shouldProcess: false}, diff --git a/pilot/pkg/config/kube/ingress/status_test.go b/pilot/pkg/config/kube/ingress/status_test.go index 776419ec495b..eb67c9a0e41a 100644 --- a/pilot/pkg/config/kube/ingress/status_test.go +++ b/pilot/pkg/config/kube/ingress/status_test.go @@ -149,8 +149,8 @@ func setAndRestoreEnv(t *testing.T, inputs map[string]string) map[string]string // representation correctly. func TestConvertIngressControllerMode(t *testing.T) { cases := []struct { - Mode meshconfig.MeshConfig_IngressControllerMode Annotation string + Mode meshconfig.MeshConfig_IngressControllerMode Ignore bool }{ { diff --git a/pilot/pkg/kube/inject/inject.go b/pilot/pkg/kube/inject/inject.go index 3713423c9322..0a67fe98b492 100644 --- a/pilot/pkg/kube/inject/inject.go +++ b/pilot/pkg/kube/inject/inject.go @@ -187,23 +187,14 @@ func ProxyImageName(hub string, tag string, debug bool) string { // Params describes configurable parameters for injecting istio proxy // into a kubernetes resource. type Params struct { - InitImage string `json:"initImage"` - RewriteAppHTTPProbe bool `json:"rewriteAppHTTPProbe"` - ProxyImage string `json:"proxyImage"` - Verbosity int `json:"verbosity"` - SidecarProxyUID uint64 `json:"sidecarProxyUID"` - Version string `json:"version"` - EnableCoreDump bool `json:"enableCoreDump"` - DebugMode bool `json:"debugMode"` - Privileged bool `json:"privileged"` - Mesh *meshconfig.MeshConfig `json:"-"` - ImagePullPolicy string `json:"imagePullPolicy"` - StatusPort int `json:"statusPort"` - ReadinessInitialDelaySeconds uint32 `json:"readinessInitialDelaySeconds"` - ReadinessPeriodSeconds uint32 `json:"readinessPeriodSeconds"` - ReadinessFailureThreshold uint32 `json:"readinessFailureThreshold"` - SDSEnabled bool `json:"sdsEnabled"` - EnableSdsTokenMount bool `json:"enableSdsTokenMount"` + InitImage string `json:"initImage"` + ProxyImage string `json:"proxyImage"` + Verbosity int `json:"verbosity"` + SidecarProxyUID uint64 `json:"sidecarProxyUID"` + Version string `json:"version"` + Mesh *meshconfig.MeshConfig `json:"-"` + ImagePullPolicy string `json:"imagePullPolicy"` + StatusPort int `json:"statusPort"` // Comma separated list of IP ranges in CIDR form. If set, only redirect outbound traffic to Envoy for these IP // ranges. All outbound traffic can be redirected with the wildcard character "*". Defaults to "*". IncludeIPRanges string `json:"includeIPRanges"` @@ -219,7 +210,16 @@ type Params struct { ExcludeInboundPorts string `json:"excludeInboundPorts"` // Comma separated list of virtual interfaces whose inbound traffic (from VM) will be treated as outbound // By default, no interfaces are configured. - KubevirtInterfaces string `json:"kubevirtInterfaces"` + KubevirtInterfaces string `json:"kubevirtInterfaces"` + ReadinessInitialDelaySeconds uint32 `json:"readinessInitialDelaySeconds"` + ReadinessPeriodSeconds uint32 `json:"readinessPeriodSeconds"` + ReadinessFailureThreshold uint32 `json:"readinessFailureThreshold"` + RewriteAppHTTPProbe bool `json:"rewriteAppHTTPProbe"` + EnableCoreDump bool `json:"enableCoreDump"` + DebugMode bool `json:"debugMode"` + Privileged bool `json:"privileged"` + SDSEnabled bool `json:"sdsEnabled"` + EnableSdsTokenMount bool `json:"enableSdsTokenMount"` } // Validate validates the parameters and returns an error if there is configuration issue. diff --git a/pilot/pkg/kube/inject/inject_test.go b/pilot/pkg/kube/inject/inject_test.go index 97a97f3f4853..0847ed4c0c16 100644 --- a/pilot/pkg/kube/inject/inject_test.go +++ b/pilot/pkg/kube/inject/inject_test.go @@ -64,13 +64,9 @@ func TestImageName(t *testing.T) { func TestIntoResourceFile(t *testing.T) { cases := []struct { - enableAuth bool in string want string imagePullPolicy string - enableCoreDump bool - debugMode bool - privileged bool duration time.Duration includeIPRanges string excludeIPRanges string @@ -78,10 +74,13 @@ func TestIntoResourceFile(t *testing.T) { excludeInboundPorts string kubevirtInterfaces string statusPort int - readinessPath string readinessInitialDelaySeconds uint32 readinessPeriodSeconds uint32 readinessFailureThreshold uint32 + enableAuth bool + enableCoreDump bool + debugMode bool + privileged bool tproxy bool }{ // "testdata/hello.yaml" is tested in http_test.go (with debug) @@ -570,9 +569,8 @@ func TestIntoResourceFile(t *testing.T) { // TestRewriteAppProbe tests the feature for pilot agent to take over app health check traffic. func TestRewriteAppProbe(t *testing.T) { cases := []struct { - in string - want string - template string + in string + want string }{ { in: "hello-probes.yaml", diff --git a/pilot/pkg/model/service.go b/pilot/pkg/model/service.go index dbdee773b387..1aeb1093e1b9 100644 --- a/pilot/pkg/model/service.go +++ b/pilot/pkg/model/service.go @@ -420,10 +420,6 @@ type IstioEndpoint struct { // Address is the address of the endpoint, using envoy proto. Address string - // EndpointPort is the port where the workload is listening, can be different - // from the service port. - EndpointPort uint32 - // ServicePortName tracks the name of the port, to avoid 'eventual consistency' issues. // Sometimes the Endpoint is visible before Service - so looking up the port number would // fail. Instead the mapping to number is made when the clusters are computed. The lazy @@ -447,6 +443,10 @@ type IstioEndpoint struct { // The locality where the endpoint is present. / separated string Locality string + // EndpointPort is the port where the workload is listening, can be different + // from the service port. + EndpointPort uint32 + // The load balancing weight associated with this endpoint. LbWeight uint32 } diff --git a/pilot/pkg/model/validation_test.go b/pilot/pkg/model/validation_test.go index bce7ca771867..2b9fd8d11214 100644 --- a/pilot/pkg/model/validation_test.go +++ b/pilot/pkg/model/validation_test.go @@ -548,10 +548,10 @@ func TestValidateMeshConfig(t *testing.T) { if err == nil { t.Errorf("expected an error on invalid proxy mesh config: %v", invalid) } else { - switch err.(type) { + switch err := err.(type) { case *multierror.Error: // each field must cause an error in the field - if len(err.(*multierror.Error).Errors) < 6 { + if len(err.Errors) < 6 { t.Errorf("expected an error for each field %v", err) } default: @@ -851,10 +851,10 @@ func TestValidateProxyConfig(t *testing.T) { if err == nil { t.Errorf("expected an error on invalid proxy mesh config: %v", invalid) } else { - switch err.(type) { + switch err := err.(type) { case *multierror.Error: // each field must cause an error in the field - if len(err.(*multierror.Error).Errors) != 12 { + if len(err.Errors) != 12 { t.Errorf("expected an error for each field %v", err) } default: diff --git a/pilot/pkg/networking/core/v1alpha3/cluster.go b/pilot/pkg/networking/core/v1alpha3/cluster.go index 1b4b5a56c893..7b3280f049d8 100644 --- a/pilot/pkg/networking/core/v1alpha3/cluster.go +++ b/pilot/pkg/networking/core/v1alpha3/cluster.go @@ -419,7 +419,7 @@ func (configgen *ConfigGeneratorImpl) buildInboundClusters(env *model.Environmen clusters = append(clusters, mgmtCluster) } } else { - if instances == nil || len(instances) == 0 { + if len(instances) == 0 { return clusters } rule := sidecarScope.Config.Spec.(*networking.Sidecar) @@ -581,7 +581,7 @@ func conditionallyConvertToIstioMtls( sniToUse = sni } subjectAltNamesToUse := tls.SubjectAltNames - if subjectAltNamesToUse == nil || len(subjectAltNamesToUse) == 0 { + if len(subjectAltNamesToUse) == 0 { subjectAltNamesToUse = serviceAccounts } return buildIstioMutualTLS(subjectAltNamesToUse, sniToUse, proxy) diff --git a/pilot/pkg/networking/core/v1alpha3/envoyfilter_test.go b/pilot/pkg/networking/core/v1alpha3/envoyfilter_test.go index 974ccc7bc37c..4c01295e438c 100644 --- a/pilot/pkg/networking/core/v1alpha3/envoyfilter_test.go +++ b/pilot/pkg/networking/core/v1alpha3/envoyfilter_test.go @@ -39,8 +39,8 @@ func TestListenerMatch(t *testing.T) { name string inputParams *plugin.InputParams listenerIP net.IP - direction networking.EnvoyFilter_ListenerMatch_ListenerType matchCondition *networking.EnvoyFilter_ListenerMatch + direction networking.EnvoyFilter_ListenerMatch_ListenerType result bool }{ { diff --git a/pilot/pkg/networking/core/v1alpha3/gateway.go b/pilot/pkg/networking/core/v1alpha3/gateway.go index 2e76f68b29ea..801f2c503034 100644 --- a/pilot/pkg/networking/core/v1alpha3/gateway.go +++ b/pilot/pkg/networking/core/v1alpha3/gateway.go @@ -642,7 +642,7 @@ func buildGatewayNetworkFiltersFromTLSRoutes(node *model.Proxy, env *model.Envir // Select the virtualService's hosts that match the ones specified in the gateway server's hosts // based on the wildcard hostname match and the namespace match func pickMatchingGatewayHosts(gatewayServerHosts map[model.Hostname]bool, virtualService model.Config) map[string]model.Hostname { - matchingHosts := make(map[string]model.Hostname, 0) + matchingHosts := make(map[string]model.Hostname) virtualServiceHosts := virtualService.Spec.(*networking.VirtualService).Hosts for _, vsvcHost := range virtualServiceHosts { for gatewayHost := range gatewayServerHosts { diff --git a/pilot/pkg/networking/core/v1alpha3/listener.go b/pilot/pkg/networking/core/v1alpha3/listener.go index 91399ec4d460..e26a3e65038c 100644 --- a/pilot/pkg/networking/core/v1alpha3/listener.go +++ b/pilot/pkg/networking/core/v1alpha3/listener.go @@ -875,11 +875,7 @@ func validatePort(node *model.Proxy, i int, bindToPort bool) bool { } proxyProcessUID := node.Metadata[model.NodeMetadataSidecarUID] - if proxyProcessUID == "0" { - return true - } - - return false + return proxyProcessUID == "0" } // buildSidecarOutboundListenerForPortOrUDS builds a single listener and @@ -1320,19 +1316,18 @@ func buildSidecarInboundMgmtListeners(node *model.Proxy, env *model.Environment, // httpListenerOpts are options for an HTTP listener type httpListenerOpts struct { - //nolint: maligned - routeConfig *xdsapi.RouteConfiguration - rds string - useRemoteAddress bool - direction http_conn.HttpConnectionManager_Tracing_OperationName + routeConfig *xdsapi.RouteConfiguration + rds string // If set, use this as a basis connectionManager *http_conn.HttpConnectionManager // stat prefix for the http connection manager // DO not set this field. Will be overridden by buildCompleteFilterChain statPrefix string + direction http_conn.HttpConnectionManager_Tracing_OperationName // addGRPCWebFilter specifies whether the envoy.grpc_web HTTP filter // should be added. addGRPCWebFilter bool + useRemoteAddress bool } // filterChainOpts describes a filter chain: a set of filters with the same TLS context @@ -1356,8 +1351,8 @@ type buildListenerOpts struct { proxyLabels model.LabelsCollection bind string port int - bindToPort bool filterChainOpts []*filterChainOpts + bindToPort bool skipUserFilters bool } diff --git a/pilot/pkg/networking/core/v1alpha3/route/route.go b/pilot/pkg/networking/core/v1alpha3/route/route.go index 3a6a3df2b8e2..812e9acdd02e 100644 --- a/pilot/pkg/networking/core/v1alpha3/route/route.go +++ b/pilot/pkg/networking/core/v1alpha3/route/route.go @@ -211,11 +211,10 @@ func GetDestinationCluster(destination *networking.Destination, service *model.S case *networking.PortSelector_Number: port = int(selector.Number) } - } else { + } else if service != nil && len(service.Ports) == 1 { // if service only has one port defined, use that as the port, otherwise use default listenerPort - if service != nil && len(service.Ports) == 1 { - port = service.Ports[0].Port - } + port = service.Ports[0].Port + // Do not return blackhole cluster for service==nil case as there is a legitimate use case for // calling this function with nil service: to route to a pre-defined statically configured cluster // declared as part of the bootstrap. @@ -459,9 +458,9 @@ func translateRoute(push *model.PushContext, node *model.Proxy, in *networking.H } if fault := in.Fault; fault != nil { if util.IsXDSMarshalingToAnyEnabled(node) { - out.TypedPerFilterConfig[xdsutil.Fault] = util.MessageToAny(translateFault(node, in.Fault)) + out.TypedPerFilterConfig[xdsutil.Fault] = util.MessageToAny(translateFault(in.Fault)) } else { - out.PerFilterConfig[xdsutil.Fault] = util.MessageToStruct(translateFault(node, in.Fault)) + out.PerFilterConfig[xdsutil.Fault] = util.MessageToStruct(translateFault(in.Fault)) } } @@ -669,7 +668,7 @@ func translateIntegerToFractionalPercent(p int32) *xdstype.FractionalPercent { } // translateFault translates networking.HTTPFaultInjection into Envoy's HTTPFault -func translateFault(node *model.Proxy, in *networking.HTTPFaultInjection) *xdshttpfault.HTTPFault { +func translateFault(in *networking.HTTPFaultInjection) *xdshttpfault.HTTPFault { if in == nil { return nil } @@ -677,18 +676,10 @@ func translateFault(node *model.Proxy, in *networking.HTTPFaultInjection) *xdsht out := xdshttpfault.HTTPFault{} if in.Delay != nil { out.Delay = &xdsfault.FaultDelay{Type: xdsfault.FaultDelay_FIXED} - if util.IsProxyVersionGE11(node) { - if in.Delay.Percentage != nil { - out.Delay.Percentage = translatePercentToFractionalPercent(in.Delay.Percentage) - } else { - out.Delay.Percentage = translateIntegerToFractionalPercent(in.Delay.Percent) - } + if in.Delay.Percentage != nil { + out.Delay.Percentage = translatePercentToFractionalPercent(in.Delay.Percentage) } else { - if in.Delay.Percentage != nil { - out.Delay.Percentage = translatePercentToFractionalPercent(in.Delay.Percentage) - } else { - out.Delay.Percentage = translateIntegerToFractionalPercent(in.Delay.Percent) - } + out.Delay.Percentage = translateIntegerToFractionalPercent(in.Delay.Percent) } switch d := in.Delay.HttpDelayType.(type) { case *networking.HTTPFaultInjection_Delay_FixedDelay: @@ -704,18 +695,10 @@ func translateFault(node *model.Proxy, in *networking.HTTPFaultInjection) *xdsht if in.Abort != nil { out.Abort = &xdshttpfault.FaultAbort{} - if util.IsProxyVersionGE11(node) { - if in.Abort.Percentage != nil { - out.Abort.Percentage = translatePercentToFractionalPercent(in.Abort.Percentage) - } else { - out.Abort.Percentage = translateIntegerToFractionalPercent(in.Abort.Percent) - } + if in.Abort.Percentage != nil { + out.Abort.Percentage = translatePercentToFractionalPercent(in.Abort.Percentage) } else { - if in.Abort.Percentage != nil { - out.Abort.Percentage = translatePercentToFractionalPercent(in.Abort.Percentage) - } else { - out.Abort.Percentage = translateIntegerToFractionalPercent(in.Abort.Percent) - } + out.Abort.Percentage = translateIntegerToFractionalPercent(in.Abort.Percent) } switch a := in.Abort.ErrorType.(type) { case *networking.HTTPFaultInjection_Abort_HttpStatus: diff --git a/pilot/pkg/request/command_test.go b/pilot/pkg/request/command_test.go index 866a2436efa2..ce73f2964bec 100644 --- a/pilot/pkg/request/command_test.go +++ b/pilot/pkg/request/command_test.go @@ -70,8 +70,8 @@ func Test_command_do(t *testing.T) { method string path string body string - pilotNotReachable bool pilotStates []pilotStubState + pilotNotReachable bool wantError bool }{ { diff --git a/pilot/pkg/serviceregistry/kube/controller.go b/pilot/pkg/serviceregistry/kube/controller.go index ec77febe1f0f..f4feee5800e1 100644 --- a/pilot/pkg/serviceregistry/kube/controller.go +++ b/pilot/pkg/serviceregistry/kube/controller.go @@ -305,19 +305,6 @@ func (c *Controller) GetService(hostname model.Hostname) (*model.Service, error) return c.servicesMap[hostname], nil } -// serviceByKey retrieves a service by name and namespace -func (c *Controller) serviceByKey(name, namespace string) (*v1.Service, bool) { - item, exists, err := c.services.informer.GetStore().GetByKey(KeyFunc(name, namespace)) - if err != nil { - log.Infof("serviceByKey(%s, %s) => error %v", name, namespace, err) - return nil, false - } - if !exists { - return nil, false - } - return item.(*v1.Service), true -} - // GetPodLocality retrieves the locality for a pod. func (c *Controller) GetPodLocality(pod *v1.Pod) string { // NodeName is set by the scheduler after the pod is created diff --git a/pilot/pkg/serviceregistry/kube/conversion.go b/pilot/pkg/serviceregistry/kube/conversion.go index 4c1dc3a58c8b..2f28a7b546f2 100644 --- a/pilot/pkg/serviceregistry/kube/conversion.go +++ b/pilot/pkg/serviceregistry/kube/conversion.go @@ -134,7 +134,7 @@ func externalNameServiceInstances(k8sSvc v1.Service, svc *model.Service) []*mode if k8sSvc.Spec.Type != v1.ServiceTypeExternalName || k8sSvc.Spec.ExternalName == "" { return nil } - var out []*model.ServiceInstance + out := make([]*model.ServiceInstance, 0, len(svc.Ports)) for _, portEntry := range svc.Ports { out = append(out, &model.ServiceInstance{ Endpoint: model.NetworkEndpoint{ diff --git a/pilot/pkg/serviceregistry/kube/pod.go b/pilot/pkg/serviceregistry/kube/pod.go index e58594a82caf..94135d7fd23d 100644 --- a/pilot/pkg/serviceregistry/kube/pod.go +++ b/pilot/pkg/serviceregistry/kube/pod.go @@ -45,9 +45,7 @@ func newPodCache(ch cacheHandler, c *Controller) *PodCache { keys: make(map[string]string), } - ch.handler.Append(func(obj interface{}, ev model.Event) error { - return out.event(obj, ev) - }) + ch.handler.Append(out.event) return out } diff --git a/pilot/pkg/serviceregistry/kube/queue_test.go b/pilot/pkg/serviceregistry/kube/queue_test.go index b2746b59d825..d3d47c30078e 100644 --- a/pilot/pkg/serviceregistry/kube/queue_test.go +++ b/pilot/pkg/serviceregistry/kube/queue_test.go @@ -30,7 +30,7 @@ func TestQueue(t *testing.T) { err := true add := func(obj interface{}, event model.Event) error { t.Logf("adding %d, error: %t", obj.(int), err) - out = out + obj.(int) + out += obj.(int) if out == 4 { close(done) } @@ -45,7 +45,7 @@ func TestQueue(t *testing.T) { q.Push(Task{handler: add, obj: 1}) q.Push(Task{handler: add, obj: 1}) q.Push(Task{handler: func(obj interface{}, event model.Event) error { - out = out + obj.(int) + out += obj.(int) if out != 3 { t.Errorf("Queue => %d, want %d", out, 3) } @@ -64,7 +64,7 @@ func TestChainedHandler(t *testing.T) { out := 0 f := func(i int) Handler { return func(obj interface{}, event model.Event) error { - out = out + i + out += i return nil } } diff --git a/pilot/tools/generate_config_crd_types.go b/pilot/tools/generate_config_crd_types.go index 757cf5b6dd95..17a1483c0eef 100644 --- a/pilot/tools/generate_config_crd_types.go +++ b/pilot/tools/generate_config_crd_types.go @@ -79,9 +79,7 @@ func main() { // Output if outputFile == nil || *outputFile == "" { fmt.Println(string(out)) - } else { - if err := ioutil.WriteFile(*outputFile, out, 0644); err != nil { - panic(err) - } + } else if err := ioutil.WriteFile(*outputFile, out, 0644); err != nil { + panic(err) } } diff --git a/pkg/adsc/adsc.go b/pkg/adsc/adsc.go index 4179be9d1f92..9de3344e4533 100644 --- a/pkg/adsc/adsc.go +++ b/pkg/adsc/adsc.go @@ -33,7 +33,7 @@ import ( xdsutil "github.com/envoyproxy/go-control-plane/pkg/util" "github.com/gogo/protobuf/jsonpb" "github.com/gogo/protobuf/proto" - types "github.com/gogo/protobuf/types" + "github.com/gogo/protobuf/types" "google.golang.org/grpc" "google.golang.org/grpc/credentials" ) @@ -257,13 +257,6 @@ func (a *ADSC) Run() error { return nil } -func (a *ADSC) update(m string) { - select { - case a.Updates <- m: - default: - } -} - func (a *ADSC) handleRecv() { for { msg, err := a.stream.Recv() @@ -326,7 +319,6 @@ func (a *ADSC) handleLDS(ll []*xdsapi.Listener) { lh := map[string]*xdsapi.Listener{} lt := map[string]*xdsapi.Listener{} - clusters := []string{} routes := []string{} ldsSize := 0 @@ -343,8 +335,7 @@ func (a *ADSC) handleLDS(ll []*xdsapi.Listener) { config, _ = xdsutil.MessageToStruct(f0.GetTypedConfig()) } c := config.Fields["cluster"].GetStringValue() - clusters = append(clusters, c) - //log.Printf("TCP: %s -> %s", l.Name, c) + log.Printf("TCP: %s -> %s", l.Name, c) } else if f0.Name == "envoy.http_connection_manager" { lh[l.Name] = l @@ -575,7 +566,6 @@ func (a *ADSC) handleRDS(configurations []*xdsapi.RouteConfiguration) { rcount := 0 size := 0 - httpClusters := []string{} rds := map[string]*xdsapi.RouteConfiguration{} for _, r := range configurations { @@ -585,7 +575,6 @@ func (a *ADSC) handleRDS(configurations []*xdsapi.RouteConfiguration) { rcount++ // Example: match: route: 0 { startupArgs = append(startupArgs, "--concurrency", fmt.Sprint(config.Concurrency)) diff --git a/pkg/bootstrap/platform/locality.go b/pkg/bootstrap/platform/locality.go index a6c30019ff0b..5713a25c4987 100644 --- a/pkg/bootstrap/platform/locality.go +++ b/pkg/bootstrap/platform/locality.go @@ -32,10 +32,7 @@ type Locality struct { // Converts a GCP zone into a region. func gcpZoneToRegion(z string) (string, error) { // Zones are in the form -, so capture everything but the suffix. - re, err := regexp.Compile("(.*)-.*") - if err != nil { - return "", err - } + re := regexp.MustCompile("(.*)-.*") m := re.FindStringSubmatch(z) if len(m) != 2 { return "", fmt.Errorf("unable to extract region from GCP zone: %s", z) diff --git a/pkg/ctrlz/ctrlz.go b/pkg/ctrlz/ctrlz.go index 2508dc289277..8bab067004fe 100644 --- a/pkg/ctrlz/ctrlz.go +++ b/pkg/ctrlz/ctrlz.go @@ -134,15 +134,8 @@ func Run(o *Options, customTopics []fw.Topic) (*Server, error) { } topicMutex.Lock() - - for _, t := range coreTopics { - allTopics = append(allTopics, t) - } - - for _, t := range customTopics { - allTopics = append(allTopics, t) - } - + allTopics = append(allTopics, coreTopics...) + allTopics = append(allTopics, customTopics...) topicMutex.Unlock() exec, _ := os.Executable() diff --git a/pkg/filewatcher/filewatcher_test.go b/pkg/filewatcher/filewatcher_test.go index 2408a4fb5e20..3f1e1b844948 100644 --- a/pkg/filewatcher/filewatcher_test.go +++ b/pkg/filewatcher/filewatcher_test.go @@ -134,10 +134,8 @@ func TestWatchFile(t *testing.T) { wg := sync.WaitGroup{} wg.Add(1) go func() { - select { - case <-events: - wg.Done() - } + <-events + wg.Done() }() // Overwriting the file and waiting its event to be received. @@ -163,10 +161,8 @@ func TestWatchFile(t *testing.T) { wg := sync.WaitGroup{} wg.Add(1) go func() { - select { - case <-events: - wg.Done() - } + <-events + wg.Done() }() // Link to another `test.conf` file @@ -200,10 +196,8 @@ func TestWatchFile(t *testing.T) { wg := sync.WaitGroup{} wg.Add(1) go func() { - select { - case <-events: - wg.Done() - } + <-events + wg.Done() }() // Overwriting the file and waiting its event to be received. diff --git a/pkg/mcp/creds/notifyWatcher_test.go b/pkg/mcp/creds/notifyWatcher_test.go index 17363a26efb7..6d57caf3f5d5 100644 --- a/pkg/mcp/creds/notifyWatcher_test.go +++ b/pkg/mcp/creds/notifyWatcher_test.go @@ -32,32 +32,6 @@ import ( "istio.io/istio/pkg/mcp/testing/testcerts" ) -type fakeWatcher struct { - events map[string]chan fsnotify.Event - errors map[string]chan error - added chan struct{} -} - -func (w *fakeWatcher) Add(path string) error { - if _, ok := w.events[path]; !ok { - w.events[path] = make(chan fsnotify.Event, 10) - } - if _, ok := w.errors[path]; !ok { - w.errors[path] = make(chan error, 10) - } - w.added <- struct{}{} - return nil -} - -func (w *fakeWatcher) Close() error { return nil } -func (w *fakeWatcher) Events(path string) chan fsnotify.Event { - return w.events[path] -} -func (w *fakeWatcher) Errors(path string) chan error { - return w.errors[path] -} -func (w *fakeWatcher) Remove(path string) error { panic("not supported") } - var ( certFile = "foo.pem" keyFile = "key.pem" diff --git a/pkg/mcp/creds/pollingWatcher_test.go b/pkg/mcp/creds/pollingWatcher_test.go index b392816a35e4..9a26b4b1a72b 100644 --- a/pkg/mcp/creds/pollingWatcher_test.go +++ b/pkg/mcp/creds/pollingWatcher_test.go @@ -415,7 +415,6 @@ func (f *watcherFixture) waitForNoPollError(t *testing.T) { if err != nil { t.Fatal(err) } - return } func (f *watcherFixture) newTmpFile(t *testing.T) string { diff --git a/pkg/mcp/server/listchecker_test.go b/pkg/mcp/server/listchecker_test.go index 1ea8ba30a4f3..989753dcb549 100644 --- a/pkg/mcp/server/listchecker_test.go +++ b/pkg/mcp/server/listchecker_test.go @@ -28,14 +28,13 @@ import ( func TestListAuthChecker(t *testing.T) { testCases := []struct { name string - mode AuthListMode authInfo credentials.AuthInfo extractIDsFn func(exts []pkix.Extension) ([]string, error) err string - remove bool // Remove the added entry - set bool // Use set to add the entry ids []string allowed []string + mode AuthListMode + remove bool // Remove the added entry }{ { name: "nil", @@ -224,10 +223,10 @@ func (a *authInfo) AuthType() string { func TestListAuthChecker_Allowed(t *testing.T) { cases := []struct { - mode AuthListMode id string testid string expect bool + mode AuthListMode }{ {mode: AuthBlackList, testid: "foo", expect: true}, {mode: AuthBlackList, id: "foo", testid: "foo", expect: false}, diff --git a/pkg/mcp/server/server_test.go b/pkg/mcp/server/server_test.go index c96fbbf32be2..eade622136c3 100644 --- a/pkg/mcp/server/server_test.go +++ b/pkg/mcp/server/server_test.go @@ -27,7 +27,6 @@ import ( "github.com/gogo/status" "google.golang.org/grpc" "google.golang.org/grpc/codes" - "google.golang.org/grpc/credentials" "google.golang.org/grpc/peer" mcp "istio.io/api/mcp/v1alpha1" @@ -219,14 +218,6 @@ func TestMultipleRequests(t *testing.T) { } } -type fakeAuthChecker struct { - err error -} - -func (f *fakeAuthChecker) Check(authInfo credentials.AuthInfo) error { - return f.err -} - func TestAuthCheck_Failure(t *testing.T) { config := makeMockConfigWatcher() config.setResponse(&source.WatchResponse{ @@ -670,7 +661,6 @@ func TestNACK(t *testing.T) { nonces := make(map[string]bool) var prevNonce string pushedVersion := "1" - numResponses := 0 first := true finish := time.Now().Add(1 * time.Second) for i := 0; ; i++ { @@ -700,7 +690,7 @@ func TestNACK(t *testing.T) { prevNonce = response.Nonce } else { select { - case response = <-stream.sent: + case <-stream.sent: t.Fatal("received unexpected response after NACK") case <-time.After(50 * time.Millisecond): } @@ -717,9 +707,4 @@ func TestNACK(t *testing.T) { break } } - wantResponses := 0 - if numResponses != wantResponses { - t.Fatalf("wrong number of responses: got %v want %v", - numResponses, wantResponses) - } } diff --git a/pkg/mcp/snapshot/inmemory.go b/pkg/mcp/snapshot/inmemory.go index 660bd4dfcbec..bfc169c82896 100644 --- a/pkg/mcp/snapshot/inmemory.go +++ b/pkg/mcp/snapshot/inmemory.go @@ -180,7 +180,7 @@ func (s *InMemory) Builder() *InMemoryBuilder { func (s *InMemory) String() string { var b bytes.Buffer - var messages []string + messages := make([]string, 0, len(s.resources)) for message := range s.resources { messages = append(messages, message) } diff --git a/pkg/mcp/source/source.go b/pkg/mcp/source/source.go index 5b5924e333cd..eac0b43545fe 100644 --- a/pkg/mcp/source/source.go +++ b/pkg/mcp/source/source.go @@ -200,7 +200,7 @@ func (s *Source) newConnection(stream Stream) *connection { queue: internal.NewUniqueScheduledQueue(len(s.collections)), } - var collections []string + collections := make([]string, 0, len(s.collections)) for i := range s.collections { collection := s.collections[i] w := &watch{ @@ -347,7 +347,7 @@ func (con *connection) pushServerResponse(w *watch, resp *WatchResponse) error { } // increment nonce - con.streamNonce = con.streamNonce + 1 + con.streamNonce++ msg.Nonce = strconv.FormatInt(con.streamNonce, 10) if err := con.stream.Send(msg); err != nil { con.reporter.RecordSendError(err, status.Code(err)) diff --git a/pkg/mcp/source/source_test.go b/pkg/mcp/source/source_test.go index c185954b3e12..d45c7f85d25e 100644 --- a/pkg/mcp/source/source_test.go +++ b/pkg/mcp/source/source_test.go @@ -60,11 +60,11 @@ type sourceTestHarness struct { recvErr error ctx context.Context nonce int - closeWatch bool watchResponses map[string]*WatchResponse pushResponseFuncs map[string][]PushResponseFunc watchCreated map[string]int client bool + closeWatch bool } func newSourceTestHarness(t *testing.T) *sourceTestHarness { diff --git a/pkg/test/framework/components/apps/apps.go b/pkg/test/framework/components/apps/apps.go index 02da6ed9871f..dc10216669fa 100644 --- a/pkg/test/framework/components/apps/apps.go +++ b/pkg/test/framework/components/apps/apps.go @@ -72,20 +72,20 @@ type Config struct { // AppCallOptions defines options for calling a DeployedAppEndpoint. type AppCallOptions struct { - // Secure indicates whether a secure connection should be established to the endpoint. - Secure bool - // Protocol indicates the protocol to be used. Protocol AppProtocol - // UseShortHostname indicates whether shortened hostnames should be used. This may be ignored by the environment. - UseShortHostname bool - // Count indicates the number of exchanges that should be made with the service endpoint. If not set (i.e. 0), defaults to 1. Count int // Headers indicates headers that should be sent in the request. Ingnored for WebSocket calls. Headers http.Header + + // Secure indicates whether a secure connection should be established to the endpoint. + Secure bool + + // UseShortHostname indicates whether shortened hostnames should be used. This may be ignored by the environment. + UseShortHostname bool } // AppEndpoint represents a single endpoint in a DeployedApp. diff --git a/pkg/test/framework/components/echo/instance.go b/pkg/test/framework/components/echo/instance.go index 8ea35f62fe8f..66b644b65006 100644 --- a/pkg/test/framework/components/echo/instance.go +++ b/pkg/test/framework/components/echo/instance.go @@ -118,20 +118,20 @@ func (c Config) String() string { // CallOptions defines options for calling a Endpoint. type CallOptions struct { - // Secure indicates whether a secure connection should be established to the endpoint. - Secure bool - // Protocol indicates the protocol to be used. Protocol Protocol - // UseShortHostname indicates whether shortened hostnames should be used. This may be ignored by the environment. - UseShortHostname bool - // Count indicates the number of exchanges that should be made with the service endpoint. If not set (i.e. 0), defaults to 1. Count int // Headers indicates headers that should be sent in the request. Ingnored for WebSocket calls. Headers http.Header + + // Secure indicates whether a secure connection should be established to the endpoint. + Secure bool + + // UseShortHostname indicates whether shortened hostnames should be used. This may be ignored by the environment. + UseShortHostname bool } // Endpoint represents a single endpoint in an Echo instance. diff --git a/pkg/test/framework/components/istio/deployment.go b/pkg/test/framework/components/istio/deployment.go index 6f7bf4246c20..76ceeb5687bf 100644 --- a/pkg/test/framework/components/istio/deployment.go +++ b/pkg/test/framework/components/istio/deployment.go @@ -80,10 +80,7 @@ func splitIstioYaml(istioYaml string) (string, string) { parts := strings.Split(istioYaml, yamlSeparator) - r, err := regexp.Compile("^apiVersion: *.*istio\\.io.*") - if err != nil { - panic(err) - } + r := regexp.MustCompile(`^apiVersion: *.*istio\.io.*`) for _, p := range parts { if r.Match([]byte(p)) { diff --git a/pkg/test/framework/components/mixer/client.go b/pkg/test/framework/components/mixer/client.go index 345ea084f422..bda659730604 100644 --- a/pkg/test/framework/components/mixer/client.go +++ b/pkg/test/framework/components/mixer/client.go @@ -94,7 +94,7 @@ func (c *client) Close() error { c.conns = make([]*grpc.ClientConn, 0) for _, fw := range c.forwarders { - fw.Close() + _ = fw.Close() } c.forwarders = make([]kube.PortForwarder, 0) @@ -116,40 +116,3 @@ func getAttrBag(attrs map[string]interface{}) istioMixerV1.CompressedAttributes requestBag.ToProto(&attrProto, nil, 0) return attrProto } - -func expandAttributeTemplates(evalFn func(string) (string, error), value interface{}) (interface{}, error) { - switch t := value.(type) { - case string: - return evalFn(t) - - case map[string]interface{}: - result := make(map[string]interface{}) - for k, v := range t { - // Expand key and string values. - k, err := evalFn(k) - if err != nil { - return nil, err - } - o, err := expandAttributeTemplates(evalFn, v) - if err != nil { - return nil, err - } - result[k] = o - } - return result, nil - - case []interface{}: - result := make([]interface{}, len(t)) - for i, v := range t { - o, err := expandAttributeTemplates(evalFn, v) - if err != nil { - return nil, err - } - result[i] = o - } - return result, nil - - default: - return value, nil - } -} diff --git a/pkg/test/framework/components/mixer/kube.go b/pkg/test/framework/components/mixer/kube.go index a3ac88f0f4e6..15479f35d766 100644 --- a/pkg/test/framework/components/mixer/kube.go +++ b/pkg/test/framework/components/mixer/kube.go @@ -90,7 +90,7 @@ func newKube(ctx resource.Context, _ Config) (*kubeComponent, error) { if err != nil { return nil, err } - if err := forwarder.Start(); err != nil { + if err = forwarder.Start(); err != nil { return nil, err } c.client.forwarders = append(c.client.forwarders, forwarder) diff --git a/pkg/test/framework/runtime/suitecontext.go b/pkg/test/framework/runtime/suitecontext.go index 0cd4923479e2..28c9bcfa9413 100644 --- a/pkg/test/framework/runtime/suitecontext.go +++ b/pkg/test/framework/runtime/suitecontext.go @@ -134,10 +134,6 @@ func (s *suiteContext) Skipf(reasonfmt string, args ...interface{}) { s.skipAll = true } -func (s *suiteContext) done() error { - return s.globalScope.done(s.settings.NoCleanup) -} - // CreateDirectory creates a new subdirectory within this context. func (s *suiteContext) CreateDirectory(name string) (string, error) { dir, err := ioutil.TempDir(s.settings.RunDir(), name) diff --git a/pkg/test/framework/runtime/testcontext.go b/pkg/test/framework/runtime/testcontext.go index 90445dd3cfad..67c38502ed96 100644 --- a/pkg/test/framework/runtime/testcontext.go +++ b/pkg/test/framework/runtime/testcontext.go @@ -146,10 +146,6 @@ func (c *testContext) RequireOrSkip(t *testing.T, envName environment.Name) { } } -func (c *testContext) newChild(t *testing.T) *testContext { - return newTestContext(c.suite, c.scope, t) -} - // Done should be called when this scope is cleaned up. func (c *testContext) Done(t *testing.T) { if t.Failed() { diff --git a/pkg/version/cobra.go b/pkg/version/cobra.go index 255f41227bc1..b5c72157e096 100644 --- a/pkg/version/cobra.go +++ b/pkg/version/cobra.go @@ -77,30 +77,32 @@ func CobraCommandWithOptions(options CobraOptions) *cobra.Command { case "": if short { if remoteVersion != nil { - fmt.Fprintf(cmd.OutOrStdout(), "client version: %s\n", version.ClientVersion.Version) + _, _ = fmt.Fprintf(cmd.OutOrStdout(), "client version: %s\n", version.ClientVersion.Version) for _, remote := range *remoteVersion { - fmt.Fprintf(cmd.OutOrStdout(), "%s version: %s\n", remote.Component, remote.Info.Version) + _, _ = fmt.Fprintf(cmd.OutOrStdout(), "%s version: %s\n", remote.Component, remote.Info.Version) } } else { - fmt.Fprintf(cmd.OutOrStdout(), "%s\n", version.ClientVersion.Version) + _, _ = fmt.Fprintf(cmd.OutOrStdout(), "%s\n", version.ClientVersion.Version) } } else { if remoteVersion != nil { - fmt.Fprintf(cmd.OutOrStdout(), "client version: %s\n", version.ClientVersion.LongForm()) + _, _ = fmt.Fprintf(cmd.OutOrStdout(), "client version: %s\n", version.ClientVersion.LongForm()) for _, remote := range *remoteVersion { - fmt.Fprintf(cmd.OutOrStdout(), "%s version: %s\n", remote.Component, remote.Info.LongForm()) + _, _ = fmt.Fprintf(cmd.OutOrStdout(), "%s version: %s\n", remote.Component, remote.Info.LongForm()) } } else { - fmt.Fprintf(cmd.OutOrStdout(), "%s\n", version.ClientVersion.LongForm()) + _, _ = fmt.Fprintf(cmd.OutOrStdout(), "%s\n", version.ClientVersion.LongForm()) } } case "yaml": - marshalled, _ := yaml.Marshal(&version) - fmt.Fprintln(cmd.OutOrStdout(), string(marshalled)) + if marshalled, err := yaml.Marshal(&version); err == nil { + _, _ = fmt.Fprintln(cmd.OutOrStdout(), string(marshalled)) + } case "json": - marshalled, _ := json.MarshalIndent(&version, "", " ") - fmt.Fprintln(cmd.OutOrStdout(), string(marshalled)) + if marshalled, err := json.MarshalIndent(&version, "", " "); err == nil { + _, _ = fmt.Fprintln(cmd.OutOrStdout(), string(marshalled)) + } } return serverErr diff --git a/pkg/version/stats.go b/pkg/version/stats.go index df35768c288b..8ebf43b7741b 100644 --- a/pkg/version/stats.go +++ b/pkg/version/stats.go @@ -16,8 +16,5 @@ package version -// RecordComponentBuildTag sets the value for a metric that will be used to track component build tags for -// tracking rollouts, etc. func (b BuildInfo) RecordComponentBuildTag(component string) { - // only for Linux } diff --git a/pkg/version/version_linux_test.go b/pkg/version/version_linux_test.go new file mode 100644 index 000000000000..329c6ea3af55 --- /dev/null +++ b/pkg/version/version_linux_test.go @@ -0,0 +1,125 @@ +// Copyright 2017 Istio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package version + +import ( + "context" + "errors" + "testing" + + "go.opencensus.io/stats/view" + "go.opencensus.io/tag" +) + +func TestRecordComponentBuildTag(t *testing.T) { + cases := []struct { + name string + in BuildInfo + wantTag string + }{ + {"record", BuildInfo{ + Version: "VER", + GitRevision: "GITREV", + Host: "HOST", + GolangVersion: "GOLANGVER", + DockerHub: "DH", + User: "USER", + BuildStatus: "STATUS", + GitTag: "1.0.5-test"}, + "1.0.5-test", + }, + } + + for _, v := range cases { + t.Run(v.name, func(tt *testing.T) { + v.in.RecordComponentBuildTag("test") + + d1, _ := view.RetrieveData("istio/build") + gauge := d1[0].Data.(*view.LastValueData) + if got, want := gauge.Value, 1.0; got != want { + tt.Errorf("bad value for build tag gauge: got %f, want %f", got, want) + } + + for _, tag := range d1[0].Tags { + if tag.Key == gitTagKey && tag.Value == v.wantTag { + return + } + } + + tt.Errorf("build tag not found for metric: %#v", d1) + }) + } +} + +func TestRecordComponentBuildTagError(t *testing.T) { + bi := BuildInfo{ + Version: "VER", + GitRevision: "GITREV", + Host: "HOST", + GolangVersion: "GOLANGVER", + DockerHub: "DH", + User: "USER", + BuildStatus: "STATUS", + GitTag: "TAG", + } + + bi.recordBuildTag("failure", func(context.Context, ...tag.Mutator) (context.Context, error) { + return context.Background(), errors.New("error") + }) + + d1, _ := view.RetrieveData("istio/build") + for _, data := range d1 { + for _, tag := range data.Tags { + if tag.Key.Name() == "component" && tag.Value == "failure" { + t.Errorf("a value was recorded for the failure component unexpectedly") + } + } + } +} + +func TestRegisterStatsPanics(t *testing.T) { + cases := []struct { + name string + newTagKeyFn func(string) (tag.Key, error) + }{ + {"tag", func(n string) (tag.Key, error) { + if n == "tag" { + return tag.Key{}, errors.New("failure") + } + return tag.NewKey(n) + }, + }, + {"component", func(n string) (tag.Key, error) { + if n == "component" { + return tag.Key{}, errors.New("failure") + } + return tag.NewKey(n) + }, + }, + {"duplicate registration", tag.NewKey}, + } + + for _, v := range cases { + t.Run(v.name, func(tt *testing.T) { + defer func() { + if r := recover(); r == nil { + tt.Fatalf("expected panic!") + } + }() + + registerStats(v.newTagKeyFn) + }) + } +} diff --git a/pkg/version/version_test.go b/pkg/version/version_test.go index 0db2707de34e..c0563b61a323 100644 --- a/pkg/version/version_test.go +++ b/pkg/version/version_test.go @@ -15,14 +15,9 @@ package version import ( - "context" - "errors" "fmt" "runtime" "testing" - - "go.opencensus.io/stats/view" - "go.opencensus.io/tag" ) func TestNewBuildInfoFromOldString(t *testing.T) { @@ -120,104 +115,3 @@ func TestBuildInfo(t *testing.T) { }) } } - -func TestRecordComponentBuildTag(t *testing.T) { - cases := []struct { - name string - in BuildInfo - wantTag string - }{ - {"record", BuildInfo{ - Version: "VER", - GitRevision: "GITREV", - Host: "HOST", - GolangVersion: "GOLANGVER", - DockerHub: "DH", - User: "USER", - BuildStatus: "STATUS", - GitTag: "1.0.5-test"}, - "1.0.5-test", - }, - } - - for _, v := range cases { - t.Run(v.name, func(tt *testing.T) { - v.in.RecordComponentBuildTag("test") - - d1, _ := view.RetrieveData("istio/build") - gauge := d1[0].Data.(*view.LastValueData) - if got, want := gauge.Value, 1.0; got != want { - tt.Errorf("bad value for build tag gauge: got %f, want %f", got, want) - } - - for _, tag := range d1[0].Tags { - if tag.Key == gitTagKey && tag.Value == v.wantTag { - return - } - } - - tt.Errorf("build tag not found for metric: %#v", d1) - }) - } -} - -func TestRecordComponentBuildTagError(t *testing.T) { - bi := BuildInfo{ - Version: "VER", - GitRevision: "GITREV", - Host: "HOST", - GolangVersion: "GOLANGVER", - DockerHub: "DH", - User: "USER", - BuildStatus: "STATUS", - GitTag: "TAG", - } - - bi.recordBuildTag("failure", func(context.Context, ...tag.Mutator) (context.Context, error) { - return context.Background(), errors.New("error") - }) - - d1, _ := view.RetrieveData("istio/build") - for _, data := range d1 { - for _, tag := range data.Tags { - if tag.Key.Name() == "component" && tag.Value == "failure" { - t.Errorf("a value was recorded for the failure component unexpectedly") - } - } - } -} - -func TestRegisterStatsPanics(t *testing.T) { - cases := []struct { - name string - newTagKeyFn func(string) (tag.Key, error) - }{ - {"tag", func(n string) (tag.Key, error) { - if n == "tag" { - return tag.Key{}, errors.New("failure") - } - return tag.NewKey(n) - }, - }, - {"component", func(n string) (tag.Key, error) { - if n == "component" { - return tag.Key{}, errors.New("failure") - } - return tag.NewKey(n) - }, - }, - {"duplicate registration", tag.NewKey}, - } - - for _, v := range cases { - t.Run(v.name, func(tt *testing.T) { - defer func() { - if r := recover(); r == nil { - tt.Fatalf("expected panic!") - } - }() - - registerStats(v.newTagKeyFn) - }) - } -} diff --git a/security/pkg/adapter/vault/ca.go b/security/pkg/adapter/vault/ca.go index fe62e8a3edee..f17aa8b00391 100644 --- a/security/pkg/adapter/vault/ca.go +++ b/security/pkg/adapter/vault/ca.go @@ -182,7 +182,7 @@ func RunProtoTypeSignCsrFlow() error { log.Errorf("ReadFile() failed (error %v)", err) return err } - _, err = setCaKeyCert(client, configCaKeyCertPath, string(keyCert[:])) + _, err = setCaKeyCert(client, configCaKeyCertPath, string(keyCert)) if err != nil { log.Errorf("setCaKeyCert() failed (error %v)", err) return err @@ -199,7 +199,7 @@ func RunProtoTypeSignCsrFlow() error { log.Errorf("ReadFile() failed (error %v)", err) return err } - res, err := signCsr(client, signCsrPath, string(testCsr[:])) + res, err := signCsr(client, signCsrPath, string(testCsr)) if err != nil { log.Errorf("signCsr() failed (error %v)", err) return err diff --git a/security/pkg/k8s/controller/workloadsecret_test.go b/security/pkg/k8s/controller/workloadsecret_test.go index 15a6c6558830..197ea12d5e46 100644 --- a/security/pkg/k8s/controller/workloadsecret_test.go +++ b/security/pkg/k8s/controller/workloadsecret_test.go @@ -185,8 +185,8 @@ func TestSecretContent(t *testing.T) { } controller.saAdded(createServiceAccount(saName, saNamespace)) - secret := ca.BuildSecret(saName, GetSecretName(saName), saNamespace, nil, nil, nil, nil, nil, IstioSecretType) - secret, err = client.CoreV1().Secrets(saNamespace).Get(GetSecretName(saName), metav1.GetOptions{}) + _ = ca.BuildSecret(saName, GetSecretName(saName), saNamespace, nil, nil, nil, nil, nil, IstioSecretType) + secret, err := client.CoreV1().Secrets(saNamespace).Get(GetSecretName(saName), metav1.GetOptions{}) if err != nil { t.Errorf("Failed to retrieve secret: %v", err) } @@ -261,9 +261,9 @@ func TestUpdateSecret(t *testing.T) { testCases := map[string]struct { expectedActions []ktesting.Action ttl time.Duration - gracePeriodRatio float32 minGracePeriod time.Duration rootCert []byte + gracePeriodRatio float32 certIsInvalid bool }{ "Does not update non-expiring secret": { diff --git a/security/pkg/k8s/tokenreview/k8sauthn_test.go b/security/pkg/k8s/tokenreview/k8sauthn_test.go index 0ce6516c4e8a..d2d76c883037 100644 --- a/security/pkg/k8s/tokenreview/k8sauthn_test.go +++ b/security/pkg/k8s/tokenreview/k8sauthn_test.go @@ -207,7 +207,7 @@ func newMockAPIServer(t *testing.T, apiPath, reviewerToken string) *mockAPIServe resp.Header().Set("Content-Type", "application/json") resp.Write(resultJSON) } - break + default: t.Logf("The request contains invalid path: %v", req.URL.Path) result := &k8sauth.TokenReview{ diff --git a/security/pkg/listwatch/listwatch.go b/security/pkg/listwatch/listwatch.go index e067d7e5b6cb..c5f681cf1812 100644 --- a/security/pkg/listwatch/listwatch.go +++ b/security/pkg/listwatch/listwatch.go @@ -37,7 +37,7 @@ func MultiNamespaceListerWatcher(namespaces []string, f func(string) cache.Liste if len(namespaces) == 1 { return f(namespaces[0]) } - var lws []cache.ListerWatcher + lws := make([]cache.ListerWatcher, 0, len(namespaces)) for _, n := range namespaces { lws = append(lws, f(n)) } @@ -53,7 +53,7 @@ type multiListerWatcher []cache.ListerWatcher // a single result. func (mlw multiListerWatcher) List(options metav1.ListOptions) (runtime.Object, error) { l := metav1.List{} - var resourceVersions []string + resourceVersions := make([]string, 0, len(mlw)) for _, lw := range mlw { list, err := lw.List(options) if err != nil { @@ -109,7 +109,7 @@ func newMultiWatch(lws []cache.ListerWatcher, resourceVersions []string, options var ( result = make(chan watch.Event) stopped = make(chan struct{}) - stoppers []func() + stoppers = make([]func(), 0, len(lws)) wg sync.WaitGroup ) @@ -175,5 +175,4 @@ func (mw *multiWatch) Stop() { } close(mw.stopped) } - return } diff --git a/security/pkg/nodeagent/cache/secretcache.go b/security/pkg/nodeagent/cache/secretcache.go index 6ab01275651c..fd28f48168cc 100644 --- a/security/pkg/nodeagent/cache/secretcache.go +++ b/security/pkg/nodeagent/cache/secretcache.go @@ -428,7 +428,7 @@ func (sc *SecretCache) rotate() { func (sc *SecretCache) generateSecret(ctx context.Context, token, resourceName string, t time.Time) (*model.SecretItem, error) { // If node agent works as ingress gateway agent, searches for kubernetes secret instead of sending // CSR to CA. - if sc.fetcher.UseCaClient == false { + if !sc.fetcher.UseCaClient { secretItem, exist := sc.fetcher.FindIngressGatewaySecret(resourceName) if !exist { return nil, fmt.Errorf("cannot find secret %s for ingress gateway", resourceName) @@ -489,7 +489,7 @@ func (sc *SecretCache) generateSecret(ctx context.Context, token, resourceName s retry := 0 backOffInMilliSec := initialBackOffIntervalInMilliSec var certChainPEM []string - for true { + for { certChainPEM, err = sc.fetcher.CaClient.CSRSign(ctx, csrPEM, exchangedToken, int64(sc.configOptions.SecretTTL.Seconds())) if err == nil { break @@ -523,7 +523,7 @@ func (sc *SecretCache) generateSecret(ctx context.Context, token, resourceName s len := len(certChainPEM) // Leaf cert is element '0'. Root cert is element 'n'. - if sc.rootCert == nil || bytes.Compare(sc.rootCert, []byte(certChainPEM[len-1])) != 0 { + if sc.rootCert == nil || !bytes.Equal(sc.rootCert, []byte(certChainPEM[len-1])) { sc.rootCertMutex.Lock() sc.rootCert = []byte(certChainPEM[len-1]) sc.rootCertMutex.Unlock() diff --git a/security/pkg/nodeagent/cache/secretcache_test.go b/security/pkg/nodeagent/cache/secretcache_test.go index 009e4a4076a0..1736f78af1c3 100644 --- a/security/pkg/nodeagent/cache/secretcache_test.go +++ b/security/pkg/nodeagent/cache/secretcache_test.go @@ -99,7 +99,7 @@ func TestWorkloadAgentGenerateSecret(t *testing.T) { t.Fatalf("Failed to get secrets: %v", err) } - if got, want := gotSecret.CertificateChain, convertToBytes(mockCertChain1st); bytes.Compare(got, want) != 0 { + if got, want := gotSecret.CertificateChain, convertToBytes(mockCertChain1st); !bytes.Equal(got, want) { t.Errorf("CertificateChain: got: %v, want: %v", got, want) } @@ -114,7 +114,7 @@ func TestWorkloadAgentGenerateSecret(t *testing.T) { if err != nil { t.Fatalf("Failed to get secrets: %v", err) } - if got, want := gotSecretRoot.RootCert, []byte("rootcert"); bytes.Compare(got, want) != 0 { + if got, want := gotSecretRoot.RootCert, []byte("rootcert"); !bytes.Equal(got, want) { t.Errorf("CertificateChain: got: %v, want: %v", got, want) } @@ -142,7 +142,7 @@ func TestWorkloadAgentGenerateSecret(t *testing.T) { if err != nil { t.Fatalf("Failed to get secrets: %v", err) } - if got, want := gotSecret.CertificateChain, convertToBytes(mockCertChainRemain); bytes.Compare(got, want) != 0 { + if got, want := gotSecret.CertificateChain, convertToBytes(mockCertChainRemain); !bytes.Equal(got, want) { t.Errorf("CertificateChain: got: %v, want: %v", got, want) } diff --git a/security/pkg/nodeagent/caclient/client_test.go b/security/pkg/nodeagent/caclient/client_test.go index 26b42e099e74..178cb49cbe1c 100644 --- a/security/pkg/nodeagent/caclient/client_test.go +++ b/security/pkg/nodeagent/caclient/client_test.go @@ -89,7 +89,7 @@ func TestGetCATLSRootCertFromConfigMap(t *testing.T) { } else { if tc.expectedErr != "" { t.Errorf("Test case [%s]: expect error: %s", id, tc.expectedErr) - } else if bytes.Compare(cert, tc.expectedCert) != 0 { + } else if !bytes.Equal(cert, tc.expectedCert) { t.Errorf("Test case [%s]: cert from ConfigMap %v does not match expected value %v", id, cert, tc.expectedCert) } } diff --git a/security/pkg/nodeagent/caclient/providers/vault/client.go b/security/pkg/nodeagent/caclient/providers/vault/client.go index b83f34173fd5..06d1b71f59b0 100644 --- a/security/pkg/nodeagent/caclient/providers/vault/client.go +++ b/security/pkg/nodeagent/caclient/providers/vault/client.go @@ -113,7 +113,7 @@ func createVaultTLSClient(vaultAddr string, tlsRootCert []byte) (*api.Client, er log.Errorf("could not get SystemCertPool: %v", err) return nil, fmt.Errorf("could not get SystemCertPool: %v", err) } - if tlsRootCert != nil && len(tlsRootCert) > 0 { + if len(tlsRootCert) > 0 { ok := pool.AppendCertsFromPEM(tlsRootCert) if !ok { return nil, fmt.Errorf("failed to append a certificate (%v) to the certificate pool", string(tlsRootCert[:])) diff --git a/security/pkg/nodeagent/caclient/providers/vault/client_test.go b/security/pkg/nodeagent/caclient/providers/vault/client_test.go index 27a05c46ac1f..0b17a07c01f6 100644 --- a/security/pkg/nodeagent/caclient/providers/vault/client_test.go +++ b/security/pkg/nodeagent/caclient/providers/vault/client_test.go @@ -258,8 +258,7 @@ func TestClientOnExampleHttpVaultCA(t *testing.T) { } for id, tc := range testCases { - var vaultAddr string - vaultAddr = tc.cliConfig.vaultAddr + vaultAddr := tc.cliConfig.vaultAddr cli, err := NewVaultClient(false, []byte{}, vaultAddr, tc.cliConfig.vaultLoginRole, tc.cliConfig.vaultLoginPath, tc.cliConfig.vaultSignCsrPath) if err != nil { @@ -289,8 +288,7 @@ func TestClientOnExampleHttpsVaultCA(t *testing.T) { } for id, tc := range testCases { - var vaultAddr string - vaultAddr = tc.cliConfig.vaultAddr + vaultAddr := tc.cliConfig.vaultAddr cli, err := NewVaultClient(true, []byte(vaultServerTLSCert), vaultAddr, tc.cliConfig.vaultLoginRole, tc.cliConfig.vaultLoginPath, tc.cliConfig.vaultSignCsrPath) if err != nil { @@ -348,7 +346,7 @@ func newMockVaultServer(t *testing.T, tls bool, loginRole, token, loginResp, sig } resp.Header().Set("Content-Type", "application/json") resp.Write([]byte(vaultServer.vaultLoginResp)) - break + case "/v1/sign": t.Logf("%v", req.URL) if req.Header.Get(vaultAuthHeaderName) != "fake-vault-token" { @@ -381,7 +379,7 @@ func newMockVaultServer(t *testing.T, tls bool, loginRole, token, loginResp, sig } resp.Header().Set("Content-Type", "application/json") resp.Write([]byte(vaultServer.vaultSignResp)) - break + default: t.Logf("The request contains invalid path: %v", req.URL) resp.WriteHeader(http.StatusNotFound) diff --git a/security/pkg/nodeagent/sds/server.go b/security/pkg/nodeagent/sds/server.go index f43409e689e4..467ced7f818d 100644 --- a/security/pkg/nodeagent/sds/server.go +++ b/security/pkg/nodeagent/sds/server.go @@ -32,13 +32,9 @@ const maxStreams = 100000 // Options provides all of the configuration parameters for secret discovery service. type Options struct { - // EnableWorkloadSDS indicates whether node agent works as SDS server for workload proxies. - EnableWorkloadSDS bool // WorkloadUDSPath is the unix domain socket through which SDS server communicates with workload proxies. WorkloadUDSPath string - // EnableIngressGatewaySDS indicates whether node agent works as ingress gateway agent. - EnableIngressGatewaySDS bool // IngressGatewayUDSPath is the unix domain socket through which SDS server communicates with // ingress gateway proxies. IngressGatewayUDSPath string @@ -77,6 +73,11 @@ type Options struct { // The Vault TLS root certificate. VaultTLSRootCert string + // EnableWorkloadSDS indicates whether node agent works as SDS server for workload proxies. + EnableWorkloadSDS bool + + // EnableIngressGatewaySDS indicates whether node agent works as ingress gateway agent. + EnableIngressGatewaySDS bool // AlwaysValidTokenFlag is set to true for if token used is always valid(ex, normal k8s JWT) AlwaysValidTokenFlag bool } diff --git a/security/pkg/pki/util/generate_cert.go b/security/pkg/pki/util/generate_cert.go index 7379d990cfab..c57d887eb160 100644 --- a/security/pkg/pki/util/generate_cert.go +++ b/security/pkg/pki/util/generate_cert.go @@ -56,6 +56,9 @@ type CertOptions struct { // Organization for this certificate. Org string + // The size of RSA private key to be generated. + RSAKeySize int + // Whether this certificate is used as signing cert for CA. IsCA bool @@ -68,9 +71,6 @@ type CertOptions struct { // Whether this certificate is for a server. IsServer bool - // The size of RSA private key to be generated. - RSAKeySize int - // Whether this certificate is for dual-use clients (SAN+CN). IsDualUse bool } @@ -169,8 +169,7 @@ func genCertTemplateFromCSR(csr *x509.CertificateRequest, subjectIDs []string, t // Dual use mode if common name in CSR is not empty. // In this case, set CN as determined by DualUseCommonName(subjectIDsInString). if len(csr.Subject.CommonName) != 0 { - cn, err := DualUseCommonName(subjectIDsInString) - if err != nil { + if cn, err := DualUseCommonName(subjectIDsInString); err != nil { // log and continue log.Errorf("dual-use failed for cert template - omitting CN (%v)", err) } else { diff --git a/security/pkg/server/ca/authenticate/kube_jwt_test.go b/security/pkg/server/ca/authenticate/kube_jwt_test.go index fa6b940241b3..6cc32ac5707d 100644 --- a/security/pkg/server/ca/authenticate/kube_jwt_test.go +++ b/security/pkg/server/ca/authenticate/kube_jwt_test.go @@ -57,7 +57,6 @@ func TestNewKubeJWTAuthenticator(t *testing.T) { testCases := map[string]struct { caCertPath string jwtPath string - trustDomain string expectedErrMsg string }{ "Invalid CA cert path": { diff --git a/security/pkg/server/ca/server_test.go b/security/pkg/server/ca/server_test.go index a41e03f7396e..9c77d58eddd5 100644 --- a/security/pkg/server/ca/server_test.go +++ b/security/pkg/server/ca/server_test.go @@ -64,28 +64,6 @@ gCojNs0xyJ77JA80HLY7iR4J6BRYsZQ/5UB/pYR55e4TGFDbI+C/6NBqLkzEfyX0 1sT/u25qExkefck= -----END CERTIFICATE REQUEST-----` -type mockCA struct { - cert string - root string - certChain string - errMsg string -} - -func (ca *mockCA) Sign(csrPEM []byte, ttl time.Duration, forCA bool) ([]byte, error) { - if ca.errMsg != "" { - return nil, fmt.Errorf(ca.errMsg) - } - return []byte(ca.cert), nil -} - -func (ca *mockCA) GetRootCertificate() []byte { - return []byte(ca.root) -} - -func (ca *mockCA) GetCertChain() []byte { - return []byte(ca.certChain) -} - type mockAuthenticator struct { authSource authenticate.AuthSource identities []string diff --git a/tests/e2e/framework/kubernetes.go b/tests/e2e/framework/kubernetes.go index a5ba8b891173..a6456a3bf059 100644 --- a/tests/e2e/framework/kubernetes.go +++ b/tests/e2e/framework/kubernetes.go @@ -999,28 +999,6 @@ func (k *KubeInfo) generateSidecarInjector(src, dst string) error { return err } -func (k *KubeInfo) generateGalleyConfigValidator(src, dst string) error { - content, err := ioutil.ReadFile(src) - if err != nil { - log.Errorf("Cannot read original yaml file %s", src) - return err - } - - if !*clusterWide { - content = replacePattern(content, istioSystem, k.Namespace) - } - - if *galleyHub != "" && *galleyTag != "" { - content = updateImage("galley", *galleyHub, *galleyTag, content) - } - - err = ioutil.WriteFile(dst, content, 0600) - if err != nil { - log.Errorf("Cannot write into generate galley config validator %s", dst) - } - return err -} - func replacePattern(content []byte, src, dest string) []byte { r := []byte(dest) p := regexp.MustCompile(src) diff --git a/tests/e2e/tests/mixer/mixer_test.go b/tests/e2e/tests/mixer/mixer_test.go index 956d8b1e1b50..dcbb79728df6 100644 --- a/tests/e2e/tests/mixer/mixer_test.go +++ b/tests/e2e/tests/mixer/mixer_test.go @@ -1322,12 +1322,6 @@ func logPolicyMetrics(t *testing.T, adapter, app string) { t.Logf("istio-policy stats (all requests): mixer checkcache hits: %f, adapter '%s' dispatches: %f", mixerCacheHits, adapter, dispatches) } -// nolint: deadcode -func mixerCheckCacheHits(promAPI v1.API) (float64, error) { - query := "sum(mixer_checkcache_cache_hits_total{job=\"istio-policy\"})" - return queryValue(promAPI, query) -} - func adapterDispatches(promAPI v1.API, adapter string) (float64, error) { query := fmt.Sprintf("sum(mixer_runtime_dispatches_total{adapter=\"%s\"})", adapter) return queryValue(promAPI, query) diff --git a/tests/e2e/tests/pilot/authn_policy_test.go b/tests/e2e/tests/pilot/authn_policy_test.go index bb7d02517231..22f9a28a8eb6 100644 --- a/tests/e2e/tests/pilot/authn_policy_test.go +++ b/tests/e2e/tests/pilot/authn_policy_test.go @@ -169,8 +169,8 @@ func TestAuthNJwt(t *testing.T) { expect string }{ // This needs to be de-flaked when authN is enabled https://github.com/istio/istio/issues/6288 - {dst: "d", src: "a", port: "", token: validJwtToken, expect: "200"}, - {dst: "d", src: "b", port: "80", token: "foo", expect: "401"}, + {dst: "d", src: "a", port: "", path: "", token: validJwtToken, expect: "200"}, + {dst: "d", src: "b", port: "80", path: "", token: "foo", expect: "401"}, } cases = append(cases, extraCases...) } diff --git a/tests/e2e/tests/pilot/egressgateway_test.go b/tests/e2e/tests/pilot/egressgateway_test.go index d908526380f3..abcc8b9706fe 100644 --- a/tests/e2e/tests/pilot/egressgateway_test.go +++ b/tests/e2e/tests/pilot/egressgateway_test.go @@ -97,7 +97,7 @@ func TestRouteSNIViaEgressGateway(t *testing.T) { for cluster := range tc.Kube.Clusters { for _, url := range []string{"https://www.google.com", "https://www.bing.com"} { runRetriableTest(t, "RouteSNIViaEgressGateway", defaultRetryBudget, func() error { - reqURL := fmt.Sprintf(url) + reqURL := url resp := ClientRequest(cluster, "a", reqURL, 100, "") count := make(map[string]int) for _, elt := range resp.Code { diff --git a/tests/e2e/tests/pilot/mesh_config_verify_test.go b/tests/e2e/tests/pilot/mesh_config_verify_test.go index 63f9689eabdb..89594de8a734 100644 --- a/tests/e2e/tests/pilot/mesh_config_verify_test.go +++ b/tests/e2e/tests/pilot/mesh_config_verify_test.go @@ -47,7 +47,7 @@ func verifyMCMeshConfig(primaryPodNames, remotePodNames []string, appEPs map[str return nil } - _, err := retry.Retry(nil, retryFn) + _, err := retry.Retry(context.TODO(), retryFn) return err } diff --git a/tests/e2e/tests/pilot/pilot_test.go b/tests/e2e/tests/pilot/pilot_test.go index 554560a8b049..b3206cbca379 100644 --- a/tests/e2e/tests/pilot/pilot_test.go +++ b/tests/e2e/tests/pilot/pilot_test.go @@ -324,7 +324,7 @@ func (t *testConfig) Setup() (err error) { for cluster, kc := range t.Kube.Clusters { if err == nil && !util.CheckPodsRunning(t.Kube.Namespace, kc) { err = fmt.Errorf("can't get all pods running in %s cluster", cluster) - break + return } } @@ -336,6 +336,7 @@ func (t *testConfig) Setup() (err error) { // Verify the service mesh config for a single cluster err = verifyMeshConfig() } + return } diff --git a/tests/e2e/tests/pilot/routing_test.go b/tests/e2e/tests/pilot/routing_test.go index 6a4af20dec11..276dffe30d83 100644 --- a/tests/e2e/tests/pilot/routing_test.go +++ b/tests/e2e/tests/pilot/routing_test.go @@ -708,7 +708,6 @@ func TestDestinationRuleExportTo(t *testing.T) { cases := []struct { testName string - description string rules map[string][]string src string dst string diff --git a/tests/util/kube_utils.go b/tests/util/kube_utils.go index 34c955f1b72c..d05841c2701c 100644 --- a/tests/util/kube_utils.go +++ b/tests/util/kube_utils.go @@ -736,14 +736,10 @@ func FetchAndSaveClusterLogs(namespace string, tempDir string, kubeconfig string if yaml, err0 := ShellMuteOutput( fmt.Sprintf("kubectl get %s -n %s -o yaml --kubeconfig=%s", resrc, namespace, kubeconfig)); err0 != nil { multiErr = multierror.Append(multiErr, err0) - } else { - if f, err1 := os.Create(filePath); err1 != nil { - multiErr = multierror.Append(multiErr, err1) - } else { - if _, err2 := f.WriteString(fmt.Sprintf("%s\n", yaml)); err2 != nil { - multiErr = multierror.Append(multiErr, err2) - } - } + } else if f, err1 := os.Create(filePath); err1 != nil { + multiErr = multierror.Append(multiErr, err1) + } else if _, err2 := f.WriteString(fmt.Sprintf("%s\n", yaml)); err2 != nil { + multiErr = multierror.Append(multiErr, err2) } } return multiErr @@ -845,14 +841,14 @@ func CheckPodRunning(n, name string, kubeconfig string) error { } // CreateMultiClusterSecret will create the secret associated with the remote cluster -func CreateMultiClusterSecret(namespace string, RemoteKubeConfig string, localKubeConfig string) error { +func CreateMultiClusterSecret(namespace string, remoteKubeConfig string, localKubeConfig string) error { const ( secretLabel = "istio/multiCluster" labelValue = "true" ) - secretName := filepath.Base(RemoteKubeConfig) + secretName := filepath.Base(remoteKubeConfig) - _, err := ShellMuteOutput("kubectl create secret generic %s --from-file %s -n %s --kubeconfig=%s", secretName, RemoteKubeConfig, namespace, localKubeConfig) + _, err := ShellMuteOutput("kubectl create secret generic %s --from-file %s -n %s --kubeconfig=%s", secretName, remoteKubeConfig, namespace, localKubeConfig) if err != nil { log.Infof("Failed to create secret %s\n", secretName) return err @@ -871,8 +867,8 @@ func CreateMultiClusterSecret(namespace string, RemoteKubeConfig string, localKu } // DeleteMultiClusterSecret delete the remote cluster secret -func DeleteMultiClusterSecret(namespace string, RemoteKubeConfig string, localKubeConfig string) error { - secretName := filepath.Base(RemoteKubeConfig) +func DeleteMultiClusterSecret(namespace string, remoteKubeConfig string, localKubeConfig string) error { + secretName := filepath.Base(remoteKubeConfig) _, err := ShellMuteOutput("kubectl delete secret %s -n %s --kubeconfig=%s", secretName, namespace, localKubeConfig) if err != nil { log.Errorf("Failed to delete secret %s: %v", secretName, err) diff --git a/tests/util/retry.go b/tests/util/retry.go index cc8c380794b7..c5021fd9375d 100644 --- a/tests/util/retry.go +++ b/tests/util/retry.go @@ -31,7 +31,7 @@ const ( func Backoff(baseDelay, maxDelay time.Duration, retries int) time.Duration { backoff, max := float64(baseDelay), float64(maxDelay) for backoff < max && retries > 0 { - backoff = backoff * backoffFactor + backoff *= backoffFactor retries-- } if backoff > max { diff --git a/tools/license/get_dep_licenses.go b/tools/license/get_dep_licenses.go index 128d58c23c42..64beedc3ce3d 100644 --- a/tools/license/get_dep_licenses.go +++ b/tools/license/get_dep_licenses.go @@ -244,7 +244,7 @@ func main() { var missing []string // TODO: detect multiple licenses. - licensePath := make(map[string]string, 0) + licensePath := make(map[string]string) for _, p := range outv { lf, err := findLicenseFile(p) if err != nil || lf == nil { @@ -256,9 +256,9 @@ func main() { licensePath[p] = lf[0] } - licenseTypes := make(map[string][]string, 0) + licenseTypes := make(map[string][]string) var reciprocalList, restrictedList, missingList []string - unknownMap := make(map[string]string, 0) + unknownMap := make(map[string]string) var licenses, exact, inexact LicenseInfos for p, lp := range licensePath { linfo := &LicenseInfo{}