Skip to content

Commit

Permalink
Fixes #16025 - Fix ERB & params to import subnets from proxy
Browse files Browse the repository at this point in the history
Before this commit, the ERB would just throw a 500 because it tried to
show @subnet.name in a template for a @subnet that was still undefined.

Aside from that, the create_multiple method in the controller didn't
send the right call to filter the params, hence making it impossible to
create the imported subnets even with the ERB fix.
  • Loading branch information
dLobatog committed Aug 12, 2016
1 parent 791fedd commit f5d9f84
Show file tree
Hide file tree
Showing 3 changed files with 119 additions and 76 deletions.
4 changes: 3 additions & 1 deletion app/controllers/subnets_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,9 @@ def create_multiple
end

params_filter = self.class.subnet_params_filter
subnet_attrs = params[:subnets].map { |s| params_filter.filter_params(s, parameter_filter_context) }
subnet_attrs = params[:subnets].map do |subnet_param|
params_filter.filter_params(subnet_param, parameter_filter_context, :none)
end
@subnets = Subnet.create(subnet_attrs).reject { |s| s.errors.empty? }
if @subnets.empty?
process_success(:object => @subnets, :success_msg => _("Imported IPv4 Subnets"))
Expand Down
21 changes: 11 additions & 10 deletions app/views/subnets/import.html.erb
Original file line number Diff line number Diff line change
@@ -1,19 +1,20 @@
<% title _("Import IPv4 subnets") %>

<h2><%= _("The following IPv4 subnets have been found") %></h2>

<h2><%= _("The following IPv4 subnets have been found. Please review the details before creating them.") %></h2>
<%= form_for "subnets[]", :url => create_multiple_subnets_path do |f| %>
<% display_all = !minimal?(@subnets) %>
<div class="accordion" id='accordion1'>
<div class="panel-group" id='accordion1'>
<% @subnets.each do |subnet| %>
<% id = 'subnet_fields_'+subnet.to_s.gsub('/','_').gsub('.','_') %>
<div class="accordion-group">
<div class="accordion-heading" >
<a class="accordion-toggle" data-toggle="collapse" data-parent='#accordion1' href="<%= '#'+id %>" data-original-title='review' rel='twipsy' > <%= subnet %> </a>
<%= link_to_function "x", "ignore_subnet(this)", :'data-original-title'=> 'ignore subnet', :rel=>'twipsy', :class => "label ignore-subnet" %>
<% id = 'subnet_fields_' + subnet.network_address.gsub('/','_').gsub('.','_') %>
<div class="panel panel-default">
<div class="panel-heading" >
<a class="panel-title" data-toggle="collapse" data-parent='#accordion1' href="<%= '#'+id %>" rel='twipsy' >
<%= subnet.network_address %>
</a>
<%= link_to_function "x", "ignore_subnet(this)", :'data-original-title'=> 'Ignore subnet', :rel=>'twipsy', :class => "label ignore-subnet" %>
</div>
<div id="<%= id %>" class="accordion-body collapse <%= 'in' if display_all %>" >
<div class="accordion-inner">
<div id="<%= id %>" class="panel-collapse collapse <%= 'in' if display_all %>" >
<div class="panel-body">
<%= fields_for "subnets[]", subnet do |s| %>
<%= render 'fields', :f => s %>
<% end %>
Expand Down
170 changes: 105 additions & 65 deletions test/functional/subnets_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,77 +55,117 @@ def test_destroy
assert !Subnet.exists?(subnet.id)
end

def test_freeip_fails_no_subnet
get :freeip, {}, set_session_user

assert_response :bad_request
end

def test_freeip_fails_subnet_not_authorized
subnet_id = setup_subnet nil

get :freeip, {subnet_id: subnet_id}, set_session_user

assert_response :not_found
end

def test_freeip_not_found
subnet = mock('subnet')
subnet.stubs(:unused_ip).returns(nil)
subnet_id = setup_subnet subnet

get :freeip, {subnet_id: subnet_id}, set_session_user

assert_response :not_found
end

def test_freeip_fails_on_error
subnet = mock('subnet')
subnet.stubs(:unused_ip).raises(StandardError, 'Exception message')
subnet_id = setup_subnet subnet

get :freeip, {subnet_id: subnet_id}, set_session_user

assert_response :internal_server_error
end

def test_freeip_returns_json_on_success
ip = '1.2.3.4'
subnet = mock('subnet')
ipam = mock()
ipam.expects(:suggest_ip).returns(ip)
ipam.stubs(:errors).returns({})
subnet.stubs(:unused_ip).returns(ipam)
subnet_id = setup_subnet subnet

get :freeip, {subnet_id: subnet_id}, set_session_user

assert_response :success
assert_equal ip, JSON.parse(response.body)['ip']
assert_empty JSON.parse(response.body)['errors']
end

test 'user with view_params rights should see parameters in a subnet' do
setup_user "edit", "subnets"
setup_user "view", "params"
subnet = FactoryGirl.create(:subnet_ipv4, :with_parameter)
get :edit, {:id => subnet.id}, set_session_user.merge(:user => users(:one).id)
assert_not_nil response.body['Parameter']
end

test 'user without view_params rights should not see parameters in a subnet' do
setup_user "edit", "subnets"
subnet = FactoryGirl.create(:subnet_ipv4, :with_parameter)
get :edit, {:id => subnet.id}, set_session_user.merge(:user => users(:one).id)
assert_nil response.body['Parameter']
context 'freeip' do
test 'fails when subnet is not provided' do
get :freeip, {}, set_session_user
assert_response :bad_request
end

test '404s when user is not authorized to see subnet' do
subnet_id = setup_subnet
get :freeip, {subnet_id: subnet_id}, set_session_user
assert_response :not_found
end

test '404s when subnet does not have a free IP' do
subnet = mock('subnet')
subnet.stubs(:unused_ip).returns(nil)
subnet_id = setup_subnet subnet

get :freeip, {subnet_id: subnet_id}, set_session_user

assert_response :not_found
end

test 'catches StandardError when fetching IP' do
subnet = mock('subnet')
subnet.stubs(:unused_ip).raises(StandardError, 'Exception message')
subnet_id = setup_subnet subnet

get :freeip, {subnet_id: subnet_id}, set_session_user

assert_response :internal_server_error
end

test 'returns JSON on success' do
ip = '1.2.3.4'
subnet = mock('subnet')
ipam = mock()
ipam.expects(:suggest_ip).returns(ip)
ipam.stubs(:errors).returns({})
subnet.stubs(:unused_ip).returns(ipam)
subnet_id = setup_subnet subnet

get :freeip, {subnet_id: subnet_id}, set_session_user

assert_response :success
assert_equal ip, JSON.parse(response.body)['ip']
assert_empty JSON.parse(response.body)['errors']
end
end

context 'parameters permissions' do
test 'with view_params user should see parameters in a subnet' do
setup_user "edit", "subnets"
setup_user "view", "params"
subnet = FactoryGirl.create(:subnet_ipv4, :with_parameter)
get :edit, {:id => subnet.id}, set_session_user.merge(:user => users(:one).id)
assert_not_nil response.body['Parameter']
end

test 'without view_params user should not see parameters in a subnet' do
setup_user "edit", "subnets"
subnet = FactoryGirl.create(:subnet_ipv4, :with_parameter)
get :edit, {:id => subnet.id}, set_session_user.merge(:user => users(:one).id)
assert_nil response.body['Parameter']
end
end

context 'import IPv4 subnets' do
setup do
SmartProxy.expects(:find).with('foo').returns(mock('proxy'))
end

test 'redirects to index if none were found' do
Subnet::Ipv4.expects(:import).returns([])
get :import, { :subnet_id => setup_subnet,
:smart_proxy_id => 'foo' }, set_session_user
assert_redirected_to :subnets
assert_match 'No new IPv4 subnets found', flash[:warning]
end

test 'renders import page with results' do
Subnet::Ipv4.expects(:import).returns([FactoryGirl.build(:subnet_ipv4)])
get :import, { :subnet_id => setup_subnet,
:smart_proxy_id => 'foo' }, set_session_user
assert_response :success
assert_template :import
assert assigns(:subnets)
end
end

test 'create_multiple filters parameters when given a list of subnets' do
sample_subnet = FactoryGirl.build(:subnet_ipv4)
subnet_hash = { :name => sample_subnet.name,
:type => sample_subnet.type,
:network => sample_subnet.network,
:mask => sample_subnet.mask,
:cidr => sample_subnet.cidr,
:ipam => sample_subnet.ipam,
:boot_mode => sample_subnet.boot_mode }
assert_difference 'Subnet.count', 1 do
post :create_multiple, { :subnets => [subnet_hash] }, set_session_user
end
assert_response :redirect
assert_redirected_to subnets_url
end

private

def setup_subnet(subnet)
def setup_subnet(subnet = nil)
subnet_id = 10
scope = mock('scope')
scope.expects(:find).with(subnet_id).returns(subnet)
scope.stubs(:find).with(subnet_id).returns(subnet)
Subnet.stubs(:authorized).with(:view_subnets).returns(scope)
subnet_id
end
Expand Down

0 comments on commit f5d9f84

Please sign in to comment.