mirror of
https://github.com/hashicorp/vault.git
synced 2026-02-03 20:40:45 -05:00
VAULT-40224: pipeline(github): limit comments bodies and pull request messages to 65536 characters (#10771) (#10877)
* pipeline(github): limit comments bodies and pull request messages to 65536 characters * comment/message cleanup Co-authored-by: Luis (LT) Carbonell <lt.carbonell@hashicorp.com>
This commit is contained in:
parent
77f1980d29
commit
9f946960bc
5 changed files with 127 additions and 3 deletions
|
|
@ -303,6 +303,7 @@ func (r *CopyPullRequestReq) Run(
|
|||
err = fmt.Errorf("creating copy pull request body %w", err)
|
||||
return res, errors.Join(cherryPickErr, err)
|
||||
}
|
||||
limitedPRBody := limitCharacters(prBody)
|
||||
|
||||
res.PullRequest, _, err = github.PullRequests.Create(
|
||||
ctx, r.ToOwner, r.ToRepo, &libgithub.NewPullRequest{
|
||||
|
|
@ -310,7 +311,7 @@ func (r *CopyPullRequestReq) Run(
|
|||
Head: &branchName,
|
||||
HeadRepo: &r.ToRepo,
|
||||
Base: &baseRef,
|
||||
Body: &prBody,
|
||||
Body: &limitedPRBody,
|
||||
},
|
||||
)
|
||||
if err != nil {
|
||||
|
|
|
|||
|
|
@ -697,13 +697,14 @@ func (r *CreateBackportReq) backportRef(
|
|||
res.Error = fmt.Errorf("creating backport pull request body %w", err)
|
||||
return res
|
||||
}
|
||||
limitedPRBody := limitCharacters(prBody)
|
||||
res.PullRequest, _, err = github.PullRequests.Create(
|
||||
ctx, r.Owner, r.Repo, &libgithub.NewPullRequest{
|
||||
Title: &prTitle,
|
||||
Head: &branchName,
|
||||
HeadRepo: &r.Repo,
|
||||
Base: &ref,
|
||||
Body: &prBody,
|
||||
Body: &limitedPRBody,
|
||||
},
|
||||
)
|
||||
if err != nil {
|
||||
|
|
|
|||
|
|
@ -30,9 +30,10 @@ func createPullRequestComment(
|
|||
slog.Default().DebugContext(ctx, "creating pull request comment")
|
||||
|
||||
// Always try and write a comment on the pull request
|
||||
limitedBody := limitCharacters(body)
|
||||
comment, _, err := github.Issues.CreateComment(
|
||||
ctx, owner, repo, pullNumber, &libgithub.IssueComment{
|
||||
Body: &body,
|
||||
Body: &limitedBody,
|
||||
},
|
||||
)
|
||||
if err != nil {
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ package github
|
|||
import (
|
||||
"bytes"
|
||||
"embed"
|
||||
"fmt"
|
||||
"io"
|
||||
"os"
|
||||
"text/template"
|
||||
|
|
@ -71,3 +72,40 @@ func renderEmbeddedTemplate(name string, data any) (string, error) {
|
|||
|
||||
return buf.String(), nil
|
||||
}
|
||||
|
||||
// limitCharacters truncates a string to maxGitHubMessageChars while preserving
|
||||
// markdown formatting integrity. It adds a truncation notice if truncation occurs.
|
||||
const maxGitHubMessageChars = 65536
|
||||
|
||||
func limitCharacters(content string) string {
|
||||
if len(content) <= maxGitHubMessageChars {
|
||||
return content
|
||||
}
|
||||
|
||||
// Define the truncation notice to calculate its length
|
||||
truncationNotice := "\n\n---\n\n:scissors: **Message truncated due to GitHub's character limit**\n\n" +
|
||||
fmt.Sprintf("This message was automatically truncated because it exceeded GitHub's %d character limit for "+
|
||||
"comments and pull request descriptions.", maxGitHubMessageChars)
|
||||
|
||||
// Calculate how much space the notice needs
|
||||
noticeLen := len(truncationNotice)
|
||||
maxContentLen := maxGitHubMessageChars - noticeLen
|
||||
|
||||
// Find a good truncation point to avoid breaking markdown
|
||||
truncateAt := maxContentLen
|
||||
for truncateAt > 0 && content[truncateAt] != '\n' {
|
||||
truncateAt--
|
||||
}
|
||||
|
||||
// If we can't find a newline within reasonable bounds, just truncate
|
||||
if truncateAt == 0 || (maxContentLen-truncateAt) > 1000 {
|
||||
truncateAt = maxContentLen
|
||||
}
|
||||
|
||||
// Ensure we don't go out of bounds
|
||||
if truncateAt > len(content) {
|
||||
truncateAt = len(content)
|
||||
}
|
||||
|
||||
return content[:truncateAt] + truncationNotice
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,83 @@
|
|||
// Copyright IBM Corp. 2016, 2025
|
||||
// SPDX-License-Identifier: BUSL-1.1
|
||||
|
||||
package github
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
func Test_limitCharacters(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
content string
|
||||
expected func(string) bool
|
||||
}{
|
||||
{
|
||||
name: "short content is unchanged",
|
||||
content: "This is a short message",
|
||||
expected: func(result string) bool {
|
||||
return result == "This is a short message"
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "content at limit is unchanged",
|
||||
content: strings.Repeat("a", maxGitHubMessageChars),
|
||||
expected: func(result string) bool {
|
||||
return len(result) == maxGitHubMessageChars && strings.HasPrefix(result, "aaa")
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "content over limit is truncated",
|
||||
content: strings.Repeat("a", maxGitHubMessageChars+1000),
|
||||
expected: func(result string) bool {
|
||||
return len(result) <= maxGitHubMessageChars &&
|
||||
strings.Contains(result, "Message truncated due to GitHub's character limit")
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "markdown content with newlines truncates at newline",
|
||||
content: strings.Repeat("line\n", maxGitHubMessageChars/5+100),
|
||||
expected: func(result string) bool {
|
||||
return len(result) <= maxGitHubMessageChars &&
|
||||
strings.Contains(result, "Message truncated due to GitHub's character limit") &&
|
||||
strings.HasPrefix(result, "line\n")
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "very long line without newlines gets hard truncated",
|
||||
content: strings.Repeat("a", maxGitHubMessageChars+100),
|
||||
expected: func(result string) bool {
|
||||
return len(result) <= maxGitHubMessageChars &&
|
||||
strings.Contains(result, "Message truncated due to GitHub's character limit")
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
result := limitCharacters(tt.content)
|
||||
assert.True(t, tt.expected(result), "Result does not match expected criteria")
|
||||
assert.LessOrEqual(t, len(result), maxGitHubMessageChars, "Result exceeds maximum character limit")
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func Test_limitCharacters_preserves_original_under_limit(t *testing.T) {
|
||||
content := "# PR Title\n\nThis is the body content.\n\n## Details\n\nSome details here."
|
||||
result := limitCharacters(content)
|
||||
assert.Equal(t, content, result, "Short content should be unchanged")
|
||||
}
|
||||
|
||||
func Test_limitCharacters_adds_truncation_notice(t *testing.T) {
|
||||
longContent := strings.Repeat("This is a very long line that will exceed the GitHub character limit. ", maxGitHubMessageChars/70+10)
|
||||
result := limitCharacters(longContent)
|
||||
|
||||
assert.LessOrEqual(t, len(result), maxGitHubMessageChars, "Result should not exceed max chars")
|
||||
assert.Contains(t, result, ":scissors: **Message truncated due to GitHub's character limit**", "Should contain truncation notice")
|
||||
assert.Contains(t, result, fmt.Sprintf("%d character limit", maxGitHubMessageChars), "Should mention the specific limit")
|
||||
}
|
||||
Loading…
Reference in a new issue