Neutron parses the wrong json revert to shell/csv.

The neutron network provider is parsing the wrong json format leading
to an error when you're unlucky. Affected providers:
 - neutron_port
 - neutron_subnet
 - neutron_router
 - neutron_network

The neutron provider parses the json of the cliff-tablib's output while
the official one is the cliff' output[1].

As discussed in the ML[2] this patch implements a simple revert of the
json change.

Revert "Use json output instead plain-text"

This reverts commit 79f0e636c07b80b2c6699d934b42641bfef4352b[3]
 and part of commit 791a0f146b28544ec3d70c0ab2a950c4d5ca9f98[4]

[1]: https://bugs.launchpad.net/python-neutronclient/+bug/1529914
[2]: http://lists.openstack.org/pipermail/openstack-dev/2015-December/083172.html
[3]: https://review.openstack.org/#/c/238156/
[4]: https://review.openstack.org/#/c/261418/

Change-Id: I93263256eb9d83544910ca4055bb3e9a463645e3
Closes-bug: 1530163
This commit is contained in:
Sofer Athlan-Guyot 2015-12-31 16:36:05 +01:00
parent 0076a67070
commit 273ef1b228
3 changed files with 57 additions and 206 deletions

View File

@ -1,5 +1,4 @@
require 'csv'
require 'json'
require 'puppet/util/inifile'
class Puppet::Provider::Neutron < Puppet::Provider
@ -132,93 +131,64 @@ correctly configured.")
@neutron_credentials = nil
end
def self.find_and_parse_json(text)
# separate json from any possible garbage around it and parse
rv = []
if text.is_a? String
text = text.split("\n")
elsif !text.is_a? Array
return rv
end
found = false
(0..text.size-1).reverse_each do |line_no|
if text[line_no] =~ /\]\s*$/
end_of_json_line_no = line_no
(0..end_of_json_line_no).reverse_each do |start_of_json_line_no|
if text[start_of_json_line_no] =~ /^\s*\[/
begin
js_txt = text[start_of_json_line_no..end_of_json_line_no].join('')
rv = JSON.parse(js_txt)
found = true
rescue
# do nothing, next iteration please, because found==false
end
end
break if found
end
end
break if found
end
rv
end
def self.list_neutron_resources(type)
ids = []
list = auth_neutron("#{type}-list", '--format=json',
'--column=id')
list = auth_neutron("#{type}-list", '--format=csv',
'--column=id', '--quote=none')
if list.nil?
raise(Puppet::ExecutionFailure, "Can't retrieve #{type}-list because Neutron or Keystone API is not available.")
end
self.find_and_parse_json(list).each do |line|
ids << line['id']
(list.split("\n")[1..-1] || []).compact.collect do |line|
ids << line.strip
end
return ids
end
def self.get_neutron_resource_attrs(type, id)
attrs = {}
net = auth_neutron("#{type}-show", '--format=json', id)
net = auth_neutron("#{type}-show", '--format=shell', id)
if net.nil?
raise(Puppet::ExecutionFailure, "Can't retrieve #{type}-show because Neutron or Keystone API is not available.")
end
self.find_and_parse_json(net).each do |line|
k = line['Field']
v = line['Value']
if ['True', 'False'].include? v.to_s.capitalize
v = "#{v}".capitalize
elsif v.is_a? String and v =~ /\n/
v = v.split(/\n/)
elsif v.is_a? Numeric
v = "#{v}"
last_key = nil
(net.split("\n") || []).compact.collect do |line|
if line.include? '='
k, v = line.split('=', 2)
attrs[k] = v.gsub(/\A"|"\Z/, '')
last_key = k
else
v = "#{v}"
# Handle the case of a list of values
v = line.gsub(/\A"|"\Z/, '')
attrs[last_key] = [attrs[last_key], v].flatten
end
attrs[k] = v
end
return attrs
end
def self.list_router_ports(router_name_or_id)
results = []
cmd_output = auth_neutron("router-port-list",
'--format=json',
'--format=csv',
router_name_or_id)
self.find_and_parse_json(cmd_output).each do |port|
if port['fixed_ips']
if !port['fixed_ips'].empty?
fixed_ips = JSON.parse(port['fixed_ips'])
port['subnet_id'] = fixed_ips['subnet_id']
end
port.delete('fixed_ips')
end
results << port
if ! cmd_output
return results
end
headers = nil
CSV.parse(cmd_output) do |row|
if headers == nil
headers = row
else
result = Hash[*headers.zip(row).flatten]
match_data = /.*"subnet_id": "(.*)", .*/.match(result['fixed_ips'])
if match_data
result['subnet_id'] = match_data[1]
end
results << result
end
end
return results
end

View File

@ -93,25 +93,15 @@ tenant_id="60f9544eb94c42a6b7e8e98c2be981b1"'
end
it 'should detect a gateway net id' do
output = <<-EOT
bla-bla-bla
[{"Field": "external_gateway_info",
"Value": "{\\"network_id\\": \\"1b-b1\\", \\"enable_snat\\": true, \\"external_fixed_ips\\": [{\\"subnet_id\\": \\"1b-b1\\", \\"ip_address\\": \\"1.1.1.1\\"}]}"
}]
EOT
klass.stubs(:auth_neutron).returns(output)
klass.stubs(:auth_neutron).returns(
'external_gateway_info="{\"network_id\": \"1b-b1\", \"enable_snat\": true, \"external_fixed_ips\": [{\"subnet_id\": \"1b-b1\", \"ip_address\": \"1.1.1.1\"}]}"'
)
result = klass.get_neutron_resource_attrs 'foo', nil
expect(provider.parse_gateway_network_id(result['external_gateway_info'])).to eql('1b-b1')
end
it 'should return empty value, if there is no net id found' do
output = <<-EOT
bla-bla-bla
[{"Field": "external_gateway_info", "Value": "{}"}]
EOT
klass.stubs(:auth_neutron).returns(output)
klass.stubs(:auth_neutron).returns('external_gateway_info="{}"')
result = klass.get_neutron_resource_attrs 'foo', nil
expect(provider.parse_gateway_network_id(result['external_gateway_info'])).to eql('')
end

View File

@ -131,9 +131,9 @@ describe Puppet::Provider::Neutron do
it 'should exclude the column header' do
output = <<-EOT
bla-bla-bla
[{"id": "net1"},{"id": "net2"}]
id
net1
net2
EOT
klass.expects(:auth_neutron).returns(output)
result = klass.list_neutron_resources('foo')
@ -141,19 +141,6 @@ describe Puppet::Provider::Neutron do
end
it 'should return empty list when there are no neutron resources' do
output = <<-EOT
bla-bla-bla
[]
bla-bla
EOT
klass.stubs(:auth_neutron).returns(output)
result = klass.list_neutron_resources('foo')
expect(result).to eql([])
end
it 'should return empty respons when there are no neutron resources' do
output = <<-EOT
EOT
klass.stubs(:auth_neutron).returns(output)
@ -173,21 +160,16 @@ describe Puppet::Provider::Neutron do
describe 'when retrieving attributes for neutron resources' do
it 'should parse single-valued attributes into a key-value pair' do
output = <<-EOT
bla-bla-bla
[{"Field": "admin_state_up", "Value": true}]
EOT
klass.expects(:auth_neutron).returns(output)
klass.expects(:auth_neutron).returns('admin_state_up="True"')
result = klass.get_neutron_resource_attrs('foo', 'id')
expect(result).to eql({"admin_state_up" => "True"})
expect(result).to eql({"admin_state_up" => 'True'})
end
it 'should parse multi-valued attributes into a key-list pair' do
output = <<-EOT
bla-bla-bla
[{"Field": "subnets", "Value": "subnet1\\nsubnet2\\nsubnet3"}]
subnets="subnet1
subnet2
subnet3"
EOT
klass.expects(:auth_neutron).returns(output)
result = klass.get_neutron_resource_attrs('foo', 'id')
@ -204,63 +186,35 @@ describe Puppet::Provider::Neutron do
it 'should handle an empty port list' do
klass.expects(:auth_neutron).with('router-port-list',
'--format=json',
'--format=csv',
router)
result = klass.list_router_ports(router)
expect(result).to eql([])
end
it 'should handle several ports' do
output = '''
[
{
"id": "1345e576-a21f-4c2e-b24a-b245639852ab",
"name": "",
"mac_address": "fa:16:3e:e3:e6:38",
"fixed_ips": "{\"subnet_id\": \"839a1d2d-2c6e-44fb-9a2b-9b011dce8c2f\", \"ip_address\": \"10.0.0.1\"}"
},
{
"id": "de0dc526-02b2-467c-9832-2c3dc69ac2b4",
"name": "",
"mac_address": "fa:16:3e:f6:b5:72",
"fixed_ips": "{\"subnet_id\": \"e4db0abd-276a-4f69-92ea-8b9e4eacfd43\", \"ip_address\": \"172.24.4.226\"}"
}
]
'''
output = <<-EOT
"id","name","mac_address","fixed_ips"
"1345e576-a21f-4c2e-b24a-b245639852ab","","fa:16:3e:e3:e6:38","{""subnet_id"": ""839a1d2d-2c6e-44fb-9a2b-9b011dce8c2f"", ""ip_address"": ""10.0.0.1""}"
"de0dc526-02b2-467c-9832-2c3dc69ac2b4","","fa:16:3e:f6:b5:72","{""subnet_id"": ""e4db0abd-276a-4f69-92ea-8b9e4eacfd43"", ""ip_address"": ""172.24.4.226""}"
EOT
expected =
[{ "name"=>"",
[{ "fixed_ips"=>
"{\"subnet_id\": \"839a1d2d-2c6e-44fb-9a2b-9b011dce8c2f\", \
\"ip_address\": \"10.0.0.1\"}",
"name"=>"",
"subnet_id"=>"839a1d2d-2c6e-44fb-9a2b-9b011dce8c2f",
"id"=>"1345e576-a21f-4c2e-b24a-b245639852ab",
"mac_address"=>"fa:16:3e:e3:e6:38"},
{ "name"=>"",
{"fixed_ips"=>
"{\"subnet_id\": \"e4db0abd-276a-4f69-92ea-8b9e4eacfd43\", \
\"ip_address\": \"172.24.4.226\"}",
"name"=>"",
"subnet_id"=>"e4db0abd-276a-4f69-92ea-8b9e4eacfd43",
"id"=>"de0dc526-02b2-467c-9832-2c3dc69ac2b4",
"mac_address"=>"fa:16:3e:f6:b5:72"}]
klass.expects(:auth_neutron).
with('router-port-list', '--format=json', router).
returns(output)
result = klass.list_router_ports(router)
expect(result).to eql(expected)
end
it 'should handle empty fixed_ips field' do
output = '''
[
{
"id": "1345e576-a21f-4c2e-b24a-b245639852ab",
"name": "",
"mac_address": "fa:16:3e:e3:e6:38",
"fixed_ips": ""
}
]
'''
expected =
[{ "name"=>"",
"id"=>"1345e576-a21f-4c2e-b24a-b245639852ab",
"mac_address"=>"fa:16:3e:e3:e6:38"}]
klass.expects(:auth_neutron).
with('router-port-list', '--format=json', router).
with('router-port-list', '--format=csv', router).
returns(output)
result = klass.list_router_ports(router)
expect(result).to eql(expected)
@ -289,67 +243,4 @@ tenant_id="3056a91768d948d399f1d79051a7f221"
end
describe 'should parse valid json output, covered by garbage' do
it 'should parse valid output into a list of hashes' do
data = '''
/usr/lib/python2.7/dist-packages/urllib3/util/ssl_.py:90: InsecurePlatformWarning: A true SSLContext object is not available. This prevents urllib3 from configuring SSL appropriately and may cause certain SSL connections to fail. For more information, see https://urllib3.readthedocs.org/en/latest/security.html#insecureplatformwarning.
InsecurePlatformWarning
/usr/lib/python2.7/dist-packages/urllib3/connection.py:251: SecurityWarning: Certificate has no `subjectAltName`, falling back to check for a `commonName` for now. This feature is being removed by major browsers and deprecated by RFC 2818. (See https://github.com/shazow/urllib3/issues/497 for details.)
SecurityWarning
[{"Field": "allocation_pools", "Value": "{\"start\": \"192.168.111.2\", \"end\": \"192.168.111.254\"}"}, {"Field": "cidr", "Value": "192.168.111.0/24"},
{"Field": "dns_nameservers", "Value": "8.8.4.4\n8.8.8.8"}, {"Field": "enable_dhcp", "Value": true}, {"Field": "gateway_ip", "Value": "192.168.111.1"}, {"Field": "host_routes", "Value": ""}, {"Field": "id", "Value": "b87fbfd1-0e52-4ab6-8987-286ef0912d1f"}, {"Field": "ip_version", "Value": 4}, {"Field": "ipv6_address_mode", "Value": ""}, {"Field": "ipv6_ra_mode", "Value": ""},
{"Field": "XXX", "Value":
[1,
2,3]},
{"Field": "name", "Value": "net04__subnet"}, {"Field": "network_id", "Value": "d70b399b-668b-4861-b092-4876ec65df60"}, {"Field": "subnetpool_id", "Value": ""}, {"Field": "tenant_id", "Value": "2764315d0ec24a07bf3773057aa51142"}]
xxx yyy zz
eof
'''
expected = [
{"Field"=>"allocation_pools", "Value"=>"{\"start\": \"192.168.111.2\", \"end\": \"192.168.111.254\"}"},
{"Field"=>"cidr", "Value"=>"192.168.111.0/24"},
{"Field"=>"dns_nameservers", "Value"=>"8.8.4.4\n8.8.8.8"},
{"Field"=>"enable_dhcp", "Value"=>true},
{"Field"=>"gateway_ip", "Value"=>"192.168.111.1"},
{"Field"=>"host_routes", "Value"=>""},
{"Field"=>"id", "Value"=>"b87fbfd1-0e52-4ab6-8987-286ef0912d1f"},
{"Field"=>"ip_version", "Value"=>4},
{"Field"=>"ipv6_address_mode", "Value"=>""},
{"Field"=>"ipv6_ra_mode", "Value"=>""},
{"Field"=>"XXX", "Value"=>[1, 2, 3]},
{"Field"=>"name", "Value"=>"net04__subnet"},
{"Field"=>"network_id", "Value"=>"d70b399b-668b-4861-b092-4876ec65df60"},
{"Field"=>"subnetpool_id", "Value"=>""},
{"Field"=>"tenant_id", "Value"=>"2764315d0ec24a07bf3773057aa51142"}]
expect(klass.find_and_parse_json(data)).to eq(expected)
end
end
describe 'should parse valid json output, and convert booleans to idempotent strings' do
it 'boolean values should converted to capitalized strings' do
output = '''
[{"Field": "allocation_pools", "Value": "{\"start\": \"192.168.111.2\", \"end\": \"192.168.111.254\"}"}, {"Field": "cidr", "Value": "192.168.111.0/24"},
{"Field": "dns_nameservers", "Value": "8.8.4.4\n8.8.8.8"}, {"Field": "enable_dhcp", "Value": true}, {"Field": "gateway_ip", "Value": "192.168.111.1"}, {"Field": "host_routes", "Value": ""}, {"Field": "id", "Value": "b87fbfd1-0e52-4ab6-8987-286ef0912d1f"}, {"Field": "ip_version", "Value": 4}, {"Field": "ipv6_address_mode", "Value": ""}, {"Field": "ipv6_ra_mode", "Value": ""},
{"Field": "YYY", "Value": false}]
'''
klass.stubs(:auth_neutron).returns(output)
result = klass.get_neutron_resource_attrs 'foo', nil
expect(result['enable_dhcp']).to eql('True')
expect(result['YYY']).to eql('False')
end
it 'stringifyed boolean values should converted to capitalized strings' do
output = '''
[{"Field": "allocation_pools", "Value": "{\"start\": \"192.168.111.2\", \"end\": \"192.168.111.254\"}"}, {"Field": "cidr", "Value": "192.168.111.0/24"},
{"Field": "dns_nameservers", "Value": "8.8.4.4\n8.8.8.8"}, {"Field": "enable_dhcp", "Value": "True"}, {"Field": "gateway_ip", "Value": "192.168.111.1"}, {"Field": "host_routes", "Value": ""}, {"Field": "id", "Value": "b87fbfd1-0e52-4ab6-8987-286ef0912d1f"}, {"Field": "ip_version", "Value": 4}, {"Field": "ipv6_address_mode", "Value": ""}, {"Field": "ipv6_ra_mode", "Value": ""},
{"Field": "YYY", "Value": "false"}]
'''
klass.stubs(:auth_neutron).returns(output)
result = klass.get_neutron_resource_attrs 'foo', nil
expect(result['enable_dhcp']).to eql('True')
expect(result['YYY']).to eql('False')
end
end
end