Bug 1404092 - Bugzilla->localconfig should directly use environmental variables and ignore the localconfig file

7 files changed, 90 insertions, 78 deletions

diff --git a/.circleci/config.yml b/.circleci/config.yml
index 966ad1c9c..de289a3c5 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -24,6 +24,7 @@ defaults:
 
   bmo_env: &bmo_env
     PORT: 8000
+    LOCALCONFIG_ENV: 1
     BMO_db_user: bugs
     BMO_db_host: 127.0.0.1
     BMO_db_pass: bugs
@@ -95,7 +96,6 @@ jobs:
       - run:
           name: run sanity tests
           command: |
-            rm /app/localconfig
             /app/scripts/entrypoint.pl prove -qf $(circleci tests glob 't/*.t' | circleci tests split) | tee artifacts/$CIRCLE_JOB.txt
       - store_artifacts:
           path: /app/artifacts
@@ -108,14 +108,14 @@ jobs:
       - checkout
       - *default_qa_setup
       - run: |
-          rm -f /app/localconfig
           /app/scripts/entrypoint.pl load_test_data
       - run:
           command: |
-            rm -f /app/localconfig
             /app/scripts/entrypoint.pl test_webservices | tee artifacts/$CIRCLE_JOB.txt
       - store_artifacts:
           path: /app/artifacts
+      - store_artifacts:
+          path: /app/logs
 
   test_selenium:
     parallelism: 1
@@ -125,14 +125,14 @@ jobs:
       - checkout
       - *default_qa_setup
       - run: |
-          rm -f /app/localconfig
           /app/scripts/entrypoint.pl load_test_data --legacy
       - run:
           command: |
-            rm -f /app/localconfig
             /app/scripts/entrypoint.pl test_selenium | tee artifacts/$CIRCLE_JOB.txt
       - store_artifacts:
           path: /app/artifacts
+      - store_artifacts:
+          path: /app/logs
 
   test_bmo:
     parallelism: 1
@@ -153,18 +153,16 @@ jobs:
       - checkout
       - run: |
           mv /opt/bmo/local /app/local
-          perl checksetup.pl --no-database --default-localconfig
+          perl checksetup.pl --no-database
           perl -MSys::Hostname -i -pE 's/<<HOSTNAME>>/hostname()/ges' $BZ_QA_ANSWERS_FILE
-          rm -f /app/localconfig
           /app/scripts/entrypoint.pl load_test_data
           mkdir artifacts
       - run: |
           BZ_BASE_URL="http://$(hostname):$PORT"
           export BZ_BASE_URL
-          rm -f /app/localconfig
           /app/scripts/entrypoint.pl test_bmo -q -f t/bmo/*.t
-
-
+      - store_artifacts:
+          path: /app/logs
 
 workflows:
   version: 2
diff --git a/Bugzilla/Install/Filesystem.pm b/Bugzilla/Install/Filesystem.pm
index 3114d64be..2a2e9b71a 100644
--- a/Bugzilla/Install/Filesystem.pm
+++ b/Bugzilla/Install/Filesystem.pm
@@ -21,7 +21,8 @@ use warnings;
 
 use Bugzilla::Constants;
 use Bugzilla::Error;
-use Bugzilla::Install::Localconfig;
+use Bugzilla::Install::Localconfig qw(ENV_KEYS);
+
 use Bugzilla::Install::Util qw(install_string);
 use Bugzilla::Util;
 use Bugzilla::Hook;
@@ -101,6 +102,14 @@ use constant INDEX_HTML => <<'EOT';
 </html>
 EOT
 
+sub HTTPD_ENV_CONF {
+
+    return join( "\n",
+      "PerlPassEnv LOCALCONFIG_ENV",
+      map { "PerlPassEnv " . $_ } ENV_KEYS
+    ) . "\n";
+}
+
 ###############
 # Permissions #
 ###############
@@ -407,6 +416,9 @@ sub FILESYSTEM {
         "robots.txt"              => { perms     => CGI_READ,
                                        overwrite => 1,
                                        contents  => \&robots_txt},
+        "httpd/env.conf"          => { perms     => CGI_READ,
+                                       overwrite => 1,
+                                       contents  => \&HTTPD_ENV_CONF },
     );
 
     # Because checksetup controls the creation of index.html separately
diff --git a/Bugzilla/Install/Localconfig.pm b/Bugzilla/Install/Localconfig.pm
index 8807a44dd..c1c8fb12e 100644
--- a/Bugzilla/Install/Localconfig.pm
+++ b/Bugzilla/Install/Localconfig.pm
@@ -30,14 +30,20 @@ use List::Util qw(first);
 use Tie::Hash::NamedCapture;
 use Safe;
 use Term::ANSIColor;
+use Taint::Util qw(untaint);
 
 use parent qw(Exporter);
 
 our @EXPORT_OK = qw(
     read_localconfig
     update_localconfig
+    ENV_KEYS
 );
 
+# might want to change this for upstream
+use constant ENV_PREFIX     => 'BMO_';
+use constant PARAM_OVERRIDE => qw( inbound_proxies shadowdb shadowdbhost shadowdbport shadowdbsock );
+
 sub _sensible_group {
     return '' if ON_WINDOWS;
     return scalar getgrgid($EGID);
@@ -82,6 +88,7 @@ use constant LOCALCONFIG_VARS => (
         default => 'bugs',
     },
     {
+
         name    => 'db_user',
         default => 'bugs',
     },
@@ -149,7 +156,38 @@ use constant LOCALCONFIG_VARS => (
     },
 );
 
-sub read_localconfig {
+use constant ENV_KEYS => (
+    (map { ENV_PREFIX . $_->{name} } LOCALCONFIG_VARS),
+    (map { ENV_PREFIX . $_ } PARAM_OVERRIDE),
+);
+
+sub _read_localconfig_from_env {
+    my %localconfig;
+
+    foreach my $var ( LOCALCONFIG_VARS ) {
+        my $name = $var->{name};
+        my $key  = ENV_PREFIX . $name;
+        if ($name eq 'param_override') {
+            foreach my $override (PARAM_OVERRIDE) {
+                my $o_key = ENV_PREFIX . $override;
+                $localconfig{param_override}{$override} = $ENV{$o_key};
+                untaint($localconfig{param_override}{$override});
+            }
+        }
+        elsif (exists $ENV{$key}) {
+            $localconfig{$name} = $ENV{$key};
+            untaint($localconfig{$name});
+        }
+        else {
+            my $default = $var->{default};
+            $localconfig{$name} = ref($default) eq 'CODE' ? $default->() : $default;
+        }
+    }
+
+    return \%localconfig;
+}
+
+sub _read_localconfig_from_file {
     my ($include_deprecated) = @_;
     my $filename = bz_locations()->{'localconfig'};
 
@@ -211,6 +249,16 @@ sub read_localconfig {
     return \%localconfig;
 }
 
+sub read_localconfig {
+    my ($include_deprecated) = @_;
+
+    if ($ENV{LOCALCONFIG_ENV}) {
+        return _read_localconfig_from_env();
+    }
+    else {
+        return _read_localconfig_from_file($include_deprecated);
+    }
+}
 
 #
 # This is quite tricky. But fun!
@@ -239,6 +287,11 @@ sub read_localconfig {
 sub update_localconfig {
     my ($params) = @_;
 
+    if ($ENV{LOCALCONFIG_ENV}) {
+        require Carp;
+        Carp::croak("update_localconfig() called with LOCALCONFIG_ENV enabled");
+    }
+
     my $output      = $params->{output} || 0;
     my $answer      = Bugzilla->installation_answers;
     my $localconfig = read_localconfig('include deprecated');
@@ -319,7 +372,7 @@ sub update_localconfig {
     }
 
     # Reset the cache for Bugzilla->localconfig so that it will be re-read
-    delete Bugzilla->request_cache->{localconfig};
+    delete Bugzilla->process_cache->{localconfig};
 
     return { old_vars => \@old_vars, new_vars => \@new_vars };
 }
diff --git a/checksetup.pl b/checksetup.pl
index 99ee12e37..1b4df3c2f 100755
--- a/checksetup.pl
+++ b/checksetup.pl
@@ -149,8 +149,10 @@ Bugzilla->installation_answers($answers_file);
 # Check and update --LOCAL-- configuration
 ###########################################################################
 
-print "Reading " .  bz_locations()->{'localconfig'} . "...\n" unless $silent;
-update_localconfig({ output => !$silent, use_defaults => $switch{'default-localconfig'} });
+unless ($ENV{LOCALCONFIG_ENV}) {
+    print "Reading " .  bz_locations()->{'localconfig'} . "...\n" unless $silent;
+    update_localconfig({ output => !$silent, use_defaults => $switch{'default-localconfig'} });
+}
 my $lc_hash = Bugzilla->localconfig;
 
 ###########################################################################
diff --git a/httpd/httpd.conf b/httpd/httpd.conf
index 9df26b9db..8d0dad36d 100644
--- a/httpd/httpd.conf
+++ b/httpd/httpd.conf
@@ -59,6 +59,8 @@ LogFormat "%{User-agent}i" agent
 ServerSignature Off
 AddDefaultCharset UTF-8
 
+Include /app/httpd/env.conf
+
 PerlSwitches -wT
 PerlRequire /app/mod_perl.pl
 DirectoryIndex index.cgi
diff --git a/mod_perl.pl b/mod_perl.pl
index 42b74fde5..c682bece6 100644
--- a/mod_perl.pl
+++ b/mod_perl.pl
@@ -76,6 +76,12 @@ my $server = Apache2::ServerUtil->server;
 my $conf = Bugzilla::ModPerl->apache_config($cgi_path);
 $server->add_config([ grep { length $_ } split("\n", $conf)]);
 
+# Pre-load localconfig. It might already be loaded, but we need to make sure.
+Bugzilla->localconfig;
+if ($ENV{LOCALCONFIG_ENV}) {
+    delete @ENV{ (Bugzilla::Install::Localconfig::ENV_KEYS) };
+}
+
 # Pre-load all extensions
 Bugzilla::Extension->load_all();
 
diff --git a/scripts/entrypoint.pl b/scripts/entrypoint.pl
index faec25719..40e7cd577 100755
--- a/scripts/entrypoint.pl
+++ b/scripts/entrypoint.pl
@@ -31,7 +31,6 @@ my $opts = __PACKAGE__->can("opt_$cmd") // sub { @ARGV };
 fix_path();
 check_user();
 check_env() unless $cmd eq 'shell';
-write_localconfig( localconfig_from_env() );
 
 $func->(@ARGV);
 
@@ -218,7 +217,7 @@ sub wait_for_httpd {
             }
             elsif ( $process->is_exited ) {
                 $timer->stop;
-                $is_running_f->fail("process exited early");
+                $is_running_f->fail("httpd process exited early");
             }
             elsif ( $ticks++ > 60 ) {
                 $timer->stop;
@@ -284,67 +283,6 @@ sub on_finish {
     };
 }
 
-sub localconfig_from_env {
-    my %localconfig = ( webservergroup => 'app' );
-
-    my %override = (
-        'inbound_proxies' => 1,
-        'shadowdb'        => 1,
-        'shadowdbhost'    => 1,
-        'shadowdbport'    => 1,
-        'shadowdbsock'    => 1
-    );
-
-    foreach my $key ( keys %ENV ) {
-        if ( $key =~ /^BMO_(.+)$/ ) {
-            my $name = $1;
-            if ( $override{$name} ) {
-                $localconfig{param_override}{$name} = delete $ENV{$key};
-            }
-            else {
-                $localconfig{$name} = delete $ENV{$key};
-            }
-        }
-    }
-
-    return \%localconfig;
-}
-
-sub write_localconfig {
-    my ($localconfig) = @_;
-    no warnings 'once';
-
-    my $filename = "/app/localconfig";
-
-    die "/app/localconfig already exists!" if -f $filename;
-
-    foreach my $var (Bugzilla::Install::Localconfig::LOCALCONFIG_VARS) {
-        my $name = $var->{name};
-        my $value = $localconfig->{$name};
-        if (!defined $value) {
-            $var->{default} = &{$var->{default}} if ref($var->{default}) eq 'CODE';
-            $localconfig->{$name} = $var->{default};
-        }
-    }
-
-    # Ensure output is sorted and deterministic
-    local $Data::Dumper::Sortkeys = 1;
-
-    # Re-write localconfig
-    open my $fh, ">:utf8", $filename or die "$filename: $!";
-    foreach my $var (Bugzilla::Install::Localconfig::LOCALCONFIG_VARS) {
-        my $name = $var->{name};
-        my $desc = install_string("localconfig_$name", { root => Bugzilla::Install::Localconfig::ROOT_USER });
-        chomp($desc);
-        # Make the description into a comment.
-        $desc =~ s/^/# /mg;
-        print $fh $desc, "\n",
-                  Data::Dumper->Dump([$localconfig->{$name}],
-                                     ["*$name"]), "\n";
-   }
-   close $fh;
-}
-
 sub check_user {
     die "Effective UID must be 10001!" unless $EUID == 10001;
     my $user = getpwuid($EUID)->name;
@@ -358,6 +296,7 @@ sub check_data_dir {
 
 sub check_env {
     my @require_env = qw(
+        LOCALCONFIG_ENV
         BMO_db_host
         BMO_db_name
         BMO_db_user