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

Enable katello #96

Merged
merged 1 commit into from
Apr 6, 2015
Merged

Enable katello #96

merged 1 commit into from
Apr 6, 2015

Conversation

shlomizadok
Copy link
Member

File lib/foreman_openscap/helper.rb is now testing if cname is a uuid or a host name.

File app/models/concerns/foreman_openscap/policy_extensions.rb Adds overrides to the Puppet module to use Katello certificates. This is now ideal, as we take to assumption that the host will also have a content host.

@ohadlevy, @ares, @stbenjam - please review as well. (Thanks!)

certificate_param.override = true
case override_parameter
when CA_FILE_PARAMETER
certificate_param.default_value = '/etc/rhsm/ca/katello-server-ca.pem'
Copy link
Member

Choose a reason for hiding this comment

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

Oh, can you not read the rhsm.conf on the client uploading tool instead of sending them via Puppet?

Depending on when the system was registered these could be different.... at one point, the file wasn't named /etc/rhsm/ca/katello-server-ca.pem.

Copy link
Member

Choose a reason for hiding this comment

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

reading another system config files does not sound ideal either, can the installer / some other thing maintain another config file / links to the ca etc ?

Copy link
Member

Choose a reason for hiding this comment

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

Why not? They're authenticating with the RHSM certificates; they should use the canonical source for that information, which is /etc/rhsm/rhsm.conf. katello-agent does the same, although that is more closely linked to rhsm.

For whatever reason, the certificate was renamed from /etc/rhsm/ca/candlepin-local.pem to /etc/rhsm/ca/katello-server-ca.pem, so older clients may have the older cert location.

If you don't read from rhsm.conf, you'd be forcing users to regenerate all of their certificate RPMs and redeploy them with the newer names, or change all of their older clients some other way? I don't think that's a reasonable ask.

The client knows where its certificates are, why is it being sent from the Foreman server anyway?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Maybe you could have a boolean for using the rhsm to upload.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

How would a proxy connected to Katello be using puppet certs??

You all can solve it however you want, but if you hardcode RHSM cert locations, you're going to run into problems. That's my only comment about this PR.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

subscription-manager config but it's basically just catting /etc/rhsm/rhsm.conf

Copy link
Member

Choose a reason for hiding this comment

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

Actually, subscription-manager config may be preferable, because it does the variable substitution you'd have to deal with in rhsm.conf,

repo_ca_cert = /etc/rhsm/ca/katello-server-ca.pem

So, yea, that could be turned into a fact

@shlomizadok
Copy link
Member Author

@stbenjam @ohadlevy @ares, thank you all for the comments and help.
I have remove the hard coded puppet class params overrides.
Instead, I am testing which certificates to use on the puppet module.
Would love if you could take a look and review theforeman/puppet-foreman_scap_client#6 in addition to this PR.
Thanks!

@shlomizadok
Copy link
Member Author

@ares @isimluk, I believe this PR is also ready to merge.

@ohadlevy
Copy link
Member

ohadlevy commented Apr 1, 2015

@isimluk ping? any reason not to merge?

@ares
Copy link
Member

ares commented Apr 6, 2015

Thanks @shlomizadok, merging!

@ares
Copy link
Member

ares commented Apr 6, 2015

@shlomizadok actually before merge, could you squash changes in policy_extensions.rb so we don't have one commit adding stuff that is removed in second? I did review of the whole then realized that there are three commits and most of 1st and 2nd neutralize each other :-) I'd probably squash into one as this is one feature as a whole

@shlomizadok
Copy link
Member Author

@ares squashed. Thanks.

ares added a commit that referenced this pull request Apr 6, 2015
@ares ares merged commit 0f35b5a into theforeman:master Apr 6, 2015
@ares
Copy link
Member

ares commented Apr 6, 2015

Merged, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants