-
Notifications
You must be signed in to change notification settings - Fork 9
Add support for GCR #5
Conversation
|
@benjlevesque Thanks for the PR, I'll take a look, and would definitely like to get this landed. |
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.
Looks really nice, couple of minor issues but otherwise, I'd be really happy for this go go in.
| Use: "pubsub", | ||
| Short: "update repositories in response to gcr pubsub events", | ||
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| zapl, _ := zap.NewProduction() |
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 wonder if some of this code which is duplicated from the http command could be extracted and reused?
pkg/hooks/gcr/hook_test.go
Outdated
| return b | ||
| } | ||
|
|
||
| type failingReader struct { |
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'd probably be inclined to move this to a file in https://github.com/gitops-tools/image-updater/tree/main/test (it'll need to become FailingReader
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 actually forgot to remove it, it is only used in handler_test now, is it fine to leave it there?
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 don't have an easy way to test the GCR part of this, but it looks good to me.
This PR adds a new command
pubsubto receive Google Cloud Registry events.To support both HTTP requests and Pubsub events,
PushEventParserhave been modified to accept[]byteinstead ofhttp.Request.The HTTP request Body now being parsed in the
handler, tests related to the parsing of an invalid request have been moved to thehandlerpackage.This project is exactly what I have been looking for to manage image updates, thanks!
Also, I'm pretty new to go, so feel free to suggest changes / refactorings.