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

Add SSL services annotation #62

Merged
merged 1 commit into from
Nov 3, 2016

Conversation

jmastr
Copy link

@jmastr jmastr commented Nov 2, 2016

If an application in a container requires HTTPS, one needs to set the
'proxy_path https://...' instead of 'proxy_pass http://...'. With this
patch one can use the annotation:

'nginx.org/ssl-services: "service1[,service2,...]"'

to specify, which services require HTTPS.

Fixes #61

@pleshakov
Copy link
Contributor

Hi @jmastr
Thanks for the pull request! Looks good!

I have a few small suggestions:

  1. Could you rename the annotation from nginx.org/ssl-backends to nginx.org/ssl-services? This will make the naming consistent: we already have nginx.org/websocket-services and nginx.com/sticky-cookie-services. My bad suggesting the wrong name in proxy_pass hard-coded to HTTP #61
  2. As a consequence of renaming, please rename the example folder to ssl-services, the title of the README.md to SSL Services Support, the name of the service from ssl-backend-svc to ssl-svc.
  3. Could you also replace an application speaking HTTPS with an application that requires HTTPS , one of which is speaking SSL with one of which requires HTTPS and is a service for the SSL backend application with is a service for an HTTPS application

If an application in a container requires HTTPS, one needs to set the
'proxy_path https://...' instead of 'proxy_pass http://...'. With this
patch one can use the annotation:

  'nginx.org/ssl-services: "service1[,service2,...]"'

to specify, which services require HTTPS.

Fixes nginxinc#61
@jmastr jmastr changed the title Add SSL backend annotation Add SSL services annotation Nov 3, 2016
@jmastr
Copy link
Author

jmastr commented Nov 3, 2016

@pleshakov done

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.

2 participants