Repository: crossplane-contrib/provider-aws (go)

Issue:
AWS EC2 Route Tables with Tags, never gets Ready
<!--
Thank you for helping to improve Crossplane!

Please be sure to search for open issues before raising a new one. We use issues
for bug reports and feature requests. Please find us at https://slack.crossplane.io/
for questions, support, and discussion.
-->

### What happened?
<!--
Please let us know what behaviour you expected and how Crossplane diverged from
that behaviour.
-->
After upgrading from version 0.49.2 to 0.52.0 we've detected that the EC2 RouteTable started updating non-stop.
We've been troubleshooting and debugging and it seems that the changes introduced in https://github.com/crossplane-contrib/provider-aws/commit/138b8cf25debbe75a98f5af919cb8e05b2850fd3 have changed the routetable.go code:
https://github.com/crossplane-contrib/provider-aws/blob/e02a4c2de22c21f188f5a514297b54e9bd7677cc/pkg/clients/ec2/routetable.go#L163

We have done some tests, and reverting back to `SortTagsV1Beta1` and also changing the 1st parameter to  `targetCopy.Tags`, we had success 
`SortTagsV1Beta1(targetCopy.Tags, in.Tags)`


### How can we reproduce it?
<!--
Help us to reproduce your bug as succinctly and precisely as possible. Artifacts
such as example manifests or a script that triggers the issue are highly
appreciated!
-->
Create a RouteTable CR in version 0.49.0 with a list of unordered tags, upgrade to 0.52.0 and the result with show itself.

### What environment did it happen in?
Crossplane version: 0.52.0
Cloud provider: AWS EKS 1.30


Pull request:
fix(ec2): Partial revert of 138b8cf to fix issue crossplane-contrib#2196
<!--
Thank you for helping to improve Crossplane!

Please read through https://git.io/fj2m9 if this is your first time opening a
Crossplane pull request. Find us in https://slack.crossplane.io/messages/dev if
you need any help contributing.
-->

### Description of your changes

This PR reverts partially reverts the changes of 138b8cf to not detect changes when only the order of the tags change between what is returned by AWS and what is being submitted on the `RouteTable` CR.

The revert of this change will avoid detecting order-related differences because the `SortTagsV1Beta1(targetCopy.Tags, in.Tags)` function changes the order of `targetCopy.Tags` and `in.Tags` by reference which enables a proper comparison afterwards in:
`jsonPatch, err := jsonpatch.CreateJSONPatch(*currentParams, targetCopy)`

The `currentParams.Tags` will have the same tag order of `in.Tags` from the `LateInitializeRT(currentParams, &in)`.

Note: the `CompareTags` added in 138b8cf is effectively doing nothing as the result is not being used and nothing is changed by reference.

<!--
Briefly describe what this pull request does. Be sure to direct your reviewers'
attention to anything that needs special consideration.

We love pull requests that resolve an open Crossplane issue. If yours does, you
can uncomment the below line to indicate which issue your PR fixes, for example
"Fixes #500":

-->
Fixes #2196

I have:

- [X] Read and followed Crossplane's [contribution process].
- [x] Run `make reviewable test` to ensure this PR is ready for review.

### How has this code been tested

Added new unit test that validates the different order of the tags will still return an empty patch result (test fails with current code).

<!--
Before reviewers can be confident in the correctness of this pull request, it
needs to tested and shown to be correct. Briefly describe the testing that has
already been done or which is planned for this change.
-->

[contribution process]: https://git.io/fj2m9


Patch:
diff --git a/pkg/clients/ec2/routetable.go b/pkg/clients/ec2/routetable.go
index ff5ddf437..54a75dc01 100644
--- a/pkg/clients/ec2/routetable.go
+++ b/pkg/clients/ec2/routetable.go
@@ -160,7 +160,7 @@ func CreateRTPatch(in ec2types.RouteTable, target v1beta1.RouteTableParameters)
 	targetCopy := target.DeepCopy()
 	currentParams := &v1beta1.RouteTableParameters{}
 
-	CompareTags(target.Tags, in.Tags)
+	SortTagsV1Beta1(targetCopy.Tags, in.Tags)
 
 	if !pointer.BoolValue(target.IgnoreRoutes) {
 		// Add the default route for fair comparison.
diff --git a/pkg/clients/ec2/tags_v1beta1.go b/pkg/clients/ec2/tags_v1beta1.go
index 46b34a9a2..1f376714d 100644
--- a/pkg/clients/ec2/tags_v1beta1.go
+++ b/pkg/clients/ec2/tags_v1beta1.go
@@ -17,6 +17,8 @@ limitations under the License.
 package ec2
 
 import (
+	"sort"
+
 	"github.com/aws/aws-sdk-go-v2/aws"
 	ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types"
 
@@ -53,3 +55,14 @@ func CompareTags(spec []svcapitypes.Tag, current []ec2types.Tag) bool {
 	toAdd, toRemove := DiffEC2Tags(GenerateEC2TagsV1Beta1(spec), current)
 	return len(toAdd) == 0 && len(toRemove) == 0
 }
+
+// SortTags sorts array of v1beta1.Tag and ec2type.Tag on 'Key'
+func SortTagsV1Beta1(tags []svcapitypes.Tag, ec2Tags []ec2types.Tag) {
+	sort.Slice(tags, func(i, j int) bool {
+		return tags[i].Key < tags[j].Key
+	})
+
+	sort.Slice(ec2Tags, func(i, j int) bool {
+		return *ec2Tags[i].Key < *ec2Tags[j].Key
+	})
+}


Test patch:
diff --git a/pkg/clients/ec2/routetable_test.go b/pkg/clients/ec2/routetable_test.go
index 6c7551e84..cc8e809a6 100644
--- a/pkg/clients/ec2/routetable_test.go
+++ b/pkg/clients/ec2/routetable_test.go
@@ -12,11 +12,15 @@ import (
 )
 
 var (
-	rtVPC      = "some vpc"
-	otherRtVPC = "some other vpc"
-	rtID       = "some RT Id"
-	rtSubnetID = "some subnet"
-	rtOwner    = "some owner"
+	rtVPC           = "some vpc"
+	otherRtVPC      = "some other vpc"
+	rtID            = "some RT Id"
+	rtSubnetID      = "some subnet"
+	rtOwner         = "some owner"
+	rtTagName       = "tag1"
+	rtTagValue      = "value1"
+	otherRtTagName  = "tag2"
+	otherRtTagValue = "value2"
 )
 
 func specAssociations() []v1beta1.Association {
@@ -160,6 +164,23 @@ func TestCreateRTPatch(t *testing.T) {
 				},
 			},
 		},
+		"DifferentTagOrder": {
+			args: args{
+				rt: ec2types.RouteTable{
+					Tags:         []ec2types.Tag{{Key: &rtTagName, Value: &rtTagValue}, {Key: &otherRtTagName, Value: &otherRtTagValue}},
+					Associations: rtAssociations(),
+					VpcId:        aws.String(rtVPC),
+				},
+				p: &v1beta1.RouteTableParameters{
+					Tags:         []v1beta1.Tag{{Key: otherRtTagName, Value: otherRtTagValue}, {Key: rtTagName, Value: rtTagValue}},
+					Associations: specAssociations(),
+					VPCID:        aws.String(vpcID),
+				},
+			},
+			want: want{
+				patch: &v1beta1.RouteTableParameters{},
+			},
+		},
 	}
 
 	for name, tc := range cases {