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

Fix up removing reliance on bosh link named NATS #216

Merged
merged 2 commits into from
Aug 18, 2021
Merged

Fix up removing reliance on bosh link named NATS #216

merged 2 commits into from
Aug 18, 2021

Conversation

46bit
Copy link
Contributor

@46bit 46bit commented Aug 11, 2021

  • Context: this provides some changes minor requested in a comment by @acrmp

  • A short explanation of the proposed change: this removes the deprecated nats.tls.hostname property, and fixes a misnamed field in a test

  • I have viewed signed and have submitted the Contributor License Agreement

  • I have made this pull request to the develop branch

  • I have run all the unit tests using scripts/run-unit-tests-in-docker

  • (Optional) I have run Routing Acceptance Tests and Routing Smoke Tests on bosh lite

  • (Optional) I have run CF Acceptance Tests on bosh lite

This property used to be used to set the hostname to connect
to NATS over mTLS. We now get that from the nats-tls link and
this property is already no longer used.
@cf-gitbot
Copy link

We have created an issue in Pivotal Tracker to manage this. Unfortunately, the Pivotal Tracker project is private so you may be unable to view the contents of the story.

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

@46bit
Copy link
Contributor Author

46bit commented Aug 11, 2021

If we can get this shipped in a new version soon that'd be a big help internally for SAP. ❤️

@46bit
Copy link
Contributor Author

46bit commented Aug 11, 2021

Ready for review.

@acrmp
Copy link
Member

acrmp commented Aug 17, 2021

@46bit Thanks for making these changes!

It looks like the template specs are failing with these changes, can you take a look?

Thanks,

Andrew.

We use both 'host' and 'hostname' in different places to store
the same data. This led to some mistakes in the testsuite. This
commit fixes the last mistaken use of 'host' in the NATS bosh link.
@46bit
Copy link
Contributor Author

46bit commented Aug 18, 2021

@acrmp Confused how I missed that! Fixed.

@ameowlia ameowlia merged commit 46014b3 into cloudfoundry:develop Aug 18, 2021
@ameowlia
Copy link
Member

Thank you @46bit for this fix and thank you @acrmp for finding the 🐛 .

@46bit 46bit deleted the fix-up-removing-reliance-on-bosh-link-named-nats branch August 18, 2021 19:41
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.

None yet

4 participants