Skip to content

Conversation

olragon
Copy link
Contributor

@olragon olragon commented May 4, 2021

Some apps (eg: tiredofit/docker-baserow) require multiple domains. To reduce friction for the user, we don't want to split the app into multiple sub-apps or require the user to manually change Nginx template config or mess with Le'tsEncrypt certs. The simplest solution is to allow users to add app's subdomain.

For Caprover's server with root domain is root.domain.com, we have app1 at app1.root.domain.com. Allow users to connect subdomain api.app1.root.domain.com.

Some apps (eg: tiredofit/docker-baserow) require multiple domains. To reduce friction for the user, we don't want to split the app into multiple sub-apps or require the user to manually change Nginx template config or mess with Le'tsEncrypt certs. The simplest solution is to allow users to add app's subdomain.

For Caprover's server with root domain is root.domain.com, we have app1 at app1.root.domain.com. Allow users to connect subdomain api.app1.root.domain.com.
@olragon olragon mentioned this pull request May 4, 2021
5 tasks
@@ -301,7 +302,7 @@ class ServiceManager {
customDomain.indexOf(dotRootDomain) >= 0 &&
customDomain.indexOf(dotRootDomain) +
dotRootDomain.length ===
customDomain.length
appDomain.length
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not gonna work because it allows the user to choose a custom domain that could be used for another app.

For example:

  • Your root domain: root.domain.com
  • Your app: app1
  • It allows the user to choose app10.root.domain.com as a custom domain.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed 2888903 & small refactor backed by test case

@githubsaturn
Copy link
Collaborator

Looks good, thanks!

@githubsaturn githubsaturn merged commit 8af7ed2 into caprover:master May 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants