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

Fixes #25451 - Make certificate loading robust #6239

Merged
merged 1 commit into from Nov 14, 2018

Conversation

ekohl
Copy link
Member

@ekohl ekohl commented Nov 13, 2018

This is copied from Katello's app/services/cert/rhsm_client.rb which should probably be merged in a cleaner way.

app/services/certificate_extract.rb Outdated Show resolved Hide resolved
app/services/certificate_extract.rb Outdated Show resolved Hide resolved
@ehelms
Copy link
Member

ehelms commented Nov 13, 2018

I tested this code change on nightly and the puppet bats tests passed. So if unit tests are happy, I think this is good by me and the WIP can be dropped.

@ekohl ekohl changed the title [WIP] Make certificate loading robust Fixes #25451 - Make certificate loading robust Nov 13, 2018
@ekohl
Copy link
Member Author

ekohl commented Nov 13, 2018

I've added some tests, created an issue and dropped the WIP. Ready for review now.

@timogoebel
Copy link
Member

Two test classes in a single file aren't the rails way (at least with my limited understanding). I would have used two context blocks. Otherwise, this looks good. I'd be ok to merge this.

This is copied from Katello's app/services/cert/rhsm_client.rb with
added tests.
@ekohl
Copy link
Member Author

ekohl commented Nov 13, 2018

I agree this looks cleaner.

@timogoebel timogoebel merged commit 401aac8 into theforeman:develop Nov 14, 2018
@timogoebel
Copy link
Member

Thanks, @ekohl.

@ekohl ekohl deleted the robust-cert-loading branch November 14, 2018 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants