Skip to content

Commit

Permalink
Use golangci-lint for linting (#2034)
Browse files Browse the repository at this point in the history
* Use golangci-lint for linting
Rebase

Signed-off-by: Ed Snible <[email protected]>

* add golangci-lint to install tools

Signed-off-by: Ed Snible <[email protected]>

* Allow small drops in coverage

Signed-off-by: Ed Snible <[email protected]>

* Use Go 1.18 for linting, clean .golintci.yml removing things not needed for operator

Signed-off-by: Ed Snible <[email protected]>

* Use nolint directive instead of suppressing context return

Signed-off-by: Ed Snible <[email protected]>

Signed-off-by: Ed Snible <[email protected]>
  • Loading branch information
esnible authored Aug 18, 2022
1 parent 6db7884 commit 2ccf2d4
Show file tree
Hide file tree
Showing 90 changed files with 382 additions and 407 deletions.
7 changes: 7 additions & 0 deletions .codecov.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
coverage:
status:
project:
default:
target: auto
# this allows a 0.1% drop from the previous base commit coverage
threshold: 0.1%
ignore:
- "apis/v1/zz_generated.deepcopy.go"
- "apis/v1/zz_generated.defaults.go"
Expand Down
30 changes: 30 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
run:
go: '1.18'
timeout: 10m

linters-settings:
goimports:
local-prefixes: github.com/jaegertracing/jaeger-operator
gosimple:
go: "1.18"

linters:
enable:
- depguard
- gofmt
- gofumpt
- goimports
- gosec
- govet
- misspell
- bidichk
disable:
- errcheck

issues:
# Excluding configuration per-path, per-linter, per-text and per-source
exclude-rules:
# Exclude some linters from running on tests files.
- path: _test\.go
linters:
- gosec
2 changes: 2 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ PHONY: lint
lint:
$(ECHO) Linting...
$(VECHO)GOPATH=${GOPATH} ./.ci/lint.sh
golangci-lint -v run

.PHONY: vet
vet: ## Run go vet against code.
Expand Down Expand Up @@ -437,6 +438,7 @@ tools: kustomize controller-gen operator-sdk

.PHONY: install-tools
install-tools: operator-sdk
go install github.com/golangci/golangci-lint/cmd/[email protected]
$(VECHO)${GO_FLAGS} ./.ci/vgot.sh \
golang.org/x/lint/golint \
golang.org/x/tools/cmd/goimports
Expand Down
5 changes: 2 additions & 3 deletions apis/v1/jaeger_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ type JaegerQuerySpec struct {
// +optional
// TracingEnabled if set to false adds the JAEGER_DISABLED environment flag and removes the injected
// agent container from the query component to disable tracing requests to the query service.
// The default, if ommited, is true
// The default, if omitted, is true
TracingEnabled *bool `json:"tracingEnabled,omitempty"`

// +optional
Expand Down Expand Up @@ -381,7 +381,7 @@ type JaegerAllInOneSpec struct {
// +optional
// TracingEnabled if set to false adds the JAEGER_DISABLED environment flag and removes the injected
// agent container from the query component to disable tracing requests to the query service.
// The default, if ommited, is true
// The default, if omitted, is true
TracingEnabled *bool `json:"tracingEnabled,omitempty"`

// +optional
Expand All @@ -405,7 +405,6 @@ type AutoScaleSpec struct {

// JaegerCollectorSpec defines the options to be used when deploying the collector
type JaegerCollectorSpec struct {

// +optional
AutoScaleSpec `json:",inline,omitempty"`

Expand Down
12 changes: 6 additions & 6 deletions apis/v1/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ type Values map[string]interface{}
func (v *Values) DeepCopy() *Values {
out := make(Values, len(*v))
for key, val := range *v {
switch val.(type) {
switch val := val.(type) {
case string:
out[key] = val

case []string:
out[key] = append([]string(nil), val.([]string)...)
out[key] = append([]string(nil), val...)
}
}
return &out
Expand Down Expand Up @@ -132,11 +132,11 @@ func (o *Options) ToArgs() []string {
if len(o.opts) > 0 {
args := make([]string, 0, len(o.opts))
for k, v := range o.opts {
switch v.(type) {
switch v := v.(type) {
case string:
args = append(args, fmt.Sprintf("--%s=%v", k, v))
case []string:
for _, vv := range v.([]string) {
for _, vv := range v {
args = append(args, fmt.Sprintf("--%s=%v", k, vv))
}
}
Expand All @@ -157,9 +157,9 @@ func (o *Options) Map() map[string]interface{} {
func (o *Options) StringMap() map[string]string {
smap := make(map[string]string)
for k, v := range o.opts {
switch v.(type) {
switch v := v.(type) {
case string:
smap[k] = v.(string)
smap[k] = v
}
}
return smap
Expand Down
1 change: 0 additions & 1 deletion apis/v1/options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,5 +186,4 @@ func TestRepetitiveArguments(t *testing.T) {

assert.Len(t, args, 3)
assert.Equal(t, expected, args)

}
2 changes: 0 additions & 2 deletions controllers/appsv1/deployment_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ func (d *deploymentInterceptor) Handle(ctx context.Context, req admission.Reques
if dep.Labels["app"] == "jaeger" && dep.Labels["app.kubernetes.io/component"] != "query" {
// Don't touch jaeger deployments
return admission.Allowed("is jaeger deployment, we do not touch it")

}

ns := &corev1.Namespace{}
Expand Down Expand Up @@ -149,7 +148,6 @@ func (d *deploymentInterceptor) Handle(ctx context.Context, req admission.Reques

return admission.PatchResponseFromRaw(req.Object.Raw, marshaledDeploy)
}

}
return admission.Allowed("no action needed")
}
Expand Down
8 changes: 5 additions & 3 deletions controllers/appsv1/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@ import (
// +kubebuilder:scaffold:imports
)

var k8sClient client.Client
var testEnv *envtest.Environment
var testScheme *runtime.Scheme = scheme.Scheme
var (
k8sClient client.Client
testEnv *envtest.Environment
testScheme *runtime.Scheme = scheme.Scheme
)

func TestMain(m *testing.M) {
testEnv = &envtest.Environment{
Expand Down
8 changes: 5 additions & 3 deletions controllers/jaegertracing/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@ import (
// +kubebuilder:scaffold:imports
)

var k8sClient client.Client
var testEnv *envtest.Environment
var testScheme *runtime.Scheme = scheme.Scheme
var (
k8sClient client.Client
testEnv *envtest.Environment
testScheme *runtime.Scheme = scheme.Scheme
)

func TestMain(m *testing.M) {
testEnv = &envtest.Environment{
Expand Down
2 changes: 1 addition & 1 deletion pkg/account/oauth_proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func OAuthProxy(jaeger *v1.Jaeger) *corev1.ServiceAccount {
"serviceaccounts.openshift.io/oauth-redirectreference.primary": getOAuthRedirectReference(jaeger),
},
OwnerReferences: []metav1.OwnerReference{
metav1.OwnerReference{
{
APIVersion: jaeger.APIVersion,
Kind: jaeger.Kind,
Name: jaeger.Name,
Expand Down
8 changes: 2 additions & 6 deletions pkg/autodetect/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,8 @@ func (b *Background) Start() {

go func() {
for {
select {
case <-b.ticker.C:
b.autoDetectCapabilities()
}
<-b.ticker.C
b.autoDetectCapabilities()
}
}()
}
Expand All @@ -97,7 +95,6 @@ func (b *Background) autoDetectCapabilities() {
b.firstRun.Do(func() {
// the platform won't change during the execution of the operator, need to run it only once
b.detectPlatform(ctx, apiList)

})

b.detectElasticsearch(ctx, apiList)
Expand Down Expand Up @@ -132,7 +129,6 @@ func (b *Background) detectCronjobsVersion(ctx context.Context) {
log.Log.V(2).Info(
fmt.Sprintf("Did not find the cronjobs api in %s", strings.Join(apiGroupVersions, " or ")),
)

}

// AvailableAPIs returns available list of CRDs from the cluster.
Expand Down
8 changes: 3 additions & 5 deletions pkg/autodetect/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,8 @@ func TestStartContinuesInBackground(t *testing.T) {
case <-time.After(6 * time.Second): // this one might take up to 5 seconds to run again + processing time
assert.Fail(t, "timed out waiting for the start process to detect the new capabilities")
}

}

func TestAutoDetectWithServerGroupsError(t *testing.T) {
// prepare
defer viper.Reset()
Expand Down Expand Up @@ -282,7 +282,6 @@ func TestAutoDetectEsProvisionWithEsOperator(t *testing.T) {
}

t.Run("kind Elasticsearch", func(t *testing.T) {

dcl.ServerResourcesForGroupVersionFunc = func(_ string) (apiGroupList *metav1.APIResourceList, err error) {
return &metav1.APIResourceList{
GroupVersion: "logging.openshift.io/v1",
Expand All @@ -300,7 +299,6 @@ func TestAutoDetectEsProvisionWithEsOperator(t *testing.T) {
t.Run("no kind Elasticsearch", func(t *testing.T) {
dcl.ServerResourcesForGroupVersionFunc = func(_ string) (apiGroupList *metav1.APIResourceList, err error) {
return &metav1.APIResourceList{

GroupVersion: "logging.openshift.io/v1",
APIResources: []metav1.APIResource{
{
Expand Down Expand Up @@ -433,7 +431,7 @@ func TestAutoDetectCronJobsVersion(t *testing.T) {
apiGroupVersions := []string{v1.FlagCronJobsVersionBatchV1, v1.FlagCronJobsVersionBatchV1Beta1}
for _, apiGroup := range apiGroupVersions {
dcl := &fakeDiscoveryClient{}
cl := fake.NewFakeClient()
cl := fake.NewFakeClient() // nolint:staticcheck
b := WithClients(cl, dcl, cl)
dcl.ServerGroupsFunc = func() (apiGroupList *metav1.APIGroupList, err error) {
return &metav1.APIGroupList{Groups: []metav1.APIGroup{{
Expand Down Expand Up @@ -594,7 +592,7 @@ func TestCleanDeployments(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{},
Spec: corev1.PodSpec{
Containers: []corev1.Container{
corev1.Container{
{
Name: "C1",
Image: "image1",
},
Expand Down
3 changes: 1 addition & 2 deletions pkg/cmd/generate/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ func createSpecFromYAML(filename string) (*v1.Jaeger, error) {
}

func generate(_ *cobra.Command, _ []string) error {

var loggingLevel zapcore.Level
switch strings.ToLower(viper.GetString("log-level")) {
case "panic":
Expand Down Expand Up @@ -104,7 +103,7 @@ func generate(_ *cobra.Command, _ []string) error {

outputName := viper.GetString("output")
pathToFile := filepath.Clean(outputName)
out, err := os.OpenFile(pathToFile, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0600)
out, err := os.OpenFile(pathToFile, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0o600)
if err != nil {
return err
}
Expand Down
16 changes: 7 additions & 9 deletions pkg/cmd/start/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ func init() {
}

func bootstrap(ctx context.Context) manager.Manager {

namespace := getNamespace(ctx)
tracing.Bootstrap(ctx, namespace)

Expand Down Expand Up @@ -230,7 +229,7 @@ func detectNamespacePermissions(ctx context.Context, mgr manager.Manager) {

func setOperatorScope(ctx context.Context, namespace string) {
tracer := otel.GetTracerProvider().Tracer(v1.BootstrapTracer)
ctx, span := tracer.Start(ctx, "setOperatorScope")
ctx, span := tracer.Start(ctx, "setOperatorScope") // nolint:ineffassign,staticcheck
defer span.End()

// set what's the namespace to watch
Expand All @@ -249,7 +248,7 @@ func setOperatorScope(ctx context.Context, namespace string) {

func setLogLevel(ctx context.Context) {
tracer := otel.GetTracerProvider().Tracer(v1.BootstrapTracer)
ctx, span := tracer.Start(ctx, "setLogLevel")
ctx, span := tracer.Start(ctx, "setLogLevel") // nolint:ineffassign,staticcheck
defer span.End()

var loggingLevel zapcore.Level
Expand Down Expand Up @@ -277,12 +276,11 @@ func setLogLevel(ctx context.Context) {
pflag.CommandLine.AddGoFlagSet(flag.CommandLine)

ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts)))

}

func buildIdentity(ctx context.Context, podNamespace string) {
tracer := otel.GetTracerProvider().Tracer(v1.BootstrapTracer)
ctx, span := tracer.Start(ctx, "buildIdentity")
ctx, span := tracer.Start(ctx, "buildIdentity") // nolint:ineffassign,staticcheck
defer span.End()

operatorName, found := os.LookupEnv("OPERATOR_NAME")
Expand All @@ -297,7 +295,7 @@ func buildIdentity(ctx context.Context, podNamespace string) {
if len(podNamespace) > 0 {
identity = fmt.Sprintf("%s.%s", podNamespace, operatorName)
} else {
identity = fmt.Sprintf("%s", operatorName)
identity = operatorName
}

span.SetAttributes(otelattribute.String(v1.ConfigIdentity, identity))
Expand All @@ -306,7 +304,7 @@ func buildIdentity(ctx context.Context, podNamespace string) {

func createManager(ctx context.Context, cfg *rest.Config) manager.Manager {
tracer := otel.GetTracerProvider().Tracer(v1.BootstrapTracer)
ctx, span := tracer.Start(ctx, "createManager")
ctx, span := tracer.Start(ctx, "createManager") // nolint:ineffassign,staticcheck
defer span.End()

metricsHost := viper.GetString("metrics-host")
Expand Down Expand Up @@ -378,7 +376,7 @@ func performUpgrades(ctx context.Context, mgr manager.Manager) {

func setupControllers(ctx context.Context, mgr manager.Manager) {
tracer := otel.GetTracerProvider().Tracer(v1.BootstrapTracer)
ctx, span := tracer.Start(ctx, "setupControllers")
ctx, span := tracer.Start(ctx, "setupControllers") // nolint:ineffassign,staticcheck
clientReader := mgr.GetAPIReader()
client := mgr.GetClient()
schema := mgr.GetScheme()
Expand Down Expand Up @@ -419,7 +417,7 @@ func setupWebhooks(_ context.Context, mgr manager.Manager) {

func getNamespace(ctx context.Context) string {
tracer := otel.GetTracerProvider().Tracer(v1.BootstrapTracer)
ctx, span := tracer.Start(ctx, "getNamespace")
ctx, span := tracer.Start(ctx, "getNamespace") // nolint:ineffassign,staticcheck
defer span.End()

podNamespace, found := os.LookupEnv("POD_NAMESPACE")
Expand Down
5 changes: 2 additions & 3 deletions pkg/config/sampling/sampling.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func (u *Config) Get() *corev1.ConfigMap {
Namespace: u.jaeger.Namespace,
Labels: util.Labels(fmt.Sprintf("%s-sampling-configuration", u.jaeger.Name), "sampling-configuration", *u.jaeger),
OwnerReferences: []metav1.OwnerReference{
metav1.OwnerReference{
{
APIVersion: u.jaeger.APIVersion,
Kind: u.jaeger.Kind,
Name: u.jaeger.Name,
Expand Down Expand Up @@ -97,7 +97,6 @@ func CheckForSamplingConfigFile(jaeger *v1.Jaeger) bool {
// Update will modify the supplied common spec and options to include
// support for the Sampling configmap.
func Update(jaeger *v1.Jaeger, commonSpec *v1.JaegerCommonSpec, options *[]string) {

if CheckForSamplingConfigFile(jaeger) {
return
}
Expand All @@ -110,7 +109,7 @@ func Update(jaeger *v1.Jaeger, commonSpec *v1.JaegerCommonSpec, options *[]strin
Name: fmt.Sprintf("%s-sampling-configuration", jaeger.Name),
},
Items: []corev1.KeyToPath{
corev1.KeyToPath{
{
Key: "sampling",
Path: "sampling.json",
},
Expand Down
4 changes: 2 additions & 2 deletions pkg/config/ui/ui.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func (u *UIConfig) Get() *corev1.ConfigMap {
Namespace: u.jaeger.Namespace,
Labels: util.Labels(fmt.Sprintf("%s-ui-configuration", u.jaeger.Name), "ui-configuration", *u.jaeger),
OwnerReferences: []metav1.OwnerReference{
metav1.OwnerReference{
{
APIVersion: u.jaeger.APIVersion,
Kind: u.jaeger.Kind,
Name: u.jaeger.Name,
Expand Down Expand Up @@ -77,7 +77,7 @@ func Update(jaeger *v1.Jaeger, commonSpec *v1.JaegerCommonSpec, options *[]strin
Name: fmt.Sprintf("%s-ui-configuration", jaeger.Name),
},
Items: []corev1.KeyToPath{
corev1.KeyToPath{
{
Key: "ui",
Path: "ui.json",
},
Expand Down
Loading

0 comments on commit 2ccf2d4

Please sign in to comment.