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

Static configuration of webserver and dbserver localhost ports, bind only to localhost, fixes #1491, fixes #941, #642 too #1502

Merged
merged 26 commits into from Mar 28, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
4a4fe93
go.mod add phayes/freeport to get explicit free tcp port
rfay Mar 20, 2019
cc5013b
Add configuration to expliictly bind to a static host db port, fixes …
rfay Mar 20, 2019
02cc79a
Add static binding to webserver localhost port
rfay Mar 20, 2019
b224270
Fix panic in test
rfay Mar 21, 2019
b650164
Use docker IP instead of 127.0.0.1 in docker-compose.yaml
rfay Mar 21, 2019
b277d16
Refactor util.GetFreePort and add TestGetFreePort()
rfay Mar 22, 2019
9bd0ce6
Revert "go.mod add phayes/freeport to get explicit free tcp port"
rfay Mar 22, 2019
5ce7ab0
Fix signature for GetFreePort()
rfay Mar 22, 2019
cd5fc83
Add check for docker toolbox port
rfay Mar 22, 2019
fb4fd80
Work on test proves that this isn't quite valid
rfay Mar 22, 2019
84d538f
Rework strategy to use map to determine availability
rfay Mar 22, 2019
1521bfd
Add TestPortSpecifications to validate port collision behavior
rfay Mar 22, 2019
33521db
Switch to a project list strategy, much more expandable
rfay Mar 22, 2019
4114e5a
Remove config.yaml between projects in TestConfigInvalidProjectname
rfay Mar 23, 2019
a7bc650
Need fullpath in NewApp()
rfay Mar 24, 2019
d62115e
Add check that localhost web container bind port is working
rfay Mar 25, 2019
aa917f3
Do quick and dirty test of mysql connectivity on localhost port
rfay Mar 25, 2019
e143931
Fix minor rebase fail
rfay Mar 25, 2019
96cbae1
Don't always reserve ports, only reserve if in config.yaml
rfay Mar 26, 2019
607adeb
Rework TestPortSpecifications to match updated behavior
rfay Mar 26, 2019
fe9e614
More work on TestPortSpecifications
rfay Mar 26, 2019
709c6dc
Make TestCmdGlobalConfig less fragile
rfay Mar 27, 2019
8512473
Add --host-db-port and --host-webserver-port
rfay Mar 27, 2019
b66cc08
Use flow only for UsedHostPorts
rfay Mar 27, 2019
82fef8a
Add test coverage for --host-db-port and --host-webserver-port
rfay Mar 27, 2019
6701e16
Omit host ports when empty in global config
rfay Mar 27, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 5 additions & 8 deletions cmd/ddev/cmd/config-global_test.go
Expand Up @@ -37,13 +37,10 @@ func TestCmdGlobalConfig(t *testing.T) {
assert.NoError(err)
assert.Contains(string(out), "Global configuration:\ninstrumentation-opt-in=false\nomit-containers=[dba,ddev-ssh-agent]")

found, err := fileutil.FgrepStringInFile(configFile, "\ninstrumentation_opt_in: false")
err = globalconfig.ReadGlobalConfig()
assert.NoError(err)
assert.True(found)
found, err = fileutil.FgrepStringInFile(configFile, "\n- dba")
assert.NoError(err)
assert.True(found)
found, err = fileutil.FgrepStringInFile(configFile, "\n- ddev-ssh-agent")
assert.NoError(err)
assert.True(found)
assert.False(globalconfig.DdevGlobalConfig.InstrumentationOptIn)
assert.Contains(globalconfig.DdevGlobalConfig.OmitContainers, "ddev-ssh-agent")
assert.Contains(globalconfig.DdevGlobalConfig.OmitContainers, "dba")
assert.Len(globalconfig.DdevGlobalConfig.OmitContainers, 2)
}
22 changes: 19 additions & 3 deletions cmd/ddev/cmd/config.go
Expand Up @@ -104,6 +104,12 @@ var (

// nfsMountEnabled sets nfs_mount_enabled
nfsMountEnabled bool

// hostDBPortArg sets host_db_port
hostDBPortArg string

// hostWebserverPortArg sets host_webserver_port
hostWebserverPortArg string
)

var providerName = ddevapp.ProviderDefault
Expand All @@ -115,7 +121,7 @@ var extraFlagsHandlingFunc func(cmd *cobra.Command, args []string, app *ddevapp.
var ConfigCommand *cobra.Command = &cobra.Command{
Use: "config [provider or 'global']",
Short: "Create or modify a ddev project configuration in the current directory",
Example: `"ddev config" or "ddev config --docroot=. --project-name=d7-kickstart --project-type=drupal7"`,
Example: `"ddev config" or "ddev config --docroot=web --project-type=drupal8"`,
Args: cobra.ExactArgs(0),
Run: handleConfigRun,
}
Expand Down Expand Up @@ -183,8 +189,8 @@ func init() {
ConfigCommand.Flags().StringVar(&docrootRelPathArg, "docroot", "", "Provide the relative docroot of the project, like 'docroot' or 'htdocs' or 'web', defaults to empty, the current directory")
ConfigCommand.Flags().StringVar(&projectTypeArg, "project-type", "", projectTypeUsage)
ConfigCommand.Flags().StringVar(&phpVersionArg, "php-version", "", "The version of PHP that will be enabled in the web container")
ConfigCommand.Flags().StringVar(&httpPortArg, "http-port", "", "The web container's exposed HTTP port")
ConfigCommand.Flags().StringVar(&httpsPortArg, "https-port", "", "The web container's exposed HTTPS port")
ConfigCommand.Flags().StringVar(&httpPortArg, "http-port", "", "The router HTTP port for this project")
ConfigCommand.Flags().StringVar(&httpsPortArg, "https-port", "", "The router HTTPS port for this project")
ConfigCommand.Flags().BoolVar(&xdebugEnabledArg, "xdebug-enabled", false, "Whether or not XDebug is enabled in the web container")
ConfigCommand.Flags().StringVar(&additionalHostnamesArg, "additional-hostnames", "", "A comma-delimited list of hostnames for the project")
ConfigCommand.Flags().StringVar(&additionalFQDNsArg, "additional-fqdns", "", "A comma-delimited list of FQDNs for the project")
Expand All @@ -209,6 +215,8 @@ func init() {
ConfigCommand.Flags().BoolVar(&workingDirDefaultsArg, "working-dir-defaults", false, "Unsets all service working directory overrides")
ConfigCommand.Flags().StringVar(&mariaDBVersionArg, "mariadb-version", "10.2", "mariadb version to use")
ConfigCommand.Flags().BoolVar(&nfsMountEnabled, "nfs-mount-enabled", false, "enable NFS mounting of project in container")
ConfigCommand.Flags().StringVar(&hostWebserverPortArg, "host-webserver-port", "", "The web container's localhost-bound port")
ConfigCommand.Flags().StringVar(&hostDBPortArg, "host-db-port", "", "The db container's localhost-bound port")

// projectname flag exists for backwards compatability.
ConfigCommand.Flags().StringVar(&projectNameArg, "projectname", "", projectNameUsage)
Expand Down Expand Up @@ -360,6 +368,14 @@ func handleMainConfigArgs(cmd *cobra.Command, args []string, app *ddevapp.DdevAp
app.RouterHTTPSPort = httpsPortArg
}

if hostWebserverPortArg != "" {
app.HostWebserverPort = hostWebserverPortArg
}

if hostDBPortArg != "" {
app.HostDBPort = hostDBPortArg
}

if mariaDBVersionArg != "" {
app.MariaDBVersion = mariaDBVersionArg
}
Expand Down
8 changes: 8 additions & 0 deletions cmd/ddev/cmd/config_test.go
Expand Up @@ -108,6 +108,8 @@ func TestConfigSetValues(t *testing.T) {
phpVersion := ddevapp.PHP71
httpPort := "81"
httpsPort := "444"
hostDBPort := "60001"
hostWebserverPort := "60002"
xdebugEnabled := true
additionalHostnamesSlice := []string{"abc", "123", "xyz"}
additionalHostnames := strings.Join(additionalHostnamesSlice, ",")
Expand Down Expand Up @@ -145,6 +147,8 @@ func TestConfigSetValues(t *testing.T) {
"--db-working-dir", dbWorkingDir,
"--dba-working-dir", dbaWorkingDir,
"--omit-containers", omitContainers,
"--host-db-port", hostDBPort,
"--host-webserver-port", hostWebserverPort,
}

_, err = exec.RunCommand(DdevBin, args)
Expand All @@ -167,6 +171,8 @@ func TestConfigSetValues(t *testing.T) {
assert.Equal(phpVersion, app.PHPVersion)
assert.Equal(httpPort, app.RouterHTTPPort)
assert.Equal(httpsPort, app.RouterHTTPSPort)
assert.Equal(hostWebserverPort, app.HostWebserverPort)
assert.Equal(hostDBPort, app.HostDBPort)
assert.Equal(xdebugEnabled, app.XdebugEnabled)
assert.Equal(additionalHostnamesSlice, app.AdditionalHostnames)
assert.Equal(additionalFQDNsSlice, app.AdditionalFQDNs)
Expand Down Expand Up @@ -269,6 +275,7 @@ func TestConfigInvalidProjectname(t *testing.T) {
assert.NoError(err)
assert.NotContains(out, "is not a valid project name")
assert.Contains(out, "You may now run 'ddev start'")
_ = os.Remove(filepath.Join(tmpdir, ".ddev", "config.yaml"))
}

// test some invalid project names.
Expand All @@ -282,6 +289,7 @@ func TestConfigInvalidProjectname(t *testing.T) {
assert.Error(err)
assert.Contains(out, fmt.Sprintf("%s is not a valid project name", projName))
assert.NotContains(out, "You may now run 'ddev start'")
_ = os.Remove(filepath.Join(tmpdir, ".ddev", "config.yaml"))
}
}

Expand Down
4 changes: 4 additions & 0 deletions pkg/appports/ports.go
Expand Up @@ -17,10 +17,14 @@ var mailhogPort = "8025"
// dbPort defines the default DB (MySQL) port.
var dbPort = "3306"

// webPort defines the internal web port
var webPort = "80"

var ports = map[string]string{
"mailhog": mailhogPort,
"dba": dbaPort,
"db": dbPort,
"web": webPort,
}

// GetPort returns the external router (as a string) for the given service. This can be used to find a given port for docker-compose manifests,
Expand Down
44 changes: 39 additions & 5 deletions pkg/ddevapp/config.go
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"fmt"
"github.com/drud/ddev/pkg/dockerutil"
"github.com/drud/ddev/pkg/nodeps"
"html/template"
"io/ioutil"
"os"
Expand Down Expand Up @@ -142,6 +143,13 @@ func (app *DdevApp) WriteConfig() error {
appcopy.BgsyncImage = ""
}

// We now want to reserve the port we're writing for HostDBPort and HostWebserverPort and so they don't
// accidentally get used for other projects.
err := app.CheckAndReserveHostPorts()
if err != nil {
return err
}

// Don't write default working dir values to config
defaults := appcopy.DefaultWorkingDirMap()
for service, defaultWorkingDir := range defaults {
Expand All @@ -150,7 +158,7 @@ func (app *DdevApp) WriteConfig() error {
}
}

err := PrepDdevDirectory(filepath.Dir(appcopy.ConfigPath))
err = PrepDdevDirectory(filepath.Dir(appcopy.ConfigPath))
if err != nil {
return err
}
Expand Down Expand Up @@ -191,6 +199,27 @@ func (app *DdevApp) WriteConfig() error {
return nil
}

// CheckAndReserveHostPorts checks that configured host ports are not already
// reserved by another project.
func (app *DdevApp) CheckAndReserveHostPorts() error {
portsToReserve := []string{}
if app.HostDBPort != "" {
portsToReserve = append(portsToReserve, app.HostDBPort)
}
if app.HostWebserverPort != "" {
portsToReserve = append(portsToReserve, app.HostWebserverPort)
}
if len(portsToReserve) > 0 {
err := globalconfig.CheckHostPortsAvailable(app.Name, portsToReserve)
if err != nil {
return err
}
}
err := globalconfig.ReservePorts(app.Name, portsToReserve)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this call to globalconfig.ReservePorts is moved up inside the if block right before it (if len(portsToReserve) > 0 {), the project info map won't be written to the global config file in the case where no ports are requested. There may be some unintended consequences there that I'm not considering, but it does seem a touch cleaner in the general case from what I can tell.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, but I want the project info map always, for future use. I'm expecting it to be there for every project by the next sprint, containing a minimum of the approot. Given that I want it in the projectlist, are you good with it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, in that case it's totally fine after your most recent commit.


return err
}

// ReadConfig reads project configuration, falling
// back to defaults for config values not defined in the read config file.
func (app *DdevApp) ReadConfig(includeOverrides bool) ([]string, error) {
Expand Down Expand Up @@ -347,12 +376,12 @@ func (app *DdevApp) ValidateConfig() error {
}

if !IsValidOmitContainers(app.OmitContainers) {
return fmt.Errorf("Invalid omit_containers: %s, must be one of %s", app.OmitContainers, GetValidOmitContainers()).(InvalidOmitContainers)
return fmt.Errorf("invalid omit_containers: %s, must be one of %s", app.OmitContainers, GetValidOmitContainers()).(InvalidOmitContainers)
}

// Validate mariadb version
if !IsValidMariaDBVersion(app.MariaDBVersion) {
return fmt.Errorf("Invalid mariadb_version: %s, must be one of %s", app.MariaDBVersion, GetValidMariaDBVersions()).(invalidMariaDBVersion)
return fmt.Errorf("invalid mariadb_version: %s, must be one of %s", app.MariaDBVersion, GetValidMariaDBVersions()).(invalidMariaDBVersion)
}

if app.WebcacheEnabled && app.NFSMountEnabled {
Expand Down Expand Up @@ -492,6 +521,7 @@ type composeYAMLVars struct {
WebcacheEnabled bool
NFSMountEnabled bool
NFSSource string
DockerIP string
IsWindowsFS bool
}

Expand Down Expand Up @@ -523,8 +553,8 @@ func (app *DdevApp) RenderComposeYAML() (string, error) {
DdevGenerated: DdevFileSignature,
HostDockerInternalIP: hostDockerInternalIP,
ComposeVersion: version.DockerComposeFileFormatVersion,
OmitDBA: util.ArrayContainsString(app.OmitContainers, "dba"),
OmitSSHAgent: util.ArrayContainsString(app.OmitContainers, "ddev-ssh-agent"),
OmitDBA: nodeps.ArrayContainsString(app.OmitContainers, "dba"),
OmitSSHAgent: nodeps.ArrayContainsString(app.OmitContainers, "ddev-ssh-agent"),
WebcacheEnabled: app.WebcacheEnabled,
NFSMountEnabled: app.NFSMountEnabled,
NFSSource: "",
Expand All @@ -546,6 +576,10 @@ func (app *DdevApp) RenderComposeYAML() (string, error) {
templateVars.NFSSource = dockerutil.MassageWIndowsNFSMount(app.AppRoot)
}
}
templateVars.DockerIP, err = dockerutil.GetDockerIP()
if err != nil {
return "", err
}

err = templ.Execute(&doc, templateVars)
return doc.String(), err
Expand Down
34 changes: 28 additions & 6 deletions pkg/ddevapp/ddevapp.go
Expand Up @@ -2,6 +2,8 @@ package ddevapp

import (
"fmt"
"github.com/drud/ddev/pkg/globalconfig"
"github.com/drud/ddev/pkg/nodeps"
"github.com/mattn/go-isatty"
"io/ioutil"
"os"
Expand Down Expand Up @@ -95,7 +97,9 @@ type DdevApp struct {
Commands map[string][]Command `yaml:"hooks,omitempty"`
UploadDir string `yaml:"upload_dir,omitempty"`
WorkingDir map[string]string `yaml:"working_dir,omitempty"`
OmitContainers []string `yaml:"omit_containers,omitempty"`
OmitContainers []string `yaml:"omit_containers,omitempty,flow"`
HostDBPort string `yaml:"host_db_port,omitempty"`
HostWebserverPort string `yaml:"host_webserver_port,omitempty"`
}

// GetType returns the application type as a (lowercase) string
Expand Down Expand Up @@ -183,7 +187,7 @@ func (app *DdevApp) Describe() (map[string]interface{}, error) {
appDesc["dbinfo"] = dbinfo

appDesc["mailhog_url"] = "http://" + app.GetHostname() + ":" + appports.GetPort("mailhog")
if !util.ArrayContainsString(app.OmitContainers, "dba") {
if !nodeps.ArrayContainsString(app.OmitContainers, "dba") {
appDesc["phpmyadmin_url"] = "http://" + app.GetHostname() + ":" + appports.GetPort("dba")
}
}
Expand All @@ -208,14 +212,14 @@ func (app *DdevApp) Describe() (map[string]interface{}, error) {

// GetPublishedPort returns the host-exposed public port of a container.
func (app *DdevApp) GetPublishedPort(serviceName string) (int, error) {
dbContainer, err := app.FindContainerByType(serviceName)
if err != nil {
container, err := app.FindContainerByType(serviceName)
if err != nil || container == nil {
return -1, fmt.Errorf("Failed to find container of type %s: %v", serviceName, err)
}

privatePort, _ := strconv.ParseInt(appports.GetPort(serviceName), 10, 16)

publishedPort := dockerutil.GetPublishedPort(privatePort, *dbContainer)
publishedPort := dockerutil.GetPublishedPort(privatePort, *container)
return publishedPort, nil
}

Expand Down Expand Up @@ -665,12 +669,18 @@ func (app *DdevApp) Start() error {
util.Warning("Your %s version is %s, but ddev is version %s. \nPlease run 'ddev config' to update your config.yaml. \nddev may not operate correctly until you do.", app.ConfigPath, app.APIVersion, version.DdevVersion)
}

// Make sure that any ports allocated are available.
err = app.CheckAndReserveHostPorts()
if err != nil {
return err
}

err = app.ProcessHooks("pre-start")
if err != nil {
return err
}

if !util.ArrayContainsString(app.OmitContainers, "ddev-ssh-agent") {
if !nodeps.ArrayContainsString(app.OmitContainers, "ddev-ssh-agent") {
err = app.EnsureSSHAgentContainer()
if err != nil {
return err
Expand Down Expand Up @@ -917,6 +927,8 @@ func (app *DdevApp) DockerEnv() {
"DDEV_WEBIMAGE": app.WebImage,
"DDEV_BGSYNCIMAGE": app.BgsyncImage,
"DDEV_APPROOT": app.AppRoot,
"DDEV_HOST_DB_PORT": app.HostDBPort,
"DDEV_HOST_WEBSERVER_PORT": app.HostWebserverPort,
"DDEV_DOCROOT": app.Docroot,
"DDEV_URL": app.GetHTTPURL(),
"DDEV_HOSTNAME": app.HostName(),
Expand Down Expand Up @@ -1183,6 +1195,11 @@ func (app *DdevApp) Down(removeData bool, createSnapshot bool) error {
if err = app.RemoveHostsEntries(); err != nil {
return fmt.Errorf("failed to remove hosts entries: %v", err)
}
app.RemoveGlobalProjectInfo()
err = globalconfig.WriteGlobalConfig(globalconfig.DdevGlobalConfig)
if err != nil {
util.Warning("could not WriteGlobalConfig: %v", err)
}

for _, volName := range []string{app.Name + "-mariadb", app.GetUnisonCatalogVolName(), app.GetWebcacheVolName()} {
err = dockerutil.RemoveVolume(volName)
Expand All @@ -1197,6 +1214,11 @@ func (app *DdevApp) Down(removeData bool, createSnapshot bool) error {
return err
}

// RemoveReservedHostPorts() deletes the app from the UsedHostPorts
func (app *DdevApp) RemoveGlobalProjectInfo() {
delete(globalconfig.DdevGlobalConfig.ProjectList, app.Name)
}

// GetHTTPURL returns the HTTP URL for an app.
func (app *DdevApp) GetHTTPURL() string {
url := "http://" + app.GetHostname()
Expand Down