Skip to content

Commit

Permalink
fix(topic): Improve topic policy json diff check
Browse files Browse the repository at this point in the history
Signed-off-by: Xabi <xmartinez1702@gmail.com>
  • Loading branch information
x4b1 committed Jun 26, 2023
1 parent 6216ce7 commit fd65255
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 8 deletions.
35 changes: 32 additions & 3 deletions pkg/clients/sns/topic.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down
57 changes: 55 additions & 2 deletions pkg/clients/sns/topic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -190,7 +198,6 @@ func TestGenerateCreateTopicInput(t *testing.T) {
}

func TestGetChangedAttributes(t *testing.T) {

type args struct {
p v1beta1.TopicParameters
attr *map[string]string
Expand Down Expand Up @@ -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)
}
Expand Down
9 changes: 6 additions & 3 deletions pkg/controller/sns/topic/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
Expand Down

0 comments on commit fd65255

Please sign in to comment.