From 7aec8c873e459403797d67a6c9c2647001f395af Mon Sep 17 00:00:00 2001 From: Quinn James Date: Mon, 25 Sep 2023 15:21:34 +0000 Subject: [PATCH] Fixes #36776 - Fixed issue where composite content views promoted to lifecycle environments referencing the env name would error on promotion. Changed env replacement source. Force pushing to rerun tests. Rubocop fixes Changed several factors for how the container name is rendered. Documented thought process. Fixed existing unit test to test on new naming schema instead of testing for overlap with a poor env pattern. Rubocop fixes Added test to check for docker repo container name overlap. Modified documentation to rerun tests. Rubocop fixes --- app/models/katello/repository.rb | 37 ++++++++++++++++++++++++++----- test/models/content_view_test.rb | 38 ++++++++++++++++++++++++++------ 2 files changed, 62 insertions(+), 13 deletions(-) diff --git a/app/models/katello/repository.rb b/app/models/katello/repository.rb index 6888d9e0b5a..d1cb0f6e323 100644 --- a/app/models/katello/repository.rb +++ b/app/models/katello/repository.rb @@ -860,20 +860,45 @@ def self.search_by_content_label(_key, operator, value) end def self.safe_render_container_name(repository, pattern = nil) - if (pattern && !pattern.blank?) || (repository.environment && !repository.environment.registry_name_pattern.empty?) + # pattern provided / env pattern provided + # | 00 | 01 | 11 | 10 + # ----------------+-----+-----+-----+------ + # env exists / 00| 4 | n/a | n/a | 4 + # is cv default 01| 2 | n/a | n/a | 4 + # 11| 2 | 1 | 1 | 1 + # 10| 3 | 1 | 1 | 1 + # + # This table shows the name to render given the properties of + # the container provided. Branches numbered as ordered below. + # + # 1 - Render promotion pattern (or env pattern if no promo pattern) + # 2 - -- + # 3 - ---- + # 4 - ---- + is_pattern_provided = pattern.present? + env_exists = repository.environment.present? + is_env_pattern_provided = env_exists && repository.environment.registry_name_pattern.present? + is_cv_default = repository.content_view.default? + + if is_env_pattern_provided || (is_pattern_provided && env_exists) pattern ||= repository.environment.registry_name_pattern allowed_methods = {} allowed_vars = {} - scope_variables = {repository: repository, organization: repository.organization, product: repository.product, - lifecycle_environment: repository.environment, content_view: repository.content_view_version.content_view, - content_view_version: repository.content_view_version} + scope_variables = { + repository: repository, + organization: repository.organization, + product: repository.product, + lifecycle_environment: repository.environment, + content_view: repository.content_view_version.content_view, + content_view_version: repository.content_view_version + } box = Safemode::Box.new(repository, allowed_methods) erb = ERB.new(pattern) pattern = box.eval(erb.src, allowed_vars, scope_variables) return Repository.clean_container_name(pattern) - elsif repository.content_view.default? + elsif is_cv_default && !is_pattern_provided items = [repository.organization.label, repository.product.label, repository.label] - elsif repository.environment + elsif env_exists items = [repository.organization.label, repository.environment.label, repository.content_view.label, repository.product.label, repository.label] else items = [repository.organization.label, repository.content_view.label, repository.content_view_version.version, repository.product.label, repository.label] diff --git a/test/models/content_view_test.rb b/test/models/content_view_test.rb index 90f580c0154..8570d50f83d 100644 --- a/test/models/content_view_test.rb +++ b/test/models/content_view_test.rb @@ -375,7 +375,28 @@ def test_docker_repo_conflicts assert composite.errors.full_messages.first =~ /^Container Image repo '#{repo.name}' is present in multiple/ end - def test_docker_repo_container_names + def test_docker_repo_container_name_overlap + product = create(:katello_product, provider: @organization.anonymous_provider, organization: @organization) + @dev.registry_name_pattern = "abcdef" + + repo1_lib = create(:docker_repository, product: product, content_view_version: @organization.default_content_view.versions.first) + view1 = create(:katello_content_view, organization: @organization) + view1.repositories << repo1_lib + repo1_cv = create(:docker_repository, product: product, content_view_version: @organization.default_content_view.versions.first, library_instance_id: repo1_lib.id, environment: @dev) + + repo2_lib = create(:docker_repository, product: product, content_view_version: @organization.default_content_view.versions.first) + view2 = create(:katello_content_view, organization: @organization) + view2.repositories << repo2_lib + + assert_equal repo1_cv.container_repository_name, "abcdef" + error = assert_raises(ActiveRecord::RecordInvalid) do + create(:docker_repository, product: product, content_view_version: @organization.default_content_view.versions.first, library_instance_id: repo2_lib.id, environment: @dev) + end + match_regex = /.*Container repository name for repository .* is not unique and cannot be created in .*\. Its Container .* conflicts with an existing repository\..*/ + assert_match match_regex, error.message + end + + def test_docker_repo_container_name_resolution # rubocop:disable Metrics/AbcSize composite = ContentView.find(katello_content_views(:composite_view).id) product = create(:katello_product, provider: @organization.anonymous_provider, organization: @organization) @@ -391,15 +412,18 @@ def test_docker_repo_container_names repo2_cv = build(:docker_repository, product: product, content_view_version: @organization.default_content_view.versions.first, library_instance_id: repo2_lib.id) version2 = create(:katello_content_view_version, :content_view => view2, :repositories => [repo2_cv]) - composite.update(component_ids: [version1.id]) + composite.update(component_ids: [version1.id, version2.id]) assert composite.valid? - @dev.registry_name_pattern = "abcdef" assert composite.check_docker_repository_names!([@dev]) - composite.update(component_ids: [version1.id, version2.id]) - assert composite.valid? - assert_raises(RuntimeError) do - composite.check_docker_repository_names!([@dev]) + # ensure that all containers are stored with the proper auto-generated names + [version1, version2].each do |version| + name = "#{@dev.organization.name.downcase.sub!(" ", "_")}-"\ + "#{version.content_view.label.downcase}-"\ + "#{version.content_view.content_view_versions[0].version.sub!(".", "_")}-"\ + "#{version.repositories[0].product.label.downcase}-"\ + "#{version.repositories[0].name.downcase.sub!(" ", "_")}" + assert_equal name, version.repositories[0].container_repository_name end end