gcp: NewProvider: Avoid returning nil

As discussed in #1144 (comment)
elastic · Jul 26, 2023 · 5363be2 · 5363be2
1 parent e171e1f
commit 5363be2
Show file tree

Hide file tree

Showing 5 changed files with 70 additions and 85 deletions.
diff --git a/flavors/benchmark/gcp.go b/flavors/benchmark/gcp.go
@@ -37,7 +37,7 @@ type GCP struct{}
 func (G *GCP) Run(context.Context) error { return nil }
 
 func (G *GCP) Initialize(ctx context.Context, log *logp.Logger, cfg *config.Config, ch chan fetching.ResourceInfo, dependencies *Dependencies) (registry.Registry, dataprovider.CommonDataProvider, error) {
-	gcpClientConfig, err := auth.GetGcpClientConfig(cfg, log)
+	gcpClientConfig, err := auth.GetGcpClientConfig(cfg.CloudConfig.Gcp, log)
 	if err != nil {
 		return nil, nil, fmt.Errorf("failed to initialize gcp config: %w", err)
 	}

diff --git a/flavors/posture.go b/flavors/posture.go
@@ -75,7 +75,7 @@ func newPostureFromCfg(cfg *config.Config) (*posture, error) {
 		AwsCfgProvider:           awslib.ConfigProvider{MetadataProvider: awslib.Ec2MetadataProvider{}},
 		AwsIdentityProvider:      awslib.IdentityProvider{},
 		AwsAccountProvider:       awslib.AccountProvider{},
-		GcpIdentityProvider:      identity.NewProvider(ctx, cfg, log),
+		GcpIdentityProvider:      identity.NewProvider(log),
 		KubernetesClientProvider: k8s.ClientGetter{},
 		AwsMetadataProvider:      awslib.Ec2MetadataProvider{},
 		EksClusterNameProvider:   awslib.EKSClusterNameProvider{},

diff --git a/resources/providers/gcplib/auth/credentials.go b/resources/providers/gcplib/auth/credentials.go
@@ -35,27 +35,26 @@ type GcpFactoryConfig struct {
 	ClientOpts []option.ClientOption
 }
 
-func GetGcpClientConfig(cfg *config.Config, log *logp.Logger) ([]option.ClientOption, error) {
+func GetGcpClientConfig(cfg config.GcpConfig, log *logp.Logger) ([]option.ClientOption, error) {
 	log.Info("GetGCPClientConfig create credentials options")
-	gcpCred := cfg.CloudConfig.Gcp
-	if gcpCred.CredentialsJSON == "" && gcpCred.CredentialsFilePath == "" {
+	if cfg.CredentialsJSON == "" && cfg.CredentialsFilePath == "" {
 		return nil, errors.New("the credentials file path or credentials JSON have not been specified")
 	}
 
 	var opts []option.ClientOption
-	if gcpCred.CredentialsFilePath != "" {
-		if err := validateJSONFromFile(gcpCred.CredentialsFilePath); err == nil {
-			log.Infof("Appending credentials file path to gcp client options: %s", gcpCred.CredentialsFilePath)
-			opts = append(opts, option.WithCredentialsFile(gcpCred.CredentialsFilePath))
+	if cfg.CredentialsFilePath != "" {
+		if err := validateJSONFromFile(cfg.CredentialsFilePath); err == nil {
+			log.Infof("Appending credentials file path to gcp client options: %s", cfg.CredentialsFilePath)
+			opts = append(opts, option.WithCredentialsFile(cfg.CredentialsFilePath))
 		} else {
 			return nil, err
 		}
 	}
 
-	if gcpCred.CredentialsJSON != "" {
-		if json.Valid([]byte(gcpCred.CredentialsJSON)) {
+	if cfg.CredentialsJSON != "" {
+		if json.Valid([]byte(cfg.CredentialsJSON)) {
 			log.Info("Appending credentials JSON to client options")
-			opts = append(opts, option.WithCredentialsJSON([]byte(gcpCred.CredentialsJSON)))
+			opts = append(opts, option.WithCredentialsJSON([]byte(cfg.CredentialsJSON)))
 		} else {
 			return nil, errors.New("invalid credentials JSON")
 		}
@@ -66,7 +65,7 @@ func GetGcpClientConfig(cfg *config.Config, log *logp.Logger) ([]option.ClientOp
 
 func validateJSONFromFile(filePath string) error {
 	if _, err := os.Stat(filePath); errors.Is(err, fs.ErrNotExist) {
-		return fmt.Errorf("The file %q cannot be found", filePath)
+		return fmt.Errorf("file %q cannot be found", filePath)
 	}
 
 	b, err := os.ReadFile(filePath)

diff --git a/resources/providers/gcplib/auth/credentials_test.go b/resources/providers/gcplib/auth/credentials_test.go
@@ -19,10 +19,11 @@ package auth
 
 import (
 	"os"
-	"reflect"
 	"testing"
 
 	"github.com/elastic/elastic-agent-libs/logp"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"google.golang.org/api/option"
 
 	"github.com/elastic/cloudbeat/config"
@@ -35,80 +36,62 @@ const (
 
 func TestGetGcpClientConfig(t *testing.T) {
 	f := createServiceAccountFile(t)
-	defer os.Remove(f.Name())
+	defer func() {
+		require.NoError(t, os.Remove(f.Name()))
+	}()
 
 	tests := []struct {
 		name    string
-		cfg     *config.Config
+		cfg     config.GcpConfig
 		want    []option.ClientOption
 		wantErr bool
 	}{
 		{
 			name: "Should return a GcpClientConfig using SA credentials file path",
-			cfg: &config.Config{
-				CloudConfig: config.CloudConfig{
-					Gcp: config.GcpConfig{
-						GcpClientOpt: config.GcpClientOpt{
-							CredentialsFilePath: saFilePath,
-						},
-					},
+			cfg: config.GcpConfig{
+				GcpClientOpt: config.GcpClientOpt{
+					CredentialsFilePath: saFilePath,
 				},
 			},
 			want:    []option.ClientOption{option.WithCredentialsFile(saFilePath)},
 			wantErr: false,
 		},
 		{
 			name: "Should return an error due to invalid SA credentials file path",
-			cfg: &config.Config{
-				CloudConfig: config.CloudConfig{
-					Gcp: config.GcpConfig{
-						GcpClientOpt: config.GcpClientOpt{
-							CredentialsFilePath: "invalid path",
-						},
-					},
+			cfg: config.GcpConfig{
+				GcpClientOpt: config.GcpClientOpt{
+					CredentialsFilePath: "invalid path",
 				},
 			},
 			want:    nil,
 			wantErr: true,
 		},
 		{
 			name: "Should return a GcpClientConfig using SA credentials json",
-			cfg: &config.Config{
-				CloudConfig: config.CloudConfig{
-					Gcp: config.GcpConfig{
-						GcpClientOpt: config.GcpClientOpt{
-							CredentialsJSON: saCredentialsJSON,
-						},
-					},
+			cfg: config.GcpConfig{
+				GcpClientOpt: config.GcpClientOpt{
+					CredentialsJSON: saCredentialsJSON,
 				},
 			},
 			want:    []option.ClientOption{option.WithCredentialsJSON([]byte(saCredentialsJSON))},
 			wantErr: false,
 		},
 		{
 			name: "Should return an error due to invalid SA json",
-			cfg: &config.Config{
-				CloudConfig: config.CloudConfig{
-					Gcp: config.GcpConfig{
-						GcpClientOpt: config.GcpClientOpt{
-							CredentialsJSON: "invalid json",
-						},
-					},
+			cfg: config.GcpConfig{
+				GcpClientOpt: config.GcpClientOpt{
+					CredentialsJSON: "invalid json",
 				},
 			},
 			want:    nil,
 			wantErr: true,
 		},
 		{
 			name: "Should return client options with both credentials_file_path and credentials_json",
-			cfg: &config.Config{
-				CloudConfig: config.CloudConfig{
-					Gcp: config.GcpConfig{
-						GcpClientOpt: config.GcpClientOpt{
-							CredentialsFilePath: saFilePath,
-							CredentialsJSON:     saCredentialsJSON,
-						},
-					},
+			cfg: config.GcpConfig{
+				GcpClientOpt: config.GcpClientOpt{
+					CredentialsFilePath: saFilePath,
+					CredentialsJSON:     saCredentialsJSON,
 				},
 			},
 			want: []option.ClientOption{
@@ -121,31 +104,26 @@ func TestGetGcpClientConfig(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			got, err := GetGcpClientConfig(tt.cfg, logp.NewLogger("gcp credentials test"))
-			if (err != nil) != tt.wantErr {
-				t.Errorf("GetGcpClientConfig() error = %v, wantErr %v", err, tt.wantErr)
-				return
+			if tt.wantErr {
+				assert.Error(t, err)
+			} else {
+				require.NoError(t, err)
 			}
 
-			if !reflect.DeepEqual(got, tt.want) {
-				t.Errorf("GetGcpClientConfig() got = %v, want %v", got, tt.want)
-			}
+			assert.Equal(t, tt.want, got)
 		})
 	}
 }
 
 // Creates a test sa account file to be used in the tests
 func createServiceAccountFile(t *testing.T) *os.File {
 	f, err := os.Create(saFilePath)
-	if err != nil {
-		t.Fatal(err)
-	}
-	defer f.Close()
+	require.NoError(t, err)
+	defer func() {
+		require.NoError(t, f.Close())
+	}()
 
-	if err != nil {
-		t.Fatal(err)
-	}
-	if _, err := f.WriteString(saCredentialsJSON); err != nil {
-		t.Fatal(err)
-	}
+	_, err = f.WriteString(saCredentialsJSON)
+	require.NoError(t, err)
 	return f
 }
diff --git a/resources/providers/gcplib/identity/identity_provider.go b/resources/providers/gcplib/identity/identity_provider.go
@@ -50,27 +50,16 @@ type ResourceManager interface {
 	projectsGet(context.Context, string) (*cloudresourcemanager.Project, error)
 }
 
-func NewProvider(ctx context.Context, cfg *config.Config, logger *logp.Logger) *Provider {
-	gcpClientOpt, err := auth.GetGcpClientConfig(cfg, logger)
-	if err != nil {
-		logger.Errorf("failed to get GCP client config: %v", err)
-		return nil
-	}
-	gcpClientOpt = append(gcpClientOpt, option.WithScopes(cloudresourcemanager.CloudPlatformReadOnlyScope))
-	crmService, err := cloudresourcemanager.NewService(ctx, gcpClientOpt...)
-	if err != nil {
-		logger.Errorf("failed to create GCP resource manager service: %v", err)
-		return nil
-	}
-
-	return &Provider{
-		service: &CloudResourceManagerService{service: crmService},
-		logger:  logger,
-	}
+func NewProvider(logger *logp.Logger) *Provider {
+	return &Provider{logger: logger.Named("gcp.identity")}
 }
 
 // GetIdentity returns GCP identity information
 func (p *Provider) GetIdentity(ctx context.Context, cfg config.GcpConfig) (*cloud.Identity, error) {
+	if err := p.initialize(ctx, cfg); err != nil {
+		return nil, err
+	}
+
 	proj, err := p.service.projectsGet(ctx, "projects/"+cfg.ProjectId)
 	if err != nil {
 		return nil, err
@@ -83,6 +72,25 @@ func (p *Provider) GetIdentity(ctx context.Context, cfg config.GcpConfig) (*clou
 	}, nil
 }
 
+func (p *Provider) initialize(ctx context.Context, cfg config.GcpConfig) error {
+	if p.service != nil {
+		return nil
+	}
+
+	gcpClientOpt, err := auth.GetGcpClientConfig(cfg, p.logger)
+	if err != nil {
+		return err
+	}
+	gcpClientOpt = append(gcpClientOpt, option.WithScopes(cloudresourcemanager.CloudPlatformReadOnlyScope))
+	crmService, err := cloudresourcemanager.NewService(ctx, gcpClientOpt...)
+	if err != nil {
+		return err
+	}
+
+	p.service = &CloudResourceManagerService{service: crmService}
+	return nil
+}
+
 func (p *CloudResourceManagerService) projectsGet(ctx context.Context, id string) (*cloudresourcemanager.Project, error) {
 	project, err := p.service.Projects.Get(id).Context(ctx).Do()
 	if err != nil {