openstack/puppet-nova
Code Issues Proposed changes

Remove resource types to manage security groups and rules

Browse Source 
The nova_security_group resource type and the nova_security_rule
resource type are dependent on subcommands of nova cli which were
already removed during Pike cycle[1][2]. This change removes these
invalid resource types.

Deprecation process is skipped because these resource types have not
worked as intended for a long period and we don't expect any user is
still using these invalid implementations.

[1] security group      : a298b29cc7e6b7330945b1890f0a4bd4c9f3fde6
[2] security groip rules: 0896bdc52a307c0b9598da0b6b837a95f0c00b9a

Closes-Bug: #1941947
Change-Id: Ic72911bab169b0ab171a1701b6871a3d03f7951b
This commit is contained in:
Takashi Kajinami 2021-08-28 19:52:53 +09:00
parent f57463ca68
commit 55874cb8ea
9 changed files with 9 additions and 642 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

80
lib/puppet/provider/nova_security_group/openstack.rb
View File

@ -1,80 +0,0 @@
require File.join(File.dirname(__FILE__), '..','..','..', 'puppet/provider/nova')
Puppet::Type.type(:nova_security_group).provide(
:openstack,
:parent => Puppet::Provider::Nova
) do
desc <<-EOT
Manage nova security groups
EOT
@credentials = Puppet::Provider::Openstack::CredentialsV3.new
def initialize(value={})
super(value)
@property_flush = {}
end
def create
opts = [@resource[:name]]
(opts << '--description' << @resource[:description]) if @resource[:description]
@property_hash = self.class.nova_request('security group', 'create', nil, opts)
@property_hash[:ensure] = :present
end
def exists?
@property_hash[:ensure] == :present
end
def destroy
self.class.request('security group', 'delete', @resource[:name])
end
mk_resource_methods
def id=(value)
fail('id is read only')
end
def name=(value)
fail('name is read only')
end
def description=(value)
@property_flush[:description] = value
end
def self.instances
# NOTE(mnaser): The OpenStack client makes a request to the Neutron endpoint
# to get security groups and if it has an admin role, it will
# retrieve all security groups. The following helps filter it.
project_id = self.nova_request('token', 'issue', nil, ['-c', 'project_id', '-f', 'value']).strip
self.nova_request('security group', 'list', nil, ['--project', project_id]).collect do |attrs|
new(
:ensure => :present,
:id => attrs[:id],
:name => attrs[:name],
:description => attrs[:description]
)
end
end
def self.prefetch(resources)
security_groups = instances
resources.keys.each do |name|
if provider = security_groups.find { |security_group| security_group.name == name }
resources[name].provider = provider
end
end
end
def flush
unless @property_flush.empty?
opts = [@resource[:name]]
(opts << '--description' << @resource[:description]) if @resource[:description]
self.class.request('security group', 'set', opts)
@property_flush.clear
end
end
end

122
lib/puppet/provider/nova_security_rule/openstack.rb
View File

@ -1,122 +0,0 @@
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 << "--remote-ip" << @resource[:ip_range]
else
opts << "--remote-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

66
lib/puppet/type/nova_security_group.rb
View File

@ -1,66 +0,0 @@
# Copyright (C) 2016 Mirantis Inc.
#
# Author: Alexey Deryugin <aderyugin@mirantis.com>
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
#
# nova_security_group type
#
# == Parameters
# [*name*]
# Name for the new security group
# Required
#
# [*description*]
# Description for the new security group
# Optional
#
require 'puppet'
Puppet::Type.newtype(:nova_security_group) do
@doc = "Manage creation of nova security groups."
ensurable
newparam(:name, :namevar => true) do
desc 'Name for the new security group'
validate do |value|
if not value.is_a? String
raise ArgumentError, "name parameter must be a String"
end
unless value =~ /^[a-zA-Z0-9\-_]+$/
raise ArgumentError, "#{value} is not a valid name"
end
end
end
newproperty(:id) do
desc 'The unique Id of the security group'
validate do |v|
raise ArgumentError, 'This is a read only property'
end
end
newproperty(:description) do
desc "Description of the security group"
defaultto ''
end
validate do
raise ArgumentError, 'Name type must be set' unless self[:name]
end
end

140
lib/puppet/type/nova_security_rule.rb
View File

@ -1,140 +0,0 @@
# -*- coding: utf-8 -*-
#
# Copyright (C) 2016 Mirantis Inc.
#
# Author: Alexey Deryugin <aderyugin@mirantis.com>
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
#
# nova_security_group type
#
# == Parameters
# [*ip_protocol*]
# IP protocol from new security rule
# Required
#
# [*from_port*]
# Port range begin for security rule
# Required
#
# [*to_port*]
# Port range end for security rule
# Required
#
# [*ip_range*]
# IP range for security rule
# Optional
#
# [*source_group*]
# Source group for security rule
# Optional
#
# [*security_group*]
# Target security group for security rule
# Required
#
require 'puppet'
Puppet::Type.newtype(:nova_security_rule) do
desc "Manage nova security rules"
ensurable
newparam(:name) do
isnamevar
end
newparam(:ip_protocol) do
newvalues 'tcp', 'udp', 'icmp'
end
newparam(:from_port) do
newvalues(/\d+/)
validate do |value|
if value.to_i < -1 or value.to_i >= 65536
raise Puppet::Error, 'Incorrect from port!'
end
end
end
newparam(:to_port) do
newvalues(/\d+/)
validate do |value|
if value.to_i < -1 or value.to_i >= 65536
raise Puppet::Error, 'Incorrect to port!'
end
end
end
newparam(:ip_range) do
validate do |value|
def is_cidr_net?(value)
begin
address, mask = value.split('/')
return false unless address and mask
octets = address.split('.')
return false unless octets.length == 4
cidr = true
octets.each do |octet|
n = octet.to_i
cidr = false unless n <= 255
cidr = false unless n >= 0
break unless cidr
end
cidr = false unless mask.to_i <= 32
cidr = false unless mask.to_i >= 0
cidr
rescue
false
end
end
raise Puppet::Error, 'Incorrect ip_range!' unless is_cidr_net? value
end
end
newparam(:source_group)
newparam(:security_group)
validate do
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].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
autorequire(:nova_security_group) do
self[:security_group]
end
end

9
releasenotes/notes/remove-sg-resource-types-de1a92488d6f1c19.yaml Normal file
View File

@ -0,0 +1,9 @@
---
upgrade:
- |
The following two resource types have been removed. These are dependent on
the APIs of python-novaclient which were already removed during Pike cycle.
Use the resource types provided by puppet-neutron instead.
- ``nova_security_group``
- ``nova_security_rule``

48
spec/unit/provider/nova_security_group/openstack_spec.rb
View File

@ -1,48 +0,0 @@
require 'puppet'
require 'spec_helper'
require 'puppet/provider/nova_flavor/openstack'
provider_class = Puppet::Type.type(:nova_security_group).provider(:openstack)
describe provider_class do
describe 'managing security groups' do
let(:secgroup_attrs) do
{
:name => "scg0",
:description => "Security Group",
}
end
let :resource do
Puppet::Type::Nova_security_group.new(secgroup_attrs)
end
let(:provider) do
provider_class.new(resource)
end
describe "#create" do
it 'should create security group' do
provider.class.stubs(:openstack)
.with('security group', 'list', ['--all'])
.returns('"ID", "Name", "Description", "Project"')
provider.class.stubs(:openstack)
.with('security group', 'create', ['scg0', '--description', 'Security Group'])
.returns('id="f630dd92-3ff7-49bc-b012-b211451aa419"
name="scg0"
description="Security Group"')
end
end
describe '#destroy' do
it 'removes flavor' do
provider_class.expects(:openstack)
.with('security group', 'delete', 'scg0')
provider.instance_variable_set(:@property_hash, secgroup_attrs)
provider.destroy
expect(provider.exists?).to be_falsey
end
end
end
end

61
spec/unit/provider/nova_security_rule/openstack_spec.rb
View File

@ -1,61 +0,0 @@
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:5000/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', '--remote-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

20
spec/unit/type/nova_security_group_spec.rb
View File

@ -1,20 +0,0 @@
require 'puppet'
require 'puppet/type/nova_security_group'
describe 'Puppet::Type.type(:nova_security_group)' do
it 'should reject invalid name value' do
expect { Puppet::Type.type(:nova_security_group).new(:name => 65535) }.to raise_error(Puppet::Error, /name parameter must be a String/)
expect { Puppet::Type.type(:nova_security_group).new(:name => 'sc g0') }.to raise_error(Puppet::Error, /is not a valid name/)
end
it 'should accept a valid name value' do
Puppet::Type.type(:nova_security_group).new(:name => 'scg0')
end
it 'should accept description' do
Puppet::Type.type(:nova_security_group).new(:name => 'scg0',
:description => 'Security Group')
end
end

105
spec/unit/type/nova_security_rule_spec.rb
View File

@ -1,105 +0,0 @@
require 'puppet'
require 'puppet/type/nova_security_group'
describe 'Puppet::Type.type(:nova_security_group)' do
it 'should reject an invalid ipv4 CIDR value' do
expect { Puppet::Type.type(:nova_security_rule).new(:name => 'scr0',
:ip_protocol => 'tcp',
:from_port => '22',
:to_port => '22',
:ip_range => '192.168.1.0',
:security_group => 'scg0') }.to raise_error(Puppet::Error, /Incorrect ip_range!/)
expect { Puppet::Type.type(:nova_security_rule).new(:name => 'scr0',
:ip_protocol => 'tcp',
:from_port => '22',
:to_port => '22',
:ip_range => '::1/24',
:security_group => 'scg0') }.to raise_error(Puppet::Error, /Incorrect ip_range!/)
end
it 'should reject an invalid from port value' do
expect { Puppet::Type.type(:nova_security_rule).new(:name => 'scr0',
:ip_protocol => 'tcp',
:from_port => '-22',
:to_port => '22',
:ip_range => '192.168.1.0/24',
:security_group => 'scg0') }.to raise_error(Puppet::Error, /Incorrect from port!/)
expect { Puppet::Type.type(:nova_security_rule).new(:name => 'scr0',
:ip_protocol => 'tcp',
:to_port => '22',
:ip_range => '192.168.1.0/24',
:security_group => 'scg0') }.to raise_error(Puppet::Error, /You should give the source port/)
end
it 'should reject an invalid from port value' do
expect { Puppet::Type.type(:nova_security_rule).new(:name => 'scr0',
:ip_protocol => 'tcp',
:from_port => '22',
:to_port => '-22',
:ip_range => '192.168.1.0/24',
:security_group => 'scg0') }.to raise_error(Puppet::Error, /Incorrect to port!/)
expect { Puppet::Type.type(:nova_security_rule).new(:name => 'scr0',
:ip_protocol => 'tcp',
:from_port => '22',
:ip_range => '192.168.1.0/24',
:security_group => 'scg0') }.to raise_error(Puppet::Error, /You should give the destination port/)
end
it 'should fails with security group not specified' do
expect { Puppet::Type.type(:nova_security_rule).new(:name => 'scr0',
:ip_protocol => 'tcp',
:from_port => '22',
:to_port => '22',
:ip_range => '192.168.1.0/24') }.to raise_error(Puppet::Error, /You should provide the security group/)
end
it 'should fails with none of ip_range and source_group specified' do
expect { Puppet::Type.type(:nova_security_rule).new(:name => 'scr0',
:ip_protocol => 'tcp',
:from_port => '22',
:to_port => '22',
:security_group => 'scg0') }.to raise_error(Puppet::Error, /You should give either ip_range or source_group/)
end
it 'should fails with both ip_range and source group specified' do
expect { Puppet::Type.type(:nova_security_rule).new(:name => 'scr0',
:ip_protocol => 'tcp',
:from_port => '22',
:to_port => '22',
:ip_range => '192.168.1.0/24',
:source_group => 'tenant',
:security_group => 'scg0') }.to raise_error(Puppet::Error, /You should give either ip_range or source_group/)
end
it 'should accept a valid parameters' do
Puppet::Type.type(:nova_security_rule).new(:name => 'scr0',
:ip_protocol => 'tcp',
:from_port => '22',
:to_port => '22',
:ip_range => '192.168.1.0/24',
:security_group => 'scg0')
end
it 'should autorequire the related nova security group' do
catalog = Puppet::Resource::Catalog.new
s_group = Puppet::Type.type(:nova_security_group).new(
:name => 'allow_all',
:description => 'Allow all traffic'
)
s_rule = Puppet::Type.type(:nova_security_rule).new(
:name => 'all_01',
:ip_protocol => 'tcp',
:from_port => '1',
:to_port => '65535',
:ip_range => '0.0.0.0/0',
:security_group => 'allow_all'
)
catalog.add_resource s_group, s_rule
dependency = s_rule.autorequire
expect(dependency.size).to eq(1)
expect(dependency[0].target).to eq(s_rule)
expect(dependency[0].source).to eq(s_group)
end
end