Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Enable Lint Rules: confusing-results & receiver-naming #5524

Merged
merged 3 commits into from
Jun 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -210,15 +210,9 @@ linters-settings:
# investigate, could be real bugs. But didn't recent Go version changed loop variables semantics?
- name: range-val-address
disabled: true
# enable after cleanup
- name: confusing-results
disabled: true
# enable after cleanup: "tag on not-exported field"
- name: struct-tag
disabled: true
# enable after cleanup
- name: receiver-naming
disabled: true
# this is idiocy, promotes less readable code. Don't enable.
- name: var-declaration
disabled: true
Expand Down
6 changes: 3 additions & 3 deletions model/converter/thrift/zipkin/to_domain.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,9 +198,9 @@
}

// Get a correct start time to use for the span if it's not set directly
func (td toDomain) getStartTimeAndDuration(zSpan *zipkincore.Span) (int64, int64) {
timestamp := zSpan.GetTimestamp()
duration := zSpan.GetDuration()
func (td toDomain) getStartTimeAndDuration(zSpan *zipkincore.Span) (timestamp, duration int64) {
timestamp = zSpan.GetTimestamp()
duration = zSpan.GetDuration()

Check warning on line 203 in model/converter/thrift/zipkin/to_domain.go

View check run for this annotation

Codecov / codecov/patch

model/converter/thrift/zipkin/to_domain.go#L201-L203

Added lines #L201 - L203 were not covered by tests
if timestamp == 0 {
cs := td.findAnnotation(zSpan, zipkincore.CLIENT_SEND)
sr := td.findAnnotation(zSpan, zipkincore.SERVER_RECV)
Expand Down
48 changes: 24 additions & 24 deletions pkg/config/tlscfg/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,82 +46,82 @@
var systemCertPool = x509.SystemCertPool // to allow overriding in unit test

// Config loads TLS certificates and returns a TLS Config.
func (p *Options) Config(logger *zap.Logger) (*tls.Config, error) {
func (o *Options) Config(logger *zap.Logger) (*tls.Config, error) {
var minVersionId, maxVersionId uint16

certPool, err := p.loadCertPool()
certPool, err := o.loadCertPool()
if err != nil {
return nil, fmt.Errorf("failed to load CA CertPool: %w", err)
}

cipherSuiteIds, err := CipherSuiteNamesToIDs(p.CipherSuites)
cipherSuiteIds, err := CipherSuiteNamesToIDs(o.CipherSuites)
if err != nil {
return nil, fmt.Errorf("failed to get cipher suite ids from cipher suite names: %w", err)
}

if p.MinVersion != "" {
minVersionId, err = VersionNameToID(p.MinVersion)
if o.MinVersion != "" {
minVersionId, err = VersionNameToID(o.MinVersion)

Check warning on line 63 in pkg/config/tlscfg/options.go

View check run for this annotation

Codecov / codecov/patch

pkg/config/tlscfg/options.go#L63

Added line #L63 was not covered by tests
if err != nil {
return nil, fmt.Errorf("failed to get minimum tls version: %w", err)
}
}

if p.MaxVersion != "" {
maxVersionId, err = VersionNameToID(p.MaxVersion)
if o.MaxVersion != "" {
maxVersionId, err = VersionNameToID(o.MaxVersion)

Check warning on line 70 in pkg/config/tlscfg/options.go

View check run for this annotation

Codecov / codecov/patch

pkg/config/tlscfg/options.go#L70

Added line #L70 was not covered by tests
if err != nil {
return nil, fmt.Errorf("failed to get maximum tls version: %w", err)
}
}

if p.MinVersion != "" && p.MaxVersion != "" {
if o.MinVersion != "" && o.MaxVersion != "" {
if minVersionId > maxVersionId {
return nil, fmt.Errorf("minimum tls version can't be greater than maximum tls version")
}
}

tlsCfg := &tls.Config{
RootCAs: certPool,
ServerName: p.ServerName,
InsecureSkipVerify: p.SkipHostVerify, /* #nosec G402*/
ServerName: o.ServerName,
InsecureSkipVerify: o.SkipHostVerify, /* #nosec G402*/
CipherSuites: cipherSuiteIds,
MinVersion: minVersionId,
MaxVersion: maxVersionId,
}

if p.ClientCAPath != "" {
if o.ClientCAPath != "" {
// TODO this should be moved to certWatcher, since it already loads key pair
certPool := x509.NewCertPool()
if err := addCertToPool(p.ClientCAPath, certPool); err != nil {
if err := addCertToPool(o.ClientCAPath, certPool); err != nil {

Check warning on line 94 in pkg/config/tlscfg/options.go

View check run for this annotation

Codecov / codecov/patch

pkg/config/tlscfg/options.go#L94

Added line #L94 was not covered by tests
return nil, err
}
tlsCfg.ClientCAs = certPool
tlsCfg.ClientAuth = tls.RequireAndVerifyClientCert
}

certWatcher, err := newCertWatcher(*p, logger, tlsCfg.RootCAs, tlsCfg.ClientCAs)
certWatcher, err := newCertWatcher(*o, logger, tlsCfg.RootCAs, tlsCfg.ClientCAs)
if err != nil {
return nil, err
}
p.certWatcher = certWatcher
o.certWatcher = certWatcher

if (p.CertPath == "" && p.KeyPath != "") || (p.CertPath != "" && p.KeyPath == "") {
if (o.CertPath == "" && o.KeyPath != "") || (o.CertPath != "" && o.KeyPath == "") {
return nil, fmt.Errorf("for client auth via TLS, either both client certificate and key must be supplied, or neither")
}
if p.CertPath != "" && p.KeyPath != "" {
if o.CertPath != "" && o.KeyPath != "" {
tlsCfg.GetCertificate = func(*tls.ClientHelloInfo) (*tls.Certificate, error) {
return p.certWatcher.certificate(), nil
return o.certWatcher.certificate(), nil

Check warning on line 112 in pkg/config/tlscfg/options.go

View check run for this annotation

Codecov / codecov/patch

pkg/config/tlscfg/options.go#L112

Added line #L112 was not covered by tests
}
// GetClientCertificate is used on the client side when server is configured with tls.RequireAndVerifyClientCert e.g. mTLS
tlsCfg.GetClientCertificate = func(*tls.CertificateRequestInfo) (*tls.Certificate, error) {
return p.certWatcher.certificate(), nil
return o.certWatcher.certificate(), nil

Check warning on line 116 in pkg/config/tlscfg/options.go

View check run for this annotation

Codecov / codecov/patch

pkg/config/tlscfg/options.go#L116

Added line #L116 was not covered by tests
}
}

return tlsCfg, nil
}

func (p Options) loadCertPool() (*x509.CertPool, error) {
if len(p.CAPath) == 0 { // no truststore given, use SystemCertPool
func (o Options) loadCertPool() (*x509.CertPool, error) {
if len(o.CAPath) == 0 { // no truststore given, use SystemCertPool
certPool, err := loadSystemCertPool()
if err != nil {
return nil, fmt.Errorf("failed to load SystemCertPool: %w", err)
Expand All @@ -130,7 +130,7 @@
}
certPool := x509.NewCertPool()
// setup user specified truststore
if err := addCertToPool(p.CAPath, certPool); err != nil {
if err := addCertToPool(o.CAPath, certPool); err != nil {

Check warning on line 133 in pkg/config/tlscfg/options.go

View check run for this annotation

Codecov / codecov/patch

pkg/config/tlscfg/options.go#L133

Added line #L133 was not covered by tests
return nil, err
}
return certPool, nil
Expand Down Expand Up @@ -168,9 +168,9 @@
var _ io.Closer = (*Options)(nil)

// Close shuts down the embedded certificate watcher.
func (p *Options) Close() error {
if p.certWatcher != nil {
return p.certWatcher.Close()
func (o *Options) Close() error {
if o.certWatcher != nil {
return o.certWatcher.Close()
}
return nil
}
6 changes: 3 additions & 3 deletions plugin/storage/es/mappings/mapping.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,12 @@ func (mb *MappingBuilder) GetMapping(mapping string) (string, error) {
}

// GetSpanServiceMappings returns span and service mappings
func (mb *MappingBuilder) GetSpanServiceMappings() (string, string, error) {
spanMapping, err := mb.GetMapping("jaeger-span")
func (mb *MappingBuilder) GetSpanServiceMappings() (spanMapping string, serviceMapping string, err error) {
spanMapping, err = mb.GetMapping("jaeger-span")
if err != nil {
return "", "", err
}
serviceMapping, err := mb.GetMapping("jaeger-service")
serviceMapping, err = mb.GetMapping("jaeger-service")
if err != nil {
return "", "", err
}
Expand Down
Loading