Skip to content
This repository has been archived by the owner on May 6, 2020. It is now read-only.

deis open opens http://<app name>.localhost in browser #111

Closed
sgoings opened this issue Dec 17, 2015 · 18 comments
Closed

deis open opens http://<app name>.localhost in browser #111

sgoings opened this issue Dec 17, 2015 · 18 comments
Assignees
Labels
Milestone

Comments

@sgoings
Copy link
Member

sgoings commented Dec 17, 2015

After creating a cluster, registering a deis client, and doing a deis pull type deployment... when I run:

../workflow/client/deis create --no-remote
Creating Application... done, created calmer-handbill
remote available at ssh://git@deis.$IP.xip.io:2222/calmer-handbill.git
 ~/go/src/github.com/deis/example-go  ⑂ master    ../workflow/client/deis pull deis/example-go -a calmer-handbill
Creating build... done
 ~/go/src/github.com/deis/example-go  ⑂ master    ../workflow/client/deis open -a calmer-handbill

My browser opens with the URL: http://calmer-handbill.localhost/

It should open to http://calmer-handbill.$IP.xip.io/

@sgoings sgoings changed the title deis open ... opens http://<app name>.localhost in browser deis open opens http://<app name>.localhost in browser Dec 17, 2015
@mboersma mboersma added the bug label Dec 17, 2015
@mboersma
Copy link
Member

Is this code still using the /deis/platform/domain key? That defaults to localhost. If so, we need either to update that mechanism not to use etcd, or document how to set that key correctly.

@bacongobbler
Copy link
Member

Huh, I was under the assumption that it'd use the base domain that you used when you initially ran deis login. Good catch!

@mboersma
Copy link
Member

Confirmed: if you etcdct set /deis/platform/domain 10.245.1.3.xip.io (or the appropriate IP), then deis open works correctly again, so the workflow controller is using the etcd key for this purpose.

@krancour
Copy link
Contributor

So... my $0.02... you have to tell the router the platform domain and the router doesn't use etcd at all... so making people set an etcd key is a DRY exception we'd be forcing on our users. I like the idea of the CLI deriving the platform domain automatically by scraping the deis. off the front of the controller's base URL and using what's left.

@helgi
Copy link
Contributor

helgi commented Dec 30, 2015

Looked at this a little bit, problem is that we use ALLOWED_HOST = * in Django so using get_host or similar mechanisms are spoofable, depends if we care about that. That'd be determine if we do any sort of scraping or not

For now I'm going to make it so you can set a DEIS_CONTROLLER_DOMAIN and DEIS_CONTROLLER_SUBDOMAIN env var as a stop gap measure until we decide if we want people to add the platform domain on BOTH router and workflow

@krancour
Copy link
Contributor

I really think there's no need for that. Workflow does not need to know the platform domain. Only the router and the client do. The router knows the platform domain already (maybe; @bacobgobbler made it optional recently) but workflow and builder shouldn't need to know it...

git push can end with directions to use deis open. The deis CLI can infer the platform domain from the URL of the controller it is logged into.

@helgi
Copy link
Contributor

helgi commented Dec 30, 2015

https://github.com/deis/workflow/search?utf8=%E2%9C%93&q=DEIS_DOMAIN is where the platform domain is "used".

The models.py usage of it seems to be not used but the BuildHook one appears to be in some sort of use, at least there are tests for it. I'd have to look deeper to confirm that. To be honest I'm all for reducing the need for workflow to know anything like this

helgi added a commit to helgi/controller that referenced this issue Dec 30, 2015
Addresses the workflow side of deis#111 but doesn't do the client part so that it opens up the right domain
helgi added a commit to helgi/controller that referenced this issue Jan 4, 2016
Addresses the workflow side of deis#111 but doesn't do the client part so that it opens up the right domain
helgi added a commit to helgi/controller that referenced this issue Jan 5, 2016
Addresses the workflow side of deis#111 but doesn't do the client part so that it opens up the right domain
helgi added a commit to helgi/controller that referenced this issue Jan 5, 2016
Addresses the workflow side of deis#111 but doesn't do the client part so that it opens up the right domain
helgi added a commit to helgi/controller that referenced this issue Jan 5, 2016
Addresses the workflow side of deis#111 but doesn't do the client part so that it opens up the right domain
helgi added a commit to helgi/controller that referenced this issue Jan 5, 2016
Addresses the workflow side of deis#111 but doesn't do the client part so that it opens up the right domain
@helgi
Copy link
Contributor

helgi commented Jan 5, 2016

Is someone able to verify that deis open behaves more like advertised using a newer binary?

@helgi helgi self-assigned this Jan 5, 2016
@helgi helgi added this to the v2.0-beta1 milestone Jan 5, 2016
@krancour
Copy link
Contributor

krancour commented Jan 6, 2016

Yes. It works as expected.

@krancour krancour closed this as completed Jan 6, 2016
@krancour
Copy link
Contributor

krancour commented Jan 7, 2016

I'm reopening. This was working for me for much of the day. Now, it's not.

What I'm experiencing is that with my client logged into http://deis.micro-kube.io and testing with an application named cheery-yardbird, deis open is directing my browser to http://cheery-yardbird.deis.micro-kube.com

The "deis" doesn't belong in that URL.

If think the reason this worked for me for most of the day is because I was not specifying a platform domain for most of the day. Without a platform domain specified, the router would have created a vhost that matches the pattern ~^cheery-yardbird\.(?<domain>.+)$-- and that happens to handle the incorrect URL just fine.

Now that I've specified a platform domain, vhosts are a little more specific-- like cheery-yardbird.micro-kube.io.

I'll try to track down the error in the client code.

@krancour krancour reopened this Jan 7, 2016
@helgi
Copy link
Contributor

helgi commented Jan 7, 2016

Ah, I was trying to strip out the subdomain initially but it is not super straightforward when you are dealing with scenarios that do not include a subdomain like the tests and especially when you are dealing with xip.io or straight up IPs. That really only happens when you are not going via the router.

I had a brainfart and decided we didn't need to strip but obviously that doesn't pan out all that great.

@krancour
Copy link
Contributor

krancour commented Jan 7, 2016

Isn't the subdomain of the controller always deis? I think something like this works:

const workflowURLPrefix = "deis."
...
app.URL = fmt.Sprintf("%s.%s", app.ID, strings.TrimPrefix(c.ControllerURL.Host, workflowURLPrefix))

Stay tuned for PR.

@krancour
Copy link
Contributor

krancour commented Jan 7, 2016

Ah... now that I think about it, the subdomain of the controller doesn't have to be deis. In v1.x that would have been true, but in v2, the controller is "just another app" from the router's perspective. The controller could have any address.

@krancour
Copy link
Contributor

krancour commented Jan 7, 2016

But the code I posted will safely strip deis. from the beginning of the controller URL if it's there. So it at least improves the situation for the average case.

@helgi
Copy link
Contributor

helgi commented Jan 7, 2016

It's configurable in django right now (https://github.com/deis/workflow/blob/master/rootfs/templates/confd_settings.py#L17 - defaults to deis). We could hardcode it... or not... I just don't want to try to reintroduce passing back URL information

@krancour
Copy link
Contributor

krancour commented Jan 7, 2016

@helgi but the controller URL can literally be anything now. It can be my-awesome-controller.my-deis.com as long as you've annotated the service to make that one of the domains.

It's virtually impossible to accurately infer the app URL from a controller URL that could look like anything.

@krancour
Copy link
Contributor

krancour commented Jan 7, 2016

So we settle for this heuristic?

@helgi
Copy link
Contributor

helgi commented Jan 7, 2016

Yeah - I commented in the PR on what the potential solution could be if we wanted to tackle that. tl;dr put onus on end user

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

6 participants