From 7d3d1ec5a7b3ee13397af587c6ecb3a3fc297006 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 30 Mar 2017 15:24:46 +0200
Subject: [PATCH] Create container repository on successful push auth

Because we do not have yet two way communication between container
registry and GitLab, we need to eagerly create a new container
repository objects in database. We now do that after user/build
successfully authenticates a push action using auth service.
---
 app/models/container_repository.rb            |  3 +-
 ...ntainer_registry_authentication_service.rb | 16 ++++++
 lib/container_registry/path.rb                |  4 ++
 ...er_registry_authentication_service_spec.rb | 52 +++++++++++++++++--
 4 files changed, 70 insertions(+), 5 deletions(-)

diff --git a/app/models/container_repository.rb b/app/models/container_repository.rb
index 33e2574d389..5663b3db92f 100644
--- a/app/models/container_repository.rb
+++ b/app/models/container_repository.rb
@@ -60,6 +60,7 @@ class ContainerRepository < ActiveRecord::Base
   end
 
   def self.create_from_path(path)
-    self.create(project: path.repository_project, name: path.repository_name)
+    self.create(project: path.repository_project,
+                name: path.repository_name)
   end
 end
diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb
index b050f1dd51b..839f514ad58 100644
--- a/app/services/auth/container_registry_authentication_service.rb
+++ b/app/services/auth/container_registry_authentication_service.rb
@@ -74,9 +74,25 @@ module Auth
 
       return unless actions.present?
 
+      # At this point user/build is already authenticated.
+      #
+      ensure_container_repository!(path, actions)
+
       { type: type, name: path.to_s, actions: actions }
     end
 
+    ##
+    # Because we do not have two way communication with registry yet,
+    # we create a container repository image resource when push to the
+    # registry is successfuly authorized.
+    #
+    def ensure_container_repository!(path, actions)
+      return if path.has_repository?
+      return unless actions.include?('push')
+
+      ContainerRepository.create_from_path(path)
+    end
+
     def can_access?(requested_project, requested_action)
       return false unless requested_project.container_registry_enabled?
 
diff --git a/lib/container_registry/path.rb b/lib/container_registry/path.rb
index 3a6fde08e8f..27e0e7897ff 100644
--- a/lib/container_registry/path.rb
+++ b/lib/container_registry/path.rb
@@ -44,6 +44,10 @@ module ContainerRegistry
         .where(name: repository_name).any?
     end
 
+    def root_repository?
+      @path == repository_project.full_path
+    end
+
     def repository_project
       @project ||= Project.where_full_path_in(components.first(3))&.first
     end
diff --git a/spec/services/auth/container_registry_authentication_service_spec.rb b/spec/services/auth/container_registry_authentication_service_spec.rb
index fac7f1b1235..a4a6430011e 100644
--- a/spec/services/auth/container_registry_authentication_service_spec.rb
+++ b/spec/services/auth/container_registry_authentication_service_spec.rb
@@ -80,6 +80,19 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
     it { is_expected.not_to include(:token) }
   end
 
+  shared_examples 'container repository factory' do
+    it 'creates a new containe repository resource' do
+      expect { subject }
+        .to change { project.container_repositories.count }.by(1)
+    end
+  end
+
+  shared_examples 'container repository factory' do
+    it 'does not create a new container repository resource' do
+      expect { subject }.not_to change { ContainerRepository.count }
+    end
+  end
+
   describe '#full_access_token' do
     let(:project) { create(:empty_project) }
     let(:token) { described_class.full_access_token(project.path_with_namespace) }
@@ -89,6 +102,8 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
     it_behaves_like 'an accessible' do
       let(:actions) { ['*'] }
     end
+
+    it_behaves_like 'not a container repository factory'
   end
 
   context 'user authorization' do
@@ -109,16 +124,20 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
         end
 
         it_behaves_like 'a pushable'
+        it_behaves_like 'container repository factory'
       end
 
       context 'allow reporter to pull images' do
         before { project.team << [current_user, :reporter] }
 
-        let(:current_params) do
-          { scope: "repository:#{project.path_with_namespace}:pull" }
-        end
+        context 'when pulling from root level repository' do
+          let(:current_params) do
+            { scope: "repository:#{project.path_with_namespace}:pull" }
+          end
 
-        it_behaves_like 'a pullable'
+          it_behaves_like 'a pullable'
+          it_behaves_like 'not a container repository factory'
+        end
       end
 
       context 'return a least of privileges' do
@@ -129,6 +148,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
         end
 
         it_behaves_like 'a pullable'
+        it_behaves_like 'not a container repository factory'
       end
 
       context 'disallow guest to pull or push images' do
@@ -139,6 +159,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
         end
 
         it_behaves_like 'an inaccessible'
+        it_behaves_like 'not a container repository factory'
       end
     end
 
@@ -151,6 +172,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
         end
 
         it_behaves_like 'a pullable'
+        it_behaves_like 'not a container repository factory'
       end
 
       context 'disallow anyone to push images' do
@@ -159,6 +181,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
         end
 
         it_behaves_like 'an inaccessible'
+        it_behaves_like 'not a container repository factory'
       end
     end
 
@@ -172,6 +195,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
           end
 
           it_behaves_like 'a pullable'
+          it_behaves_like 'not a container repository factory'
         end
 
         context 'disallow anyone to push images' do
@@ -180,6 +204,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
           end
 
           it_behaves_like 'an inaccessible'
+          it_behaves_like 'not a container repository factory'
         end
       end
 
@@ -190,6 +215,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
         end
 
         it_behaves_like 'an inaccessible'
+        it_behaves_like 'not a container repository factory'
       end
     end
   end
@@ -216,6 +242,10 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
       it_behaves_like 'a pullable and pushable' do
         let(:project) { current_project }
       end
+
+      it_behaves_like 'container repository factory' do
+        let(:project) { current_project }
+      end
     end
 
     context 'for other projects' do
@@ -228,11 +258,13 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
           let(:project) { create(:empty_project, :public) }
 
           it_behaves_like 'a pullable'
+          it_behaves_like 'not a container repository factory'
         end
 
         shared_examples 'pullable for being team member' do
           context 'when you are not member' do
             it_behaves_like 'an inaccessible'
+            it_behaves_like 'not a container repository factory'
           end
 
           context 'when you are member' do
@@ -241,12 +273,14 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
             end
 
             it_behaves_like 'a pullable'
+            it_behaves_like 'not a container repository factory'
           end
 
           context 'when you are owner' do
             let(:project) { create(:empty_project, namespace: current_user.namespace) }
 
             it_behaves_like 'a pullable'
+            it_behaves_like 'not a container repository factory'
           end
         end
 
@@ -260,6 +294,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
 
             context 'when you are not member' do
               it_behaves_like 'an inaccessible'
+              it_behaves_like 'not a container repository factory'
             end
 
             context 'when you are member' do
@@ -268,12 +303,14 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
               end
 
               it_behaves_like 'a pullable'
+              it_behaves_like 'not a container repository factory'
             end
 
             context 'when you are owner' do
               let(:project) { create(:empty_project, namespace: current_user.namespace) }
 
               it_behaves_like 'a pullable'
+              it_behaves_like 'not a container repository factory'
             end
           end
         end
@@ -293,12 +330,14 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
             end
 
             it_behaves_like 'an inaccessible'
+            it_behaves_like 'not a container repository factory'
           end
 
           context 'when you are owner' do
             let(:project) { create(:empty_project, :public, namespace: current_user.namespace) }
 
             it_behaves_like 'an inaccessible'
+            it_behaves_like 'not a container repository factory'
           end
         end
       end
@@ -315,6 +354,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
         end
 
         it_behaves_like 'an inaccessible'
+        it_behaves_like 'not a container repository factory'
       end
     end
   end
@@ -322,6 +362,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
   context 'unauthorized' do
     context 'disallow to use scope-less authentication' do
       it_behaves_like 'a forbidden'
+      it_behaves_like 'not a container repository factory'
     end
 
     context 'for invalid scope' do
@@ -330,6 +371,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
       end
 
       it_behaves_like 'a forbidden'
+      it_behaves_like 'not a container repository factory'
     end
 
     context 'for private project' do
@@ -351,6 +393,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
         end
 
         it_behaves_like 'a pullable'
+        it_behaves_like 'not a container repository factory'
       end
 
       context 'when pushing' do
@@ -359,6 +402,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
         end
 
         it_behaves_like 'a forbidden'
+        it_behaves_like 'not a container repository factory'
       end
     end
   end