-
Notifications
You must be signed in to change notification settings - Fork 510
<broker> conf to allow alias under cloud domain - bug 1040257 #4401
Conversation
Need someone to review the approach; not sure if I should do something to keep the user from possibly creating an alias that conflicts with another app's name. |
[test] |
Online Test Results: SUCCESS (https://ci.dev.openshift.redhat.com/jenkins/job/test_pull_requests/6127/) |
controller/app/models/application.rb
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using chomp will try to match the regex against aliases that have nothing to do with the domain_suffix. might be better to wrap the whole thing in an if block, like
if fqdn.end_with?(Rails.configuration.openshift[:domain_suffix])
return false if !Rails.configuration.openshift[:allow_alias_in_domain]
return false if fqdn.chomp(Rails.configuration.openshift[:domain_suffix]) =~ /\A\w+-\w+\.\z/
end
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will, but unless your alias is "foo-bar" with no domain it won't be rejected.
Hmm. I guess someone could want to do that. Fair enough.
Online Merge Results: SUCCESS (https://ci.dev.openshift.redhat.com/jenkins/job/merge_pull_requests/4539/) (Image: devenv_4295) |
goes along with https://github.com/openshift/li/pull/2288 |
[merge] |
[merge] again. |
Evaluated for online up to ca8b065 |
Merged by openshift-bot
https://bugzilla.redhat.com/show_bug.cgi?id=1040257