tus / tusd

Reference server implementation in Go of tus: the open protocol for resumable file uploads
https://tus.github.io/tusd
MIT License
2.93k stars 465 forks source link

GCS: use application default credentials #448

Open adamhooper opened 3 years ago

adamhooper commented 3 years ago

Is your feature request related to a problem? Please describe.

I want to configure tusd's GCS authentication the way I configure other GCS clients.

In particular, my GKE containers already have workload identities. Other clients don't need a file on disk or an environment variable.

Describe the solution you'd like

I'd like tusd to authenticate using the existing logic in cloud.google.com/go/storage, as documented at https://cloud.google.com/docs/authentication/production, as opposed to with its own logic.

For most users, this would mean setting GOOGLE_APPLICATION_CREDENTIALS instead of GCS_APPLICATION_CREDENTIALS. Wouldn't that be swell! It'd be like with other apps.

But for users like me, who use GCE, GKE, etc., we'd have an even happier outcome: we wouldn't need to configure anything at all. Authentication would Just Work, no environment variable needed.

This diff ignores the backwards-compatibility problem. It might be wise to put SetEnv("GOOGLE_APPLICATION_CREDENTIALS") to equal GetEnv("GCS_SERVICE_ACCOUNT_FILE") when it is set, to preserve backwards-compatibility.

diff --git a/cmd/tusd/cli/composer.go b/cmd/tusd/cli/composer.go
index 71a410f..20b2a40 100644
--- a/cmd/tusd/cli/composer.go
+++ b/cmd/tusd/cli/composer.go
@@ -59,14 +59,7 @@ func CreateComposer() {
                                "Please remove underscore from the value", Flags.GCSObjectPrefix)
                }

-               // Derivce credentials from service account file path passed in
-               // GCS_SERVICE_ACCOUNT_FILE environment variable.
-               gcsSAF := os.Getenv("GCS_SERVICE_ACCOUNT_FILE")
-               if gcsSAF == "" {
-                       stderr.Fatalf("No service account file provided for Google Cloud Storage using the GCS_SERVICE_ACCOUNT_FILE environment variable.\n")
-               }
-
-               service, err := gcsstore.NewGCSService(gcsSAF)
+               service, err := gcsstore.NewGCSService()
                if err != nil {
                        stderr.Fatalf("Unable to create Google Cloud Storage service: %s\n", err)
                }
diff --git a/pkg/gcsstore/gcsservice.go b/pkg/gcsstore/gcsservice.go
index 4667622..8f95270 100644
--- a/pkg/gcsstore/gcsservice.go
+++ b/pkg/gcsstore/gcsservice.go
@@ -77,10 +77,12 @@ type GCSService struct {
        Client *storage.Client
 }

-// NewGCSService returns a GCSService object given a GCloud service account file path.
-func NewGCSService(filename string) (*GCSService, error) {
+// NewGCSService returns a GCSService object
+// Authentication happens according to https://cloud.google.com/docs/authentication/production.
+// Primer: set GOOGLE_APLICATION_CREDENTIALS=[path].
+func NewGCSService() (*GCSService, error) {
        ctx := context.Background()
-       client, err := storage.NewClient(ctx, option.WithCredentialsFile(filename))
+       client, err := storage.NewClient(ctx)
        if err != nil {
                return nil, err
        }

Describe alternatives you've considered

Currently, I create a service-account key, store it in a Kubernetes secret, mount the secret as a volume on the tusd container, and set the GCS_SERVICE_ACCOUNT_FILE variable to point to it.

This adds a security problem: that security key is visible within the container, and it lasts for 10 years. (Under my proposed approach, the workload-identity tokens given by Google cloud services last only 30min.)

It also adds a maintenance problem: the security key expires in 10 years, and I must address that problem manually. (Under my proposed approach, maintenance is automatic.)

Also, I just made a mistake today and it frustrated me :)

Can you provide help with implementing this feature?

I think that diff ought to do it. I haven't tested it, though.

Additional context

All the cool kids authenticate without environment variables.

Acconut commented 3 years ago

Good idea! We do not use GCS much on our end, so we are not sure how other tools handle this! However, we are happy to accept PRs in order to make tusd work more uniform with the rest of the ecosystem (in a backwards compatible way) :)