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.

Sign up for GitHub

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

CLOUDP-280012: Add http dry run transport for Atlas #2026

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+371 −82
Open

CLOUDP-280012: Add http dry run transport for Atlas #2026

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d73b459
add dryrun transport
s-urbaniak Jan 7, 2025
65c8493
add dry run recorder to atlas provider
s-urbaniak Jan 8, 2025
3b28ee9
add object to workflow context
s-urbaniak Jan 8, 2025
fbd1adf
wire dry running in controllers
s-urbaniak Jan 8, 2025
b4bf714
fix unit tests expecting wrong context
s-urbaniak Jan 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 40 additions & 24 deletions internal/controller/atlas/provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,18 @@ import (
"runtime"
"strings"

"go.mongodb.org/atlas-sdk/v20231115008/admin"
"github.com/mongodb-forks/digest"
adminv20231115008 "go.mongodb.org/atlas-sdk/v20231115008/admin"
josvazg marked this conversation as resolved.
Show resolved Hide resolved
adminv20241113001 "go.mongodb.org/atlas-sdk/v20241113001/admin"
"go.mongodb.org/atlas/mongodbatlas"
"go.uber.org/zap"
corev1 "k8s.io/api/core/v1"
"k8s.io/client-go/tools/record"
"sigs.k8s.io/controller-runtime/pkg/client"

//v20241113001
"github.com/mongodb/mongodb-atlas-kubernetes/v2/api"
akov2 "github.com/mongodb/mongodb-atlas-kubernetes/v2/api/v1"
"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/dryrun"
"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/httputil"
"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/version"
)
Expand All @@ -31,21 +33,22 @@ const (

type Provider interface {
Client(ctx context.Context, secretRef *client.ObjectKey, log *zap.SugaredLogger) (*mongodbatlas.Client, string, error)
SdkClient(ctx context.Context, secretRef *client.ObjectKey, log *zap.SugaredLogger) (*admin.APIClient, string, error)
SdkClient(ctx context.Context, secretRef *client.ObjectKey, log *zap.SugaredLogger) (*adminv20231115008.APIClient, string, error)
SdkClientSet(ctx context.Context, secretRef *client.ObjectKey, log *zap.SugaredLogger) (*ClientSet, string, error)
IsCloudGov() bool
IsResourceSupported(resource api.AtlasCustomResource) bool
}

type ClientSet struct {
//SdkClient20231115008 *admin.APIClient
SdkClient20231115008 *adminv20231115008.APIClient
SdkClient20241113001 *adminv20241113001.APIClient
}

type ProductionProvider struct {
k8sClient client.Client
domain string
globalSecretRef client.ObjectKey
dryRunRecorder record.EventRecorder
}

type credentialsSecret struct {
Expand All @@ -54,11 +57,12 @@ type credentialsSecret struct {
PrivateKey string
}

func NewProductionProvider(atlasDomain string, globalSecretRef client.ObjectKey, k8sClient client.Client) *ProductionProvider {
func NewProductionProvider(atlasDomain string, globalSecretRef client.ObjectKey, k8sClient client.Client, dryRunRecorder record.EventRecorder) *ProductionProvider {
return &ProductionProvider{
k8sClient: k8sClient,
domain: atlasDomain,
globalSecretRef: globalSecretRef,
dryRunRecorder: dryRunRecorder,
}
}

Expand Down Expand Up @@ -111,7 +115,9 @@ func (p *ProductionProvider) Client(ctx context.Context, secretRef *client.Objec
httputil.Digest(secretData.PublicKey, secretData.PrivateKey),
httputil.LoggingTransport(log),
}
httpClient, err := httputil.DecorateClient(&http.Client{Transport: http.DefaultTransport}, clientCfg...)

transport := p.newDryRunTransport(http.DefaultTransport)
httpClient, err := httputil.DecorateClient(&http.Client{Transport: transport}, clientCfg...)
if err != nil {
return nil, "", err
}
Expand All @@ -121,48 +127,58 @@ func (p *ProductionProvider) Client(ctx context.Context, secretRef *client.Objec
return c, secretData.OrgID, err
}

func (p *ProductionProvider) SdkClient(ctx context.Context, secretRef *client.ObjectKey, log *zap.SugaredLogger) (*admin.APIClient, string, error) {
secretData, err := getSecrets(ctx, p.k8sClient, secretRef, &p.globalSecretRef)
func (p *ProductionProvider) SdkClient(ctx context.Context, secretRef *client.ObjectKey, log *zap.SugaredLogger) (*adminv20231115008.APIClient, string, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: is it worth mentioning why this is a special case? or maybe even marking this method as deprecated to encourage always using SdkClientSet in future?

clientSet, orgID, err := p.SdkClientSet(ctx, secretRef, log)
if err != nil {
return nil, "", err
}

// TODO review we need add a custom logger to http client
//httpClientWithCustomLogger := http.DefaultClient
//err = httputil.LoggingTransport(log)(http.DefaultClient)
//if err != nil {
// return nil, "", err
//}
// Special case: SdkClient only returns the v20231115008 client.
return clientSet.SdkClient20231115008, orgID, nil
}

c, err := NewClient(p.domain, secretData.PublicKey, secretData.PrivateKey)
func (p *ProductionProvider) SdkClientSet(ctx context.Context, secretRef *client.ObjectKey, log *zap.SugaredLogger) (*ClientSet, string, error) {
secretData, err := getSecrets(ctx, p.k8sClient, secretRef, &p.globalSecretRef)
if err != nil {
return nil, "", err
}

return c, secretData.OrgID, nil
}
var transport http.RoundTripper = digest.NewTransport(secretData.PublicKey, secretData.PrivateKey)
transport = p.newDryRunTransport(transport)
transport = httputil.NewLoggingTransport(log, false, transport)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean we will be logging all of the SDK Atlas API calls? This will be lovely for debugging


func (p *ProductionProvider) SdkClientSet(ctx context.Context, secretRef *client.ObjectKey, log *zap.SugaredLogger) (*ClientSet, string, error) {
secretData, err := getSecrets(ctx, p.k8sClient, secretRef, &p.globalSecretRef)
httpClient := &http.Client{Transport: transport}

clientv20231115008, err := adminv20231115008.NewClient(
adminv20231115008.UseBaseURL(p.domain),
adminv20231115008.UseHTTPClient(httpClient),
adminv20231115008.UseUserAgent(operatorUserAgent()))
if err != nil {
return nil, "", err
}

// Instead of constantly extending the interface above, consider grouping all SDK Clients here
// New SDK (v20241113001) SDK
c2024, err := adminv20241113001.NewClient(
clientv20241113001, err := adminv20241113001.NewClient(
adminv20241113001.UseBaseURL(p.domain),
adminv20241113001.UseDigestAuth(secretData.PublicKey, secretData.PrivateKey),
adminv20241113001.UseHTTPClient(httpClient),
adminv20241113001.UseUserAgent(operatorUserAgent()))
if err != nil {
return nil, "", err
}

return &ClientSet{
SdkClient20241113001: c2024,
SdkClient20231115008: clientv20231115008,
SdkClient20241113001: clientv20241113001,
}, secretData.OrgID, nil
}

func (p *ProductionProvider) newDryRunTransport(delegate http.RoundTripper) http.RoundTripper {
if p.dryRunRecorder == nil {
return delegate
}

return dryrun.NewDryRunTransport(p.dryRunRecorder, delegate)
}

func getSecrets(ctx context.Context, k8sClient client.Client, secretRef, fallbackRef *client.ObjectKey) (*credentialsSecret, error) {
if secretRef == nil {
secretRef = fallbackRef
Expand Down
12 changes: 6 additions & 6 deletions internal/controller/atlas/provider_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func TestProvider_Client(t *testing.T) {
Build()

t.Run("should return Atlas API client and organization id using global secret", func(t *testing.T) {
p := NewProductionProvider("https://cloud.mongodb.com/", client.ObjectKey{Name: "api-secret", Namespace: "default"}, k8sClient)
p := NewProductionProvider("https://cloud.mongodb.com/", client.ObjectKey{Name: "api-secret", Namespace: "default"}, k8sClient, nil)

c, id, err := p.Client(context.Background(), nil, zaptest.NewLogger(t).Sugar())
assert.NoError(t, err)
Expand All @@ -52,7 +52,7 @@ func TestProvider_Client(t *testing.T) {
})

t.Run("should return Atlas API client and organization id using connection secret", func(t *testing.T) {
p := NewProductionProvider("https://cloud.mongodb.com/", client.ObjectKey{Name: "global-secret", Namespace: "default"}, k8sClient)
p := NewProductionProvider("https://cloud.mongodb.com/", client.ObjectKey{Name: "global-secret", Namespace: "default"}, k8sClient, nil)

c, id, err := p.Client(context.Background(), &client.ObjectKey{Name: "api-secret", Namespace: "default"}, zaptest.NewLogger(t).Sugar())
assert.NoError(t, err)
Expand All @@ -63,17 +63,17 @@ func TestProvider_Client(t *testing.T) {

func TestProvider_IsCloudGov(t *testing.T) {
t.Run("should return false for invalid domain", func(t *testing.T) {
p := NewProductionProvider("http://x:namedport", client.ObjectKey{}, nil)
p := NewProductionProvider("http://x:namedport", client.ObjectKey{}, nil, nil)
assert.False(t, p.IsCloudGov())
})

t.Run("should return false for commercial Atlas domain", func(t *testing.T) {
p := NewProductionProvider("https://cloud.mongodb.com/", client.ObjectKey{}, nil)
p := NewProductionProvider("https://cloud.mongodb.com/", client.ObjectKey{}, nil, nil)
assert.False(t, p.IsCloudGov())
})

t.Run("should return true for Atlas for government domain", func(t *testing.T) {
p := NewProductionProvider("https://cloud.mongodbgov.com/", client.ObjectKey{}, nil)
p := NewProductionProvider("https://cloud.mongodbgov.com/", client.ObjectKey{}, nil, nil)
assert.True(t, p.IsCloudGov())
})
}
Expand Down Expand Up @@ -164,7 +164,7 @@ func TestProvider_IsResourceSupported(t *testing.T) {

for desc, data := range dataProvider {
t.Run(desc, func(t *testing.T) {
p := NewProductionProvider(data.domain, client.ObjectKey{}, nil)
p := NewProductionProvider(data.domain, client.ObjectKey{}, nil, nil)
assert.Equal(t, data.expectation, p.IsResourceSupported(data.resource))
})
}
Expand Down
8 changes: 8 additions & 0 deletions internal/httputil/loggedclient.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,14 @@ func LoggingTransport(log *zap.SugaredLogger) ClientOpt {
}
}

func NewLoggingTransport(log *zap.SugaredLogger, logBody bool, delegate http.RoundTripper) http.RoundTripper {
return &loggedRoundTripper{
rt: delegate,
log: log,
logBody: logBody,
}
}

type loggedRoundTripper struct {
rt http.RoundTripper
log *zap.SugaredLogger
Expand Down
2 changes: 1 addition & 1 deletion internal/operator/builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ func (b *Builder) Build(ctx context.Context) (manager.Manager, error) {
}

if b.atlasProvider == nil {
b.atlasProvider = atlas.NewProductionProvider(b.atlasDomain, b.apiSecret, mgr.GetClient())
b.atlasProvider = atlas.NewProductionProvider(b.atlasDomain, b.apiSecret, mgr.GetClient(), nil)
}

projectReconciler := atlasproject.NewAtlasProjectReconciler(
Expand Down