Issue 4641 - Append performance fix to trac cgi file

modules/trac/manifests/init.pp

 class trac(
     $domain,
     $certificate,
     $private_key,
     $fcgi_config_dir = '/etc/nginx/trac.d',
     $is_default = false) inherits private::trac {
 
   package {['python-mysqldb','python-pip','subversion', 'tofrodos', 'graphviz']:
     ensure => present
   }
 
-  include nginx, spawn-fcgi
+  include stdlib,
+    nginx,
+    spawn-fcgi
 
   file {$fcgi_config_dir:
     ensure => directory,
     owner => 'root',
     mode => 755,
     require => Package['nginx'],
   }
 
   nginx::hostconfig {$domain:
     content => "include $fcgi_config_dir/*;",
@@ skipping 71 matching lines @@
     command => "pip install TracThemeEngine",
     require => Package['python-pip'],
     unless => "python -c 'import themeengine'",
   }
 
   package { 'spambayes':
         ensure => "installed"
   }
 
   exec { 'install_TracSpamFilter':
-    command => "pip install svn+http://svn.edgewall.com/repos/trac/plugins/1.0/spam-filter",
+    command => "pip install svn+https://svn.edgewall.org/repos/trac/plugins/1.0/spam-filter",
     require => Package[
         'spambayes',
         'python-pip'],
     unless => "python -c 'import tracspamfilter'",
   }
 
   exec { 'install_Tractags':
     command => "pip install svn+https://trac-hacks.org/svn/tagsplugin/tags/0.7/",
     require => Package['python-pip'],
     unless => "python -c 'import tractags'",
@@ skipping 23 matching lines @@
     owner => 'trac',
     mode => 644,
   }
 
   file {'/home/trac/robots.txt':
     ensure => 'present',
     source => 'puppet:///modules/trac/robots.txt',
     owner => 'trac',
     mode => 644,
   }
+
+  file {"trac_performance_fix_py":
+    ensure => present,
+    path => '/usr/local/lib/python2.7/dist-packages/trac_performance_fix.py',
+    source => 'puppet:///modules/trac/trac_performance_fix.py',
+    owner => 'root',
+    mode => 644,
+  }
+
 
   define instance (
       $config = 'trac/trac.ini.erb',
       $description = 'Issue Tracker',
       $location = '/',
       $logo = 'puppet:///modules/trac/logo.png',
       $database = 'trac',
       $permissions = 'puppet:///modules/trac/permissions.csv',
       $theme = 'puppet:///modules/trac/theme.css') {
 
@@ skipping 68 matching lines @@
         Exec['install_TicketTemplate'],
         Exec['install_NeverNotifyUpdater'],
         Exec['install_MasterTickets'],
         Exec['install_ThemeEngine'],
         Exec['install_Tractags'],
         Exec['install_TracSpamFilter'],
         Exec['install_PrivateTickets'],
         Exec['install_TracXMLRPC'],
         Exec['install_TracHTTPAuth']],
     }
-
-# Ref https://issues.adblockplus.org/ticket/4641
-$cgi_performance_fix = "

[Wladimir Palant, 2016/11/16 09:26:18]

Nit: This isn't really related to CGI in any way, it's a Trac performance fix -
so maybe this variable should be called $trac_performance_fix.

[f.nicolaisen, 2016/11/16 15:01:28]

Acknowledged.

-### Local modifications to the trac.fcgi file
-from trac.web.session import Session
-orig_get = Session.get
-def patched_get(self, key, default=None):
-    if key == 'accesskeys':
-        return '1'
-    return orig_get(self, key, default)
-Session.get = patched_get
-### End of local modifications
-"
-
+  
     exec {"deploy_$name":
       command => "trac-admin /home/trac/$environment \
         deploy /home/trac/htdocs-$name \
         && fromdos /home/trac/htdocs-$name/cgi-bin/trac.fcgi \
         && chmod 755 /home/trac/htdocs-$name/cgi-bin/trac.fcgi",
       user => trac,
       path => '/usr/bin:/usr/sbin:/bin:/usr/local/bin',
       require => [
         Exec["update_env_$name"],
         Package["tofrodos"]],
-    } ->
-    exec {"append_cgi_config_to_$name":
-      command => "echo \"$cgi_performance_fix\" >> /home/trac/htdocs-$name/cgi-bin/trac.fcgi",

[Wladimir Palant, 2016/11/16 09:26:18]

Actually, this needs to be added at the top of the file - fcgi_frontend.run()
only returns when the server shuts down so the code you added here won't run.
Currently this code has been placed after the copyright comment but it could
also go directly after the shebang.

[f.nicolaisen, 2016/11/16 15:01:28]

Acknowledged.

-      user => trac,
-      path => '/usr/bin:/usr/sbin:/bin:/usr/local/bin',

[Wladimir Palant, 2016/11/16 09:26:18]

The way I see it, this command will run every time you provision the server, so
you will end up adding the code multiple times. I guess that something like
`unless => 'grep "Local modifications"
/home/trac/htdocs-$name/cgi-bin/trac.fcgi'` should work here.

And then we still need to consider the scenario that our local modifications
code changes in future. So maybe run the modifications unconditionally but
replace the existing "Local modifications" section if there is one already?
Getting rather complicated, I guess that this logic would be better off in a
Python script.

[f.nicolaisen, 2016/11/16 15:01:28]

Won't it only run after the "deploy_$name" exec is run?

[Wladimir Palant, 2016/11/16 15:16:44]

Given that deploy_$name runs every time - sure. Now that you mention it, I guess
that deploy_$name overwrites trac.fcgi so that there is nothing wrong with
adding our modifications to it after that. Feel free to test this however.

+    }
+  
+    file_line { "patch $name trac.fcgi":
+      path => "/home/trac/htdocs-$name/cgi-bin/trac.fcgi",
+      match => '^# Author.*$',
+      line => "# Author\nimport trac_performance_fix",
+      require => Exec["deploy_$name"],
     }
 
     file {"/home/trac/htdocs-$name/htdocs/common/logo.png":
       ensure => present,
       source => $logo,
       owner => trac,
       require => Exec["deploy_$name"],
     }
 
     spawn-fcgi::pool {"tracd_${name}":
@@ skipping 15 matching lines @@
   # Daily restart required for log rotation of all instances at once
   cron {'restart-trac-daily':
     command => 'service spawn-fcgi restart >/tmp/spawn-fcgi-restart.log',
     environment => hiera('cron::environment', []),
     hour => '1',
     minute => '0',
     user => 'root',
   }
 }