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

Support wildcard default cert by making TLS secretName not required like k8ix #426

Closed
FossPrime opened this issue Nov 7, 2018 · 15 comments · Fixed by #496
Closed

Support wildcard default cert by making TLS secretName not required like k8ix #426

FossPrime opened this issue Nov 7, 2018 · 15 comments · Fixed by #496
Labels
enhancement Pull requests for new features/feature enhancements proposal An issue that proposes a feature request

Comments

@FossPrime
Copy link

FossPrime commented Nov 7, 2018

Describe the bug
Specifying a host but not a secret will cause your ingress to be rejected
ejected: Error retrieving secret for Ingress production-custom-deploy: resource name may not be empty and the https version to output a 404 error.

This is counter to the best wildcard certificate solution in kubernetes/ingress-nginx. Which lets you configure TLS, but leave out the certificate, instead defaulting to your default certificate. In our cases, a wildcard certificate.
kubernetes/ingress-nginx#2170 (comment)

To Reproduce
Steps to reproduce the behavior:

  1. Configure kubernetes-ingress
  2. Configure an ingress with tls, but leave out secretName
  3. See error

Expected behavior
The default certificate defined in the controller should be used and the rest of the ingress config obayed. This is not easy to workaround as ingress' are often namespaced and wildcard certificates are usually out of reach

Your environment

  • Version of the Ingress Controller - 1.3.2
  • Version of Kubernetes 1.10
  • Kubernetes platform GCP
  • Using NGINX
@pleshakov
Copy link
Contributor

@rayfoss
the -default-server-tls-secret cli argument is not meant to be used to specify the default certificate/key for Ingress resources -- only for the catch-all default server that handles http and htttps requests for non-existing hostnames.

A single wildcard certificate that could be applied to all Ingress resources is a valid use case. But for that I think it is better to have a separate command line argument that takes secret like -default-tls-sercret, rather than reusing -default-server-tls-secret argument.

@pleshakov pleshakov added proposal An issue that proposes a feature request enhancement Pull requests for new features/feature enhancements labels Nov 7, 2018
@FossPrime FossPrime changed the title TLS secretName is required Support wildcard default cert by making TLS secretName not required like k8ix Nov 7, 2018
@pleshakov
Copy link
Contributor

A workaround is available with mergeable Ingress resources -- https://github.com/nginxinc/kubernetes-ingress/tree/master/examples/mergeable-ingress-types

In this case it is possible to define master Ingress resources with TLS termination in one namespace, where all master will reference a particular wildcard secret, and then define minion Ingress resources with the same hostnames in other namespaces.

@FossPrime
Copy link
Author

FossPrime commented Nov 7, 2018

@pleshakov I heavily rely on websockets... masters don't work with them. My current work around is to manually script the copying of the certificate to all the needed namespaces, I realise this is not as secure and it's tedious.

@FossPrime
Copy link
Author

Other than adding a new argument that reuses default-server-tls-secret code, making that the default secret and then updating the schema to make it not required. Are there any other considerations? I'm considering putting together an PR, this would greatly simplify devops on my end... I hate k8s devops with a passion

@pleshakov
Copy link
Contributor

I heavily rely on websockets... masters don't work with them.

Yep. So the websocket annotation must be used in minions.

Other than adding a new argument that reuses default-server-tls-secret code, making that the default secret and then updating the schema to make it not required. Are there any other considerations?

Right now the default server secret is required for running the IC. We have been considering removing the default server secret requirement after we changed how we treated Ingress resources without secrets -- #399 (this change will appear in the next 1.4.0 release).

Considering the default secret case, it might be a good idea to just change the existing default server secret feature into the default secret feature. However, further thinking about this idea is required to make sure all the cases are considered. Let me think about it and I will update this issue.

@pleshakov
Copy link
Contributor

Below is a proposal about the default (wildcard) secret:

  • we remove the default-server-tls-secret parameter. The default server that listens on port 443 for HTTPS requests for non-existing hostnames will start breaking any attempt to establish an SSL connection to it.
  • we introduce a new parameter -default-tls-secret that expects a secret in the form of namespace/name. This secret will be used for TLS termination for any Ingress resource that configures TLS but does not reference any TLS secret. This will allow us to have one wildcard certificate that is shared across any ingress resources, no matter what namespace each resource is in.
  • we also monitor the specified secret for updates, updating NGINX when the secret changes. If the secret is removed, any HTTPS requests to the affected hostnames will result into an error when trying to establish an SSL connection.

@rayfoss
It seems to me that this solves the problem you're facing. Is it the case?

Implementing this proposal is an involved task and we'd like to take responsibility for it. This week we're releasing Release 1.4.0 and this proposal is a feature candidate for 1.5.0

@FossPrime
Copy link
Author

Sounds good... Thanks for getting into the edge cases. I know tls is technically more correct to use tls-secret, but k8ix is using --default-ssl-certificate... Using the same language should ease frustration for people switching

@pleshakov
Copy link
Contributor

pleshakov commented Dec 20, 2018

Here is an updated course of action for #426 (comment) - instead of removing -default-server-tls-secret parameter, make it optional

ETA for this feature - end of January

@pleshakov
Copy link
Contributor

for the time being, a better workaround is described here -- #466 (comment)

@ameijer-corsha
Copy link
Contributor

hello @pleshakov , I just wanted to follow up on this feature - do you have an updated ETA?

@Rulox
Copy link
Contributor

Rulox commented Feb 20, 2019

Hi @ameijer-corsha the feature is almost ready and will be merged by the end of next week.

Please note that this means you will be able to use it in the edge version of the Ingress Controller, but it won't be released until next IC version is out (among other new features).

More info about releases https://github.com/nginxinc/kubernetes-ingress/#nginx-ingress-controller-releases

Rulox pushed a commit that referenced this issue Feb 22, 2019
Rulox pushed a commit that referenced this issue Feb 22, 2019
@Rulox
Copy link
Contributor

Rulox commented Feb 22, 2019

@ameijer-corsha This is merged in master already (edge version of the IC). Have a look at the wildcard-tls-secret argument in https://github.com/nginxinc/kubernetes-ingress/blob/master/docs/cli-arguments.md Enjoy!

@FossPrime
Copy link
Author

The edge image worked perfectly for me... can't wait for 1.5 stable

        - -v=3
        - -default-server-tls-secret=$(POD_NAMESPACE)/wild-my-app-staging-tls
        - -wildcard-tls-secret=$(POD_NAMESPACE)/wild-my-app-staging-tls

@robinjoseph08
Copy link

I also can't wait for this to hit stable! Is there any ETA for when the next stable release will be published?

@Rulox
Copy link
Contributor

Rulox commented Apr 12, 2019

Hi @robinjoseph08

Our plan is to release 1.5 in the second half of May. Apart from this wildcard cert feature, there are a lot of other new features for 1.5 and we need to be sure everything works for the release.

Stay tuned!
Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Pull requests for new features/feature enhancements proposal An issue that proposes a feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants