pact-foundation / pact_broker

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

Improve validation for matrix selectors #641

Open bethesque opened 1 year ago

bethesque commented 1 year ago

This query should have returned a 400

{
  "q": [
    {
      "pacticipant": "X",
      "version": "123",
      "environment": "B"
    }
  ]
}

TODO: Work out if this is an error or a valid query - it causes an SQL error.

{
  "q": [
    {
      "pacticipant": "x",
      "version": "134234"
    },
    {
      "pacticipant": "x",
      "latest": "true",
      "environment": "B"
    }
  ]
}

This is a placeholder issue - it needs the rules written out properly before it can be picked up.

bethesque commented 3 months ago

Low priority.

Almost all matrix queries come via the can-i-deploy CLI, which will protect against most invalid queries. Full validation is generally only relevant if someone hand crafts a matrix query, which we occasionally see via the Postman interface when someone is playing around with the API. I believe this particular error (selectors with duplicate pacticipant names) may be able to be produced via the CLI by inputting two selectors with the same pacticipant name, but I have not checked this.

A quick fix for this particular error would be to update the validate_selectors method in the PactBroker::Matrix::Service class to ensure that there were no selectors with duplicate pacticipant names.

If this was to be implemented properly, the validate_selectors code should be moved into a Dry Validation contract class, and the relevant business rules regarding what fields were supported together should be implemented using the same pattern as ConsumerVersionSelectorContract.

I think it's probably not worth it, for the number of times a year that it happens.