Skip to content

Commit

Permalink
fix: Adjusting testing for GCS bucket existence check (#3810)
Browse files Browse the repository at this point in the history
* fix: Adjusting testing for GCS bucket existence check

Adjusting mocking implementation to avoid global anonymous function in GCS bucket existence check, as discussed in #3756.

* fix: Fixing integration test for bucket creation

* fix: Plumbing through some ctx
  • Loading branch information
yhakbar authored Jan 27, 2025
1 parent a70c834 commit 41e8c07
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 53 deletions.
89 changes: 55 additions & 34 deletions remote/remote_state_gcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,12 +110,16 @@ func (initializer GCSInitializer) NeedsInitialization(remoteState *RemoteState,
return false, err
}

gcsClient, err := CreateGCSClient(*gcsConfig)
ctx := context.Background()

gcsClient, err := CreateGCSClient(ctx, *gcsConfig)
if err != nil {
return false, err
}

if !DoesGCSBucketExist(gcsClient, gcsConfig) {
bucketHandle := gcsClient.Bucket(gcsConfig.Bucket)

if !DoesGCSBucketExist(ctx, bucketHandle) {
return true, nil
}

Expand Down Expand Up @@ -182,8 +186,10 @@ func (initializer GCSInitializer) Initialize(ctx context.Context, remoteState *R
return err
}

if err := ValidateGCSConfig(gcsConfigExtended); err != nil {
return err
if !gcsConfigExtended.SkipBucketCreation {
if err := ValidateGCSConfig(ctx, gcsConfigExtended); err != nil {
return err
}
}

var gcsConfig = gcsConfigExtended.remoteStateConfigGCS
Expand All @@ -203,7 +209,7 @@ func (initializer GCSInitializer) Initialize(ctx context.Context, remoteState *R
}

// TODO: Remove lint suppression
gcsClient, err := CreateGCSClient(gcsConfig) //nolint:contextcheck
gcsClient, err := CreateGCSClient(ctx, gcsConfig) //nolint:contextcheck
if err != nil {
return err
}
Expand Down Expand Up @@ -275,27 +281,18 @@ func ParseExtendedGCSConfig(config map[string]interface{}) (*ExtendedRemoteState
}

// ValidateGCSConfig validates the configuration for GCS remote state.
func ValidateGCSConfig(extendedConfig *ExtendedRemoteStateConfigGCS) error {
func ValidateGCSConfig(ctx context.Context, extendedConfig *ExtendedRemoteStateConfigGCS) error {
config := extendedConfig.remoteStateConfigGCS

// If skip_bucket_creation is true, bypass all validation
// This allows using existing buckets without restrictions
if extendedConfig.SkipBucketCreation {
return nil
}

// Bucket is always a required configuration parameter
// Bucket is always a required configuration parameter when not skipping bucket creation
// so we check it here to make sure we have handle to the bucket
// before we start validating the rest of the configuration.
if config.Bucket == "" {
return errors.New(MissingRequiredGCSRemoteStateConfig("bucket"))
}

// If both project and location are provided, the configuration is valid
if extendedConfig.Project != "" && extendedConfig.Location != "" {
return nil
}

// Create a GCS client to check bucket existence
gcsClient, err := CreateGCSClient(config)
gcsClient, err := CreateGCSClient(ctx, config)
if err != nil {
return fmt.Errorf("error creating GCS client: %w", err)
}
Expand All @@ -306,8 +303,31 @@ func ValidateGCSConfig(extendedConfig *ExtendedRemoteStateConfigGCS) error {
}
}()

bucketHandle := gcsClient.Bucket(config.Bucket)

if err := ValidateGCSConfigWithHandle(ctx, bucketHandle, extendedConfig); err != nil {
return err
}

return nil
}

// ValidateGCSConfigWithHandle validates the configuration for GCS remote state.
func ValidateGCSConfigWithHandle(ctx context.Context, bucketHandle BucketHandle, extendedConfig *ExtendedRemoteStateConfigGCS) error {
config := extendedConfig.remoteStateConfigGCS

// Bucket is always a required configuration parameter
if config.Bucket == "" {
return errors.New(MissingRequiredGCSRemoteStateConfig("bucket"))
}

// If both project and location are provided, the configuration is valid
if extendedConfig.Project != "" && extendedConfig.Location != "" {
return nil
}

// Check if the bucket exists
bucketExists := DoesGCSBucketExist(gcsClient, &config)
bucketExists := DoesGCSBucketExist(ctx, bucketHandle)
if bucketExists {
return nil
}
Expand All @@ -327,8 +347,9 @@ func ValidateGCSConfig(extendedConfig *ExtendedRemoteStateConfigGCS) error {
// If the bucket specified in the given config doesn't already exist, prompt the user to create it, and if the user
// confirms, create the bucket and enable versioning for it.
func createGCSBucketIfNecessary(ctx context.Context, gcsClient *storage.Client, config *ExtendedRemoteStateConfigGCS, terragruntOptions *options.TerragruntOptions) error {
// TODO: Remove lint suppression
if !DoesGCSBucketExist(gcsClient, &config.remoteStateConfigGCS) { //nolint:contextcheck
bucketHandle := gcsClient.Bucket(config.remoteStateConfigGCS.Bucket)

if !DoesGCSBucketExist(ctx, bucketHandle) {
terragruntOptions.Logger.Debugf("Remote state GCS bucket %s does not exist. Attempting to create it", config.remoteStateConfigGCS.Bucket)

// A project must be specified in order for terragrunt to automatically create a storage bucket.
Expand Down Expand Up @@ -476,8 +497,10 @@ func CreateGCSBucket(gcsClient *storage.Client, config *ExtendedRemoteStateConfi
func WaitUntilGCSBucketExists(gcsClient *storage.Client, config *RemoteStateConfigGCS, terragruntOptions *options.TerragruntOptions) error {
terragruntOptions.Logger.Debugf("Waiting for bucket %s to be created", config.Bucket)

bucketHandle := gcsClient.Bucket(config.Bucket)

for retries := 0; retries < MaxRetriesWaitingForGcsBucket; retries++ {
if DoesGCSBucketExist(gcsClient, config) {
if DoesGCSBucketExist(context.Background(), bucketHandle) {
terragruntOptions.Logger.Debugf("GCS bucket %s created.", config.Bucket)
return nil
} else if retries < MaxRetriesWaitingForGcsBucket-1 {
Expand All @@ -491,33 +514,31 @@ func WaitUntilGCSBucketExists(gcsClient *storage.Client, config *RemoteStateConf

// DoesGCSBucketExist returns true if the GCS bucket specified in the given config exists and the current user has the
// ability to access it.
var DoesGCSBucketExist = func(gcsClient *storage.Client, config *RemoteStateConfigGCS) bool {
ctx := context.Background()

// Creates a Bucket instance.
bucket := gcsClient.Bucket(config.Bucket)

func DoesGCSBucketExist(ctx context.Context, bucketHandle BucketHandle) bool {
// TODO - the code below attempts to determine whether the storage bucket exists by making a making a number of API
// calls, then attempting to list the contents of the bucket. It was adapted from Google's own integration tests and
// should be improved once the appropriate API call is added. For more info see:
// https://github.com/GoogleCloudPlatform/google-cloud-go/blob/de879f7be552d57556875b8aaa383bce9396cc8c/storage/integration_test.go#L1231
if _, err := bucket.Attrs(ctx); err != nil {
if _, err := bucketHandle.Attrs(ctx); err != nil {
// ErrBucketNotExist
return false
}

it := bucket.Objects(ctx, nil)
it := bucketHandle.Objects(ctx, nil)
if _, err := it.Next(); errors.Is(err, storage.ErrBucketNotExist) {
return false
}

return true
}

// CreateGCSClient creates an authenticated client for GCS
var CreateGCSClient = func(gcsConfigRemote RemoteStateConfigGCS) (*storage.Client, error) {
ctx := context.Background()
type BucketHandle interface {
Attrs(ctx context.Context) (*storage.BucketAttrs, error)
Objects(ctx context.Context, q *storage.Query) *storage.ObjectIterator
}

// CreateGCSClient creates an authenticated client for GCS
func CreateGCSClient(ctx context.Context, gcsConfigRemote RemoteStateConfigGCS) (*storage.Client, error) {
var opts []option.ClientOption

if gcsConfigRemote.Credentials != "" {
Expand Down
38 changes: 29 additions & 9 deletions remote/remote_state_gcs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@
package remote_test

import (
"context"
"testing"

"cloud.google.com/go/storage"
"github.com/gruntwork-io/terragrunt/internal/errors"
"github.com/gruntwork-io/terragrunt/options"
"github.com/gruntwork-io/terragrunt/remote"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -133,6 +135,8 @@ func TestGcpConfigValuesEqual(t *testing.T) {
}

func TestValidateGCSConfig(t *testing.T) {
t.Parallel()

testCases := map[string]struct {
config map[string]interface{}
expectedError bool
Expand Down Expand Up @@ -212,21 +216,17 @@ func TestValidateGCSConfig(t *testing.T) {
},
}

// Mock the DoesGCSBucketExist function to simulate bucket existence
originalDoesGCSBucketExist := remote.DoesGCSBucketExist
defer func() { remote.DoesGCSBucketExist = originalDoesGCSBucketExist }()

for name, tc := range testCases {
t.Run(name, func(t *testing.T) {
// Set up the mock bucket existence check
remote.DoesGCSBucketExist = func(gcsClient *storage.Client, config *remote.RemoteStateConfigGCS) bool {
return tc.mockBucketExists
}
t.Parallel()

// Set up the mock bucket handle for existence check
mockBucketHandle := &mockBucketHandle{doesExist: tc.mockBucketExists}

extendedConfig, err := remote.ParseExtendedGCSConfig(tc.config)
require.NoError(t, err)

err = remote.ValidateGCSConfig(extendedConfig)
err = remote.ValidateGCSConfigWithHandle(context.TODO(), mockBucketHandle, extendedConfig)

if tc.expectedError {
require.Error(t, err)
Expand All @@ -236,3 +236,23 @@ func TestValidateGCSConfig(t *testing.T) {
})
}
}

type mockBucketHandle struct {
doesExist bool
}

func (m *mockBucketHandle) Attrs(ctx context.Context) (*storage.BucketAttrs, error) {
if m.doesExist {
return &storage.BucketAttrs{}, nil
}

return nil, errors.New("bucket does not exist")
}

func (m *mockBucketHandle) Objects(ctx context.Context, q *storage.Query) *storage.ObjectIterator {
if m.doesExist {
return &storage.ObjectIterator{}
}

return &storage.ObjectIterator{}
}
26 changes: 16 additions & 10 deletions test/integration_gcp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,15 +166,19 @@ func validateGCSBucketExistsAndIsLabeled(t *testing.T, location string, bucketNa

remoteStateConfig := remote.RemoteStateConfigGCS{Bucket: bucketName}

gcsClient, err := remote.CreateGCSClient(remoteStateConfig)
ctx := context.TODO()

gcsClient, err := remote.CreateGCSClient(ctx, remoteStateConfig)
require.NoError(t, err, "Error creating GCS client")

bucketHandle := gcsClient.Bucket(bucketName)

// verify the bucket exists
assert.True(t, remote.DoesGCSBucketExist(gcsClient, &remoteStateConfig), "Terragrunt failed to create remote state GCS bucket %s", bucketName)
assert.True(t, remote.DoesGCSBucketExist(ctx, bucketHandle), "Terragrunt failed to create remote state GCS bucket %s", bucketName)

// verify the bucket location
bucket := gcsClient.Bucket(bucketName)
attrs, err := bucket.Attrs(context.Background())
attrs, err := bucket.Attrs(ctx)
require.NoError(t, err)

assert.Equal(t, strings.ToUpper(location), attrs.Location, "Did not find GCS bucket in expected location.")
Expand All @@ -188,14 +192,15 @@ func validateGCSBucketExistsAndIsLabeled(t *testing.T, location string, bucketNa
func gcsObjectAttrs(t *testing.T, bucketName string, objectName string) *storage.ObjectAttrs {
t.Helper()

ctx := context.Background()

remoteStateConfig := remote.RemoteStateConfigGCS{Bucket: bucketName}

gcsClient, err := remote.CreateGCSClient(remoteStateConfig)
gcsClient, err := remote.CreateGCSClient(ctx, remoteStateConfig)
if err != nil {
t.Fatalf("Error creating GCS client: %v", err)
}

ctx := context.Background()
bucket := gcsClient.Bucket(bucketName)

handle := bucket.Object(objectName)
Expand Down Expand Up @@ -230,15 +235,16 @@ func assertGCSLabels(t *testing.T, expectedLabels map[string]string, bucketName
func createGCSBucket(t *testing.T, projectID string, location string, bucketName string) {
t.Helper()

ctx := context.Background()

var gcsConfig remote.RemoteStateConfigGCS
gcsClient, err := remote.CreateGCSClient(gcsConfig)
gcsClient, err := remote.CreateGCSClient(ctx, gcsConfig)
if err != nil {
t.Fatalf("Error creating GCS client: %v", err)
}

t.Logf("Creating test GCS bucket %s in project %s, location %s", bucketName, projectID, location)

ctx := context.Background()
bucket := gcsClient.Bucket(bucketName)

bucketAttrs := &storage.BucketAttrs{
Expand All @@ -255,16 +261,16 @@ func createGCSBucket(t *testing.T, projectID string, location string, bucketName
func deleteGCSBucket(t *testing.T, bucketName string) {
t.Helper()

ctx := context.Background()

var gcsConfig remote.RemoteStateConfigGCS
gcsClient, err := remote.CreateGCSClient(gcsConfig)
gcsClient, err := remote.CreateGCSClient(ctx, gcsConfig)
if err != nil {
t.Fatalf("Error creating GCS client: %v", err)
}

t.Logf("Deleting test GCS bucket %s", bucketName)

ctx := context.Background()

// List all objects including their versions in the bucket
bucket := gcsClient.Bucket(bucketName)
q := &storage.Query{
Expand Down

0 comments on commit 41e8c07

Please sign in to comment.