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

Standalone Pact verifier doing extra unnecessary tests when verifying by URL and publishing #350

Closed
gregtyler opened this issue Dec 19, 2023 · 2 comments
Labels
enhancement Indicates new feature requests

Comments

@gregtyler
Copy link

I've struggled a bit to narrow down the cause of this completely, because it involves interacting with real services and publishing to our actual broker, but I think my interpretation is correct.

Problem

I have a provider with three consumers. I'm setting up a CI job that verifies a consumer's Pact, by its URL, against its provider (triggered from the contract_requiring_verification_published webhook). I want to publish these results back to the broker. Therefore I've ended up with a command like:

pact_verifier_cli --url={pact url} --publish

Understandably, the verifier needs to know where the broker is so that it can publish the results. So I add that argument (I've omitted auth here):

pact_verifier_cli --url={pact url} --publish --broker-url={broker url}

However, the verifier now verifies four pacts: the one provided with --url and the latest from each of the three consumers. As far as I know it only publishes verification for the --url one.

Verifying the extra pacts is a waste of time in CI, but it's also a potential blocker. Instead of pulling the latest pacts from the main branches of the consumers, it's getting them from the broker path /pacts/provider/{provider}/latest. This returns (and therefore it tries to verify) the absolute most recent of each consumer, even if they're from branches with invalid contracts. Thus the verifier may fail even though the one pact it was asked to check is fine.

Cause

I think the underlying issue here is that the --broker-url argument is being used for two things:

  1. Specifying where results publication should go (as a dependency of --publish)
  2. Identifying pacts to verify (as specified in the docs)

And so you end up verifying extra pacts just because you want to publish.

Specifically, this block of code seems to be where it's using the presence of the broker URL argument to go and fetch additional pacts.

Mitigation

I'm looking at a couple of fixes I can try to unblock us:

  • Set --consumer-version-selectors to {"branch": "main"} so that it's at least running against pacts that we know work
    • But this is still verifying additional pacts, which wastes CI time (and the money it costs to do so) on every verification
  • Stop using --url, and instead craft --consumer-version-selectors that would match the same contract
    • This looks promising but would require me to rewrite a bunch of integration points, webhooks etc. to pass the consumer name/branch rather than the URL

I also tried using nonsense --consumer-version-selectors ({"branch":"-invalid"}) so that it wouldn't return anything but, quite reasonably, that failed because it didn't return anything 😄

Potential fixes for pact_verifier_cli

(Assuming I've understood all of the above correctly!)

You could only fetch pacts from the broker if --consumer-version-selectors or --consumer-version-tags is provided (therefore removing this else clause), but that would be a breaking change. And presumably the else clause is there for good reason.

You could add a --publish-broker-url argument that is used for publication but doesn't trigger fetching additional pacts, but that feels verbose and easy to confuse.

You could add a --no-broker-fetch argument that explicitly opts out this behaviour when --broker-url is provided, which wouldn't be breaking but again feels verbose and a little unintuitive.

@rholshausen
Copy link
Contributor

I've added a --webhook-callback-url option which will do what you need. You should be able to run it with

pact_verifier_cli --webhook-callback-url={pact url} --publish --broker-url={broker url} --provider-name {provider name}

@gregtyler
Copy link
Author

@rholshausen Sorry for the slow response, but that seems to be working great, thank you very much!

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

No branches or pull requests

2 participants