Skip to content
6 changes: 3 additions & 3 deletions tools/integration_tests/dentry_cache/notifier_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func (s *notifierTest) TestWriteFileWithDentryCacheEnabled() {
err = operations.WriteFile(filePath, "ShouldNotWrite")

// First Write File attempt should fail because file has been clobbered.
operations.ValidateESTALEError(s.T(), err)
operations.ValidateESTALEOrEIOError(s.T(), err)
// Second Write File attempt.
err = operations.WriteFile(filePath, "ShouldWrite")
// The notifier is triggered after the first write failure, invalidating the kernel cache entry.
Expand All @@ -97,7 +97,7 @@ func (s *notifierTest) TestReadFileWithDentryCacheEnabled() {
// First Read File attempt.
_, err = operations.ReadFile(filePath)
// First Read File attempt should fail because file has been clobbered.
operations.ValidateESTALEError(s.T(), err)
operations.ValidateESTALEOrEIOError(s.T(), err)
// Second Read File attempt.
_, err = operations.ReadFile(filePath)
// The notifier is triggered after the first read failure, invalidating the kernel cache entry.
Expand All @@ -119,7 +119,7 @@ func (s *notifierTest) TestDeleteFileWithDentryCacheEnabled() {
// Read File to call the notifier to invalidate entry.
_, err = operations.ReadFile(filePath)
// The notifier is triggered after the first read failure, invalidating the kernel cache entry.
operations.ValidateESTALEError(s.T(), err)
operations.ValidateESTALEOrEIOError(s.T(), err)

// Stat again, it should give error as entry does not exist.
_, err = os.Stat(filePath)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,13 @@ func TestMain(m *testing.M) {
}

testEnv.ctx = context.Background()

// Check if the GCP project is allowlisted before doing setup or initializing clients.
if !creds_tests.IsAllowlistedProject(testEnv.ctx) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is this package not run on GKE?

log.Printf("The active GCP project is not one of the allowlisted projects. Skipping managed folders tests.")
os.Exit(0)
}

testEnv.bucketType = setup.TestEnvironment(testEnv.ctx, &cfg.ManagedFolders[0])
testEnv.cfg = &cfg.ManagedFolders[0]

Expand Down
3 changes: 3 additions & 0 deletions tools/integration_tests/mount_timeout/mount_access_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,9 @@ func (testSuite *MountAccessTest) mountWithKeyFile(bucketName, keyFile string) (
}

func (testSuite *MountAccessTest) TestMountingWithMinimalAccessSucceeds() {
if !creds_tests.IsAllowlistedProject(gCtx) {
testSuite.T().Skip("Skipping credentials test on non-allowlisted project.")
}
if !client.CheckBucketAccess(gCtx, gStorageClient, testBucket) {
testSuite.T().Skipf("Skipping test as bucket %q is not accessible", testBucket)
}
Expand Down
4 changes: 4 additions & 0 deletions tools/integration_tests/requester_pays_bucket/setup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,10 @@ func TestMain(m *testing.M) {

// When not running in GKE environment.
if cfg.RequesterPaysBucket[0].GKEMountedDirectory == "" {
if !creds_tests.IsAllowlistedProject(testEnv.ctx) {
log.Printf("The active GCP project is not one of the allowlisted projects. Skipping requester pays bucket tests.")
os.Exit(0)
}
// Replace --billing-project= placeholder in flags with the default billing project.
for i := range cfg.RequesterPaysBucket[0].Configs {
for j, flag := range cfg.RequesterPaysBucket[0].Configs[i].Flags {
Expand Down
44 changes: 37 additions & 7 deletions tools/integration_tests/util/creds_tests/creds.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"os"
"slices"
"strings"
"sync"
"testing"
"time"

Expand All @@ -39,22 +40,47 @@ import (
const NameOfServiceAccount = "creds-integration-tests"
const CredentialsSecretName = "gcsfuse-integration-tests"

var WhitelistedGcpProjects = []string{"gcs-fuse-test", "gcs-fuse-test-ml"}
var AllowlistedGcpProjects = []string{"gcs-fuse-test", "gcs-fuse-test-ml"}

var (
cachedProjectID string
projectIDOnce sync.Once
)

func getCachedProjectID(ctx context.Context) (string, error) {
var err error
projectIDOnce.Do(func() {
cachedProjectID, err = metadata.ProjectIDWithContext(ctx)
})
return cachedProjectID, err
}

func IsAllowlistedProject(ctx context.Context) bool {
id, err := getCachedProjectID(ctx)
if err != nil {
log.Printf("Error in fetching project id: %v", err)
return false
}
if strings.Contains(id, "cloudtop") {
return true
}
return slices.Contains(AllowlistedGcpProjects, id)
}
Comment on lines +58 to +68

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Every call to metadata.ProjectIDWithContext(ctx) performs an HTTP request to the local metadata server. Since IsAllowlistedProject is called first, and then projectID is called again during credential creation, this results in redundant network roundtrips.

We can cache the fetched project ID in a package-level variable to avoid these duplicate requests. Additionally, please update the projectID(ctx) function to use fetchProjectID(ctx) instead of calling metadata.ProjectIDWithContext(ctx) directly.

var (
	projectIDCache    string
	projectIDCacheErr error
	projectIDFetched  bool
)

func fetchProjectID(ctx context.Context) (string, error) {
	if !projectIDFetched {
		projectIDCache, projectIDCacheErr = metadata.ProjectIDWithContext(ctx)
		projectIDFetched = true
	}
	return projectIDCache, projectIDCacheErr
}

func IsAllowlistedProject(ctx context.Context) bool {
	id, err := fetchProjectID(ctx)
	if err != nil {
		log.Printf("Error in fetching project id: %v", err)
		return false
	}
	if strings.Contains(id, "cloudtop") {
		return true
	}
	return slices.Contains(AllowlistedGcpProjects, id)
}


func projectID(ctx context.Context) string {
// Fetching project-id to get service account id.
id, err := metadata.ProjectIDWithContext(ctx)
id, err := getCachedProjectID(ctx)
if err != nil {
setup.LogAndExit(fmt.Sprintf("Error in fetching project id: %v", err))
}
if strings.Contains(id, "cloudtop") {
// In cloudtop environments, well known path is used for auth. So explicitly set the project as whitelisted.
id = WhitelistedGcpProjects[0]
// In cloudtop environments, well known path is used for auth. So explicitly set the project as allowlisted.
id = AllowlistedGcpProjects[0]
}

// return if active GCP project is not in whitelisted gcp projects
if !slices.Contains(WhitelistedGcpProjects, id) {
log.Printf("The active GCP project is not one of: %s. So the credentials test will not run.", strings.Join(WhitelistedGcpProjects, ", "))
// return if active GCP project is not in allowlisted gcp projects
if !slices.Contains(AllowlistedGcpProjects, id) {
log.Printf("The active GCP project is not one of: %s. So the credentials test will not run.", strings.Join(AllowlistedGcpProjects, ", "))
}
return id
}
Expand Down Expand Up @@ -161,6 +187,10 @@ func RevokeCustomRoleFromServiceAccountOnBucket(ctx context.Context, storageClie
}

func RunTestsForDifferentAuthMethods(ctx context.Context, cfg *test_suite.TestConfig, storageClient *storage.Client, testFlagSet [][]string, permission string, m *testing.M) (successCode int) {
if !IsAllowlistedProject(ctx) {
log.Printf("The active GCP project is not one of: %s. So the credentials test will not run.", strings.Join(AllowlistedGcpProjects, ", "))
return 0
}
serviceAccount, localKeyFilePath := CreateCredentials(ctx)
defer func() {
if err := os.Remove(localKeyFilePath); err != nil {
Expand Down
6 changes: 3 additions & 3 deletions tools/integration_tests/util/operations/file_operations.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func CopyFileAllowOverwrite(srcFileName, newFileName string) (err error) {
func ReadFile(filePath string) (content []byte, err error) {
content, err = os.ReadFile(filePath)
if err != nil {
err = fmt.Errorf("ReadFile: %v", err)
err = fmt.Errorf("ReadFile: %w", err)
return
}
return
Expand Down Expand Up @@ -140,7 +140,7 @@ func RenameFile(fileName string, newFileName string) (err error) {
func WriteFileInAppendMode(fileName string, content string) (err error) {
f, err := os.OpenFile(fileName, os.O_APPEND|os.O_WRONLY|syscall.O_DIRECT, FilePermission_0600)
if err != nil {
err = fmt.Errorf("open file for append: %v", err)
err = fmt.Errorf("open file for append: %w", err)
return
}

Expand All @@ -155,7 +155,7 @@ func WriteFileInAppendMode(fileName string, content string) (err error) {
func WriteFile(fileName string, content string) (err error) {
f, err := os.OpenFile(fileName, os.O_RDWR|syscall.O_DIRECT, FilePermission_0600)
if err != nil {
err = fmt.Errorf("open file for write at start: %v", err)
err = fmt.Errorf("open file for write at start: %w", err)
return
}

Expand Down
19 changes: 17 additions & 2 deletions tools/integration_tests/util/operations/validation_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,29 @@ func ValidateESTALEError(t *testing.T, err error) {
t.Helper()

require.Error(t, err)
assert.Regexp(t, syscall.ESTALE.Error(), err.Error())
if !errors.Is(err, syscall.ESTALE) {
t.Fatalf("Expected error to be %q (ESTALE). Got: %v", syscall.ESTALE, err)
}
}

func ValidateESTALEOrEIOError(t *testing.T, err error) {
t.Helper()

require.Error(t, err)
// FUSE kernel driver can translate ESTALE to EIO (input/output error) on some operations/kernels.
// So we accept both "stale file handle" (ESTALE) and "input/output error" (EIO).
if !errors.Is(err, syscall.ESTALE) && !errors.Is(err, syscall.EIO) {
t.Fatalf("Expected error to be %q (ESTALE) or %q (EIO). Got: %v", syscall.ESTALE, syscall.EIO, err)
}
}
Comment on lines +60 to +69

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Using strings.Contains on err.Error() to check for specific system errors like syscall.ESTALE or syscall.EIO can be fragile. In Go, the idiomatic and robust way to assert wrapped system errors (such as those wrapped in os.PathError) is using errors.Is.

func ValidateESTALEOrEIOError(t *testing.T, err error) {
	t.Helper()

	require.Error(t, err)
	// FUSE kernel driver can translate ESTALE to EIO (input/output error) on some operations/kernels.
	// So we accept both "stale file handle" (ESTALE) and "input/output error" (EIO).
	if !errors.Is(err, syscall.ESTALE) && !errors.Is(err, syscall.EIO) {
		t.Fatalf("Expected error to be %v or %v. Got: %v", syscall.ESTALE, syscall.EIO, err)
	}
}


func ValidateEIOError(t *testing.T, err error) {
t.Helper()

require.Error(t, err)
assert.Regexp(t, syscall.EIO.Error(), err.Error())
if !errors.Is(err, syscall.EIO) {
t.Fatalf("Expected error to be %q (EIO). Got: %v", syscall.EIO, err)
}
}

func CheckErrorForReadOnlyFileSystem(t *testing.T, err error) {
Expand Down
Loading