[FR] Provide hook for script to run when testing examples

[mogul]

Is your feature request related to a problem? Please describe. When writing a brokerpak, we need to test attributes of the provisioned instance to demonstrate compliance, and sometimes also to test that provision/bind arguments were properly applied.

@blgm added a new CSB feature to spin up the CSB for the duration of csb run-examples. That's great, and removes a lot of developer donkey-work! However, it still leaves no affordance for testing attributes of the the provisioned instance.

Describe the solution you'd like For each example in the service YAML, I want to specify a test script. When csb run-examples is invoked, it will call the specified test script between the provision+bind/unbind+deprovision steps.

Describe alternatives you've considered We currently do this with a fairly elaborate Makefile: Understanding and maintaining the Makefile is a lot of friction to deal with a fairly simple requirement. There's a lot of scaffolding to stand up the CSB running the brokerpak, provision and bind an instance, then run the tests, then tear down the instance. It's finicky code, repetitive across repositories, and has a tendency to drift as people tinker with it. Also it's easy to forget to restart the CSB to pick up brokerpak changes between test runs.

Additional Context Example Makefile and test script (which verifies that DMARC and SPF are configured correctly) for a simple SMTP service. Note how the Makefile has to rely on a Docker image to do its thing. So in addition to brokerpak and Terraform knowhow, someone may have to know a lot about Docker and Makefiles to start working on brokerpak code... Definitely not ideal!

Priority Low

Priority Context We have the Makefile workaround, though we're not happy with it. If you prioritize it, it will make our lives easier because we can drop fairly opaque code that no one wants to understand. It may also draw more people to development of brokerpaks because it's so easy to incorporate tests into the development workflow.

Platform N/A

Applicable Services All

[cf-gitbot]

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

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

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

[blgm]

Hi @mogul, this is a great idea.

Currently an example definition looks like:

examples:
- name: An example
  description: Create a thing.
  plan_id: 3ba75688-ac57-11ed-9b25-fbc8396b95f4
  provision_params: {"memory_size_gb": 1}
  bind_params: {}

I was thinking that this could be extended to look something like:

examples:
- name: An example
  description: Create a thing.
  plan_id: 3ba75688-ac57-11ed-9b25-fbc8396b95f4
  provision_params: {"memory_size_gb": 1}
  bind_params: {}
  validation:
    command: ./test-script.sh

Is that the sort of thing that you were thinking too?

I would imagine passing the binding credentials to the test script via STDIN, because it raises fewer security concerns than using a temporary file or passing JSON as a command line parameter.

One of the potential limitations of this approach would be networking. For example, when creating an RDS, it's quite common to put it in a subnet that does not have public access, so a validation script might not be able to connect. There are ways to work around that, but I just thought it was worth mentioning.

For quite a lot of the tests that I write, I create more than one binding. This is to make sure that what you write with one binding can be read by another. I was thinking that for examples testing, perhaps it would be sufficient to have just one binding. Do you have any thoughts about that?

[mogul]

Hi @mogul, this is a great idea.

Thanks!

Is that the sort of thing that you were thinking too?

Yes, exactly.

I would imagine passing the binding credentials to the test script via STDIN, because it raises fewer security concerns than using a temporary file or passing JSON as a command line parameter.

That would be fine. An environment variable would also be fine, and maybe better if there are also other common parameters to be passed. For example, a common test script specified for many examples may need to examine the binding params in order to validate that the provided binding's behavior matches the expected configuration.

There are ways to work around that, but I just thought it was worth mentioning.

It hasn't been an obstacle for the kinds of services we've been providing so far, but I think it's a reasonable limitation.

For quite a lot of the tests that I write, I create more than one binding. This is to make sure that what you write with one binding can be read by another. I was thinking that for examples testing, perhaps it would be sufficient to have just one binding. Do you have any thoughts about that?

I could have sworn we had a test that involved more than one binding existing at the same time, but I'm having trouble remembering what that might have been... @nickumia-reisys do you remember? And also, do you have thoughts about this proposed feature based on the more complicated/templated Solr testing code you have here?

[mogul]

One more affordance we added to our Makefiles that would be good to bring into run-examples: We inject an env var (either the user name for manual testing, or the identifier of the GitHub Action), and then use that both in naming the instance and to tag all the resources we create for it. That way over time as we hit quota limits or things fail and we end up with orphaned resources, it's pretty obvious which resources can be cleaned up.

run-examples can similarly inject the example name and tag it as a test fixture, but it would help if it also included a similar facility for somehow naming or tagging based on a parameter. That way multiple people (and CI runs) can work in parallel in the same account without stomping on each other.

[mogul]

One more thing on this point:

One of the potential limitations of this approach would be networking. For example, when creating an RDS, it's quite common to put it in a subnet that does not have public access, so a validation script might not be able to connect. There are ways to work around that, but I just thought it was worth mentioning.

It hasn't been an obstacle for the kinds of services we've been providing so far, but I think it's a reasonable limitation.

We already work around this in a few places, for example here, where we inject the IP of the host that's provisioning the instances to the set of client ranges allowed to use the service. I think it's reasonable to expect brokerpak authors to do that, or else ensure that the test account being used for provisioning resources is otherwise accessible to the test script.

[nickumia-reisys]

In response to

I could have sworn we had a test that involved more than one binding existing at the same time, but I'm having trouble remembering what that might have been... @nickumia-reisys do you remember? And also, do you have thoughts about this proposed feature based on the more complicated/templated Solr testing code you have here?

We did write a test that had multiple bindings. It was because we were doing really custom things with updating credentials in solr manually (i.e. running curls to update credentials) and we wanted to make sure of a few things,

• When two bindings were created and one of them was destroyed, the other one remained in tact and was not lost.
• When two bindings were created, they could be assigned different permission levels and Solr would honor the permissions.
• When two bindings were created, the credentials would actually be different.
  • Note: This might be a stupid test (in most cases) since it is technically something that the CSB should do by default, but since we were generating everything manually, I think we were just hesitant that something would leak somewhere.

I think, overall, the feature is still valid to be able to run things against the provisioned resources, but it's a very thin line before this could be abused. For example, in the EKS brokerpak, when we were creating a domain and checking that the domain was actually available, that is technically testing infrastructure and is valid. Spinning up solr and testing that it can create a core is not a good test case. BUT testing that, after a restart, the core is left intact, that is technically testing the persistent storage infrastructure. But are all of these tests necessary? I don't know, it depends on how confidence in the infrastructure is defined.

[blgm]

Hi @mogul, I have a chat with some folks about this, and one suggestion that came up was that maybe it would make sense to work on making the csb create-service, etc... commands work better in scripts, as that would ultimately give more flexibility. The concern is that examples tests will always have some constraints that have to be worked around, but perhaps if there were better building blocks that folks could combine as they wish, then it would be a better experience for everyone.

For example:

SERVICE="$(service-name-generator)"
csb create-service my-service-offering my-plan "$SERVICE"
KEY="$(csb create-service-key "$SERVICE" my-key")
./test-script.sh "$KEY"
csb delete-service "$SERVICE"

The advantage would be that in a scripting language you are free to create multiple bindings, multiple services, etc... and have ultimate flexibility. The above would not work right now, but something like it could be made to work with a few changes.

What are your thoughts?

[mogul]

It's a good point and I think that's a fair direction to consider.

I do like the single-sourcing of examples and documentation in the manifest. Maybe if we just had a command that would give us the examples as JSON for use in the control loop of that script, that would be good enough. (We already have some code that tries to do that here, for example... It would be good to get those examples back into the service YAML.)

When it comes down to it we just want a straightforward way to say "here's how to loop through and test compliance attributes of all our documented examples".

[blgm]

Hi @mogul. I think that the future could involve both approaches. In the future, examples could have a validation script so that they are fully tested documentation, and also we might have a way to write more flexible tests with the csb create-service etc... commands.

Adding the extension to the example tests is unfortunately not going to be at the top of our priority list, so if you (or anyone else reading this thread) wants to submit a PR, then you'd be very welcome to.

[blgm]

Also, in the latest CSB release v0.17.0 the csb client run-examples command was removed in favour of the newer csb run-examples: https://github.com/cloudfoundry/cloud-service-broker/pull/729

On reflection, it may have been too soon to make that change. Please feel free to ask for us to revert that if it causes you problems.

[mogul]

I don't believe that's a problem for any of our repos.

[blgm]

Closing due to lack of activity