From 3e50f7947ecb448968c57d7562cc65418d04e4ef Mon Sep 17 00:00:00 2001 From: Artur Fedorov <afedorov@gitlab.com> Date: Wed, 24 Jul 2024 11:07:28 +0200 Subject: [PATCH] This MR adds new field for update service New encrypted field shared secret was added to a model API services were updated accordingly Changelog: changed EE: true --- doc/api/status_checks.md | 1 + .../branch_rules/external_status_checks/update_service.rb | 2 +- ee/app/services/external_status_checks/update_service.rb | 2 +- ee/lib/api/status_checks.rb | 1 + ee/spec/requests/api/status_checks_spec.rb | 4 ++-- .../external_status_checks/update_service_spec.rb | 5 ++++- .../services/external_status_checks/update_service_spec.rb | 3 ++- 7 files changed, 12 insertions(+), 6 deletions(-) diff --git a/doc/api/status_checks.md b/doc/api/status_checks.md index 36a00c4c8aa54..b4f596a1e3d05 100644 --- a/doc/api/status_checks.md +++ b/doc/api/status_checks.md @@ -80,6 +80,7 @@ PUT /projects/:id/external_status_checks/:check_id | `check_id` | integer | yes | ID of an external status check service | | `name` | string | no | Display name of external status check service | | `external_url` | string | no | URL of external status check service | +| `shared_secret` | string | no | HMAC secret for external status check | | `protected_branch_ids` | `array<Integer>` | no | IDs of protected branches to scope the rule by | ## Delete external status check service diff --git a/ee/app/services/branch_rules/external_status_checks/update_service.rb b/ee/app/services/branch_rules/external_status_checks/update_service.rb index c767e4dccd01e..5d7ed7e6668c9 100644 --- a/ee/app/services/branch_rules/external_status_checks/update_service.rb +++ b/ee/app/services/branch_rules/external_status_checks/update_service.rb @@ -42,7 +42,7 @@ def execute_on_all_protected_branches_rule end def permitted_params - %i[check_id name external_url] + %i[check_id name external_url shared_secret] end end end diff --git a/ee/app/services/external_status_checks/update_service.rb b/ee/app/services/external_status_checks/update_service.rb index 2d871c538871d..c12399ace766d 100644 --- a/ee/app/services/external_status_checks/update_service.rb +++ b/ee/app/services/external_status_checks/update_service.rb @@ -27,7 +27,7 @@ def can_update_external_status_check? end def resource_params - params.slice(:name, :external_url, :protected_branch_ids) + params.slice(:name, :shared_secret, :external_url, :protected_branch_ids) end def external_status_check diff --git a/ee/lib/api/status_checks.rb b/ee/lib/api/status_checks.rb index 2b81fb330b3f1..b6a97397c06a3 100644 --- a/ee/lib/api/status_checks.rb +++ b/ee/lib/api/status_checks.rb @@ -70,6 +70,7 @@ def check_feature_enabled! desc: 'ID of an external status check', documentation: { example: 1 } optional :name, type: String, desc: 'Display name of external status check', documentation: { example: 'QA' } + optional :shared_secret, type: String, desc: 'HMAC shared secret', documentation: { example: 'hmac-sha256' } optional :external_url, type: String, desc: 'URL of external status check resource', diff --git a/ee/spec/requests/api/status_checks_spec.rb b/ee/spec/requests/api/status_checks_spec.rb index a27802fb646e8..8d3fea5e98a01 100644 --- a/ee/spec/requests/api/status_checks_spec.rb +++ b/ee/spec/requests/api/status_checks_spec.rb @@ -481,7 +481,7 @@ let_it_be(:protected_branch) { create(:protected_branch, project: project) } let(:params) do - { name: 'New rule', external_url: 'https://gitlab.com/test/example.json', protected_branch_ids: protected_branch.id } + { name: 'New rule', external_url: 'https://gitlab.com/test/example.json', protected_branch_ids: protected_branch.id, shared_secret: 'shared_secret' } end subject do @@ -503,7 +503,7 @@ expect(json_response['id']).not_to be_nil expect(json_response['name']).to eq('New rule') - expect(json_response['hmac']).to eq(false) + expect(json_response['hmac']).to eq(true) expect(json_response['external_url']).to eq('https://gitlab.com/test/example.json') expect(json_response['protected_branches'].size).to eq(1) end diff --git a/ee/spec/services/branch_rules/external_status_checks/update_service_spec.rb b/ee/spec/services/branch_rules/external_status_checks/update_service_spec.rb index 971ff0b2e5024..03b94c303e9d0 100644 --- a/ee/spec/services/branch_rules/external_status_checks/update_service_spec.rb +++ b/ee/spec/services/branch_rules/external_status_checks/update_service_spec.rb @@ -13,7 +13,8 @@ create(:external_status_check, project: project, protected_branches: [protected_branch]) end - let(:params) { { check_id: external_status_check.id, name: 'Updated name', external_url: 'https://external_url_updated.com' } } + let(:shared_secret) { 'shared secret' } + let(:params) { { check_id: external_status_check.id, name: 'Updated name', external_url: 'https://external_url_updated.com', shared_secret: shared_secret } } subject(:execute) { described_class.new(branch_rule, user, params).execute } @@ -36,6 +37,7 @@ external_status_check.reload expect(external_status_check.name).to eq('Updated name') expect(external_status_check.external_url).to eq('https://external_url_updated.com') + expect(external_status_check.shared_secret).to eq(shared_secret) end it 'includes the updated external_status_check record in payload' do @@ -45,6 +47,7 @@ expect(external_status_check.project).to eq(project) expect(external_status_check.name).to eq('Updated name') expect(external_status_check.external_url).to eq('https://external_url_updated.com') + expect(external_status_check.shared_secret).to eq(shared_secret) expect(external_status_check.protected_branches).to contain_exactly(protected_branch) end end diff --git a/ee/spec/services/external_status_checks/update_service_spec.rb b/ee/spec/services/external_status_checks/update_service_spec.rb index e776b918c2c1e..78d5e95d5b0df 100644 --- a/ee/spec/services/external_status_checks/update_service_spec.rb +++ b/ee/spec/services/external_status_checks/update_service_spec.rb @@ -8,7 +8,7 @@ let_it_be(:protected_branch) { create(:protected_branch, project: project) } let(:current_user) { project.first_owner } - let(:params) { { id: project.id, check_id: check.id, external_url: 'http://newvalue.com', name: 'new name', protected_branch_ids: [protected_branch.id] } } + let(:params) { { id: project.id, check_id: check.id, external_url: 'http://newvalue.com', name: 'new name', protected_branch_ids: [protected_branch.id], shared_secret: 'shared_secret' } } subject(:execute) { described_class.new(container: project, current_user: current_user, params: params).execute } @@ -21,6 +21,7 @@ expect(check.external_url).to eq('http://newvalue.com') expect(check.name).to eq('new name') expect(check.protected_branches).to contain_exactly(protected_branch) + expect(check.shared_secret).to eq('shared_secret') end it 'is successful' do -- GitLab