Add audit interface for validation API

[xiekeyang]

Fix #686.

Each content validation function should return both hard error and
warned errors, like:

func validateDescriptor(r io.Reader) ([]error, error)

Add a ValidationAudit interface on Validate API argument. This
interface allows consumer to implement Warns method, to decide how to
handle warned errors. like:

func (v Validator) Validate(src io.Reader, audit ValidationAudit) error

Signed-off-by: xiekeyang xiekeyang@huawei.com

ping @stevvooe @vbatts If it is on the right way?

[stevvooe]

@xiekeyang I am not sure that having methods return ([]error, error) is at a good a approach. There is already a design, proposed in #686 (comment), that attempts to do this. It might be better to explore that and work from there.

[xiekeyang]

@stevvooe I had read the comment, and guess you want consumer implement Warnings and Errors interface. After that, consumer can collect separated warns and hard error. In the case of hard error, API schema.Validate() should be failed, and in warns, consumer handle them using their own methods. For that we are consistent.

In your comment:

if err := validate(content); err != nil {
  for _, warning := range Warnings(err) {
    // do something with warnings
  }
}

I feel it is a little unclear that you likely want to exec customized methods (Warnings) out of API, and input arg is just err. That mean API return just 1 error, maybe warn or hard error.

If above understand is right, content validation will be broken when it encounter a warn. That might impact consumer badly even he doesn't care warn.

in my implementation, API and its mapped-functions(validateManifest, validateDescriptor...) determine which is hard error(unconditional break function), which are warns(just throw to consumer). If consumer care warn, it can also break validation with audit.Warns() and return error. That makes sure validation run smoothly.

Maybe I miss your point, please point it, and make some more description on your code segment. I would figure out a better way.

[stevvooe]

@xiekeyang The goal here is to treat all validation problems as errors. If a user wants to opt out, they can. This API prevents complexity for the common case. Please follow it, as designed.

The validation code is really broken as of 1.0, so I don't think breaking is a problem.

[xiekeyang]

@stevvooe While I still a little misunderstand, here I guess you want:

1:

validMediatypeMap returns []error, instead error for all validated problems, which used as arg of interface methods:

func validateDescriptor(r io.Reader) []error

func validateConfig(r io.Reader) []error

func validateManifest(r io.Reader) []error

2:

And we define Warnings and Errors interface like:

type Warnings interface{
  Warnings([]error) error
}

type Errors interface{
  Errors([]error) error
}

Then customized methods can be done inside validation API:

func (v Validator) Validate(src io.Reader, warns Warnings, errors Errors) error{
  ...
  errs := validateMediatypeMap(src)
  err := warns.Warnings(errs)
  err = errors.Errors(errs)
  ...
} 

Above design makes sure that:

1. validation media-typed map returns all problems as common error, be compatible to spec;
2. Consumer must use uniform interface, with customized methods;
3. Validate API input freely(interface), and return simply(error);
   This design left freely handle to consumer, and make sure internal validation strictly.

---

If you are unsatisfied with above, then please help me to clear some problems about your issue comment, and I would amend:

Problem 1

if err := validate(content); err != nil {
  for _, warning := range Warnings(err) {
    // do something with warnings
  }
}

if err := validate(content); err != nil && IgnoreWarnings(err) {
   // handle hard errors
}

validate is the API function, return only one common error. Next handlers are done by consumer, that all are decoupled to validation API. So the validation API is designed as a plain function:

func validate(content) error{
  err:=validMediatypeMap(content);
  err=gojsonschema.Validate()
  ...
  return err
}

It seems a weak function.

Problem 2

And for this code line: for _, warning := range Warnings(err): how consumer to get error list from one error?

Problem 3

type Warnings {
  Warnings() []error
}

type Errors {
  Errors() []error
}

If validation API just need validate problems and return 1 error, why it need define interface?

[stevvooe]

I'm going to close this PR in favor of discussing design on #686.