ottogroup · grzr · Apr 25, 2024 · Apr 22, 2024 · Apr 23, 2024 · Apr 23, 2024
diff --git a/Readme.md b/Readme.md
diff --git a/frontend/dist/penelope_48.png b/frontend/dist/penelope_48.png
diff --git a/frontend/index.html b/frontend/index.html
@@ -3,7 +3,7 @@
   <head>
     <meta charset="UTF-8" />
     <meta name="viewport" content="width=device-width, initial-scale=1.0" />
-    <link rel="icon" href="/favicon.png" />
+    <link rel="icon" href="/penelope_48.png" />
     <title>Penelope</title>
   </head>
 

diff --git a/frontend/public/Penelope.jpeg b/frontend/public/Penelope.jpeg
diff --git a/frontend/public/Penelope_250.jpeg b/frontend/public/Penelope_250.jpeg
diff --git a/frontend/public/penelope_48.png b/frontend/public/penelope_48.png
diff --git a/frontend/src/App.vue b/frontend/src/App.vue
@@ -1,5 +1,5 @@
 <script setup lang="ts">
-import logo from "@/assets/penelope_32.png";
+import logo from "@/assets/penelope_48.png";
 import Sandbox from "@/views/Sandbox.vue";
 import { ref } from "vue";
 

diff --git a/frontend/src/assets/penelope_48.png b/frontend/src/assets/penelope_48.png
diff --git a/frontend/src/components/BackupCreateDialog.vue b/frontend/src/components/BackupCreateDialog.vue
@@ -128,7 +128,12 @@ const apiRequestBody = () => {
     recovery_time_objective: Number(request.value.recovery_time_objective),
     type: request.value.type,
     strategy: request.value.strategy,
-    target: request.value.target,
+    target: {
+      storage_class: request.value.target?.storage_class,
+      region: request.value.target?.region,
+      dual_region: request.value.target?.dual_region,
+      archive_ttm: Number(request.value.target?.archive_ttm),
+    },
     snapshot_options: {
       lifetime_in_days: Number(request.value.snapshot_options?.lifetime_in_days),
       frequency_in_hours: Number(request.value.snapshot_options?.frequency_in_hours),

diff --git a/frontend/src/components/BackupTable.vue b/frontend/src/components/BackupTable.vue
@@ -88,21 +88,21 @@ const projectLink = (project: string) => {
     <template #[`item.source`]="{ item }">
       <template v-if="item.type === BackupType.BIG_QUERY">
         BigQuery:
-        <a :href="bigqueryDatasetLink(item.project ?? '', item.bigquery_options?.dataset ?? '')">{{
+        <a :href="bigqueryDatasetLink(item.project ?? '', item.bigquery_options?.dataset ?? '')"  target="_blank">{{
             item.bigquery_options?.dataset
           }}</a>
         <ul>
           <li v-for="table in item.bigquery_options?.table">
             Table:
-            <a :href="bigqueryTableLink(item.project ?? '', item.bigquery_options?.dataset ?? '', table)">{{
+            <a :href="bigqueryTableLink(item.project ?? '', item.bigquery_options?.dataset ?? '', table)"  target="_blank">{{
                 table
               }}</a>
           </li>
         </ul>
       </template>
       <template v-if="item.type === BackupType.CLOUD_STORAGE">
         Bucket:
-        <a :href="cloudStorageLink(item.project ?? '', item.gcs_options?.bucket ?? '')">{{
+        <a :href="cloudStorageLink(item.project ?? '', item.gcs_options?.bucket ?? '')"  target="_blank">{{
             item.gcs_options?.bucket
           }}</a>
       </template>

diff --git a/go.mod b/go.mod
@@ -6,6 +6,7 @@ require (
 	cloud.google.com/go/iam v1.1.7
 	cloud.google.com/go/logging v1.9.0
 	cloud.google.com/go/monitoring v1.18.2
+	cloud.google.com/go/resourcemanager v1.9.6
 	cloud.google.com/go/storage v1.40.0
 	contrib.go.opencensus.io/exporter/stackdriver v0.13.14
 	github.com/aws/aws-sdk-go v1.51.23

diff --git a/go.sum b/go.sum
@@ -19,6 +19,8 @@ cloud.google.com/go/longrunning v0.5.6 h1:xAe8+0YaWoCKr9t1+aWe+OeQgN/iJK1fEgZSXm
 cloud.google.com/go/longrunning v0.5.6/go.mod h1:vUaDrWYOMKRuhiv6JBnn49YxCPz2Ayn9GqyjaBT8/mA=
 cloud.google.com/go/monitoring v1.18.2 h1:nIQdZdf1/9M0cEmXSlLB2jMq/k3CRh9p3oUzS06VDG8=
 cloud.google.com/go/monitoring v1.18.2/go.mod h1:MuL95M6d9HtXQOaWP9JxhFZJKP+fdTF0Gt5xl4IDsew=
+cloud.google.com/go/resourcemanager v1.9.6 h1:VPfJFbWxrTYQzEXCDbJNpcvSB8eZhTSM0YHH146fIB8=
+cloud.google.com/go/resourcemanager v1.9.6/go.mod h1:d+XUOGbxg6Aka3lmC4fDiserslux3d15uX08C6a0MBg=
 cloud.google.com/go/storage v1.40.0 h1:VEpDQV5CJxFmJ6ueWNsKxcr1QAYOXEgxDa+sBbJahPw=
 cloud.google.com/go/storage v1.40.0/go.mod h1:Rrj7/hKlG87BLqDJYtwR0fbPld8uJPbQ2ucUMY7Ir0g=
 cloud.google.com/go/trace v1.10.6 h1:XF0Ejdw0NpRfAvuZUeQe3ClAG4R/9w5JYICo7l2weaw=

diff --git a/pkg/http/actions/updating.go b/pkg/http/actions/updating.go
@@ -37,8 +37,5 @@ func (dl *UpdateBackupHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 		prepareResponse(w, logMsg, respMsg, http.StatusBadRequest)
 		return
 	}
-	if !checkRecoveryPointsAreValid(w, request.RecoveryPointObjective, request.RecoveryTimeObjective) {
-		return
-	}
 	handleRequestByProcessor(ctx, w, r, request, http.StatusOK, dl.processorBuilder.ProcessorForUpdating)
 }
diff --git a/pkg/http/mock/fixtures.go b/pkg/http/mock/fixtures.go
@@ -30,7 +30,8 @@ var (
 	// BucketAttrsHTTPMock request
 	BucketAttrsHTTPMock = NewMockedHTTPRequest("GET", "/storage/v1/b/.*", bucketAttrsResponse)
 	// PatchBucketAttrsHTTPMock request
-	PatchBucketAttrsHTTPMock = NewMockedHTTPRequest("PATCH", "/storage/v1/b/.*", patchBucketAttrsResponse)
+	PatchBucketAttrsHTTPMock   = NewMockedHTTPRequest("PATCH", "/storage/v1/b/.*", patchBucketAttrsResponse)
+	BucketSetIAMPolicyHTTPMock = NewMockedHTTPRequest("POST", "/storage/v1/b/.*/iam", bucketSetIAMPolicy)
 	// SinkCreatedHTTPpMock request
 	SinkCreatedHTTPpMock = NewMockedHTTPRequest("POST", "/storage/v1/b", sinkCreatedResponse)
 	// SinkDeletedHTTPMock request
@@ -60,6 +61,7 @@ var (
 	ListPoliciesSafeHTTPMock   = NewMockedHTTPRequest("GET", "policies/cloudresourcemanager.googleapis.com%252Fprojects%252Ftest-example-safe/denypolicies", listPoliciesResultResponse)
 
 	ListServiceUsageHTTPMock = NewMockedHTTPRequest("GET", "projects/.*/services", listServiceUsageOkResponse)
+	GetPRojectHTTPMock       = NewMockedHTTPRequest("GET", "/v3/projects/.*/", getProjectOkResponse)
 )
 
 const (
@@ -201,6 +203,47 @@ Content-Type: application/json; charset=UTF-8
 
 {"services": [],"nextPageToken": null}`
 
+	getProjectOkResponse = `HTTP/1.1 200
+Content-Type: application/json; charset=UTF-8
+
+{
+  "name": "projects/00000000000",
+  "parent": "folders/00000000001",
+  "projectId": "project1",
+  "state": "ACTIVE",
+  "displayName": "project1",
+  "createTime": "2021-03-12T21:16:05.027Z",
+  "updateTime": "2023-07-05T07:58:52.363128Z",
+  "etag": "W/\"tagValue\"",
+  "labels": {
+    "env": "dev",
+    "service_owner": "[email protected]",
+    "team": "team1"
+  }
+}`
+	bucketSetIAMPolicy = `HTTP/1.1 200
+Content-Type: application/json; charset=UTF-8
+
+{
+  "version": 10,
+  "kind": "storage#policy",
+  "resourceId": "string",
+  "bindings": [
+    {
+      "role": "string",
+      "members": [
+        "string"
+      ]
+      "condition": {
+        "title": "string",
+        "description": "string",
+        "expression": "2023-08-13 16:08:44+02:00"
+      }
+    }
+  ],
+  "etag": "string"
+}`
+
 	listPoliciesResultResponse = `HTTP/1.1 200
 Content-Type: application/json; charset=UTF-8
 

diff --git a/pkg/processor/compliance_processor_factory.go b/pkg/processor/compliance_processor_factory.go
@@ -321,7 +321,18 @@ func (c *backupWithSingleWriterCheck) Check(ctx context.Context, request request
 	it := policiesClient.ListPolicies(ctx, &iampb.ListPoliciesRequest{
 		Parent: fmt.Sprintf("policies/%s/denypolicies", attachmentPoint),
 	})
-
+	gcsClient, err := gcs.NewCloudStorageClient(ctx, c.tokenSourceProvider, targetProject)
+	if err != nil {
+		glog.Errorf("could not create new GCS client: %s", targetProject, err)
+		return requestobjects.ComplianceCheck{}, err
+	}
+	defer gcsClient.Close(ctx)
+	project, err := gcsClient.GetProject(ctx, targetProject)
+	if err != nil {
+		glog.Errorf("could not get project %s info: %s", targetProject, err)
+		return requestobjects.ComplianceCheck{}, err
+	}
+	stsSinkAccount := fmt.Sprintf(sinkSTSAccountScheme, strings.ReplaceAll(project.Name, "projects/", ""))
 	for {
 		policy, err := it.Next()
 		if errors.Is(err, iterator.Done) {
@@ -331,10 +342,18 @@ func (c *backupWithSingleWriterCheck) Check(ctx context.Context, request request
 			glog.Errorf("could not get next policy: %s", err)
 			break
 		}
+		// list policies for the target project comes without rules
+		fullPolicy, err := policiesClient.GetPolicy(ctx, &iampb.GetPolicyRequest{
+			Name: policy.Name,
+		})
+		if err != nil {
+			glog.Errorf("could not get full policy: %s", err)
+			break
+		}
 
 		// We need to check if deny edit permission for cloud storage is set for all principals except for the
 		// target backup service account.
-		for _, rule := range policy.Rules {
+		for _, rule := range fullPolicy.Rules {
 			deniedPermissions := rule.GetDenyRule().GetDeniedPermissions()
 			deniedPrincipals := rule.GetDenyRule().GetDeniedPrincipals()
 			exceptionPrincipals := rule.GetDenyRule().GetExceptionPrincipals()
@@ -347,7 +366,7 @@ func (c *backupWithSingleWriterCheck) Check(ctx context.Context, request request
 				continue
 			}
 
-			if !containsOnlyBackupServiceAccountAsException(targetPrincipal, exceptionPrincipals) {
+			if !containsOnlyBackupServiceAccountsAsException(stsSinkAccount, targetPrincipal, exceptionPrincipals) {
 				continue
 			}
 
@@ -384,8 +403,18 @@ const (
 	allPrincipals = "principalSet://goog/public:all"
 )
 
-func containsOnlyBackupServiceAccountAsException(targetPrincipal string, principals []string) bool {
-	return len(principals) == 1 && strings.EqualFold(principals[0], fmt.Sprintf("principal://iam.googleapis.com/projects/-/serviceAccounts/%s", targetPrincipal))
+func containsOnlyBackupServiceAccountsAsException(stsServiceAccount, targetPrincipal string, principals []string) bool {
+	// sts & backup service accounts can write
+	allowedPrincipals := []string{
+		fmt.Sprintf("principal://iam.googleapis.com/projects/-/serviceAccounts/%s", targetPrincipal),
+		fmt.Sprintf("principal://iam.googleapis.com/projects/-/serviceAccounts/%s", stsServiceAccount),
+	}
+	for _, principal := range principals {
+		if !contains(allowedPrincipals, principal) {
+			return false
+		}
+	}
+	return len(principals) == len(allowedPrincipals)
 }
 
 func containsAllPrincipals(principals []string) bool {

diff --git a/pkg/processor/creating_processor_factory.go b/pkg/processor/creating_processor_factory.go
@@ -1,6 +1,7 @@
 package processor
 
 import (
+	"cloud.google.com/go/iam"
 	"context"
 	"fmt"
 	"strings"
@@ -20,6 +21,8 @@ import (
 	"go.opencensus.io/trace"
 )
 
+const sinkSTSAccountScheme = "project-%[email protected]"
+
 type CreatingProcessorFactory interface {
 	CreateProcessor(ctxIn context.Context) (Operation[requestobjects.CreateRequest, requestobjects.BackupResponse], error)
 }
@@ -420,8 +423,30 @@ func prepareSink(ctxIn context.Context, cloudStorageClient gcs.CloudStorageClien
 		}
 
 		err = cloudStorageClient.CreateBucket(ctx, backup.TargetProject, backup.Sink, backup.Region, backup.DualRegion, backup.StorageClass, lifetimeInDays, backup.ArchiveTTM)
-		if err == nil {
-			return cloudStorageClient.CreateObject(ctx, backup.Sink, fmt.Sprintf("%s/THIS_TRASHCAN_CONTAINS_DELETED_OBJECTS_FROM_SOURCE", backup.GetTrashcanPath()), "")
+		if err != nil {
+			return err
+		}
+		err = cloudStorageClient.CreateObject(ctx, backup.Sink, fmt.Sprintf("%s/THIS_TRASHCAN_CONTAINS_DELETED_OBJECTS_FROM_SOURCE", backup.GetTrashcanPath()), "")
+		if err != nil {
+			return err
+		}
+		if backup.Type != repository.CloudStorage {
+			return nil
+		}
+		project, err := cloudStorageClient.GetProject(ctx, backup.TargetProject)
+		if err != nil {
+			return err
+		}
+		projectNumber := strings.ReplaceAll(project.Name, "projects/", "")
+		bucketPolicy := &iam.Policy{}
+		// Storage Transfer Service needs to write to and read from the sink bucket
+		// based on https://cloud.google.com/storage-transfer/docs/sink-cloud-storage#required_permissions
+		storageTransferGSABinding := "serviceAccount:" + fmt.Sprintf(sinkSTSAccountScheme, projectNumber)
+		bucketPolicy.Add(storageTransferGSABinding, "roles/storage.legacyBucketWriter")
+		bucketPolicy.Add(storageTransferGSABinding, "roles/storage.legacyBucketReader")
+		err = cloudStorageClient.SetBucketIAMPolicy(ctx, backup.Sink, bucketPolicy)
+		if err != nil {
+			return err
 		}
 	}
 

diff --git a/pkg/processor/schedule_processor_test.go b/pkg/processor/schedule_processor_test.go
@@ -1,6 +1,8 @@
 package processor
 
 import (
+	"cloud.google.com/go/iam"
+	"cloud.google.com/go/resourcemanager/apiv3/resourcemanagerpb"
 	"context"
 	"regexp"
 	"testing"
@@ -443,6 +445,14 @@ type stubGcsClient struct {
 	fDeleteObjectsErr error
 }
 
+func (g *stubGcsClient) GetProject(ctxIn context.Context, projectID string) (*resourcemanagerpb.Project, error) {
+	panic("implement me")
+}
+
+func (g *stubGcsClient) SetBucketIAMPolicy(ctxIn context.Context, bucket string, policy *iam.Policy) error {
+	panic("implement me")
+}
+
 func (g *stubGcsClient) Close(context.Context) {
 	panic("implement me")
 }

diff --git a/pkg/processor/updating_processor_factory.go b/pkg/processor/updating_processor_factory.go
@@ -85,7 +85,7 @@ func (c updatingProcessor) Process(ctxIn context.Context, args *Argument[request
 	// handle status change
 	if request.Status != "" && !backup.Status.EqualTo(request.Status) {
 		if !isBackupStatusTransitionValid(backup.Status, repository.BackupStatus(request.Status)) {
-			return requestobjects.UpdateResponse{}, fmt.Errorf("backup status update not allowed from %s to %s", request.BackupID, request.Status)
+			return requestobjects.UpdateResponse{}, fmt.Errorf("backup status update not allowed from %s to %s", backup.Status, request.Status)
 		}
 		// make a shortcut from NotStarted -> ToDelete to NotStarted -> BackupDeleted
 		if repository.NotStarted == backup.Status && repository.ToDelete.EqualTo(request.Status) {

diff --git a/pkg/repository/backup_repository.go b/pkg/repository/backup_repository.go
@@ -212,19 +212,29 @@ func (d *defaultBackupRepository) UpdateBackup(ctxIn context.Context, fields Upd
 	}
 
 	columns := []string{
-		"snapshot_lifetime_in_days",
 		"bigquery_table",
 		"bigquery_excluded_tables",
 		"cloudstorage_include_path",
 		"cloudstorage_exclude_path",
 		"audit_updated_timestamp",
 		"audit_deleted_timestamp",
-		"mirror_lifetime_in_days",
-		"archive_ttm",
-		"recovery_point_objective",
-		"recovery_time_objective",
 	}
 
+	if fields.RecoveryPointObjective > 0 {
+		columns = append(columns, "recovery_point_objective")
+	}
+	if fields.RecoveryTimeObjective > 0 {
+		columns = append(columns, "recovery_time_objective")
+	}
+	if fields.ArchiveTTM > 0 {
+		columns = append(columns, "archive_ttm")
+	}
+	if fields.MirrorTTL > 0 {
+		columns = append(columns, "mirror_lifetime_in_days")
+	}
+	if fields.SnapshotTTL > 0 {
+		columns = append(columns, "snapshot_lifetime_in_days")
+	}
 	if fields.Status != "" {
 		columns = append(columns, "status")
 	}