Fix bug in credential overriding in backup. #4047
Conversation
Right now, the credentials were only being used in a part of the process, which meant the process would fail if the default credentials are invalid or expired. This rewrites the handler to accept the credentials before the process starts. Manually checked that overriding now works as expected by setting the default credentials incorrectly and getting the backup to work by overriding the credentials.
![pullrequest[bot]](https://avatars.githubusercontent.com/in/3434?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ A review job has been created and sent to the PullRequest network.
@martinmr you can click here to see the review status or cancel the code review job.
There was a problem hiding this comment.
Nice work! LGTM. This is good clean-up/refactoring. Makes it well-encapsulated and easier to read.
Reviewed with ❤️ by PullRequest
There was a problem hiding this comment.
Reviewed 5 of 5 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @martinmr)
ee/backup/s3_handler.go, line 58 at r1 (raw file):
} func (h *s3Handler) overrideCredentials() bool {
hasCredentials?
There was a problem hiding this comment.
Reviewable status: 4 of 5 files reviewed, 1 unresolved discussion (waiting on @manishrjain)
ee/backup/s3_handler.go, line 58 at r1 (raw file):
Previously, manishrjain (Manish R Jain) wrote…
hasCredentials?
Done.
Right now, the credentials were only being used in a part of the
process, which meant the process would fail if the default credentials
are invalid or expired.
This rewrites the handler to accept the credentials before the process
starts.
Manually checked that overriding now works as expected by setting the
default credentials incorrectly and getting the backup to work by
overriding the credentials.
Fixes #4044
This change is