Bug 1458817 - [RFE] Prioritize attribute order in puppet classes limited to 255 chars
Summary: [RFE] Prioritize attribute order in puppet classes limited to 255 chars
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Satellite 6
Classification: Red Hat
Component: Smart Variables
Version: 6.2.9
Hardware: x86_64
OS: Linux
high
high vote
Target Milestone: Unspecified
Assignee: Tomer Brisker
QA Contact: Katello QA List
URL:
Whiteboard:
Keywords: FutureFeature, Triaged
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2017-06-05 14:26 UTC by Mihir Lele
Modified: 2019-04-01 20:27 UTC (History)
11 users (show)

(edit)
Clone Of:
(edit)
Last Closed: 2018-02-21 12:39:59 UTC


Attachments (Terms of Use)


External Trackers
Tracker ID Priority Status Summary Last Updated
Red Hat Product Errata RHSA-2018:0336 normal SHIPPED_LIVE Important: Satellite 6.3 security, bug fix, and enhancement update 2018-02-21 22:43:42 UTC
Foreman Issue Tracker 19934 None None None 2017-06-06 07:22 UTC

Description Mihir Lele 2017-06-05 14:26:38 UTC
Description of problem:

Prioritize attribute order feature for puppet class override does not allow more than 255 characters.

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


How reproducible: Always


Steps to Reproduce:
1. Go to Sat Webui, "Configure" --> "Classes" --> *Select a class* 
2. Go to "Smart Class Parameter" --> *Select a class and click on override*
3. Enter/paste more than 255 chars in the "Order" field and try to save it

Actual results:

Errors out with "is too long (maximum is 255 characters)"

Expected results:

Should get accepted

Additional info:

Customer is trying for 604 chars

Comment 3 Andrew Schofield 2017-06-05 21:38:30 UTC
This is:
# select key,path  from lookup_keys where key='max_luns';
   key    |               path
----------+-----------------------------------
 max_luns | gsd-all-00-ob_n_dhporacle30\r    +
          | gsd-all-00-ob_p_dhporacle30\r    +
          | gsd-all-00-ob_n_dhporacle\r      +
          | gsd-all-00-ob_p_dhporacle\r      +
          | gsd-all-00-ob_n_dhporacledvsz\r  +
          | gsd-all-00-ob_n_dhporacledvsz30\r+
          | gsd-all-00-ob_p_dhporacledvsz\r  +
          | gsd-all-00-ob_p_dhporacledvsz30
(1 row)

And

# \d lookup_keys;
                                         Table "public.lookup_keys"
       Column       |            Type             |                        Modifiers
--------------------+-----------------------------+----------------------------------------------------------
 id                 | integer                     | not null default nextval('lookup_keys_id_seq'::regclass)
 key                | character varying(255)      |
 created_at         | timestamp without time zone |
 updated_at         | timestamp without time zone |
 puppetclass_id     | integer                     |
 default_value      | text                        |
 path               | character varying(255)      |
 description        | text                        |
 validator_type     | character varying(255)      |
 validator_rule     | character varying(255)      |
 key_type           | character varying(255)      |
 override           | boolean                     | default false
 required           | boolean                     | default false
 merge_overrides    | boolean                     | not null default false
 avoid_duplicates   | boolean                     | not null default false
 use_puppet_default | boolean                     |
 type               | character varying(255)      |
 merge_default      | boolean                     | not null default false
 hidden_value       | boolean                     | default false
Indexes:
    "lookup_keys_pkey" PRIMARY KEY, btree (id)
    "index_lookup_keys_on_key" btree (key)
    "index_lookup_keys_on_path" btree (path)
    "index_lookup_keys_on_puppetclass_id" btree (puppetclass_id)
    "index_lookup_keys_on_type" btree (type)
Foreign-key constraints:
    "lookup_keys_puppetclass_id_fk" FOREIGN KEY (puppetclass_id) REFERENCES puppetclasses(id)
Referenced by:
    TABLE "environment_classes" CONSTRAINT "environment_classes_lookup_key_id_fk" FOREIGN KEY (puppetclass_lookup_key_id) REFERENCES lookup_keys(id)
    TABLE "lookup_values" CONSTRAINT "lookup_values_lookup_key_id_fk" FOREIGN KEY (lookup_key_id) REFERENCES lookup_keys(id)

Comment 4 Tomer Brisker 2017-06-06 07:22:02 UTC
Created redmine issue http://projects.theforeman.org/issues/19934 from this bug

Comment 6 Daniel Lobato Garcia 2017-06-06 10:24:58 UTC
This is really an RFE, the field 'Order' is only meant to contain the types of matchers, not the values of the matchers themselves. 

fqdn
hostgroup
os
domain

Instead I believe you want to input hostgroup labels directly and have Satellite automatically understand you want to match hostgroups params in a certain order.

We can trivially remove the limit but it won't help with the goal you have, as the field isn't meant for that, hence why I'm changing it to RFE (it makes sense though)

Comment 7 pm-sat@redhat.com 2017-06-06 12:11:25 UTC
Upstream bug assigned to tbrisker@redhat.com

Comment 8 pm-sat@redhat.com 2017-06-06 12:11:30 UTC
Upstream bug assigned to tbrisker@redhat.com

Comment 9 Andrew Schofield 2017-06-06 13:50:51 UTC
(In reply to Daniel Lobato Garcia from comment #6)
> This is really an RFE, the field 'Order' is only meant to contain the types
> of matchers, not the values of the matchers themselves. 
> 
> fqdn
> hostgroup
> os
> domain
> 
> Instead I believe you want to input hostgroup labels directly and have
> Satellite automatically understand you want to match hostgroups params in a
> certain order.
> 
> We can trivially remove the limit but it won't help with the goal you have,
> as the field isn't meant for that, hence why I'm changing it to RFE (it
> makes sense though)

Hey Daniel, no that is not the case. Our use case is *simply* that this field is not large enough to contain all the types we need to add.

Our environment is global. As the matchers are simplistic (key = value) we simply have a fact that returns a key for each AD group a server is a member of and set the value to true. 

# dzdo facter -p | grep gsd
gsd-all-00-ob_n_dhporacledvsz => true
gsd-all-00-ob_n_dhporacledvsz30 => true

This has certain advantages over checking if the value is in an array in that we can be deterministic with priority (i.e if a server is in multiple AD groups)

Our matchers then are:

if gsd-all-00-ob_n_dhporacledvsz = true then <value>
if gsd-all-00-ob_n_dhporacledvsz30 = true then <value>

Hope this clarifies. 

In our lab we have updated the length of the path column to 1024 chars. We need a support commitment before we can do the same to production though.

Comment 10 pm-sat@redhat.com 2017-06-07 08:11:29 UTC
Moving this bug to POST for triage into Satellite 6 since the upstream issue http://projects.theforeman.org/issues/19934 has been resolved.

Comment 12 Oleksandr Shtaier 2017-09-26 08:57:23 UTC
Verified on latest 6.3 snap for both smart class parameters and smart variables

Used proposed list in comment1  multiplied by 2 (around 600 characters) and everything saved successfully

Comment 16 errata-xmlrpc 2018-02-21 12:39:59 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://access.redhat.com/errata/RHSA-2018:0336


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