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
KVM: Prevent regenerating keystore on provisionCertificate API #3075
Conversation
Can we reuse keystore table for this maybe? |
...cloudstack/api/command/admin/direct/download/UploadTemplateDirectDownloadCertificateCmd.java
Outdated
Show resolved
Hide resolved
server/src/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java
Outdated
Show resolved
Hide resolved
@blueorangutan package |
@borisstoyanov a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2610 |
@blueorangutan test |
1 similar comment
@blueorangutan test |
@borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
Trillian test result (tid-3409)
|
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.
@nvazquez there seems to be something wrong with the SSVM tests here, can you look when you have time.
Sure @borisstoyanov let me take a look later today |
322ca1a
to
35fa34d
Compare
@blueorangutan package |
@nvazquez a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2614 |
@borisstoyanov I have rebased the 4.11 branch and tested those files locally without errors on failing tests (results below). It seems to be errors with checksums tests. test_ssvm.py:
test_iso.py, test_templates.py, test_volumes.py:
|
@nvazquez unsupported parameters provided. Supported mgmt server os are: |
@blueorangutan test |
@nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
Trillian test result (tid-3410)
|
@blueorangutan package |
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2843 |
@blueorangutan test |
@nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
@rhtyd this is ready for review |
@@ -38,9 +38,6 @@ if [ -z "${KS_PASS// }" ]; then | |||
exit 1 | |||
fi | |||
|
|||
# Use a new keystore file | |||
NEW_KS_FILE="$KS_FILE.new" |
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 script was create to be idempotent for jks creation and cert import. I'll have to test concurrent cases and see if this script would pass/fail. I'll test and review today and keep you posted @nvazquez thanks.
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'll also need to test for certificate renewal.
Trillian test result (tid-3645)
|
4c9bca6
to
07a550c
Compare
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
@nvazquez LGTM, I reviewed and fixed few issues around certificate renewal |
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2847 |
@blueorangutan test |
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
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.
code changes look good, testing this is done and i'll have to trust that but it is scary ;)
Trillian test result (tid-3650)
|
Thanks for the last fix @rhtyd |
Description
It was found out that direct download feature stops working after 'provisionCertificate' API is executed, as both features import certificates into the same
/etc/cloudstack/agent/cloud.jks
keystore.The 'provisionCertificate' API invokes a script which regenerates the keystore before adding the new certificate, removing the existing certificates from the keystore. This fix prevents regenerating the keystore before adding the new certificates.
Types of changes
Screenshots (if appropriate):
How Has This Been Tested?