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

candlepin: remove nssdb dependency #174

Merged
merged 1 commit into from
May 18, 2018

Conversation

timogoebel
Copy link
Member

I don't see any reasoning for this code being there. Basically, it adds candlepin's java-client cert (the cert candlepin uses to connect to qpidd) to the qpidd truststore.
Candlepin doesn't need this to boot up. And it doesn't need this to connect to qpidd. We don't do anything similar elsewhere. I mean, isn't the idea of a PKI to not whitelist individual certificates but trust the CA?

Anyways, this removes another dependency. Goes along with theforeman/puppet-katello#215 to remove the qpidd dependency.

@ekohl
Copy link
Member

ekohl commented Sep 24, 2017

I was a bit unsure about this in the first place and agree with your reasoning but I hadn't looked far enough into this to check why it's there in the first place.

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.

The pipeline still passes with this patch so 👍

@ekohl ekohl requested a review from ehelms October 4, 2017 11:39
@ekohl
Copy link
Member

ekohl commented Oct 4, 2017

@ehelms
Copy link
Member

ehelms commented Oct 4, 2017

@ekohl did you check that the Candlepin listener event is happy and receiving events?

@ekohl
Copy link
Member

ekohl commented Oct 4, 2017

@ehelms if it's covered by bats then yes, otherwise no.

@timogoebel
Copy link
Member Author

Removed qpidd in acceptance test.

Copy link
Member

@sean797 sean797 left a comment

Choose a reason for hiding this comment

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

This looks sane to me, I tested; the Listen on candlepin events task, manifest import, client registration & attaching client subs worked all just fine 👍

@ekohl ekohl merged commit 159b4d2 into theforeman:master May 18, 2018
@ekohl
Copy link
Member

ekohl commented May 18, 2018

Thanks @timogoebel!

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