Merge pull request #1528 from erhancagirici/lb-listener-custom-diff

add custom diff for LBListener forward options

Makefile

@@ -12,7 +12,7 @@ PROJECT_REPO := github.com/upbound/$(PROJECT_NAME)
 
 export TERRAFORM_VERSION := 1.5.5
 export TERRAFORM_PROVIDER_VERSION := 5.68.0
-export TERRAFORM_PROVIDER_RELEASE := v$(TERRAFORM_PROVIDER_VERSION)-upjet.1
+export TERRAFORM_PROVIDER_RELEASE := v$(TERRAFORM_PROVIDER_VERSION)-upjet.2
 export TERRAFORM_PROVIDER_SOURCE := hashicorp/aws
 export TERRAFORM_PROVIDER_REPO ?= https://github.com/hashicorp/terraform-provider-aws
 export TERRAFORM_DOCS_PATH ?= website/docs/r

config/elbv2/config.go

@@ -4,10 +4,17 @@
 
 package elbv2
 
-import "github.com/crossplane/upjet/pkg/config"
+import (
+	"regexp"
+	"strings"
+
+	"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
+
+	"github.com/crossplane/upjet/pkg/config"
+)
 
 // Configure adds configurations for the elbv2 group.
-func Configure(p *config.Provider) {
+func Configure(p *config.Provider) { //nolint:gocyclo
 	p.AddResourceConfigurator("aws_lb", func(r *config.Resource) {
 		r.ExternalName.OmittedFields = append(r.ExternalName.OmittedFields, "name_prefix")
 		r.References = config.References{
@@ -44,6 +51,64 @@ func Configure(p *config.Provider) {
 				TerraformName: "aws_lb_target_group",
 			},
 		}
+
+		// lb_listener schema allows to configure "default_action" with type
+		// "forward", in 2 different ways.
+		// 1. you can specify, default_action.0.forward, which allows configuring
+		//    multiple target groups.
+		// 2. you can specify default_action.0.target_group_arn if you want
+		//    to configure only one target group.
+		// Former is a more general way, and latter is more of a shortcut for
+		// a specific case, which can already be expressed with #1.
+		// TF implementation instructs to specify either #1 or #2, not both.
+		// However, they both end up in the state redundantly and cause
+		// unnecessary diff.
+		r.TerraformCustomDiff = func(diff *terraform.InstanceDiff, _ *terraform.InstanceState, _ *terraform.ResourceConfig) (*terraform.InstanceDiff, error) { //nolint:gocyclo
+			// skip no diff or destroy diffs
+			if diff == nil || diff.Empty() || diff.Destroy || diff.Attributes == nil {
+				return diff, nil
+			}
+			duplicatedAction := ""
+			for k, attrDiff := range diff.Attributes {
+				// user specified the "default_action.0.target_group_arn" and
+				// "default_action.0.forward" is not specified in config.
+				// In that case, default_action.0.forward is populated
+				// by the AWS API, which can cause an unnecessary diff,
+				// trying to remove that "auto-populated" element.
+				if regexp.MustCompile(`^default_action\.\d+\.forward\.#$`).MatchString(k) &&
+					attrDiff.New == "0" && attrDiff.Old == "1" {
+					delete(diff.Attributes, k)
+					// save this attribute path to remove remaining diffs
+					// of its nested fields if any in a second pass
+					duplicatedAction = strings.TrimSuffix(k, ".#")
+				}
+				// this is the same case as above
+				if regexp.MustCompile(`^default_action\.\d+\.forward\.\d+\.target_group\.#$`).MatchString(k) &&
+					attrDiff.New == "0" && attrDiff.Old == "1" {
+					delete(diff.Attributes, k)
+				}
+				// this is the same case but vice versa. user specified
+				// forward target via default_action.0.forward, and
+				// default_action.0.target_group_arn is omitted.
+				// In that case, default_action.0.target_group_arn is populated
+				// by the AWS API and ends up in the state, causing
+				// an unnecessary diff, which we omit here.
+				if regexp.MustCompile(`^default_action\.\d+\.target_group_arn$`).MatchString(k) &&
+					attrDiff.New == "" && attrDiff.Old != "" && attrDiff.NewRemoved {
+					delete(diff.Attributes, k)
+				}
+			}
+			// if we have caught an unnecessary diff for default_action.0.forward, remove
+			// any sub-element diffs of it.
+			if duplicatedAction != "" {
+				for k := range diff.Attributes {
+					if strings.HasPrefix(k, duplicatedAction) {
+						delete(diff.Attributes, k)
+					}
+				}
+			}
+			return diff, nil
+		}
 	})
 
 	p.AddResourceConfigurator("aws_lb_target_group", func(r *config.Resource) {
@@ -54,6 +119,19 @@ func Configure(p *config.Provider) {
 			s.Computed = false
 		}
 		r.LateInitializer.IgnoredFields = []string{"target_failover"}
+		r.TerraformCustomDiff = func(diff *terraform.InstanceDiff, _ *terraform.InstanceState, _ *terraform.ResourceConfig) (*terraform.InstanceDiff, error) {
+			// skip no diff or destroy diffs
+			if diff == nil || diff.Empty() || diff.Destroy || diff.Attributes == nil {
+				return diff, nil
+			}
+
+			// ignore diff due to defaulting in the TF schema
+			udiDiff, ok := diff.Attributes["target_health_state.0.unhealthy_draining_interval"]
+			if ok && udiDiff.Old == "" && udiDiff.New == "0" {
+				delete(diff.Attributes, "target_health_state.0.unhealthy_draining_interval")
+			}
+			return diff, nil
+		}
 	})
 
 	p.AddResourceConfigurator("aws_lb_target_group_attachment", func(r *config.Resource) {

examples/elbv2/v1beta2/lblistener-forward-multiple-targetgroups.yaml

@@ -0,0 +1,195 @@
+# SPDX-FileCopyrightText: 2024 The Crossplane Authors <https://crossplane.io>
+#
+# SPDX-License-Identifier: Apache-2.0
+
+apiVersion: elbv2.aws.upbound.io/v1beta2
+kind: LBListener
+metadata:
+  annotations:
+    meta.upbound.io/example-id: elbv2/v1beta2/lblistener
+  labels:
+    testing.upbound.io/example-name: lblistener-forward-multiple
+  name: front-end
+spec:
+  forProvider:
+    defaultAction:
+    - type: forward
+      forward:
+        targetGroup:
+          - arnSelector:
+              matchLabels:
+                testing.upbound.io/example-name: lblistener-forward-multiple
+          - arnSelector:
+              matchLabels:
+                testing.upbound.io/example-name: lblistener-forward-multiple-2
+        stickiness:
+          enabled: true
+          duration: 3600
+    loadBalancerArnSelector:
+      matchLabels:
+        testing.upbound.io/example-name: lblistener-forward-multiple
+    port: 80
+    protocol: HTTP
+    region: us-west-1
+---
+
+apiVersion: elbv2.aws.upbound.io/v1beta2
+kind: LBTargetGroup
+metadata:
+  annotations:
+    meta.upbound.io/example-id: elbv2/v1beta2/lblistener
+  labels:
+    testing.upbound.io/example-name: lblistener-forward-multiple-2
+  name: test-2
+spec:
+  forProvider:
+    name: example-lb-tg-2
+    port: 8080
+    protocol: HTTP
+    region: us-west-1
+    targetType: ip
+    vpcIdSelector:
+      matchLabels:
+        testing.upbound.io/example-name: lblistener-forward-multiple
+
+---
+
+apiVersion: elbv2.aws.upbound.io/v1beta2
+kind: LBTargetGroup
+metadata:
+  annotations:
+    meta.upbound.io/example-id: elbv2/v1beta2/lblistener
+  labels:
+    testing.upbound.io/example-name: lblistener-forward-multiple
+  name: test
+spec:
+  forProvider:
+    name: example-lb-tg
+    port: 80
+    protocol: HTTP
+    region: us-west-1
+    targetType: ip
+    vpcIdSelector:
+      matchLabels:
+        testing.upbound.io/example-name: lblistener-forward-multiple
+
+---
+
+apiVersion: elbv2.aws.upbound.io/v1beta2
+kind: LB
+metadata:
+  annotations:
+    meta.upbound.io/example-id: elbv2/v1beta2/lblistener
+  labels:
+    testing.upbound.io/example-name: lblistener-forward-multiple
+  name: test
+spec:
+  forProvider:
+    enableDeletionProtection: false
+    internal: false
+    loadBalancerType: application
+    name: test-lb-tf
+    region: us-west-1
+    securityGroupSelector:
+      matchLabels:
+        testing.upbound.io/example-name: lblistener-forward-multiple
+    subnetSelector:
+      matchLabels:
+        testing.upbound.io/example-name: lblistener-forward-multiple
+    tags:
+      Environment: production
+
+---
+
+apiVersion: ec2.aws.upbound.io/v1beta1
+kind: VPC
+metadata:
+  annotations:
+    meta.upbound.io/example-id: elbv2/v1beta2/lblistener
+  labels:
+    testing.upbound.io/example-name: lblistener-forward-multiple
+  name: elbv2-vpc
+spec:
+  forProvider:
+    cidrBlock: 172.16.0.0/16
+    region: us-west-1
+    tags:
+      Name: DemoVpc
+
+---
+
+apiVersion: ec2.aws.upbound.io/v1beta1
+kind: InternetGateway
+metadata:
+  annotations:
+    meta.upbound.io/example-id: elbv2/v1beta2/lblistener
+  labels:
+    testing.upbound.io/example-name: lblistener-forward-multiple
+  name: elbv2-internet-gateway
+spec:
+  forProvider:
+    region: us-west-1
+    tags:
+      Name: testing
+    vpcIdSelector:
+      matchLabels:
+        testing.upbound.io/example-name: lblistener-forward-multiple
+
+---
+
+apiVersion: ec2.aws.upbound.io/v1beta1
+kind: Subnet
+metadata:
+  annotations:
+    meta.upbound.io/example-id: elbv2/v1beta2/lblistener
+  labels:
+    testing.upbound.io/example-name: lblistener-forward-multiple
+  name: elbv2-subnet
+spec:
+  forProvider:
+    availabilityZone: us-west-1a
+    cidrBlock: 172.16.10.0/24
+    region: us-west-1
+    vpcIdSelector:
+      matchLabels:
+        testing.upbound.io/example-name: lblistener-forward-multiple
+
+---
+
+apiVersion: ec2.aws.upbound.io/v1beta1
+kind: Subnet
+metadata:
+  annotations:
+    meta.upbound.io/example-id: elbv2/v1beta2/lblistener
+  labels:
+    testing.upbound.io/example-name: lblistener-forward-multiple
+  name: elbv2-subnet-2
+spec:
+  forProvider:
+    availabilityZone: us-west-1b
+    cidrBlock: 172.16.20.0/24
+    region: us-west-1
+    vpcIdSelector:
+      matchLabels:
+        testing.upbound.io/example-name: lblistener-forward-multiple
+
+---
+
+apiVersion: ec2.aws.upbound.io/v1beta1
+kind: SecurityGroup
+metadata:
+  annotations:
+    meta.upbound.io/example-id: elbv2/v1beta2/lblistener
+  labels:
+    testing.upbound.io/example-name: lblistener-forward-multiple
+  name: elbv2-securitygroup
+spec:
+  forProvider:
+    description: Allow TLS inbound traffic
+    name: allow_tls
+    region: us-west-1
+    tags:
+      Name: allow_tls
+    vpcIdSelector:
+      matchLabels:
+        testing.upbound.io/example-name: lblistener-forward-multiple