Skip to content

Certificate and VM hostname validation improvements #10051

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.

Sign up for GitHub

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

Jump to bottom
Merged
rohityadavcloud merged 2 commits into from
Dec 20, 2024
Merged

Certificate and VM hostname validation improvements #10051

rohityadavcloud merged 2 commits into from
Dec 20, 2024

Conversation

@sureshanaparti
Copy link
Contributor

@sureshanaparti sureshanaparti commented Dec 5, 2024 •
edited
Loading

Description

This PR improves the Certificate and VM hostname validations, during direct download certificate setup and VM IP address check.

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • build/CI
  • test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

Tested upload templatedirectdownloadcertificate with valid and invalid names.

How did you try to break this feature and the system with this change?

@sureshanaparti
Copy link
Contributor Author

sureshanaparti commented Dec 5, 2024

@blueorangutan package

@blueorangutan
Copy link

blueorangutan commented Dec 5, 2024

@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@codecov
Copy link

codecov bot commented Dec 5, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 15.38462% with 22 lines in your changes missing coverage. Please review.

Project coverage is 15.12%. Comparing base (a9587bf) to head (e0c707c).
Report is 2 commits behind head on 4.19.

Files with missing lines Patch % Lines
...er/xenbase/CitrixGetVmIpAddressCommandWrapper.java 14.28% 6 Missing ⚠️
.../src/main/java/com/cloud/vm/UserVmManagerImpl.java 0.00% 5 Missing ⚠️
...ack/direct/download/DirectDownloadManagerImpl.java 0.00% 5 Missing ⚠️
...e/wrapper/LibvirtGetVmIpAddressCommandWrapper.java 0.00% 2 Missing ⚠️
...tSetupDirectDownloadCertificateCommandWrapper.java 0.00% 2 Missing ⚠️
...ls/src/main/java/com/cloud/utils/net/NetUtils.java 60.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##               4.19   #10051      +/-   ##
============================================
- Coverage     15.13%   15.12%   -0.01%     
  Complexity    11266    11266              
============================================
  Files          5408     5408              
  Lines        473890   473900      +10     
  Branches      57787    57791       +4     
============================================
- Hits          71704    71697       -7     
- Misses       394183   394202      +19     
+ Partials       8003     8001       -2
Flag Coverage Δ
uitests 4.30% <ø> (ø)
unittests 15.84% <15.38%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@blueorangutan
Copy link

blueorangutan commented Dec 5, 2024

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 11727

@rohityadavcloud rohityadavcloud added this to the 4.19.2 milestone Dec 6, 2024
@sureshanaparti sureshanaparti force-pushed the certificate-and-vm-hostname-validation-improvements branch from 95c2f76 to e0c707c Compare December 19, 2024 04:08
@sureshanaparti
Copy link
Contributor Author

sureshanaparti commented Dec 19, 2024

@blueorangutan package

@blueorangutan
Copy link

blueorangutan commented Dec 19, 2024

@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

vishesh92
vishesh92 approved these changes Dec 19, 2024
View reviewed changes
Copy link
Member

@vishesh92 vishesh92 left a comment

Choose a reason for hiding this comment

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

clgtm. didn't test.

@blueorangutan
Copy link

blueorangutan commented Dec 19, 2024

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 11845

kiranchavala
kiranchavala approved these changes Dec 19, 2024
View reviewed changes
Copy link
Contributor

@kiranchavala kiranchavala left a comment

Choose a reason for hiding this comment

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

LGTM

Executed the API “uploadtemplatedirectdownloadcertificate" by providing an invalid name value

https://cloudstack.apache.org/api/apidocs-4.18/apis/listTemplateDirectDownloadCertificates.html

For example
Invalid name with
greater than 63 chars
should not start or end with '-'
should only contain alpha numeric

example:

(localcloud) 🐱 > upload templatedirectdownloadcertificate hypervisor=KVM name=-kiran certificate="MIIF7zCCA9egAwIBAgIUcC+POP+CQ6CUMfeMJqelQO/zSxAwDQYJKoZIhvcNAQELBQAwgYYxCzAJBgNVBAYTAlhYMRIwEAYDVQQIDAlTdGF0ZU5hbWUxETAPBgNVBAcMCENpdHlOYW1lMRQwEgYDVQQKDAtDb21wYW55TmFtZTEbMBkGA1UECwwSQ29tcGFueVNlY3Rpb25OYW1lMR0wGwYDVQQDDBRDb21tb25OYW1lT3JIb3N0bmFtZTAeFw0yNDEyMTkwNjQ5MjRaFw0zNDEyMTcwNjQ5MjRaMIGGMQswCQYDVQQGEwJYWDESMBAGA1UECAwJU3RhdGVOYW1lMREwDwYDVQQHDAhDaXR5TmFtZTEUMBIGA1UECgwLQ29tcGFueU5hbWUxGzAZBgNVBAsMEkNvbXBhbnlTZWN0aW9uTmFtZTEdMBsGA1UEAwwUQ29tbW9uTmFtZU9ySG9zdG5hbWUwggIiMA0GCSqGSIb3DQEBAQUAA4ICDwAwggIKAoICAQCxSaia77e0mHOQLEW6hQyEft0Il1cejoLLGk3jUxNqW6G6OZT0OoJVUhErpnYmM3DU6z6c6Yv9qU7nN3TCkJtsCwJuKuyQY1nfTXOsFrgaDSF6cpsb3uion1u+ZhQ3DRW/VQcytwZd+kymeziIryiiFSQGSe+hjb+8masAViS5QhFQJDIGkiuG1WCTk9ov/EMDVsS8sdaWBIM82KpfC733JgTC5ObrxCvVuuB3RQGEDXzNGomO1HZUzOTJQI+OVG5ziWD9Oys2SPfs7IfNd7UfNwedix1zMiMQShfgS0wfQW80Lt9RH5WV9KgT3hh+J9edZd8yRdukSom0dr/4LEdWyrz20UZ7LWUFWnnjjmoc/Imti27MNcTNkmK4EexBaqFVHdN8TI+EU6SFmL6BVJUZGva/l21ZnO4DUnKAPV1l2WV79RlNyjavMEkxwR3BHlDUVogdMuf//uxjk8PUacpGH2UYBqlouPMW1qyq2WRV6IdUAbtRt/xKMe9FaFMY4ubFiYjNT5H6LxnysBboA0Eij6gQzrVzFRu10T72AMn0dfSZ6KEgaHMPZwQhJoa9eypW4oGESY43MtmHk3RtXoTDvic3EcivZVrRqydhvPIUnT/19DnpV4YkZ7u2rI+vRQ8/OZ7YfoIvGXuto3e9kZHVCghibUh9Ng40WKppVTcXxwIDAQABo1MwUTAdBgNVHQ4EFgQUSm5OwGtfZsSBvTvLF9lhNv3Pka4wHwYDVR0jBBgwFoAUSm5OwGtfZsSBvTvLF9lhNv3Pka4wDwYDVR0TAQH/BAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAgEAjGTX9QG0Z89ALBeuJaLfdQ/6FTUjTJCtt/aN4mVhHyxq5TL4rQMVb2wWtUUbvvmVktu31sc+YBerIIQ/n/1a5ZidsUBaKwXIgPHF5mIDV9/oSGSAxo1kX7PHy0kM42dEPnkSeoMabAeYV0F2cLkzm/Y1aX+8AX4R1G+2FcwMfLOfXVZi8rZXR1fkZ4Mr01rNseJJ60fJlBtOPV0ege9E4p27mLZLJd1xp07hHzJAfRM8P4Emkn4smLX0GhsB2H87s3canXWO3Wnh1nJgRmJ2SUhKf3WaWV8vMx9h4i2J73WKaf/6UUPw7LtOsEt11In0WUse27NcfEMeFzGBny24c6NC6kBZ0KUP+9p4hb3cKYzIWr/6tZ+Bz5V2Qvv00ryGw2o5frtMOAnJOG7rh0tO4nIJfy4tKgNMXXTERFikbuRVfurmFlibRzWDhLdRXq8FFgQuF7TXP4ljwwvwMHniGRkXdPwj6rXUBOiugCjlcui/eRvJCvITJjla4BIOwtXGIWveZ4BOPoooQlWte5ntnJuuXGYOkbozeP/P2LKM4pF4zKbdSbbrssJIc+drLUPe2K4WpChOHUDH8JUxZFHVAlzW2gKEOINwiI4yTCAFeiwZ/K8I8gyqb9hysd6dgsIDWkW+rXudoIyYy9o28QtXglKz2N/kkBKnGmW75geX/4E=" zoneid=5e86fe3c-ea4d-47ee-af4e-79095601ce1d
🙈 Error: (HTTP 530, error code 9999) The provided certificate name is invalid, please provide a valid name

Executed the api call

https://cloudstack.apache.org/api/apidocs-4.18/apis/listTemplateDirectDownloadCertificates.html

(localcloud) 🐱 > list templatedirectdownloadcertificates listall=true

The failed certificates were not present

@kiranchavala
Copy link
Contributor

kiranchavala commented Dec 19, 2024

@blueorangutan test

@blueorangutan
Copy link

blueorangutan commented Dec 19, 2024

@kiranchavala a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

@sureshanaparti sureshanaparti marked this pull request as ready for review December 19, 2024 08:22
@blueorangutan
Copy link

blueorangutan commented Dec 19, 2024

[SF] Trillian test result (tid-11944)
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 44281 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr10051-t11944-kvm-ol8.zip
Smoke tests completed. 133 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File

@rohityadavcloud rohityadavcloud merged commit 188eacd into apache:4.19 Dec 20, 2024
26 checks passed
@rohityadavcloud rohityadavcloud deleted the certificate-and-vm-hostname-validation-improvements branch December 20, 2024 04:48
DaanHoogland added a commit that referenced this pull request Dec 20, 2024
@DaanHoogland
Merge release branch 4.20 to main
9295a16 
* 4.20:
  VR: apply iptables rules when add/remove static routes (#10064)
  Certificate and VM hostname validation improvements (#10051)
  set ulimit for server according to redhat spec (#10040)
  kvm-storage: provide isVMMigrate information to storage plugins (#10093)
  Allow config drive deletion of migrated VM, on host maintenance (#10045)
  linstor: improve heartbeat check with also asking linstor (#10105)
  server: simplify role change validation (#9173)
  UI: create VPC network offering with conserve mode (#10082)
  server: fix typo removeaccessvpn in VirtualRouterElement (#10086)
  UI: remove duplicated Instance Name in Public IP details page (#10087)
  UI: Fixes in the Usage UI (#10000)
  SAML2: add cookie with HttpOnly too #10013 (#10047)
  ui: Allow font-awesome icon usage and optimise icon size inconsistency (#9744)
dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Dec 26, 2024
@sureshanaparti @dhslove
Certificate and VM hostname validation improvements (apache#10051)
ab08389 
* Certificate and VM hostname validation improvements

* Improve certificate name validation and some code/log improvements
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DaanHoogland DaanHoogland DaanHoogland left review comments

@kiranchavala kiranchavala kiranchavala approved these changes

@vishesh92 vishesh92 vishesh92 approved these changes

Assignees

No one assigned

Labels

component:kvm component:XenServer

Projects

None yet

Milestone

4.19.2

Development

Successfully merging this pull request may close these issues.

6 participants

@sureshanaparti @blueorangutan @kiranchavala @DaanHoogland @vishesh92 @rohityadavcloud