-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Changes error message when using invalid endpoint.url
#8603
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
base: main
Are you sure you want to change the base?
Conversation
@blueorangutan package |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8603 +/- ##
============================================
- Coverage 17.56% 17.56% -0.01%
Complexity 15500 15500
============================================
Files 5899 5899
Lines 527793 527793
Branches 64479 64477 -2
============================================
- Hits 92714 92711 -3
- Misses 424653 424659 +6
+ Partials 10426 10423 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@blueorangutan package |
@weizhouapache 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. |
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8536 |
@blueorangutan test |
@DaanHoogland a [SL] 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.
clgtm
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.
overall code lgtm
left 2 minor comments
api/src/main/java/org/apache/cloudstack/config/ApiServiceConfiguration.java
Show resolved
Hide resolved
api/src/main/java/org/apache/cloudstack/config/ApiServiceConfiguration.java
Outdated
Show resolved
Hide resolved
@weizhouapache, thanks for your review. I've made changes to incorporate your suggestions in the latest commit (4cf74fc), so if you could provide another review, I would greatly appreciate it! |
@blueorangutan package |
@weizhouapache 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. |
@lucas-a-martins |
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8556 |
[SF] Trillian test result (tid-9096)
|
Hey, @lucas-a-martins please change your logger imports to use log4j2 |
@lucas-a-martins do you want to target this for 4.20?
|
@DaanHoogland I do. I'm just testing the code with log4j2. |
api/src/main/java/org/apache/cloudstack/config/ApiServiceConfiguration.java
Outdated
Show resolved
Hide resolved
@blueorangutan package |
@weizhouapache 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. |
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8694 |
@blueorangutan test |
@DaanHoogland a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
[SF] Trillian test result (tid-9288)
|
@lucas-a-martins this was tested ok (#8603 (review)) and has enough reviews, but also conflicts. Can you resolve those? |
…guration.java Co-authored-by: Daniel Augusto Veronezi Salvador <[email protected]>
6b86329
to
aaabe66
Compare
@DaanHoogland I have rebased the branch and resolved the conflicts. Is there anything else that has to be done? |
I’ll rerun regression tests. |
@blueorangutan package |
@DaanHoogland 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. |
Packaging result [SF]: ✖️ el8 ✖️ el9 ✔️ debian ✖️ suse15. SL-JID 15407 |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15428 |
@blueorangutan test |
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
[SF] Trillian Build Failed (tid-14647) |
@blueorangutan test |
@rosi-shapeblue a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
[SF] Trillian test result (tid-14650)
|
Description
When validating the
endpoint.url
global setting, such as when using thecreateKubernetesCluster
API, we encounter the exception messageGlobal setting endpoint.url has to be set to the Management Server's API endpoint
. This message lacks clarity about the problem to be solved and exposes information about the environment. Upon further analysis, it was found that this same message was used across several classes, repeating the same code in each one of them.This pull request addresses this issue by replacing the exception with a generic one that does not expose any information about the environment. Also, it introduces a more detailed and explicit message to the logs, including the current value of
endpoint.url
. Furthermore, a new method has been implemented to validate theendpoint.url
whenever necessary.Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
I tested by using CloudMonkey and tried to create a Kubernetes cluster. I implemented unit tests for the new method too.
Exception before changes:
Exception after changes: