Skip to content

Commit

Permalink
Enable Lint Rules: confusing-results & receiver-naming (jaegertracing…
Browse files Browse the repository at this point in the history
…#5524)

## Which problem is this PR solving?
-  Partial Fix for jaegertracing#5506

## Description of the changes
- Enabled confusing-results & receiver-naming in revive linter
- Used named outputs when datatype of outputs are same
- Made changes to maintain the same receiver name 

## How was this change tested?
- `make lint` `make test`

## Checklist
- [x] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [x] I have signed all commits
- [ ] I have added unit tests for the new functionality
- [x] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `yarn lint` and `yarn test`

---------

Signed-off-by: FlamingSaint <raghuramkannan400@gmail.com>
Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
  • Loading branch information
FlamingSaint and yurishkuro authored Jun 4, 2024
1 parent b295383 commit 2438ec0
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 36 deletions.
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 @@ func (toDomain) getFlags(zSpan *zipkincore.Span) model.Flags {
}

// 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()
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 @@ type Options struct {
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)
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)
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 {
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
}
// 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
}
}

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 @@ func (p Options) loadCertPool() (*x509.CertPool, error) {
}
certPool := x509.NewCertPool()
// setup user specified truststore
if err := addCertToPool(p.CAPath, certPool); err != nil {
if err := addCertToPool(o.CAPath, certPool); err != nil {
return nil, err
}
return certPool, nil
Expand Down Expand Up @@ -168,9 +168,9 @@ func addCertToPool(caPath string, certPool *x509.CertPool) error {
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

0 comments on commit 2438ec0

Please sign in to comment.