feat: pull request polling

Keep TerraformPullRequest resources in sync with open remote pull/merge
requests even without a webhook configured, by periodically listing them
via the provider API (GitHub/GitLab) and reconciling the desired set
against existing TerraformPullRequest objects (create/update/delete).

Following review feedback on the initial implementation:
- Polling now lives in the TerraformRepository controller
  (internal/controllers/terraformrepository/pullrequests.go), run
  best-effort on every repository Reconcile, instead of being bolted
  onto the TerraformPullRequest controller. This keeps each controller
  responsible for a single resource type and avoids conflating PR and
  Repository reconcile requests under the same watch/queue.
- The TerraformPullRequest controller's Reconcile is back to handling
  only TerraformPullRequest objects.

Also updates the git-webhook documentation to mention PR/MR polling
outside of the polling-limitations warning block.

Author: Guillaume Leccese

.gitignore

@@ -50,4 +50,4 @@ credentials/
 .aws/
 
 test-repo/
-TIltfile
+TIltfile

deploy/charts/burrito/values-dev.yaml

@@ -3,16 +3,20 @@ config:
     runner:
       image:
         repository: burrito
-        tag: DEV_TAG
+        tag: "1783089222"
         pullPolicy: Never
     datastore:
       storage:
         mock: true
+    controller:
+      timers:
+        driftDetection: 10m
+        repositorySync: 2m
 global:
   deployment:
     image:
       repository: burrito
-      tag: DEV_TAG
+      tag: "1783089222"
       pullPolicy: Never
 tenants:
   - namespace:

docs/operator-manual/git-webhook.md

@@ -2,11 +2,10 @@
 
 Using the webhook feature, Burrito can automatically trigger actions based on events in your Git repository, such as pull requests or pushes. This allows for real-time updates and interactions with your infrastructure as code.
 
-Otherwise, Burrito will poll repositories for changes, which may not be as efficient or timely as using webhooks.
+Otherwise, Burrito will poll repositories for changes, which may not be as efficient or timely as using webhooks. Burrito also polls open Pull Requests and Merge Requests to keep `TerraformPullRequest` resources in sync.
 
 !!! warning "Burrito polling limitations"
     Burrito's automatic polling of repositories only works for changes on referenced branches in TerraformLayers.
-    **Automatic polling of Pull Requests is not implemented yet.**
 
 ## Expose Burrito server to the internet
 

internal/controllers/terraformpullrequest/comment/common.go

@@ -1,5 +1,22 @@
 package comment
 
+import "strings"
+
+// The hidden marker lets providers update Burrito's previous PR/MR comment
+// instead of creating duplicates when Kubernetes status persistence is retried.
+const managedCommentMarker = "<!-- burrito:pull-request-comment -->"
+
 type Comment interface {
 	Generate(string) (string, error)
 }
+
+func WithManagedMarker(body string) string {
+	if HasManagedMarker(body) {
+		return body
+	}
+	return body + "\n\n" + managedCommentMarker
+}
+
+func HasManagedMarker(body string) bool {
+	return strings.Contains(body, managedCommentMarker)
+}

internal/controllers/terraformpullrequest/comment/default_test.go

@@ -1,36 +1,132 @@
 package comment
 
 import (
-	_ "embed"
+	"errors"
+	"strings"
+	"testing"
+
+	configv1alpha1 "github.com/padok-team/burrito/api/v1alpha1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-// func TestDefaultComment_Generate(t *testing.T) {
-// 	type fields struct {
-// 		layers  []configv1alpha1.TerraformLayer
-// 		storage storage.Storage
-// 	}
-// 	tests := []struct {
-// 		name    string
-// 		fields  fields
-// 		want    string
-// 		wantErr bool
-// 	}{
-// 		// TODO: Add test cases.
-// 	}
-// 	for _, tt := range tests {
-// 		t.Run(tt.name, func(t *testing.T) {
-// 			c := &DefaultComment{
-// 				layers:  tt.fields.layers,
-// 				storage: tt.fields.storage,
-// 			}
-// 			got, err := c.Generate()
-// 			if (err != nil) != tt.wantErr {
-// 				t.Errorf("DefaultComment.Generate() error = %v, wantErr %v", err, tt.wantErr)
-// 				return
-// 			}
-// 			if got != tt.want {
-// 				t.Errorf("DefaultComment.Generate() = %v, want %v", got, tt.want)
-// 			}
-// 		})
-// 	}
-// }
+type fakeDatastore struct {
+	plans       map[string][]byte
+	err         error
+	errByFormat map[string]error
+}
+
+func (f *fakeDatastore) GetPlan(namespace string, layer string, run string, attempt string, format string) ([]byte, error) {
+	if f.err != nil {
+		return nil, f.err
+	}
+	if err, ok := f.errByFormat[format]; ok {
+		return nil, err
+	}
+	return f.plans[format], nil
+}
+
+func (f *fakeDatastore) PutPlan(namespace string, layer string, run string, attempt string, format string, content []byte) error {
+	return nil
+}
+
+func (f *fakeDatastore) GetLogs(namespace string, layer string, run string, attempt string) ([]string, error) {
+	return nil, nil
+}
+
+func (f *fakeDatastore) PutLogs(namespace string, layer string, run string, attempt string, content []byte) error {
+	return nil
+}
+
+func (f *fakeDatastore) PutGitBundle(namespace string, name string, ref string, revision string, bundle []byte) error {
+	return nil
+}
+
+func (f *fakeDatastore) CheckGitBundle(namespace string, name string, ref string, revision string) (bool, error) {
+	return false, nil
+}
+
+func (f *fakeDatastore) GetGitBundle(namespace string, name string, ref string, revision string) ([]byte, error) {
+	return nil, nil
+}
+
+func TestDefaultCommentGenerate(t *testing.T) {
+	comment := NewDefaultComment([]configv1alpha1.TerraformLayer{
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "layer-a",
+				Namespace: "default",
+			},
+			Spec: configv1alpha1.TerraformLayerSpec{
+				Path: "terraform/",
+			},
+			Status: configv1alpha1.TerraformLayerStatus{
+				LastRun: configv1alpha1.TerraformLayerRun{
+					Name: "run-a",
+				},
+			},
+		},
+	}, &fakeDatastore{
+		plans: map[string][]byte{
+			"pretty": []byte("pretty plan"),
+			"short":  []byte("+ create"),
+		},
+	})
+
+	got, err := comment.Generate("abc123")
+	if err != nil {
+		t.Fatalf("Generate returned error: %v", err)
+	}
+
+	for _, expected := range []string{"Burrito Report", "layer-a", "terraform/", "+ create", "pretty plan", "abc123"} {
+		if !strings.Contains(got, expected) {
+			t.Fatalf("expected generated comment to contain %q, got:\n%s", expected, got)
+		}
+	}
+}
+
+func TestDefaultCommentGenerateReturnsDatastoreError(t *testing.T) {
+	expectedErr := errors.New("datastore unavailable")
+	comment := NewDefaultComment([]configv1alpha1.TerraformLayer{{}}, &fakeDatastore{err: expectedErr})
+
+	_, err := comment.Generate("abc123")
+	if !errors.Is(err, expectedErr) {
+		t.Fatalf("expected %v, got %v", expectedErr, err)
+	}
+}
+
+func TestDefaultCommentGenerateReturnsShortPlanError(t *testing.T) {
+	expectedErr := errors.New("short plan unavailable")
+	comment := NewDefaultComment([]configv1alpha1.TerraformLayer{{}}, &fakeDatastore{
+		plans: map[string][]byte{
+			"pretty": []byte("pretty plan"),
+		},
+		errByFormat: map[string]error{
+			"short": expectedErr,
+		},
+	})
+
+	_, err := comment.Generate("abc123")
+	if !errors.Is(err, expectedErr) {
+		t.Fatalf("expected %v, got %v", expectedErr, err)
+	}
+}
+
+func TestManagedMarkerHelpers(t *testing.T) {
+	body := "hello"
+	withMarker := WithManagedMarker(body)
+	if !HasManagedMarker(withMarker) {
+		t.Fatalf("expected generated body to contain managed marker")
+	}
+	if got := WithManagedMarker(withMarker); got != withMarker {
+		t.Fatalf("expected WithManagedMarker to be idempotent")
+	}
+	if HasManagedMarker(body) {
+		t.Fatalf("did not expect unmanaged body to contain marker")
+	}
+}
+
+func TestNewInitialComment(t *testing.T) {
+	if NewInitialComment() == nil {
+		t.Fatalf("expected NewInitialComment to return a comment")
+	}
+}

internal/controllers/terraformpullrequest/conditions_test.go

@@ -0,0 +1,150 @@
+package terraformpullrequest
+
+import (
+	"testing"
+
+	configv1alpha1 "github.com/padok-team/burrito/api/v1alpha1"
+	"github.com/padok-team/burrito/internal/annotations"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+)
+
+func TestIsLastCommitDiscoveredWhenDiscoveredCommitIsStale(t *testing.T) {
+	pr := terraformPullRequest("default", "repo-1", "1", "feature", "new-sha")
+	pr.Status.LastDiscoveredCommit = "old-sha"
+
+	condition, discovered := (&Reconciler{}).IsLastCommitDiscovered(pr)
+	if discovered {
+		t.Fatalf("expected stale discovered commit to require discovery")
+	}
+	if condition.Reason != "LastCommitNotDiscovered" {
+		t.Fatalf("expected LastCommitNotDiscovered reason, got %q", condition.Reason)
+	}
+}
+
+func TestAreLayersStillPlanningCoversBranches(t *testing.T) {
+	scheme := newTerraformPullRequestTestScheme(t)
+	tests := []struct {
+		name    string
+		pr      *configv1alpha1.TerraformPullRequest
+		objects []client.Object
+		want    bool
+		reason  string
+	}{
+		{
+			name: "missing branch commit",
+			pr: func() *configv1alpha1.TerraformPullRequest {
+				pr := planningPullRequest("default", "repo-1", "1", "feature", "sha")
+				delete(pr.Annotations, annotations.LastBranchCommit)
+				return pr
+			}(),
+			want:   true,
+			reason: "NoBranchCommitOnPR",
+		},
+		{
+			name:   "no discovered commit",
+			pr:     planningPullRequest("default", "repo-1", "1", "feature", "sha"),
+			want:   true,
+			reason: "NoCommitDiscovered",
+		},
+		{
+			name:   "discovery still needed",
+			pr:     planningPullRequestWithDiscoveredCommit("default", "repo-1", "1", "feature", "sha", "other"),
+			want:   true,
+			reason: "StillNeedsDiscovery",
+		},
+		{
+			name:   "no linked layers",
+			pr:     planningPullRequestWithDiscoveredCommit("default", "repo-1", "1", "feature", "sha", "sha"),
+			want:   false,
+			reason: "LayersNotPlanning",
+		},
+		{
+			name: "layer missing plan commit",
+			pr:   planningPullRequestWithDiscoveredCommit("default", "repo-1", "1", "feature", "sha", "sha"),
+			objects: []client.Object{
+				planningLayer("default", "layer-1", managedByLabelValue(planningPullRequestWithDiscoveredCommit("default", "repo-1", "1", "feature", "sha", "sha")), nil, nil),
+			},
+			want:   true,
+			reason: "LayersStillPlanning",
+		},
+		{
+			name: "layer missing relevant commit",
+			pr:   planningPullRequestWithDiscoveredCommit("default", "repo-1", "1", "feature", "sha", "sha"),
+			objects: []client.Object{
+				planningLayer("default", "layer-1", managedByLabelValue(planningPullRequestWithDiscoveredCommit("default", "repo-1", "1", "feature", "sha", "sha")), strPtr("plan-sha"), nil),
+			},
+			want:   true,
+			reason: "NoRelevantCommitOnLayer",
+		},
+		{
+			name: "layer finished planning",
+			pr:   planningPullRequestWithDiscoveredCommit("default", "repo-1", "1", "feature", "sha", "sha"),
+			objects: []client.Object{
+				planningLayer("default", "layer-1", managedByLabelValue(planningPullRequestWithDiscoveredCommit("default", "repo-1", "1", "feature", "sha", "sha")), strPtr("plan-sha"), strPtr("plan-sha")),
+			},
+			want:   false,
+			reason: "LayersNotPlanning",
+		},
+		{
+			name: "layer still planning",
+			pr:   planningPullRequestWithDiscoveredCommit("default", "repo-1", "1", "feature", "sha", "sha"),
+			objects: []client.Object{
+				planningLayer("default", "layer-1", managedByLabelValue(planningPullRequestWithDiscoveredCommit("default", "repo-1", "1", "feature", "sha", "sha")), strPtr("plan-sha"), strPtr("other-sha")),
+			},
+			want:   true,
+			reason: "LayersStillPlanning",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(tt.objects...).Build()
+			reconciler := &Reconciler{Client: cl}
+			condition, got := reconciler.AreLayersStillPlanning(tt.pr)
+			if got != tt.want {
+				t.Fatalf("expected %t, got %t", tt.want, got)
+			}
+			if condition.Reason != tt.reason {
+				t.Fatalf("expected reason %q, got %q", tt.reason, condition.Reason)
+			}
+		})
+	}
+}
+
+func planningPullRequest(namespace, name, id, branch, commit string) *configv1alpha1.TerraformPullRequest {
+	pr := terraformPullRequest(namespace, name, id, branch, commit)
+	pr.Status.LastDiscoveredCommit = ""
+	return pr
+}
+
+func planningPullRequestWithDiscoveredCommit(namespace, name, id, branch, commit, discoveredCommit string) *configv1alpha1.TerraformPullRequest {
+	pr := terraformPullRequest(namespace, name, id, branch, commit)
+	pr.Status.LastDiscoveredCommit = discoveredCommit
+	return pr
+}
+
+func planningLayer(namespace, name, managedLabel string, lastPlanCommit, lastRelevantCommit *string) *configv1alpha1.TerraformLayer {
+	layer := &configv1alpha1.TerraformLayer{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: namespace,
+			Labels: map[string]string{
+				managedByLabel: managedLabel,
+			},
+			Annotations: map[string]string{},
+		},
+	}
+	if lastPlanCommit != nil {
+		layer.Annotations[annotations.LastPlanCommit] = *lastPlanCommit
+	}
+	if lastRelevantCommit != nil {
+		layer.Annotations[annotations.LastRelevantCommit] = *lastRelevantCommit
+	}
+	return layer
+}
+
+func strPtr(value string) *string {
+	return &value
+}