Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
reubenmoes opened this issue Dec 10, 2021 · 4 comments
Labels
Support A support request

Comments

@reubenmoes
Copy link

reubenmoes commented Dec 10, 2021

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();
  }



  • BLT version: 12.x or 13.x

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();
   }

   /**

@reubenmoes reubenmoes added the Support A support request label Dec 10, 2021
@reubenmoes reubenmoes changed the title The recommended way to use a custom Environment Detector in a non-acquia environment. [looking for] The recommended way to use a custom Environment Detector in a non-acquia environment. Dec 10, 2021
@reubenmoes reubenmoes changed the title [looking for] The recommended way to use a custom Environment Detector in a non-acquia environment. [looking for] The recommended way to use a custom Environment Detector in a non-acquia production environment. Dec 10, 2021
@mikemadison13
Copy link
Contributor

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
Copy link
Contributor

shelane commented Dec 16, 2021

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
Copy link
Author

@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.

**

reubenmoes pushed a commit to reubenmoes/blt that referenced this issue Jan 28, 2022
jedubois added a commit to jedubois/blt that referenced this issue Jun 3, 2022
Add a new function that custom environment detectors can have return true when a non-local environment is detected.
@robbiehobby
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Support A support request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants