Skip to content
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

CCT-10: Ensure IPv6-based URLs are properly formatted #3346

Merged
merged 1 commit into from
Oct 18, 2023

Conversation

m-horky
Copy link
Contributor

@m-horky m-horky commented Oct 17, 2023

  • Card ID: CCT-10

Recent changes (e83e637) fixed parsing information from IPv6 URLs. This patch fixes writing these addresses back as strings, mainly into configuration files.

Previously, passing in IPv6 URL in e.g. --baseurl during registration resulted in broken address in the config file:

$ subscription-manager register --baseurl https://[::1]:8443/prefix $ cat /etc/rhsm/rhsm.conf | grep baseurl
baseurl=https://::1:8443/prefix

After this patch, the square brackets are written when port is specified:

$ cat /etc/rhsm/rhsm.conf | grep baseurl
baseurl=https://[::1]:8443/prefix

Due to the state of the code, it is likely that this problem also exists in other parts. If that is true, it is most likely in less sensitive parts, such as during logging. Looking back, using ipaddress stdlib would have been the right way to do this, but it is too late to do that.

@cnsnyder cnsnyder requested review from a team and wottop and removed request for a team October 17, 2023 07:13
@github-actions
Copy link

github-actions bot commented Oct 17, 2023

Coverage

Coverage (computed on Fedora latest) •
FileStmtsMissCoverMissing
subscription_manager
   utils.py3246280%58–59, 61, 63–65, 77, 80, 113, 167–170, 173–174, 176–178, 181–186, 200, 204, 217–218, 262–264, 267, 279, 281–284, 316–317, 321–323, 327, 347, 353–357, 359–360, 362, 364–366, 368, 408, 418, 616, 620, 652–653
TOTAL18159456074% 

Tests Skipped Failures Errors Time
2640 9 💤 0 ❌ 0 🔥 1m 3s ⏱️

Copy link
Contributor

@jirihnidek jirihnidek left a comment

Choose a reason for hiding this comment

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

LGTM

@m-horky m-horky force-pushed the mhorky/CCT-10_ipv6-format branch from 8dfd89f to c463cfc Compare October 17, 2023 08:55
* Card ID: CCT-10

Recent changes (e83e637) fixed parsing information from IPv6 URLs. This
patch fixes writing these addresses back as strings, mainly into
configuration files.

Previously, passing in IPv6 URL in e.g. `--baseurl` during registration
resulted in broken address in the config file:

$ subscription-manager register --baseurl https://[::1]:8443/prefix
$ cat /etc/rhsm/rhsm.conf | grep baseurl
baseurl=https://::1:8443/prefix

After this patch, the square brackets are written when port is
specified:

$ cat /etc/rhsm/rhsm.conf | grep baseurl
baseurl=https://[::1]:8443/prefix

Due to the state of the code, it is likely that this problem also exists
in other parts. If that is true, it is most likely in less sensitive
parts, such as during logging. Looking back, using `ipaddress` stdlib
would have been the right way to do this, but it is too late to do that.
@m-horky m-horky force-pushed the mhorky/CCT-10_ipv6-format branch from c463cfc to 9e002d1 Compare October 17, 2023 09:01
@jirihnidek jirihnidek merged commit a45afb7 into main Oct 18, 2023
10 checks passed
@jirihnidek jirihnidek deleted the mhorky/CCT-10_ipv6-format branch October 18, 2023 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants