loads / loads-web

Web dashboard for Loads
Apache License 2.0
5 stars 3 forks source link

Validate project JSON data against some validator so we know that they're valid #59

Closed pdehaan closed 7 years ago

pdehaan commented 9 years ago

Initially, I'd suggest we use something like Joi since we are already using it for the Hapi route parameter validation.

I think the latest pushgo.json file that I've seen is at https://raw.githubusercontent.com/loads/loads-broker/master/pushgo.json.

I probably have a bit of time to play with this and see if I can have it validate that pushgo.json file locally, and then we can work on converting it into a Hapi POST route which accepts a JSON blob and returns a Boolean success/failure type response.

Once we have that sorted, we can consult w/ @bbangert and @kitcambridge and make sure our schema has the correct things as required and optional.

pdehaan commented 9 years ago

OK, here's a rough stab at a standalone Joi-based pushgo.json schema validator:

// schema.js
'use strict';

var Joi = require('joi');

module.exports = Joi.object().keys({
  'name': Joi.string(),
  'plans': Joi.array().items(
    Joi.object().keys({
      'name': Joi.string(),
      'description': Joi.string().optional(),
      'steps': Joi.array().items(
        Joi.object().keys({
          'additional_command_args': Joi.string().optional(),
          'container_name': Joi.string(),
          'container_url': Joi.string(),
          'dns_name': Joi.string(),
          'docker_series': Joi.string(),
          'environment_data': Joi.array().optional(),
          'instance_count': Joi.number().integer(),
          'instance_region': Joi.string(),
          'instance_type': Joi.string(),
          'name': Joi.string(),
          'port_mapping': Joi.string(),
          'prune_running': Joi.boolean().optional(),
          'run_delay': Joi.number().integer().optional(),
          'run_max_time': Joi.number().integer(),
          'volume_mapping': Joi.string()
        })
      )
    })
  )
});

And I grabbed those fields from https://raw.githubusercontent.com/loads/loads-broker/master/pushgo.json which I assume has the latest field names. I did my best at marking fields that weren't consistent with .optional() to get this to pass, but I could very easily have been to generous or strict. I mostly just stuck with .string() unless I knew otherwise.

We can probably get a lot stricter and verify the instance_type and instance_region against a known-good list of AWS types and regions (which we'd probably maintain in some JSON file so it could be used to populate the <select> fields in the Angular front end as well).

My index.js file which loads and verifies the schema is this gem:

// index.js
'use strict';

var Joi = require('joi');

var schema = require('./schema');
var json = require('./pushgo.json');

Joi.validate(json, schema, function (err, value) {
  if (err) {
    throw err;
  }
  console.log(JSON.stringify(value, null, 2));
});

This isn't entirely readable, so I'll work on making this standalone pushgo.json linter it's own repo for ease of review and bickering.

pdehaan commented 9 years ago

Poorly written code can now be found and publicly mocked in my pdehaan/pushgojsonlint repo.

Right now it's essentially the same garbage code that I wrote above (only lints a hardcoded file in the repo). Ideally I'd suck a lot less and make it a Hapi project so you could lint any arbitrary JSON file via URL (for example: https://pushgojsonlint.herokuapp.com/lint/https://raw.githubusercontent.com/loads/loads-broker/master/pushgo.json — but I haven't gone that far [yet]).

pdehaan commented 9 years ago

Screw it, I created a quick Hapi version in a separate branch: pdehaan/pushgojsonlint#hapi.

pdehaan commented 9 years ago

... And Heroku app deployed at https://pushgojsonlint.herokuapp.com/ with direct validation (as well as URL support).

pdehaan commented 9 years ago

Possibly easiest to add a Hapi server route of /project/validate which takes POST data and returns a success/error message wrapped in JSON.

{
  valid: true,
  result: {...}
}

or

{
  "valid": false,
  "details": [
    {
      "message": "\"name\" is required",
      "path": "name",
      "type": "any.required",
      "context": {
        "key": "name"
      }
    }
  ],
  "message": "child \"name\" fails because [\"name\" is required]"
}

We'll probably want to check valid Boolean, and if false, use the details[0].message, as the other message field (derived from Error.message) can be a bit ugly for more complex validations:

{
  "valid": false,
  "details": [
    {
      "message": "\"instance_region\" must be one of [ap-northeast-1, ap-southeast-1, ap-southeast-2, eu-central-1, eu-west-1, sa-east-1, us-east-1, us-west-1, us-west-2]",
      "path": "plans.0.steps.0.instance_region",
      "type": "any.allowOnly",
      "context": {
        "valids": [
          "ap-northeast-1",
          "ap-southeast-1",
          "ap-southeast-2",
          "eu-central-1",
          "eu-west-1",
          "sa-east-1",
          "us-east-1",
          "us-west-1",
          "us-west-2"
        ],
        "key": "instance_region"
      }
    }
  ],
  "message": "child \"plans\" fails because [\"plans\" at position 0 fails because [child \"steps\" fails because [\"steps\" at position 0 fails because [child \"instance_region\" fails because [\"instance_region\" must be one of [ap-northeast-1, ap-southeast-1, ap-southeast-2, eu-central-1, eu-west-1, sa-east-1, us-east-1, us-west-1, us-west-2]]]]]]"
}
pdehaan commented 9 years ago

This will hypothetically be fixed by #67 — where you can pass an arbitrary JSON blob (as a string) to an API endpoint and get a response of valid or not).

chartjes commented 7 years ago

wontfix