Fix plan artefact storage with gcp buckets (#1251)

* Fixing of bug in GCP plan artefact storage

.github/workflows/cli_test.yml

@@ -27,13 +27,11 @@ jobs:
 
       - name: Deps
         run: |
-          pwd
           go get -v ./...
         working-directory: cli
 
       - name: Build
         run: |
-          pwd
           go build -v ./cmd/digger
         working-directory: cli
 

.github/workflows/cli_test_e2e.yml

@@ -0,0 +1,50 @@
+name: Cli e2e tests
+on:
+  push:
+  pull_request:
+    types: [opened, reopened]
+
+jobs:
+
+  build:
+    permissions:
+      contents: 'read'
+      id-token: 'write'
+
+    name: Build
+    runs-on: ubuntu-latest
+    steps:
+
+      - name: Check out code into the Go module directory
+        uses: actions/checkout@v4
+
+      - name: Download Go
+        uses: actions/setup-go@v5
+        with:
+          go-version: 1.22.0
+        id: go
+
+      - name: Check out code into the Go module directory
+        uses: actions/checkout@v4
+
+      - name: Deps cli
+        run: |
+          go get -v ./...
+        working-directory: cli
+
+      - name: Deps e2e
+        run: |
+          go get -v ./...
+        working-directory: cli_e2e
+
+      - name: Test
+        run: |
+          echo '${{ secrets.GCP_CREDENTIALS }}' > /tmp/gcp.json
+          go test -v ./...
+        working-directory: cli_e2e
+        env:
+          GOOGLE_APPLICATION_CREDENTIALS: /tmp/gcp.json
+          GOOGLE_STORAGE_BUCKET: gcp-plan-artefacts
+
+
+

cli/pkg/core/execution/execution.go

@@ -6,6 +6,7 @@ import (
 	"os"
 	"path"
 	"regexp"
+	"strconv"
 	"strings"
 
 	"github.com/diggerhq/digger/cli/pkg/core/locking"
@@ -118,10 +119,10 @@ type PlanPathProvider interface {
 	LocalPlanFilePath() string
 	StoredPlanFilePath() string
 	ArtifactName() string
-	PlanFileName() string
 }
 
 type ProjectPathProvider struct {
+	PRNumber         *int
 	ProjectPath      string
 	ProjectNamespace string
 	ProjectName      string
@@ -131,29 +132,31 @@ func (d ProjectPathProvider) ArtifactName() string {
 	return d.ProjectName
 }
 
-func (d ProjectPathProvider) PlanFileName() string {
-	return strings.ReplaceAll(d.ProjectNamespace, "/", "-") + "-" + d.ProjectName + ".tfplan"
-}
+func (d ProjectPathProvider) StoredPlanFilePath() string {
+	if d.PRNumber != nil {
+		prNumber := strconv.Itoa(*d.PRNumber)
+		return strings.ReplaceAll(d.ProjectNamespace, "/", "-") + "-" + prNumber + "-" + d.ProjectName + ".tfplan"
+	} else {
+		return strings.ReplaceAll(d.ProjectNamespace, "/", "-") + "-" + d.ProjectName + ".tfplan"
+	}
 
-func (d ProjectPathProvider) LocalPlanFilePath() string {
-	return path.Join(d.ProjectPath, d.PlanFileName())
 }
 
-func (d ProjectPathProvider) StoredPlanFilePath() string {
-	return path.Join(d.ProjectNamespace, d.PlanFileName())
+func (d ProjectPathProvider) LocalPlanFilePath() string {
+	return path.Join(d.ProjectPath, d.StoredPlanFilePath())
 }
 
 func (d DiggerExecutor) RetrievePlanJson() (string, error) {
 	executor := d
 	planStorage := executor.PlanStorage
 	planPathProvider := executor.PlanPathProvider
-	storedPlanExists, err := planStorage.PlanExists(planPathProvider.ArtifactName())
+	storedPlanExists, err := planStorage.PlanExists(planPathProvider.ArtifactName(), planPathProvider.StoredPlanFilePath())
 	if err != nil {
 		return "", fmt.Errorf("failed to check if stored plan exists. %v", err)
 	}
 	if storedPlanExists {
 		log.Printf("Pre-apply plan retrieval: stored plan exists in artefact, retrieving")
-		storedPlanPath, err := planStorage.RetrievePlan(planPathProvider.LocalPlanFilePath(), planPathProvider.ArtifactName())
+		storedPlanPath, err := planStorage.RetrievePlan(planPathProvider.LocalPlanFilePath(), planPathProvider.ArtifactName(), planPathProvider.StoredPlanFilePath())
 		if err != nil {
 			return "", fmt.Errorf("failed to retrieve stored plan path. %v", err)
 		}
@@ -218,7 +221,7 @@ func (d DiggerExecutor) Plan() (*terraform.PlanSummary, bool, bool, string, stri
 			}
 
 			if !isEmptyPlan {
-				nonEmptyPlanFilepath := strings.Replace(d.PlanPathProvider.LocalPlanFilePath(), d.PlanPathProvider.PlanFileName(), "isNonEmptyPlan.txt", 1)
+				nonEmptyPlanFilepath := strings.Replace(d.PlanPathProvider.LocalPlanFilePath(), d.PlanPathProvider.StoredPlanFilePath(), "isNonEmptyPlan.txt", 1)
 				file, err := os.Create(nonEmptyPlanFilepath)
 				if err != nil {
 					return nil, false, false, "", "", fmt.Errorf("unable to create file: %v", err)
@@ -237,7 +240,7 @@ func (d DiggerExecutor) Plan() (*terraform.PlanSummary, bool, bool, string, stri
 					return nil, false, false, "", "", fmt.Errorf("error reading file bytes: %v", err)
 				}
 
-				err = d.PlanStorage.StorePlanFile(fileBytes, d.PlanPathProvider.ArtifactName(), d.PlanPathProvider.PlanFileName())
+				err = d.PlanStorage.StorePlanFile(fileBytes, d.PlanPathProvider.ArtifactName(), d.PlanPathProvider.StoredPlanFilePath())
 				if err != nil {
 					fmt.Println("Error storing artifact file:", err)
 					return nil, false, false, "", "", fmt.Errorf("error storing artifact file: %v", err)
@@ -284,7 +287,7 @@ func (d DiggerExecutor) Apply() (bool, string, error) {
 	var plansFilename *string
 	if d.PlanStorage != nil {
 		var err error
-		plansFilename, err = d.PlanStorage.RetrievePlan(d.PlanPathProvider.LocalPlanFilePath(), d.PlanPathProvider.ArtifactName())
+		plansFilename, err = d.PlanStorage.RetrievePlan(d.PlanPathProvider.LocalPlanFilePath(), d.PlanPathProvider.ArtifactName(), d.PlanPathProvider.StoredPlanFilePath())
 		if err != nil {
 			return false, "", fmt.Errorf("error retrieving plan: %v", err)
 		}

cli/pkg/core/storage/plan_storage.go

@@ -1,9 +1,8 @@
 package storage
 
 type PlanStorage interface {
-	StorePlan(localPlanFilePath string, storedPlanFilePath string) error
-	StorePlanFile(fileContents []byte, artifactName string, fileName string) error
-	RetrievePlan(localPlanFilePath string, storedPlanFilePath string) (*string, error)
-	DeleteStoredPlan(storedPlanFilePath string) error
-	PlanExists(storedPlanFilePath string) (bool, error)
+	StorePlanFile(fileContents []byte, artifactName string, storedPlanFilePath string) error
+	RetrievePlan(localPlanFilePath string, artifactName string, storedPlanFilePath string) (*string, error)
+	DeleteStoredPlan(artifactName string, storedPlanFilePath string) error
+	PlanExists(artifactName string, storedPlanFilePath string) (bool, error)
 }

cli/pkg/digger/digger.go

@@ -106,7 +106,7 @@ func RunJobs(
 				continue
 			}
 
-			executorResult, output, err := run(command, job, policyChecker, orgService, SCMOrganisation, SCMrepository, job.RequestedBy, reporter, lock, prService, job.Namespace, workingDir, planStorage, appliesPerProject)
+			executorResult, output, err := run(command, job, policyChecker, orgService, SCMOrganisation, SCMrepository, job.PullRequestNumber, job.RequestedBy, reporter, lock, prService, job.Namespace, workingDir, planStorage, appliesPerProject)
 			if err != nil {
 				reportErr := backendApi.ReportProjectRun(SCMOrganisation+"-"+SCMrepository, job.ProjectName, runStartedAt, time.Now(), "FAILED", command, output)
 				if reportErr != nil {
@@ -181,7 +181,7 @@ func reportPolicyError(projectName string, command string, requestedBy string, r
 	return msg
 }
 
-func run(command string, job orchestrator.Job, policyChecker policy.Checker, orgService orchestrator.OrgService, SCMOrganisation string, SCMrepository string, requestedBy string, reporter core_reporting.Reporter, lock core_locking.Lock, prService orchestrator.PullRequestService, projectNamespace string, workingDir string, planStorage storage.PlanStorage, appliesPerProject map[string]bool) (*execution.DiggerExecutorResult, string, error) {
+func run(command string, job orchestrator.Job, policyChecker policy.Checker, orgService orchestrator.OrgService, SCMOrganisation string, SCMrepository string, PRNumber *int, requestedBy string, reporter core_reporting.Reporter, lock core_locking.Lock, prService orchestrator.PullRequestService, projectNamespace string, workingDir string, planStorage storage.PlanStorage, appliesPerProject map[string]bool) (*execution.DiggerExecutorResult, string, error) {
 	log.Printf("Running '%s' for project '%s' (workflow: %s)\n", command, job.ProjectName, job.ProjectWorkflow)
 
 	allowedToPerformCommand, err := policyChecker.CheckAccessPolicy(orgService, &prService, SCMOrganisation, SCMrepository, job.ProjectName, command, job.PullRequestNumber, requestedBy, []string{})
@@ -225,6 +225,7 @@ func run(command string, job orchestrator.Job, policyChecker policy.Checker, org
 		ProjectPath:      projectPath,
 		ProjectNamespace: projectNamespace,
 		ProjectName:      job.ProjectName,
+		PRNumber:         PRNumber,
 	}
 
 	diggerExecutor := execution.LockingExecutorWrapper{
@@ -442,7 +443,7 @@ func run(command string, job orchestrator.Job, policyChecker policy.Checker, org
 		}
 
 		if planStorage != nil {
-			err = planStorage.DeleteStoredPlan(planPathProvider.StoredPlanFilePath())
+			err = planStorage.DeleteStoredPlan(planPathProvider.ArtifactName(), planPathProvider.StoredPlanFilePath())
 			if err != nil {
 				log.Printf("failed to delete stored plan file '%v':  %v", planPathProvider.StoredPlanFilePath(), err)
 			}
@@ -560,6 +561,7 @@ func RunJob(
 			ProjectPath:      projectPath,
 			ProjectNamespace: repo,
 			ProjectName:      job.ProjectName,
+			PRNumber:         job.PullRequestNumber,
 		}
 
 		diggerExecutor := execution.DiggerExecutor{

cli/pkg/digger/digger_test.go

@@ -191,27 +191,22 @@ type MockPlanStorage struct {
 	Commands []RunInfo
 }
 
-func (m *MockPlanStorage) StorePlan(localPlanFilePath string, storedPlanFilePath string) error {
-	m.Commands = append(m.Commands, RunInfo{"StorePlan", localPlanFilePath, time.Now()})
-	return nil
-}
-
 func (m *MockPlanStorage) StorePlanFile(fileContents []byte, artifactName string, fileName string) error {
 	m.Commands = append(m.Commands, RunInfo{"StorePlanFile", artifactName, time.Now()})
 	return nil
 }
 
-func (m *MockPlanStorage) RetrievePlan(localPlanFilePath string, storedPlanFilePath string) (*string, error) {
+func (m *MockPlanStorage) RetrievePlan(localPlanFilePath string, artifactName string, storedPlanFilePath string) (*string, error) {
 	m.Commands = append(m.Commands, RunInfo{"RetrievePlan", localPlanFilePath, time.Now()})
 	return nil, nil
 }
 
-func (m *MockPlanStorage) DeleteStoredPlan(storedPlanFilePath string) error {
+func (m *MockPlanStorage) DeleteStoredPlan(artifactName string, storedPlanFilePath string) error {
 	m.Commands = append(m.Commands, RunInfo{"DeleteStoredPlan", storedPlanFilePath, time.Now()})
 	return nil
 }
 
-func (m *MockPlanStorage) PlanExists(storedPlanFilePath string) (bool, error) {
+func (m *MockPlanStorage) PlanExists(artifactName string, storedPlanFilePath string) (bool, error) {
 	m.Commands = append(m.Commands, RunInfo{"PlanExists", storedPlanFilePath, time.Now()})
 	return false, nil
 }
@@ -225,8 +220,8 @@ func (m MockPlanPathProvider) ArtifactName() string {
 	return "plan"
 }
 
-func (m MockPlanPathProvider) PlanFileName() string {
-	m.Commands = append(m.Commands, RunInfo{"PlanFileName", "", time.Now()})
+func (m MockPlanPathProvider) StoredPlanFilePath() string {
+	m.Commands = append(m.Commands, RunInfo{"StoredPlanFilePath", "", time.Now()})
 	return "plan"
 }
 
@@ -235,11 +230,6 @@ func (m MockPlanPathProvider) LocalPlanFilePath() string {
 	return "plan"
 }
 
-func (m MockPlanPathProvider) StoredPlanFilePath() string {
-	m.Commands = append(m.Commands, RunInfo{"StoredPlanFilePath", "", time.Now()})
-	return "plan"
-}
-
 func TestCorrectCommandExecutionWhenApplying(t *testing.T) {
 
 	commandRunner := &MockCommandRunner{}

cli/pkg/storage/plan_storage.go

@@ -32,7 +32,7 @@ type GithubPlanStorage struct {
 	ZipManager        utils.Zipper
 }
 
-func (psg *PlanStorageGcp) PlanExists(storedPlanFilePath string) (bool, error) {
+func (psg *PlanStorageGcp) PlanExists(artifactName string, storedPlanFilePath string) (bool, error) {
 	obj := psg.Bucket.Object(storedPlanFilePath)
 	_, err := obj.Attrs(psg.Context)
 	if err != nil {
@@ -44,34 +44,20 @@ func (psg *PlanStorageGcp) PlanExists(storedPlanFilePath string) (bool, error) {
 	return true, nil
 }
 
-func (psg *PlanStorageGcp) StorePlan(localPlanFilePath string, storedPlanFilePath string) error {
-	file, err := os.Open(localPlanFilePath)
-	if err != nil {
-		return fmt.Errorf("unable to open file: %v", err)
-	}
-	defer file.Close()
-
-	obj := psg.Bucket.Object(storedPlanFilePath)
-	wc := obj.NewWriter(psg.Context)
-
-	if _, err = io.Copy(wc, file); err != nil {
-		wc.Close()
-		return fmt.Errorf("unable to write data to bucket: %v", err)
-	}
+func (psg *PlanStorageGcp) StorePlanFile(fileContents []byte, artifactName string, fileName string) error {
+	fullPath := fileName
+	obj := psg.Bucket.Object(fullPath)
+	writer := obj.NewWriter(context.Background())
+	defer writer.Close()
 
-	if err := wc.Close(); err != nil {
-		return fmt.Errorf("unable to close writer: %v", err)
+	if _, err := writer.Write(fileContents); err != nil {
+		log.Printf("Failed to write file to bucket: %v", err)
+		return err
 	}
-
-	return nil
-}
-
-func (psg *PlanStorageGcp) StorePlanFile(fileContents []byte, artifactName string, fileName string) error {
-	// TODO: implement me
 	return nil
 }
 
-func (psg *PlanStorageGcp) RetrievePlan(localPlanFilePath string, storedPlanFilePath string) (*string, error) {
+func (psg *PlanStorageGcp) RetrievePlan(localPlanFilePath string, artifactName string, storedPlanFilePath string) (*string, error) {
 	obj := psg.Bucket.Object(storedPlanFilePath)
 	rc, err := obj.NewReader(psg.Context)
 	if err != nil {
@@ -95,7 +81,7 @@ func (psg *PlanStorageGcp) RetrievePlan(localPlanFilePath string, storedPlanFile
 	return &fileName, nil
 }
 
-func (psg *PlanStorageGcp) DeleteStoredPlan(storedPlanFilePath string) error {
+func (psg *PlanStorageGcp) DeleteStoredPlan(artifactName string, storedPlanFilePath string) error {
 	obj := psg.Bucket.Object(storedPlanFilePath)
 	err := obj.Delete(psg.Context)
 
@@ -105,12 +91,7 @@ func (psg *PlanStorageGcp) DeleteStoredPlan(storedPlanFilePath string) error {
 	return nil
 }
 
-func (gps *GithubPlanStorage) StorePlan(localPlanFilePath string, storedPlanFilePath string) error {
-	_ = fmt.Sprintf("Skipping storing plan %s. It should be achieved using actions/upload-artifact@v3", localPlanFilePath)
-	return nil
-}
-
-func (gps *GithubPlanStorage) StorePlanFile(fileContents []byte, artifactName string, fileName string) error {
+func (gps *GithubPlanStorage) StorePlanFile(fileContents []byte, artifactName string, storedPlanFilePath string) error {
 	actionsRuntimeToken := os.Getenv("ACTIONS_RUNTIME_TOKEN")
 	actionsRuntimeURL := os.Getenv("ACTIONS_RUNTIME_URL")
 	githubRunID := os.Getenv("GITHUB_RUN_ID")
@@ -139,7 +120,7 @@ func (gps *GithubPlanStorage) StorePlanFile(fileContents []byte, artifactName st
 	resourceURL := createArtifactResponseMap["fileContainerResourceUrl"].(string)
 
 	// Upload Data
-	uploadURL := fmt.Sprintf("%s?itemPath=%s/%s", resourceURL, artifactName, fileName)
+	uploadURL := fmt.Sprintf("%s?itemPath=%s/%s", resourceURL, artifactName, storedPlanFilePath)
 	uploadData := fileContents
 	dataLen := len(uploadData)
 	headers["Content-Type"] = "application/octet-stream"
@@ -191,8 +172,8 @@ func doRequest(method, url string, headers map[string]string, body []byte) (*htt
 	return resp, nil
 }
 
-func (gps *GithubPlanStorage) RetrievePlan(localPlanFilePath string, storedPlanFilePath string) (*string, error) {
-	plansFilename, err := gps.DownloadLatestPlans(storedPlanFilePath)
+func (gps *GithubPlanStorage) RetrievePlan(localPlanFilePath string, artifactName string, storedPlanFilePath string) (*string, error) {
+	plansFilename, err := gps.DownloadLatestPlans(artifactName)
 
 	if err != nil {
 		return nil, fmt.Errorf("error downloading plan: %v", err)
@@ -210,7 +191,7 @@ func (gps *GithubPlanStorage) RetrievePlan(localPlanFilePath string, storedPlanF
 	return &plansFilename, nil
 }
 
-func (gps *GithubPlanStorage) PlanExists(storedPlanFilePath string) (bool, error) {
+func (gps *GithubPlanStorage) PlanExists(artifactName string, storedPlanFilePath string) (bool, error) {
 	artifacts, _, err := gps.Client.Actions.ListArtifacts(context.Background(), gps.Owner, gps.RepoName, &github.ListOptions{
 		PerPage: 100,
 	})
@@ -219,15 +200,15 @@ func (gps *GithubPlanStorage) PlanExists(storedPlanFilePath string) (bool, error
 		return false, err
 	}
 
-	latestPlans := getLatestArtifactWithName(artifacts.Artifacts, storedPlanFilePath)
+	latestPlans := getLatestArtifactWithName(artifacts.Artifacts, artifactName)
 
 	if latestPlans == nil {
 		return false, nil
 	}
 	return true, nil
 }
 
-func (gps *GithubPlanStorage) DeleteStoredPlan(storedPlanFilePath string) error {
+func (gps *GithubPlanStorage) DeleteStoredPlan(artifactName string, storedPlanFilePath string) error {
 	return nil
 }