Skip to content

Commit 1eabf1c

Browse files
theakshaypantzakisk
authored andcommitted
feat(cel): require body and headers flags
Make --body and --headers flags required since both are essential for the cel command to function. Refactor tests accordingly. Jira: https://issues.redhat.com/browse/SRVKP-9400 Signed-off-by: Akshay Pant <[email protected]>
1 parent 565420c commit 1eabf1c

File tree

3 files changed

+105
-56
lines changed

3 files changed

+105
-56
lines changed

docs/content/docs/guide/cli.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -454,8 +454,8 @@ The payload is the JSON content of the webhook request, The headers file support
454454
tkn pac cel -b <body.json> -H <headers.txt>
455455
```
456456

457-
* `-b, --body`: Path to JSON body file (webhook payload)
458-
* `-H, --headers`: Path to headers file (plain text, JSON, or gosmee script)
457+
* `-b, --body`: Path to JSON body file (webhook payload) **[required]**
458+
* `-H, --headers`: Path to headers file (plain text, JSON, or gosmee script) **[required]**
459459
* `-p, --provider`: Provider (auto, github, gitlab, bitbucket-cloud, bitbucket-datacenter, gitea)
460460

461461
#### Interactive Mode

pkg/cmd/tknpac/cel/cel.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -736,9 +736,13 @@ that would be used in PipelineRun configurations.`,
736736
Annotations: map[string]string{"commandType": "main"},
737737
}
738738

739-
cmd.Flags().StringVarP(&bodyFile, bodyFileFlag, "b", "", "path to JSON body file")
740-
cmd.Flags().StringVarP(&headersFile, headersFileFlag, "H", "", "path to headers file (JSON, HTTP format, or gosmee-generated shell script)")
739+
cmd.Flags().StringVarP(&bodyFile, bodyFileFlag, "b", "", "path to JSON body file (required)")
740+
cmd.Flags().StringVarP(&headersFile, headersFileFlag, "H", "", "path to headers file (required, JSON, HTTP format, or gosmee-generated shell script)")
741741
cmd.Flags().StringVarP(&provider, providerFlag, "p", "auto", "payload provider (auto, github, gitlab, bitbucket-cloud, bitbucket-datacenter, gitea)")
742742
cmd.Flags().StringVarP(&githubToken, githubTokenFlag, "t", "", "GitHub personal access token for API enrichment (enables full event processing)")
743+
// Mark body and headers flags as required.
744+
// These errors are safe to ignore as the flags are defined above.
745+
_ = cmd.MarkFlagRequired(bodyFileFlag)
746+
_ = cmd.MarkFlagRequired(headersFileFlag)
743747
return cmd
744748
}

pkg/cmd/tknpac/cel/cel_test.go

Lines changed: 97 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,38 @@ const testPullRequestPayload = `{
7171
}
7272
}`
7373

74+
// minimalGitHubPRPayload returns a minimal valid GitHub pull request webhook payload
75+
// for testing purposes.
76+
const minimalGitHubPRPayload = `{
77+
"action": "opened",
78+
"pull_request": {
79+
"number": 1,
80+
"title": "Test",
81+
"head": {
82+
"ref": "test-branch",
83+
"sha": "abc123",
84+
"repo": {
85+
"html_url": "https://github.com/test/test"
86+
}
87+
},
88+
"base": {
89+
"ref": "main",
90+
"repo": {
91+
"html_url": "https://github.com/test/test"
92+
}
93+
},
94+
"html_url": "https://github.com/test/test/pull/1",
95+
"labels": []
96+
},
97+
"repository": {
98+
"name": "test",
99+
"owner": {"login": "test"},
100+
"html_url": "https://github.com/test/test",
101+
"default_branch": "main"
102+
},
103+
"sender": {"login": "test"}
104+
}`
105+
74106
func newIOStream() (*cli.IOStreams, *bytes.Buffer, *bytes.Buffer) {
75107
in := &bytes.Buffer{}
76108
out := &bytes.Buffer{}
@@ -82,6 +114,38 @@ func newIOStream() (*cli.IOStreams, *bytes.Buffer, *bytes.Buffer) {
82114
}, out, errOut
83115
}
84116

117+
// setupCELTest creates temporary files with the given body and headers content,
118+
// executes the CEL command, and returns the error (if any).
119+
func setupCELTest(t *testing.T, bodyContent, headersContent, provider string) error {
120+
t.Helper()
121+
122+
tempDir := fs.NewDir(t, "cel-test")
123+
defer tempDir.Remove()
124+
125+
// Create body file
126+
bodyFile := tempDir.Join("body.json")
127+
err := os.WriteFile(bodyFile, []byte(bodyContent), 0o600)
128+
assert.NilError(t, err)
129+
130+
// Create headers file
131+
headersFile := tempDir.Join("headers.txt")
132+
err = os.WriteFile(headersFile, []byte(headersContent), 0o600)
133+
assert.NilError(t, err)
134+
135+
ioStreams, _, _ := newIOStream()
136+
// Write empty input to stdin to exit immediately
137+
ioStreams.In = io.NopCloser(strings.NewReader("\n"))
138+
139+
cmd := Command(ioStreams)
140+
cmd.SetArgs([]string{
141+
"--provider", provider,
142+
"--body", bodyFile,
143+
"--headers", headersFile,
144+
})
145+
146+
return cmd.Execute()
147+
}
148+
85149
func TestParseHTTPHeaders(t *testing.T) {
86150
tests := []struct {
87151
name string
@@ -971,7 +1035,7 @@ X-GitHub-Event: pull_request`
9711035
bodyContent: pullRequestPayload,
9721036
headersContent: headers,
9731037
provider: "github",
974-
wantErr: false, // Interactive mode should exit gracefully on EOF
1038+
wantErr: false,
9751039
wantOutContains: []string{
9761040
"Important Notice",
9771041
"Provider: github",
@@ -981,14 +1045,14 @@ X-GitHub-Event: pull_request`
9811045
name: "no files provided",
9821046
provider: "github",
9831047
wantErr: true,
984-
wantErrContains: "unknown X-Github-Event",
1048+
wantErrContains: "required flag(s) \"body\", \"headers\" not set",
9851049
},
9861050
{
9871051
name: "no body file",
9881052
headersContent: headers,
9891053
provider: "github",
9901054
wantErr: true,
991-
wantErrContains: "unexpected end of JSON input",
1055+
wantErrContains: "required flag(s) \"body\" not set",
9921056
},
9931057
{
9941058
name: "unsupported provider",
@@ -998,14 +1062,6 @@ X-GitHub-Event: pull_request`
9981062
wantErr: true,
9991063
wantErrContains: "unsupported provider invalid-provider",
10001064
},
1001-
{
1002-
name: "invalid json body",
1003-
bodyContent: `{"invalid": json}`,
1004-
headersContent: headers,
1005-
provider: "github",
1006-
wantErr: true,
1007-
wantErrContains: "invalid character",
1008-
},
10091065
}
10101066

10111067
for _, tt := range tests {
@@ -1081,77 +1137,51 @@ X-GitHub-Event: pull_request`
10811137
}
10821138
}
10831139

1084-
func TestCommandFileHandling(t *testing.T) {
1140+
func TestHeaderFormats(t *testing.T) {
10851141
tests := []struct {
10861142
name string
1143+
bodyContent string
10871144
headersContent string
1088-
isJSON bool
10891145
wantErr bool
10901146
wantErrContains string
10911147
}{
10921148
{
1093-
name: "plain text headers",
1149+
name: "plain text headers",
1150+
bodyContent: minimalGitHubPRPayload,
10941151
headersContent: `Accept: */*
10951152
Content-Type: application/json
10961153
X-GitHub-Event: pull_request`,
1097-
isJSON: false,
1098-
wantErr: true,
1099-
wantErrContains: "unexpected end of JSON input",
1154+
wantErr: false,
11001155
},
11011156
{
1102-
name: "json headers",
1157+
name: "json headers",
1158+
bodyContent: minimalGitHubPRPayload,
11031159
headersContent: `{
11041160
"Accept": "*/*",
11051161
"Content-Type": "application/json",
11061162
"X-GitHub-Event": "pull_request"
11071163
}`,
1108-
isJSON: true,
1109-
wantErr: true,
1110-
wantErrContains: "unexpected end of JSON input",
1164+
wantErr: false,
11111165
},
11121166
{
11131167
name: "invalid json headers",
1168+
bodyContent: minimalGitHubPRPayload,
11141169
headersContent: `{"invalid": json}`,
1115-
isJSON: true,
11161170
wantErr: true,
11171171
wantErrContains: "invalid character",
11181172
},
11191173
{
11201174
name: "empty headers file",
1175+
bodyContent: minimalGitHubPRPayload,
11211176
headersContent: "",
11221177
wantErr: true,
11231178
wantErrContains: "unknown X-Github-Event",
11241179
},
1125-
{
1126-
name: "gosmee script",
1127-
headersContent: `#!/usr/bin/env bash
1128-
set -euxfo pipefail
1129-
curl -sSi -H "Content-Type: application/json" -H "X-GitHub-Event: pull_request" -H "User-Agent: GitHub-Hookshot/2d5e4d4" -X POST -d @payload.json http://localhost:8080`,
1130-
wantErr: true,
1131-
wantErrContains: "unexpected end of JSON input", // Still expects body file
1132-
},
11331180
}
11341181

11351182
for _, tt := range tests {
11361183
t.Run(tt.name, func(t *testing.T) {
1137-
tempDir := fs.NewDir(t, "cel-headers-test")
1138-
defer tempDir.Remove()
1139-
1140-
headersFile := tempDir.Join("headers.txt")
1141-
err := os.WriteFile(headersFile, []byte(tt.headersContent), 0o600)
1142-
assert.NilError(t, err)
1143-
1144-
ioStreams, _, _ := newIOStream()
1145-
// Write empty input to stdin to exit immediately
1146-
ioStreams.In = io.NopCloser(strings.NewReader("\n"))
1147-
1148-
cmd := Command(ioStreams)
1149-
cmd.SetArgs([]string{
1150-
"--provider", "github",
1151-
"--headers", headersFile,
1152-
})
1153-
1154-
err = cmd.Execute()
1184+
err := setupCELTest(t, tt.bodyContent, tt.headersContent, "github")
11551185

11561186
if tt.wantErr {
11571187
assert.Assert(t, err != nil)
@@ -1174,12 +1204,12 @@ func TestCommandFlags(t *testing.T) {
11741204
bodyFlag := cmd.Flags().Lookup("body")
11751205
assert.Assert(t, bodyFlag != nil)
11761206
assert.Equal(t, bodyFlag.Shorthand, "b")
1177-
assert.Equal(t, bodyFlag.Usage, "path to JSON body file")
1207+
assert.Equal(t, bodyFlag.Usage, "path to JSON body file (required)")
11781208

11791209
headersFlag := cmd.Flags().Lookup("headers")
11801210
assert.Assert(t, headersFlag != nil)
11811211
assert.Equal(t, headersFlag.Shorthand, "H")
1182-
assert.Equal(t, headersFlag.Usage, "path to headers file (JSON, HTTP format, or gosmee-generated shell script)")
1212+
assert.Equal(t, headersFlag.Usage, "path to headers file (required, JSON, HTTP format, or gosmee-generated shell script)")
11831213

11841214
providerFlag := cmd.Flags().Lookup("provider")
11851215
assert.Assert(t, providerFlag != nil)
@@ -1222,10 +1252,22 @@ func TestInvalidFiles(t *testing.T) {
12221252
tempDir := fs.NewDir(t, "cel-invalid-test")
12231253
defer tempDir.Remove()
12241254

1255+
// Create valid body file (unless we're testing body file errors)
1256+
validBodyFile := tempDir.Join("valid-body.json")
1257+
err := os.WriteFile(validBodyFile, []byte(minimalGitHubPRPayload), 0o600)
1258+
assert.NilError(t, err)
1259+
1260+
// Create valid headers file (unless we're testing headers file errors)
1261+
validHeadersFile := tempDir.Join("valid-headers.txt")
1262+
validHeadersContent := `X-GitHub-Event: pull_request
1263+
Content-Type: application/json`
1264+
err = os.WriteFile(validHeadersFile, []byte(validHeadersContent), 0o600)
1265+
assert.NilError(t, err)
1266+
12251267
var filePath string
12261268
if tt.createFile {
12271269
filePath = tempDir.Join("test-file")
1228-
err := os.WriteFile(filePath, []byte(tt.fileContent), 0o600)
1270+
err = os.WriteFile(filePath, []byte(tt.fileContent), 0o600)
12291271
assert.NilError(t, err)
12301272
} else {
12311273
filePath = filepath.Join(tempDir.Path(), "non-existent-file")
@@ -1237,15 +1279,18 @@ func TestInvalidFiles(t *testing.T) {
12371279
cmd := Command(ioStreams)
12381280
args := []string{"--provider", "github"}
12391281

1282+
// Add both body and headers flags, using the invalid one for the test target
12401283
if tt.fileFlag == "body" {
12411284
args = append(args, "--body", filePath)
1285+
args = append(args, "--headers", validHeadersFile)
12421286
} else {
1287+
args = append(args, "--body", validBodyFile)
12431288
args = append(args, "--headers", filePath)
12441289
}
12451290

12461291
cmd.SetArgs(args)
12471292

1248-
err := cmd.Execute()
1293+
err = cmd.Execute()
12491294
assert.Assert(t, err != nil)
12501295
assert.Assert(t, strings.Contains(err.Error(), tt.wantErrContains),
12511296
"error %q should contain %q", err.Error(), tt.wantErrContains)

0 commit comments

Comments
 (0)