From da5e9aba771d140dab3463515c6e93299907baf4 Mon Sep 17 00:00:00 2001 From: Luke Duncalfe <lduncalfe@eml.cc> Date: Thu, 20 Apr 2023 13:08:10 +1200 Subject: [PATCH 1/2] GH-style Jira DVCS endpoints return 404 by default The endpoints in `API::v3::GitHub` have been deprecated since 15.1. Due to uncertainty about the impact of a full removal in 16.0, all endpoints return `404` by default but we allow customers to toggle a flag to reverse this breaking change. See https://gitlab.com/gitlab-org/gitlab/-/issues/362168#note_1347692683. I future we will make the breaking change irreversible https://gitlab.com/gitlab-org/gitlab/-/issues/408148. Changelog: removed --- .../jira_dvcs_end_of_life_amnesty.yml | 8 ++++ ee/spec/requests/api/v3/github_spec.rb | 4 ++ lib/api/v3/github.rb | 20 +++++++-- spec/requests/api/v3/github_spec.rb | 43 +++++++++++++++++++ ...e_jira_dvcs_end_of_life_shared_examples.rb | 23 ++++++++++ 5 files changed, 95 insertions(+), 3 deletions(-) create mode 100644 config/feature_flags/development/jira_dvcs_end_of_life_amnesty.yml create mode 100644 spec/support/shared_examples/requests/api/integrations/github_enterprise_jira_dvcs_end_of_life_shared_examples.rb diff --git a/config/feature_flags/development/jira_dvcs_end_of_life_amnesty.yml b/config/feature_flags/development/jira_dvcs_end_of_life_amnesty.yml new file mode 100644 index 0000000000000000..7f0ea9f53b30bdf8 --- /dev/null +++ b/config/feature_flags/development/jira_dvcs_end_of_life_amnesty.yml @@ -0,0 +1,8 @@ +--- +name: jira_dvcs_end_of_life_amnesty +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/118126 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/408148 +milestone: '16.0' +type: development +group: group::import and integrate +default_enabled: false # This should remain false diff --git a/ee/spec/requests/api/v3/github_spec.rb b/ee/spec/requests/api/v3/github_spec.rb index f0802582e53db531..db159f929424b833 100644 --- a/ee/spec/requests/api/v3/github_spec.rb +++ b/ee/spec/requests/api/v3/github_spec.rb @@ -16,6 +16,10 @@ private_group.add_maintainer(user) end + it_behaves_like 'a GitHub Enterprise Jira DVCS reversible end of life endpoint' do + subject { get api(path, user, version: 'v3'), headers: { 'User-Agent' => user_agent } } + end + it 'returns status 200' do get api(path, user, version: 'v3'), headers: { 'User-Agent' => user_agent } diff --git a/lib/api/v3/github.rb b/lib/api/v3/github.rb index db71e823b1d85524..7d8c37cd39b03c1e 100644 --- a/lib/api/v3/github.rb +++ b/lib/api/v3/github.rb @@ -1,10 +1,11 @@ # frozen_string_literal: true +# The endpoints by default return `404` in preparation for their removal +# (also see comment above `#reversible_end_of_life!`). +# https://gitlab.com/gitlab-org/gitlab/-/issues/362168 +# # These endpoints partially mimic Github API behavior in order to successfully # integrate with Jira Development Panel. -# Endpoints returning an empty list were temporarily added to avoid 404's -# during Jira's DVCS integration. -# module API module V3 class Github < ::API::Base @@ -28,6 +29,8 @@ class Github < ::API::Base feature_category :integrations before do + reversible_end_of_life! + authorize_jira_user_agent!(request) authenticate! end @@ -38,6 +41,17 @@ class Github < ::API::Base requires :project, type: String end + # The endpoints in this class have been deprecated since 15.1. + # + # Due to uncertainty about the impact of a full removal in 16.0, all endpoints return `404` + # by default but we allow customers to toggle a flag to reverse this breaking change. + # See https://gitlab.com/gitlab-org/gitlab/-/issues/362168#note_1347692683. + # + # TODO Make the breaking change irreversible https://gitlab.com/gitlab-org/gitlab/-/issues/408148. + def reversible_end_of_life! + not_found! unless Feature.enabled?(:jira_dvcs_end_of_life_amnesty) + end + def authorize_jira_user_agent!(request) not_found! unless Gitlab::Jira::Middleware.jira_dvcs_connector?(request.env) end diff --git a/spec/requests/api/v3/github_spec.rb b/spec/requests/api/v3/github_spec.rb index 4a7b552293c8c3c5..b6fccd9b7cb21a22 100644 --- a/spec/requests/api/v3/github_spec.rb +++ b/spec/requests/api/v3/github_spec.rb @@ -13,6 +13,13 @@ end describe 'GET /orgs/:namespace/repos' do + it_behaves_like 'a GitHub Enterprise Jira DVCS reversible end of life endpoint' do + subject do + group = create(:group) + jira_get v3_api("/orgs/#{group.path}/repos", user) + end + end + it 'returns an empty array' do group = create(:group) @@ -32,6 +39,10 @@ end describe 'GET /user/repos' do + it_behaves_like 'a GitHub Enterprise Jira DVCS reversible end of life endpoint' do + subject { jira_get v3_api('/user/repos', user) } + end + it 'returns an empty array' do jira_get v3_api('/user/repos', user) @@ -117,6 +128,10 @@ describe 'GET /users/:username' do let!(:user1) { create(:user, username: 'jane.porter') } + it_behaves_like 'a GitHub Enterprise Jira DVCS reversible end of life endpoint' do + subject { jira_get v3_api("/users/#{user.username}", user) } + end + context 'user exists' do it 'responds with the expected user' do jira_get v3_api("/users/#{user.username}", user) @@ -155,6 +170,10 @@ let(:project) { create(:project, :empty_repo, path: 'project.with.dot', group: group) } let(:events_path) { "/repos/#{group.path}/#{project.path}/events" } + it_behaves_like 'a GitHub Enterprise Jira DVCS reversible end of life endpoint' do + subject { jira_get v3_api(events_path, user) } + end + context 'if there are no merge requests' do it 'returns an empty array' do jira_get v3_api(events_path, user) @@ -232,6 +251,10 @@ def perform_request describe 'GET /-/jira/pulls' do let(:route) { '/repos/-/jira/pulls' } + it_behaves_like 'a GitHub Enterprise Jira DVCS reversible end of life endpoint' do + subject { perform_request } + end + it 'returns an array of merge requests with github format' do perform_request @@ -258,6 +281,10 @@ def perform_request describe 'GET /repos/:namespace/:project/pulls' do let(:route) { "/repos/#{project.namespace.path}/#{project.path}/pulls" } + it_behaves_like 'a GitHub Enterprise Jira DVCS reversible end of life endpoint' do + subject { perform_request } + end + it 'returns an array of merge requests for the proper project in github format' do perform_request @@ -279,6 +306,10 @@ def perform_request end describe 'GET /repos/:namespace/:project/pulls/:id' do + it_behaves_like 'a GitHub Enterprise Jira DVCS reversible end of life endpoint' do + subject { jira_get v3_api("/repos/#{project.namespace.path}/#{project.path}/pulls/#{merge_request.id}", user) } + end + context 'when user has access to the merge requests' do it 'returns the requested merge request in github format' do jira_get v3_api("/repos/#{project.namespace.path}/#{project.path}/pulls/#{merge_request.id}", user) @@ -331,6 +362,10 @@ def expect_project_under_namespace(projects, namespace, user, admin_mode = false expect(json_response.size).to eq(projects.size) end + it_behaves_like 'a GitHub Enterprise Jira DVCS reversible end of life endpoint' do + subject { jira_get v3_api("/users/#{user.namespace.path}/repos", user) } + end + context 'group namespace' do let(:project) { create(:project, group: group) } let!(:project2) { create(:project, :public, group: group) } @@ -423,6 +458,10 @@ def expect_project_under_namespace(projects, namespace, user, admin_mode = false describe 'GET /repos/:namespace/:project/branches' do context 'authenticated' do + it_behaves_like 'a GitHub Enterprise Jira DVCS reversible end of life endpoint' do + subject { jira_get v3_api("/repos/#{project.namespace.path}/#{project.path}/branches", user) } + end + context 'updating project feature usage' do it 'counts Jira Cloud integration as enabled' do user_agent = 'Jira DVCS Connector Vertigo/4.42.0' @@ -516,6 +555,10 @@ def response_diff_files(response) end context 'authenticated' do + it_behaves_like 'a GitHub Enterprise Jira DVCS reversible end of life endpoint' do + subject { call_api } + end + it 'returns commit with github format' do call_api diff --git a/spec/support/shared_examples/requests/api/integrations/github_enterprise_jira_dvcs_end_of_life_shared_examples.rb b/spec/support/shared_examples/requests/api/integrations/github_enterprise_jira_dvcs_end_of_life_shared_examples.rb new file mode 100644 index 0000000000000000..6799dec7b80548d2 --- /dev/null +++ b/spec/support/shared_examples/requests/api/integrations/github_enterprise_jira_dvcs_end_of_life_shared_examples.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.shared_examples 'a GitHub Enterprise Jira DVCS reversible end of life endpoint' do + it 'is a reachable endpoint' do + subject + + expect(response).not_to have_gitlab_http_status(:not_found) + end + + context 'when the flag is disabled' do + before do + stub_feature_flags(jira_dvcs_end_of_life_amnesty: false) + end + + it 'presents as an endpoint that does not exist' do + subject + + expect(response).to have_gitlab_http_status(:not_found) + end + end +end -- GitLab From e79cd910e9b7e9d12ba9821d01846b9de61dc3c3 Mon Sep 17 00:00:00 2001 From: Andy Soiron <asoiron@gitlab.com> Date: Tue, 2 May 2023 03:26:48 +0000 Subject: [PATCH 2/2] Apply 1 suggestion(s) to 1 file(s) --- .../feature_flags/development/jira_dvcs_end_of_life_amnesty.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/feature_flags/development/jira_dvcs_end_of_life_amnesty.yml b/config/feature_flags/development/jira_dvcs_end_of_life_amnesty.yml index 7f0ea9f53b30bdf8..1d0f364580af5cc0 100644 --- a/config/feature_flags/development/jira_dvcs_end_of_life_amnesty.yml +++ b/config/feature_flags/development/jira_dvcs_end_of_life_amnesty.yml @@ -5,4 +5,4 @@ rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/408148 milestone: '16.0' type: development group: group::import and integrate -default_enabled: false # This should remain false +default_enabled: true -- GitLab