Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot comment on PR #12

Closed
asiegman opened this issue Jul 15, 2019 · 7 comments
Closed

Cannot comment on PR #12

asiegman opened this issue Jul 15, 2019 · 7 comments

Comments

@asiegman
Copy link

Given the following config in codefresh to use the container:

  add_url_to_comment_on_pr:
    title: Comment on PR with the deployed URL
    stage: Deploy
    image: cloudposse/github-commenter
    environment:
      - GITHUB_TOKEN=${{GITHUB_REPO_STATUS_TOKEN}}
      - GITHUB_OWNER=${{CF_REPO_OWNER}}
      - GITHUB_REPO=${{CF_REPO_NAME}}
      - GITHUB_COMMENT_TYPE=pr
      - GITHUB_PR_ISSUE_NUMBER=${{CF_PULL_REQUEST_NUMBER}}
      - GITHUB_COMMENT_FORMAT="My comment:<br/>{{.}}"
      - GITHUB_COMMENT="Version ${{CF_SHORT_REVISION}} should be up, the health page is at http://${{APP_HOST}}/healthz/"
    when:
      condition:
        all:
          deployLabel: "match('${{CF_PULL_REQUEST_LABELS}}', 'deploy', false) == true"
          githubNotificationsEnabled: "'${{GITHUB_NOTIFICATIONS_ENABLED}}' == 'true'"

The container exits with the following error

2019/07/15 21:26:44 POST https://api.github.com/repos/example-org/example-app/issues/82/comments: 404 Not Found []      

It looks like the URL is incorrect according to the documentation here:

https://developer.github.com/v3/pulls/comments/#create-a-comment

POST /repos/:owner/:repo/pulls/:pull_number/comments

If I get time, I'll try to fix this issue myself, just ran out of work day today.

@asiegman
Copy link
Author

It seems while you're using the github API library, your code treats PR ands Issues the same:

https://github.com/cloudposse/github-commenter/blob/master/main.go#L238

I haven't had a chance to look at the github library to see if that follows their logic as well.

@osterman
Copy link
Member

@asiegman we use this successfully to create PR comments here:

https://github.com/cloudposse/testing.cloudposse.co/blob/master/codefresh/terraform/pipeline.yml

and demonstrated here: cloudposse/testing.cloudposse.co#75

In my experience, the 404 Not Found [] usually (and misleadingly) means that the token used was incorrect.

@osterman
Copy link
Member

I think it's missing: cf_export GITHUB_COMMENT_TYPE=pr

@Nuru
Copy link
Contributor

Nuru commented Jul 15, 2019

I think it's missing: cf_export GITHUB_COMMENT_TYPE=pr

It is setting GITHUB_COMMENT_TYPE=pr explicitly in the environment of the comment step, so it does not need to use cf_export (which is there to allow the environment to carry over from step to step). You can verify that it is being set by looking at the log output and seeing that it is correctly trying to comment on the issue by number. (Internally to GitHub, issue and pr are nearly synonymous.)

I think the issue is that the token needs but does not have full repo scope to comment on a private repo, unlike the status updater which only needs repo:status,public_repo scopes. GitHub likes to return 404 rather than 401 or 403 when you try to access a protected resource to prevent people from mapping out the structure of private repos.

@asiegman
Copy link
Author

It's got GITHUB_COMMENT_TYPE=pr in the environment section of the Codefresh step.

If it is a token issue, that 404 is very misleading, it should be a 403, but I'm sure you're repeating what's coming from Github. That's what made me check the docs and see that a PR comment had a different URL from what was displayed in the error.

I'll keep seeing if I can narrow down the issue. It might be my usage, or something weird in github, and not an actual issue with this binary.

@osterman
Copy link
Member

If it is a token issue, that 404 is very misleading, it should be a 403, but I'm sure you're repeating what's coming from Github.

GitHub returns the 404, we're not casting it to a 403.

@asiegman
Copy link
Author

I changed the token to have full repo permissions and that worked. May want to make a note of that in the README, it says you only need status and public repo permissions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants