Server version not reliably detected from config/packages/doctrine.yaml during symfony deploy when multiple connections configured

[jdevinemt]

What is the issue?

I have a platform project that is configured to use a mariadb service with a version of 10.1. I have two doctrine dbal connections configured, both of which have a server_version of 10.1 configured. When I try to deploy the project using the symfony deploy command, I receive the following error:

How can the issue be replicated?

The issue can be replicated by configuring multiple doctrine dbal connections in config/packages/doctrine.yaml, neither of which are named default, and both of which define the server version via the server_version parameter instead of including it in the database url. A database service with a specified version must also be configured in .platform/services.yaml. When trying to run the symfony deploy command the error will occur.

I am using Symfony 5.4 (migrating to Platform.sh to try to ease the process of upgrading), but I don't think the doctrine configuration related to this issue has changed since this version.

Example config:

# .platform/services.yaml
db:
   type: mariadb:10.1

# config/packages/doctrine.yaml
doctrine:
    dbal:
        default_connection: 'main'
        connections:
            main:
                # in most cases you would use an environment variable (but this should still be valid)
                url: 'mysql://db_user:db_password@1.2.3.4:3306/db_name'
                server_version: '10.1'
                dbname: 'main'
            legacy:
                url: 'mysql://db_user:db_password@1.2.3.4:3306/db_name'
                server_version: '10.1'
                dbname: 'legacy'

What might be the cause of the issue?

I believe the issue is that the function that reads the DB version from the doctrine config file is only looking for the server_version value directly at doctrine.dbal.server_version and doctrine.dbal.connections.default, even though there is support to specify a default database by any valid name via doctrine.dbal.default_connection.

As of now, this is the function that reads the DB version from the doctrine config file:

func ReadDBVersionFromDoctrineConfigYAML(projectDir string) (string, error) {
	path := filepath.Join(projectDir, "config", "packages", "doctrine.yaml")
	if _, err := os.Stat(path); errors.Is(err, os.ErrNotExist) {
		return "", nil
	}

	doctrineConfigYAML, err := os.ReadFile(path)
	if err != nil {
		return "", err
	}

	var doctrineConfig struct {
		Doctrine struct {
			Dbal struct {
				ServerVersion string `yaml:"server_version"`
				Connections   struct {
					Default struct {
						ServerVersion string `yaml:"server_version"`
					} `yaml:"default"`
				}
			} `yaml:"dbal"`
		} `yaml:"doctrine"`
	}
	if err := yaml.Unmarshal(doctrineConfigYAML, &doctrineConfig); err != nil {
		// format is wrong
		return "", err
	}

        // HERE IS WHERE IT IS LOOKING EXPLICITLY AT A CONNECTION NAMED 'default'
	version := doctrineConfig.Doctrine.Dbal.Connections.Default.ServerVersion
	if version == "" {
		version = doctrineConfig.Doctrine.Dbal.ServerVersion
	}
	if version == "" {
		// empty version
		return "", nil
	}
	if version[0] == '%' && version[len(version)-1] == '%' {
		// references an env var, ignore
		return "", nil
	}
	return version, nil
}

What is a possible solution to the issue?

Since the default connection can either be implicitly implied as the first connection, or explicitly defined via doctrine:dbal:default_connection, the function in question should account for these possibilities. If not both, at least the latter.

I would create a PR myself, but I'm not familiar with the Go language. Because I am not familiar with Go or the libraries used in this project, I am not able to determine if there is a good reason that either of these proposed fixes are not already implemented. I may still give it a try if I get the time to familiarize myself enough with the language.

Additional Thoughts

Other Potential Improvements

Looking at the code for reading the server version from .env, it seems to look specifically for DATABASE_URL, which seems reasonable enough even though there is no guarantee that this value is used to configure a doctrine connection. This gives raises a couple thoughts for me.

First, since we are ok with assuming DATABASE_URL is the correct variable to look at here, could we also assume that DATABASE_SERVER_VERSION could be checked first?

Second, should we account for the possibility that the doctrine.dbal.url or doctrine.dbal.connections.name.url value is explicitly defined as a string in the doctrine config file and thus parse for the serverVersion query parameter on it if the server_version parameter is not defined?

Current Workaround

My current workaround, which I have only tested to a limited degree, is to just modify my config/packages/doctrine.yaml file to include a dummy default dbal connection.

# config/packages/doctrine.yaml
doctrine:
    dbal:
        default_connection: 'main'
        connections:
            # WORKAROUND
            default:
                server_version: '10.1'
            main:
                url: 'mysql://db_user:db_password@1.2.3.4:3306/db_name'
                server_version: '10.1'
                dbname: 'main'
            legacy:
                url: 'mysql://db_user:db_password@1.2.3.4:3306/db_name'
                server_version: '10.1'
                dbname: 'legacy'

Use Case

In my real world project I'm using an application parameter for the database url, which is composed from several environment variables. This is because I still use a custom PDO connection that requires the components that compose the url to be used individually in some places. That is my reasoning for not just using a single DATABASE_URL environment variable. Although, maybe there's a better way for me to approach this.

I could rename the 'main' connection to 'default', but it seems like this should not be necessary for this command to work. I also like injecting named connections into my services so that one can be sure which database is being used when reading our source code.