Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
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
services/horizon/docker/ledgerexporter: deploy ledgerexporter image as service #4490
services/horizon/docker/ledgerexporter: deploy ledgerexporter image as service #4490
Changes from 1 commit
846c6fc
cdd60d5
2cc3a3d
b1589f0
466b366
433ed4f
ffd3990
873e79f
b56e089
2c8589c
f0321b6
6c95fe8
452b20c
eebaedd
a02547b
415950e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i've config'd the horizon-ledgermeta-pubnet bucket in same AWS account as Batch, with
bucket owner enforced
withACLs disabled
and a inline policy that defines allowed/disallowed statements, following aws recommendations .There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean you changed the existing bucket settings?
Unfortunately the S3-writing code (HistoryArchive) code assumes ACLs are enabled (because it writes them). That's why they where enabled.
I am not against the change but then we should also change the writing code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't changed any existing bucket permissions, but this looks fairly constrained on ACL usage though, only one place in s3 writing code sets the object ACL to include
public read
during put which I removed and left a comment on one potential to migrate off that.. There are several other places where s3 object writing(puts) are done with s3uploadmanager and those don't specify ACLs.So, the net seems to be in identifying how many existing buckets which could have been written to by this routine in
historyarchive/s3_archive.go
. With just removal of ACL here, the net effect is that put still works to the existing buckets but the objects won't have public read until the bucket permissions are updated to have ACL's disabled and a policy added with statement for AllowEveryone/Public Read
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated approach and added ACL config option on s3 - 452b20c, this way client can work with buckets in either permissions config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@2opremio , I added more to permissions on
horizon-ledgermeta-prodnet-test
andhorizon-index
, added policy to grant write to ec2(Batch access) and an IAM user(k8s access) and a PublicRead rule.I noticed slight diff on their main config, horizon-ledgermeta-prodnet-test had
ACLs enabled/bucket owner preferred
, and horizon-index hadACLs disabled/bucket owner enforced
. The policy applies on top of either.