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

Adding HTTPS support (mainly for subscription service) #8

Closed
aragilar opened this issue Sep 3, 2019 · 7 comments · Fixed by #11
Closed

Adding HTTPS support (mainly for subscription service) #8

aragilar opened this issue Sep 3, 2019 · 7 comments · Fixed by #11

Comments

@aragilar
Copy link
Contributor

aragilar commented Sep 3, 2019

Currently the subscription service (as a client) can only use HTTP, not HTTPS. I'm planning on using requests to handle the whole HTTPS ecosystem (so that I don't need to worry about certificates from a client's perspective), but currently there's no support in requests (nor in its underlying stack) to use sendfile (also, sendfile over https requires kernel support, and wrappers around the kernel, so that's a whole different thing). I'm going to implement this via only using requests for HTTPS, and putting all HTTPS through requests (i.e. no sendfile over HTTPS), so there shouldn't be any breaking changes. Would a PR containing this be fine on top of master, or would it need to go into next?

@rtobar
Copy link
Contributor

rtobar commented Sep 3, 2019

Hi @aragilar, similarly to #9, could you clarify the final intent of the changes? The description here and in #9 give me the impression that you either want to add full HTTPS support to NGAS as a whole (not a minor task), or that you'll be pushing files through HTTPS to a separate service. Or are you planning to put a proxy between the two instances to receive via HTTPS and internally proxy via HTTP? I remember you had questions about proxying, so maybe that's what you are planning to do?

Regardless of that, I don't see in principle any problems with adding this.

@rtobar
Copy link
Contributor

rtobar commented Sep 3, 2019

Ohhh, by the way, I should actually merge next into master, it doesn't really make sense to maintain two "bleeding edge" branches at all. After that you should be OK branching off the master branch for both PRs.

@aragilar
Copy link
Contributor Author

aragilar commented Sep 3, 2019

So I thought wrapping the server with apache (at AAO North Ryde) would be enough to get https, but the subscription service (on the client side at the AAT) only supports http (so we can't talk to the https server), which is where we need to add the https (so we're not touching the server logic, apart from adding https to the subscription internals). From what internals I've read, as long as we handle the switching between http and https inside ngramsHttpUtils module, then we should have the subscription service (from the client side) have full https support (plus the ngamsPClient will probably get it as a bonus, though I'll leave any explicit support to a later PR if needed).

@rtobar
Copy link
Contributor

rtobar commented Sep 3, 2019

Good, that sounds great :)

@aragilar
Copy link
Contributor Author

I've nearly got something working, I'm still kinda confused what's the purpose of the ngas_hosts table, is that shared between the nodes? It looks like it's used in the proxying logic, but the host_id and domain look to be really short? If the main purpose is proxying, would it be possible to add a column (or another table) with the url the service is running at (including protocol, so we can switch between http and https)?

@awicenec
Copy link
Contributor

awicenec commented Sep 12, 2019 via email

@rtobar
Copy link
Contributor

rtobar commented Sep 13, 2019

@aragilar thanks for the update, I'm glad things are going well on your end.

If I understand correctly, you are trying to deploy a subscription like this: NGAS A ----https---> your-proxy ----http---> NGAS B. In this scenario A and B are two separate clusters, have their own database (hence they can't "see" each other that way), and thus there is nothing to be gained by adding, say, an https url in B's ngas_hosts table.

As you found out, and @awicenec points out, the ngas_hosts table makes sense when you think on a deployment of several servers against a single database (hence, a single cluster). In that scenario they all get to know who is present in this shared deployment, which files are were, how to proxy, etc. So yes, it's used for the proxying, but also for other things in multi-server deployments. I agree with Andreas that, if anything, we would add a protocol column, but since the NGAS server currently supports http only there is no need for even that.

Does that clarify a bit the situation?

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 a pull request may close this issue.

3 participants