From 618be58a267193bd307702f00834254b376213a6 Mon Sep 17 00:00:00 2001 From: "Christian Schaible (EXT)" Date: Tue, 25 Mar 2025 08:40:27 +0000 Subject: [PATCH] Merged PR 752362: feat: Apply stricter linter rules Security-concept-update-needed: false. JIRA Work Item: STACKITALO-184 --- .golangci.yml | 272 ++++++++++++++++++++++++ audit/api/api.go | 8 +- audit/api/api_legacy_converter.go | 46 ++-- audit/api/api_legacy_dynamic.go | 2 +- audit/api/api_legacy_dynamic_test.go | 40 ++-- audit/api/api_mock_test.go | 8 +- audit/api/base64.go | 2 +- audit/api/builder.go | 8 +- audit/api/model.go | 89 ++++---- audit/api/model_test.go | 14 +- audit/api/trace.go | 2 +- audit/messaging/amqp_connection.go | 4 +- audit/messaging/amqp_connection_pool.go | 7 +- audit/utils/sequence_generator.go | 4 +- log/log.go | 7 +- 15 files changed, 388 insertions(+), 125 deletions(-) create mode 100644 .golangci.yml diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..c29f184 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,272 @@ +linters-settings: + errcheck: + # report about not checking of errors in type assetions: `a := b.(MyStruct)`; + # default is false: such cases aren't reported by default. + check-type-assertions: true + + # report about assignment of errors to blank identifier: `num, _ := strconv.Atoi(numStr)`; + # default is false: such cases aren't reported by default. + check-blank: true + exhaustive: + # Presence of "default" case in switch statements satisfies exhaustiveness, + # even if all enum members are not listed. + default-signifies-exhaustive: true + funlen: + lines: 100 + statements: 50 + gocyclo: + min-complexity: 45 + gocognit: + min-complexity: 45 + dupl: + threshold: 150 + goconst: + min-len: 3 + min-occurrences: 5 + govet: + enable-all: true + disable: + - fieldalignment + lll: + line-length: 180 + tab-width: 1 + cyclop: + # the maximal code complexity to report + max-complexity: 45 + # the maximal average package complexity. If it's higher than 0.0 (float) the check is enabled (default 0.0) + package-average: 0.0 + unused: + # treat code as a program (not a library) and report unused exported identifiers; default is false. + # XXX: if you enable this setting, unused will report a lot of false-positives in text editors: + # if it's called for subdir of a project it can't find funcs usages. All text editor integrations + # with golangci-lint call it on a directory with the changed file. + check-exported: false + unparam: + # Inspect exported functions, default is false. Set to true if no external program/library imports your code. + # XXX: if you enable this setting, unparam will report a lot of false-positives in text editors: + # if it's called for subdir of a project it can't find external interfaces. All text editor integrations + # with golangci-lint call it on a directory with the changed file. + check-exported: false + nakedret: + # make an issue if func has more lines of code than this setting and it has naked returns; default is 30 + max-func-lines: 5 + prealloc: + # XXX: we don't recommend using this linter before doing performance profiling. + # For most programs usage of prealloc will be a premature optimization. + + # Report preallocation suggestions only on simple loops that have no returns/breaks/continues/gotos in them. + # True by default. + simple: true + range-loops: true # Report preallocation suggestions on range loops, true by default + for-loops: true # Report preallocation suggestions on for loops, false by default + gocritic: + enabled-tags: + - diagnostic + - experimental + - opinionated + - performance + - style + disabled-checks: + - dupImport # https://github.com/go-critic/go-critic/issues/845 + - octalLiteral + - unnamedResult + # Settings passed to gocritic. + # The settings key is the name of a supported gocritic checker. + # The list of supported checkers can be find in https://go-critic.github.io/overview. + settings: + hugeParam: + # Size in bytes that makes the warning trigger. + # Default: 80 + sizeThreshold: 121 + dogsled: + # checks assignments with too many blank identifiers; default is 2 + max-blank-identifiers: 2 + whitespace: + multi-if: false # Enforces newlines (or comments) after every multi-line if statement + multi-func: false # Enforces newlines (or comments) after every multi-line function signature + gomoddirectives: + # List of allowed `replace` directives. Default is empty. + # Add your allowed `replace` targets here, this rule is so you don't accidentally commit replacements you added for testing + replace-allow-list: [] + nolintlint: + allow-leading-space: false # require machine-readable nolint directives (i.e. with no leading space) + allow-unused: false # report any unused nolint directives + require-explanation: true # require an explanation for nolint directives + require-specific: true # require nolint directives to be specific about which linter is being skipped + nlreturn: + # Size of the block (including return statement that is still "OK") + # so no return split required. + block-size: 5 + stylecheck: + initialisms: ["ACL", "API", "ASCII", "CPU", "CSS", "DNS", "EOF", "GUID", "HTML", "HTTP", "HTTPS", "ID", "IP", "JSON", "QPS", "RAM", "RPC", "SLA", "SMTP", "SQL", "SSH", "TCP", "TLS", "TTL", "UDP", "UI", "GID", "UID", "UUID", "URI", "URL", "UTF8", "VM", "XML", "XMPP", "XSRF", "XSS", "SIP", "RTP", "AMQP", "DB", "TS"] + revive: + rules: + - name: context-keys-type + disabled: false + - name: time-naming + disabled: false + - name: var-declaration + disabled: false + - name: unexported-return + disabled: false + - name: errorf + disabled: false + - name: blank-imports + disabled: false + - name: context-as-argument + disabled: false + - name: dot-imports + disabled: false + - name: error-return + disabled: false + - name: error-strings + disabled: false + - name: error-naming + disabled: false + - name: exported + disabled: false + - name: increment-decrement + disabled: false + - name: var-naming + disabled: true + - name: package-comments + disabled: false + - name: range + disabled: false + - name: receiver-naming + disabled: false + - name: indent-error-flow + disabled: false + nestif: + min-complexity: 10 + +linters: + # please, do not use `enable-all`: it's deprecated and will be removed soon. + # inverted configuration with `enable-all` and `disable` is not scalable during updates of golangci-lint + disable-all: true + enable: + - dogsled + - dupl + - copyloopvar + - exhaustive + - gochecknoinits + - goconst + - gocritic + - gocyclo + - goprintffuncname + - gosec + - govet + - ineffassign + - lll + - gosimple + - misspell + - nakedret + - nolintlint + - revive + - staticcheck + - typecheck + - unconvert + - unused + - gochecknoglobals + - prealloc + - asciicheck + - nestif + - bodyclose + - cyclop + - durationcheck + - errcheck + - errorlint + - forbidigo + - forcetypeassert + - gocognit + - gomoddirectives + - gomodguard + - importas + - makezero + - nilerr + - noctx + - predeclared + - promlinter + - rowserrcheck + - sqlclosecheck + - tparallel + - unparam + - wastedassign + +issues: + # Excluding configuration per-path, per-linter, per-text and per-source + exclude-rules: + - path: audit/api/api_common.go + text: 'context-as-argument' + - path: audit/api/api.go|log/log.go|audit/api/model.go|telemetry/telemetry.go + linters: + - gochecknoglobals + - path: audit/api/api_.*.go + linters: + - dupl + - path: audit/api/model.go + text: 'exported: type name will be used as api.ApiRequest by other packages' + - path: audit/api/model_test.go|audit/api/model.go + text: 'G115' + - path: audit/api/test_data.go + linters: + - gosec + - path: _test\.go + linters: + - gochecknoglobals + - noctx + - forcetypeassert + - dogsled + - goconst + - unparam + - dupl + - errcheck + - forbidigo + - lll + - gocritic + - nestif + - revive + - gocognit + - unconvert + - unparam + - wsl + - gosimple + - ineffassign + - nakedret + - nlreturn + - staticcheck + - wastedassign + - text: 'declaration of "err" shadows declaration' + linters: + - govet + - path: test_.*\.go|audit/messaging/solace.go + linters: + - gochecknoglobals + - noctx + - forcetypeassert + - dogsled + - goconst + - unparam + - dupl + - errcheck + - forbidigo + - lll + - gocritic + - nestif + - revive + - gocognit + - unconvert + - unparam + - wsl + - gosimple + - ineffassign + - nakedret + - nlreturn + - staticcheck + - wastedassign + max-same-issues: 0 + max-issues-per-linter: 0 +run: + timeout: 10m + issues-exit-code: 1 + tests: true diff --git a/audit/api/api.go b/audit/api/api.go index 0c22dff..c43cc7d 100644 --- a/audit/api/api.go +++ b/audit/api/api.go @@ -47,13 +47,7 @@ func ObjectTypeFromPluralString(value string) ObjectType { func (t ObjectType) IsSupportedType() error { switch t { - case ObjectTypeOrganization: - fallthrough - case ObjectTypeFolder: - fallthrough - case ObjectTypeProject: - fallthrough - case ObjectTypeSystem: + case ObjectTypeOrganization, ObjectTypeFolder, ObjectTypeProject, ObjectTypeSystem: return nil default: return ErrUnknownObjectType diff --git a/audit/api/api_legacy_converter.go b/audit/api/api_legacy_converter.go index 840c486..feed0f8 100644 --- a/audit/api/api_legacy_converter.go +++ b/audit/api/api_legacy_converter.go @@ -22,15 +22,16 @@ func convertAndSerializeIntoLegacyFormat( // Event type var eventType string - if strings.HasSuffix(event.LogName, string(EventTypeAdminActivity)) { + switch { + case strings.HasSuffix(event.LogName, string(EventTypeAdminActivity)): eventType = "ADMIN_ACTIVITY" - } else if strings.HasSuffix(event.LogName, string(EventTypeSystemEvent)) { + case strings.HasSuffix(event.LogName, string(EventTypeSystemEvent)): eventType = "SYSTEM_EVENT" - } else if strings.HasSuffix(event.LogName, string(EventTypePolicyDenied)) { + case strings.HasSuffix(event.LogName, string(EventTypePolicyDenied)): eventType = "POLICY_DENIED" - } else if strings.HasSuffix(event.LogName, string(EventTypeDataAccess)) { + case strings.HasSuffix(event.LogName, string(EventTypeDataAccess)): return nil, ErrUnsupportedEventTypeDataAccess - } else { + default: return nil, errors.New("unsupported event type") } @@ -46,7 +47,7 @@ func convertAndSerializeIntoLegacyFormat( } // Principals - var serviceAccountDelegationInfo *LegacyAuditEventServiceAccountDelegationInfo = nil + var serviceAccountDelegationInfo *LegacyAuditEventServiceAccountDelegationInfo if len(event.ProtoPayload.AuthenticationInfo.ServiceAccountDelegationInfo) > 0 { var principals []LegacyAuditEventPrincipal for _, principal := range event.ProtoPayload.AuthenticationInfo.ServiceAccountDelegationInfo { @@ -73,7 +74,7 @@ func convertAndSerializeIntoLegacyFormat( Endpoint: "none", } } else { - var parameters map[string]interface{} = nil + var parameters map[string]interface{} if event.ProtoPayload.RequestMetadata.RequestAttributes.Path != "" && event.ProtoPayload.RequestMetadata.RequestAttributes.Query != nil && *event.ProtoPayload.RequestMetadata.RequestAttributes.Query != "" { @@ -95,11 +96,11 @@ func convertAndSerializeIntoLegacyFormat( } } - var body map[string]interface{} = nil + var body map[string]interface{} if event.ProtoPayload.Request != nil { body = event.ProtoPayload.Request.AsMap() } - var headers map[string]interface{} = nil + var headers map[string]interface{} if event.ProtoPayload.RequestMetadata.RequestAttributes.Headers != nil { headers = map[string]interface{}{} for key, value := range event.ProtoPayload.RequestMetadata.RequestAttributes.Headers { @@ -152,6 +153,8 @@ func convertAndSerializeIntoLegacyFormat( visibility = "PUBLIC" case auditV1.Visibility_VISIBILITY_PRIVATE: visibility = "PRIVATE" + case auditV1.Visibility_VISIBILITY_UNSPECIFIED: + visibility = "" } // Details @@ -171,23 +174,16 @@ func convertAndSerializeIntoLegacyFormat( // Severity var severity string switch event.Severity { - case auditV1.LogSeverity_LOG_SEVERITY_DEFAULT: - fallthrough - case auditV1.LogSeverity_LOG_SEVERITY_DEBUG: - fallthrough - case auditV1.LogSeverity_LOG_SEVERITY_INFO: - fallthrough - case auditV1.LogSeverity_LOG_SEVERITY_NOTICE: - fallthrough - case auditV1.LogSeverity_LOG_SEVERITY_WARNING: + case auditV1.LogSeverity_LOG_SEVERITY_DEFAULT, + auditV1.LogSeverity_LOG_SEVERITY_DEBUG, + auditV1.LogSeverity_LOG_SEVERITY_INFO, + auditV1.LogSeverity_LOG_SEVERITY_NOTICE, + auditV1.LogSeverity_LOG_SEVERITY_WARNING: severity = "INFO" - case auditV1.LogSeverity_LOG_SEVERITY_ERROR: - fallthrough - case auditV1.LogSeverity_LOG_SEVERITY_CRITICAL: - fallthrough - case auditV1.LogSeverity_LOG_SEVERITY_ALERT: - fallthrough - case auditV1.LogSeverity_LOG_SEVERITY_EMERGENCY: + case auditV1.LogSeverity_LOG_SEVERITY_ERROR, + auditV1.LogSeverity_LOG_SEVERITY_CRITICAL, + auditV1.LogSeverity_LOG_SEVERITY_ALERT, + auditV1.LogSeverity_LOG_SEVERITY_EMERGENCY: severity = "ERROR" default: return nil, ErrUnsupportedSeverity diff --git a/audit/api/api_legacy_dynamic.go b/audit/api/api_legacy_dynamic.go index c2e08d1..129f2ee 100644 --- a/audit/api/api_legacy_dynamic.go +++ b/audit/api/api_legacy_dynamic.go @@ -135,7 +135,7 @@ func (a *DynamicLegacyAuditApi) Send( return ErrNoTopicNameProvided } topicName := fmt.Sprintf("%s", rawTopicName) - if len(topicName) == 0 { + if topicName == "" { return ErrTopicNameEmpty } diff --git a/audit/api/api_legacy_dynamic_test.go b/audit/api/api_legacy_dynamic_test.go index 7a26d6b..399eee0 100644 --- a/audit/api/api_legacy_dynamic_test.go +++ b/audit/api/api_legacy_dynamic_test.go @@ -69,9 +69,9 @@ func TestDynamicLegacyAuditApi(t *testing.T) { // Log the event to solace visibility := auditV1.Visibility_VISIBILITY_PUBLIC - ctx := context.WithValue(ctx, ContextKeyTopic, topicName) + ctxWithTopic := context.WithValue(ctx, ContextKeyTopic, topicName) assert.ErrorIs(t, auditApi.Log( - ctx, + ctxWithTopic, event, visibility, NewRoutableIdentifier(objectIdentifier), @@ -102,9 +102,9 @@ func TestDynamicLegacyAuditApi(t *testing.T) { // Log the event to solace visibility := auditV1.Visibility_VISIBILITY_PUBLIC - ctx := context.WithValue(ctx, ContextKeyTopic, topicName) + ctxWithTopic := context.WithValue(ctx, ContextKeyTopic, topicName) assert.NoError(t, auditApi.Log( - ctx, + ctxWithTopic, event, visibility, NewRoutableIdentifier(objectIdentifier), @@ -139,9 +139,9 @@ func TestDynamicLegacyAuditApi(t *testing.T) { // Log the event to solace visibility := auditV1.Visibility_VISIBILITY_PRIVATE - ctx := context.WithValue(ctx, ContextKeyTopic, topicName) + ctxWithTopic := context.WithValue(ctx, ContextKeyTopic, topicName) assert.NoError(t, auditApi.Log( - ctx, + ctxWithTopic, event, visibility, NewRoutableIdentifier(objectIdentifier), @@ -177,9 +177,9 @@ func TestDynamicLegacyAuditApi(t *testing.T) { // Log the event to solace visibility := auditV1.Visibility_VISIBILITY_PUBLIC - ctx := context.WithValue(ctx, ContextKeyTopic, topicName) + ctxWithTopic := context.WithValue(ctx, ContextKeyTopic, topicName) assert.NoError(t, auditApi.Log( - ctx, + ctxWithTopic, event, visibility, NewRoutableIdentifier(objectIdentifier), @@ -214,9 +214,9 @@ func TestDynamicLegacyAuditApi(t *testing.T) { // Log the event to solace visibility := auditV1.Visibility_VISIBILITY_PRIVATE - ctx := context.WithValue(ctx, ContextKeyTopic, topicName) + ctxWithTopic := context.WithValue(ctx, ContextKeyTopic, topicName) assert.NoError(t, auditApi.Log( - ctx, + ctxWithTopic, event, visibility, NewRoutableIdentifier(objectIdentifier), @@ -252,9 +252,9 @@ func TestDynamicLegacyAuditApi(t *testing.T) { // Log the event to solace visibility := auditV1.Visibility_VISIBILITY_PUBLIC - ctx := context.WithValue(ctx, ContextKeyTopic, topicName) + ctxWithTopic := context.WithValue(ctx, ContextKeyTopic, topicName) assert.NoError(t, auditApi.Log( - ctx, + ctxWithTopic, event, visibility, NewRoutableIdentifier(objectIdentifier), @@ -289,9 +289,9 @@ func TestDynamicLegacyAuditApi(t *testing.T) { // Log the event to solace visibility := auditV1.Visibility_VISIBILITY_PRIVATE - ctx := context.WithValue(ctx, ContextKeyTopic, topicName) + ctxWithTopic := context.WithValue(ctx, ContextKeyTopic, topicName) assert.NoError(t, auditApi.Log( - ctx, + ctxWithTopic, event, visibility, NewRoutableIdentifier(objectIdentifier), @@ -326,10 +326,10 @@ func TestDynamicLegacyAuditApi(t *testing.T) { // Log the event to solace visibility := auditV1.Visibility_VISIBILITY_PRIVATE - ctx := context.WithValue(ctx, ContextKeyTopic, topicName) + ctxWithTopic := context.WithValue(ctx, ContextKeyTopic, topicName) assert.NoError(t, auditApi.Log( - ctx, + ctxWithTopic, event, visibility, RoutableSystemIdentifier, @@ -382,10 +382,10 @@ func TestDynamicLegacyAuditApi(t *testing.T) { // Log the event to solace visibility := auditV1.Visibility_VISIBILITY_PRIVATE - ctx := context.WithValue(ctx, ContextKeyTopic, topicName) + ctxWithTopic := context.WithValue(ctx, ContextKeyTopic, topicName) assert.NoError(t, auditApi.Log( - ctx, + ctxWithTopic, event, visibility, RoutableSystemIdentifier, @@ -439,9 +439,9 @@ func TestDynamicLegacyAuditApi(t *testing.T) { // Log the event to solace visibility := auditV1.Visibility_VISIBILITY_PUBLIC - ctx := context.WithValue(ctx, ContextKeyTopic, topicName) + ctxWithTopic := context.WithValue(ctx, ContextKeyTopic, topicName) assert.NoError(t, auditApi.Log( - ctx, + ctxWithTopic, event, visibility, NewRoutableIdentifier(objectIdentifier), diff --git a/audit/api/api_mock_test.go b/audit/api/api_mock_test.go index 4ebaaf1..f69337b 100644 --- a/audit/api/api_mock_test.go +++ b/audit/api/api_mock_test.go @@ -26,12 +26,12 @@ func TestMockAuditApi_Log(t *testing.T) { }) t.Run("reject data access event", func(t *testing.T) { - event, objectIdentifier := newOrganizationAuditEvent(nil) - event.LogName = strings.Replace(event.LogName, string(EventTypeAdminActivity), string(EventTypeDataAccess), 1) - routableObjectIdentifier := NewRoutableIdentifier(objectIdentifier) + orgEvent, objIdentifier := newOrganizationAuditEvent(nil) + orgEvent.LogName = strings.Replace(orgEvent.LogName, string(EventTypeAdminActivity), string(EventTypeDataAccess), 1) + rtIdentifier := NewRoutableIdentifier(objIdentifier) assert.ErrorIs(t, auditApi.Log( - context.Background(), event, auditV1.Visibility_VISIBILITY_PUBLIC, routableObjectIdentifier), + context.Background(), orgEvent, auditV1.Visibility_VISIBILITY_PUBLIC, rtIdentifier), ErrUnsupportedEventTypeDataAccess) }) diff --git a/audit/api/base64.go b/audit/api/base64.go index a2d7b4a..c0bf93e 100644 --- a/audit/api/base64.go +++ b/audit/api/base64.go @@ -41,7 +41,7 @@ func ToBase64( } base64Str := base64.StdEncoding.EncodeToString(serializedEvent) - base64Str = base64Str + base64AuditEventV1 + base64Str += base64AuditEventV1 return &base64Str, nil } diff --git a/audit/api/builder.go b/audit/api/builder.go index 6fccb29..a71fdaa 100644 --- a/audit/api/builder.go +++ b/audit/api/builder.go @@ -296,7 +296,7 @@ func (builder *AuditLogEntryBuilder) WithResponseBody(responseBody any) *AuditLo } // WithResponseBodyBytes adds the response body as bytes (serialized json or protobuf message expected) -func (builder *AuditLogEntryBuilder) WithResponseBodyBytes(responseBody *[]byte) *AuditLogEntryBuilder { +func (builder *AuditLogEntryBuilder) WithResponseBodyBytes(responseBody []byte) *AuditLogEntryBuilder { builder.auditResponse.ResponseBodyBytes = responseBody return builder } @@ -359,9 +359,9 @@ func (builder *AuditLogEntryBuilder) Build(ctx context.Context, sequenceNumber S builder.auditMetadata.AuditLogName = fmt.Sprintf("%s/%s/logs/%s", logType.Plural(), logIdentifier, builder.auditParams.EventType) builder.auditMetadata.AuditResourceName = resourceName - var details *map[string]interface{} = nil + var details map[string]interface{} if len(builder.auditParams.Details) > 0 { - details = &builder.auditParams.Details + details = builder.auditParams.Details } // Instantiate the audit event @@ -575,7 +575,7 @@ func (builder *AuditEventBuilder) WithResponseBody(responseBody any) *AuditEvent } // WithResponseBodyBytes adds the response body as bytes (serialized json or protobuf message expected) -func (builder *AuditEventBuilder) WithResponseBodyBytes(responseBody *[]byte) *AuditEventBuilder { +func (builder *AuditEventBuilder) WithResponseBodyBytes(responseBody []byte) *AuditEventBuilder { builder.auditLogEntryBuilder.WithResponseBodyBytes(responseBody) return builder } diff --git a/audit/api/model.go b/audit/api/model.go index 50316fa..a353c63 100644 --- a/audit/api/model.go +++ b/audit/api/model.go @@ -31,14 +31,14 @@ var ErrInvalidAuthorizationHeaderValue = errors.New("invalid authorization heade var ErrInvalidBearerToken = errors.New("invalid bearer token") var ErrTokenIsNotBearerToken = errors.New("token is not a bearer token") -var objectTypeIdPattern, _ = regexp.Compile(".*/(projects|folders|organizations)/([0-9a-fA-F-]{36})(?:/.*)?") +var objectTypeIdPattern = regexp.MustCompile(".*/(projects|folders|organizations)/([0-9a-fA-F-]{36})(?:/.*)?") type ApiRequest struct { // Body // // Required: false - Body *[]byte + Body []byte // The (HTTP) request headers / gRPC metadata. // @@ -145,7 +145,7 @@ type AuditResponse struct { // elsewhere in the log record. // // Required: false - ResponseBodyBytes *[]byte + ResponseBodyBytes []byte // The http or gRPC status code. // @@ -294,7 +294,7 @@ func NewAuditLogEntry( auditResponse AuditResponse, // Optional map that is added as "details" to the message - eventMetadata *map[string]interface{}, + eventMetadata map[string]interface{}, // Required metadata auditMetadata AuditMetadata, @@ -309,9 +309,9 @@ func NewAuditLogEntry( if err != nil { return nil, errors.Join(err, ErrInvalidResponse) } - var responseLength *int64 = nil + var responseLength *int64 if responseBody != nil { - length := int64(len(*auditResponse.ResponseBodyBytes)) + length := int64(len(auditResponse.ResponseBodyBytes)) responseLength = &length } @@ -332,7 +332,7 @@ func NewAuditLogEntry( scheme := auditRequest.Request.Scheme // Initialize authorization info if available - var authorizationInfo []*auditV1.AuthorizationInfo = nil + var authorizationInfo []*auditV1.AuthorizationInfo if auditMetadata.AuditPermission != nil && auditMetadata.AuditPermissionGranted != nil { authorizationInfo = []*auditV1.AuthorizationInfo{ NewAuthorizationInfo( @@ -342,15 +342,15 @@ func NewAuditLogEntry( } // Initialize labels if available - var labels map[string]string = nil + var labels map[string]string if auditMetadata.AuditLabels != nil { labels = *auditMetadata.AuditLabels } // Initialize metadata/details - var metadata *structpb.Struct = nil + var metadata *structpb.Struct if eventMetadata != nil { - metadataStruct, err := structpb.NewStruct(*eventMetadata) + metadataStruct, err := structpb.NewStruct(eventMetadata) if err != nil { return nil, err } @@ -480,7 +480,7 @@ func NewRequestAttributes( ) *auditV1.AttributeContext_Request { rawQuery := request.URL.RawQuery - var query *string = nil + var query *string if rawQuery != nil && *rawQuery != "" { escapedQuery := url.QueryEscape(*rawQuery) query = &escapedQuery @@ -505,7 +505,7 @@ func NewRequestAttributes( } // NewAuthorizationInfo returns protobuf AuthorizationInfo for the given parameters. -func NewAuthorizationInfo(resourceName string, permission string, granted bool) *auditV1.AuthorizationInfo { +func NewAuthorizationInfo(resourceName, permission string, granted bool) *auditV1.AuthorizationInfo { return &auditV1.AuthorizationInfo{ Resource: resourceName, Permission: &permission, @@ -514,14 +514,14 @@ func NewAuthorizationInfo(resourceName string, permission string, granted bool) } // NewInsertId returns a correctly formatted insert id. -func NewInsertId(insertTime time.Time, location string, workerId string, eventSequenceNumber uint64) string { +func NewInsertId(insertTime time.Time, location, workerId string, eventSequenceNumber uint64) string { return fmt.Sprintf("%d/%s/%s/%d", insertTime.UnixNano(), location, workerId, eventSequenceNumber) } // NewResponseMetadata returns protobuf response status with status code and short message. -func NewResponseMetadata(statusCode int, numResponseItems *int64, responseSize *int64, headers map[string]string, responseTime time.Time) *auditV1.ResponseMetadata { +func NewResponseMetadata(statusCode int, numResponseItems, responseSize *int64, headers map[string]string, responseTime time.Time) *auditV1.ResponseMetadata { - var message *string = nil + var message *string if statusCode >= 400 && statusCode < 500 { text := "Client error" message = &text @@ -530,7 +530,7 @@ func NewResponseMetadata(statusCode int, numResponseItems *int64, responseSize * message = &text } - var size *wrapperspb.Int64Value = nil + var size *wrapperspb.Int64Value if responseSize != nil { size = wrapperspb.Int64(*responseSize) } @@ -548,26 +548,26 @@ func NewResponseMetadata(statusCode int, numResponseItems *int64, responseSize * } // NewResponseBody converts the JSON byte response into a protobuf struct. -func NewResponseBody(response *[]byte) (*structpb.Struct, error) { +func NewResponseBody(response []byte) (*structpb.Struct, error) { // Return if nil - if response == nil || len(*response) == 0 { + if len(response) == 0 { return nil, nil } // Convert to protobuf struct - return byteArrayToPbStruct(*response) + return byteArrayToPbStruct(response) } // NewRequestBody converts the request body into a protobuf struct. func NewRequestBody(request *ApiRequest) (*structpb.Struct, error) { - if request.Body == nil || len(*request.Body) == 0 { + if len(request.Body) == 0 { return nil, nil } // Convert to protobuf struct - return byteArrayToPbStruct(*request.Body) + return byteArrayToPbStruct(request.Body) } // byteArrayToPbStruct converts a given json byte array into a protobuf struct. @@ -636,14 +636,17 @@ func AuditAttributesFromAuthorizationHeader(request *ApiRequest) ( error, ) { + var authenticationPrincipal = "none/none" var principalId = "none" var principalEmail = EmailAddressDoNotReplyAtStackItDotCloud - emptyClaims, _ := structpb.NewStruct(make(map[string]interface{})) + emptyClaims, err := structpb.NewStruct(make(map[string]interface{})) + if err != nil { + return nil, authenticationPrincipal, nil, nil, err + } var auditClaims = emptyClaims - var authenticationPrincipal = "none/none" - var serviceAccountName *string = nil + var serviceAccountName *string audiences := make([]string, 0) - var delegationInfo []*auditV1.ServiceAccountDelegationInfo = nil + var delegationInfo []*auditV1.ServiceAccountDelegationInfo authorizationHeaders := request.Header["Authorization"] if len(authorizationHeaders) == 0 { @@ -652,7 +655,7 @@ func AuditAttributesFromAuthorizationHeader(request *ApiRequest) ( } authorizationHeader := strings.Join(authorizationHeaders, ",") trimmedAuthorizationHeader := strings.TrimSpace(authorizationHeader) - if len(trimmedAuthorizationHeader) > 0 { + if trimmedAuthorizationHeader != "" { // Parse claims token, err := parseToken(trimmedAuthorizationHeader) @@ -712,9 +715,8 @@ func getTokenClaim(token jwt.Token, claimName string) *string { if claimExists { claimString := fmt.Sprintf("%s", claim) return &claimString - } else { - return nil } + return nil } func extractAuthenticationPrincipal(token jwt.Token) string { @@ -792,9 +794,8 @@ func extractServiceAccountDelegationInfoDetails(actClaims map[string]interface{} nestedDelegations := extractServiceAccountDelegationInfo(actClaims) if len(nestedDelegations) > 0 { return append(delegations, nestedDelegations...) - } else { - return delegations } + return delegations } func extractServiceAccountDelegationInfo(claims map[string]interface{}) []*auditV1.ServiceAccountDelegationInfo { @@ -840,7 +841,7 @@ func extractSubjectAndEmail(token jwt.Token) (string, string) { // - PATCH - update // - DELETE - delete // - others - read -func OperationNameFromUrlPath(path string, requestMethod string) string { +func OperationNameFromUrlPath(path, requestMethod string) string { queryIdx := strings.Index(path, "?") if queryIdx != -1 { path = path[:queryIdx] @@ -862,13 +863,12 @@ func OperationNameFromUrlPath(path string, requestMethod string) string { operation = strings.ReplaceAll(operation, "/", ".") operation = strings.TrimPrefix(operation, ".") operation = strings.ToLower(operation) - if len(operation) > 0 { + if operation != "" { method := StringToHttpMethod(requestMethod) var action string switch method { - case auditV1.AttributeContext_HTTP_METHOD_PUT: - fallthrough - case auditV1.AttributeContext_HTTP_METHOD_PATCH: + case auditV1.AttributeContext_HTTP_METHOD_PUT, + auditV1.AttributeContext_HTTP_METHOD_PATCH: action = "update" case auditV1.AttributeContext_HTTP_METHOD_POST: action = "create" @@ -936,14 +936,14 @@ func StringAttributeFromMetadata(metadata map[string][]string, name string) stri } // ResponseBodyToBytes converts a JSON or Protobuf response into a byte array -func ResponseBodyToBytes(response any) (*[]byte, error) { +func ResponseBodyToBytes(response any) ([]byte, error) { if response == nil { return nil, nil } responseBytes, isBytes := response.([]byte) if isBytes { - return &responseBytes, nil + return responseBytes, nil } responseProtoMessage, isProtoMessage := response.(proto.Message) @@ -952,12 +952,13 @@ func ResponseBodyToBytes(response any) (*[]byte, error) { if err != nil { return nil, err } - return &responseJson, nil - } else { - responseJson, err := json.Marshal(response) - if err != nil { - return nil, err - } - return &responseJson, nil + return responseJson, nil } + + responseJson, err := json.Marshal(response) + if err != nil { + return nil, err + } + return responseJson, nil + } diff --git a/audit/api/model_test.go b/audit/api/model_test.go index b4ef29a..8cf5cc8 100644 --- a/audit/api/model_test.go +++ b/audit/api/model_test.go @@ -787,7 +787,7 @@ func Test_NewAuditLogEntry(t *testing.T) { Proto: "HTTP/1.1", Scheme: "http", Header: requestHeaders, - Body: &requestBodyBytes, + Body: requestBodyBytes, } clientIp := "127.0.0.1" @@ -814,7 +814,7 @@ func Test_NewAuditLogEntry(t *testing.T) { responseTime := time.Now().UTC() auditResponse := AuditResponse{ - ResponseBodyBytes: &responseBody, + ResponseBodyBytes: responseBody, ResponseStatusCode: responseStatusCode, ResponseHeaders: responseHeader, ResponseNumItems: &responseNumItems, @@ -853,7 +853,7 @@ func Test_NewAuditLogEntry(t *testing.T) { logEntry, _ := NewAuditLogEntry( auditRequest, auditResponse, - &eventMetadata, + eventMetadata, auditMetadata) assert.Equal(t, logName, logEntry.LogName) @@ -1104,7 +1104,7 @@ func Test_ResponseBodyToBytes(t *testing.T) { responseBody := []byte("data") bytes, err := ResponseBodyToBytes(responseBody) assert.Nil(t, err) - assert.Equal(t, &responseBody, bytes) + assert.Equal(t, responseBody, bytes) }, ) @@ -1116,7 +1116,7 @@ func Test_ResponseBodyToBytes(t *testing.T) { expected, err := protojson.Marshal(&protobufMessage) assert.Nil(t, err) - assert.Equal(t, &expected, bytes) + assert.Equal(t, expected, bytes) }, ) @@ -1132,7 +1132,7 @@ func Test_ResponseBodyToBytes(t *testing.T) { expected, err := json.Marshal(responseBody) assert.Nil(t, err) - assert.Equal(t, &expected, bytes) + assert.Equal(t, expected, bytes) }, ) @@ -1145,7 +1145,7 @@ func Test_ResponseBodyToBytes(t *testing.T) { expected, err := json.Marshal(responseBody) assert.Nil(t, err) - assert.Equal(t, &expected, bytes) + assert.Equal(t, expected, bytes) }, ) } diff --git a/audit/api/trace.go b/audit/api/trace.go index 33a02ca..06a35a8 100644 --- a/audit/api/trace.go +++ b/audit/api/trace.go @@ -24,7 +24,7 @@ func TraceParentAndStateFromContext(ctx context.Context) (string, string) { } // AddTraceParentAndStateToContext adds trace and state related information to the given context. -func AddTraceParentAndStateToContext(ctx context.Context, traceParent string, traceState string) context.Context { +func AddTraceParentAndStateToContext(ctx context.Context, traceParent, traceState string) context.Context { mapCarrier := propagation.MapCarrier{} mapCarrier[traceParentHeader] = traceParent mapCarrier[traceStateHeader] = traceState diff --git a/audit/messaging/amqp_connection.go b/audit/messaging/amqp_connection.go index b585d33..c39f725 100644 --- a/audit/messaging/amqp_connection.go +++ b/audit/messaging/amqp_connection.go @@ -10,7 +10,7 @@ import ( "time" ) -var ConnectionClosedError = errors.New("amqp connection is closed") +var ErrConnectionClosed = errors.New("amqp connection is closed") type AmqpConnection struct { connectionName string @@ -115,7 +115,7 @@ func (c *AmqpConnection) NewSender(ctx context.Context, topic string) (*AmqpSend } if c.internalIsClosed() { - return nil, ConnectionClosedError + return nil, ErrConnectionClosed } c.lock.RLock() diff --git a/audit/messaging/amqp_connection_pool.go b/audit/messaging/amqp_connection_pool.go index b014c91..9fa3f7a 100644 --- a/audit/messaging/amqp_connection_pool.go +++ b/audit/messaging/amqp_connection_pool.go @@ -137,9 +137,9 @@ func (p *AmqpConnectionPool) NewHandle() *ConnectionPoolHandle { defer p.lock.Unlock() offset := p.handleOffset - p.handleOffset += 1 + p.handleOffset++ - offset = offset % p.config.PoolSize + offset %= p.config.PoolSize return &ConnectionPoolHandle{ connectionOffset: offset, @@ -224,7 +224,6 @@ func (p *AmqpConnectionPool) nextConnectionForHandle(handle *ConnectionPoolHandl func (p *AmqpConnectionPool) connectionIndex(handle *ConnectionPoolHandle, iteration int) int { if iteration+handle.connectionOffset >= p.config.PoolSize { return (iteration + handle.connectionOffset) % p.config.PoolSize - } else { - return iteration + handle.connectionOffset } + return iteration + handle.connectionOffset } diff --git a/audit/utils/sequence_generator.go b/audit/utils/sequence_generator.go index c06c60e..c83ec54 100644 --- a/audit/utils/sequence_generator.go +++ b/audit/utils/sequence_generator.go @@ -40,7 +40,7 @@ func (g *DefaultSequenceNumberGenerator) Next() uint64 { var next uint64 if len(g.backlog) == 0 { next = g.sequenceNumber - g.sequenceNumber += 1 + g.sequenceNumber++ } else { next = g.backlog[0] g.backlog = g.backlog[1:] @@ -53,7 +53,7 @@ func (g *DefaultSequenceNumberGenerator) Revert(value uint64) { g.sequenceNumberLock.Lock() defer g.sequenceNumberLock.Unlock() if value == g.sequenceNumber-1 { - g.sequenceNumber -= 1 + g.sequenceNumber-- } else if !slices.Contains(g.backlog, value) { g.backlog = append(g.backlog, value) } diff --git a/log/log.go b/log/log.go index fb5ba88..097210f 100644 --- a/log/log.go +++ b/log/log.go @@ -16,11 +16,12 @@ type Logger interface { func wrapErr(err []error) error { var e error - if len(err) == 0 { + switch { + case len(err) == 0: e = nil - } else if len(err) == 1 { + case len(err) == 1: e = err[0] - } else { + default: e = errors.Join(err...) } return e