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

Masking http(s)_proxy password from startup output. #17116

Merged
merged 5 commits into from
Sep 20, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions pkg/minikube/node/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ import (
"os"
"os/exec"
"path/filepath"
"regexp"
"strconv"
"strings"
"sync"

"github.com/spf13/viper"
Expand All @@ -47,6 +49,16 @@ func showVersionInfo(k8sVersion string, cr cruntime.Manager) {
out.Infof("opt {{.docker_option}}", out.V{"docker_option": v})
}
for _, v := range config.DockerEnv {
parts := strings.Split(v, "=")
Copy link
Member

Choose a reason for hiding this comment

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

I recommend, putting this into a func and then add unit tests for them.
in config_test.go

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@medyagh Will do that. But turns out "make test" is completely inconsistent in my WSL/Ubuntu-22.04. This function:

func TestValidatePorts(t *testing.T) {

is failing on different ports each time I run "make test" and sometimes it's even running without any failures. To be sure it's not related to my commit, I ran it on the original repo and it's behaving the same way. Guess I'll try to figure out what's happening here before going any further. Perhaps should I file a new issue? What do you think?

Anyway... I'll also test on Ubuntu running on a Virtual Machine to rule out some WSL related quirky behaviour. 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@medyagh Moved the mask password to a func and implemented config_test.go. Let me know how it looks.

if len(parts) == 2 {
key := strings.ToUpper(parts[0])
if key == "HTTP_PROXY" || key == "HTTPS_PROXY" {
pattern := `//(\w+):\w+@`
regexpPattern := regexp.MustCompile(pattern)
value := regexpPattern.ReplaceAllString(parts[1], "//$1:*****@")
v = key + "=" + value
}
}
out.Infof("env {{.docker_env}}", out.V{"docker_env": v})
}
}
Expand Down
8 changes: 7 additions & 1 deletion pkg/minikube/node/start.go
100644 → 100755
Original file line number Diff line number Diff line change
Expand Up @@ -723,9 +723,15 @@ func validateNetwork(h *host.Host, r command.Runner, imageRepository string) (st
out.Styled(style.Internet, "Found network options:")
optSeen = true
}
k = strings.ToUpper(k) // let's get the key right away to mask password from output
// If http(s)_proxy contains password, let's not splatter on the screen
if k == "HTTP_PROXY" || k == "HTTPS_PROXY" {
pattern := `//(\w+):\w+@`
regexpPattern := regexp.MustCompile(pattern)
v = regexpPattern.ReplaceAllString(v, "//$1:*****@")
}
out.Infof("{{.key}}={{.value}}", out.V{"key": k, "value": v})
ipExcluded := proxy.IsIPExcluded(ip) // Skip warning if minikube ip is already in NO_PROXY
k = strings.ToUpper(k) // for http_proxy & https_proxy
if (k == "HTTP_PROXY" || k == "HTTPS_PROXY") && !ipExcluded && !warnedOnce {
out.WarningT("You appear to be using a proxy, but your NO_PROXY environment does not include the minikube IP ({{.ip_address}}).", out.V{"ip_address": ip})
out.Styled(style.Documentation, "Please see {{.documentation_url}} for more details", out.V{"documentation_url": "https://minikube.sigs.k8s.io/docs/handbook/vpn_and_proxy/"})
Expand Down