Skip to content
New issue

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

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

Already on GitHub? # to your account

Drop apache user group config #269

Merged
merged 1 commit into from
Nov 28, 2018
Merged

Conversation

ehelms
Copy link
Member

@ehelms ehelms commented Nov 28, 2018

Note that Puppet doesn't remove the apache user from Foreman and it can lead to dependency cycles. We don't control the Apache user (puppetlabs-apache does) so we can't set membership => inclusive property. We handle it in katello-installer (Katello/katello-installer#727) but upgrading users must take care of this themselves.

@ehelms
Copy link
Member Author

ehelms commented Nov 28, 2018

Will require theforeman/puppet-foreman#697 or else there are failures in the installer

@ehelms
Copy link
Member Author

ehelms commented Nov 28, 2018

I believe we can drop this as the original intent was to ensure that Pulp could read the CA key. But we no longer configure Pulp to need the CA key.

@ekohl
Copy link
Member

ekohl commented Nov 28, 2018

#270 should fix the tests

@ehelms
Copy link
Member Author

ehelms commented Nov 28, 2018

@ekohl Thanks for the test fixes. I re-ran this and it's now green.

@ekohl
Copy link
Member

ekohl commented Nov 28, 2018

Another potential reason was the legacy export. IIRC Katello does some operations on the Pulp files directly, for example in the old export. While this is a bad design that I'd love to get rid of, I'm not sure we should be doing this change before 3.10 is branched.

@ehelms
Copy link
Member Author

ehelms commented Nov 28, 2018

The original message when this was added was:

    adding apache user to the foreman group

    so pulp can read the candlepin private key

To me this has naught to do with export. Merging this is only a concern if we plan to release all of the puppet-katello changes into 3.10 which, as I understand things, we do not. A full nightly pipeline did pass with this change. I ran a test of export with these changes and all was good.

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't fully understand it so I was a bit hesitant, but I'm ok with merging this.

@ehelms ehelms merged commit 4a0f1b2 into theforeman:master Nov 28, 2018
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants