Skip to content

Commit 932d12b

Browse files
committed
fix: use logger only for github messages
Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
1 parent a1c54ee commit 932d12b

File tree

5 files changed

+148
-39
lines changed

5 files changed

+148
-39
lines changed

cmd/flag_types.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ func (o *outputType) String() string {
4141
func (o *outputType) Set(value string) error {
4242
switch value {
4343
case string(gitHub):
44-
logger = loggerConfig.SetGithubOutput()
44+
logger = loggerConfig.SetGithubOutput(os.Stdout)
4545
fallthrough
4646
case string(text):
4747
rootValues.output = outputType(value)

cmd/regex_compare.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -132,8 +132,8 @@ func performCompare(processAll bool, ctx *processors.Context) error {
132132
if err != nil && len(chainOffsetString) > 0 {
133133
return errors.New("failed to match chain offset. Value must not be larger than 255")
134134
}
135-
regex := runAssemble(filePath, ctx)
136-
err = processRegexForCompare(id, uint8(chainOffset), regex, ctx)
135+
rx := runAssemble(filePath, ctx)
136+
err = processRegexForCompare(id, uint8(chainOffset), rx, ctx)
137137
if err != nil && errors.Is(err, &ComparisonError{}) {
138138
failed = true
139139
return nil
@@ -148,9 +148,8 @@ func performCompare(processAll bool, ctx *processors.Context) error {
148148
if err != nil {
149149
logger.Fatal().Err(err).Msg("Failed to compare expressions")
150150
}
151-
if failed && rootValues.output == gitHub {
152-
fmt.Println("::error::All rules need to be up to date.",
153-
"Please run `crs-toolchain regex update --all`")
151+
if failed {
152+
logger.Error().Msg("All rules need to be up to date. Please run `crs-toolchain regex update --all`")
154153
return &ComparisonError{}
155154
}
156155
} else {

cmd/regex_format.go

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ import (
2424

2525
const (
2626
regexAssemblyStandardHeader = "##! Please refer to the documentation at\n##! https://coreruleset.org/docs/development/regex_assembly/.\n"
27-
showCharsAround = 20
2827
)
2928

3029
// formatCmd represents the generate command
@@ -149,18 +148,14 @@ func processAll(ctxt *processors.Context, checkOnly bool) error {
149148
return err
150149
}
151150
if failed {
152-
if rootValues.output == gitHub {
153-
fmt.Println("::error::All assembly files need to be properly formatted.",
154-
"Please run `crs-toolchain regex format --all`")
155-
}
151+
logger.Error().Msg("All assembly files need to be properly formatted. Please run `crs-toolchain regex format --all`")
156152
return &UnformattedFileError{}
157153
}
158154
return nil
159155
}
160156

161157
func processFile(filePath string, ctxt *processors.Context, checkOnly bool) error {
162158
var processFileError error
163-
message := ""
164159
filename := path.Base(filePath)
165160
logger.Info().Msgf("Processing %s", filename)
166161
file, err := os.Open(filePath)
@@ -213,8 +208,7 @@ func processFile(filePath string, ctxt *processors.Context, checkOnly bool) erro
213208
}
214209
equalContent := bytes.Equal(currentContents, newContents)
215210
if !equalContent || foundUppercase {
216-
message = formatMessage(fmt.Sprintf("File %s not properly formatted", filePath))
217-
fmt.Println(message)
211+
logger.Error().Msgf("File %s not properly formatted", filePath)
218212
processFileError = &UnformattedFileError{filePath: filePath}
219213
}
220214
} else {
@@ -281,13 +275,6 @@ func processLine(line []byte, indent int) ([]byte, int, error) {
281275
return trimmedLine, nextIndent, nil
282276
}
283277

284-
func formatMessage(message string) string {
285-
if rootValues.output == gitHub {
286-
message = fmt.Sprintf("::warning ::%s\n", message)
287-
}
288-
return message
289-
}
290-
291278
func formatEndOfFile(lines []string) []string {
292279
eof := len(lines) - 1
293280
if eof < 0 {

logger/logger.go

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package logger
55

66
import (
77
"fmt"
8+
"io"
89
"os"
910

1011
"github.com/rs/zerolog"
@@ -13,31 +14,23 @@ import (
1314

1415
const DefaultLogLevel zerolog.Level = zerolog.InfoLevel
1516

16-
var consoleOutput = zerolog.ConsoleWriter{Out: os.Stderr, TimeFormat: "03:04:05"}
17-
1817
func init() {
19-
log.Logger = log.Output(consoleOutput).With().Caller().Logger()
18+
log.Logger = log.Output(zerolog.ConsoleWriter{Out: os.Stderr, TimeFormat: "03:04:05"}).With().Caller().Logger()
2019
zerolog.SetGlobalLevel(DefaultLogLevel)
2120
}
2221

23-
// SetGithubOutput changes the standard logging format to be compatible with GitHub's.
24-
// See https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#example-creating-an-annotation-for-an-error
25-
// Levels on github are:
26-
// - debug, notice, error, warning
27-
// Another possibility is to add the following strings between the level and the message:
28-
// file={name},line={line},endLine={endLine},title={title}
29-
func SetGithubOutput() zerolog.Logger {
30-
// the following formatlevel loosely translates from posix levels to github levels
31-
consoleOutput.FormatLevel = func(i interface{}) string {
22+
func SetGithubOutput(w io.Writer) zerolog.Logger {
23+
ghOutput := zerolog.ConsoleWriter{Out: w, TimeFormat: "03:04:05"}
24+
ghOutput.FormatLevel = func(i interface{}) string {
3225
var l string
3326
if ll, ok := i.(string); ok {
3427
switch ll {
3528
case zerolog.LevelTraceValue, zerolog.LevelDebugValue:
3629
l = "debug"
3730
case zerolog.LevelInfoValue:
38-
l = "notice "
31+
l = "notice"
3932
case zerolog.LevelWarnValue:
40-
l = "warn "
33+
l = "warn"
4134
case zerolog.LevelErrorValue, zerolog.LevelFatalValue, zerolog.LevelPanicValue:
4235
l = "error "
4336
default:
@@ -50,9 +43,15 @@ func SetGithubOutput() zerolog.Logger {
5043
}
5144
return fmt.Sprintf("::%s", l)
5245
}
53-
consoleOutput.FormatMessage = func(i interface{}) string {
54-
return fmt.Sprintf("::%s", i)
46+
ghOutput.FormatMessage = func(i interface{}) string {
47+
return fmt.Sprintf("::%s\n", i)
48+
}
49+
ghOutput.PartsExclude = []string{zerolog.TimestampFieldName, zerolog.CallerFieldName}
50+
ghOutput.PartsOrder = []string{
51+
zerolog.LevelFieldName,
52+
zerolog.MessageFieldName,
5553
}
56-
consoleOutput.PartsExclude = []string{zerolog.TimestampFieldName}
57-
return zerolog.New(consoleOutput).With().Logger()
54+
ghOutput.NoColor = true
55+
56+
return log.Output(ghOutput).With().Caller().Logger()
5857
}

logger/logger_test.go

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
// Copyright 2024 OWASP ModSecurity Core Rule Set Project
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package logger
5+
6+
import (
7+
"bytes"
8+
"testing"
9+
10+
"github.com/rs/zerolog"
11+
"github.com/stretchr/testify/suite"
12+
)
13+
14+
type loggerTestSuite struct {
15+
suite.Suite
16+
out *bytes.Buffer
17+
logger zerolog.Logger
18+
}
19+
20+
func TestRunLoggerTestSuite(t *testing.T) {
21+
suite.Run(t, new(loggerTestSuite))
22+
}
23+
24+
var testJsonBase = []struct {
25+
name string
26+
text string
27+
logType zerolog.Level
28+
want string
29+
}{
30+
{
31+
name: "JsonBaseOutput",
32+
text: "hello",
33+
logType: zerolog.InfoLevel,
34+
want: "message\":\"hello\"",
35+
},
36+
}
37+
38+
var testConsoleBase = []struct {
39+
name string
40+
text string
41+
logType zerolog.Level
42+
want string
43+
}{
44+
{
45+
name: "BaseConsoleOutput",
46+
text: "hello",
47+
logType: zerolog.InfoLevel,
48+
want: "INF hello component=parser-test",
49+
},
50+
}
51+
52+
var testGithub = []struct {
53+
name string
54+
text string
55+
logType zerolog.Level
56+
want string
57+
}{
58+
{
59+
name: "TestGithubInfoOutput",
60+
text: "this is an info message",
61+
logType: zerolog.InfoLevel,
62+
want: "::notice ::this is an info message",
63+
},
64+
{
65+
name: "TestGithubWarningOutput",
66+
text: "this is a warning message",
67+
logType: zerolog.WarnLevel,
68+
want: "::warn ::this is a warning message",
69+
},
70+
{
71+
name: "TestGithubTraceOutput",
72+
text: "this is a trace message that will show as debug",
73+
logType: zerolog.TraceLevel,
74+
want: "::debug ::this is a trace message that will show as debug",
75+
},
76+
{
77+
name: "TestGithubDebugOutput",
78+
text: "this is a debug message",
79+
logType: zerolog.DebugLevel,
80+
want: "::debug ::this is a debug message",
81+
},
82+
}
83+
84+
func (s *loggerTestSuite) SetupTest() {
85+
// reset logger
86+
s.out = &bytes.Buffer{}
87+
s.logger = zerolog.New(s.out).With().Str("component", "parser-test").Logger()
88+
zerolog.SetGlobalLevel(zerolog.TraceLevel)
89+
}
90+
91+
func (s *loggerTestSuite) TestJsonOutput() {
92+
for _, t := range testJsonBase {
93+
s.Run(t.name, func() {
94+
s.logger.WithLevel(t.logType).Msg(t.text)
95+
s.Contains(s.out.String(), t.want)
96+
s.out.Reset()
97+
})
98+
}
99+
}
100+
101+
func (s *loggerTestSuite) TestConsoleOutput() {
102+
s.logger = s.logger.Output(zerolog.ConsoleWriter{Out: s.out, NoColor: true, TimeFormat: "03:04:05"})
103+
for _, t := range testConsoleBase {
104+
s.Run(t.name, func() {
105+
s.logger.WithLevel(t.logType).Msg(t.text)
106+
s.Contains(s.out.String(), t.want)
107+
s.out.Reset()
108+
})
109+
}
110+
}
111+
112+
//s.log = log.Output(zerolog.ConsoleWriter{Out: os.Stderr, TimeFormat: "03:04:05"}).With().Caller().Logger()
113+
114+
func (s *loggerTestSuite) TestSetGithubOutput() {
115+
// send logs to buffer
116+
logger := SetGithubOutput(s.out)
117+
for _, t := range testGithub {
118+
s.Run(t.name, func() {
119+
logger.WithLevel(t.logType).Msg(t.text)
120+
s.Contains(s.out.String(), t.want)
121+
s.out.Reset()
122+
})
123+
}
124+
}

0 commit comments

Comments
 (0)