Merge pull request #5252 from parthaa/fix-protected

Fixes #10620 - Repos no longer protected by default

app/controllers/katello/api/v2/repositories_controller.rb

@@ -89,9 +89,9 @@ def create
       end
       repo_params[:label] = labelize_params(repo_params)
       repo_params[:url] = nil if repo_params[:url].blank?
-
+      unprotected =  repo_params.key?(:unprotected) ? repo_params[:unprotected] : true
       repository = @product.add_repo(repo_params[:label], repo_params[:name], repo_params[:url],
-                                     repo_params[:content_type], repo_params[:unprotected],
+                                     repo_params[:content_type], unprotected,
                                      gpg_key, repository_params[:checksum_type])
       repository.docker_upstream_name = repo_params[:docker_upstream_name] if repo_params[:docker_upstream_name]
       sync_task(::Actions::Katello::Repository::Create, repository, false, true)

app/models/katello/repository.rb

@@ -61,6 +61,7 @@ class Repository < Katello::Model
       :with => /^([a-z0-9\-_]{4,30}\/)?[a-z0-9\-_\.]{3,30}$/,
       :message => (_("must be a valid docker name"))
     }
+
     #validates :content_id, :presence => true #add back after fixing add_repo orchestration
     validates_with Validators::KatelloLabelFormatValidator, :attributes => :label
     validates_with Validators::KatelloNameFormatValidator, :attributes => :name
@@ -75,6 +76,7 @@ class Repository < Katello::Model
       :message => (_("must be one of the following: %s") % TYPES.join(', '))
     }
     validate :ensure_valid_docker_attributes, :if => :docker?
+    validate :ensure_docker_repo_unprotected, :if => :docker?
 
     # TODO: remove this default scope
     # rubocop:disable Rails/DefaultScope
@@ -515,5 +517,12 @@ def ensure_valid_docker_attributes
         errors.add(:base, N_("Repository URL or Upstream Name is empty. Both are required for syncing from the upstream."))
       end
     end
+
+    def ensure_docker_repo_unprotected
+      unless unprotected
+        errors.add(:base, N_("Docker Repositories are not protected at this time. " \
+                             "They need to be published via http to be available to containers."))
+      end
+    end
   end
 end

test/controllers/api/v2/repositories_controller_test.rb

@@ -204,7 +204,7 @@ def test_create
         'Fedora Repository',
         'http://www.google.com',
         'yum',
-        nil,
+        true,
         nil,
         nil
       ])
@@ -213,6 +213,7 @@ def test_create
       product.expect(:gpg_key, nil)
       product.expect(:organization, @organization)
       product.expect(:redhat?, false)
+      product.expect(:unprotected?, true)
       assert_sync_task(::Actions::Katello::Repository::Create, @repository, false, true)
 
       Product.stub(:find, product) do
@@ -233,7 +234,7 @@ def test_create_with_empty_string_url
         'Fedora Repository',
         nil,
         'yum',
-        nil,
+        true,
         nil,
         nil
       ])
@@ -267,7 +268,7 @@ def test_create_with_gpg_key
         'Fedora Repository',
         'http://www.google.com',
         'yum',
-        nil,
+        true,
         key,
         nil
       ])
@@ -293,7 +294,7 @@ def test_create_with_checksum
         'Fedora Repository',
         nil,
         'yum',
-        nil,
+        true,
         nil,
         'sha256'
       ])
@@ -316,6 +317,70 @@ def test_create_with_checksum
       end
     end
 
+    def test_create_with_protected_true
+      product = MiniTest::Mock.new
+      product.expect(:add_repo, @repository, [
+        'Fedora_Repository',
+        'Fedora Repository',
+        'http://www.google.com',
+        'yum',
+        false,
+        nil,
+        nil
+      ])
+
+      product.expect(:editable?, @product.editable?)
+      product.expect(:gpg_key, nil)
+      product.expect(:organization, @organization)
+      product.expect(:redhat?, false)
+      product.expect(:unprotected?, false)
+      assert_sync_task(::Actions::Katello::Repository::Create, @repository, false, true)
+
+      Product.stub(:find, product) do
+        post :create, :name => 'Fedora Repository',
+                      :product_id => @product.id,
+                      :url => 'http://www.google.com',
+                      :content_type => 'yum',
+                      :unprotected => false
+
+        assert_response :success
+        assert_template 'api/v2/repositories/show'
+      end
+    end
+
+    def test_create_with_protected_docker
+      docker_upstream_name = "busybox"
+      product = MiniTest::Mock.new
+      product.expect(:add_repo, @repository, [
+        'Fedora_Repository',
+        'Fedora Repository',
+        'http://hub.registry.com',
+        'docker',
+        true,
+        nil,
+        nil
+      ])
+
+      product.expect(:editable?, @product.editable?)
+      product.expect(:gpg_key, nil)
+      product.expect(:organization, @organization)
+      product.expect(:redhat?, false)
+      product.expect(:unprotected?, true)
+      product.expect(:docker_upstream_name, docker_upstream_name)
+      assert_sync_task(::Actions::Katello::Repository::Create, @repository, false, true)
+
+      Product.stub(:find, product) do
+        post :create, :name => 'Fedora Repository',
+                      :product_id => @product.id,
+                      :url => 'http://hub.registry.com',
+                      :content_type => 'docker',
+                      :docker_upstream_name => "busybox"
+
+        assert_response :success
+        assert_template 'api/v2/repositories/show'
+      end
+    end
+
     def test_create_without_label_or_name
       post :create, :product_id => @product.id
       # TODO: fix this test, returns 500 because of undefined method add_repo for Katello::Product

test/fixtures/models/katello_repositories.yml

@@ -220,6 +220,7 @@ redis:
   product_id:           <%= ActiveRecord::Fixtures.identify(:puppet_product) %>
   gpg_key_id:           <%= ActiveRecord::Fixtures.identify(:fedora_gpg_key) %>
   content_view_version_id: <%= ActiveRecord::Fixtures.identify(:library_default_version) %>
+  unprotected:           <%= true %>
 
 busybox:
   name:                 busybox
@@ -232,6 +233,7 @@ busybox:
   product_id:           <%= ActiveRecord::Fixtures.identify(:puppet_product) %>
   gpg_key_id:           <%= ActiveRecord::Fixtures.identify(:fedora_gpg_key) %>
   content_view_version_id: <%= ActiveRecord::Fixtures.identify(:library_default_version) %>
+  unprotected:           <%= true %>
 
 busybox_view1:
   name:                 busybox
@@ -245,6 +247,7 @@ busybox_view1:
   product_id:           <%= ActiveRecord::Fixtures.identify(:puppet_product) %>
   gpg_key_id:           <%= ActiveRecord::Fixtures.identify(:fedora_gpg_key) %>
   content_view_version_id: <%= ActiveRecord::Fixtures.identify(:library_view_version_1) %>
+  unprotected:           <%= true %>
 
 busybox_view2:
   name:                 busybox
@@ -258,6 +261,7 @@ busybox_view2:
   product_id:           <%= ActiveRecord::Fixtures.identify(:puppet_product) %>
   gpg_key_id:           <%= ActiveRecord::Fixtures.identify(:fedora_gpg_key) %>
   content_view_version_id: <%= ActiveRecord::Fixtures.identify(:library_view_version_2) %>
+  unprotected:           <%= true %>
 
 rhel_6_x86_64_composite_view_version_1:
   name:                 RHEL 6 x86_64

test/models/repository_test.rb

@@ -35,6 +35,7 @@ def test_unique_repository_name_per_product_and_environment
     end
 
     def test_docker_repository_docker_upstream_name_url
+      @repo.unprotected = true
       @repo.content_type = 'docker'
       @repo.docker_upstream_name = ""
       @repo.url = "http://registry.com"
@@ -52,6 +53,7 @@ def test_docker_repository_docker_upstream_name_url
     end
 
     def test_docker_repository_docker_upstream_name_format
+      @repo.unprotected = true
       @repo.content_type = 'docker'
       @repo.docker_upstream_name = 'valid'
       assert @repo.valid?
@@ -109,10 +111,22 @@ def test_docker_pulp_id
       @repo.pulp_id = 'PULP-ID'
       @repo.content_type = Repository::DOCKER_TYPE
       @repo.docker_upstream_name = "haha"
+      @repo.unprotected = true
       assert @repo.save
       assert @repo.pulp_id.ends_with?('pulp-id')
     end
 
+    def test_docker_repo_unprotected
+      @repo.name = 'docker_repo'
+      @repo.pulp_id = 'PULP-ID'
+      @repo.content_type = Repository::DOCKER_TYPE
+      @repo.docker_upstream_name = "haha"
+      @repo.unprotected = true
+      assert @repo.save
+      @repo.unprotected = false
+      refute @repo.save
+    end
+
     def test_yum_type_pulp_id
       @repo.pulp_id = 'PULP-ID'
       @repo.content_type = Repository::YUM_TYPE