Redesign config file

[zhongyi-zhang]

Create a new issue to continue the discussion in https://github.com/openservicebrokerapi/osb-checker/pull/41.

I do think the config structure needs a redesign to make more sense, so that broker authors can customize it easier. The running order for these test cases is not obvious, e.g. deprovision is mixed into provisions in current config file but actually it runs after the binding cases below. My proposal is that: each case focus on a service with a plan. Broker authors customize only a valid lifecycle and optional validations. For 4xx/5xx-code-expected cases, OSB checker should be able to generate them by itself e.g. the "conflict" scenario case in current config file -- it's not hard to generate invalid request body from an author-provided valid lifecycle.

Just draft a pseudo case for that:

{
  "name": "my abc service test",
  "service_id" :"blabla",
  "plan_id": "blabla",
  "organization_guid": "org-guid-here",
  "space_guid": "space-guid-here",
  "lifecycle": [
    {
      "operation": "provision",
      "async": true,
      "parameters": {
        "blabla": "blabla"
      },
    },
    {
      "operation": "update",
      "async": true,
      "parameters": {
        "blabla": "blabla"
      },
    },
    {
      "operation": "bind",
      "parameters": {
        "blabla": "blabla"
      },
      "expectedCredentialSchema": { // optional
        "blablakey": "<string>"
      }
    },
    {
      "operation": "unbind",
    },
    {
      "operation": "deprovision",
      "async": true
    }
  ]
}

More improvement can be done in this direction.

Of course test.js would need a big refactor to adjust these changes.

[mattmcneeney]

Hey @zhongyi-zhang. I just want to make sure I've understood this proposal correctly. Would service broker authors be responsible for defining a single configuration file like the example above, where they can state the service_id, plan_id and a set of lifecycle commands that are supported (i.e. provision and bind)? Would the test suite then use that configuration file when running the tests?

[zhongyi-zhang]

@mattmcneeney yes, I think service broker authors would be responsible for defining the configuration file, and the test suite would use the configuration file. At this point, it is the same to current design, isn't it? Though getting catalog gives the schemas of the services, still we can't ensure we will input exactly valid provision parameters to create service instances. (Provision parameters could be required and complicated.) The configuration file is for the author to lead a successful provision. Then the checker can continue to test bind, unbind, update, and deprovision on the service instance just created. (At least the happy path lifecycle should succeed.) And different services provided by a service broker can behave different on all above. The authors can add cases for all the services if they like. This is my understanding about OSB checker. And the redesign makes it easier to configure the checker to test against service brokers and cut out unnecessary specification e.g. conflict which can be easily covered in checker side by sending two provision requests with a same instance ID. Am I in the right direction?

[mattmcneeney]

This looks good to me. @kieron-pivotal - I know you tried to do this with the ODB broker. Do you have any thoughts top of mind as to what made this osb-checker difficult to integrate into your test pipelines?

[kieron-dev]

@mattmcneeney - our main problem is the management of state on the broker in order for the test commands to act on real OD brokers / services. This means provisioning brokers and services and providing IDs to the osb-checker so that updates and deletes have the data required to provoke the correct response.

We would really need a comprehensive set of pre- and post- test hooks for setting up and tearing down the real broker side, which can also pass the required IDs into the tests dynamically.

[zhongyi-zhang]

True. The checker doesn't know how to set up and tear down unless authors provide customized approaches to do that -- what will make the config too complicated. Can it require authors to provide extra scripts to do that?

[leonwanghui]

I think this proposal would solve the questions, but it seems to missing the GetLastOperation test case. Like what #47 states, we should figure out how to guarantee the async operation be finished completely.

[zhongyi-zhang]

@leonwanghui it can be addressed by the flag async in the proposed example config. For example, if async is set to true in provision, OSB checker should expect response with 202 from the successful case, then it polls until getting state succeeded (or failed) to guarantee the async operation be finished completely.

[leonwanghui]

@zhongyi-zhang I know the async flag would address the difference between sync and async operation, but what I mean is sometimes it would take mins to finish for async operation, and the test case should know when to execute next operation on the same service instance, or else the service broker would return 422 code.

[zhongyi-zhang]

@leonwanghui I think that's what the test framework should solve, and the config file doesn't need to contain GetLastOperation (a.k.a. Poll). All the async operations can be always followed by poll in the test framework. It is an underlying test case for async operations.

[leonwanghui]

Got it, thanks for your explanation : )

[leonwanghui]

IMO it's time to close this issue : )