Bug 1171738

Summary: No error message is displayed for invalid yaml file on widget import
Product: Red Hat CloudForms Management Engine Reporter: Jan Krocil <jkrocil>
Component: UI - OPSAssignee: eclarizi
Status: CLOSED ERRATA QA Contact: Milan Falešník <mfalesni>
Severity: low Docs Contact:
Priority: low    
Version: 5.3.0CC: eclarizi, jprause, mfalesni, obarenbo
Target Milestone: GA   
Target Release: 5.4.0   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
: 1201922 (view as bug list) Environment:
Last Closed: 2015-06-16 12:46:03 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 1201922    

Description Jan Krocil 2014-12-08 14:00:13 UTC
Description of problem:
There is no error message shown when the files being uploaded in widget import section are invalid.

Version-Release number of selected component (if applicable):
5.3.1.10

How reproducible:
Always

Steps to Reproduce:
1. Go to Cloud Intelligence > Reports > Import/Export > Widgets
2. Create a file with random (nonsensical) content
3. Choose that file for upload and click "Upload"

Actual results:
A flash message indicating success "Import file was uploaded successfully".

Expected results:
An error flash message saying that the file's type and content is invalid etc...

Additional info:
This probably isn't that big an issue considering that customers hopefully aren't going to try to import random stuff as widgets, but it might become an issue if some of the current widget yaml fields become obsoleted by later versions of CFME (if something like that can happen) and the user will not know what's going on when trying to import such obsolete file of correct type and (to the customer) seemingly correct content.

A correct, descriptive error message in such case could save us a support ticket.

Comment 2 CFME Bot 2015-03-04 20:00:51 UTC
New commit detected on manageiq/master:
https://github.com/ManageIQ/manageiq/commit/de3f9073adef5d09e826304097f5892d3cead6ab

commit de3f9073adef5d09e826304097f5892d3cead6ab
Author:     Erik Clarizio <eclarizi>
AuthorDate: Mon Feb 9 17:01:58 2015 -0800
Commit:     Erik Clarizio <eclarizi>
CommitDate: Wed Mar 4 09:18:44 2015 -0800

    Add flash message when trying to import yaml that is not all widgets
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1171738

 vmdb/app/controllers/report_controller.rb        |  2 ++
 vmdb/app/models/widget_import_validator.rb       |  5 ++++-
 vmdb/spec/controllers/report_controller_spec.rb  | 17 ++++++++++++++
 vmdb/spec/models/widget_import_validator_spec.rb | 28 +++++++++++++++++++++---
 4 files changed, 48 insertions(+), 4 deletions(-)

Comment 3 CFME Bot 2015-03-04 20:00:54 UTC
New commit detected on manageiq/master:
https://github.com/ManageIQ/manageiq/commit/a8f403db1b0e3aa6494209acfd45d785794d2654

commit a8f403db1b0e3aa6494209acfd45d785794d2654
Author:     Erik Clarizio <eclarizi>
AuthorDate: Wed Mar 4 10:50:04 2015 -0800
Commit:     Erik Clarizio <eclarizi>
CommitDate: Wed Mar 4 10:50:04 2015 -0800

    Add logic for handling if the yml file is a hash instead of array
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1171738

 vmdb/app/models/widget_import_validator.rb       |  4 +++-
 vmdb/spec/models/widget_import_validator_spec.rb | 10 +++++++++-
 2 files changed, 12 insertions(+), 2 deletions(-)

Comment 4 CFME Bot 2015-03-09 19:16:52 UTC
New commit detected on cfme/5.3.z:
https://code.engineering.redhat.com/gerrit/gitweb?p=cfme.git;a=commitdiff;h=b004b9383c5cca51c420ac1c73fdd669cf7a1bd4

commit b004b9383c5cca51c420ac1c73fdd669cf7a1bd4
Author:     Erik Clarizio <eclarizi>
AuthorDate: Mon Feb 9 17:01:58 2015 -0800
Commit:     Erik Clarizio <eclarizi>
CommitDate: Mon Mar 9 12:14:05 2015 -0700

    Add flash message when trying to import yaml that is not all widgets
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1171738
    
    Conflicts:
    	vmdb/app/controllers/report_controller.rb
    	vmdb/app/models/widget_import_validator.rb
    	vmdb/spec/models/widget_import_validator_spec.rb

 vmdb/app/controllers/report_controller.rb        |  8 ++--
 vmdb/app/models/widget_import_validator.rb       | 12 ++++++
 vmdb/config/locales/en.yml                       |  3 ++
 vmdb/spec/controllers/report_controller_spec.rb  | 19 ++++++++-
 vmdb/spec/models/widget_import_validator_spec.rb | 51 ++++++++++++++++++++++++
 5 files changed, 89 insertions(+), 4 deletions(-)
 create mode 100644 vmdb/app/models/widget_import_validator.rb
 create mode 100644 vmdb/spec/models/widget_import_validator_spec.rb

Comment 5 CFME Bot 2015-03-09 19:16:55 UTC
New commit detected on cfme/5.3.z:
https://code.engineering.redhat.com/gerrit/gitweb?p=cfme.git;a=commitdiff;h=9426f4b6c78c22fd6bb977f55e23210be7d23335

commit 9426f4b6c78c22fd6bb977f55e23210be7d23335
Author:     Erik Clarizio <eclarizi>
AuthorDate: Wed Mar 4 10:50:04 2015 -0800
Commit:     Erik Clarizio <eclarizi>
CommitDate: Mon Mar 9 12:14:37 2015 -0700

    Add logic for handling if the yml file is a hash instead of array
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1171738

 vmdb/app/models/widget_import_validator.rb       |  4 +++-
 vmdb/spec/models/widget_import_validator_spec.rb | 10 +++++++++-
 2 files changed, 12 insertions(+), 2 deletions(-)

Comment 7 Milan Falešník 2015-04-22 15:53:25 UTC
Just checked on 5.4.0.0.22

When I have a YAML that has no widgets, it works correctly and says there are no widgets in the file. But when I throw nonsense to it (like a kickstart starting with <% for example), the spinner gets stuck and the background request raises a 500 error and returns a HTML with CFME page containing an error:

undefined method `all?' for '<%':String [report/upload_widget_import_file]


Does this block verification or we assume customer won't throw such things to it? I looked in the source code of the PR and [1] shows it should catch such things.

[1] https://github.com/ManageIQ/manageiq/commit/de3f9073adef5d09e826304097f5892d3cead6ab#diff-d2cf6d096cd49cc7524aca1aa9e29842R289

Comment 8 eclarizi 2015-05-07 16:29:25 UTC
Well, technically a simple string such as "<%" is valid YAML, and won't throw a Psych::SyntaxError when YAML.load is called.

I think we're assuming that the customer isn't even generating the YAML by themselves, since this is supposed to be an export of the widgets from perhaps a different application that they want to import into their current application.

However, the fix is pretty simple here, we'll just catch any other errors that might happen and show that the YAML was not proper widget yaml.

https://github.com/ManageIQ/manageiq/pull/2871

Comment 9 CFME Bot 2015-05-07 19:21:02 UTC
New commit detected on manageiq/master:
https://github.com/ManageIQ/manageiq/commit/c36a75d284356475279150aeecd28b50629ed5fc

commit c36a75d284356475279150aeecd28b50629ed5fc
Author:     Erik Clarizio <eclarizi>
AuthorDate: Thu May 7 09:27:18 2015 -0700
Commit:     Erik Clarizio <eclarizi>
CommitDate: Thu May 7 09:28:05 2015 -0700

    Handle when YAML is valid but does not parse into an enumerable
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1171738

 vmdb/app/models/widget_import_validator.rb       |  2 ++
 vmdb/spec/models/widget_import_validator_spec.rb | 10 ++++++++++
 2 files changed, 12 insertions(+)

Comment 10 Milan Falešník 2015-05-19 13:45:08 UTC
Verified in 5.4.0.1

Comment 12 errata-xmlrpc 2015-06-16 12:46:03 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://rhn.redhat.com/errata/RHBA-2015-1100.html