MatthewJohn / terrareg

Open source Terraform module registry with UI, optional Git integration and deep analysis
https://gitlab.dockstudios.co.uk/pub/terrareg
GNU General Public License v3.0
273 stars 24 forks source link

Allow numbers on module name #7

Closed jose-sky closed 1 year ago

jose-sky commented 1 year ago

Hi,

Feature request

I would like to name my module S3. Right now that is throwing an error. It doesn't say specifically the problem but I assume is the number.

Module name is invalid

jose-sky commented 1 year ago

Okay I can see why it is, the regexp only allow a minimum of 3 characters. Same is happening for the creation of a namespace call db

if not re.match(r'^[0-9a-zA-Z][0-9a-zA-Z-_]+[0-9A-Za-z]$', name):

perhaps it would be acceptable to change it to:

if not re.match(r'^[0-9a-zA-Z-_]+[0-9A-Za-z]$', name):

jose-sky commented 1 year ago

Actually if you don't want to allow the first character to be _ or - then it will be the opposite

if not re.match(r'^[0-9A-Za-z][0-9a-zA-Z-_]+$', name):

MatthewJohn commented 1 year ago

Hey @jose-sky ,

Thanks for raising this :)

I'd be a little concerned with starting/ending with dash/underscore - how would this sound?

^[0-9A-Za-z][0-9a-zA-Z-_]*[0-9A-Za-z]$

So at least 2 characters of: any alpha-numeric character at beginning and end, with dashes/underscores allowed in the characters in-between?

Many thanks Matt

MatthewJohn commented 1 year ago

Hey @jose-sky

Assuming you're happy with this suggestion, I've made an upstream ticket (https://gitlab.dockstudios.co.uk/pub/terrareg/-/issues/397) and a PR with the fix (https://gitlab.dockstudios.co.uk/pub/terrareg/-/merge_requests/332).

Let me know if you're happy/have any concerns with this implementation and I'll merge once you've confirmed :)

Many thanks Matt

jose-sky commented 1 year ago

This sounds great and cover all our use case for now 👍 Thanks

MatthewJohn commented 1 year ago

Excellent - I'll merge the PR :) Apologies, I heard about the mail delivery issues - hopefully they should be sorted now :)

MatthewJohn commented 1 year ago

That's now merged and released in v2.69.0 :)