Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fixes #17949 - adjust generation of pulp_id
This changes how we generate a pulp_id.  Now
for all library instances, we generate a random uuid
and then for each clone we construct a combination of
org id, env label, cv label, and the library instance's uuid.

This also seperates the pulp_id and the docker repository name in pulp
allowing for these to be different
  • Loading branch information
jlsherrill committed Feb 16, 2017
1 parent 0919536 commit 7e3bb1b
Show file tree
Hide file tree
Showing 19 changed files with 173 additions and 115 deletions.
3 changes: 2 additions & 1 deletion app/lib/actions/katello/repository/create.rb
Expand Up @@ -30,7 +30,8 @@ def plan(repository, clone = false, plan_create = false)
mirror_on_sync: repository.mirror_on_sync?,
ssl_validation: certs[:ssl_validation],
upstream_username: repository.upstream_username,
upstream_password: repository.upstream_password)
upstream_password: repository.upstream_password,
repo_registry_id: repository.container_repository_name)

return if create_action.error

Expand Down
2 changes: 0 additions & 2 deletions app/lib/actions/katello/repository_set/scan_cdn.rb
Expand Up @@ -59,7 +59,6 @@ def prepare_results_docker_content
substitutions: {},
path: mapper.feed_url,
repo_name: mapper.name,
pulp_id: mapper.pulp_id,
registry_name: registry["name"],
enabled: !repo.nil?,
promoted: (!repo.nil? && repo.promoted?)
Expand All @@ -74,7 +73,6 @@ def prepare_result(substitutions, _path)
path: mapper.path,
repo_name: mapper.name,
name: mapper.content.name,
pulp_id: mapper.pulp_id,
enabled: !repo.nil?,
promoted: (!repo.nil? && repo.promoted?)
}
Expand Down
3 changes: 2 additions & 1 deletion app/lib/actions/pulp/repository/create.rb
Expand Up @@ -162,7 +162,8 @@ def puppet_install_distributor
def docker_distributor
options = { protected: !input[:unprotected] || false,
id: input[:pulp_id],
auto_publish: true }
auto_publish: true,
repo_registry_id: input[:repo_registry_id]}
Runcible::Models::DockerDistributor.new(options)
end

Expand Down
14 changes: 3 additions & 11 deletions app/models/katello/candlepin/content.rb
Expand Up @@ -46,8 +46,10 @@ def initialize(product, content, substitutions)

def find_repository
::Katello::Repository.where(product_id: product.id,
content_id: content.id,
environment_id: product.organization.library.id,
pulp_id: pulp_id).first
minor: minor,
arch: arch).first
end

def build_repository
Expand All @@ -56,7 +58,6 @@ def build_repository
repository = Repository.new(
:environment => product.organization.library,
:product => product,
:pulp_id => pulp_id,
:cp_label => content.label,
:content_id => content.id,
:arch => arch,
Expand Down Expand Up @@ -107,10 +108,6 @@ def name
repo_name_parts.join(" ").gsub(/[^a-z0-9\-\._ ]/i, "")
end

def pulp_id
product.repo_id(name)
end

def path
substitutions.inject(content.contentUrl) do |url, (key, value)|
url.gsub("$#{key}", value)
Expand Down Expand Up @@ -212,7 +209,6 @@ def build_repository
end
::Katello::Repository.new(:environment => product.organization.library,
:product => product,
:pulp_id => pulp_id,
:cp_label => content.label,
:content_id => content.id,
:relative_path => relative_path,
Expand All @@ -237,10 +233,6 @@ def name
"#{content.name} - (#{registry['name']})"
end

def pulp_id
product.repo_id(content.name, nil, registry['name'])
end

def feed_url
cdn_uri = URI.parse(product.provider.repository_url)
docker_repo_uri = URI.parse(registry["url"])
Expand Down
7 changes: 1 addition & 6 deletions app/models/katello/content_view.rb
Expand Up @@ -476,15 +476,10 @@ def build_puppet_env(options)
content_view = self
to_version = version || content_view.version(to_env)

# Construct the pulp id using org/view/version or org/env/view
pulp_id = ContentViewPuppetEnvironment.generate_pulp_id(organization.label, to_env.try(:label),
self.label, version.try(:version))

ContentViewPuppetEnvironment.new(
:environment => to_env,
:content_view_version => to_version,
:name => self.name,
:pulp_id => pulp_id
:name => self.name
)
end

Expand Down
12 changes: 10 additions & 2 deletions app/models/katello/content_view_puppet_environment.rb
Expand Up @@ -27,6 +27,8 @@ class ContentViewPuppetEnvironment < Katello::Model
validates_with Validators::KatelloNameFormatValidator, :attributes => :name
validates :puppet_environment, :presence => true, :if => :environment

before_validation :set_pulp_id

scope :non_archived, -> { where('environment_id is not NULL') }
scope :archived, -> { where('environment_id is NULL') }

Expand Down Expand Up @@ -84,8 +86,14 @@ def generate_puppet_env_name
end
end

def self.generate_pulp_id(organization_label, env_label, view_label, version)
[organization_label, env_label, view_label, version].compact.join("-").gsub(/[^-\w]/, "_")
def set_pulp_id
if self.environment
label = "#{self.content_view.label}-#{self.environment.label}-puppet-#{SecureRandom.uuid}"
else
version = self.content_view_version.version.gsub('.', '_')
label = "#{self.content_view.label}-v#{version}-puppet-#{SecureRandom.uuid}"
end
self.pulp_id ||= "#{self.organization.id}-#{label}"
end

def index_content(puppet_module_uuids)
Expand Down
5 changes: 3 additions & 2 deletions app/models/katello/glue/pulp/repo.rb
Expand Up @@ -270,7 +270,8 @@ def generate_distributors(capsule = nil)

distributors = [puppet_dist, puppet_install_dist]
when Repository::DOCKER_TYPE
options = { :protected => !self.unprotected, :id => self.pulp_id, :auto_publish => true}
options = { :protected => !self.unprotected, :id => self.pulp_id, :auto_publish => true,
:repo_registry_id => container_repository_name}
docker_dist = Runcible::Models::DockerDistributor.new(options)
distributors = [docker_dist]
when Repository::OSTREE_TYPE
Expand Down Expand Up @@ -690,7 +691,7 @@ def full_path(smart_proxy = nil, force_https = false)
pulp_uri = URI.parse(smart_proxy ? smart_proxy.url : SETTINGS[:katello][:pulp][:url])
scheme = (self.unprotected && !force_https) ? 'http' : 'https'
if docker?
"#{pulp_uri.host.downcase}:#{Setting['pulp_docker_registry_port']}/#{pulp_id}"
"#{pulp_uri.host.downcase}:#{Setting['pulp_docker_registry_port']}/#{container_repository_name}"
elsif file?
"#{scheme}://#{pulp_uri.host.downcase}/pulp/isos/#{pulp_id}/"
elsif puppet?
Expand Down
8 changes: 0 additions & 8 deletions app/models/katello/glue/pulp/repos.rb
Expand Up @@ -205,13 +205,6 @@ def sync_size
end
end

def repo_id(content_name, env_label = nil, docker_repo_name = nil)
return if content_name.nil?
return content_name if content_name.include?(self.organization.label) && content_name.include?(self.label.to_s)
Repository.repo_id(self.label.to_s, content_name.to_s, env_label,
self.organization.label, nil, nil, docker_repo_name)
end

def repo_url(content_url)
if self.provider.provider_type == Provider::CUSTOM
content_url.dup
Expand All @@ -234,7 +227,6 @@ def add_repo(label, name, url, repo_type, unprotected = false, gpg = nil, checks
end
Repository.new(:environment => self.organization.library,
:product => self,
:pulp_id => repo_id(label),
:relative_path => rel_path,
:arch => arch,
:name => name,
Expand Down
59 changes: 34 additions & 25 deletions app/models/katello/repository.rb
Expand Up @@ -3,6 +3,9 @@ module Katello
class Repository < Katello::Model
self.include_root_in_json = false

#pulp uses pulp id to sync with 'yum_distributor' on the end
PULP_ID_MAX_LENGTH = 220

validates_lengths_from_database :except => [:label]
before_destroy :assert_deletable
before_create :downcase_pulp_id
Expand Down Expand Up @@ -108,6 +111,9 @@ class Repository < Katello::Model
validate :ensure_has_url_for_ostree, :if => :ostree?
validate :ensure_ostree_repo_protected, :if => :ostree?

before_validation :set_pulp_id
before_validation :set_container_repository_name, :if => :docker?

scope :has_url, -> { where('url IS NOT NULL') }
scope :in_default_view, -> { joins(:content_view_version => :content_view).where("#{Katello::ContentView.table_name}.default" => true) }

Expand Down Expand Up @@ -142,6 +148,34 @@ def organization
end
end

def set_pulp_id
return if self.pulp_id

if self.content_view.default?
items = [SecureRandom.uuid]
elsif self.environment
items = [organization.id, content_view.label, environment.label, library_instance.pulp_id]
else
version = self.content_view_version.version.gsub('.', '_')
items = [organization.id, content_view.label, "v#{version}", library_instance.pulp_id]
end
self.pulp_id = items.join('-')
self.pulp_id = SecureRandom.uuid if self.pulp_id.length > PULP_ID_MAX_LENGTH
end

def set_container_repository_name
return if container_repository_name
if content_view.default?
items = [organization.label, product.label, label]
elsif environment
items = [organization.label, environment.label, content_view.label, product.label, label]
else
items = [organization.label, content_view.label, content_view_version.version, product.label, label]
end
# docker repo names need to be in lower case
self.container_repository_name = items.compact.join("-").gsub(/[^-\w]/, "_").downcase
end

def content_view
self.content_view_version.content_view
end
Expand Down Expand Up @@ -324,26 +358,6 @@ def self.clone_docker_repo_path(options)
end
end

def self.repo_id(product_label, repo_label, env_label, organization_label,
view_label, version, docker_repo_name = nil)
actual_repo_id = [organization_label,
env_label,
view_label,
version,
product_label,
repo_label,
docker_repo_name].compact.join("-").gsub(/[^-\w]/, "_")
# docker repo names need to be in lower case
actual_repo_id = actual_repo_id.downcase if docker_repo_name
actual_repo_id
end

def clone_id(env, content_view, version = nil)
Repository.repo_id(self.product.label, self.label, env.try(:label),
organization.label, content_view.label,
version)
end

def packages_without_errata
if errata_filenames.any?
self.rpms.where("#{Rpm.table_name}.filename NOT in (?)", errata_filenames)
Expand All @@ -361,10 +375,6 @@ def errata_filenames
where("#{RepositoryErratum.table_name}.repository_id" => self.id).pluck("#{Katello::ErratumPackage.table_name}.filename")
end

def container_repository_name
pulp_id if docker?
end

# TODO: break up method
# rubocop:disable MethodLength
def build_clone(options)
Expand Down Expand Up @@ -415,7 +425,6 @@ def build_clone(options)
:download_policy => download_policy,
:unprotected => self.unprotected) do |clone|
clone.checksum_type = self.checksum_type
clone.pulp_id = clone.clone_id(to_env, content_view, version.try(:version))
options = {
:repository => self,
:environment => to_env,
Expand Down
1 change: 1 addition & 0 deletions app/views/katello/api/v2/repositories/base.json.rabl
Expand Up @@ -3,6 +3,7 @@ object @resource
extends 'katello/api/v2/common/identifier'

attributes :content_type, :url, :relative_path
attributes :pulp_id => :backend_identifier

child :product do |_product|
attributes :id, :cp_id, :name
Expand Down
14 changes: 14 additions & 0 deletions db/migrate/20170208215148_add_docker_repo_name.rb
@@ -0,0 +1,14 @@
class AddDockerRepoName < ActiveRecord::Migration
def up
add_column :katello_repositories, :container_repository_name, :string

Katello::Repository.docker_type.find_each do |repo|
repo.set_container_repository_name
repo.save!
end
end

def down
remove_column :katello_repositories, :container_repository_name
end
end
Expand Up @@ -14,6 +14,9 @@ <h4 translate>Basic Information</h4>
<dt translate>Label</dt>
<dd>{{ repository.label }}</dd>

<dt translate>Backend Identifier</dt>
<dd>{{ repository.backend_identifier }}</dd>

<dt translate>Type</dt>
<dd>{{ repository.content_type }}</dd>

Expand Down
22 changes: 0 additions & 22 deletions spec/models/product_spec.rb
Expand Up @@ -105,28 +105,6 @@ module Katello
end
end

describe "product repos" do
before(:each) do
disable_product_orchestration
Katello.pulp_server.extensions.repository.stubs(:publish_all).returns([])
end

describe "repo id" do
before do
Resources::Candlepin::Product.stubs(:create).returns(:id => ProductTestData::PRODUCT_ID)
@p = Product.create!(ProductTestData::SIMPLE_PRODUCT.merge(:organization_id => @organization.id))
end

specify "format" do
@p.repo_id('123', 'root').must_equal("#{@organization.label}-root-#{ProductTestData::SIMPLE_PRODUCT[:label]}-123")
end

it "should be the same as content id for cloned repository" do
@p.repo_id("#{@organization.label}-root-#{ProductTestData::SIMPLE_PRODUCT[:label]}-123").must_equal("#{@organization.label}-root-#{ProductTestData::SIMPLE_PRODUCT[:label]}-123")
end
end
end

describe "#environments" do
it "should contain a unique list of environments" do
disable_repo_orchestration
Expand Down
8 changes: 1 addition & 7 deletions test/actions/katello/docker_repository_set_test.rb
Expand Up @@ -58,13 +58,10 @@ class DockerTestBase < ActiveSupport::TestCase
}
end

let(:expected_pulp_id) { "empty_organization-redhat_label-docker_content_123-#{registry_name}".downcase }
let(:expected_relative_path) { "Empty_Organization/library_label/product/x86_64/6Server" }

def repository_already_enabled!
katello_repositories(:rhel_6_x86_64).
update_attributes!(relative_path: "#{expected_relative_path}",
pulp_id: expected_pulp_id,
katello_repositories(:rhel_6_x86_64).update_attributes!(relative_path: "#{expected_relative_path}",
docker_upstream_name: registry_name)
end
end
Expand Down Expand Up @@ -93,7 +90,6 @@ class DockerScanCdnTest < DockerTestBase
[{ "substitutions" => {},
"path" => "https://#{registry_feed_url}",
"repo_name" => "#{content.name} - (#{registry_name})",
"pulp_id" => expected_pulp_id,
"registry_name" => "dream-registry",
"enabled" => false,
"promoted" => false}])
Expand All @@ -118,7 +114,6 @@ class EnableDockerRepositoryTest < DockerTestBase

it 'plans' do
action.expects(:action_subject).with do |repository|
repository.pulp_id.must_equal expected_pulp_id
repository.docker_upstream_name.must_equal registry_name
repository.url.must_equal "https://#{registry_feed_url}"
end
Expand All @@ -142,7 +137,6 @@ class DisableDockerRepositoryTest < DockerTestBase
repository_already_enabled!

action.expects(:action_subject).with do |repository|
repository.pulp_id.must_equal expected_pulp_id
repository.docker_upstream_name.must_equal registry_name
end
plan_action action, product, content, options
Expand Down

0 comments on commit 7e3bb1b

Please sign in to comment.