From bce829b07f28720d7959eca907ceba38216af7d0 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 | 15 +++ .../cloud_manager/orchestration_template.rb | 39 +++++- .../heat_parameters.json | 114 +++++++++++++++++- .../heat_parameters.yml | 49 +++++--- .../cloud_manager/orchestration_stack_spec.rb | 10 ++ .../orchestration_template_spec.rb | 48 ++++++-- 6 files changed, 243 insertions(+), 32 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..3f7bbf7ef 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,17 @@ 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) + # 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 == 'comma_delimited_list' + 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..58402fbcc 100644 --- a/app/models/manageiq/providers/openstack/cloud_manager/orchestration_template.rb +++ b/app/models/manageiq/providers/openstack/cloud_manager/orchestration_template.rb @@ -29,10 +29,10 @@ def parameters(content_hash = nil) :name => key, :label => val.key?('label') ? val['label'] : key.titleize, :data_type => val['type'], - :default_value => val['default'], + :default_value => parse_default_value(val), :description => val['description'], :hidden => val['hidden'] == true, - :constraints => val.key?('constraints') ? parse_constraints(val['constraints']) : nil, + :constraints => ([constraint_from_type(val['type'])] + parse_constraints(val['constraints'])).compact, :required => true ) end @@ -97,8 +97,41 @@ def validate_format_json err.message end + def parse_default_value(parameter) + case parameter['type'] + when 'json' + JSON.pretty_generate(parameter['default'] || {'sample(please delete)' => 'JSON format'}) + when 'comma_delimited_list' + (parameter['default'] || ['sample(please delete)'] * 2).join("\n") + when 'boolean' + ([true, 1] + %w(t T y Y yes Yes YES true True TRUE 1)).include?(parameter['default']) + else + parameter['default'] + end + end + + def constraint_from_type(parameter_type) + case parameter_type + when 'json' + OrchestrationTemplate::OrchestrationParameterMultiline.new( + :description => 'Parameter in JSON format' + ) + when 'comma_delimited_list' + OrchestrationTemplate::OrchestrationParameterMultiline.new( + :description => 'Parameter in list format' + ) + when 'boolean' + OrchestrationTemplate::OrchestrationParameterBoolean.new + when 'number' + OrchestrationTemplate::OrchestrationParameterPattern.new( + :pattern => '^[+-]?([1-9]\d*|0)(\.\d+)?$', + :description => 'Numeric parameter' + ) + end + end + def parse_constraints(raw_constraints) - raw_constraints.collect do |raw_constraint| + (raw_constraints || []).collect do |raw_constraint| if raw_constraint.key?('allowed_values') parse_allowed_values(raw_constraint) elsif raw_constraint.key?('allowed_pattern') diff --git a/spec/fixtures/orchestration_templates/heat_parameters.json b/spec/fixtures/orchestration_templates/heat_parameters.json index 2efa8c9b5..8f15c75e7 100644 --- a/spec/fixtures/orchestration_templates/heat_parameters.json +++ b/spec/fixtures/orchestration_templates/heat_parameters.json @@ -1 +1,113 @@ -{"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", + "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": "comma_delimited_list", + "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", + "default": { + "ver": "test" + } + }, + "skip_failed": { + "type": "boolean", + "default": "t" + } + } +} \ No newline at end of file diff --git a/spec/fixtures/orchestration_templates/heat_parameters.yml b/spec/fixtures/orchestration_templates/heat_parameters.yml index c5126def5..375087e75 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,23 @@ 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 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..384229cc9 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,12 +76,15 @@ 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 => "comma_delimited_list", + :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) @@ -114,10 +118,11 @@ def assert_min_max_value(parameter) :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 @@ -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