-
Notifications
You must be signed in to change notification settings - Fork 241
Description
Context
Smithy currently allows you to constrain type definitions and structure members based on patterns, length, range, etc via built-in traits. These constraint traits are particularly useful because they are used by code generators to implement input (and possibly output) validation. Input validation on the server side is of particular interest because it is used as a means of verifying that untrusted inputs are, in fact, safe to use.
The only catch with these constraint traits is the consequences of modifying them may not be easily understood in some scenarios -- for example, regular expressions as used by the @pattern trait are notoriously difficult to understand when non-trivial. In order to guarantee that the implementation allows the intended values while disallowing unintended values, service builders must currently define positive and negative test cases and apply them to the final implementation manually.
Proposal
We should make available and implement a @testVectors trait which accepts a list of allowed and disallowed values on a per shape and/or per member basis for which constraint traits can be applied to. The list of allowed and disallowed values will be checked against the configured constraints at build time and discrepancies will cause validation errors to occur.
For example, a valid usage may look like this:
@length(min: 36, max: 36)
@pattern("^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$")
@testVectors({
allowed: [
"00000000-0000-0000-0000-000000000000"
"12345678-90ab-cdef-1234-567890abcdef"
"ffffffff-ffff-ffff-ffff-ffffffffffff"
]
disallowed: [
""
"0-0-0-0-0"
"A0000000-0000-0000-0000-000000000000"
"FFFFFFFF-FFFF-FFFF-FFFF-FFFFFFFFFFFF"
]
})
string Uuid
This new trait helps model authors 1/ express their intent for what values MUST and MUST NOT look like, 2/ have a mechanism to ensure that emergent "problem" values can be imbued into the interface definition, and 3/ understand the consequences of constraint changes. Similarly, it helps model consumers understand the intent from the model author and have concrete understanding of the allowed and disallowed value.
This proposal does not seek to eliminate the ultimate need of service builders to verify that their implementation matches their definition; however, it may be useful from a test automation standpoint as these test vectors could be used to auto-generate per-field tests which validate their implementation's behavior against the definition.
Changes Required
- A new, public
testVectorstrait added to the prelude and private list type for the allowed and disallowed members list. - A new Validator implementation to verify the configured test vectors align with the constraint traits
NodeValidationVisitoralready implements a majority of the validation logic needed
See main...brandondahler:smithy:features/TestVectors for a proof of concept of the changes above.
Further commentary
In principle this could be implemented as a separate component or a third party library; however, given the current constraint traits are defined in the core smith-model package, I think it follows that this would also belong there.
I'm willing to continue on the linked proof of concept if there is high-level alignment on the feature as proposed.