Skip to content

Commit 9633304

Browse files
committed
fix(proxy): fix data race in DialContext
1 parent 7bd466a commit 9633304

File tree

2 files changed

+19
-13
lines changed

2 files changed

+19
-13
lines changed

Makefile

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -940,6 +940,14 @@ test-cover: ## Run unit and integration tests and generate a coverage report
940940

941941
.PHONY: test-infrastructure
942942
test-infrastructure: $(SETUP_ENVTEST) ## Run unit and integration tests with race detector for docker infrastructure provider
943+
# Note: Fuzz tests are not executed with race detector because they would just time out.
944+
# To achieve that, all files with fuzz tests have the "!race" build tag, to still run fuzz tests
945+
# we have an additional `go test` run that focuses on "TestFuzzyConversion".
946+
cd test/infrastructure; KUBEBUILDER_ASSETS="$(KUBEBUILDER_ASSETS)" go test -race ./... $(TEST_ARGS)
947+
$(MAKE) test-infrastructure-conversions TEST_ARGS="$(TEST_ARGS)"
948+
949+
.PHONY: test-infrastructure-no-race
950+
test-infrastructure-no-race: $(SETUP_ENVTEST) ## Run unit and integration tests with no race detector for docker infrastructure provider
943951
# Note: Fuzz tests are not executed with race detector because they would just time out.
944952
# To achieve that, all files with fuzz tests have the "!race" build tag, to still run fuzz tests
945953
# we have an additional `go test` run that focuses on "TestFuzzyConversion".
@@ -956,7 +964,7 @@ test-infrastructure-verbose: ## Run unit and integration tests with race detecto
956964

957965
.PHONY: test-infrastructure-junit
958966
test-infrastructure-junit: $(SETUP_ENVTEST) $(GOTESTSUM) ## Run unit and integration tests with race detector and generate a junit report for docker infrastructure provider
959-
cd test/infrastructure; set +o errexit; (KUBEBUILDER_ASSETS="$(KUBEBUILDER_ASSETS)" go test -json ./... $(TEST_ARGS); echo $$? > $(ARTIFACTS)/junit.infra_docker.exitcode) | tee $(ARTIFACTS)/junit.infra_docker.stdout
967+
cd test/infrastructure; set +o errexit; (KUBEBUILDER_ASSETS="$(KUBEBUILDER_ASSETS)" go test -race -json ./... $(TEST_ARGS); echo $$? > $(ARTIFACTS)/junit.infra_docker.exitcode) | tee $(ARTIFACTS)/junit.infra_docker.stdout
960968
$(GOTESTSUM) --junitfile $(ARTIFACTS)/junit.infra_docker.xml --raw-command cat $(ARTIFACTS)/junit.infra_docker.stdout
961969
exit $$(cat $(ARTIFACTS)/junit.infra_docker.exitcode)
962970
cd test/infrastructure; set +o errexit; (KUBEBUILDER_ASSETS="$(KUBEBUILDER_ASSETS)" go test -run "^TestFuzzyConversion$$" -json ./... $(TEST_ARGS); echo $$? > $(ARTIFACTS)/junit-fuzz.infra_docker.exitcode) | tee $(ARTIFACTS)/junit-fuzz.infra_docker.stdout

test/infrastructure/inmemory/pkg/server/proxy/dial.go

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,9 @@ const defaultTimeout = 10 * time.Second
3535

3636
// Dialer creates connections using Kubernetes API Server port-forwarding.
3737
type Dialer struct {
38-
proxy Proxy
39-
clientset *kubernetes.Clientset
40-
proxyTransport http.RoundTripper
41-
upgrader spdy.Upgrader
42-
timeout time.Duration
38+
proxy Proxy
39+
clientset *kubernetes.Clientset
40+
timeout time.Duration
4341
}
4442

4543
// NewDialer creates a new dialer for a given API server scope.
@@ -67,12 +65,6 @@ func NewDialer(p Proxy, options ...func(*Dialer) error) (*Dialer, error) {
6765
if err != nil {
6866
return nil, err
6967
}
70-
proxyTransport, upgrader, err := spdy.RoundTripperFor(p.KubeConfig)
71-
if err != nil {
72-
return nil, err
73-
}
74-
dialer.proxyTransport = proxyTransport
75-
dialer.upgrader = upgrader
7668
dialer.clientset = clientset
7769
return dialer, nil
7870
}
@@ -85,14 +77,20 @@ func (d *Dialer) DialContextWithAddr(ctx context.Context, addr string) (net.Conn
8577
// DialContext creates proxied port-forwarded connections.
8678
// ctx is currently unused, but fulfils the type signature used by GRPC.
8779
func (d *Dialer) DialContext(_ context.Context, _ string, addr string) (net.Conn, error) {
80+
proxyTransport, upgrader, err := spdy.RoundTripperFor(d.proxy.KubeConfig)
81+
if err != nil {
82+
return nil, err
83+
}
84+
httpClient := &http.Client{Transport: proxyTransport}
85+
8886
req := d.clientset.CoreV1().RESTClient().
8987
Post().
9088
Resource(d.proxy.Kind).
9189
Namespace(d.proxy.Namespace).
9290
Name(addr).
9391
SubResource("portforward")
9492

95-
dialer := spdy.NewDialer(d.upgrader, &http.Client{Transport: d.proxyTransport}, "POST", req.URL())
93+
dialer := spdy.NewDialer(upgrader, httpClient, "POST", req.URL())
9694

9795
// Create a new connection from the dialer.
9896
//

0 commit comments

Comments
 (0)