Commit 4e4ce830 authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Merge branch '227466-handle-container-registry-errors' into 'master'

Better support for connection errors with the Container registry

See merge request gitlab-org/gitlab!70941
parents 1855eb3a f547e8cf
......@@ -5,7 +5,7 @@ import { s__, __ } from '~/locale';
export const CONTAINER_REGISTRY_TITLE = s__('ContainerRegistry|Container Registry');
export const CONNECTION_ERROR_TITLE = s__('ContainerRegistry|Docker connection error');
export const CONNECTION_ERROR_MESSAGE = s__(
`ContainerRegistry|We are having trouble connecting to the Registry, which could be due to an issue with your project name or path. %{docLinkStart}More information%{docLinkEnd}`,
`ContainerRegistry|We are having trouble connecting to the Container Registry. Please try refreshing the page. If this error persists, please review %{docLinkStart}the troubleshooting documentation%{docLinkEnd}.`,
);
export const LIST_INTRO_TEXT = s__(
`ContainerRegistry|With the GitLab Container Registry, every project can have its own space to store images. %{docLinkStart}More information%{docLinkEnd}`,
......
......@@ -36,6 +36,8 @@ export default () => {
isAdmin,
showCleanupPolicyOnAlert,
showUnfinishedTagCleanupCallout,
connectionError,
invalidPathError,
...config
} = el.dataset;
......@@ -67,6 +69,8 @@ export default () => {
isAdmin: parseBoolean(isAdmin),
showCleanupPolicyOnAlert: parseBoolean(showCleanupPolicyOnAlert),
showUnfinishedTagCleanupCallout: parseBoolean(showUnfinishedTagCleanupCallout),
connectionError: parseBoolean(connectionError),
invalidPathError: parseBoolean(invalidPathError),
},
/* eslint-disable @gitlab/require-i18n-strings */
dockerBuildCommand: `docker build -t ${config.repositoryUrl} .`,
......
......@@ -171,6 +171,9 @@ export default {
showDeleteAlert() {
return this.deleteAlertType && this.itemToDelete?.path;
},
showConnectionError() {
return this.config.connectionError || this.config.invalidPathError;
},
deleteImageAlertMessage() {
return this.deleteAlertType === 'success'
? DELETE_IMAGE_SUCCESS_MESSAGE
......@@ -292,7 +295,7 @@ export default {
/>
<gl-empty-state
v-if="config.characterError"
v-if="showConnectionError"
:title="$options.i18n.CONNECTION_ERROR_TITLE"
:svg-path="config.containersErrorImage"
>
......
# frozen_string_literal: true
module Registry
module ConnectionErrorsHandler
extend ActiveSupport::Concern
included do
rescue_from ContainerRegistry::Path::InvalidRegistryPathError, with: :invalid_registry_path
rescue_from Faraday::Error, with: :connection_error
before_action :ping_container_registry
end
private
# rubocop:disable Gitlab/ModuleWithInstanceVariables
# These instance variables are only read by a view helper to pass
# them to the frontend
# See app/views/projects/registry/repositories/index.html.haml
# app/views/groups/registry/repositories/index.html.haml
def invalid_registry_path
@invalid_path_error = true
render :index
end
def connection_error
@connection_error = true
render :index
end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
def ping_container_registry
ContainerRegistry::Client.registry_info
end
end
end
......@@ -3,6 +3,7 @@ module Groups
module Registry
class RepositoriesController < Groups::ApplicationController
include PackagesHelper
include ::Registry::ConnectionErrorsHandler
before_action :verify_container_registry_enabled!
before_action :authorize_read_container_image!
......
......@@ -4,6 +4,7 @@ module Projects
module Registry
class RepositoriesController < ::Projects::Registry::ApplicationController
include PackagesHelper
include ::Registry::ConnectionErrorsHandler
before_action :authorize_update_container_image!, only: [:destroy]
......@@ -48,8 +49,6 @@ module Projects
repository.save! if repository.has_tags?
end
end
rescue ContainerRegistry::Path::InvalidRegistryPathError
@character_error = true
end
end
end
......
......@@ -17,5 +17,11 @@ module Types
def can_delete
Ability.allowed?(current_user, :destroy_container_image, object)
end
def tags
object.tags
rescue Faraday::Error
raise ::Gitlab::Graphql::Errors::ResourceNotAvailable, 'We are having trouble connecting to the Container Registry. If this error persists, please review the troubleshooting documentation.'
end
end
end
......@@ -28,5 +28,11 @@ module Types
def project
Gitlab::Graphql::Loaders::BatchModelLoader.new(Project, object.project_id).find
end
def tags_count
object.tags_count
rescue Faraday::Error
raise ::Gitlab::Graphql::Errors::ResourceNotAvailable, 'We are having trouble connecting to the Container Registry. If this error persists, please review the troubleshooting documentation.'
end
end
end
......@@ -16,7 +16,8 @@
is_group_page: "true",
"group_path": @group.full_path,
"gid_prefix": container_repository_gid_prefix,
character_error: @character_error.to_s,
connection_error: (!!@connection_error).to_s,
invalid_path_error: (!!@invalid_path_error).to_s,
user_callouts_path: user_callouts_path,
user_callout_id: UserCalloutsHelper::UNFINISHED_TAG_CLEANUP_CALLOUT,
show_unfinished_tag_cleanup_callout: show_unfinished_tag_cleanup_callout?.to_s } }
......@@ -20,7 +20,8 @@
"is_admin": current_user&.admin.to_s,
"show_cleanup_policy_on_alert": show_cleanup_policy_on_alert(@project).to_s,
"cleanup_policies_settings_path": project_settings_packages_and_registries_path(@project),
character_error: @character_error.to_s,
connection_error: (!!@connection_error).to_s,
invalid_path_error: (!!@invalid_path_error).to_s,
user_callouts_path: user_callouts_path,
user_callout_id: UserCalloutsHelper::UNFINISHED_TAG_CLEANUP_CALLOUT,
show_unfinished_tag_cleanup_callout: show_unfinished_tag_cleanup_callout?.to_s, } }
......@@ -229,6 +229,7 @@ RSpec.describe '[EE] Internal Project Access' do
before do
stub_container_registry_tags(repository: :any, tags: ['latest'])
stub_container_registry_config(enabled: true)
stub_container_registry_info
project.container_repositories << container_repository
end
......
......@@ -185,6 +185,7 @@ RSpec.describe '[EE] Private Project Access' do
before do
stub_container_registry_tags(repository: :any, tags: ['latest'])
stub_container_registry_config(enabled: true)
stub_container_registry_info
project.container_repositories << container_repository
end
......
......@@ -253,6 +253,7 @@ RSpec.describe '[EE] Public Project Access' do
before do
stub_container_registry_tags(repository: :any, tags: ['latest'])
stub_container_registry_config(enabled: true)
stub_container_registry_info
project.container_repositories << container_repository
end
......
......@@ -3,6 +3,8 @@
module API
class ContainerRepositories < ::API::Base
include Gitlab::Utils::StrongMemoize
include ::API::Helpers::ContainerRegistryHelpers
helpers ::API::Helpers::PackagesHelpers
before { authenticate! }
......
......@@ -3,6 +3,7 @@
module API
class GroupContainerRepositories < ::API::Base
include PaginationParams
include ::API::Helpers::ContainerRegistryHelpers
helpers ::API::Helpers::PackagesHelpers
......
......@@ -430,8 +430,8 @@ module API
render_api_error!('406 Not Acceptable', 406)
end
def service_unavailable!
render_api_error!('503 Service Unavailable', 503)
def service_unavailable!(message = nil)
render_api_error!(message || '503 Service Unavailable', 503)
end
def conflict!(message = nil)
......
# frozen_string_literal: true
module API
module Helpers
module ContainerRegistryHelpers
extend ActiveSupport::Concern
included do
rescue_from Faraday::Error, ContainerRegistry::Path::InvalidRegistryPathError do |e|
service_unavailable!('We are having trouble connecting to the Container Registry. If this error persists, please review the troubleshooting documentation.')
end
end
end
end
end
......@@ -3,6 +3,8 @@
module API
class ProjectContainerRepositories < ::API::Base
include PaginationParams
include ::API::Helpers::ContainerRegistryHelpers
helpers ::API::Helpers::PackagesHelpers
REPOSITORY_ENDPOINT_REQUIREMENTS = API::NAMESPACE_OR_PROJECT_REQUIREMENTS.merge(
......
......@@ -51,6 +51,15 @@ module ContainerRegistry
client.supports_tag_delete?
end
def self.registry_info
registry_config = Gitlab.config.registry
return unless registry_config.enabled && registry_config.api_url.present?
token = Auth::ContainerRegistryAuthenticationService.access_token([], [])
client = new(registry_config.api_url, token: token)
client.registry_info
end
def initialize(base_uri, options = {})
@base_uri = base_uri
@options = options
......
......@@ -9111,7 +9111,7 @@ msgstr ""
msgid "ContainerRegistry|To widen your search, change or remove the filters above."
msgstr ""
msgid "ContainerRegistry|We are having trouble connecting to the Registry, which could be due to an issue with your project name or path. %{docLinkStart}More information%{docLinkEnd}"
msgid "ContainerRegistry|We are having trouble connecting to the Container Registry. Please try refreshing the page. If this error persists, please review %{docLinkStart}the troubleshooting documentation%{docLinkEnd}."
msgstr ""
msgid "ContainerRegistry|With the Container Registry, every project can have its own space to store its Docker images. %{docLinkStart}More Information%{docLinkEnd}"
......
......@@ -19,6 +19,7 @@ RSpec.describe Groups::Registry::RepositoriesController do
before do
stub_container_registry_config(enabled: true)
stub_container_registry_tags(repository: :any, tags: [])
stub_container_registry_info
group.add_owner(user)
group.add_guest(guest)
sign_in(user)
......@@ -37,6 +38,18 @@ RSpec.describe Groups::Registry::RepositoriesController do
'name' => repo.name
)
end
[ContainerRegistry::Path::InvalidRegistryPathError, Faraday::Error].each do |error_class|
context "when there is a #{error_class}" do
it 'displays a connection error message' do
expect(::ContainerRegistry::Client).to receive(:registry_info).and_raise(error_class, nil, nil)
subject
expect(response).to have_gitlab_http_status(:ok)
end
end
end
end
shared_examples 'with name parameter' do
......@@ -71,6 +84,18 @@ RSpec.describe Groups::Registry::RepositoriesController do
expect(response).to have_gitlab_http_status(:ok)
expect_no_snowplow_event
end
[ContainerRegistry::Path::InvalidRegistryPathError, Faraday::Error].each do |error_class|
context "when there is an invalid path error #{error_class}" do
it 'displays a connection error message' do
expect(::ContainerRegistry::Client).to receive(:registry_info).and_raise(error_class, nil, nil)
subject
expect(response).to have_gitlab_http_status(:ok)
end
end
end
end
context 'json format' do
......
......@@ -9,6 +9,7 @@ RSpec.describe Projects::Registry::RepositoriesController do
before do
sign_in(user)
stub_container_registry_config(enabled: true)
stub_container_registry_info
end
context 'when user has access to registry' do
......@@ -30,6 +31,18 @@ RSpec.describe Projects::Registry::RepositoriesController do
expect(response).to have_gitlab_http_status(:not_found)
end
[ContainerRegistry::Path::InvalidRegistryPathError, Faraday::Error].each do |error_class|
context "when there is a #{error_class}" do
it 'displays a connection error message' do
expect(::ContainerRegistry::Client).to receive(:registry_info).and_raise(error_class, nil, nil)
go_to_index
expect(response).to have_gitlab_http_status(:ok)
end
end
end
end
shared_examples 'renders a list of repositories' do
......
......@@ -16,6 +16,7 @@ RSpec.describe 'Container Registry', :js do
sign_in(user)
stub_container_registry_config(enabled: true)
stub_container_registry_tags(repository: :any, tags: [])
stub_container_registry_info
end
it 'has a page title set' do
......@@ -57,6 +58,16 @@ RSpec.describe 'Container Registry', :js do
expect(page).to have_content 'latest'
end
[ContainerRegistry::Path::InvalidRegistryPathError, Faraday::Error].each do |error_class|
context "when there is a #{error_class}" do
before do
expect(::ContainerRegistry::Client).to receive(:registry_info).and_raise(error_class, nil, nil)
end
it_behaves_like 'handling feature network errors with the container registry'
end
end
describe 'image repo details' do
before do
visit_container_registry_details 'my/image'
......
......@@ -20,6 +20,7 @@ RSpec.describe 'Container Registry', :js do
sign_in(user)
project.add_developer(user)
stub_container_registry_config(enabled: true)
stub_container_registry_info
stub_container_registry_tags(repository: :any, tags: [])
end
......@@ -122,6 +123,16 @@ RSpec.describe 'Container Registry', :js do
expect(page).to have_content('Digest: N/A')
end
end
[ContainerRegistry::Path::InvalidRegistryPathError, Faraday::Error].each do |error_class|
context "when there is a #{error_class}" do
before do
expect(::ContainerRegistry::Client).to receive(:registry_info).and_raise(error_class, nil, nil)
end
it_behaves_like 'handling feature network errors with the container registry'
end
end
end
describe 'image repo details when image has no name' do
......
......@@ -553,6 +553,7 @@ RSpec.describe "Internal Project Access" do
before do
stub_container_registry_tags(repository: :any, tags: ['latest'])
stub_container_registry_config(enabled: true)
stub_container_registry_info
project.container_repositories << container_repository
end
......
......@@ -570,6 +570,7 @@ RSpec.describe "Private Project Access" do
before do
stub_container_registry_tags(repository: :any, tags: ['latest'])
stub_container_registry_config(enabled: true)
stub_container_registry_info
project.container_repositories << container_repository
end
......
......@@ -552,6 +552,7 @@ RSpec.describe "Public Project Access" do
before do
stub_container_registry_tags(repository: :any, tags: ['latest'])
stub_container_registry_config(enabled: true)
stub_container_registry_info
project.container_repositories << container_repository
end
......
......@@ -129,13 +129,16 @@ describe('List Page', () => {
});
});
describe('connection error', () => {
describe.each([
{ error: 'connectionError', errorName: 'connection error' },
{ error: 'invalidPathError', errorName: 'invalid path error' },
])('handling $errorName', ({ error }) => {
const config = {
characterError: true,
containersErrorImage: 'foo',
helpPagePath: 'bar',
isGroupPage: false,
};
config[error] = true;
it('should show an empty state', () => {
mountComponent({ config });
......
......@@ -111,6 +111,49 @@ RSpec.describe ContainerRegistry::Client do
it_behaves_like 'handling timeouts'
end
shared_examples 'handling repository info' do
context 'when the check is successful' do
context 'when using the GitLab container registry' do
before do
stub_registry_info(headers: {
'GitLab-Container-Registry-Version' => '2.9.1-gitlab',
'GitLab-Container-Registry-Features' => 'a,b,c'
})
end
it 'identifies the vendor as "gitlab"' do
expect(subject).to include(vendor: 'gitlab')
end
it 'identifies version and features' do
expect(subject).to include(version: '2.9.1-gitlab', features: %w[a b c])
end
end
context 'when using a third-party container registry' do
before do
stub_registry_info
end
it 'identifies the vendor as "other"' do
expect(subject).to include(vendor: 'other')
end
it 'does not identify version or features' do
expect(subject).to include(version: nil, features: [])
end
end
end
context 'when the check is not successful' do
it 'does not identify vendor, version or features' do
stub_registry_info(status: 500)
expect(subject).to eq({})
end
end
end
describe '#repository_manifest' do
subject { client.repository_manifest('group/test', 'mytag') }
......@@ -316,46 +359,7 @@ RSpec.describe ContainerRegistry::Client do
describe '#registry_info' do
subject { client.registry_info }
context 'when the check is successful' do
context 'when using the GitLab container registry' do
before do
stub_registry_info(headers: {
'GitLab-Container-Registry-Version' => '2.9.1-gitlab',
'GitLab-Container-Registry-Features' => 'a,b,c'
})
end
it 'identifies the vendor as "gitlab"' do
expect(subject).to include(vendor: 'gitlab')
end
it 'identifies version and features' do
expect(subject).to include(version: '2.9.1-gitlab', features: %w[a b c])
end
end
context 'when using a third-party container registry' do
before do
stub_registry_info
end
it 'identifies the vendor as "other"' do
expect(subject).to include(vendor: 'other')
end
it 'does not identify version or features' do
expect(subject).to include(version: nil, features: [])
end
end
end
context 'when the check is not successful' do
it 'does not identify vendor, version or features' do
stub_registry_info(status: 500)
expect(subject).to eq({})
end
end
it_behaves_like 'handling repository info'
end
describe '.supports_tag_delete?' do
......@@ -418,6 +422,16 @@ RSpec.describe ContainerRegistry::Client do
end
end
describe '.registry_info' do
subject { described_class.registry_info }
before do
stub_container_registry_config(enabled: true, api_url: registry_api_url, key: 'spec/fixtures/x509_certificate_pk.key')
end
it_behaves_like 'handling repository info'
end
def stub_upload(path, content, digest, status = 200)
stub_request(:post, "#{registry_api_url}/v2/#{path}/blobs/uploads/")
.with(headers: headers_with_accept_types)
......
......@@ -48,6 +48,19 @@ RSpec.describe API::ContainerRepositories do
expect(response).to match_response_schema('registry/repository')
end
context 'with a network error' do
before do
stub_container_registry_network_error(client_method: :repository_tags)
end
it 'returns a matching schema' do
subject
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('registry/repository')
end
end
context 'with tags param' do
let(:url) { "/registry/repositories/#{repository.id}?tags=true" }
......@@ -61,6 +74,19 @@ RSpec.describe API::ContainerRepositories do
expect(json_response['id']).to eq(repository.id)
expect(response.body).to include('tags')
end
context 'with a network error' do
before do
stub_container_registry_network_error(client_method: :repository_tags)
end
it 'returns a connection error message' do
subject
expect(response).to have_gitlab_http_status(:service_unavailable)
expect(json_response['message']).to include('We are having trouble connecting to the Container Registry')
end
end
end
context 'with tags_count param' do
......
......@@ -153,4 +153,6 @@ RSpec.describe 'container repository details' do
end
end
end
it_behaves_like 'handling graphql network errors with the container registry'
end
......@@ -14,11 +14,12 @@ RSpec.describe 'getting container repositories in a group' do
let_it_be(:container_repositories) { [container_repository, container_repositories_delete_scheduled, container_repositories_delete_failed].flatten }
let_it_be(:container_expiration_policy) { project.container_expiration_policy }
let(:excluded_fields) { [] }
let(:container_repositories_fields) do
<<~GQL
edges {
node {
#{all_graphql_fields_for('container_repositories'.classify, max_depth: 1)}
#{all_graphql_fields_for('container_repositories'.classify, max_depth: 1, excluded: excluded_fields)}
}
}
GQL
......@@ -152,6 +153,12 @@ RSpec.describe 'getting container repositories in a group' do
end
end
it_behaves_like 'handling graphql network errors with the container registry'
it_behaves_like 'not hitting graphql network errors with the container registry' do
let(:excluded_fields) { %w[tags tagsCount] }
end
it 'returns the total count of container repositories' do
subject
......
......@@ -12,11 +12,12 @@ RSpec.describe 'getting container repositories in a project' do
let_it_be(:container_repositories) { [container_repository, container_repositories_delete_scheduled, container_repositories_delete_failed].flatten }
let_it_be(:container_expiration_policy) { project.container_expiration_policy }
let(:excluded_fields) { %w[pipeline jobs] }
let(:container_repositories_fields) do
<<~GQL
edges {
node {
#{all_graphql_fields_for('container_repositories'.classify, excluded: %w(pipeline jobs))}
#{all_graphql_fields_for('container_repositories'.classify, excluded: excluded_fields)}
}
}
GQL
......@@ -151,6 +152,12 @@ RSpec.describe 'getting container repositories in a project' do
end
end
it_behaves_like 'handling graphql network errors with the container registry'
it_behaves_like 'not hitting graphql network errors with the container registry' do
let(:excluded_fields) { %w[pipeline jobs tags tagsCount] }
end
it 'returns the total count of container repositories' do
subject
......
......@@ -20,12 +20,14 @@ RSpec.describe API::GroupContainerRepositories do
end
let(:api_user) { reporter }
let(:params) { {} }
before do
group.add_reporter(reporter)
group.add_guest(guest)
stub_container_registry_config(enabled: true)
stub_container_registry_info
root_repository
test_repository
......@@ -35,10 +37,13 @@ RSpec.describe API::GroupContainerRepositories do
let(:url) { "/groups/#{group.id}/registry/repositories" }
let(:snowplow_gitlab_standard_context) { { user: api_user, namespace: group } }
subject { get api(url, api_user) }
subject { get api(url, api_user), params: params }
it_behaves_like 'rejected container repository access', :guest, :forbidden
it_behaves_like 'rejected container repository access', :anonymous, :not_found
it_behaves_like 'handling network errors with the container registry' do
let(:params) { { tags: true } }
end
it_behaves_like 'returns repositories for allowed users', :reporter, 'group' do
let(:object) { group }
......
......@@ -52,6 +52,7 @@ RSpec.describe API::ProjectContainerRepositories do
test_repository
stub_container_registry_config(enabled: true)
stub_container_registry_info
end
shared_context 'using API user' do
......@@ -105,6 +106,9 @@ RSpec.describe API::ProjectContainerRepositories do
it_behaves_like 'rejected container repository access', :guest, :forbidden unless context == 'using job token'
it_behaves_like 'rejected container repository access', :anonymous, :not_found
it_behaves_like 'a package tracking event', described_class.name, 'list_repositories'
it_behaves_like 'handling network errors with the container registry' do
let(:params) { { tags: true } }
end
it_behaves_like 'returns repositories for allowed users', :reporter, 'project' do
let(:object) { project }
......@@ -154,6 +158,7 @@ RSpec.describe API::ProjectContainerRepositories do
it_behaves_like 'rejected container repository access', :guest, :forbidden unless context == 'using job token'
it_behaves_like 'rejected container repository access', :anonymous, :not_found
it_behaves_like 'handling network errors with the container registry'
context 'for reporter' do
let(:api_user) { reporter }
......
......@@ -9,6 +9,7 @@ RSpec.describe Groups::Registry::RepositoriesController do
before do
stub_container_registry_config(enabled: true)
stub_container_registry_tags(repository: :any, tags: [])
stub_container_registry_info
group.add_reporter(user)
login_as(user)
end
......
......@@ -79,6 +79,18 @@ module StubGitlabCalls
end
end
def stub_container_registry_info(info: {})
allow(ContainerRegistry::Client)
.to receive(:registry_info)
.and_return(info)
end
def stub_container_registry_network_error(client_method:)
allow_next_instance_of(ContainerRegistry::Client) do |client|
allow(client).to receive(client_method).and_raise(::Faraday::Error, nil, nil)
end
end
def stub_commonmark_sourcepos_disabled
allow_any_instance_of(Banzai::Filter::MarkdownEngines::CommonMark)
.to receive(:render_options)
......
# frozen_string_literal: true
RSpec.shared_examples 'handling feature network errors with the container registry' do
it 'displays the error message' do
visit_container_registry
expect(page).to have_content 'We are having trouble connecting to the Container Registry'
end
end
......@@ -79,3 +79,40 @@ RSpec.shared_examples 'returns repositories for allowed users' do |user_type, sc
end
end
end
RSpec.shared_examples 'handling network errors with the container registry' do
before do
stub_container_registry_network_error(client_method: :repository_tags)
end
it 'returns a connection error' do
subject
expect(response).to have_gitlab_http_status(:service_unavailable)
expect(json_response['message']).to include('We are having trouble connecting to the Container Registry')
end
end
RSpec.shared_examples 'handling graphql network errors with the container registry' do
before do
stub_container_registry_network_error(client_method: :repository_tags)
end
it 'returns a connection error' do
subject
expect_graphql_errors_to_include('We are having trouble connecting to the Container Registry')
end
end
RSpec.shared_examples 'not hitting graphql network errors with the container registry' do
before do
stub_container_registry_network_error(client_method: :repository_tags)
end
it 'does not return any error' do
subject
expect_graphql_errors_to_be_empty
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment