Project

General

Profile

Revision 2b80fdd9

Added by Ori Rabin almost 6 years ago

Fixes #13043 - Change Parameter inheritance to DockerParameter

View differences:

app/controllers/api/v2/containers_controller.rb
53 53
          param :memory, String
54 54
          param :cpu_shares, :number
55 55
          param :cpu_set, String
56
          param :environment_variables, Hash
56
          param :environment_variables, Array, :desc => N_("Optional array of environment variables hashes. e.g. 'environment_variables': [{'name' => 'example', 'value' => '123'}]")
57 57
          param :attach_stdout, :bool
58 58
          param :attach_stdin, :bool
59 59
          param :attach_stderr, :bool
......
80 80
          set_container_taxonomies
81 81
          process_response @container.save
82 82
        end
83
      rescue ActiveModel::MassAssignmentSecurity::Error => e
84
        render :json => { :error  => _("Wrong attributes: %s") % e.message },
85
               :status => :unprocessable_entity
86 83
      end
87 84

  
88 85
      api :DELETE, '/containers/:id/', N_('Delete a container')
......
169 166
        end
170 167

  
171 168
        if params[:container][:environment_variables].present?
172
          wizard_state.environment.environment_variables =
173
            params[:container][:environment_variables]
169
          environment_variables = []
170
          params[:container][:environment_variables].each do |env_var|
171
            environment_variable = DockerContainerWizardStates::EnvironmentVariable.new
172
            environment_variable.key = env_var[:key]
173
            environment_variable.value = env_var[:value]
174
            environment_variables << environment_variable
175
          end
176
          wizard_state.environment.environment_variables = environment_variables
174 177
        end
175 178
        wizard_state.tap(&:save)
176 179
      end
app/controllers/containers/steps_controller.rb
39 39
      instance_variable_set("@docker_container_wizard_states_#{step}", s)
40 40
    end
41 41

  
42
    def docker_parameters_permited_params
43
      [:key, :reference_id, :value, :_destroy, :id]
44
    end
45

  
42 46
    def state_params
43 47
      attrs = case step
44 48
              when :preliminary
......
50 54
              when :environment
51 55
                [:tty, :docker_container_wizard_state_id,
52 56
                 :attach_stdin, :attach_stdout, :attach_stderr,
53
                 :exposed_ports_attributes => [], :environment_variables_attributes => [],
54
                 :dns_attributes => []
57
                 :exposed_ports_attributes => docker_parameters_permited_params,
58
                 :environment_variables_attributes => docker_parameters_permited_params,
59
                 :dns_attributes => docker_parameters_permited_params
55 60
                ]
56 61
              end
57 62

  
......
72 77
                  else
73 78
                    service.create_container!(@state)
74 79
                  end
80
      
75 81
      if container.present?
76 82
        process_success(:object => container, :success_redirect => container_path(container))
77 83
      else
app/models/concerns/foreman_docker/parameter_validators.rb
1 1
module ForemanDocker
2 2
  module ParameterValidators
3 3
    extend ActiveSupport::Concern
4
    include ::ParameterValidators
5 4

  
6
    def parameters_symbol
7
      return :environment_variables if is_a? Container
8
      super
5
    included do
6
      validate :validate_unique_parameter_keys
7
    end
8

  
9
    def validate_unique_parameter_keys
10
      parameters_symbol = [:environment_variables, :exposed_ports, :dns]
11
      parameters_symbol.each do |param_symbol|
12
        keys  = []
13
        errors = false
14

  
15
        self.public_send(param_symbol).each do |param|
16
          errors = duplicate_key?(keys, param)
17
        end
18

  
19
        self.errors[param_symbol] = _('Please ensure the following parameters are unique') if errors
20
      end
21
    end
22

  
23
    def duplicate_key?(keys, param)
24
      if keys.include?(param.key)
25
        param.errors[:key] = _('has already been taken')
26
        return true
27
      else
28
        keys << param.key
29
      end
30

  
31
      false
9 32
    end
10 33
  end
11 34
end
app/models/container.rb
6 6
  belongs_to :registry, :class_name => "DockerRegistry", :foreign_key => :registry_id
7 7
  has_many :environment_variables, :dependent  => :destroy, :foreign_key => :reference_id,
8 8
                                   :inverse_of => :container,
9
                                   :class_name => 'EnvironmentVariable',
10
                                   :validate => false
9
                                   :class_name => 'EnvironmentVariable'
10

  
11 11
  accepts_nested_attributes_for :environment_variables, :allow_destroy => true
12
  include ForemanDocker::ParameterValidators
13 12

  
14 13
  has_many :exposed_ports,  :dependent  => :destroy, :foreign_key => :reference_id,
15 14
                            :inverse_of => :container,
16
                            :class_name => 'ExposedPort',
17
                            :validate => true
15
                            :class_name => 'ExposedPort'
18 16

  
19 17
  has_many :dns,  :dependent  => :destroy, :foreign_key => :reference_id,
20 18
                  :inverse_of => :container,
21
                  :class_name => 'ForemanDocker::Dns',
22
                  :validate => true
19
                  :class_name => 'ForemanDocker::Dns'
20

  
21
  include ForemanDocker::ParameterValidators
23 22

  
24 23
  accepts_nested_attributes_for :exposed_ports, :allow_destroy => true
25 24
  scoped_search :on => :name
......
41 40
      'AttachStdout' => attach_stdout,          'AttachStdin'  => attach_stdin,
42 41
      'AttachStderr' => attach_stderr,          'CpuShares'    => cpu_shares,
43 42
      'Cpuset'       => cpu_set,
44
      'Env' => environment_variables.map { |env| "#{env.name}=#{env.value}" },
45
      'ExposedPorts' => Hash[*exposed_ports.map { |v| [v.name + "/" + v.value, {}] }.flatten],
43
      'Env' => environment_variables.map { |env| "#{env.key}=#{env.value}" },
44
      'ExposedPorts' => Hash[*exposed_ports.map { |v| [v.key + "/" + v.value, {}] }.flatten],
46 45
      'HostConfig' => {
47
        'Dns' => dns.map { |env| "#{env.name}" }
46
        'Dns' => dns.map { |env| "#{env.key}" }
48 47
      } }
49 48
  end
50 49

  
app/models/docker_container_wizard_states/dns.rb
1 1
require 'resolv'
2 2

  
3 3
module DockerContainerWizardStates
4
  class Dns < Parameter
5
    # The Parameter class from which this Dns class inherits,validates for the
6
    # presence of an associated domain,  operating system, host or host group.
7
    # We will have to reset those validations for the Dns class as they do not
8
    # make any sense for the context in which this class is being used here.
9
    Dns.reset_callbacks(:validate)
10

  
4
  class Dns < DockerParameter
11 5
    belongs_to :environment,  :foreign_key => :reference_id,
12 6
                              :inverse_of => :dns,
13 7
                              :class_name => 'DockerContainerWizardStates::Environment'
14
    validates :name, :uniqueness => { :scope => :reference_id },
8
    validates :key, :uniqueness => { :scope => :reference_id },
15 9
                     :format => {
16 10
                       :with => Regexp.union(Resolv::IPv4::Regex,
17 11
                                             Resolv::IPv6::Regex,
app/models/docker_container_wizard_states/environment.rb
3 3
    self.table_name_prefix = 'docker_container_wizard_states_'
4 4
    belongs_to :wizard_state, :class_name => DockerContainerWizardState
5 5

  
6
    # Fix me:
7
    # Validations are off on this association as there's a bug in ::Parameter
8
    # that forces validation of reference_id. This will fail on new records as
9
    # validations are executed before parent and children records have been persisted.
10 6
    has_many :environment_variables, :dependent  => :destroy, :foreign_key => :reference_id,
11 7
                                     :inverse_of => :environment,
12 8
                                     :class_name =>
13
                                       'DockerContainerWizardStates::EnvironmentVariable',
14
                                     :validate => false
15
    include ::ParameterValidators
9
                                       'DockerContainerWizardStates::EnvironmentVariable'
16 10

  
17 11
    has_many :exposed_ports,  :dependent  => :destroy, :foreign_key => :reference_id,
18 12
                              :inverse_of => :environment,
19
                              :class_name => 'DockerContainerWizardStates::ExposedPort',
20
                              :validate => true
13
                              :class_name => 'DockerContainerWizardStates::ExposedPort'
21 14
    has_many :dns,  :dependent  => :destroy, :foreign_key => :reference_id,
22 15
                    :inverse_of => :environment,
23
                    :class_name => 'DockerContainerWizardStates::Dns',
24
                    :validate => true
16
                    :class_name => 'DockerContainerWizardStates::Dns'
25 17

  
18
    include ForemanDocker::ParameterValidators
26 19
    accepts_nested_attributes_for :environment_variables, :allow_destroy => true
27 20
    accepts_nested_attributes_for :exposed_ports, :allow_destroy => true
28 21
    accepts_nested_attributes_for :dns, :allow_destroy => true
29 22

  
30
    def parameters_symbol
31
      :environment_variables
32
    end
33 23
  end
34 24
end
app/models/docker_container_wizard_states/environment_variable.rb
1 1
module DockerContainerWizardStates
2
  class EnvironmentVariable < Parameter
2
  class EnvironmentVariable < DockerParameter
3 3
    belongs_to :environment, :foreign_key => :reference_id, :inverse_of => :environment_variables,
4 4
                             :class_name => 'DockerContainerWizardStates::Environment'
5
    validates :name, :uniqueness => { :scope => :reference_id }
5
    validates :key, :uniqueness => { :scope => :reference_id }
6 6
  end
7 7
end
app/models/docker_container_wizard_states/exposed_port.rb
1 1
module DockerContainerWizardStates
2
  class ExposedPort < Parameter
3
    # The Parameter class from which ExposedPort class inherits,validates for the
4
    # presence of an  associated domain, operating system, host or host group. We
5
    # will have to reset those validations for the  ExposedPort class as  they do
6
    # not make any sense for the context in which this class is being used here.
7
    ExposedPort.reset_callbacks(:validate)
8

  
2
  class ExposedPort < DockerParameter
9 3
    belongs_to :environment,  :foreign_key => :reference_id, :inverse_of => :exposed_ports,
10 4
                              :class_name => 'DockerContainerWizardStates::Environment'
11
    validates :name,  :uniqueness => { :scope => :reference_id },
5
    validates :key,  :uniqueness => { :scope => :reference_id },
12 6
                      :numericality => { :only_integer => true,
13 7
                                         :greater_than => 0,
14 8
                                         :less_than_or_equal_to => 655_35 }
app/models/docker_parameter.rb
1
class DockerParameter < ActiveRecord::Base
2
  extend FriendlyId
3
  friendly_id :key
4
  include Parameterizable::ByIdName
5

  
6
  validates_lengths_from_database
7

  
8
  include Authorizable
9
  validates :key, :presence => true, :no_whitespace => true
10

  
11
  scoped_search :on => :key, :complete_value => true
12

  
13
  default_scope -> { order("docker_parameters.key") }
14

  
15
  before_validation :strip_whitespaces
16

  
17
  def strip_whitespaces
18
    self.value.strip! unless value.blank?
19
  end
20
end
app/models/environment_variable.rb
1
class EnvironmentVariable < Parameter
1
class EnvironmentVariable < DockerParameter
2 2
  belongs_to :container, :foreign_key => :reference_id, :inverse_of => :environment_variables
3
  audited :except => [:priority], :associated_with => :container, :allow_mass_assignment => true
4
  validates :name, :uniqueness => { :scope => :reference_id }
3
  audited :associated_with => :container, :allow_mass_assignment => true
4
  validates :key, :uniqueness => { :scope => :reference_id }
5 5
end
app/models/exposed_port.rb
1
class ExposedPort < Parameter
2
  # The Parameter class from which ExposedPort class inherits,validates for the
3
  # presence of an  associated domain, operating system, host or host group. We
4
  # will have to reset those validations for the  ExposedPort class as  they do
5
  # not make any sense for the context in which this class is being used here.
6
  ExposedPort.reset_callbacks(:validate)
7

  
1
class ExposedPort < DockerParameter
8 2
  belongs_to :container, :foreign_key => :reference_id, :inverse_of => :exposed_ports
9
  audited :except => [:priority], :associated_with => :container, :allow_mass_assignment => true
10
  validates :name,  :uniqueness => { :scope => :reference_id }
11
  validates :name,  :numericality => { :only_integer => true,
3
  audited :associated_with => :container, :allow_mass_assignment => true
4
  validates :key,  :uniqueness => { :scope => :reference_id }
5
  validates :key,  :numericality => { :only_integer => true,
12 6
                                       :greater_than => 0,
13 7
                                       :less_than_or_equal_to => 655_35,
14 8
                                       :message => "%{value} is not a valid port number" }
app/models/foreman_docker/dns.rb
1 1
require 'resolv'
2 2

  
3 3
module ForemanDocker
4
  class Dns < Parameter
5
    # The Parameter class from which this Dns class inherits,validates for the
6
    # presence of an associated domain,  operating system, host or host group.
7
    # We will have to reset those validations for the Dns class as they do not
8
    # make any sense for the context in which this class is being used here.
9
    ForemanDocker::Dns.reset_callbacks(:validate)
10

  
4
  class Dns < DockerParameter
11 5
    belongs_to :container, :foreign_key => :reference_id,
12 6
                           :inverse_of => :dns,
13 7
                           :class_name => "Container"
14 8

  
15
    audited :except => [:priority], :associated_with => :container, :allow_mass_assignment => true
16
    validates :name, :uniqueness => { :scope => :reference_id },
9
    audited :associated_with => :container, :allow_mass_assignment => true
10
    validates :key, :uniqueness => { :scope => :reference_id },
17 11
                     :format => {
18 12
                       :with => Regexp.union(Resolv::IPv4::Regex,
19 13
                                             Resolv::IPv6::Regex,
app/models/service/containers.rb
80 80
    def load_environment_variables(state, r)
81 81
      state.environment_variables.each do |environment_variable|
82 82
        var = r.environment_variables.build
83
        var.name = environment_variable.name
83
        var.key = environment_variable.key
84 84
        var.value = environment_variable.value
85
        var.priority = environment_variable.priority
86 85
      end
87 86
    end
88 87

  
89 88
    def load_exposed_ports(state, r)
90 89
      state.exposed_ports.each do |e|
91 90
        port = r.exposed_ports.build
92
        port.name = e.name
91
        port.key = e.key
93 92
        port.value = e.value
94
        port.priority = e.priority
95 93
      end
96 94
    end
97 95

  
98 96
    def load_dns(state, r)
99 97
      state.dns.each do |e|
100 98
        dns = r.dns.build
101
        dns.name = e.name
102
        dns.priority = e.priority
99
        dns.key = e.key
103 100
      end
104 101
    end
105 102

  
app/views/containers/show.html.erb
69 69
              <td><%= _('Environment Variables') %></td>
70 70
              <td>
71 71
                <table id="environment_variables" class="table table-bordered" style="table-layout:fixed; word-wrap: break-word">
72
                  <% (@container.in_fog.attributes['config_env'] || []).each do |environment_variable| %>
72
                  <% (@container.in_fog.environment_variables || []).each do |environment_variable| %>
73 73
                  <% pair = environment_variable.split("=") %>
74 74
                    <tr>
75 75
                      <td><b><%= pair.first %></b></td>
app/views/foreman_docker/common_parameters/_dns_entry.html.erb
1 1
<tr class="fields form-group <%= 'has-error' if f.object.errors.any? %>">
2
  <td><%= f.text_field(:name,  :class => "form-control",
2
  <td><%= f.text_field(:key,  :class => "form-control",
3 3
                       :placeholder => _("DNS e.g. 8.8.8.8")) %></td>
4 4
  <td style='vertical-align: middle' class='text-center'>
5 5
    <span class="help-block">
app/views/foreman_docker/common_parameters/_environment_variable.html.erb
1 1
<tr class="fields form-group <%= 'error' if f.object.errors.any? %>">
2
  <td><%= f.text_field(:name, :class => "form-control",
2
  <td><%= f.text_field(:key, :class => "form-control",
3 3
                       :placeholder  => _("Name, e.g. PING_HOST")) %></td>
4 4
  <td><%= f.text_field(:value, :class => "form-control",
5 5
                       :placeholder   => _("Value, e.g. example.org")) %></td>
app/views/foreman_docker/common_parameters/_exposed_port.html.erb
1 1
<tr class="fields form-group <%= 'has-error' if f.object.errors.any? %>">
2
  <td><%= f.text_field(:name,  :class => "form-control",
2
  <td><%= f.text_field(:key,  :class => "form-control",
3 3
                       :placeholder => _("Port number e.g. 22")) %></td>
4 4
  <td><%= f.select :value, [['TCP','tcp'],['UDP','udp']]  %></td>
5 5
  <td style='vertical-align: middle' class='text-center'>
db/migrate/20160605133025_create_docker_parameters.rb
1
class CreateDockerParameters < ActiveRecord::Migration
2
  def change
3
    create_table :docker_parameters do |t|
4
      t.string :key, :limit => 255
5
      t.text  :value
6
      t.string :type, :limit => 255
7
      t.integer :reference_id
8
      t.timestamps
9
    end
10

  
11
    add_index :docker_parameters, [:type, :reference_id, :key], :unique => true
12
  end
13
end
db/migrate/20160605134652_move_parameters_to_docker_parameters.rb
1
class MoveParametersToDockerParameters < ActiveRecord::Migration
2
  class FakeDockerParameter < ActiveRecord::Base
3
    self.table_name = 'docker_parameters'
4
  end
5

  
6
  class FakeParameter < ActiveRecord::Base
7
    self.table_name = 'parameters'
8
  end
9

  
10
  def up
11
  # All the  DockerContainerWizardStates::PARAMETER are temporary for the wizard step so no need to keep them
12
   docker_params = FakeParameter.unscoped.where(:type => ['EnvironmentVariable', 'ForemanDocker::Dns', 'ExposedPort'])
13
   docker_params.each do |param|
14
     DockerParameter.create(:key => param['name'], :value => param['value'], :reference_id => param['reference_id'], :type => param['type'])
15
   end
16
   docker_params.delete_all
17
  end
18

  
19
  def down
20
    docker_params = FakeDockerParameter.unscoped.where(:type => ['EnvironmentVariable', 'ForemanDocker::Dns', 'ExposedPort'])
21
    docker_params.each do |param|
22
      Parameter.create(:key => param['name'], :value => param['value'], :reference_id => param['reference_id'], :type => param['type'])
23
    end
24

  
25
    docker_params.delete_all
26
  end
27
end
test/functionals/api/v2/containers_controller_test.rb
109 109
                                          :tag => tag }
110 110
            assert_response :created
111 111
          end
112

  
113
          test 'creates a container with correct params' do
114
            repository_name = "centos"
115
            tag = "8"
116
            name = "foo2"
117
            Service::Containers.any_instance.expects(:pull_image).returns(true)
118
            Service::Containers.any_instance.expects(:start_container).returns(true)
119
            post :create, :container => { :compute_resource_id => @compute_resource.id,
120
                                          :name => name,
121
                                          :registry_id => @registry.id,
122
                                          :repository_name => repository_name,
123
                                          :tag => tag,
124
                                          :environment_variables => [{:key => 'ping_host', :value => 'example.com'}]}
125
            assert_response :created
126
          end
112 127
        end
113 128

  
114 129
        test 'creates a katello container with correct params' do
test/functionals/containers_steps_controller_test.rb
43 43
        :docker_container_wizard_state_id => state.id
44 44
      }
45 45
      state.environment = DockerContainerWizardStates::Environment.create!(environment_options)
46
      state.environment.exposed_ports.create!(:name => '1654', :value => 'tcp')
47
      state.environment.exposed_ports.create!(:name => '1655', :value => 'udp')
46
      state.environment.exposed_ports.create!(:key => '1654', :value => 'tcp')
47
      state.environment.exposed_ports.create!(:key => '1655', :value => 'udp')
48 48
      get :show, { :wizard_state_id => state.id, :id => :environment }, set_session_user
49 49
      assert response.body.include?("1654")
50 50
      assert response.body.include?("1655")
51 51

  
52 52
      # Load ExposedPort variables into container
53 53
      state.environment.exposed_ports.each do |e|
54
        @container.exposed_ports.build :name => e.name,
55
                                       :value => e.value,
56
                                       :priority => e.priority
54
        @container.exposed_ports.build :key => e.key,
55
                                       :value => e.value
57 56
      end
58 57
      # Check if parametrized value of container matches Docker API's expectations
59 58
      assert @container.parametrize.key? "ExposedPorts"
......
67 66
        :docker_container_wizard_state_id => state.id
68 67
      }
69 68
      state.environment = DockerContainerWizardStates::Environment.create!(environment_options)
70
      state.environment.dns.create!(:name => '18.18.18.18')
71
      state.environment.dns.create!(:name => '19.19.19.19')
69
      state.environment.dns.create!(:key => '18.18.18.18')
70
      state.environment.dns.create!(:key => '19.19.19.19')
72 71
      get :show, { :wizard_state_id => state.id, :id => :environment }, set_session_user
73 72
      assert response.body.include?("18.18.18.18")
74 73
      assert response.body.include?("19.19.19.19")
75 74

  
76 75
      # Load Dns variables into container
77 76
      state.environment.dns.each do |e|
78
        @container.dns.build :name => e.name,
79
                             :priority => e.priority
77
        @container.dns.build :key => e.key
80 78
      end
81 79
      # Check if parametrized value of container matches Docker API's expectations
82 80
      assert @container.parametrize.key? "HostConfig"
83 81
      assert @container.parametrize["HostConfig"].key? "Dns"
84 82
      assert @container.parametrize["HostConfig"].value? ["18.18.18.18", "19.19.19.19"]
85 83
    end
84

  
85
    test "does not create a container with 2 exposed ports with the same key" do
86
      state = DockerContainerWizardState.new
87
      environment_options = {
88
          :docker_container_wizard_state_id => state.id
89
      }
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
95
    end
86 96
  end
87 97
end

Also available in: Unified diff