-
Notifications
You must be signed in to change notification settings - Fork 43
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 REST datasource driver #5009
Conversation
fe4cde6
to
852f8ba
Compare
fbb974b
to
f508ead
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the common schema validation, I'm not sure if this is done yet (I saw more changes sneak in while reviewing)
@@ -19,18 +19,26 @@ import ( | |||
// ValidateSchemaUpdate validates that the new json schema doesn't break | |||
// profiles using this rule type | |||
func ValidateSchemaUpdate(oldRuleSchema *structpb.Struct, newRuleSchema *structpb.Struct) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the lovely new functions in schemavalidate for conversion, would it make sense to file a cleanup ticket to make these take a pair of jsonschema.Schema
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would make sense
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for _, desc := range errs { | ||
problems = append(problems, desc.Error()) | ||
} | ||
return fmt.Errorf("invalid json schema: %s", strings.TrimSpace(strings.Join(problems, "\n"))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want to use errors.Join(...error)
here (or implement Unwrap() []error
upstream) to allow callers to pick apart the errors?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. Join would make sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll leave this as a TODO for another PR since this was moved from existing code.
329d34d
to
fc6d9ae
Compare
The security scan failure is a false positive. I don't even know where it's getting that file from 😕 |
We're hitting aquasecurity/trivy#7970 |
This adds an initial implementation of the REST datasource driver. This allows the user to register multiple REST endpoints to call a data soruce and thus interact with it. This initial implementation left several TODO items which need to be addressed before unleashing the implementation. Signed-off-by: Juan Antonio Osorio <[email protected]>
Signed-off-by: Juan Antonio Osorio <[email protected]>
This is instead handled by the registry. Signed-off-by: Juan Antonio Osorio <[email protected]>
Signed-off-by: Juan Antonio Osorio <[email protected]>
Signed-off-by: Juan Antonio Osorio <[email protected]>
Signed-off-by: Juan Antonio Osorio <[email protected]>
Signed-off-by: Juan Antonio Osorio <[email protected]>
… rest driver Signed-off-by: Juan Antonio Osorio <[email protected]>
fc6d9ae
to
549f6f4
Compare
Summary
This adds an initial implementation of the REST datasource driver. This
allows the user to register multiple REST endpoints to call a data
soruce and thus interact with it.
This initial implementation left several TODO items which need to be
addressed before unleashing the implementation.
Change Type
Mark the type of change your PR introduces:
Testing
Outline how the changes were tested, including steps to reproduce and any relevant configurations.
Attach screenshots if helpful.
Review Checklist: