drush icon indicating copy to clipboard operation
drush copied to clipboard

Alter site aliases for all command types

Open bomoko opened this issue 2 years ago • 15 comments

Existing document

Please provide a link to the existing document that is unclear or incomplete.

What are you attempting to do

The example given in the documentation implementing a @hook pre-init works very well when running commands like drush @somealias cr. However, I'm not able to get it to work in the case of a command like sql-sync where the aliases are passed as arguments, it's not clear the right way to approach this.

We're in the position that different site aliases are dynamically created as environment come online, which means we can't have a fixed site alias file. The sites themselves also potentially have different ssh details, as they may be sitting on different clusters.

This may actually be a feature request if this isn't possible.

One potential route would be some mechanism to either wrap or alter the siteAliasManager itself. But any way of being able to manipulate the siteAliasManager before it's consumed by the command functions would be great.

In what way is the existing documentation unclear or incomplete

It's not clear how, with drush commands like drush sql-sync @sourcealias @targetalias we are able to manipulate the values given by the siteAliasManager

What should the documentation say instead?

As mentioned above, this may be a feature request rather than a documentation request - but ideally there would be some documentation demonstrating how to accomplish this.

Thank you so much for any information you might be able to provide.

bomoko avatar Oct 13 '22 01:10 bomoko

Apologies for closing, I thought I might have figured out a way to achieve it. If anyone could help, it would be really appreciated.

bomoko avatar Oct 13 '22 02:10 bomoko

In general this is not possible. Suggested approach is to have a command/script that rebuilds the list as needed (or even as a pre step before running a drush command). How would you dynamically determine the list of aliases and their details?

weitzman avatar Oct 13 '22 02:10 weitzman

See #5225

weitzman avatar Oct 13 '22 02:10 weitzman

@weitzman - thanks for the reply. I see we're looping around to an earlier Lagoon issue as well. We've changed the way that the ssh service in Lagoon works in order to support direct ssh'ing to a cluster, not proxied via a central service as it currently is.

To answer your question about how we would determine the list of aliases, I can show you the POC where I was testing out the pre-init hook here https://github.com/amazeeio/drupal-integrations/pull/9/files

You'll see that our environments are centrally managed and that we pull their ssh details via a graphql call.

We could certainly write an alias file as a pre-step before running a command. In the long run, being able to hook into the siteAliasManager would be very cool - but I'm happy to take what's possible.

bomoko avatar Oct 13 '22 03:10 bomoko

How can we dynamically tell drush to read from an alias file we create, if we can't control the drush command - assuming there is also a hook to read the alias file?

tobybellwood avatar Oct 13 '22 03:10 tobybellwood

There are some default paths where drush looks for alias files, plus you can add more

  1. --alias-path cli option
  2. drush config. See https://www.drush.org/latest/using-drush-configuration/#specify-the-folders-to-search-for-drush-alias-files-siteyml.

weitzman avatar Oct 13 '22 03:10 weitzman

Hmm, will have to consider how much we can set any of these hardcoded files, switches or paths dynamically, hence the preference for using the drush internals. On a site-by-site basis it's easy, but doing it within our Drupal-integrations package is way preferable.

tobybellwood avatar Oct 13 '22 04:10 tobybellwood

One more thing I'll mention in case its helpful is that Drush recognizes site specifications in addition to site alias names. See https://www.drush.org/latest/site-aliases/#site-specifications. So you could have a wrapper like foo drush core:status and then foo does the graphql and finally runs drush user@server/path/to/drupal#uri core:status. No site alias file ever gets written.

weitzman avatar Oct 13 '22 11:10 weitzman

Thanks for that @weitzman - does makes sense, but before I run off and write a POC for this, does that address the case with sql-sync where we pass in aliases as arguments? That is, are we able to pass in multiple site specifications somehow, if not, I'm not entirely sure it's going to help.

Again, thank you for all of your help and insight on this. I'm thinking that we can likely close this particular issue. The one thing I would say, though, is it would be really cool if it were possible to provide a custom implementation of the SiteAliasManager - I don't know how feasible that would be, but just having the opportunity to override and replace the class would likely sort out most all the problems we've seen in the various issues linked to around this. I'm happy to try dig into this problem if you think it's worth a go.

bomoko avatar Oct 14 '22 18:10 bomoko

Site specifications should work fine when passed as argument to SQL sync. The docs even say say.

I also think replacing the SAM would be handy here. Maybe @greg-1-anderson can say more about the feasibility of that. We havent had swappable services in Drush yet so it would be a pretty big change. The alias manager is already a service in the Drush container.

weitzman avatar Oct 14 '22 19:10 weitzman

It's been a while since I've looked at this code. Isn't there an alias alter hook that you could attach to rather than using pre-init?

Regarding swapping out services, Drush isn't really designed for that. The DI container will provide references to a service to any object that requests one. By the time Drush is ready to run hooks, there are references to the site alias manager present in a number of files. The DI container doesn't keep a list (it just returns references; it never receives any info about who requested them, or which responses were cached). If you figured out all of the places where you had to fix up the site alias manager, that might work for the moment, but would be in danger of breaking at any time if another reference was added to the DI container.

We have talked about making some services indirect, e.g. a logger service that dispatches to the actual loggers, so that they could be dynamically altered by hooks. Do we want to do this for every service? Probably not.

Of all of the options, the one I would prefer would be adding hooks to the site alias manager, e.g. alias-not-found hook, populate additional alias fields hook, etc.; these would look like "register" / "addListener" methods on the site alias manager, with Drush code to find all of the site alias hooks and register them.

greg-1-anderson avatar Oct 14 '22 20:10 greg-1-anderson

Alias alter example is at https://raw.githubusercontent.com/drush-ops/drush/11.x/examples/Commands/SiteAliasAlterCommands.php. I'm not sure if that would work for a remote dispatch (e.g. drush @foo status) or for a sql sync (e.g. drush sql:sync @foo @bar). Would need some investigation.

weitzman avatar Oct 14 '22 21:10 weitzman

Hm, okay, so our alias alter example doesn't really alter the alias, it just examines the alias and sets global input options. I think that hook pre-init happens before redispatch, which I believe is just a hook init. I think it would be even better to have an actual alias alter hook. If no one does the work for that, then maybe the hook pre-init trick will work well enough.

greg-1-anderson avatar Oct 15 '22 00:10 greg-1-anderson

@greg-1-anderson - I'm more than happy to take a swing at doing an alias alter hook. Presumably this paragraph above gives the requisite design hints? Of all of the options, the one I would prefer would be adding hooks to the site alias manager, e.g. alias-not-found hook, populate additional alias fields hook, etc.; these would look like "register" / "addListener" methods on the site alias manager, with Drush code to find all of the site alias hooks and register them.

bomoko avatar Oct 16 '22 01:10 bomoko

@bomoko Yes; please let me know if you want any further clarification on any of my ideas.

greg-1-anderson avatar Oct 16 '22 04:10 greg-1-anderson