Skip to content

Commit a1ddd86

Browse files
committed
feat: improve code quality
1 parent 84c09e5 commit a1ddd86

File tree

16 files changed

+180
-182
lines changed

16 files changed

+180
-182
lines changed

.env.example

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
# debug
22
APP_DEBUG_ENABLED=false
33

4-
# githup app (required)
4+
# github app (required)
55
APP_GITHUB_APP_ID=123456
66
APP_GITHUB_APP_PRIVATE_KEY_PATH=./.local/private-key.pem
77
APP_GITHUB_INSTALLATION_ID=987654
88
APP_GITHUB_ORG=cruxstack
99
APP_GITHUB_WEBHOOK_SECRET=your-webhook-secret-here
1010

11-
# githu pr compliance (optional)
11+
# github pr compliance (optional)
1212
APP_PR_COMPLIANCE_ENABLED=true
1313
APP_PR_MONITORED_BRANCHES=main,master
1414

@@ -33,3 +33,6 @@ APP_SLACK_CHANNEL=C01234ABCDE
3333

3434
# api gateway base path (optional, for lambda deployments with stage prefix)
3535
# APP_BASE_PATH=v1
36+
37+
# server port (optional, for cmd/server only, default: 8080)
38+
# APP_PORT=8080

cmd/server/main.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func main() {
3939
mux := http.NewServeMux()
4040
mux.HandleFunc("/", httpHandler)
4141

42-
port := os.Getenv("PORT")
42+
port := os.Getenv("APP_PORT")
4343
if port == "" {
4444
port = "8080"
4545
}

internal/app/app.go

Lines changed: 24 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@ import (
1010
"github.com/cockroachdb/errors"
1111
"github.com/cruxstack/github-ops-app/internal/config"
1212
internalerrors "github.com/cruxstack/github-ops-app/internal/errors"
13-
"github.com/cruxstack/github-ops-app/internal/github"
13+
"github.com/cruxstack/github-ops-app/internal/github/client"
14+
"github.com/cruxstack/github-ops-app/internal/github/webhooks"
1415
"github.com/cruxstack/github-ops-app/internal/notifiers"
1516
"github.com/cruxstack/github-ops-app/internal/okta"
1617
gh "github.com/google/go-github/v79/github"
@@ -21,7 +22,7 @@ import (
2122
type App struct {
2223
Config *config.Config
2324
Logger *slog.Logger
24-
GitHubClient *github.Client
25+
GitHubClient *client.Client
2526
OktaClient *okta.Client
2627
Notifier *notifiers.SlackNotifier
2728
}
@@ -37,9 +38,9 @@ func New(ctx context.Context, cfg *config.Config) (*App, error) {
3738
}
3839

3940
if cfg.IsGitHubConfigured() {
40-
ghClient, err := github.NewAppClientWithBaseURL(
41+
ghClient, err := client.NewAppClientWithBaseURL(
4142
cfg.GitHubAppID,
42-
cfg.GitHubInstallID,
43+
cfg.GitHubInstallationID,
4344
cfg.GitHubAppPrivateKey,
4445
cfg.GitHubOrg,
4546
cfg.GitHubBaseURL,
@@ -174,7 +175,7 @@ func (a *App) handleOktaSync(ctx context.Context) error {
174175
// handlePullRequestWebhook processes GitHub pull request webhook events.
175176
// checks merged PRs for branch protection compliance violations.
176177
func (a *App) handlePullRequestWebhook(ctx context.Context, payload []byte) error {
177-
prEvent, err := github.ParsePullRequestEvent(payload)
178+
prEvent, err := webhooks.ParsePullRequestEvent(payload)
178179
if err != nil {
179180
return err
180181
}
@@ -196,8 +197,8 @@ func (a *App) handlePullRequestWebhook(ctx context.Context, payload []byte) erro
196197

197198
ghClient := a.GitHubClient
198199

199-
if prEvent.GetInstallationID() != 0 && prEvent.GetInstallationID() != a.Config.GitHubInstallID {
200-
installClient, err := github.NewAppClientWithBaseURL(
200+
if prEvent.GetInstallationID() != 0 && prEvent.GetInstallationID() != a.Config.GitHubInstallationID {
201+
installClient, err := client.NewAppClientWithBaseURL(
201202
a.Config.GitHubAppID,
202203
prEvent.GetInstallationID(),
203204
a.Config.GitHubAppPrivateKey,
@@ -243,7 +244,7 @@ func (a *App) handlePullRequestWebhook(ctx context.Context, payload []byte) erro
243244
// handleTeamWebhook processes GitHub team webhook events.
244245
// triggers Okta sync when team changes are made externally.
245246
func (a *App) handleTeamWebhook(ctx context.Context, payload []byte) error {
246-
teamEvent, err := github.ParseTeamEvent(payload)
247+
teamEvent, err := webhooks.ParseTeamEvent(payload)
247248
if err != nil {
248249
return err
249250
}
@@ -255,7 +256,7 @@ func (a *App) handleTeamWebhook(ctx context.Context, payload []byte) error {
255256
return nil
256257
}
257258

258-
if a.shouldIgnoreTeamChange(ctx, teamEvent) {
259+
if a.shouldIgnoreWebhookChange(ctx, teamEvent) {
259260
if a.Config.DebugEnabled {
260261
a.Logger.Debug("ignoring team change from bot/app",
261262
slog.String("action", teamEvent.Action),
@@ -275,7 +276,7 @@ func (a *App) handleTeamWebhook(ctx context.Context, payload []byte) error {
275276
// handleMembershipWebhook processes GitHub membership webhook events.
276277
// triggers Okta sync when team memberships are changed externally.
277278
func (a *App) handleMembershipWebhook(ctx context.Context, payload []byte) error {
278-
membershipEvent, err := github.ParseMembershipEvent(payload)
279+
membershipEvent, err := webhooks.ParseMembershipEvent(payload)
279280
if err != nil {
280281
return err
281282
}
@@ -294,7 +295,7 @@ func (a *App) handleMembershipWebhook(ctx context.Context, payload []byte) error
294295
return nil
295296
}
296297

297-
if a.shouldIgnoreMembershipChange(ctx, membershipEvent) {
298+
if a.shouldIgnoreWebhookChange(ctx, membershipEvent) {
298299
if a.Config.DebugEnabled {
299300
a.Logger.Debug("ignoring membership change from bot/app",
300301
slog.String("action", membershipEvent.Action),
@@ -312,35 +313,16 @@ func (a *App) handleMembershipWebhook(ctx context.Context, payload []byte) error
312313
return a.handleOktaSync(ctx)
313314
}
314315

315-
// shouldIgnoreTeamChange checks if a team webhook should be ignored.
316-
// ignores changes made by bots or the GitHub App itself to prevent loops.
317-
func (a *App) shouldIgnoreTeamChange(ctx context.Context, event *github.TeamEvent) bool {
318-
senderType := event.GetSenderType()
319-
if senderType == "Bot" {
320-
return true
321-
}
322-
323-
if a.GitHubClient != nil {
324-
appSlug, err := a.GitHubClient.GetAppSlug(ctx)
325-
if err != nil {
326-
a.Logger.Warn("failed to get app slug", slog.String("error", err.Error()))
327-
return false
328-
}
329-
senderLogin := event.GetSenderLogin()
330-
if senderLogin == appSlug+"[bot]" {
331-
return true
332-
}
333-
}
334-
335-
return false
316+
// webhookSender provides sender information for webhook events.
317+
type webhookSender interface {
318+
GetSenderType() string
319+
GetSenderLogin() string
336320
}
337321

338-
// shouldIgnoreMembershipChange checks if a membership webhook should be
339-
// ignored. ignores changes made by bots or the GitHub App itself to prevent
340-
// loops.
341-
func (a *App) shouldIgnoreMembershipChange(ctx context.Context, event *github.MembershipEvent) bool {
342-
senderType := event.GetSenderType()
343-
if senderType == "Bot" {
322+
// shouldIgnoreWebhookChange checks if a webhook should be ignored.
323+
// ignores changes made by bots or the GitHub App itself to prevent loops.
324+
func (a *App) shouldIgnoreWebhookChange(ctx context.Context, event webhookSender) bool {
325+
if event.GetSenderType() == "Bot" {
344326
return true
345327
}
346328

@@ -350,8 +332,7 @@ func (a *App) shouldIgnoreMembershipChange(ctx context.Context, event *github.Me
350332
a.Logger.Warn("failed to get app slug", slog.String("error", err.Error()))
351333
return false
352334
}
353-
senderLogin := event.GetSenderLogin()
354-
if senderLogin == appSlug+"[bot]" {
335+
if event.GetSenderLogin() == appSlug+"[bot]" {
355336
return true
356337
}
357338
}
@@ -388,13 +369,13 @@ func (a *App) handleSlackTest(ctx context.Context) error {
388369
}
389370

390371
// fakePRComplianceResult returns sample PR compliance data for testing.
391-
func fakePRComplianceResult() *github.PRComplianceResult {
372+
func fakePRComplianceResult() *client.PRComplianceResult {
392373
prNumber := 42
393374
prTitle := "Add new authentication feature"
394375
prURL := "https://github.com/acme-corp/demo-repo/pull/42"
395376
mergedByLogin := "test-user"
396377

397-
return &github.PRComplianceResult{
378+
return &client.PRComplianceResult{
398379
PR: &gh.PullRequest{
399380
Number: &prNumber,
400381
Title: &prTitle,
@@ -406,7 +387,7 @@ func fakePRComplianceResult() *github.PRComplianceResult {
406387
BaseBranch: "main",
407388
UserHasBypass: true,
408389
UserBypassReason: "repository admin",
409-
Violations: []github.ComplianceViolation{
390+
Violations: []client.ComplianceViolation{
410391
{Type: "insufficient_reviews", Description: "required 2 approving reviews, had 0"},
411392
{Type: "missing_status_check", Description: "required check 'ci/build' did not pass"},
412393
},

internal/app/app_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import (
77
"testing"
88

99
"github.com/cruxstack/github-ops-app/internal/config"
10-
"github.com/cruxstack/github-ops-app/internal/github"
10+
"github.com/cruxstack/github-ops-app/internal/github/client"
1111
"github.com/cruxstack/github-ops-app/internal/okta"
1212
)
1313

@@ -161,7 +161,7 @@ func TestProcessScheduledEvent_UnknownAction(t *testing.T) {
161161
// verify fake data types match expected interfaces
162162
func TestFakeDataTypes(t *testing.T) {
163163
// ensure fake PR result is compatible with notifier
164-
var _ *github.PRComplianceResult = fakePRComplianceResult()
164+
var _ *client.PRComplianceResult = fakePRComplianceResult()
165165

166166
// ensure fake sync reports are compatible with notifier
167167
var _ []*okta.SyncReport = fakeOktaSyncReports()

internal/app/request.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
// Package app provides unified request/response handling for all runtimes.
21
package app
32

43
import (
@@ -7,7 +6,7 @@ import (
76
"log/slog"
87
"strings"
98

10-
"github.com/cruxstack/github-ops-app/internal/github"
9+
"github.com/cruxstack/github-ops-app/internal/github/webhooks"
1110
)
1211

1312
// RequestType identifies the category of incoming request.
@@ -133,7 +132,7 @@ func (a *App) handleWebhookRequest(ctx context.Context, req Request) Response {
133132
eventType := req.Headers["x-github-event"]
134133
signature := req.Headers["x-hub-signature-256"]
135134

136-
if err := github.ValidateWebhookSignature(
135+
if err := webhooks.ValidateWebhookSignature(
137136
req.Body,
138137
signature,
139138
a.Config.GitHubWebhookSecret,

0 commit comments

Comments
 (0)