Skip to content
Closed
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
60 changes: 60 additions & 0 deletions pkg/behaviourtest/drivers/install/ensure.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,14 @@ func (e *repoEnsurer) doEnsure(ctx context.Context, org, repoName string) error
return err
}

// Step 2b: wait for the App installation's permission graph to
// settle for the new repo ID. GitHub's permission consistency is
// a separate domain from repo visibility; dispatch's
// GetCollaboratorPermission can 404 until propagation completes.
if err := e.awaitPermissionPropagation(ctx, org, repoName, target); err != nil {
return err
}

// Step 3: the repo is always freshly created (step 1 deleted any
// prior version), so fullsend is never pre-installed. Run the full
// install flow and settle for Actions readiness.
Expand Down Expand Up @@ -317,6 +325,58 @@ func (e *repoEnsurer) awaitCreation(ctx context.Context, org, repoName, target s
return fmt.Errorf("repo %s not visible after %d attempts following creation", target, resetMaxAttempts)
}

// awaitPermissionPropagation polls GetCollaboratorPermission with
// exponential backoff until the authenticated user's permission is
// visible on the newly created repo. After a repo is deleted and
// recreated, the GitHub App installation's permission graph for the
// new repo ID is a separate consistency domain from repo visibility.
// Without this wait, dispatch's collaborator-permission lookup can
// 404, causing the actor to be treated as unauthorized (RoleNone)
// and the harness matrix to be empty.
//
// When the forge client does not implement GitHubExtensions (e.g., a
// GitLab backend), the check is skipped — permission propagation is a
// GitHub-specific concern.
func (e *repoEnsurer) awaitPermissionPropagation(ctx context.Context, org, repoName, target string) error {
gh, ok := e.client.(forge.GitHubExtensions)
if !ok {
e.logf("[ensure] %s: client does not support permission checks, skipping", target)
return nil
}

login, err := e.client.GetAuthenticatedUser(ctx)
if err != nil {
return fmt.Errorf("resolving bot login for permission wait on %s: %w", target, err)
}

e.logf("[ensure] waiting for %s permission to propagate for %s", target, login)
delay := resetRetryDelay
for attempt := 1; attempt <= resetMaxAttempts; attempt++ {
if err := ctx.Err(); err != nil {
return fmt.Errorf("context cancelled while waiting for %s permission: %w", target, err)
}
role, permErr := gh.GetCollaboratorPermission(ctx, org, repoName, login)
if permErr == nil {
e.logf("[ensure] %s permission confirmed for %s: role=%s (attempt %d)", target, login, role, attempt)
return nil
}
if !forge.IsNotFound(permErr) {
return fmt.Errorf("checking permission on %s for %s: %w", target, login, permErr)
}
if attempt < resetMaxAttempts {
e.logf("[ensure] %s permission not yet visible for %s, attempt %d/%d — backing off %v",
target, login, attempt, resetMaxAttempts, delay)
select {
case <-ctx.Done():
return fmt.Errorf("context cancelled while waiting for %s permission: %w", target, ctx.Err())
case <-time.After(delay):
}
delay *= 2
}
}
return fmt.Errorf("permission for %s on %s not visible after %d attempts", login, target, resetMaxAttempts)
}

// installFullsend runs inference provision (when a GCP project is
// configured) and fullsend github setup for the target repo.
func (e *repoEnsurer) installFullsend(_ context.Context, _, _, target string) error {
Expand Down
166 changes: 166 additions & 0 deletions pkg/behaviourtest/drivers/install/ensure_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,14 @@ type stubClient struct {
// When nil and installed is true, GetWorkflow returns a valid Workflow.
getWorkflowErr error
getWorkflowCalled atomic.Int32

// authenticatedUser is the login returned by GetAuthenticatedUser.
// Defaults to "test-bot[bot]" when empty.
authenticatedUser string

// collaboratorRole is the role returned by GetCollaboratorPermission.
// Defaults to "admin" when empty (immediate success).
collaboratorRole string
}

func (s *stubClient) GetRepo(_ context.Context, _, repo string) (*forge.Repository, error) {
Expand Down Expand Up @@ -202,6 +210,32 @@ func (s *stubClient) GetWorkflow(_ context.Context, _, _, _ string) (*forge.Work
return &forge.Workflow{ID: 1, Name: "fullsend", Path: ".github/workflows/fullsend.yaml", State: "active"}, nil
}

func (s *stubClient) GetAuthenticatedUser(_ context.Context) (string, error) {
if s.authenticatedUser != "" {
return s.authenticatedUser, nil
}
return "test-bot[bot]", nil
}

// GitHubExtensions methods — needed so the type assertion in
// awaitPermissionPropagation succeeds for stubClient.

func (s *stubClient) ListOrgInstallations(_ context.Context, _ string) ([]forge.Installation, error) {
return nil, nil
}

func (s *stubClient) GetAppClientID(_ context.Context, _ string) (string, error) {
return "", forge.ErrNotFound
}

func (s *stubClient) GetCollaboratorPermission(_ context.Context, _, _, _ string) (string, error) {
role := s.collaboratorRole
if role == "" {
role = "admin"
}
return role, nil
}

func TestNewRepoEnsurer_ReturnsNonNil(t *testing.T) {
sc := &stubClient{}
e := newRepoEnsurer(e2etest.EnvConfig{}, sc, "tok", "/bin/true", t.Logf)
Expand Down Expand Up @@ -1017,3 +1051,135 @@ func TestAwaitCreation_ContextCancellation(t *testing.T) {
require.Error(t, err)
assert.Contains(t, err.Error(), "context cancelled")
}

// --- awaitPermissionPropagation unit tests ---

// countingPermissionClient returns different GetCollaboratorPermission
// results after a configurable number of calls. It also implements
// GitHubExtensions so the type assertion in awaitPermissionPropagation
// succeeds.
type countingPermissionClient struct {
forge.Client
mu sync.Mutex
permCalls int
switchAfter int // after this many calls, switch to switchedRole
initialErr error // returned for calls 1..switchAfter
switchedRole string // returned for calls switchAfter+1..
switchedErr error // returned for calls switchAfter+1..
authenticatedAs string // login returned by GetAuthenticatedUser
}

func (c *countingPermissionClient) GetAuthenticatedUser(_ context.Context) (string, error) {
if c.authenticatedAs != "" {
return c.authenticatedAs, nil
}
return "test-bot[bot]", nil
}

func (c *countingPermissionClient) ListOrgInstallations(_ context.Context, _ string) ([]forge.Installation, error) {
return nil, nil
}

func (c *countingPermissionClient) GetAppClientID(_ context.Context, _ string) (string, error) {
return "", forge.ErrNotFound
}

func (c *countingPermissionClient) GetCollaboratorPermission(_ context.Context, _, _, _ string) (string, error) {
c.mu.Lock()
defer c.mu.Unlock()
c.permCalls++
if c.permCalls > c.switchAfter {
return c.switchedRole, c.switchedErr
}
return "", c.initialErr
}

func TestAwaitPermissionPropagation_ImmediateSuccess(t *testing.T) {
speedUpResetRetries(t)
client := &countingPermissionClient{
switchAfter: 0,
switchedRole: "write",
}
e := &repoEnsurer{client: client, logf: t.Logf}

err := e.awaitPermissionPropagation(context.Background(), "org", "repo", "org/repo")
require.NoError(t, err)
assert.Equal(t, 1, client.permCalls, "should confirm on first poll")
}

func TestAwaitPermissionPropagation_RetriesUntilConfirmed(t *testing.T) {
speedUpResetRetries(t)
client := &countingPermissionClient{
switchAfter: 3,
initialErr: forge.ErrNotFound, // 404 for 3 calls
switchedRole: "write", // then found
}
e := &repoEnsurer{client: client, logf: t.Logf}

err := e.awaitPermissionPropagation(context.Background(), "org", "repo", "org/repo")
require.NoError(t, err)
assert.Equal(t, 4, client.permCalls, "should retry until permission visible")
}

func TestAwaitPermissionPropagation_FailsAfterMaxAttempts(t *testing.T) {
speedUpResetRetries(t)
client := &countingPermissionClient{
switchAfter: resetMaxAttempts + 1, // never switches
initialErr: forge.ErrNotFound, // always 404
}
e := &repoEnsurer{client: client, logf: t.Logf}

err := e.awaitPermissionPropagation(context.Background(), "org", "repo", "org/repo")
require.Error(t, err, "should error when permission never becomes visible")
assert.Contains(t, err.Error(), "not visible after")
assert.Contains(t, err.Error(), "test-bot[bot]")
}

func TestAwaitPermissionPropagation_PropagatesNonNotFoundError(t *testing.T) {
speedUpResetRetries(t)
client := &countingPermissionClient{
switchAfter: 0,
initialErr: assert.AnError,
switchedErr: assert.AnError,
}
e := &repoEnsurer{client: client, logf: t.Logf}

err := e.awaitPermissionPropagation(context.Background(), "org", "repo", "org/repo")
require.Error(t, err)
assert.Contains(t, err.Error(), "checking permission")
}

func TestAwaitPermissionPropagation_ContextCancellation(t *testing.T) {
speedUpResetRetries(t)
client := &countingPermissionClient{
switchAfter: resetMaxAttempts + 1,
initialErr: forge.ErrNotFound,
}
e := &repoEnsurer{client: client, logf: t.Logf}

ctx, cancel := context.WithCancel(context.Background())
cancel() // cancel immediately

err := e.awaitPermissionPropagation(ctx, "org", "repo", "org/repo")
require.Error(t, err)
assert.Contains(t, err.Error(), "context cancelled")
}

// noGHExtClient satisfies forge.Client but does NOT implement
// GitHubExtensions. Used to verify awaitPermissionPropagation skips
// the check for non-GitHub forges.
type noGHExtClient struct {
forge.Client
}

func (c *noGHExtClient) GetAuthenticatedUser(_ context.Context) (string, error) {
return "bot", nil
}

func TestAwaitPermissionPropagation_SkipsWhenNoGitHubExtensions(t *testing.T) {
client := &noGHExtClient{}
e := &repoEnsurer{client: client, logf: t.Logf}

err := e.awaitPermissionPropagation(context.Background(), "org", "repo", "org/repo")
require.NoError(t, err, "should skip gracefully when client lacks GitHubExtensions")
}
Loading