Skip to content

Commit

Permalink
Fix small timeout in token acquisition (#127)
Browse files Browse the repository at this point in the history
  • Loading branch information
NetanelBollag authored Feb 5, 2023
1 parent d366541 commit f0161ca
Show file tree
Hide file tree
Showing 5 changed files with 91 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
otterizev1alpha2 "github.com/otterize/intents-operator/src/operator/api/v1alpha2"
"github.com/otterize/intents-operator/src/shared/injectablerecorder"
"github.com/samber/lo"
"github.com/sirupsen/logrus"
"k8s.io/apimachinery/pkg/runtime"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -43,8 +44,11 @@ func (r *OtterizeCloudReconciler) Reconcile(ctx context.Context, req reconcile.R
}

if err = r.otterizeClient.ReportAppliedIntents(ctx, lo.ToPtr(req.Namespace), intentsInput); err != nil {
logrus.WithError(err).Error("failed to report applied intents")
return ctrl.Result{}, err
}

logrus.Infof("successfully reported %d applied intents", len(clientIntentsList.Items))

return ctrl.Result{}, nil
}
10 changes: 3 additions & 7 deletions src/operator/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package main
import (
"context"
"github.com/bombsimon/logrusr/v3"
otterizev1alpha2 "github.com/otterize/intents-operator/src/operator/api/v1alpha2"
"github.com/otterize/intents-operator/src/operator/controllers"
"github.com/otterize/intents-operator/src/operator/controllers/external_traffic"
"github.com/otterize/intents-operator/src/operator/controllers/intents_reconcilers/otterizecloud"
Expand All @@ -29,9 +30,6 @@ import (
"github.com/spf13/pflag"
"os"
"sigs.k8s.io/controller-runtime/pkg/cache"
"time"

otterizev1alpha2 "github.com/otterize/intents-operator/src/operator/api/v1alpha2"
// Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.)
// to ensure that exec-entrypoint and run can make use of them.
_ "k8s.io/client-go/plugin/pkg/client/auth"
Expand Down Expand Up @@ -161,14 +159,12 @@ func main() {
}

signalHandlerCtx := ctrl.SetupSignalHandler()
timeoutCtx, cancelFunc := context.WithTimeout(signalHandlerCtx, 10*time.Second)
defer cancelFunc()
otterizeCloudClient, connectedToCloud, err := otterizecloud.NewClient(timeoutCtx)
otterizeCloudClient, connectedToCloud, err := otterizecloud.NewClient(signalHandlerCtx)
if err != nil {
logrus.WithError(err).Error("Failed to initialize Otterize Cloud client")
}
if connectedToCloud {
err := otterizeCloudClient.ReportIntentsOperatorConfiguration(timeoutCtx, graphqlclient.IntentsOperatorConfigurationInput{
err := otterizeCloudClient.ReportIntentsOperatorConfiguration(signalHandlerCtx, graphqlclient.IntentsOperatorConfigurationInput{
GlobalEnforcementEnabled: enforcementEnabledGlobally,
NetworkPolicyEnforcementEnabled: enforcementEnabledGlobally && enableNetworkPolicyCreation,
KafkaACLEnforcementEnabled: enforcementEnabledGlobally && enableKafkaACLCreation,
Expand Down
19 changes: 17 additions & 2 deletions src/shared/otterizecloud/otterizecloudclient/cloud_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,14 @@ import (
"github.com/spf13/viper"
"golang.org/x/oauth2"
"golang.org/x/oauth2/clientcredentials"
"net/http"
)

func NewClient(ctx context.Context) (graphql.Client, bool, error) {
clientID := viper.GetString(ApiClientIdKey)
secret := viper.GetString(ApiClientSecretKey)
apiAddress := viper.GetString(OtterizeAPIAddressKey)
clientTimeout := viper.GetDuration(CloudClientTimeoutKey)
if clientID == "" && secret == "" {
return nil, false, nil
}
Expand All @@ -31,9 +33,22 @@ func NewClient(ctx context.Context) (graphql.Client, bool, error) {
AuthStyle: oauth2.AuthStyleInParams,
}

tokenSrc := cfg.TokenSource(ctx)
// Timeout for oauth token acquisition is set by http client passed to the token source context
// See example 'Example (CustomHTTP)' in https://pkg.go.dev/golang.org/x/oauth2
clientWithTimeout := &http.Client{Timeout: clientTimeout}
ctxWithClient := context.WithValue(ctx, oauth2.HTTPClient, clientWithTimeout)

tokenSrc := cfg.TokenSource(ctxWithClient)
graphqlUrl := fmt.Sprintf("%s/graphql/v1beta", apiAddress)
httpClient := oauth2.NewClient(ctx, tokenSrc)
httpClient := oauth2.NewClient(ctxWithClient, tokenSrc)

// Timeout in context isn't used in the client itself
// as mentioned in https://pkg.go.dev/golang.org/x/oauth2#NewClient:
//
// "Note that if a custom *http.Client is provided via the
// Context it is used only for token acquisition and is not
// used to configure the *http.Client returned from NewClient"
httpClient.Timeout = clientTimeout

return graphql.NewClient(graphqlUrl, httpClient), true, nil
}
64 changes: 64 additions & 0 deletions src/shared/otterizecloud/otterizecloudclient/cloud_client_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package otterizecloudclient

import (
"context"
"github.com/spf13/viper"
"github.com/stretchr/testify/suite"
"testing"
)

type CloudClientTestSuite struct {
suite.Suite
}

func (s *CloudClientTestSuite) SetupTest() {
viper.Reset()
}

func (s *CloudClientTestSuite) TearDownTest() {
viper.Reset()
}

func (s *CloudClientTestSuite) TestNewClientSuccess() {
viper.Set(ApiClientIdKey, "cli_id1234")
viper.Set(ApiClientSecretKey, "secret1234")
viper.Set(OtterizeAPIAddressKey, "https://testapp.otterize.com/api")
viper.Set(CloudClientTimeoutKey, "30s")

ctx := context.Background()
client, ok, err := NewClient(ctx)
s.Nil(err)
s.True(ok)
s.NotNil(client)
}

func (s *CloudClientTestSuite) TestNoCredentialsNoClient() {
viper.Set(ApiClientIdKey, "")
viper.Set(ApiClientSecretKey, "")

ctx := context.Background()
client, ok, err := NewClient(ctx)
s.Nil(err)
s.False(ok)
s.Nil(client)
}

func (s *CloudClientTestSuite) TestPartialCredentialsReturnError() {
viper.Set(ApiClientIdKey, "cli_id1234")
viper.Set(ApiClientSecretKey, "")

ctx := context.Background()
_, _, err := NewClient(ctx)
s.NotNil(err)

viper.Set(ApiClientIdKey, "")
viper.Set(ApiClientSecretKey, "secret1234")

ctx = context.Background()
_, _, err = NewClient(ctx)
s.NotNil(err)
}

func TestNewClientTestSuite(t *testing.T) {
suite.Run(t, &CloudClientTestSuite{})
}
3 changes: 3 additions & 0 deletions src/shared/otterizecloud/otterizecloudclient/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ const (
ApiClientIdKey = "client-id"
ApiClientSecretKey = "client-secret"
OtterizeAPIAddressKey = "api-address"
CloudClientTimeoutKey = "cloud-client-timeout"
CloudClientTimeoutDefault = "30s"
OtterizeAPIAddressDefault = "https://app.otterize.com/api"
ComponentReportIntervalKey = "component-report-interval"
ComponentReportIntervalDefault = 60
Expand All @@ -18,6 +20,7 @@ const (
func init() {
viper.SetDefault(OtterizeAPIAddressKey, OtterizeAPIAddressDefault)
viper.SetDefault(ComponentReportIntervalKey, ComponentReportIntervalDefault)
viper.SetDefault(CloudClientTimeoutKey, CloudClientTimeoutDefault)
viper.SetEnvPrefix(EnvPrefix)
viper.SetEnvKeyReplacer(strings.NewReplacer("-", "_"))
viper.AutomaticEnv()
Expand Down

0 comments on commit f0161ca

Please sign in to comment.