fix (jkube-kit) : Don't pass Invalid port in host headers for Helm OCI push #2418
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Eclipse JKube CI ReportStarted new GH workflow run for #2418 (2023-10-11T09:05:42Z) ⚙️ JKube E2E Tests (6480092030)
|
Codecov Report
@@ Coverage Diff @@
## master #2418 +/- ##
============================================
+ Coverage 59.36% 61.15% +1.79%
- Complexity 4586 4803 +217
============================================
Files 500 518 +18
Lines 21211 21399 +188
Branches 2830 2827 -3
============================================
+ Hits 12591 13087 +496
+ Misses 7370 7089 -281
+ Partials 1250 1223 -27
... and 71 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
manusa
reviewed
Oct 11, 2023
Comment on lines
59
to
61
| String hostStr = baseUrl.getHost(); | ||
| String portStr = baseUrl.getPort() > 0 ? (":" + baseUrl.getPort()) : EMPTY; | ||
| return String.format("%s%s", hostStr, portStr); |
There was a problem hiding this comment.
nit
This is extremely verbose and complicated to read. You're mixing template strings with ternaries and string concatenation.
Something like:
final StringBuilder host = new StringBuilder(baseUrl.getHost());
if (baseUrl.getPort() > 0 && baseUrl.getPort() != 80 && baseUrl.getPort() != 443) {
test.append(":").append(baseUrl.getPort());
}
System.out.println(test.toString());is easier to understand and doesn't mix 3 different ways of concatenating strings.
…I push Passing host:port seems to work when host url contains a valid port. However, this becomes host:-1 for some host that doesn't contain port in URL string (like r.example.com). Add case for these kind of hosts so that we pass correct host in Host HTTP headers. Signed-off-by: Rohan Kumar <[email protected]>
rohanKanojia
force-pushed
the
pr/issue2417
branch
from
1f35443 to
78a7459
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
manusa
approved these changes
Oct 11, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Passing host:port seems to work when host url contains a valid port. However, this becomes host:-1 for some host that doesn't contain port in URL string (like r.example.com). Add case for these kind of hosts so that we pass correct host in Host HTTP headers.
Fixes #2417
Type of change
test, version modification, documentation, etc.)
Checklist