Switch nova_security_rule to openstack provider

The current nova provider for nova_security_rule is broken at the
moment.  Due to the fact that the commands are getting deprecated,
the warnings are confusing the text parsing and result in repeated
creation of security group rules.

This patch resolves this issue by switching it to the new openstack
provider.  It also adds the instances and prefetch methods which
will allow `puppet resource` CLI usage.

Change-Id: I9f84728f89a8344685f64b79c8079f3bf74ff979
This commit is contained in:
Mohammed Naser 2017-07-26 11:50:45 -04:00
parent 0f8c04df8e
commit 93b7656774
No known key found for this signature in database
GPG Key ID: 481CBC90384AEC42
5 changed files with 200 additions and 120 deletions

View File

@ -1,51 +0,0 @@
require File.join(File.dirname(__FILE__), '..','..','..',
'puppet/provider/nova')
Puppet::Type.type(:nova_security_rule).provide(
:nova,
:parent => Puppet::Provider::Nova
) do
desc "Manage nova security rules"
commands :nova => 'nova'
mk_resource_methods
def exists?
@property_hash[:ensure] == :present
end
def destroy
args = Array.new
args << "#{@resource[:security_group]}"
args << "#{@resource[:ip_protocol]}"
args << "#{@resource[:from_port]}"
args << "#{@resource[:to_port]}"
if not @resource[:ip_range].nil?
args << "#{@resource[:ip_range]}"
else
args << "#{@resource[:source_group]}"
end
auth_nova("secgroup-delete-rule", args)
@property_hash[:ensure] = :absent
end
def create
args = Array.new
args << "#{@resource[:security_group]}"
args << "#{@resource[:ip_protocol]}"
args << "#{@resource[:from_port]}"
args << "#{@resource[:to_port]}"
if not @resource[:ip_range].nil?
args << "#{@resource[:ip_range]}"
else
args << "#{@resource[:source_group]}"
end
result = auth_nova("secgroup-add-rule", args)
end
end

View File

@ -0,0 +1,122 @@
require File.join(File.dirname(__FILE__), '..','..','..', 'puppet/provider/nova')
Puppet::Type.type(:nova_security_rule).provide(
:openstack,
:parent => Puppet::Provider::Nova
) do
desc <<-EOT
Manage nova security rules
EOT
@credentials = Puppet::Provider::Openstack::CredentialsV3.new
def create
opts = [@resource[:security_group]]
opts << '--protocol' << @resource[:ip_protocol]
if @resource[:ip_protocol].to_s == 'icmp'
unless @resource[:from_port].to_i == -1 and @resource[:to_port].to_i == -1
opts << "--icmp-type" << @resource[:from_port]
opts << "--icmp-code" << @resource[:to_port]
end
else
opts << "--dst-port" << "#{@resource[:from_port]}:#{@resource[:to_port]}"
end
unless @resource[:ip_range].nil?
opts << "--src-ip" << @resource[:ip_range]
else
opts << "--src-group" << @resource[:source_group]
end
@property_hash = self.class.nova_request('security group rule', 'create', nil, opts)
@property_hash[:ensure] = :present
end
def exists?
@property_hash[:ensure] == :present
end
def destroy
self.class.request('security group rule', 'delete', @property_hash[:name])
@property_hash[:ensure] == :absent
end
mk_resource_methods
def self.instances
rules = []
secgroup_provider = Puppet::Type.type(:nova_security_group).provider(:openstack)
groups = secgroup_provider.instances
groups.each do |g|
self.nova_request('security group rule', 'list', nil, ['--long', g.id]).each do |attrs|
# NOTE(mnaser): Originally, security groups were ingress only so to maintain
# backwards compatibility, we ignore all egress rules.
next if attrs[:direction] == 'egress'
# NOTE(mnaser): With Neutron, an empty ip_range means all networks, therefore
# we replace it by '0.0.0.0/0' for backwards compatibility.
attrs[:ip_range] = '0.0.0.0/0' if attrs[:ip_range].empty? and attrs[:remote_security_group].empty?
# NOTE(mnaser): Another quirk, Neutron can have an empty port range which means
# all ports, we adjust the field accordingly for the protocol.
if attrs[:port_range].empty?
if ['tcp', 'udp'].include? attrs[:ip_protocol]
attrs[:from_port] = 0
attrs[:to_port] = 65536
else
attrs[:from_port] = -1
attrs[:to_port] = -1
end
else
attrs[:from_port], attrs[:to_port] = attrs[:port_range].split(':')
end
rule = {
:ensure => :present,
:name => attrs[:id],
:security_group => g.name,
:from_port => attrs[:from_port],
:to_port => attrs[:to_port],
}
# NOTE(mnaser): The puppet type does not like getting source_group even if it's not set.
unless attrs[:ip_range].empty?
rule[:ip_range] = attrs[:ip_range]
else
rule[:source_group] = attrs[:remote_security_group]
end
# NOTE(mnaser): With Neutron, it is possible to have the ip_protocol empty
# which means all 3 protocols are allowed. We create three
# resources to maintain backwards compatible.
if attrs[:ip_protocol].empty?
rules << new(rule.merge(:ip_protocol => 'tcp', :from_port => 0, :to_port => 65536))
rules << new(rule.merge(:ip_protocol => 'udp', :from_port => 0, :to_port => 65536))
rules << new(rule.merge(:ip_protocol => 'icmp', :from_port => -1, :to_port => -1))
else
rules << new(rule.merge(:ip_protocol => attrs[:ip_protocol]))
end
end
end
rules
end
def self.prefetch(resources)
security_group_rules = instances
resources.keys.each do |name|
resource = resources[name].to_hash
rule = security_group_rules.find do |r|
r.security_group == resource[:security_group] && \
r.ip_protocol.to_s == resource[:ip_protocol].to_s && \
r.from_port.to_s == resource[:from_port].to_s && \
r.to_port.to_s == resource[:to_port].to_s
end
resources[name].provider = rule if rule
end
end
end

View File

@ -58,29 +58,22 @@ Puppet::Type.newtype(:nova_security_rule) do
end
newparam(:ip_protocol) do
defaultto do
raise Puppet::Error, 'You should give protocol!'
end
newvalues 'tcp', 'udp', 'icmp'
end
newparam(:from_port) do
defaultto do
raise Puppet::Error, 'You should give the source port!'
end
newvalues(/\d+/)
validate do |value|
if value !~ /\d+/ or value.to_i <= -1 or value.to_i >= 65536
if value.to_i < -1 or value.to_i >= 65536
raise Puppet::Error, 'Incorrect from port!'
end
end
end
newparam(:to_port) do
defaultto do
raise Puppet::Error, 'You should give the destination port!'
end
newvalues(/\d+/)
validate do |value|
if value !~ /\d+/ or value.to_i <= -1 or value.to_i >= 65536
if value.to_i < -1 or value.to_i >= 65536
raise Puppet::Error, 'Incorrect to port!'
end
end
@ -116,23 +109,26 @@ Puppet::Type.newtype(:nova_security_rule) do
end
end
newparam(:source_group) do
end
newparam(:security_group) do
defaultto do
raise Puppet::Error, 'You should provide the security group to add this rule to!'
end
end
newparam(:source_group)
newparam(:security_group)
validate do
unless !!self[:ip_range] ^ !!self[:source_group]
unless self[:from_port]
raise Puppet::Error, 'You should give the source port!'
end
unless self[:to_port]
raise Puppet::Error, 'You should give the destination port!'
end
unless self[:security_group]
raise Puppet::Error, 'You should provide the security group to add this rule to!'
end
unless self[:ip_range].to_s.empty? ^ self[:source_group].to_s.empty?
raise Puppet::Error, 'You should give either ip_range or source_group. Not none or both!'
end
unless self[:from_port].to_i <= self[:to_port].to_i
raise Puppet::Error, 'From_port should be lesser or equal to to_port!'
end
if self[:ip_protocol] != 'icmp' and (self[:from_port].to_i <= 0 || self[:to_port].to_i <= 0)
if self[:ip_protocol].to_s != 'icmp' and (self[:from_port].to_i <= 0 || self[:to_port].to_i <= 0)
raise Puppet::Error, 'From_port and To_port should not be less than 0 unless IP protocol is ICMP'
end
end

View File

@ -1,48 +0,0 @@
require 'puppet'
require 'puppet/provider/nova_security_rule/nova'
require 'tempfile'
provider_class = Puppet::Type.type(:nova_security_rule).provider(:nova)
describe provider_class do
let :secrule_attrs do
{
:name => "scr0",
:ip_protocol => "tcp",
:from_port => '22',
:to_port => '23',
:ip_range => '0.0.0.0/0',
:security_group => 'scg0'
}
end
let :resource do
Puppet::Type::Nova_security_rule.new(secrule_attrs)
end
let :provider do
provider_class.new(resource)
end
shared_examples "nova_security_rule" do
describe "#create" do
it 'should create security rule' do
provider.expects(:auth_nova).with('secgroup-add-rule', ['scg0', 'tcp', '22', '23', '0.0.0.0/0']).returns('+-------------+-----------+---------+-----------+--------------+\n| IP Protocol | From Port | To Port | IP Range | Source Group |\n+-------------+-----------+---------+-----------+--------------+\n| tcp | 22 | 23 | 0.0.0.0/0 | |\n+-------------+-----------+---------+-----------+--------------+')
provider.create
end
end
describe "#destroy" do
it 'should destroy security rule' do
provider.expects(:auth_nova).with('secgroup-delete-rule', ['scg0', 'tcp', '22', '23', '0.0.0.0/0']).returns('+-------------+-----------+---------+-----------+--------------+\n| IP Protocol | From Port | To Port | IP Range | Source Group |\n+-------------+-----------+---------+-----------+--------------+\n| tcp | 22 | 23 | 0.0.0.0/0 | |\n+-------------+-----------+---------+-----------+--------------+')
provider.destroy
end
end
end
it_behaves_like('nova_security_rule')
end

View File

@ -0,0 +1,61 @@
require 'puppet'
require 'spec_helper'
require 'puppet/provider/nova_security_rule/openstack'
provider_class = Puppet::Type.type(:nova_security_rule).provider(:openstack)
describe provider_class do
shared_examples 'authenticated with environment variables' do
ENV['OS_USERNAME'] = 'test'
ENV['OS_PASSWORD'] = 'abc123'
ENV['OS_PROJECT_NAME'] = 'test'
ENV['OS_AUTH_URL'] = 'http://127.0.0.1:35357/v3'
end
describe 'managing security group rules' do
let :secrule_attrs do
{
:name => "scr0",
:ip_protocol => "tcp",
:from_port => '22',
:to_port => '23',
:ip_range => '0.0.0.0/0',
:security_group => 'scg0'
}
end
let :resource do
Puppet::Type::Nova_security_rule.new(secrule_attrs)
end
let :provider do
provider_class.new(resource)
end
it_behaves_like 'authenticated with environment variables' do
describe "#create" do
it 'should create security group rule' do
provider.class.stubs(:openstack)
.with('security group rule', 'create', ['scg0', '--protocol', 'tcp', '--dst-port', '22:23', '--src-ip', '0.0.0.0/0'])
.returns('id="021114fb-67e0-4882-b2ed-e7c5328d8aa8"
protocol="tcp"
port_range_max="22"
port_range_min="23"
remote_ip_prefix="0.0.0.0/0"
security_group_id="4812fe3c-69d4-4b27-992b-163a20dc82d1"')
end
end
describe '#destroy' do
it 'removes security group rule' do
provider_class.expects(:openstack)
.with('security group rule', 'delete', 'scr0')
provider.instance_variable_set(:@property_hash, secrule_attrs)
provider.destroy
expect(provider.exists?).to be_falsey
end
end
end
end
end