From e43b2e81dab3cade773d479f2ae56478e3113207 Mon Sep 17 00:00:00 2001 From: Andre Guedes Date: Thu, 27 Oct 2016 17:39:32 -0200 Subject: 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 -- cgit v1.2.1 From dcd4beb8eb7bb7d0c2f720ef85c3da9f97a3dfe6 Mon Sep 17 00:00:00 2001 From: Andre Guedes Date: Wed, 2 Nov 2016 00:33:35 -0200 Subject: 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 + .../container_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 +++ ...029153736_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 -- cgit v1.2.1 From eed0b85ad084ad4d13cc26907102063d9372fe75 Mon Sep 17 00:00:00 2001 From: Andre Guedes Date: Wed, 23 Nov 2016 14:50:30 -0200 Subject: 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 ++++++++----- app/services/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 ------- .../projects/container_registry/_image.html.haml | 35 ++++++++++++++++++++++ .../projects/container_registry/_tag.html.haml | 2 +- .../projects/container_registry/index.html.haml | 20 +++---------- ...029153736_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 -- cgit v1.2.1 From 246df2bd1151d39a04ef553064144eb75ee3e980 Mon Sep 17 00:00:00 2001 From: Andre Guedes Date: Tue, 13 Dec 2016 23:42:43 -0200 Subject: Adding registry endpoint authorization --- .../admin/application_settings_controller.rb | 6 +++++ .../admin/container_registry_controller.rb | 11 ++++++++ app/models/application_setting.rb | 6 +++++ app/views/admin/container_registry/show.html.haml | 31 ++++++++++++++++++++++ app/views/admin/dashboard/_head.html.haml | 4 +++ config/routes/admin.rb | 2 ++ ...egistry_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' -- cgit v1.2.1 From e4fa80f3b67f1ef30c262cd4df28516ccff6336a Mon Sep 17 00:00:00 2001 From: Andre Guedes Date: Fri, 16 Dec 2016 01:24:05 -0200 Subject: 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 ++---- .../container_registry_authentication_service.rb | 3 +- app/services/container_images/destroy_service.rb | 1 - app/services/projects/transfer_service.rb | 6 +- .../projects/container_registry/_image.html.haml | 2 +- .../projects/container_registry/_tag.html.haml | 2 +- .../projects/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 +- spec/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 +++--------- spec/services/projects/destroy_service_spec.rb | 15 +++-- spec/services/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 container images in Container Registry for this project') } + end - it { expect(page).to have_content('No images in Container Registry for this project') } + 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) - context 'when there are tags' do - it { expect(page).to have_content(tag_name) } - it { expect(page).to have_content('d7a513a66') } + 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) } -- cgit v1.2.1 From 164ef8a348cac86097313bc453493ccf739adffe Mon Sep 17 00:00:00 2001 From: Andre Guedes Date: Fri, 16 Dec 2016 11:12:37 -0200 Subject: 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. -- cgit v1.2.1 From b408a192e0fbf630d4f9a4112f6835be50a681d8 Mon Sep 17 00:00:00 2001 From: Andre Guedes Date: Fri, 16 Dec 2016 12:07:20 -0200 Subject: 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 -- cgit v1.2.1 From ea17df5c4c23890c48cd51af17e2517f04f7c88b Mon Sep 17 00:00:00 2001 From: Andre Guedes Date: Wed, 25 Jan 2017 10:02:09 -0200 Subject: 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. -- cgit v1.2.1 From 8294756fc110fdb84036e4ae097940410a8ad6de Mon Sep 17 00:00:00 2001 From: Andre Guedes Date: Wed, 25 Jan 2017 10:24:50 -0200 Subject: 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 -- cgit v1.2.1 From db5b4b8b1a9b8aa07c8310dde53b7c3ed391bafd Mon Sep 17 00:00:00 2001 From: Andre Guedes Date: Wed, 22 Feb 2017 11:19:23 -0300 Subject: Creates specs for destroy service and improves namespace container image query performance --- app/models/namespace.rb | 2 +- app/services/container_images/destroy_service.rb | 26 ++--------------- app/views/admin/container_registry/show.html.haml | 2 +- .../20161031013926_create_container_image.rb | 16 ---------- ...egistry_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, :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 + return false unless can?(current_user, :update_container_image, project) - 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 -- cgit v1.2.1 From b1f386841ea0fcf08f33bda90b3dea49dac06c08 Mon Sep 17 00:00:00 2001 From: Matthew Bender Date: Fri, 17 Mar 2017 18:57:45 -0400 Subject: Improves Jira integration documentation --- .../29670-jira-integration-documentation-improvment.yml | 4 ++++ .../project/integrations/img/jira_project_settings.png | Bin 0 -> 32791 bytes doc/user/project/integrations/jira.md | 5 +++++ 3 files changed, 9 insertions(+) create mode 100644 changelogs/unreleased/29670-jira-integration-documentation-improvment.yml create mode 100644 doc/user/project/integrations/img/jira_project_settings.png diff --git a/changelogs/unreleased/29670-jira-integration-documentation-improvment.yml b/changelogs/unreleased/29670-jira-integration-documentation-improvment.yml new file mode 100644 index 00000000000..8975f0b6ef3 --- /dev/null +++ b/changelogs/unreleased/29670-jira-integration-documentation-improvment.yml @@ -0,0 +1,4 @@ +--- +title: Added clarification to the Jira integration documentation. +merge_request: 10066 +author: Matthew Bender diff --git a/doc/user/project/integrations/img/jira_project_settings.png b/doc/user/project/integrations/img/jira_project_settings.png new file mode 100644 index 00000000000..cb6a6ba14ce Binary files /dev/null and b/doc/user/project/integrations/img/jira_project_settings.png differ diff --git a/doc/user/project/integrations/jira.md b/doc/user/project/integrations/jira.md index 4c64d1e0907..e02f81fd972 100644 --- a/doc/user/project/integrations/jira.md +++ b/doc/user/project/integrations/jira.md @@ -157,6 +157,11 @@ the same goal: where `PROJECT-1` is the issue ID of the JIRA project. +>**Note:** +- Only commits and merges into the project's default branch (usually **master**) will + close an issue in Jira. You can change your projects default branch under + [project settings](img/jira_project_settings.png). + ### JIRA issue closing example Let's consider the following example: -- cgit v1.2.1 From 16cca3a0ea7f4b95e99d7b3e8d4953334fa7bec7 Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Fri, 17 Mar 2017 17:25:17 +0100 Subject: 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. --- app/assets/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 -- cgit v1.2.1 From 53d332d3c73f8a883fa54d8eaaf91f92da73c33f Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 21 Mar 2017 13:39:57 +0100 Subject: 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 -- cgit v1.2.1 From c64d36306cafac463f20d49e750f397a9b32960b Mon Sep 17 00:00:00 2001 From: Andre Guedes Date: Tue, 21 Mar 2017 10:35:02 -0300 Subject: Makes ContainerImages Routable Conflicts: db/schema.rb --- app/models/container_image.rb | 14 ++++++++++++-- .../auth/container_registry_authentication_service.rb | 2 +- app/views/projects/container_registry/_image.html.haml | 3 +-- app/views/projects/container_registry/index.html.haml | 3 +-- db/migrate/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) -- cgit v1.2.1 From 68a2fa54dedcdbe893ec811413d1703e5f6ac2dc Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 22 Mar 2017 11:08:23 +0100 Subject: 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 --- app/views/admin/container_registry/show.html.haml | 31 ----------- app/views/admin/dashboard/_head.html.haml | 4 -- config/routes/admin.rb | 2 - ...egistry_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 -- cgit v1.2.1 From 29c34267556198ee3dbbe2f13bc81708f5e60f10 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 22 Mar 2017 11:41:05 +0100 Subject: Move container images migration to the present time --- db/migrate/20161031013926_create_container_image.rb | 17 ----------------- db/migrate/20170322013926_create_container_image.rb | 13 +++++++++++++ 2 files changed, 13 insertions(+), 17 deletions(-) delete mode 100644 db/migrate/20161031013926_create_container_image.rb create mode 100644 db/migrate/20170322013926_create_container_image.rb diff --git a/db/migrate/20161031013926_create_container_image.rb b/db/migrate/20161031013926_create_container_image.rb deleted file mode 100644 index 06c409857da..00000000000 --- a/db/migrate/20161031013926_create_container_image.rb +++ /dev/null @@ -1,17 +0,0 @@ -# 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 - create_table :container_images do |t| - t.integer :project_id - t.string :name - t.string :path - end - end -end diff --git a/db/migrate/20170322013926_create_container_image.rb b/db/migrate/20170322013926_create_container_image.rb new file mode 100644 index 00000000000..c494f2a56c7 --- /dev/null +++ b/db/migrate/20170322013926_create_container_image.rb @@ -0,0 +1,13 @@ +class CreateContainerImage < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + create_table :container_images do |t| + t.integer :project_id + t.string :name + t.string :path + end + end +end -- cgit v1.2.1 From 95e2c0196b7e492f8c03c6cfeb6b37e97f75813e Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 22 Mar 2017 12:28:23 +0100 Subject: 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 -- cgit v1.2.1 From 896b13b929369c02f72fa881eda24ca4a6a0d900 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 22 Mar 2017 16:07:27 +0100 Subject: Refactor splitting container image full path [ci skip] --- app/models/container_image.rb | 17 +++++++---------- .../auth/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| -- cgit v1.2.1 From 4005eb643657e5ee8b1f328e36a3204253e3acf4 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 23 Mar 2017 11:41:16 +0100 Subject: Fix communication between GitLab and Container Registry --- app/models/container_image.rb | 23 ++++++++++++++-------- .../container_registry_authentication_service.rb | 17 ++++++++-------- 2 files changed, 24 insertions(+), 16 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?('/') - - 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) + def self.project_from_path(image_path) + return unless image_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('/')) + + ## + # 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| -- cgit v1.2.1 From bd8c8df6ed8dd7321608ce652e23d86ef3bd6899 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 23 Mar 2017 12:01:12 +0100 Subject: 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| -- cgit v1.2.1 From 01d159b409d8b24d36204979a73de249843d71bf Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 23 Mar 2017 14:00:41 +0100 Subject: Rename container image model to container repository --- app/models/container_image.rb | 62 ---------------------- app/models/container_repository.rb | 62 ++++++++++++++++++++++ .../container_registry_authentication_service.rb | 2 +- .../20170322013926_create_container_image.rb | 13 ----- .../20170322013926_create_container_repository.rb | 12 +++++ db/schema.rb | 10 ++-- 6 files changed, 79 insertions(+), 82 deletions(-) delete mode 100644 app/models/container_image.rb create mode 100644 app/models/container_repository.rb delete mode 100644 db/migrate/20170322013926_create_container_image.rb create mode 100644 db/migrate/20170322013926_create_container_repository.rb diff --git a/app/models/container_image.rb b/app/models/container_image.rb deleted file mode 100644 index 434302159b0..00000000000 --- a/app/models/container_image.rb +++ /dev/null @@ -1,62 +0,0 @@ -class ContainerImage < ActiveRecord::Base - belongs_to :project - - delegate :container_registry, to: :project - delegate :client, to: :container_registry - - validates :manifest, presence: true - - before_destroy :delete_tags - - def registry - # TODO, container registry with image access level - token = Auth::ContainerRegistryAuthenticationService.image_token(self) - 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 - - digests = tags.map {|tag| tag.digest }.to_set - digests.all? do |digest| - client.delete_repository_tag(name_with_namespace, digest) - end - end - - def self.project_from_path(image_path) - return unless image_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('/')) - - ## - # 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/models/container_repository.rb b/app/models/container_repository.rb new file mode 100644 index 00000000000..2e78fc148b4 --- /dev/null +++ b/app/models/container_repository.rb @@ -0,0 +1,62 @@ +class ContainerRepository < ActiveRecord::Base + belongs_to :project + + delegate :container_registry, to: :project + delegate :client, to: :container_registry + + validates :manifest, presence: true + + before_destroy :delete_tags + + def registry + # TODO, container registry with image access level + token = Auth::ContainerRegistryAuthenticationService.image_token(self) + end + + def tag(tag) + ContainerRegistry::Tag.new(self, tag) + end + + def manifest + @manifest ||= client.repository_tags(self.path) + 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 + + digests = tags.map {|tag| tag.digest }.to_set + digests.all? do |digest| + client.delete_repository_tag(self.path, digest) + end + end + + 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 = 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(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_image.rb deleted file mode 100644 index c494f2a56c7..00000000000 --- a/db/migrate/20170322013926_create_container_image.rb +++ /dev/null @@ -1,13 +0,0 @@ -class CreateContainerImage < ActiveRecord::Migration - include Gitlab::Database::MigrationHelpers - - DOWNTIME = false - - def change - create_table :container_images do |t| - t.integer :project_id - t.string :name - t.string :path - end - end -end diff --git a/db/migrate/20170322013926_create_container_repository.rb b/db/migrate/20170322013926_create_container_repository.rb new file mode 100644 index 00000000000..0235fd7e096 --- /dev/null +++ b/db/migrate/20170322013926_create_container_repository.rb @@ -0,0 +1,12 @@ +class CreateContainerRepository < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + create_table :container_repositories do |t| + t.integer :project_id + t.string :path + end + 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 -- cgit v1.2.1 From ea16ea5bfcb78f66c6bb37e470d387bf1ac26c9f Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 23 Mar 2017 14:09:01 +0100 Subject: 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| -- cgit v1.2.1 From f451173a191474be681d208eceb6a0148ba2c0d0 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 23 Mar 2017 14:37:17 +0100 Subject: 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 -- cgit v1.2.1 From 9c9aac3d16cfbbcbe24b9b33eb7a6d4c14c24f26 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 23 Mar 2017 09:46:05 -0400 Subject: 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; } -- cgit v1.2.1 From 249084b48a86a99c02eefe45b507ebcdf811c20f Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 23 Mar 2017 14:48:24 +0100 Subject: 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, {}) } -- cgit v1.2.1 From 3e01fed5cb36962065f5d19ab6a0cef1dfc14b48 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 23 Mar 2017 14:57:33 +0100 Subject: 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 -- cgit v1.2.1 From dcd2eeb1cfb633f4a28ddd9bc79deac0e3171d3f Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 23 Mar 2017 15:54:59 +0100 Subject: 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 ++++---- spec/features/security/project/internal_access_spec.rb | 4 ++-- spec/features/security/project/private_access_spec.rb | 4 ++-- spec/features/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 ++-- spec/services/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) } -- cgit v1.2.1 From af42dd29a0e81d524731f4ce3ced2ed17bac9903 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 24 Mar 2017 12:31:34 +0100 Subject: 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 -- cgit v1.2.1 From 7db1f22673f1b6890b6ce4b9db9b367eae3988f0 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 24 Mar 2017 12:41:42 +0100 Subject: Fix specs for services related to container registry --- .../container_images/destroy_service_spec.rb | 66 ++++++++++++---------- spec/services/projects/destroy_service_spec.rb | 2 +- spec/services/projects/transfer_service_spec.rb | 2 +- 3 files changed, 39 insertions(+), 31 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 } - - 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 - project.team << [user, :master] - end - - it 'removes all tags before destroy' do - service = described_class.new(project, user) - - 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_repository).to receive(:delete_tags).and_return(false) - expect { service.execute(container_repository) }.to raise_error(ActiveRecord::RecordNotDestroyed) - end +describe ContainerImages::DestroyService, '#execute', :services do + let(:user) { create(:user) } + + let(:container_repository) do + create(:container_repository, name: 'myimage', tags: %w[latest]) + end + + let(:project) do + create(:project, path: 'test', + namespace: user.namespace, + container_repositories: [container_repository]) + end + + it { expect(container_repository).to be_valid } + it { expect(project.container_repositories).not_to be_empty } + + context 'when container image has tags' do + before do + project.add_master(user) + end + + it 'removes all tags before destroy' do + service = described_class.new(project, user) + + 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) } -- cgit v1.2.1 From 4fdcaca6a148475a18aa5931b62c4173301ded8c Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 24 Mar 2017 13:08:21 +0100 Subject: Fix container registry blob specs --- spec/lib/container_registry/blob_spec.rb | 114 ++++++++++++++++--------------- 1 file changed, 59 insertions(+), 55 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(:group) { create(:group, name: 'group') } + let(:project) { create(:project, path: 'test', group: group) } + + let(:repository) do + create(:container_repository, name: 'image', + tags: %w[latest rc1], + project: project) + end + let(:config) do - { - 'digest' => digest, + { 'digest' => 'sha256:0123456789012345', 'mediaType' => 'binary', - 'size' => 1000 - } + '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(: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 -- cgit v1.2.1 From 16e645c6563b81ce03e481b094abc2d331d05f36 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 24 Mar 2017 13:27:05 +0100 Subject: 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 } -- cgit v1.2.1 From d6f37a34c1c262f49a92f26dd187819419d56c2f Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 24 Mar 2017 13:27:18 +0100 Subject: 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 -- cgit v1.2.1 From dce706bfcbddf7952e2c42c0c42825044cbb43a2 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 24 Mar 2017 13:47:29 +0100 Subject: 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?('/') -- cgit v1.2.1 From 7ada193e0fd28b4a6eca1fda7dda6f0ebe6b2d72 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 24 Mar 2017 14:58:25 +0100 Subject: 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) -- cgit v1.2.1 From f09e3fe85116743c0cb537fb958a8b0ab1ad19fb Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 27 Mar 2017 16:04:38 +0200 Subject: 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 -- cgit v1.2.1 From 576ffef6c1ddff3493099f088a21c867477d9160 Mon Sep 17 00:00:00 2001 From: Kushal Pandya Date: Tue, 14 Mar 2017 16:51:24 +0530 Subject: 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("") -- cgit v1.2.1 From 25ac232f749243aded03acefd247374de39a5938 Mon Sep 17 00:00:00 2001 From: Kushal Pandya Date: Tue, 14 Mar 2017 16:57:16 +0530 Subject: 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: -- cgit v1.2.1 From 625af1684f5f0487d07f12b20d6c33e30c54b969 Mon Sep 17 00:00:00 2001 From: Kushal Pandya Date: Tue, 14 Mar 2017 17:18:42 +0530 Subject: 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: -- cgit v1.2.1 From aa637e69e04f92f0f8e8178a919ada5df088a186 Mon Sep 17 00:00:00 2001 From: Kushal Pandya Date: Tue, 28 Mar 2017 11:02:57 +0530 Subject: 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); } }); -- cgit v1.2.1 From a055622ae0b41550968da4269dd312844fcf6196 Mon Sep 17 00:00:00 2001 From: Kushal Pandya Date: Tue, 28 Mar 2017 11:45:57 +0530 Subject: 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 -- cgit v1.2.1 From b15d9042e2688f29b002f90e0154e793ff1544ff Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 28 Mar 2017 14:34:56 +0200 Subject: 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 -- cgit v1.2.1 From bdc1e1b9e005eeaf564b79698d61a801c3c6360f Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 28 Mar 2017 14:57:22 +0200 Subject: 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 -- cgit v1.2.1 From f6f56dddfcfed352dfba8dc32dad554096552a7d Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Mon, 27 Mar 2017 14:14:12 +0100 Subject: Add back expandable folders behavior --- .../javascripts/environments/components/environments_table.js | 9 +++++++++ app/assets/javascripts/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) { + + } } -- cgit v1.2.1 From adec9194ef9b825a3a79dc262975987012639f23 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Mon, 27 Mar 2017 17:37:26 +0100 Subject: Adds expandable folder back. Makes request to get environments --- .../environments/components/environment.js | 29 +++++++++++++++-- .../environments/components/environment_item.js | 35 +++++++++++++------- .../environments/components/environments_table.js | 3 +- .../environments/services/environments_service.js | 14 +++++++- .../environments/stores/environments_store.js | 38 ++++++++++++++++++++-- 5 files changed, 100 insertions(+), 19 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}} - + + + +