Drush alias failure with consolidation/site-alias requires change to BLT

[shelane]

The lightning profile includes consolidation/site-alias in composer requirements. Apparently, BLT has to change how drush alias files are created.
consolidation/site-alias#18

[greg-1-anderson]

Changing how Drush alias files are created is not necessary; the only change needed is to call $alias->hasRoot() prior to calling $alias->root(). The behavior of the former was ambiguous when the root was not defined. Now it throws an exception.

$alias->localRoot() may also be used. It is well-defined to return false if there is no root, or if the alias is remote. Otherwise it returns $alias->root().

[greg-1-anderson]

I decided to go ahead and roll back the breaking bugfix on the 1.x branch. BLT users may now update to site-alias 1.1.10 to continue using the current version of BLT. Pinning to 1.1.5 is still also possible, but updating to 1.1.10 is preferred.

BLT should update to site-alias ^2 as soon as convenient, and make the changes described above. There will be no further development on the 1.x branch.

[mikemadison13]

@greg-1-anderson thanks for jumping in on this. i think we can update the version no problem. I scanned our codebase and the $alias->root() method didn't immediately jump out at me, is there anything we need to do other than update consolidation/site-alias?

The only real place BLT actually "generates" aliases is here:
https://github.com/acquia/blt/blob/10.0.x/src/Robo/Commands/Generate/AliasesCommand.php

[greg-1-anderson]

Try upgrading to ^2; then the exception that @shelane encountered will return. I think she may have more info about the code path in blt where this is encountered.

[mikemadison13]

OK great. Yeah, @shelane if you have any additional details on how to reproduce your issue I'd love to try and get this fixed for a 10.x version tag.

[shelane]

I simply tried to do an install with blt drupal:install which ended up returning an error:

In AliasRecord.php line 76:
                                                      
  [Exception]                                         
  Site alias @default.local does not specify a root.  

Details are listed in the linked issue for consolidation/site-alias in my original message above.

[mikemadison13]

@shelane how long has it been since you generated your aliases (what version of blt / drush, etc.)?

i am looking at my project alias for what i'm on right now, and i have roots defined for all my aliases for both local and acquia cloud alias files, and i see it as an option here:

https://github.com/acquia/blt/blob/10.0.x/src/Robo/Commands/Generate/AliasesCommand.php#L339

local:
    root: /var/www/<project>/docroot
    uri: 'http://local.<project>.com'
    host: local.<project>.com
    user: vagrant
    ssh: { options: '-o PasswordAuthentication=no -i $HOME/.vagrant.d/insecure_private_key' }

[shelane]

Drupal version : 8.6.2
BLT 9.2.1

I just deleted my local.drush.yml file, ran blt:init:settings and a new drush file was created with no root.

[mikemadison13]

Ah, I see the issue. Was thinking actual drush aliases, not the local.drush file. Thanks! Will roll a PR for testing shortly.

[mikemadison13]

Actually, it looks like drush has this locked into version 1.5.x...

https://packagist.org/packages/drush/drush#9.5.2

Not sure if we can update this ahead of them?

We could certainly put in a PR to modify the behavior for local.drush.yml generation though, I think it's just a matter of adding the docroot as the default root in https://github.com/acquia/blt/blob/10.0.x/settings/default.local.drush.yml

[greg-1-anderson]

Drush 9.6.0-beta1 has been tagged, and allows for site-alias ^2.

You are not required to have a root. You are required to use hasRoot() to check to see if you have a root before calling root(). You can also use localRoot(), which works as root() used to, but only for aliases that do not have a remote host entry

[mikemadison13]

@greg-1-anderson that's the thing though, as far as I can tell, there is no BLT code that actually calls the root() method... i'm wondering if this bug should actually be on the drush side, not the BLT side?

[greg-1-anderson]

Possibly. Test with 9.6.0-beta1. If that clears up the problem, peg to site-alias ^1 until Drupal 9.6.0 stable is available.

[mikemadison13]

FYI to @balsama for visibility

[greg-1-anderson]

You can also do what Drush does, and select ^1.1.11|^2

[mikemadison13]

Part of the reason I tagged the Lightning team, is that BLT doesn't have an explicit requirement for this at all, so I want to make sure that the right things are in the right places (so if the new drush and the right version of site-alias needs to be locked in, that likely needs to be done in Lightning not BLT since we don't require it at all)

[greg-1-anderson]

Yeah if you do not use site-alias directly, then leave it out of your composer.json. It should be fine now, as ^1 will bring in 1.1.11 which should still work.

[danepowell]

Sounds like there are no remaining action items here for BLT.

[shelane]

Looks like the blt composer.lock file has updated consolidation/site-alias to 3.0 but did not actually update the method for handling site roots has not been updated. consolidation/site-alias#20

I now get this exception:

In SiteAliasTrait.php line 30:
                                                      
  [Exception]                                         
  Site alias @default.local does not specify a root.  
                                                      

Exception trace:
 () at /var/www/vendor/consolidation/site-alias/src/SiteAliasTrait.php:30
 Consolidation\SiteAlias\SiteAlias->root() at /var/www/vendor/drush/drush/src/Commands/core/SiteInstallCommands.php:297
 Drush\Commands\core\SiteInstallCommands->pre() at /var/www/vendor/consolidation/annotated-command/src/Hooks/Dispatchers/ValidateHookDispatcher.php:43
 Consolidation\AnnotatedCommand\Hooks\Dispatchers\ValidateHookDispatcher->callValidator() at /var/www/vendor/consolidation/annotated-command/src/Hooks/Dispatchers/ValidateHookDispatcher.php:27
 Consolidation\AnnotatedCommand\Hooks\Dispatchers\ValidateHookDispatcher->validate() at /var/www/vendor/consolidation/annotated-command/src/CommandProcessor.php:195
 Consolidation\AnnotatedCommand\CommandProcessor->validateRunAndAlter() at /var/www/vendor/consolidation/annotated-command/src/CommandProcessor.php:178
 Consolidation\AnnotatedCommand\CommandProcessor->process() at /var/www/vendor/consolidation/annotated-command/src/AnnotatedCommand.php:302
 Consolidation\AnnotatedCommand\AnnotatedCommand->execute() at /var/www/vendor/symfony/console/Command/Command.php:255
 Symfony\Component\Console\Command\Command->run() at /var/www/vendor/symfony/console/Application.php:978
 Symfony\Component\Console\Application->doRunCommand() at /var/www/vendor/symfony/console/Application.php:255
 Symfony\Component\Console\Application->doRun() at /var/www/vendor/symfony/console/Application.php:148
 Symfony\Component\Console\Application->run() at /var/www/vendor/drush/drush/src/Runtime/Runtime.php:118
 Drush\Runtime\Runtime->doRun() at /var/www/vendor/drush/drush/src/Runtime/Runtime.php:49
 Drush\Runtime\Runtime->run() at /var/www/vendor/drush/drush/drush.php:72
 require() at /var/www/vendor/drush/drush/drush:4

It should be pinned to 1.1.11.

[danepowell]

@shelane precisely what steps did you take to generate that error?

I'm assuming you are running a Drupal install on a project using BLT. In that case, BLT's own composer.lock should have zero influence on what version of site-alias gets installed (it's only used if you are actually developing against BLT, it doesn't get applied to downstream projects).

I suspect there's something wrong with your site aliases since BLT never directly invokes site-alias, and we have numerous automated tests to ensure that drupal installs work without error.

[greg-1-anderson]

@shelane From the stack trace above it looks like this error is in Drush site-install, not in BLT. What version of Drush are you using, and what does your alias look like? Maybe move this report to drush-ops/drush, as it looks like the bug is probably there.

[shelane]

Using Drush 9.6.2 and the command blt drupal:install -n to install Drupal.

The drush/sites/default.site.yml file looks like:

local:
  uri: '${env.VIRTUAL_HOST}'

And the docroot/sites/default/local.drush.yml file looks like:

options:
  uri: 'http://llnl8.docksal'

I know the "site root" is not defined in there and BLT doesn't put it in there when it generates the alias file.

[greg-1-anderson]

Hm, this is hard for me to reproduce. If @default.local does not have a root element, then presumably blt drupal:install -n is using the cwd to indicate which site to install. The failure cited in the backtrace is caused by the self alias not having a site root. I could fix the symptom by checking hasRoot(), but if there was no root at this point all I could do is throw an exception. This would improve the error message, but would not help the site:install command to work.

Not sure why blt drupal:install works sometimes but not in this instance. A bug report with more information on what is happening with Drush (e.g. steps to reproduce without blt) would be welcome in the Drush queue.

[shelane]

I really wish I had tome to get to the source of this. However, I have taken the path of backing out the upgrades I did to the point that things were working. I've locally restricted site-alias 1.1.11 and then ran the acquia/blt update to 9.2.6. This leaves the drush version at 9.5.2 as well and things are working fine.

[danepowell]

If you want to follow up, the next steps would be to run blt setup with -vvv, copy the full drush command for the site install (drush site-install ...), run it on its own to take BLT out of the equation, then create a Drush issue as Greg proposes with a copy of that command, all the output, and your alias files.

[dpagini]

Not sure if it's appropriate to re-open this... but what I've noticed is that BLT will work with a single site install, b/c the drush site-install command gets run with @self. If you have a multi-site, however, and you're using a local alias for those multi-sites, it is throwing the error.

That sort of explains why the OP was seeing the error with the @default.local site alias. I'm guessing that's explicitly set for her, but not for everyone out of the box with BLT.

[shelane]

@dpagini Yes! Thank you. I do have a multi-site setup.

[dpagini]

Wondering if the BLT fix might be to specify --root as part of the site-install command to fix this issue?
cc: @danepowell

[danepowell]

Isn't the proper fix here to make sure your aliases define a root? I just realized that aliases generated by the multisite command don't have a root, if that's where yours came from we can fix that.

Drush tasks in BLT specifically do not include a --root because we'd prefer to delegate that to Drush. For instance, running BLT against a host *AMP stack will have a different root than a VM. Having to manage those kinds of environment specifics in BLT would be heinous. Drush aliases were exactly made for that.

[dpagini]

So I'm not sure if this is a BLT thing (I could probably check pretty easy, will later if I have a chance) but we have a drush/sites/mysite.yml file, which has a local entry for everyone. The local entry just defines the hostname that's defined in the main BLT file, I believe. This file is checked into version control.

However, we don't set --root here for exactly that reason... this is a version control file, and every developer could have a different root path. So while we CAN do it for the Drupal VM b/c it's predictable based on the VM config, non VM users can clone the repo and run the site from any directory they wish.

Again, not a problem for the default local site which uses @self - but the multisites is definitely a problem. Maybe all multisite commands need to use @self and just specify the --site variable? Would that fix it?

[greg-1-anderson]

The use case for defining an alias without a 'root' element is for instances where ssh-ing in to the provided 'host' sets your cwd to a directory where Drush may implicitly find the Drupal root.

I do not think that a local entry for everyone alias is useful. Is everyone expected to edit this version-controlled file, and then be careful not to commit it back to your repository? Perhaps there is some good way to determine the root in this instance that I am not considering; however, failing that, I am 👎 on this instance of rootless (local) site aliases.

There are two good ways in Drush 9 to share aliases in a site repository.

1. Environment variables:

Drush 9 aliases can expand environment variables. If you require everyone to:

a. Check out your site name into a directory named after the repo (git default)
b. Define an environment variable pointing to the directory where all local sites are stored

Then you can set 'root' to ${env.LOCAL_SITES}/mysite

2. Alias file merging

Require all developers to set up a simple local alias ~/.drush/sites/mysite.yml that includes just:

dev:
  root: /path/to/wherever/i/put/mysite
  uri: https://dev.mysite.com

Then, you can define drush/sites/self.yml, and this alias file will automatically be merged with mysite.yml during alias resolution. So, if you ask for @mysite.live, then Drush will find @mysite.dev, will identify the root entry, and from that it will find the drush/sites/self.yml definition, merge it, and find @mysite.live (presuming, of course, that self.yml has the key live: with the definition for the live site). With this technique, you can share your remote aliases in your repo, and team members only need to define an alias for their local site. (If they do not, they can still use the cwd and @self.live to get to the live site.)

So, alias files that do not define a root should be very rare, and should only be used for remote aliases. All local aliases should define a root.

[danepowell]

So from a BLT perspective, it seems like we should recommend that the root always be set via an alias. Either because you are using a VM / Lando where the root can be hardcoded, or via one of the methods described by Greg (these should be documented in BLT and / or Drush). Add a note about this to the multisite alias generation script, which can't set a root for non-VM users.

BLT could also add a configuration option for root that will be used by any Drush commands if it's set. Individual developers could then set the root via their local.blt.yml. However we'd probably keep it a hidden-ish option, and you'd probably have to use this at your own risk, since I have no idea what edge cases could come up with that (interaction with roots defined in alias files, etc...). I agree with Greg that this is probably not best practice, although I understand why it might be preferable in some cases.

[greg-1-anderson]

If you are expecting to identify your site via cwd, and then use rootless aliases only to set the site's uri, then I could see that as a potential use-case. Drush doesn't do that at the moment, though; if that's your aim, then please file a feature request in the Drush queue.

Also, the Drush site-install bug is still a bug, regardless of the type of alias used to produce it. If I can get a clear description with steps to reproduce in the Drush queue, then I'll fix it.

[danepowell]

Yeah actually I agree, this should probably get fixed upstream in either Drush or consolidation/site-alias. Upstream issue: drush-ops/drush#4059

Even our internal tests are now failing when we try to use any alias other than self: #3614

[greg-1-anderson]

Hm I don't know if I missed that Drush issue you posted or what, because it looks like it's clear enough how to reproduce it. I'll try it again and see if I can make it behave more uniformly.

[danepowell]

I fixed our internal tests, and suggest folks work around this until the Drush issue is resolved, using Greg's comment in #3614 (basically just set the root to ${env.cwd}/docroot)

If you regenerated your multisite aliases using #3614 then BLT should set the root this way for you.

Is any other action required of BLT or can we close this issue?

[dpagini]

suggest folks work around this until the Drush issue is resolved, using Greg's comment in #3614 (basically just set the root to ${env.cwd}/docroot)

So if I'm not mistaken, the work-around means you could only use the alias from the BLT project root at this time, right? So right now, I can call $ drush @second_site.local status from any folder in the project and it will work for me, it just breaks the site-install command, right? If I set the root with that environment variable, I am fixing site-install but breaking the functionality of the alias from every other folder.... is that right?

[greg-1-anderson]

@dpagini I'm not sure if I understand your use-case. How can $ drush @second_site.local status possibly work if @second_site.local does not have a root element, and your cwd is not at the Drupal root or project root for second_site?

OIC, you currently have the ability to set the cwd to any directory inside the project, not just at the project root. That is what you are referring to, correct?

As far as I know, the bug reported here only affects site-install.

[dpagini]

OIC, you currently have the ability to set the cwd to any directory inside the project, not just at the project root. That is what you are referring to, correct?

Correct. I mean, BLT sets the CWD for all it's commands, so the "workaround" would work for BLT commands (I think).
I guess I was saying that while the work-around fixes BLT commands, it breaks the fact that the aliases I have now for my multi-site are working as I would expect. So this "workaround" is actually making things a little worse for me, but it fixes the site-install bug.

[lcatlett]

Should BLT not install sites locally identical to how sites are installed by the hosting provider? The change in BLT to use an alias rather than the default @self alias, namely in https://github.com/acquia/blt/pull/3614/files#diff-036b3640d738b718066597b1db31e407R195, was previously found to cause ssh recursion to localhost within the VM and was a deliberate fix in #3231. #3614 is not really an approach that Acquia will be able to support

In general, drush aliases are rarely, if ever, used in site-install by design, as this would require aliases to be managed and updated by hosting provider on every site install request or creation of a new multisite, as the use of aliases during site install it would likely mean that no values could be overridden in ~/.drush and site-local drush alias files. This is why --root and --uri and other command line overrides should always be used as a safeguard because on large multisite hosting platforms like Acquia Site Factory for example, the user could theoretically install a site over the default site's database or execute other unauthorized task which could compromise the integrity of the platform. This approach would also require an alias to be committed and updated prior to installing every new site, which creates a fundamental chicken/egg problem for sites/providers where the site name and uri is not known in advance and committed to code.

This shift in BLT will also likely result in additional challenges on hosting providers that do not allow write access to the file system, as the --uri used to install or create a production site is almost always different than than the "go live" uri and instead often uses the vendor-provided default uri. For scenarios where a user feels they must use an alias in BLT or other projects, a more scalable and portable solution could be to use dynamic aliases which instead use the current request to populate variables in templates rather than hard-coded config that has to be updated and deployed for values such as --host, --root, --uri.

@danepowell and @nateacquia it would be helpful if the BLT team could include testing steps and/or clarify when PRs such as #3614 have not been reviewed or tested on multisite or products like ACSF, as doing otherwise is creating a burden on both users and support/engineering teams.

[greg-1-anderson]

I think all of this is just a workaround until someone manages to fix the bug in drush site-install. Haven't had a chance to get to that myself yet.

[no-response]

This issue has been automatically closed because there has been no response to our request for more information from the original author. With only the information that is currently in the issue, we don't have enough information to take action. Please reach out if you have or find the answers we need so that we can investigate further.

[dpagini]

So I know we are talking about a workaround for this above, but I'm still unclear how this is supposed to work...
Sorry to comment on a closed issue, I just don't know at this time if it's worth opening a new issue... but if deemed so, I'm happy to open and document.

Our team is not on a standard build (Drupal-VM, Lando, etc).. but let's say I could get every developer on the same hard-coded root for multi-site... I then still have my Travis builds which I'm not sure I can change (maybe I can, but man this is getting complicated).

If there's not an immediate fix for drush on the horizon (the issue has been opened for almost 3 months now), is there any chance BLT could consider passing in the root to the site-install command? Is there any downside to that? Presumably BLT knows the site docroot, right? It doesn't have to be for all commands, but just site-install (and just for multisites if we want to go that far).

[danepowell]

I'm not sure I fully understand the issue here, because no one provided clear steps to reproduce on a standard BLT project. Feel free to open a new issue if you can provide steps to reproduce. However is this not a dupe of #3467?

[dpagini]

I don't think this is a dupe. I'll open a new ticket. Thanks @danepowell!