providers/aws: Add support for S3 website redirect #1909
Conversation
| redirectAllRequestsTo := website["redirect_all_requests_to"].(string) | ||
|
|
||
| if indexDocument == "" && redirectAllRequestsTo == "" { | ||
| return fmt.Errorf("Must specify either index_document or redirect_all_requests_to.") |
There was a problem hiding this comment.
Is there a way to enforce this in the schema?
|
I'd be happy to add protocol field as well, I just didn't know how often it's used, and how to name the variables: |
|
It's also worth mentioning that the AWS console UI doesn't expose the protocol portion of the API: https://docs.aws.amazon.com/AmazonS3/latest/dev/website-hosting-custom-domain-walkthrough.html |
justincampbell
force-pushed
the
s3-redirect
branch
from
025bbf4
to
bffe0b0
Compare
justincampbell
force-pushed
the
s3-redirect
branch
from
bffe0b0
to
bbe0012
Compare
| ConflictsWith: []string{ | ||
| "website.0.index_document", | ||
| "website.0.error_document", | ||
| }, |
There was a problem hiding this comment.
The build is failing because InternalValidate in helper/schema/schema.go doesn't support this nested notation for ConflictsWith, even though in usage it seems to work correctly.
There was a problem hiding this comment.
Being the original author of ConflictsWith, I know about this limitation, but I was finding it hard to create any nested schema (can't remember what was the problem exactly though), putting aside ConflictsWith.
There was a problem hiding this comment.
Shouldn't the website actually be TypeSet?
There was a problem hiding this comment.
@radeksimko we settled on list vs set for this, but I forget why
There was a problem hiding this comment.
Fixed with #1926!
The runtime impl of ConfictsWith uses Resource.Get(), which makes it work with any other attribute of the resource - the InternalValidate was only checking against the local schemaMap though, preventing subResource from using ConflictsWith properly. It's a lot of wiring and it's a bit ugly, but it's not runtime code, so I'm a bit less concerned about that aspect. This should take care of the problem mentioned in #1909
justincampbell
force-pushed
the
s3-redirect
branch
from
bbe0012
to
fd06b5a
Compare
justincampbell
force-pushed
the
s3-redirect
branch
from
fd06b5a
to
b8cd6b2
Compare
|
LGTM |
providers/aws: Add support for S3 website redirect
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
ghost
locked and limited conversation to collaborators
This adds support for redirecting all requests for an S3 website endpoint to another hostname.
The API supports both a hostname and protocol, but this only adds the hostname field.