From f528641d16fb8fef597cf0255faa6a0e6a5fa026 Mon Sep 17 00:00:00 2001
From: t-kikuc <tkikuchi07f@gmail.com>
Date: Tue, 5 Nov 2024 12:01:19 +0900
Subject: [PATCH] Lambda: clone manifests not to modify original manifests

Signed-off-by: t-kikuc <tkikuchi07f@gmail.com>
---
 .../piped/driftdetector/lambda/detector.go    | 34 ++++++++++-----
 .../driftdetector/lambda/detector_test.go     | 43 ++++++++++++++++---
 2 files changed, 61 insertions(+), 16 deletions(-)

diff --git a/pkg/app/piped/driftdetector/lambda/detector.go b/pkg/app/piped/driftdetector/lambda/detector.go
index 6187680c19..fcfa57accf 100644
--- a/pkg/app/piped/driftdetector/lambda/detector.go
+++ b/pkg/app/piped/driftdetector/lambda/detector.go
@@ -207,7 +207,12 @@ func (d *detector) checkApplication(ctx context.Context, app *model.Application,
 	}
 	d.logger.Info(fmt.Sprintf("application %s has a live function manifest", app.Id))
 
-	ignoreAndSortParameters(&headManifest.Spec)
+	clonedSpec := ignoreAndSortParameters(headManifest.Spec)
+	head := provider.FunctionManifest{
+		Kind:       headManifest.Kind,
+		APIVersion: headManifest.APIVersion,
+		Spec:       clonedSpec,
+	}
 
 	// WithIgnoreAddingMapKeys option ignores all of followings:
 	//  - default value of Architecture
@@ -216,7 +221,7 @@ func (d *detector) checkApplication(ctx context.Context, app *model.Application,
 	//  - tags added in live states, including pipecd managed tags
 	result, err := provider.Diff(
 		liveManifest,
-		headManifest,
+		head,
 		diff.WithEquateEmpty(),
 		diff.WithIgnoreAddingMapKeys(),
 		diff.WithCompareNumberAndNumericString(),
@@ -238,22 +243,31 @@ func (d *detector) checkApplication(ctx context.Context, app *model.Application,
 // sorts: (Lambda sorts them in liveSpec)
 //   - Architectures in headSpec
 //   - SubnetIDs in headSpec
-func ignoreAndSortParameters(headSpec *provider.FunctionManifestSpec) {
+func ignoreAndSortParameters(headSpec provider.FunctionManifestSpec) provider.FunctionManifestSpec {
+	cloneSpec := headSpec
 	// We cannot compare SourceCode and S3 packaging because live states do not have them.
-	headSpec.SourceCode = provider.SourceCode{}
-	headSpec.S3Bucket = ""
-	headSpec.S3Key = ""
-	headSpec.S3ObjectVersion = ""
+	cloneSpec.SourceCode = provider.SourceCode{}
+	cloneSpec.S3Bucket = ""
+	cloneSpec.S3Key = ""
+	cloneSpec.S3ObjectVersion = ""
 
 	// Architectures, Environments, SubnetIDs, and Tags are sorted in live states.
 	if len(headSpec.Architectures) > 1 {
-		sort.Slice(headSpec.Architectures, func(i, j int) bool {
-			return strings.Compare(headSpec.Architectures[i].Name, headSpec.Architectures[j].Name) < 0
+		cloneSpec.Architectures = slices.Clone(headSpec.Architectures)
+		sort.Slice(cloneSpec.Architectures, func(i, j int) bool {
+			return strings.Compare(cloneSpec.Architectures[i].Name, cloneSpec.Architectures[j].Name) < 0
 		})
 	}
 	if headSpec.VPCConfig != nil && len(headSpec.VPCConfig.SubnetIDs) > 1 {
-		slices.Sort(headSpec.VPCConfig.SubnetIDs)
+		cloneSubnets := slices.Clone(headSpec.VPCConfig.SubnetIDs)
+		slices.Sort(cloneSubnets)
+		cloneSpec.VPCConfig = &provider.VPCConfig{
+			SecurityGroupIDs: headSpec.VPCConfig.SecurityGroupIDs,
+			SubnetIDs:        cloneSubnets,
+		}
 	}
+
+	return cloneSpec
 }
 
 func (d *detector) loadHeadFunctionManifest(app *model.Application, repo git.Repo, headCommit git.Commit) (provider.FunctionManifest, error) {
diff --git a/pkg/app/piped/driftdetector/lambda/detector_test.go b/pkg/app/piped/driftdetector/lambda/detector_test.go
index 415a8416a5..77b170a750 100644
--- a/pkg/app/piped/driftdetector/lambda/detector_test.go
+++ b/pkg/app/piped/driftdetector/lambda/detector_test.go
@@ -15,7 +15,6 @@
 package lambda
 
 import (
-	"fmt"
 	"testing"
 
 	"github.com/aws/aws-sdk-go-v2/service/lambda/types"
@@ -124,8 +123,7 @@ func TestIgnoreAndSortParameters(t *testing.T) {
 			expectDiff: false,
 		},
 		{
-
-			name: "Not ignore added fields in headstate",
+			name: "Not ignore added fields in headspec",
 			liveSpec: provider.FunctionManifestSpec{
 				Tags: map[string]string{
 					"key1":                    "value1",
@@ -149,17 +147,50 @@ func TestIgnoreAndSortParameters(t *testing.T) {
 		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
-			ignoreAndSortParameters(&tc.headSpec)
+			ignored := ignoreAndSortParameters(tc.headSpec)
 			result, err := provider.Diff(
 				provider.FunctionManifest{Spec: tc.liveSpec},
-				provider.FunctionManifest{Spec: tc.headSpec},
+				provider.FunctionManifest{Spec: ignored},
 				diff.WithEquateEmpty(),
 				diff.WithIgnoreAddingMapKeys(),
 				diff.WithCompareNumberAndNumericString(),
 			)
 			assert.NoError(t, err)
-			fmt.Println(result.Render(provider.DiffRenderOptions{}))
 			assert.Equal(t, tc.expectDiff, result.Diff.HasDiff())
 		})
 	}
 }
+
+func TestIgnoreAndSortParametersNotChangeOriginal(t *testing.T) {
+	t.Parallel()
+
+	headSpec := provider.FunctionManifestSpec{
+		S3Bucket: "test-bucket",
+		SourceCode: provider.SourceCode{
+			Git:  "https://test-repo.git",
+			Ref:  "test-ref",
+			Path: "test-path",
+		},
+		Architectures: []provider.Architecture{
+			{Name: string(types.ArchitectureX8664)},
+			{Name: string(types.ArchitectureArm64)},
+		},
+		VPCConfig: &provider.VPCConfig{
+			SubnetIDs: []string{"subnet-2", "subnet-1"},
+		},
+	}
+
+	_ = ignoreAndSortParameters(headSpec)
+
+	assert.Equal(t, "test-bucket", headSpec.S3Bucket)
+	assert.Equal(t, provider.SourceCode{
+		Git:  "https://test-repo.git",
+		Ref:  "test-ref",
+		Path: "test-path",
+	}, headSpec.SourceCode)
+	assert.Equal(t, []provider.Architecture{
+		{Name: string(types.ArchitectureX8664)},
+		{Name: string(types.ArchitectureArm64)}},
+		headSpec.Architectures)
+	assert.Equal(t, []string{"subnet-2", "subnet-1"}, headSpec.VPCConfig.SubnetIDs)
+}