Skip to content

fix bug in parsing proxies with dashes #20648

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
merged 2 commits into from
May 13, 2025
Merged

fix bug in parsing proxies with dashes #20648

merged 2 commits into from
May 13, 2025

Conversation

@ComradeProgrammer
Copy link
Member

@ComradeProgrammer ComradeProgrammer commented Apr 22, 2025 •
edited
Loading

fix: fix bug in MaskProxyPassword
FIX #20624

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 22, 2025
@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Apr 22, 2025
@k8s-ci-robot k8s-ci-robot requested a review from spowelljr April 22, 2025 20:18
@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 22, 2025
medyagh
medyagh requested changes Apr 24, 2025
View reviewed changes
Copy link
Member

@medyagh medyagh left a comment

Choose a reason for hiding this comment

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

thank you for fixing this, can we please move these functions into the proxy package?
I would like util package to be as small as possible

@medyagh
Copy link
Member

medyagh commented Apr 24, 2025

/ok-to-test

@k8s-ci-robot k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Apr 24, 2025
@minikube-pr-bot

This comment has been minimized.

Sign in to view
@medyagh medyagh changed the title fix: fix bug in MaskProxyPassword fix bug in parsing proxies with dashes Apr 30, 2025
medyagh
medyagh requested changes Apr 30, 2025
View reviewed changes
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels May 7, 2025
@ComradeProgrammer ComradeProgrammer requested a review from medyagh May 7, 2025 17:58
@minikube-pr-bot
Copy link

minikube-pr-bot commented May 7, 2025

kvm2 driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 20648) |
+----------------+----------+---------------------+
| minikube start | 52.2s    | 52.8s               |
| enable ingress | 17.3s    | 18.3s               |
+----------------+----------+---------------------+

Times for minikube start: 53.1s 50.5s 53.6s 52.8s 51.0s
Times for minikube (PR 20648) start: 51.4s 52.4s 57.2s 51.7s 51.5s

Times for minikube (PR 20648) ingress: 18.1s 19.0s 19.6s 19.1s 15.6s
Times for minikube ingress: 18.6s 19.1s 15.1s 18.6s 15.1s

docker driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 20648) |
+----------------+----------+---------------------+
| minikube start | 25.2s    | 23.6s               |
| enable ingress | 13.0s    | 13.1s               |
+----------------+----------+---------------------+

Times for minikube start: 26.3s 24.8s 26.2s 24.3s 24.2s
Times for minikube (PR 20648) start: 23.8s 23.9s 23.1s 24.3s 22.8s

Times for minikube ingress: 12.8s 13.8s 12.8s 12.3s 13.4s
Times for minikube (PR 20648) ingress: 13.3s 12.9s 12.9s 12.9s 13.3s

docker driver with containerd runtime

+-------------------+----------+---------------------+
|      COMMAND      | MINIKUBE | MINIKUBE (PR 20648) |
+-------------------+----------+---------------------+
| minikube start    | 24.8s    | 23.9s               |
| ⚠️  enable ingress | 22.9s    | 32.5s ⚠️             |
+-------------------+----------+---------------------+

Times for minikube ingress: 22.9s 22.8s 22.8s 22.9s 22.9s
Times for minikube (PR 20648) ingress: 38.8s 22.9s 22.8s 38.8s 38.9s

Times for minikube (PR 20648) start: 25.4s 22.0s 23.4s 22.3s 26.3s
Times for minikube start: 25.1s 24.9s 24.3s 25.6s 24.0s

medyagh
medyagh approved these changes May 13, 2025
View reviewed changes
Copy link
Member

@medyagh medyagh left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 13, 2025
@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented May 13, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ComradeProgrammer, medyagh

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [ComradeProgrammer,medyagh]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@medyagh medyagh merged commit 4072869 into kubernetes:master May 13, 2025
31 of 38 checks passed
@medyagh medyagh mentioned this pull request May 13, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@medyagh medyagh medyagh approved these changes

@spowelljr spowelljr Awaiting requested review from spowelljr

Assignees

@medyagh medyagh

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

minikube fails to parse http_proxy with - in the address

4 participants

@ComradeProgrammer @medyagh @minikube-pr-bot @k8s-ci-robot