Summary:
Depends on D19323. Ref T13120. Ref T12414.
Move editing to modern stuff and fix some implementation errors from D19323 (mostly copy/paste stuff).
Test Plan:
- Created and edited interfaces.
- Tried to create/edit an interface with a bogus/empty address/port, got errors.
- Tried to create an interface on a bogus device, got an error.
- Tried to create an interface on a device I could not edit, got an error.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13120, T12414
Differential Revision: https://secure.phabricator.com/D19324
Summary:
Depends on D19322. Ref T13120. Ref T12414.
Currently, `AlmanacDevice` has a bit of a beast of a `TYPE_INTERFACE` transaction that fully creates a complex Interface object. This isn't very flexible or consistent, and Interfaces are complex enough to reasonably have their own object behaviors (for example, they have their own PHIDs).
The complexity of this transaction makes modularizing `AlmanacDevice` transactions tricky. To simplify this, move Interface toward having its own set of normal transactions.
This change just adds some reasonable-looking transactions; it doesn't actually hook them up in the UI or make them reachable. I'll test that they actually work as I swap the UI over.
We may also have some code using the `TYPE_INTERFACE` transaction in Phacility support stuff, so that may need to wait a week to actually phase out.
Test Plan: Ran `bin/storage upgrade` and `arc liberate`. This code isn't reachable yet.
Reviewers: amckinley
Reviewed By: amckinley
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13120, T12414
Differential Revision: https://secure.phabricator.com/D19323
Summary: Depends on D19321. Ref T13120. Ref T12414. Move transactions for Almanac Networks (just "name") to ModularTransactions.
Test Plan:
- Created a new network.
- Renamed a network.
- Tried to create a network with no name (got an error).
- Grepped for `AlmanacNetworkTransaction::`.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13120, T12414
Differential Revision: https://secure.phabricator.com/D19322
Summary: Depends on D19320. Ref T13120. Ref T12414. Move transactions for Almanac Bindings to ModularTransactions.
Test Plan:
- Created a new binding.
- Tried to create a duplicate binding, got an error.
- Edited a binding to rebind it to a different device.
- Disabled and enabled bindings.
- Grepped for `AlmanacBindingTransaction::` constants.
When a binding is created, it currently renders a bad "changed the interface from ??? to X" transaction. This is because creation isn't currently using EditEngine. I plan to swap it shortly, which will turn this into a real "Create" transaction and fix the issue.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13120, T12414
Differential Revision: https://secure.phabricator.com/D19321
Summary: Depends on D19318. Ref T13120. Ref T12414. Move transactions for Almanac Namespaces ("name" is the only meaningful one) to ModularTransactions.
Test Plan:
- Created a new namespace.
- Edited a namespace.
- Tried to choose no name, an invalid name, a duplicate name, and a name in a namespace I can't edit; got appropriate errors.
- Grepped for `AlmanacNamespaceTransaction::TYPE_NAME`.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13120, T12414
Differential Revision: https://secure.phabricator.com/D19320
Summary:
Depends on D19317. Ref T13120. Ref T12414. See PHI145. See PHI473.
This adds a Conduit-only "type" transaction for Almanac services. This is very similar to the approach in D18849 for Drydock blueprints.
Test Plan:
- Tried to create an empty service via "almanac.service.edit", was told to pick a type.
- Tried to pick a bad type, was told to pick a good type.
- Created a new Almanac service via "almanac.service.edit".
- Tried to edit the service to change the type, wasn't allowed to.
- Created and edited via the web UI, nothing changed from before.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13120, T12414
Differential Revision: https://secure.phabricator.com/D19318
Summary:
Ref T13120. Ref T12414. See PHI145. See PHI473. This partially modernizes AlmanacService transactions by moving them to ModularTransactions.
This isn't complete because the "update property" and "remove property" transactions aren't modularized. They still //work//, since the parent Editor implements them, but they no longer render properly on the timeline since the `Transaction` object no longer has rendering logic for them.
Tentatively, I'm going to try to convert the rest of the Almanac objects and then modularize those transactions. (Currently, all of Binding, Device, Namespace and Service support properties, although they can only actually be edited on Service, Device and Binding.)
If that turns out to be really tricky for some reason I can just copy/paste the timeline rendering for now, but I think it won't be too hard.
Test Plan:
- Created and edited Services.
- Tried to create a service with: a bad name, no name, a name which put it in a namespace I can't edit (got errors in all cases).
- Edited and removed properties. The edits worked, the timeline just renders a generic story now ('X edited this object (transaction type "almanac:property:update").').
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13120, T12414
Differential Revision: https://secure.phabricator.com/D19317
Summary:
Depends on D19315. Ref T13120. Ref T12414. See PHI145. See PHI473. I want to move Almanac services to ModularTransactions but ran into this old piece of dead/unused code along the way.
Long ago, Almanac services could be individually "locked", but this didn't really work out very well. It was replaced by "Can Manage Cluster Services" in D15339 and prior changes, but not all of the old "Lock" code got cleaned up.
I don't expect to restore this feature, so clean it up now.
Test Plan:
- Grepped for `AlmanacServiceTransaction::TYPE_LOCK`, `TYPE_LOCK`, etc.
- Grepped for `updateServiceLock()`, no callsites.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13120, T12414
Differential Revision: https://secure.phabricator.com/D19316
Summary:
See T13120. See T12414. See PHI145. See PHI473. Almanac services require a type before they can do anything, and EditEngine currently builds one with no type. We then fatal when trying to do mundane things like generate documentation.
Instead, build a generic but complete Service for documentation generation in the web UI. This is similar to the previous Drydock Blueprint change from D18849 (or some earlier diff in that series).
(You still probably can't use this method to //create// a service; I'll fix that in the next change.)
Test Plan:
- Viewed "almanac.service.edit" in the web UI.
- Before: immediate fatal ("No Almanac service type "" exists!").
- After: Page works. No claims about the method doing anything useful.
Reviewers: amckinley
Reviewed By: amckinley
Differential Revision: https://secure.phabricator.com/D19315
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
Summary: Noticed a couple of typos in the docs, and then things got out of hand.
Test Plan:
- Stared at the words until my eyes watered and the letters began to swim on the screen.
- Consulted a dictionary.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, yelirekim, PHID-OPKG-gm6ozazyms6q6i22gyam
Differential Revision: https://secure.phabricator.com/D18693
Summary: See T12414. This just gets started; we still need edit endpoints for network interfaces and bindings.
Test Plan: Created some devices/services from the conduit UI.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18605
Summary: Try to dis-ambiguate various button types and colors. Moves `simple` to `phui-button-simple` and moves colors to `button-color`.
Test Plan: Grep for buttons still inline, UIExamples, PHUIX, Herald, and Email Preferences.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18077
Summary:
Ref T11954. This is kind of complex and I'm not sure I want to actually land it, but it gives us a fairly good improvement for clustered repositories so I'm leaning toward moving forward.
When we make (or receive) clustered repository requests, we must first load a bunch of stuff out of Almanac to figure out where to send the request (or if we can handle the request ourselves).
This involves several round trip queries into Almanac (service, device, interfaces, bindings, properties) and generally is fairly slow/expensive. The actual data we get out of it is just a list of URIs.
Caching this would be very easy, except that invalidating the cache is difficult, since editing any binding, property, interface, or device may invalidate the cache for indirectly connected services and repositories.
To address this, introduce `PhabricatorCacheEngine`, which is an extensible engine like `PhabricatorDestructionEngine` for propagating cache updates. It has two modes:
- Discover linked objects (that is: find related objects which may need to have caches invalidated).
- Invalidate caches (that is: nuke any caches which need to be nuked).
Both modes are extensible, so third-party code can build repository-dependent caches or whatever. This may be overkill but even if Almanac is the only thing we use it for it feels like a fairly clean solution to the problem.
With `CacheEngine`, make any edit to Almanac stuff propagate up to the Service, and then from the Service to any linked Repositories.
Once we hit repositories, invalidate their caches when Almanac changes.
Test Plan:
- Observed a 20-30ms performance improvement with `ab -n 100`.
- (The main page making Conduit calls also gets a performance improvement, although that's a little trickier to measure directly.)
- Added debugging code to the cache engine stuff to observe the linking and invalidation phases.
- Made invalidation throw; verified that editing properties, bindings, etc, properly invalidates the cache of any indirectly linked repositories.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11954
Differential Revision: https://secure.phabricator.com/D17000
Summary:
This has been replaced by `PolicyCodex` after D16830. Also:
- Rebuild Celerity map to fix grumpy unit test.
- Fix one issue on the policy exception workflow to accommodate the new code.
Test Plan:
- `arc unit --everything`
- Viewed policy explanations.
- Viewed policy errors.
Reviewers: chad
Reviewed By: chad
Subscribers: hach-que, PHID-OPKG-gm6ozazyms6q6i22gyam
Differential Revision: https://secure.phabricator.com/D16831
Summary: Fixes T11501. Let's you pass in a full PHUIIconView or just the icon name to give ObjectListItem a large icon.
Test Plan: Alamanac, Applications, Drydock, Settings, Search Typeahead, Config page...
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T11501
Differential Revision: https://secure.phabricator.com/D16421
Summary:
Ref T10917. This cheats fairly heavily to generate SSH key mail:
- Generate normal transaction mail.
- Force it to go to the user.
- Use `setForceDelivery()` to force it to actually be delivered.
- Add some warning language to the mail body.
This doesn't move us much closer to Glorious Infrastructure for this whole class of events, but should do what it needs to for now and doesn't really require anything sketchy.
Test Plan: Created and edited SSH keys, got security notice mail.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10917
Differential Revision: https://secure.phabricator.com/D15948
Summary:
Ref T10917. This primarily prepares these for transactions by giving us a place to:
- review old deactivated keys; and
- review changes to keys.
Future changes will add transactions and a timeline so key changes are recorded exhaustively and can be more easily audited.
Test Plan:
{F1652089}
{F1652090}
{F1652091}
{F1652092}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10917
Differential Revision: https://secure.phabricator.com/D15946
Summary:
Ref T10917. Currently, when you delete an SSH key, we really truly delete it forever.
This isn't very consistent with other applications, but we built this stuff a long time ago before we were as rigorous about retaining data and making it auditable.
In partiular, destroying data isn't good for auditing after security issues, since it means we can't show you logs of any changes an attacker might have made to your keys.
To prepare to improve this, stop destoying data. This will allow later changes to become transaction-oriented and show normal transaction logs.
The tricky part here is that we have a `UNIQUE KEY` on the public key part of the key.
Instead, I changed this to `UNIQUE (key, isActive)`, where `isActive` is a nullable boolean column. This works because MySQL does not enforce "unique" if part of the key is `NULL`.
So you can't have two rows with `("A", 1)`, but you can have as many rows as you want with `("A", null)`. This lets us keep the "each key may only be active for one user/object" rule without requiring us to delete any data.
Test Plan:
- Ran schema changes.
- Viewed public keys.
- Tried to add a duplicate key, got rejected (already associated with another object).
- Deleted SSH key.
- Verified that the key was no longer actually deleted from the database, just marked inactive (in future changes, I'll update the UI to be more clear about this).
- Uploaded a new copy of the same public key, worked fine (no duplicate key rejection).
- Tried to upload yet another copy, got rejected.
- Generated a new keypair.
- Tried to upload a duplicate to an Almanac device, got rejected.
- Generated a new pair for a device.
- Trusted a device key.
- Untrusted a device key.
- "Deleted" a device key.
- Tried to trust a deleted device key, got "inactive" message.
- Ran `bin/ssh-auth`, got good output with unique keys.
- Ran `cat ~/.ssh/id_rsa.pub | ./bin/ssh-auth-key`, got good output with one key.
- Used `auth.querypublickeys` Conduit method to query keys, got good active keys.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10917
Differential Revision: https://secure.phabricator.com/D15943
Summary:
Ref T4292. Currently, we hold one big lock around the whole `bin/repository update` workflow.
When running multiple daemons on different hosts, this lock can end up being contentious. In particular, we'll hold it during `git fetch` on every host globally, even though it's only useful to hold it locally per-device (that is, it's fine/good/expected if `repo001` and `repo002` happen to be fetching from a repository they are observing at the same time).
Instead, split it into two locks:
- One lock is scoped to the current device, and held during pull (usually `git fetch`). This just keeps multiple daemons accidentally running on the same host from making a mess when trying to initialize or update a working copy.
- One lock is scoped globally, and held during discovery. This makes sure daemons on different hosts don't step on each other when updating the database.
If we fail to acquire either lock, assume some other process is legitimately doing the work and bail more quietly instead of fataling. In approximately 100% of cases where users have hit this lock contention, that was the case: some other daemon was running somewhere doing the work and the error didn't actually represent an issue.
If there's an actual problem, we still raise a diagnostically useful message if you run `bin/repository update` manually, so there are still tools to figure out that something is hung or whatever.
Test Plan:
- Ran `bin/repository update`, `pull`, `discover`.
- Added `sleep(5)`, forced processes to contend, got lock exceptions and graceful exit with diagnostic message.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4292
Differential Revision: https://secure.phabricator.com/D15903
Summary:
Ref T4292. This is a required step in configuring a cluster: document and explain it.
Previously `bin/almanac register` could //also// add and trust keys. I've removed this capability since I think it's needless and complicated. If there's some real use for it eventually, we could add a `bin/almanac add-key` or whatever. The workflow is simpler and has better guard rails that point you in the correct direction now.
Test Plan:
- Read documentation.
- Ran `bin/almanac` with various good/bad flags.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4292
Differential Revision: https://secure.phabricator.com/D15795
Summary:
Ref T4292. Before we write or read a hosted, clustered Git repository over SSH, check if another version of the repository exists on another node that is more up-to-date.
If such a version does exist, fetch that version first. This allows reads and writes of any node to always act on the most up-to-date code.
Test Plan: Faked my way through this and got a fetch via `bin/repository update`; this is difficult to test locally and needs more work before we can put it in production.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4292
Differential Revision: https://secure.phabricator.com/D15757
Summary:
Ref T4292. This consolidates code for figuring out which user we should connect to hosts with.
Also narrows a lock window.
Test Plan: Browsed Diffusion, pulled and pushed through an SSH proxy.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4292
Differential Revision: https://secure.phabricator.com/D15754
Summary:
Ref T10751. We currently have a placeholder Almanac document, and a fairly-bad-advice section in Daemons.
Pull these into the modern cluster documentation.
Test Plan: 17 phabricator PHDs
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10751
Differential Revision: https://secure.phabricator.com/D15689
Summary:
Ref T10756. When repositories are properly configured for the cluster (which is hard to set up today), be smart about which repositories are expected to exist on the current host, and only pull them.
This generally allows daemons to pretty much do the right thing no matter how many copies are running, although there may still be some lock contention issues that need to be sorted out.
Test Plan: {F1214483}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10756
Differential Revision: https://secure.phabricator.com/D15682
Summary: Going to render these all normal case instead of all caps, and bump up the font size. Should be more consistent. Yellow if you green anything orange.
Test Plan: grep, lint
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15645
Summary: Cleans up EditEngine, adds new layout to EditEngine and descendents
Test Plan: Test creating a new form, reordering, marking and unmarking defaults. View new forms.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15531
Summary: Adds headers, new layout to edit panels on Almanac.
Test Plan: Pull up each edit panel in sandbox, save form.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15527
Summary:
Ref T10537. For Nuance, I want to introduce new sources (like "GitHub" or "GitHub via Nuance" or something) but this needs to modularize eventually.
Split ContentSource apart so applications can add new content sources.
Test Plan:
This change has huge surface area, so I'll hold it until post-release. I think it's fairly safe (and if it does break anything, the breaks should be fatals, not anything subtle or difficult to fix), there's just no reason not to hold it for a few hours.
- Viewed new module page.
- Grepped for all removed functions/constants.
- Viewed some transactions.
- Hovered over timestamps to get content source details.
- Added a comment via Conduit.
- Added a comment via web.
- Ran `bin/storage upgrade --namespace XXXXX --no-quickstart -f` to re-run all historic migrations.
- Generated some objects with `bin/lipsum`.
- Ran a bulk job on some tasks.
- Ran unit tests.
{F1190182}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10537
Differential Revision: https://secure.phabricator.com/D15521
Summary:
Ref T10537. More infrastructure:
- Put a `bin/nuance` in place with `bin/nuance import`. This has no useful behavior yet.
- Allow sources to be searched by substring. This supports `bin/nuance import --source whatever` so you don't have to dig up PHIDs.
Test Plan:
- Applied migrations.
- Ran `bin/nuance import --source ...` (no meaningful effect, but works fine).
- Searched for sources by substring in the UI.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10537
Differential Revision: https://secure.phabricator.com/D15436
Summary: Ref T10537. Minor updates to simplify and modernize these codepaths.
Test Plan: Searched for queues and sources.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10537
Differential Revision: https://secure.phabricator.com/D15429
Summary: Convert Almanac interfaces to Curtain views.
Test Plan: Viewed Services, Bindings, Devices, Namespaces and Networks.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D15415
Summary: Moves over to the new layout. Fixes T10521
Test Plan: Make a binding, view page, add some properties.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T10521
Differential Revision: https://secure.phabricator.com/D15404
Summary: Fixes T10449. Almanac doesn't do a whole lot for the average user, but is in good shape technically and works well, and exposing it in the cluster won't let installs destroy themselves now.
Test Plan: Re-read documentation; grepped for `TODO` (there are a couple, but reasonable to push off); browsed around all the UI things (new two-column looks great), called API methods.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10449
Differential Revision: https://secure.phabricator.com/D15400
Summary: Ref T10449. Modernize the AlmanacDevice code a bit.
Test Plan:
- Created a device.
- Edited a device.
- Listed devices.
- Viewed a device.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10449
Differential Revision: https://secure.phabricator.com/D15399
Summary: Ref T10449. This modernizes the service creation/editing flow and updates the list view code a little bit.
Test Plan:
- Created a service.
- Edited a service.
- Browsed services.
- Hit policy exception for editing cluster services with no permission.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10449
Differential Revision: https://secure.phabricator.com/D15398
Summary: Updates Almanac to the new layout, adds some header icons for interest.
Test Plan: Click on all the different almanac pages.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15381
Summary:
Ref T10449. Currently, we store classes (like "AlmanacClusterRepositoryServiceType") in the database.
Instead, store types (like "cluster.repository").
This is a small change, but types are a little more flexible (they let us freely reanme classes), a little cleaner (fewer magic strings in the codebase), and a little better for API usage (they're more human readable).
Make this minor usability change now, before we unprototype.
Also make services searchable by type.
Also remove old Almanac API endpoints.
Test Plan:
- Ran migration, verified all data migrated properly.
- Created, edited, rebound, and changed properties of services.
- Searched for services by service type.
- Reviewed available Conduit methods.
Reviewers: chad
Reviewed By: chad
Subscribers: yelirekim
Maniphest Tasks: T10449
Differential Revision: https://secure.phabricator.com/D15346
Summary:
Fixes T9762. Ref T10246.
**Disabling Bindings**: Previously, there was no formal way to disable bindings. The internal callers sometimes check some informal property on the binding, but this is a common need and deserves first-class support in the UI. Allow bindings to be disabled.
**Deleting Interfaces**: Previously, you could not delete interfaces. Now, you can delete unused interfaces.
Also some minor cleanup and slightly less mysterious documentation.
Test Plan: Disabled bindings and deleted interfaces.
Reviewers: chad
Reviewed By: chad
Subscribers: yelirekim
Maniphest Tasks: T9762, T10246
Differential Revision: https://secure.phabricator.com/D15345
Summary:
Fixes T6741. Ref T10246. Broadly, we want to protect Almanac cluster services:
- Today, against users in the Phacility cluster accidentally breaking their own instances.
- In the future, against attackers compromising administrative accounts and adding a new "cluster database" which points at hardware they control.
The way this works right now is really complicated: there's a global "can create cluster services" setting, and then separate per-service and per-device locks.
Instead, change "Can Create Cluster Services" into "Can Manage Cluster Services". Require this permission (in addition to normal permissions) to edit or create any cluster service.
This permission can be locked to "No One" via config (as we do in the Phacility cluster) so we only need this one simple setting.
There's also zero reason to individually lock //some// of the cluster services.
Also improve extended policy errors.
The UI here is still a little heavy-handed, but should be good enough for the moment.
Test Plan:
- Ran migrations.
- Verified that cluster services and bindings reported that they belonged to the cluster.
- Edited a cluster binding.
- Verified that the bound device was marked as a cluster device
- Moved a cluster binding, verified the old device was unmarked as a cluster device.
- Tried to edit a cluster device as an unprivileged user, got a sensible error.
{F1126552}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T6741, T10246
Differential Revision: https://secure.phabricator.com/D15339
Summary: Fixes T10411. Ref T10246. There are probably still some rough edges with this, but replace the old-school endpoints with modern ones so we don't unprototype with deprecated stuff.
Test Plan:
- Made a bunch of calls to the new endpoints with various constraints/attachments.
- Created and edited services, devices, interfaces, bindings, and properties on everything.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10246, T10411
Differential Revision: https://secure.phabricator.com/D15329
Summary:
Ref T10411. This cleans up / modernizes things and lets me get an `almanac.network.edit` API in the future.
This is mostly straightforward, except that Services have an extra "choose type" screen in front of them.
Test Plan:
- Created and edited Almanac networks, services, and devices.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10411
Differential Revision: https://secure.phabricator.com/D15326
Summary:
Fixes T10410. Immediate impact of this is that you can now actually delete properties from Almanac services, devices and bindings.
The meat of the change is switching from CustomField to EditEngine for most of the actual editing logic. CustomField creates a lot of problems with using EditEngine for everything else (D15326), and weird, hard-to-resolve bugs like this one (not being able to delete stuff).
Using EditEngine to do this stuff instead seems like it works out much better -- I did this in ProfilePanel first and am happy with how it looks.
This also makes the internal storage for properties JSON instead of raw text.
Test Plan:
- Created, edited and deleted properties on services, devices and bindings.
- Edited and reset builtin properties on repository services.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10410
Differential Revision: https://secure.phabricator.com/D15327
Summary:
Ref T10246. Ref T6741.
When you have a namespace like "phacility.net", require users creating services and devices within it to have edit permission on the namespace.
This primarily allows us to lock down future device names in the cluster, so instances can't break themselves once they get access to Almanac.
Test Plan:
- Configured a `phacility.net` namespace, locked myself out of it.
- Could not create new `stuff.phacility.net` services/devices.
- Could still edit existing devices I had permission for.
- Configured a `free.phacility.net` namespace with more liberal policies.
- Could create `me.free.phacility.net`.
- Still could not create `other.phacility.net`.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T6741, T10246
Differential Revision: https://secure.phabricator.com/D15325
Summary:
Ref T6741. Ref T10246.
Root problem: to provide Drydock in the cluster, we need to expose Almanac, and doing so would let users accidentally or intentionally create a bunch of `repo006.phacility.net` devices/services which could conflict with the real ones we manage.
There's currently no way to say "you can't create anything named `*.blah.net`". This adds "namespaces", which let you do that (well, not yet, but they will after the next diff).
After the next diff, if you try to create `repo003.phacility.net`, but the namespace `phacility.net` already exists and you don't have permission to edit it, you'll be asked to choose a different name.
Also various modernizations and some new docs.
Test Plan:
- Created cool namespaces like `this.computer`.
- Almanac namespaces don't actually enforce policies yet.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T6741, T10246
Differential Revision: https://secure.phabricator.com/D15324
Summary: Ref T6741. Ref T10246. This is largely modernization, but will partially support namespace locking in Almanac.
Test Plan:
Searched for Almanac networks by name substring.
{F1121740}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T6741, T10246
Differential Revision: https://secure.phabricator.com/D15322
Summary: Ref T10246. Build an ngram index for Almanac services, and use it to support improved search.
Test Plan: {F1121725}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10246
Differential Revision: https://secure.phabricator.com/D15321
Summary:
Fixes T10205. Ref T10246. Previously, the issue was that the result set was not ordered, so "More Results" would not have been able to work in a reasonable way if there were more than 100 matching interfaces.
You would have seen 100 interfaces more or less at random, then clicked "more" and gotten 100 more random interfaces.
Now, you would see 100 "a" interfaces, then click more to get the next 100 alphabetical interfaces (say, "b" and "c" interfaces).
Test Plan:
- Clicked browse when binding an interface.
- Got a browse dialog.
- Artificially set query limit to 1, paged through "local" interfaces in an ordered, consistent way.
{F1121313}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10205, T10246
Differential Revision: https://secure.phabricator.com/D15320
Summary:
Ref T10205. Ref T10246. This is general modernization, but also supports fixing the interface datasource in T10205.
- Update Query.
- Update SearchEngine.
- Use an ngrams index for searching names efficiently.
Test Plan:
- Ran migrations.
- Searched Almanac devices by name.
- Created a new device, searched for it by name.
{F1121303}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10205, T10246
Differential Revision: https://secure.phabricator.com/D15319
Summary: Mostly for consistency, we're not using other forms of icons and this makes all classes that use an icon call it in the same way.
Test Plan: tested uiexamples, lots of other random pages.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15125
Summary:
Ref T9979. The general shape of "engine" code feels pretty good, and I plan to move indexing to be more in line with other modern engines, with the ultimate goal of supporting subprojects (T10010) and several intermediate goals.
Before moving indexing, clean up Destruction, since some of the new indexes will need destruction hooks and destruction currently has a lot of `instanceof` stuff that should be easy to fix by applying more modern approaches.
Test Plan:
- Used `bin/remove destroy` to destory an Almanac device.
- Verified that properties for the device were destroyed.
- Viewed module panel in UI.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9979
Differential Revision: https://secure.phabricator.com/D14831
Summary: Ref T9762. Currently it is not possible to destroy an Alamanac device because any associate bindings cannot be destroyed.
Test Plan: Destroyed an Almanac device.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin
Maniphest Tasks: T9762
Differential Revision: https://secure.phabricator.com/D14461
Summary:
Ref T9253. See discussion in D13843.
I want to let Drydock blueprints for Almanac services choose those services from a typeahead, but only list appropriate services in the typeahead. To do this:
- Provide a StandardCustomField for an arbitrary datasource.
- Adjust the AlmanacServiceDatasource to allow filtering by service class.
This implementation is substantially the same as the one in D13843, with some adjustments:
- I lifted most of the code in the `Users` standard custom field into a new `Tokenizer` standard custom field.
- The `Users` and `Datasource` custom fields now extend the `Tokenizer` custom field and can share most of the code it uses.
- I exposed this field fully as a configurable field. I don't think anyone will ever use it, but this generality costs us nearly nothing and improves consistency.
- The code in D13843 didn't actually pass the parameters over the wire, since the object that responds to the request is not the same object that renders the field. Use the "parameters" mechanism in datasources to get things passed over the wire.
Test Plan:
- Created a custom "users" field in Maniphest and made sure it still wokred.
- Created a custom "almanc services" field in Maniphest and selected some services for a task.
- With additional changes from D13843, selected an appropriate Almanac service in a new Drydock blueprint.
Reviewers: hach-que, chad
Reviewed By: hach-que, chad
Maniphest Tasks: T9253
Differential Revision: https://secure.phabricator.com/D14111
Summary: Ref T9253. See D13843 for some discussion. This is very bare-bones for now since I believe that almost all interesting configuration (e.g., credentials) should live in Drydock, although I imagine it getting some configuration eventually.
Test Plan: Used {nav Almanac > Services > Create Service} to create a new service of this type.
Reviewers: hach-que, chad
Reviewed By: hach-que, chad
Maniphest Tasks: T9253
Differential Revision: https://secure.phabricator.com/D14109
Summary: Ref T8780. I think this fixes the issue.
Test Plan: @chad
Reviewers: chad
Reviewed By: chad
Subscribers: chad, epriestley
Maniphest Tasks: T8780
Differential Revision: https://secure.phabricator.com/D13584
Summary: Use `PhutilClassMapQuery` where appropriate.
Test Plan: Browsed around the UI to verify things seemed somewhat working.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13429
Summary: Ref T8686. This method has the wrong name and calls a nonexistent method on `$service`.
Test Plan: Set page size to 5, paged locally, hit error, applied changes, got clean paging.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8686
Differential Revision: https://secure.phabricator.com/D13460
Summary: Ref T8099. This adds a new class which all search engines return for layout. I thought about this a number of ways, and I think this is the cleanest path. Each Engine can return whatever UI bits they needs, and AppSearch or Dashboard picks and lays the bits out as needed. In the AppSearch case, interfaces like Notifications, Calendar, Legalpad all need more custom layouts. I think this also leaves a resonable path forward for NUX as well. Also, not sure I implemented the class correctly, but assume thats easy to fix?
Test Plan: Review and do a search in each application changed. Grep for all call sites.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13332
Summary: See also D13186.
Test Plan: Ran `arc unit --everything`
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D13201
Summary:
Ref T8441. Ref T7715. Automatically generate a modern "Order" control in ApplicationSearch for engines which fully support SearchField.
Notably, this allows the standard "Order" control to automatically support custom field orders. We do this in Maniphest today, but in an ad-hoc way.
Test Plan: Performed order-by queries in Almanac (Services), Pholio, Files, People, Projects, and Paste.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7715, T8441
Differential Revision: https://secure.phabricator.com/D13193
Summary: This got removed by accident in D12933.
Test Plan: Editing names works now.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D13144
Summary: Converts most all tables to be directly set via `setTable` to an ObjectBox. I think this path is more flexible design wise, as we can change the box based on children, and not just CSS. We also already do this with PropertyList, Forms, ObjectList, and Header. `setCollapsed` is added to ObjectBox to all children objects to bleed to the edges (like diffs).
Test Plan: I did a grep of `appendChild($table)` as well as searches for `PHUIObjectBoxView`, also with manual opening of hundreds of files. I'm sure I missed 5-8 places. If you just appendChild($table) nothing breaks, it just looks a little funny.
Reviewers: epriestley, btrahan
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D12955
Summary: Ref T6403. This was actually simple stuff.
Test Plan: changed the edit policy of a paste. changed the edit and join policy of a phame blog.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6403
Differential Revision: https://secure.phabricator.com/D12933
Summary: Ref T6403. This does TYPE_EDGE since I just had to deal with T8252. Look like this fixes a few editors (maybe) that would have had fatals with mentions like slowvote and ponder.
Test Plan: made a phame post mentioning a task and it worked! joined / left a project, watched / unwatched a project and that worked! blind faith for other sites.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6403
Differential Revision: https://secure.phabricator.com/D12929
Summary:
Fixes T6956. Before this change, we called PhabricatorUser::getOmnipotentUser in the various delete methods to query the data. Now, we use $engine->getViewer(), since its always a good thing to have less calls to PhabricatorUser::getOmnipotentUser thrown around the codebase.
I used the "codemod" tool to audit the existing calls to PhabricatorDestructorEngine (all of them) so ostensibly this gets all the spots. If I missed something though, its still going to work, so this change is very low risk.
Test Plan: ./bin/remove destroy P1; visit P1 and get a 404
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6956
Differential Revision: https://secure.phabricator.com/D12866
Summary:
New, cleaner, ObjectItemLists. Lots of minor style tweaks, basic overview:
- Remove FootIcons
- Remove Stackable
- Remove Plain List
- Add StatusIcon
- Add setting ObjectList to an ObjectBox
- Minor retouches to Headers
Mostly, this should give us an idea of life with the new Object Lists. I'll take another application by application pass down the road. This mostly looks at implementation in Maniphest, Differential, Audit, Workboards. Checked a few other areas and dialogs while testing, and everything looks square.
Test Plan: Maniphest, Differential, Homepage, Audit, People, and other applications. Drag reorder, etc.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D12865
Summary: Ref T4100. Let datasources specify a more meaningful title than the class name.
Test Plan: Browsed some sources.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: chad, epriestley
Maniphest Tasks: T4100
Differential Revision: https://secure.phabricator.com/D12469
Summary:
Ref T4100. Ref T5595. These functions are trivial for now, but move us toward being able to define more default query behavior by default.
Future changes will give these methods meaningful, nontrivial behaviors.
Test Plan: `arc unit --everything`
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5595, T4100
Differential Revision: https://secure.phabricator.com/D12454
Summary:
Ref T4100. Ref T5595.
To support a unified "Projects:" query across all applications, a future diff is going to add a set of "Edge Logic" capabilities to `PolicyAwareQuery` which write the required SELECT, JOIN, WHERE, HAVING and GROUP clauses for you.
With the addition of "Edge Logic", we'll have three systems which may need to build components of query claues: ordering/paging, customfields/applicationsearch, and edge logic.
For most clauses, queries don't currently call into the parent explicitly to get default components. I want to move more query construction logic up the class tree so it can be shared.
For most methods, this isn't a problem, but many subclasses define a `buildWhereClause()`. Make all such definitions protected and consistent.
This causes no behavioral changes.
Test Plan: Ran `arc unit --everything`, which does a pretty through job of verifying this statically.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: yelirekim, hach-que, epriestley
Maniphest Tasks: T4100, T5595
Differential Revision: https://secure.phabricator.com/D12453
Summary:
Ref T5750. These are a pain to modernize and most don't matter, so cheat:
- Mark a bunch nonbrowsable, including some that probably should be browsable but which I don't want to deal with for now.
- For static datasources, add an easy server-side filter (this isn't really cheating, and is appropriate for the status/priority/application datasources).
- Make composite sources browsable if their components are browsable.
Test Plan:
- Tried to browse an unbrowsable source, got a 404.
- Browsed a composite source.
- Browsed static sources (priority/status/applications).
- Browsed normal sources (people/projects).
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5750
Differential Revision: https://secure.phabricator.com/D12438
Summary: Ref T5750. Update the Almanac service query to be browsable.
Test Plan:
- Browsed and reordered Diffusion.
- Browsed and reordered services in Almanac.
{F373735}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5750
Differential Revision: https://secure.phabricator.com/D12433
Summary:
Ref T7803. Ref T5873. I want to drive Conduit through more shared infrastructure, but can't currently add parameters automatically.
Put a `getX()` around the `defineX()` methods so the parent can provide default behaviors.
Also like 60% of methods don't define any special error types; don't require them to implement this method. I want to move away from this in general.
Test Plan:
- Ran `arc unit --everything`.
- Called `conduit.query`.
- Browsed Conduit UI.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: hach-que, epriestley
Maniphest Tasks: T5873, T7803
Differential Revision: https://secure.phabricator.com/D12380
Summary: Ref T7689. Use the more modern handle load mechanisms in Almanac.
Test Plan:
- Viewed Almanac binding detail page.
- Viewed Almanac device interface list.
- Grepped for other callsites to InterfaceTableView.
- Viewed Almanac binding table.
- Grepped for other callsites to BindingTableView.
- Viewed Alamanc service table.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7689
Differential Revision: https://secure.phabricator.com/D12340
Summary:
Ref T7199. Convert the single help menu item into a dropdown and allow applications to list multiple items there.
When an application has mail command objects, link them in the menu.
Test Plan:
{F355925}
{F355926}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7199
Differential Revision: https://secure.phabricator.com/D12244
Summary:
Ref T7689. This serves two goals:
- I want to remove Controller->loadViewerHandles(). A nontrivial number of these callsites are loading handles to pass to tokenizers. Since tokenizers need to take strings eventually anyway, we can do less work by letting them take PHIDs now.
- A few changes out, I want tokenizers to accept parameterized tokens (like `viewer()`, `members(differential)`, etc.), so the `setValues()` signature needs to change eventually anyway.
I made this work and converted a handful of callsites as an example; upcoming changes will convert more.
Test Plan:
- Viewed Almanac binding editor; used "Interface".
- Edited Almanac services; used "Projects".
- Edited Almanac devices; used "Projects".
- Searched for commits; used "Auditors"; "Commit Authors", "Repositories".
- Searched for calendar events; used "Created By"; used "Invited".
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7689
Differential Revision: https://secure.phabricator.com/D12218
Summary: Ref T7627. This centralizes this transaction construction code so the unit tests and Instances can both use it.
Test Plan: See D12116.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7627
Differential Revision: https://secure.phabricator.com/D12118
Summary: Renames the method in PHUIObjectBoxView to match the new PHUIInfoView class.
Test Plan: grepped codebase. Went to Calendar and tried a new status.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D12005
Summary: Since this element isn't strictly about errors, re-label as info view instead.
Test Plan: Grepped for all callsites, tested UIExamples and a few other random pages.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: hach-que, Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11867
Summary: See D11882 for context and rationale.
Test Plan:
- Ran `almanac.querydevices`.
- Ran `almanac.queryserices`.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D11883
Summary:
This already exists on the Query, just expose it via API.
For context, I want to fix `bin/remote restart` so it actually identifies the services on a box rather than faking it.
To do this, I'll look up the device PHID of the box, then look up all the services which are bound to it, then look up all the instances attached to those services. This allows me to do the second step.
Test Plan: Called method from web UI, got expected result.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D11882
Summary: `ssh-keygen` declines to run on a too-public key. Write the correctly-restricted key a little earlier in the workflow.
Test Plan:
```
epriestley@orbital ~/dev/phabricator $ chmod 644 ~/dev/core/conf/keys/daemon.key
epriestley@orbital ~/dev/phabricator $ ./bin/almanac register --private-key ~/dev/core/conf/keys/daemon.key --identify-as local.phacility.net --device daemon.phacility.net --force --allow-key-reuse
Installing public key...
Installing private key...
Installing device ID...
HOST REGISTERED This host has been registered as "local.phacility.net" and a trusted keypair has been installed.
epriestley@orbital ~/dev/phabricator $
```
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D11700
Summary: Fixes T7118. This does the basic "filter the list" thing, though it ends up being a little manual since I guess this hasn't come up before? There is also potential weird behavior if the user was using an app and lost access to it - they will have nothing selected on edit - but I think this is actually correct behavior in this circumstance.
Test Plan:
used a user who couldn't get access to the "quick create" apps and noted that the dropdown list on dashboard panel create was missing the expected engines
ran `arc unit --everything` to verify abstract method implemented everywhere
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7118
Differential Revision: https://secure.phabricator.com/D11687
Summary: Clean up the error view styling.
Test Plan:
Tested as many as I could find, built additional tests in UIExamples
{F280452}
{F280453}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: hach-que, Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11605
Summary: Select a similar or better FontAwesome icon to represent each application
Test Plan: Visual inspection
Reviewers: epriestley, btrahan
Subscribers: hach-que, Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11489
Summary:
Ref T5833. In some cases, we need to know if an Almanac device is the localhost or not, so we can either handle or forward the request.
To accomplish this, write a device ID when running `bin/almanac register`.
Using `--allow-key-reuse` and `--identify-as`, multiple devices are permitted to //authenticate// as one device but //identify// as different devices. In the Phacility cluster, this allows all the `repoXXX` machines to have one keypair (making key management much easier) but still work as separate devices. This is an advanced feature; normal installs with 1-3 hosts would just generate a key + device per host and identify/authenticate as the same device.
Test Plan: Ran commands with lots of flags like `PHACILITY_INSTANCE=local sudo -E ./bin/almanac register --device daemon.phacility.net --private-key ~/dev/core/conf/keys/daemon.key --force --allow-key-reuse --identify-as local001.phacility.net`. Got a good result from `AlmanacKeys::getDeviceID()` afterward.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5833
Differential Revision: https://secure.phabricator.com/D11452
