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.

Sign up for GitHub

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

Jump to bottom

TLS only configured if secretName is set #81

Closed
dozer75 opened this issue Feb 2, 2024 · 2 comments
Closed

TLS only configured if secretName is set #81

dozer75 opened this issue Feb 2, 2024 · 2 comments

Comments

@dozer75
Copy link
Contributor

dozer75 commented Feb 2, 2024

The TLS section of the ingress is not configured unless a secretName is provided, however this is not always necessary. An NGINX ingress controller can have a default certificate configured using the default-ssl-certificate. In these cases the NGINX will always use the certificate configured here for TLS where this is not configured in the ingress.

The issue is here. The conditional setting should not use secretName only as a condition for enabling TLS in the ingress. It should also enable e.g. if tlsHost are set or maybe a new enableTLS flag.
dozer75 pushed a commit to dozer75/idsvr-helm that referenced this issue Oct 30, 2024
Add tlsEnabled flag for ingress TLS handling
65e4f87 
To be able to add TLS to an ingress without having a secret (which is
a fully valid option see curityio#81 for reasoning) this
change adds the tlsEnabled flag, which will handle the TLS configuration
correct.

If the tlsEnabled is set to false, the legacy TLS logic applies.
dozer75 added a commit to dozer75/idsvr-helm that referenced this issue Oct 30, 2024
@dozer75
Add tlsEnabled flag for ingress TLS handling
3df5368 
To be able to add TLS to an ingress without having a secret (which is
a fully valid option see curityio#81 for reasoning) this
change adds the tlsEnabled flag, which will handle the TLS configuration
correct.

If the tlsEnabled is set to false, the legacy TLS logic applies.
@dozer75 dozer75 mentioned this issue Oct 30, 2024
Merged
@dozer75
Copy link
Contributor Author

dozer75 commented Oct 30, 2024

Added a PR for this today as it blocks our usage of the helm chart...

@dozer75
Copy link
Contributor Author

dozer75 commented Nov 4, 2024

Closed by #88

@dozer75 dozer75 closed this as completed Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@dozer75