Create pytorch-ci-artifacts s3 bucket #33
Conversation
There was a problem hiding this comment.
LGTM, but it would be good to have test plan of sorts. For example, I don't see where it says, that it would apply public read policy to all objects created in this bucket by default (though it would not matter much right now, as it's not used to store anything yet)
Create the pytorch-ci-artifacts s3 bucket for storing build artifacts and apply the necessary access policy to the build nodes for the actions runners. This creates a publicly available S3 Bucket for read access to anonymous users. While requiring OIDC authentication for write access by GitHub Actions. Issue: #34 Signed-off-by: Thanh Ha <thanh.ha@linuxfoundation.org>
zxiiro
force-pushed
the
zxiiro/s3-artifacts
branch
from
2249692 to
b95e7dd
Compare
I was working on that this morning (sorry the issue #34 was created after this PR and I had further discussions with @jeanschmidt on Slack). The updated PR creates a public access S3 Bucket for read access. Write access is permitted through OIDC authentication which the GitHub Actions pushing to this will need to handle via the configure-aws-credentials action.. |
| } | ||
| } | ||
|
|
||
| resource "aws_s3_bucket_website_configuration" "pytorch_ci_artifacts_website" { |
There was a problem hiding this comment.
This makes a publicly available web URL for the S3 bucket to download artifacts from. We purposely disallow navigating the website to prevent snooping so users will need to know the direct URL for the artifacts they want to access.
What typically I've seen other projects do is create a index.html to list all of the artifacts for the build and store the build URL for the S3 bucket to that index file so that there's only 1 URL per build to remember and download the rest of the artifacts from.
|
LGTM, as a slight improvement I would rename the file to Still this is a organizational decision I don't have strong view. But we need to pick a standard, if we go by this layout might be important to move everything to its own file mechanic-related instead of by type-related. |
I don't have a strong opinion either but I do like that everything related to this specific S3 Bucket is in 1 file. Less jumping around to figure out how this bucket is configured. Especially because this is a global bucket that needs to be a well known resource. |
Create the pytorch-ci-artifacts s3 bucket for storing build artifacts and apply the necessary access policy to the build nodes for the actions runners.
Issue: #34