Fixes broken and missing tests

app/assets/stylesheets/pages/container_registry.scss

@@ -3,14 +3,14 @@
  */
 .container-image {
-  border-bottom: 1px solid #f0f0f0;
+  border-bottom: 1px solid $white-normal;
 }
 .container-image-head {
-  padding: 0px 16px;
+  padding: 0 16px;
   line-height: 4;
 }
 .table.tags {
-  margin-bottom: 0px;
+  margin-bottom: 0;
 }

app/controllers/projects/container_registry_controller.rb

@@ -20,7 +20,6 @@ class Projects::ContainerRegistryController < Projects::ApplicationController
         redirect_to url, alert: 'Failed to remove image'
       end
     end
-
   end
   private

app/models/container_image.rb

@@ -22,7 +22,7 @@ class ContainerImage < ActiveRecord::Base
   end
   def name_with_namespace
-    [container_registry_path_with_namespace, name].compact.join('/')
+    [container_registry_path_with_namespace, name].reject(&:blank?).join('/')
   end
   def tag(tag)
@@ -55,6 +55,8 @@ class ContainerImage < ActiveRecord::Base
     end
   end
+  # rubocop:disable RedundantReturn
+
   def self.split_namespace(full_path)
     image_name = full_path.split('/').last
     namespace = full_path.gsub(/(.*)(#{Regexp.escape('/' + image_name)})/, '\1')

app/models/namespace.rb

@@ -118,8 +118,8 @@ class Namespace < ActiveRecord::Base
   end
   def move_dir
-    if any_project_has_container_registry_tags?
-      raise Gitlab::UpdatePathError.new('Namespace cannot be moved, because at least one project has tags in container registry')
+    if any_project_has_container_registry_images?
+      raise Gitlab::UpdatePathError.new('Namespace cannot be moved, because at least one project has images in container registry')
     end
     # Move the namespace directory in all storages paths used by member projects
@@ -154,8 +154,8 @@ class Namespace < ActiveRecord::Base
     end
   end
-  def any_project_has_container_registry_tags?
-    projects.any?(&:has_container_registry_tags?)
+  def any_project_has_container_registry_images?
+    projects.any? { |project| project.container_images.present? }
   end
   def send_update_instructions

app/models/project.rb

@@ -421,18 +421,12 @@ class Project < ActiveRecord::Base
     end
   end
-  def container_registry_repository_url
+  def container_registry_url
     if Gitlab.config.registry.enabled
       "#{Gitlab.config.registry.host_port}/#{container_registry_path_with_namespace}"
     end
   end
-  def has_container_registry_tags?
-    return unless container_images
-
-    container_images.first.tags.any?
-  end
-
   def commit(ref = 'HEAD')
     repository.commit(ref)
   end
@@ -913,11 +907,11 @@ class Project < ActiveRecord::Base
     expire_caches_before_rename(old_path_with_namespace)
-    if has_container_registry_tags?
-      Rails.logger.error "Project #{old_path_with_namespace} cannot be renamed because container registry tags are present"
-      # we currently doesn't support renaming repository if it contains tags in container registry
-      raise StandardError.new('Project cannot be renamed, because tags are present in its container registry')
+    if container_images.present?
+      Rails.logger.error "Project #{old_path_with_namespace} cannot be renamed because container registry images are present"
+      # we currently doesn't support renaming repository if it contains images in container registry
+      raise StandardError.new('Project cannot be renamed, because images are present in its container registry')
     end
     if gitlab_shell.mv_repository(repository_storage_path, old_path_with_namespace, new_path_with_namespace)
@@ -1264,7 +1258,7 @@ class Project < ActiveRecord::Base
     ]
     if container_registry_enabled?
-      variables << { key: 'CI_REGISTRY_IMAGE', value: container_registry_repository_url, public: true }
+      variables << { key: 'CI_REGISTRY_IMAGE', value: container_registry_url, public: true }
     end
     variables

app/services/auth/container_registry_authentication_service.rb

@@ -16,7 +16,8 @@ module Auth
       { token: authorized_token(scope).encoded }
     end
-    def self.full_access_token(names)
+    def self.full_access_token(*names)
+      names = names.flatten
       registry = Gitlab.config.registry
       token = JSONWebToken::RSAToken.new(registry.key)
       token.issuer = registry.issuer

app/services/container_images/destroy_service.rb

 module ContainerImages
   class DestroyService < BaseService
-
     class DestroyError < StandardError; end
     def execute(container_image)

app/services/projects/transfer_service.rb

@@ -36,9 +36,9 @@ module Projects
           raise TransferError.new("Project with same path in target namespace already exists")
         end
-        if project.has_container_registry_tags?
-          # we currently doesn't support renaming repository if it contains tags in container registry
-          raise TransferError.new('Project cannot be transferred, because tags are present in its container registry')
+        unless project.container_images.empty?
+          # we currently doesn't support renaming repository if it contains images in container registry
+          raise TransferError.new('Project cannot be transferred, because images are present in its container registry')
         end
         project.expire_caches_before_rename(old_path)

app/views/projects/container_registry/_image.html.haml

@@ -10,7 +10,7 @@
       = escape_once(image.name)
       = clipboard_button(clipboard_text: "docker pull #{image.path}")
     .controls.hidden-xs.pull-right
-      = link_to namespace_project_container_registry_path(@project.namespace, @project, image.id), class: 'btn btn-remove has-tooltip', title: "Remove", data: { confirm: "Are you sure?" }, method: :delete do
+      = link_to namespace_project_container_registry_path(@project.namespace, @project, image.id), class: 'btn btn-remove has-tooltip', title: "Remove image", data: { confirm: "Are you sure?" }, method: :delete do
         = icon("trash cred")

app/views/projects/container_registry/_tag.html.haml

@@ -25,5 +25,5 @@
   - if can?(current_user, :update_container_image, @project)
     %td.content
       .controls.hidden-xs.pull-right
-        = link_to namespace_project_container_registry_path(@project.namespace, @project, { id: tag.repository.id, tag: tag.name} ), class: 'btn btn-remove has-tooltip', title: "Remove", data: { confirm: "Are you sure?" }, method: :delete do
+        = link_to namespace_project_container_registry_path(@project.namespace, @project, { id: tag.repository.id, tag: tag.name} ), class: 'btn btn-remove has-tooltip', title: "Remove tag", data: { confirm: "Are you sure?" }, method: :delete do
           = icon("trash cred")

app/views/projects/container_registry/index.html.haml

@@ -15,9 +15,9 @@
       %br
       Then you are free to create and upload a container image with build and push commands:
       %pre
-        docker build -t #{escape_once(@project.container_registry_repository_url)} .
+        docker build -t #{escape_once(@project.container_registry_url)} .
         %br
-        docker push #{escape_once(@project.container_registry_repository_url)}
+        docker push #{escape_once(@project.container_registry_url)}
   - if @images.blank?
     .nothing-here-block No container images in Container Registry for this project.

db/schema.rb

@@ -109,6 +109,7 @@ ActiveRecord::Schema.define(version: 20170215200045) do
     t.boolean "html_emails_enabled", default: true
     t.string "plantuml_url"
     t.boolean "plantuml_enabled"
+    t.string "container_registry_access_token"
     t.integer "max_pages_size", default: 100, null: false
     t.integer "terminal_max_session_time", default: 0, null: false
   end
@@ -392,6 +393,11 @@ ActiveRecord::Schema.define(version: 20170215200045) do
   add_index "ci_variables", ["gl_project_id"], name: "index_ci_variables_on_gl_project_id", using: :btree
+  create_table "container_images", force: :cascade do |t|
+    t.integer "project_id"
+    t.string "name"
+  end
+
   create_table "deploy_keys_projects", force: :cascade do |t|
     t.integer "deploy_key_id", null: false
     t.integer "project_id", null: false

lib/api/registry_events.rb

@@ -46,9 +46,7 @@ module API
             if project
               container_image = project.container_images.find_or_create_by(name: container_image_name)
-              if container_image.valid?
-                puts('Valid!')
-              else
+              unless container_image.valid?
                 render_api_error!({ error: "Failed to create container image!" }, 400)
               end
             else

lib/container_registry/blob.rb

@@ -38,11 +38,11 @@ module ContainerRegistry
     end
     def delete
-      client.delete_blob(repository.name, digest)
+      client.delete_blob(repository.name_with_namespace, digest)
     end
     def data
-      @data ||= client.blob(repository.name, digest, type)
+      @data ||= client.blob(repository.name_with_namespace, digest, type)
     end
   end
 end

lib/container_registry/registry.rb

@@ -8,10 +8,6 @@ module ContainerRegistry
       @client = ContainerRegistry::Client.new(uri, options)
     end
-    def repository(name)
-      ContainerRegistry::Repository.new(self, name)
-    end
-
     private
     def default_path

lib/container_registry/repository.rb

-module ContainerRegistry
-  class Repository
-    attr_reader :registry, :name
-
-    delegate :client, to: :registry
-
-    def initialize(registry, name)
-      @registry, @name = registry, name
-    end
-
-    def path
-      [registry.path, name].compact.join('/')
-    end
-
-    def tag(tag)
-      ContainerRegistry::Tag.new(self, tag)
-    end
-
-    def manifest
-      return @manifest if defined?(@manifest)
-
-      @manifest = client.repository_tags(name)
-    end
-
-    def valid?
-      manifest.present?
-    end
-
-    def tags
-      return @tags if defined?(@tags)
-      return [] unless manifest && manifest['tags']
-
-      @tags = manifest['tags'].map do |tag|
-        ContainerRegistry::Tag.new(self, tag)
-      end
-    end
-
-    def blob(config)
-      ContainerRegistry::Blob.new(self, config)
-    end
-
-    def delete_tags
-      return unless tags
-
-      tags.all?(&:delete)
-    end
-  end
-end

spec/factories/container_images.rb

+FactoryGirl.define do
+  factory :container_image do
+    name "test_container_image"
+    project
+
+    transient do
+      tags ['tag']
+      stubbed true
+    end
+
+    after(:build) do |image, evaluator|
+      if evaluator.stubbed
+        allow(Gitlab.config.registry).to receive(:enabled).and_return(true)
+        allow(image.client).to receive(:repository_tags).and_return({
+          name: image.name_with_namespace,
+          tags: evaluator.tags
+        })
+      end
+    end
+  end
+end

spec/features/container_registry_spec.rb

@@ -2,15 +2,18 @@ require 'spec_helper'
 describe "Container Registry" do
   let(:project) { create(:empty_project) }
-  let(:repository) { project.container_registry_repository }
+  let(:registry) { project.container_registry }
   let(:tag_name) { 'latest' }
   let(:tags) { [tag_name] }
+  let(:container_image) { create(:container_image) }
+  let(:image_name) { container_image.name }
   before do
     login_as(:user)
     project.team << [@user, :developer]
-    stub_container_registry_tags(*tags)
     stub_container_registry_config(enabled: true)
+    stub_container_registry_tags(*tags)
+    project.container_images << container_image unless container_image.nil?
     allow(Auth::ContainerRegistryAuthenticationService).to receive(:full_access_token).and_return('token')
   end
@@ -19,15 +22,26 @@ describe "Container Registry" do
       visit namespace_project_container_registry_index_path(project.namespace, project)
     end
-    context 'when no tags' do
-      let(:tags) { [] }
-      it { expect(page).to have_content('No images in Container Registry for this project') }
+    context 'when no images' do
+      let(:container_image) { }
+
+      it { expect(page).to have_content('No container images in Container Registry for this project') }
+    end
+    context 'when there are images' do
+      it { expect(page).to have_content(image_name) }
     end
-    context 'when there are tags' do
-      it { expect(page).to have_content(tag_name) }
-      it { expect(page).to have_content('d7a513a66') }
+  end
+
+  describe 'DELETE /:project/container_registry/:image_id' do
+    before do
+      visit namespace_project_container_registry_index_path(project.namespace, project)
+    end
+
+    it do
+      expect_any_instance_of(ContainerImage).to receive(:delete_tags).and_return(true)
+      click_on 'Remove image'
     end
   end
@@ -39,7 +53,7 @@ describe "Container Registry" do
     it do
       expect_any_instance_of(::ContainerRegistry::Tag).to receive(:delete).and_return(true)
-      click_on 'Remove'
+      click_on 'Remove tag'
     end
   end
 end

spec/features/security/project/internal_access_spec.rb

@@ -429,9 +429,12 @@ describe "Internal Project Access", feature: true  do
   end
   describe "GET /:project_path/container_registry" do
+    let(:container_image) { create(:container_image) }
+
     before do
       stub_container_registry_tags('latest')
       stub_container_registry_config(enabled: true)
+      project.container_images << container_image
     end
     subject { namespace_project_container_registry_index_path(project.namespace, project) }

spec/features/security/project/private_access_spec.rb

@@ -418,9 +418,12 @@ describe "Private Project Access", feature: true  do
   end
   describe "GET /:project_path/container_registry" do
+    let(:container_image) { create(:container_image) }
+
     before do
       stub_container_registry_tags('latest')
       stub_container_registry_config(enabled: true)
+      project.container_images << container_image
     end
     subject { namespace_project_container_registry_index_path(project.namespace, project) }