Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Patching the appliance overwrites updated configuration files #105

Open
miq-bot opened this issue Oct 12, 2020 · 6 comments
Open

Patching the appliance overwrites updated configuration files #105

miq-bot opened this issue Oct 12, 2020 · 6 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@miq-bot
Copy link
Member

miq-bot commented Oct 12, 2020

Original BZ: 1886422

Description of problem:
Properly configured configuration files should be created as .rpmnew files when the live file has been altered. Specifically, /etc/httpd/manageiq-https-appliance.conf gets replaced every time CFME is updated.

Version-Release number of selected component (if applicable):
5.11.x

How reproducible:
Always

Steps to Reproduce:

  1. modify /etc/httpd/manageiq-https-appliance.conf
  2. update appliance through yum/dnf update

Actual results:
File is overwritten

Expected results:
modified file should remain intact


This issue was moved to this repository from ManageIQ/manageiq#20673, originally opened by @dmetzger57

@simaishi
Copy link
Contributor

simaishi commented Dec 11, 2020

@Fryguy @bdunne What would be the appropriate setting to use for this? We can set either:

%config - The updated file will be used, the edited file will be renamed to .rpmsave
%config(noreplace) - The edited file will be used, the file from update wil be installed as .rpmnew

The latter will keep the user's edits in place, so users won't lose any custom settings by default. But, if we have some critical updates, that will be missed.

Either way, users will need to compare the config files and make edits as needed. But I'm not sure which should be the default behavior for httpd config files.

@Fryguy
Copy link
Member

Fryguy commented Dec 11, 2020

I thought we went with an approach where our config is in a dedicated file, but the user can create their own separate file for overrides?

@simaishi
Copy link
Contributor

I'm not sure if <VirtualHost> directive can be overridden from another file. I tried to set a different SSLProtocol (which is specified in manageiq-https-application.conf) and that doesn't seem to work. Only if I comment out the line in manageiq-https-appliance.conf and I add the new SSLProtocol setting globally (without VirtualHost directive), it worked.

Either I'm not setting things correctly, or you can't override it...

Regardless, I think it's wrong to delete the edited file on update, and I think it makes sense to use %config, so I'll change to that.

@Fryguy
Copy link
Member

Fryguy commented Jan 4, 2021

@bdunne Thoughts here?

@bdunne
Copy link
Member

bdunne commented Jan 4, 2021

I'd be interested to know what people are changing in that file. Maybe https://github.com/ManageIQ/manageiq-appliance/blob/945e128e7ac95a1767549edb24705866237dfd5b/COPY/etc/httpd/conf.d/manageiq-https-application.conf#L22 ? I know different organizations allow different SSL protocols, maybe we can be more restrictive and change our default to eliminate this issue.

If not, I think %config(noreplace) is probably better. We don't change that file often.

@simaishi
Copy link
Contributor

simaishi commented Jan 6, 2021

I'd be interested to know what people are changing in that file. Maybe https://github.com/ManageIQ/manageiq-appliance/blob/945e128e7ac95a1767549edb24705866237dfd5b/COPY/etc/httpd/conf.d/manageiq-https-application.conf#L22 ? I know different organizations allow different SSL protocols, maybe we can be more restrictive and change our default to eliminate this issue.

I think SSL protocol was the reason for editing the file. Maybe we should do both (make the default conf to be more restrictive as well as setting the file attr properly in rpm)?

If not, I think %config(noreplace) is probably better. We don't change that file often.

That was the reason I thought %config will be better... Since we don't change the file often, when we do change the file, that's most likely the change we want people to use. But I can go either way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

5 participants