diff --git a/go.mod b/go.mod index 6bf20c47..ca0d69cf 100644 --- a/go.mod +++ b/go.mod @@ -34,6 +34,7 @@ require ( github.com/fluxcd/pkg/git/gogit v0.35.1 github.com/fluxcd/pkg/gittestserver v0.17.0 github.com/fluxcd/pkg/helmtestserver v0.24.0 + github.com/fluxcd/pkg/http/transport v0.6.0 github.com/fluxcd/pkg/lockedfile v0.6.0 github.com/fluxcd/pkg/masktoken v0.7.0 github.com/fluxcd/pkg/oci v0.49.0 diff --git a/go.sum b/go.sum index 6fe36706..6338281a 100644 --- a/go.sum +++ b/go.sum @@ -385,6 +385,8 @@ github.com/fluxcd/pkg/gittestserver v0.17.0 h1:JlBvWZQTDOI+np5Z+084m3DkeAH1hMusE github.com/fluxcd/pkg/gittestserver v0.17.0/go.mod h1:E/40EmLoXcMqd6gLuLDC9F6KJxqHVGbBBeMNKk5XdxU= github.com/fluxcd/pkg/helmtestserver v0.24.0 h1:9sSfRG17GnDIup4sI8V+fdvKROtunU4JyIo34uvXq3Q= github.com/fluxcd/pkg/helmtestserver v0.24.0/go.mod h1:jMCCzTV9r3N+0kD8Uo09nbgQ1iTaw54LFKKMlztlBhs= +github.com/fluxcd/pkg/http/transport v0.6.0 h1:ryzy81tpNYWZ/qsDd3tLdO6Bfn1wYLI1zdbepPBY/mo= +github.com/fluxcd/pkg/http/transport v0.6.0/go.mod h1:95TBlrNsDdKMDCKvJnne2VC3SuZ5/JIj+r/yssaXz4w= github.com/fluxcd/pkg/lockedfile v0.6.0 h1:64RRMiPv3ZK9Y4sjI8c78kZAdfEo+Sjr2iP8a9pZeZo= github.com/fluxcd/pkg/lockedfile v0.6.0/go.mod h1:gpdUVm7+05NIT1ZvzuNnHfnT81OhZtIySlxxkZ68pXk= github.com/fluxcd/pkg/masktoken v0.7.0 h1:pitmyOg2pUVdW+nn2Lk/xqm2TaA08uxvOC0ns3sz6bM= diff --git a/internal/helm/repository/chart_repository.go b/internal/helm/repository/chart_repository.go index 9837224f..e8030ec7 100644 --- a/internal/helm/repository/chart_repository.go +++ b/internal/helm/repository/chart_repository.go @@ -40,9 +40,9 @@ import ( "github.com/fluxcd/pkg/version" + "github.com/fluxcd/pkg/http/transport" "github.com/fluxcd/source-controller/internal/helm" "github.com/fluxcd/source-controller/internal/oci" - "github.com/fluxcd/source-controller/internal/transport" ) var ( diff --git a/internal/helm/repository/oci_chart_repository.go b/internal/helm/repository/oci_chart_repository.go index c858beff..2bed964a 100644 --- a/internal/helm/repository/oci_chart_repository.go +++ b/internal/helm/repository/oci_chart_repository.go @@ -36,9 +36,9 @@ import ( "github.com/Masterminds/semver/v3" "github.com/google/go-containerregistry/pkg/name" + "github.com/fluxcd/pkg/http/transport" "github.com/fluxcd/pkg/version" "github.com/fluxcd/source-controller/internal/oci" - "github.com/fluxcd/source-controller/internal/transport" ) // RegistryClient is an interface for interacting with OCI registries diff --git a/internal/transport/transport.go b/internal/transport/transport.go deleted file mode 100644 index 89286df7..00000000 --- a/internal/transport/transport.go +++ /dev/null @@ -1,103 +0,0 @@ -/* -Copyright 2022 The Flux authors - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package transport - -import ( - "crypto/tls" - "fmt" - "net" - "net/http" - "sync" - "time" -) - -// TransportPool is a progressive and non-blocking pool -// for http.Transport objects, optimised for Gargabe Collection -// and without a hard limit on number of objects created. -// -// Its main purpose is to enable for transport objects to be -// used across helm chart download requests and helm/pkg/getter -// instances by leveraging the getter.WithTransport(t) construct. -// -// The use of this pool improves the default behaviour of helm getter -// which creates a new connection per request, or per getter instance, -// resulting on unnecessary TCP connections with the target. -// -// http.Transport objects may contain sensitive material and also have -// settings that may impact the security of HTTP operations using -// them (i.e. InsecureSkipVerify). Therefore, ensure that they are -// used in a thread-safe way, and also by reseting TLS specific state -// after each use. -// -// Calling the Release(t) function will reset TLS specific state whilst -// also releasing the transport back to the pool to be reused. -// -// xref: https://github.com/helm/helm/pull/10568 -// xref2: https://github.com/fluxcd/source-controller/issues/578 -type TransportPool struct { -} - -var pool = &sync.Pool{ - New: func() interface{} { - return &http.Transport{ - DisableCompression: true, - Proxy: http.ProxyFromEnvironment, - - // Due to the non blocking nature of this approach, - // at peak usage a higher number of transport objects - // may be created. sync.Pool will ensure they are - // gargage collected when/if needed. - // - // By setting a low value to IdleConnTimeout the connections - // will be closed after that period of inactivity, allowing the - // transport to be garbage collected. - IdleConnTimeout: 60 * time.Second, - - // use safe defaults based off http.DefaultTransport - DialContext: (&net.Dialer{ - Timeout: 30 * time.Second, - KeepAlive: 30 * time.Second, - }).DialContext, - TLSHandshakeTimeout: 10 * time.Second, - ExpectContinueTimeout: 1 * time.Second, - } - }, -} - -// NewOrIdle tries to return an existing transport that is not currently being used. -// If none is found, creates a new Transport instead. -// -// tlsConfig can optionally set the TLSClientConfig for the transport. -func NewOrIdle(tlsConfig *tls.Config) *http.Transport { - t := pool.Get().(*http.Transport) - t.TLSClientConfig = tlsConfig - - return t -} - -// Release releases the transport back to the TransportPool after -// sanitising its sensitive fields. -func Release(transport *http.Transport) error { - if transport == nil { - return fmt.Errorf("cannot release nil transport") - } - - transport.TLSClientConfig = nil - - pool.Put(transport) - return nil -} diff --git a/internal/transport/transport_test.go b/internal/transport/transport_test.go deleted file mode 100644 index f0bc387d..00000000 --- a/internal/transport/transport_test.go +++ /dev/null @@ -1,58 +0,0 @@ -/* -Copyright 2022 The Flux authors - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package transport - -import ( - "crypto/tls" - "testing" -) - -func Test_TransportReuse(t *testing.T) { - t1 := NewOrIdle(nil) - t2 := NewOrIdle(nil) - - if t1 == t2 { - t.Errorf("same transported returned twice") - } - - err := Release(t2) - if err != nil { - t.Errorf("error releasing transport t2: %v", err) - } - - t3 := NewOrIdle(&tls.Config{ - ServerName: "testing", - }) - if t3.TLSClientConfig == nil || t3.TLSClientConfig.ServerName != "testing" { - t.Errorf("TLSClientConfig not properly configured") - } - - err = Release(t3) - if err != nil { - t.Errorf("error releasing transport t3: %v", err) - } - if t3.TLSClientConfig != nil { - t.Errorf("TLSClientConfig not cleared after release") - } - - err = Release(nil) - if err == nil { - t.Errorf("should not allow release nil transport") - } else if err.Error() != "cannot release nil transport" { - t.Errorf("wanted error message: 'cannot release nil transport' got: %q", err.Error()) - } -}