Skip to content

Commit 304069d

Browse files
asiyaniasiyani
authored
refector PR comments parsing (#426)
* refector PR comments parsing * fix auto plan disabled msg * fix invisible char issue * log metadata parsing failures * fix large output msg truncation * fix default not needed
1 parent 2d57971 commit 304069d

File tree

4 files changed

+237
-88
lines changed

4 files changed

+237
-88
lines changed

‎prplanner/msg.go‎

Lines changed: 98 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
package prplanner
22

33
import (
4+
"encoding/json"
45
"fmt"
6+
"log/slog"
57
"regexp"
68
"strings"
79
"time"
@@ -11,14 +13,21 @@ import (
1113
"k8s.io/apimachinery/pkg/types"
1214
)
1315

16+
const (
17+
// Metadata Prefix to identify our comments
18+
metaStart = "<!-- terraform-applier-pr-planner-metadata:"
19+
metaEnd = " -->"
20+
)
21+
1422
var (
1523
planReqMsgRegex = regexp.MustCompile("^`?@terraform-applier plan `?([\\w-.\\/]+)`?$")
1624

25+
// find our hidden JSON block
26+
metadataRegex = regexp.MustCompile(fmt.Sprintf(`(?s)%s(.*?)%s`, regexp.QuoteMeta(metaStart), regexp.QuoteMeta(metaEnd)))
27+
1728
autoPlanDisabledTml = "Auto plan is disabled for this PR.\n" +
1829
"Please post `@terraform-applier plan <module_name>` as comment if you want to request terraform plan for a particular module."
1930

20-
autoPlanDisabledRegex = regexp.MustCompile("Auto plan is disabled for this PR")
21-
2231
requestAcknowledgedMsgTml = "Received terraform plan request\n" +
2332
"```\n" +
2433
"Cluster: %s\n" +
@@ -30,8 +39,6 @@ var (
3039
"Do not edit this comment. This message will be updated once the plan run is completed.\n" +
3140
"To manually trigger plan again please post `@terraform-applier plan %s` as comment."
3241

33-
requestAcknowledgedMsgRegex = regexp.MustCompile(`Received terraform plan request\n\x60{3}\nCluster: (.+)\nModule: (.+)\nPath: (.+)\nCommit ID: (.+)\nRequested At: (.+)`)
34-
3542
runOutputMsgTml = "Terraform run output for\n" +
3643
"```\n" +
3744
"Cluster: %s\n" +
@@ -42,10 +49,57 @@ var (
4249
"<details><summary><b>%s Run Status: %s, Run Summary: %s</b></summary>" +
4350
"\n\n```terraform\n%s\n```\n</details>\n" +
4451
"\n> To manually trigger plan again please post `@terraform-applier plan %s` as comment."
52+
)
53+
54+
type MsgType string
4555

46-
runOutputMsgRegex = regexp.MustCompile(`Terraform (?:plan|run) output for\n\x60{3}\nCluster: (.+)\nModule: (.+)\nPath: (.+)\nCommit ID: (.+)\n`)
56+
const (
57+
MsgTypePlanRequest MsgType = "PlanRequest"
58+
MsgTypeRunOutput MsgType = "RunOutput"
59+
MsgTypeAutoPlanDisabled MsgType = "AutoPlanDisabled"
4760
)
4861

62+
// CommentMetadata is the hidden JSON structure
63+
type CommentMetadata struct {
64+
Type MsgType `json:"type"`
65+
Cluster string `json:"cluster,omitempty"`
66+
Module string `json:"module,omitempty"` // Stores "Namespace/Name"
67+
Path string `json:"path,omitempty"`
68+
CommitID string `json:"commit_id,omitempty"`
69+
ReqAt string `json:"req_at,omitempty"` // RFC3339 String
70+
}
71+
72+
// embedMetadata serializes the struct into a hidden HTML comment
73+
func embedMetadata(meta CommentMetadata) string {
74+
b, err := json.Marshal(meta)
75+
if err != nil {
76+
panic(fmt.Sprintf("unable to marshal pr comment metadata: %v", meta))
77+
}
78+
return fmt.Sprintf("\n\n%s %s %s", metaStart, string(b), metaEnd)
79+
}
80+
81+
// extractMetadata parses the hidden JSON from a comment body
82+
func extractMetadata(commentBody string) *CommentMetadata {
83+
matches := metadataRegex.FindStringSubmatch(commentBody)
84+
if len(matches) < 2 {
85+
return nil
86+
}
87+
88+
rawJson := matches[1]
89+
90+
// GitHub markdown/browsers often convert spaces to Non-Breaking Spaces (\u00A0)
91+
// or inject odd formatting.
92+
rawJson = strings.ReplaceAll(rawJson, "\u00A0", "")
93+
rawJson = strings.TrimSpace(rawJson)
94+
95+
var meta CommentMetadata
96+
if err := json.Unmarshal([]byte(rawJson), &meta); err != nil {
97+
slog.Error("unable to parse PR comment metadata json", "logger", "pr-planner", "err", err)
98+
return nil
99+
}
100+
return &meta
101+
}
102+
49103
func parsePlanReqMsg(commentBody string) string {
50104
matches := planReqMsgRegex.FindStringSubmatch(commentBody)
51105

@@ -57,29 +111,39 @@ func parsePlanReqMsg(commentBody string) string {
57111
}
58112

59113
func requestAcknowledgedMsg(cluster, module, path, commitID string, reqAt *metav1.Time) string {
60-
return fmt.Sprintf(requestAcknowledgedMsgTml, cluster, module, path, commitID, reqAt.Format(time.RFC3339), path)
114+
display := fmt.Sprintf(requestAcknowledgedMsgTml, cluster, module, path, commitID, reqAt.Format(time.RFC3339), path)
115+
116+
meta := CommentMetadata{
117+
Type: MsgTypePlanRequest,
118+
Cluster: cluster,
119+
Module: module,
120+
Path: path,
121+
CommitID: commitID,
122+
ReqAt: reqAt.Format(time.RFC3339),
123+
}
124+
125+
return display + embedMetadata(meta)
61126
}
62127

63128
func parseRequestAcknowledgedMsg(commentBody string) (cluster string, module types.NamespacedName, path string, commID string, ReqAt *time.Time) {
64-
matches := requestAcknowledgedMsgRegex.FindStringSubmatch(commentBody)
65-
if len(matches) == 6 {
66-
t, err := time.Parse(time.RFC3339, matches[5])
67-
if err == nil {
68-
return matches[1], parseNamespaceName(matches[2]), matches[3], matches[4], &t
69-
}
70-
return matches[1], parseNamespaceName(matches[2]), matches[3], matches[4], nil
129+
meta := extractMetadata(commentBody)
130+
if meta == nil || meta.Type != MsgTypePlanRequest {
131+
return
132+
}
133+
134+
if t, err := time.Parse(time.RFC3339, meta.ReqAt); err == nil {
135+
ReqAt = &t
71136
}
72137

73-
return
138+
return meta.Cluster, parseNamespaceName(meta.Module), meta.Path, meta.CommitID, ReqAt
74139
}
75140

76141
func parseRunOutputMsg(comment string) (cluster string, module types.NamespacedName, path string, commit string) {
77-
matches := runOutputMsgRegex.FindStringSubmatch(comment)
78-
if len(matches) == 5 {
79-
return matches[1], parseNamespaceName(matches[2]), matches[3], matches[4]
142+
meta := extractMetadata(comment)
143+
if meta == nil || meta.Type != MsgTypeRunOutput {
144+
return
80145
}
81-
82-
return
146+
return meta.Cluster, parseNamespaceName(meta.Module), meta.Path, meta.CommitID
83147
}
84148

85149
func runOutputMsg(cluster, module, path string, run *v1beta1.Run) string {
@@ -100,10 +164,20 @@ func runOutputMsg(cluster, module, path string, run *v1beta1.Run) string {
100164

101165
if len(runes) > characterLimit {
102166
runOutput = "Plan output has reached the max character limit of " + fmt.Sprintf("%d", characterLimit) + " characters. " +
103-
"The output is truncated from the top.\n" + string(runes[characterLimit:])
167+
"The output is truncated from the top.\n" + string(runes[(len(runes)-characterLimit):])
168+
}
169+
170+
display := fmt.Sprintf(runOutputMsgTml, cluster, module, path, run.CommitHash, statusSymbol, run.Status, run.Summary, runOutput, path)
171+
172+
meta := CommentMetadata{
173+
Type: MsgTypeRunOutput,
174+
Cluster: cluster,
175+
Module: module,
176+
Path: path,
177+
CommitID: run.CommitHash,
104178
}
105179

106-
return fmt.Sprintf(runOutputMsgTml, cluster, module, path, run.CommitHash, statusSymbol, run.Status, run.Summary, runOutput, path)
180+
return display + embedMetadata(meta)
107181
}
108182

109183
func parseNamespaceName(str string) types.NamespacedName {
@@ -122,7 +196,8 @@ func parseNamespaceName(str string) types.NamespacedName {
122196

123197
func isAutoPlanDisabledCommentPosted(prComments []prComment) bool {
124198
for _, comment := range prComments {
125-
if autoPlanDisabledRegex.MatchString(comment.Body) {
199+
meta := extractMetadata(comment.Body)
200+
if meta != nil && meta.Type == MsgTypeAutoPlanDisabled {
126201
return true
127202
}
128203
}
@@ -131,7 +206,5 @@ func isAutoPlanDisabledCommentPosted(prComments []prComment) bool {
131206

132207
// IsSelfComment will return true if comments matches TF applier comment templates
133208
func IsSelfComment(comment string) bool {
134-
return runOutputMsgRegex.MatchString(comment) ||
135-
requestAcknowledgedMsgRegex.MatchString(comment) ||
136-
autoPlanDisabledRegex.MatchString(comment)
209+
return strings.Contains(comment, metaStart)
137210
}

‎prplanner/msg_test.go‎

Lines changed: 47 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ package prplanner
22

33
import (
44
"fmt"
5-
reflect "reflect"
5+
"reflect"
66
"testing"
77
"time"
88

@@ -64,7 +64,7 @@ func Test_parsePlanReqMsg(t *testing.T) {
6464
},
6565
{
6666
name: "do not trigger plan on module limit comment",
67-
args: args{commentBody: autoPlanDisabledTml},
67+
args: args{commentBody: autoPlanDisabledTml + embedMetadata(CommentMetadata{Type: MsgTypeAutoPlanDisabled})},
6868
wantModuleNameOrPath: "",
6969
},
7070
{
@@ -128,7 +128,15 @@ func Test_requestAcknowledgedMsg(t *testing.T) {
128128
"Requested At: 2006-01-02T15:04:05+07:00\n" +
129129
"```\n" +
130130
"Do not edit this comment. This message will be updated once the plan run is completed.\n" +
131-
"To manually trigger plan again please post `@terraform-applier plan path/to/module/one` as comment.",
131+
"To manually trigger plan again please post `@terraform-applier plan path/to/module/one` as comment." +
132+
embedMetadata(CommentMetadata{
133+
Type: MsgTypePlanRequest,
134+
Cluster: "default",
135+
Module: "foo/one",
136+
Path: "path/to/module/one",
137+
CommitID: "hash1",
138+
ReqAt: "2006-01-02T15:04:05+07:00",
139+
}),
132140
},
133141
}
134142
for _, tt := range tests {
@@ -269,7 +277,14 @@ func Test_runOutputMsg(t *testing.T) {
269277
"Terraform apply output....\n" +
270278
"```\n" +
271279
"</details>\n" +
272-
"\n> To manually trigger plan again please post `@terraform-applier plan path/baz/one` as comment.",
280+
"\n> To manually trigger plan again please post `@terraform-applier plan path/baz/one` as comment." +
281+
embedMetadata(CommentMetadata{
282+
Type: MsgTypeRunOutput,
283+
Cluster: "default",
284+
Module: "baz/one",
285+
Path: "path/baz/one",
286+
CommitID: "hash2",
287+
}),
273288
},
274289
{
275290
"2",
@@ -287,7 +302,14 @@ func Test_runOutputMsg(t *testing.T) {
287302
"Some Init Output...\nSome TF Output .....\n" +
288303
"```\n" +
289304
"</details>\n" +
290-
"\n> To manually trigger plan again please post `@terraform-applier plan path/baz/one` as comment.",
305+
"\n> To manually trigger plan again please post `@terraform-applier plan path/baz/one` as comment." +
306+
embedMetadata(CommentMetadata{
307+
Type: MsgTypeRunOutput,
308+
Cluster: "default",
309+
Module: "baz/one",
310+
Path: "path/baz/one",
311+
CommitID: "hash2",
312+
}),
291313
}, {
292314
"3",
293315
args{cluster: "default", module: "baz/one", path: "path/baz/one", run: &v1beta1.Run{Status: v1beta1.StatusOk, DiffDetected: true, CommitHash: "hash2", Summary: "Applied: x to add, x to change, x to destroy.", Output: "Terraform apply output...."}},
@@ -304,7 +326,14 @@ func Test_runOutputMsg(t *testing.T) {
304326
"Terraform apply output....\n" +
305327
"```\n" +
306328
"</details>\n" +
307-
"\n> To manually trigger plan again please post `@terraform-applier plan path/baz/one` as comment.",
329+
"\n> To manually trigger plan again please post `@terraform-applier plan path/baz/one` as comment." +
330+
embedMetadata(CommentMetadata{
331+
Type: MsgTypeRunOutput,
332+
Cluster: "default",
333+
Module: "baz/one",
334+
Path: "path/baz/one",
335+
CommitID: "hash2",
336+
}),
308337
},
309338
}
310339
for _, tt := range tests {
@@ -330,27 +359,6 @@ func Test_parseRunOutputMsg(t *testing.T) {
330359
wantPath string
331360
wantCommit string
332361
}{
333-
{
334-
name: "old msg-NamespaceName + Commit ID",
335-
args: args{comment: "Terraform plan output for\n" +
336-
"```\n" +
337-
"Cluster: default\n" +
338-
"Module: baz/one\n" +
339-
"Path: path/baz/one\n" +
340-
"Commit ID: hash12\n" +
341-
"```\n" +
342-
"<details><summary><b>⛔ Run Status: Errored, Run Summary: unable to plan module</b></summary>\n\n" +
343-
"```" +
344-
"terraform\n" +
345-
"Some Init Output...\nSome TF Output .....\n" +
346-
"```\n" +
347-
"</details>\n" +
348-
"\n> To manually trigger plan again please post `@terraform-applier plan path/baz/one` as comment."},
349-
wantModule: types.NamespacedName{Namespace: "baz", Name: "one"},
350-
wantCluster: "default",
351-
wantPath: "path/baz/one",
352-
wantCommit: "hash12",
353-
},
354362
{
355363
name: "NamespaceName + Commit ID",
356364
args: args{comment: runOutputMsg("default", "baz/one", "foo/one", &v1beta1.Run{CommitHash: "hash2", Summary: "Plan: x to add, x to change, x to destroy."})},
@@ -368,18 +376,12 @@ func Test_parseRunOutputMsg(t *testing.T) {
368376
wantCommit: "hash2",
369377
},
370378
{
371-
name: "Module Name only",
372-
args: args{comment: runOutputMsg("default", "one", "foo/one", &v1beta1.Run{Summary: "Plan: x to add, x to change, x to destroy."})},
373-
wantModule: types.NamespacedName{},
374-
wantPath: "",
375-
wantCommit: "",
376-
},
377-
{
378-
name: "Module Name missing",
379-
args: args{comment: runOutputMsg("default", "", "foo/one", &v1beta1.Run{CommitHash: "hash2", Summary: "Plan: x to add, x to change, x to destroy."})},
380-
wantModule: types.NamespacedName{},
381-
wantPath: "",
382-
wantCommit: "",
379+
name: "Module Name only",
380+
args: args{comment: runOutputMsg("default", "one", "foo/one", &v1beta1.Run{CommitHash: "", Summary: "Plan: x to add..."})},
381+
wantModule: types.NamespacedName{Name: "one"},
382+
wantCluster: "default",
383+
wantPath: "foo/one",
384+
wantCommit: "",
383385
},
384386
{
385387
name: "Module Name + Commit ID",
@@ -513,21 +515,21 @@ func Test_isAutoPlanDisabledCommentPosted(t *testing.T) {
513515
},
514516
{
515517
name: "comment posted",
516-
args: args{prComments: []prComment{{Body: autoPlanDisabledTml}, {Body: "random comment"}}},
518+
args: args{prComments: []prComment{{Body: autoPlanDisabledTml + embedMetadata(CommentMetadata{Type: MsgTypeAutoPlanDisabled})}, {Body: "random comment"}}},
517519
want: true,
518520
},
519521
}
520522

521523
for _, tt := range tests {
522524
t.Run(tt.name, func(t *testing.T) {
523525
if got := isAutoPlanDisabledCommentPosted(tt.args.prComments); !reflect.DeepEqual(got, tt.want) {
524-
t.Errorf("parseNamespaceName() = %v, want %v", got, tt.want)
526+
t.Errorf("isAutoPlanDisabledCommentPosted() = %v, want %v", got, tt.want)
525527
}
526528
})
527529
}
528530
}
529531

530-
func Test_isSelfAddedComment(t *testing.T) {
532+
func Test_IsSelfComment(t *testing.T) {
531533
type args struct {
532534
comment string
533535
}
@@ -537,15 +539,16 @@ func Test_isSelfAddedComment(t *testing.T) {
537539
want bool
538540
}{
539541
{"empty", args{""}, false},
540-
{"autoPlanDisabledTml", args{autoPlanDisabledTml}, true},
542+
// Updated to simulate comments WITH metadata
543+
{"autoPlanDisabledTml", args{autoPlanDisabledTml + embedMetadata(CommentMetadata{Type: MsgTypeAutoPlanDisabled})}, true},
541544
{"requestAcknowledgedMsg", args{requestAcknowledgedMsg("default", "foo/one", "path/to/module/one", "hash1", mustParseMetaTime("2006-01-02T15:04:05+07:00"))}, true},
542545
{"runOutputMsg", args{runOutputMsg("default", "one", "foo/one", &v1beta1.Run{CommitHash: "hash2", Summary: "Plan: x to add, x to change, x to destroy."})}, true},
543546
{"other", args{"other"}, false},
544547
}
545548
for _, tt := range tests {
546549
t.Run(tt.name, func(t *testing.T) {
547550
if got := IsSelfComment(tt.args.comment); got != tt.want {
548-
t.Errorf("isSelfAddedComment() = %v, want %v", got, tt.want)
551+
t.Errorf("IsSelfComment() = %v, want %v", got, tt.want)
549552
}
550553
})
551554
}

‎prplanner/planner.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ func (p *Planner) processPullRequest(ctx context.Context, pr *pr, kubeModuleList
124124
if skipCommitRun {
125125
// add limit msg comment if not already added
126126
if !isAutoPlanDisabledCommentPosted(pr.Comments.Nodes) {
127-
comment := prComment{Body: autoPlanDisabledTml}
127+
comment := prComment{Body: autoPlanDisabledTml + embedMetadata(CommentMetadata{Type: MsgTypeAutoPlanDisabled})}
128128
_, err := p.github.postComment(pr.BaseRepository.Owner.Login, pr.BaseRepository.Name, 0, pr.Number, comment)
129129
if err != nil {
130130
p.Log.Error("unable to post limit reached msg", "err", err)

0 commit comments

Comments
 (0)