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 #18987 - Check ueber certs on each proxy sync #6728
Conversation
@johnpmitsch, thanks for your PR! By analyzing the history of the files in this pull request, we identified @bbuckingham, @jlsherrill and @iNecas to be potential reviewers. |
@jlsherrill is this testable? Would we be able to get a cert and CA that could be verified in a test env? |
We likely could write a test that includes some certs with say a 30 year expiration for this test (most likely any ca and ueber cert from some sample install would suffice along with some stubs). |
test/services/cert/certs_test.rb
Outdated
|
||
def test_verify_ueber_cert | ||
Setting.stubs(:[]).with(:ssl_ca_file).returns("/home/vagrant/foreman/test/services/cert/helpers/ca.crt") | ||
cert_valid = Cert::Certs.verify_ueber_cert(@org) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trailing whitespace detected.
test/services/cert/certs_test.rb
Outdated
|
||
def test_verify_ueber_cert | ||
Setting.stubs(:[]).with(:ssl_ca_file).returns("/home/vagrant/foreman/test/services/cert/helpers/ca.crt") | ||
cert_valid = Cert::Certs.verify_ueber_cert(@org) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trailing whitespace detected.
1f2678a
to
4f29933
Compare
There were the following issues with the commit message:
If you don't have a ticket number, please create an issue in Redmine. More guidelines are available in Coding Standards or on the Foreman wiki. This message was auto-generated by Foreman's prprocessor |
test/services/cert/certs_test.rb
Outdated
end | ||
|
||
def test_verify_ueber_cert | ||
Setting.stubs(:[]).with(:ssl_ca_file).returns(File.join("#{Rails.root}", "/test/services/cert/helpers/ca.crt")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jlsherrill @stbenjam I keep getting this error on a test run:
Katello::CertsTest#test_verify_ueber_cert:
OpenSSL::X509::StoreError: system lib
/home/vagrant/katello/app/services/cert/certs.rb:22:in `add_file'
/home/vagrant/katello/app/services/cert/certs.rb:22:in `verify_ueber_cert'
/home/vagrant/katello/test/services/cert/certs_test.rb:20:in `test_verify_ueber_cert'
any ideas why that is? I've even tried the hardcoded full path
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because Rails.root refers to the foreman dir, not the katello dir. You want Katello::Engine.root
Also the assert won't work, you'd want to use:
@org.expects(:regenerate_ueber_cert).never
instead of the assert()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd also like to see a 'bad' case. you might use the included redhat ca cert (ca/redhat-uep.pem) to test a bad caes
4f29933
to
71bad28
Compare
There were the following issues with the commit message:
If you don't have a ticket number, please create an issue in Redmine. More guidelines are available in Coding Standards or on the Foreman wiki. This message was auto-generated by Foreman's prprocessor |
71bad28
to
31c84b9
Compare
@jlsherrill thanks for the suggestions, updated! |
[test] |
1 similar comment
[test] |
APJ |
When the CA changes for any reason (like a hostname change), we need to regenerate the ueber certs for organizations. This will automatically verify the ueber cert against the ca cert on each proxy sync.
To-do