Bug 1277734 - [Docs] [Automation] ws_values should be serialized more safely
[Docs] [Automation] ws_values should be serialized more safely
Status: CLOSED CURRENTRELEASE
Product: Red Hat CloudForms Management Engine
Classification: Red Hat
Component: Documentation (Show other bugs)
5.6.0
Unspecified Unspecified
medium Severity medium
: GA
: 5.6.0
Assigned To: Shikha
Suyog Sainkar
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2015-11-03 18:02 EST by Jeff Warnica
Modified: 2016-04-27 20:32 EDT (History)
9 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2015-12-08 05:04:28 EST
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Jeff Warnica 2015-11-03 18:02:55 EST
Description of problem:

Client has VLAN names with pipe ("|") characters in them, and this was used in a drop down; round trip of this string through service provisioning mangled the string, as ws_values is run through join/split("|").

I have no clue if/what there is a perfect un/serialize method in Ruby, but at the very least, choosing a unlikely character to be typed by a geek is a good idea. There is enough UTF that anything not in ASCII could be safe (or, ASCII 30, "Record Separator").
Comment 2 Greg McCullough 2015-11-12 09:55:05 EST
Drew - Check out the parsing in miq_request_workflow.rb that starts with the ws_values method.  Thinking we may need to support escaping the current separator so it can be passed as part of the data.
Comment 4 Greg McCullough 2015-11-18 13:44:05 EST
Jeff - As mentioned in PR #5491 linked above the provisioning API will properly handle passing a hash, so you no longer need to create a string with "|" separated entries.  Instead build a hash with the keys/values.  This will allow you to use the "|" symbol where needed.  This change should already be in place with the code you are running.  (But I'm not exactly sure what version that is from the details supplied with this bug.)

I would like to start promoting this as the preferred way to call the create_provision_request method and it matches what callers are using with the REST API already.

For example, if you are currently building the arguments like this:
args = ['1.1']
args << "name=#{comment_hash[:template_name]}|request_type=template"
args << "vm_memory=#{comment_hash[:vm_memory]}|number_of_cores=#{comment_hash[:number_of_cores]}"
args << "user_name=#{userid}|owner_email=#{user_mail}"
args << 'lifecycle=retire_full'
args << comment_hash.collect{|k,v| "#{k}=#{v}"}.join('|')
args << nil
args << nil
$evm.execute('create_provision_request', *args)


You can now specify them like this:
args = ['1.1']
args << {"name" => comment_hash[:template_name], "request_type" => "template"}
args << {"vm_memory"=> comment_hash[:vm_memory], "number_of_cores"=> comment_hash[:number_of_cores]}
args << {"user_name" => userid, "owner_email" => user_mail}
args << {'lifecycle' => 'retire_full'}
args << comment_hash
args << nil
args << nil
$evm.execute('create_provision_request', *args)
Comment 5 Jeff Warnica 2015-11-18 14:03:51 EST
Configure/About tells me: 5.4.3.1.20151013111241_1c155a5

This problem was discovered while running automation from the UI - no API at all.  

Our workaround was: https://github.com/rhtconsulting/miq-BASE/commit/64bed665b4d6be596b6382d698e43c1bebbb094c#diff-9e1010c0334ea024c34e008afcf6908b
Comment 6 Greg McCullough 2015-11-18 14:44:36 EST
I fully understand you are not calling the REST API, but you are using services to pass information to automate.  Then using custom automate scripts to parse and call the create_provision_request method which ends up calling through the same entry-point the REST API uses.  They use the same parsing routines which is why I made the connection.

The automate method you point to is not something we ship so you will need to work with the maintainer of that script to update it to use the newer calling format if you want to resolve the issue of passing the "|" symbol as a value without further modifications.
Comment 7 Greg McCullough 2015-11-20 21:16:37 EST
There are no code changes required for this bug but I would like to see the documentation updated to use the hash format when calling create_provision_request from automate.

One example I found is here which I have updated below to use the hash format for parameters over the string format.

https://access.redhat.com/documentation/en-US/Red_Hat_CloudForms/3.2/html/Lifecycle_and_Automation_Guide/appe-Inline_Method_to_Create_a_Provision_Request.html#Ruby_method

# arg1 = version
args = ['1.1']
# arg2 = templateFields
args << {'name' => 'App'}
# arg3 = vmFields
args << {'vm_name' => 'CRM_APP', 'request_type' => 'template'}
# arg4 =  requester
args << {'owner_email' => 'admin@asd.com', 'owner_last_name' => 'Admin', 'owner_first_name' => 'Admin', 'user_name' => 'admin'}
# arg5 =  tags
args << {'crm' => 'true'}
# arg6 =  WS Values
args << nil
# arg7 = emsCustomAttributes
args << nil
# arg8 = miqCustomAttributes
args << nil

$evm.log("info","Building provisioning request with the following arguments: <#{args.inspect}>")
result = $evm.execute('create_provision_request', *args)
Comment 8 Andrew Dahms 2015-11-23 20:37:31 EST
Changing status back to 'NEW' for triaging in the documentation queue.
Comment 9 Andrew Dahms 2015-12-03 00:26:52 EST
Assigning to Shikha for review.

Shikha - looks like we need to update any automation methods for create_provision_request so that arguments use the following format -

args << {'name' => 'App'}

rather than -

args << "name=App"

The example in the appendix there also looks like it has some issues where spaces are being shown as 'Â' - could you replace this with spaces?
Comment 11 Andrew Dahms 2015-12-06 19:35:42 EST
Assigning Suyog as the QA contact.

Suyog - could you take a look at the changes for this bug?
Comment 13 Andrew Dahms 2015-12-08 05:04:28 EST
This content is now live on the Customer Portal.

Closing.

Note You need to log in before you can comment on or make changes to this bug.