Merge pull request 'fix: avoid stale onboarding branches' (#64) from fix/stale-registration-branches into main
Reviewed-on: #64
This commit is contained in:
commit
3dbaeab745
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
Loading…
Reference in a new issue