Skip to content

Commit

Permalink
Fixes #27418 - Support docker pull with pulp3
Browse files Browse the repository at this point in the history
This supports docker pull with pulp3. Crane (pulp2) is still supported and the
registry URL is dynamically determined based on the support for Docker blobs.

Previously, the RegistryResource class would run code when loaded, setting up
the inherited class variables. This commit moves this and the new logic to a class
method. Since there are now backend calls (to check for pulp3) to set up the class
variables, it seems like this should be a more deliberate action. This also makes testing
much easier.

I'm open to other suggestions on how to handle the class inheritance and the class variables.

To test:
- Modify foreman/config/settings.plugins.d/katello.yaml, adding the pulp_registry_url
```yaml
  :container_image_registry:
    :crane_url: https://localhost:5000
    :pulp_registry_url: http://localhost:24816
    :registry_ca_cert_file: /etc/pki/katello/certs/katello-default-ca.crt
```
- Set up docker on pulp3 box. You will need an up-to-date box since pulp/pulp_docker#391 was merged
- Sync repos, you can use https://cloud.docker.com/u/jomitsch/repository/docker/jomitsch/workflow-test for a small one.
- docker login $HOSTNAME
- docker pull $HOSTNAME/default_organization-docker-workflow_test (check name in docker repo details)
- docker pull should be successful

It's worth trying out some other repos or registries (like quay.io) and also we should ensure pulp2 functionality is not expected

There will be installer changes to come that will update both dev and prod's katello.yml, these will need to be merged along
with this PR

I also updated the rubocop_todo as it was throwing warnings for duplicate entries.
  • Loading branch information
John Mitsch committed Aug 23, 2019
1 parent 257195c commit 03fcc90
Show file tree
Hide file tree
Showing 6 changed files with 91 additions and 21 deletions.
20 changes: 10 additions & 10 deletions app/controllers/katello/api/registry/registry_proxies_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,7 @@ def find_writable_repository

def authorize_repository_write
@repository = find_writable_repository
unless @repository
not_found params[:repository]
return false
end
return item_not_found(params[:repository]) unless @repository
true
end

Expand Down Expand Up @@ -110,18 +107,15 @@ def find_readable_repository

def authorize_repository_read
@repository = find_readable_repository
unless @repository
not_found params[:repository]
return false
end
return item_not_found(params[:repository]) unless @repository

if params[:tag]
if params[:tag][0..6] == 'sha256:'
manifest = Katello::DockerManifestList.where(digest: params[:tag]).first || Katello::DockerManifest.where(digest: params[:tag]).first
not_found params[:tag] unless manifest
return item_not_found(params[:tag]) unless manifest
else
tag = DockerMetaTag.where(repository_id: @repository.id, name: params[:tag]).first
not_found params[:tag] unless tag
return item_not_found(params[:tag]) unless tag
end
end

Expand Down Expand Up @@ -487,5 +481,11 @@ def process_action(method_name, *args)
::Api::V2::BaseController.instance_method(:process_action).bind(self).call(method_name, *args)
Rails.logger.debug "With body: #{response.body}\n" unless route_name == 'pull_blob'
end

def item_not_found(item)
msg = "#{item} was not found!"
# returning errors based on registry specifications in https://docs.docker.com/registry/spec/api/#errors
render json: {errors: [code: :invalid_request, message: msg, details: msg]}, status: :not_found
end
end
end
6 changes: 6 additions & 0 deletions app/lib/katello/errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -140,5 +140,11 @@ def message
end

class UpstreamEntitlementGone < StandardError; end

class ContainerRegistryNotConfigured < StandardError
def message
_("No URL found for a container registry. Please check the configuration.")
end
end
end
end
35 changes: 25 additions & 10 deletions app/lib/katello/resources/registry.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,22 +12,37 @@ def self.logger

def self.get(path, headers = {:accept => :json})
logger.debug "Sending GET request to Registry: #{path}"
client = RegistryResource.rest_client(Net::HTTP::Get, :get, path)
client = RegistryResource.load_class.rest_client(Net::HTTP::Get, :get, path)
client.get(headers)
end
end

class RegistryResource < HttpResource
if SETTINGS[:katello][:container_image_registry]
cfg = SETTINGS[:katello][:container_image_registry]
url = cfg[:crane_url]
uri = URI.parse(url)
self.prefix = uri.path
self.site = "#{uri.scheme}://#{uri.host}:#{uri.port}"
self.ca_cert_file = cfg[:crane_ca_cert_file]
end

class << self
def load_class
container_config = SETTINGS.dig(:katello, :container_image_registry)
registry_url = nil
pulp_master = ::SmartProxy.pulp_master

# Pulp 3 has its own registry
if pulp_master && pulp_master.content_pulp3_support?(::Katello::DockerBlob::CONTENT_TYPE)
registry_url = pulp_master.setting('Pulp3', 'content_app_url')
# Assume the registry uses the same CA as the Smart Proxy
ca_cert_file = Setting[:ssl_ca_file]
elsif container_config
registry_url = container_config[:crane_url]
ca_cert_file = container_config[:registry_ca_cert_file]
end

fail Errors::ContainerRegistryNotConfigured unless registry_url

uri = URI.parse(registry_url)
self.prefix = uri.path
self.site = "#{uri.scheme}://#{uri.host}:#{uri.port}"
self.ca_cert_file = ca_cert_file
self
end

def process_response(response)
debug_level = response.code >= 400 ? :error : :debug
logger.send(debug_level, "Registry request returned with code #{response.code}")
Expand Down
24 changes: 24 additions & 0 deletions test/controllers/api/registry/registry_proxies_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,30 @@ def setup_permissions
assert response.header['Content-Type'] =~ /MEDIATYPE/
assert_equal response.header['Docker-Content-Digest'], "sha256:#{Digest::SHA256.hexdigest(manifest)}"
end

it "pull manifest repo not found" do
@controller.stubs(:registry_authorize).returns(true)
@controller.stubs(:find_readable_repository).returns(nil)

get :pull_manifest, params: { repository: "doesnotexist", tag: "latest" }
assert_response 404
response_body = JSON.parse(response.body)
assert response_body['errors'].length >= 1
response_body['errors'].first.assert_valid_keys('code', 'message', 'details')
end

it "pull manifest repo tag not found" do
manifest = '{"mediaType":"MEDIATYPE"}'
@controller.stubs(:registry_authorize).returns(true)
@controller.stubs(:find_readable_repository).returns(@docker_repo)
Resources::Registry::Proxy.stubs(:get).returns(manifest)

get :pull_manifest, params: { repository: @docker_repo.name, tag: "doesnotexist" }
assert_response 404
response_body = JSON.parse(response.body)
assert response_body['errors'].length >= 1
response_body['errors'].first.assert_valid_keys('code', 'message', 'details')
end
end

describe "docker push" do
Expand Down
3 changes: 2 additions & 1 deletion test/factories/smart_proxy_factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@

smart_proxy_feature = proxy.smart_proxy_features.select { |spf| spf.feature_id == v3_feature.id }.first
smart_proxy_feature.capabilities = plugins
smart_proxy_feature.settings = {pulp_url: "https://#{Socket.gethostname}"}
smart_proxy_feature.settings = { pulp_url: "https://#{Socket.gethostname}",
content_app_url: "http://localhost:24816" }
smart_proxy_feature.save!
end
end
Expand Down
24 changes: 24 additions & 0 deletions test/lib/resources/registry_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
require 'katello_test_helper'

module Katello
module Resources
class RegistryTest < ActiveSupport::TestCase
before do
@crane_url = "https://localhost:5000"
SETTINGS[:katello][:container_image_registry] = {
crane_url: @crane_url
}
end

def test_pulp3_registry_url
pulp_master = FactoryBot.create(:smart_proxy, :default_smart_proxy, :with_pulp3)
::SmartProxy.expects(:pulp_master).at_least_once.returns(pulp_master)
assert_equal Registry::RegistryResource.load_class.site, 'http://localhost:24816'
end

def test_crane_registry_url
assert_equal Registry::RegistryResource.load_class.site, @crane_url
end
end
end
end

0 comments on commit 03fcc90

Please sign in to comment.