Improved management of settings files, introduce settings.ddev.php (#990

)

* Initial logic to create/include settings.php and settings.ddev.php

* Do a better job of managing settings, filepaths, etc.

* Move writing of common settings to its own function, new templates.

* Move getTemplateFuncMap to utils.go.

* Add user logging, pass around drupalSettings pointer.

* Update test.

* Update Drupal6/7 settings management.

* Combine repeated code into one shared function.

* Improve logging.

* Only create settings.ddev.php if one doesn't exist.

* Add .gitignore in sites/default.

* Move createGitIgnore function to utils.go.

* Improve logging, update function names.

* Apply new settings logic to Backdrop.

* Updates to tests and fix failures.

* Update comments.

* Add tests to validate settings management.

* Reduce replication in test.

* Remove test broken out into two new tests.

* Add tests to verify .gitignore creation behavior.

* Add documentation to new tests.

* Add function/constants documentation.

* Add comment to .gitignore template, update .gitignore format.

* Update the settings file expected to contain hash salt.

* Gotmetalinter disagrees with how I scope error values.

* Add ddev signature to appended includes, conditional check for ddev settings.

* Unconditionally create settings.ddev.php.

* Create test to ensure settings.ddev.php is always overwritten.

* Create struct to hold test settings location info.

* Explicitly ignore return values from os.Remove()

* Use test names as tmp dir prefixes.

* Add DrupalBackdrop to test and struct names limited to those app types.

* Add path of file to error output.

* Improve logic when appending include of settings.ddev.php, update method name.

* Do not output ddev-generated signature when modifying existing file.

* Write to an empty .gitignore file.

* Create expected directories relative to the app docroot.

* Write path of directory relevant to the error.

* Print dir, not settings file.

* Fix typo.

* Update to use path.Join in template outputs over filepath.Join.

* Use Drupal-defined path variables, update tests.

* Use standard include syntax and constants for each Drupal version and Backdrop, update template names.

* Update to ensure write permissions are set before modifying files.

* Remove debug output.

* Update settings file documentation.

* Remove unused declaration for debug output.

* Improve note about including local settings files.

docs/users/cli-usage.md

@@ -5,7 +5,7 @@ Type `ddev` or `ddev -h`in a terminal windows to see the available ddev commands
 
 ## Quickstart Guides
 
-These are quickstart instructions for WordPress, Drupal 7, Drupal 8, TYPO3, and Backdrop.
+These are quickstart instructions for WordPress, Drupal 6, Drupal 7, Drupal 8, TYPO3, and Backdrop.
 
 **Prerequisites:** Before you start, follow the [installation instructions](../index.md#installation). Make sure to [check the system requirements](../index.md#system-requirements), you will need *docker* and *docker-compose* to use ddev.
 
@@ -213,7 +213,7 @@ For in-depth application monitoring, use the command `ddev describe` to see deta
 
 ## Getting Started
 
-Check out the git repository for the project you want to work on. `cd` into the directory and run `ddev config` and follow the prompts.
+Check out the git repository for the project you want to work on. `cd` into the directory, run `ddev config`, and follow the prompts.
 
 ```
 $ cd ~/Projects
@@ -232,22 +232,7 @@ Docroot Location: web
 Found a drupal8 codebase at /Users/username/Projects/drupal8/web
 ```
 
-Configuration files have now been created for your project. (Take a look at the file on the project's .ddev/ddev.yaml file). Additionally, the `ddev config` steps attempts to create a CMS specific database settings with the DDEV specific credentials pre-populated. Here's a Drupal specific example that is mirrored in the other CMSes.
-
-* If settings.php does not exist, create it.
-* If a settings.php file exists that DDEV manages, recreate it.
-* If a settings.php file exists that DDEV does not manage, create settings.local.php.
-* If a settings.local.php file exists that DDEV manages, recreate it.
-* If a settings.local.php file exists that DDEV does not manage, warn the user and proceed.
-
-How do you know if DDEV manages a database settings file? You will see the following comment. Remove the comment and DDEV will not attempt to overwrite it!
-
-```
-/**
- #ddev-generated: Automatically generated Drupal settings.php file.
- ddev manages this file and may delete or overwrite the file unless this comment is removed.
- */
-```
+Configuration files have now been created for your project. Take a look at the project's .ddev/config.yaml file.
 
 Now that the configuration files have been created, you can start your project with `ddev start` (still from within the project working directory):
 
@@ -264,7 +249,29 @@ Your project can be reached at: http://drupal8.ddev.local
 
 And you can now visit your working project. Enjoy!
 
-_Please note that if you're providing the settings.php or wp-config.php and ddev is creating the settings.local.php (or wordpress wp-config-local.php), the main settings file must explicitly include the appropriate "settings.local.php" or equivalent._
+### Configuration files
+_**Note:** If you're providing the settings.php or wp-config.php and DDEV is creating the settings.ddev.php (or wp-config-local.php, AdditionalConfig.php, or similar), the main settings file must explicitly include the appropriate DDEV-generated settings file._
+
+The `ddev config` command attempts to create a CMS-specific settings file with DDEV credentials pre-populated.
+
+For **Drupal** and **Backdrop**, DDEV settings are written to a DDEV-managed file, settings.ddev.php. The `ddev config` command will ensure that these settings are included in your settings.php through the following steps:
+
+* Write DDEV settings to settings.ddev.php
+* If no settings.php file exists, create one that includes settings.ddev.php
+* If a settings.php file already exists, ensure that it includes settings.ddev.php, modifying settings.php to write the include if necessary
+
+For **TYPO3**, DDEV settings are written to AdditionalConfiguration.php.  If AdditionalConfiguration.php exists and is not managed by DDEV, it will not be modified.
+
+For **Wordpress**, DDEV settings are written to wp-config.php.
+
+How do you know if DDEV manages a settings file? You will see the following comment. Remove the comment and DDEV will not attempt to overwrite it!
+
+```
+/**
+ #ddev-generated: Automatically generated Drupal settings.php file.
+ ddev manages this file and may delete or overwrite the file unless this comment is removed.
+ */
+```
 
 ## Listing project information
 

pkg/ddevapp/backdrop.go

@@ -6,46 +6,70 @@ import (
 	"path/filepath"
 	"text/template"
 
-	"github.com/Masterminds/sprig"
+	"io/ioutil"
+
 	"github.com/drud/ddev/pkg/appports"
+	"github.com/drud/ddev/pkg/fileutil"
 	"github.com/drud/ddev/pkg/output"
 	"github.com/drud/ddev/pkg/util"
 )
 
 // BackdropSettings holds database connection details for Backdrop.
 type BackdropSettings struct {
-	DatabaseName     string
-	DatabaseUsername string
-	DatabasePassword string
-	DatabaseHost     string
-	DatabaseDriver   string
-	DatabasePort     string
-	DatabasePrefix   string
-	HashSalt         string
-	Signature        string
+	DatabaseName      string
+	DatabaseUsername  string
+	DatabasePassword  string
+	DatabaseHost      string
+	DatabaseDriver    string
+	DatabasePort      string
+	DatabasePrefix    string
+	HashSalt          string
+	Signature         string
+	SiteSettings      string
+	SiteSettingsLocal string
 }
 
 // NewBackdropSettings produces a BackdropSettings object with default values.
 func NewBackdropSettings() *BackdropSettings {
 	return &BackdropSettings{
-		DatabaseName:     "db",
-		DatabaseUsername: "db",
-		DatabasePassword: "db",
-		DatabaseHost:     "db",
-		DatabaseDriver:   "mysql",
-		DatabasePort:     appports.GetPort("db"),
-		DatabasePrefix:   "",
-		HashSalt:         util.RandString(64),
-		Signature:        DdevFileSignature,
+		DatabaseName:      "db",
+		DatabaseUsername:  "db",
+		DatabasePassword:  "db",
+		DatabaseHost:      "db",
+		DatabaseDriver:    "mysql",
+		DatabasePort:      appports.GetPort("db"),
+		DatabasePrefix:    "",
+		HashSalt:          util.RandString(64),
+		Signature:         DdevFileSignature,
+		SiteSettings:      "settings.php",
+		SiteSettingsLocal: "settings.ddev.php",
 	}
 }
 
-// Note that this template will almost always be used for settings.local.php
-// because Backdrop ships with it's own default settings.php.
-const backdropTemplate = `<?php
+// backdropMainSettingsTemplate defines the template that will become settings.php in
+// the event that one does not already exist.
+const backdropMainSettingsTemplate = `<?php
+{{ $config := . }}
+// {{ $config.Signature }}: Automatically generated Backdrop settings file.
+if (file_exists(__DIR__ . '/{{ $config.SiteSettingsLocal }}')) {
+  include __DIR__ . '/{{ $config.SiteSettingsLocal }}';
+}
+`
+
+// backdropSettingsAppendTemplate defines the template that will be appended to
+// settings.php in the event that one exists.
+const backdropSettingsAppendTemplate = `{{ $config := . }}
+// Automatically generated include for settings managed by ddev.
+if (file_exists(__DIR__ . '/{{ $config.SiteSettingsLocal }}')) {
+  include __DIR__ . '/{{ $config.SiteSettingsLocal }}';
+}
+`
+
+// backdropLocalSettingsTemplate defines the template that will become settings.ddev.php.
+const backdropLocalSettingsTemplate = `<?php
 {{ $config := . }}
 /**
- {{ $config.Signature }}: Automatically generated Backdrop settings.php file.
+ {{ $config.Signature }}: Automatically generated Backdrop settings file.
  ddev manages this file and may delete or overwrite the file unless this comment is removed.
  */
 
@@ -62,50 +86,96 @@ ini_set('session.gc_maxlifetime', 200000);
 ini_set('session.cookie_lifetime', 2000000);
 `
 
-// createBackdropSettingsFile creates the app's settings.php or equivalent,
-// adding things like database host, name, and password.
-// Returns the full path to the settings file and err.
+// createBackdropSettingsFile manages creation and modification of settings.php and settings.ddev.php.
+// If a settings.php file already exists, it will be modified to ensure that it includes
+// settings.ddev.php, which contains ddev-specific configuration.
 func createBackdropSettingsFile(app *DdevApp) (string, error) {
-	settingsFilePath, err := app.DetermineSettingsPathLocation()
+	settings := NewBackdropSettings()
+
+	if !fileutil.FileExists(app.SiteSettingsPath) {
+		output.UserOut.Printf("No %s file exists, creating one", settings.SiteSettings)
+		if err := writeBackdropMainSettingsFile(settings, app.SiteSettingsPath); err != nil {
+			return "", err
+		}
+	}
+
+	included, err := fileutil.FgrepStringInFile(app.SiteSettingsPath, settings.SiteSettingsLocal)
 	if err != nil {
-		return "", fmt.Errorf("Failed to get Backdrop settings file path: %v", err.Error())
+		return "", err
 	}
-	output.UserOut.Printf("Generating %s file for database connection.", filepath.Base(settingsFilePath))
 
-	backdropConfig := NewBackdropSettings()
+	if included {
+		output.UserOut.Printf("Existing %s includes %s", settings.SiteSettings, settings.SiteSettingsLocal)
+	} else {
+		output.UserOut.Printf("Existing %s file does not include %s, modifying to include ddev settings", settings.SiteSettings, settings.SiteSettingsLocal)
 
-	err = writeBackdropSettingsFile(backdropConfig, settingsFilePath)
-	if err != nil {
-		return settingsFilePath, fmt.Errorf("Failed to write Backdrop settings file: %v", err.Error())
+		if err := appendIncludeToBackdropSettingsFile(settings, app.SiteSettingsPath); err != nil {
+			return "", fmt.Errorf("failed to include %s in %s: %v", settings.SiteSettingsLocal, settings.SiteSettings, err)
+		}
 	}
 
-	return settingsFilePath, nil
+	if err := writeBackdropDdevSettingsFile(settings, app.SiteLocalSettingsPath); err != nil {
+		return "", fmt.Errorf("failed to write Drupal settings file %s: %v", app.SiteLocalSettingsPath, err)
+	}
+
+	if err := createGitIgnore(filepath.Dir(app.SiteLocalSettingsPath), settings.SiteSettingsLocal); err != nil {
+		output.UserOut.Warnf("Failed to write .gitignore in %s: %v", filepath.Dir(app.SiteLocalSettingsPath), err)
+	}
+
+	return app.SiteLocalSettingsPath, nil
 }
 
-// writeBackdropSettingsFile dynamically produces a valid settings.php file by
+// writeBackdropMainSettingsFile dynamically produces a valid settings.php file by
 // combining a configuration object with a data-driven template.
-func writeBackdropSettingsFile(settings *BackdropSettings, filePath string) error {
-	tmpl, err := template.New("settings").Funcs(sprig.TxtFuncMap()).Parse(backdropTemplate)
+func writeBackdropMainSettingsFile(settings *BackdropSettings, filePath string) error {
+	tmpl, err := template.New("settings").Funcs(getTemplateFuncMap()).Parse(backdropMainSettingsTemplate)
 	if err != nil {
 		return err
 	}
 
 	// Ensure target directory is writable.
 	dir := filepath.Dir(filePath)
-	err = os.Chmod(dir, 0755)
-	if err != nil {
+	if err = os.Chmod(dir, 0755); err != nil {
 		return err
 	}
 
 	file, err := os.Create(filePath)
 	if err != nil {
 		return err
 	}
-	err = tmpl.Execute(file, settings)
+	defer util.CheckClose(file)
+
+	if err := tmpl.Execute(file, settings); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// writeBackdropDdevSettingsFile dynamically produces a valid settings.ddev.php file
+// by combining a configuration object with a data-driven template.
+func writeBackdropDdevSettingsFile(settings *BackdropSettings, filePath string) error {
+	tmpl, err := template.New("settings").Funcs(getTemplateFuncMap()).Parse(backdropLocalSettingsTemplate)
 	if err != nil {
 		return err
 	}
-	util.CheckClose(file)
+
+	// Ensure target directory is writable
+	dir := filepath.Dir(filePath)
+	if err = os.Chmod(dir, 0755); err != nil {
+		return err
+	}
+
+	file, err := os.Create(filePath)
+	if err != nil {
+		return err
+	}
+	defer util.CheckClose(file)
+
+	if err := tmpl.Execute(file, settings); err != nil {
+		return err
+	}
+
 	return nil
 }
 
@@ -125,8 +195,10 @@ func getBackdropHooks() []byte {
 
 // setBackdropSiteSettingsPaths sets the paths to settings.php for templating.
 func setBackdropSiteSettingsPaths(app *DdevApp) {
+	settings := NewBackdropSettings()
 	settingsFileBasePath := filepath.Join(app.AppRoot, app.Docroot)
-	app.SiteSettingsPath = filepath.Join(settingsFileBasePath, "settings.local.php")
+	app.SiteSettingsPath = filepath.Join(settingsFileBasePath, settings.SiteSettings)
+	app.SiteLocalSettingsPath = filepath.Join(settingsFileBasePath, settings.SiteSettingsLocal)
 }
 
 // isBackdropApp returns true if the app is of type "backdrop".
@@ -143,3 +215,37 @@ func backdropPostImportDBAction(app *DdevApp) error {
 	util.Warning("Backdrop sites require your config JSON files to be located in your site's \"active\" configuration directory. Please refer to the Backdrop documentation (https://backdropcms.org/user-guide/moving-backdrop-site) for more information about this process.")
 	return nil
 }
+
+// appendIncludeToBackdropSettingsFile modifies the settings.php file to include the settings.ddev.php
+// file, which contains ddev-specific configuration.
+func appendIncludeToBackdropSettingsFile(settings *BackdropSettings, siteSettingsPath string) error {
+	// Check if file is empty
+	contents, err := ioutil.ReadFile(siteSettingsPath)
+	if err != nil {
+		return err
+	}
+
+	// If the file is empty, write the complete settings template and return
+	if len(contents) == 0 {
+		return writeBackdropMainSettingsFile(settings, siteSettingsPath)
+	}
+
+	// The file is not empty, open it for appending
+	file, err := os.OpenFile(siteSettingsPath, os.O_RDWR|os.O_APPEND, 0644)
+	if err != nil {
+		return err
+	}
+	defer util.CheckClose(file)
+
+	tmpl, err := template.New("settings").Funcs(getTemplateFuncMap()).Parse(backdropSettingsAppendTemplate)
+	if err != nil {
+		return err
+	}
+
+	// Write the template to the file
+	if err := tmpl.Execute(file, settings); err != nil {
+		return err
+	}
+
+	return nil
+}

pkg/ddevapp/ddevapp_test.go

@@ -460,11 +460,11 @@ func TestDdevImportDB(t *testing.T) {
 			// Test that a settings file has correct hash_salt format
 			switch app.Type {
 			case "drupal7":
-				drupalHashSalt, err := fileutil.FgrepStringInFile(app.SiteSettingsPath, "$drupal_hash_salt")
+				drupalHashSalt, err := fileutil.FgrepStringInFile(app.SiteLocalSettingsPath, "$drupal_hash_salt")
 				assert.NoError(err)
 				assert.True(drupalHashSalt)
 			case "drupal8":
-				settingsHashSalt, err := fileutil.FgrepStringInFile(app.SiteSettingsPath, "settings['hash_salt']")
+				settingsHashSalt, err := fileutil.FgrepStringInFile(app.SiteLocalSettingsPath, "settings['hash_salt']")
 				assert.NoError(err)
 				assert.True(settingsHashSalt)
 			case "wordpress":