Fixes #39287 - validate domain name format

[Alleny244]

Why

Domain names were validated only for presence/uniqueness in the Domain model.
This allowed invalid characters and malformed values to be accepted.

What changed

• Added format validation for Domain#name using Net::Validations::HOST_REGEXP
• Added domain-specific validation message constant:
  • Net::Validations::DOMAIN_NAME_FORMAT_ERR_MSG
• Extended domain_test with:
  • accepted example (example.com)
  • rejected examples (Example.com, example_domain.com)
  • additional invalid cases (*, *.*, *.example.com, example.*.com, !!!, example..com, etc.)
• Normalized test fixtures/inputs to lowercase where needed

Test plan

Steps

• bundle exec rails test test/models/domain_test.rb

Expected result

• Domain format validation rejects invalid names
• Existing domain behavior remains unchanged for valid lowercase dotted names

[chris1984]

One small comment

[ekohl]

What about data in the database that is already invalid? Have you considered how users deal with that?

[ekohl]

Domain names are case insensitive. It confuses me a bit why HOST_REGEXP does enforce lower case.

[Alleny244]

Domain names are case insensitive. It confuses me a bit why HOST_REGEXP does enforce lower case.

I followed HOST_REGEXP behavior here, but I agree enforcing lowercase for case-insensitive domain names is a bit confusing.

[Alleny244]

What about data in the database that is already invalid? Have you considered how users deal with that?

Actually I hadn’t thought through the existing invalid records yet. I’ll investigate possible migration options.

[stejskalleos]

Actually I hadn’t thought through the existing invalid records yet. I’ll investigate possible migration options.

An idea for two-release enrollment:

1. You can run the validation only for new objects. Existing won't be affected.
2. Create a rake task for checking if all hosts and domains have a valid name, and list the invalid ones. This can be mentioned in the migration path as a step for the upgrade.
3. Switch the validation to run for all objects, including existing ones.

[Alleny244]

• Create a rake task for checking if all hosts and domains have a valid name, and list the invalid ones. This can be mentioned in the migration path as a step for the upgrade.

Yeah, got it. I have updated the pr to enforce new regex validation on new record creation. Also should I include the rake task in this same pr ?

[Alleny244]

Actually I hadn’t thought through the existing invalid records yet. I’ll investigate possible migration options.

An idea for two-release enrollment:

1. You can run the validation only for new objects. Existing won't be affected.
2. Create a rake task for checking if all hosts and domains have a valid name, and list the invalid ones. This can be mentioned in the migration path as a step for the upgrade.
3. Switch the validation to run for all objects, including existing ones.

I have added rake task for checking and listing invalid domains

[nacoool]

Waiting for the reviews to be completed so the changes can be tested.

[stejskalleos]

Suggested change

	Exits 0 if all domain names match, 1 if any invalid rows were found.

This behavior is expected; no need to mention it.

[stejskalleos]

Suggested change

	if invalid_domains.zero?
	puts "Domains: all #{domain_total} records match DOMAIN_NAME_REGEXP."
	else
	puts "Domains: #{invalid_domains} invalid (of #{domain_total} total)"
	end
	exit_code = invalid_domains.zero? ? 0 : 1
	puts
	if exit_code.zero?
	puts 'Done: no invalid domain names found.'
	else
	puts 'Done: fix invalid domain names before enforcing stricter validations on existing records.'
	end
	exit exit_code
	if invalid_domains.zero?
	puts 'Done: no invalid domain names found.'
	exit 0
	else
	puts "Domains: #{invalid_domains} invalid (of #{domain_total} total)"
	invalid_domains.each do |invalid_domain|
	puts "- #{invalid_domain.name}"
	end
	exit 1
	end

(The code is not tested; I wrote it here without an actual run)
For the change, we should store names in the invalid_domains as an array, instead of a number, so we can list the invalid ones.

[Alleny244]

Thanks for the suggestion. Since each invalid domain is already printed inside the find_each loop, is there any advantage to storing them in an array as well, or is printing them as they are found sufficient?