fix(topic): Improve topic policy json diff check

Signed-off-by: Xabi <[email protected]>
Signed-off-by: Xabi <[email protected]>

pkg/clients/sns/topic.go

@@ -23,6 +23,7 @@ import (
 
 	"github.com/crossplane-contrib/provider-aws/apis/sns/v1beta1"
 	awsclients "github.com/crossplane-contrib/provider-aws/pkg/clients"
+	policyutils "github.com/crossplane-contrib/provider-aws/pkg/utils/policy"
 
 	"github.com/aws/aws-sdk-go-v2/aws"
 	"github.com/aws/aws-sdk-go-v2/service/sns"
@@ -152,18 +153,46 @@ func GenerateTopicObservation(attr map[string]string) v1beta1.TopicObservation {
 }
 
 // IsSNSTopicUpToDate checks if object is up to date
-func IsSNSTopicUpToDate(p v1beta1.TopicParameters, attr map[string]string) bool {
+func IsSNSTopicUpToDate(p v1beta1.TopicParameters, attr map[string]string) (bool, error) {
 	fifoTopic, _ := strconv.ParseBool(attr[string(TopicFifoTopic)])
 
+	policyChanged, err := IsSNSPolicyChanged(p, attr)
+	if err != nil {
+		return false, err
+	}
+
 	return aws.ToString(p.DeliveryPolicy) == attr[string(TopicDeliveryPolicy)] &&
 		aws.ToString(p.DisplayName) == attr[string(TopicDisplayName)] &&
 		aws.ToString(p.KMSMasterKeyID) == attr[string(TopicKmsMasterKeyID)] &&
 		aws.ToBool(p.FifoTopic) == fifoTopic &&
-		aws.ToString(p.Policy) == attr[string(TopicPolicy)]
+		!policyChanged, nil
 }
 
-func getTopicAttributes(p v1beta1.TopicParameters) map[string]string {
+// IsSNSPolicyChanged determines whether a SNS topic policy needs to be updated
+func IsSNSPolicyChanged(p v1beta1.TopicParameters, attr map[string]string) (bool, error) {
+	currPolicyStr := attr[string(TopicPolicy)]
+	specPolicyStr := awsclients.StringValue(p.Policy)
+
+	if currPolicyStr == specPolicyStr {
+		return false, nil
+	}
+
+	currPolicy, err := policyutils.ParsePolicyString(attr[string(TopicPolicy)])
+	if err != nil {
+		return false, err
+	}
 
+	specPolicy, err := policyutils.ParsePolicyString(awsclients.StringValue(p.Policy))
+	if err != nil {
+		return false, err
+	}
+
+	equalPolicies, _ := policyutils.ArePoliciesEqal(&currPolicy, &specPolicy)
+
+	return equalPolicies, nil
+}
+
+func getTopicAttributes(p v1beta1.TopicParameters) map[string]string {
 	topicAttr := make(map[string]string)
 
 	topicAttr[string(TopicDeliveryPolicy)] = aws.ToString(p.DeliveryPolicy)

pkg/clients/sns/topic_test.go

@@ -29,6 +29,8 @@ import (
 	awssns "github.com/aws/aws-sdk-go-v2/service/sns"
 	awssnstypes "github.com/aws/aws-sdk-go-v2/service/sns/types"
 	"github.com/google/go-cmp/cmp"
+
+	awsclients "github.com/crossplane-contrib/provider-aws/pkg/clients"
 )
 
 var (
@@ -95,6 +97,12 @@ func withAttrFifoTopic(b *bool) topicAttrModifier {
 	}
 }
 
+func withAttrPolicy(s *string) topicAttrModifier {
+	return func(attr *map[string]string) {
+		(*attr)[string(TopicPolicy)] = *s
+	}
+}
+
 // topic Observation Modifier
 type topicObservationModifier func(*v1beta1.TopicObservation)
 
@@ -190,7 +198,6 @@ func TestGenerateCreateTopicInput(t *testing.T) {
 }
 
 func TestGetChangedAttributes(t *testing.T) {
-
 	type args struct {
 		p    v1beta1.TopicParameters
 		attr *map[string]string
@@ -330,11 +337,57 @@ func TestIsSNSTopicUpToDate(t *testing.T) {
 			},
 			want: false,
 		},
+		"NoUpdateExistsWithshuffledPolicy": {
+			args: args{
+				attr: topicAttributes(
+					withAttrPolicy(awsclients.String(`{
+						"Version": "2012-10-17",
+						"Statement": [
+							{
+								"Sid": "PublishToTopic",
+								"Effect": "Allow",
+								"Principal": {
+									"AWS": "arn:aws:iam::*****:role/my-role"
+								},
+								"Action": [
+									"SNS:Publish",
+									"SNS:GetTopicAttributes"
+								],
+								"Resource": "arn:aws:sns:eu-west-1:******:my-queue"
+							}
+						]
+					}`)),
+				),
+				p: v1beta1.TopicParameters{
+					Policy: awsclients.String(`{
+						"Statement": [
+							{
+								"Sid": "PublishToTopic",
+								"Effect": "Allow",
+								"Principal": {
+									"AWS": "arn:aws:iam::*****:role/my-role"
+								},
+								"Action": [
+									"SNS:Publish",
+									"SNS:GetTopicAttributes"
+								],
+								"Resource": "arn:aws:sns:eu-west-1:******:my-queue"
+							}
+						],
+						"Version": "2012-10-17"
+					}`),
+				},
+			},
+			want: false,
+		},
 	}
 
 	for name, tc := range cases {
 		t.Run(name, func(t *testing.T) {
-			got := IsSNSTopicUpToDate(tc.args.p, *tc.args.attr)
+			got, err := IsSNSTopicUpToDate(tc.args.p, *tc.args.attr)
+			if err != nil {
+				t.Error(err)
+			}
 			if diff := cmp.Diff(tc.want, got); diff != "" {
 				t.Errorf("Topic : -want, +got:\n%s", diff)
 			}

pkg/controller/sns/topic/controller.go

@@ -125,15 +125,19 @@ func (e *external) Observe(ctx context.Context, mgd resource.Managed) (managed.E
 	// GenerateObservation for SNS Topic
 	cr.Status.AtProvider = snsclient.GenerateTopicObservation(res.Attributes)
 
+	upToDate, err := snsclient.IsSNSTopicUpToDate(cr.Spec.ForProvider, res.Attributes)
+	if err != nil {
+		return managed.ExternalObservation{}, err
+	}
+
 	return managed.ExternalObservation{
 		ResourceExists:          true,
-		ResourceUpToDate:        snsclient.IsSNSTopicUpToDate(cr.Spec.ForProvider, res.Attributes),
+		ResourceUpToDate:        upToDate,
 		ResourceLateInitialized: !reflect.DeepEqual(current, &cr.Spec.ForProvider),
 	}, nil
 }
 
 func (e *external) Create(ctx context.Context, mgd resource.Managed) (managed.ExternalCreation, error) {
-
 	cr, ok := mgd.(*v1beta1.Topic)
 	if !ok {
 		return managed.ExternalCreation{}, errors.New(errUnexpectedObject)
@@ -170,7 +174,6 @@ func (e *external) Update(ctx context.Context, mgd resource.Managed) (managed.Ex
 			AttributeValue: aws.String(v),
 			TopicArn:       aws.String(meta.GetExternalName(cr)),
 		})
-
 	}
 	return managed.ExternalUpdate{}, awsclient.Wrap(err, errUpdate)
 }