Skip to content
This repository was archived by the owner on Jan 5, 2019. It is now read-only.

Add extra validations to bosh-init manifest: cloud_provider.mbus and cloud_provider.agent.mbus properties should user https protocol and have the same creds and ports #73

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from

Conversation

allomov
Copy link
Contributor

@allomov allomov commented Apr 19, 2016

Hey, all.

This PR add validations to prevent two a common errors that are not so easy to debug for a newcomer.

The first problem is that bosh-agent does not expect having http as mbus protocol, so it simply skips connecting to mbus. This means that mbus properties should always have https protocol.

The second error is typos in passwords/usernames/ports of mbus parameters. This PR adds checks if this properties are similar in mbus URLs in cloud_properties.mbus and cloud_properties.properties.agent.mbus.

What do you think on this changes?

@cfdreddbot
Copy link

Hey allomov!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA.

@cppforlife
Copy link
Contributor

@allomov unfortunately not all CPIs define that property, so we cant really rely that agent.mbus is there. may be make that check conditional on the property presence?

@allomov
Copy link
Contributor Author

allomov commented Apr 28, 2016

@cppforlife thank you for the valuable update. I will add it now.

@allomov
Copy link
Contributor Author

allomov commented Apr 28, 2016

@cppforlife as far as I understand, cloud_provider.mbus is a required field. Could you confirm it's true?

@cppforlife
Copy link
Contributor

@allomov yup that one is required.

@allomov
Copy link
Contributor Author

allomov commented May 4, 2016

@cppforlife I've updated the brunch to follow your comments.

@cppforlife
Copy link
Contributor

@allomov Cool. Thanks.

@dpb587-pivotal
Copy link
Contributor

PR sounds useful, although I haven't personally hit the cases where it's trying to help. Only thing that stands out to me is we should probably say "must" instead of "should" since the user has no option but to comply with the errors it raises, à la RFC 2119.

I think there's some upcoming work in bosh-init planned. It'll probably be easier for a pair to merge this during/after that effort once we've built up context in this repo again.

@allomov
Copy link
Contributor Author

allomov commented Jun 9, 2016

@dpb587-pivotal thank you for the response. I've merge latest develop into the branch and changed error messages.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants