Project

General

Profile

Revision f6a9acbd

Added by Sebastian Gräßl over 5 years ago

Fixes #18518 - Add proper validation for container image

For now the image has only been validate via the result
of the search, but not enforced on the server side.

The model validation brings now proper feedback when
the form has been submitted to the server.

View differences:

app/helpers/container_steps_helper.rb
51 51
    end
52 52
  end
53 53

  
54
  def image_search_wrapper_class(model)
55
    if model.errors.messages[:image]
56
      'form-group has-error'
57
    else
58
      'form-group'
59
    end
60
  end
61

  
62
  def tab_active?(registry)
63
    active_tab == registry.to_sym
64
  end
65

  
54 66
  def active_tab
55 67
    if @docker_container_wizard_states_image.katello?
56 68
      :katello
app/models/docker_container_wizard_state.rb
9 9
                        :dependent => :destroy, :validate => true, :autosave => true
10 10

  
11 11
  delegate :compute_resource_id,   :to => :preliminary
12
  delegate :compute_resource, :to => :preliminary
13

  
12 14
  delegate :environment_variables, :to => :environment
13 15
  delegate :exposed_ports, :to => :environment
14 16
  delegate :dns, :to => :environment
app/models/docker_container_wizard_states/image.rb
4 4
    belongs_to :wizard_state, :class_name => DockerContainerWizardState,
5 5
                              :foreign_key => :docker_container_wizard_state_id
6 6
    delegate :compute_resource_id, :to => :wizard_state
7
    delegate :compute_resource, :to => :wizard_state
7 8

  
8 9
    validates :tag,             :presence => true
9 10
    validates :repository_name, :presence => true
11
    validate :image_exists
12

  
13
    def name
14
      "#{repository_name}:#{tag}"
15
    end
16

  
17
    def registry_api
18
      if registry_id
19
        DockerRegistry.find(registry_id).api
20
      else
21
        Service::RegistryApi.docker_hub
22
      end
23
    end
24

  
25
    def sources
26
      [compute_resource, registry_api]
27
    end
28

  
29
    def image_search_service
30
      ForemanDocker::ImageSearch.new(*sources)
31
    end
32

  
33
    def image_exists
34
      return true if image_search_service.available?(name)
35
      error_msg = _("Container image %{image_name} is not available.") % {
36
        image_name: "#{name}",
37
      }
38
      errors.add(:image, error_msg)
39
    end
10 40
  end
11 41
end
app/models/docker_container_wizard_states/preliminary.rb
6 6
    belongs_to :wizard_state, :class_name => DockerContainerWizardState,
7 7
                              :foreign_key => :docker_container_wizard_state_id
8 8

  
9
    validates :compute_resource_id, :presence => true
9
    belongs_to :compute_resource, :required => true
10 10

  
11 11
    def used_location_ids
12 12
      Location.joins(:taxable_taxonomies).where(
app/models/service/registry_api.rb
59 59
      get('/v2/'.freeze).is_a? Hash
60 60
    end
61 61

  
62
    def ok?
63
      get('/v1/').match("Docker Registry API")
64
    rescue
65
      get('/v2/').is_a? Hash
66
    end
67

  
68 62
    def self.docker_hub
69 63
      @@docker_hub ||= new(url: DOCKER_HUB)
70 64
    end
......
87 81

  
88 82
    def tags_v2(image_name)
89 83
      get("/v2/#{image_name}/tags/list")['tags'].map { |tag| { 'name' => tag } }
84
    rescue Docker::Error::NotFoundError
85
      []
90 86
    end
91 87

  
92 88
    def credentials
app/views/containers/steps/_image_hub_tab.html.erb
4 4
  :url => wizard_path,
5 5
  :method => :put,
6 6
  :html => {:data => {:registry => registry}} do |f| %>
7

  
8
  <% model.errors.messages.each do |field, field_errors| %>
9
    <div class="alert alert-danger">
10
      <span class="pficon pficon-error-circle-o"></span>
11
      <ul>
12
        <% field_errors.each do |message| %>
13
          <li><%= message %></li>
14
        <% end %>
15
      </ul>
16
    </div>
17
  <% end %>
18

  
7 19
  <% if registry == "registry" -%>
8 20
    <div class="input-group col-md-6">
9 21
      <%= select_registry f %>
10 22
    </div>
11 23
  <% end -%>
12 24

  
13
  <div class="form-group">
14
    <div class='row'>
15
      <% help_type = f.object.errors[:repository_name].present? ? :help_block : :help_inline %>
16
      <%= text_f(
17
        f,
18
        :repository_name,
19
        :label => _('Search'),
20
        :size => 'col-md-6',
21
        :'data-url'  => auto_complete_repository_name_image_search_path(model.compute_resource_id),
22
        :value       => f.object.repository_name.present? ? f.object.repository_name : '',
23
        :'data-registry' => registry,
24
        :'data-search' => true,
25
        :focus_on_load => true,
26
        :placeholder => _('Find your favorite container, e.g. centos'),
27
        help_type => link_to_function(
28
            icon_text('search', ''),
29
            'searchRepo(this)',
30
            :class => 'btn btn-default',
31
            :id => "search_repository_#{registry}",
32
            :'data-registry' => registry,
33
            :'data-url' => search_repository_image_search_path(model.compute_resource_id)
34
          ) + content_tag(:span, '', :id => 'image-confirmation').html_safe) %>
35
    </div>
36
    <div class='row'>
37
      <%= text_f f, :tag,
25
  <% help_type = f.object.errors[:repository_name].present? ? :help_block : :help_inline %>
26
  <%= text_f(f, :repository_name,
27
    :label => _('Search'),
28
    :size => 'col-md-6',
29
    :wrapper_class => (image_search_wrapper_class(model) if tab_active?(registry)),
30
    :'data-url'  => auto_complete_repository_name_image_search_path(model.compute_resource_id),
31
    :value       => f.object.repository_name.present? ? f.object.repository_name : '',
32
    :'data-registry' => registry,
33
    :'data-image' => true,
34
    :'data-search' => true,
35
    :'data-min-length' => 1,
36
    :focus_on_load => true,
37
    :placeholder => _('Find your favorite container, e.g. centos'),
38
    :control_group_id => "#{registry}_image_search",
39
    help_type => link_to_function(
40
        icon_text('search', ''),
41
        'searchRepo(this)',
42
        :class => 'btn btn-default',
43
        :id => "search_repository_#{registry}",
38 44
        :'data-registry' => registry,
39
        :'data-tag' => true,
40
        :'data-url' => auto_complete_image_tag_image_search_path(model.compute_resource_id) %>
41
    </div>
42
  </div>
45
        :'data-url' => search_repository_image_search_path(model.compute_resource_id)
46
      ) + content_tag(:span, '', :id => 'image-confirmation').html_safe) %>
47

  
48
  <%= text_f f, :tag,
49
    :control_group_id => "#{registry}_tag_search",
50
    :'data-registry' => registry,
51
    :'data-tag' => true,
52
    :'data-url' => auto_complete_image_tag_image_search_path(model.compute_resource_id) %>
43 53

  
44 54
  <div class="col-md-12">
45 55
    <div data-search-spinner=true class='col-md-offset-3 hide'>
test/functionals/api/v2/containers_controller_test.rb
3 3
module Api
4 4
  module V2
5 5
    class ContainersControllerTest < ActionController::TestCase
6
      setup do
7
        stub_image_existance
8
        stub_registry_api
9
      end
10

  
6 11
      test 'index returns a list of all containers' do
7 12
        get :index, {}, set_session_user
8 13
        assert_response :success
test/functionals/containers_controller_test.rb
1 1
require 'test_plugin_helper'
2 2

  
3 3
class ContainersControllerTest < ActionController::TestCase
4
  setup do
5
    stub_image_existance
6
    stub_registry_api
7
  end
8

  
4 9
  test 'redirect if Docker provider is not available' do
5 10
    get :index, {}, set_session_user
6 11
    assert_redirected_to new_compute_resource_path
test/functionals/containers_steps_controller_test.rb
3 3
module Containers
4 4
  class StepsControllerTest < ActionController::TestCase
5 5
    setup do
6
      stub_image_existance
7
      stub_registry_api
6 8
      @container = FactoryGirl.create(:container)
9
      @state = DockerContainerWizardState.create!
7 10
    end
8 11

  
9 12
    test 'wizard finishes with a redirect to the managed container' do
10
      state = DockerContainerWizardState.create!
11
      Service::Containers.any_instance.expects(:start_container!).with(equals(state))
13
      Service::Containers.any_instance.expects(:start_container!).with(equals(@state))
12 14
        .returns(@container)
13
      put :update, { :wizard_state_id => state.id,
15
      put :update, { :wizard_state_id => @state.id,
14 16
                     :id => :environment,
15 17
                     :start_on_create => true,
16 18
                     :docker_container_wizard_states_environment => { :tty => false } },
......
19 21
      assert_redirected_to container_path(:id => @container.id)
20 22
    end
21 23

  
22
    test 'image show doesnot load katello' do
23
      compute_resource = FactoryGirl.create(:docker_cr)
24
      state = DockerContainerWizardState.create!
25
      create_options = { :wizard_state => state,
26
                         :compute_resource_id => compute_resource.id
24
    describe 'on image step' do
25
      setup do
26
        @compute_resource = FactoryGirl.create(:docker_cr)
27
        @create_options = { :wizard_state => @state,
28
                           :compute_resource_id => @compute_resource.id }
29
        @state.preliminary = DockerContainerWizardStates::Preliminary.create!(@create_options)
30
        DockerContainerWizardState.expects(:find).at_least_once.returns(@state)
31
      end
32

  
33
      test 'image show doesnot load katello' do
34
        get :show, { :wizard_state_id => @state.id, :id => :image }, set_session_user
35
        refute @state.image.katello?
36
        refute response.body.include?("katello") # this is code generated by katello partial
37
        docker_image = @controller.instance_eval do
38
          @docker_container_wizard_states_image
39
        end
40
        assert_equal @state.image, docker_image
41
      end
42

  
43
      describe 'submitting' do
44
        setup do
45
          @image_params = {
46
            docker_container_wizard_states_image: {
47
              repository_name: 'test',
48
              tag: 'test'
49
          }}
50
          @params = @image_params.merge({
51
            wizard_state_id: @state.id,
52
            id: :image,
53
          })
54
        end
55

  
56
        test 'has no errors if the image exists' do
57
          put :update, @params, set_session_user
58
          assert_valid @state.image
59
          assert css_select('#hub_image_search.has-error').size == 0
60
        end
27 61

  
28
                       }
29
      state.preliminary = DockerContainerWizardStates::Preliminary.create!(create_options)
30
      DockerContainerWizardState.expects(:find).at_least_once.returns(state)
31
      get :show, { :wizard_state_id => state.id, :id => :image }, set_session_user
32
      refute state.image.katello?
33
      refute response.body.include?("katello") # this is code generated by katello partial
34
      docker_image = @controller.instance_eval do
35
        @docker_container_wizard_states_image
62
        test 'shows an error when the image does not exist' do
63
          stub_image_existance(false)
64
          put :update, @params, set_session_user
65
          refute_valid @state.image
66
          assert_select '#hub_image_search.has-error'
67
        end
36 68
      end
37
      assert_equal state.image, docker_image
38 69
    end
39 70

  
40 71
    test 'new container respects exposed_ports configuration' do
41
      state = DockerContainerWizardState.create!
42 72
      environment_options = {
43
        :docker_container_wizard_state_id => state.id
73
        :docker_container_wizard_state_id => @state.id
44 74
      }
45
      state.environment = DockerContainerWizardStates::Environment.create!(environment_options)
46
      state.environment.exposed_ports.create!(:key => '1654', :value => 'tcp')
47
      state.environment.exposed_ports.create!(:key => '1655', :value => 'udp')
48
      get :show, { :wizard_state_id => state.id, :id => :environment }, set_session_user
75
      @state.environment = DockerContainerWizardStates::Environment.create!(environment_options)
76
      @state.environment.exposed_ports.create!(:key => '1654', :value => 'tcp')
77
      @state.environment.exposed_ports.create!(:key => '1655', :value => 'udp')
78
      get :show, { :wizard_state_id => @state.id, :id => :environment }, set_session_user
49 79
      assert response.body.include?("1654")
50 80
      assert response.body.include?("1655")
51 81

  
52 82
      # Load ExposedPort variables into container
53
      state.environment.exposed_ports.each do |e|
83
      @state.environment.exposed_ports.each do |e|
54 84
        @container.exposed_ports.build :key => e.key,
55 85
                                       :value => e.value
56 86
      end
......
61 91
    end
62 92

  
63 93
    test 'new container respects dns configuration' do
64
      state = DockerContainerWizardState.create!
65 94
      environment_options = {
66
        :docker_container_wizard_state_id => state.id
95
        :docker_container_wizard_state_id => @state.id
67 96
      }
68
      state.environment = DockerContainerWizardStates::Environment.create!(environment_options)
69
      state.environment.dns.create!(:key => '18.18.18.18')
70
      state.environment.dns.create!(:key => '19.19.19.19')
71
      get :show, { :wizard_state_id => state.id, :id => :environment }, set_session_user
97
      @state.environment = DockerContainerWizardStates::Environment.create!(environment_options)
98
      @state.environment.dns.create!(:key => '18.18.18.18')
99
      @state.environment.dns.create!(:key => '19.19.19.19')
100
      get :show, { :wizard_state_id => @state.id, :id => :environment }, set_session_user
72 101
      assert response.body.include?("18.18.18.18")
73 102
      assert response.body.include?("19.19.19.19")
74 103

  
75 104
      # Load Dns variables into container
76
      state.environment.dns.each do |e|
105
      @state.environment.dns.each do |e|
77 106
        @container.dns.build :key => e.key
78 107
      end
79 108
      # Check if parametrized value of container matches Docker API's expectations
......
83 112
    end
84 113

  
85 114
    test "does not create a container with 2 exposed ports with the same key" do
86
      state = DockerContainerWizardState.new
87 115
      environment_options = {
88
          :docker_container_wizard_state_id => state.id
116
          :docker_container_wizard_state_id => @state.id
89 117
      }
90
      state.environment = DockerContainerWizardStates::Environment.new(environment_options)
91
      state.environment.exposed_ports.new(:key => '1654', :value => 'tcp')
92
      state.environment.exposed_ports.new(:key => '1654', :value => 'udp')
93
      refute_valid state
94
      assert_equal "Please ensure the following parameters are unique", state.errors[:'environment.exposed_ports'].first
118
      @state.environment = DockerContainerWizardStates::Environment.new(environment_options)
119
      @state.environment.exposed_ports.new(:key => '1654', :value => 'tcp')
120
      @state.environment.exposed_ports.new(:key => '1654', :value => 'udp')
121
      refute_valid @state
122
      assert_equal "Please ensure the following parameters are unique", @state.errors[:'environment.exposed_ports'].first
95 123
    end
96 124
  end
97 125
end
test/test_plugin_helper.rb
22 22
  Service::RegistryApi.any_instance.stubs(:get).returns({'results' => []})
23 23
  Docker::Image.stubs(:all).returns([])
24 24
end
25

  
test/units/containers_service_test.rb
3 3

  
4 4
class ContainersServiceTest < ActiveSupport::TestCase
5 5
  setup do
6
    stub_image_existance
7
    stub_registry_api
8

  
6 9
    @state = DockerContainerWizardState.create! do |s|
7 10
      s.build_preliminary(:compute_resource_id => FactoryGirl.create(:docker_cr).id,
8 11
                          :locations           => [taxonomies(:location1)],
9 12
                          :organizations       => [taxonomies(:organization1)])
10
      s.build_image(:repository_name => 'test', :tag => 'test')
13
      s.build_image(:repository_name => 'test', :tag => 'test', :wizard_state => s)
11 14
      s.build_configuration(:name => 'test', :command => '/bin/bash')
12 15
      s.build_environment(:tty => false)
13 16
    end
test/units/docker_container_wizard_states/image_test.rb
1
require 'test_plugin_helper'
2

  
3
module DockerContainerWizardStates
4
  class ImageTest < ActiveSupport::TestCase
5
    let(:image) { 'centos' }
6
    let(:tags) { ['latest', '5', '4.3'] }
7
    let(:docker_hub) { Service::RegistryApi.new(url: 'https://nothub.com') }
8
    let(:registry) { FactoryGirl.create(:docker_registry) }
9
    let(:compute_resource) { FactoryGirl.create(:docker_cr) }
10
    let(:image_search_service) { ForemanDocker::ImageSearch.new }
11
    let(:wizard_state) do
12
      DockerContainerWizardState.create
13
    end
14
    let(:preliminary) do
15
      DockerContainerWizardStates::Preliminary.create(
16
        compute_resource: compute_resource,
17
        wizard_state: wizard_state
18
      )
19
    end
20

  
21
    subject do
22
      Image.new(
23
        repository_name: image,
24
        tag: tags.first,
25
        wizard_state: wizard_state
26
      )
27
    end
28

  
29
    setup do
30
      ForemanDocker::ImageSearch.any_instance.unstub(:available?)
31
      wizard_state.preliminary = preliminary
32
      Service::RegistryApi.stubs(:docker_hub).returns(docker_hub)
33
    end
34

  
35

  
36
    describe 'it validates that the image is available' do
37
      test 'validates via the image_search_service' do
38
        DockerContainerWizardStates::Image.any_instance
39
                                          .stubs(:image_search_service)
40
                                          .returns(image_search_service)
41
        available = [true, false].sample
42
        image_search_service.expects(:available?)
43
                            .with("#{image}:#{tags.first}").at_least_once
44
                            .returns(available)
45
        assert_equal available, subject.valid?
46
      end
47

  
48
      context 'when no registy is set' do
49
        test 'it queries the compute_resource and docker_hub' do
50
          compute_resource.expects(:image).with(image).at_least_once
51
                          .returns(image)
52
          compute_resource.expects(:tags_for_local_image).at_least_once
53
                          .with(image, tags.first)
54
                          .returns([])
55
          docker_hub.expects(:tags).at_least_once
56
                    .returns([])
57

  
58
          subject.validate
59
        end
60
      end
61

  
62
      context 'when a registy is set' do
63
        setup do
64
          subject.stubs(:registry_id).returns(registry.id)
65
          DockerRegistry.expects(:find).with(registry.id)
66
                        .returns(registry)
67
        end
68

  
69
        test 'it queries the compute_resource and registry' do
70
          compute_resource.expects(:image).with(image).at_least_once
71
                          .returns(image)
72
          compute_resource.expects(:tags_for_local_image).at_least_once
73
                          .with(image, tags.first)
74
                          .returns([])
75
          docker_hub.expects(:tags).never
76
          registry.api.expects(:tags).with(image, tags.first).at_least_once
77
                  .returns([])
78

  
79
          subject.validate
80
        end
81
      end
82
    end
83
  end
84
end

Also available in: Unified diff