Skip to content

Commit 8b5d68b

Browse files
authored
lint-monster: replace magic firewall status codes and collapse codemod helper args via options struct (#42049)
1 parent cfe175a commit 8b5d68b

5 files changed

Lines changed: 47 additions & 37 deletions

File tree

pkg/cli/codemod_engine_max_runs.go

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,15 @@ func getEngineMaxRunsToTopLevelCodemod() Codemod {
1616
return migrateEngineFieldToTopLevel(
1717
content,
1818
frontmatter,
19-
"max-runs",
20-
"max-turns",
21-
[]string{"max-runs", "max-turns"},
22-
engineMaxRunsCodemodLog,
23-
"Skipping engine.max-runs migration for inline-map engine syntax; migrate to top-level max-turns manually",
24-
"Removed deprecated engine.max-runs (top-level max-runs/max-turns already present)",
25-
"Migrated engine.max-runs to top-level max-turns",
19+
migrateEngineFieldToTopLevelOptions{
20+
engineField: "max-runs",
21+
targetTopLevelField: "max-turns",
22+
preserveTopLevelFields: []string{"max-runs", "max-turns"},
23+
log: engineMaxRunsCodemodLog,
24+
skipInlineMessage: "Skipping engine.max-runs migration for inline-map engine syntax; migrate to top-level max-turns manually",
25+
removedMessage: "Removed deprecated engine.max-runs (top-level max-runs/max-turns already present)",
26+
migratedMessage: "Migrated engine.max-runs to top-level max-turns",
27+
},
2628
)
2729
},
2830
}

pkg/cli/codemod_engine_max_turns.go

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,15 @@ func getEngineMaxTurnsToTopLevelCodemod() Codemod {
1616
return migrateEngineFieldToTopLevel(
1717
content,
1818
frontmatter,
19-
"max-turns",
20-
"max-turns",
21-
[]string{"max-turns"},
22-
engineMaxTurnsCodemodLog,
23-
"Skipping engine.max-turns migration for inline-map engine syntax; migrate to top-level max-turns manually",
24-
"Removed deprecated engine.max-turns (top-level max-turns already present)",
25-
"Migrated engine.max-turns to top-level max-turns",
19+
migrateEngineFieldToTopLevelOptions{
20+
engineField: "max-turns",
21+
targetTopLevelField: "max-turns",
22+
preserveTopLevelFields: []string{"max-turns"},
23+
log: engineMaxTurnsCodemodLog,
24+
skipInlineMessage: "Skipping engine.max-turns migration for inline-map engine syntax; migrate to top-level max-turns manually",
25+
removedMessage: "Removed deprecated engine.max-turns (top-level max-turns already present)",
26+
migratedMessage: "Migrated engine.max-turns to top-level max-turns",
27+
},
2628
)
2729
},
2830
}

pkg/cli/codemod_engine_to_top_level_helpers.go

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,20 @@ import (
66
"github.com/github/gh-aw/pkg/logger"
77
)
88

9+
type migrateEngineFieldToTopLevelOptions struct {
10+
engineField string
11+
targetTopLevelField string
12+
preserveTopLevelFields []string
13+
log *logger.Logger
14+
skipInlineMessage string
15+
removedMessage string
16+
migratedMessage string
17+
}
18+
919
func migrateEngineFieldToTopLevel(
1020
content string,
1121
frontmatter map[string]any,
12-
engineField string,
13-
targetTopLevelField string,
14-
preserveTopLevelFields []string,
15-
log *logger.Logger,
16-
skipInlineMessage string,
17-
removedMessage string,
18-
migratedMessage string,
22+
opts migrateEngineFieldToTopLevelOptions,
1923
) (string, bool, error) {
2024
engineValue, hasEngine := frontmatter["engine"]
2125
if !hasEngine {
@@ -25,12 +29,12 @@ func migrateEngineFieldToTopLevel(
2529
if !ok {
2630
return content, false, nil
2731
}
28-
if _, hasEngineField := engineMap[engineField]; !hasEngineField {
32+
if _, hasEngineField := engineMap[opts.engineField]; !hasEngineField {
2933
return content, false, nil
3034
}
3135

3236
hasPreservedTopLevelField := false
33-
for _, field := range preserveTopLevelFields {
37+
for _, field := range opts.preserveTopLevelFields {
3438
if _, exists := frontmatter[field]; exists {
3539
hasPreservedTopLevelField = true
3640
break
@@ -44,9 +48,9 @@ func migrateEngineFieldToTopLevel(
4448
continue
4549
}
4650
inlineValue := strings.TrimSpace(strings.TrimPrefix(trimmed, "engine:"))
47-
if strings.HasPrefix(inlineValue, "{") && strings.Contains(inlineValue, engineField+":") {
48-
if log != nil {
49-
log.Print(skipInlineMessage)
51+
if strings.HasPrefix(inlineValue, "{") && strings.Contains(inlineValue, opts.engineField+":") {
52+
if opts.log != nil {
53+
opts.log.Print(opts.skipInlineMessage)
5054
}
5155
return lines, false
5256
}
@@ -55,7 +59,7 @@ func migrateEngineFieldToTopLevel(
5559
fieldSuffix := ""
5660
inEngineBlock := false
5761
engineIndent := ""
58-
engineFieldPrefix := engineField + ":"
62+
engineFieldPrefix := opts.engineField + ":"
5963
for _, line := range lines {
6064
trimmed := strings.TrimSpace(line)
6165
if isTopLevelKey(line) && strings.HasPrefix(trimmed, "engine:") {
@@ -75,14 +79,14 @@ func migrateEngineFieldToTopLevel(
7579
}
7680
}
7781

78-
result, removed := removeFieldFromBlock(lines, engineField, "engine")
82+
result, removed := removeFieldFromBlock(lines, opts.engineField, "engine")
7983
if !removed {
8084
return lines, false
8185
}
8286

8387
if hasPreservedTopLevelField {
84-
if log != nil {
85-
log.Print(removedMessage)
88+
if opts.log != nil {
89+
opts.log.Print(opts.removedMessage)
8690
}
8791
return result, true
8892
}
@@ -95,14 +99,14 @@ func migrateEngineFieldToTopLevel(
9599
}
96100
}
97101

98-
topLevelLine := targetTopLevelField + ":" + fieldSuffix
102+
topLevelLine := opts.targetTopLevelField + ":" + fieldSuffix
99103
withTopLevel := make([]string, 0, len(result)+1)
100104
withTopLevel = append(withTopLevel, result[:insertAt]...)
101105
withTopLevel = append(withTopLevel, topLevelLine)
102106
withTopLevel = append(withTopLevel, result[insertAt:]...)
103107

104-
if log != nil {
105-
log.Print(migratedMessage)
108+
if opts.log != nil {
109+
opts.log.Print(opts.migratedMessage)
106110
}
107111
return withTopLevel, true
108112
})

pkg/cli/firewall_log.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package cli
33
import (
44
"bufio"
55
"fmt"
6+
"net/http"
67
"os"
78
"path/filepath"
89
"regexp"
@@ -243,10 +244,10 @@ func parseFirewallLogLine(line string) *FirewallLogEntry {
243244
func isRequestAllowed(decision, status string) bool {
244245
// Check status code first
245246
if statusCode, err := strconv.Atoi(status); err == nil {
246-
if statusCode == 200 || statusCode == 206 || statusCode == 304 {
247+
if statusCode == http.StatusOK || statusCode == http.StatusPartialContent || statusCode == http.StatusNotModified {
247248
return true
248249
}
249-
if statusCode == 403 || statusCode == 407 {
250+
if statusCode == http.StatusForbidden || statusCode == http.StatusProxyAuthRequired {
250251
return false
251252
}
252253
}

pkg/cli/firewall_policy.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"bufio"
55
"encoding/json"
66
"fmt"
7+
"net/http"
78
"os"
89
"path/filepath"
910
"regexp"
@@ -234,10 +235,10 @@ func isEntryHTTPS(entry AuditLogEntry) bool {
234235
// This mirrors the classification logic used by the firewall log parser.
235236
func isEntryAllowed(entry AuditLogEntry) bool {
236237
status := entry.Status
237-
if status == 200 || status == 206 || status == 304 {
238+
if status == http.StatusOK || status == http.StatusPartialContent || status == http.StatusNotModified {
238239
return true
239240
}
240-
if status == 403 || status == 407 {
241+
if status == http.StatusForbidden || status == http.StatusProxyAuthRequired {
241242
return false
242243
}
243244
decision := entry.Decision

0 commit comments

Comments
 (0)