acquia / blt

Acquia's toolset for automating Drupal 8 and 9 development, testing, and deployment.
https://docs.acquia.com/blt/
GNU General Public License v2.0
442 stars 394 forks source link

[looking for] The recommended way to use a custom Environment Detector in a non-acquia production environment. #4437

Open reubenmoes opened 2 years ago

reubenmoes commented 2 years ago

I want to... I want to use BLT with a custom environment detector when hosting in non-acquia environments. Some of our clients are hosted on acquia, and others are not, but we like to use BLT for consistency in tooling. After BLT 11, the documented method seems to no longer work as expeted. https://docs.acquia.com/blt/extending-blt/#overriding-the-environment-detector

It's not working because... Environment Detector will incorrectly return true. What we will see, is in production environments config splits for local and prod are active.

$ cat blt/src/Blt/Plugin/EnvironmentDetector/CustomEnvironmentDetector.php 
<?php

namespace SmusSetup\Blt\Plugin\EnvironmentDetector;

use Acquia\Blt\Robo\Common\EnvironmentDetector;

class CustomEnvironmentDetector extends EnvironmentDetector {

  public static function isLocalEnv() {
    return !self::isStageEnv() && !self::isProdEnv() && !self::isDevEnv();
  }

  public static function isDevEnv() {
    return getenv('ENVIRONMENT_ID') == 'dev';
  }

  public static function isStageEnv() {
    return getenv('ENVIRONMENT_ID') == 'stage';
  }

  public static function isProdEnv() {
    return getenv('ENVIRONMENT_ID') == 'prod';
  }

}

After digging into it, it looks like it is related to: vendor/acquia/blt/src/Robo/Common/EnvironmentDetector.php which calls parent::isLocalEnv(), which will return true if it does not have the AH_ENV.

#File: vendor/acquia/blt/src/Robo/Common/EnvironmentDetector.php 
....
  /**
   * Is local.
   */
  public static function isLocalEnv() {
    $results = self::getSubclassResults(__FUNCTION__);
    if ($results) {
      return TRUE;
    }

    return parent::isLocalEnv() && !self::isPantheonEnv() && !self::isCiEnv();
  }
#file vendor/acquia/drupal-environment-detector/src/AcquiaDrupalEnvironmentDetector.php

  /**
   * If this isn't a Cloud environment, assume it's local.
   */
  public static function isLocalEnv() {
    return !self::isAhEnv() || self::isAcquiaLandoEnv();
  }
...
...
  /**
   * Get Acquia hosting environment.
   *
   * @return string
   *   Environment name (e.g. dev, stage, prod).
   */
  public static function getAhEnv() {
    return getenv('AH_SITE_ENVIRONMENT');
  }

...
...
  /**
   * Is AH env.
   */
  public static function isAhEnv() {
    return (bool) self::getAhEnv();
  }

I have made a workaround by patching blt, however I don't think this is the right approach.

$ cat patches/blt-environment-detector-local-override.patch 
diff --git a/src/Robo/Common/EnvironmentDetector.php b/src/Robo/Common/EnvironmentDetector.php
index e75f42f7..90bc7420 100644
--- a/src/Robo/Common/EnvironmentDetector.php
+++ b/src/Robo/Common/EnvironmentDetector.php
@@ -108,7 +108,7 @@ class EnvironmentDetector extends AcquiaDrupalEnvironmentDetector {
       return TRUE;
     }

-    return parent::isLocalEnv() && !self::isPantheonEnv() && !self::isCiEnv();
+    return !self::isDevEnv() && !self::isStageEnv() && !self::isProdEnv() && !self::isPantheonEnv() && !self::isCiEnv();
   }

   /**
mikemadison13 commented 2 years ago

here's an example of how to extend it to use in a non-acquia environment: https://github.com/mikemadison13/blt-gitlab-pipelines/blob/main/src/Blt/Plugin/EnvironmentDetector/GitlabDetector.php#L7

In this case, I'm adding a detection for Gitlab (which is non-Acquia).

shelane commented 2 years ago

Though there is something amiss with the environment detector.

 public static function isLocalEnv() {
    $results = self::getSubclassResults(__FUNCTION__);
    if ($results) {
      print ' says it is local from results being true ';
      return TRUE;
    }

    return parent::isLocalEnv() && !self::isPantheonEnv() && !self::isCiEnv();
  }

(I added some debugging code above). The "results" are false, so it doesn't not return true. It then continues on to the last line

return parent::isLocalEnv() && !self::isPantheonEnv() && !self::isCiEnv();

which then evaluates to true. So there is really no way for a custom environment detector to say that it is "not" local.

I have been meaning to report this as a bug.

reubenmoes commented 2 years ago

@mikemadison13 With your example, I still think the EnvironmentDetector would return true for isLocalEnv().

@shelane that's exactly it! **So there is really no way for a custom environment detector to say that it is "not" local.

**

robbiehobby commented 2 years ago

In the meantime, this patch completely disables the OOTB environment detection. Not suggesting this gets committed. Only a temporary workaround to force handoff to custom environment detectors.

acquia-env-detector-disable-defaults.txt