Remove plaintext NATS

[46bit]

WHAT is this change about?

This commit removes the plaintext NATS nodes. All the software used in cf-deployment has been modified to handle this and use NATS over mTLS.

Without the nats job, and without the nats link, everything standard in cf-deployment will use the nats-tls job and link. Any users with custom components that still require plaintext NATS can re-enable it by keeping the plaintext NATS job.

What customer problem is being addressed?

Until now cf-deployment has deployed a NATS cluster that contained both plaintext and TLS nodes. Internal communication was over mTLS, but clients could choose to connect over plaintext.

Communicating over plaintext is undesirable, as discussed by many users in #906. Previous work on this issue has stopped components relying on plaintext NATS, but we should carry on and remove it entirely.

Please provide any contextual information.

This PR addresses this issue: #929

We have already merged all prerequisite PRs:

• Remove reliance on bosh link named 'nats' routing-release#214
• Add TLS NATS support to service-discovery-controller cf-networking-release#88
• Use mTLS between service-discovery-controller and NATS #931

Has a cf-deployment including this change passed cf-acceptance-tests?

• YES
• NO

Does this PR introduce a breaking change? Please take a moment to read through the examples before answering the question.

YES - removes the existing nats job which could impact operators with custom components using NATS

All route registrars will also need a change introduced in cloudfoundry/routing-release#214.

How should this change be described in cf-deployment release notes?

As an operator I now know that all NATS traffic is sent over mTLS.

Does this PR introduce a new BOSH release into the base cf-deployment.yml manifest or any ops-files?

No

Does this PR make a change to an experimental or GA'd feature/component?

GA'd feature/component

Please provide Acceptance Criteria for this change?

Deploy this change, run the acceptance tests, and see how they pass despite the lack of plaintext NATS.

What is the level of urgency for publishing this change?

Slightly Less than Urgent

Tag your pair, your PM, and/or team!

Collaborating with @ameowlia on this

[cf-gitbot]

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/179038645

The labels on this github issue will be updated when the story is started.

[davewalter]

Looks good. I just had one question about the change to the route_emitter configuration.

[46bit]

@davewalter there’s a commit message about that removal

[46bit]

This is now only waiting on #931. Once that is merged then we can merge this.

[ameowlia]

Hi all!

@46bit has been doing amazing work to make this possible and we would really love to get this change in soon if possible. If there is anything @46bit can do to unblock it, please let them know.

Thank you!

[46bit]

Thanks everyone that's been working to get this work merged! Just this one PR left :)

[davewalter]

@46bit Can I go ahead and merge this on top of #931 or should I cut a release (once CI is green) and then merge this to avoid downtime during upgrade between releases?

[46bit]

@davewalter Unfortunately yes you are best to do two releases to avoid downtime. Embarrassingly I've forgotten what service-discovery-controller does so I'm not sure what the downtime impact would be. Maybe @ameowlia knows?

[davewalter]

It's a container networking component that reconciles internal routes emitted by the route emitter via the NATS message queue.

I am happy to cut a release, merge the last PR, and then cut a second fast-follow release.

[plowin]

Hi @davewalter, thanks a lot!
Do we roughly know when this could be done?

[b1tamara]

Hi @davewalter,
is there a concrete plan when the new release with this PR will be created in order to make the change productive?

[davewalter]

Hi @b1tamara,

Unfortunately, this change has caused a number of failures in the CI pipeline that I need to resolve before I can cut a new release. I should have time to look at them tomorrow to figure out what changes need to be made to make everything work.

Regards,
Dave

cc @46bit

[davewalter]

I managed to look at the failure in the Windows test:

Task 74 | 21:49:49 | Error: Failed to resolve links from deployment 'cf'. See errors below:
  - Failed to resolve link 'nats' with type 'nats' from job 'route_emitter_windows' in instance group 'windows2019-cell'. Details below:
    - No link providers found

This is puzzling to me since both the nats and nats-tls BOSH links are marked as optional in the route_emitter_windows job spec.

I will keep digging tomorrow.

[davewalter]

The other failures are the clean install with isolation segments and upgrade install with isolated diego cells:

Error: Unable to render instance groups for deployment. Errors are:
  - Unable to render jobs for instance group 'isolated-diego-cell'. Errors are:
    - Unable to render templates for job 'route_emitter'. Errors are:
      - Error filling in template 'route_emitter.json.erb' (line 54: undefined method `collect' for nil:NilClass)
  - Unable to render jobs for instance group 'iso-seg-router'. Errors are:
    - Unable to render templates for job 'gorouter'. Errors are:
      - Error filling in template 'gorouter.yml.erb' (line 12: NATS server port number not found in properties nor in "nats" link. This value can be specified using the "nats.port" property.)

These appear to be more straightforward.

The route_emitter jobs route_emitter.json.erb template requires the diego.route_emitter.nats.tls.enabled property to be set to true for it to switch to "NATS TLS" mode. While this was set for the diego cells in the main manifest, it was not set for the isolation segment diego cells we test in CI. Fixed via a0dc959.

Similarly, the gorouter job's gorouter.yml.erb template requires the nats.tls_enabled property to be set for it to switch to "NATS TLS" mode. While this was set for the routers in the main manifest, it was not set for the isolation segment routers we test in CI. Fixed via 5988a3c.

[davewalter]

I've also fixed the Windows test failure, via 56860d5. The windows2019-cell.yml ops-file still had the entries to exclude the ip_addresses from the nats and nats-tls links, which was causing BOSH to require the nats link. At this point, CI should go green over the weekend, which will allow me to cut a new release next week.

[davewalter]

CI is finally green so I cut v17.0.0