feat(controller): add wildcard subdomain support

[obmarg]

This alters the check for wildcard subdomains in the controller to not
explicitly disallow wildcard subdomains. It adds some basic checks to
make sure the wildcard is at the start or end, as nginx doesn't support
wildcards anywhere else. It also ensures that the wildcard domain can
not obstruct the deis controller domain, as nginx always prefers
wildcard domains over regex domains (which the controller uses).

I've been running wildcard domains in deis for some time by manually
adding them into etcd, so I'm reasonably confident that the controller
validation is the only thing preventing this from working.

This fixes #1608.

EDIT (@bacongobbler): also closes #4685

[carmstrong]

👍

[bacongobbler]

The main blocker why wildcard domain support was not added was due to logic like how Heroku handles wildcard domains:

You can add a wildcard domain if you own all existing apps already using the same top-level domain (TLD). For example, if an app is already using www.example.com, you must own it to add *.example.com.

If one of your apps has a wildcard domain, you can still add specific subdomains of the same top-level domain (TLD) to any of your other apps. Specific subdomains are evaluated before wildcard domains when routing requests.

Would you mind writing some logic to check that against others? This should be easy to do in serializers.py.

[obmarg]

Does that bit of the heroku validation make sense in deis? Heroku is obviously a multi-tenant platform, so it makes sense for them to block a user from piggy-backing on another users domain, but deis is explicitly not suitable for multi-tenant hosting, so I'm not sure if the same restrictions should apply?

Adding checks like this could presumably make it impossible to add a wildcard domain in certain situations as well? Unless I'm misunderstanding, if a cluster had 2 apps with www.example.com and app.example.com owned by different people, then it'd be impossible for anyone to add *.example.com, even though app.example.com and www.example.com would continue working fine in that situation.

Nginx is always going to prefer a specific URL over a wildcard domain, so it's not like I could clobber an existing specific app domain used by another user...

I'm wondering if clashes around this should be managed by an administrator rather than explicitly disallowed (which I think was suggested in #1608). What do you think?

[krancour]

@obmarg it is applicable to deis. Deis apps all have an owner and optionally a list of privileged collaborators. If you are not on that list for a given app, you can not deploy, view or edit configuration, releases, etc., etc.

Real life scenario: in a large org, a central devops team maintains a large deis cluster used for development by multiple depts. / lines of business. One area has created product-a-dev.example.com. Another dept. has no privileges with respect to that application, yet if they create an application of their own with the custom domain *.example.com they can completely hijack the routing and have traffic intended for the first app flowing to their own.

[obmarg]

From the nginx docs:

When searching for a virtual server by name, if name matches more than one of the specified variants, e.g. both wildcard name and regular expression match, the first matching variant will be chosen, in the following order of precedence:

1. exact name
2. longest wildcard name starting with an asterisk, e.g. “*.example.org”
3. longest wildcard name ending with an asterisk, e.g. “mail.*”
4. first matching regular expression (in order of appearance in a configuration file)

So the exact scenario you described shouldn't be able to take place. I am sure there are similar situations that could take place with wildcard domains like *.app.example.com taking over *.example.com, but I get the impression that the logic for detecting that might get pretty complicated. Still, willing to give it a go if it's required.

[obmarg]

Though actually, I guess that matching order means that I need to check for people adding app.example.com when *.example.com already exists (and is owned by someone else) So yeah, I'll look into implementing that.

[krancour]

The issue is that regex is the last thing to be matched and here is what an existing server / vhost definition looks like in the router:

server {                            
    server_name ~^quoted-aqualung\.(?<domain>.+)$;
    ...
}

[obmarg]

Ah, I see - so I need to validate that wildcard domains couldn't match against the name of any apps not owned by the current user, as well as ensuring they're not going to clash with any custom domains not owned by the current user?

[krancour]

You got it.

[obmarg]

Although having said that my code already disallows *.deisapp.local (where deisapp.local is the /deis/platform/domain). I'm not sure we can do better than that for the regex ~^quoted-aqualung\.(?<domain>.+)$, because any domain with a leading wildcard is going to have overlap with that regular expression...

[krancour]

I think custom domain names are not matched using a regex (I'd have to double-check that, but I see no reason they would be), so if you're disallowing this for the platform domain, ya... I can see where maybe there isn't an issue after all. @bacongobbler ?

[bacongobbler]

Yes I don't think that's a problem. Here's the situation:

• platform domain: *.deisapp.local
• app (default) domain: quoted-aqualung.(?<domain>.+)
• quoted aqualung is owned by the same user, but has domain bar.foo.com
• app named foo with wildcard domain *.foo.com

Some example requests:

GET foo.foo.com --> foo
GET quoted-aqualung.deisapp.local --> quoted-aqualung
GET bar.foo.com --> quoted-aqualung (hardcoded subdomains take precedence over wildcards)
GET quoted-aqualung.foo.com --> foo (wildcard takes precedence over regex)

With that in mind, I still think that in order for this PR to be 🚢'd we need to ensure all wildcard domains added do not clash with any domains not owned by the current user as @obmarg mentioned in #4762 (comment).

To justify why this check is necessary, Deis is not a multi-tenant workflow yet (heck, we say that explicitly in the docs). However, I do believe that one of the main goals of Deis is to literally become an "open source Heroku" at some point, which would include ensuring that Deis can be run as a public SaaS. In those cases, having something like the rules which Heroku follows for domain ownership is crucial.

To show a real-world example where this shows off why it's necessary for this check:

• user 1 owns app foo with domain bar.car.com
• user 2 owns app hello with domain *.car.com

User 1: GET bar.car.com --> foo. "Yay! My app!"
User 2: GET bar.car.com --> foo. "What? This isn't my app!"

User 2 expected to be routed to their app named hello, but instead got sent to another user's app. This is not multi-tenant and which is why this check is necessary to accommodate that.

[obmarg]

Ok, so I've updated this with a relatively simple bit of validation to avoid clashes with wildcard subdomains. It goes through each of the existing domains to check if any other domains share the same prefix, and rejects if they do (and the user does not have permissions to the clashing app).

I did remove support for trailing wildcards as part of this, as they would have complicated the logic a fair bit, and I wasn't confident in getting things right if I kept them in.

[sstarcher]

Any update on this?

[helgi]

Working on it currently for v2

[carmstrong]

I think we should consider this for inclusion in the LTS release as well. /cc @mboersma

[sstarcher]

@carmstrong if you need any testing of this feature or any help let me know and I could probably allocate some time to help. Currently we use a script to query our DNS records to set all of the --subject-alt-names for our wild cards. It's ugly and this would make that all go away.

[bacongobbler]

looks good enough for a first pass.

[carmstrong]

Code LGTM, but I'd like someone on the LTS team to manually test this.

[krancour]

attn: @helgi. If this gets into LTS, and it looks like it will, we will have to port this forward to v2. Just an fyi. I think you've already been thinking about this.

[krancour]

I don't think this handles one specific case. You are checking that the addition of a wildcard domain isn't going to obstruct access to an existing application unless they have the same owner, but I believe the inverse needs to be checked as well.

Supposed *.foo.com already exists and is mapped to some app foo. If I come along, even as a user with no permissions on the app foo, if I am not mistaken, as it currently stands with this PR, I could add bar.foo.com to an app of my own and no conflict would be detected. Requests to bar.foo.com would be intercepted by my new application with the owner of *.foo.com being none the wiser.

Can anyone else (@bacongobbler?) verify that I'm reading that right?

[bacongobbler]

@krancour yes you are reading that correct. From one of my previous comments, that problem has yet to be resolved. Only owners of the wildcard domain are allowed to add specific subdomains to their applications. I think we'll need to do that validation logic before merging this in.

[krancour]

Removing the awaiting review label since we've established that there are edge cases that need to be handled before we could consider merging this.

[bacongobbler]

ping @obmarg. If you feel like tackling this for the LTS release we hope to cut it in a few days, otherwise we're gonna close this in favor of #4986 since this feature's been implemented for v2.

[obmarg]

@bacongobbler I'm not going to be spending any more time on this. Close away