Cherry-pick #5344 #5351 (#5352)

* Handle ECS Apps DeploymentConfiguration and containers Environments drift detection (#5344) * Handle ECS Apps DeploymentConfiguration and containers Environments driff detection Signed-off-by: HoangNguyen689 <[email protected]> * applied Signed-off-by: HoangNguyen689 <[email protected]> --------- Signed-off-by: HoangNguyen689 <[email protected]> Signed-off-by: pipecd-bot <[email protected]> * Update RELEASE to v0.49.4 (#5351) Signed-off-by: Yoshiki Fujikane <[email protected]> Signed-off-by: pipecd-bot <[email protected]> --------- Signed-off-by: HoangNguyen689 <[email protected]> Signed-off-by: pipecd-bot <[email protected]> Signed-off-by: Yoshiki Fujikane <[email protected]> Co-authored-by: HoangNguyen689 <[email protected]> Co-authored-by: Yoshiki Fujikane <[email protected]>
pipe-cd · Nov 20, 2024 · af2d227 · af2d227
1 parent 7bbc4d5
commit af2d227
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 1 deletion.
diff --git a/RELEASE b/RELEASE
@@ -1,6 +1,6 @@
 # Generated by `make release` command.
 # DO NOT EDIT.
-tag: v0.49.3
+tag: v0.49.4
 
 releaseNoteGenerator:
   showCommitter: false

diff --git a/pkg/app/piped/driftdetector/ecs/detector.go b/pkg/app/piped/driftdetector/ecs/detector.go
@@ -20,6 +20,7 @@ import (
 	"os"
 	"path/filepath"
 	"slices"
+	"sort"
 	"strings"
 	"time"
 
@@ -235,6 +236,7 @@ func (d *detector) checkApplication(ctx context.Context, app *model.Application,
 //   - taskDefinition.ContainerDefinitions[].PortMappings[].HostPort
 //
 // TODO: Maybe we should check diff of following fields when not set in the head manifests in some way. Currently they are ignored:
+//   - service.DeploymentConfiguration
 //   - service.PlatformVersion
 //   - service.RoleArn
 func ignoreParameters(liveManifests provider.ECSManifests, headManifests provider.ECSManifests) (live, head provider.ECSManifests) {
@@ -261,6 +263,8 @@ func ignoreParameters(liveManifests provider.ECSManifests, headManifests provide
 		liveTask.Revision = 0 // TODO: Find a way to compare the revision if possible.
 		liveTask.TaskDefinitionArn = nil
 		for i := range liveTask.ContainerDefinitions {
+			liveTask.ContainerDefinitions[i].Environment = sortKeyPairs(liveTask.ContainerDefinitions[i].Environment)
+
 			for j := range liveTask.ContainerDefinitions[i].PortMappings {
 				// We ignore diff of HostPort because it has several default values. See https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_ContainerDefinition.html#ECS-Type-ContainerDefinition-portMappings.
 				liveTask.ContainerDefinitions[i].PortMappings[j].HostPort = nil
@@ -299,6 +303,10 @@ func ignoreParameters(liveManifests provider.ECSManifests, headManifests provide
 		liveService.NetworkConfiguration = &types.NetworkConfiguration{AwsvpcConfiguration: &awsvpcCfg}
 	}
 
+	if headService.DeploymentConfiguration == nil {
+		liveService.DeploymentConfiguration = nil
+	}
+
 	// TODO: In order to check diff of the tags, we need to add pipecd-managed tags and sort.
 	liveService.Tags = nil
 	headService.Tags = nil
@@ -316,6 +324,9 @@ func ignoreParameters(liveManifests provider.ECSManifests, headManifests provide
 			// Essential is true by default. See https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_ContainerDefinition.html#ECS-Type-ContainerDefinition-es.
 			cd.Essential = aws.Bool(true)
 		}
+
+		cd.Environment = sortKeyPairs(cd.Environment)
+
 		cd.PortMappings = slices.Clone(cd.PortMappings)
 		for j := range cd.PortMappings {
 			pm := &cd.PortMappings[j]
@@ -480,3 +491,12 @@ func ignoreAutoScalingDiff(r *provider.DiffResult) bool {
 		r.New.ServiceDefinition.DesiredCount == 0 && // When desiredCount is 0 or not defined in the head manifest, autoscaling may be enabled.
 		r.Old.ServiceDefinition.DesiredCount != r.New.ServiceDefinition.DesiredCount
 }
+
+func sortKeyPairs(kps []types.KeyValuePair) []types.KeyValuePair {
+	sorted := slices.Clone(kps)
+	sort.Slice(sorted, func(i, j int) bool {
+		return *sorted[i].Name < *sorted[j].Name
+	})
+
+	return sorted
+}
diff --git a/pkg/app/piped/driftdetector/ecs/detector_test.go b/pkg/app/piped/driftdetector/ecs/detector_test.go
@@ -49,6 +49,10 @@ func TestIgnoreParameters(t *testing.T) {
 					SecurityGroups: []string{"1_test-sg", "0_test-sg"},
 				},
 			},
+			DeploymentConfiguration: &types.DeploymentConfiguration{
+				MaximumPercent:        aws.Int32(200),
+				MinimumHealthyPercent: aws.Int32(100),
+			},
 			PendingCount:    3,
 			PlatformFamily:  aws.String("LINUX"),
 			PlatformVersion: aws.String("1.4"),
@@ -104,6 +108,16 @@ func TestIgnoreParameters(t *testing.T) {
 							Protocol: types.TransportProtocolTcp,
 						},
 					},
+					Environment: []types.KeyValuePair{
+						{
+							Name:  aws.String("A-TEST-ENV"),
+							Value: aws.String("a-test-value"),
+						},
+						{
+							Name:  aws.String("B-TEST-ENV"),
+							Value: aws.String("b-test-value"),
+						},
+					},
 				},
 				{
 					Essential: aws.Bool(true),
@@ -149,6 +163,16 @@ func TestIgnoreParameters(t *testing.T) {
 						// Protocol will be automatically tcp
 						{}, {},
 					},
+					Environment: []types.KeyValuePair{
+						{
+							Name:  aws.String("B-TEST-ENV"),
+							Value: aws.String("b-test-value"),
+						},
+						{
+							Name:  aws.String("A-TEST-ENV"),
+							Value: aws.String("a-test-value"),
+						},
+					},
 				},
 				{
 					// Use default value for 'Essential'