Improve debuggability of PullRequest PipelineResource

[bobcatfish]

Expected Behavior

If I make simple mistakes in my usage of the PullRequest PipelineResource, I should get an informative error. For example errors like:

• Using the wrong fieldName for the secret (Can't use PullRequest PipelineResource to update status #1777)
• Using the wrong format for a file that represents a mutation to make (e.g. a comment)

Actual Behavior

In some cases, things just succeed silently but do nothing. In other cases you get #1777 where things fail cryptically.

Steps to Reproduce the Problem

See #1777 to repro the fieldName issue (basically use something besides authToken for the secret https://github.com/tektoncd/pipeline/blob/master/docs/resources.md#pull-request-resource).

To reproduce an issue where the wrong format is used, you can use the old format (e.g. https://github.com/bobcatfish/catservice/blob/master/tekton/set-status.yaml) for a status update (pre #1550). The old format was:

        cat <<EOF > /workspace/output/pr/status/tekton.json
        {
          "ID": "tekton",
          "Code": "$(inputs.params.status)",
          "Description": "Tekton is running"
        }
        EOF

The new format is:

        cat <<EOF > /workspace/output/pr/status/poop.json
        {
          "Label": "poop",
          "State": "$(inputs.params.status)",
          "Desc": "Tekton is running"
        }
        EOF

When I try to use the wrong format, the Task silently passes and in the logs I can see:

{"level":"info","ts":1578434462.1463728,"logger":"fallback-logger","caller":"pullrequest/api.go:284","msg":"Skipping setting  because it already matches","resource_type":"pullrequest","mode":"upload","provider":"github","owner":"bobcatfish","repo":"tekton-lab","pr":"4"}

And then I sadly gaze at my open pull request where nothing has happened (true story).

Additional Info

Since as @wlynch has pointed out that some of the trouble is the way the github api responds (e.g. if you dont specify the token properly, you end up with a 404 from github), one idea for how we can handle this is to add more validation:

• If we see an unexpected fieldname in secrets, fail early
• Have pullrequest-init do as much validation as possible on the format of the files on disk

[vdemeester]

/kind feature

[adshmh]

@bobcatfish if there are other validations that would be helpful, I can submit a PR. So far, the secrets field and the uploaded status are covered.

[tekton-robot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close.

/lifecycle stale

Send feedback to tektoncd/plumbing.

[tekton-robot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

/close

Send feedback to tektoncd/plumbing.

[tekton-robot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close.

/lifecycle rotten

Send feedback to tektoncd/plumbing.

[tekton-robot]

@tekton-robot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

/close

Send feedback to tektoncd/plumbing.

Instructions 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.

[vdemeester]

/remove-lifecycle rotten
/remove-lifecycle stale
/reopen

[tekton-robot]

@vdemeester: Reopened this issue.

In response to this:

/remove-lifecycle rotten
/remove-lifecycle stale
/reopen

Instructions 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.

[tekton-robot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close.

/lifecycle stale

Send feedback to tektoncd/plumbing.

[tekton-robot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten with a justification.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle rotten

Send feedback to tektoncd/plumbing.

[bobcatfish]

I think it's reasonable to close this for now, esp. since the future of this pipeline resource is uncertain. I think we'll be supporting this logic somewhere regardless of whether it's part of a pipelineresource or not but i think it's fine to close this for now.

[tekton-robot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen with a justification.
Mark the issue as fresh with /remove-lifecycle rotten with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/close

Send feedback to tektoncd/plumbing.

[tekton-robot]

@tekton-robot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen with a justification.
Mark the issue as fresh with /remove-lifecycle rotten with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/close

Send feedback to tektoncd/plumbing.

Instructions 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.