gloo icon indicating copy to clipboard operation
gloo copied to clipboard

Fix Route Sanitization and Validation

Open ianmacclancy opened this issue 2 years ago • 1 comments

While working on https://github.com/solo-io/gloo/issues/6098

It became apparent that our current route validation and sanitization do not work as expected.

This task is to track the work to fix our current code before merging new route errors and validation

Issues Discovered

  • Code errors
    • We never process route errors onto the proxy report - Incorrect Conversion of sanitized errors from the resource report to warnings in the proxy report - the error string used here https://github.com/solo-io/gloo/blob/master/projects/gloo/pkg/validation/server.go#L296 doe not match the string used as a key in the map of warnings
    • Incorrect error output on ProcessingError. Reason: no destination type specified. The formatted error should include the route name and instead just lists a message of "ProcessingError. Reason: no destination type specified. Route Name: " with a missing route name
  • Test Errors - Route Errors and Sanitization are Not Tested by the Server_Test.go file
    • https://github.com/solo-io/gloo/blob/master/projects/gloo/pkg/validation/server_test.go#L118 - this test, which is supposed to test the after effects of route sanitization, passes an xdsSanitizer that has a RouteReplacementSanitizer that has enabled set to false so no sanitization happens for it to test.

Design of Flow Issues

  • We correct the proxy report outside of sanitization when that correction cannot happen without sanitization - rather than reconciling the two reports after sanitization - sanitization could update both
  • We sanitize outside of translation - but might be able to sanitize as part of translation rather than after

Done Criteria

  • K2e test with rejected resources
  • Tests correctly test route errors with enabled sanitizers
  • Proxy Report updated after sanitization
  • Design for restructure of code - we update the proxy report after sanitization when it should happen as part of sanitization and sanitization occurs outside translation

ianmacclancy avatar Jun 14 '22 14:06 ianmacclancy

Initial work on issue - https://github.com/solo-io/gloo/tree/issue-6563

ianmacclancy avatar Jun 14 '22 18:06 ianmacclancy