Skip to content
This repository was archived by the owner on May 21, 2024. It is now read-only.

Add support for GCR #5

Merged
merged 3 commits into from
Oct 24, 2020
Merged

Conversation

benjlevesque
Copy link
Contributor

This PR adds a new command pubsub to receive Google Cloud Registry events.

To support both HTTP requests and Pubsub events, PushEventParser have been modified to accept []byte instead of http.Request.

The HTTP request Body now being parsed in the handler, tests related to the parsing of an invalid request have been moved to the handler package.


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.

@benjlevesque benjlevesque changed the title Add GCR pubsub Add support for GCR Oct 21, 2020
@bigkevmcd
Copy link
Collaborator

@benjlevesque Thanks for the PR, I'll take a look, and would definitely like to get this landed.

Copy link
Collaborator

@bigkevmcd bigkevmcd left a 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()
Copy link
Collaborator

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?

return b
}

type failingReader struct {
Copy link
Collaborator

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

Copy link
Contributor Author

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?

Copy link
Collaborator

@bigkevmcd bigkevmcd left a 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.

@bigkevmcd bigkevmcd merged commit ea82051 into gitops-tools:main Oct 24, 2020
@benjlevesque benjlevesque deleted the gcr-pubsub branch October 26, 2020 11:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants