Skip to content

Commit

Permalink
fix(auth): choose quota project envvar over file when both present (#…
Browse files Browse the repository at this point in the history
…10807)

Because we were explicitly setting the value of quota project in
the credentials package the logic that should have checked the
environment variable was never hit if the value was set in the creds
file. So for now we will not set it in this package. The getter on
the creds will then default to the correct logic in internal.go.

Also mark an integration test as such so it does not run when the
short flag is passed.

Fixes: #10804
  • Loading branch information
codyoss authored Sep 3, 2024
1 parent d8a53d6 commit 2d8dd77
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 7 deletions.
44 changes: 44 additions & 0 deletions auth/credentials/detect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,50 @@ func TestDefaultCredentials_UserCredentialsKey(t *testing.T) {
}
}

func TestDefaultCredentials_QuotaProjectPrecedence(t *testing.T) {
want := "take-this-value"
t.Setenv("GOOGLE_CLOUD_QUOTA_PROJECT", want)
ctx := context.Background()
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
resp := &tokResp{
AccessToken: "a_fake_token",
TokenType: internal.TokenTypeBearer,
ExpiresIn: 60,
}
if err := json.NewEncoder(w).Encode(&resp); err != nil {
t.Fatal(err)
}
}))

creds, err := DetectDefault(&DetectOptions{
CredentialsFile: "../internal/testdata/user.json",
Scopes: []string{"https://www.googleapis.com/auth/cloud-platform"},
TokenURL: ts.URL,
})
if err != nil {
t.Fatal(err)
}
got, err := creds.QuotaProjectID(ctx)
if err != nil {
t.Fatal(err)
}
if got != want {
t.Fatalf("got %q, want %q", got, want)
}

// unset the env to fallback to the value in creds
t.Setenv("GOOGLE_CLOUD_QUOTA_PROJECT", "")
want = "fake_project2"
got, err = creds.QuotaProjectID(ctx)
if err != nil {
t.Fatal(err)
}
if got != want {
t.Fatalf("got %q, want %q", got, want)
}
}

func TestDefaultCredentials_UserCredentialsKey_UniverseDomain(t *testing.T) {
ctx := context.Background()
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
Expand Down
11 changes: 4 additions & 7 deletions auth/credentials/filetypes.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func fileCredentials(b []byte, opts *DetectOptions) (*auth.Credentials, error) {
return nil, err
}

var projectID, quotaProjectID, universeDomain string
var projectID, universeDomain string
var tp auth.TokenProvider
switch fileType {
case credsfile.ServiceAccountKey:
Expand All @@ -56,7 +56,6 @@ func fileCredentials(b []byte, opts *DetectOptions) (*auth.Credentials, error) {
if err != nil {
return nil, err
}
quotaProjectID = f.QuotaProjectID
universeDomain = f.UniverseDomain
case credsfile.ExternalAccountKey:
f, err := credsfile.ParseExternalAccount(b)
Expand All @@ -67,7 +66,6 @@ func fileCredentials(b []byte, opts *DetectOptions) (*auth.Credentials, error) {
if err != nil {
return nil, err
}
quotaProjectID = f.QuotaProjectID
universeDomain = resolveUniverseDomain(opts.UniverseDomain, f.UniverseDomain)
case credsfile.ExternalAccountAuthorizedUserKey:
f, err := credsfile.ParseExternalAccountAuthorizedUser(b)
Expand All @@ -78,7 +76,6 @@ func fileCredentials(b []byte, opts *DetectOptions) (*auth.Credentials, error) {
if err != nil {
return nil, err
}
quotaProjectID = f.QuotaProjectID
universeDomain = f.UniverseDomain
case credsfile.ImpersonatedServiceAccountKey:
f, err := credsfile.ParseImpersonatedServiceAccount(b)
Expand Down Expand Up @@ -108,9 +105,9 @@ func fileCredentials(b []byte, opts *DetectOptions) (*auth.Credentials, error) {
TokenProvider: auth.NewCachedTokenProvider(tp, &auth.CachedTokenProviderOptions{
ExpireEarly: opts.EarlyTokenRefresh,
}),
JSON: b,
ProjectIDProvider: internalauth.StaticCredentialsProperty(projectID),
QuotaProjectIDProvider: internalauth.StaticCredentialsProperty(quotaProjectID),
JSON: b,
ProjectIDProvider: internalauth.StaticCredentialsProperty(projectID),
// TODO(codyoss): only set quota project here if there was a user override
UniverseDomainProvider: internalauth.StaticCredentialsProperty(universeDomain),
}), nil
}
Expand Down
1 change: 1 addition & 0 deletions auth/credentials/idtoken/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ func TestNewCredentials_CredentialsFile(t *testing.T) {
}

func TestNewCredentials_CredentialsJSON(t *testing.T) {
testutil.IntegrationTestCheck(t)
ctx := context.Background()
f := os.Getenv(credsfile.GoogleAppCredsEnvVar)
if f == "" {
Expand Down

0 comments on commit 2d8dd77

Please sign in to comment.