Bug 1171738 - No error message is displayed for invalid yaml file on widget import
Summary: No error message is displayed for invalid yaml file on widget import
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat CloudForms Management Engine
Classification: Red Hat
Component: UI - OPS
Version: 5.3.0
Hardware: Unspecified
OS: Unspecified
low
low
Target Milestone: GA
: 5.4.0
Assignee: eclarizi
QA Contact: Milan Falešník
URL:
Whiteboard:
Depends On:
Blocks: 1201922
TreeView+ depends on / blocked
 
Reported: 2014-12-08 14:00 UTC by Jan Krocil
Modified: 2015-06-16 12:46 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
: 1201922 (view as bug list)
Environment:
Last Closed: 2015-06-16 12:46:03 UTC
Category: ---
Cloudforms Team: ---
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2015:1100 0 normal SHIPPED_LIVE CFME 5.4.0 bug fixes, and enhancement update 2015-06-16 16:28:42 UTC

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


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