feat: refacto github/gitlab clients #413
Conversation
|
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #413 +/- ##
==========================================
- Coverage 51.12% 50.05% -1.08%
==========================================
Files 73 72 -1
Lines 4350 4805 +455
==========================================
+ Hits 2224 2405 +181
- Misses 1937 2197 +260
- Partials 189 203 +14 ☔ View full report in Codecov by Sentry. |
LucasMrqes
force-pushed
the
feat/refacto-git-authentication
branch
from
9900f2e to
fc2a0cb
Compare
There was a problem hiding this comment.
We have across Burrito multiple Provider interfaces that represent Git providers.
I think we should create an universal interface named GitProvider to be used by:
- TerraformPullRequestController (to get and comment PRs)
- Runner (to clone repos)
- Webhook server (to decode events)
This will ease future implementations of Git providers in Burrito and will help also for future Burrito components that interact with Git online services.
| apiVersion: rbac.authorization.k8s.io/v1 | ||
| kind: Role | ||
| metadata: | ||
| name: secret-access |
There was a problem hiding this comment.
Should be a ClusterRole named burrito-server-secret-access
LucasMrqes
force-pushed
the
feat/refacto-git-authentication
branch
from
8229285 to
2bf2f08
Compare
| log.Infof("kubernetes resources successfully retrieved") | ||
| return nil | ||
| } | ||
|
|
||
| func (r *Runner) initGitProvider() error { |
There was a problem hiding this comment.
Awesome, it totally abstracts the provider!
| e.Logger.Fatal(e.Start(s.config.Server.Addr)) | ||
| log.Infof("burrito server started on addr %s", s.config.Server.Addr) | ||
| } | ||
|
|
||
| func handleHealthz(c echo.Context) error { | ||
| return c.String(http.StatusOK, "OK") | ||
| } | ||
|
|
||
| func (s *Server) refreshWebhookHandlers(ctx context.Context) { |
There was a problem hiding this comment.
So this refreshes the webhook configuration every 5 minutes in case of the user has configured a new repository?
There was a problem hiding this comment.
Yes, the timing choice is a bit random, I will make it customizable in the config
internal/webhook/webhook.go
Outdated
| } | ||
|
|
||
| func New(c *config.Config) *Webhook { | ||
| return &Webhook{ | ||
| Config: c, | ||
| Config: c, | ||
| Providers: make(map[string][]gitprovider.Provider), | ||
| } | ||
| } | ||
|
|
||
| type Provider interface { |
There was a problem hiding this comment.
Is this interface still needed?
There was a problem hiding this comment.
This was the old one, good catch. I'm deleting it
Currently, the way burrito connects to a remote repository and to github/gitlab is quite messy.
TerraformRepository.spec.repository.secretName)burrito.controller.githubConfigandburrito.controller.gitlabConfig.burrito.server.webhook.github.secretandburrito.server.webhook.gitlab.secret)This poses several issues:
With this PR:
Finally, in terms of code structure: git clients are now centralized in a single package
internal/utils/gitproviders/. Adding support for a new provider such as bitbucket is easier and less error prone, since in only requires to implement all the functions for authenticating and interacting with the external repository in one spot.