Merge pull request #13751 from bparees/clear_proxy

Merged by openshift-bot

pkg/build/builder/cmd/builder.go

@@ -23,6 +23,7 @@ import (
 	"github.com/openshift/origin/pkg/build/api/validation"
 	bld "github.com/openshift/origin/pkg/build/builder"
 	"github.com/openshift/origin/pkg/build/builder/cmd/scmauth"
+	"github.com/openshift/origin/pkg/build/util"
 	"github.com/openshift/origin/pkg/client"
 	"github.com/openshift/origin/pkg/generate/git"
 	"github.com/openshift/origin/pkg/version"
@@ -47,17 +48,32 @@ func newBuilderConfigFromEnvironment(out io.Writer) (*builderConfig, error) {
 
 	cfg.out = out
 
-	// build (BUILD)
 	buildStr := os.Getenv("BUILD")
-	glog.V(4).Infof("$BUILD env var is %s \n", buildStr)
 	cfg.build = &api.Build{}
-	if err := runtime.DecodeInto(kapi.Codecs.UniversalDecoder(), []byte(buildStr), cfg.build); err != nil {
-		return nil, fmt.Errorf("unable to parse build: %v", err)
+
+	obj, groupVersionKind, err := kapi.Codecs.UniversalDecoder().Decode([]byte(buildStr), nil, nil)
+	if err != nil {
+		return nil, fmt.Errorf("unable to parse build string: %v", err)
+	}
+	ok := false
+	cfg.build, ok = obj.(*api.Build)
+	if !ok {
+		return nil, fmt.Errorf("build string is not a build: %v", err)
+	}
+	if glog.V(4) {
+		redactedBuild := util.SafeForLoggingBuild(cfg.build)
+		if err != nil {
+			return nil, fmt.Errorf("unable to strip proxy credentials from build: %v", err)
+		}
+		bytes, err := runtime.Encode(kapi.Codecs.LegacyCodec(groupVersionKind.GroupVersion()), redactedBuild)
+		if err != nil {
+			return nil, fmt.Errorf("unable to serialize build: %v", err)
+		}
+		glog.V(4).Infof("redacted build: %v", string(bytes))
 	}
 	if errs := validation.ValidateBuild(cfg.build); len(errs) > 0 {
 		return nil, errors.NewInvalid(schema.GroupKind{Kind: "Build"}, cfg.build.Name, errs)
 	}
-	glog.V(4).Infof("Build: %#v", cfg.build)
 
 	masterVersion := os.Getenv(api.OriginVersion)
 	thisVersion := version.Get().String()

pkg/build/builder/dockerutil.go

@@ -16,10 +16,12 @@ import (
 	dockertypes "github.com/docker/engine-api/types"
 	docker "github.com/fsouza/go-dockerclient"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
-	"k8s.io/kubernetes/pkg/credentialprovider"
-	"k8s.io/kubernetes/pkg/util/interrupt"
 
 	"github.com/openshift/source-to-image/pkg/tar"
+	s2iutil "github.com/openshift/source-to-image/pkg/util"
+
+	"k8s.io/kubernetes/pkg/credentialprovider"
+	"k8s.io/kubernetes/pkg/util/interrupt"
 
 	"github.com/openshift/imagebuilder"
 	"github.com/openshift/imagebuilder/dockerclient"
@@ -281,7 +283,12 @@ func tagImage(dockerClient DockerClient, image, name string) error {
 // removed after it terminates.
 func dockerRun(client DockerClient, createOpts docker.CreateContainerOptions, attachOpts docker.AttachToContainerOptions) error {
 	// Create a new container.
-	glog.V(4).Infof("Creating container with options {Name:%q Config:%+v HostConfig:%+v} ...", createOpts.Name, createOpts.Config, createOpts.HostConfig)
+	// First strip any inlined proxy credentials from the *proxy* env variables,
+	// before logging the env variables.
+	if glog.Is(4) {
+		redactedOpts := SafeForLoggingDockerCreateOptions(&createOpts)
+		glog.V(4).Infof("Creating container with options {Name:%q Config:%+v HostConfig:%+v} ...", redactedOpts.Name, redactedOpts.Config, redactedOpts.HostConfig)
+	}
 	c, err := client.CreateContainer(createOpts)
 	if err != nil {
 		return fmt.Errorf("create container %q: %v", createOpts.Name, err)
@@ -483,3 +490,23 @@ func GetDockerClient() (client *docker.Client, endpoint string, err error) {
 	}
 	return
 }
+
+// SafeForLoggingDockerConfig returns a copy of a docker config struct
+// where any proxy credentials in the env section of the config
+// have been redacted.
+func SafeForLoggingDockerConfig(config *docker.Config) *docker.Config {
+	origEnv := config.Env
+	newConfig := *config
+	newConfig.Env = s2iutil.SafeForLoggingEnv(origEnv)
+	return &newConfig
+}
+
+// SafeForLoggingDockerCreateOptions returns a copy of a docker
+// create container options struct where any proxy credentials in the env section of
+// the config have been redacted.
+func SafeForLoggingDockerCreateOptions(opts *docker.CreateContainerOptions) *docker.CreateContainerOptions {
+	origConfig := opts.Config
+	newOpts := *opts
+	newOpts.Config = SafeForLoggingDockerConfig(origConfig)
+	return &newOpts
+}

pkg/build/builder/dockerutil_test.go

@@ -12,7 +12,7 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/fsouza/go-dockerclient"
+	docker "github.com/fsouza/go-dockerclient"
 )
 
 type FakeDocker struct {
@@ -522,3 +522,44 @@ func TestSimpleProgress(t *testing.T) {
 		}
 	}
 }
+
+var credsRegex = regexp.MustCompile("user:password")
+var redactedRegex = regexp.MustCompile("redacted")
+
+func TestSafeForLoggingDockerCreateOptions(t *testing.T) {
+	opts := &docker.CreateContainerOptions{
+		Config: &docker.Config{
+
+			Env: []string{
+				"http_proxy=http://user:[email protected]",
+				"ignore=http://user:[email protected]",
+			},
+		},
+	}
+	stripped := SafeForLoggingDockerCreateOptions(opts)
+	if credsRegex.MatchString(stripped.Config.Env[0]) {
+		t.Errorf("stripped proxy variable %s should not contain credentials", stripped.Config.Env[0])
+	}
+	if !redactedRegex.MatchString(stripped.Config.Env[0]) {
+		t.Errorf("stripped proxy variable %s should contain redacted", stripped.Config.Env[0])
+	}
+	if !credsRegex.MatchString(stripped.Config.Env[1]) {
+		t.Errorf("stripped other variable %s should contain credentials", stripped.Config.Env[1])
+	}
+	if redactedRegex.MatchString(stripped.Config.Env[1]) {
+		t.Errorf("stripped other variable %s should not contain redacted", stripped.Config.Env[1])
+	}
+
+	if !credsRegex.MatchString(opts.Config.Env[0]) {
+		t.Errorf("original proxy variable %s should contain credentials", opts.Config.Env[0])
+	}
+	if redactedRegex.MatchString(opts.Config.Env[0]) {
+		t.Errorf("original proxy variable %s should not contain redacted", opts.Config.Env[0])
+	}
+	if !credsRegex.MatchString(opts.Config.Env[1]) {
+		t.Errorf("original other variable %s should contain credentials", opts.Config.Env[1])
+	}
+	if redactedRegex.MatchString(opts.Config.Env[1]) {
+		t.Errorf("original other variable %s should not contain redacted", opts.Config.Env[1])
+	}
+}

pkg/build/builder/sti.go

@@ -23,6 +23,7 @@ import (
 	"github.com/openshift/origin/pkg/build/builder/cmd/dockercfg"
 	"github.com/openshift/origin/pkg/build/builder/timing"
 	"github.com/openshift/origin/pkg/build/controller/strategy"
+	"github.com/openshift/origin/pkg/build/util"
 	"github.com/openshift/origin/pkg/client"
 	"github.com/openshift/origin/pkg/generate/git"
 
@@ -192,8 +193,9 @@ func (s *S2IBuilder) Build() error {
 	}
 	if scriptDownloadProxyConfig != nil {
 		glog.V(0).Infof("Using HTTP proxy %v and HTTPS proxy %v for script download",
-			scriptDownloadProxyConfig.HTTPProxy,
-			scriptDownloadProxyConfig.HTTPSProxy)
+			util.SafeForLoggingURL(scriptDownloadProxyConfig.HTTPProxy),
+			util.SafeForLoggingURL(scriptDownloadProxyConfig.HTTPSProxy),
+		)
 	}
 
 	var incremental bool
@@ -283,7 +285,10 @@ func (s *S2IBuilder) Build() error {
 	if err != nil {
 		return err
 	}
-	glog.V(4).Infof("Creating a new S2I builder with build config: %#v\n", describe.Config(client, config))
+	if glog.Is(4) {
+		redactedConfig := util.SafeForLoggingS2IConfig(config)
+		glog.V(4).Infof("Creating a new S2I builder with config: %#v\n", describe.Config(client, redactedConfig))
+	}
 	builder, buildInfo, err := s.builder.Builder(config, s2ibuild.Overrides{Downloader: nil})
 	if err != nil {
 		s.build.Status.Phase = api.BuildPhaseFailed