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

Use relative links instead of absolute #49

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

benzman81
Copy link

Use relative links instead of absolute to make project useable behind ingress using a context path, i.e. https://server:port/contextPath/

Fixes #48

Copy link
Member

@JasonEtco JasonEtco left a comment

Choose a reason for hiding this comment

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

@benzman81 I'm not sure I understand this change; Kubernetes-based applications can support absolute paths. I don't think this needs to change, and I'm in favor of not fixing something that isn't broken. Can you elaborate on what you've tried?

@benzman81
Copy link
Author

@JasonEtco we deployed smee.io in kunernetes. To make it available from outside, we added an ingress rule to make smee available under https://ourDomain.com/smee/. Our other apps and services are configured the same way, like https://ourdomain.com/serviceA, https://ourdomain.com/serviceB or https://ourdomain.com/appC. Generally it is always an issue if you route from a load balancer using a custom context path to smee. Of course, one can configure it at root, but this is not always possible or even desired. Hope this explains it a bit more.

@JasonEtco
Copy link
Member

Ah thanks for that @benzman81, that does help. However, I'm not sure that that's something that Smee needs to support. That's not a common pattern, and I don't want to build functionality with such an uncommon edge case in mind. I hope that makes sense 🙏

To suggest two alternatives:

  • Your Kube environment could include an nginx reverse proxy, that rewrites requests to the correct location.
  • Not sure how viable this is for you but you could use subdomain (like smee.your-domain.com) instead of a sub-path.

@benzman81
Copy link
Author

@JasonEtco I'm not quite familiar with reverse proxy, but I think we are already using it, as all requests to https://ourDomain.com/smee/ are correctly routed to smee docker containers root path. The problem is, for example, the https://ourDomain.com/smee/index.html contains absolute links. By this, the css file, for example, will then reference the file https://ourDomain.com/public/main.min.css and not https://ourDomain.com/smee/public/main.min.css. I dont know how one can handle it by a reverse proxy if you have running multiple services on the same domain. So using absolute paths is at least a restriction by smee.io to only be runnable on root paths. Unfortunatelly, subdomains are not viable for us, which would be the only way to handle this on our side. Currently, we change the urls to be relative using 'sed' before docker build, so at least we can work around this, for now.

@kammerjaeger
Copy link

I have the same setup, I would like to use smee in a sub path through a reverse proxy. All our github apps are already setup like that so it makes sense to have smee also running like that.
Not needing another dns entry + certificate would make my life easier. Will this be supported at some point?
I can probably rewrite the requests through the proxy but in the past that always created strange problems at some corner case.

@kanimaru
Copy link

Yes I have also the Problem our Github Instance is very restricted and I need to deploy Smee in our Kubernetes. Like benz said the way back is the problem cause of the absolute paths.

@grinish21
Copy link

Just landed on this trying to see how to add context path to smee.io. Its been a while on this issue do we plan to have this in this is a real problem when deploying in K8s with ingress. we have multiple apps running under the same hostname like @benzman81 said we need context path. Thoughts???

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.

Project does not work using a context parh
5 participants