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

[chore] Enable goleak oidcauthextension #35282

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
42 changes: 30 additions & 12 deletions extension/oidcauthextension/extension.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,11 @@ import (
type oidcExtension struct {
cfg *Config

provider *oidc.Provider
verifier *oidc.IDTokenVerifier

logger *zap.Logger
provider *oidc.Provider
verifier *oidc.IDTokenVerifier
client *http.Client
logger *zap.Logger
transport *http.Transport
}

var (
Expand All @@ -53,19 +54,35 @@ func newExtension(cfg *Config, logger *zap.Logger) auth.Server {
cfg: cfg,
logger: logger,
}
return auth.NewServer(auth.WithServerStart(oe.start), auth.WithServerAuthenticate(oe.authenticate))
return auth.NewServer(
auth.WithServerStart(oe.start),
auth.WithServerAuthenticate(oe.authenticate),
auth.WithServerShutdown(oe.shutdown),
)
}

func (e *oidcExtension) start(context.Context, component.Host) error {
provider, err := getProviderForConfig(e.cfg)
func (e *oidcExtension) start(ctx context.Context, _ component.Host) error {
provider, client, transport, err := getProviderForConfig(ctx, e.cfg)
// In case of failure, the client and transport need to be there for the shutdown method.
e.transport = transport
e.client = client
if err != nil {
return fmt.Errorf("failed to get configuration from the auth server: %w", err)
}
e.provider = provider

e.verifier = e.provider.Verifier(&oidc.Config{
ClientID: e.cfg.Audience,
})
return nil
}

func (e *oidcExtension) shutdown(context.Context) error {
if e.client != nil {
e.client.CloseIdleConnections()
}
if e.transport != nil {
e.transport.CloseIdleConnections()
}

return nil
}
Expand Down Expand Up @@ -167,7 +184,7 @@ func getGroupsFromClaims(claims map[string]any, groupsClaim string) ([]string, e
return []string{}, nil
}

func getProviderForConfig(config *Config) (*oidc.Provider, error) {
func getProviderForConfig(ctx context.Context, config *Config) (*oidc.Provider, *http.Client, *http.Transport, error) {
t := &http.Transport{
Proxy: http.ProxyFromEnvironment,
DialContext: (&net.Dialer{
Expand All @@ -184,7 +201,7 @@ func getProviderForConfig(config *Config) (*oidc.Provider, error) {

cert, err := getIssuerCACertFromPath(config.IssuerCAPath)
if err != nil {
return nil, err // the errors from this path have enough context already
return nil, nil, nil, err // the errors from this path have enough context already
}

if cert != nil {
Expand All @@ -198,8 +215,9 @@ func getProviderForConfig(config *Config) (*oidc.Provider, error) {
Timeout: 5 * time.Second,
Transport: t,
}
oidcContext := oidc.ClientContext(context.Background(), client)
return oidc.NewProvider(oidcContext, config.IssuerURL)
oidcContext := oidc.ClientContext(ctx, client)
provider, err := oidc.NewProvider(oidcContext, config.IssuerURL)
return provider, client, t, err
}

func getIssuerCACertFromPath(path string) (*x509.Certificate, error) {
Expand Down
11 changes: 9 additions & 2 deletions extension/oidcauthextension/extension_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,11 +112,13 @@ func TestOIDCProviderForConfigWithTLS(t *testing.T) {
}

// test
provider, err := getProviderForConfig(config)
provider, client, transport, err := getProviderForConfig(context.Background(), config)

// verify
assert.NoError(t, err)
assert.NotNil(t, provider)
assert.NotNil(t, client)
assert.NotNil(t, transport)
}

func TestOIDCLoadIssuerCAFromPath(t *testing.T) {
Expand Down Expand Up @@ -185,11 +187,13 @@ func TestOIDCFailedToLoadIssuerCAFromPathInvalidContent(t *testing.T) {
}

// test
provider, err := getProviderForConfig(config) // cross test with getIssuerCACertFromPath
provider, client, transport, err := getProviderForConfig(context.Background(), config) // cross test with getIssuerCACertFromPath

// verify
assert.Error(t, err)
assert.Nil(t, provider)
assert.Nil(t, client)
assert.Nil(t, transport)
}

func TestOIDCInvalidAuthHeader(t *testing.T) {
Expand Down Expand Up @@ -234,6 +238,9 @@ func TestProviderNotReacheable(t *testing.T) {

// verify
assert.Error(t, err)

err = p.Shutdown(context.Background())
assert.NoError(t, err)
}

func TestFailedToVerifyToken(t *testing.T) {
Expand Down
5 changes: 2 additions & 3 deletions extension/oidcauthextension/generated_package_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions extension/oidcauthextension/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ require (
go.opentelemetry.io/collector/confmap v1.15.1-0.20240920203249-d17559b6e89a
go.opentelemetry.io/collector/extension v0.109.1-0.20240920203249-d17559b6e89a
go.opentelemetry.io/collector/extension/auth v0.109.1-0.20240920203249-d17559b6e89a
go.uber.org/goleak v1.3.0
go.uber.org/zap v1.27.0
)

Expand Down
2 changes: 0 additions & 2 deletions extension/oidcauthextension/metadata.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,3 @@ status:
tests:
config:
skip_lifecycle: true
goleak:
skip: true
Loading