Conversation
ssicard
force-pushed
the
ss/processor-wasm-support
branch
from
714cf14 to
362b3ad
Compare
| @@ -0,0 +1,7 @@ | |||
| package conduit | |||
There was a problem hiding this comment.
It looks like you are creating this interface to be able to mock it out in the tests. Did you take a look at the built in go package httptest ? You can use it to create a mock server for testing, without having to create your own interface.
There was a problem hiding this comment.
I looked into it, but I mocking out net/http gives more control over the return. Looking into httptest, it seems like that would be better for testing our api, but I think since the aim of the http calls in validation is to services external to operator, mock is an easier setup for controlling api response.
Also, if it makes you feel better, this was moving code around, the http mocking was already in place in the validation. I just separated them into different files in this PR so that the different packages being mocked would be in different files for better organization.
| return nil | ||
| } | ||
|
|
||
| var registryFactory = func(r *v1alpha.SchemaRegistry, fp *field.Path) (validation.PluginRegistry, *field.Error) { |
There was a problem hiding this comment.
Just curious on your approach around creating the factory function as opposed to having one function that combines the factory implementation directly with the registry function implementation.
There was a problem hiding this comment.
I purely did this to be able to mock out the registry call in the webhook unit tests.
Co-authored-by: Nathan Stehr <nstehr@gmail.com>
| // formatPluginName converts the plugin name into the format | ||
| // "conduit-connector-connectorName" | ||
| // Returns the github organization and the transformed connector name | ||
| // Returns the transformed connector name |
There was a problem hiding this comment.
Fixing outdated comment
| @@ -0,0 +1,7 @@ | |||
| package conduit | |||
There was a problem hiding this comment.
I looked into it, but I mocking out net/http gives more control over the return. Looking into httptest, it seems like that would be better for testing our api, but I think since the aim of the http calls in validation is to services external to operator, mock is an easier setup for controlling api response.
Also, if it makes you feel better, this was moving code around, the http mocking was already in place in the validation. I just separated them into different files in this PR so that the different packages being mocked would be in different files for better organization.
| } | ||
|
|
||
| func (v *Validator) ValidateProcessorPlugin(p *v1alpha.ConduitProcessor, fp *field.Path) *field.Error { | ||
| func (v *Validator) ValidateProcessor(ctx context.Context, p *v1alpha.ConduitProcessor, reg PluginRegistry, fp *field.Path) *field.Error { |
There was a problem hiding this comment.
Since we have one ValidateProcessor call, if we fail on validateProcessorPlugin, only that error will be returned, it is not being returned as a list of all errors.
| return nil | ||
| } | ||
|
|
||
| var registryFactory = func(r *v1alpha.SchemaRegistry, fp *field.Path) (validation.PluginRegistry, *field.Error) { |
There was a problem hiding this comment.
I purely did this to be able to mock out the registry call in the webhook unit tests.
This builds on #84. It adds the functionality to allow for the download and installation of a custom processor using the ProcessorURL field. The processor will be saved to the volume used by the conduit pod. Right now, when the pod is recreated it will download the processor again. There is currently no checking of checksums, versions, etc if any are saved and will overwrite the existing processor, given the name being the same.
2 participants
Description
Adds validation for wasm processors in the operator webhook
Fixes # (issue)
Quick checks: