-
Notifications
You must be signed in to change notification settings - Fork 192
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
Create a taskomatic job to refresh the trusted root CAs in ISSv3 #9660
base: issv3
Are you sure you want to change the base?
Create a taskomatic job to refresh the trusted root CAs in ISSv3 #9660
Conversation
👋 Hello! Thanks for contributing to our project. If you are unsure the failing tests are related to your code, you can check the "reference jobs". These are jobs that run on a scheduled time with code from master. If they fail for the same reason as your build, it means the tests or the infrastructure are broken. If they do not fail, but yours do, it means it is related to your code. Reference tests: KNOWN ISSUES Sometimes the build can fail when pulling new jar files from download.opensuse.org . This is a known limitation. Given this happens rarely, when it does, all you need to do is rerun the test. Sorry for the inconvenience. For more tips on troubleshooting, see the troubleshooting guide. Happy hacking! |
Suggested tests to cover this Pull Request
|
e594d37
to
761c557
Compare
872a7f9
to
b640370
Compare
private void saveCertificate(String fileName, String rootCaCertContent) throws IOException { | ||
String fullFileName = CertificateUtils.CERTS_PATH.resolve(fileName).toString(); | ||
try (FileWriter fw = new FileWriter(fullFileName, false)) { | ||
fw.write(rootCaCertContent); | ||
} |
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.
We could treat an empty rootCaCertContent
as a request to remove the certificate.
Instead of having a file with 0 byte content, we should remove it (if it exist)
|
||
TaskomaticApi taskomaticApi = new TaskomaticApi(); | ||
try { | ||
taskomaticApi.scheduleSingleRootCaCertUpdate(filenameToRootCaCertMap); |
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.
As this task run in taskomatic, I wonder if we need to call with via taskomatic API.
I think your idea was already to put the logic into CertificateUtils and call if from here and from RootCaCertUpdateTask.
What has changed your mind?
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.
Taskomatic API: you're right, my fault. I created a public entry in RootCaCertUpdateTask (saveAndUpdateCaCertificates) and call the stuff from there.
The reason I did not put the logic in CertificateUtils is because updating the certificates calls "executeExtCmd" which is a method of RhnJob class.
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.
The reason I did not put the logic in CertificateUtils is because updating the certificates calls "executeExtCmd" which is a method of RhnJob class.
Ahh, right. Let's see if there is a way without going over the network.
11cc22a
to
1a51d4c
Compare
1a51d4c
to
965eca6
Compare
java/code/src/com/redhat/rhn/taskomatic/task/RootCaCertUpdateTask.java
Outdated
Show resolved
Hide resolved
CertificateUtils.saveCertificates(filenameToRootCaCertMap); | ||
updateCaCertificates(); | ||
} | ||
|
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 tried to move all I could to CertificateUtils class. Unfortunately updateCaCertificates uses "executeExtCmd" (from RhnJavaJob) and hence it could not be moved. The same happens in PaygUpdateHostsTask.loadHttpsCertificates. Any suggestion is welcome.
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.
Ok
The code looks good. But we should fix the tests before we merge. |
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.
XMLRPC looks fine 👍
0920d8e
to
8d0e837
Compare
What does this PR change?
When onboarding a peripheral or a hub, a root certificate might be needed if the two servers do not share a common one. The certificate is transferred during the registration process and stored in the database.
However, this does not alter the system trust configuration. This step cannot be implemented from the web UI/API backend, as Tomcat does not run with the required root privileges.
Therefore, we need to implement a Taskomatic job that checks if new certificates have been added or updated and stores them accordingly in the trusted certificate path
GUI diff
No difference.
Documentation
Test coverage
Links
Issue(s): https://github.com/SUSE/spacewalk/issues/26180
Port(s):
Changelogs
Re-run a test