Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement post-key for manipulating the OM model via the HTTP admin interface #56

Merged
merged 8 commits into from
Jan 3, 2020

Conversation

sesquipedalian-dev
Copy link
Contributor

@sesquipedalian-dev sesquipedalian-dev commented Dec 30, 2019

Current

Openmock has an admin interface (HTTP on port 9998 by default) that allows user to POST new templates. Pseudo code:

Unmarshal POST body as YAML into an open mock model (400 if fails)
Foreach mock in loaded model
S = marshal mock to YAML
Redis hset redis_templates_storage S
End
Restart OM process

When the OM (re)starts, it loads up YAML strings from the templates directory files, and from the redis key redis_templates_storage, and concatenates them into one big YAML string, which it then unmarshals into an OM model. The redis stuff is loaded with redis HGETALL redis_templates_storage

The other functionality to note is that, when evaluating go templates in om (e.g. in conditions, http_reply bodies, etc), the redisDo commands lets mock users run arbitrary redis commands in the instance OM is using. This could potentially interfere with the template storage database.

What we’re trying to do
We would like to add a new endpoint to post sets of templates. This would allow remote services to functionally ‘delete’ the mocks that they’ve added, by doing HTTP DELETE at the same endpoint. This is easier to implement than deleting each key of the mock since the job that does the empty post won’t have to parse the mocks or know anything about them, just the unique key used.

Proposed
Add a new endpoint to the admin API to store sets of templates under a unique key (/api/v1/template_sets/:key). POSTing here causes om to save the mocks with HSET (redis_templates_storage_<post key>) …. DELETEing the same path that deletes the key redis_templates_storage_.
When loading templates in load.go, first find all keys in the DB that start with redis_template_store, and concatenate the strings from all those hsets in the same way as we currently do with just redis_template_store. This should load in all the mocks that were posted as a template set in addition to the base ones.

Testing
Added unit testing for new functionality.
Manual test script:

  1. post a template with set-key ABC
  2. post the same template with set-key DEF
  3. GET the templates, observe the posted template was added
  4. DELETE templates with set-key ABC
  5. GET templates, observe the posted template still present
  6. DELETE templates with set-key DEF
  7. GET templates, observe the posted template is gone now that we've eliminated all sources of it

Work not directly related to immediate goal
Should we take this opportunity to define the admin IF with goswagger?
If we do, we could also make part of omctl update use the generated client?
It seems like a fair amount of work to define OM’s model in swagger terms and use it throughout, maybe save it for hack week projects
We should restrict the redisDo template helper so that any of the ‘args’ to the redis command are checked to see if they start with the string ‘redis_templates_storage’, and thow an error if so. This would prevent users from accidentally messing with OM’s template storage.

Other solutions
Considered storing any additional post keys in a separate redis set so that we don’t have to use the KEYs command to find keys while loading. KEYs is O(1) with a fairly low constant time on larger data stores, and we only have one redis instance for all of an IIE. We should see if performance becomes a problem here, and if so implement that solution. I don’t jump straight to it since it adds some complexity to the implementation in OM.

Copy link
Contributor

@zhouzhuojie zhouzhuojie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉 NICE!

admin_test.go Outdated Show resolved Hide resolved
@zhouzhuojie
Copy link
Contributor

Also, please change the description of this PR

@sesquipedalian-dev
Copy link
Contributor Author

Updated the description to include the change that made this a separate endpoint instead of a header!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants