search

Azure / open-service-broker-azure

The Open Service Broker API Server for Azure Services
https://osba.sh
MIT License
248 stars 100 forks source link

Support specifying server name, admin username and password of postgreSQL #711

Closed norshtein closed 5 years ago

krancour commented 5 years ago

I don't want to necessarily discourage this PR from being merged, but I do want to provide some context-- the historic rationale for deliberately not allowing users to specify server names, usernames, and passwords.

  1. With provisioning and binding being automated, human users aren't meant to directly handle things like usernames and passwords. With human convenience / memory not being a factor, it shouldn't matter what they are. (This is the "pets vs cattle" argument.)

  2. There is a security benefit to identifiers incorporating a UUID and passwords being generated from random strings. Human users will tend to use usernames like "admin" or "postgres" and will often pair the username with an insufficiently complex password. Letting the system generate these values instead safeguards against brute force attacks.

  3. Lastly, a more pragmatic reason that we incorporated UUIDs into system-generated identifiers (like usernames and server names) was that it almost completely eliminated the possibility of naming conflicts. Doing so meant we did not have to implement any logic to detect and mitigate naming conflicts. I do see that this PR adds a function isServerNameAvailable() and that's a step toward mitigating that. Race conditions are possible, however. Any given server name can become unavailable in between the time it's determined to be available and the moment when provisioning begins.

Again... I don't want to say not to do this. I only want to make sure that the historic rationale for not doing this is understood and carefully considered.

norshtein commented 5 years ago

Hi @krancour , many thanks for the comment, sent you an email explaining the reason : )

norshtein commented 5 years ago

Noticed something went wrong with mssql-dr lifecycle test. As current PR has no relationship with it and the feature in current PR is urgently needed, I'm going to force merge the PR into master.

norshtein commented 5 years ago

@zhongyi-zhang we should investigate what happened with mssql-dr module as the lifecycle test has been run for several times and mssql-dr test failed every time.