Bug 1662319 - Warn when custom attributes contain spaces in their names - they will not work properly in all of reporting
Summary: Warn when custom attributes contain spaces in their names - they will not wor...
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Red Hat CloudForms Management Engine
Classification: Red Hat
Component: Reporting
Version: unspecified
Hardware: All
OS: All
high
medium
Target Milestone: GA
: 5.11.0
Assignee: Joe Rafaniello
QA Contact: Parthvi Vala
Red Hat CloudForms Documentation
URL:
Whiteboard:
Depends On:
Blocks: 1693722
TreeView+ depends on / blocked
 
Reported: 2018-12-27 16:35 UTC by Felix Dewaleyne
Modified: 2019-12-13 15:16 UTC (History)
12 users (show)

Fixed In Version: 5.11.0.1
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
: 1693722 (view as bug list)
Environment:
Last Closed: 2019-12-13 15:16:53 UTC
Category: ---
Cloudforms Team: CFME Core
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)
sample.rb (395 bytes, text/plain)
2018-12-27 16:35 UTC, Felix Dewaleyne
no flags Details

Description Felix Dewaleyne 2018-12-27 16:35:09 UTC
Created attachment 1517091 [details]
sample.rb

Description of problem:
custom fields using spaces in their names don't have their data displayed in reports

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

How reproducible:
all the time

Steps to Reproduce:
1. assign fields with spaces in their names to vms
2. create a report displaying the value of the custom fields
3. run the report & display it

Actual results:
there is no data for the custom fields that have spaces in their names

Expected results:
the values are displayed

Additional info:
sample automation used to apply fields to vms attached
based on https://access.redhat.com/webassets/avalon/d/Mastering_CloudForms_Automation_Red_Hat/Mastering_CloudForms_Automation_Red_Hat.pdf

Comment 6 Joe Rafaniello 2019-01-28 21:21:53 UTC
Loic,

If custom fields are the same as custom attributes and have been populated through automate with spaces in the names, I believe the end results will be the same as bug #1553750.  The relevant comments around here: https://bugzilla.redhat.com/show_bug.cgi?id=1553750#c10

This is a substantial change for an RFE or a bug for reasons mentioned in the other bug.  Is it possible to use valid column names for the "name" column and the description column to populate the friendly name of the attribute?  These field names can be used in the reporting engine, the UI, api, expression editor and backend parser, policy engine, automate, etc. and all of these areas have expected column names in the past and would need to be changed to support spaces or other invalid column name characters.

Comment 7 Joe Rafaniello 2019-01-28 21:26:51 UTC
Felix,

Do you know if they can use description instead of the name field for the "friendly" name they want to use on reports?  In other words, treat the name field as a column and use column friendly naming and use the description for whatever friendly name they want to use for the custom attribute?

Comment 10 Joe Rafaniello 2019-02-19 19:01:32 UTC
The reason we can't validate custom attributes don't have invalid characters is because external systems could be defining the custom attributes.  In openshift, native tags are stored in custom attributes and the names can have either spaces or special characters.  Other systems could do similar things.  If users want to leverage custom attributes in cloudforms, they'll need to define the attribute name as they would a column name, with limited characters and word length.

It's less than ideal but the code around custom attributes has always treated the names as column names and can't easily be changed due to the numerous places custom attributes are accessible.

Comment 12 Joe Rafaniello 2019-02-26 17:41:46 UTC
I'm not sure what we can do about this.  No other reportable "column" allows non-column characters in it, such as spaces.  I can deprecate miq_custom_set [1] or add_custom_attribute [2] or log a general warning when someone tries to use invalid characters in a custom attribute column name:
[1] https://github.com/ManageIQ/manageiq/blob/3670332c354c52fe92da20a3720e7fd29f7813d6/app/models/mixins/custom_attribute_mixin.rb#L104
[2] https://github.com/ManageIQ/manageiq/blob/3670332c354c52fe92da20a3720e7fd29f7813d6/app/models/mixins/custom_attribute_mixin.rb#L41

Note, this is still a duplicate of #1553750.  We tried adding validation in bug #1558618 but didn't because openshift knowingly creates custom attributes based on labels/tags and those are created outside of us and can have spaces.  Those are known to not work in all of reporting or the various places that normal columns can be used such as the expression editor.

We can create a technote so automate authors know that "columns" you want in reporting should follow column naming conventions.

Perhaps Adam or Greg can comment around openshift or automate.

Comment 13 Adam Grare 2019-02-28 15:15:33 UTC
We could reject labels with invalid format when mapping to tags during refresh.

+Ladas

Comment 14 Ladislav Smola 2019-02-28 15:36:09 UTC
Rather than reject, we can just encode it changing \s to e.g. underscore

Comment 16 Joe Rafaniello 2019-03-07 22:24:14 UTC
Felix,

The steps we have can take:

1) We deprecate and warn when any provider refresh or automate code tries to set a custom attribute with an invalid name, this would get logged
2) Based on the deprecation warning, provider refresh could convert invalid characters to characters in the valid "column name" set of characters, perhaps converting all invalid characters to _
3) Any automate code provided by us or written by users would have to be updated to no longer log the deprecation warning.

For the purpose of this bug, I think we can only do 1)

Does that make sense?

Comment 19 CFME Bot 2019-03-19 10:51:09 UTC
New commit detected on ManageIQ/manageiq/master:

https://github.com/ManageIQ/manageiq/commit/eaad9fb1536c4123351477cf30bde41a7f98d881
commit eaad9fb1536c4123351477cf30bde41a7f98d881
Author:     Joe Rafaniello <jrafanie>
AuthorDate: Fri Mar  8 18:04:03 2019 -0500
Commit:     Joe Rafaniello <jrafanie>
CommitDate: Fri Mar  8 18:04:03 2019 -0500

    Deprecate invalid custom attribute names

    add_custom_attribute/miq_custom_set will now log a deprecation if we try
    to define or add a custom attribute name that is an invalid column name.

    https://bugzilla.redhat.com/show_bug.cgi?id=1662319

    From:
    https://www.postgresql.org/docs/current/static/sql-syntax-lexical.html#SQL-SYNTAX-IDENTIFIERS

    SQL identifiers and key words must begin with a letter (a-z, but also
    letters with diacritical marks and non-Latin letters) or an underscore
    (_). Subsequent characters in an identifier or key word can be letters,
    underscores, digits (0-9), or dollar signs ($). Note that dollar signs
    are not allowed in identifiers according to the letter of the SQL
    standard, so their use might render applications less portable. The SQL
    standard will not define a key word that contains digits or starts or
    ends with an underscore, so identifiers of this form are safe against
    possible conflict with future extensions of the standard.

    Similar to https://bugzilla.redhat.com/show_bug.cgi?id=1558618

 app/models/mixins/custom_attribute_mixin.rb | 9 +
 spec/models/mixins/custom_attribute_mixin_spec.rb | 34 +
 2 files changed, 43 insertions(+)

Comment 21 Parthvi Vala 2019-05-02 12:57:09 UTC
FIXED. Verified on 5.11.0.2.20190430174828_0e34dea.


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