Skip to content

Commit

Permalink
feat: do not allow contract content for a consumer version to be modi…
Browse files Browse the repository at this point in the history
…fied for all new Broker instances (#484)
  • Loading branch information
bethesque authored Aug 11, 2021
1 parent f696329 commit b181974
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 13 deletions.
17 changes: 17 additions & 0 deletions db/migrations/20210810_set_allow_contract_modification.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
Sequel.migration do
up do
if from(:pact_publications).count != 0
from(:config).insert(
name: "allow_dangerous_contract_modification",
type: "boolean",
value: "1",
created_at: Sequel.datetime_class.now,
updated_at: Sequel.datetime_class.now
)
end
end

down do

end
end
21 changes: 15 additions & 6 deletions lib/pact_broker/api/resources/pact.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
require "pact_broker/api/resources/pacticipant_resource_methods"
require "pact_broker/api/decorators/pact_decorator"
require "pact_broker/api/decorators/extended_pact_decorator"
require "pact_broker/json"
require "pact_broker/messages"
require "pact_broker/pacts/pact_params"
require "pact_broker/api/contracts/put_pact_params_contract"
require "pact_broker/webhooks/execution_configuration"
Expand All @@ -17,6 +17,7 @@ class Pact < BaseResource
include PacticipantResourceMethods
include WebhookExecutionMethods
include PactResourceMethods
include PactBroker::Messages

def content_types_provided
[
Expand All @@ -36,16 +37,14 @@ def allowed_methods
end

def is_conflict?
merge_conflict = request.patch? && resource_exists? &&
Pacts::Merger.conflict?(pact.json_content, pact_params.json_content)
merge_conflict = request.patch? && resource_exists? && Pacts::Merger.conflict?(pact.json_content, pact_params.json_content)

potential_duplicate_pacticipants?(pact_params.pacticipant_names) || merge_conflict
potential_duplicate_pacticipants?(pact_params.pacticipant_names) || merge_conflict || disallowed_modification?
end

def malformed_request?
if request.patch? || request.put?
invalid_json? ||
contract_validation_errors?(Contracts::PutPactParamsContract.new(pact_params), pact_params)
invalid_json? || contract_validation_errors?(Contracts::PutPactParamsContract.new(pact_params), pact_params)
else
false
end
Expand Down Expand Up @@ -108,6 +107,16 @@ def policy_pacticipant
def pact
@pact ||= pact_service.find_pact(pact_params)
end

def disallowed_modification?
if request.really_put? && pact_service.disallowed_modification?(pact, pact_params.json_content)
message_params = { consumer_name: pact_params.consumer_name, consumer_version_number: pact_params.consumer_version_number }
set_json_error_message(message("errors.validation.pact_content_modification_not_allowed", message_params))
true
else
false
end
end
end
end
end
Expand Down
1 change: 1 addition & 0 deletions lib/pact_broker/config/runtime_configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ class RuntimeConfiguration < Anyway::Config
use_first_tag_as_branch_time_limit: 10,
auto_detect_main_branch: true,
main_branch_candidates: ["develop", "main", "master"],
allow_dangerous_contract_modification: false,
semver_formats: ["%M.%m.%p%s%d", "%M.%m", "%M"],
seed_example_data: true,
features: []
Expand Down
1 change: 1 addition & 0 deletions lib/pact_broker/locale/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ en:
invalid_http_method: "Invalid HTTP method '%{method}'"
invalid_url: "Invalid URL '%{url}'. Expected format: http://example.org"
pact_missing_pacticipant_name: "was not found at expected path $.%{pacticipant}.name in the submitted pact file."
pact_content_modification_not_allowed: "Cannot change the content of the pact for %{consumer_name} version %{consumer_version_number}, as race conditions will cause unreliable results for can-i-deploy. Each pact must be published with a unique consumer version number. For more information see https://docs.pact.io/go/versioning"
consumer_version_number_missing: "Please specify the consumer version number by setting the X-Pact-Consumer-Version header."
consumer_version_number_header_invalid: "X-Pact-Consumer-Version '%{consumer_version_number}' cannot be parsed to a version number. The expected format (unless this configuration has been overridden) is a semantic version. eg. 1.3.0 or 2.0.4.rc1"
consumer_version_number_invalid: "Consumer version number '%{consumer_version_number}' cannot be parsed to a version number. The expected format (unless this configuration has been overridden) is a semantic version. eg. 1.3.0 or 2.0.4.rc1"
Expand Down
4 changes: 4 additions & 0 deletions lib/pact_broker/pacts/service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,10 @@ def update_pact params, existing_pact
updated_pact
end

def disallowed_modification?(existing_pact, new_json_content)
!PactBroker.configuration.allow_dangerous_contract_modification && existing_pact && existing_pact.pact_version_sha != generate_sha(new_json_content)
end

private :update_pact

# When no publication for the given consumer/provider/consumer version number exists
Expand Down
28 changes: 21 additions & 7 deletions spec/features/publish_pact_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,32 @@

before do
td.create_pact_with_hierarchy("A Consumer", "1.2.3", "A Provider").and_return(:pact)
allow(PactBroker.configuration).to receive(:allow_dangerous_contract_modification).and_return(allow_dangerous_contract_modification)
end

it "returns a 200 Success" do
expect(subject.status).to be 200
end
context "when the content is different" do
context "when pact modification is allowed" do
let(:allow_dangerous_contract_modification) { true }

it "returns an application/json Content-Type" do
expect(subject.headers["Content-Type"]).to eq "application/hal+json;charset=utf-8"
it "returns a 200 Success" do
expect(subject.status).to be 200
end

it "returns an application/json Content-Type" do
expect(subject.headers["Content-Type"]).to eq "application/hal+json;charset=utf-8"
end

it "returns the pact in the response body" do
expect(response_body_json).to match_pact JSON.parse(pact_content)
end
end
end

it "returns the pact in the response body" do
expect(response_body_json).to match_pact JSON.parse(pact_content)
context "when pact modification is not allowed" do
let(:allow_dangerous_contract_modification) { false }

its(:status) { is_expected.to eq 409 }
its(:body) { is_expected.to include "Cannot change the content of the pact for A Consumer version 1.2.3" }
end
end

Expand Down

0 comments on commit b181974

Please sign in to comment.