feat(vfs): add Linode (Akamai) object storage schema support#18138
feat(vfs): add Linode (Akamai) object storage schema support#18138k8s-ci-robot merged 1 commit intokubernetes:masterfrom
Conversation
|
Skipping CI for Draft Pull Request. |
4a37174 to
0c9db52
Compare
|
/test all |
| // Some S3-compatible backends (including Linode Object Storage) do not | ||
| // implement GetObjectAcl. In that case we conservatively treat the object | ||
| // as non-public and continue. | ||
| if AWSErrorCode(err) == "NotImplemented" { | ||
| klog.V(2).Infof("GetObjectAcl not implemented for %q, treating object as non-public", p) | ||
| return false, nil | ||
| } |
There was a problem hiding this comment.
During my testing (I think it was as part of nodeup, but can't recall at that point because it's been a couple of days), it failed to start because of errors while trying to pull one of the binaries from my storage bucket with this error. Our object storage API doesn't have GetObjectAcl implemented so I needed to work around it.
Does that make sense?
There was a problem hiding this comment.
Please move to the beginning of the function and return early if p.scheme == "linode".
I understand why you need this, but probably we need to look at a different approach for later.
There was a problem hiding this comment.
Sounds good! I'll move it now.
0c9db52 to
31b673d
Compare
|
/test all |
| if p.scheme == "linode" { | ||
| // Linode (Akamai) does not implement GetObjectAcl. In that case we conservatively treat the object as non-public and continue. | ||
| return false, nil | ||
| } |
There was a problem hiding this comment.
This should be the first thing in the function, there is no need to do any other work before returning this.
In the future, we can add a secondary check for bucket level ACL, but not needed for now.
Signed-off-by: Moshe Vayner <moshe@vayner.me>
31b673d to
21de341
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hakman The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/test all |
|
Thanks for the review @hakman ! ❤️ |
|
/retest |
What this PR does / why we need it:
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Special notes for your reviewer: