diff --git a/deployer/dev/task/sync.php b/deployer/dev/task/sync.php index 5ea1bac..7dcbb43 100644 --- a/deployer/dev/task/sync.php +++ b/deployer/dev/task/sync.php @@ -38,7 +38,8 @@ if ($target !== 'prod' && $target !== 'stage') { $dbSyncToolSync = get('dev_db_sync_tool_default_sync'); - $dbSyncToolOriginPath = str_replace('', $target, get('dev_db_sync_tool_origin_path')); + // the remote path carries the instance name, not the raw branch name + $dbSyncToolOriginPath = str_replace('', getFeatureName($target), get('dev_db_sync_tool_origin_path')); $additionalOptions = "--origin-path $dbSyncToolOriginPath"; } else { $dbSyncToolSync = $target === 'prod' ? get('dev_db_sync_tool_prod_sync') : get('dev_db_sync_tool_default_sync'); diff --git a/deployer/feature/task/feature_init.php b/deployer/feature/task/feature_init.php index 5e8ef44..0ff7447 100644 --- a/deployer/feature/task/feature_init.php +++ b/deployer/feature/task/feature_init.php @@ -55,6 +55,8 @@ function initFeature(?string $feature = null): ?string return $array[2]; }, listFeatureInstances())); } + // branch names may contain path separators ("feature/ABC-12"), the instance name must stay flat + $feature = getFeatureName($feature); set('feature', $feature); if (isUrlShortener()) { diff --git a/deployer/feature/task/feature_setup.php b/deployer/feature/task/feature_setup.php index b632f99..b35088d 100644 --- a/deployer/feature/task/feature_setup.php +++ b/deployer/feature/task/feature_setup.php @@ -67,7 +67,8 @@ function renderRemoteTemplates(): void { debug('Rendering remote template'); $databaseName = DbUtility::getDatabaseManager()->getDatabaseName(); - $feature = input()->getOption('feature'); + // the normalized name, matching the instance directory and url segment + $feature = get('feature'); $templates = get('feature_templates'); if (!$templates) { @@ -144,13 +145,3 @@ function uploadTemplate($localTemplate, $remoteTarget, $arguments): void { upload($temporaryFileName,get('deploy_path') . $remoteTarget); unlink($temporaryFileName); } - -/** - * @param ?string $feature - * @return array|string|string[]|null - */ -function getFeatureName(?string $feature = null) { - $feature = $feature ?: input()->getOption('feature'); - - return preg_replace('/[^A-Za-z0-9\_\-.]/', '', $feature); -} diff --git a/deployer/feature/task/feature_stop.php b/deployer/feature/task/feature_stop.php index feb06e1..2db8ef7 100644 --- a/deployer/feature/task/feature_stop.php +++ b/deployer/feature/task/feature_stop.php @@ -33,7 +33,7 @@ */ function deleteFeature(?string $feature = null, $needConfirmation = false): void { - $feature = $feature ?: input()->getOption('feature'); + $feature = getFeatureName($feature); $filesRemoveCommand = "rm -rf " . get('deploy_path'); diff --git a/deployer/functions.php b/deployer/functions.php index cb147ba..f9ea253 100644 --- a/deployer/functions.php +++ b/deployer/functions.php @@ -3,6 +3,7 @@ namespace Deployer; use Deployer\Exception\RunException; +use MoveElevator\DeployerTools\Utility\FeatureUtility; use Symfony\Component\Console\Output\OutputInterface; /** @@ -55,6 +56,26 @@ function featureRequested(): bool return null !== $feature && '' !== trim((string)$feature); } +/** + * Normalize a feature identifier into the flat instance name used for the deploy + * directory, the public url segment, the url shortener symlink and the database name. + * + * Branch names may carry path separators ("feature/ABC-12"), which would otherwise + * nest the instance directory and break listing, cleanup and deletion. Identifiers + * that already consist of allowed characters only are returned unchanged. + * + * @param ?string $feature + * @return string + */ +function getFeatureName(?string $feature = null): string +{ + if (null === $feature || '' === trim($feature)) { + $feature = featureRequested() ? (string)input()->getOption('feature') : ''; + } + + return FeatureUtility::normalize($feature); +} + /** * Extend the deployer configuration with available environment variables (starting with "DEPLOYER_CONFIG_"): * diff --git a/docs/FEATURE.md b/docs/FEATURE.md index 5fca0d3..fd091ba 100644 --- a/docs/FEATURE.md +++ b/docs/FEATURE.md @@ -55,6 +55,21 @@ The `feature:setup` command represent the initialization of a new feature branch $ vendor/bin/dep feature:setup stage --feature=TEST-01 ``` +The `--feature=` value may be a full branch name. Path separators are replaced by a hyphen so the instance stays a single flat directory, url segment and database suffix: + +| `--feature=` | instance name | +|----------------------|----------------------| +| `TEST-01` | `TEST-01` | +| `feature/TEST-01` | `feature-TEST-01` | +| `bugfix/TEST-01` | `bugfix-TEST-01` | +| `release/1.2.0` | `release-1.2.0` | + +Names that already consist of letters, digits, `_`, `-` and `.` are used unchanged, so existing instances and their databases stay reachable. Since the branch prefix is kept, `feature/TEST-01` and `bugfix/TEST-01` remain two separate instances. The same normalization is applied by `feature:cleanup` when it compares remote git branches with the deployed instances. + +A `--feature=` value that yields no usable name is rejected instead of falling back to the base instance, which the feature scaffolding would otherwise overwrite. That covers values without any allowed character as well as `.` and `..`. + +> Because the separator is replaced rather than encoded, `feature/TEST-01` and a branch literally named `feature-TEST-01` map to the same instance. Do not use both spellings for different branches in one project. + The recipe already wires this task into the deploy flow, together with a `feature:init` before `deploy:info`. That ordering matters: `deploy:info` resolves `{{release_name}}` and deployer caches the result for the rest of the run, so the feature instance has to be known before it runs. Do not hook `feature:setup` any earlier yourself. > Upgrading: if your `deploy.php` carries a `before('deploy:info', 'feature:init')` (or an equivalent `feature:setup` hook) as a workaround for that ordering, remove it — the recipe registers it now and the hook would otherwise run twice. @@ -75,7 +90,7 @@ This configuration defines the local template file as well as the remote target | `DEPLOYER_CONFIG_DATABASE_PORT` | default is `3306`, overwrite with deployer `set('database_port', '3306');` | | `DEPLOYER_CONFIG_DATABASE_USER` | should be defined with `database_user` in the host configuration | | `DEPLOYER_CONFIG_DATABASE_NAME` | will be dynamically generated | -| `DEPLOYER_CONFIG_FEATURE_NAME` | will be provide with the `--feature=` command line argument | +| `DEPLOYER_CONFIG_FEATURE_NAME` | the normalized instance name derived from the `--feature=` command line argument | | `DEPLOYER_CONFIG_FEATURE_URL` | will be dynamically generated | | `DEPLOYER_CONFIG_FEATURE_PATH` | will be dynamically generated | diff --git a/src/Database/Manager/AbstractManager.php b/src/Database/Manager/AbstractManager.php index d6b7b5b..6c760f0 100644 --- a/src/Database/Manager/AbstractManager.php +++ b/src/Database/Manager/AbstractManager.php @@ -5,6 +5,7 @@ namespace MoveElevator\DeployerTools\Database\Manager; use MoveElevator\DeployerTools\Database\Exception\DatabaseException; +use MoveElevator\DeployerTools\Utility\FeatureUtility; use MoveElevator\DeployerTools\Utility\VarUtility; use function Deployer\get; @@ -41,16 +42,21 @@ public function run(string $command, bool $useDoubleQuotes = true): string */ public function getDatabaseName(?string $feature = null): string { - $feature = $feature ?: input()->getOption('feature'); - $project = get('project'); - return substr($this->getFeatureName("{$project}--{$feature}"), 0, 63); + // both parts are normalized separately, so the "--" separator survives + $project = FeatureUtility::normalize((string) get('project')); + $feature = $this->getFeatureName($feature); + + return substr($project . '--' . $feature, 0, 63); } public function getFeatureName(?string $feature = null): string { - $feature = $feature ?: input()->getOption('feature'); + // only null and blank count as absent, "0" is a valid instance name + if (null === $feature || '' === trim($feature)) { + $feature = (string) input()->getOption('feature'); + } - return preg_replace('/[^A-Za-z0-9\_\-.]/', '', (string) $feature); + return FeatureUtility::normalize($feature); } } diff --git a/src/Database/Manager/Simple.php b/src/Database/Manager/Simple.php index 5b3d0c7..5c2112b 100644 --- a/src/Database/Manager/Simple.php +++ b/src/Database/Manager/Simple.php @@ -11,7 +11,6 @@ use function Deployer\set; use function Deployer\has; use function Deployer\run; -use function Deployer\input; use function Deployer\upload; use function Deployer\runExtended; use function Deployer\test; @@ -50,7 +49,7 @@ public function delete(string $feature): void debug('Deleting database'); $this->ensureDatabasePoolExists(); $this->initDatabaseConfiguration(feature: $feature); - $this->assignmentManager->removeAssignment($feature); + $this->assignmentManager->removeAssignment($this->getFeatureName($feature)); $this->run($this->generateDropTablesQuery($this->getDatabaseName($feature))); } @@ -78,8 +77,7 @@ public function exists(?string $feature = null): bool public function getDatabaseName(?string $feature = null): string { - $feature = $feature ?: input()->getOption('feature'); - $databaseAssignment = $this->assignmentManager->getAssignment($feature); + $databaseAssignment = $this->assignmentManager->getAssignment($this->getFeatureName($feature)); if (!$databaseAssignment) { return ''; @@ -115,7 +113,7 @@ private function initDatabaseConfiguration(?string $database = null, ?string $fe { $pool = get('database_pool'); if (!$database) { - $database = $this->assignmentManager->getAssignment($feature ?: $this->getFeatureName()); + $database = $this->assignmentManager->getAssignment($this->getFeatureName($feature)); } if (!isset($pool[$database])) { diff --git a/src/Utility/FeatureUtility.php b/src/Utility/FeatureUtility.php new file mode 100644 index 0000000..8d9a964 --- /dev/null +++ b/src/Utility/FeatureUtility.php @@ -0,0 +1,45 @@ +