Skip to content

Commit

Permalink
Fix groups transmogrify according to AR-G-9
Browse files Browse the repository at this point in the history
  • Loading branch information
Ioannis committed Nov 7, 2025
1 parent 6851f47 commit 05aa027
Show file tree
Hide file tree
Showing 7 changed files with 409 additions and 84 deletions.
43 changes: 43 additions & 0 deletions app/plugins/Transmogrify/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,12 @@ These options come directly from TransmogrifyCommand::buildOptionParser.
- --orgidentities-health
- Run Org Identities health check (eligibility/exclusion breakdown based on non-historical links and person existence) and print a transmogrification readiness report, then exit.

- --groups-health
- Run Groups health check (AR‑Group‑9: invalid Standard group names) and print a transmogrification readiness report, then exit.

- --groups-colon-replacement STRING
- Optional: replace ":" with STRING in Standard group names during migration (opt‑in). Use with care; the name "CO" remains invalid and will not be auto‑renamed.


## Typical usage

Expand Down Expand Up @@ -168,3 +174,40 @@ You’ll see a fixed‑width table with Reason, Included/Excluded counts, and an
- C) Has at least one non‑historical link with a non‑NULL co_person_id (included)

Totals summarize overall readiness. Use this report to address data conditions (eg, missing person links) so that important Org Identities are eligible for migration.

### Recommended preflight: Groups Health command (Standard naming rule)

Transmogrify enforces a naming rule for Standard groups: a Standard group is considered invalid if its name contains a colon (:) or equals “CO” (case‑insensitive, trimmed). Invalid Standard groups will not be migrated by default and require admin action (eg, rename) before proceeding.

Run this health check to see how many groups are affected and where action is needed:

```bash
bin/cake transmogrify --groups-health
```

You’ll see a fixed‑width table with Reason, Included/Excluded counts, and an Indicator (✓ valid/eligible, x invalid/action required). Reasons are:
- Invalid: Standard group name contains “:” (excluded)
- Standard groups (type S) whose name includes a colon are considered invalid by default and require renaming before migration.

- Invalid: Standard group name equals “CO” (excluded)
- Standard groups (type S) named exactly “CO” (case‑insensitive, trimmed) are considered invalid and require renaming.

- Valid: Does not violate the naming rule (included)
- All other groups: either not Standard type, or Standard whose name does not contain “:” and is not exactly “CO”.

Totals summarize overall readiness:
- Invalid (total): total number of invalid Standard groups (sum of the invalid reasons).
- Valid (total): total number of groups eligible to migrate without renaming.
- Total Groups: grand total of groups evaluated (valid + invalid).

Use this report to identify Standard groups that must be renamed to comply with the rule. After remediation, re‑run the health check and verify that Invalid (total) is 0 and all groups you intend to migrate appear under Valid.

Optional remediation helper (opt‑in): colon replacement
- By default, Transmogrify does not change group names and will error on invalid Standard names.
- You can opt in to replace “:” in Standard group names with a safer character or string during migration. The special name “CO” remains invalid and is not auto‑renamed.

Example (replace ":" with "-"):

```bash
bin/cake transmogrify --groups-colon-replacement '-'
```
1 change: 1 addition & 0 deletions app/plugins/Transmogrify/config/schema/tables.json
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@
"displayField": "name",
"cache": ["co_id", "owners_group_id"],
"booleans": ["nesting_mode_all", "open"],
"preRow": "applyCheckGroupNameARRule",
"fieldMap": {
"auto": null,
"co_group_id": "group_id",
Expand Down
160 changes: 98 additions & 62 deletions app/plugins/Transmogrify/src/Command/TransmogrifyCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*
* @link https://www.internet2.edu/comanage COmanage Project
* @package registry
* @since COmanage Registry v5.0.0
Expand Down Expand Up @@ -52,6 +52,7 @@
use Transmogrify\Lib\Util\DbInfoPrinter;
use Transmogrify\Lib\Util\RawSqlQueries;
use Transmogrify\Lib\Util\OrgIdentitiesHealth;
use Transmogrify\Lib\Util\GroupsHealth;

class TransmogrifyCommand extends BaseCommand {
use CacheTrait;
Expand Down Expand Up @@ -116,66 +117,82 @@ public function run(array $argv, ConsoleIo $io): int
* @param ConsoleOptionParser $parser ConsoleOptionParser
* @return ConsoleOptionParser ConsoleOptionParser
*/

protected function buildOptionParser(ConsoleOptionParser $parser): ConsoleOptionParser
{
// Allow overriding the tables config path
$parser->addOption('tables-config', [
'help' => 'Path to transmogrify tables JSON config',
'default' => TransmogrifyEnum::TABLES_JSON_PATH
]);
$parser->addOption('dump-tables-config', [
'help' => 'Output the effective tables configuration (after schema extension) and exit',
'boolean' => true
]);
// Specify a table (or repeat option) to migrate only a subset
$parser->addOption('table', [
'help' => 'Migrate only the specified table. Repeat the option to migrate multiple tables',
'multiple' => true
]);
// List available target tables and exit
$parser->addOption('list-tables', [
'help' => 'List available target tables from the transmogrify config and exit',
'boolean' => true
]);
// Info options integrated into TransmogrifyCommand
$parser->addOption('info', [
'help' => 'Print source and target database configuration and exit',
'boolean' => true
]);
$parser->addOption('info-json', [
'help' => 'Output info in JSON (use with --info)',
'boolean' => true
]);
$parser->addOption('info-ping', [
'help' => 'Ping connections and include connectivity + server version (use with --info or --info-schema)',
'boolean' => true
]);
$parser->addOption('info-schema', [
'help' => 'Print schema information and whether the database is empty (defaults to target). Use --info-schema-role to select source/target',
'boolean' => true
]);
$parser->addOption('info-schema-role', [
'help' => 'When using --info-schema, which database to inspect: source or target (default: target)'
]);
$parser->addOption('login-identifier-copy', [
'help' => __d('command', 'tm.login-identifier-copy'),
'boolean' => true
]);

$parser->addOption('login-identifier-type', [
'help' => __d('command', 'tm.login-identifier-type')
]);

// Health report option (Org Identities readiness)
$parser->addOption('orgidentities-health', [
'help' => 'Run Org Identities health check (eligibility/exclusion breakdown) and exit',
'boolean' => true
]);

$parser->setEpilog(__d('command', 'tm.epilog'));

return $parser;
// Allow overriding the tables config path
$parser->addOption('tables-config', [
'help' => 'Path to transmogrify tables JSON config',
'default' => TransmogrifyEnum::TABLES_JSON_PATH
]);
$parser->addOption('dump-tables-config', [
'help' => 'Output the effective tables configuration (after schema extension) and exit',
'boolean' => true
]);
// Specify a table (or repeat option) to migrate only a subset
$parser->addOption('table', [
'help' => 'Migrate only the specified table. Repeat the option to migrate multiple tables',
'multiple' => true
]);
// List available target tables and exit
$parser->addOption('list-tables', [
'help' => 'List available target tables from the transmogrify config and exit',
'boolean' => true
]);
// Info options integrated into TransmogrifyCommand
$parser->addOption('info', [
'help' => 'Print source and target database configuration and exit',
'boolean' => true
]);
$parser->addOption('info-json', [
'help' => 'Output info in JSON (use with --info)',
'boolean' => true
]);
$parser->addOption('info-ping', [
'help' => 'Ping connections and include connectivity + server version (use with --info or --info-schema)',
'boolean' => true
]);
$parser->addOption('info-schema', [
'help' => 'Print schema information and whether the database is empty (defaults to target). Use --info-schema-role to select source/target',
'boolean' => true
]);
$parser->addOption('info-schema-role', [
'help' => 'When using --info-schema, which database to inspect: source or target (default: target)'
]);
$parser->addOption('login-identifier-copy', [
'help' => __d('command', 'tm.login-identifier-copy'),
'boolean' => true
]);

$parser->addOption('login-identifier-type', [
'help' => __d('command', 'tm.login-identifier-type')
]);

// Health report option (Org Identities readiness)
$parser->addOption('orgidentities-health', [
'help' => 'Run Org Identities health check (eligibility/exclusion breakdown) and exit',
'boolean' => true
]);
// Health report option (Groups naming rule readiness)
$parser->addOption('groups-health', [
'help' => 'Run Groups health check (AR-Group-9: invalid Standard names) and exit',
'boolean' => true
]);
// Optional: replace colons in Standard group names during migration (opt-in, off by default)
$parser->addOption('groups-colon-replacement', [
'help' => 'If set, replace ":" with this value in Standard group names during migration. WARNING: name "CO" remains invalid and is not auto-renamed.'
]);
// Convenience flag for using a literal dash as replacement
$parser->addOption('groups-colon-replacement-dash', [
'help' => 'Use "-" as the replacement for ":" in Standard group names (shorthand when passing a lone "-" is problematic)',
'boolean' => true
]);



$parser->setEpilog(__d('command', 'tm.epilog'));

return $parser;
}

/**
Expand Down Expand Up @@ -209,6 +226,10 @@ public function execute(Arguments $args, ConsoleIo $io): int
OrgIdentitiesHealth::run($this->inconn, $this->io);
return BaseCommand::CODE_SUCCESS;
}
if ($this->args->getOption('groups-health')) {
GroupsHealth::run($this->inconn, $this->io);
return BaseCommand::CODE_SUCCESS;
}

// Load tables configuration (from JSON) and extend it with schema data
$this->loadTablesConfig();
Expand Down Expand Up @@ -237,7 +258,13 @@ public function execute(Arguments $args, ConsoleIo $io): int
// Register the current version for future upgrade purposes
$this->metaTable = TableRegistry::getTableLocator()->get('Meta');
$this->metaTable->setUpgradeVersion();


// Track remaining selected tables (if any) so we can exit early when done
$pendingSelected = [];
if (!empty($selected)) {
$pendingSelected = array_fill_keys($selected, true);
}

foreach(array_keys($this->tables) as $t) {
$modeltableEmpty = true;
$notSelected = false;
Expand Down Expand Up @@ -283,7 +310,7 @@ public function execute(Arguments $args, ConsoleIo $io): int
$modeltableEmpty = false;
$io->warning("Table (" . $t . ") is not empty. We will not overwrite existing data.");
}

// Step 7: Get total count of source records for progress tracking
$qualifiedTableName = $this->inconn->qualifyTableName($this->tables[$t]['source']);

Expand Down Expand Up @@ -383,6 +410,15 @@ public function execute(Arguments $args, ConsoleIo $io): int
$this->runPostTableHook($t);
}

// If user selected a subset, exit as soon as all selected tables are processed
if (!empty($pendingSelected) && isset($pendingSelected[$t])) {
unset($pendingSelected[$t]);
if (empty($pendingSelected)) {
$io->info('All selected tables have been processed. Exiting.');
return BaseCommand::CODE_SUCCESS;
}
}

// Prompt for confirmation before processing table
$tables = array_keys($this->tables);
$currentIndex = array_search($t, $tables);
Expand All @@ -397,7 +433,7 @@ public function execute(Arguments $args, ConsoleIo $io): int

return BaseCommand::CODE_SUCCESS;
}


/**
* Validate incompatible/invalid "info" related options.
Expand Down
56 changes: 54 additions & 2 deletions app/plugins/Transmogrify/src/Lib/Traits/RowTransformationTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,62 @@
namespace Transmogrify\Lib\Traits;

use App\Command\Util\InvalidArgumentException;
use App\Lib\Enum\GroupTypeEnum;
use Transmogrify\Lib\Util\RawSqlQueries;
use Doctrine\DBAL\Exception;

trait RowTransformationTrait
{

/**
* Apply validation rules for CoGroup names during transformation
*
* @param array $origRow Original row data from source database
* @param array $row Row data to be transformed, passed by reference
* @return void
* @throws \InvalidArgumentException If group name validation fails
*/
protected function applyCheckGroupNameARRule(array $origRow, array &$row): void
{
// Default selection rule for CoGroups (AR-Group-9):
// For any Standard ("S") group:
// - If name equals 'CO' (case-insensitive, trimmed), throw and skip migration (always).
// - If name contains ':', by default throw and skip migration.
// If the optional --groups-colon-replacement flag is set, replace ':' with the provided string instead.

$gtype = $origRow['group_type'] ?? null;
$name = (string)($origRow['name'] ?? '');

if ($gtype === GroupTypeEnum::Standard) {
// Disallow exact "CO" (case-insensitive, trimmed) unconditionally
if ($name !== '' && strtoupper(trim($name)) === 'CO') {
throw new \InvalidArgumentException('Standard CoGroup name "CO" is invalid by default and cannot be auto-renamed');
}

// Handle colon rule
if ($name !== '' && str_contains($name, ':')) {
// Optional, opt-in replacement
$replacement = (string)($this->args?->getOption('groups-colon-replacement') ?? '');
if ($replacement === '' && ($this->args?->getOption('groups-colon-replacement-dash') === true)) {
$replacement = '-';
}
if ($replacement !== '') {
$newName = str_replace(':', $replacement, $name);
if ($newName === '') {
// Guard against accidental empty name
throw new \InvalidArgumentException('Replacing ":" produced an empty Standard CoGroup name; adjust --groups-colon-replacement');
}
$row['name'] = $newName;
$this->io?->verbose(sprintf('Replaced ":" in Standard CoGroup name "%s" -> "%s"', $name, $newName));
} else {
// Default: error out (no auto-replacement)
throw new \InvalidArgumentException('Standard CoGroup names cannot contain a colon by default: ' . $name);
}
}
}
}


/**
* Check if a group membership is actually asserted, and reassign ownerships.
*
Expand Down Expand Up @@ -332,7 +383,7 @@ protected function mapExternalIdentityToExternalIdentityRole(array $origRow, arr
}

$tableName = 'external_identity_roles';

// Fix up changelog and booleans prior to insert
$this->populateChangelogDefaults($tableName, $roleRow, true);
$this->normalizeBooleanFieldsForDb($tableName, $roleRow);
Expand All @@ -354,6 +405,7 @@ private function performNoMapping(array &$row, string $oldname): void
unset($row[$oldname]);
}


/**
* Compute value for a field via a mapping function name (without the leading &).
* Reuses the original field name in-place. Throws if the mapping yields a falsy value.
Expand All @@ -363,7 +415,7 @@ private function performNoMapping(array &$row, string $oldname): void
* @param string $funcName Name of mapping function to call
* @param string $table Table name for error reporting
* @return void
* @throws \InvalidArgumentException When mapping returns falsy value
* @throws \InvalidArgumentException When mapping returns falsy value or function not found
*/
private function performFunctionMapping(array &$row, string $oldname, string $funcName, string $table): void
{
Expand Down
12 changes: 6 additions & 6 deletions app/plugins/Transmogrify/src/Lib/Traits/TypeMapperTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,12 @@ trait TypeMapperTrait

public const SERVER_TYPE_MAP = [
'HT' => 'CoreServer.HttpServers',
'KA' => 'CoreServer.KafkaServerS',
'KC' => 'CoreServer.KdcServerS',
'LD' => 'CoreServer.LdapServerS',
'MT' => 'CoreServer.MatchServerS',
'O2' => 'CoreServer.Oauth2ServerS',
'SQ' => 'CoreServer.SqlServerS',
'KA' => 'CoreServer.KafkaServers',
'KC' => 'CoreServer.KdcServers',
'LD' => 'CoreServer.LdapServers',
'MT' => 'CoreServer.MatchServers',
'O2' => 'CoreServer.Oauth2Servers',
'SQ' => 'CoreServer.SqlServers',
];

/**
Expand Down
Loading

0 comments on commit 05aa027

Please sign in to comment.