Description of problem: $SUMMARY Version-Release number of selected component (if applicable): Version 5.8.0.10-beta1.20170411212748_e47d319 How reproducible: always Steps to Reproduce: 1. Go to /ops/explorer, Advanced tab. Glorious yaml appears 2. Edit it to :ssl: :ssl_ca_file: "/etc/pki/ca-trust/extracted/openssl/ca-bundle.trust.crt" This is near line 1216. Ctrl+F is your friend. 3. Save, check it changed, it did. 4. Edit the same to :ssl: :ssl_ca_file: null or :ssl: :ssl_ca_file: 5. Save Actual results: :ssl_ca_file: "/etc/pki/ca-trust/extracted/openssl/ca-bundle.trust.crt" Expected results: :ssl_ca_file: null or :ssl_ca_file: Additional info: Changing to value that doesn't eval to somethinig like null works fine.
This is a bug in the underlying config/deep_merge gems where nil values in hashes don't overwrite the input hash's value for that same key: https://github.com/railsconfig/config/issues/13 https://github.com/danielsdeleo/deep_merge/pull/20 We'll try to fix this in those gems but in the interim, you can use '' to get it to save. :ssl: :ssl_ca_file: '' Somewhat related: https://bugzilla.redhat.com/show_bug.cgi?id=1358448
Jaroslav, I believe this was also an issue in 5.7 since we were using the Config/Deep Merge gems then. Can you confirm if this is a new issue to 5.8 or existed previously? Note, the fix isn't trivial in the above gems since it will be a change in behavior for consumers of those gems so it's likely we'll have to provide an "option" to "overwrite with nil values" since they probably won't do this behavior by default. Thanks!
First upstream PR: https://github.com/danielsdeleo/deep_merge/pull/33
Status: 1. deep_merge PR is merged. 2. second deep_merge PR was proposed (not merged): https://github.com/danielsdeleo/deep_merge/pull/34 3. Then, we'll need a release of deep_merge on rubygems.org 4. Then, I'll open a PR on the config gem to utilize the new behavior of deep_merge. 5. Once the config gem PR is merged, we'll need to get a release of config gem 6. I'll open a PR on manageiq to require the new release of the config gem. We're currently at step 2.
(In reply to Joe Rafaniello from comment #3) > Jaroslav, > > I believe this was also an issue in 5.7 since we were using the Config/Deep > Merge gems then. Can you confirm if this is a new issue to 5.8 or existed > previously? It is in 5.7 as well. > > Note, the fix isn't trivial in the above gems since it will be a change in > behavior for consumers of those gems so it's likely we'll have to provide an > "option" to "overwrite with nil values" since they probably won't do this > behavior by default. > > Thanks!
Gah, apologies for the spam, we did checked this against latest upstream with the adv config rest support and this still exists so making it a priority for resolution in 6.0
Status: 1. deep_merge PR is merged: https://github.com/danielsdeleo/deep_merge/pull/33 2. second deep_merge PR was merged: https://github.com/danielsdeleo/deep_merge/pull/34 3. Then, we'll need a release of deep_merge on rubygems.org (released in 1.2.0) 4. Then, I'll open a PR on the config gem to utilize the new behavior of deep_merge: https://github.com/railsconfig/config/pull/196 (not merged yet) 5. Once the config gem PR is merged, we'll need to get a release of config gem 6. I'll open a PR on manageiq to require the new release of the config gem. We're currently at step 4.
https://github.com/railsconfig/config/pull/196 has been merged and should be released as 1.7.0 soon. We are now at step 5 in the prior comment.
https://github.com/ManageIQ/manageiq/pull/16982
the same issue: https://bugzilla.redhat.com/show_bug.cgi?id=1553857
*** Bug 1553857 has been marked as a duplicate of this bug. ***
*** Bug 1558878 has been marked as a duplicate of this bug. ***
I just want to update this bug since it's a known problem and we thought we had a solution. We've made changes to the deep_merge and config gems to allow us to merge nils back into the settings hash...this allows us to reset an originally nil value back to nil. In other words: nil => non-nil value => nil. The problem with this change is any non-nil values will override settings from other settings.yml in addition to the nil that we want. This change in behavior is not expected and in my opinion is much worse than this original issue. In other words: Provider A settings.yml: :ems :foo :bar Provider B settings.yml: :ems :foo :bar :baz: true If Provider A settings.yml is resolved after the B provider, the entire bar section will be overwritten with nil, removing the settings from provider B. This would require us to strip all nil value/sections out of existing repo settings.yml files, somehow prevent adding new nil value/sections that could be populated in other repos' settings.yml, etc. The config gem has the concept of validating configuration files found here: https://github.com/railsconfig/config/tree/2439d5a41061cf303aa912ce7f03b8b7e69ada4d#validation This validation is great but it doesn't tell the full deeply nested path to the nil value, nor where it's coming from. In the example above, it would say, ":bar is nil". It does NOT tell you :ems, :foo, :bar from B provider settings.yml is nil. In addition, we'd have to enumerate all deeply nested keys that could be shared in different settings.yml files and specify they can't be nil. There wasn't a blanket: "warn on nil values" validation, so we'd hit this whenever a new shared section is added to different settings.yml files. This change in behavior would be much worse than this original problem. At this point, it seems like the best recourse might to expose the settings_changes table and allow row to be deleted.
*** Bug 1559025 has been marked as a duplicate of this bug. ***
Until we get a fully-validated configuration file, or perhaps tests to verify that a provider doesn't introduce a setting that could blow away some part of the configuration, then I agree with Joe that we should wait on fixing this issue in the manner we thought we could.
*** This bug has been marked as a duplicate of bug 1576984 ***