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.

Sign up for GitHub

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

Add extra header to signed url #4971

Merged
merged 24 commits into from
Mar 12, 2024
Merged

Add extra header to signed url #4971

Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
908171f
Add extra header to signed url
pingsutw Feb 28, 2024
08e23b2
nit
pingsutw Feb 28, 2024
2ddce9f
AddMetadata
pingsutw Feb 28, 2024
4d749fc
Make generate
pingsutw Feb 28, 2024
2c2aceb
test
pingsutw Feb 28, 2024
c4ece1e
revert
pingsutw Feb 28, 2024
705b776
nit
pingsutw Feb 28, 2024
671c255
unit test
pingsutw Feb 28, 2024
3bf30c2
nit
pingsutw Feb 28, 2024
660cfd0
Merge branch 'master' of github.com:flyteorg/flyte into fix-azure-upload
pingsutw Feb 28, 2024
112b9e0
nit
pingsutw Feb 28, 2024
36d899b
lint
pingsutw Feb 28, 2024
6f461cc
address comment
pingsutw Feb 29, 2024
393db9e
Merge branch 'master' of github.com:flyteorg/flyte into fix-azure-upload
pingsutw Mar 1, 2024
6043a7e
update logger
pingsutw Mar 5, 2024
597f07c
update idl
pingsutw Mar 5, 2024
f5fd24e
wip
pingsutw Mar 5, 2024
19eb056
update stow
pingsutw Mar 11, 2024
6e3a84b
update stow
pingsutw Mar 11, 2024
d9a8027
fix tests
pingsutw Mar 11, 2024
f1153ad
remove stow
pingsutw Mar 11, 2024
7ec484b
update test
pingsutw Mar 11, 2024
60721be
lint
pingsutw Mar 11, 2024
afe2bf6
lint
pingsutw Mar 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion datacatalog/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ require (
github.com/emicklei/go-restful/v3 v3.9.0 // indirect
github.com/evanphx/json-patch/v5 v5.6.0 // indirect
github.com/fatih/color v1.13.0 // indirect
github.com/flyteorg/stow v0.3.8 // indirect
github.com/flyteorg/stow v0.3.9 // indirect
github.com/fsnotify/fsnotify v1.6.0 // indirect
github.com/ghodss/yaml v1.0.0 // indirect
github.com/go-gormigrate/gormigrate/v2 v2.1.1 // indirect
Expand Down
4 changes: 2 additions & 2 deletions datacatalog/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,8 @@ github.com/evanphx/json-patch/v5 v5.6.0 h1:b91NhWfaz02IuVxO9faSllyAtNXHMPkC5J8sJ
github.com/evanphx/json-patch/v5 v5.6.0/go.mod h1:G79N1coSVB93tBe7j6PhzjmR3/2VvlbKOFpnXhI9Bw4=
github.com/fatih/color v1.13.0 h1:8LOYc1KYPPmyKMuN8QV2DNRWNbLo6LZ0iLs8+mlH53w=
github.com/fatih/color v1.13.0/go.mod h1:kLAiJbzzSOZDVNGyDpeOxJ47H46qBXwg5ILebYFFOfk=
github.com/flyteorg/stow v0.3.8 h1:4a6BtfgDR86fUwa48DkkZTcp6WK4oQXSfewPd/kN0Z4=
github.com/flyteorg/stow v0.3.8/go.mod h1:fArjMpsYJNWkp/hyDKKdbcv07gxbuLmKFcb7YT1aSOM=
github.com/flyteorg/stow v0.3.9 h1:oA9tKMSvdCBnTnVpx8vncwrjRf8h/w8Ks9c4VGm2Img=
github.com/flyteorg/stow v0.3.9/go.mod h1:fArjMpsYJNWkp/hyDKKdbcv07gxbuLmKFcb7YT1aSOM=
github.com/fsnotify/fsnotify v1.6.0 h1:n+5WquG0fcWoWp6xPWfHdbskMCQaFnG6PfBrh1Ky4HY=
github.com/fsnotify/fsnotify v1.6.0/go.mod h1:sl3t1tCWJFWoRz9R8WJCbQihKKwmorjAbSClcnxKAGw=
github.com/ghodss/yaml v1.0.0 h1:wQHKEahhL6wmXdzwWG11gIVCkOv05bNOh+Rxn0yngAk=
Expand Down
32 changes: 23 additions & 9 deletions flyteadmin/dataproxy/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,13 @@ func (s Service) CreateUploadLocation(ctx context.Context, req *service.CreateUp
// If it doesn't exist, then proceed as normal.

if len(req.Project) == 0 || len(req.Domain) == 0 {
logger.Infof(ctx, "project and domain are required parameters. Project [%v]. Domain [%v]", req.Project, req.Domain)
return nil, errors.NewFlyteAdminErrorf(codes.InvalidArgument, "project and domain are required parameters")
}

// At least one of the hash or manually given prefix must be provided.
if len(req.FilenameRoot) == 0 && len(req.ContentMd5) == 0 {
logger.Infof(ctx, "content_md5 or filename_root is a required parameter. FilenameRoot [%v], ContentMD5 [%v]", req.FilenameRoot, req.ContentMd5)
return nil, errors.NewFlyteAdminErrorf(codes.InvalidArgument,
"content_md5 or filename_root is a required parameter")
}
Expand All @@ -63,10 +65,12 @@ func (s Service) CreateUploadLocation(ctx context.Context, req *service.CreateUp
knownLocation, err := createStorageLocation(ctx, s.dataStore, s.cfg.Upload,
req.Project, req.Domain, req.FilenameRoot, req.Filename)
if err != nil {
logger.Errorf(ctx, "failed to create storage location. Error %v", err)
return nil, errors.NewFlyteAdminErrorf(codes.Internal, "failed to create storage location, Error: %v", err)
}
metadata, err := s.dataStore.Head(ctx, knownLocation)
if err != nil {
logger.Errorf(ctx, "failed to check if file exists at location [%s], Error: %v", knownLocation.String(), err)
return nil, errors.NewFlyteAdminErrorf(codes.Internal, "failed to check if file exists at location [%s], Error: %v", knownLocation.String(), err)
}
if metadata.Exists() {
Expand All @@ -76,12 +80,18 @@ func (s Service) CreateUploadLocation(ctx context.Context, req *service.CreateUp
if len(req.ContentMd5) == 0 {
return nil, errors.NewFlyteAdminErrorf(codes.AlreadyExists, "file already exists at location [%v], specify a matching hash if you wish to rewrite", knownLocation)
}
// Re-encode the hash 3-ways to support matching, hex, base32 and base64
hexDigest := hex.EncodeToString(req.ContentMd5)
base32Digest := base32.StdEncoding.EncodeToString(req.ContentMd5)
base64Digest := base64.StdEncoding.EncodeToString(req.ContentMd5)
if hexDigest != metadata.Etag() && base32Digest != metadata.Etag() && base64Digest != metadata.Etag() {
logger.Debugf(ctx, "File already exists at location [%v] but hashes do not match", knownLocation)
if len(metadata.ContentMD5()) == 0 {
// For backward compatibility, dataproxy assumes that the Etag exists if ContentMD5 is not in the metadata.
// Data proxy won't allow people to overwrite the file if both the Etag and the ContentMD5 do not exist.
hexDigest := hex.EncodeToString(req.ContentMd5)
base32Digest := base32.StdEncoding.EncodeToString(req.ContentMd5)
if hexDigest != metadata.Etag() && base32Digest != metadata.Etag() && base64Digest != metadata.Etag() {
logger.Errorf(ctx, "File already exists at location [%v] but hashes do not match", knownLocation)
return nil, errors.NewFlyteAdminErrorf(codes.AlreadyExists, "file already exists at location [%v], specify a matching hash if you wish to rewrite", knownLocation)
}
} else if base64Digest != metadata.ContentMD5() {
logger.Errorf(ctx, "File already exists at location [%v] but hashes do not match", knownLocation)
return nil, errors.NewFlyteAdminErrorf(codes.AlreadyExists, "file already exists at location [%v], specify a matching hash if you wish to rewrite", knownLocation)
}
logger.Debugf(ctx, "File already exists at location [%v] but allowing rewrite", knownLocation)
Expand All @@ -105,7 +115,7 @@ func (s Service) CreateUploadLocation(ctx context.Context, req *service.CreateUp
req.Filename = rand.String(s.cfg.Upload.DefaultFileNameLength)
}

md5 := base64.StdEncoding.EncodeToString(req.ContentMd5)
base64digestMD5 := base64.StdEncoding.EncodeToString(req.ContentMd5)

var prefix string
if len(req.FilenameRoot) > 0 {
Expand All @@ -117,23 +127,27 @@ func (s Service) CreateUploadLocation(ctx context.Context, req *service.CreateUp
storagePath, err := createStorageLocation(ctx, s.dataStore, s.cfg.Upload,
req.Project, req.Domain, prefix, req.Filename)
if err != nil {
logger.Errorf(ctx, "failed to create shardedStorageLocation. Error %v", err)
return nil, errors.NewFlyteAdminErrorf(codes.Internal, "failed to create shardedStorageLocation, Error: %v", err)
}

resp, err := s.dataStore.CreateSignedURL(ctx, storagePath, storage.SignedURLProperties{
Scope: stow.ClientMethodPut,
ExpiresIn: req.ExpiresIn.AsDuration(),
ContentMD5: md5,
Scope: stow.ClientMethodPut,
ExpiresIn: req.ExpiresIn.AsDuration(),
ContentMD5: base64digestMD5,
AddContentMD5Metadata: req.AddContentMd5Metadata,
})

if err != nil {
logger.Errorf(ctx, "failed to create signed url. Error:", err)
return nil, errors.NewFlyteAdminErrorf(codes.Internal, "failed to create a signed url. Error: %v", err)
}

return &service.CreateUploadLocationResponse{
SignedUrl: resp.URL.String(),
NativeUrl: storagePath.String(),
ExpiresAt: timestamppb.New(time.Now().Add(req.ExpiresIn.AsDuration())),
Headers: resp.RequiredRequestHeaders,
}, nil
}

Expand Down
14 changes: 14 additions & 0 deletions flyteadmin/dataproxy/service_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,3 +422,17 @@ func TestService_Error(t *testing.T) {
assert.Error(t, err, "no task executions")
})
}

func TestGetExtraHeader(t *testing.T) {
reference := storage.DataReference("s3://bucket/key")
headers := getExtraHeaders(reference, "md5", true)
assert.Equal(t, map[string]string{"Content-MD5": "md5", "Content-Length": "3", "x-amz-meta-flyteContentMD5": "md5"}, headers)

reference = "gs://bucket/key"
headers = getExtraHeaders(reference, "md5", true)
assert.Equal(t, map[string]string{"Content-MD5": "md5", "Content-Length": "3", "x-goog-meta-flyteContentMD5": "md5"}, headers)

reference = "abfs://bucket/key"
headers = getExtraHeaders(reference, "md5", true)
assert.Equal(t, map[string]string{"Content-MD5": "md5", "Content-Length": "3", "x-ms-meta-flyteContentMD5": "md5", "x-ms-blob-type": "BlockBlob"}, headers)
}
2 changes: 1 addition & 1 deletion flyteadmin/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ require (
github.com/flyteorg/flyte/flyteplugins v0.0.0-00010101000000-000000000000
github.com/flyteorg/flyte/flytepropeller v0.0.0-00010101000000-000000000000
github.com/flyteorg/flyte/flytestdlib v0.0.0-00010101000000-000000000000
github.com/flyteorg/stow v0.3.8
github.com/flyteorg/stow v0.3.9
github.com/ghodss/yaml v1.0.0
github.com/go-gormigrate/gormigrate/v2 v2.1.1
github.com/golang-jwt/jwt/v4 v4.5.0
Expand Down
4 changes: 2 additions & 2 deletions flyteadmin/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,8 +237,8 @@ github.com/fatih/structs v1.1.0/go.mod h1:9NiDSp5zOcgEDl+j00MP/WkGVPOlPRLejGD8Ga
github.com/felixge/httpsnoop v1.0.1/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U=
github.com/felixge/httpsnoop v1.0.3 h1:s/nj+GCswXYzN5v2DpNMuMQYe+0DDwt5WVCU6CWBdXk=
github.com/felixge/httpsnoop v1.0.3/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U=
github.com/flyteorg/stow v0.3.8 h1:4a6BtfgDR86fUwa48DkkZTcp6WK4oQXSfewPd/kN0Z4=
github.com/flyteorg/stow v0.3.8/go.mod h1:fArjMpsYJNWkp/hyDKKdbcv07gxbuLmKFcb7YT1aSOM=
github.com/flyteorg/stow v0.3.9 h1:oA9tKMSvdCBnTnVpx8vncwrjRf8h/w8Ks9c4VGm2Img=
github.com/flyteorg/stow v0.3.9/go.mod h1:fArjMpsYJNWkp/hyDKKdbcv07gxbuLmKFcb7YT1aSOM=
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/fortytw2/leaktest v1.3.0 h1:u8491cBMTQ8ft8aeV+adlcytMZylmA5nnwwkRZjI8vw=
Expand Down
2 changes: 1 addition & 1 deletion flytecopilot/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ require (
github.com/emicklei/go-restful/v3 v3.9.0 // indirect
github.com/evanphx/json-patch/v5 v5.6.0 // indirect
github.com/fatih/color v1.13.0 // indirect
github.com/flyteorg/stow v0.3.8 // indirect
github.com/flyteorg/stow v0.3.9 // indirect
github.com/go-logr/logr v1.2.4 // indirect
github.com/go-logr/stdr v1.2.2 // indirect
github.com/go-openapi/jsonpointer v0.19.6 // indirect
Expand Down
4 changes: 2 additions & 2 deletions flytecopilot/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ github.com/evanphx/json-patch/v5 v5.6.0 h1:b91NhWfaz02IuVxO9faSllyAtNXHMPkC5J8sJ
github.com/evanphx/json-patch/v5 v5.6.0/go.mod h1:G79N1coSVB93tBe7j6PhzjmR3/2VvlbKOFpnXhI9Bw4=
github.com/fatih/color v1.13.0 h1:8LOYc1KYPPmyKMuN8QV2DNRWNbLo6LZ0iLs8+mlH53w=
github.com/fatih/color v1.13.0/go.mod h1:kLAiJbzzSOZDVNGyDpeOxJ47H46qBXwg5ILebYFFOfk=
github.com/flyteorg/stow v0.3.8 h1:4a6BtfgDR86fUwa48DkkZTcp6WK4oQXSfewPd/kN0Z4=
github.com/flyteorg/stow v0.3.8/go.mod h1:fArjMpsYJNWkp/hyDKKdbcv07gxbuLmKFcb7YT1aSOM=
github.com/flyteorg/stow v0.3.9 h1:oA9tKMSvdCBnTnVpx8vncwrjRf8h/w8Ks9c4VGm2Img=
github.com/flyteorg/stow v0.3.9/go.mod h1:fArjMpsYJNWkp/hyDKKdbcv07gxbuLmKFcb7YT1aSOM=
github.com/fsnotify/fsnotify v1.6.0 h1:n+5WquG0fcWoWp6xPWfHdbskMCQaFnG6PfBrh1Ky4HY=
github.com/fsnotify/fsnotify v1.6.0/go.mod h1:sl3t1tCWJFWoRz9R8WJCbQihKKwmorjAbSClcnxKAGw=
github.com/ghodss/yaml v1.0.0 h1:wQHKEahhL6wmXdzwWG11gIVCkOv05bNOh+Rxn0yngAk=
Expand Down
18 changes: 18 additions & 0 deletions flyteidl/gen/pb-es/flyteidl/service/dataproxy_pb.ts
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading