From e43b2e81dab3cade773d479f2ae56478e3113207 Mon Sep 17 00:00:00 2001 From: Andre Guedes Date: Thu, 27 Oct 2016 17:39:32 -0200 Subject: [PATCH 001/278] Added MR Road map --- lib/container_registry/ROADMAP.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 lib/container_registry/ROADMAP.md diff --git a/lib/container_registry/ROADMAP.md b/lib/container_registry/ROADMAP.md new file mode 100644 index 00000000000..e0a20776404 --- /dev/null +++ b/lib/container_registry/ROADMAP.md @@ -0,0 +1,7 @@ +## Road map + +### Initial thoughts + +- Determine if image names will be persisted or fetched from API +- If persisted, how to update the stored names upon modification +- If fetched, how to fetch only images of a given project From dcd4beb8eb7bb7d0c2f720ef85c3da9f97a3dfe6 Mon Sep 17 00:00:00 2001 From: Andre Guedes Date: Wed, 2 Nov 2016 00:33:35 -0200 Subject: [PATCH 002/278] Multi-level container image names backend implementation - Adds Registry events API endpoint - Adds container_images_repository and container_images models - Changes JWT authentication to allow multi-level scopes - Adds services for container image maintenance --- app/models/container_image.rb | 58 +++++++++++++++++++ app/models/container_images_repository.rb | 26 +++++++++ app/models/project.rb | 1 + ...ntainer_registry_authentication_service.rb | 9 ++- .../container_images/create_service.rb | 16 +++++ .../container_images/destroy_service.rb | 11 ++++ .../container_images/push_service.rb | 26 +++++++++ .../create_service.rb | 7 +++ ...3736_create_container_images_repository.rb | 31 ++++++++++ .../20161031013926_create_container_image.rb | 32 ++++++++++ lib/api/api.rb | 1 + lib/api/registry_events.rb | 52 +++++++++++++++++ lib/container_registry/client.rb | 4 ++ lib/container_registry/tag.rb | 8 +-- 14 files changed, 275 insertions(+), 7 deletions(-) create mode 100644 app/models/container_image.rb create mode 100644 app/models/container_images_repository.rb create mode 100644 app/services/container_images_repositories/container_images/create_service.rb create mode 100644 app/services/container_images_repositories/container_images/destroy_service.rb create mode 100644 app/services/container_images_repositories/container_images/push_service.rb create mode 100644 app/services/container_images_repositories/create_service.rb create mode 100644 db/migrate/20161029153736_create_container_images_repository.rb create mode 100644 db/migrate/20161031013926_create_container_image.rb create mode 100644 lib/api/registry_events.rb diff --git a/app/models/container_image.rb b/app/models/container_image.rb new file mode 100644 index 00000000000..dcc4a7af629 --- /dev/null +++ b/app/models/container_image.rb @@ -0,0 +1,58 @@ +class ContainerImage < ActiveRecord::Base + belongs_to :container_images_repository + + delegate :registry, :registry_path_with_namespace, :client, to: :container_images_repository + + validates :manifest, presence: true + + before_validation :update_token, on: :create + def update_token + paths = container_images_repository.allowed_paths << name_with_namespace + token = Auth::ContainerRegistryAuthenticationService.full_access_token(paths) + client.update_token(token) + end + + def path + [registry.path, name_with_namespace].compact.join('/') + end + + def name_with_namespace + [registry_path_with_namespace, name].compact.join('/') + end + + def tag(tag) + ContainerRegistry::Tag.new(self, tag) + end + + def manifest + @manifest ||= client.repository_tags(name_with_namespace) + 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 + + def self.split_namespace(full_path) + image_name = full_path.split('/').last + namespace = full_path.gsub(/(.*)(#{Regexp.escape('/' + image_name)})/, '\1') + if namespace.count('/') < 1 + namespace, image_name = full_path, "" + end + return namespace, image_name + end +end diff --git a/app/models/container_images_repository.rb b/app/models/container_images_repository.rb new file mode 100644 index 00000000000..99e94d2a6d0 --- /dev/null +++ b/app/models/container_images_repository.rb @@ -0,0 +1,26 @@ +class ContainerImagesRepository < ActiveRecord::Base + + belongs_to :project + + has_many :container_images, dependent: :destroy + + delegate :client, to: :registry + + def registry_path_with_namespace + project.path_with_namespace.downcase + end + + def allowed_paths + @allowed_paths ||= [registry_path_with_namespace] + + container_images.map { |i| i.name_with_namespace } + end + + def registry + @registry ||= begin + token = Auth::ContainerRegistryAuthenticationService.full_access_token(allowed_paths) + url = Gitlab.config.registry.api_url + host_port = Gitlab.config.registry.host_port + ContainerRegistry::Registry.new(url, token: token, path: host_port) + end + end +end diff --git a/app/models/project.rb b/app/models/project.rb index 411299eef63..703e24eb79a 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -157,6 +157,7 @@ class Project < ActiveRecord::Base has_one :import_data, dependent: :destroy, class_name: "ProjectImportData" has_one :project_feature, dependent: :destroy has_one :statistics, class_name: 'ProjectStatistics', dependent: :delete + has_one :container_images_repository, dependent: :destroy has_many :commit_statuses, dependent: :destroy, foreign_key: :gl_project_id has_many :pipelines, dependent: :destroy, class_name: 'Ci::Pipeline', foreign_key: :gl_project_id diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb index 5cb7a86a5ee..6b83b38fa4d 100644 --- a/app/services/auth/container_registry_authentication_service.rb +++ b/app/services/auth/container_registry_authentication_service.rb @@ -16,7 +16,7 @@ module Auth { token: authorized_token(scope).encoded } end - def self.full_access_token(*names) + def self.full_access_token(names) registry = Gitlab.config.registry token = JSONWebToken::RSAToken.new(registry.key) token.issuer = registry.issuer @@ -61,7 +61,12 @@ module Auth end def process_repository_access(type, name, actions) - requested_project = Project.find_by_full_path(name) + # Strips image name due to lack of + # per image authentication. + # Removes only last occurence in light + # of future nested groups + namespace, _ = ContainerImage::split_namespace(name) + requested_project = Project.find_by_full_path(namespace) return unless requested_project actions = actions.select do |action| diff --git a/app/services/container_images_repositories/container_images/create_service.rb b/app/services/container_images_repositories/container_images/create_service.rb new file mode 100644 index 00000000000..0c2c69d5183 --- /dev/null +++ b/app/services/container_images_repositories/container_images/create_service.rb @@ -0,0 +1,16 @@ +module ContainerImagesRepositories + module ContainerImages + class CreateService < BaseService + def execute + @container_image = container_images_repository.container_images.create(params) + @container_image if @container_image.valid? + end + + private + + def container_images_repository + @container_images_repository ||= project.container_images_repository + end + end + end +end diff --git a/app/services/container_images_repositories/container_images/destroy_service.rb b/app/services/container_images_repositories/container_images/destroy_service.rb new file mode 100644 index 00000000000..91b8cfeea47 --- /dev/null +++ b/app/services/container_images_repositories/container_images/destroy_service.rb @@ -0,0 +1,11 @@ +module ContainerImagesRepositories + module ContainerImages + class DestroyService < BaseService + def execute(container_image) + return false unless container_image + + container_image.destroy + end + end + end +end diff --git a/app/services/container_images_repositories/container_images/push_service.rb b/app/services/container_images_repositories/container_images/push_service.rb new file mode 100644 index 00000000000..2731cf1d52e --- /dev/null +++ b/app/services/container_images_repositories/container_images/push_service.rb @@ -0,0 +1,26 @@ +module ContainerImagesRepositories + module ContainerImages + class PushService < BaseService + def execute(container_image_name, event) + find_or_create_container_image(container_image_name).valid? + end + + private + + def find_or_create_container_image(container_image_name) + options = {name: container_image_name} + container_images.find_by(options) || + ::ContainerImagesRepositories::ContainerImages::CreateService.new(project, + current_user, options).execute + end + + def container_images_repository + @container_images_repository ||= project.container_images_repository + end + + def container_images + @container_images ||= container_images_repository.container_images + end + end + end +end diff --git a/app/services/container_images_repositories/create_service.rb b/app/services/container_images_repositories/create_service.rb new file mode 100644 index 00000000000..7e9dd3abe5f --- /dev/null +++ b/app/services/container_images_repositories/create_service.rb @@ -0,0 +1,7 @@ +module ContainerImagesRepositories + class CreateService < BaseService + def execute + project.container_images_repository || ::ContainerImagesRepository.create(project: project) + end + end +end diff --git a/db/migrate/20161029153736_create_container_images_repository.rb b/db/migrate/20161029153736_create_container_images_repository.rb new file mode 100644 index 00000000000..d93180b1674 --- /dev/null +++ b/db/migrate/20161029153736_create_container_images_repository.rb @@ -0,0 +1,31 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class CreateContainerImagesRepository < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + # When a migration requires downtime you **must** uncomment the following + # constant and define a short and easy to understand explanation as to why the + # migration requires downtime. + # DOWNTIME_REASON = '' + + # When using the methods "add_concurrent_index" or "add_column_with_default" + # you must disable the use of transactions as these methods can not run in an + # existing transaction. When using "add_concurrent_index" make sure that this + # method is the _only_ method called in the migration, any other changes + # should go in a separate migration. This ensures that upon failure _only_ the + # index creation fails and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + # disable_ddl_transaction! + + def change + create_table :container_images_repositories do |t| + t.integer :project_id, null: false + end + end +end diff --git a/db/migrate/20161031013926_create_container_image.rb b/db/migrate/20161031013926_create_container_image.rb new file mode 100644 index 00000000000..94feae280a6 --- /dev/null +++ b/db/migrate/20161031013926_create_container_image.rb @@ -0,0 +1,32 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class CreateContainerImage < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + # When a migration requires downtime you **must** uncomment the following + # constant and define a short and easy to understand explanation as to why the + # migration requires downtime. + # DOWNTIME_REASON = '' + + # When using the methods "add_concurrent_index" or "add_column_with_default" + # you must disable the use of transactions as these methods can not run in an + # existing transaction. When using "add_concurrent_index" make sure that this + # method is the _only_ method called in the migration, any other changes + # should go in a separate migration. This ensures that upon failure _only_ the + # index creation fails and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + # disable_ddl_transaction! + + def change + create_table :container_images do |t| + t.integer :container_images_repository_id + t.string :name + end + end +end diff --git a/lib/api/api.rb b/lib/api/api.rb index a0282ff8deb..ed775f898d2 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -84,6 +84,7 @@ module API mount ::API::Namespaces mount ::API::Notes mount ::API::NotificationSettings + mount ::API::RegistryEvents mount ::API::Pipelines mount ::API::ProjectHooks mount ::API::Projects diff --git a/lib/api/registry_events.rb b/lib/api/registry_events.rb new file mode 100644 index 00000000000..c0473051424 --- /dev/null +++ b/lib/api/registry_events.rb @@ -0,0 +1,52 @@ +module API + # RegistryEvents API + class RegistryEvents < Grape::API + # before { authenticate! } + + content_type :json, 'application/vnd.docker.distribution.events.v1+json' + + params do + requires :events, type: Array, desc: 'The ID of a project' do + requires :id, type: String, desc: 'The ID of the event' + requires :timestamp, type: String, desc: 'Timestamp of the event' + requires :action, type: String, desc: 'Action performed by event' + requires :target, type: Hash, desc: 'Target of the event' do + optional :mediaType, type: String, desc: 'Media type of the target' + optional :size, type: Integer, desc: 'Size in bytes of the target' + requires :digest, type: String, desc: 'Digest of the target' + requires :repository, type: String, desc: 'Repository of target' + optional :url, type: String, desc: 'Url of the target' + optional :tag, type: String, desc: 'Tag of the target' + end + requires :request, type: Hash, desc: 'Request of the event' do + requires :id, type: String, desc: 'The ID of the request' + optional :addr, type: String, desc: 'IP Address of the request client' + optional :host, type: String, desc: 'Hostname of the registry instance' + requires :method, type: String, desc: 'Request method' + requires :useragent, type: String, desc: 'UserAgent header of the request' + end + requires :actor, type: Hash, desc: 'Actor that initiated the event' do + optional :name, type: String, desc: 'Actor name' + end + requires :source, type: Hash, desc: 'Source of the event' do + optional :addr, type: String, desc: 'Hostname of source registry node' + optional :instanceID, type: String, desc: 'Source registry node instanceID' + end + end + end + resource :registry_events do + post do + params['events'].each do |event| + repository = event['target']['repository'] + + if event['action'] == 'push' and !!event['target']['tag'] + namespace, container_image_name = ContainerImage::split_namespace(repository) + ::ContainerImagesRepositories::ContainerImages::PushService.new( + Project::find_with_namespace(namespace), current_user + ).execute(container_image_name, event) + end + end + end + end + end +end diff --git a/lib/container_registry/client.rb b/lib/container_registry/client.rb index 2edddb84fc3..2cbb7bfb67d 100644 --- a/lib/container_registry/client.rb +++ b/lib/container_registry/client.rb @@ -15,6 +15,10 @@ module ContainerRegistry @options = options end + def update_token(token) + @options[:token] = token + end + def repository_tags(name) response_body faraday.get("/v2/#{name}/tags/list") end diff --git a/lib/container_registry/tag.rb b/lib/container_registry/tag.rb index 59040199920..68dd87c979d 100644 --- a/lib/container_registry/tag.rb +++ b/lib/container_registry/tag.rb @@ -22,9 +22,7 @@ module ContainerRegistry end def manifest - return @manifest if defined?(@manifest) - - @manifest = client.repository_manifest(repository.name, name) + @manifest ||= client.repository_manifest(repository.name_with_namespace, name) end def path @@ -40,7 +38,7 @@ module ContainerRegistry def digest return @digest if defined?(@digest) - @digest = client.repository_tag_digest(repository.name, name) + @digest = client.repository_tag_digest(repository.name_with_namespace, name) end def config_blob @@ -82,7 +80,7 @@ module ContainerRegistry def delete return unless digest - client.delete_repository_tag(repository.name, digest) + client.delete_repository_tag(repository.name_with_namespace, digest) end end end From eed0b85ad084ad4d13cc26907102063d9372fe75 Mon Sep 17 00:00:00 2001 From: Andre Guedes Date: Wed, 23 Nov 2016 14:50:30 -0200 Subject: [PATCH 003/278] First iteration of container_image view - Fixes project, container_image and tag deletion - Removed container_images_repository [ci skip] --- .../stylesheets/pages/container_registry.scss | 16 +++++++++ .../projects/container_registry_controller.rb | 28 +++++++++++---- app/models/container_image.rb | 20 +++++++---- app/models/container_images_repository.rb | 26 -------------- app/models/project.rb | 20 ++++++----- .../container_images/destroy_service.rb | 32 +++++++++++++++++ .../container_images/create_service.rb | 16 --------- .../container_images/destroy_service.rb | 11 ------ .../container_images/push_service.rb | 26 -------------- .../create_service.rb | 7 ---- app/services/projects/destroy_service.rb | 10 ------ .../container_registry/_image.html.haml | 35 +++++++++++++++++++ .../container_registry/_tag.html.haml | 2 +- .../container_registry/index.html.haml | 20 +++-------- ...3736_create_container_images_repository.rb | 31 ---------------- .../20161031013926_create_container_image.rb | 2 +- lib/api/registry_events.rb | 16 +++++++-- 17 files changed, 149 insertions(+), 169 deletions(-) create mode 100644 app/assets/stylesheets/pages/container_registry.scss delete mode 100644 app/models/container_images_repository.rb create mode 100644 app/services/container_images/destroy_service.rb delete mode 100644 app/services/container_images_repositories/container_images/create_service.rb delete mode 100644 app/services/container_images_repositories/container_images/destroy_service.rb delete mode 100644 app/services/container_images_repositories/container_images/push_service.rb delete mode 100644 app/services/container_images_repositories/create_service.rb create mode 100644 app/views/projects/container_registry/_image.html.haml delete mode 100644 db/migrate/20161029153736_create_container_images_repository.rb diff --git a/app/assets/stylesheets/pages/container_registry.scss b/app/assets/stylesheets/pages/container_registry.scss new file mode 100644 index 00000000000..7d68eae3c97 --- /dev/null +++ b/app/assets/stylesheets/pages/container_registry.scss @@ -0,0 +1,16 @@ +/** + * Container Registry + */ + +.container-image { + border-bottom: 1px solid #f0f0f0; +} + +.container-image-head { + padding: 0px 16px; + line-height: 4; +} + +.table.tags { + margin-bottom: 0px; +} diff --git a/app/controllers/projects/container_registry_controller.rb b/app/controllers/projects/container_registry_controller.rb index d1f46497207..54bcb5f504a 100644 --- a/app/controllers/projects/container_registry_controller.rb +++ b/app/controllers/projects/container_registry_controller.rb @@ -5,17 +5,22 @@ class Projects::ContainerRegistryController < Projects::ApplicationController layout 'project' def index - @tags = container_registry_repository.tags + @images = project.container_images end def destroy url = namespace_project_container_registry_index_path(project.namespace, project) - if tag.delete - redirect_to url + if tag + delete_tag(url) else - redirect_to url, alert: 'Failed to remove tag' + if image.destroy + redirect_to url + else + redirect_to url, alert: 'Failed to remove image' + end end + end private @@ -24,11 +29,20 @@ class Projects::ContainerRegistryController < Projects::ApplicationController render_404 unless Gitlab.config.registry.enabled end - def container_registry_repository - @container_registry_repository ||= project.container_registry_repository + def delete_tag(url) + if tag.delete + image.destroy if image.tags.empty? + redirect_to url + else + redirect_to url, alert: 'Failed to remove tag' + end + end + + def image + @image ||= project.container_images.find_by(id: params[:id]) end def tag - @tag ||= container_registry_repository.tag(params[:id]) + @tag ||= image.tag(params[:tag]) if params[:tag].present? end end diff --git a/app/models/container_image.rb b/app/models/container_image.rb index dcc4a7af629..7721c53a6fc 100644 --- a/app/models/container_image.rb +++ b/app/models/container_image.rb @@ -1,23 +1,28 @@ class ContainerImage < ActiveRecord::Base - belongs_to :container_images_repository + belongs_to :project - delegate :registry, :registry_path_with_namespace, :client, to: :container_images_repository + delegate :container_registry, :container_registry_allowed_paths, + :container_registry_path_with_namespace, to: :project + + delegate :client, to: :container_registry validates :manifest, presence: true + before_destroy :delete_tags + before_validation :update_token, on: :create def update_token - paths = container_images_repository.allowed_paths << name_with_namespace + paths = container_registry_allowed_paths << name_with_namespace token = Auth::ContainerRegistryAuthenticationService.full_access_token(paths) client.update_token(token) end def path - [registry.path, name_with_namespace].compact.join('/') + [container_registry.path, name_with_namespace].compact.join('/') end def name_with_namespace - [registry_path_with_namespace, name].compact.join('/') + [container_registry_path_with_namespace, name].compact.join('/') end def tag(tag) @@ -44,7 +49,10 @@ class ContainerImage < ActiveRecord::Base def delete_tags return unless tags - tags.all?(&:delete) + digests = tags.map {|tag| tag.digest }.to_set + digests.all? do |digest| + client.delete_repository_tag(name_with_namespace, digest) + end end def self.split_namespace(full_path) diff --git a/app/models/container_images_repository.rb b/app/models/container_images_repository.rb deleted file mode 100644 index 99e94d2a6d0..00000000000 --- a/app/models/container_images_repository.rb +++ /dev/null @@ -1,26 +0,0 @@ -class ContainerImagesRepository < ActiveRecord::Base - - belongs_to :project - - has_many :container_images, dependent: :destroy - - delegate :client, to: :registry - - def registry_path_with_namespace - project.path_with_namespace.downcase - end - - def allowed_paths - @allowed_paths ||= [registry_path_with_namespace] + - container_images.map { |i| i.name_with_namespace } - end - - def registry - @registry ||= begin - token = Auth::ContainerRegistryAuthenticationService.full_access_token(allowed_paths) - url = Gitlab.config.registry.api_url - host_port = Gitlab.config.registry.host_port - ContainerRegistry::Registry.new(url, token: token, path: host_port) - end - end -end diff --git a/app/models/project.rb b/app/models/project.rb index 703e24eb79a..afaf2095a4c 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -157,7 +157,7 @@ class Project < ActiveRecord::Base has_one :import_data, dependent: :destroy, class_name: "ProjectImportData" has_one :project_feature, dependent: :destroy has_one :statistics, class_name: 'ProjectStatistics', dependent: :delete - has_one :container_images_repository, dependent: :destroy + has_many :container_images, dependent: :destroy has_many :commit_statuses, dependent: :destroy, foreign_key: :gl_project_id has_many :pipelines, dependent: :destroy, class_name: 'Ci::Pipeline', foreign_key: :gl_project_id @@ -405,15 +405,19 @@ class Project < ActiveRecord::Base path_with_namespace.downcase end - def container_registry_repository + def container_registry_allowed_paths + @container_registry_allowed_paths ||= [container_registry_path_with_namespace] + + container_images.map { |i| i.name_with_namespace } + end + + def container_registry return unless Gitlab.config.registry.enabled - @container_registry_repository ||= begin - token = Auth::ContainerRegistryAuthenticationService.full_access_token(container_registry_path_with_namespace) + @container_registry ||= begin + token = Auth::ContainerRegistryAuthenticationService.full_access_token(container_registry_allowed_paths) url = Gitlab.config.registry.api_url host_port = Gitlab.config.registry.host_port - registry = ContainerRegistry::Registry.new(url, token: token, path: host_port) - registry.repository(container_registry_path_with_namespace) + ContainerRegistry::Registry.new(url, token: token, path: host_port) end end @@ -424,9 +428,9 @@ class Project < ActiveRecord::Base end def has_container_registry_tags? - return unless container_registry_repository + return unless container_images - container_registry_repository.tags.any? + container_images.first.tags.any? end def commit(ref = 'HEAD') diff --git a/app/services/container_images/destroy_service.rb b/app/services/container_images/destroy_service.rb new file mode 100644 index 00000000000..bc5b53fd055 --- /dev/null +++ b/app/services/container_images/destroy_service.rb @@ -0,0 +1,32 @@ +module ContainerImages + class DestroyService < BaseService + + class DestroyError < StandardError; end + + def execute(container_image) + @container_image = container_image + + return false unless can?(current_user, :remove_project, project) + + ContainerImage.transaction do + container_image.destroy! + + unless remove_container_image_tags + raise_error('Failed to remove container image tags. Please try again or contact administrator') + end + end + + true + end + + private + + def raise_error(message) + raise DestroyError.new(message) + end + + def remove_container_image_tags + container_image.delete_tags + end + end +end diff --git a/app/services/container_images_repositories/container_images/create_service.rb b/app/services/container_images_repositories/container_images/create_service.rb deleted file mode 100644 index 0c2c69d5183..00000000000 --- a/app/services/container_images_repositories/container_images/create_service.rb +++ /dev/null @@ -1,16 +0,0 @@ -module ContainerImagesRepositories - module ContainerImages - class CreateService < BaseService - def execute - @container_image = container_images_repository.container_images.create(params) - @container_image if @container_image.valid? - end - - private - - def container_images_repository - @container_images_repository ||= project.container_images_repository - end - end - end -end diff --git a/app/services/container_images_repositories/container_images/destroy_service.rb b/app/services/container_images_repositories/container_images/destroy_service.rb deleted file mode 100644 index 91b8cfeea47..00000000000 --- a/app/services/container_images_repositories/container_images/destroy_service.rb +++ /dev/null @@ -1,11 +0,0 @@ -module ContainerImagesRepositories - module ContainerImages - class DestroyService < BaseService - def execute(container_image) - return false unless container_image - - container_image.destroy - end - end - end -end diff --git a/app/services/container_images_repositories/container_images/push_service.rb b/app/services/container_images_repositories/container_images/push_service.rb deleted file mode 100644 index 2731cf1d52e..00000000000 --- a/app/services/container_images_repositories/container_images/push_service.rb +++ /dev/null @@ -1,26 +0,0 @@ -module ContainerImagesRepositories - module ContainerImages - class PushService < BaseService - def execute(container_image_name, event) - find_or_create_container_image(container_image_name).valid? - end - - private - - def find_or_create_container_image(container_image_name) - options = {name: container_image_name} - container_images.find_by(options) || - ::ContainerImagesRepositories::ContainerImages::CreateService.new(project, - current_user, options).execute - end - - def container_images_repository - @container_images_repository ||= project.container_images_repository - end - - def container_images - @container_images ||= container_images_repository.container_images - end - end - end -end diff --git a/app/services/container_images_repositories/create_service.rb b/app/services/container_images_repositories/create_service.rb deleted file mode 100644 index 7e9dd3abe5f..00000000000 --- a/app/services/container_images_repositories/create_service.rb +++ /dev/null @@ -1,7 +0,0 @@ -module ContainerImagesRepositories - class CreateService < BaseService - def execute - project.container_images_repository || ::ContainerImagesRepository.create(project: project) - end - end -end diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb index 9716a1780a9..ba410b79e8c 100644 --- a/app/services/projects/destroy_service.rb +++ b/app/services/projects/destroy_service.rb @@ -31,10 +31,6 @@ module Projects project.team.truncate project.destroy! - unless remove_registry_tags - raise_error('Failed to remove project container registry. Please try again or contact administrator') - end - unless remove_repository(repo_path) raise_error('Failed to remove project repository. Please try again or contact administrator') end @@ -68,12 +64,6 @@ module Projects end end - def remove_registry_tags - return true unless Gitlab.config.registry.enabled - - project.container_registry_repository.delete_tags - end - def raise_error(message) raise DestroyError.new(message) end diff --git a/app/views/projects/container_registry/_image.html.haml b/app/views/projects/container_registry/_image.html.haml new file mode 100644 index 00000000000..b1d62e34a97 --- /dev/null +++ b/app/views/projects/container_registry/_image.html.haml @@ -0,0 +1,35 @@ +- expanded = false +.container-image.js-toggle-container + .container-image-head + = link_to "#", class: "js-toggle-button" do + - if expanded + = icon("chevron-up") + - else + = icon("chevron-down") + + = 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 + = icon("trash cred") + + + .container-image-tags.js-toggle-content{ class: ("hide" unless expanded) } + - if image.tags.blank? + %li + .nothing-here-block No tags in Container Registry for this container image. + + - else + .table-holder + %table.table.tags + %thead + %tr + %th Name + %th Image ID + %th Size + %th Created + - if can?(current_user, :update_container_image, @project) + %th + + - image.tags.each do |tag| + = render 'tag', tag: tag diff --git a/app/views/projects/container_registry/_tag.html.haml b/app/views/projects/container_registry/_tag.html.haml index 10822b6184c..00345ec26de 100644 --- a/app/views/projects/container_registry/_tag.html.haml +++ b/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, 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", data: { confirm: "Are you sure?" }, method: :delete do = icon("trash cred") diff --git a/app/views/projects/container_registry/index.html.haml b/app/views/projects/container_registry/index.html.haml index 993da27310f..f074ce6be6d 100644 --- a/app/views/projects/container_registry/index.html.haml +++ b/app/views/projects/container_registry/index.html.haml @@ -19,21 +19,9 @@ %br docker push #{escape_once(@project.container_registry_repository_url)} - - if @tags.blank? - %li - .nothing-here-block No images in Container Registry for this project. + - if @images.blank? + .nothing-here-block No container images in Container Registry for this project. - else - .table-holder - %table.table.tags - %thead - %tr - %th Name - %th Image ID - %th Size - %th Created - - if can?(current_user, :update_container_image, @project) - %th - - - @tags.each do |tag| - = render 'tag', tag: tag + - @images.each do |image| + = render 'image', image: image diff --git a/db/migrate/20161029153736_create_container_images_repository.rb b/db/migrate/20161029153736_create_container_images_repository.rb deleted file mode 100644 index d93180b1674..00000000000 --- a/db/migrate/20161029153736_create_container_images_repository.rb +++ /dev/null @@ -1,31 +0,0 @@ -# See http://doc.gitlab.com/ce/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - -class CreateContainerImagesRepository < ActiveRecord::Migration - include Gitlab::Database::MigrationHelpers - - # Set this constant to true if this migration requires downtime. - DOWNTIME = false - - # When a migration requires downtime you **must** uncomment the following - # constant and define a short and easy to understand explanation as to why the - # migration requires downtime. - # DOWNTIME_REASON = '' - - # When using the methods "add_concurrent_index" or "add_column_with_default" - # you must disable the use of transactions as these methods can not run in an - # existing transaction. When using "add_concurrent_index" make sure that this - # method is the _only_ method called in the migration, any other changes - # should go in a separate migration. This ensures that upon failure _only_ the - # index creation fails and can be retried or reverted easily. - # - # To disable transactions uncomment the following line and remove these - # comments: - # disable_ddl_transaction! - - def change - create_table :container_images_repositories do |t| - t.integer :project_id, null: false - end - end -end diff --git a/db/migrate/20161031013926_create_container_image.rb b/db/migrate/20161031013926_create_container_image.rb index 94feae280a6..85c0913b8f3 100644 --- a/db/migrate/20161031013926_create_container_image.rb +++ b/db/migrate/20161031013926_create_container_image.rb @@ -25,7 +25,7 @@ class CreateContainerImage < ActiveRecord::Migration def change create_table :container_images do |t| - t.integer :container_images_repository_id + t.integer :project_id t.string :name end end diff --git a/lib/api/registry_events.rb b/lib/api/registry_events.rb index c0473051424..dc7279d2b75 100644 --- a/lib/api/registry_events.rb +++ b/lib/api/registry_events.rb @@ -41,9 +41,19 @@ module API if event['action'] == 'push' and !!event['target']['tag'] namespace, container_image_name = ContainerImage::split_namespace(repository) - ::ContainerImagesRepositories::ContainerImages::PushService.new( - Project::find_with_namespace(namespace), current_user - ).execute(container_image_name, event) + project = Project::find_with_namespace(namespace) + + if project + container_image = project.container_images.find_or_create_by(name: container_image_name) + + if container_image.valid? + puts('Valid!') + else + render_api_error!({ error: "Failed to create container image!" }, 400) + end + else + not_found!('Project') + end end end end From 246df2bd1151d39a04ef553064144eb75ee3e980 Mon Sep 17 00:00:00 2001 From: Andre Guedes Date: Tue, 13 Dec 2016 23:42:43 -0200 Subject: [PATCH 004/278] Adding registry endpoint authorization --- .../admin/application_settings_controller.rb | 6 ++++ .../admin/container_registry_controller.rb | 11 +++++++ app/models/application_setting.rb | 6 ++++ .../admin/container_registry/show.html.haml | 31 +++++++++++++++++++ app/views/admin/dashboard/_head.html.haml | 4 +++ config/routes/admin.rb | 2 ++ ...ry_access_token_to_application_settings.rb | 29 +++++++++++++++++ doc/administration/container_registry.md | 22 +++++++++++-- doc/ci/docker/using_docker_build.md | 8 ++--- doc/user/project/container_registry.md | 19 ++++++------ lib/api/helpers.rb | 10 ++++++ lib/api/registry_events.rb | 2 +- 12 files changed, 133 insertions(+), 17 deletions(-) create mode 100644 app/controllers/admin/container_registry_controller.rb create mode 100644 app/views/admin/container_registry/show.html.haml create mode 100644 db/migrate/20161213212947_add_container_registry_access_token_to_application_settings.rb diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index b0f5d4a9933..fb6df1a06d2 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -29,6 +29,12 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController redirect_to :back end + def reset_container_registry_token + @application_setting.reset_container_registry_access_token! + flash[:notice] = 'New container registry access token has been generated!' + redirect_to :back + end + def clear_repository_check_states RepositoryCheck::ClearWorker.perform_async diff --git a/app/controllers/admin/container_registry_controller.rb b/app/controllers/admin/container_registry_controller.rb new file mode 100644 index 00000000000..265c032c67d --- /dev/null +++ b/app/controllers/admin/container_registry_controller.rb @@ -0,0 +1,11 @@ +class Admin::ContainerRegistryController < Admin::ApplicationController + def show + @access_token = container_registry_access_token + end + + private + + def container_registry_access_token + current_application_settings.container_registry_access_token + end +end diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 74b358d8c40..b94a71e1ea7 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -4,6 +4,7 @@ class ApplicationSetting < ActiveRecord::Base add_authentication_token_field :runners_registration_token add_authentication_token_field :health_check_access_token + add_authentication_token_field :container_registry_access_token CACHE_KEY = 'application_setting.last' DOMAIN_LIST_SEPARATOR = %r{\s*[,;]\s* # comma or semicolon, optionally surrounded by whitespace @@ -141,6 +142,7 @@ class ApplicationSetting < ActiveRecord::Base before_save :ensure_runners_registration_token before_save :ensure_health_check_access_token + before_save :ensure_container_registry_access_token after_commit do Rails.cache.write(CACHE_KEY, self) @@ -276,6 +278,10 @@ class ApplicationSetting < ActiveRecord::Base ensure_health_check_access_token! end + def container_registry_access_token + ensure_container_registry_access_token! + end + def sidekiq_throttling_enabled? return false unless sidekiq_throttling_column_exists? diff --git a/app/views/admin/container_registry/show.html.haml b/app/views/admin/container_registry/show.html.haml new file mode 100644 index 00000000000..8803eddda69 --- /dev/null +++ b/app/views/admin/container_registry/show.html.haml @@ -0,0 +1,31 @@ +- @no_container = true += render "admin/dashboard/head" + +%div{ class: container_class } + + %p.prepend-top-default + %span + To properly configure the Container Registry you should add the following + access token to the Docker Registry config.yml as follows: + %pre + %code + :plain + notifications: + endpoints: + - ... + headers: + X-Registry-Token: [#{@access_token}] + %br + Access token is + %code{ id: 'registry-token' } #{@access_token} + + .bs-callout.clearfix + .pull-left + %p + You can reset container registry access token by pressing the button below. + %p + = button_to reset_container_registry_token_admin_application_settings_path, + method: :put, class: 'btn btn-default', + data: { confirm: 'Are you sure you want to reset container registry token?' } do + = icon('refresh') + Reset container registry access token diff --git a/app/views/admin/dashboard/_head.html.haml b/app/views/admin/dashboard/_head.html.haml index 7893c1dee97..dbd039547fa 100644 --- a/app/views/admin/dashboard/_head.html.haml +++ b/app/views/admin/dashboard/_head.html.haml @@ -27,3 +27,7 @@ = link_to admin_runners_path, title: 'Runners' do %span Runners + = nav_link path: 'container_registry#show' do + = link_to admin_container_registry_path, title: 'Registry' do + %span + Registry diff --git a/config/routes/admin.rb b/config/routes/admin.rb index 8e99239f350..b09c05826a7 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -58,6 +58,7 @@ namespace :admin do resource :background_jobs, controller: 'background_jobs', only: [:show] resource :system_info, controller: 'system_info', only: [:show] resources :requests_profiles, only: [:index, :show], param: :name, constraints: { name: /.+\.html/ } + resource :container_registry, controller: 'container_registry', only: [:show] resources :projects, only: [:index] @@ -88,6 +89,7 @@ namespace :admin do resources :services, only: [:index, :edit, :update] put :reset_runners_token put :reset_health_check_token + put :reset_container_registry_token put :clear_repository_check_states end diff --git a/db/migrate/20161213212947_add_container_registry_access_token_to_application_settings.rb b/db/migrate/20161213212947_add_container_registry_access_token_to_application_settings.rb new file mode 100644 index 00000000000..f89f9b00a5f --- /dev/null +++ b/db/migrate/20161213212947_add_container_registry_access_token_to_application_settings.rb @@ -0,0 +1,29 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddContainerRegistryAccessTokenToApplicationSettings < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + # When a migration requires downtime you **must** uncomment the following + # constant and define a short and easy to understand explanation as to why the + # migration requires downtime. + # DOWNTIME_REASON = '' + + # When using the methods "add_concurrent_index" or "add_column_with_default" + # you must disable the use of transactions as these methods can not run in an + # existing transaction. When using "add_concurrent_index" make sure that this + # method is the _only_ method called in the migration, any other changes + # should go in a separate migration. This ensures that upon failure _only_ the + # index creation fails and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + # disable_ddl_transaction! + + def change + add_column :application_settings, :container_registry_access_token, :string + end +end diff --git a/doc/administration/container_registry.md b/doc/administration/container_registry.md index a6300e18dc0..14795601246 100644 --- a/doc/administration/container_registry.md +++ b/doc/administration/container_registry.md @@ -76,7 +76,7 @@ you modify its settings. Read the upstream documentation on how to achieve that. At the absolute minimum, make sure your [Registry configuration][registry-auth] has `container_registry` as the service and `https://gitlab.example.com/jwt/auth` -as the realm: +as the realm. ``` auth: @@ -87,6 +87,23 @@ auth: rootcertbundle: /root/certs/certbundle ``` +Also a notification endpoint must be configured with the token from +Admin Area -> Overview -> Registry (`/admin/container_registry`) like in the following sample: + +``` +notifications: + endpoints: + - name: listener + url: https://gitlab.example.com/api/v3/registry_events + headers: + X-Registry-Token: [57Cx95fc2zHFh93VTiGD] + timeout: 500ms + threshold: 5 + backoff: 1s +``` + +Check the [Registry endpoint configuration][registry-endpoint] for details. + ## Container Registry domain configuration There are two ways you can configure the Registry's external domain. @@ -477,7 +494,7 @@ configurable in future releases. **GitLab 8.8 ([source docs][8-8-docs])** - GitLab Container Registry feature was introduced. - +i [reconfigure gitlab]: restart_gitlab.md#omnibus-gitlab-reconfigure [restart gitlab]: restart_gitlab.md#installations-from-source [wildcard certificate]: https://en.wikipedia.org/wiki/Wildcard_certificate @@ -487,6 +504,7 @@ configurable in future releases. [storage-config]: https://docs.docker.com/registry/configuration/#storage [registry-http-config]: https://docs.docker.com/registry/configuration/#http [registry-auth]: https://docs.docker.com/registry/configuration/#auth +[registry-endpoint]: https://docs.docker.com/registry/notifications/#/configuration [token-config]: https://docs.docker.com/registry/configuration/#token [8-8-docs]: https://gitlab.com/gitlab-org/gitlab-ce/blob/8-8-stable/doc/administration/container_registry.md [registry-ssl]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/lib/support/nginx/registry-ssl diff --git a/doc/ci/docker/using_docker_build.md b/doc/ci/docker/using_docker_build.md index 8620984d40d..6ae6269b28a 100644 --- a/doc/ci/docker/using_docker_build.md +++ b/doc/ci/docker/using_docker_build.md @@ -299,8 +299,8 @@ could look like: stage: build script: - docker login -u gitlab-ci-token -p $CI_BUILD_TOKEN registry.example.com - - docker build -t registry.example.com/group/project:latest . - - docker push registry.example.com/group/project:latest + - docker build -t registry.example.com/group/project/image:latest . + - docker push registry.example.com/group/project/image:latest ``` You have to use the special `gitlab-ci-token` user created for you in order to @@ -350,8 +350,8 @@ stages: - deploy variables: - CONTAINER_TEST_IMAGE: registry.example.com/my-group/my-project:$CI_BUILD_REF_NAME - CONTAINER_RELEASE_IMAGE: registry.example.com/my-group/my-project:latest + CONTAINER_TEST_IMAGE: registry.example.com/my-group/my-project/my-image:$CI_BUILD_REF_NAME + CONTAINER_RELEASE_IMAGE: registry.example.com/my-group/my-project/my-image:latest before_script: - docker login -u gitlab-ci-token -p $CI_BUILD_TOKEN registry.example.com diff --git a/doc/user/project/container_registry.md b/doc/user/project/container_registry.md index 91b35c73b34..eada8e04227 100644 --- a/doc/user/project/container_registry.md +++ b/doc/user/project/container_registry.md @@ -10,6 +10,7 @@ - Starting from GitLab 8.12, if you have 2FA enabled in your account, you need to pass a personal access token instead of your password in order to login to GitLab's Container Registry. +- Multiple level image names support was added in GitLab ?8.15? With the Docker Container Registry integrated into GitLab, every project can have its own space to store its Docker images. @@ -54,26 +55,23 @@ sure that you are using the Registry URL with the namespace and project name that is hosted on GitLab: ``` -docker build -t registry.example.com/group/project . -docker push registry.example.com/group/project +docker build -t registry.example.com/group/project/image . +docker push registry.example.com/group/project/image ``` Your image will be named after the following scheme: ``` -// +/// ``` -As such, the name of the image is unique, but you can differentiate the images -using tags. - ## Use images from GitLab Container Registry To download and run a container from images hosted in GitLab Container Registry, use `docker run`: ``` -docker run [options] registry.example.com/group/project [arguments] +docker run [options] registry.example.com/group/project/image [arguments] ``` For more information on running Docker containers, visit the @@ -87,7 +85,8 @@ and click **Registry** in the project menu. This view will show you all tags in your project and will easily allow you to delete them. -![Container Registry panel](img/container_registry_panel.png) +![Container Registry panel](image-needs-update) +[//]: # (img/container_registry_panel.png) ## Build and push images using GitLab CI @@ -136,7 +135,7 @@ A user attempted to enable an S3-backed Registry. The `docker login` step went fine. However, when pushing an image, the output showed: ``` -The push refers to a repository [s3-testing.myregistry.com:4567/root/docker-test] +The push refers to a repository [s3-testing.myregistry.com:4567/root/docker-test/docker-image] dc5e59c14160: Pushing [==================================================>] 14.85 kB 03c20c1a019a: Pushing [==================================================>] 2.048 kB a08f14ef632e: Pushing [==================================================>] 2.048 kB @@ -229,7 +228,7 @@ a container image. You may need to run as root to do this. For example: ```sh docker login s3-testing.myregistry.com:4567 -docker push s3-testing.myregistry.com:4567/root/docker-test +docker push s3-testing.myregistry.com:4567/root/docker-test/docker-image ``` In the example above, we see the following trace on the mitmproxy window: diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index a1db2099693..0fd2b1587e3 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -111,6 +111,16 @@ module API end end + def authenticate_container_registry_access_token! + token = request.headers['X-Registry-Token'] + unless token.present? && ActiveSupport::SecurityUtils.variable_size_secure_compare( + token, + current_application_settings.container_registry_access_token + ) + unauthorized! + end + end + def authenticated_as_admin! authenticate! forbidden! unless current_user.is_admin? diff --git a/lib/api/registry_events.rb b/lib/api/registry_events.rb index dc7279d2b75..e52433339eb 100644 --- a/lib/api/registry_events.rb +++ b/lib/api/registry_events.rb @@ -1,7 +1,7 @@ module API # RegistryEvents API class RegistryEvents < Grape::API - # before { authenticate! } + before { authenticate_container_registry_access_token! } content_type :json, 'application/vnd.docker.distribution.events.v1+json' From e4fa80f3b67f1ef30c262cd4df28516ccff6336a Mon Sep 17 00:00:00 2001 From: Andre Guedes Date: Fri, 16 Dec 2016 01:24:05 -0200 Subject: [PATCH 005/278] Fixes broken and missing tests --- .../stylesheets/pages/container_registry.scss | 6 +- .../projects/container_registry_controller.rb | 1 - app/models/container_image.rb | 4 +- app/models/namespace.rb | 8 +- app/models/project.rb | 18 ++--- ...ntainer_registry_authentication_service.rb | 3 +- .../container_images/destroy_service.rb | 1 - app/services/projects/transfer_service.rb | 6 +- .../container_registry/_image.html.haml | 2 +- .../container_registry/_tag.html.haml | 2 +- .../container_registry/index.html.haml | 4 +- db/schema.rb | 6 ++ lib/api/registry_events.rb | 4 +- lib/container_registry/blob.rb | 4 +- lib/container_registry/registry.rb | 4 - lib/container_registry/repository.rb | 48 ------------ spec/factories/container_images.rb | 21 ++++++ spec/features/container_registry_spec.rb | 32 +++++--- .../security/project/internal_access_spec.rb | 3 + .../security/project/private_access_spec.rb | 3 + .../security/project/public_access_spec.rb | 3 + spec/lib/container_registry/blob_spec.rb | 15 +++- spec/lib/container_registry/registry_spec.rb | 2 +- .../lib/container_registry/repository_spec.rb | 65 ----------------- spec/lib/container_registry/tag_spec.rb | 11 ++- spec/lib/gitlab/import_export/all_models.yml | 3 + spec/models/ci/build_spec.rb | 2 +- spec/models/container_image_spec.rb | 73 +++++++++++++++++++ spec/models/namespace_spec.rb | 8 +- spec/models/project_spec.rb | 41 ++--------- .../services/projects/destroy_service_spec.rb | 15 ++-- .../projects/transfer_service_spec.rb | 3 + 32 files changed, 211 insertions(+), 210 deletions(-) delete mode 100644 lib/container_registry/repository.rb create mode 100644 spec/factories/container_images.rb delete mode 100644 spec/lib/container_registry/repository_spec.rb create mode 100644 spec/models/container_image_spec.rb diff --git a/app/assets/stylesheets/pages/container_registry.scss b/app/assets/stylesheets/pages/container_registry.scss index 7d68eae3c97..92543d7d714 100644 --- a/app/assets/stylesheets/pages/container_registry.scss +++ b/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; } diff --git a/app/controllers/projects/container_registry_controller.rb b/app/controllers/projects/container_registry_controller.rb index 54bcb5f504a..f656f86fcdb 100644 --- a/app/controllers/projects/container_registry_controller.rb +++ b/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 diff --git a/app/models/container_image.rb b/app/models/container_image.rb index 7721c53a6fc..583cb977910 100644 --- a/app/models/container_image.rb +++ b/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') diff --git a/app/models/namespace.rb b/app/models/namespace.rb index bd0336c984a..c8e329044e0 100644 --- a/app/models/namespace.rb +++ b/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 diff --git a/app/models/project.rb b/app/models/project.rb index afaf2095a4c..d4f5584f53d 100644 --- a/app/models/project.rb +++ b/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" + 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 tags in container registry - raise StandardError.new('Project cannot be renamed, because tags are present in its container registry') + # 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 diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb index 6b83b38fa4d..5b2fcdf3b16 100644 --- a/app/services/auth/container_registry_authentication_service.rb +++ b/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 diff --git a/app/services/container_images/destroy_service.rb b/app/services/container_images/destroy_service.rb index bc5b53fd055..c73b6cfefba 100644 --- a/app/services/container_images/destroy_service.rb +++ b/app/services/container_images/destroy_service.rb @@ -1,6 +1,5 @@ module ContainerImages class DestroyService < BaseService - class DestroyError < StandardError; end def execute(container_image) diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb index 20dfbddc823..3e241b9e7c0 100644 --- a/app/services/projects/transfer_service.rb +++ b/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) diff --git a/app/views/projects/container_registry/_image.html.haml b/app/views/projects/container_registry/_image.html.haml index b1d62e34a97..5845efd345a 100644 --- a/app/views/projects/container_registry/_image.html.haml +++ b/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") diff --git a/app/views/projects/container_registry/_tag.html.haml b/app/views/projects/container_registry/_tag.html.haml index 00345ec26de..b35a9cb621f 100644 --- a/app/views/projects/container_registry/_tag.html.haml +++ b/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") diff --git a/app/views/projects/container_registry/index.html.haml b/app/views/projects/container_registry/index.html.haml index f074ce6be6d..ab6213f03d8 100644 --- a/app/views/projects/container_registry/index.html.haml +++ b/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. diff --git a/db/schema.rb b/db/schema.rb index 88aaa6c3c55..36df20fc8f2 100644 --- a/db/schema.rb +++ b/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 diff --git a/lib/api/registry_events.rb b/lib/api/registry_events.rb index e52433339eb..12305a49f0f 100644 --- a/lib/api/registry_events.rb +++ b/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 diff --git a/lib/container_registry/blob.rb b/lib/container_registry/blob.rb index eb5a2596177..8db8e483b1d 100644 --- a/lib/container_registry/blob.rb +++ b/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 diff --git a/lib/container_registry/registry.rb b/lib/container_registry/registry.rb index 0e634f6b6ef..63bce655f57 100644 --- a/lib/container_registry/registry.rb +++ b/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 diff --git a/lib/container_registry/repository.rb b/lib/container_registry/repository.rb deleted file mode 100644 index 0e4a7cb3cc9..00000000000 --- a/lib/container_registry/repository.rb +++ /dev/null @@ -1,48 +0,0 @@ -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 diff --git a/spec/factories/container_images.rb b/spec/factories/container_images.rb new file mode 100644 index 00000000000..6141a519a75 --- /dev/null +++ b/spec/factories/container_images.rb @@ -0,0 +1,21 @@ +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 diff --git a/spec/features/container_registry_spec.rb b/spec/features/container_registry_spec.rb index 203e55a36f2..862c9fbf6c0 100644 --- a/spec/features/container_registry_spec.rb +++ b/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) { [] } + context 'when no images' do + let(:container_image) { } - it { expect(page).to have_content('No images in Container Registry for this project') } + it { expect(page).to have_content('No container images in Container Registry for this project') } end - context 'when there are tags' do - it { expect(page).to have_content(tag_name) } - it { expect(page).to have_content('d7a513a66') } + context 'when there are images' do + it { expect(page).to have_content(image_name) } + end + 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 diff --git a/spec/features/security/project/internal_access_spec.rb b/spec/features/security/project/internal_access_spec.rb index 24af062d763..4e7a2c0ecc0 100644 --- a/spec/features/security/project/internal_access_spec.rb +++ b/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) } diff --git a/spec/features/security/project/private_access_spec.rb b/spec/features/security/project/private_access_spec.rb index c511dcfa18e..c74cdc05593 100644 --- a/spec/features/security/project/private_access_spec.rb +++ b/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) } diff --git a/spec/features/security/project/public_access_spec.rb b/spec/features/security/project/public_access_spec.rb index d8cc012c27e..485ef335b78 100644 --- a/spec/features/security/project/public_access_spec.rb +++ b/spec/features/security/project/public_access_spec.rb @@ -429,9 +429,12 @@ describe "Public 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) } diff --git a/spec/lib/container_registry/blob_spec.rb b/spec/lib/container_registry/blob_spec.rb index bbacdc67ebd..f092449c4bd 100644 --- a/spec/lib/container_registry/blob_spec.rb +++ b/spec/lib/container_registry/blob_spec.rb @@ -9,12 +9,19 @@ describe ContainerRegistry::Blob do 'size' => 1000 } end - let(:token) { 'authorization-token' } - - let(:registry) { ContainerRegistry::Registry.new('http://example.com', token: token) } - let(:repository) { registry.repository('group/test') } + let(:token) { 'token' } + + let(:group) { create(:group, name: 'group') } + let(:project) { create(:project, path: 'test', group: group) } + let(:example_host) { 'example.com' } + let(:registry_url) { 'http://' + example_host } + let(:repository) { create(:container_image, name: '', project: project) } let(:blob) { repository.blob(config) } + before do + stub_container_registry_config(enabled: true, api_url: registry_url, host_port: example_host) + end + it { expect(blob).to respond_to(:repository) } it { expect(blob).to delegate_method(:registry).to(:repository) } it { expect(blob).to delegate_method(:client).to(:repository) } diff --git a/spec/lib/container_registry/registry_spec.rb b/spec/lib/container_registry/registry_spec.rb index 4f3f8b24fc4..4d6eea94bf0 100644 --- a/spec/lib/container_registry/registry_spec.rb +++ b/spec/lib/container_registry/registry_spec.rb @@ -10,7 +10,7 @@ describe ContainerRegistry::Registry do it { is_expected.to respond_to(:uri) } it { is_expected.to respond_to(:path) } - it { expect(subject.repository('test')).not_to be_nil } + it { expect(subject).not_to be_nil } context '#path' do subject { registry.path } diff --git a/spec/lib/container_registry/repository_spec.rb b/spec/lib/container_registry/repository_spec.rb deleted file mode 100644 index c364e759108..00000000000 --- a/spec/lib/container_registry/repository_spec.rb +++ /dev/null @@ -1,65 +0,0 @@ -require 'spec_helper' - -describe ContainerRegistry::Repository do - let(:registry) { ContainerRegistry::Registry.new('http://example.com') } - let(:repository) { registry.repository('group/test') } - - it { expect(repository).to respond_to(:registry) } - it { expect(repository).to delegate_method(:client).to(:registry) } - it { expect(repository.tag('test')).not_to be_nil } - - context '#path' do - subject { repository.path } - - it { is_expected.to eq('example.com/group/test') } - end - - context 'manifest processing' do - before do - stub_request(:get, 'http://example.com/v2/group/test/tags/list'). - with(headers: { 'Accept' => 'application/vnd.docker.distribution.manifest.v2+json' }). - to_return( - status: 200, - body: JSON.dump(tags: ['test']), - headers: { 'Content-Type' => 'application/json' }) - end - - context '#manifest' do - subject { repository.manifest } - - it { is_expected.not_to be_nil } - end - - context '#valid?' do - subject { repository.valid? } - - it { is_expected.to be_truthy } - end - - context '#tags' do - subject { repository.tags } - - it { is_expected.not_to be_empty } - end - end - - context '#delete_tags' do - let(:tag) { ContainerRegistry::Tag.new(repository, 'tag') } - - before { expect(repository).to receive(:tags).twice.and_return([tag]) } - - subject { repository.delete_tags } - - context 'succeeds' do - before { expect(tag).to receive(:delete).and_return(true) } - - it { is_expected.to be_truthy } - end - - context 'any fails' do - before { expect(tag).to receive(:delete).and_return(false) } - - it { is_expected.to be_falsey } - end - end -end diff --git a/spec/lib/container_registry/tag_spec.rb b/spec/lib/container_registry/tag_spec.rb index c5e31ae82b6..cdd0fe66bc3 100644 --- a/spec/lib/container_registry/tag_spec.rb +++ b/spec/lib/container_registry/tag_spec.rb @@ -1,11 +1,18 @@ require 'spec_helper' describe ContainerRegistry::Tag do - let(:registry) { ContainerRegistry::Registry.new('http://example.com') } - let(:repository) { registry.repository('group/test') } + let(:group) { create(:group, name: 'group') } + let(:project) { create(:project, path: 'test', group: group) } + let(:example_host) { 'example.com' } + let(:registry_url) { 'http://' + example_host } + let(:repository) { create(:container_image, name: '', project: project) } let(:tag) { repository.tag('tag') } let(:headers) { { 'Accept' => 'application/vnd.docker.distribution.manifest.v2+json' } } + before do + stub_container_registry_config(enabled: true, api_url: registry_url, host_port: example_host) + end + it { expect(tag).to respond_to(:repository) } it { expect(tag).to delegate_method(:registry).to(:repository) } it { expect(tag).to delegate_method(:client).to(:repository) } diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 06617f3b007..9c08f41fe82 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -114,6 +114,8 @@ merge_access_levels: - protected_branch push_access_levels: - protected_branch +container_images: +- name project: - taggings - base_tags @@ -197,6 +199,7 @@ project: - project_authorizations - route - statistics +- container_images award_emoji: - awardable - user diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 2dfca8bcfce..83a2efb55b9 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1397,7 +1397,7 @@ describe Ci::Build, :models do { key: 'CI_REGISTRY', value: 'registry.example.com', public: true } end let(:ci_registry_image) do - { key: 'CI_REGISTRY_IMAGE', value: project.container_registry_repository_url, public: true } + { key: 'CI_REGISTRY_IMAGE', value: project.container_registry_url, public: true } end context 'and is disabled for project' do diff --git a/spec/models/container_image_spec.rb b/spec/models/container_image_spec.rb new file mode 100644 index 00000000000..e0bea737f59 --- /dev/null +++ b/spec/models/container_image_spec.rb @@ -0,0 +1,73 @@ +require 'spec_helper' + +describe ContainerImage do + let(:group) { create(:group, name: 'group') } + let(:project) { create(:project, path: 'test', group: group) } + let(:example_host) { 'example.com' } + let(:registry_url) { 'http://' + example_host } + let(:container_image) { create(:container_image, name: '', project: project, stubbed: false) } + + before do + stub_container_registry_config(enabled: true, api_url: registry_url, host_port: example_host) + stub_request(:get, 'http://example.com/v2/group/test/tags/list'). + with(headers: { 'Accept' => 'application/vnd.docker.distribution.manifest.v2+json' }). + to_return( + status: 200, + body: JSON.dump(tags: ['test']), + headers: { 'Content-Type' => 'application/json' }) + end + + it { expect(container_image).to respond_to(:project) } + it { expect(container_image).to delegate_method(:container_registry).to(:project) } + it { expect(container_image).to delegate_method(:client).to(:container_registry) } + it { expect(container_image.tag('test')).not_to be_nil } + + context '#path' do + subject { container_image.path } + + it { is_expected.to eq('example.com/group/test') } + end + + context 'manifest processing' do + context '#manifest' do + subject { container_image.manifest } + + it { is_expected.not_to be_nil } + end + + context '#valid?' do + subject { container_image.valid? } + + it { is_expected.to be_truthy } + end + + context '#tags' do + subject { container_image.tags } + + it { is_expected.not_to be_empty } + end + end + + context '#delete_tags' do + let(:tag) { ContainerRegistry::Tag.new(container_image, 'tag') } + + before do + expect(container_image).to receive(:tags).twice.and_return([tag]) + expect(tag).to receive(:digest).and_return('sha256:4c8e63ca4cb663ce6c688cb06f1c3672a172b088dac5b6d7ad7d49cd620d85cf') + end + + subject { container_image.delete_tags } + + context 'succeeds' do + before { expect(container_image.client).to receive(:delete_repository_tag).and_return(true) } + + it { is_expected.to be_truthy } + end + + context 'any fails' do + before { expect(container_image.client).to receive(:delete_repository_tag).and_return(false) } + + it { is_expected.to be_falsey } + end + end +end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 35d932f1c64..aeb4eeb0b55 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -134,18 +134,20 @@ describe Namespace, models: true do expect(@namespace.move_dir).to be_truthy end - context "when any project has container tags" do + context "when any project has container images" do + let(:container_image) { create(:container_image) } + before do stub_container_registry_config(enabled: true) stub_container_registry_tags('tag') - create(:empty_project, namespace: @namespace) + create(:empty_project, namespace: @namespace, container_images: [container_image]) allow(@namespace).to receive(:path_was).and_return(@namespace.path) allow(@namespace).to receive(:path).and_return('new_path') end - it { expect { @namespace.move_dir }.to raise_error('Namespace cannot be moved, because at least one project has tags in container registry') } + it { expect { @namespace.move_dir }.to raise_error('Namespace cannot be moved, because at least one project has images in container registry') } end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index b0087a9e15d..77f2ff3d17b 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1173,10 +1173,13 @@ describe Project, models: true do project.rename_repo end - context 'container registry with tags' do + context 'container registry with images' do + let(:container_image) { create(:container_image) } + before do stub_container_registry_config(enabled: true) stub_container_registry_tags('tag') + project.container_images << container_image end subject { project.rename_repo } @@ -1383,20 +1386,20 @@ describe Project, models: true do it { is_expected.to eq(project.path_with_namespace.downcase) } end - describe '#container_registry_repository' do + describe '#container_registry' do let(:project) { create(:empty_project) } before { stub_container_registry_config(enabled: true) } - subject { project.container_registry_repository } + subject { project.container_registry } it { is_expected.not_to be_nil } end - describe '#container_registry_repository_url' do + describe '#container_registry_url' do let(:project) { create(:empty_project) } - subject { project.container_registry_repository_url } + subject { project.container_registry_url } before { stub_container_registry_config(**registry_settings) } @@ -1422,34 +1425,6 @@ describe Project, models: true do end end - describe '#has_container_registry_tags?' do - let(:project) { create(:empty_project) } - - subject { project.has_container_registry_tags? } - - context 'for enabled registry' do - before { stub_container_registry_config(enabled: true) } - - context 'with tags' do - before { stub_container_registry_tags('test', 'test2') } - - it { is_expected.to be_truthy } - end - - context 'when no tags' do - before { stub_container_registry_tags } - - it { is_expected.to be_falsey } - end - end - - context 'for disabled registry' do - before { stub_container_registry_config(enabled: false) } - - it { is_expected.to be_falsey } - end - end - describe '#latest_successful_builds_for' do def create_pipeline(status = 'success') create(:ci_pipeline, project: project, diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb index 74bfba44dfd..270e630e70e 100644 --- a/spec/services/projects/destroy_service_spec.rb +++ b/spec/services/projects/destroy_service_spec.rb @@ -90,25 +90,30 @@ describe Projects::DestroyService, services: true do end context 'container registry' do + let(:container_image) { create(:container_image) } + before do stub_container_registry_config(enabled: true) stub_container_registry_tags('tag') + project.container_images << container_image end - context 'tags deletion succeeds' do + context 'images deletion succeeds' do it do - expect_any_instance_of(ContainerRegistry::Tag).to receive(:delete).and_return(true) + expect_any_instance_of(ContainerImage).to receive(:delete_tags).and_return(true) destroy_project(project, user, {}) end end - context 'tags deletion fails' do - before { expect_any_instance_of(ContainerRegistry::Tag).to receive(:delete).and_return(false) } + context 'images deletion fails' do + before do + expect_any_instance_of(ContainerImage).to receive(:delete_tags).and_return(false) + end subject { destroy_project(project, user, {}) } - it { expect{subject}.to raise_error(Projects::DestroyService::DestroyError) } + it { expect{subject}.to raise_error(ActiveRecord::RecordNotDestroyed) } end end diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index 5c6fbea8d0e..5e56226ff91 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -29,9 +29,12 @@ describe Projects::TransferService, services: true do end context 'disallow transfering of project with tags' do + let(:container_image) { create(:container_image) } + before do stub_container_registry_config(enabled: true) stub_container_registry_tags('tag') + project.container_images << container_image end subject { transfer_project(project, user, group) } From 164ef8a348cac86097313bc453493ccf739adffe Mon Sep 17 00:00:00 2001 From: Andre Guedes Date: Fri, 16 Dec 2016 11:12:37 -0200 Subject: [PATCH 006/278] Fixing typos in docs --- doc/administration/container_registry.md | 4 ++-- doc/user/project/container_registry.md | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/administration/container_registry.md b/doc/administration/container_registry.md index 14795601246..4d1cb391e69 100644 --- a/doc/administration/container_registry.md +++ b/doc/administration/container_registry.md @@ -76,7 +76,7 @@ you modify its settings. Read the upstream documentation on how to achieve that. At the absolute minimum, make sure your [Registry configuration][registry-auth] has `container_registry` as the service and `https://gitlab.example.com/jwt/auth` -as the realm. +as the realm: ``` auth: @@ -494,7 +494,7 @@ configurable in future releases. **GitLab 8.8 ([source docs][8-8-docs])** - GitLab Container Registry feature was introduced. -i + [reconfigure gitlab]: restart_gitlab.md#omnibus-gitlab-reconfigure [restart gitlab]: restart_gitlab.md#installations-from-source [wildcard certificate]: https://en.wikipedia.org/wiki/Wildcard_certificate diff --git a/doc/user/project/container_registry.md b/doc/user/project/container_registry.md index eada8e04227..c5b2266ff19 100644 --- a/doc/user/project/container_registry.md +++ b/doc/user/project/container_registry.md @@ -10,7 +10,7 @@ - Starting from GitLab 8.12, if you have 2FA enabled in your account, you need to pass a personal access token instead of your password in order to login to GitLab's Container Registry. -- Multiple level image names support was added in GitLab ?8.15? +- Multiple level image names support was added in GitLab 8.15 With the Docker Container Registry integrated into GitLab, every project can have its own space to store its Docker images. From b408a192e0fbf630d4f9a4112f6835be50a681d8 Mon Sep 17 00:00:00 2001 From: Andre Guedes Date: Fri, 16 Dec 2016 12:07:20 -0200 Subject: [PATCH 007/278] Adding mock for full_access_token --- spec/factories/container_images.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/factories/container_images.rb b/spec/factories/container_images.rb index 6141a519a75..3693865101d 100644 --- a/spec/factories/container_images.rb +++ b/spec/factories/container_images.rb @@ -11,6 +11,7 @@ FactoryGirl.define do after(:build) do |image, evaluator| if evaluator.stubbed allow(Gitlab.config.registry).to receive(:enabled).and_return(true) + allow(Auth::ContainerRegistryAuthenticationService).to receive(:full_access_token).and_return('token') allow(image.client).to receive(:repository_tags).and_return({ name: image.name_with_namespace, tags: evaluator.tags From ea17df5c4c23890c48cd51af17e2517f04f7c88b Mon Sep 17 00:00:00 2001 From: Andre Guedes Date: Wed, 25 Jan 2017 10:02:09 -0200 Subject: [PATCH 008/278] Fixing minor view issues --- app/views/projects/container_registry/_image.html.haml | 6 +++--- app/views/projects/container_registry/_tag.html.haml | 2 +- app/views/projects/container_registry/index.html.haml | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/views/projects/container_registry/_image.html.haml b/app/views/projects/container_registry/_image.html.haml index 5845efd345a..72f2103b862 100644 --- a/app/views/projects/container_registry/_image.html.haml +++ b/app/views/projects/container_registry/_image.html.haml @@ -8,7 +8,7 @@ = icon("chevron-down") = escape_once(image.name) - = clipboard_button(clipboard_text: "docker pull #{image.path}") + = 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 image", data: { confirm: "Are you sure?" }, method: :delete do = icon("trash cred") @@ -24,8 +24,8 @@ %table.table.tags %thead %tr - %th Name - %th Image ID + %th Tag + %th Tag ID %th Size %th Created - if can?(current_user, :update_container_image, @project) diff --git a/app/views/projects/container_registry/_tag.html.haml b/app/views/projects/container_registry/_tag.html.haml index b35a9cb621f..f7161e85428 100644 --- a/app/views/projects/container_registry/_tag.html.haml +++ b/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 tag", 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: "Due to a Docker limitation, all tags with the same ID will also be deleted. Are you sure?" }, method: :delete do = icon("trash cred") diff --git a/app/views/projects/container_registry/index.html.haml b/app/views/projects/container_registry/index.html.haml index ab6213f03d8..5508a3de396 100644 --- a/app/views/projects/container_registry/index.html.haml +++ b/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_url)} . + docker build -t #{escape_once(@project.container_registry_url)}/image . %br - docker push #{escape_once(@project.container_registry_url)} + docker push #{escape_once(@project.container_registry_url)}/image - if @images.blank? .nothing-here-block No container images in Container Registry for this project. From 8294756fc110fdb84036e4ae097940410a8ad6de Mon Sep 17 00:00:00 2001 From: Andre Guedes Date: Wed, 25 Jan 2017 10:24:50 -0200 Subject: [PATCH 009/278] Improved readability in tag/image delete condition --- .../projects/container_registry_controller.rb | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/app/controllers/projects/container_registry_controller.rb b/app/controllers/projects/container_registry_controller.rb index f656f86fcdb..4981e57ed22 100644 --- a/app/controllers/projects/container_registry_controller.rb +++ b/app/controllers/projects/container_registry_controller.rb @@ -9,31 +9,37 @@ class Projects::ContainerRegistryController < Projects::ApplicationController end def destroy - url = namespace_project_container_registry_index_path(project.namespace, project) - if tag - delete_tag(url) + delete_tag else - if image.destroy - redirect_to url - else - redirect_to url, alert: 'Failed to remove image' - end + delete_image end end private + def registry_url + @registry_url ||= namespace_project_container_registry_index_path(project.namespace, project) + end + def verify_registry_enabled render_404 unless Gitlab.config.registry.enabled end - def delete_tag(url) + def delete_image + if image.destroy + redirect_to registry_url + else + redirect_to registry_url, alert: 'Failed to remove image' + end + end + + def delete_tag if tag.delete image.destroy if image.tags.empty? - redirect_to url + redirect_to registry_url else - redirect_to url, alert: 'Failed to remove tag' + redirect_to registry_url, alert: 'Failed to remove tag' end end From db5b4b8b1a9b8aa07c8310dde53b7c3ed391bafd Mon Sep 17 00:00:00 2001 From: Andre Guedes Date: Wed, 22 Feb 2017 11:19:23 -0300 Subject: [PATCH 010/278] Creates specs for destroy service and improves namespace container image query performance --- app/models/namespace.rb | 2 +- .../container_images/destroy_service.rb | 26 ++------------ .../admin/container_registry/show.html.haml | 2 +- .../20161031013926_create_container_image.rb | 16 --------- ...ry_access_token_to_application_settings.rb | 16 --------- db/schema.rb | 10 +++--- lib/api/registry_events.rb | 4 +-- .../container_images/destroy_service_spec.rb | 34 +++++++++++++++++++ 8 files changed, 45 insertions(+), 65 deletions(-) create mode 100644 spec/services/container_images/destroy_service_spec.rb diff --git a/app/models/namespace.rb b/app/models/namespace.rb index c8e329044e0..a803be2e780 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -155,7 +155,7 @@ class Namespace < ActiveRecord::Base end def any_project_has_container_registry_images? - projects.any? { |project| project.container_images.present? } + projects.joins(:container_images).any? end def send_update_instructions diff --git a/app/services/container_images/destroy_service.rb b/app/services/container_images/destroy_service.rb index c73b6cfefba..15dca227291 100644 --- a/app/services/container_images/destroy_service.rb +++ b/app/services/container_images/destroy_service.rb @@ -1,31 +1,9 @@ module ContainerImages class DestroyService < BaseService - class DestroyError < StandardError; end - def execute(container_image) - @container_image = container_image + return false unless can?(current_user, :update_container_image, project) - return false unless can?(current_user, :remove_project, project) - - ContainerImage.transaction do - container_image.destroy! - - unless remove_container_image_tags - raise_error('Failed to remove container image tags. Please try again or contact administrator') - end - end - - true - end - - private - - def raise_error(message) - raise DestroyError.new(message) - end - - def remove_container_image_tags - container_image.delete_tags + container_image.destroy! end end end diff --git a/app/views/admin/container_registry/show.html.haml b/app/views/admin/container_registry/show.html.haml index 8803eddda69..ffaa7736d65 100644 --- a/app/views/admin/container_registry/show.html.haml +++ b/app/views/admin/container_registry/show.html.haml @@ -17,7 +17,7 @@ X-Registry-Token: [#{@access_token}] %br Access token is - %code{ id: 'registry-token' } #{@access_token} + %code{ id: 'registry-token' }= @access_token .bs-callout.clearfix .pull-left diff --git a/db/migrate/20161031013926_create_container_image.rb b/db/migrate/20161031013926_create_container_image.rb index 85c0913b8f3..884c78880eb 100644 --- a/db/migrate/20161031013926_create_container_image.rb +++ b/db/migrate/20161031013926_create_container_image.rb @@ -7,22 +7,6 @@ class CreateContainerImage < ActiveRecord::Migration # Set this constant to true if this migration requires downtime. DOWNTIME = false - # When a migration requires downtime you **must** uncomment the following - # constant and define a short and easy to understand explanation as to why the - # migration requires downtime. - # DOWNTIME_REASON = '' - - # When using the methods "add_concurrent_index" or "add_column_with_default" - # you must disable the use of transactions as these methods can not run in an - # existing transaction. When using "add_concurrent_index" make sure that this - # method is the _only_ method called in the migration, any other changes - # should go in a separate migration. This ensures that upon failure _only_ the - # index creation fails and can be retried or reverted easily. - # - # To disable transactions uncomment the following line and remove these - # comments: - # disable_ddl_transaction! - def change create_table :container_images do |t| t.integer :project_id diff --git a/db/migrate/20161213212947_add_container_registry_access_token_to_application_settings.rb b/db/migrate/20161213212947_add_container_registry_access_token_to_application_settings.rb index f89f9b00a5f..23d87cc6d0a 100644 --- a/db/migrate/20161213212947_add_container_registry_access_token_to_application_settings.rb +++ b/db/migrate/20161213212947_add_container_registry_access_token_to_application_settings.rb @@ -7,22 +7,6 @@ class AddContainerRegistryAccessTokenToApplicationSettings < ActiveRecord::Migra # Set this constant to true if this migration requires downtime. DOWNTIME = false - # When a migration requires downtime you **must** uncomment the following - # constant and define a short and easy to understand explanation as to why the - # migration requires downtime. - # DOWNTIME_REASON = '' - - # When using the methods "add_concurrent_index" or "add_column_with_default" - # you must disable the use of transactions as these methods can not run in an - # existing transaction. When using "add_concurrent_index" make sure that this - # method is the _only_ method called in the migration, any other changes - # should go in a separate migration. This ensures that upon failure _only_ the - # index creation fails and can be retried or reverted easily. - # - # To disable transactions uncomment the following line and remove these - # comments: - # disable_ddl_transaction! - def change add_column :application_settings, :container_registry_access_token, :string end diff --git a/db/schema.rb b/db/schema.rb index 36df20fc8f2..08d11546800 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -61,6 +61,7 @@ ActiveRecord::Schema.define(version: 20170215200045) do t.boolean "shared_runners_enabled", default: true, null: false t.integer "max_artifacts_size", default: 100, null: false t.string "runners_registration_token" + t.integer "max_pages_size", default: 100, null: false t.boolean "require_two_factor_authentication", default: false t.integer "two_factor_grace_period", default: 48 t.boolean "metrics_enabled", default: false @@ -107,10 +108,9 @@ ActiveRecord::Schema.define(version: 20170215200045) do t.string "sidekiq_throttling_queues" t.decimal "sidekiq_throttling_factor" t.boolean "html_emails_enabled", default: true + t.string "container_registry_access_token" 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 @@ -586,9 +586,9 @@ ActiveRecord::Schema.define(version: 20170215200045) do end add_index "labels", ["group_id", "project_id", "title"], name: "index_labels_on_group_id_and_project_id_and_title", unique: true, using: :btree - add_index "labels", ["type", "project_id"], name: "index_labels_on_type_and_project_id", using: :btree add_index "labels", ["project_id"], name: "index_labels_on_project_id", using: :btree add_index "labels", ["title"], name: "index_labels_on_title", using: :btree + add_index "labels", ["type", "project_id"], name: "index_labels_on_type_and_project_id", using: :btree create_table "lfs_objects", force: :cascade do |t| t.string "oid", null: false @@ -761,8 +761,8 @@ ActiveRecord::Schema.define(version: 20170215200045) do t.integer "visibility_level", default: 20, null: false t.boolean "request_access_enabled", default: false, null: false t.datetime "deleted_at" - t.boolean "lfs_enabled" t.text "description_html" + t.boolean "lfs_enabled" t.integer "parent_id" end @@ -1283,8 +1283,8 @@ ActiveRecord::Schema.define(version: 20170215200045) do t.datetime "otp_grace_period_started_at" t.boolean "ldap_email", default: false, null: false t.boolean "external", default: false - t.string "organization" t.string "incoming_email_token" + t.string "organization" t.boolean "authorized_projects_populated" t.boolean "notified_of_own_activity", default: false, null: false end diff --git a/lib/api/registry_events.rb b/lib/api/registry_events.rb index 12305a49f0f..fc6fc0b97e0 100644 --- a/lib/api/registry_events.rb +++ b/lib/api/registry_events.rb @@ -39,9 +39,9 @@ module API params['events'].each do |event| repository = event['target']['repository'] - if event['action'] == 'push' and !!event['target']['tag'] + if event['action'] == 'push' && !!event['target']['tag'] namespace, container_image_name = ContainerImage::split_namespace(repository) - project = Project::find_with_namespace(namespace) + project = Project::find_by_full_path(namespace) if project container_image = project.container_images.find_or_create_by(name: container_image_name) diff --git a/spec/services/container_images/destroy_service_spec.rb b/spec/services/container_images/destroy_service_spec.rb new file mode 100644 index 00000000000..5b4dbaa7934 --- /dev/null +++ b/spec/services/container_images/destroy_service_spec.rb @@ -0,0 +1,34 @@ +require 'spec_helper' + +describe ContainerImages::DestroyService, services: true do + describe '#execute' do + let(:user) { create(:user) } + let(:container_image) { create(:container_image, name: '') } + let(:project) { create(:project, path: 'test', namespace: user.namespace, container_images: [container_image]) } + let(:example_host) { 'example.com' } + let(:registry_url) { 'http://' + example_host } + + it { expect(container_image).to be_valid } + it { expect(project.container_images).not_to be_empty } + + context 'when container image has tags' do + before do + project.team << [user, :master] + end + + it 'removes all tags before destroy' do + service = described_class.new(project, user) + + expect(container_image).to receive(:delete_tags).and_return(true) + expect { service.execute(container_image) }.to change(project.container_images, :count).by(-1) + end + + it 'fails when tags are not removed' do + service = described_class.new(project, user) + + expect(container_image).to receive(:delete_tags).and_return(false) + expect { service.execute(container_image) }.to raise_error(ActiveRecord::RecordNotDestroyed) + end + end + end +end From 16cca3a0ea7f4b95e99d7b3e8d4953334fa7bec7 Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Fri, 17 Mar 2017 17:25:17 +0100 Subject: [PATCH 011/278] Expose if action is playable in JSON To avoid a manual build action being played (resulting in a 404), expose `playable?` in the JSON so the frontend can disable/hide the play button if it's not playable. --- .../javascripts/environments/components/environment_item.js | 1 + app/serializers/build_action_entity.rb | 2 ++ app/serializers/build_entity.rb | 1 + spec/serializers/build_action_entity_spec.rb | 4 ++++ spec/serializers/build_entity_spec.rb | 4 ++++ 5 files changed, 12 insertions(+) diff --git a/app/assets/javascripts/environments/components/environment_item.js b/app/assets/javascripts/environments/components/environment_item.js index 93919d41c60..9d753b4f808 100644 --- a/app/assets/javascripts/environments/components/environment_item.js +++ b/app/assets/javascripts/environments/components/environment_item.js @@ -141,6 +141,7 @@ export default { const parsedAction = { name: gl.text.humanize(action.name), play_path: action.play_path, + playable: action.playable, }; return parsedAction; }); diff --git a/app/serializers/build_action_entity.rb b/app/serializers/build_action_entity.rb index 184f5fd4b52..184b4b7a681 100644 --- a/app/serializers/build_action_entity.rb +++ b/app/serializers/build_action_entity.rb @@ -11,4 +11,6 @@ class BuildActionEntity < Grape::Entity build.project, build) end + + expose :playable?, as: :playable end diff --git a/app/serializers/build_entity.rb b/app/serializers/build_entity.rb index 5bcbe285052..2c116102888 100644 --- a/app/serializers/build_entity.rb +++ b/app/serializers/build_entity.rb @@ -16,6 +16,7 @@ class BuildEntity < Grape::Entity path_to(:play_namespace_project_build, build) end + expose :playable?, as: :playable expose :created_at expose :updated_at diff --git a/spec/serializers/build_action_entity_spec.rb b/spec/serializers/build_action_entity_spec.rb index 0f7be8b2c39..54ac17447b1 100644 --- a/spec/serializers/build_action_entity_spec.rb +++ b/spec/serializers/build_action_entity_spec.rb @@ -17,5 +17,9 @@ describe BuildActionEntity do it 'contains path to the action play' do expect(subject[:path]).to include "builds/#{build.id}/play" end + + it 'contains whether it is playable' do + expect(subject[:playable]).to eq build.playable? + end end end diff --git a/spec/serializers/build_entity_spec.rb b/spec/serializers/build_entity_spec.rb index 60c9642ee2c..eed957fa5ef 100644 --- a/spec/serializers/build_entity_spec.rb +++ b/spec/serializers/build_entity_spec.rb @@ -18,6 +18,10 @@ describe BuildEntity do expect(subject).not_to include(/variables/) end + it 'contains whether it is playable' do + expect(subject[:playable]).to eq build.playable? + end + it 'contains timestamps' do expect(subject).to include(:created_at, :updated_at) end From 53d332d3c73f8a883fa54d8eaaf91f92da73c33f Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 21 Mar 2017 13:39:57 +0100 Subject: [PATCH 012/278] Add configured container registry key to .gitignore --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 0b602d613c7..5e9f19d8319 100644 --- a/.gitignore +++ b/.gitignore @@ -30,6 +30,7 @@ eslint-report.html /config/unicorn.rb /config/secrets.yml /config/sidekiq.yml +/config/registry.key /coverage/* /coverage-javascript/ /db/*.sqlite3 From c64d36306cafac463f20d49e750f397a9b32960b Mon Sep 17 00:00:00 2001 From: Andre Guedes Date: Tue, 21 Mar 2017 10:35:02 -0300 Subject: [PATCH 013/278] Makes ContainerImages Routable Conflicts: db/schema.rb --- app/models/container_image.rb | 14 ++++++++++++-- .../container_registry_authentication_service.rb | 2 +- .../projects/container_registry/_image.html.haml | 3 +-- .../projects/container_registry/index.html.haml | 3 +-- .../20161031013926_create_container_image.rb | 1 + db/schema.rb | 3 ++- lib/api/registry_events.rb | 2 +- 7 files changed, 19 insertions(+), 9 deletions(-) diff --git a/app/models/container_image.rb b/app/models/container_image.rb index 583cb977910..a362ea3adbc 100644 --- a/app/models/container_image.rb +++ b/app/models/container_image.rb @@ -1,4 +1,6 @@ class ContainerImage < ActiveRecord::Base + include Routable + belongs_to :project delegate :container_registry, :container_registry_allowed_paths, @@ -17,10 +19,18 @@ class ContainerImage < ActiveRecord::Base client.update_token(token) end - def path - [container_registry.path, name_with_namespace].compact.join('/') + def parent + project end + def parent_changed? + project_id_changed? + end + + # def path + # [container_registry.path, name_with_namespace].compact.join('/') + # end + def name_with_namespace [container_registry_path_with_namespace, name].reject(&:blank?).join('/') end diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb index 08fe6e3293e..a3c8d77bf09 100644 --- a/app/services/auth/container_registry_authentication_service.rb +++ b/app/services/auth/container_registry_authentication_service.rb @@ -66,7 +66,7 @@ module Auth # per image authentication. # Removes only last occurence in light # of future nested groups - namespace, _ = ContainerImage::split_namespace(name) + namespace, a = ContainerImage::split_namespace(name) requested_project = Project.find_by_full_path(namespace) return unless requested_project diff --git a/app/views/projects/container_registry/_image.html.haml b/app/views/projects/container_registry/_image.html.haml index 72f2103b862..4fd642a56c9 100644 --- a/app/views/projects/container_registry/_image.html.haml +++ b/app/views/projects/container_registry/_image.html.haml @@ -31,5 +31,4 @@ - if can?(current_user, :update_container_image, @project) %th - - image.tags.each do |tag| - = render 'tag', tag: tag + = render partial: 'tag', collection: image.tags diff --git a/app/views/projects/container_registry/index.html.haml b/app/views/projects/container_registry/index.html.haml index 5508a3de396..1b5d000e801 100644 --- a/app/views/projects/container_registry/index.html.haml +++ b/app/views/projects/container_registry/index.html.haml @@ -23,5 +23,4 @@ .nothing-here-block No container images in Container Registry for this project. - else - - @images.each do |image| - = render 'image', image: image + = render partial: 'image', collection: @images diff --git a/db/migrate/20161031013926_create_container_image.rb b/db/migrate/20161031013926_create_container_image.rb index 884c78880eb..06c409857da 100644 --- a/db/migrate/20161031013926_create_container_image.rb +++ b/db/migrate/20161031013926_create_container_image.rb @@ -11,6 +11,7 @@ class CreateContainerImage < ActiveRecord::Migration create_table :container_images do |t| t.integer :project_id t.string :name + t.string :path end end end diff --git a/db/schema.rb b/db/schema.rb index db57fb0a548..a1fc5dc1f58 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -108,7 +108,6 @@ ActiveRecord::Schema.define(version: 20170315194013) do t.string "sidekiq_throttling_queues" t.decimal "sidekiq_throttling_factor" t.boolean "html_emails_enabled", default: true - t.string "container_registry_access_token" t.string "plantuml_url" t.boolean "plantuml_enabled" t.integer "terminal_max_session_time", default: 0, null: false @@ -117,6 +116,7 @@ ActiveRecord::Schema.define(version: 20170315194013) do t.integer "unique_ips_limit_per_user" t.integer "unique_ips_limit_time_window" t.boolean "unique_ips_limit_enabled", default: false, null: false + t.string "container_registry_access_token" end create_table "audit_events", force: :cascade do |t| @@ -327,6 +327,7 @@ ActiveRecord::Schema.define(version: 20170315194013) do create_table "container_images", force: :cascade do |t| t.integer "project_id" t.string "name" + t.string "path" end create_table "deploy_keys_projects", force: :cascade do |t| diff --git a/lib/api/registry_events.rb b/lib/api/registry_events.rb index fc6fc0b97e0..8c53e0fcfc0 100644 --- a/lib/api/registry_events.rb +++ b/lib/api/registry_events.rb @@ -44,7 +44,7 @@ module API project = Project::find_by_full_path(namespace) if project - container_image = project.container_images.find_or_create_by(name: container_image_name) + container_image = project.container_images.find_or_create_by(name: container_image_name, path: container_image_name) unless container_image.valid? render_api_error!({ error: "Failed to create container image!" }, 400) From 68a2fa54dedcdbe893ec811413d1703e5f6ac2dc Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 22 Mar 2017 11:08:23 +0100 Subject: [PATCH 014/278] Remove out-of-scope changes for multi-level images --- .../admin/application_settings_controller.rb | 6 -- .../admin/container_registry_controller.rb | 11 ---- app/models/application_setting.rb | 6 -- .../admin/container_registry/show.html.haml | 31 ---------- app/views/admin/dashboard/_head.html.haml | 4 -- config/routes/admin.rb | 2 - ...ry_access_token_to_application_settings.rb | 13 ---- doc/administration/container_registry.md | 18 ------ lib/api/api.rb | 1 - lib/api/helpers.rb | 10 ---- lib/api/registry_events.rb | 60 ------------------- lib/container_registry/ROADMAP.md | 7 --- 12 files changed, 169 deletions(-) delete mode 100644 app/controllers/admin/container_registry_controller.rb delete mode 100644 app/views/admin/container_registry/show.html.haml delete mode 100644 db/migrate/20161213212947_add_container_registry_access_token_to_application_settings.rb delete mode 100644 lib/api/registry_events.rb delete mode 100644 lib/container_registry/ROADMAP.md diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index 1d0bd6e0b81..8d831ffdd70 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -29,12 +29,6 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController redirect_to :back end - def reset_container_registry_token - @application_setting.reset_container_registry_access_token! - flash[:notice] = 'New container registry access token has been generated!' - redirect_to :back - end - def clear_repository_check_states RepositoryCheck::ClearWorker.perform_async diff --git a/app/controllers/admin/container_registry_controller.rb b/app/controllers/admin/container_registry_controller.rb deleted file mode 100644 index 265c032c67d..00000000000 --- a/app/controllers/admin/container_registry_controller.rb +++ /dev/null @@ -1,11 +0,0 @@ -class Admin::ContainerRegistryController < Admin::ApplicationController - def show - @access_token = container_registry_access_token - end - - private - - def container_registry_access_token - current_application_settings.container_registry_access_token - end -end diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 9d01a70c77d..671a0fe98cc 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -4,7 +4,6 @@ class ApplicationSetting < ActiveRecord::Base add_authentication_token_field :runners_registration_token add_authentication_token_field :health_check_access_token - add_authentication_token_field :container_registry_access_token CACHE_KEY = 'application_setting.last'.freeze DOMAIN_LIST_SEPARATOR = %r{\s*[,;]\s* # comma or semicolon, optionally surrounded by whitespace @@ -158,7 +157,6 @@ class ApplicationSetting < ActiveRecord::Base before_save :ensure_runners_registration_token before_save :ensure_health_check_access_token - before_save :ensure_container_registry_access_token after_commit do Rails.cache.write(CACHE_KEY, self) @@ -332,10 +330,6 @@ class ApplicationSetting < ActiveRecord::Base ensure_health_check_access_token! end - def container_registry_access_token - ensure_container_registry_access_token! - end - def sidekiq_throttling_enabled? return false unless sidekiq_throttling_column_exists? diff --git a/app/views/admin/container_registry/show.html.haml b/app/views/admin/container_registry/show.html.haml deleted file mode 100644 index ffaa7736d65..00000000000 --- a/app/views/admin/container_registry/show.html.haml +++ /dev/null @@ -1,31 +0,0 @@ -- @no_container = true -= render "admin/dashboard/head" - -%div{ class: container_class } - - %p.prepend-top-default - %span - To properly configure the Container Registry you should add the following - access token to the Docker Registry config.yml as follows: - %pre - %code - :plain - notifications: - endpoints: - - ... - headers: - X-Registry-Token: [#{@access_token}] - %br - Access token is - %code{ id: 'registry-token' }= @access_token - - .bs-callout.clearfix - .pull-left - %p - You can reset container registry access token by pressing the button below. - %p - = button_to reset_container_registry_token_admin_application_settings_path, - method: :put, class: 'btn btn-default', - data: { confirm: 'Are you sure you want to reset container registry token?' } do - = icon('refresh') - Reset container registry access token diff --git a/app/views/admin/dashboard/_head.html.haml b/app/views/admin/dashboard/_head.html.haml index dbd039547fa..7893c1dee97 100644 --- a/app/views/admin/dashboard/_head.html.haml +++ b/app/views/admin/dashboard/_head.html.haml @@ -27,7 +27,3 @@ = link_to admin_runners_path, title: 'Runners' do %span Runners - = nav_link path: 'container_registry#show' do - = link_to admin_container_registry_path, title: 'Registry' do - %span - Registry diff --git a/config/routes/admin.rb b/config/routes/admin.rb index fcbe2e2c435..486ce3c5c87 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -63,7 +63,6 @@ namespace :admin do resource :background_jobs, controller: 'background_jobs', only: [:show] resource :system_info, controller: 'system_info', only: [:show] resources :requests_profiles, only: [:index, :show], param: :name, constraints: { name: /.+\.html/ } - resource :container_registry, controller: 'container_registry', only: [:show] resources :projects, only: [:index] @@ -94,7 +93,6 @@ namespace :admin do resources :services, only: [:index, :edit, :update] put :reset_runners_token put :reset_health_check_token - put :reset_container_registry_token put :clear_repository_check_states end diff --git a/db/migrate/20161213212947_add_container_registry_access_token_to_application_settings.rb b/db/migrate/20161213212947_add_container_registry_access_token_to_application_settings.rb deleted file mode 100644 index 23d87cc6d0a..00000000000 --- a/db/migrate/20161213212947_add_container_registry_access_token_to_application_settings.rb +++ /dev/null @@ -1,13 +0,0 @@ -# See http://doc.gitlab.com/ce/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - -class AddContainerRegistryAccessTokenToApplicationSettings < ActiveRecord::Migration - include Gitlab::Database::MigrationHelpers - - # Set this constant to true if this migration requires downtime. - DOWNTIME = false - - def change - add_column :application_settings, :container_registry_access_token, :string - end -end diff --git a/doc/administration/container_registry.md b/doc/administration/container_registry.md index dc4e57f25fb..f707039827b 100644 --- a/doc/administration/container_registry.md +++ b/doc/administration/container_registry.md @@ -87,23 +87,6 @@ auth: rootcertbundle: /root/certs/certbundle ``` -Also a notification endpoint must be configured with the token from -Admin Area -> Overview -> Registry (`/admin/container_registry`) like in the following sample: - -``` -notifications: - endpoints: - - name: listener - url: https://gitlab.example.com/api/v3/registry_events - headers: - X-Registry-Token: [57Cx95fc2zHFh93VTiGD] - timeout: 500ms - threshold: 5 - backoff: 1s -``` - -Check the [Registry endpoint configuration][registry-endpoint] for details. - ## Container Registry domain configuration There are two ways you can configure the Registry's external domain. @@ -600,7 +583,6 @@ notifications: [storage-config]: https://docs.docker.com/registry/configuration/#storage [registry-http-config]: https://docs.docker.com/registry/configuration/#http [registry-auth]: https://docs.docker.com/registry/configuration/#auth -[registry-endpoint]: https://docs.docker.com/registry/notifications/#/configuration [token-config]: https://docs.docker.com/registry/configuration/#token [8-8-docs]: https://gitlab.com/gitlab-org/gitlab-ce/blob/8-8-stable/doc/administration/container_registry.md [registry-ssl]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/lib/support/nginx/registry-ssl diff --git a/lib/api/api.rb b/lib/api/api.rb index 7c7bfada7d0..1bf20f76ad6 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -104,7 +104,6 @@ module API mount ::API::Namespaces mount ::API::Notes mount ::API::NotificationSettings - mount ::API::RegistryEvents mount ::API::Pipelines mount ::API::ProjectHooks mount ::API::Projects diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 3c173b544aa..bd22b82476b 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -111,16 +111,6 @@ module API end end - def authenticate_container_registry_access_token! - token = request.headers['X-Registry-Token'] - unless token.present? && ActiveSupport::SecurityUtils.variable_size_secure_compare( - token, - current_application_settings.container_registry_access_token - ) - unauthorized! - end - end - def authenticated_as_admin! authenticate! forbidden! unless current_user.is_admin? diff --git a/lib/api/registry_events.rb b/lib/api/registry_events.rb deleted file mode 100644 index 8c53e0fcfc0..00000000000 --- a/lib/api/registry_events.rb +++ /dev/null @@ -1,60 +0,0 @@ -module API - # RegistryEvents API - class RegistryEvents < Grape::API - before { authenticate_container_registry_access_token! } - - content_type :json, 'application/vnd.docker.distribution.events.v1+json' - - params do - requires :events, type: Array, desc: 'The ID of a project' do - requires :id, type: String, desc: 'The ID of the event' - requires :timestamp, type: String, desc: 'Timestamp of the event' - requires :action, type: String, desc: 'Action performed by event' - requires :target, type: Hash, desc: 'Target of the event' do - optional :mediaType, type: String, desc: 'Media type of the target' - optional :size, type: Integer, desc: 'Size in bytes of the target' - requires :digest, type: String, desc: 'Digest of the target' - requires :repository, type: String, desc: 'Repository of target' - optional :url, type: String, desc: 'Url of the target' - optional :tag, type: String, desc: 'Tag of the target' - end - requires :request, type: Hash, desc: 'Request of the event' do - requires :id, type: String, desc: 'The ID of the request' - optional :addr, type: String, desc: 'IP Address of the request client' - optional :host, type: String, desc: 'Hostname of the registry instance' - requires :method, type: String, desc: 'Request method' - requires :useragent, type: String, desc: 'UserAgent header of the request' - end - requires :actor, type: Hash, desc: 'Actor that initiated the event' do - optional :name, type: String, desc: 'Actor name' - end - requires :source, type: Hash, desc: 'Source of the event' do - optional :addr, type: String, desc: 'Hostname of source registry node' - optional :instanceID, type: String, desc: 'Source registry node instanceID' - end - end - end - resource :registry_events do - post do - params['events'].each do |event| - repository = event['target']['repository'] - - if event['action'] == 'push' && !!event['target']['tag'] - namespace, container_image_name = ContainerImage::split_namespace(repository) - project = Project::find_by_full_path(namespace) - - if project - container_image = project.container_images.find_or_create_by(name: container_image_name, path: container_image_name) - - unless container_image.valid? - render_api_error!({ error: "Failed to create container image!" }, 400) - end - else - not_found!('Project') - end - end - end - end - end - end -end diff --git a/lib/container_registry/ROADMAP.md b/lib/container_registry/ROADMAP.md deleted file mode 100644 index e0a20776404..00000000000 --- a/lib/container_registry/ROADMAP.md +++ /dev/null @@ -1,7 +0,0 @@ -## Road map - -### Initial thoughts - -- Determine if image names will be persisted or fetched from API -- If persisted, how to update the stored names upon modification -- If fetched, how to fetch only images of a given project From 29c34267556198ee3dbbe2f13bc81708f5e60f10 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 22 Mar 2017 11:41:05 +0100 Subject: [PATCH 015/278] Move container images migration to the present time --- ...iner_image.rb => 20170322013926_create_container_image.rb} | 4 ---- 1 file changed, 4 deletions(-) rename db/migrate/{20161031013926_create_container_image.rb => 20170322013926_create_container_image.rb} (56%) diff --git a/db/migrate/20161031013926_create_container_image.rb b/db/migrate/20170322013926_create_container_image.rb similarity index 56% rename from db/migrate/20161031013926_create_container_image.rb rename to db/migrate/20170322013926_create_container_image.rb index 06c409857da..c494f2a56c7 100644 --- a/db/migrate/20161031013926_create_container_image.rb +++ b/db/migrate/20170322013926_create_container_image.rb @@ -1,10 +1,6 @@ -# See http://doc.gitlab.com/ce/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - class CreateContainerImage < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers - # Set this constant to true if this migration requires downtime. DOWNTIME = false def change From 95e2c0196b7e492f8c03c6cfeb6b37e97f75813e Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 22 Mar 2017 12:28:23 +0100 Subject: [PATCH 016/278] Clean code related to accessing registry from project [ci skip] --- app/models/container_image.rb | 28 ++++------------------------ app/models/project.rb | 16 +++++----------- 2 files changed, 9 insertions(+), 35 deletions(-) diff --git a/app/models/container_image.rb b/app/models/container_image.rb index a362ea3adbc..411617ccd71 100644 --- a/app/models/container_image.rb +++ b/app/models/container_image.rb @@ -3,36 +3,16 @@ class ContainerImage < ActiveRecord::Base belongs_to :project - delegate :container_registry, :container_registry_allowed_paths, - :container_registry_path_with_namespace, to: :project - + delegate :container_registry, to: :project delegate :client, to: :container_registry validates :manifest, presence: true before_destroy :delete_tags - before_validation :update_token, on: :create - def update_token - paths = container_registry_allowed_paths << name_with_namespace - token = Auth::ContainerRegistryAuthenticationService.full_access_token(paths) - client.update_token(token) - end - - def parent - project - end - - def parent_changed? - project_id_changed? - end - - # def path - # [container_registry.path, name_with_namespace].compact.join('/') - # end - - def name_with_namespace - [container_registry_path_with_namespace, name].reject(&:blank?).join('/') + def registry + # TODO, container registry with image access level + token = Auth::ContainerRegistryAuthenticationService.image_token(self) end def tag(tag) diff --git a/app/models/project.rb b/app/models/project.rb index 928965643a0..4aa9c6bb2f2 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -405,29 +405,23 @@ class Project < ActiveRecord::Base @repository ||= Repository.new(path_with_namespace, self) end - def container_registry_path_with_namespace - path_with_namespace.downcase - end - - def container_registry_allowed_paths - @container_registry_allowed_paths ||= [container_registry_path_with_namespace] + - container_images.map { |i| i.name_with_namespace } - end - def container_registry return unless Gitlab.config.registry.enabled @container_registry ||= begin - token = Auth::ContainerRegistryAuthenticationService.full_access_token(container_registry_allowed_paths) + token = Auth::ContainerRegistryAuthenticationService.full_access_token(project) + url = Gitlab.config.registry.api_url host_port = Gitlab.config.registry.host_port + # TODO, move configuration vars into ContainerRegistry::Registry, clean + # this method up afterwards ContainerRegistry::Registry.new(url, token: token, path: host_port) end end def container_registry_url if Gitlab.config.registry.enabled - "#{Gitlab.config.registry.host_port}/#{container_registry_path_with_namespace}" + "#{Gitlab.config.registry.host_port}/#{path_with_namespace.downcase}" end end From 896b13b929369c02f72fa881eda24ca4a6a0d900 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 22 Mar 2017 16:07:27 +0100 Subject: [PATCH 017/278] Refactor splitting container image full path [ci skip] --- app/models/container_image.rb | 17 +++++++---------- ...container_registry_authentication_service.rb | 7 +------ 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/app/models/container_image.rb b/app/models/container_image.rb index 411617ccd71..6e9a060d7a8 100644 --- a/app/models/container_image.rb +++ b/app/models/container_image.rb @@ -1,6 +1,4 @@ class ContainerImage < ActiveRecord::Base - include Routable - belongs_to :project delegate :container_registry, to: :project @@ -45,14 +43,13 @@ class ContainerImage < ActiveRecord::Base end end - # rubocop:disable RedundantReturn + def self.from_path(full_path) + return unless full_path.include?('/') - def self.split_namespace(full_path) - image_name = full_path.split('/').last - namespace = full_path.gsub(/(.*)(#{Regexp.escape('/' + image_name)})/, '\1') - if namespace.count('/') < 1 - namespace, image_name = full_path, "" - end - return namespace, image_name + path = full_path[0...full_path.rindex('/')] + name = full_path[full_path.rindex('/')+1..-1] + project = Project.find_by_full_path(path) + + self.new(name: name, path: path, project: project) end end diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb index a3c8d77bf09..7e412040c7c 100644 --- a/app/services/auth/container_registry_authentication_service.rb +++ b/app/services/auth/container_registry_authentication_service.rb @@ -62,12 +62,7 @@ module Auth end def process_repository_access(type, name, actions) - # Strips image name due to lack of - # per image authentication. - # Removes only last occurence in light - # of future nested groups - namespace, a = ContainerImage::split_namespace(name) - requested_project = Project.find_by_full_path(namespace) + requested_project = ContainerImage.from_path(name).project return unless requested_project actions = actions.select do |action| From 4005eb643657e5ee8b1f328e36a3204253e3acf4 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 23 Mar 2017 11:41:16 +0100 Subject: [PATCH 018/278] Fix communication between GitLab and Container Registry --- app/models/container_image.rb | 19 +++++++++++++------ ...ntainer_registry_authentication_service.rb | 17 +++++++++-------- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/app/models/container_image.rb b/app/models/container_image.rb index 6e9a060d7a8..434302159b0 100644 --- a/app/models/container_image.rb +++ b/app/models/container_image.rb @@ -43,13 +43,20 @@ class ContainerImage < ActiveRecord::Base end end - def self.from_path(full_path) - return unless full_path.include?('/') + def self.project_from_path(image_path) + return unless image_path.include?('/') - path = full_path[0...full_path.rindex('/')] - name = full_path[full_path.rindex('/')+1..-1] - project = Project.find_by_full_path(path) + ## + # Projects are always located inside a namespace, so we can remove + # the last node, and see if project with that path exists. + # + truncated_path = image_path.slice(0...image_path.rindex('/')) - self.new(name: name, path: path, project: project) + ## + # We still make it possible to search projects by a full image path + # in order to maintain backwards compatibility. + # + Project.find_by_full_path(truncated_path) || + Project.find_by_full_path(image_path) end end diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb index 7e412040c7c..2205b0897e2 100644 --- a/app/services/auth/container_registry_authentication_service.rb +++ b/app/services/auth/container_registry_authentication_service.rb @@ -38,13 +38,13 @@ module Auth private def authorized_token(*accesses) - token = JSONWebToken::RSAToken.new(registry.key) - token.issuer = registry.issuer - token.audience = params[:service] - token.subject = current_user.try(:username) - token.expire_time = self.class.token_expire_at - token[:access] = accesses.compact - token + JSONWebToken::RSAToken.new(registry.key).tap do |token| + token.issuer = registry.issuer + token.audience = params[:service] + token.subject = current_user.try(:username) + token.expire_time = self.class.token_expire_at + token[:access] = accesses.compact + end end def scope @@ -62,7 +62,8 @@ module Auth end def process_repository_access(type, name, actions) - requested_project = ContainerImage.from_path(name).project + requested_project = ContainerImage.project_from_path(name) + return unless requested_project actions = actions.select do |action| From bd8c8df6ed8dd7321608ce652e23d86ef3bd6899 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 23 Mar 2017 12:01:12 +0100 Subject: [PATCH 019/278] Fix database schema --- db/schema.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index a1fc5dc1f58..b5bd6fc3121 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170315194013) do +ActiveRecord::Schema.define(version: 20170322013926) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -111,12 +111,10 @@ ActiveRecord::Schema.define(version: 20170315194013) do t.string "plantuml_url" t.boolean "plantuml_enabled" t.integer "terminal_max_session_time", default: 0, null: false - t.integer "max_pages_size", default: 100, null: false t.string "default_artifacts_expire_in", default: "0", null: false t.integer "unique_ips_limit_per_user" t.integer "unique_ips_limit_time_window" t.boolean "unique_ips_limit_enabled", default: false, null: false - t.string "container_registry_access_token" end create_table "audit_events", force: :cascade do |t| @@ -993,6 +991,7 @@ ActiveRecord::Schema.define(version: 20170315194013) do end add_index "routes", ["path"], name: "index_routes_on_path", unique: true, using: :btree + add_index "routes", ["path"], name: "index_routes_on_path_text_pattern_ops", using: :btree, opclasses: {"path"=>"varchar_pattern_ops"} add_index "routes", ["source_type", "source_id"], name: "index_routes_on_source_type_and_source_id", unique: true, using: :btree create_table "sent_notifications", force: :cascade do |t| From 01d159b409d8b24d36204979a73de249843d71bf Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 23 Mar 2017 14:00:41 +0100 Subject: [PATCH 020/278] Rename container image model to container repository --- ...{container_image.rb => container_repository.rb} | 14 +++++++------- .../container_registry_authentication_service.rb | 2 +- ... 20170322013926_create_container_repository.rb} | 5 ++--- db/schema.rb | 10 ++++------ 4 files changed, 14 insertions(+), 17 deletions(-) rename app/models/{container_image.rb => container_repository.rb} (75%) rename db/migrate/{20170322013926_create_container_image.rb => 20170322013926_create_container_repository.rb} (55%) diff --git a/app/models/container_image.rb b/app/models/container_repository.rb similarity index 75% rename from app/models/container_image.rb rename to app/models/container_repository.rb index 434302159b0..2e78fc148b4 100644 --- a/app/models/container_image.rb +++ b/app/models/container_repository.rb @@ -1,4 +1,4 @@ -class ContainerImage < ActiveRecord::Base +class ContainerRepository < ActiveRecord::Base belongs_to :project delegate :container_registry, to: :project @@ -18,7 +18,7 @@ class ContainerImage < ActiveRecord::Base end def manifest - @manifest ||= client.repository_tags(name_with_namespace) + @manifest ||= client.repository_tags(self.path) end def tags @@ -39,24 +39,24 @@ class ContainerImage < ActiveRecord::Base digests = tags.map {|tag| tag.digest }.to_set digests.all? do |digest| - client.delete_repository_tag(name_with_namespace, digest) + client.delete_repository_tag(self.path, digest) end end - def self.project_from_path(image_path) - return unless image_path.include?('/') + def self.project_from_path(repository_path) + return unless repository_path.include?('/') ## # Projects are always located inside a namespace, so we can remove # the last node, and see if project with that path exists. # - truncated_path = image_path.slice(0...image_path.rindex('/')) + truncated_path = repository_path.slice(0...repository_path.rindex('/')) ## # We still make it possible to search projects by a full image path # in order to maintain backwards compatibility. # Project.find_by_full_path(truncated_path) || - Project.find_by_full_path(image_path) + Project.find_by_full_path(repository_path) end end diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb index 2205b0897e2..3d151c6a357 100644 --- a/app/services/auth/container_registry_authentication_service.rb +++ b/app/services/auth/container_registry_authentication_service.rb @@ -62,7 +62,7 @@ module Auth end def process_repository_access(type, name, actions) - requested_project = ContainerImage.project_from_path(name) + requested_project = ContainerRepository.project_from_path(name) return unless requested_project diff --git a/db/migrate/20170322013926_create_container_image.rb b/db/migrate/20170322013926_create_container_repository.rb similarity index 55% rename from db/migrate/20170322013926_create_container_image.rb rename to db/migrate/20170322013926_create_container_repository.rb index c494f2a56c7..0235fd7e096 100644 --- a/db/migrate/20170322013926_create_container_image.rb +++ b/db/migrate/20170322013926_create_container_repository.rb @@ -1,12 +1,11 @@ -class CreateContainerImage < ActiveRecord::Migration +class CreateContainerRepository < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers DOWNTIME = false def change - create_table :container_images do |t| + create_table :container_repositories do |t| t.integer :project_id - t.string :name t.string :path end end diff --git a/db/schema.rb b/db/schema.rb index b5bd6fc3121..be7604a14d5 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -61,7 +61,6 @@ ActiveRecord::Schema.define(version: 20170322013926) do t.boolean "shared_runners_enabled", default: true, null: false t.integer "max_artifacts_size", default: 100, null: false t.string "runners_registration_token" - t.integer "max_pages_size", default: 100, null: false t.boolean "require_two_factor_authentication", default: false t.integer "two_factor_grace_period", default: 48 t.boolean "metrics_enabled", default: false @@ -111,6 +110,7 @@ ActiveRecord::Schema.define(version: 20170322013926) do t.string "plantuml_url" t.boolean "plantuml_enabled" t.integer "terminal_max_session_time", default: 0, null: false + t.integer "max_pages_size", default: 100, null: false t.string "default_artifacts_expire_in", default: "0", null: false t.integer "unique_ips_limit_per_user" t.integer "unique_ips_limit_time_window" @@ -322,9 +322,8 @@ ActiveRecord::Schema.define(version: 20170322013926) do add_index "ci_variables", ["project_id"], name: "index_ci_variables_on_project_id", using: :btree - create_table "container_images", force: :cascade do |t| + create_table "container_repositories", force: :cascade do |t| t.integer "project_id" - t.string "name" t.string "path" end @@ -694,8 +693,8 @@ ActiveRecord::Schema.define(version: 20170322013926) do t.integer "visibility_level", default: 20, null: false t.boolean "request_access_enabled", default: false, null: false t.datetime "deleted_at" - t.text "description_html" t.boolean "lfs_enabled" + t.text "description_html" t.integer "parent_id" end @@ -991,7 +990,6 @@ ActiveRecord::Schema.define(version: 20170322013926) do end add_index "routes", ["path"], name: "index_routes_on_path", unique: true, using: :btree - add_index "routes", ["path"], name: "index_routes_on_path_text_pattern_ops", using: :btree, opclasses: {"path"=>"varchar_pattern_ops"} add_index "routes", ["source_type", "source_id"], name: "index_routes_on_source_type_and_source_id", unique: true, using: :btree create_table "sent_notifications", force: :cascade do |t| @@ -1238,8 +1236,8 @@ ActiveRecord::Schema.define(version: 20170322013926) do t.datetime "otp_grace_period_started_at" t.boolean "ldap_email", default: false, null: false t.boolean "external", default: false - t.string "incoming_email_token" t.string "organization" + t.string "incoming_email_token" t.boolean "authorized_projects_populated" t.boolean "ghost" end From ea16ea5bfcb78f66c6bb37e470d387bf1ac26c9f Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 23 Mar 2017 14:09:01 +0100 Subject: [PATCH 021/278] Identify container repository by a single name --- db/migrate/20170322013926_create_container_repository.rb | 2 +- db/schema.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/db/migrate/20170322013926_create_container_repository.rb b/db/migrate/20170322013926_create_container_repository.rb index 0235fd7e096..87a1523724c 100644 --- a/db/migrate/20170322013926_create_container_repository.rb +++ b/db/migrate/20170322013926_create_container_repository.rb @@ -6,7 +6,7 @@ class CreateContainerRepository < ActiveRecord::Migration def change create_table :container_repositories do |t| t.integer :project_id - t.string :path + t.string :name end end end diff --git a/db/schema.rb b/db/schema.rb index be7604a14d5..5d9b219ebea 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -324,7 +324,7 @@ ActiveRecord::Schema.define(version: 20170322013926) do create_table "container_repositories", force: :cascade do |t| t.integer "project_id" - t.string "path" + t.string "name" end create_table "deploy_keys_projects", force: :cascade do |t| From f451173a191474be681d208eceb6a0148ba2c0d0 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 23 Mar 2017 14:37:17 +0100 Subject: [PATCH 022/278] Fix specs for container repository model class --- app/models/container_repository.rb | 21 ++++-- spec/factories/container_images.rb | 22 ------ spec/factories/container_repositories.rb | 22 ++++++ spec/models/container_image_spec.rb | 73 ------------------ spec/models/container_repository_spec.rb | 96 ++++++++++++++++++++++++ 5 files changed, 132 insertions(+), 102 deletions(-) delete mode 100644 spec/factories/container_images.rb create mode 100644 spec/factories/container_repositories.rb delete mode 100644 spec/models/container_image_spec.rb create mode 100644 spec/models/container_repository_spec.rb diff --git a/app/models/container_repository.rb b/app/models/container_repository.rb index 2e78fc148b4..b3a8ec691de 100644 --- a/app/models/container_repository.rb +++ b/app/models/container_repository.rb @@ -1,16 +1,23 @@ class ContainerRepository < ActiveRecord::Base belongs_to :project - - delegate :container_registry, to: :project - delegate :client, to: :container_registry - + delegate :client, to: :registry validates :manifest, presence: true - + validates :name, presence: true before_destroy :delete_tags def registry - # TODO, container registry with image access level - token = Auth::ContainerRegistryAuthenticationService.image_token(self) + @registry ||= begin + token = Auth::ContainerRegistryAuthenticationService.full_access_token(path) + + url = Gitlab.config.registry.api_url + host_port = Gitlab.config.registry.host_port + + ContainerRegistry::Registry.new(url, token: token, path: host_port) + end + end + + def path + @path ||= "#{project.full_path}/#{name}" end def tag(tag) diff --git a/spec/factories/container_images.rb b/spec/factories/container_images.rb deleted file mode 100644 index 3693865101d..00000000000 --- a/spec/factories/container_images.rb +++ /dev/null @@ -1,22 +0,0 @@ -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(Auth::ContainerRegistryAuthenticationService).to receive(:full_access_token).and_return('token') - allow(image.client).to receive(:repository_tags).and_return({ - name: image.name_with_namespace, - tags: evaluator.tags - }) - end - end - end -end diff --git a/spec/factories/container_repositories.rb b/spec/factories/container_repositories.rb new file mode 100644 index 00000000000..fbf6bf62dfd --- /dev/null +++ b/spec/factories/container_repositories.rb @@ -0,0 +1,22 @@ +FactoryGirl.define do + factory :container_repository do + name "test_container_image" + project + + transient do + tags ['tag'] + end + + after(:build) do |image, evaluator| + # if evaluator.tags.to_a.any? + # allow(Gitlab.config.registry).to receive(:enabled).and_return(true) + # allow(Auth::ContainerRegistryAuthenticationService) + # .to receive(:full_access_token).and_return('token') + # allow(image.client).to receive(:repository_tags).and_return({ + # name: image.name_with_namespace, + # tags: evaluator.tags + # }) + # end + end + end +end diff --git a/spec/models/container_image_spec.rb b/spec/models/container_image_spec.rb deleted file mode 100644 index e0bea737f59..00000000000 --- a/spec/models/container_image_spec.rb +++ /dev/null @@ -1,73 +0,0 @@ -require 'spec_helper' - -describe ContainerImage do - let(:group) { create(:group, name: 'group') } - let(:project) { create(:project, path: 'test', group: group) } - let(:example_host) { 'example.com' } - let(:registry_url) { 'http://' + example_host } - let(:container_image) { create(:container_image, name: '', project: project, stubbed: false) } - - before do - stub_container_registry_config(enabled: true, api_url: registry_url, host_port: example_host) - stub_request(:get, 'http://example.com/v2/group/test/tags/list'). - with(headers: { 'Accept' => 'application/vnd.docker.distribution.manifest.v2+json' }). - to_return( - status: 200, - body: JSON.dump(tags: ['test']), - headers: { 'Content-Type' => 'application/json' }) - end - - it { expect(container_image).to respond_to(:project) } - it { expect(container_image).to delegate_method(:container_registry).to(:project) } - it { expect(container_image).to delegate_method(:client).to(:container_registry) } - it { expect(container_image.tag('test')).not_to be_nil } - - context '#path' do - subject { container_image.path } - - it { is_expected.to eq('example.com/group/test') } - end - - context 'manifest processing' do - context '#manifest' do - subject { container_image.manifest } - - it { is_expected.not_to be_nil } - end - - context '#valid?' do - subject { container_image.valid? } - - it { is_expected.to be_truthy } - end - - context '#tags' do - subject { container_image.tags } - - it { is_expected.not_to be_empty } - end - end - - context '#delete_tags' do - let(:tag) { ContainerRegistry::Tag.new(container_image, 'tag') } - - before do - expect(container_image).to receive(:tags).twice.and_return([tag]) - expect(tag).to receive(:digest).and_return('sha256:4c8e63ca4cb663ce6c688cb06f1c3672a172b088dac5b6d7ad7d49cd620d85cf') - end - - subject { container_image.delete_tags } - - context 'succeeds' do - before { expect(container_image.client).to receive(:delete_repository_tag).and_return(true) } - - it { is_expected.to be_truthy } - end - - context 'any fails' do - before { expect(container_image.client).to receive(:delete_repository_tag).and_return(false) } - - it { is_expected.to be_falsey } - end - end -end diff --git a/spec/models/container_repository_spec.rb b/spec/models/container_repository_spec.rb new file mode 100644 index 00000000000..3997c4ca682 --- /dev/null +++ b/spec/models/container_repository_spec.rb @@ -0,0 +1,96 @@ +require 'spec_helper' + +describe ContainerRepository do + let(:group) { create(:group, name: 'group') } + let(:project) { create(:project, path: 'test', group: group) } + + let(:container_repository) do + create(:container_repository, name: 'my_image', project: project) + end + + before do + stub_container_registry_config(enabled: true, + api_url: 'http://registry.gitlab', + host_port: 'registry.gitlab') + + stub_request(:get, 'http://registry.gitlab/v2/group/test/my_image/tags/list') + .with(headers: { + 'Accept' => 'application/vnd.docker.distribution.manifest.v2+json' }) + .to_return( + status: 200, + body: JSON.dump(tags: ['test_tag']), + headers: { 'Content-Type' => 'application/json' }) + end + + describe 'associations' do + it 'belongs to the project' do + expect(container_repository).to belong_to(:project) + end + end + + describe '#tag' do + it 'has a test tag' do + expect(container_repository.tag('test')).not_to be_nil + end + end + + describe '#path' do + it 'returns a full path to the repository' do + expect(container_repository.path).to eq('group/test/my_image') + end + end + + describe '#manifest' do + subject { container_repository.manifest } + + it { is_expected.not_to be_nil } + end + + describe '#valid?' do + subject { container_repository.valid? } + + it { is_expected.to be_truthy } + end + + describe '#tags' do + subject { container_repository.tags } + + it { is_expected.not_to be_empty } + end + + # TODO, improve these specs + # + describe '#delete_tags' do + let(:tag) { ContainerRegistry::Tag.new(container_repository, 'tag') } + + before do + allow(container_repository).to receive(:tags).twice.and_return([tag]) + allow(tag).to receive(:digest) + .and_return('sha256:4c8e63ca4cb663ce6c688cb06f1c3672a172b088dac5b6d7ad7d49cd620d85cf') + end + + context 'when action succeeds' do + before do + allow(container_repository.client) + .to receive(:delete_repository_tag) + .and_return(true) + end + + it 'returns status that indicates success' do + expect(container_repository.delete_tags).to be_truthy + end + end + + context 'when action fails' do + before do + allow(container_repository.client) + .to receive(:delete_repository_tag) + .and_return(false) + end + + it 'returns status that indicates failure' do + expect(container_repository.delete_tags).to be_falsey + end + end + end +end From 9c9aac3d16cfbbcbe24b9b33eb7a6d4c14c24f26 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 23 Mar 2017 09:46:05 -0400 Subject: [PATCH 023/278] Prevent Trigger action buttons from wrapping --- app/assets/stylesheets/pages/settings_ci_cd.scss | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/assets/stylesheets/pages/settings_ci_cd.scss b/app/assets/stylesheets/pages/settings_ci_cd.scss index b97a29cd1a0..fe22d186af1 100644 --- a/app/assets/stylesheets/pages/settings_ci_cd.scss +++ b/app/assets/stylesheets/pages/settings_ci_cd.scss @@ -6,6 +6,8 @@ } .trigger-actions { + white-space: nowrap; + .btn { margin-left: 10px; } From 249084b48a86a99c02eefe45b507ebcdf811c20f Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 23 Mar 2017 14:48:24 +0100 Subject: [PATCH 024/278] Fix some specs using the old ContainerImage const --- spec/features/container_registry_spec.rb | 2 +- spec/services/projects/destroy_service_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/features/container_registry_spec.rb b/spec/features/container_registry_spec.rb index 862c9fbf6c0..0199d08ab63 100644 --- a/spec/features/container_registry_spec.rb +++ b/spec/features/container_registry_spec.rb @@ -39,7 +39,7 @@ describe "Container Registry" do end it do - expect_any_instance_of(ContainerImage).to receive(:delete_tags).and_return(true) + expect_any_instance_of(ContainerRepository).to receive(:delete_tags).and_return(true) click_on 'Remove image' end diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb index 270e630e70e..f91d62ebdaf 100644 --- a/spec/services/projects/destroy_service_spec.rb +++ b/spec/services/projects/destroy_service_spec.rb @@ -100,7 +100,7 @@ describe Projects::DestroyService, services: true do context 'images deletion succeeds' do it do - expect_any_instance_of(ContainerImage).to receive(:delete_tags).and_return(true) + expect_any_instance_of(ContainerRepository).to receive(:delete_tags).and_return(true) destroy_project(project, user, {}) end @@ -108,7 +108,7 @@ describe Projects::DestroyService, services: true do context 'images deletion fails' do before do - expect_any_instance_of(ContainerImage).to receive(:delete_tags).and_return(false) + expect_any_instance_of(ContainerRepository).to receive(:delete_tags).and_return(false) end subject { destroy_project(project, user, {}) } From 3e01fed5cb36962065f5d19ab6a0cef1dfc14b48 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 23 Mar 2017 14:57:33 +0100 Subject: [PATCH 025/278] Fix Rubocop offenses in container repository code --- app/models/container_repository.rb | 2 +- spec/models/container_repository_spec.rb | 9 ++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/app/models/container_repository.rb b/app/models/container_repository.rb index b3a8ec691de..2f0fd3014a8 100644 --- a/app/models/container_repository.rb +++ b/app/models/container_repository.rb @@ -64,6 +64,6 @@ class ContainerRepository < ActiveRecord::Base # in order to maintain backwards compatibility. # Project.find_by_full_path(truncated_path) || - Project.find_by_full_path(repository_path) + Project.find_by_full_path(repository_path) end end diff --git a/spec/models/container_repository_spec.rb b/spec/models/container_repository_spec.rb index 3997c4ca682..e3180e01758 100644 --- a/spec/models/container_repository_spec.rb +++ b/spec/models/container_repository_spec.rb @@ -14,12 +14,11 @@ describe ContainerRepository do host_port: 'registry.gitlab') stub_request(:get, 'http://registry.gitlab/v2/group/test/my_image/tags/list') - .with(headers: { - 'Accept' => 'application/vnd.docker.distribution.manifest.v2+json' }) + .with(headers: { 'Accept' => 'application/vnd.docker.distribution.manifest.v2+json' }) .to_return( - status: 200, - body: JSON.dump(tags: ['test_tag']), - headers: { 'Content-Type' => 'application/json' }) + status: 200, + body: JSON.dump(tags: ['test_tag']), + headers: { 'Content-Type' => 'application/json' }) end describe 'associations' do From dcd2eeb1cfb633f4a28ddd9bc79deac0e3171d3f Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 23 Mar 2017 15:54:59 +0100 Subject: [PATCH 026/278] Rename container image to repository in specs --- app/models/namespace.rb | 2 +- app/models/project.rb | 4 ++-- app/services/projects/transfer_service.rb | 2 +- spec/features/container_registry_spec.rb | 8 ++++---- .../security/project/internal_access_spec.rb | 4 ++-- .../security/project/private_access_spec.rb | 4 ++-- .../security/project/public_access_spec.rb | 4 ++-- spec/lib/container_registry/blob_spec.rb | 2 +- spec/lib/container_registry/tag_spec.rb | 2 +- spec/lib/gitlab/import_export/all_models.yml | 2 +- spec/models/namespace_spec.rb | 4 ++-- spec/models/project_spec.rb | 4 ++-- .../container_images/destroy_service_spec.rb | 16 ++++++++-------- spec/services/projects/destroy_service_spec.rb | 4 ++-- spec/services/projects/transfer_service_spec.rb | 4 ++-- 15 files changed, 33 insertions(+), 33 deletions(-) diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 4ae9d0122f2..ac03f098908 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -150,7 +150,7 @@ class Namespace < ActiveRecord::Base end def any_project_has_container_registry_images? - projects.joins(:container_images).any? + projects.joins(:container_repositories).any? end def send_update_instructions diff --git a/app/models/project.rb b/app/models/project.rb index 4aa9c6bb2f2..5c6672c95b3 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -157,7 +157,7 @@ class Project < ActiveRecord::Base has_one :import_data, dependent: :destroy, class_name: "ProjectImportData" has_one :project_feature, dependent: :destroy has_one :statistics, class_name: 'ProjectStatistics', dependent: :delete - has_many :container_images, dependent: :destroy + has_many :container_repositories, dependent: :destroy has_many :commit_statuses, dependent: :destroy has_many :pipelines, dependent: :destroy, class_name: 'Ci::Pipeline' @@ -908,7 +908,7 @@ class Project < ActiveRecord::Base expire_caches_before_rename(old_path_with_namespace) - if container_images.present? + if container_repositories.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 diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb index 6d9e7de4f24..f46bf884e37 100644 --- a/app/services/projects/transfer_service.rb +++ b/app/services/projects/transfer_service.rb @@ -36,7 +36,7 @@ module Projects raise TransferError.new("Project with same path in target namespace already exists") end - unless project.container_images.empty? + unless project.container_repositories.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 diff --git a/spec/features/container_registry_spec.rb b/spec/features/container_registry_spec.rb index 0199d08ab63..88642f72772 100644 --- a/spec/features/container_registry_spec.rb +++ b/spec/features/container_registry_spec.rb @@ -5,15 +5,15 @@ describe "Container Registry" do 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 } + let(:container_repository) { create(:container_repository) } + let(:image_name) { container_repository.name } before do login_as(:user) project.team << [@user, :developer] stub_container_registry_config(enabled: true) stub_container_registry_tags(*tags) - project.container_images << container_image unless container_image.nil? + project.container_repositories << container_repository unless container_repository.nil? allow(Auth::ContainerRegistryAuthenticationService).to receive(:full_access_token).and_return('token') end @@ -23,7 +23,7 @@ describe "Container Registry" do end context 'when no images' do - let(:container_image) { } + let(:container_repository) { } it { expect(page).to have_content('No container images in Container Registry for this project') } end diff --git a/spec/features/security/project/internal_access_spec.rb b/spec/features/security/project/internal_access_spec.rb index 350de2e5b6b..a961d8b4f69 100644 --- a/spec/features/security/project/internal_access_spec.rb +++ b/spec/features/security/project/internal_access_spec.rb @@ -443,12 +443,12 @@ describe "Internal Project Access", feature: true do end describe "GET /:project_path/container_registry" do - let(:container_image) { create(:container_image) } + let(:container_repository) { create(:container_repository) } before do stub_container_registry_tags('latest') stub_container_registry_config(enabled: true) - project.container_images << container_image + project.container_repositories << container_repository end subject { namespace_project_container_registry_index_path(project.namespace, project) } diff --git a/spec/features/security/project/private_access_spec.rb b/spec/features/security/project/private_access_spec.rb index 62364206440..b7e42e67d82 100644 --- a/spec/features/security/project/private_access_spec.rb +++ b/spec/features/security/project/private_access_spec.rb @@ -432,12 +432,12 @@ describe "Private Project Access", feature: true do end describe "GET /:project_path/container_registry" do - let(:container_image) { create(:container_image) } + let(:container_repository) { create(:container_repository) } before do stub_container_registry_tags('latest') stub_container_registry_config(enabled: true) - project.container_images << container_image + project.container_repositories << container_repository end subject { namespace_project_container_registry_index_path(project.namespace, project) } diff --git a/spec/features/security/project/public_access_spec.rb b/spec/features/security/project/public_access_spec.rb index 0e0c3140fd0..02660984b29 100644 --- a/spec/features/security/project/public_access_spec.rb +++ b/spec/features/security/project/public_access_spec.rb @@ -443,12 +443,12 @@ describe "Public Project Access", feature: true do end describe "GET /:project_path/container_registry" do - let(:container_image) { create(:container_image) } + let(:container_repository) { create(:container_repository) } before do stub_container_registry_tags('latest') stub_container_registry_config(enabled: true) - project.container_images << container_image + project.container_repositories << container_repository end subject { namespace_project_container_registry_index_path(project.namespace, project) } diff --git a/spec/lib/container_registry/blob_spec.rb b/spec/lib/container_registry/blob_spec.rb index f092449c4bd..718a61ba291 100644 --- a/spec/lib/container_registry/blob_spec.rb +++ b/spec/lib/container_registry/blob_spec.rb @@ -15,7 +15,7 @@ describe ContainerRegistry::Blob do let(:project) { create(:project, path: 'test', group: group) } let(:example_host) { 'example.com' } let(:registry_url) { 'http://' + example_host } - let(:repository) { create(:container_image, name: '', project: project) } + let(:repository) { create(:container_repository, name: '', project: project) } let(:blob) { repository.blob(config) } before do diff --git a/spec/lib/container_registry/tag_spec.rb b/spec/lib/container_registry/tag_spec.rb index cdd0fe66bc3..01153a6eca9 100644 --- a/spec/lib/container_registry/tag_spec.rb +++ b/spec/lib/container_registry/tag_spec.rb @@ -5,7 +5,7 @@ describe ContainerRegistry::Tag do let(:project) { create(:project, path: 'test', group: group) } let(:example_host) { 'example.com' } let(:registry_url) { 'http://' + example_host } - let(:repository) { create(:container_image, name: '', project: project) } + let(:repository) { create(:container_repository, name: '', project: project) } let(:tag) { repository.tag('tag') } let(:headers) { { 'Accept' => 'application/vnd.docker.distribution.manifest.v2+json' } } diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index c3ee743035a..0429636486c 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -115,7 +115,7 @@ merge_access_levels: - protected_branch push_access_levels: - protected_branch -container_images: +container_repositories: - name project: - taggings diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index d22447c602f..bc70c6f4aa2 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -149,13 +149,13 @@ describe Namespace, models: true do end context "when any project has container images" do - let(:container_image) { create(:container_image) } + let(:container_repository) { create(:container_repository) } before do stub_container_registry_config(enabled: true) stub_container_registry_tags('tag') - create(:empty_project, namespace: @namespace, container_images: [container_image]) + create(:empty_project, namespace: @namespace, container_repositories: [container_repository]) allow(@namespace).to receive(:path_was).and_return(@namespace.path) allow(@namespace).to receive(:path).and_return('new_path') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index aefbedf0b93..69b7906bb4e 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1186,12 +1186,12 @@ describe Project, models: true do end context 'container registry with images' do - let(:container_image) { create(:container_image) } + let(:container_repository) { create(:container_repository) } before do stub_container_registry_config(enabled: true) stub_container_registry_tags('tag') - project.container_images << container_image + project.container_repositories << container_repository end subject { project.rename_repo } diff --git a/spec/services/container_images/destroy_service_spec.rb b/spec/services/container_images/destroy_service_spec.rb index 5b4dbaa7934..ebc15598cce 100644 --- a/spec/services/container_images/destroy_service_spec.rb +++ b/spec/services/container_images/destroy_service_spec.rb @@ -3,13 +3,13 @@ require 'spec_helper' describe ContainerImages::DestroyService, services: true do describe '#execute' do let(:user) { create(:user) } - let(:container_image) { create(:container_image, name: '') } - let(:project) { create(:project, path: 'test', namespace: user.namespace, container_images: [container_image]) } + let(:container_repository) { create(:container_repository, name: '') } + let(:project) { create(:project, path: 'test', namespace: user.namespace, container_repositorys: [container_repository]) } let(:example_host) { 'example.com' } let(:registry_url) { 'http://' + example_host } - it { expect(container_image).to be_valid } - it { expect(project.container_images).not_to be_empty } + it { expect(container_repository).to be_valid } + it { expect(project.container_repositorys).not_to be_empty } context 'when container image has tags' do before do @@ -19,15 +19,15 @@ describe ContainerImages::DestroyService, services: true do it 'removes all tags before destroy' do service = described_class.new(project, user) - expect(container_image).to receive(:delete_tags).and_return(true) - expect { service.execute(container_image) }.to change(project.container_images, :count).by(-1) + expect(container_repository).to receive(:delete_tags).and_return(true) + expect { service.execute(container_repository) }.to change(project.container_repositorys, :count).by(-1) end it 'fails when tags are not removed' do service = described_class.new(project, user) - expect(container_image).to receive(:delete_tags).and_return(false) - expect { service.execute(container_image) }.to raise_error(ActiveRecord::RecordNotDestroyed) + expect(container_repository).to receive(:delete_tags).and_return(false) + expect { service.execute(container_repository) }.to raise_error(ActiveRecord::RecordNotDestroyed) end end end diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb index f91d62ebdaf..daad65d478a 100644 --- a/spec/services/projects/destroy_service_spec.rb +++ b/spec/services/projects/destroy_service_spec.rb @@ -90,12 +90,12 @@ describe Projects::DestroyService, services: true do end context 'container registry' do - let(:container_image) { create(:container_image) } + let(:container_repository) { create(:container_repository) } before do stub_container_registry_config(enabled: true) stub_container_registry_tags('tag') - project.container_images << container_image + project.container_repositorys << container_repository end context 'images deletion succeeds' do diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index 5e56226ff91..adf8ede5086 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -29,12 +29,12 @@ describe Projects::TransferService, services: true do end context 'disallow transfering of project with tags' do - let(:container_image) { create(:container_image) } + let(:container_repository) { create(:container_repository) } before do stub_container_registry_config(enabled: true) stub_container_registry_tags('tag') - project.container_images << container_image + project.container_repositorys << container_repository end subject { transfer_project(project, user, group) } From af42dd29a0e81d524731f4ce3ced2ed17bac9903 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 24 Mar 2017 12:31:34 +0100 Subject: [PATCH 027/278] Fix specs for container repository tags --- app/models/container_repository.rb | 8 ++-- lib/container_registry/blob.rb | 4 +- lib/container_registry/tag.rb | 6 +-- spec/factories/container_repositories.rb | 23 +++++----- spec/lib/container_registry/tag_spec.rb | 58 ++++++++++++++++++------ 5 files changed, 64 insertions(+), 35 deletions(-) diff --git a/app/models/container_repository.rb b/app/models/container_repository.rb index 2f0fd3014a8..e5076f30c8e 100644 --- a/app/models/container_repository.rb +++ b/app/models/container_repository.rb @@ -1,8 +1,10 @@ class ContainerRepository < ActiveRecord::Base belongs_to :project - delegate :client, to: :registry + validates :manifest, presence: true - validates :name, presence: true + validates :name, length: { minimum: 0, allow_nil: false } + + delegate :client, to: :registry before_destroy :delete_tags def registry @@ -17,7 +19,7 @@ class ContainerRepository < ActiveRecord::Base end def path - @path ||= "#{project.full_path}/#{name}" + @path ||= [project.full_path, name].select(&:present?).join('/') end def tag(tag) diff --git a/lib/container_registry/blob.rb b/lib/container_registry/blob.rb index 8db8e483b1d..d5f85f9fcad 100644 --- a/lib/container_registry/blob.rb +++ b/lib/container_registry/blob.rb @@ -38,11 +38,11 @@ module ContainerRegistry end def delete - client.delete_blob(repository.name_with_namespace, digest) + client.delete_blob(repository.path, digest) end def data - @data ||= client.blob(repository.name_with_namespace, digest, type) + @data ||= client.blob(repository.path, digest, type) end end end diff --git a/lib/container_registry/tag.rb b/lib/container_registry/tag.rb index 68dd87c979d..d653deb3bf1 100644 --- a/lib/container_registry/tag.rb +++ b/lib/container_registry/tag.rb @@ -22,7 +22,7 @@ module ContainerRegistry end def manifest - @manifest ||= client.repository_manifest(repository.name_with_namespace, name) + @manifest ||= client.repository_manifest(repository.path, name) end def path @@ -38,7 +38,7 @@ module ContainerRegistry def digest return @digest if defined?(@digest) - @digest = client.repository_tag_digest(repository.name_with_namespace, name) + @digest = client.repository_tag_digest(repository.path, name) end def config_blob @@ -80,7 +80,7 @@ module ContainerRegistry def delete return unless digest - client.delete_repository_tag(repository.name_with_namespace, digest) + client.delete_repository_tag(repository.path, digest) end end end diff --git a/spec/factories/container_repositories.rb b/spec/factories/container_repositories.rb index fbf6bf62dfd..295b3596ee9 100644 --- a/spec/factories/container_repositories.rb +++ b/spec/factories/container_repositories.rb @@ -1,22 +1,21 @@ FactoryGirl.define do factory :container_repository do - name "test_container_image" + name 'test_container_image' project transient do - tags ['tag'] + tags [] end - after(:build) do |image, evaluator| - # if evaluator.tags.to_a.any? - # allow(Gitlab.config.registry).to receive(:enabled).and_return(true) - # allow(Auth::ContainerRegistryAuthenticationService) - # .to receive(:full_access_token).and_return('token') - # allow(image.client).to receive(:repository_tags).and_return({ - # name: image.name_with_namespace, - # tags: evaluator.tags - # }) - # end + after(:build) do |repository, evaluator| + if evaluator.tags.any? + allow(repository.client) + .to receive(:repository_tags) + .and_return({ + name: repository.path, + tags: evaluator.tags + }) + end end end end diff --git a/spec/lib/container_registry/tag_spec.rb b/spec/lib/container_registry/tag_spec.rb index 01153a6eca9..37eaa10f4a4 100644 --- a/spec/lib/container_registry/tag_spec.rb +++ b/spec/lib/container_registry/tag_spec.rb @@ -3,30 +3,58 @@ require 'spec_helper' describe ContainerRegistry::Tag do let(:group) { create(:group, name: 'group') } let(:project) { create(:project, path: 'test', group: group) } - let(:example_host) { 'example.com' } - let(:registry_url) { 'http://' + example_host } - let(:repository) { create(:container_repository, name: '', project: project) } - let(:tag) { repository.tag('tag') } - let(:headers) { { 'Accept' => 'application/vnd.docker.distribution.manifest.v2+json' } } + + let(:repository) do + create(:container_repository, name: '', tags: %w[latest], project: project) + end + + # TODO, move stubs to helper with this header + let(:headers) do + { 'Accept' => 'application/vnd.docker.distribution.manifest.v2+json' } + end + + let(:tag) { described_class.new(repository, 'tag') } before do - stub_container_registry_config(enabled: true, api_url: registry_url, host_port: example_host) + stub_container_registry_config(enabled: true, + api_url: 'http://registry.gitlab', + host_port: 'registry.gitlab') end it { expect(tag).to respond_to(:repository) } it { expect(tag).to delegate_method(:registry).to(:repository) } it { expect(tag).to delegate_method(:client).to(:repository) } - context '#path' do - subject { tag.path } + describe '#path' do + context 'when tag belongs to zero-level repository' do + let(:repository) do + create(:container_repository, name: '', + tags: %w[rc1], + project: project) + end - it { is_expected.to eq('example.com/group/test:tag') } + it 'returns path to the image' do + expect(tag.path).to eq('group/test:tag') + end + end + + context 'when tag belongs to first-level repository' do + let(:repository) do + create(:container_repository, name: 'my_image', + tags: %w[latest], + project: project) + end + + it 'returns path to the image' do + expect(tag.path).to eq('group/test/my_image:tag') + end + end end context 'manifest processing' do context 'schema v1' do before do - stub_request(:get, 'http://example.com/v2/group/test/manifests/tag'). + stub_request(:get, 'http://registry.gitlab/v2/group/test/manifests/tag'). with(headers: headers). to_return( status: 200, @@ -63,7 +91,7 @@ describe ContainerRegistry::Tag do context 'schema v2' do before do - stub_request(:get, 'http://example.com/v2/group/test/manifests/tag'). + stub_request(:get, 'http://registry.gitlab/v2/group/test/manifests/tag'). with(headers: headers). to_return( status: 200, @@ -100,7 +128,7 @@ describe ContainerRegistry::Tag do context 'when locally stored' do before do - stub_request(:get, 'http://example.com/v2/group/test/blobs/sha256:d7a513a663c1a6dcdba9ed832ca53c02ac2af0c333322cd6ca92936d1d9917ac'). + stub_request(:get, 'http://registry.gitlab/v2/group/test/blobs/sha256:d7a513a663c1a6dcdba9ed832ca53c02ac2af0c333322cd6ca92936d1d9917ac'). with(headers: { 'Accept' => 'application/octet-stream' }). to_return( status: 200, @@ -112,7 +140,7 @@ describe ContainerRegistry::Tag do context 'when externally stored' do before do - stub_request(:get, 'http://example.com/v2/group/test/blobs/sha256:d7a513a663c1a6dcdba9ed832ca53c02ac2af0c333322cd6ca92936d1d9917ac'). + stub_request(:get, 'http://registry.gitlab/v2/group/test/blobs/sha256:d7a513a663c1a6dcdba9ed832ca53c02ac2af0c333322cd6ca92936d1d9917ac'). with(headers: { 'Accept' => 'application/octet-stream' }). to_return( status: 307, @@ -132,7 +160,7 @@ describe ContainerRegistry::Tag do context 'manifest digest' do before do - stub_request(:head, 'http://example.com/v2/group/test/manifests/tag'). + stub_request(:head, 'http://registry.gitlab/v2/group/test/manifests/tag'). with(headers: headers). to_return(status: 200, headers: { 'Docker-Content-Digest' => 'sha256:digest' }) end @@ -145,7 +173,7 @@ describe ContainerRegistry::Tag do context '#delete' do before do - stub_request(:delete, 'http://example.com/v2/group/test/manifests/sha256:digest'). + stub_request(:delete, 'http://registry.gitlab/v2/group/test/manifests/sha256:digest'). with(headers: headers). to_return(status: 200) end From 7db1f22673f1b6890b6ce4b9db9b367eae3988f0 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 24 Mar 2017 12:41:42 +0100 Subject: [PATCH 028/278] Fix specs for services related to container registry --- .../container_images/destroy_service_spec.rb | 54 +++++++++++-------- .../services/projects/destroy_service_spec.rb | 2 +- .../projects/transfer_service_spec.rb | 2 +- 3 files changed, 33 insertions(+), 25 deletions(-) diff --git a/spec/services/container_images/destroy_service_spec.rb b/spec/services/container_images/destroy_service_spec.rb index ebc15598cce..ab5ebe5eac8 100644 --- a/spec/services/container_images/destroy_service_spec.rb +++ b/spec/services/container_images/destroy_service_spec.rb @@ -1,34 +1,42 @@ require 'spec_helper' -describe ContainerImages::DestroyService, services: true do - describe '#execute' do - let(:user) { create(:user) } - let(:container_repository) { create(:container_repository, name: '') } - let(:project) { create(:project, path: 'test', namespace: user.namespace, container_repositorys: [container_repository]) } - let(:example_host) { 'example.com' } - let(:registry_url) { 'http://' + example_host } +describe ContainerImages::DestroyService, '#execute', :services do + let(:user) { create(:user) } - it { expect(container_repository).to be_valid } - it { expect(project.container_repositorys).not_to be_empty } + let(:container_repository) do + create(:container_repository, name: 'myimage', tags: %w[latest]) + end - context 'when container image has tags' do - before do - project.team << [user, :master] - end + let(:project) do + create(:project, path: 'test', + namespace: user.namespace, + container_repositories: [container_repository]) + end - it 'removes all tags before destroy' do - service = described_class.new(project, user) + it { expect(container_repository).to be_valid } + it { expect(project.container_repositories).not_to be_empty } - expect(container_repository).to receive(:delete_tags).and_return(true) - expect { service.execute(container_repository) }.to change(project.container_repositorys, :count).by(-1) - end + context 'when container image has tags' do + before do + project.add_master(user) + end - it 'fails when tags are not removed' do - service = described_class.new(project, user) + it 'removes all tags before destroy' do + service = described_class.new(project, user) - expect(container_repository).to receive(:delete_tags).and_return(false) - expect { service.execute(container_repository) }.to raise_error(ActiveRecord::RecordNotDestroyed) - end + expect(container_repository) + .to receive(:delete_tags).and_return(true) + expect { service.execute(container_repository) } + .to change(project.container_repositories, :count).by(-1) + end + + it 'fails when tags are not removed' do + service = described_class.new(project, user) + + expect(container_repository) + .to receive(:delete_tags).and_return(false) + expect { service.execute(container_repository) } + .to raise_error(ActiveRecord::RecordNotDestroyed) end end end diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb index daad65d478a..44e0286350b 100644 --- a/spec/services/projects/destroy_service_spec.rb +++ b/spec/services/projects/destroy_service_spec.rb @@ -95,7 +95,7 @@ describe Projects::DestroyService, services: true do before do stub_container_registry_config(enabled: true) stub_container_registry_tags('tag') - project.container_repositorys << container_repository + project.container_repositories << container_repository end context 'images deletion succeeds' do diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index adf8ede5086..a3babaf1e0b 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -34,7 +34,7 @@ describe Projects::TransferService, services: true do before do stub_container_registry_config(enabled: true) stub_container_registry_tags('tag') - project.container_repositorys << container_repository + project.container_repositories << container_repository end subject { transfer_project(project, user, group) } From 4fdcaca6a148475a18aa5931b62c4173301ded8c Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 24 Mar 2017 13:08:21 +0100 Subject: [PATCH 029/278] Fix container registry blob specs --- spec/lib/container_registry/blob_spec.rb | 118 ++++++++++++----------- 1 file changed, 61 insertions(+), 57 deletions(-) diff --git a/spec/lib/container_registry/blob_spec.rb b/spec/lib/container_registry/blob_spec.rb index 718a61ba291..76ea29666ea 100644 --- a/spec/lib/container_registry/blob_spec.rb +++ b/spec/lib/container_registry/blob_spec.rb @@ -1,117 +1,121 @@ require 'spec_helper' describe ContainerRegistry::Blob do - let(:digest) { 'sha256:0123456789012345' } - let(:config) do - { - 'digest' => digest, - 'mediaType' => 'binary', - 'size' => 1000 - } - end - let(:token) { 'token' } - let(:group) { create(:group, name: 'group') } let(:project) { create(:project, path: 'test', group: group) } - let(:example_host) { 'example.com' } - let(:registry_url) { 'http://' + example_host } - let(:repository) { create(:container_repository, name: '', project: project) } - let(:blob) { repository.blob(config) } + + let(:repository) do + create(:container_repository, name: 'image', + tags: %w[latest rc1], + project: project) + end + + let(:config) do + { 'digest' => 'sha256:0123456789012345', + 'mediaType' => 'binary', + 'size' => 1000 } + end + + let(:blob) { described_class.new(repository, config) } before do - stub_container_registry_config(enabled: true, api_url: registry_url, host_port: example_host) + stub_container_registry_config(enabled: true, + api_url: 'http://registry.gitlab', + host_port: 'registry.gitlab') end it { expect(blob).to respond_to(:repository) } it { expect(blob).to delegate_method(:registry).to(:repository) } it { expect(blob).to delegate_method(:client).to(:repository) } - context '#path' do - subject { blob.path } - - it { is_expected.to eq('example.com/group/test@sha256:0123456789012345') } + describe '#path' do + it 'returns a valid path to the blob' do + expect(blob.path).to eq('group/test/image@sha256:0123456789012345') + end end - context '#digest' do - subject { blob.digest } - - it { is_expected.to eq(digest) } + describe '#digest' do + it 'return correct digest value' do + expect(blob.digest).to eq 'sha256:0123456789012345' + end end - context '#type' do - subject { blob.type } - - it { is_expected.to eq('binary') } + describe '#type' do + it 'returns a correct type' do + expect(blob.type).to eq 'binary' + end end - context '#revision' do - subject { blob.revision } - - it { is_expected.to eq('0123456789012345') } + describe '#revision' do + it 'returns a correct blob SHA' do + expect(blob.revision).to eq '0123456789012345' + end end - context '#short_revision' do - subject { blob.short_revision } - - it { is_expected.to eq('012345678') } + describe '#short_revision' do + it 'return a short SHA' do + expect(blob.short_revision).to eq '012345678' + end end - context '#delete' do + describe '#delete' do before do - stub_request(:delete, 'http://example.com/v2/group/test/blobs/sha256:0123456789012345'). - to_return(status: 200) + stub_request(:delete, 'http://registry.gitlab/v2/group/test/image/blobs/sha256:0123456789012345') + .to_return(status: 200) end - subject { blob.delete } - - it { is_expected.to be_truthy } + it 'returns true when blob has been successfuly deleted' do + expect(blob.delete).to be_truthy + end end - context '#data' do - let(:data) { '{"key":"value"}' } - - subject { blob.data } - + describe '#data' do context 'when locally stored' do before do - stub_request(:get, 'http://example.com/v2/group/test/blobs/sha256:0123456789012345'). + stub_request(:get, 'http://registry.gitlab/v2/group/test/image/blobs/sha256:0123456789012345'). to_return( status: 200, headers: { 'Content-Type' => 'application/json' }, - body: data) + body: '{"key":"value"}') end - it { is_expected.to eq(data) } + it 'returns a correct blob data' do + expect(blob.data).to eq '{"key":"value"}' + end end context 'when externally stored' do + let(:location) { 'http://external.com/blob/file' } + before do - stub_request(:get, 'http://example.com/v2/group/test/blobs/sha256:0123456789012345'). - with(headers: { 'Authorization' => "bearer #{token}" }). - to_return( + stub_request(:get, 'http://registry.gitlab/v2/group/test/image/blobs/sha256:0123456789012345') + .with(headers: { 'Authorization' => 'bearer token' }) + .to_return( status: 307, headers: { 'Location' => location }) end context 'for a valid address' do - let(:location) { 'http://external.com/blob/file' } - before do stub_request(:get, location). with(headers: { 'Authorization' => nil }). to_return( status: 200, headers: { 'Content-Type' => 'application/json' }, - body: data) + body: '{"key":"value"}') end - it { is_expected.to eq(data) } + it 'returns correct data' do + expect(blob.data).to eq '{"key":"value"}' + end end context 'for invalid file' do let(:location) { 'file:///etc/passwd' } - it { expect{ subject }.to raise_error(ArgumentError, 'invalid address') } + it 'raises an error' do + expect { blob.data }.to raise_error(ArgumentError, 'invalid address') + end end end end From 16e645c6563b81ce03e481b094abc2d331d05f36 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 24 Mar 2017 13:27:05 +0100 Subject: [PATCH 030/278] Remove container_registry method from project class --- app/models/project.rb | 14 -------------- spec/models/project_spec.rb | 29 +++-------------------------- 2 files changed, 3 insertions(+), 40 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index 5c6672c95b3..a579ac7dc64 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -405,20 +405,6 @@ class Project < ActiveRecord::Base @repository ||= Repository.new(path_with_namespace, self) end - def container_registry - return unless Gitlab.config.registry.enabled - - @container_registry ||= begin - token = Auth::ContainerRegistryAuthenticationService.full_access_token(project) - - url = Gitlab.config.registry.api_url - host_port = Gitlab.config.registry.host_port - # TODO, move configuration vars into ContainerRegistry::Registry, clean - # this method up afterwards - ContainerRegistry::Registry.new(url, token: token, path: host_port) - end - end - def container_registry_url if Gitlab.config.registry.enabled "#{Gitlab.config.registry.host_port}/#{path_with_namespace.downcase}" diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 69b7906bb4e..e4e75cc6a09 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1389,25 +1389,6 @@ describe Project, models: true do end end - describe '#container_registry_path_with_namespace' do - let(:project) { create(:empty_project, path: 'PROJECT') } - - subject { project.container_registry_path_with_namespace } - - it { is_expected.not_to eq(project.path_with_namespace) } - it { is_expected.to eq(project.path_with_namespace.downcase) } - end - - describe '#container_registry' do - let(:project) { create(:empty_project) } - - before { stub_container_registry_config(enabled: true) } - - subject { project.container_registry } - - it { is_expected.not_to be_nil } - end - describe '#container_registry_url' do let(:project) { create(:empty_project) } @@ -1417,10 +1398,8 @@ describe Project, models: true do context 'for enabled registry' do let(:registry_settings) do - { - enabled: true, - host_port: 'example.com', - } + { enabled: true, + host_port: 'example.com' } end it { is_expected.not_to be_nil } @@ -1428,9 +1407,7 @@ describe Project, models: true do context 'for disabled registry' do let(:registry_settings) do - { - enabled: false - } + { enabled: false } end it { is_expected.to be_nil } From d6f37a34c1c262f49a92f26dd187819419d56c2f Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 24 Mar 2017 13:27:18 +0100 Subject: [PATCH 031/278] Fix feature specs related to container registry --- app/controllers/projects/container_registry_controller.rb | 4 ++-- spec/lib/gitlab/import_export/all_models.yml | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/app/controllers/projects/container_registry_controller.rb b/app/controllers/projects/container_registry_controller.rb index 4981e57ed22..8929bd0aa55 100644 --- a/app/controllers/projects/container_registry_controller.rb +++ b/app/controllers/projects/container_registry_controller.rb @@ -5,7 +5,7 @@ class Projects::ContainerRegistryController < Projects::ApplicationController layout 'project' def index - @images = project.container_images + @images = project.container_repositories end def destroy @@ -44,7 +44,7 @@ class Projects::ContainerRegistryController < Projects::ApplicationController end def image - @image ||= project.container_images.find_by(id: params[:id]) + @image ||= project.container_repositories.find_by(id: params[:id]) end def tag diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 0429636486c..e5bba29d85b 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -116,6 +116,7 @@ merge_access_levels: push_access_levels: - protected_branch container_repositories: +- project - name project: - taggings @@ -201,7 +202,7 @@ project: - project_authorizations - route - statistics -- container_images +- container_repositories - uploads award_emoji: - awardable From dce706bfcbddf7952e2c42c0c42825044cbb43a2 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 24 Mar 2017 13:47:29 +0100 Subject: [PATCH 032/278] Do not require a manifest for container repository Container repository can be empty - no tags or blogs is OK. --- app/models/container_repository.rb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/app/models/container_repository.rb b/app/models/container_repository.rb index e5076f30c8e..c8c56e69269 100644 --- a/app/models/container_repository.rb +++ b/app/models/container_repository.rb @@ -1,7 +1,6 @@ class ContainerRepository < ActiveRecord::Base belongs_to :project - validates :manifest, presence: true validates :name, length: { minimum: 0, allow_nil: false } delegate :client, to: :registry @@ -43,6 +42,8 @@ class ContainerRepository < ActiveRecord::Base ContainerRegistry::Blob.new(self, config) end + # TODO, add bang to this method + # def delete_tags return unless tags @@ -52,6 +53,14 @@ class ContainerRepository < ActiveRecord::Base end end + # TODO, specs needed + # + def empty? + tags.none? + end + + # TODO, we will return a new ContainerRepository object here + # def self.project_from_path(repository_path) return unless repository_path.include?('/') From 7ada193e0fd28b4a6eca1fda7dda6f0ebe6b2d72 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 24 Mar 2017 14:58:25 +0100 Subject: [PATCH 033/278] Fix specs for container repository destroy service --- spec/features/container_registry_spec.rb | 1 - spec/services/container_images/destroy_service_spec.rb | 4 ++++ spec/support/stub_gitlab_calls.rb | 3 ++- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/spec/features/container_registry_spec.rb b/spec/features/container_registry_spec.rb index 88642f72772..73b22fa1b7d 100644 --- a/spec/features/container_registry_spec.rb +++ b/spec/features/container_registry_spec.rb @@ -14,7 +14,6 @@ describe "Container Registry" do stub_container_registry_config(enabled: true) stub_container_registry_tags(*tags) project.container_repositories << container_repository unless container_repository.nil? - allow(Auth::ContainerRegistryAuthenticationService).to receive(:full_access_token).and_return('token') end describe 'GET /:project/container_registry' do diff --git a/spec/services/container_images/destroy_service_spec.rb b/spec/services/container_images/destroy_service_spec.rb index ab5ebe5eac8..6931b503e6d 100644 --- a/spec/services/container_images/destroy_service_spec.rb +++ b/spec/services/container_images/destroy_service_spec.rb @@ -13,6 +13,10 @@ describe ContainerImages::DestroyService, '#execute', :services do container_repositories: [container_repository]) end + before do + stub_container_registry_config(enabled: true) + end + it { expect(container_repository).to be_valid } it { expect(project.container_repositories).not_to be_empty } diff --git a/spec/support/stub_gitlab_calls.rb b/spec/support/stub_gitlab_calls.rb index a01ef576234..3949784aabb 100644 --- a/spec/support/stub_gitlab_calls.rb +++ b/spec/support/stub_gitlab_calls.rb @@ -27,7 +27,8 @@ module StubGitlabCalls def stub_container_registry_config(registry_settings) allow(Gitlab.config.registry).to receive_messages(registry_settings) - allow(Auth::ContainerRegistryAuthenticationService).to receive(:full_access_token).and_return('token') + allow(Auth::ContainerRegistryAuthenticationService) + .to receive(:full_access_token).and_return('token') end def stub_container_registry_tags(*tags) From f09e3fe85116743c0cb537fb958a8b0ab1ad19fb Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 27 Mar 2017 16:04:38 +0200 Subject: [PATCH 034/278] Add a few pending specs for container repository --- spec/models/container_repository_spec.rb | 25 ++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/spec/models/container_repository_spec.rb b/spec/models/container_repository_spec.rb index e3180e01758..296b9e713a8 100644 --- a/spec/models/container_repository_spec.rb +++ b/spec/models/container_repository_spec.rb @@ -92,4 +92,29 @@ describe ContainerRepository do end end end + + describe '#from_repository_path' do + context 'when received multi-level repository path' do + let(:repository) do + described_class.from_repository_path('group/test/some/image/name') + end + + pending 'fabricates object within a correct project' do + expect(repository.project).to eq project + end + + pending 'it fabricates project with a correct name' do + expect(repository.name).to eq 'some/image/name' + end + end + + context 'when path contains too many nodes' do + end + + context 'when received multi-level repository with nested groups' do + end + + context 'when received root repository path' do + end + end end From 576ffef6c1ddff3493099f088a21c867477d9160 Mon Sep 17 00:00:00 2001 From: Kushal Pandya Date: Tue, 14 Mar 2017 16:51:24 +0530 Subject: [PATCH 035/278] Fix project title validation, prevent clicking on disabled button --- app/views/projects/new.html.haml | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/app/views/projects/new.html.haml b/app/views/projects/new.html.haml index 34a1214a350..252929cdb9f 100644 --- a/app/views/projects/new.html.haml +++ b/app/views/projects/new.html.haml @@ -123,23 +123,16 @@ $(".btn_import_gitlab_project").attr("href", _href + '?namespace_id=' + $("#project_namespace_id").val() + '&path=' + $("#project_path").val()); }); - $('.btn_import_gitlab_project').attr('disabled',true) + $('.btn_import_gitlab_project').attr('disabled', $('#project_path').val().trim().length === 0); $('.import_gitlab_project').attr('title', 'Project path and name required.'); - $('.import_gitlab_project').click(function( event ) { - if($('.btn_import_gitlab_project').attr('disabled')) { - event.preventDefault(); - new Flash("Please enter path and name for the project to be imported to."); - } - }); - $('#new_project').submit(function(){ var $path = $('#project_path'); $path.val($path.val().trim()); }); $('#project_path').keyup(function(){ - if($(this).val().length !=0) { + if($(this).val().trim().length !== 0) { $('.btn_import_gitlab_project').attr('disabled', false); $('.import_gitlab_project').attr('title',''); $(".flash-container").html("") From 25ac232f749243aded03acefd247374de39a5938 Mon Sep 17 00:00:00 2001 From: Kushal Pandya Date: Tue, 14 Mar 2017 16:57:16 +0530 Subject: [PATCH 036/278] Add changelog entry --- changelogs/unreleased/29432-prevent-click-disabled-btn.yml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changelogs/unreleased/29432-prevent-click-disabled-btn.yml diff --git a/changelogs/unreleased/29432-prevent-click-disabled-btn.yml b/changelogs/unreleased/29432-prevent-click-disabled-btn.yml new file mode 100644 index 00000000000..68371464588 --- /dev/null +++ b/changelogs/unreleased/29432-prevent-click-disabled-btn.yml @@ -0,0 +1,4 @@ +--- +title: Fix project title validation, prevent clicking on disabled button +merge_request: +author: From 625af1684f5f0487d07f12b20d6c33e30c54b969 Mon Sep 17 00:00:00 2001 From: Kushal Pandya Date: Tue, 14 Mar 2017 17:18:42 +0530 Subject: [PATCH 037/278] Add MR number to changelog --- changelogs/unreleased/29432-prevent-click-disabled-btn.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelogs/unreleased/29432-prevent-click-disabled-btn.yml b/changelogs/unreleased/29432-prevent-click-disabled-btn.yml index 68371464588..f30570cf68b 100644 --- a/changelogs/unreleased/29432-prevent-click-disabled-btn.yml +++ b/changelogs/unreleased/29432-prevent-click-disabled-btn.yml @@ -1,4 +1,4 @@ --- title: Fix project title validation, prevent clicking on disabled button -merge_request: +merge_request: 9931 author: From aa637e69e04f92f0f8e8178a919ada5df088a186 Mon Sep 17 00:00:00 2001 From: Kushal Pandya Date: Tue, 28 Mar 2017 11:02:57 +0530 Subject: [PATCH 038/278] Update export button tooltip --- app/views/projects/new.html.haml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/views/projects/new.html.haml b/app/views/projects/new.html.haml index 252929cdb9f..3a3db3e26a9 100644 --- a/app/views/projects/new.html.haml +++ b/app/views/projects/new.html.haml @@ -109,6 +109,8 @@ %p Please wait a moment, this page will automatically refresh when ready. :javascript + var importBtnTooltip = "Please enter a valid project name."; + $('.how_to_import_link').bind('click', function (e) { e.preventDefault(); var import_modal = $(this).next(".modal").show(); @@ -124,7 +126,7 @@ }); $('.btn_import_gitlab_project').attr('disabled', $('#project_path').val().trim().length === 0); - $('.import_gitlab_project').attr('title', 'Project path and name required.'); + $('.import_gitlab_project').attr('title', importBtnTooltip); $('#new_project').submit(function(){ var $path = $('#project_path'); @@ -138,7 +140,7 @@ $(".flash-container").html("") } else { $('.btn_import_gitlab_project').attr('disabled',true); - $('.import_gitlab_project').attr('title', 'Project path and name required.'); + $('.import_gitlab_project').attr('title', importBtnTooltip); } }); From a055622ae0b41550968da4269dd312844fcf6196 Mon Sep 17 00:00:00 2001 From: Kushal Pandya Date: Tue, 28 Mar 2017 11:45:57 +0530 Subject: [PATCH 039/278] Update tests --- spec/features/projects/import_export/import_file_spec.rb | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/spec/features/projects/import_export/import_file_spec.rb b/spec/features/projects/import_export/import_file_spec.rb index 2d1106ea3e8..583f479ec18 100644 --- a/spec/features/projects/import_export/import_file_spec.rb +++ b/spec/features/projects/import_export/import_file_spec.rb @@ -69,12 +69,8 @@ feature 'Import/Export - project import integration test', feature: true, js: tr select2(namespace.id, from: '#project_namespace_id') - # click on disabled element - find(:link, 'GitLab export').trigger('click') - - page.within('.flash-container') do - expect(page).to have_content('Please enter path and name') - end + # Check for tooltip disabled import button + expect(find('.import_gitlab_project')['title']).to eq('Please enter a valid project name.') end end From b15d9042e2688f29b002f90e0154e793ff1544ff Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 28 Mar 2017 14:34:56 +0200 Subject: [PATCH 040/278] Implement container repository path class --- lib/container_registry/path.rb | 29 ++++++ spec/lib/container_registry/path_spec.rb | 119 +++++++++++++++++++++++ 2 files changed, 148 insertions(+) create mode 100644 lib/container_registry/path.rb create mode 100644 spec/lib/container_registry/path_spec.rb diff --git a/lib/container_registry/path.rb b/lib/container_registry/path.rb new file mode 100644 index 00000000000..f32df1bc0d1 --- /dev/null +++ b/lib/container_registry/path.rb @@ -0,0 +1,29 @@ +module ContainerRegistry + class Path + InvalidRegistryPathError = Class.new(StandardError) + + def initialize(name) + @nodes = name.to_s.split('/') + end + + def valid? + @nodes.size > 1 && + @nodes.size < Namespace::NUMBER_OF_ANCESTORS_ALLOWED + end + + def components + raise InvalidRegistryPathError unless valid? + + @components ||= @nodes.size.downto(2).map do |length| + @nodes.take(length).join('/') + end + end + + def repository_project + @project ||= Project.where_full_path_in(components.first(3))&.first + end + + def repository_name + end + end +end diff --git a/spec/lib/container_registry/path_spec.rb b/spec/lib/container_registry/path_spec.rb new file mode 100644 index 00000000000..32f25f5e527 --- /dev/null +++ b/spec/lib/container_registry/path_spec.rb @@ -0,0 +1,119 @@ +require 'spec_helper' + +describe ContainerRegistry::Path do + let(:path) { described_class.new(name) } + + describe '#components' do + context 'when repository path is valid' do + let(:name) { 'path/to/some/project' } + + it 'return all project-like components in reverse order' do + expect(path.components).to eq %w[path/to/some/project + path/to/some + path/to] + end + end + + context 'when repository path is invalid' do + let(:name) { '' } + + it 'rasises en error' do + expect { path.components } + .to raise_error described_class::InvalidRegistryPathError + end + end + end + + describe '#valid?' do + context 'when path has less than two components' do + let(:name) { 'something/' } + + it 'is not valid' do + expect(path).not_to be_valid + end + end + + context 'when path has more than allowed number of components' do + let(:name) { 'a/b/c/d/e/f/g/h/i/j/k/l/m/n/o/p/r/s/t/u/w/y/z' } + + it 'is not valid' do + expect(path).not_to be_valid + end + end + + context 'when path has two or more components' do + let(:name) { 'some/path' } + + it 'is valid' do + expect(path).to be_valid + end + end + end + + describe '#repository_project' do + let(:group) { create(:group, path: 'some_group') } + + context 'when project for given path exists' do + let(:name) { 'some_group/some_project' } + + before do + create(:empty_project, group: group, name: 'some_project') + create(:empty_project, name: 'some_project') + end + + it 'returns a correct project' do + expect(path.repository_project.group).to eq group + end + end + + context 'when project for given path does not exist' do + let(:name) { 'not/matching' } + + it 'returns nil' do + expect(path.repository_project).to be_nil + end + end + + context 'when matching multi-level path' do + let(:project) do + create(:empty_project, group: group, name: 'some_project') + end + + context 'when using the zero-level path' do + let(:name) { project.full_path } + + it 'supports zero-level path' do + expect(path.repository_project).to eq project + end + end + + context 'when using first-level path' do + let(:name) { "#{project.full_path}/repository" } + + it 'supports first-level path' do + expect(path.repository_project).to eq project + end + end + + context 'when using second-level path' do + let(:name) { "#{project.full_path}/repository/name" } + + it 'supports second-level path' do + expect(path.repository_project).to eq project + end + end + + context 'when using too deep nesting in the path' do + let(:name) { "#{project.full_path}/repository/name/invalid" } + + it 'does not support three-levels of nesting' do + expect(path.repository_project).to be_nil + end + end + end + end + + describe '#repository_name' do + pending 'returns a correct name' + end +end From bdc1e1b9e005eeaf564b79698d61a801c3c6360f Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 28 Mar 2017 14:57:22 +0200 Subject: [PATCH 041/278] Implement method matching container repository names --- lib/container_registry/path.rb | 8 +++++ spec/lib/container_registry/path_spec.rb | 45 +++++++++++++++++++++++- 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/lib/container_registry/path.rb b/lib/container_registry/path.rb index f32df1bc0d1..89ef396f374 100644 --- a/lib/container_registry/path.rb +++ b/lib/container_registry/path.rb @@ -3,6 +3,7 @@ module ContainerRegistry InvalidRegistryPathError = Class.new(StandardError) def initialize(name) + @name = name @nodes = name.to_s.split('/') end @@ -19,11 +20,18 @@ module ContainerRegistry end end + def has_repository? + # ContainerRepository.find_by_full_path(@name).present? + end + def repository_project @project ||= Project.where_full_path_in(components.first(3))&.first end def repository_name + return unless repository_project + + @name.remove(%r(^?#{Regexp.escape(repository_project.full_path)}/?)) end end end diff --git a/spec/lib/container_registry/path_spec.rb b/spec/lib/container_registry/path_spec.rb index 32f25f5e527..278b1fc1b55 100644 --- a/spec/lib/container_registry/path_spec.rb +++ b/spec/lib/container_registry/path_spec.rb @@ -114,6 +114,49 @@ describe ContainerRegistry::Path do end describe '#repository_name' do - pending 'returns a correct name' + context 'when project does not exist' do + let(:name) { 'some/name' } + + it 'returns nil' do + expect(path.repository_name).to be_nil + end + end + + context 'when project exists' do + let(:group) { create(:group, path: 'some_group') } + + let(:project) do + create(:empty_project, group: group, name: 'some_project') + end + + before do + allow(path).to receive(:repository_project) + .and_return(project) + end + + context 'when project path equal repository path' do + let(:name) { 'some_group/some_project' } + + it 'returns an empty string' do + expect(path.repository_name).to eq '' + end + end + + context 'when repository path has one additional level' do + let(:name) { 'some_group/some_project/repository' } + + it 'returns a correct repository name' do + expect(path.repository_name).to eq 'repository' + end + end + + context 'when repository path has two additional levels' do + let(:name) { 'some_group/some_project/repository/image' } + + it 'returns a correct repository name' do + expect(path.repository_name).to eq 'repository/image' + end + end + end end end From f6f56dddfcfed352dfba8dc32dad554096552a7d Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Mon, 27 Mar 2017 14:14:12 +0100 Subject: [PATCH 042/278] Add back expandable folders behavior --- .../environments/components/environments_table.js | 9 +++++++++ .../environments/stores/environments_store.js | 4 ++++ 2 files changed, 13 insertions(+) diff --git a/app/assets/javascripts/environments/components/environments_table.js b/app/assets/javascripts/environments/components/environments_table.js index 338dff40bc9..486389973d3 100644 --- a/app/assets/javascripts/environments/components/environments_table.js +++ b/app/assets/javascripts/environments/components/environments_table.js @@ -53,6 +53,15 @@ export default { :can-create-deployment="canCreateDeployment" :can-read-environment="canReadEnvironment" :service="service"> + + diff --git a/app/assets/javascripts/environments/stores/environments_store.js b/app/assets/javascripts/environments/stores/environments_store.js index 3c3084f3b78..f05fe6e60ae 100644 --- a/app/assets/javascripts/environments/stores/environments_store.js +++ b/app/assets/javascripts/environments/stores/environments_store.js @@ -85,4 +85,8 @@ export default class EnvironmentsStore { this.state.stoppedCounter = count; return count; } + + toggleRow(name) { + + } } From adec9194ef9b825a3a79dc262975987012639f23 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Mon, 27 Mar 2017 17:37:26 +0100 Subject: [PATCH 043/278] Adds expandable folder back. Makes request to get environments --- .../environments/components/environment.js | 29 +++++++++++++- .../components/environment_item.js | 35 ++++++++++------ .../components/environments_table.js | 3 +- .../services/environments_service.js | 14 ++++++- .../environments/stores/environments_store.js | 40 +++++++++++++++++-- 5 files changed, 101 insertions(+), 20 deletions(-) diff --git a/app/assets/javascripts/environments/components/environment.js b/app/assets/javascripts/environments/components/environment.js index 51aab8460f6..57599876f5a 100644 --- a/app/assets/javascripts/environments/components/environment.js +++ b/app/assets/javascripts/environments/components/environment.js @@ -24,6 +24,7 @@ export default Vue.component('environment-component', { state: store.state, visibility: 'available', isLoading: false, + isLoadingFolderContent: false, cssContainerClass: environmentsData.cssClass, endpoint: environmentsData.environmentsDataEndpoint, canCreateDeployment: environmentsData.canCreateDeployment, @@ -68,15 +69,21 @@ export default Vue.component('environment-component', { this.fetchEnvironments(); eventHub.$on('refreshEnvironments', this.fetchEnvironments); + eventHub.$on('toggleFolder', this.toggleFolder); }, beforeDestroyed() { eventHub.$off('refreshEnvironments'); + eventHub.$off('toggleFolder'); }, methods: { - toggleRow(model) { - return this.store.toggleFolder(model.name); + toggleFolder(folder) { + this.store.toggleFolder(folder); + + if (!folder.isOpen) { + this.fetchChildEnvironments(folder); + } }, /** @@ -117,6 +124,24 @@ export default Vue.component('environment-component', { new Flash('An error occurred while fetching the environments.'); }); }, + + fetchChildEnvironments(folder) { + this.isLoadingFolderContent = true; + + this.service.getFolderContent(folder.folderName) + .then(resp => resp.json()) + .then((response) => { + console.log(response); + this.store.folderContent(folder, response.environments); + }) + .then(() => { + this.isLoadingFolderContent = false; + }) + .catch(() => { + this.isLoadingFolderContent = false; + new Flash('An error occurred while fetching the environments.'); + }); + }, }, template: ` diff --git a/app/assets/javascripts/environments/components/environment_item.js b/app/assets/javascripts/environments/components/environment_item.js index 66ed10e19d1..8cfaeb69233 100644 --- a/app/assets/javascripts/environments/components/environment_item.js +++ b/app/assets/javascripts/environments/components/environment_item.js @@ -6,6 +6,7 @@ import StopComponent from './environment_stop'; import RollbackComponent from './environment_rollback'; import TerminalButtonComponent from './environment_terminal_button'; import CommitComponent from '../../vue_shared/components/commit'; +import eventHub from '../event_hub'; /** * Envrionment Item Component @@ -391,16 +392,6 @@ export default { return ''; }, - - /** - * Constructs folder URL based on the current location and the folder id. - * - * @return {String} - */ - folderUrl() { - return `${window.location.pathname}/folders/${this.model.folderName}`; - }, - }, /** @@ -418,6 +409,12 @@ export default { return true; }, + methods: { + onClickFolder() { + eventHub.$emit('toggleFolder', this.model); + }, + }, + template: ` @@ -426,7 +423,21 @@ export default { :href="environmentPath"> {{model.name}} - + + + +