Repository navigation
fix(github-add-comment): add 0.8 with bounded results to avoid 4096 byte limit - #1386
vdemeester wants to merge 2 commits into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Diff between version 0.7 and 0.8diff --git a/task/github-add-comment/0.7/README.md b/task/github-add-comment/0.8/README.md
index f3951b9..e05aac2 100644
--- a/task/github-add-comment/0.7/README.md
+++ b/task/github-add-comment/0.8/README.md
@@ -6,7 +6,7 @@ issue.
## Install the Task
```
-kubectl apply -f https://api.hub.tekton.dev/v1/resource/tekton/task/github-add-comment/0.7/raw
+kubectl apply -f https://api.hub.tekton.dev/v1/resource/tekton/task/github-add-comment/0.8/raw
```
## Secrets
@@ -42,8 +42,13 @@ See GitHub's documentation on [Understanding scopes for OAuth Apps](https://deve
## Results
-- **OLD_COMMENT:**: The old text of the comment, if any.
-- **NEW_COMMENT:**: The new text of the comment, if any.
+- **COMMENT_ID:**: The numeric id of the comment that was added or updated.
+- **COMMENT_URL:**: The `html_url` of the comment that was added or updated.
+
+> **Note:** In versions `0.4`–`0.7` the results (`OLD_COMMENT`/`NEW_COMMENT`)
+> contained the full GitHub API response object, which frequently exceeded the
+> 4096 byte task result (termination message) limit and caused the TaskRun to
+> fail. Starting with `0.8` only small, bounded values are written to results.
## Workspaces
diff --git a/task/github-add-comment/0.7/github-add-comment.yaml b/task/github-add-comment/0.8/github-add-comment.yaml
index 82683d8..b8f4352 100644
--- a/task/github-add-comment/0.7/github-add-comment.yaml
+++ b/task/github-add-comment/0.8/github-add-comment.yaml
@@ -4,7 +4,7 @@ kind: Task
metadata:
name: github-add-comment
labels:
- app.kubernetes.io/version: "0.7"
+ app.kubernetes.io/version: "0.8"
annotations:
tekton.dev/categories: Git
tekton.dev/pipelines.minVersion: "0.17.0"
@@ -24,11 +24,11 @@ spec:
description: The optional workspace containing comment file to be posted.
results:
- - name: OLD_COMMENT
- description: The old text of the comment, if any.
+ - name: COMMENT_ID
+ description: The numeric id of the comment that was added or updated.
- - name: NEW_COMMENT
- description: The new text of the comment, if any.
+ - name: COMMENT_URL
+ description: The html_url of the comment that was added or updated.
params:
- name: GITHUB_HOST_URL
@@ -162,8 +162,6 @@ spec:
# If more than one comment is found take the last one
matching_comment = [x for x in comments if '$(params.COMMENT_TAG)' in x['body']][-1:]
if matching_comment:
- with open("$(results.OLD_COMMENT.path)", "w") as result_old:
- result_old.write(str(matching_comment[0]))
matching_comment = matching_comment[0]['url']
if matching_comment:
@@ -189,7 +187,14 @@ spec:
print(resp.read())
sys.exit(1)
else:
- with open("$(results.NEW_COMMENT.path)", "wb") as result_new:
- result_new.write(resp.read())
+ # Only write small, bounded values to the results to avoid
+ # exceeding the 4096 byte termination message limit shared by all
+ # step results. Writing the full API response (as previous
+ # versions did) can easily overflow that limit for large comments.
+ comment = json.loads(resp.read())
+ with open("$(results.COMMENT_ID.path)", "w") as result_id:
+ result_id.write(str(comment.get("id", "")))
+ with open("$(results.COMMENT_URL.path)", "w") as result_url:
+ result_url.write(str(comment.get("html_url", "")))
print("a GitHub comment has been {} to $(params.REQUEST_URL)".format(
"updated" if matching_comment else "added"))
diff --git a/task/github-add-comment/0.7/tests/fixtures/github-post-comment.yaml b/task/github-add-comment/0.8/tests/fixtures/github-post-comment.yaml
index bdc851a..d06ee15 100644
--- a/task/github-add-comment/0.7/tests/fixtures/github-post-comment.yaml
+++ b/task/github-add-comment/0.8/tests/fixtures/github-post-comment.yaml
@@ -5,7 +5,7 @@ headers:
path: /repos/{repo:[^/]+/[^/]+}/issues/comments/{comment:[0-9]+}
response:
status: 200
- output: '{"status": 200}'
+ output: '{"id": 2222222222, "html_url": "https://github.com/tektoncd/catalog/issues/1#issuecomment-2222222222", "body": "Hello from TektonCD test<!-- TEST_TAG123 -->"}'
content-type: text/json
---
headers:
@@ -23,7 +23,7 @@ headers:
path: /api/v3/repos/{repo:[^/]+/[^/]+}/issues/comments/{comment:[0-9]+}
response:
status: 200
- output: '{"status": 200}'
+ output: '{"id": 2222222222, "html_url": "https://github.com/tektoncd/catalog/issues/1#issuecomment-2222222222", "body": "Hello from TektonCD test<!-- TEST_TAG123 -->"}'
content-type: text/json
---
headers:
diff --git a/task/github-add-comment/0.7/tests/run.yaml b/task/github-add-comment/0.8/tests/run.yaml
index 788bcd5..46b7e40 100644
--- a/task/github-add-comment/0.7/tests/run.yaml
+++ b/task/github-add-comment/0.8/tests/run.yaml
@@ -8,9 +8,9 @@ spec:
- name: API_PATH_PREFIX
type: string
default: ""
- - name: EXPECTED_OLD_COMMENT
+ - name: EXPECTED_COMMENT_ID
type: string
- default: "Comment on GH public"
+ default: "2222222222"
workspaces:
- name: comment-file
optional: true
@@ -38,22 +38,27 @@ spec:
runAfter:
- add-comment
params:
- - name: EXPECTED_OLD_COMMENT
- value: $(params.EXPECTED_OLD_COMMENT)
- - name: ACTUAL_OLD_COMMENT
- value: $(tasks.add-comment.results.OLD_COMMENT)
+ - name: EXPECTED_COMMENT_ID
+ value: $(params.EXPECTED_COMMENT_ID)
+ - name: ACTUAL_COMMENT_ID
+ value: $(tasks.add-comment.results.COMMENT_ID)
+ - name: ACTUAL_COMMENT_URL
+ value: $(tasks.add-comment.results.COMMENT_URL)
taskSpec:
params:
- - name: EXPECTED_OLD_COMMENT
+ - name: EXPECTED_COMMENT_ID
type: string
- - name: ACTUAL_OLD_COMMENT
+ - name: ACTUAL_COMMENT_ID
+ type: string
+ - name: ACTUAL_COMMENT_URL
type: string
steps:
- image: registry.access.redhat.com/ubi8/ubi-minimal:8.2
script: |
#!/usr/libexec/platform-python
- assert "$(params.EXPECTED_OLD_COMMENT)" in "$(params.ACTUAL_OLD_COMMENT)"
+ assert "$(params.EXPECTED_COMMENT_ID)" == "$(params.ACTUAL_COMMENT_ID)"
+ assert "$(params.ACTUAL_COMMENT_URL)".startswith("https://")
---
apiVersion: tekton.dev/v1beta1
kind: PipelineRun
@@ -73,5 +78,5 @@ spec:
params:
- name: API_PATH_PREFIX
value: "/api/v3"
- - name: EXPECTED_OLD_COMMENT
- value: "Comment on GHE"
+ - name: EXPECTED_COMMENT_ID
+ value: "2222222222" |
Signed-off-by: Vincent Demeester <vdemeest@redhat.com>
6d08e88 to
e1dd876
Compare
Diff between version 0.7 and 0.8diff --git a/task/github-add-comment/0.7/README.md b/task/github-add-comment/0.8/README.md
index f3951b9..e05aac2 100644
--- a/task/github-add-comment/0.7/README.md
+++ b/task/github-add-comment/0.8/README.md
@@ -6,7 +6,7 @@ issue.
## Install the Task
```
-kubectl apply -f https://api.hub.tekton.dev/v1/resource/tekton/task/github-add-comment/0.7/raw
+kubectl apply -f https://api.hub.tekton.dev/v1/resource/tekton/task/github-add-comment/0.8/raw
```
## Secrets
@@ -42,8 +42,13 @@ See GitHub's documentation on [Understanding scopes for OAuth Apps](https://deve
## Results
-- **OLD_COMMENT:**: The old text of the comment, if any.
-- **NEW_COMMENT:**: The new text of the comment, if any.
+- **COMMENT_ID:**: The numeric id of the comment that was added or updated.
+- **COMMENT_URL:**: The `html_url` of the comment that was added or updated.
+
+> **Note:** In versions `0.4`–`0.7` the results (`OLD_COMMENT`/`NEW_COMMENT`)
+> contained the full GitHub API response object, which frequently exceeded the
+> 4096 byte task result (termination message) limit and caused the TaskRun to
+> fail. Starting with `0.8` only small, bounded values are written to results.
## Workspaces
diff --git a/task/github-add-comment/0.7/github-add-comment.yaml b/task/github-add-comment/0.8/github-add-comment.yaml
index 82683d8..b8f4352 100644
--- a/task/github-add-comment/0.7/github-add-comment.yaml
+++ b/task/github-add-comment/0.8/github-add-comment.yaml
@@ -4,7 +4,7 @@ kind: Task
metadata:
name: github-add-comment
labels:
- app.kubernetes.io/version: "0.7"
+ app.kubernetes.io/version: "0.8"
annotations:
tekton.dev/categories: Git
tekton.dev/pipelines.minVersion: "0.17.0"
@@ -24,11 +24,11 @@ spec:
description: The optional workspace containing comment file to be posted.
results:
- - name: OLD_COMMENT
- description: The old text of the comment, if any.
+ - name: COMMENT_ID
+ description: The numeric id of the comment that was added or updated.
- - name: NEW_COMMENT
- description: The new text of the comment, if any.
+ - name: COMMENT_URL
+ description: The html_url of the comment that was added or updated.
params:
- name: GITHUB_HOST_URL
@@ -162,8 +162,6 @@ spec:
# If more than one comment is found take the last one
matching_comment = [x for x in comments if '$(params.COMMENT_TAG)' in x['body']][-1:]
if matching_comment:
- with open("$(results.OLD_COMMENT.path)", "w") as result_old:
- result_old.write(str(matching_comment[0]))
matching_comment = matching_comment[0]['url']
if matching_comment:
@@ -189,7 +187,14 @@ spec:
print(resp.read())
sys.exit(1)
else:
- with open("$(results.NEW_COMMENT.path)", "wb") as result_new:
- result_new.write(resp.read())
+ # Only write small, bounded values to the results to avoid
+ # exceeding the 4096 byte termination message limit shared by all
+ # step results. Writing the full API response (as previous
+ # versions did) can easily overflow that limit for large comments.
+ comment = json.loads(resp.read())
+ with open("$(results.COMMENT_ID.path)", "w") as result_id:
+ result_id.write(str(comment.get("id", "")))
+ with open("$(results.COMMENT_URL.path)", "w") as result_url:
+ result_url.write(str(comment.get("html_url", "")))
print("a GitHub comment has been {} to $(params.REQUEST_URL)".format(
"updated" if matching_comment else "added"))
diff --git a/task/github-add-comment/0.7/tests/fixtures/github-post-comment.yaml b/task/github-add-comment/0.8/tests/fixtures/github-post-comment.yaml
index bdc851a..d06ee15 100644
--- a/task/github-add-comment/0.7/tests/fixtures/github-post-comment.yaml
+++ b/task/github-add-comment/0.8/tests/fixtures/github-post-comment.yaml
@@ -5,7 +5,7 @@ headers:
path: /repos/{repo:[^/]+/[^/]+}/issues/comments/{comment:[0-9]+}
response:
status: 200
- output: '{"status": 200}'
+ output: '{"id": 2222222222, "html_url": "https://github.com/tektoncd/catalog/issues/1#issuecomment-2222222222", "body": "Hello from TektonCD test<!-- TEST_TAG123 -->"}'
content-type: text/json
---
headers:
@@ -23,7 +23,7 @@ headers:
path: /api/v3/repos/{repo:[^/]+/[^/]+}/issues/comments/{comment:[0-9]+}
response:
status: 200
- output: '{"status": 200}'
+ output: '{"id": 2222222222, "html_url": "https://github.com/tektoncd/catalog/issues/1#issuecomment-2222222222", "body": "Hello from TektonCD test<!-- TEST_TAG123 -->"}'
content-type: text/json
---
headers:
diff --git a/task/github-add-comment/0.7/tests/run.yaml b/task/github-add-comment/0.8/tests/run.yaml
index 788bcd5..46b7e40 100644
--- a/task/github-add-comment/0.7/tests/run.yaml
+++ b/task/github-add-comment/0.8/tests/run.yaml
@@ -8,9 +8,9 @@ spec:
- name: API_PATH_PREFIX
type: string
default: ""
- - name: EXPECTED_OLD_COMMENT
+ - name: EXPECTED_COMMENT_ID
type: string
- default: "Comment on GH public"
+ default: "2222222222"
workspaces:
- name: comment-file
optional: true
@@ -38,22 +38,27 @@ spec:
runAfter:
- add-comment
params:
- - name: EXPECTED_OLD_COMMENT
- value: $(params.EXPECTED_OLD_COMMENT)
- - name: ACTUAL_OLD_COMMENT
- value: $(tasks.add-comment.results.OLD_COMMENT)
+ - name: EXPECTED_COMMENT_ID
+ value: $(params.EXPECTED_COMMENT_ID)
+ - name: ACTUAL_COMMENT_ID
+ value: $(tasks.add-comment.results.COMMENT_ID)
+ - name: ACTUAL_COMMENT_URL
+ value: $(tasks.add-comment.results.COMMENT_URL)
taskSpec:
params:
- - name: EXPECTED_OLD_COMMENT
+ - name: EXPECTED_COMMENT_ID
type: string
- - name: ACTUAL_OLD_COMMENT
+ - name: ACTUAL_COMMENT_ID
+ type: string
+ - name: ACTUAL_COMMENT_URL
type: string
steps:
- image: registry.access.redhat.com/ubi8/ubi-minimal:8.2
script: |
#!/usr/libexec/platform-python
- assert "$(params.EXPECTED_OLD_COMMENT)" in "$(params.ACTUAL_OLD_COMMENT)"
+ assert "$(params.EXPECTED_COMMENT_ID)" == "$(params.ACTUAL_COMMENT_ID)"
+ assert "$(params.ACTUAL_COMMENT_URL)".startswith("https://")
---
apiVersion: tekton.dev/v1beta1
kind: PipelineRun
@@ -73,5 +78,5 @@ spec:
params:
- name: API_PATH_PREFIX
value: "/api/v3"
- - name: EXPECTED_OLD_COMMENT
- value: "Comment on GHE"
+ - name: EXPECTED_COMMENT_ID
+ value: "2222222222" |
|
cc @vinamra28 |
|
@vdemeester can you please squash the commits? |
Versions 0.4-0.7 wrote the full GitHub API response object to the OLD_COMMENT and NEW_COMMENT results. Tekton stores step results in the pod termination message, which is capped at 4096 bytes shared across all results. A full comment API response (body plus user, reactions, URLs, timestamps, etc.) routinely exceeds that limit, causing the TaskRun to fail with: Error while writing message: Termination message is above max allowed size 4096, caused by large task result. Version 0.8 replaces those results with the small, bounded COMMENT_ID and COMMENT_URL values parsed from the response. Fixes tektoncd#1369 Signed-off-by: Vincent Demeester <vdemeest@redhat.com>
3ca7141 to
dbe8a55
Compare
|
@vinamra28 done 👼🏼 I kept two as the "rules" in tektoncd/catalog
|
|
@vdemeester: PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Changes
Adds
github-add-commentversion 0.8 to fix #1369.Versions
0.4–0.7wrote the entire GitHub API response object to theOLD_COMMENTandNEW_COMMENTresults. Tekton stores step results in the podtermination message, which is capped at 4096 bytes shared across all results
of the step. A full comment response (
bodyplususerobject,reactions,all
*_urlfields, timestamps,node_id, …) routinely exceeds that limit,causing the TaskRun to fail with:
It was also a semantic bug: the results were documented as "the old/new text
of the comment" but actually contained the full API object.
Version 0.8 replaces those results with small, bounded values parsed from the
response:
COMMENT_ID— the numeric comment idCOMMENT_URL— the commenthtml_urlThis keeps the useful output (identifying/linking the comment) while staying well
under the 4096 byte limit regardless of comment size.
Commits follow the catalog copy-then-modify convention (pure copy from 0.7, then
the fix). Tests and fixtures were updated to exercise and verify the new results.
Fixes #1369
/kind bug
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
contains
/kind <type>. Valid types are bug, cleanup, design, documentation,feature, flake, misc, question, tep
<kind>/<name>/<version>/name.yamlREADME.mdat<kind>/<name>/<version>/README.mdmetadata.labels-app.kubernetes.io/versionthe same as the<version>of the resourcemetadata.annotationstekton.dev/pipelines.minVersionspec.descriptionfollows the conventionSee the contribution guide for more details.