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

Route-Emitter: Support mTLS-only authentication with NATS #609

Closed
domdom82 opened this issue Dec 3, 2021 · 3 comments
Closed

Route-Emitter: Support mTLS-only authentication with NATS #609

domdom82 opened this issue Dec 3, 2021 · 3 comments

Comments

@domdom82
Copy link
Contributor

domdom82 commented Dec 3, 2021

Enter an issue title

Add support for mTLS-only authentication with NATS for Route-Emitter

Summary

With the adoption of nats-tls we now have client authentication built-in with the mTLS handshake that verifies the client certificate.
This makes the additional basic authentication with the "nats" user obsolete. Currently, the erb for route-emitter requires a username and password to be set via the bosh link. This PR makes the template rendering of route-emitter work even when there is no user and password set by NATS.

Diego repo

route-emitter

Describe alternatives you've considered (optional)

An alternative to outright removing the password on NATS side would have been to set a default of "" (empty string) for the NATS password which would have made sure that there is a password and user set at all times. This would have made this PR unnecessary because the link property would always be set. However, I refrained from this approach because empty passwords are bad practise. So on cf-deployment level there should either be set a meaningful (secure) password or no password at all - which is now possible.

Additional Text Output, Screenshots, or contextual information (optional)

There is a slack conversation about the issue.

Since this basically affects all NATS clients and the NATS release, there are a set of PRs to be merged as companions to this PR:

NATS-release: cloudfoundry/nats-release#39
routing-release: cloudfoundry/routing-release#244
metrics-discovery-release: cloudfoundry/metrics-discovery-release#14

Once they are all merged, you can remove this section from cf-deployment.yml. This could be done directly or via an ops-file that keeps the basic authentication as a default.

@ameowlia
Copy link
Member

ameowlia commented Dec 7, 2021

Oops, I somehow missed this issue in my mass-merge event. It looks like this can be closed now.

@domdom82 - can you close this issue or point out what is left to do here?

Thanks!

@ameowlia ameowlia moved this from Inbox to Reviewer Assigned in DEPRECATED App Platform - Diego Dec 7, 2021
@ameowlia ameowlia moved this from Reviewer Assigned to Review in Progress in DEPRECATED App Platform - Diego Dec 7, 2021
@xavierW
Copy link

xavierW commented Dec 7, 2021 via email

@domdom82
Copy link
Contributor Author

domdom82 commented Dec 9, 2021

@ameowlia I opened it as an enhancement to follow the guidelines for diego PRs. Closing as it is merged now.

@domdom82 domdom82 closed this as completed Dec 9, 2021
@ameowlia ameowlia moved this from Review in Progress to Done in DEPRECATED App Platform - Diego Jan 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

No branches or pull requests

3 participants