From 0e5201b6f714742a8f1064b752a6186356058799 Mon Sep 17 00:00:00 2001 From: eweziyi Date: Tue, 21 Jan 2025 16:56:27 +0800 Subject: [PATCH 1/2] Fix treatment service and plugin's method of initiating google client --- plugins/turing/manager/experiment_manager.go | 13 +++++-- treatment-service/models/storage.go | 39 +++++++++++--------- 2 files changed, 30 insertions(+), 22 deletions(-) diff --git a/plugins/turing/manager/experiment_manager.go b/plugins/turing/manager/experiment_manager.go index 9c93c2f..68351b0 100644 --- a/plugins/turing/manager/experiment_manager.go +++ b/plugins/turing/manager/experiment_manager.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "net/http" "strconv" "time" @@ -167,15 +168,19 @@ func NewExperimentManager(configData json.RawMessage) (manager.CustomExperimentM } // Create Google Client + httpClient := http.DefaultClient googleClient, err := auth.InitGoogleClient(context.Background()) - if err != nil { - return nil, err + if err == nil { + googleClient.Timeout = defaultRequestTimeout + httpClient = googleClient + } else { + log.Infof("Google default credential not found. Fallback to HTTP default client") } - googleClient.Timeout = defaultRequestTimeout + // Create XP client client, err := xpclient.NewClientWithResponses( config.BaseURL, - xpclient.WithHTTPClient(googleClient), + xpclient.WithHTTPClient(httpClient), ) if err != nil { return nil, fmt.Errorf("Unable to create XP management client: %s", err.Error()) diff --git a/treatment-service/models/storage.go b/treatment-service/models/storage.go index 52d0529..acaf713 100644 --- a/treatment-service/models/storage.go +++ b/treatment-service/models/storage.go @@ -566,26 +566,29 @@ func NewLocalStorage( ) (*LocalStorage, error) { // Set up Request Modifiers clientOptions := []managementClient.ClientOption{} - if authzEnabled { - var googleClient *http.Client - var err error - // Init Google client for Authz. When using a non-empty googleApplicationCredentialsEnvVar that contains a file - // path to a credentials file, the credentials file MUST contain a Google SERVICE ACCOUNT for authentication to - // work correctly - if filepath := os.Getenv(googleApplicationCredentialsEnvVar); filepath != "" { - googleClient, err = auth.InitGoogleClientFromCredentialsFile(context.Background(), filepath) - } else { - googleClient, err = auth.InitGoogleClient(context.Background()) - } - if err != nil { - return nil, err - } - clientOptions = append( - clientOptions, - managementClient.WithHTTPClient(googleClient), - ) + httpClient := http.DefaultClient + var googleClient *http.Client + var err error + // Init Google client for Authz. When using a non-empty googleApplicationCredentialsEnvVar that contains a file + // path to a credentials file, the credentials file MUST contain a Google SERVICE ACCOUNT for authentication to + // work correctly + if filepath := os.Getenv(googleApplicationCredentialsEnvVar); filepath != "" { + googleClient, err = auth.InitGoogleClientFromCredentialsFile(context.Background(), filepath) + } else { + googleClient, err = auth.InitGoogleClient(context.Background()) } + + if err == nil { + httpClient = googleClient + } else { + log.Println("Google default credential not found. Fallback to HTTP default client") + } + + clientOptions = append( + clientOptions, + managementClient.WithHTTPClient(httpClient), + ) xpClient, err := managementClient.NewClientWithResponses(xpServer, clientOptions...) if err != nil { return nil, err From 753c92eea55cfcb2b6cb6af88f14ee2037cb2eeb Mon Sep 17 00:00:00 2001 From: eweziyi Date: Tue, 21 Jan 2025 18:03:07 +0800 Subject: [PATCH 2/2] Remove redundant argument --- treatment-service/appcontext/appcontext.go | 1 - treatment-service/appcontext/appcontext_test.go | 1 - treatment-service/integration-test/fetch_treatment_it_test.go | 2 +- treatment-service/models/storage.go | 1 - 4 files changed, 1 insertion(+), 4 deletions(-) diff --git a/treatment-service/appcontext/appcontext.go b/treatment-service/appcontext/appcontext.go index e74c1fb..d574aa7 100644 --- a/treatment-service/appcontext/appcontext.go +++ b/treatment-service/appcontext/appcontext.go @@ -32,7 +32,6 @@ func NewAppContext(cfg *config.Config) (*AppContext, error) { localStorage, err := models.NewLocalStorage( cfg.GetProjectIds(), cfg.ManagementService.URL, - cfg.ManagementService.AuthorizationEnabled, cfg.DeploymentConfig.GoogleApplicationCredentialsEnvVar, ) if err != nil { diff --git a/treatment-service/appcontext/appcontext_test.go b/treatment-service/appcontext/appcontext_test.go index 172e309..734d6d5 100644 --- a/treatment-service/appcontext/appcontext_test.go +++ b/treatment-service/appcontext/appcontext_test.go @@ -78,7 +78,6 @@ func TestContext(t *testing.T) { localStorage, err := models.NewLocalStorage( testConfig.GetProjectIds(), testConfig.ManagementService.URL, - testConfig.ManagementService.AuthorizationEnabled, "", ) if err != nil { diff --git a/treatment-service/integration-test/fetch_treatment_it_test.go b/treatment-service/integration-test/fetch_treatment_it_test.go index 76f8b2d..6a43f7c 100644 --- a/treatment-service/integration-test/fetch_treatment_it_test.go +++ b/treatment-service/integration-test/fetch_treatment_it_test.go @@ -807,7 +807,7 @@ func (suite *TreatmentServiceTestSuite) TestAllFiltersSwitchback() { } func (suite *TreatmentServiceTestSuite) TestLocalStorage() { - storage, err := models.NewLocalStorage([]models.ProjectId{1}, suite.managementServiceServer.URL, false, "") + storage, err := models.NewLocalStorage([]models.ProjectId{1}, suite.managementServiceServer.URL, "") suite.Require().NoError(err) suite.Require().NotEmpty(storage) diff --git a/treatment-service/models/storage.go b/treatment-service/models/storage.go index acaf713..1b95981 100644 --- a/treatment-service/models/storage.go +++ b/treatment-service/models/storage.go @@ -561,7 +561,6 @@ func (s *LocalStorage) getAllProjects() ([]*pubsub.ProjectSettings, error) { func NewLocalStorage( projectIds []ProjectId, xpServer string, - authzEnabled bool, googleApplicationCredentialsEnvVar string, ) (*LocalStorage, error) { // Set up Request Modifiers