Change Details

Ref T12509. This upgrades a `weakDigest()` callsite to SHA256-HMAC and removes three config options: - `celerity.resource-hash`: Now generated automatically by the SHA256-HMAC code. You could go purge it by touching `auth.auth_hmackey` directly, but we've never actually directed any users to fiddle with this, I believe. - `celerity.enable-deflate`: Intended to make cache inspection easier, but we haven't needed to inspect caches in ~forever. - `celerity.minify`: Intended to make debugging minification easier, but we haven't needed to debug this in ~forever. In the latter two cases, the options were purely developer-focused, and it's easy to go add an `&& false` somewhere in the code if we need to disable these features to debug something, but the relevant parts of the code basically work properly and never need debugging. These options were excessively paranoid, based on the static resource enviroment at Facebook being far more perilous. The first case theoretically had end-user utility for fixing stuck content caches, but we'd be better off providing a `bin/celerity bad-stuff-got-cached-oops` command that regenerates the HMAC key if we actually need this -- in modern Phabricator, it's not intuitive that you'd go adjust a Config option to fix this. I don't recall any users ever actually running into problems here, though. Note that this creates one odd side effect: we have a unit test which makes sure the Celerity map is up to date. This unit test can access config, but can not access the database, so it can no longer generate a new map because it must now access the database to read the HMAC key. I think this unit test is somewhat questionable and intend to just move "map rebuild" to a "merge action / during-land" sort of step, but for now: make it work by embedding the HMAC key which was used to generate the map into the map itself. This generally seems reasonable/sensible.

Ref T12509. This upgrades a `weakDigest()` callsite to SHA256-HMAC and removes three config options: - `celerity.resource-hash`: Now hard-coded, since the use case for ever adjusting it was very weak. - `celerity.enable-deflate`: Intended to make cache inspection easier, but we haven't needed to inspect caches in ~forever. - `celerity.minify`: Intended to make debugging minification easier, but we haven't needed to debug this in ~forever. In the latter two cases, the options were purely developer-focused, and it's easy to go add an `&& false` somewhere in the code if we need to disable these features to debug something, but the relevant parts of the code basically work properly and never need debugging. These options were excessively paranoid, based on the static resource enviroment at Facebook being far more perilous. The first case theoretically had end-user utility for fixing stuck content caches. In modern Phabricator, it's not intuitive that you'd go adjust a Config option to fix this. I don't recall any users ever actually running into problems here, though. (An earlier version of this change did more magic with `celerity.resource-hash`, but this ended up with a more substantial simplification.)

Ref T12509. This upgrades a `weakDigest()` callsite to SHA256-HMAC and removes three config options: - `celerity.resource-hash`: Now generated automatically by the SHA256-HMAC code. You could go purge it by touching `auth.auth_hmackey` directly, but we've never actually directed any users to fiddle with thishard-coded, I believesince the use case for ever adjusting it was very weak. - `celerity.enable-deflate`: Intended to make cache inspection easier, but we haven't needed to inspect caches in ~forever. - `celerity.minify`: Intended to make debugging minification easier, but we haven't needed to debug this in ~forever. In the latter two cases, the options were purely developer-focused, and it's easy to go add an `&& false` somewhere in the code if we need to disable these features to debug something, but the relevant parts of the code basically work properly and never need debugging. These options were excessively paranoid, based on the static resource enviroment at Facebook being far more perilous. The first case theoretically had end-user utility for fixing stuck content caches,. but we'd be better off providing a `bin/celerity bad-stuff-got-cached-oops` command that regenerates the HMAC key if we actually need this -- iIn modern Phabricator, it's not intuitive that you'd go adjust a Config option to fix this. I don't recall any users ever actually running into problems here, though. Note that this creates one odd side effect: we have a unit test which makes sure the Celerity map is up to date. This unit test can access config, but can not access the database, so it can no longer generate a new map because it must now access the database to read the HMAC key. I think this unit test is somewhat questionable and intend to just move "map rebuild" to a "merge action / during-land" sort of step(An earlier version of this change did more magic with `celerity.resource-hash`, but for now: make it work by embedding the HMAC key which was used to generate the map into the map itself. This generally seems reasonable/sensible.this ended up with a more substantial simplification.)