From 6eb78fc43538aec47a899ba1bf1a52bae354811b Mon Sep 17 00:00:00 2001 From: Bill Wei Date: Thu, 28 Sep 2017 15:16:05 -0400 Subject: [PATCH] Enhance parameter type handling Better support non-string parameter types including comma-delimited-list, number, boolean https://bugzilla.redhat.com/show_bug.cgi?id=1489908 --- .../cloud_manager/orchestration_stack.rb | 16 ++ .../cloud_manager/orchestration_template.rb | 137 +++++++++++++++--- .../heat_parameters.json | 87 ++++++++++- .../heat_parameters.yml | 59 ++++++-- .../cloud_manager/orchestration_stack_spec.rb | 10 ++ .../orchestration_template_spec.rb | 66 ++++++--- 6 files changed, 320 insertions(+), 55 deletions(-) diff --git a/app/models/manageiq/providers/openstack/cloud_manager/orchestration_stack.rb b/app/models/manageiq/providers/openstack/cloud_manager/orchestration_stack.rb index 39390cab0..7655897d8 100644 --- a/app/models/manageiq/providers/openstack/cloud_manager/orchestration_stack.rb +++ b/app/models/manageiq/providers/openstack/cloud_manager/orchestration_stack.rb @@ -3,6 +3,7 @@ class ManageIQ::Providers::Openstack::CloudManager::OrchestrationStack < ManageI def self.raw_create_stack(orchestration_manager, stack_name, template, options = {}) create_options = {:stack_name => stack_name, :template => template.content}.merge(options).except(:tenant_name) + transform_parameters(template, create_options[:parameters]) if create_options[:parameters] connection_options = {:service => "Orchestration"}.merge(options.slice(:tenant_name)) orchestration_manager.with_provider_connection(connection_options) do |service| service.stacks.new.save(create_options)["id"] @@ -14,6 +15,7 @@ def self.raw_create_stack(orchestration_manager, stack_name, template, options = def raw_update_stack(template, options) update_options = {:template => template.content}.merge(options.except(:disable_rollback, :timeout_mins)) + self.class.transform_parameters(template, update_options[:parameters]) if update_options[:parameters] connection_options = {:service => "Orchestration"} connection_options[:tenant_name] = cloud_tenant.name if cloud_tenant ext_management_system.with_provider_connection(connection_options) do |service| @@ -51,4 +53,18 @@ def raw_status _log.error "stack=[#{name}], error: #{err}" raise MiqException::MiqOrchestrationStatusError, err.to_s, err.backtrace end + + def self.transform_parameters(template, deploy_parameters) + list_re = /^(comma_delimited_list)|(CommaDelimitedList)|(List<.+>)$/ + # convert multiline text to comma delimited string + template.parameter_groups.each do |group| + group.parameters.each do |para_def| + next unless para_def.data_type =~ list_re + parameter = deploy_parameters[para_def.name] + next if parameter.nil? || !parameter.kind_of?(String) + parameter.chomp!('') + parameter.tr!("\n", ",") + end + end + end end diff --git a/app/models/manageiq/providers/openstack/cloud_manager/orchestration_template.rb b/app/models/manageiq/providers/openstack/cloud_manager/orchestration_template.rb index 4149b7316..131297ffc 100644 --- a/app/models/manageiq/providers/openstack/cloud_manager/orchestration_template.rb +++ b/app/models/manageiq/providers/openstack/cloud_manager/orchestration_template.rb @@ -22,17 +22,22 @@ def parameter_groups end end + # Parsing parameters for both Hot and Cloudformation formats + # Keys in Hot are lower case snake style, + # label, json, boolean, constraints are Hot only + # Keys in Cloudformation are upper case camel style + # List<> is Cloudformation only def parameters(content_hash = nil) content_hash ||= parse (content_hash["parameters"] || content_hash["Parameters"] || {}).collect do |key, val| OrchestrationTemplate::OrchestrationParameter.new( :name => key, :label => val.key?('label') ? val['label'] : key.titleize, - :data_type => val['type'], - :default_value => val['default'], - :description => val['description'], - :hidden => val['hidden'] == true, - :constraints => val.key?('constraints') ? parse_constraints(val['constraints']) : nil, + :data_type => val['type'] || val['Type'], + :default_value => parse_default_value(val), + :description => val['description'] || val['Description'], + :hidden => parse_hidden(val), + :constraints => ([constraint_from_type(val)] + parse_constraints(val)).compact, :required => true ) end @@ -97,39 +102,90 @@ def validate_format_json err.message end - def parse_constraints(raw_constraints) - raw_constraints.collect do |raw_constraint| + def parse_default_value(parameter) + raw_default = parameter['default'] || parameter['Default'] + case parameter['type'] || parameter['Type'] + when 'json' + JSON.pretty_generate(raw_default || {'sample(please delete)' => 'JSON format'}) + when 'comma_delimited_list', 'CommaDelimitedList', /^List<.+>$/ + multiline_value_for_list(raw_default) + when 'boolean' + ([true, 1] + %w(t T y Y yes Yes YES true True TRUE 1)).include?(raw_default) + else + raw_default + end + end + + def multiline_value_for_list(val) + return val.join("\n") if val.kind_of?(Array) + return val.tr!(",", "\n") if val.kind_of?(String) + "sample1(please delete)\nsample2(please delete)" + end + + def parse_hidden(parameter) + val = parameter.key?('hidden') ? parameter['hidden'] : parameter['NoEcho'] + return true if val == true || val == 'true' + false + end + + def constraint_from_type(parameter) + case parameter['type'] || parameter['Type'] + when 'json' + OrchestrationTemplate::OrchestrationParameterMultiline.new( + :description => 'Parameter in JSON format' + ) + when 'comma_delimited_list', 'CommaDelimitedList', /^List<.*>$/ + OrchestrationTemplate::OrchestrationParameterMultiline.new( + :description => 'Parameter in list format' + ) + when 'boolean' + OrchestrationTemplate::OrchestrationParameterBoolean.new + when 'number', 'Number' + OrchestrationTemplate::OrchestrationParameterPattern.new( + :pattern => '^[+-]?([1-9]\d*|0)(\.\d+)?$', + :description => 'Numeric parameter' + ) + end + end + + def parse_constraints(parameter) + return parse_constraints_hot(parameter['constraints']) if parameter.key?('constraints') + parse_constraints_cfn(parameter) + end + + def parse_constraints_hot(raw_constraints) + (raw_constraints || []).collect do |raw_constraint| if raw_constraint.key?('allowed_values') - parse_allowed_values(raw_constraint) + parse_allowed_values_hot(raw_constraint) elsif raw_constraint.key?('allowed_pattern') - parse_pattern(raw_constraint) + parse_pattern_hot(raw_constraint) elsif raw_constraint.key?('length') - parse_length_constraint(raw_constraint) + parse_length_constraint_hot(raw_constraint) elsif raw_constraint.key?('range') - parse_value_constraint(raw_constraint) + parse_value_constraint_hot(raw_constraint) elsif raw_constraint.key?('custom_constraint') - parse_custom_constraint(raw_constraint) + parse_custom_constraint_hot(raw_constraint) else raise MiqException::MiqParsingError, _("Unknown constraint %{constraint}") % {:constraint => raw_constraint} end end end - def parse_allowed_values(hash) + def parse_allowed_values_hot(hash) OrchestrationTemplate::OrchestrationParameterAllowed.new( :allowed_values => hash['allowed_values'], :description => hash['description'] ) end - def parse_pattern(hash) + def parse_pattern_hot(hash) OrchestrationTemplate::OrchestrationParameterPattern.new( :pattern => hash['allowed_pattern'], :description => hash['description'] ) end - def parse_length_constraint(hash) + def parse_length_constraint_hot(hash) OrchestrationTemplate::OrchestrationParameterLength.new( :min_length => hash['length']['min'], :max_length => hash['length']['max'], @@ -137,7 +193,7 @@ def parse_length_constraint(hash) ) end - def parse_value_constraint(hash) + def parse_value_constraint_hot(hash) OrchestrationTemplate::OrchestrationParameterRange.new( :min_value => hash['range']['min'], :max_value => hash['range']['max'], @@ -145,10 +201,57 @@ def parse_value_constraint(hash) ) end - def parse_custom_constraint(hash) + def parse_custom_constraint_hot(hash) OrchestrationTemplate::OrchestrationParameterCustom.new( :custom_constraint => hash['custom_constraint'], :description => hash['description'] ) end + + def parse_constraints_cfn(raw_constraints) + constraints = [] + if raw_constraints.key?('AllowedValues') + constraints << parse_allowed_values_cfn(raw_constraints) + end + if raw_constraints.key?('AllowedPattern') + constraints << parse_pattern_cfn(raw_constraints) + end + if raw_constraints.key?('MinLength') || raw_constraints.key?('MaxLength') + constraints << parse_length_constraint_cfn(raw_constraints) + end + if raw_constraints.key?('MinValue') || raw_constraints.key?('MaxValue') + constraints << parse_value_constraint_cfn(raw_constraints) + end + constraints + end + + def parse_allowed_values_cfn(hash) + OrchestrationTemplate::OrchestrationParameterAllowed.new( + :allowed_values => hash['AllowedValues'], + :description => hash['ConstraintDescription'] + ) + end + + def parse_pattern_cfn(hash) + OrchestrationTemplate::OrchestrationParameterPattern.new( + :pattern => hash['AllowedPattern'], + :description => hash['ConstraintDescription'] + ) + end + + def parse_length_constraint_cfn(hash) + OrchestrationTemplate::OrchestrationParameterLength.new( + :min_length => hash['MinLength'], + :max_length => hash['MaxLength'], + :description => hash['ConstraintDescription'] + ) + end + + def parse_value_constraint_cfn(hash) + OrchestrationTemplate::OrchestrationParameterRange.new( + :min_value => hash['MinValue'], + :max_value => hash['MaxValue'], + :description => hash['ConstraintDescription'] + ) + end end diff --git a/spec/fixtures/orchestration_templates/heat_parameters.json b/spec/fixtures/orchestration_templates/heat_parameters.json index 2efa8c9b5..ac27d4194 100644 --- a/spec/fixtures/orchestration_templates/heat_parameters.json +++ b/spec/fixtures/orchestration_templates/heat_parameters.json @@ -1 +1,86 @@ -{"heat_template_version":"2013-05-23","description":"Incomplete sample template for testing parsing of various parameters","parameter_groups":[{"label":"General parameters","description":"General parameters","parameters":["flavor","image_id","cartridges"]},{"parameters":["admin_pass","db_port","metadata"]}],"parameters":{"admin_pass":{"type":"string","description":"Admin password","hidden":true,"constraints":[{"length":{"min":6,"max":8},"description":"Admin password must be between 6 and 8 characters long.\n"},{"allowed_pattern":"[a-zA-Z0-9]+","description":"Password must consist of characters and numbers only"},{"allowed_pattern":"[A-Z]+[a-zA-Z0-9]*","description":"Password must start with an uppercase character"}]},"flavor":{"type":"string","description":"Flavor for the instances to be created","default":"m1.small","constraints":[{"custom_constraint":"nova.flavor","description":"Must be a flavor known to Nova"}]},"cartridges":{"description":"Cartridges to install. \"all\" for all cartridges; \"standard\" for all cartridges except for JBossEWS or JBossEAP\n","type":"string","default":"cron,diy,haproxy,mysql,nodejs,perl,php,postgresql,python,ruby"},"db_port":{"type":"number","label":"Port Number","description":"Database port number","default":50000,"constraints":[{"range":{"min":40000,"max":60000},"description":"Port number must be between 40000 and 60000"}]},"image_id":{"type":"string","description":"ID of the image to use for the instance to be created.","default":"F18-x86_64-cfntools","constraints":[{"allowed_values":["F18-i386-cfntools","F18-x86_64-cfntools"],"description":"Image ID must be either F18-i386-cfntools or F18-x86_64-cfntools."}]},"metadata":{"type":"json"}}} \ No newline at end of file +{ + "heat_template_version": "2013-05-23", + "description": "Incomplete sample template for testing parsing of various parameters", + "parameter_groups": [ + { + "label": "General parameters", + "description": "General parameters", + "parameters": [ + "flavor", + "image_id", + "cartridges" + ] + }, + { + "parameters": [ + "admin_pass", + "db_port", + "metadata", + "skip_failed" + ] + } + ], + "parameters": { + "admin_pass": { + "Type": "String", + "Description": "Admin password", + "NoEcho": true, + "MinLength": "1", + "MaxLength": "41", + "AllowedPattern": "[a-zA-Z0-9]+", + "AllowedPattern": "[A-Z]+[a-zA-Z0-9]*" + }, + "flavor": { + "type": "string", + "description": "Flavor for the instances to be created", + "default": "m1.small", + "constraints": [ + { + "custom_constraint": "nova.flavor", + "description": "Must be a flavor known to Nova" + } + ] + }, + "cartridges": { + "Description": "Cartridges to install. \"all\" for all cartridges; \"standard\" for all cartridges except for JBossEWS or JBossEAP\n", + "Type": "CommaDelimitedList", + "Default": "cron, diy, haproxy, mysql, nodejs, perl, php, postgresql, python, ruby" + }, + "db_port": { + "Type": "Number", + "label": "Port Number", + "Description": "Database port number", + "Default": "50000", + "MinValue": "4000", + "MaxValue": "6000", + "ConstraintDescription": "Port number must be between 40000 and 60000" + }, + "image_id": { + "Type": "string", + "Description": "ID of the image to use for the instance to be created.", + "Default": "F18-x86_64-cfntools", + "AllowedValues": ["F18-i386-cfntools", "F18-x86_64-cfntools"], + "ConstraintDescription": "Image ID must be either F18-i386-cfntools or F18-x86_64-cfntools." + }, + "metadata": { + "type": "json", + "default": { + "ver": "test" + } + }, + "skip_failed": { + "type": "boolean", + "default": "t" + }, + "subnets": { + "Description": "Subnet IDs", + "Type": "List", + "Default": ["subnet-123a351e", "subnet-123a351f"] + }, + "my_key_pair": { + "Description": "Amazon EC2 key pair", + "Type": "AWS::EC2::KeyPair::KeyName", + "Default": "my-key" + } + } +} diff --git a/spec/fixtures/orchestration_templates/heat_parameters.yml b/spec/fixtures/orchestration_templates/heat_parameters.yml index c5126def5..c230cb383 100644 --- a/spec/fixtures/orchestration_templates/heat_parameters.yml +++ b/spec/fixtures/orchestration_templates/heat_parameters.yml @@ -13,6 +13,7 @@ parameter_groups: - admin_pass - db_port - metadata + - skip_failed parameters: admin_pass: @@ -20,27 +21,37 @@ parameters: description: Admin password hidden: true constraints: - - length: { min: 6, max: 8 } - description: > - Admin password must be between 6 and 8 characters long. - - allowed_pattern: "[a-zA-Z0-9]+" - description: Password must consist of characters and numbers only - - allowed_pattern: "[A-Z]+[a-zA-Z0-9]*" - description: Password must start with an uppercase character + - length: { min: 6, max: 8 } + description: > + Admin password must be between 6 and 8 characters long. + - allowed_pattern: "[a-zA-Z0-9]+" + description: Password must consist of characters and numbers only + - allowed_pattern: "[A-Z]+[a-zA-Z0-9]*" + description: Password must start with an uppercase character flavor: type: string description: Flavor for the instances to be created default: m1.small constraints: - - custom_constraint: nova.flavor - description: Must be a flavor known to Nova + - custom_constraint: nova.flavor + description: Must be a flavor known to Nova cartridges: description: > Cartridges to install. "all" for all cartridges; "standard" for all cartridges except for JBossEWS or JBossEAP - type: string - default: "cron,diy,haproxy,mysql,nodejs,perl,php,postgresql,python,ruby" + type: comma_delimited_list + default: + - cron + - diy + - haproxy + - mysql + - nodejs + - perl + - php + - postgresql + - python + - ruby db_port: type: number @@ -48,17 +59,33 @@ parameters: description: Database port number default: 50000 constraints: - - range: { min: 40000, max: 60000 } - description: Port number must be between 40000 and 60000 + - range: { min: 40000, max: 60000 } + description: Port number must be between 40000 and 60000 image_id: type: string description: ID of the image to use for the instance to be created. default: F18-x86_64-cfntools constraints: - - allowed_values: [ F18-i386-cfntools, F18-x86_64-cfntools ] - description: - Image ID must be either F18-i386-cfntools or F18-x86_64-cfntools. + - allowed_values: [ F18-i386-cfntools, F18-x86_64-cfntools ] + description: + Image ID must be either F18-i386-cfntools or F18-x86_64-cfntools. metadata: type: json + default: + ver: test + + skip_failed: + type: boolean + default: t + + subnets: + Description: Subnet IDs, + Type: List + Default: [subnet-123a351e, subnet-123a351f] + + my_key_pair: + Description: Amazon EC2 key pair, + Type: AWS::EC2::KeyPair::KeyName, + Default: my-key diff --git a/spec/models/manageiq/providers/openstack/cloud_manager/orchestration_stack_spec.rb b/spec/models/manageiq/providers/openstack/cloud_manager/orchestration_stack_spec.rb index 4e73e79d1..20d049868 100644 --- a/spec/models/manageiq/providers/openstack/cloud_manager/orchestration_stack_spec.rb +++ b/spec/models/manageiq/providers/openstack/cloud_manager/orchestration_stack_spec.rb @@ -107,4 +107,14 @@ end end end + + describe '.transform_parameters' do + let(:template) { FactoryGirl.create(:orchestration_template_openstack_in_yaml) } + it 'converts multiline text into one comma delimitered string' do + parameters = {'cartridges' => "test1\ntest2\n\n"} + + described_class.transform_parameters(template, parameters) + expect(parameters).to eq('cartridges' => "test1,test2") + end + end end diff --git a/spec/models/manageiq/providers/openstack/cloud_manager/orchestration_template_spec.rb b/spec/models/manageiq/providers/openstack/cloud_manager/orchestration_template_spec.rb index 591523d48..aca39e80e 100644 --- a/spec/models/manageiq/providers/openstack/cloud_manager/orchestration_template_spec.rb +++ b/spec/models/manageiq/providers/openstack/cloud_manager/orchestration_template_spec.rb @@ -48,6 +48,7 @@ def assert_db_group(group) assert_hidden_length_patterns(group.parameters[0]) assert_min_max_value(group.parameters[1]) assert_json_type(group.parameters[2]) + assert_boolean_type(group.parameters[3]) end def assert_custom_constraint(parameter) @@ -75,23 +76,26 @@ def assert_list_string_type(parameter) :name => "cartridges", :label => "Cartridges", :description => "Cartridges to install. \"all\" for all cartridges; \"standard\" for all cartridges except for JBossEWS or JBossEAP\n", - :data_type => "string", # HOT has type comma_delimited_list, but all examples use type string. Why? - :default_value => "cron,diy,haproxy,mysql,nodejs,perl,php,postgresql,python,ruby", + :data_type => a_string_matching(/comma_delimited_list|CommaDelimitedList/), + :default_value => %w(cron diy haproxy mysql nodejs perl php postgresql python ruby).join("\n"), :hidden => false, :required => true, - :constraints => [], ) + constraints = parameter.constraints + expect(constraints.size).to eq(1) + expect(constraints[0]).to be_a ::OrchestrationTemplate::OrchestrationParameterMultiline + expect(constraints[0]).to be_kind_of ::OrchestrationTemplate::OrchestrationParameterConstraint end def assert_allowed_values(parameter) expect(parameter).to have_attributes( - :name => "image_id", - :label => "Image", # String#titleize removes trailing id - :description => "ID of the image to use for the instance to be created.", - :data_type => "string", - :default_value => "F18-x86_64-cfntools", - :hidden => false, - :required => true + #:name => "image_id", + #:label => "Image", # String#titleize removes trailing id + #:description => "ID of the image to use for the instance to be created.", + :data_type => a_string_starting_with('s'), + #:default_value => "F18-x86_64-cfntools", + #:hidden => false, + #:required => true ) constraints = parameter.constraints expect(constraints.size).to eq(1) @@ -108,16 +112,17 @@ def assert_min_max_value(parameter) :name => "db_port", :label => "Port Number", # provided by template :description => "Database port number", - :data_type => "number", + :data_type => a_string_matching(/[nN]umber/), :default_value => 50_000, :hidden => false, :required => true ) constraints = parameter.constraints - expect(constraints.size).to eq(1) - expect(constraints[0]).to be_a ::OrchestrationTemplate::OrchestrationParameterRange - expect(constraints[0]).to be_kind_of ::OrchestrationTemplate::OrchestrationParameterConstraint - expect(constraints[0]).to have_attributes( + expect(constraints.size).to eq(2) + expect(constraints[0]).to be_a ::OrchestrationTemplate::OrchestrationParameterPattern + expect(constraints[1]).to be_a ::OrchestrationTemplate::OrchestrationParameterRange + expect(constraints[1]).to be_kind_of ::OrchestrationTemplate::OrchestrationParameterConstraint + expect(constraints[1]).to have_attributes( :description => "Port number must be between 40000 and 60000", :min_value => 40_000, :max_value => 60_000 @@ -129,7 +134,7 @@ def assert_hidden_length_patterns(parameter) :name => "admin_pass", :label => "Admin Pass", :description => "Admin password", - :data_type => "string", + :data_type => a_string_matching(/[sS]tring/), :default_value => nil, :hidden => true, :required => true @@ -161,15 +166,34 @@ def assert_hidden_length_patterns(parameter) def assert_json_type(parameter) expect(parameter).to have_attributes( - :name => "metadata", - :label => "Metadata", + :name => "metadata", + :label => "Metadata", + :description => nil, + :data_type => "json", + :hidden => false, + :required => true, + ) + expect(JSON.parse(parameter.default_value)).to eq('ver' => 'test') + constraints = parameter.constraints + expect(constraints.size).to eq(1) + expect(constraints[0]).to be_a ::OrchestrationTemplate::OrchestrationParameterMultiline + expect(constraints[0]).to be_kind_of ::OrchestrationTemplate::OrchestrationParameterConstraint + end + + def assert_boolean_type(parameter) + expect(parameter).to have_attributes( + :name => "skip_failed", + :label => "Skip Failed", :description => nil, - :data_type => "json", - :default_value => nil, + :data_type => "boolean", + :default_value => true, :hidden => false, :required => true, - :constraints => [], ) + constraints = parameter.constraints + expect(constraints.size).to eq(1) + expect(constraints[0]).to be_a ::OrchestrationTemplate::OrchestrationParameterBoolean + expect(constraints[0]).to be_kind_of ::OrchestrationTemplate::OrchestrationParameterConstraint end describe '#validate_format' do