search

cloudfoundry / eirini

Pluggable container orchestration for Cloud Foundry, and a Kubernetes backend
Apache License 2.0
115 stars 30 forks source link

Fix custom key spelling for LoggregatorCertPath #103

Closed Birdrock closed 4 years ago

Birdrock commented 4 years ago

Thanks for contributing to Eirini! In order for your pull request to be accepted, we would like to ask you the following:

  1. Please base your PR off the master branch. Forked on master.

  2. Describe the change on a conceptual level. How does it work, and why should it exist? Ideally, you contribute a few words to the README, too. Unfortunately, it looks like a typo made it into the custom key for yaml unmarshalling for MetricsCollectorConfig. The key is inconsistent with other loggregator keys.

  3. Please provide automated tests (preferred) or instructions for manually testing your change. Test coverage does not currently extend to unmarshalling out of json; test fixtures provide data directly. I can add test coverage if it is desired.

cf-gitbot commented 4 years ago

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

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

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

Birdrock commented 4 years ago

This is most definitely a breaking change since it is a change to the custom key used to unmarshal.

jimmykarily commented 4 years ago

We should also change this: https://github.com/cloudfoundry-incubator/eirini-release/blob/master/helm/eirini/templates/configmap.yaml#L71

Birdrock commented 4 years ago

@jimmykarily Would you like me to make a PR against the eirini-release repository, and reference it here?

jimmykarily commented 4 years ago

My comment was just a note so we don't forget but sure, that would be great.

Birdrock commented 4 years ago

Corresponding PR for eirini-release

georgethebeatle commented 4 years ago

Thanks for contributing @Birdrock