diff --git a/Gemfile b/Gemfile index 6674edc1d0c..20bf1c43f45 100644 --- a/Gemfile +++ b/Gemfile @@ -416,7 +416,7 @@ group :ed25519 do end # Gitaly GRPC client -gem 'gitaly-proto', '~> 0.118.1', require: 'gitaly' +gem 'gitaly-proto', '~> 0.123.0', require: 'gitaly' gem 'grpc', '~> 1.15.0' gem 'google-protobuf', '~> 3.6' diff --git a/Gemfile.lock b/Gemfile.lock index e755b0e0a8d..d9aae0b5297 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -274,9 +274,8 @@ GEM gettext_i18n_rails (>= 0.7.1) po_to_json (>= 1.0.0) rails (>= 3.2.0) - gitaly-proto (0.118.1) - google-protobuf (~> 3.1) - grpc (~> 1.10) + gitaly-proto (0.123.0) + grpc (~> 1.0) github-markup (1.7.0) gitlab-markup (1.6.4) gitlab-sidekiq-fetcher (0.3.0) @@ -1000,7 +999,7 @@ DEPENDENCIES gettext (~> 3.2.2) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.3) - gitaly-proto (~> 0.118.1) + gitaly-proto (~> 0.123.0) github-markup (~> 1.7.0) gitlab-markup (~> 1.6.4) gitlab-sidekiq-fetcher @@ -1158,4 +1157,4 @@ DEPENDENCIES wikicloth (= 0.8.1) BUNDLED WITH - 1.16.6 + 1.17.1 diff --git a/Gemfile.rails5.lock b/Gemfile.rails5.lock index 6ae7444f18f..b656365103f 100644 --- a/Gemfile.rails5.lock +++ b/Gemfile.rails5.lock @@ -277,9 +277,8 @@ GEM gettext_i18n_rails (>= 0.7.1) po_to_json (>= 1.0.0) rails (>= 3.2.0) - gitaly-proto (0.118.1) - google-protobuf (~> 3.1) - grpc (~> 1.10) + gitaly-proto (0.123.0) + grpc (~> 1.0) github-markup (1.7.0) gitlab-markup (1.6.4) gitlab-sidekiq-fetcher (0.3.0) @@ -1009,7 +1008,7 @@ DEPENDENCIES gettext (~> 3.2.2) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.3) - gitaly-proto (~> 0.118.1) + gitaly-proto (~> 0.123.0) github-markup (~> 1.7.0) gitlab-markup (~> 1.6.4) gitlab-sidekiq-fetcher @@ -1167,4 +1166,4 @@ DEPENDENCIES wikicloth (= 0.8.1) BUNDLED WITH - 1.16.6 + 1.17.1 diff --git a/app/models/repository.rb b/app/models/repository.rb index ee5579329a8..6e179f61a7b 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -1014,6 +1014,18 @@ class Repository message: merge_request.title) end + def update_submodule(user, submodule, commit_sha, message:, branch:) + with_cache_hooks do + raw.update_submodule( + user: user, + submodule: submodule, + commit_sha: commit_sha, + branch: branch, + message: message + ) + end + end + def blob_data_at(sha, path) blob = blob_at(sha, path) return unless blob diff --git a/app/services/submodules/update_service.rb b/app/services/submodules/update_service.rb new file mode 100644 index 00000000000..a6011a920bd --- /dev/null +++ b/app/services/submodules/update_service.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module Submodules + class UpdateService < Commits::CreateService + include Gitlab::Utils::StrongMemoize + + def initialize(*args) + super + + @start_branch = @branch_name + @commit_sha = params[:commit_sha].presence + @submodule = params[:submodule].presence + @commit_message = params[:commit_message].presence || "Update submodule #{@submodule} with oid #{@commit_sha}" + end + + def validate! + super + + raise ValidationError, 'The repository is empty' if repository.empty? + end + + def execute + super + rescue StandardError => e + error(e.message) + end + + def create_commit! + repository.update_submodule(current_user, + @submodule, + @commit_sha, + message: @commit_message, + branch: @branch_name) + rescue ArgumentError, TypeError + raise ValidationError, 'Invalid parameters' + end + end +end diff --git a/changelogs/unreleased/fj-41213-api-update-submodule-commit.yml b/changelogs/unreleased/fj-41213-api-update-submodule-commit.yml new file mode 100644 index 00000000000..c06b02b05e8 --- /dev/null +++ b/changelogs/unreleased/fj-41213-api-update-submodule-commit.yml @@ -0,0 +1,5 @@ +--- +title: Add endpoint to update a git submodule reference +merge_request: 20949 +author: +type: added diff --git a/doc/api/README.md b/doc/api/README.md index a620a13a3b3..19abbdc7a1e 100644 --- a/doc/api/README.md +++ b/doc/api/README.md @@ -61,6 +61,7 @@ following locations: - [Protected Tags](protected_tags.md) - [Repositories](repositories.md) - [Repository Files](repository_files.md) +- [Repository Submodules](repository_submodules.md) - [Runners](runners.md) - [Search](search.md) - [Services](services.md) @@ -234,7 +235,7 @@ provided you are authenticated as an administrator with an OAuth or Personal Acc You need to pass the `sudo` parameter either via query string or a header with an ID/username of the user you want to perform the operation as. If passed as a header, the -header name must be `Sudo`. +header name must be `Sudo`. NOTE: **Note:** Usernames are case insensitive. diff --git a/doc/api/repository_submodules.md b/doc/api/repository_submodules.md new file mode 100644 index 00000000000..2e6797f18f4 --- /dev/null +++ b/doc/api/repository_submodules.md @@ -0,0 +1,49 @@ +# Repository submodules API + +> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/41213) in GitLab 11.5 + +## Update existing submodule reference in repository + +In some workflows, especially automated ones, it can be useful to update a +submodule's reference to keep up to date other projects that use it. +This endpoint allows you to update a [Git submodule](https://git-scm.com/book/en/v2/Git-Tools-Submodules) reference in a +specific branch. + +``` +PUT /projects/:id/repository/submodules/:submodule +``` + +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user | +| `submodule` | string | yes | URL encoded full path to the submodule. For example, `lib%2Fclass%2Erb` | +| `branch` | string | yes | Name of the branch to commit into | +| `commit_sha` | string | yes | Full commit SHA to update the submodule to | +| `commit_message` | string | no | Commit message. If no message is provided, a default one will be set | + +```sh +curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/projects/5/repositories/submodules/lib%2Fmodules%2Fexample" +--data "branch=master&commit_sha=3ddec28ea23acc5caa5d8331a6ecb2a65fc03e88&commit_message=Update submodule reference" +``` + +Example response: + +```json +{ + "id": "ed899a2f4b50b4370feeea94676502b42383c746", + "short_id": "ed899a2f4b5", + "title": "Updated submodule example_submodule with oid 3ddec28ea23acc5caa5d8331a6ecb2a65fc03e88", + "author_name": "Dmitriy Zaporozhets", + "author_email": "dzaporozhets@sphereconsultinginc.com", + "committer_name": "Dmitriy Zaporozhets", + "committer_email": "dzaporozhets@sphereconsultinginc.com", + "created_at": "2018-09-20T09:26:24.000-07:00", + "message": "Updated submodule example_submodule with oid 3ddec28ea23acc5caa5d8331a6ecb2a65fc03e88", + "parent_ids": [ + "ae1d9fb46aa2b07ee9836d49862ec4e2c46fbbba" + ], + "committed_date": "2018-09-20T09:26:24.000-07:00", + "authored_date": "2018-09-20T09:26:24.000-07:00", + "status": null +} +``` diff --git a/lib/api/api.rb b/lib/api/api.rb index c49c52213bf..8e259961828 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -143,6 +143,7 @@ module API mount ::API::Settings mount ::API::SidekiqMetrics mount ::API::Snippets + mount ::API::Submodules mount ::API::Subscriptions mount ::API::SystemHooks mount ::API::Tags diff --git a/lib/api/submodules.rb b/lib/api/submodules.rb new file mode 100644 index 00000000000..72d7d994102 --- /dev/null +++ b/lib/api/submodules.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +module API + class Submodules < Grape::API + before { authenticate! } + + helpers do + def commit_params(attrs) + { + submodule: attrs[:submodule], + commit_sha: attrs[:commit_sha], + branch_name: attrs[:branch], + commit_message: attrs[:commit_message] + } + end + end + + params do + requires :id, type: String, desc: 'The project ID' + end + resource :projects, requirements: Files::FILE_ENDPOINT_REQUIREMENTS do + desc 'Update existing submodule reference in repository' do + success Entities::Commit + end + params do + requires :submodule, type: String, desc: 'Url encoded full path to submodule.' + requires :commit_sha, type: String, desc: 'Commit sha to update the submodule to.' + requires :branch, type: String, desc: 'Name of the branch to commit into.' + optional :commit_message, type: String, desc: 'Commit message. If no message is provided a default one will be set.' + end + put ":id/repository/submodules/:submodule", requirements: Files::FILE_ENDPOINT_REQUIREMENTS do + authorize! :push_code, user_project + + submodule_params = declared_params(include_missing: false) + + result = ::Submodules::UpdateService.new(user_project, current_user, commit_params(submodule_params)).execute + + if result[:status] == :success + commit_detail = user_project.repository.commit(result[:result]) + present commit_detail, with: Entities::CommitDetail + else + render_api_error!(result[:message], result[:http_status] || 400) + end + end + end + end +end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 20cd257bb98..1642c4c5687 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -571,6 +571,20 @@ module Gitlab end end + def update_submodule(user:, submodule:, commit_sha:, message:, branch:) + args = { + user: user, + submodule: submodule, + commit_sha: commit_sha, + branch: branch, + message: message + } + + wrapped_gitaly_errors do + gitaly_operation_client.user_update_submodule(args) + end + end + # Delete the specified branch from the repository def delete_branch(branch_name) wrapped_gitaly_errors do diff --git a/lib/gitlab/gitaly_client/operation_service.rb b/lib/gitlab/gitaly_client/operation_service.rb index 0f148614b20..1f42f657f68 100644 --- a/lib/gitlab/gitaly_client/operation_service.rb +++ b/lib/gitlab/gitaly_client/operation_service.rb @@ -230,6 +230,32 @@ module Gitlab response.squash_sha end + def user_update_submodule(user:, submodule:, commit_sha:, branch:, message:) + request = Gitaly::UserUpdateSubmoduleRequest.new( + repository: @gitaly_repo, + user: Gitlab::Git::User.from_gitlab(user).to_gitaly, + commit_sha: commit_sha, + branch: encode_binary(branch), + submodule: encode_binary(submodule), + commit_message: encode_binary(message) + ) + + response = GitalyClient.call( + @repository.storage, + :operation_service, + :user_update_submodule, + request + ) + + if response.pre_receive_error.present? + raise Gitlab::Git::PreReceiveError, response.pre_receive_error + elsif response.commit_error.present? + raise Gitlab::Git::CommitError, response.commit_error + else + Gitlab::Git::OperationService::BranchUpdate.from_gitaly(response.branch_update) + end + end + def user_commit_files( user, branch_name, commit_message, actions, author_email, author_name, start_branch_name, start_repository) diff --git a/spec/requests/api/submodules_spec.rb b/spec/requests/api/submodules_spec.rb new file mode 100644 index 00000000000..fa447c028c2 --- /dev/null +++ b/spec/requests/api/submodules_spec.rb @@ -0,0 +1,99 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe API::Submodules do + let(:user) { create(:user) } + let!(:project) { create(:project, :repository, namespace: user.namespace ) } + let(:guest) { create(:user) { |u| project.add_guest(u) } } + let(:submodule) { 'six' } + let(:commit_sha) { 'e25eda1fece24ac7a03624ed1320f82396f35bd8' } + let(:branch) { 'master' } + let(:commit_message) { 'whatever' } + + let(:params) do + { + submodule: submodule, + commit_sha: commit_sha, + branch: branch, + commit_message: commit_message + } + end + + before do + project.add_developer(user) + end + + def route(submodule = nil) + "/projects/#{project.id}/repository/submodules/#{submodule}" + end + + describe "PUT /projects/:id/repository/submodule/:submodule" do + context 'when unauthenticated' do + it 'returns 401' do + put api(route(submodule)), params + + expect(response).to have_gitlab_http_status(401) + end + end + + context 'when authenticated', 'as a guest' do + it 'returns 403' do + put api(route(submodule), guest), params + + expect(response).to have_gitlab_http_status(403) + end + end + + context 'when authenticated', 'as a developer' do + it 'returns 400 if params is missing' do + put api(route(submodule), user) + + expect(response).to have_gitlab_http_status(400) + end + + it 'returns 400 if branch is missing' do + put api(route(submodule), user), params.except(:branch) + + expect(response).to have_gitlab_http_status(400) + end + + it 'returns 400 if commit_sha is missing' do + put api(route(submodule), user), params.except(:commit_sha) + + expect(response).to have_gitlab_http_status(400) + end + + it 'returns the commmit' do + head_commit = project.repository.commit.id + + put api(route(submodule), user), params + + expect(response).to have_gitlab_http_status(200) + expect(json_response['message']).to eq commit_message + expect(json_response['author_name']).to eq user.name + expect(json_response['committer_name']).to eq user.name + expect(json_response['parent_ids'].first).to eq head_commit + end + + context 'when the submodule name is urlencoded' do + let(:submodule) { 'test_inside_folder/another_folder/six' } + let(:branch) { 'submodule_inside_folder' } + let(:encoded_submodule) { CGI.escape(submodule) } + + it 'returns the commmit' do + expect(Submodules::UpdateService) + .to receive(:new) + .with(any_args, hash_including(submodule: submodule)) + .and_call_original + + put api(route(encoded_submodule), user), params + + expect(response).to have_gitlab_http_status(200) + expect(json_response['id']).to eq project.repository.commit(branch).id + expect(project.repository.blob_at(branch, submodule).id).to eq commit_sha + end + end + end + end +end diff --git a/spec/services/submodules/update_service_spec.rb b/spec/services/submodules/update_service_spec.rb new file mode 100644 index 00000000000..cf92350c1b2 --- /dev/null +++ b/spec/services/submodules/update_service_spec.rb @@ -0,0 +1,212 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe Submodules::UpdateService do + let(:project) { create(:project, :repository) } + let(:repository) { project.repository } + let(:user) { create(:user, :commit_email) } + let(:branch_name) { project.default_branch } + let(:submodule) { 'six' } + let(:commit_sha) { 'e25eda1fece24ac7a03624ed1320f82396f35bd8' } + let(:commit_message) { 'whatever' } + let(:current_sha) { repository.blob_at('HEAD', submodule).id } + let(:commit_params) do + { + submodule: submodule, + commit_message: commit_message, + commit_sha: commit_sha, + branch_name: branch_name + } + end + + subject { described_class.new(project, user, commit_params) } + + describe "#execute" do + shared_examples 'returns error result' do + it do + result = subject.execute + + expect(result[:status]).to eq :error + expect(result[:message]).to eq error_message + end + end + + context 'when the user is not authorized' do + it_behaves_like 'returns error result' do + let(:error_message) { 'You are not allowed to push into this branch' } + end + end + + context 'when the user is authorized' do + before do + project.add_maintainer(user) + end + + context 'when the branch is protected' do + before do + create(:protected_branch, :no_one_can_push, project: project, name: branch_name) + end + + it_behaves_like 'returns error result' do + let(:error_message) { 'You are not allowed to push into this branch' } + end + end + + context 'validations' do + context 'when submodule' do + context 'is empty' do + let(:submodule) { '' } + + it_behaves_like 'returns error result' do + let(:error_message) { 'Invalid parameters' } + end + end + + context 'is not present' do + let(:submodule) { nil } + + it_behaves_like 'returns error result' do + let(:error_message) { 'Invalid parameters' } + end + end + + context 'is invalid' do + let(:submodule) { 'VERSION' } + + it_behaves_like 'returns error result' do + let(:error_message) { 'Invalid submodule path' } + end + end + + context 'does not exist' do + let(:submodule) { 'non-existent-submodule' } + + it_behaves_like 'returns error result' do + let(:error_message) { 'Invalid submodule path' } + end + end + + context 'has traversal path' do + let(:submodule) { '../six' } + + it_behaves_like 'returns error result' do + let(:error_message) { 'Invalid parameters' } + end + end + end + + context 'commit_sha' do + context 'is empty' do + let(:commit_sha) { '' } + + it_behaves_like 'returns error result' do + let(:error_message) { 'Invalid parameters' } + end + end + + context 'is not present' do + let(:commit_sha) { nil } + + it_behaves_like 'returns error result' do + let(:error_message) { 'Invalid parameters' } + end + end + + context 'is invalid' do + let(:commit_sha) { '1' } + + it_behaves_like 'returns error result' do + let(:error_message) { 'Invalid parameters' } + end + end + + context 'is the same as the current ref' do + let(:commit_sha) { current_sha } + + it_behaves_like 'returns error result' do + let(:error_message) { "The submodule #{submodule} is already at #{commit_sha}" } + end + end + end + + context 'branch_name' do + context 'is empty' do + let(:branch_name) { '' } + + it_behaves_like 'returns error result' do + let(:error_message) { 'You can only create or edit files when you are on a branch' } + end + end + + context 'is not present' do + let(:branch_name) { nil } + + it_behaves_like 'returns error result' do + let(:error_message) { 'You can only create or edit files when you are on a branch' } + end + end + + context 'does not exist' do + let(:branch_name) { 'non/existent-branch' } + + it_behaves_like 'returns error result' do + let(:error_message) { 'You can only create or edit files when you are on a branch' } + end + end + + context 'when commit message is empty' do + let(:commit_message) { '' } + + it 'a default commit message is set' do + message = "Update submodule #{submodule} with oid #{commit_sha}" + + expect(repository).to receive(:update_submodule).with(any_args, hash_including(message: message)) + + subject.execute + end + end + end + end + + context 'when there is an unexpected error' do + before do + allow(repository).to receive(:update_submodule).and_raise(StandardError, 'error message') + end + + it_behaves_like 'returns error result' do + let(:error_message) { 'error message' } + end + end + + it 'updates the submodule reference' do + result = subject.execute + + expect(result[:status]).to eq :success + expect(result[:result]).to eq repository.head_commit.id + expect(repository.blob_at('HEAD', submodule).id).to eq commit_sha + end + + context 'when submodule is inside a directory' do + let(:submodule) { 'test_inside_folder/another_folder/six' } + let(:branch_name) { 'submodule_inside_folder' } + + it 'updates the submodule reference' do + expect(repository.blob_at(branch_name, submodule).id).not_to eq commit_sha + + subject.execute + + expect(repository.blob_at(branch_name, submodule).id).to eq commit_sha + end + end + + context 'when repository is empty' do + let(:project) { create(:project, :empty_repo) } + let(:branch_name) { 'master' } + + it_behaves_like 'returns error result' do + let(:error_message) { 'The repository is empty' } + end + end + end + end +end diff --git a/spec/support/helpers/test_env.rb b/spec/support/helpers/test_env.rb index 80b96f20e3f..9e87b877b93 100644 --- a/spec/support/helpers/test_env.rb +++ b/spec/support/helpers/test_env.rb @@ -58,7 +58,8 @@ module TestEnv 'before-create-delete-modify-move' => '845009f', 'between-create-delete-modify-move' => '3f5f443', 'after-create-delete-modify-move' => 'ba3faa7', - 'with-codeowners' => '219560e' + 'with-codeowners' => '219560e', + 'submodule_inside_folder' => 'b491b92' }.freeze # gitlab-test-fork is a fork of gitlab-fork, but we don't necessarily