Skip to content

Commit

Permalink
Require Content MD5 for SignedURL (flyteorg#393)
Browse files Browse the repository at this point in the history 
* Require Content MD5 for SignedURL

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* Send MD5 as []byte

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* Update deps

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* fix unit test

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* Revert local config change

Signed-off-by: Haytham Abuelfutuh <[email protected]>
  • Loading branch information
@EngHabu
EngHabu authored Apr 8, 2022
1 parent 0b18036 commit 2f49a5a
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 23 deletions.
25 changes: 16 additions & 9 deletions flyteadmin/dataproxy/service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package dataproxy

import (
"context"
"encoding/base64"
"fmt"
"strings"
"time"
Expand Down Expand Up @@ -37,6 +38,10 @@ func (s Service) CreateUploadLocation(ctx context.Context, req *service.CreateUp
return nil, fmt.Errorf("prjoect and domain are required parameters")
}

if len(req.ContentMd5) == 0 {
return nil, fmt.Errorf("content_md5 is a required parameter")
}

if expiresIn := req.ExpiresIn; expiresIn != nil {
if !expiresIn.IsValid() {
return nil, fmt.Errorf("expiresIn [%v] is invalid", expiresIn)
Expand All @@ -50,19 +55,21 @@ func (s Service) CreateUploadLocation(ctx context.Context, req *service.CreateUp
req.ExpiresIn = durationpb.New(s.cfg.Upload.MaxExpiresIn.Duration)
}

if len(req.Suffix) == 0 {
req.Suffix = rand.String(20)
if len(req.Filename) == 0 {
req.Filename = rand.String(s.cfg.Upload.DefaultFileNameLength)
}

storagePath, err := createShardedStorageLocation(ctx, req, s.shardSelector, s.dataStore, s.cfg.Upload)
md5 := base64.StdEncoding.EncodeToString(req.ContentMd5)
storagePath, err := createShardedStorageLocation(ctx, s.shardSelector, s.dataStore, s.cfg.Upload,
req.Project, req.Domain, md5, req.Filename)
if err != nil {
return nil, err
}

resp, err := s.dataStore.CreateSignedURL(ctx, storagePath, storage.SignedURLProperties{
Scope: stow.ClientMethodPut,
ExpiresIn: req.ExpiresIn.AsDuration(),
// TODO: pass max allowed upload size
Scope: stow.ClientMethodPut,
ExpiresIn: req.ExpiresIn.AsDuration(),
ContentMD5: md5,
})

if err != nil {
Expand All @@ -78,14 +85,14 @@ func (s Service) CreateUploadLocation(ctx context.Context, req *service.CreateUp

// createShardedStorageLocation creates a location in storage destination to maximize read/write performance in most
// block stores. The final location should look something like: s3://<my bucket>/<shard length>/<file name>
func createShardedStorageLocation(ctx context.Context, req *service.CreateUploadLocationRequest,
shardSelector ioutils.ShardSelector, store *storage.DataStore, cfg config.DataProxyUploadConfig) (storage.DataReference, error) {
func createShardedStorageLocation(ctx context.Context, shardSelector ioutils.ShardSelector, store *storage.DataStore,
cfg config.DataProxyUploadConfig, keyParts ...string) (storage.DataReference, error) {
keySuffixArr := make([]string, 0, 4)
if len(cfg.StoragePrefix) > 0 {
keySuffixArr = append(keySuffixArr, cfg.StoragePrefix)
}

keySuffixArr = append(keySuffixArr, req.Project, req.Domain, req.Suffix)
keySuffixArr = append(keySuffixArr, keyParts...)
prefix, err := shardSelector.GetShardPrefix(ctx, []byte(strings.Join(keySuffixArr, "/")))
if err != nil {
return "", err
Expand Down
9 changes: 4 additions & 5 deletions flyteadmin/dataproxy/service_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,11 @@ func Test_createShardedStorageLocation(t *testing.T) {
assert.NoError(t, err)
dataStore, err := storage.NewDataStore(&storage.Config{Type: storage.TypeMemory}, promutils.NewTestScope())
assert.NoError(t, err)
loc, err := createShardedStorageLocation(context.Background(), &service.CreateUploadLocationRequest{},
selector, dataStore, config.DataProxyUploadConfig{
StoragePrefix: "blah",
})
loc, err := createShardedStorageLocation(context.Background(), selector, dataStore, config.DataProxyUploadConfig{
StoragePrefix: "blah",
})
assert.NoError(t, err)
assert.Equal(t, "/4n/blah///", loc.String())
assert.Equal(t, "/u8/blah", loc.String())
}

func TestCreateUploadLocation(t *testing.T) {
Expand Down
6 changes: 3 additions & 3 deletions flyteadmin/go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ require (
github.com/cloudevents/sdk-go/v2 v2.8.0
github.com/coreos/go-oidc v2.2.1+incompatible
github.com/evanphx/json-patch v4.9.0+incompatible
github.com/flyteorg/flyteidl v0.24.10
github.com/flyteorg/flyteidl v0.24.15
github.com/flyteorg/flyteplugins v0.10.16
github.com/flyteorg/flytepropeller v0.16.36
github.com/flyteorg/flytestdlib v0.4.20
github.com/flyteorg/stow v0.3.1
github.com/flyteorg/flytestdlib v0.4.21
github.com/flyteorg/stow v0.3.3
github.com/ghodss/yaml v1.0.0
github.com/go-gormigrate/gormigrate/v2 v2.0.0
github.com/gogo/protobuf v1.3.2
Expand Down
12 changes: 6 additions & 6 deletions flyteadmin/go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -369,18 +369,18 @@ github.com/felixge/httpsnoop v1.0.1 h1:lvB5Jl89CsZtGIWuTcDM1E/vkVs49/Ml7JJe07l8S
github.com/felixge/httpsnoop v1.0.1/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U=
github.com/flyteorg/flyteidl v0.23.0/go.mod h1:576W2ViEyjTpT+kEVHAGbrTP3HARNUZ/eCwrNPmdx9U=
github.com/flyteorg/flyteidl v0.24.0/go.mod h1:576W2ViEyjTpT+kEVHAGbrTP3HARNUZ/eCwrNPmdx9U=
github.com/flyteorg/flyteidl v0.24.10 h1:fCYpfp5fxKbhRMSkP0Hdw5lOPBTItLU1A3WA1Lc7sEU=
github.com/flyteorg/flyteidl v0.24.10/go.mod h1:vHSugApgS3hRITIafzQDU8DZD/W8wFRfFcgaFU35Dww=
github.com/flyteorg/flyteidl v0.24.15 h1:Iqbwx3w1a4Dh6byRZrZMlHsKPKoOZbBiS9vR0iXzacY=
github.com/flyteorg/flyteidl v0.24.15/go.mod h1:vHSugApgS3hRITIafzQDU8DZD/W8wFRfFcgaFU35Dww=
github.com/flyteorg/flyteplugins v0.10.16 h1:rwNI2MACPbcST2O6CEUsNW6bccz7ZLni0GiY3orevfw=
github.com/flyteorg/flyteplugins v0.10.16/go.mod h1:YBWV8QnFakDJfLyua8pYddiWqszAqseBKIJPNMERlos=
github.com/flyteorg/flytepropeller v0.16.36 h1:5uE8JsutrPVyLVrRJ8BgvhZUOmTBFkEkn5xmIOo21nU=
github.com/flyteorg/flytepropeller v0.16.36/go.mod h1:DGCjQSRp8VYOBH56aQyAZfNf1Vgh+GNpwQL7uhottYM=
github.com/flyteorg/flytestdlib v0.3.13/go.mod h1:Tz8JCECAbX6VWGwFT6cmEQ+RJpZ/6L9pswu3fzWs220=
github.com/flyteorg/flytestdlib v0.4.13/go.mod h1:fv1ar34LJLMTaf0tbfetisLykUlARi7rP+NQTUn6QQs=
github.com/flyteorg/flytestdlib v0.4.20 h1:ehRk6YSdXhv5E5xtijXxIeVs7vFSlWGjvKvp+A2vzW0=
github.com/flyteorg/flytestdlib v0.4.20/go.mod h1:OOYar6yVopFcMIkXDtghmRfjcWxASC+a0ge2m6arLHw=
github.com/flyteorg/stow v0.3.1 h1:cBMbWl03Gsy5KoA5mutUYTuYpqtT7Pb8+ANGCLnmFEs=
github.com/flyteorg/stow v0.3.1/go.mod h1:HBld7ud0i4khMHwJjkO8v+NSP7ddKa/ruhf4I8fliaA=
github.com/flyteorg/flytestdlib v0.4.21 h1:IiIYzxwSehtFXH68EJC2J/6n2duemOZkn5LRVSw4YiE=
github.com/flyteorg/flytestdlib v0.4.21/go.mod h1:QSVN5wIM1lM9d60eAEbX7NwweQXW96t5x4jbyftn89c=
github.com/flyteorg/stow v0.3.3 h1:tzeNl8mSZFL3oJDi0ACZj6FAineQAF4qyEp6bXtIdQY=
github.com/flyteorg/stow v0.3.3/go.mod h1:HBld7ud0i4khMHwJjkO8v+NSP7ddKa/ruhf4I8fliaA=
github.com/fogleman/gg v1.2.1-0.20190220221249-0403632d5b90/go.mod h1:R/bRT+9gY/C5z7JzPU0zXsXHKM4/ayA+zqcVNZzPa1k=
github.com/fogleman/gg v1.3.0/go.mod h1:R/bRT+9gY/C5z7JzPU0zXsXHKM4/ayA+zqcVNZzPa1k=
github.com/form3tech-oss/jwt-go v3.2.2+incompatible h1:TcekIExNqud5crz4xD2pavyTgWiPvpYe4Xau31I0PRk=
Expand Down

0 comments on commit 2f49a5a

Please sign in to comment.