From ac55f7a651028e9c2bafef90aad6180d6a6179b7 Mon Sep 17 00:00:00 2001
From: Hitesh Raghuvanshi <hraghuvanshi@gitlab.com>
Date: Mon, 12 Jun 2023 21:49:55 +0530
Subject: [PATCH 1/2] Update API for instance headers
Changelog: added
EE: true
GraphQL API for updating streaming headers in instance level audit
event external destinations
---
doc/api/graphql/reference/index.md | 27 ++++
ee/app/graphql/ee/types/mutation_type.rb | 1 +
.../streaming/instance_headers/base.rb | 10 ++
.../streaming/instance_headers/update.rb | 45 ++++++
.../streaming/headers/update_service.rb | 10 +-
.../streaming/headers_operations.rb | 8 +
.../instance_headers/update_service.rb | 14 ++
.../streaming/instance_headers/update_spec.rb | 143 ++++++++++++++++++
.../streaming/headers/update_service_spec.rb | 8 +-
.../instance_headers/update_service_spec.rb | 27 ++++
.../headers_operations_shared_examples.rb | 37 +++++
11 files changed, 318 insertions(+), 12 deletions(-)
create mode 100644 ee/app/graphql/mutations/audit_events/streaming/instance_headers/update.rb
create mode 100644 ee/app/services/audit_events/streaming/instance_headers/update_service.rb
create mode 100644 ee/spec/requests/api/graphql/audit_events/streaming/instance_headers/update_spec.rb
create mode 100644 ee/spec/services/audit_events/streaming/instance_headers/update_service_spec.rb
diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md
index ff5eeafaadf826..9066d3fe585bf0 100644
--- a/doc/api/graphql/reference/index.md
+++ b/doc/api/graphql/reference/index.md
@@ -1293,6 +1293,27 @@ Input type: `AuditEventsStreamingInstanceHeadersCreateInput`
| <a id="mutationauditeventsstreaminginstanceheaderscreateerrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. |
| <a id="mutationauditeventsstreaminginstanceheaderscreateheader"></a>`header` | [`AuditEventsStreamingInstanceHeader`](#auditeventsstreaminginstanceheader) | Created header. |
+### `Mutation.auditEventsStreamingInstanceHeadersUpdate`
+
+Input type: `AuditEventsStreamingInstanceHeadersUpdateInput`
+
+#### Arguments
+
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| <a id="mutationauditeventsstreaminginstanceheadersupdateclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
+| <a id="mutationauditeventsstreaminginstanceheadersupdateheaderid"></a>`headerId` | [`AuditEventsStreamingInstanceHeaderID!`](#auditeventsstreaminginstanceheaderid) | Header to update. |
+| <a id="mutationauditeventsstreaminginstanceheadersupdatekey"></a>`key` | [`String!`](#string) | Header key. |
+| <a id="mutationauditeventsstreaminginstanceheadersupdatevalue"></a>`value` | [`String!`](#string) | Header value. |
+
+#### Fields
+
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| <a id="mutationauditeventsstreaminginstanceheadersupdateclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
+| <a id="mutationauditeventsstreaminginstanceheadersupdateerrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. |
+| <a id="mutationauditeventsstreaminginstanceheadersupdateheader"></a>`header` | [`AuditEventsStreamingInstanceHeader`](#auditeventsstreaminginstanceheader) | Updates header. |
+
### `Mutation.awardEmojiAdd`
Input type: `AwardEmojiAddInput`
@@ -26746,6 +26767,12 @@ A `AuditEventsStreamingHeaderID` is a global ID. It is encoded as a string.
An example `AuditEventsStreamingHeaderID` is: `"gid://gitlab/AuditEvents::Streaming::Header/1"`.
+### `AuditEventsStreamingInstanceHeaderID`
+
+A `AuditEventsStreamingInstanceHeaderID` is a global ID. It is encoded as a string.
+
+An example `AuditEventsStreamingInstanceHeaderID` is: `"gid://gitlab/AuditEvents::Streaming::InstanceHeader/1"`.
+
### `AwardableID`
A `AwardableID` is a global ID. It is encoded as a string.
diff --git a/ee/app/graphql/ee/types/mutation_type.rb b/ee/app/graphql/ee/types/mutation_type.rb
index 56a4bf5aeda511..1cfef0d8ecf8b8 100644
--- a/ee/app/graphql/ee/types/mutation_type.rb
+++ b/ee/app/graphql/ee/types/mutation_type.rb
@@ -120,6 +120,7 @@ module MutationType
mount_mutation ::Mutations::AuditEvents::GoogleCloudLoggingConfigurations::Update
mount_mutation ::Mutations::Forecasting::BuildForecast, alpha: { milestone: '16.0' }
mount_mutation ::Mutations::AuditEvents::Streaming::InstanceHeaders::Create
+ mount_mutation ::Mutations::AuditEvents::Streaming::InstanceHeaders::Update
prepend(Types::DeprecatedMutations)
end
diff --git a/ee/app/graphql/mutations/audit_events/streaming/instance_headers/base.rb b/ee/app/graphql/mutations/audit_events/streaming/instance_headers/base.rb
index 126de195516899..b2d0fbd78797a4 100644
--- a/ee/app/graphql/mutations/audit_events/streaming/instance_headers/base.rb
+++ b/ee/app/graphql/mutations/audit_events/streaming/instance_headers/base.rb
@@ -29,6 +29,16 @@ def find_destination(destination_id)
destination
end
+
+ def find_header(header_id)
+ header = GitlabSchema.object_from_id(
+ header_id, expected_type: ::AuditEvents::Streaming::InstanceHeader
+ ).sync
+
+ raise_resource_not_available_error! if header.blank?
+
+ header
+ end
end
end
end
diff --git a/ee/app/graphql/mutations/audit_events/streaming/instance_headers/update.rb b/ee/app/graphql/mutations/audit_events/streaming/instance_headers/update.rb
new file mode 100644
index 00000000000000..2081f0c37ff821
--- /dev/null
+++ b/ee/app/graphql/mutations/audit_events/streaming/instance_headers/update.rb
@@ -0,0 +1,45 @@
+# frozen_string_literal: true
+
+module Mutations
+ module AuditEvents
+ module Streaming
+ module InstanceHeaders
+ class Update < Base
+ graphql_name 'AuditEventsStreamingInstanceHeadersUpdate'
+
+ argument :header_id, ::Types::GlobalIDType[::AuditEvents::Streaming::InstanceHeader],
+ required: true,
+ description: 'Header to update.'
+
+ argument :key, GraphQL::Types::String,
+ required: true,
+ description: 'Header key.'
+
+ argument :value, GraphQL::Types::String,
+ required: true,
+ description: 'Header value.'
+
+ field :header, ::Types::AuditEvents::Streaming::InstanceHeaderType,
+ null: true,
+ description: 'Updates header.'
+
+ def resolve(header_id:, key:, value:)
+ header = find_header(header_id)
+
+ response = ::AuditEvents::Streaming::InstanceHeaders::UpdateService.new(
+ params: { header: header, key: key, value: value }
+ ).execute
+
+ if response.success?
+ { header: response.payload[:header], errors: [] }
+ elsif header.present?
+ { header: header.reset, errors: response.errors }
+ else
+ { errors: response.errors }
+ end
+ end
+ end
+ end
+ end
+ end
+end
diff --git a/ee/app/services/audit_events/streaming/headers/update_service.rb b/ee/app/services/audit_events/streaming/headers/update_service.rb
index c8bb539c5021a5..b6bba694ef9366 100644
--- a/ee/app/services/audit_events/streaming/headers/update_service.rb
+++ b/ee/app/services/audit_events/streaming/headers/update_service.rb
@@ -11,12 +11,10 @@ def execute
audit_message = audit_message(header.key, params[:key])
- if header.update(key: params[:key], value: params[:value])
- audit(action: :update, header: header, message: audit_message)
- ServiceResponse.success(payload: { header: header, errors: [] })
- else
- ServiceResponse.error(message: Array(header.errors))
- end
+ success, response = update_header(header, params[:key], params[:value])
+
+ audit(action: :update, header: header, message: audit_message) if success
+ response
end
private
diff --git a/ee/app/services/audit_events/streaming/headers_operations.rb b/ee/app/services/audit_events/streaming/headers_operations.rb
index 1da4c4b612c324..fd4f3f787990aa 100644
--- a/ee/app/services/audit_events/streaming/headers_operations.rb
+++ b/ee/app/services/audit_events/streaming/headers_operations.rb
@@ -10,6 +10,14 @@ def create_header(destination, key, value)
[false, ServiceResponse.error(message: Array(header.errors)), nil]
end
+
+ def update_header(header, key, value)
+ if header.update(key: key, value: value)
+ [true, ServiceResponse.success(payload: { header: header, errors: [] })]
+ else
+ [false, ServiceResponse.error(message: Array(header.errors))]
+ end
+ end
end
end
end
diff --git a/ee/app/services/audit_events/streaming/instance_headers/update_service.rb b/ee/app/services/audit_events/streaming/instance_headers/update_service.rb
new file mode 100644
index 00000000000000..d21cbc1fd5c75e
--- /dev/null
+++ b/ee/app/services/audit_events/streaming/instance_headers/update_service.rb
@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+module AuditEvents
+ module Streaming
+ module InstanceHeaders
+ class UpdateService < BaseService
+ def execute
+ _, response = update_header(params[:header], params[:key], params[:value])
+ response
+ end
+ end
+ end
+ end
+end
diff --git a/ee/spec/requests/api/graphql/audit_events/streaming/instance_headers/update_spec.rb b/ee/spec/requests/api/graphql/audit_events/streaming/instance_headers/update_spec.rb
new file mode 100644
index 00000000000000..156971fc27f91f
--- /dev/null
+++ b/ee/spec/requests/api/graphql/audit_events/streaming/instance_headers/update_spec.rb
@@ -0,0 +1,143 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'Update an external audit event destination header', feature_category: :audit_events do
+ include GraphqlHelpers
+
+ let_it_be(:destination) { create(:instance_external_audit_event_destination) }
+ let_it_be(:header) do
+ create(:instance_audit_events_streaming_header,
+ key: 'key-1',
+ instance_external_audit_event_destination: destination
+ )
+ end
+
+ let_it_be(:admin) { create(:admin) }
+ let_it_be(:user) { create(:user) }
+
+ let(:current_user) { admin }
+ let(:mutation) { graphql_mutation(:audit_events_streaming_instance_headers_update, input) }
+ let(:mutation_response) { graphql_mutation_response(:audit_events_streaming_instance_headers_update) }
+
+ let(:input) do
+ {
+ headerId: header.to_gid,
+ key: 'new-key',
+ value: 'new-value'
+ }
+ end
+
+ subject { post_graphql_mutation(mutation, current_user: current_user) }
+
+ shared_examples 'a mutation that does not update a header' do
+ it 'does not update a header key' do
+ expect { post_graphql_mutation(mutation, current_user: actioner) }.not_to change { header.key }
+ end
+
+ it 'does not update a header value' do
+ expect { post_graphql_mutation(mutation, current_user: actioner) }.not_to change { header.value }
+ end
+ end
+
+ context 'when feature is licensed' do
+ before do
+ stub_licensed_features(external_audit_events: true)
+ end
+
+ context 'when feature flag `ff_external_audit_events` is enabled' do
+ context 'when current user is instance admin' do
+ it 'updates the header with the correct attributes', :aggregate_failures do
+ expect { subject }.to change { header.reload.key }.from('key-1').to('new-key')
+ .and change { header.reload.value }.from('bar')
+ .to('new-value')
+ end
+
+ context 'when the header attributes are invalid' do
+ let(:invalid_key_input) do
+ {
+ headerId: header.to_gid,
+ key: '',
+ value: 'bar'
+ }
+ end
+
+ let(:mutation) { graphql_mutation(:audit_events_streaming_instance_headers_update, invalid_key_input) }
+
+ it 'returns correct errors' do
+ subject
+
+ expect(mutation_response['errors']).to contain_exactly("Key can't be blank")
+ end
+
+ it 'returns the unmutated attribute values', :aggregate_failures do
+ subject
+
+ expect(mutation_response.dig('header', 'key')).to eq('key-1')
+ expect(mutation_response.dig('header', 'value')).to eq('bar')
+ end
+
+ it_behaves_like 'a mutation that does not update a header' do
+ let_it_be(:actioner) { admin }
+ end
+ end
+
+ context 'when the header id is wrong' do
+ let_it_be(:invalid_header_input) do
+ {
+ headerId: "gid://gitlab/AuditEvents::Streaming::InstanceHeader/-1",
+ key: 'foo',
+ value: 'bar'
+ }
+ end
+
+ let(:mutation) { graphql_mutation(:audit_events_streaming_instance_headers_update, invalid_header_input) }
+
+ it_behaves_like 'a mutation that returns top-level errors',
+ errors: [Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR]
+
+ it_behaves_like 'a mutation that does not update a header' do
+ let_it_be(:actioner) { admin }
+ end
+ end
+ end
+
+ context 'when current user is not instance admin' do
+ it_behaves_like 'a mutation that does not update a header' do
+ let_it_be(:actioner) { user }
+ end
+ end
+ end
+
+ context 'when feature flag `ff_external_audit_events` is disabled' do
+ before do
+ stub_feature_flags(ff_external_audit_events: false)
+ end
+
+ context 'when current user is instance admin' do
+ it_behaves_like 'a mutation that does not update a header' do
+ let_it_be(:actioner) { admin }
+ end
+ end
+
+ context 'when current user is not instance admin' do
+ it_behaves_like 'a mutation that does not update a header' do
+ let_it_be(:actioner) { user }
+ end
+ end
+ end
+ end
+
+ context 'when feature is unlicensed' do
+ before do
+ stub_licensed_features(external_audit_events: false)
+ end
+
+ it_behaves_like 'a mutation that returns top-level errors',
+ errors: [Mutations::AuditEvents::Streaming::InstanceHeaders::Base::ERROR_MESSAGE]
+
+ it_behaves_like 'a mutation that does not update a header' do
+ let_it_be(:actioner) { admin }
+ end
+ end
+end
diff --git a/ee/spec/services/audit_events/streaming/headers/update_service_spec.rb b/ee/spec/services/audit_events/streaming/headers/update_service_spec.rb
index f40e312317caf5..5e54087578976d 100644
--- a/ee/spec/services/audit_events/streaming/headers/update_service_spec.rb
+++ b/ee/spec/services/audit_events/streaming/headers/update_service_spec.rb
@@ -41,13 +41,9 @@
end
end
- context 'when the header is updated successfully' do
- it 'updates the header' do
- expect(response).to be_success
- expect(header.reload.key).to eq 'new'
- expect(header.value).to eq 'new'
- end
+ it_behaves_like 'header updation'
+ context 'when the header is updated successfully' do
it 'sends the audit streaming event' do
audit_context = {
name: 'audit_events_streaming_headers_update',
diff --git a/ee/spec/services/audit_events/streaming/instance_headers/update_service_spec.rb b/ee/spec/services/audit_events/streaming/instance_headers/update_service_spec.rb
new file mode 100644
index 00000000000000..bee161718b92f6
--- /dev/null
+++ b/ee/spec/services/audit_events/streaming/instance_headers/update_service_spec.rb
@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe AuditEvents::Streaming::InstanceHeaders::UpdateService, feature_category: :audit_events do
+ let(:header) { create(:instance_audit_events_streaming_header, key: 'old', value: 'old') }
+
+ let(:params) do
+ {
+ header: header,
+ key: 'new',
+ value: 'new'
+ }
+ end
+
+ subject(:service) do
+ described_class.new(
+ params: params
+ )
+ end
+
+ describe '#execute' do
+ subject(:response) { service.execute }
+
+ it_behaves_like 'header updation'
+ end
+end
diff --git a/ee/spec/support/shared_examples/services/audit_events/streaming/headers_operations_shared_examples.rb b/ee/spec/support/shared_examples/services/audit_events/streaming/headers_operations_shared_examples.rb
index d7fe64df8b04be..b024535e2bf4a4 100644
--- a/ee/spec/support/shared_examples/services/audit_events/streaming/headers_operations_shared_examples.rb
+++ b/ee/spec/support/shared_examples/services/audit_events/streaming/headers_operations_shared_examples.rb
@@ -27,3 +27,40 @@
expect(header.value).to eq('a_value')
end
end
+
+RSpec.shared_examples 'header updation' do
+ context 'when header updation is successful' do
+ it 'has the header in the response payload' do
+ expect(response).to be_success
+ expect(response.payload[:header].key).to eq 'new'
+ expect(response.payload[:header].value).to eq 'new'
+ end
+
+ it 'updates the header' do
+ expect(response).to be_success
+ expect(header.reload.key).to eq 'new'
+ expect(header.value).to eq 'new'
+ end
+ end
+
+ context 'when header updation is unsuccessful' do
+ let(:params) do
+ {
+ header: header,
+ key: '',
+ value: 'new'
+ }
+ end
+
+ it 'does not update the header' do
+ expect { subject }.not_to change { header.reload.key }
+ expect(header.value).to eq 'old'
+ end
+
+ it 'has an error response' do
+ expect(response).to be_error
+ expect(response.errors)
+ .to match_array ["Key can't be blank"]
+ end
+ end
+end
--
GitLab
From 27f3964195e980d67c054a11e5c5f089666a22cc Mon Sep 17 00:00:00 2001
From: Hitesh Raghuvanshi <hraghuvanshi@gitlab.com>
Date: Tue, 13 Jun 2023 15:35:37 +0530
Subject: [PATCH 2/2] Removed unnecessary condition
---
.../mutations/audit_events/streaming/instance_headers/update.rb | 2 --
1 file changed, 2 deletions(-)
diff --git a/ee/app/graphql/mutations/audit_events/streaming/instance_headers/update.rb b/ee/app/graphql/mutations/audit_events/streaming/instance_headers/update.rb
index 2081f0c37ff821..008c2365737c22 100644
--- a/ee/app/graphql/mutations/audit_events/streaming/instance_headers/update.rb
+++ b/ee/app/graphql/mutations/audit_events/streaming/instance_headers/update.rb
@@ -34,8 +34,6 @@ def resolve(header_id:, key:, value:)
{ header: response.payload[:header], errors: [] }
elsif header.present?
{ header: header.reset, errors: response.errors }
- else
- { errors: response.errors }
end
end
end
--
GitLab