openservicebrokerapi / osb-checker

An automatic checker to verify an Open Service Broker API implementation against the specification
https://github.com/openservicebrokerapi/servicebroker/
Apache License 2.0
48 stars 40 forks source link

Add configuration file for fusionstage broker #40

Closed leonwanghui closed 4 years ago

leonwanghui commented 6 years ago

Signed-off-by: leonwanghui wanghui71leon@gmail.com

This patch is proposed for contributing the configuration file of Huawei FusionStage brokers into osb-checker, personally it's quite exciting to see brokers from more and more service providers involved into this project to keep the compliance with OSB API, which would absolutely be a win-win solution for both sides.

Please feel free to ask if you have any questions.

cfdreddbot commented 6 years ago

Hey leonwanghui!

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.

duglin commented 6 years ago

I have a more general question... why are there any company specific config files at all in this repo? E.g. https://github.com/openservicebrokerapi/osb-checker/blob/master/2.13/tests/test/configs/config_azure.json

TBH I'm not that familiar with how all of this testing is done but I would have expected people to run the testing framework and provide the user/password/url/etc.. to their Broker at runtime and it would run the tests. Why are files like this needed at all? What's the point of testing against a static json response file?

mattmcneeney commented 6 years ago

@Haishi2016 could you help out with @duglin's question above?

norshtein commented 5 years ago

Hi @duglin , the purpose of the config file is to provide needed information, just "user/password/url/etc..." is not enough to finish the testing. For example, to provision a service instance, we should send a PUT request to /v2/service_instances/:instance_id, with service_id, service_plan, organization_guid, space_guid, and parameters provided in the request body. The tester doesn't know what these fields should be, it should be provided by the author of the service broker. And then the tester will read these information from the config file, construct and send request to the service broker, observe whether the service broker behaves correctly.

I think it's OK for different company to provide the config file for their service broker, in case anyone is interested in the service broker and want to test whether the broker violates the spec.

Hi @leonwanghui , one thing I don't understand is why this config file has so many fake IDs? That's ID like acb56d7c-XXXX-XXXX-XXXX-feb140a59a66. To test one service, the real ID of the service should be provided.

duglin commented 5 years ago

@norshtein re: things like instance_id, service_plan... I would think these would be dynamically determined at runtime by looking at the catalog response or the response from a provision.

norshtein commented 5 years ago

Hi @duglin , yes, things like instance_id, service_plan could be fetched by looking at the catalog, but parameter can't be determined by the checker, so it must be provided by a config file.

leonwanghui commented 5 years ago

Hi @norshtein , thanks for pointing out the question. I think as what @duglin commented, we don't know the dynamic ID of service, service plan... so we just left the fake one there. IMO maybe it could make more sense if we remove these fields and fetch them through GetCatalog(). Any thoughts?

norshtein commented 5 years ago

Hi @leonwanghui , what do you mean by "dynamic ID of service, service plan"? I think for any service/plan in the service broker, the ID of it should be fixed and never changed.

norshtein commented 5 years ago

And GetCatalog() doesn't know the provisioning details about the service/plan, it doesn't know what provision parameters looks like, but our goal is to pass all needed information to the osb-checker and make it provisioning real service instance and test its behavior, that's something like lifecycle test.

leonwanghui commented 5 years ago

The "dynamic" I mentioned means it differs with different service brokers. And if we pre-configure all of them in the config file, the whole file would be hard to maintain. Just my thought : )

norshtein commented 5 years ago

Ideally, executing the checker is one step in CI, so when something like provisioning parameter changes, the one who modifies the code should also update the config file. This helps the broker owner make sure any change on the code will not violate the spec.