httpcli: Move init function from conf package into httpcli (#62318)

There was a weird dependency between internal/conf and internal/httpcli because
of a cyclic import. This is resolved by using conftypes instead, so we can properly
split those concerns.

Test plan:

Existing tests don't break.
This commit is contained in:
Erik Seliger 2024-05-02 18:30:44 +02:00 committed by GitHub
parent a3bc85ec4c
commit 7a30830dc2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 81 additions and 79 deletions

View File

@ -47,6 +47,7 @@ go_library(
"//internal/database/connections/live",
"//internal/debugserver",
"//internal/env",
"//internal/httpcli",
"//internal/observation",
"//internal/oobmigration/migrations/register",
"//internal/service",

View File

@ -45,6 +45,7 @@ import (
"github.com/sourcegraph/sourcegraph/internal/conf/conftypes"
"github.com/sourcegraph/sourcegraph/internal/database"
connections "github.com/sourcegraph/sourcegraph/internal/database/connections/live"
"github.com/sourcegraph/sourcegraph/internal/httpcli"
"github.com/sourcegraph/sourcegraph/internal/observation"
"github.com/sourcegraph/sourcegraph/schema"
)
@ -138,7 +139,7 @@ func SwitchableSiteConfig() conftypes.WatchableSiteConfig {
go func() {
<-AutoUpgradeDone
conf.EnsureHTTPClientIsConfigured()
httpcli.Configure(confClient)
switchable.WatchableSiteConfig = confClient
for _, watcher := range switchable.watchers {
confClient.Watch(watcher)

View File

@ -33,7 +33,6 @@ go_library(
"//internal/dotcom",
"//internal/env",
"//internal/hashutil",
"//internal/httpcli",
"//internal/jsonc",
"//internal/license",
"//internal/src-cli",

View File

@ -1,12 +1,5 @@
package conf
import (
"reflect"
"sync"
"github.com/sourcegraph/sourcegraph/internal/httpcli"
)
// Init function completes the initialization process of the conf package, starting the configuration continuous changes polling
// if in client mode. The conf.Watch function can safely be called before calling Init to register callbacks reacting to the changes.
//
@ -18,38 +11,4 @@ func Init() {
go DefaultClient().continuouslyUpdate(nil)
close(configurationServerFrontendOnlyInitialized)
}
EnsureHTTPClientIsConfigured()
}
var ensureHTTPClientIsConfiguredOnce sync.Once
// EnsureHTTPClientIsConfigured configures the httpcli package settings. We have to do this
// in this package as conf itself uses httpcli's internal client.
func EnsureHTTPClientIsConfigured() {
ensureHTTPClientIsConfiguredOnce.Do(func() {
go Watch(func() {
// TLS external config
tlsBefore := httpcli.TLSExternalConfig()
tlsAfter := Get().ExperimentalFeatures.TlsExternal
if !reflect.DeepEqual(tlsBefore, tlsAfter) {
httpcli.SetTLSExternalConfig(tlsAfter)
}
// Outbound request log limit and redact headers
outboundRequestLogLimitBefore := httpcli.OutboundRequestLogLimit()
outboundRequestLogLimitAfter := int32(Get().OutboundRequestLogLimit)
if outboundRequestLogLimitBefore != outboundRequestLogLimitAfter {
httpcli.SetOutboundRequestLogLimit(outboundRequestLogLimitAfter)
}
redactOutboundRequestHeadersBefore := httpcli.RedactOutboundRequestHeaders()
redactOutboundRequestHeadersAfter := true
if Get().RedactOutboundRequestHeaders != nil {
redactOutboundRequestHeadersAfter = *Get().RedactOutboundRequestHeaders
}
if redactOutboundRequestHeadersBefore != redactOutboundRequestHeadersAfter {
httpcli.SetRedactOutboundRequestHeaders(redactOutboundRequestHeadersAfter)
}
})
})
}

View File

@ -7,6 +7,7 @@ go_library(
"client.go",
"doc.go",
"external.go",
"init.go",
"noop_response_cache.go",
"redis_logger_middleware.go",
"transport.go",
@ -15,6 +16,7 @@ go_library(
visibility = ["//:__subpackages__"],
deps = [
"//internal/actor",
"//internal/conf/conftypes",
"//internal/conf/deploy",
"//internal/env",
"//internal/hostmatcher",

View File

@ -22,49 +22,49 @@ type externalTransport struct {
effective *http.Transport
}
var tlsExternalConfig struct {
var tlsExternalConfigStore struct {
sync.RWMutex
*schema.TlsExternal
}
var outboundRequestLogLimit atomic.Int32
var redactOutboundRequestHeaders atomic.Bool
var outboundRequestLogLimitStore atomic.Int32
var redactOutboundRequestHeadersStore atomic.Bool
// SetTLSExternalConfig is called by the conf package whenever TLSExternalConfig changes.
// setTLSExternalConfig is called by the conf package whenever TLSExternalConfig changes.
// This is needed to avoid circular imports.
func SetTLSExternalConfig(c *schema.TlsExternal) {
tlsExternalConfig.Lock()
tlsExternalConfig.TlsExternal = c
tlsExternalConfig.Unlock()
func setTLSExternalConfig(c *schema.TlsExternal) {
tlsExternalConfigStore.Lock()
tlsExternalConfigStore.TlsExternal = c
tlsExternalConfigStore.Unlock()
}
// TLSExternalConfig returns the current value of the global TLS external config.
func TLSExternalConfig() *schema.TlsExternal {
tlsExternalConfig.RLock()
defer tlsExternalConfig.RUnlock()
return tlsExternalConfig.TlsExternal
// tlsExternalConfig returns the current value of the global TLS external config.
func tlsExternalConfig() *schema.TlsExternal {
tlsExternalConfigStore.RLock()
defer tlsExternalConfigStore.RUnlock()
return tlsExternalConfigStore.TlsExternal
}
// SetOutboundRequestLogLimit is called by the conf package whenever OutboundRequestLogLimit changes.
// setOutboundRequestLogLimit is called by the conf package whenever OutboundRequestLogLimit changes.
// This is needed to avoid circular imports.
func SetOutboundRequestLogLimit(i int32) {
outboundRequestLogLimit.Store(i)
func setOutboundRequestLogLimit(i int32) {
outboundRequestLogLimitStore.Store(i)
}
// OutboundRequestLogLimit returns the current value of the global OutboundRequestLogLimit value.
func OutboundRequestLogLimit() int32 {
return outboundRequestLogLimit.Load()
// outboundRequestLogLimit returns the current value of the global OutboundRequestLogLimit value.
func outboundRequestLogLimit() int32 {
return outboundRequestLogLimitStore.Load()
}
// SetRedactOutboundRequestHeaders is called by the conf package whenever the RedactOutboundRequestHeaders setting changes.
// setRedactOutboundRequestHeaders is called by the conf package whenever the RedactOutboundRequestHeaders setting changes.
// This is needed to avoid circular imports.
func SetRedactOutboundRequestHeaders(b bool) {
redactOutboundRequestHeaders.Store(b)
func setRedactOutboundRequestHeaders(b bool) {
redactOutboundRequestHeadersStore.Store(b)
}
// RedactOutboundRequestHeaders returns the current value of the global RedactOutboundRequestHeaders setting.
func RedactOutboundRequestHeaders() bool {
return redactOutboundRequestHeaders.Load()
// redactOutboundRequestHeaders returns the current value of the global redactOutboundRequestHeaders setting.
func redactOutboundRequestHeaders() bool {
return redactOutboundRequestHeadersStore.Load()
}
func (t *externalTransport) RoundTrip(r *http.Request) (*http.Response, error) {
@ -72,7 +72,7 @@ func (t *externalTransport) RoundTrip(r *http.Request) (*http.Response, error) {
config, effective := t.config, t.effective
t.mu.RUnlock()
if current := TLSExternalConfig(); current == nil {
if current := tlsExternalConfig(); current == nil {
return t.base.RoundTrip(r)
} else if !reflect.DeepEqual(config, current) {
effective = t.update(r.Context(), current)

36
internal/httpcli/init.go Normal file
View File

@ -0,0 +1,36 @@
package httpcli
import (
"reflect"
"github.com/sourcegraph/sourcegraph/internal/conf/conftypes"
)
// Configure configures the httpcli package settings with TLS and outbound request
// logging settings from the site configuration.
func Configure(cfg conftypes.WatchableSiteConfig) {
cfg.Watch(func() {
siteConfig := cfg.SiteConfig()
// TLS external config
tlsBefore := tlsExternalConfig()
tlsAfter := siteConfig.ExperimentalFeatures.TlsExternal
if !reflect.DeepEqual(tlsBefore, tlsAfter) {
setTLSExternalConfig(tlsAfter)
}
// Outbound request log limit and redact headers
outboundRequestLogLimitBefore := outboundRequestLogLimit()
outboundRequestLogLimitAfter := int32(siteConfig.OutboundRequestLogLimit)
if outboundRequestLogLimitBefore != outboundRequestLogLimitAfter {
setOutboundRequestLogLimit(outboundRequestLogLimitAfter)
}
redactOutboundRequestHeadersBefore := redactOutboundRequestHeaders()
redactOutboundRequestHeadersAfter := true
if siteConfig.RedactOutboundRequestHeaders != nil {
redactOutboundRequestHeadersAfter = *siteConfig.RedactOutboundRequestHeaders
}
if redactOutboundRequestHeadersBefore != redactOutboundRequestHeadersAfter {
setRedactOutboundRequestHeaders(redactOutboundRequestHeadersAfter)
}
})
}

View File

@ -13,6 +13,7 @@ import (
"time"
"github.com/sourcegraph/log"
"github.com/sourcegraph/sourcegraph/internal/conf/deploy"
"github.com/sourcegraph/sourcegraph/internal/rcache"
"github.com/sourcegraph/sourcegraph/internal/types"
@ -21,7 +22,7 @@ import (
// outboundRequestsRedisFIFOList is a FIFO redis cache to store the requests.
var outboundRequestsRedisFIFOList = rcache.NewFIFOListDynamic("outbound-requests", func() int {
return int(OutboundRequestLogLimit())
return int(outboundRequestLogLimit())
})
const sourcegraphPrefix = "github.com/sourcegraph/sourcegraph/"
@ -34,8 +35,8 @@ func redisLoggerMiddleware() Middleware {
resp, err := cli.Do(req)
duration := time.Since(start)
limit := OutboundRequestLogLimit()
shouldRedactSensitiveHeaders := !deploy.IsDev(deploy.Type()) || RedactOutboundRequestHeaders()
limit := outboundRequestLogLimit()
shouldRedactSensitiveHeaders := !deploy.IsDev(deploy.Type()) || redactOutboundRequestHeaders()
// Feature is turned off, do not log
if limit == 0 {
@ -126,7 +127,7 @@ func redisLoggerMiddleware() Middleware {
// GetOutboundRequestLogItems returns all outbound request log items after the given key,
// in ascending order, trimmed to maximum {limit} items. Example for `after`: "2021-01-01T00_00_00.000000".
func GetOutboundRequestLogItems(ctx context.Context, after string) ([]*types.OutboundRequestLogItem, error) {
var limit = int(OutboundRequestLogLimit())
var limit = int(outboundRequestLogLimit())
if limit == 0 {
return []*types.OutboundRequestLogItem{}, nil

View File

@ -88,7 +88,7 @@ func TestRedisLoggerMiddleware(t *testing.T) {
}
// Enable feature
setOutboundRequestLogLimit(t, 1)
mockOutboundRequestLogLimit(t, 1)
for _, tc := range testCases {
tc := tc
@ -138,7 +138,7 @@ func TestRedisLoggerMiddleware_multiple(t *testing.T) {
rcache.SetupForTest(t)
// Enable the feature
setOutboundRequestLogLimit(t, int32(limit))
mockOutboundRequestLogLimit(t, int32(limit))
// Build client with middleware
cli := redisLoggerMiddleware()(newFakeClient(http.StatusOK, []byte(`{"responseBody":true}`), nil))
@ -284,8 +284,8 @@ func TestRedisLoggerMiddleware_formatStackFrame(t *testing.T) {
}
}
func setOutboundRequestLogLimit(t *testing.T, limit int32) {
old := OutboundRequestLogLimit()
SetOutboundRequestLogLimit(limit)
t.Cleanup(func() { SetOutboundRequestLogLimit(old) })
func mockOutboundRequestLogLimit(t *testing.T, limit int32) {
old := outboundRequestLogLimit()
setOutboundRequestLogLimit(limit)
t.Cleanup(func() { setOutboundRequestLogLimit(old) })
}

View File

@ -10,6 +10,7 @@ go_library(
"//internal/debugserver",
"//internal/env",
"//internal/hostname",
"//internal/httpcli",
"//internal/logging",
"//internal/observation",
"//internal/profiler",

View File

@ -12,6 +12,7 @@ import (
"github.com/sourcegraph/sourcegraph/internal/debugserver"
"github.com/sourcegraph/sourcegraph/internal/env"
"github.com/sourcegraph/sourcegraph/internal/hostname"
"github.com/sourcegraph/sourcegraph/internal/httpcli"
"github.com/sourcegraph/sourcegraph/internal/logging"
"github.com/sourcegraph/sourcegraph/internal/observation"
"github.com/sourcegraph/sourcegraph/internal/profiler"
@ -95,6 +96,7 @@ func run(
Logging: conf.NewLogsSinksSource(conf.DefaultClient()),
Tracing: tracer.ConfConfigurationSource{WatchableSiteConfig: conf.DefaultClient()},
}
httpcli.Configure(conf.DefaultClient())
}
if oobConfig.Logging != nil {