Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow manually specified approot. #133

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ygale
Copy link

@ygale ygale commented Jan 12, 2016

For web apps, keter sets the APPROOT environment variable to inform the app, which sits behind keter's reverse proxying, of the correct approot as seen from the outside.

However, keter's algorithm to determine the approot is currently rather simplistic.

For example, keter always uses the first entry from the hosts list in the app bundle's keter.yaml, even if this particular request came in on a different hostname.

For the URL method - HTTP or HTTPS - keter inserts the method of the request as received. That is not always correct; for example, when SSL processing is delegated to an upstream router.

See also this yesodweb thread.

This PR provides a quick-and-dirty workaround for these problems. It allows you to override keter's approot algorithm completely by hard-coding the approot in the app bundle's keter.yaml file. A new parameter approot is now supported in webapp stanzas.

In the future, it would nice to expand this feature to give fine-grained control over the approot algorithm - to specify each of method, domain, and port separately as either a hard-coded value or taken from the request. That is compatible with this PR - specify approot as an object with fields instead of a single string value.

It also would be a good idea to improve keter's default approot algorithm. That is outside the scope of this PR.

@snoyberg
Copy link
Owner

I still don't understand why you're not just implementing this in your app directly.

@ygale
Copy link
Author

ygale commented Jan 12, 2016

Because yesod approot settings are overridden by keter's APPROOT environment variable.

@snoyberg
Copy link
Owner

That's not true, you can control that behavior, as I was trying to tell you
in the mailing list thread.

On Tue, Jan 12, 2016 at 12:09 PM, ygale [email protected] wrote:

Because yesod approot settings are overridden by keter's APPROOT
environment variable.


Reply to this email directly or view it on GitHub
#133 (comment).

@ygale
Copy link
Author

ygale commented Jan 12, 2016

Anyway, hard-coding the approot in the yesod application and/or settings is not the right approach.

We deploy each app build in many environments. The hostname for the current environment - and various other related settings - is defined in keter (or other deployment automation technologies we will be using). Why should we have to track that in parallel in the app settings, when there is already a mechanism to get it from the deployment framework?

And besides - keter's approot feature really does need improvement. Are you opposed to this PR for some reason?

@snoyberg
Copy link
Owner

I'm not maintaining Keter at this point, so it's Christopher's call, and I
don't have an opinion. I just am commenting that this is approaching things
in the most backwards way possible IMO, and that it seems like this
conversation is continuing to be very one-sided. You've asked how to do
this, I've explained how it's done. You've then insisted that Yesod does
something contrary to what I've said, and are now pushing the blame at a
different project when you have a very simple mechanism to fix this in your
own project.

If you really, really think that updating the keter.yaml file in your
project is the only way you should be updating settings, there are plenty
of better ways of doing this still that don't require changes to other
projects. For example: why not have a CUSTOM_APPROOT environment variable?
Or you could have your application parse the keter.yaml file directly.

I really don't have any objection in principle. It's just that the approach
you're taking implies a strong preference to ignore simple, ready solutions
and insist on changing everything around you.

On Tue, Jan 12, 2016 at 12:52 PM, ygale [email protected] wrote:

Anyway, hard-coding the approot in the yesod application and/or settings
is not the right approach.

We deploy each app build in many environments. The hostname for the
current environment - and various other related settings - is defined in
keter (or other deployment automation technologies we will be using). Why
should we have to track that in parallel in the app settings, when there is
already a mechanism to get it from the deployment framework?

And besides - keter's approot feature really does need improvement. Are
you opposed to this PR for some reason?


Reply to this email directly or view it on GitHub
#133 (comment).

@creichert
Copy link
Collaborator

@ygale I haven't chimed in here because I'm not certain I understand the scope of the problem well enough to merge it. If @snoyberg claims this is possible from a different level of the stack, I'm inclined to believe that's the best direction to configure this.

@tolysz
Copy link
Contributor

tolysz commented May 25, 2016

Just a note: for yesod apps we can say whatever we want, it needs a small change in settings.yml
https://github.com/tolysz/video/blob/master/config/settings.yml

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.

4 participants