-
Notifications
You must be signed in to change notification settings - Fork 8
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
Initial webhook source setup #271
Conversation
pkg/materialize/source_webhook.go
Outdated
size string // TODO: size is not supported for webhook sources | ||
bodyFormat string | ||
includeHeaders bool | ||
checkOptions []string |
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.
Should this be a custom struct like
type checkOptions struct {
field string
alias string
}
Then in the resource we can put in validation to ensure they are only using a field of type BODY
, HEADER
, or SECRET
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.
Just updated this, but also realized that it might be a bit more complex than my current implementation as I'll have to add a check for the SECRET as well to make sure that an actual secret is provided
`, sourceName) | ||
} | ||
|
||
func testAccCheckSourceWebhookExists(name string) resource.TestCheckFunc { |
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.
Realizing we could just use the same function for all source acceptance tests rather than defining a custom one for each source type. At least for any source that just uses ScanSource
.
Optional: true, | ||
ForceNew: true, | ||
}, | ||
"size": { |
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.
Based on the TODO above. Size will eventually be supported?
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.
Not 100% sure if size will ever be supported, as far as I can tell the IN CLUSTER
parameter is required. So this is a bit different compared to the rest of the sources that we have. The TODO is mainly for coming up with a way of reimplementing the sourceQuery
in a clean way.
if err := b.Create(); err != nil { | ||
return diag.FromErr(err) | ||
} | ||
|
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.
Since ownership
can be set. Will need a block for the adding ownership.
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! Just added this.
65502a6
to
d5d713c
Compare
d5d713c
to
6570806
Compare
Adding the very initial work for supporting webhook sources.
Things that still need to be ironed out:
checkExpression
checkOptions
size
does not apply to the webhook source, at least judging by the grammar fileCloses #270