diff --git a/app/models/repository.rb b/app/models/repository.rb
index c6810bd89aeb59f09db3fd44e5b5a86ee4572277..3cf3879672c404ecc8c69a7ea50915dbdf4f859d 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -901,9 +901,13 @@ def commit_files(user, **options)
options[:start_repository] = start_project.repository.raw_repository
end
- skip_target_sha = options.delete(:skip_target_sha)
- unless skip_target_sha
- options[:target_sha] = self.commit(options[:branch_name])&.sha
+ if Feature.enabled?(:commit_files_target_sha, project)
+ options[:target_sha] = self.commit(options[:branch_name])&.sha || blank_ref
+ else
+ skip_target_sha = options.delete(:skip_target_sha)
+ unless skip_target_sha
+ options[:target_sha] = self.commit(options[:branch_name])&.sha
+ end
end
with_cache_hooks { raw.commit_files(user, **options) }
diff --git a/app/services/snippets/create_service.rb b/app/services/snippets/create_service.rb
index 58da5195c44477a166a1c45e159ee30f120e5d89..4a6d2ec3a2fb439f30675849c42f3ace9c833d5f 100644
--- a/app/services/snippets/create_service.rb
+++ b/app/services/snippets/create_service.rb
@@ -126,6 +126,8 @@ def restricted_files_actions
end
def commit_attrs(snippet, msg)
+ return super if Feature.enabled?(:commit_files_target_sha, snippet.project)
+
super.merge(skip_target_sha: true)
end
end
diff --git a/config/feature_flags/gitlab_com_derisk/commit_files_target_sha.yml b/config/feature_flags/gitlab_com_derisk/commit_files_target_sha.yml
new file mode 100644
index 0000000000000000000000000000000000000000..26c7901fcc59cf8e2bad211c817fab488d165990
--- /dev/null
+++ b/config/feature_flags/gitlab_com_derisk/commit_files_target_sha.yml
@@ -0,0 +1,9 @@
+---
+name: commit_files_target_sha
+feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/517122
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/181459
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/520058
+milestone: '17.10'
+group: group::source code
+type: gitlab_com_derisk
+default_enabled: false
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 24583107f1395ad85c22be5ca2765ab28fbf7cc8..2ed9137f5a34401572a4202974e0ee19913e3e3a 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -4397,14 +4397,38 @@ def update_storages(storage_hash)
context 'with an empty branch' do
let_it_be(:project) { create(:project, :empty_repo) }
- let(:target_sha) { nil }
- it 'calls UserCommitFiles RPC' do
- expect_next_instance_of(Gitlab::GitalyClient::OperationService) do |client|
- expect(client).to receive(:user_commit_files).with(*expected_params)
+ context 'when feature flag is enabled' do
+ before do
+ stub_feature_flags(commit_files_target_sha: true)
end
- subject
+ let(:target_sha) { repository.blank_ref }
+
+ it 'calls UserCommitFiles RPC' do
+ expect_next_instance_of(Gitlab::GitalyClient::OperationService) do |client|
+ expect(client).to receive(:user_commit_files).with(*expected_params)
+ end
+
+ subject
+ end
+ end
+
+ context 'when feature flag is disabled' do
+ let(:project) { create(:project, :empty_repo) }
+ let(:target_sha) { nil }
+
+ before do
+ stub_feature_flags(commit_files_target_sha: false)
+ end
+
+ it 'calls UserCommitFiles RPC with nil target_sha' do
+ expect_next_instance_of(Gitlab::GitalyClient::OperationService) do |client|
+ expect(client).to receive(:user_commit_files).with(*expected_params)
+ end
+
+ subject
+ end
end
end
end
diff --git a/spec/services/design_management/save_designs_service_spec.rb b/spec/services/design_management/save_designs_service_spec.rb
index c8ea024282cdd28e6815b3bed7a6a42c45c330e3..006a029d192b698434cc999e02d109b4e18baf10 100644
--- a/spec/services/design_management/save_designs_service_spec.rb
+++ b/spec/services/design_management/save_designs_service_spec.rb
@@ -366,7 +366,7 @@ def repository_exists
expect { service.execute }
.to change { issue.designs.count }.from(0).to(2)
.and change { DesignManagement::Version.count }.by(1)
- .and change { Gitlab::GitalyClient.get_request_count }.by(4)
+ .and change { Gitlab::GitalyClient.get_request_count }.by(5)
.and change { commit_count }.by(1)
.and trigger_internal_events(Gitlab::UsageDataCounters::IssueActivityUniqueCounter::ISSUE_DESIGNS_ADDED)
.twice.with(user: user, project: project, category: 'InternalEventTracking')
diff --git a/spec/services/snippets/create_service_spec.rb b/spec/services/snippets/create_service_spec.rb
index 501f847b953e29392686746f8ea897b35c8562ab..d8fe2ac31c0a7febe29aa53133e9a863850fc87f 100644
--- a/spec/services/snippets/create_service_spec.rb
+++ b/spec/services/snippets/create_service_spec.rb
@@ -118,12 +118,32 @@
expect(blob.data).to eq base_opts[:content]
end
- it 'passes along correct commit attributes' do
- expect_next_instance_of(Repository) do |repository|
- expect(repository).to receive(:commit_files).with(anything, a_hash_including(skip_target_sha: true))
+ context 'when feature flag is enabled' do
+ before do
+ stub_feature_flags(commit_files_target_sha: true)
end
- subject
+ it 'does not pass skip_target_sha' do
+ expect_next_instance_of(Repository) do |repository|
+ expect(repository).to receive(:commit_files).with(anything, hash_excluding(:skip_target_sha))
+ end
+
+ subject
+ end
+ end
+
+ context 'when feature flag is disabled' do
+ before do
+ stub_feature_flags(commit_files_target_sha: false)
+ end
+
+ it 'passes skip_target_sha as true' do
+ expect_next_instance_of(Repository) do |repository|
+ expect(repository).to receive(:commit_files).with(anything, a_hash_including(skip_target_sha: true))
+ end
+
+ subject
+ end
end
context 'when repository creation action fails' do
diff --git a/spec/support/shared_examples/services/security/ci_configuration/create_service_shared_examples.rb b/spec/support/shared_examples/services/security/ci_configuration/create_service_shared_examples.rb
index 16c828c72fe4825eb1d2622728c341d3c81fa9a3..e5f87d924d7534f29e4618acd2a3fc9a2a3d2c67 100644
--- a/spec/support/shared_examples/services/security/ci_configuration/create_service_shared_examples.rb
+++ b/spec/support/shared_examples/services/security/ci_configuration/create_service_shared_examples.rb
@@ -149,16 +149,6 @@
end
end
- context 'when parsing existing ci config gives any other error' do
- let(:params) { {} }
- let_it_be(:repository) { project.repository }
-
- it 'is successful' do
- expect(repository).to receive(:commit).twice.and_return(nil)
- expect(result.status).to eq(:success)
- end
- end
-
unless skip_w_params
context 'with parameters' do
let(:params) { non_empty_params }