Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for GCP Service Account as JSON or Path in Default Application Credentials #953

Merged
merged 5 commits into from May 6, 2022
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 23 additions & 5 deletions gcpkms/keysource.go
Expand Up @@ -3,16 +3,17 @@ package gcpkms //import "go.mozilla.org/sops/v3/gcpkms"
import (
"encoding/base64"
"fmt"
"google.golang.org/api/option"
"io/ioutil"
"os"
"regexp"
"strings"
"time"

"go.mozilla.org/sops/v3/logging"

"golang.org/x/net/context"
"golang.org/x/oauth2/google"

"github.com/sirupsen/logrus"
"golang.org/x/net/context"
cloudkms "google.golang.org/api/cloudkms/v1"
)

Expand Down Expand Up @@ -131,12 +132,13 @@ func (key MasterKey) createCloudKMSService() (*cloudkms.Service, error) {
}

ctx := context.Background()
client, err := google.DefaultClient(ctx, cloudkms.CloudPlatformScope)

creds, err := getDefaultApplicationCredentials()
if err != nil {
return nil, err
}

cloudkmsService, err := cloudkms.New(client)
cloudkmsService, err := cloudkms.NewService(ctx, option.WithCredentialsJSON(creds))
if err != nil {
return nil, err
}
Expand All @@ -151,3 +153,19 @@ func (key MasterKey) ToMap() map[string]interface{} {
out["created_at"] = key.CreationDate.UTC().Format(time.RFC3339)
return out
}

// getDefaultApplicationCredentials allows for passing GCP Service Account
// Credentials as either a path to a file, or directly as an environment variable
// in JSON format.
func getDefaultApplicationCredentials() (token []byte, err error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why use this when https://pkg.go.dev/golang.org/x/oauth2/google#FindDefaultCredentials exists? Shouldn't we check if it returns an error first and then do this custom implementation?

Copy link

Choose a reason for hiding this comment

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

It would be worth:

So that we have the special use-case when GOOGLE_CREDENTIALS is set, otherwise sops fallback to the default behavior of the SDK, what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, though I'd suggest that we reverse the order. It's good to stay as close to the common cloud provider SDK's as possible and this would ensure we don't break any backwards compatibility.

Copy link
Member

Choose a reason for hiding this comment

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

I think @multani's suggestion already is backwards compatible. Given it is not a standard environment variable at the moment, there is some intent for it to be used.

A regular user would not have this set, while by changing it around, it would not be taken into account on GCP instances where the right environment default values are automatically inject at times. Which makes FindDefaultCredentials always work, and GOOGLE_CREDENTIALS unreachable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree with @multani's thoughts here. I'll get the the call to FindDefaultCredentials integrated. Seems like it would be better to support both cases GOOGLE_CREDENTIALS and GOOGLE_APPLICATION_CREDENTIALS to integrate with a wider set of implementations.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok sounds good.

var defaultCredentials = os.Getenv("GOOGLE_APPLICATION_CREDENTIALS")

if _, err := os.Stat(defaultCredentials); err == nil {
if token, err = ioutil.ReadFile(defaultCredentials); err != nil {
joshkaplinsky marked this conversation as resolved.
Show resolved Hide resolved
return nil, err
}
} else {
token = []byte(defaultCredentials)
}
return
}