Add other backends (HTTP/WebDAV, Globus, S3/blob) to internal Pelican#3234
Add other backends (HTTP/WebDAV, Globus, S3/blob) to internal Pelican#3234bbockelm wants to merge 10 commits into
Conversation
fd6656b to
188f86d
Compare
a05ebe9 to
efff8da
Compare
efff8da to
bd2536c
Compare
b4baff2 to
dc87f72
Compare
turetske
left a comment
There was a problem hiding this comment.
Please deal with the merge conflicts as well.
| // If per-export S3 credentials were provided, set them in the environment | ||
| // so the gocloud AWS credential chain picks them up. | ||
| // | ||
| // FIXME(deploy): this mutates global process environment, which has two |
There was a problem hiding this comment.
Is there an existing issue for this FIXME? Otherwise please create one as I worry this will just get lost.
|
|
||
| if server_structs.OriginStorageType(param.Origin_StorageType.GetString()) != server_structs.OriginStorageGlobus { | ||
| return errors.Errorf("failed to initialize Globus backend: Origin.StorageType is not Globus: %s", | ||
| ost := server_structs.OriginStorageType(param.Origin_StorageType.GetString()) |
There was a problem hiding this comment.
Small nitpick, but xrootdGid doesn't apply if using OriginStorageGlobusv2 and should be behind a conditional, I think.
| oldKey := blobKey(oldName) | ||
| newKey := blobKey(newName) | ||
|
|
||
| if err := fs.bucket.Copy(ctx, newKey, oldKey, nil); err != nil { |
There was a problem hiding this comment.
This isn't a recursive copy. I believe this will rename oldKey to newKey, but any child objects in the collection will be under oldKey. Which is obviously bad. If this will ever be used on a collection rather than an individual "leaf" object, it needs to iterate recursively through and do a copy/delete.
| // Handle POSIXv2 and SSH-specific initialization now that the web server is running | ||
| storageType := param.Origin_StorageType.GetString() | ||
| useXRootD := storageType != string(server_structs.OriginStoragePosixv2) && storageType != string(server_structs.OriginStorageSSH) | ||
| useXRootD := storageType != string(server_structs.OriginStoragePosixv2) && |
There was a problem hiding this comment.
This is an identical repeated code segment to lines 59-63. Should useXRootD become part of a helper function to avoid repeated code?
|
|
||
| // globusIssuerURL returns the configurable Globus OIDC issuer URL. | ||
| func globusIssuerURL() string { | ||
| if v := param.Origin_GlobusIssuerURL.GetString(); v != "" { |
There was a problem hiding this comment.
This is set in defaults.yaml, is it possible for it to be an empty string? Unless someone sets it explicitly, which seems odd.
|
|
||
| // globusTransferAPIBaseURL returns the configurable Globus Transfer API base URL. | ||
| func globusTransferAPIBaseURL() string { | ||
| if v := param.Origin_GlobusTransferAPIBaseUrl.GetString(); v != "" { |
| github.com/youmark/pkcs8 v0.0.0-20201027041543-1326539a0a0a | ||
| github.com/zsais/go-gin-prometheus v0.1.0 | ||
| go.uber.org/atomic v1.11.0 | ||
| gocloud.dev v0.45.0 |
There was a problem hiding this comment.
So, I think this is pretty large. Since we are splitting the client/server binaries (and that's merged into main already, but may not be part of this PR) and this should only be in the server binaries, make sure this dependency is only pulled in there as the whole point of the split is to reduce the client binary size.
| if _, peekErr := peekIter.Next(ctx); peekErr == nil { | ||
| // It is a directory — return a lazy dir handle (a fresh iterator | ||
| // will be created when Readdir is called). | ||
| return &blobDirFile{name: name, bucket: fs.bucket, prefix: dirPrefix}, nil |
There was a problem hiding this comment.
Should this store the context for the later Readdir call?
|
|
||
| func (fi *blobFileInfo) Name() string { return fi.name } | ||
| func (fi *blobFileInfo) Size() int64 { return fi.size } | ||
| func (fi *blobFileInfo) Mode() os.FileMode { return 0444 } |
There was a problem hiding this comment.
Are we always read only? If so, this is fine, but I want to check.
dc87f72 to
330d1b0
Compare
- Do not chown Globus files if we aren't accessing via XRootD - Use simpler detection of whether XRootD is in use. - Switch secret keys from being injected via environment to explicitly (and redacting, as needed) on open.
1532734 to
2222182
Compare
This is an initial draft of moving other backends to be embedded inside the Pelican process. Quite a bit left to go for code review and testing - but I thought it'd be good to open for other eyeballs ASAP.
@whwjiang - thought you'd have fun playing with this one.