Bug 1358433 - [Regression] Advanced Config allows adding of settings, but not deleting them
Summary: [Regression] Advanced Config allows adding of settings, but not deleting them
Keywords:
Status: CLOSED DUPLICATE of bug 1576984
Alias: None
Product: Red Hat CloudForms Management Engine
Classification: Red Hat
Component: Appliance
Version: 5.7.0
Hardware: Unspecified
OS: Unspecified
high
high
Target Milestone: GA
: 5.7.0
Assignee: Gregg Tanzillo
QA Contact: Dave Johnson
URL:
Whiteboard:
: 1374018 1498661 1501861 1507285 (view as bug list)
Depends On:
Blocks: 1358448
TreeView+ depends on / blocked
 
Reported: 2016-07-20 16:30 UTC by Dan Clarizio
Modified: 2021-06-10 11:24 UTC (History)
17 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
: 1358448 (view as bug list)
Environment:
Last Closed: 2018-08-06 19:31:59 UTC
Category: ---
Cloudforms Team: ---
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)

Description Dan Clarizio 2016-07-20 16:30:10 UTC
Description of problem:
In prior releases, users could a keys, such as ":product: :containers: true", which can still be done.  However, unlike prior releases, removing the line with the new key no longer removes they key.

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

How reproducible:
Add a key in advanced config, save, delete the key, save . . . key remains

Actual results:
Delete keys are not removed

Expected results:
Should either allow keys to be deleted or NOT allow keys to be added

Additional info:
See discussion here https://github.com/ManageIQ/manageiq/pull/9808#issuecomment-233676159

Comment 3 Jason Frey 2016-07-20 16:45:14 UTC
Discussed this offline with Dan and we came to a number of points.

For one, we probably should not allow users to delete or add keys *at all* to the advanced settings.  If they delete the `:server, :role`, for example, it will kill the system.  Granted we have a big red warning banner on top, but that's not going to stop a user.  So, what needs to happen is a validation of the keys present and that they all exists.  This is relatively easy to add.

Unfortunately, we have this concept of "hidden settings", which include development only settings like `:report_sync` as well as prototype setting flags for experimental or future features.  This complicates the matter because the settings don't exist in the base yml file, so a validator would have no way of knowing it should or should not be allowed.  Additionally, the config code needs to know remove them, which is presently doesn't do at all, and is complicated to write.

My proposal is to just get rid of hidden settings and just have them as part of the base settings.yml.

For development settings, if we really cared, we *might* be able to keep them in the settings/development.yml which would restrict them to development only environments.  I'm not sure this really matters though.

For protoype settings, those are used development but also for POC purposes, which runs in production mode.  So, for those, we can't do any restrictions.  We could possibly *hide* them via the calls the UI makes so they are not visible in Advanced Settings, but then that just makes turning them on harder for the POC person (though they could SSH into the box and set it in a settings.local.yml file).

John, what are your requirements on that last bit?  Do they really have to stay hidden?  I doubt a production customer would turn them on for "fun", especially if they are nested under a top-level `:prototype` or `:experimental` key, so I'm not sure what we are protecting them from.

Comment 6 Jason Frey 2017-10-05 15:10:08 UTC
*** Bug 1498661 has been marked as a duplicate of this bug. ***

Comment 7 Jason Frey 2017-11-16 19:43:41 UTC
*** Bug 1507285 has been marked as a duplicate of this bug. ***

Comment 8 Jason Frey 2017-11-16 19:44:33 UTC
*** Bug 1501861 has been marked as a duplicate of this bug. ***

Comment 9 Jason Frey 2017-11-16 19:56:18 UTC
For those coming to this issue from one of the duplicates, there is a workaround for some of the settings.  So, one of the common examples people hit is something like http proxy settings where the template holds a nil value, the database holds non-nil value, and they want to set it back to nil.  However, because a nil value is being set in the UI it is presently *ignored*, so it appears they can't unset the value.  The workaround though is to set it to an empty string.  This will force the empty string to be written to the database. 
 For example:

:http_proxy:
  :ec2:
    :host: ''

One downside to this is that one would have to verify that setting the key to empty string still works.  Most settings this should work for, but we'd have to double check each key before asking a user to do that.

========

The steps we need to take to get this over the finish line are:

- Support saving nils directly instead of ignoring them.
- Eliminate hidden settings
- Enforce keys in the Advanced Settings so they cannot be added or removed.
- Support a "removal" of a key from advanced settings as meaning "go back to the inherited value"

Comment 10 Jason Frey 2017-11-20 19:59:39 UTC
*** Bug 1374018 has been marked as a duplicate of this bug. ***

Comment 13 Yuri Rudman 2018-08-06 19:31:59 UTC
There is duplicate [RFE] Advanced settings - ability to reset to default value, delete newly added keys (https://bugzilla.redhat.com/show_bug.cgi?id=1576984) verified in  5.10.0.6

*** This bug has been marked as a duplicate of bug 1576984 ***


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