Skip to content

feat: sanitize VM names for OpenStack resource naming#219

Merged
fdiazbra merged 2 commits intoos-migrate:mainfrom
miguelntt:feature/safe-vm-name
Mar 3, 2026
Merged

feat: sanitize VM names for OpenStack resource naming#219
fdiazbra merged 2 commits intoos-migrate:mainfrom
miguelntt:feature/safe-vm-name

Conversation

@miguelntt
Copy link
Copy Markdown

Summary

VM names in VMware environments often contain accented characters,
special symbols or other characters that are not valid in OpenStack
resource names. This was causing failures or unexpected names in
Cinder volumes, Nova instances and Neutron ports.

Changes

plugins/module_utils/utils.go

Extended SafeVmName with three additional sanitization steps:

  • Transliteration: common non-ASCII characters are mapped to their
    ASCII equivalent before the regex runs (e.g. ñ→n, ß→ss, ç→c,
    á→a). Covers Spanish, Portuguese, French, German and extended Latin.
  • Underscore collapsing: consecutive underscores produced by the
    replacement step are collapsed into one (e.g. vm--01 → vm_01).
  • Length limit: result is truncated to 64 characters and any
    trailing underscore left after truncation is stripped.

plugins/modules/src/migrate/migrate.go

SafeVmName is now applied to vmName when looking up an existing
volume and when setting the name of a newly created volume.

plugins/modules/src/create_server/create_server.go

SafeVmName is now applied to moduleArgs.Name in both code paths
that create a Nova instance.

plugins/modules/src/create_network_port/create_network_port.go

SafeVmName is now applied to moduleArgs.VmName when constructing
the Neutron port name.

tests/unit/utils_test.go

Updated existing test expectations to match the new behaviour and added
test cases covering underscore collapsing, transliteration (accented
vowels, ñ, ß, ç), truncation to 64 characters and trailing-underscore
stripping after truncation.

Testing

go test -v ./tests/unit/...

Copy link
Copy Markdown
Contributor

@fdiazbra fdiazbra left a comment

Choose a reason for hiding this comment

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

Thanks for this comprehensive PR! The SafeVmName enhancement is a real improvement for handling non-ASCII VM names. However, there are a couple of issues that need to be addressed before merging:

  • Remove changes from another PR
  • Verify transliteration tests

Comment thread plugins/modules/src/migrate/migrate.go Outdated
}
} else {
logger.Log.Infof("Skipping V2V conversion...")
volMetadata = map[string]string{
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It looks like this PR includes changes from another PR

Could you please rebase this branch on the current main branch to include only the SafeVmName changes? This will make the review cleaner and avoid merge conflicts.

Comment thread tests/unit/utils_test.go
Comment thread tests/unit/utils_test.go Outdated
Extend SafeVmName to transliterate common non-ASCII characters
(Spanish, Portuguese, French, German), collapse consecutive
underscores, truncate to 64 characters, and strip trailing
underscores. Apply it consistently across migrate, create_server
and create_network_port.
@miguelntt miguelntt force-pushed the feature/safe-vm-name branch from 7300417 to c1020cf Compare February 26, 2026 05:32
@miguelntt
Copy link
Copy Markdown
Author

Thanks for the review! I've addressed both points:

1. Removed changes from another PR
Rebased the branch directly onto main, so the UpdateVolumeMetadata block from the skip-conversion fix no longer appears in this PR's diff.

2. Fixed transliteration tests
Replaced the test cases that were using plain ASCII strings with ones that actually exercise the non-ASCII characters being transliterated:

  • Spanish: Ángel_García, Açaí_VM, Muñoz-Server, Ñoño_Server
  • Portuguese/French: Château_prod, Hôtel_de_ville, São_Paulo
  • German: Müller_vm, straße_01 (ß → ss)

Copy link
Copy Markdown
Contributor

@fdiazbra fdiazbra left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@fdiazbra fdiazbra merged commit 997b1ee into os-migrate:main Mar 3, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

4 participants