search

pact-foundation / pact_broker

Enables your consumer driven contracts workflow
http://pactflow.io
MIT License
708 stars 176 forks source link

Github Status checks are broken #377

Closed bheemreddy181-zz closed 3 years ago

bheemreddy181-zz commented 3 years ago

Pre issue-raising checklist

I have already (please mark the applicable with an x):

Software versions

Expected behaviour

Github pending status checks need to be overwritten by success/failure on github pull requests

Actual behaviour

Github status checking both pending and success/failure status check for the pull requests

Steps to reproduce

  1. Give if have already had pact file generated from consumer , modify the pact file to add a new interaction or change an existing interaction
  2. Publish pacts to pact broker which will publish a pending status on the github PR
  3. After provider CI is complete, you will see a both status checks on the github PR

Relevant log files

We are not seeing any logs in the pact broker open-source on-premise hosted version

Screen Shot 2021-01-26 at 5 01 46 PM
bethesque commented 3 years ago

I think this issue might be related to this: https://github.com/pact-foundation/pact_broker/issues/362

bheemreddy181-zz commented 3 years ago

@bethesque but this was broken in the initial commit where the contract was changed , there is only one commit in this PR

bethesque commented 3 years ago

I think I've reproduced the issue, but maybe there is another one.

Can you follow these instructions for me: https://github.com/pact-foundation/pact_broker/blob/issue/pact-broker-377/ISSUES.md

You will need Docker and Docker Compose

git clone git@github.com:pact-foundation/pact_broker.git
cd pact_broker
git checkout issue/pact-broker-377
docker-compose  -f docker-compose-issue-repro-with-pact-broker-docker-image.yml up pact-broker

# new window
docker-compose -f docker-compose-issue-repro-with-pact-broker-docker-image.yml up webhook-server

# new window
docker-compose -f docker-compose-issue-repro-with-pact-broker-docker-image.yml run repro-issue

If you go to the webhook-server window, you'll see that while there were two webhook executions for the publishing, there was only one for the verification, as they both had the same content, and the verifications are now de-duplicated.

webhook-server_1  | [2021-01-29 00:22:55] INFO  WEBrick::HTTPServer#start: pid=1 port=9393
webhook-server_1  | {"eventName":"${pactbroker.eventName}","consumerVersionNumber":"1","consumerVersionTags":"main","githubVerificationStatus":"pending","providerVersionNumber":"","providerVersionTags":""}
webhook-server_1  | 172.28.0.2 - - [29/Jan/2021:00:23:03 +0000] "POST / HTTP/1.1" 200 18 0.0016
webhook-server_1  | {"eventName":"${pactbroker.eventName}","consumerVersionNumber":"2","consumerVersionTags":"feat/x, feat/y","githubVerificationStatus":"pending","providerVersionNumber":"","providerVersionTags":""}
webhook-server_1  | 172.28.0.2 - - [29/Jan/2021:00:23:03 +0000] "POST / HTTP/1.1" 200 18 0.0007

webhook-server_1  | {"eventName":"${pactbroker.eventName}","consumerVersionNumber":"2","consumerVersionTags":"feat/x, feat/y","githubVerificationStatus":"success","providerVersionNumber":"1","providerVersionTags":"main"}
webhook-server_1  | 172.28.0.2 - - [29/Jan/2021:00:23:13 +0000] "POST / HTTP/1.1" 200 18 0.0006

If the scenario in the script/reproduce-issue.rb file doesn't match yours, then please fork the repo and modify it so that it does, and then let me know.

bethesque commented 3 years ago

Are you using the new "pacts for verification" api for verifying? ie. using consumer version selectors, not just just a list of tags?

bheemreddy181-zz commented 3 years ago

we are not using any API specifically all we are using is tags for verification like here as implemented in the workshop

bethesque commented 3 years ago

So you're using the Pact Go client? What version are you using?

bheemreddy181-zz commented 3 years ago

this is happening for both ruby and go lang - and coming to go package it is v1.5.1

mefellows commented 3 years ago

Looking at that code, it's just using the standard tags, not the consumer version selectors (supported).

Would it make sense to start mapping the legacy Tags to consumer version selectors behind the scenes do you think? (or does the CLI already do that anyway?)

bethesque commented 3 years ago

Can you turn on the verbose logging so we can see which API it's using @bheemreddy181? See the verbose option documentation that I've just added to https://github.com/pact-foundation/docs.pact.io/blob/master/website/docs/implementation_guides/ruby/verifying_pacts.md#fetching-pacts-from-a-pact-broker

There will be a way to do it with the Go library as well - check the docs.

mefellows commented 3 years ago

hmmm that flag is currently a noop in Pact Go, because it broke the way the verification worked. I think it's been fixed in Ruby at least, but previously the verbose logging was interspersed with the JSON output which broke the parsing.

@bheemreddy181 you could hack the Pact go lib locally to re-connect the Verbose flag to the CLI, just note that the test will return an error. It should print out the inner workings we're after though.

bethesque commented 3 years ago

If the standalone is new enough, you can just set the environment variable VERBOSE=true and it will pick it up. Or is it using the new rust code?

bethesque commented 3 years ago

The pact-provider-verifier turns tags into selectors and uses the new API https://github.com/pact-foundation/pact-provider-verifier/blob/master/lib/pact/provider_verifier/aggregate_pact_configs.rb#L56

mefellows commented 3 years ago

👏

bethesque commented 3 years ago

If it's what I think it is, this PR will fix it https://github.com/pact-foundation/pact_broker/pull/378

bheemreddy181-zz commented 3 years ago

I think I've reproduced the issue, but maybe there is another one.

Can you follow these instructions for me: https://github.com/pact-foundation/pact_broker/blob/issue/pact-broker-377/ISSUES.md

You will need Docker and Docker Compose

git clone git@github.com:pact-foundation/pact_broker.git
cd pact_broker
git checkout issue/pact-broker-377
docker-compose  -f docker-compose-issue-repro-with-pact-broker-docker-image.yml up pact-broker

# new window
docker-compose -f docker-compose-issue-repro-with-pact-broker-docker-image.yml up webhook-server

# new window
docker-compose -f docker-compose-issue-repro-with-pact-broker-docker-image.yml run repro-issue

If you go to the webhook-server window, you'll see that while there were two webhook executions for the publishing, there was only one for the verification, as they both had the same content, and the verifications are now de-duplicated.

webhook-server_1  | [2021-01-29 00:22:55] INFO  WEBrick::HTTPServer#start: pid=1 port=9393
webhook-server_1  | {"eventName":"${pactbroker.eventName}","consumerVersionNumber":"1","consumerVersionTags":"main","githubVerificationStatus":"pending","providerVersionNumber":"","providerVersionTags":""}
webhook-server_1  | 172.28.0.2 - - [29/Jan/2021:00:23:03 +0000] "POST / HTTP/1.1" 200 18 0.0016
webhook-server_1  | {"eventName":"${pactbroker.eventName}","consumerVersionNumber":"2","consumerVersionTags":"feat/x, feat/y","githubVerificationStatus":"pending","providerVersionNumber":"","providerVersionTags":""}
webhook-server_1  | 172.28.0.2 - - [29/Jan/2021:00:23:03 +0000] "POST / HTTP/1.1" 200 18 0.0007

webhook-server_1  | {"eventName":"${pactbroker.eventName}","consumerVersionNumber":"2","consumerVersionTags":"feat/x, feat/y","githubVerificationStatus":"success","providerVersionNumber":"1","providerVersionTags":"main"}
webhook-server_1  | 172.28.0.2 - - [29/Jan/2021:00:23:13 +0000] "POST / HTTP/1.1" 200 18 0.0006

If the scenario in the script/reproduce-issue.rb file doesn't match yours, then please fork the repo and modify it so that it does, and then let me know.

I will try to replicate my issue using this , do you guys still want me to do ther verbose logging ?

bheemreddy181-zz commented 3 years ago

The newer version still didn't fix it

Screen Shot 2021-01-29 at 4 02 14 PM
bheemreddy181-zz commented 3 years ago

Here is my use-case more in detail.

Consumer-A and Provider-B

  1. Consumer and Provider has a verified pact on master tag Screen Shot 2021-01-29 at 4 19 33 PM
  2. Consumer now came in changed a pact on the branch with a new commit - which in turn triggered the verification job on the provider ( pact broker sent a pending status to consumer )
  3. Provider build ran and sent a failed status of changed pact
  4. Consumer now sees both pending and failed status on the gitui
bheemreddy181-zz commented 3 years ago

I tried digging through GIT API's to see what all statuses i can see on this commit

Screen Shot 2021-01-29 at 6 22 38 PM
bheemreddy181-zz commented 3 years ago

I could see the context different for both the statues - As per the documentation the context should be same I feel https://docs.github.com/en/rest/reference/repos#create-a-commit-status , do we need to send a default context when we set pending status. Typically the provider_version tags variable we use.

Screen Shot 2021-01-30 at 10 28 02 PM

Git documentation here

Screen Shot 2021-01-30 at 10 30 15 PM
bheemreddy181-zz commented 3 years ago

I got this fixed by hardcoding the ${pactbroker.providerVersionTags} to master on the webhook , I don't think It has anything to do with pact broker upgrade this has been an issue on all the versions

Screen Shot 2021-01-31 at 9 10 01 AM

Now on the git commit statuses, I could see both status checks having the same context

Screen Shot 2021-01-31 at 9 15 13 AM

Learned something new here , curious how do we need to handle these cases ?

bethesque commented 3 years ago

Ah - that will be because when the pending status is sent, we don't know which provider branch is going to verify it. The contexts can never match. Take the provider tags out of the context and put it in the description.

bethesque commented 3 years ago

I've updated the template here https://docs.pact.io/pact_broker/webhooks/template_library#github---publish-commit-status