Summary:
Ref T12612. This updates the rate limiting code to:
- Support a customizable token, like the client's X-Forwarded-For address, rather than always using `REMOTE_ADDR`.
- Support APCu.
- Report a little more rate limiting information.
- Not reference nonexistent documentation (removed in D16403).
I'm planning to put this into production on `secure` for now and then we can deploy it more broadly if things work well.
Test Plan:
- Enabled it locally, used `ab -n 100` to hit the limit, saw the limit enforced.
- Waited a while, was allowed to browse again.
Reviewers: chad, amckinley
Reviewed By: amckinley
Maniphest Tasks: T12612
Differential Revision: https://secure.phabricator.com/D17758
Summary:
Closes T7829 as wontfix. Closes T7965 as wontfix. Closes T7800 as wontfix. Closes T2731 as wontfix. Closes T1271 as wontfix.
We aren't maintaining this at all (see, e.g., T7829) and a user reported a technically accurate security issue via HackerOne: <https://hackerone.com/reports/222870>
Just throw it away until we get to the eventual Conphernece bot/API update and can do this stuff correctly.
Test Plan: Grepped for `phabricatorbot`.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T7965, T7829, T7800, T2731, T1271
Differential Revision: https://secure.phabricator.com/D17756
Summary: Fixes T11561. Collect guidance about local configuration which hasn't been obvious in the past.
Test Plan:
- Read document carefully.
- Used `./bin/diviner generate` to generate documentation.
- Previewed in Diviner locally:
{F4795021}
Reviewers: amckinley, chad
Reviewed By: chad
Subscribers: cspeckmim
Maniphest Tasks: T11561
Differential Revision: https://secure.phabricator.com/D17641
Summary:
Ref T12450. General adjustments:
- Try to make "Cluster: Search" more about "stuff in common + types" instead of pretty much all being Elastic-specific, so we can add Solr or whatever later.
- Provide guidance about rebuilding indexes after making a change.
- Simplify the basic examples, then provide a more advanced example at the ed.
- Really try to avoid suggesting anyone configure Elasticsearch ever for any reason.
Test Plan: Read documents, previewed in remarkup.
Reviewers: chad, 20after4
Reviewed By: 20after4
Maniphest Tasks: T12450
Differential Revision: https://secure.phabricator.com/D17602
Summary:
These exception messages & comments didn't quite match reality.
Fixed and added pht() around a couple of them.
Test Plan: I didn't test this :P
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17578
Summary:
The goal is to make fulltext search back-ends more extensible, configurable and robust.
When this is finished it will be possible to have multiple search storage back-ends and
potentially multiple instances of each.
Individual instances can be configured with roles such as 'read', 'write' which control
which hosts will receive writes to the index and which hosts will respond to queries.
These two roles make it possible to have any combination of:
* read-only
* write-only
* read-write
* disabled
This 'roles' mechanism is extensible to add new roles should that be needed in the future.
In addition to supporting multiple elasticsearch and mysql search instances, this refactors
the connection health monitoring infrastructure from PhabricatorDatabaseHealthRecord and
utilizes the same system for monitoring the health of elasticsearch nodes. This will
allow Wikimedia's phabricator to be redundant across data centers (mysql already is,
elasticsearch should be as well).
The real-world use-case I have in mind here is writing to two indexes (two elasticsearch clusters
in different data centers) but reading from only one. Then toggling the 'read' property when
we want to migrate to the other data center (and when we migrate from elasticsearch 2.x to 5.x)
Hopefully this is useful in the upstream as well.
Remaining TODO:
* test cases
* documentation
Test Plan:
(WARNING) This will most likely require the elasticsearch index to be deleted and re-created due to schema changes.
Tested with elasticsearch versions 2.4 and 5.2 using the following config:
```lang=json
"cluster.search": [
{
"type": "elasticsearch",
"hosts": [
{
"host": "localhost",
"roles": { "read": true, "write": true }
}
],
"port": 9200,
"protocol": "http",
"path": "/phabricator",
"version": 5
},
{
"type": "mysql",
"roles": { "write": true }
}
]
Also deployed the same changes to Wikimedia's production Phabricator instance without any issues whatsoever.
```
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Tags: #elasticsearch, #clusters, #wikimedia
Differential Revision: https://secure.phabricator.com/D17384
Summary: Fixes T12418. This is a fairly advanced feature and I think users can reasonably consult the documentation for their own editors to figure out how to do this.
Test Plan: Saw no more text.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12418
Differential Revision: https://secure.phabricator.com/D17510
Summary: This is incorrectly starting an ordered sublist.
Test Plan: (o_O)
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D17434
Summary: Ref T12319. The product name is misspelled in some methods, and a few places in the documentation.
Test Plan: `grep`
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12319
Differential Revision: https://secure.phabricator.com/D17419
Summary: Provides additional hint on where to find and clarification.
Test Plan: read
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17313
Summary: Ref T10978. This is just a maintenance convenience script. It can fix up overall commit state after you `bin/audit delete` stuff or nuke a bunch of stuff from the database, as I did on `secure.phabricator.com`.
Test Plan: Ran `bin/audit synchronize`, and `bin/audit update-owners`.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10978
Differential Revision: https://secure.phabricator.com/D17271
Summary:
Ref T10978.
- Generally refresh this documentation.
- Use the word "publish", not the word "push", to distinguish between review and audit, echoing the language in the "Write, Review, Merge, Publish" document.
- Mention the new "Needs Verification" state.
Test Plan: Read documentation.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10978
Differential Revision: https://secure.phabricator.com/D17253
Summary:
Ref T9640. This is no longer true.
I'm assuming PHP7 vs 7.1 issues won't be common (everyone on the cutting edge is probably on 7.1, and Ubuntu 16 is on 7.1) but we could make this more granular in the future.
Test Plan: Careful reading.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9640
Differential Revision: https://secure.phabricator.com/D17202
Summary:
Fixes T6630. Long ago, "Audit", "Diffusion" and "Repositories" were three totally separate applications.
This separation isn't useful and the three rapidly became intertwined. Ideally, they would all be one application.
This doesn't take us quite that far, but Audit no longer has any controllers and has little actual behavior.
The "Audit" screen has always just been a SearchEngine view of commits with some filters on it, and this formalizes that and puts a link to it in Diffusion. (This view has other uses, too.)
Test Plan:
- Accessed audit from home page.
- Accessed audit/commits from Diffusion.
- Could no longer uninstall Audit on its own.
- Grepped for `/audit/` and `AuditApplication`.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T6630
Differential Revision: https://secure.phabricator.com/D17186
Summary:
Fixes T12087. When transitioning into a clustered configuration for the first time, the documentation recommends using a one-device cluster as a transitional step.
However, installs may not do this for whatever reason, and we aren't as clear as we could be in warning about clusterizing directly into a multi-device cluster.
Roughly, when you do this, we end up believing that working copies exist on several different devices, but have no information about which copy or copies are up to date. //Usually// they all were already synchronized and are all up to date, but we can't make this assumption safely without risking data.
Instead, we err on the side of caution, and require a human to tell us which copy we should consider to be up-to-date, using `bin/repository thaw --promote`.
Test Plan:
```
$ ./bin/repository clusterize rLOCKS --service repos001.phacility.net
Service "repos001.phacility.net" is actively bound to more than one device
(local002.local, local001.phacility.net).
If you clusterize a repository onto this service it will be unclear which
devices have up-to-date copies of the repository. This leader/follower
ambiguity will freeze the repository. You may need to manually promote a
device to unfreeze it. See "Ambiguous Leaders" in the documentation for
discussion.
Continue anyway? [y/N]
```
Read other changes.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12087
Differential Revision: https://secure.phabricator.com/D17169
Summary:
Fixes T12040. In T12039, a user running local patches followed the report instructions as far as grabbing version information, but didn't update or revert their local changes or try against a clean install before reporting.
This obviously isn't ideal for us, but it's understandable (grabbing version information is much easier than upgrading/reverting), and we can do better about making this information useful: when compiling version information, try to figure out the branchpoint from a known upstream `master` branch by listing remotes, then running `git merge-base` against them.
Additionally, explicitly document that we want upstream hashes. We have to have a fallback case in this document anyway (for when you can't get to Config) so hopefully this makes it more likely that we get useful information in initial reports.
Test Plan: {F2229574}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12040
Differential Revision: https://secure.phabricator.com/D17103
Summary: Fixes T12001. I think we're consistent about using bytes everywhere, but users won't necessarily know that and this documentation could certainly be more clear.
Test Plan: Read new text.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12001
Differential Revision: https://secure.phabricator.com/D17037
Summary: Ref T11957.
Test Plan:
- Viewed an existing project profile.
- Viewed a user profile.
- Created a new project.
- Edited a profile menu.
- Added new profile items.
- Grepped for renamed symbols.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11957
Differential Revision: https://secure.phabricator.com/D17028
Summary: Ref T11044. Fixes T11672. In T11672, persistent connections seem to work fine, but they can require `max_connections` and other settings to be raised. Since most users don't need them, make them an advanced option.
Test Plan: Configured persistent connections, loaded some pages, observed persistent connections get used.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11044, T11672
Differential Revision: https://secure.phabricator.com/D16913
Summary:
Ref T11044. One popular tool in a modern operations environment is Puppet. The primary purpose of this tool is to randomly revert hosts to older or different configurations.
Introducing an element of chaotic unpredictability into operations trains staff to be on high alert at all times, rather than lulled into complacency by predictability or consistency.
When Puppet reverts a Phabricator host's configuration to an older version, we might start writing data to a lot of crazy places where it shouldn't go. This will create a big sticky mess that is virtually impossible to undo, mostly because we'll get two files with ID 123 or two tasks with ID 456 or whatever else and good luck with that.
Instead, after changing the partition layout, require `bin/storage partition` to be run. This writes a copy of the config everywhere.
Then, when we start serving web requests, make sure every database has the exact same config. This will foil Puppet by refusing to run requests on hosts it has reverted.
Test Plan:
- Changed partition configuration.
- Ran Phabricator.
- FOILED!
- Ran `bin/storage partition` to sync config.
- Things worked again.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11044
Differential Revision: https://secure.phabricator.com/D16910
Summary:
Ref T11044. I'm going to hold this until after the release cut, but I think it's good to go.
This allows installs to configure multiple masters in `cluster.databases` and partition applications across them (for example, put Maniphest on a dedicated database).
When we make a Maniphest connection we go look up which master we should be hitting first, then connect to it.
This has at least approximately been planned for many years, so the actual change is largely just making sure that your config makes sense.
Test Plan:
- Configured `db001.epriestley.com` and `db002.epriestley.com` as master/master.
- Partitioned applications between them.
- Interacted with various applications, saw writes go to the correct host.
- Viewed "Database Servers" and saw partitioning information.
- Ran schema upgrades.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11044
Differential Revision: https://secure.phabricator.com/D16876
Summary: Fixes T11882. Document using `~/.ssh/config` to mitigate the inconvenience of port 2222.
Test Plan: Read document.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11882
Differential Revision: https://secure.phabricator.com/D16894
Summary:
Fixes T11809. Ref
- Explicitly document the summary icon hints -- I don't think these are too hard to figure out (and maybe this stuff should just go in the tooltips) but we can start here.
- Use color + shape to distinguish between "cancelled" and "declined", not just color (for users with vision accessibility issues).
- Translate a "minute(s)" string into sensible English.
- Use RSVP status on the month view green circle thing.
Test Plan:
- Read docs.
- Looked at month view.
- Read reminder mail.
- Viewed month view mobile view.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11809
Differential Revision: https://secure.phabricator.com/D16872
Summary: Since this was written, `Ennn` became an event monogram and these became real events.
Test Plan: O__O
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D16845
Summary: This isn't spelled as well as it could be.
Test Plan: O_O
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D16827
Summary:
Ref T11809. Roughly documents most of the tricky/unintuitive stuff.
Also fixes a bug with "Make Recurring" with no "Until" date.
Test Plan: Read document.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11809
Differential Revision: https://secure.phabricator.com/D16792
Summary:
Ref T10747.
- Adds import documentation.
- Adds import/export docs to the help menu.
- Removes some weird/old/out-of-date information from the general user guide, which I'll rewrite later.
Test Plan: Read documentation somewhat thoroughly.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16766
Summary:
See D16734.
- Add ".ico" files to the Celerity map.
- Add a formal route for "/favicon.ico".
- Remove instructions to configure `/rsrc/` and `/favicon.ico` rewrite rules.
Long ago, we served resources directly via `/rsrc/` in at least some cases. As we added more features, this stopped working more and more often (for example, Apache can never serve CSS this way, because it doesn't know how to post-process `{$variables}`).
In modern code (until this change), only `/favicon.ico` is still expected to be served this way.
Instead, serve it with an explicit route via controller (this allows different Sites to have different favicons, for example).
Remove the instructions suggesting the old rewrite rules be configured. It's OK if they're still in place -- they won't break anything, so we don't need to rush to get users to delete them.
We should keep "webroot/favicon.ico" in place for now, since it needs to be there for users with the old rewrite rule.
Test Plan:
- Ran celerity map.
- Loaded `/favicon.ico`, got resource via route.
- Used `celerity_generate_resource_uri()` to get paths to other icons, loaded them, got icons.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D16737
Summary:
Ref T10747. This explains how exports work.
Also make mail exports use the same logic as other stuff.
Test Plan: Read documentation. Did some exports.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16680
Summary:
Ref T10747. Rough flow is:
- Run a query.
- Select a new "Export Events..." action.
- This lets you define an "Export", which has a unique URL you can paste into Google Calendar or Calendar.app or whatever.
Most of this does nothing yet but here's the boilerplate.
Test Plan: Doesn't do anything yet.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16675
Summary: Fixes a typo in the Almanac docs.
Test Plan: Didn't test this one.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D16529
Summary: Caught one of these while reviewing docs, grepped for the other one.
Test Plan: `grep`, reading
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D16498
Summary: Previously, the chatbot docs instructed users to get certificates for the conduit API and put the cert in a `conduit.cert` config key. In order to get the chatbot to work, I needed to instead get an API key and put it in the `conduit.token` config entry.
Test Plan: Doc fix. Tried the new documented way and it worked.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D16443
Summary: Ref T11522. This explains how to actually use `bin/repository hint`.
Test Plan: Read the document. Used `bin/repository hint` as directed.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11522
Differential Revision: https://secure.phabricator.com/D16441
Summary: Caught this while linking to it from D16405.
Test Plan: Consulted a dictionary.
Reviewers: chad, alexmv
Reviewed By: alexmv
Differential Revision: https://secure.phabricator.com/D16406
Summary:
Fixes T11487. Improve documentation for three situations:
- When you configure a cluster behind a load balancer, all requests are trusted but not all have an "X-Forwarded-For" header. Change the suggested snippet to read this header only if it exists.
- When a request goes through a series of load balancers (as with a CDN) they can end up writing a list of IPs to the header. Parse these.
- Remove the "rate limiting" stuff -- this got disabled/removed a long time ago and is misleading/incorrect.
Test Plan: Read documentation.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11487
Differential Revision: https://secure.phabricator.com/D16403
Summary: Fixes T11484. These mechanisms aren't necessarily obvious and make sense to document here.
Test Plan: Read documentation.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11484
Differential Revision: https://secure.phabricator.com/D16404
