Merge pull request #437 from mssola/reachability

Make sure that the registry is reachable when creating it

app/controllers/admin/registries_controller.rb

@@ -15,9 +15,27 @@ def new
   end
 
   # POST /admin/registries
+  #
+  # This method checks whether the given registry is reachable or not. If it
+  # is not, then it will redirect back to the :new page asking for
+  # confirmation. If the :force parameter is passed, then this check is not
+  # done. If this check is passed/skipped, then it will try to create the
+  # registry.
   def create
     @registry = Registry.new(create_params)
 
+    # Check the reachability of the registry.
+    unless params[:force]
+      msg = @registry.reachable?
+      unless msg.empty?
+        logger.info "\nRegistry not reachable:\n#{@registry.inspect}\n#{msg}\n"
+        msg = "#{msg} You can skip this check by clicking on the \"Skip remote checks\" checkbox."
+        hsh = { name: @registry.name, hostname: @registry.hostname, use_ssl: @registry.use_ssl }
+        redirect_to new_admin_registry_path(hsh), alert: msg
+        return
+      end
+    end
+
     if @registry.save
       Namespace.update_all(registry_id: @registry.id)
       redirect_to admin_registries_path, notice: "Registry was successfully created."

app/models/registry.rb

@@ -60,6 +60,41 @@ def get_namespace_from_event(event)
     [namespace, repo, tag_name]
   end
 
+  # Checks whether this registry is reachable. If it is, then an empty string
+  # is returned. Otherwise a string will be returned containing the reasoning
+  # of the reachability failure.
+  def reachable?
+    msg = ""
+
+    begin
+      r = client.reachable?
+
+      # At this point, !r is only possible if the returned code is 404, which
+      # according to the documentation we have to assume that the registry is
+      # not implementing the v2 of the API.
+      return "Error: registry does not implement v2 of the API." unless r
+    rescue Errno::ECONNREFUSED, SocketError
+      msg = "Error: connection refused. The given registry is not available!"
+    rescue Net::HTTPBadResponse
+      if use_ssl
+        msg = "Error: there's something wrong with your SSL configuration."
+      else
+        msg = "Error: not using SSL, but the given registry does use SSL."
+      end
+    rescue OpenSSL::SSL::SSLError
+      if use_ssl
+        msg = "Error: using SSL, but the given registry is not using SSL."
+      else
+        msg = "Error: there's something wrong with your SSL configuration."
+      end
+    rescue StandardError => e
+      # We don't know what went wrong :/
+      logger.info "Registry not reachable: #{e.message}"
+      msg = "Error: something went wrong. Check your configuration."
+    end
+    msg
+  end
+
   protected
 
   # Fetch the tag of the image contained in the current event. The Manifest API

app/views/admin/registries/new.html.slim

@@ -20,15 +20,29 @@ br
   .form-group
     = f.label :name, {class: 'control-label col-md-2'}
     .col-md-7
-      = f.text_field(:name, class: 'form-control', required: true, autofocus: true)
+      - if params[:name]
+        = f.text_field(:name, class: 'form-control', value: params[:name], required: true, autofocus: true)
+      - else
+        = f.text_field(:name, class: 'form-control', required: true, autofocus: true)
   .form-group
     = f.label :hostname, {class: 'control-label col-md-2'}
     .col-md-7
-      = f.text_field(:hostname, class: 'form-control', placeholder: 'registry.test.lan:5000', required: true)
+      - if params[:hostname]
+        = f.text_field(:hostname, class: 'form-control', value: params[:hostname], placeholder: 'registry.test.lan:5000', required: true)
+      - else
+        = f.text_field(:hostname, class: 'form-control', placeholder: 'registry.test.lan:5000', required: true)
   .form-group
     = f.label :use_ssl, "Use SSL", {class: 'control-label col-md-2', title: 'Set this to enable SSL in the communication between Portus and the Registry'}
     .col-md-7
-      = f.check_box(:use_ssl)
+      - if params[:use_ssl] == "true"
+        = f.check_box(:use_ssl, checked: true)
+      - else
+        = f.check_box(:use_ssl)
+  - unless flash[:alert].nil? || flash[:alert].empty?
+    .form-group.has-error
+      = label_tag :force, "Skip remote checks", {class: 'control-label col-md-2', title: "Force the creation of the registry, even if it's not reachable."}
+      .col-md-7
+        = check_box_tag(:force)
   .form-group
     .col-md-offset-2.col-md-7
       .btn-toolbar

bin/client.rb

@@ -9,7 +9,7 @@
 #   - manifest <name>[:<tag>]
 
 registry = Registry.first
-if registry.nil?
+if registry.nil? && ARGV.first != "ping"
   puts "No registry has been configured!"
   exit 1
 end
@@ -35,6 +35,28 @@
     name, tag = ARGV[1], "latest"
   end
   pp registry.client.manifest(name, tag)
+when "ping"
+  # No registry was found, trying to ping another one.
+  if registry.nil?
+    if ARGV.size == 2
+      use_ssl = false
+      puts "Beware: use_ssl omitted, assuming false."
+    elsif ARGV.size == 3
+      use_ssl = ARGV.last == "use_ssl"
+      puts "Beware: use \"use_ssl\", assuming false." unless use_ssl
+    else
+      puts "Usage: rails runner ping hostname:port [use_ssl]"
+      exit 1
+    end
+
+    registry = Registry.new(hostname: ARGV[1], use_ssl: use_ssl)
+  end
+
+  if registry.client.reachable?
+    puts "Registry reachable"
+  else
+    puts "Error: cannot reach the registry"
+  end
 else
   puts "Valid commands: catalog, delete, manifest."
   exit 1

lib/portus/registry_client.rb

@@ -16,6 +16,18 @@ def initialize(host, use_ssl = false, username = nil, password = nil)
       @password = password || Rails.application.secrets.portus_password
     end
 
+    # Returns whether the registry is reachable with the given credentials or
+    # not.
+    def reachable?
+      res = perform_request("", "get", false)
+
+      # If a 401 was retrieved, it means that at least the registry has been
+      # contacted. In order to get a 200, this registry should be created and
+      # an authorization requested. The former can be inconvenient, because we
+      # might want to test whether the registry is reachable.
+      !res.nil? && res.code.to_i == 401
+    end
+
     # Retrieves the manifest for the required repository:tag. If everything goes
     # well, it will return a parsed response from the registry, otherwise it will
     # raise either ManifestNotFoundError or a RuntimeError.

spec/controllers/admin/registries_controller_spec.rb

@@ -27,17 +27,26 @@
   end
 
   describe "POST #create" do
+    context "not using the Force" do
+      it "redirects when there's something wrong with the reachability of the registry" do
+        expect do
+          post :create, registry: attributes_for(:registry)
+        end.to change(Registry, :count).by(0)
+        expect(response).to have_http_status(:redirect)
+      end
+    end
+
     context "no registry" do
       it "creates a new registry" do
         expect do
-          post :create, registry: attributes_for(:registry)
+          post :create, registry: attributes_for(:registry), force: true
         end.to change(Registry, :count).by(1)
       end
 
       it "assigns the freshly created registry to all the existing namespaces" do
         3.times { create(:team) }
 
-        post :create, registry: attributes_for(:registry)
+        post :create, registry: attributes_for(:registry), force: true
         registry = Registry.last
 
         Namespace.all.each { |n| expect(n.registry).to eq(registry) }
@@ -49,15 +58,15 @@
         create(:registry)
 
         expect do
-          post :create, registry: attributes_for(:registry)
+          post :create, registry: attributes_for(:registry), force: true
         end.to raise_error(ActionController::RoutingError)
       end
     end
 
     context "wrong params" do
       it "redirects to the new page" do
         expect do
-          post :create, registry: { name: "foo" }
+          post :create, registry: { name: "foo" }, force: true
         end.to change(Registry, :count).by(0)
       end
     end

spec/features/admin/registries_spec.rb

@@ -14,6 +14,31 @@
     end
   end
 
+  describe "create" do
+    it "shows an alert on error, and you can force it afterwards", js: true do
+      visit new_admin_registry_path
+      expect(page).to_not have_content("Skip remote checks")
+      fill_in "registry_name", with: "registry"
+      fill_in "registry_hostname", with: "url_not_known:1234"
+      click_button "Create"
+
+      expect(page).to have_content("Skip remote checks")
+      expect(page).to have_content("something went wrong")
+      expect(Registry.any?).to be_falsey
+
+      # Use the force, Luke.
+
+      fill_in "registry_name", with: "registry"
+      fill_in "registry_hostname", with: "url_not_known:1234"
+      check "force"
+      click_button "Create"
+
+      expect(current_path).to eq admin_registries_path
+      expect(page).to have_content("Registry was successfully created.")
+      expect(Registry.any?).to be_truthy
+    end
+  end
+
   describe "update" do
     let!(:registry) { create(:registry) }
 

spec/lib/portus/registry_client_spec.rb

@@ -10,6 +10,33 @@ def initialize(host)
   end
 end
 
+# This class mocks a response object by providing the `code` method. This
+# method will return whatever has been passed in the initializer.
+class RegistryMockedStatusResponse
+  def initialize(status)
+    @status = status
+  end
+
+  def code
+    @status
+  end
+end
+
+# This class mocks the `perform_request` by returning whatever has been request
+# in the initializer.
+class RegistryPerformRequest < Portus::RegistryClient
+  def initialize(status)
+    @status = status
+  end
+
+  def perform_request(_endpoint, _verb, _authentication)
+    # We don't care about the given parameters.
+
+    return nil if @status.nil?
+    RegistryMockedStatusResponse.new(@status)
+  end
+end
+
 describe Portus::RegistryClient do
   let(:registry_server) { "registry.test.lan" }
   let(:username) { "flavio" }
@@ -141,6 +168,15 @@ def initialize(host)
     end
   end
 
+  context "is reachable or not" do
+    it "returns the proper thing in all the scenarios" do
+      [[nil, false], [200, false], [401, true]].each do |cs|
+        r = RegistryPerformRequest.new(cs.first)
+        expect(r.reachable?).to be cs.last
+      end
+    end
+  end
+
   context "fetching Image manifest" do
     let(:repository) { "foo/busybox" }
     let(:tag) { "1.0.0" }

spec/models/registry_spec.rb

@@ -32,6 +32,38 @@ def get_tag_from_manifest_test(repo, digest)
   end
 end
 
+# The mock client used by the RegistryReachable class.
+class RegistryReachableClient < Registry
+  def initialize(constant, result)
+    @constant = constant
+    @result   = result
+  end
+
+  def reachable?
+    if @constant.nil?
+      @result
+    else
+      raise @constant
+    end
+  end
+end
+
+# A Mock class for the Registry that provides a `client` method that returns an
+# object that handles the `reachable?` method.
+class RegistryReachable < Registry
+  attr_reader :use_ssl
+
+  def initialize(constant, result, ssl)
+    @constant = constant
+    @result   = result
+    @use_ssl  = ssl
+  end
+
+  def client
+    RegistryReachableClient.new(@constant, @result)
+  end
+end
+
 RSpec.describe Registry, type: :model do
   it { should have_many(:namespaces) }
 
@@ -74,6 +106,24 @@ def get_tag_from_manifest_test(repo, digest)
     end
   end
 
+  describe "#reachable" do
+    it "returns the proper message for each scenario" do
+      [
+        [nil, true, true, /^$/],
+        [nil, false, true, /registry does not implement v2/],
+        [SocketError, true, true, /The given registry is not available/],
+        [Net::HTTPBadResponse, true, true, /wrong with your SSL configuration/],
+        [Net::HTTPBadResponse, true, false, /Error: not using SSL/],
+        [OpenSSL::SSL::SSLError, true, true, /Error: using SSL/],
+        [OpenSSL::SSL::SSLError, true, false, /wrong with your SSL configuration/],
+        [StandardError, true, true, /something went wrong/]
+      ].each do |cs|
+        rr = RegistryReachable.new(cs.first, cs[1], cs[2])
+        expect(rr.reachable?).to match(cs.last)
+      end
+    end
+  end
+
   describe "#get_tag_from_manifest" do
     it "returns a tag on success" do
       mock = RegistryMock.new(false)