Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

refactor(api): extract Syncer.SyncRepo for per-repo sync triggers #1315

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions internal/api/registry/registry.go
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"log/slog"
"regexp"
"sort"
"strings"
"sync"
"time"

Expand Down Expand Up @@ -225,6 +226,25 @@ func (r *Registry) Get(id string) (*RepoEntry, bool) {
return e, ok
}

// FindManaged returns the managed entry whose GitHub owner/name match the
// arguments case-insensitively, or false when none does. The interval sync
// loop and the webhook receiver use it to map a remote-change signal (a push
// for owner/name) onto the local checkout to refresh.
//
// Only managed mirrors are considered: the unmanaged -cwd working repo must
// never be selected here, because the sync path mirror-fetches into a detached
// HEAD and would corrupt a real working tree (see safety-invariants.md).
func (r *Registry) FindManaged(owner, name string) (*RepoEntry, bool) {
r.mu.RLock()
defer r.mu.RUnlock()
for _, e := range r.entries {
if e.Managed && strings.EqualFold(e.Owner, owner) && strings.EqualFold(e.Name, name) {
return e, true
}
}
return nil, false
}

// List returns every entry sorted by ID so callers (and tests) get a stable
// ordering for serialization.
func (r *Registry) List() []*RepoEntry {
Expand Down
26 changes: 26 additions & 0 deletions internal/api/registry/registry_test.go
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,32 @@ func TestRegistry_ListReturnsSortedByID(t *testing.T) {
require.Equal(t, "c", got[2].ID)
}

func TestRegistry_FindManaged(t *testing.T) {
t.Parallel()
r := New()
require.NoError(t, r.Add(&RepoEntry{ID: "managed", Managed: true, Owner: "Octo", Name: "Widget"}))
require.NoError(t, r.Add(&RepoEntry{ID: "unmanaged", Owner: "octo", Name: "dev"}))

t.Run("matches owner/name case-insensitively", func(t *testing.T) {
t.Parallel()
e, ok := r.FindManaged("octo", "widget")
require.True(t, ok)
require.Equal(t, "managed", e.ID)
})

t.Run("skips unmanaged entries", func(t *testing.T) {
t.Parallel()
_, ok := r.FindManaged("octo", "dev")
require.False(t, ok, "the unmanaged -cwd repo must never be selected for mirror-fetch")
})

t.Run("returns false when no repo matches", func(t *testing.T) {
t.Parallel()
_, ok := r.FindManaged("octo", "missing")
require.False(t, ok)
})
}

func TestRegistry_Len(t *testing.T) {
t.Parallel()
r := New()
Expand Down
34 changes: 28 additions & 6 deletions internal/api/reposync/syncer.go
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ package reposync

import (
"context"
"errors"
"fmt"
"log/slog"
"time"

Expand All @@ -14,6 +16,11 @@ import (
"github.com/getstackit/stackit/internal/utils"
)

// ErrRepoNotManaged is returned by SyncRepo when no managed registry entry
// matches the requested owner/name — e.g. a webhook arrives for a repo that
// was never onboarded, or for the unmanaged -cwd dev repo.
var ErrRepoNotManaged = errors.New("reposync: repo not managed")

// defaultWorkers bounds concurrent fetches per pass. Fetches are network-bound,
// so a small pool is gentle on the remote and the file-descriptor table.
const defaultWorkers = 4
Expand Down Expand Up @@ -82,23 +89,38 @@ func (s *Syncer) syncOnce(ctx context.Context) {
if ctx.Err() != nil {
return
}
s.syncEntry(ctx, e)
if err := s.syncEntry(ctx, e); err != nil {
slog.Warn("sync: entry failed", "repo", e.ID, "error", err)
}
})
}

func (s *Syncer) syncEntry(ctx context.Context, e *registry.RepoEntry) {
// SyncRepo mirror-fetches and refreshes the single managed entry matching
// owner/name. It is the per-repo unit of work shared by the interval loop and
// the webhook receiver: the loop calls syncEntry directly over the entries it
// already holds, while the webhook path arrives with only GitHub coordinates
// and resolves the entry here. Returns ErrRepoNotManaged when no managed repo
// matches, so a webhook for an un-onboarded repo is a clean no-op.
func (s *Syncer) SyncRepo(ctx context.Context, owner, name string) error {
e, ok := s.reg.FindManaged(owner, name)
if !ok {
return fmt.Errorf("%w: %s/%s", ErrRepoNotManaged, owner, name)
}
return s.syncEntry(ctx, e)
}

func (s *Syncer) syncEntry(ctx context.Context, e *registry.RepoEntry) error {
token := ""
if s.provider != nil {
t, err := s.provider.InstallationToken(ctx, e.Owner, e.Name)
if err != nil {
slog.Warn("sync: installation token unavailable; skipping", "repo", e.ID, "error", err)
return
return fmt.Errorf("installation token: %w", err)
}
token = t
}
if err := s.fetch(ctx, e.RepoRoot, e.Remote, token); err != nil {
slog.Warn("sync: fetch failed", "repo", e.ID, "error", err)
return
return fmt.Errorf("fetch: %w", err)
}
s.refresh(e)
return nil
}
48 changes: 48 additions & 0 deletions internal/api/reposync/syncer_test.go
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,54 @@ func TestSyncEntryNilProviderUsesEmptyToken(t *testing.T) {
require.Equal(t, "", gotToken, "no provider means an unauthenticated (public) fetch")
}

func TestSyncRepoFetchesMatchingManagedEntry(t *testing.T) {
t.Parallel()
reg := registry.New()
require.NoError(t, reg.Add(managedEntry("m", "Octo", "Widget", "/r/m")))

var gotToken string
refreshed := false
s := New(reg, fakeProvider{token: "inst-token"}, time.Minute)
s.fetch = func(_ context.Context, _, _, token string) error { gotToken = token; return nil }
s.refresh = func(*registry.RepoEntry) { refreshed = true }

// Casing differs from the entry to prove the lookup is case-insensitive,
// matching how a webhook payload may present the coordinates.
require.NoError(t, s.SyncRepo(context.Background(), "octo", "widget"))
require.Equal(t, "inst-token", gotToken)
require.True(t, refreshed)
}

func TestSyncRepoUnknownRepoReturnsErrNotManaged(t *testing.T) {
t.Parallel()
reg := registry.New()
require.NoError(t, reg.Add(managedEntry("m", "o", "n", "/r")))

fetchCalled := false
s := New(reg, fakeProvider{token: "t"}, time.Minute)
s.fetch = func(context.Context, string, string, string) error { fetchCalled = true; return nil }
s.refresh = func(*registry.RepoEntry) {}

err := s.SyncRepo(context.Background(), "other", "repo")
require.ErrorIs(t, err, ErrRepoNotManaged)
require.False(t, fetchCalled, "an un-onboarded repo must not trigger a fetch")
}

func TestSyncRepoPropagatesFetchError(t *testing.T) {
t.Parallel()
reg := registry.New()
require.NoError(t, reg.Add(managedEntry("m", "o", "n", "/r")))

refreshed := false
s := New(reg, fakeProvider{token: "t"}, time.Minute)
s.fetch = func(context.Context, string, string, string) error { return errors.New("boom") }
s.refresh = func(*registry.RepoEntry) { refreshed = true }

err := s.SyncRepo(context.Background(), "o", "n")
require.Error(t, err)
require.False(t, refreshed, "a failed fetch must not refresh stale state")
}

func TestRunStopsOnContextCancel(t *testing.T) {
t.Parallel()
s := New(registry.New(), nil, time.Hour)
Expand Down
Loading

AltStyle によって変換されたページ (->オリジナル) /