feat: pull request polling (#936)

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.

Co-authored-by: Guillaume Leccese <guillaume.leccese@theodo.com>

Author: DjinnS

.gitignore

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

api/v1alpha1/terraformrepository_types.go

@@ -113,3 +113,20 @@ type TerraformRepositoryList struct {
 func init() {
 	SchemeBuilder.Register(&TerraformRepository{}, &TerraformRepositoryList{})
 }
+
+// Workaround needed for envtest which does not populate the TypeMeta structure
+// See https://github.com/kubernetes-sigs/controller-runtime/issues/1870
+func (repository *TerraformRepository) GetAPIVersion() string {
+	if repository.APIVersion == "" {
+		return "config.terraform.padok.cloud/v1alpha1"
+	}
+	return repository.APIVersion
+}
+
+// Same as above
+func (repository *TerraformRepository) GetKind() string {
+	if repository.Kind == "" {
+		return "TerraformRepository"
+	}
+	return repository.Kind
+}

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")
+	}
+}