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

Match existing hook without payload secret #55

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

totahuanocotl
Copy link

What is the purpose of this pull request?

  • Bug fix
  • Documentation update
  • Code cleanup
  • New feature
  • Other (please explain): React to a potential change in github api response.

What changes did you make? (Give a brief overview)
Change the desired hook config used to match against existing hooks to not include the secret property if its value is undefined.

Without this change, the _.isMatch function is not able to find a match in the existing pipelines, and attempts to create the webhook again, resulting in an api error.

The error returned by the resource before this changes looked like the following:

Error while calling Github: StatusCodeError
Response Status: 422
Message: {
  "message": "Validation Failed",
  "errors": [
    {
      "resource": "Hook",
      "code": "custom",
      "message": "Hook already exists on this repository"
    },
    {
      "resource": "Hook",
      "code": "custom",
      "message": "Invalid config attribute: content-type"
    }
  ],
  "documentation_url": "https://docs.github.com/rest/repos/webhooks#create-a-repository-webhook",
  "status": "422"
}

Is there anything specific you would like reviewers to focus on?
No

The exsiting hook matcher started failing for the scenario where the
webhook does not define a payload_secret. It's content was set to
undefined, and the _.isMatch function used to filter the hooks was
returning undefined, as it didn't match any of the retrieved hooks,
even when it should have. This resulted in an attempt to create the
webhook, even if it exsited; github returned a 422 error.

This changes the desired config, to not include the secret property
if its value is undefined. With this change, the _.isMatch function
is able to find the match, and correctly updates or no-ops the hook.

The error returned by the resource before this changes looked like
the following:
```
Error while calling Github: StatusCodeError
Response Status: 422
Message: {
  "message": "Validation Failed",
  "errors": [
    {
      "resource": "Hook",
      "code": "custom",
      "message": "Hook already exists on this repository"
    },
    {
      "resource": "Hook",
      "code": "custom",
      "message": "Invalid config attribute: content-type"
    }
  ],
  "documentation_url": "https://docs.github.com/rest/repos/webhooks#create-a-repository-webhook",
  "status": "422"
}
```
@joshbrucker
Copy link
Contributor

@billimek @GavinFigueroa This looks like a good change that resolves an issue that our previous PR introduced.

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

Successfully merging this pull request may close these issues.

2 participants