Merge branch '45104-special-characters-in-project-name-path-prevent-users-from-using-the-container-registry' into 'master'

Resolve "Special characters in project name/path prevent users from using the container registry"

Closes #62770 and #45104

See merge request gitlab-org/gitlab-ce!29616

12 files changed, 219 insertions, 89 deletions

diff --git a/app/assets/javascripts/registry/components/app.vue b/app/assets/javascripts/registry/components/app.vue
index ee973017387..7752723baac 100644
--- a/app/assets/javascripts/registry/components/app.vue
+++ b/app/assets/javascripts/registry/components/app.vue
@@ -3,22 +3,81 @@ import { mapGetters, mapActions } from 'vuex';
 import { GlLoadingIcon } from '@gitlab/ui';
 import store from '../stores';
 import CollapsibleContainer from './collapsible_container.vue';
+import SvgMessage from './svg_message.vue';
+import { s__, sprintf } from '../../locale';
 
 export default {
   name: 'RegistryListApp',
   components: {
     CollapsibleContainer,
     GlLoadingIcon,
+    SvgMessage,
   },
   props: {
     endpoint: {
       type: String,
       required: true,
     },
+    characterError: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+    helpPagePath: {
+      type: String,
+      required: true,
+    },
+    noContainersImage: {
+      type: String,
+      required: true,
+    },
+    containersErrorImage: {
+      type: String,
+      required: true,
+    },
+    repositoryUrl: {
+      type: String,
+      required: true,
+    },
   },
   store,
   computed: {
     ...mapGetters(['isLoading', 'repos']),
+    dockerConnectionErrorText() {
+      return sprintf(
+        s__(`ContainerRegistry|We are having trouble connecting to Docker, which could be due to an
+            issue with your project name or path. For more information, please review the
+            %{docLinkStart}Container Registry documentation%{docLinkEnd}.`),
+        {
+          docLinkStart: `<a href="${this.helpPagePath}#docker-connection-error">`,
+          docLinkEnd: '</a>',
+        },
+        false,
+      );
+    },
+    introText() {
+      return sprintf(
+        s__(`ContainerRegistry|With the Docker Container Registry integrated into GitLab, every 
+            project can have its own space to store its Docker images. Learn more about the 
+            %{docLinkStart}Container Registry%{docLinkEnd}.`),
+        {
+          docLinkStart: `<a href="${this.helpPagePath}">`,
+          docLinkEnd: '</a>',
+        },
+        false,
+      );
+    },
+    noContainerImagesText() {
+      return sprintf(
+        s__(`ContainerRegistry|With the Container Registry, every project can have its own space to
+            store its Docker images. Learn more about the %{docLinkStart}Container Registry%{docLinkEnd}.`),
+        {
+          docLinkStart: `<a href="${this.helpPagePath}">`,
+          docLinkEnd: '</a>',
+        },
+        false,
+      );
+    },
   },
   created() {
     this.setMainEndpoint(this.endpoint);
@@ -33,20 +92,44 @@ export default {
 </script>
 <template>
   <div>
-    <gl-loading-icon v-if="isLoading" size="md" />
+    <svg-message v-if="characterError" id="invalid-characters" :svg-path="containersErrorImage">
+      <h4>
+        {{ s__('ContainerRegistry|Docker connection error') }}
+      </h4>
+      <p v-html="dockerConnectionErrorText"></p>
+    </svg-message>
+
+    <gl-loading-icon v-else-if="isLoading" size="md" class="prepend-top-16" />
+
+    <div v-else-if="!isLoading && !characterError && repos.length">
+      <h4>{{ s__('ContainerRegistry|Container Registry') }}</h4>
+      <p v-html="introText"></p>
+      <collapsible-container v-for="item in repos" :key="item.id" :repo="item" />
+    </div>
+
+    <svg-message
+      v-else-if="!isLoading && !characterError && !repos.length"
+      id="no-container-images"
+      :svg-path="noContainersImage"
+    >
+      <h4>
+        {{ s__('ContainerRegistry|There are no container images stored for this project') }}
+      </h4>
+      <p v-html="noContainerImagesText"></p>
 
-    <collapsible-container
-      v-for="item in repos"
-      v-else-if="!isLoading && repos.length"
-      :key="item.id"
-      :repo="item"
-    />
+      <h5>{{ s__('ContainerRegistry|Quick Start') }}</h5>
+      <p>
+        {{
+          s__(
+            'ContainerRegistry|You can add an image to this registry with the following commands:',
+          )
+        }}
+      </p>
 
-    <p v-else-if="!isLoading && !repos.length">
-      {{
-        __(`No container images stored for this project.
-      Add one by following the instructions above.`)
-      }}
-    </p>
+      <pre>
+        docker build -t {{ repositoryUrl }} .
+        docker push {{ repositoryUrl }}
+      </pre>
+    </svg-message>
   </div>
 </template>
diff --git a/app/assets/javascripts/registry/components/svg_message.vue b/app/assets/javascripts/registry/components/svg_message.vue
new file mode 100644
index 00000000000..d0d44bf2d14
--- /dev/null
+++ b/app/assets/javascripts/registry/components/svg_message.vue
@@ -0,0 +1,24 @@
+<script>
+export default {
+  name: 'RegistrySvgMessage',
+  props: {
+    id: {
+      type: String,
+      required: true,
+    },
+    svgPath: {
+      type: String,
+      required: true,
+    },
+  },
+};
+</script>
+
+<template>
+  <div :id="id" class="empty-state container-message mw-70p">
+    <div class="svg-content">
+      <img :src="svgPath" class="flex-align-self-center" />
+    </div>
+    <slot></slot>
+  </div>
+</template>
diff --git a/app/assets/javascripts/registry/index.js b/app/assets/javascripts/registry/index.js
index 025afefe7f0..d8daec29fda 100644
--- a/app/assets/javascripts/registry/index.js
+++ b/app/assets/javascripts/registry/index.js
@@ -14,12 +14,22 @@ export default () =>
       const { dataset } = document.querySelector(this.$options.el);
       return {
         endpoint: dataset.endpoint,
+        characterError: Boolean(dataset.characterError),
+        helpPagePath: dataset.helpPagePath,
+        noContainersImage: dataset.noContainersImage,
+        containersErrorImage: dataset.containersErrorImage,
+        repositoryUrl: dataset.repositoryUrl,
       };
     },
     render(createElement) {
       return createElement('registry-app', {
         props: {
           endpoint: this.endpoint,
+          characterError: this.characterError,
+          helpPagePath: this.helpPagePath,
+          noContainersImage: this.noContainersImage,
+          containersErrorImage: this.containersErrorImage,
+          repositoryUrl: this.repositoryUrl,
         },
       });
     },
diff --git a/app/assets/stylesheets/pages/container_registry.scss b/app/assets/stylesheets/pages/container_registry.scss
index dfff3e15556..cca5214a508 100644
--- a/app/assets/stylesheets/pages/container_registry.scss
+++ b/app/assets/stylesheets/pages/container_registry.scss
@@ -2,6 +2,12 @@
  * Container Registry
  */
 
+.container-message {
+  pre {
+    white-space: pre-line;
+  }
+}
+
 .container-image {
   border-bottom: 1px solid $white-normal;
 }
diff --git a/app/controllers/projects/registry/repositories_controller.rb b/app/controllers/projects/registry/repositories_controller.rb
index 6d60117c37d..e205e2fd4f8 100644
--- a/app/controllers/projects/registry/repositories_controller.rb
+++ b/app/controllers/projects/registry/repositories_controller.rb
@@ -46,6 +46,8 @@ module Projects
             repository.save! if repository.has_tags?
           end
         end
+      rescue ContainerRegistry::Path::InvalidRegistryPathError
+        @character_error = true
       end
     end
   end
diff --git a/app/views/projects/registry/repositories/index.html.haml b/app/views/projects/registry/repositories/index.html.haml
index db1f15f96b8..e34973f1f43 100644
--- a/app/views/projects/registry/repositories/index.html.haml
+++ b/app/views/projects/registry/repositories/index.html.haml
@@ -1,49 +1,9 @@
-- page_title "Container Registry"
-
 %section
-  .settings-header
-    %h4
-      = page_title
-    %p
-      = s_('ContainerRegistry|With the Docker Container Registry integrated into GitLab, every project can have its own space to store its Docker images.')
-    %p.append-bottom-0
-      = succeed '.' do
-        = s_('ContainerRegistry|Learn more about')
-        = link_to _('Container Registry'), help_page_path('user/project/container_registry'), target: '_blank'
   .row.registry-placeholder.prepend-bottom-10
-    .col-lg-12
-      #js-vue-registry-images{ data: { endpoint: project_container_registry_index_path(@project, format: :json) } }
-
-  .row.prepend-top-10
-    .col-lg-12
-      .card
-        .card-header
-          = s_('ContainerRegistry|How to use the Container Registry')
-        .card-body
-          %p
-            - link_token = link_to(_('personal access token'), help_page_path('user/profile/account/two_factor_authentication', anchor: 'personal-access-tokens'), target: '_blank')
-            - link_2fa = link_to(_('2FA enabled'), help_page_path('user/profile/account/two_factor_authentication'), target: '_blank')
-            = s_('ContainerRegistry|First log in to GitLab&rsquo;s Container Registry using your GitLab username and password. If you have %{link_2fa} you need to use a %{link_token}:').html_safe % { link_2fa: link_2fa, link_token: link_token }
-          %pre
-            docker login #{Gitlab.config.registry.host_port}
-          %br
-          %p
-            - deploy_token = link_to(_('deploy token'), help_page_path('user/project/deploy_tokens/index', anchor: 'read-container-registry-images'), target: '_blank')
-            = s_('ContainerRegistry|You can also use a %{deploy_token} for read-only access to the registry images.').html_safe % { deploy_token: deploy_token }
-          %br
-          %p
-            = s_('ContainerRegistry|Once you log in, you&rsquo;re free to create and upload a container image using the common %{build} and %{push} commands').html_safe % { build: "<code>build</code>".html_safe, push: "<code>push</code>".html_safe }
-          %pre
-            :plain
-              docker build -t #{escape_once(@project.container_registry_url)} .
-              docker push #{escape_once(@project.container_registry_url)}
-          %hr
-          %h5.prepend-top-default
-            = s_('ContainerRegistry|Use different image names')
-          %p.light
-            = s_('ContainerRegistry|GitLab supports up to 3 levels of image names. The following examples of images are valid for your project:')
-          %pre
-            :plain
-              #{escape_once(@project.container_registry_url)}:tag
-              #{escape_once(@project.container_registry_url)}/optional-image-name:tag
-              #{escape_once(@project.container_registry_url)}/optional-name/optional-image-name:tag
+    .col-12
+      #js-vue-registry-images{ data: { endpoint: project_container_registry_index_path(@project, format: :json),
+        "help_page_path" => help_page_path('user/project/container_registry'),
+        "no_containers_image" => image_path('illustrations/docker-empty-state.svg'),
+        "containers_error_image" => image_path('illustrations/docker-error-state.svg'),
+        "repository_url" => escape_once(@project.container_registry_url),
+        character_error: @character_error.to_s } }
diff --git a/changelogs/unreleased/45104-special-characters-in-project-name-path-prevent-users-from-using-the-container-registry.yml b/changelogs/unreleased/45104-special-characters-in-project-name-path-prevent-users-from-using-the-container-registry.yml
new file mode 100644
index 00000000000..ddde0cc9c39
--- /dev/null
+++ b/changelogs/unreleased/45104-special-characters-in-project-name-path-prevent-users-from-using-the-container-registry.yml
@@ -0,0 +1,5 @@
+---
+title: Updated container registry to display error message when special characters in path. Documentation has also been updated.
+merge_request: 29616
+author:
+type: changed
diff --git a/doc/administration/container_registry.md b/doc/administration/container_registry.md
index 4d55f2357c1..2e4b4efa0ac 100644
--- a/doc/administration/container_registry.md
+++ b/doc/administration/container_registry.md
@@ -689,6 +689,20 @@ You can add a configuration option for backwards compatibility.
 
 1. Restart the registry for the changes to take affect.
 
+### Docker connection error
+
+A Docker connection error can occur when there are special characters in either the group,
+project or branch name. Special characters can include:
+
+* Leading underscore
+* Trailing hyphen/dash
+* Double hyphen/dash
+
+To get around this, you can [change the group path](../user/group/index.md#changing-a-groups-path), 
+[change the project path](../user/project/settings/index.md#renaming-a-repository) or change the 
+branch name. Another option is to create a [push rule](../push_rules/push_rules.html) to prevent 
+this at the instance level.
+
 
 [ce-18239]: https://gitlab.com/gitlab-org/gitlab-ce/issues/18239
 [docker-insecure-self-signed]: https://docs.docker.com/registry/insecure/#use-self-signed-certificates
diff --git a/doc/user/project/container_registry.md b/doc/user/project/container_registry.md
index fdf9ce3e225..7d567da1c9a 100644
--- a/doc/user/project/container_registry.md
+++ b/doc/user/project/container_registry.md
@@ -168,6 +168,19 @@ curl localhost:5001/debug/health
 curl localhost:5001/debug/vars
 ```
 
+#### Docker connection error
+
+A Docker connection error can occur when there are special characters in either the group,
+project or branch name. Special characters can include:
+
+* Leading underscore
+* Trailing hyphen/dash
+* Double hyphen/dash
+
+To get around this, you can [change the group path](../group/index.md#changing-a-groups-path),
+[change the project path](../project/settings/index.md#renaming-a-repository) or chanage the branch
+name.
+
 ### Advanced Troubleshooting
 
 >**NOTE:** The following section is only recommended for experts.
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index c57a974fe11..d20692210fd 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -348,9 +348,6 @@ msgstr ""
 msgid "2FA"
 msgstr ""
 
-msgid "2FA enabled"
-msgstr ""
-
 msgid "2FADevice|Registered On"
 msgstr ""
 
@@ -2943,25 +2940,19 @@ msgstr ""
 msgid "Container registry images"
 msgstr ""
 
-msgid "ContainerRegistry|First log in to GitLab&rsquo;s Container Registry using your GitLab username and password. If you have %{link_2fa} you need to use a %{link_token}:"
-msgstr ""
-
-msgid "ContainerRegistry|GitLab supports up to 3 levels of image names. The following examples of images are valid for your project:"
+msgid "ContainerRegistry|Container Registry"
 msgstr ""
 
-msgid "ContainerRegistry|How to use the Container Registry"
+msgid "ContainerRegistry|Docker connection error"
 msgstr ""
 
 msgid "ContainerRegistry|Last Updated"
 msgstr ""
 
-msgid "ContainerRegistry|Learn more about"
-msgstr ""
-
 msgid "ContainerRegistry|No tags in Container Registry for this container image."
 msgstr ""
 
-msgid "ContainerRegistry|Once you log in, you&rsquo;re free to create and upload a container image using the common %{build} and %{push} commands"
+msgid "ContainerRegistry|Quick Start"
 msgstr ""
 
 msgid "ContainerRegistry|Remove image"
@@ -2982,10 +2973,16 @@ msgstr ""
 msgid "ContainerRegistry|Tag ID"
 msgstr ""
 
-msgid "ContainerRegistry|Use different image names"
+msgid "ContainerRegistry|There are no container images stored for this project"
+msgstr ""
+
+msgid "ContainerRegistry|We are having trouble connecting to Docker, which could be due to an issue with your project name or path. For more information, please review the %{docLinkStart}Container Registry documentation%{docLinkEnd}."
 msgstr ""
 
-msgid "ContainerRegistry|With the Docker Container Registry integrated into GitLab, every project can have its own space to store its Docker images."
+msgid "ContainerRegistry|With the Container Registry, every project can have its own space to store its Docker images. Learn more about the %{docLinkStart}Container Registry%{docLinkEnd}."
+msgstr ""
+
+msgid "ContainerRegistry|With the Docker Container Registry integrated into GitLab, every project can have its own space to store its Docker images. Learn more about the %{docLinkStart}Container Registry%{docLinkEnd}."
 msgstr ""
 
 msgid "ContainerRegistry|You are about to delete the image <b>%{title}</b>. This will delete the image and all tags pointing to this image."
@@ -2994,7 +2991,7 @@ msgstr ""
 msgid "ContainerRegistry|You are about to remove repository <b>%{title}</b>. Once you confirm, this repository will be permanently deleted."
 msgstr ""
 
-msgid "ContainerRegistry|You can also use a %{deploy_token} for read-only access to the registry images."
+msgid "ContainerRegistry|You can add an image to this registry with the following commands:"
 msgstr ""
 
 msgid "Contents of .gitlab-ci.yml"
@@ -6750,9 +6747,6 @@ msgstr ""
 msgid "No connection could be made to a Gitaly Server, please check your logs!"
 msgstr ""
 
-msgid "No container images stored for this project. Add one by following the instructions above."
-msgstr ""
-
 msgid "No contributions"
 msgstr ""
 
@@ -12450,9 +12444,6 @@ msgstr[1] ""
 msgid "deleted"
 msgstr ""
 
-msgid "deploy token"
-msgstr ""
-
 msgid "detached"
 msgstr ""
 
@@ -12851,9 +12842,6 @@ msgstr[1] ""
 msgid "password"
 msgstr ""
 
-msgid "personal access token"
-msgstr ""
-
 msgid "private"
 msgstr ""
 
diff --git a/spec/features/container_registry_spec.rb b/spec/features/container_registry_spec.rb
index 21d97aba0c5..1b5943bd5d8 100644
--- a/spec/features/container_registry_spec.rb
+++ b/spec/features/container_registry_spec.rb
@@ -19,7 +19,7 @@ describe "Container Registry", :js do
     it 'user visits container registry main page' do
       visit_container_registry
 
-      expect(page).to have_content 'No container images'
+      expect(page).to have_content 'no container images'
     end
   end
 
diff --git a/spec/javascripts/registry/components/app_spec.js b/spec/javascripts/registry/components/app_spec.js
index 76a17e6fb31..87237d2853d 100644
--- a/spec/javascripts/registry/components/app_spec.js
+++ b/spec/javascripts/registry/components/app_spec.js
@@ -8,6 +8,13 @@ import { reposServerResponse } from '../mock_data';
 
 describe('Registry List', () => {
   const Component = Vue.extend(registry);
+  const props = {
+    endpoint: `${TEST_HOST}/foo`,
+    helpPagePath: 'foo',
+    noContainersImage: 'foo',
+    containersErrorImage: 'foo',
+    repositoryUrl: 'foo',
+  };
   let vm;
   let mock;
 
@@ -24,7 +31,7 @@ describe('Registry List', () => {
     beforeEach(() => {
       mock.onGet(`${TEST_HOST}/foo`).replyOnce(200, reposServerResponse);
 
-      vm = mountComponent(Component, { endpoint: `${TEST_HOST}/foo` });
+      vm = mountComponent(Component, { ...props });
     });
 
     it('should render a list of repos', done => {
@@ -72,7 +79,7 @@ describe('Registry List', () => {
     beforeEach(() => {
       mock.onGet(`${TEST_HOST}/foo`).replyOnce(200, []);
 
-      vm = mountComponent(Component, { endpoint: `${TEST_HOST}/foo` });
+      vm = mountComponent(Component, { ...props });
     });
 
     it('should render empty message', done => {
@@ -83,7 +90,7 @@ describe('Registry List', () => {
             .textContent.trim()
             .replace(/[\r\n]+/g, ' '),
         ).toEqual(
-          'No container images stored for this project. Add one by following the instructions above.',
+          'With the Container Registry, every project can have its own space to store its Docker images. Learn more about the Container Registry.',
         );
         done();
       }, 0);
@@ -94,7 +101,7 @@ describe('Registry List', () => {
     beforeEach(() => {
       mock.onGet(`${TEST_HOST}/foo`).replyOnce(200, []);
 
-      vm = mountComponent(Component, { endpoint: `${TEST_HOST}/foo` });
+      vm = mountComponent(Component, { ...props });
     });
 
     it('should render a loading spinner', done => {
@@ -104,4 +111,22 @@ describe('Registry List', () => {
       });
     });
   });
+
+  describe('invalid characters in path', () => {
+    beforeEach(() => {
+      mock.onGet(`${TEST_HOST}/foo`).replyOnce(200, []);
+
+      vm = mountComponent(Component, {
+        ...props,
+        characterError: true,
+      });
+    });
+
+    it('should render invalid characters error message', done => {
+      setTimeout(() => {
+        expect(vm.$el.querySelector('.container-message')).not.toBe(null);
+        done();
+      });
+    });
+  });
 });