trim trailing slash from base url

[davidjgoss]

This PR removes trailing slashes from the broker base URL, which can otherwise cause 404s due to // in composed URLs e.g. for matrix as used in can-i-deploy.

This is done by adding a post-processing step just after Thor processes the raw arguments to merge added/changed options onto the resolved options.

Fixes #121

[YOU54F]

Nice @davidjgoss and welcome! Cucumber Jo speaks highly of you :)

[YOU54F]

Looking at the test failures, I think it might be from it coming from a fork and not being able to get broker creds, let me push it to a branch against origin and see what happens :)

[YOU54F]

So that resolved two of the jobs, but the three test runs are failing

  1) PactBroker::Client::CLI::Broker parses the pacticipant names and versions
     Failure/Error: super(args, options, config)

     NoMethodError:
       undefined method `[]' for nil:NilClass

             @_invocations = config[:invocations] || Hash.new { |h, k| h[k] = [] }
                                   ^^^^^^^^^^^^^^
     # ./lib/pact_broker/client/cli/custom_thor.rb:18:in `initialize'
     # ./spec/lib/pact_broker/client/cli/broker_can_i_deploy_spec.rb:10:in `block (2 levels) in <module:CLI>'

  2) PactBroker::Client::CLI::Broker invokes the CanIDeploy service
     Failure/Error: super(args, options, config)

     NoMethodError:
       undefined method `[]' for nil:NilClass

             @_invocations = config[:invocations] || Hash.new { |h, k| h[k] = [] }
                                   ^^^^^^^^^^^^^^
     # ./lib/pact_broker/client/cli/custom_thor.rb:18:in `initialize'
     # ./spec/lib/pact_broker/client/cli/broker_can_i_deploy_spec.rb:10:in `block (2 levels) in <module:CLI>'

There is a warning at the start of the test run, which relates to your change

     Warning:  Attempted to create command "postprocess_options" without usage or description. Call desc if you want this method to be available as command or declare it inside a no_commands{} block. Invoked from "/home/runner/work/pact_broker-client/pact_broker-client/lib/pact_broker/client/cli/custom_thor.rb:22:in `<class:CustomThor>'".

Are you able to take a look when you have some time?

[davidjgoss]

Hey @YOU54F! Thanks for looking at this. I've pushed some changes which I think address the failing tests and warning from Thor.

Let me know if you have more feedback - I'm learning Ruby as I go here :)

[YOU54F]

awesome thanks dude

Let me know if you have more feedback - I'm learning Ruby as I go here :)

You and me both! Beth will have a keener eye that probably you, and definitely me, as this is her playground. I've tagged her review. Everything is all green so thanks for that! (bar the pull request on pull request but that is expected)

We could probably put a note in somewhere in the Pull request so people don't get shocked on either side of the fence.

Thanks for the contribution, it will certainly be useful for users, as I know its caused a fair few, a bit of a frustration

Does it make sense to remove any of the other workarounds, as you said it worked correctly on some commands but not others

[davidjgoss]

Does it make sense to remove any of the other workarounds, as you said it worked correctly on some commands but not others

Good point. Looking at how e.g. record-release (which doesn't have the issue) is implemented, it seems to be following the HAL links from the entry point, so no direct concatenation of base URL and path is happening.

The create-webhook command does do a chomp('/') though, just looking to see if that's still needed.

[bethesque]

Thanks @davidjgoss :D

The best place to put this would be in

lib/pact_broker/client/cli/custom_thor.rb

          def pact_broker_client_options
            client_options = { verbose: options.verbose, pact_broker_base_url: options.broker_base_url }
            client_options[:token] = options.broker_token || ENV['PACT_BROKER_TOKEN']
            if options.broker_username || ENV['PACT_BROKER_USERNAME']
              client_options[:basic_auth] = {
                  username: options.broker_username || ENV['PACT_BROKER_USERNAME'],
                  password: options.broker_password || ENV['PACT_BROKER_PASSWORD']
                }.compact
            end

            client_options.compact
          end

as this covers the situation where the URL is passed in via the environment variable as well (due to some dirty hacks in add_option_from_environment_variable

[davidjgoss]

It does work with the env var as-is (added a test to confirm) because the env var for the URL is handled in massage_args right at the entry point before initialize, but yeah pact_broker_client_options does seem like the better place for it anyway - I'll give that a try 👍

[YOU54F]

Thanks @davidjgoss is fixed now and released in 1.67 of the pact_broker-client and 0.51.0.3 of the docker image

Users can go wild with the trailing slashes

pact-ruby-cli on  chore/fix_ca_certs [$!?] via 🐳 desktop-linux via 💎 v2.6.10 on ☁️  (eu-west-2) took 4s 
🕙20:36:40 ❯ export PACT_BROKER_BASE_URL="https://test.pactflow.io//////"

pact-ruby-cli on  chore/fix_ca_certs [$!?] via 🐳 desktop-linux via 💎 v2.6.10 on ☁️  (eu-west-2) 
🕙20:36:45 ❯ docker run --rm \
 -w ${PWD} \
 -v ${PWD}:${PWD} \
 -e PACT_BROKER_BASE_URL \
 -e PACT_BROKER_USERNAME \
 -e PACT_BROKER_PASSWORD \
  pactfoundation/pact-cli:latest \
  publish \
  ${PWD}/example/pacts \
  --consumer-app-version fake-git-sha-for-demo-$(date +%s) \
  --tag-with-git-branch 
Created docker-example-consumer version fake-git-sha-for-demo-1684352207 with tags chore/fix_ca_certs
  Next steps:
    Configure the version branch to be the value of your repository branch.
Pact successfully published for docker-example-consumer version fake-git-sha-for-demo-1684352207 and provider docker-example-provider.
  View the published pact at https://test.pactflow.io/pacts/provider/docker-example-provider/consumer/docker-example-consumer/version/fake-git-sha-for-demo-1684352207
  Events detected: contract_published (pact content is the same as previous version with tag chore/fix_ca_certs and no new tags were applied)
  Next steps:
    * Add Pact verification tests to the docker-example-provider build. See https://docs.pact.io/go/provider_verification
    * Configure separate docker-example-provider pact verification build and webhook to trigger it when the pact content changes. See https://docs.pact.io/go/webhooks

[YOU54F]

so with can-i-deploy that can only handle a single trailing https://test.pactflow.io/ but publish seems happy with https://test.pactflow.io////////////

don't think anyone is going that wild though :)

pact-ruby-cli on  chore/fix_ca_certs [$!?] via 🐳 desktop-linux via 💎 v2.6.10 on ☁️  (eu-west-2) took 2s 
🕙20:42:52 [🔴 ERROR] ❯ export PACT_BROKER_BASE_URL="https://test.pactflow.io/"

pact-ruby-cli on  chore/fix_ca_certs [$!?] via 🐳 desktop-linux via 💎 v2.6.10 on ☁️  (eu-west-2) 
🕙20:43:06 ❯ docker run --rm \
 -e PACT_BROKER_BASE_URL \
 -e PACT_BROKER_USERNAME \
 -e PACT_BROKER_PASSWORD \
  pactfoundation/pact-cli:latest \
  pact-broker can-i-deploy \
  --pacticipant docker-example-consumer \
  --latest 
Computer says no ¯\_(ツ)_/¯

CONSUMER                | C.VERSION                        | PROVIDER                | P.VERSION | SUCCESS? | RESULT#
------------------------|----------------------------------|-------------------------|-----------|----------|--------
docker-example-consumer | fake-git-sha-for-demo-1684352207 | docker-example-provider | ???       | ???      |        

WARN: It is recommended to specify the environment into which you are deploying. Without the environment, this result will not be reliable.
WARN: It is recommended to specify the version number (rather than the tag or branch) of the pacticipant you wish to deploy to avoid race conditions. Without a version number, this result will not be reliable.
There is no verified pact between the latest version of docker-example-consumer (fake-git-sha-for-demo-1684352207) and the latest version of docker-example-provider (no such version exists)

pact-ruby-cli on  chore/fix_ca_certs [$!?] via 🐳 desktop-linux via 💎 v2.6.10 on ☁️  (eu-west-2) took 2s 
🕙20:43:13 [🔴 ERROR] ❯ export PACT_BROKER_BASE_URL="https://test.pactflow.io//"

pact-ruby-cli on  chore/fix_ca_certs [$!?] via 🐳 desktop-linux via 💎 v2.6.10 on ☁️  (eu-west-2) 
🕙20:43:18 ❯ docker run --rm \
 -e PACT_BROKER_BASE_URL \
 -e PACT_BROKER_USERNAME \
 -e PACT_BROKER_PASSWORD \
  pactfoundation/pact-cli:latest \
  pact-broker can-i-deploy \
  --pacticipant docker-example-consumer \
  --latest 
Error retrieving matrix. PactBroker::Client::Hal::ErrorResponseReturned - Error making request to https://test.pactflow.io//matrix status=404 {"error":"The requested document was not found on this server."}
/usr/lib/ruby/gems/3.1.0/gems/pact_broker-client-1.67.0/lib/pact_broker/client/hal/entity.rb:197:in `assert_success!'
/usr/lib/ruby/gems/3.1.0/gems/pact_broker-client-1.67.0/lib/pact_broker/client/hal/link.rb:45:in `get!'
/usr/lib/ruby/gems/3.1.0/gems/pact_broker-client-1.67.0/lib/pact_broker/client/matrix/query.rb:11:in `call'
/usr/lib/ruby/gems/3.1.0/gems/pact_broker-client-1.67.0/lib/pact_broker/client/base_command.rb:13:in `call'
/usr/lib/ruby/gems/3.1.0/gems/pact_broker-client-1.67.0/lib/pact_broker/client/can_i_deploy.rb:107:in `block in fetch_matrix'
/usr/lib/ruby/gems/3.1.0/gems/pact_broker-client-1.67.0/lib/pact_broker/client/retry.rb:22:in `while_error'
/usr/lib/ruby/gems/3.1.0/gems/pact_broker-client-1.67.0/lib/pact_broker/client/can_i_deploy.rb:107:in `fetch_matrix'
/usr/lib/ruby/gems/3.1.0/gems/pact_broker-client-1.67.0/lib/pact_broker/client/can_i_deploy.rb:111:in `fetch_matrix_with_retries'
/usr/lib/ruby/gems/3.1.0/gems/pact_broker-client-1.67.0/lib/pact_broker/client/can_i_deploy.rb:34:in `call'
/usr/lib/ruby/gems/3.1.0/gems/pact_broker-client-1.67.0/lib/pact_broker/client/can_i_deploy.rb:23:in `call'
/usr/lib/ruby/gems/3.1.0/gems/pact_broker-client-1.67.0/lib/pact_broker/client/cli/matrix_commands.rb:37:in `can_i_deploy'
/usr/lib/ruby/gems/3.1.0/gems/thor-1.2.2/lib/thor/command.rb:27:in `run'
/usr/lib/ruby/gems/3.1.0/gems/thor-1.2.2/lib/thor/invocation.rb:127:in `invoke_command'
/usr/lib/ruby/gems/3.1.0/gems/thor-1.2.2/lib/thor.rb:392:in `dispatch'
/usr/lib/ruby/gems/3.1.0/gems/thor-1.2.2/lib/thor/base.rb:485:in `start'
/usr/lib/ruby/gems/3.1.0/gems/pact_broker-client-1.67.0/lib/pact_broker/client/cli/custom_thor.rb:36:in `start'
/usr/lib/ruby/gems/3.1.0/gems/pact_broker-client-1.67.0/bin/pact-broker:10:in `<top (required)>'
/usr/bin/pact-broker:25:in `load'

[davidjgoss]

Awesome thanks @YOU54F!