Support for private containers with secret key gate
ClosedPublic

Authored by Gilles on Nov 17 2017, 9:59 AM.

Details

Maniphest Tasks
T169144: Serve thumb.php requests with Thumbor
Reviewers
fgiunchedi
Commits
rTHMBREXT997c67d7fbf8: Support for private containers with secret key gate
Patch without arc
git checkout -b D886 && curl -L https://phabricator.wikimedia.org/D886?download=true | git apply
Summary

Refs T169144

Diff Detail

Repository
rTHMBREXT Thumbor Plugins
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
Gilles created this revision.Nov 17 2017, 9:59 AM
fgiunchedi added inline comments.Nov 27 2017, 11:30 AM
wikimedia_thumbor/handler/images/images.py
202

This option will need some explanation in the config to e.g. point out that container names will be considered without their shard. IIRC we don't have sharded and private containers now, but might in the future.

386

We should do some validation of PRIVATE_SECRET and PRIVATE_CONTAINERS perhaps at configuration load time, off top of my head:

  • empty secret is an error
  • both secret and containers should be set (and non-empty), otherwise it is an error

I never got an email from Phabricator about your feedback, that sucks. I wonder if it's because of this new build failed status.

In D886#18081, @Gilles wrote:

I never got an email from Phabricator about your feedback, that sucks. I wonder if it's because of this new build failed status.

Could be because of build status indeed, maybe @20after4 or @hashar can shed some light if that's actually the case (i.e. build failed == no feedback from reviewers sent)

Gilles updated this revision to Diff 2439.Dec 20 2017, 8:37 AM
Gilles marked an inline comment as done.

Add empty secret check

wikimedia_thumbor/handler/images/images.py
202

Will do

Gilles updated this revision to Diff 2467.Jan 3 2018, 1:21 PM

Rebase

Gilles marked 2 inline comments as done.Jan 3 2018, 1:21 PM
Gilles requested review of this revision.Jan 3 2018, 1:21 PM
fgiunchedi accepted this revision.Jan 4 2018, 9:31 AM

Nit about header name, LGTM otherwise

wikimedia_thumbor/handler/images/images.py
385

Perhaps a more specific header name, e.g. X-Swift-Secret since all of this is swift specific

This revision is now accepted and ready to land.Jan 4 2018, 9:31 AM
This revision was automatically updated to reflect the committed changes.
Gilles marked an inline comment as done.Jan 6 2018, 12:44 PM