fix: avoid stale onboarding branches #64

Merged
eding merged 1 commit from fix/stale-registration-branches into main 2026-09-16 20:32:29 +02:00
4 changed files with 55 additions and 12 deletions

View file

@ -43,6 +43,10 @@ func OnboardApp(cfg config.Config, sourceDir string) error {
if sourceBranch != resolved.Delivery.AppRepoRef {
return errors.New("--from branch must match delivery appRepoRef")
}
sourceRevision, err := forgejo.BranchRevision(sourceDir, sourceBranch)
if err != nil {
return err
}
owner, repository, err := forgejo.RepositoryFromURL(resolved.Delivery.AppRepoURL)
if err != nil {
return err
@ -51,7 +55,7 @@ func OnboardApp(cfg config.Config, sourceDir string) error {
if _, err := sourceManager.EnsureRepository(repository, "Application build input for Maidn CI/CD"); err != nil {
return err
}
if err := publishInitialAppBranches(sourceManager, sourceDir, resolved.Delivery.AppRepoURL, sourceBranch, resolved.Delivery.AppRepoRef, resolved.Delivery.ProductionBranch); err != nil {
if err := publishInitialAppBranches(sourceManager, sourceDir, resolved.Delivery.AppRepoURL, sourceRevision, resolved.Delivery.AppRepoRef, resolved.Delivery.ProductionBranch); err != nil {
return err
}
if err := sourceManager.EnsureProtectedBranch(repository, resolved.Delivery.ProductionBranch); err != nil {
@ -60,7 +64,7 @@ func OnboardApp(cfg config.Config, sourceDir string) error {
if err := ensurePlatformBranch(sourceManager, repository, resolved.Delivery.AppRepoURL, resolved.Delivery.AppName); err != nil {
return err
}
registrationBranch := "maidn/register-" + resolved.Delivery.AppName
registrationBranch := registrationBranch(resolved.Delivery.AppName, sourceRevision)
clusterManager := newOnboardingRepoManager(resolved.Git.BaseURL, resolved.Git.Token, resolved.Git.Owner, resolved.Git.Username, "", "", resolved.Flux.Branch, "")
if _, err := clusterManager.PublishRepositoryPullRequest(resolved.Flux.RepoName, "feat: register "+resolved.Delivery.AppName+" delivery", registrationBranch, resolved.Flux.Branch, func(dir string) error {
return registerAppInCluster(dir, resolved)
@ -86,6 +90,10 @@ func OnboardApp(cfg config.Config, sourceDir string) error {
return nil
}
func registrationBranch(appName, revision string) string {
return "maidn/register-" + appName + "-" + revision[:12]
}
func platformBranch(appName string) string {
return "maidn/platform-" + appName
}
@ -106,11 +114,7 @@ func ensurePlatformBranch(manager onboardingRepoManager, repository, repositoryU
}
// publishInitialAppBranches establishes the immutable source baseline before central registration.
func publishInitialAppBranches(manager onboardingRepoManager, sourceDir, targetURL, sourceBranch, targetBranch, productionBranch string) error {
sourceRevision, err := forgejo.BranchRevision(sourceDir, sourceBranch)
if err != nil {
return err
}
func publishInitialAppBranches(manager onboardingRepoManager, sourceDir, targetURL, sourceRevision, targetBranch, productionBranch string) error {
mainRevision, err := manager.RemoteBranchRevision(targetURL, targetBranch)
if err != nil {
return fmt.Errorf("read target base branch: %w", err)

View file

@ -112,7 +112,7 @@ func TestPublishInitialAppBranchesCreatesAndPreservesProduction(t *testing.T) {
onboardingGit(t, "", "init", "--bare", target)
manager := forgejo.NewRepoManager("https://git.example.test", "", "owner", "", "", "", "main", "")
if err := publishInitialAppBranches(manager, source, target, "source", "main", "production"); err != nil {
if err := publishInitialAppBranches(manager, source, target, sourceRevision, "main", "production"); err != nil {
t.Fatal(err)
}
for _, branch := range []string{"main", "production"} {
@ -139,7 +139,7 @@ func TestPublishInitialAppBranchesCreatesAndPreservesProduction(t *testing.T) {
existingProduction := onboardingGit(t, production, "rev-parse", "production")
onboardingGit(t, production, "push", preservedTarget, "production:production")
if err := publishInitialAppBranches(manager, source, preservedTarget, "source", "main", "production"); err != nil {
if err := publishInitialAppBranches(manager, source, preservedTarget, sourceRevision, "main", "production"); err != nil {
t.Fatal(err)
}
if got := onboardingGit(t, "", "--git-dir", preservedTarget, "rev-parse", "refs/heads/production"); got != existingProduction {
@ -149,7 +149,7 @@ func TestPublishInitialAppBranchesCreatesAndPreservesProduction(t *testing.T) {
t.Fatal(err)
}
onboardingGit(t, source, "commit", "-am", "updated source")
if err := publishInitialAppBranches(manager, source, preservedTarget, "source", "main", "production"); err != nil {
if err := publishInitialAppBranches(manager, source, preservedTarget, sourceRevision, "main", "production"); err != nil {
t.Fatal(err)
}
if got := onboardingGit(t, "", "--git-dir", preservedTarget, "rev-parse", "refs/heads/main"); got != sourceRevision {
@ -269,7 +269,8 @@ func TestOnboardAppUsesCanonicalSourceAndCentralClusterManagers(t *testing.T) {
if strings.Contains(sourceCalls, "delivery") || strings.Contains(sourceCalls, "ensure-pr") || strings.Contains(sourceCalls, "merge-pr") {
t.Fatalf("source manager published a delivery change: %q", sourceCalls)
}
if got := strings.Join(clusterManager.calls, "\n"); got != "register test-org-2/cluster:maidn/register-web-ui" {
sourceRevision := onboardingGit(t, source, "rev-parse", "main")
if got := strings.Join(clusterManager.calls, "\n"); got != "register test-org-2/cluster:maidn/register-web-ui-"+sourceRevision[:12] {
t.Fatalf("cluster manager calls = %q", got)
}
}

View file

@ -97,6 +97,7 @@ type accessToken struct {
}
var copyGit = runGit
var hasRemoteBranch = (*RepoManager).HasRemoteBranch
func (e *APIError) Error() string {
return fmt.Sprintf("forgejo returned %s", e.Status)
@ -515,10 +516,19 @@ func (rm *RepoManager) PublishRepositoryPullRequest(repo, title, branch, base st
return false, errors.New("repository pull request requires distinct non-empty branches")
}
repoURL := CloneURL(rm.BaseURL, rm.Owner, repo)
hasBranch, err := rm.HasRemoteBranch(repoURL, branch)
hasBranch, err := hasRemoteBranch(rm, repoURL, branch)
if err != nil {
return false, err
}
if hasBranch {
open, err := rm.HasOpenPullRequest(repo, branch)
if err != nil {
return false, err
}
if !open {
return false, fmt.Errorf("Forgejo branch %q exists without exactly one open pull request; refusing to reuse it", branch)
}
}
temporary, err := os.MkdirTemp("", "maidn-registration-*")
if err != nil {
return false, err

View file

@ -400,6 +400,34 @@ func TestHasRemoteBranchReturnsFalseForMissingBranch(t *testing.T) {
}
}
func TestPublishRepositoryPullRequestRejectsExistingBranchWithoutOpenPullRequest(t *testing.T) {
original := hasRemoteBranch
t.Cleanup(func() { hasRemoteBranch = original })
server := httptest.NewServer(http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) {
if request.Method != http.MethodGet || request.URL.Path != "/api/v1/repos/owner/cluster/pulls" || request.URL.Query().Get("state") != "open" || request.URL.Query().Get("head") != "maidn/register-web-ui-deadbeefcafe" {
t.Fatalf("unexpected pull request lookup: %s %s", request.Method, request.URL.String())
}
_ = json.NewEncoder(writer).Encode([]pullRequest{})
}))
defer server.Close()
manager := NewRepoManager(server.URL, "test-token", "owner", "user", "", "", "main", "")
manager.HTTPClient = server.Client()
hasRemoteBranch = func(got *RepoManager, repoURL, branch string) (bool, error) {
if got != manager || repoURL != CloneURL(server.URL, "owner", "cluster") || branch != "maidn/register-web-ui-deadbeefcafe" {
t.Fatalf("unexpected remote branch lookup: %q %q", repoURL, branch)
}
return true, nil
}
_, err := manager.PublishRepositoryPullRequest("cluster", "register web-ui", "maidn/register-web-ui-deadbeefcafe", "main", func(string) error {
t.Fatal("change ran for a stale registration branch")
return nil
})
if err == nil || !strings.Contains(err.Error(), "without exactly one open pull request") {
t.Fatalf("PublishRepositoryPullRequest() error = %v", err)
}
}
func TestMergePullRequest(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) {
switch request.Method {