1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-11 09:22:40 +01:00
Commit graph

15137 commits

Author SHA1 Message Date
epriestley
06bbf237fe Give Drydock Resources more modern status treatment
Summary: Ref T13073. Depends on D19074. Update icons and UI for resource status.

Test Plan: Viewed resources in detail view and list view, saw better status icons.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13073

Differential Revision: https://secure.phabricator.com/D19075
2018-02-13 13:16:46 -08:00
epriestley
27c3793d40 Give Drydock Leases more modern status treatment
Summary:
Depends on D19073. Ref T13073. Give leases a normal header tag and try to wrangle their status constants a bit.

Also, try to capture the "status class" pattern a bit. Since we target PHP 5.2.3 we can't use `static::` so the actual subclass is kind of a mess. Not exactly sure if I want to stick with this or not. We could consider targeting PHP 5.3.0 instead to get `static::` / late static binding.

Test Plan: Viewed leases and lease lists, saw better and more conventional status information.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13073

Differential Revision: https://secure.phabricator.com/D19074
2018-02-13 13:15:57 -08:00
epriestley
07028cfc30 When bin/drydock lease is interrupted, release leases
Summary:
Depends on D19072. Ref T13073. Currently, you can leave leases stranded by using `^C` to interrupt the script. Handle signals and release leases on destruction if they haven't activated yet.

Also, print out more useful information before and after activation.

Test Plan: Mashed ^C while runnning `bin/drydock lease ... --trace`, saw the lease release.

Subscribers: yelirekim, PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13073

Differential Revision: https://secure.phabricator.com/D19073
2018-02-13 13:14:21 -08:00
epriestley
b833e324bd While waiting for a "bin/drydock" lease to activate, entertain the user with log output
Summary: Depends on D19071. Ref T13073. While the daemons are supposedly doing things, show the user any logs they generate. There's often something relevant but unearthing it can be involved.

Test Plan: {F5427773}

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13073

Differential Revision: https://secure.phabricator.com/D19072
2018-02-13 13:13:52 -08:00
epriestley
3ec80a36db In Drydock log views, respect newlines
Summary: Depends on D19070. Ref T13073. Some messages contain an interesting story or a clever anecdote. Respect newlines during rendering to preserve authorial intent.

Test Plan:
Viewed a message with linebreaks and could still read it.

{F5427754}

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13073

Differential Revision: https://secure.phabricator.com/D19071
2018-02-13 13:12:31 -08:00
epriestley
4dd32dca3e When a Drydock Blueprint promises us a resource but can't deliver, continue believing in it
Summary:
Ref T13073. When a Blueprint says it will be able to allocate a resource but then throws an exception while attempting that allocation, we currently fail the lease permanently.

This is excessively harsh. This blueprint may have the best of intentions and have encountered a legitimately unforseeable failure (like a `vm.new` call to build a VM failed) and be able to succeed in the future.

Even if this blueprint is a dirty liar, other blueprints (or existing resources) may be able to satisfy the lease in the future.

Even if every blueprint is implemented incorrectly, leaving the lease alive lets it converge to success after the blueprints are fixed.

Instead of failing, log the issue and yield.

(In the future, it might make sense to distinguish more narrowly between "actually, all the resources are used up" and all other failure types, since the former is likely more routine and less concerning.)

Test Plan:
  - Wrote a broken `Hoax` blueprint which always claims it can allocate but never actually allocates (just `throw` in `allocateResource()`).
  - Used `bin/phd drydock lease` to acquire a Hoax lease.
  - Before patch: lease abruptly failed permanently.
  - After patch: lease yields after allocation fails.

{F5427747}

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13073

Differential Revision: https://secure.phabricator.com/D19070
2018-02-13 13:11:55 -08:00
epriestley
6a4d5ce3c9 Don't allow Herald Diff rules to "Call Webhooks"
Summary: Like "Commit Hook" rules, these also fire oddly and don't have an object PHID or a list of transactions.

Test Plan: Verified that "Call Webhooks" was no longer available from Diff rules, but still available from other rule types.

Differential Revision: https://secure.phabricator.com/D19069
2018-02-12 17:52:05 -08:00
epriestley
894e9dd852 Update a handful of missed HarbormasterBuildableStatus constants
Summary: See <https://discourse.phabricator-community.org/t/exception-undefined-class-status-building/1103>.

Test Plan: Used `grep` more carefully.

Differential Revision: https://secure.phabricator.com/D19068
2018-02-12 15:33:24 -08:00
epriestley
11c9994134 Allow "arc diff --plan-changes" to work with drafts enabled
Summary:
See PHI346. Ref T13054. If you have prototypes enabled on the server but use `master` / `stable` on the client and run `arc diff --plan-changes`, the transition is rejected because "Draft -> Changes Planned" isn't currently a legal transition.

Allow this transition if not coming from the web UI (to keep it out of the dropdown).

Test Plan:
  - Ran `arc diff --plan-changes` on `master`, got a "Changes Planned" revision instead of a validation error.
  - Ran `arc diff` without `--plan-changes`, got a draft, verified that "Plan Changes" still doesn't appear in the action dropdown.

Maniphest Tasks: T13054

Differential Revision: https://secure.phabricator.com/D19067
2018-02-12 13:15:13 -08:00
epriestley
6f508a2258 Update buildable containerPHIDs in a proper way via BuildWorker rather than via sneaky uncoordinated write
Summary:
Depends on D19065. Ref T13054. Instead of just updating `containerPHID` and hoping for the best, queue a proper BuildWorker to process a "your container has changed, update it" message.

We also need to remove a (superfluous) `withContainerPHIDs()` when loading active diffs for a revision.

Test Plan:
  - Without daemons, created a revision and saw builds stick in "preparing" with no container PHID, but also stay in draft mode.
  - With daemons, saw builds actually build and get the right container PHID.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13054

Differential Revision: https://secure.phabricator.com/D19066
2018-02-12 12:18:52 -08:00
epriestley
66f20595e4 Start buildables in "PREPARING", move them to "BUILDING" after builds queue
Summary:
Depends on D19064. Ref T13054. See that task for additional discussion.

When buildables are created by `arc` and have lint/unit messages, they can currently pass or fail before Herald triggers actual builds. This puts them in a pre-build state where they can't complete until Herald says it's okay.

On its own, this change intentionally strands `arc diff --only` diffs in the "PREPARING" stage forever.

Test Plan:
  - Ran a build with `bin/harbormaster`, saw it build normally.
  - Ran a build with web UI, saw it build normally.
  - Ran a build with `arc diff`, saw it build normally.
  - Ran a build with `arc diff --only`, saw it hang in "PREPARING" forever.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13054

Differential Revision: https://secure.phabricator.com/D19065
2018-02-12 12:18:29 -08:00
epriestley
f939a2b12e Make Harbormaster buildable status more of a nice flexible map and less of a bunch of switch statements
Summary: Depends on D19063. Ref T13054. Prepare for the addition of a new `PREPARING` status by getting rid of the "scattered mess of switch statements" pattern of status management.

Test Plan: Searched/browsed buildables. Viewed buildables. Viewed revisions. Grepped for all affected symbols.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13054

Differential Revision: https://secure.phabricator.com/D19064
2018-02-12 12:18:06 -08:00
epriestley
c42bbd6f5c Rename HarbormasterBuildMessage "buildTargetPHID" to "receiverPHID"
Summary: Ref T13054. Companion storage change for D19062.

Test Plan: Applied migration and adjustments. Viewed messages in Harbormaster; created them with `harbormaster.sendmessage`; processed them with `bin/phd debug task`.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13054

Differential Revision: https://secure.phabricator.com/D19063
2018-02-12 12:17:44 -08:00
epriestley
ed0ba41cd2 Allow a HarbormasterBuildMessage to be sent to any object
Summary:
See T13054. This prepares for Buildables to be sent messages ("attach", "done scheduling builds") to fix races between Harbormaster and Differential.

The `buildTargetPHID` is replaced with a `recipientPHID` in the API. An additional change will fix the storage.

In the future, this table could probably also replace `HarbormasterBuildCommand` now, which is approximately the same bus, but for Builds.

Test Plan: Viewed builds with messages. Sent messages with `harbormaster.sendmessage`. Processed messages with `bin/phd debug task`.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Differential Revision: https://secure.phabricator.com/D19062
2018-02-12 12:16:03 -08:00
epriestley
4fa99374be Prevent "Call webhooks" Herald action from appearing in UI for adapters which can't fire it
Summary:
See <https://discourse.phabricator-community.org/t/herald-webhook-is-not-called-but-herald-transcript-tells-me-the-opposite/1098>.

The "Commit Hook" events don't operate on objects and don't use TransactionEditors. They can't call webhooks in a normal way and currently don't call webhooks at all. Stop offering these actions in the UI.

The  "Outbound Mail" event also fires oddly and likely doesn't make much sense to hook anyway.

Test Plan: Verified that these events no longer offer "Call webhooks", while normal events still do.

Differential Revision: https://secure.phabricator.com/D19061
2018-02-11 06:15:29 -08:00
epriestley
5e6e9fcc56 When purging drafts after a transaction edit, purge all drafts
Summary: Fixes T13071. See that task for discusison. I think this `<= version` constraint is needless in normal cases (it should match everything in the table anyway), and slightly harmful in bizarre cases where a draft somehow gets a much larger ID than it should have.

Test Plan:
  - Gave a draft an unreasonably large ID.
  - Pre-patch, observed: submitting comments on the draft's object does not clear the draft.
  - Post-patch: submitting comments on the draft's object now clears the draft correctly.
  - Also added comments/actions, reloaded pages, saw drafts stick properly.

Maniphest Tasks: T13071

Differential Revision: https://secure.phabricator.com/D19060
2018-02-11 06:01:09 -08:00
epriestley
653bc0fa01 Read lock all transaction edits
Summary: Ref T13054. Fixes T12714. Applies read locks to all transactions instead of only a very select subset (chat messages in Conpherence).

Test Plan: See <T13054#235650> for discussion and testing.

Maniphest Tasks: T13054, T12714

Differential Revision: https://secure.phabricator.com/D19059
2018-02-10 20:07:46 -08:00
epriestley
f43d08c2bb Completely remove the legacy hunk table
Summary: Depends on D19056. Fixes T8475. Ref T13054. Merges "ModernHunk" back into "Hunk".

Test Plan: Grepped for `modernhunk`. Reviewed revisions. Created a new revision. Used `bin/differential migrate-hunk` to migrate hunks between storage formats and back.

Maniphest Tasks: T13054, T8475

Differential Revision: https://secure.phabricator.com/D19057
2018-02-10 16:12:50 -08:00
epriestley
b0d1d46a73 Drop the legacy hunk table
Summary: Ref T13054. Ref T8475. This table has had no readers or writers for more than a year after it was migrated to the modern table.

Test Plan: Ran migration, verified that all the data was still around.

Maniphest Tasks: T13054, T8475

Differential Revision: https://secure.phabricator.com/D19056
2018-02-10 16:09:31 -08:00
epriestley
a2d02aed22 When a build is aborted, fail the buildable
Summary:
Ref T13054. Fixes T10746. Fixes T11154. This is really a one-line fix (include `ABORTED` in `BuildEngine->updateBuildable()`) but try to structure the code a little more clearly too and reduce (at least slightly) the number of random lists of status attributes spread throughout the codebase.

Also add a header tag for buildable status.

Test Plan: Aborted a build, saw buildable fail properly.

Subscribers: yelirekim, PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13054, T11154, T10746

Differential Revision: https://secure.phabricator.com/D19055
2018-02-10 16:08:41 -08:00
epriestley
d47c5de9d0 Fix PHPMailer/SMTP configuration typo for legacy SMTP configurations
Summary: See <https://discourse.phabricator-community.org/t/phpmailer-cannot-read-configuration-value-because-of-typo/1094>.

Test Plan: Browsed a dictionary. (This doesn't get hit when configured via `cluster.mailers`, which is how I previously re-tested SMTP configuration.)

Differential Revision: https://secure.phabricator.com/D19058
2018-02-10 16:06:11 -08:00
epriestley
ffc5c95c2f Correct flipped transaction constants in "Closed Date" migration
Summary: These transaction constants are flipped, which can produce the wrong result in some cases.

Test Plan: `./bin/storage upgrade -f --apply phabricator:20180208.maniphest.02.populate.php`

Differential Revision: https://secure.phabricator.com/D19054
2018-02-10 06:10:55 -08:00
epriestley
c64aae052f Make sure auditors are attached to commits on new pathways
Companion change to D19022 for commits. Mentioning and subscribing to commits
can load them without audit data.
2018-02-09 17:09:00 -08:00
epriestley
9b7d5b74d4 Purge ssh-auth key cache after trust/untrust
Summary: See PHI358. The `bin/almanac [un]trust-key` workflows don't properly purge the SSH key cache, but should.

Test Plan:
  - Added key `ssh-rsa xyz` to a device.
  - Used `bin/ssh-auth | grep xyz` to test for the presence of the key.
  - Before patch: Saw it not present, trusted it, saw it still not present.
  - After patch: Saw it not present, trusted it, saw it now present. Untrusted it, saw it no longer present.

Differential Revision: https://secure.phabricator.com/D19053
2018-02-09 14:58:45 -08:00
epriestley
9c8484de32 Document the STMP port option
Summary: Ref T12677. This slipped thorugh in the upgrade.

Test Plan: Read documentation.

Maniphest Tasks: T12677

Differential Revision: https://secure.phabricator.com/D19052
2018-02-09 14:49:46 -08:00
epriestley
4fef0a6128 Allow a wider range of characters in macro names, including emoji
Summary:
Fixes T6121. See PHI357.

  - Allow emoji and other unicode (like Chinese characters) as long as you have at least three of them.
  - Disallow macros with only latin symbols. These were previously allowed.

Test Plan: Created a macro for "🐶🐶🐶", then used it in a comment.

Maniphest Tasks: T6121

Differential Revision: https://secure.phabricator.com/D19051
2018-02-09 14:34:30 -08:00
epriestley
64177cb16e Document how webhooks work
Summary: Depends on D19049. Ref T11330. Adds some documentation for webhooks.

Test Plan: Read the documentation and found it to be exceptionally accurate and helpful.

Maniphest Tasks: T11330

Differential Revision: https://secure.phabricator.com/D19050
2018-02-09 13:57:19 -08:00
epriestley
98c701ffc5 Add a "Call webhooks" action to Herald
Summary: Depends on D19048. Fixes T11330.

Test Plan: Wrote rules to call webhooks selectively, saw them fire appropriately with correct trigger attribution.

Maniphest Tasks: T11330

Differential Revision: https://secure.phabricator.com/D19049
2018-02-09 13:56:57 -08:00
epriestley
41d28abfcc Trigger all "Firehose" webhooks on all transactional edits
Summary: Depends on D19047. Ref T11330. Triggers every firehose hook on every edit; prepares for Herald triggers.

Test Plan: Configured a firehose hook, edited some objects, saw callbacks.

Maniphest Tasks: T11330

Differential Revision: https://secure.phabricator.com/D19048
2018-02-09 13:56:34 -08:00
epriestley
4887c6aa80 Allow "transaction.search" to be constrained by PHIDs
Summary: Depends on D19046. Ref T11330. Supports querying for specific transactions while responding to webhooks.

Test Plan: Called `transaction.search` with and without PHID constraints.

Maniphest Tasks: T11330

Differential Revision: https://secure.phabricator.com/D19047
2018-02-09 13:56:16 -08:00
epriestley
dc2995c4ca Refine core webhook implementation somewhat
Summary:
Depends on D19045. Ref T11330.

  - View/regenerate HMAC keys.
  - Pretty JSON.
  - Readable status transactions.
  - test, silent, secure flags.
  - Dates on request view.
  - More icons.
  - Can test any object.
  - GC for requests.

Test Plan: Went through each feature poking at it in the web UI and with `bin/webhook call ...` / `bin/garbage collect ...`.

Subscribers: ftdysa

Maniphest Tasks: T11330

Differential Revision: https://secure.phabricator.com/D19046
2018-02-09 13:55:55 -08:00
epriestley
0470125d9e Add skeleton code for webhooks
Summary: Ref T11330. Adds general support for webhooks. This is still rough and missing a lot of pieces -- and not yet useful for anything -- but can make HTTP requests.

Test Plan: Used `bin/webhook call ...` to complete requests to a test endpoint.

Maniphest Tasks: T11330

Differential Revision: https://secure.phabricator.com/D19045
2018-02-09 13:55:04 -08:00
epriestley
9386e436fe Remove red coloration from "Logout" menu item
Summary: I made the red stronger (always visible, not just a hover state) for the "Mute" feature, but this made Logout look a little intense. Just make it normal-colored, logging out isn't a big deal.

Test Plan: No longer saw bright red logout action in profile dropdown menu.

Differential Revision: https://secure.phabricator.com/D19044
2018-02-09 05:26:38 -08:00
epriestley
8de794d3c2 Make optional options actually optional in cluster mailer config validation 2018-02-08 17:58:14 -08:00
epriestley
d45952344b Use setOptions() to trigger mailer option validation, not validateOptions() 2018-02-08 17:55:02 -08:00
epriestley
09b446b269 Don't run older mail setup checks if "cluster.mailers" is configured
Summary: Ref T12677. Skip these checks if we're doing the new stuff. Also, allow priority to be unspecified.

Test Plan: Will deploy.

Maniphest Tasks: T12677

Differential Revision: https://secure.phabricator.com/D19043
2018-02-08 17:51:32 -08:00
epriestley
7d4362690f Fix transposed name/email in Mailgun adapter
Summary: Ref T12677. This argument order was swapped.

Test Plan: Will push/verify.

Maniphest Tasks: T12677

Differential Revision: https://secure.phabricator.com/D19042
2018-02-08 17:49:32 -08:00
epriestley
261a4a0e51 Add inline comment counts to the filetree view
Summary: See PHI356. Adds inline comment and done counts to the filetree. Also makes the filetree wider by default.

Test Plan: Fiddled with filetrees in different browsers on different revisions. Added inlines, marked them done/undone.

Differential Revision: https://secure.phabricator.com/D19041
2018-02-08 17:15:36 -08:00
epriestley
6ea1b8df9b Colorize filetree for adds, moves, and deletes
Summary: See PHI356. Makes it easier to pick out change types in the filetree view in Differential.

Test Plan: Created a diff with adds, copies, moves, deletions, and binary files. Viewed in Differential, had an easier time picking stuff out.

Differential Revision: https://secure.phabricator.com/D19040
2018-02-08 16:11:35 -08:00
epriestley
e26a784dcf Allow tasks to be filtered and ordered by closed date
Summary: Depends on D19038. Fixes T4434. Updates the SearchEngine and Query to handle these fields.

Test Plan: Filtered and ordered by date and closer.

Maniphest Tasks: T4434

Differential Revision: https://secure.phabricator.com/D19039
2018-02-08 15:42:26 -08:00
epriestley
4c4707e467 Provide task closed date via Conduit API, data export pipeline, and in list UI
Summary:
Depends on D19037. Ref T4434. Adds closed date to `maniphest.search` and "Export Data".

When a task has been closed, show the closed date with a checkmark in the UI instead of the modified date.

Test Plan:
  - Exported data to CSV, saw close information.
  - Saw close information in `/maniphest/`.
  - Queried for close information via `maniphest.search`.

Maniphest Tasks: T4434

Differential Revision: https://secure.phabricator.com/D19038
2018-02-08 15:41:54 -08:00
epriestley
f028aa6f60 Track closed date and closing user for tasks explicitly
Summary:
Ref T4434. Although some of the use cases for this data are better fits for Facts, this data is reasonable to track separately.

I have an approximate view of it already ("closed, ordered by date modified") that's useful to review things that were fixed recently. This lets us make that view more effective.

This just adds (and populates) the storage. Followups will add Conduit, Export, Search, and UI support.

This is slightly tricky because merges work oddly (see T13020).

Test Plan:
  - Ran migration, checked database for sensible results.
  - Created a task in open/closed status, got the right database values.
  - Modified a task to close/open it, got the right values.
  - Merged an open task, got updates.

Maniphest Tasks: T4434

Differential Revision: https://secure.phabricator.com/D19037
2018-02-08 15:40:49 -08:00
epriestley
d1e273daf6 Remove completely pointless load of every repository when viewing a repository URI
Summary:
See D18176. This query has no effect (other than wasting resources) and the result is unused.

`$repository` already has the URI loaded because we load them unconditionally during request initialization.

Test Plan: Viewed repository URIs.

Subscribers: jmeador

Differential Revision: https://secure.phabricator.com/D19036
2018-02-08 12:47:48 -08:00
epriestley
705ff8d33d Remove addHighlight() action view methods
Summary: These didn't actually get used by D19033.

Test Plan: Grep.

Differential Revision: https://secure.phabricator.com/D19034
2018-02-08 12:15:24 -08:00
epriestley
ab04d2179b Add "Mute/Unmute" for subscribable objects
Summary: Ref T13053. See PHI126. Add an explicit "Mute" action to kill mail and notifications for a particular object.

Test Plan: Muted and umuted an object while interacting with it. Saw mail route appropriately.

Maniphest Tasks: T13053

Differential Revision: https://secure.phabricator.com/D19033
2018-02-08 11:06:22 -08:00
epriestley
0402a79e0e Render object remarkup references in a text context as "Dxxx <uri>"
Summary:
Depends on D19031. Fixes T11389. Currently, we render `Dxxx` in a text context (plain text email) as just a URI.

Instead, render it like `Dxxx <uri>`. This is more faithful to the original intent and preserves `T123/T456` as two separate, usable links.

Test Plan: Wrote `T123/T234` in a task, pulled mail for it with `bin/mail show-outbound`, saw separate clickable links.

Maniphest Tasks: T11389

Differential Revision: https://secure.phabricator.com/D19032
2018-02-08 09:52:41 -08:00
epriestley
bca9c08953 Add an "Acting user" field to Herald
Summary:
Ref T13053. Fixes T7804. Adds "Acting user" so you can have "always email me" stuff skip things you did or keep an eye on suspicious interns.

For the test console, the current user is the acting user.

For pushes, the pusher is the acting user.

Test Plan: Wrote acting user rules, triggered them via test console and via multiple actors on real objects.

Maniphest Tasks: T13053, T7804

Differential Revision: https://secure.phabricator.com/D19031
2018-02-08 09:52:18 -08:00
epriestley
6186f0aa91 Briefly document mail stamps and remove obsolete header documentation
Summary: Fixes T10189. Ref T13053. We haven't sent these headers in a very long time. Briefly mention the new stamps header instead, although I expect to integrate stamp documentation into the UI in a more cohesive way in the future.

Test Plan: Read documentation.

Maniphest Tasks: T13053, T10189

Differential Revision: https://secure.phabricator.com/D19030
2018-02-08 09:31:12 -08:00
epriestley
bae9f459ab Pass HTML bodies to push email
Summary: Depends on D19028. Ref T13053. Fixes T6576. An HTML body was built here, but not passed to the actual mail message.

Test Plan: Will verify production push mail.

Maniphest Tasks: T13053, T6576

Differential Revision: https://secure.phabricator.com/D19029
2018-02-08 09:12:03 -08:00
epriestley
a8f937d313 Only add the Mail "STAMPS" body section if there are stamps
Summary: Ref T13053. Some mail (like push notification mail) doesn't currently generate any stamps. Drop this section if there aren't any stamps on the mail.

Test Plan: Will check push mail in production.

Maniphest Tasks: T13053

Differential Revision: https://secure.phabricator.com/D19028
2018-02-08 09:09:28 -08:00