From 55c4a525466cf6792f86431eb176bc4d182a5dd8 Mon Sep 17 00:00:00 2001 From: Lucas Caparelli Date: Thu, 28 Sep 2023 20:24:08 -0300 Subject: [PATCH] fix(ec2/SecurityGroupRule): respect prefixListId Signed-off-by: Lucas Caparelli --- .../ec2/securitygrouprule/controller.go | 86 ++++--- .../ec2/securitygrouprule/controller_test.go | 225 +++++++++++++++++- 2 files changed, 283 insertions(+), 28 deletions(-) diff --git a/pkg/controller/ec2/securitygrouprule/controller.go b/pkg/controller/ec2/securitygrouprule/controller.go index c965d205e6..9f8f5413c7 100644 --- a/pkg/controller/ec2/securitygrouprule/controller.go +++ b/pkg/controller/ec2/securitygrouprule/controller.go @@ -178,6 +178,17 @@ func (e *external) createSgr(ctx context.Context, sgr *manualv1alpha1.SecurityGr }}, }} } + if providerValues.PrefixListID != nil { + input.IpPermissions = []awsec2types.IpPermission{{ + FromPort: providerValues.FromPort, + ToPort: providerValues.ToPort, + IpProtocol: providerValues.Protocol, + PrefixListIds: []awsec2types.PrefixListId{{ + Description: providerValues.Description, + PrefixListId: providerValues.PrefixListID, + }}, + }} + } result, err := e.client.AuthorizeSecurityGroupIngress(ctx, input) if err != nil { @@ -232,6 +243,17 @@ func (e *external) createSgr(ctx context.Context, sgr *manualv1alpha1.SecurityGr }}, }} } + if providerValues.PrefixListID != nil { + input.IpPermissions = []awsec2types.IpPermission{{ + FromPort: providerValues.FromPort, + ToPort: providerValues.ToPort, + IpProtocol: providerValues.Protocol, + PrefixListIds: []awsec2types.PrefixListId{{ + Description: providerValues.Description, + PrefixListId: providerValues.PrefixListID, + }}, + }} + } result, err := e.client.AuthorizeSecurityGroupEgress(ctx, input) if err != nil { @@ -320,37 +342,42 @@ func (e *external) Update(ctx context.Context, mgd resource.Managed) (managed.Ex // We return an error to fore a recreation, as we cant create a new sgr and update externalName here return managed.ExternalUpdate{}, errors.New("Update needs recreation") } - externalName := meta.GetExternalName(cr) - input := &awsec2.ModifySecurityGroupRulesInput{ - GroupId: cr.Spec.ForProvider.SecurityGroupID, - SecurityGroupRules: []awsec2types.SecurityGroupRuleUpdate{{ - SecurityGroupRuleId: &externalName, - SecurityGroupRule: &awsec2types.SecurityGroupRuleRequest{ - FromPort: cr.Spec.ForProvider.FromPort, - ToPort: cr.Spec.ForProvider.ToPort, - Description: cr.Spec.ForProvider.Description, - IpProtocol: cr.Spec.ForProvider.Protocol, - }, - }}, - } - if cr.Spec.ForProvider.CidrBlock != nil { - input.SecurityGroupRules[0].SecurityGroupRule.CidrIpv4 = cr.Spec.ForProvider.CidrBlock - } - if cr.Spec.ForProvider.Ipv6CidrBlock != nil { - input.SecurityGroupRules[0].SecurityGroupRule.CidrIpv6 = cr.Spec.ForProvider.Ipv6CidrBlock - } - if cr.Spec.ForProvider.SourceSecurityGroupID != nil { - input.SecurityGroupRules[0].SecurityGroupRule.ReferencedGroupId = cr.Spec.ForProvider.SourceSecurityGroupID - } - - _, err := e.client.ModifySecurityGroupRules(ctx, input) - if err != nil { - return managed.ExternalUpdate{}, err - } + return e.updateSgr(ctx, cr) } return managed.ExternalUpdate{}, nil } +func (e *external) updateSgr(ctx context.Context, cr *manualv1alpha1.SecurityGroupRule) (managed.ExternalUpdate, error) { + externalName := meta.GetExternalName(cr) + input := &awsec2.ModifySecurityGroupRulesInput{ + GroupId: cr.Spec.ForProvider.SecurityGroupID, + SecurityGroupRules: []awsec2types.SecurityGroupRuleUpdate{{ + SecurityGroupRuleId: &externalName, + SecurityGroupRule: &awsec2types.SecurityGroupRuleRequest{ + FromPort: cr.Spec.ForProvider.FromPort, + ToPort: cr.Spec.ForProvider.ToPort, + Description: cr.Spec.ForProvider.Description, + IpProtocol: cr.Spec.ForProvider.Protocol, + }, + }}, + } + if cr.Spec.ForProvider.CidrBlock != nil { + input.SecurityGroupRules[0].SecurityGroupRule.CidrIpv4 = cr.Spec.ForProvider.CidrBlock + } + if cr.Spec.ForProvider.Ipv6CidrBlock != nil { + input.SecurityGroupRules[0].SecurityGroupRule.CidrIpv6 = cr.Spec.ForProvider.Ipv6CidrBlock + } + if cr.Spec.ForProvider.SourceSecurityGroupID != nil { + input.SecurityGroupRules[0].SecurityGroupRule.ReferencedGroupId = cr.Spec.ForProvider.SourceSecurityGroupID + } + if cr.Spec.ForProvider.PrefixListID != nil { + input.SecurityGroupRules[0].SecurityGroupRule.PrefixListId = cr.Spec.ForProvider.PrefixListID + } + + _, err := e.client.ModifySecurityGroupRules(ctx, input) + return managed.ExternalUpdate{}, err +} + func compareSgr(desired *manualv1alpha1.SecurityGroupRuleParameters, actual *manualv1alpha1.SecurityGroupRuleParameters) (needsUpdate bool, recreate bool, typechange bool) { needsUpdate = false @@ -381,6 +408,10 @@ func compareSgr(desired *manualv1alpha1.SecurityGroupRuleParameters, actual *man needsUpdate = true } + if awsclient.StringValue(desired.PrefixListID) != awsclient.StringValue(actual.PrefixListID) { + needsUpdate = true + } + if awsclient.StringValue(desired.Type) != awsclient.StringValue(actual.Type) { needsUpdate = true recreate = true @@ -412,6 +443,7 @@ func (e *external) getExternalSgr(ctx context.Context, externalName string) (*ma Protocol: existingSgr.IpProtocol, CidrBlock: existingSgr.CidrIpv4, Ipv6CidrBlock: existingSgr.CidrIpv6, + PrefixListID: existingSgr.PrefixListId, } if existingSgr.ReferencedGroupInfo != nil { cr.SourceSecurityGroupID = existingSgr.ReferencedGroupInfo.GroupId diff --git a/pkg/controller/ec2/securitygrouprule/controller_test.go b/pkg/controller/ec2/securitygrouprule/controller_test.go index 571436d396..fe12235604 100644 --- a/pkg/controller/ec2/securitygrouprule/controller_test.go +++ b/pkg/controller/ec2/securitygrouprule/controller_test.go @@ -29,6 +29,7 @@ var ( sgrIDEgress = "some sgr egress ID" cidrIpv4Block = "172.1.0.0/16" cidrIpv6Block = "2001:0DB8:7654:0010:FEDC:0000:0000:3210/128" + prefixListID = "some prefix ID" fromPort int32 = 10 wrongFromPort int32 = 0 toPort int32 = 20 @@ -75,7 +76,7 @@ func withConditions(c ...xpv1.Condition) sgrModifier { var _ managed.ExternalClient = &external{} var _ managed.ExternalConnecter = &connector{} -func TestObserce(t *testing.T) { +func TestObserve(t *testing.T) { type want struct { cr *manualv1alpha1.SecurityGroupRule result managed.ExternalObservation @@ -510,6 +511,117 @@ func TestCreate(t *testing.T) { err: nil, }, }, + "CreateIngressPrefixListId": { + // Create an ingress sgr with a prefix list id + args: args{ + sgr: &fake.MockSecurityGroupRuleClient{ + MockAuthorizeIngress: func(ctx context.Context, input *awsec2.AuthorizeSecurityGroupIngressInput, opts []func(*awsec2.Options)) (*awsec2.AuthorizeSecurityGroupIngressOutput, error) { + var cidrIpv6 *string = nil + var cidrIpv4 *string = nil + var prefixListID *string = nil + var description *string = nil + var refSg *types.ReferencedSecurityGroup = nil + if input.IpPermissions[0].Ipv6Ranges != nil && len(input.IpPermissions[0].Ipv6Ranges) > 0 { + cidrIpv6 = input.IpPermissions[0].Ipv6Ranges[0].CidrIpv6 + description = input.IpPermissions[0].Ipv6Ranges[0].Description + } + if input.IpPermissions[0].IpRanges != nil && len(input.IpPermissions[0].IpRanges) > 0 { + cidrIpv4 = input.IpPermissions[0].IpRanges[0].CidrIp + description = input.IpPermissions[0].IpRanges[0].Description + } + if input.IpPermissions[0].PrefixListIds != nil && len(input.IpPermissions[0].PrefixListIds) > 0 { + prefixListID = input.IpPermissions[0].PrefixListIds[0].PrefixListId + description = input.IpPermissions[0].PrefixListIds[0].Description + } + if len(input.IpPermissions[0].UserIdGroupPairs) > 0 { + refSg = &types.ReferencedSecurityGroup{ + GroupId: input.IpPermissions[0].UserIdGroupPairs[0].GroupId, + } + description = input.IpPermissions[0].UserIdGroupPairs[0].Description + } + return &awsec2.AuthorizeSecurityGroupIngressOutput{ + Return: &trueValue, + SecurityGroupRules: []types.SecurityGroupRule{ + { + CidrIpv4: cidrIpv4, + CidrIpv6: cidrIpv6, + PrefixListId: prefixListID, + Description: description, + GroupId: input.GroupId, + SecurityGroupRuleId: &sgrIDIngress, + ReferencedGroupInfo: refSg, + FromPort: input.IpPermissions[0].FromPort, + ToPort: input.IpPermissions[0].ToPort, + IsEgress: &falseValue, + }, + }, + }, nil + }, + MockAuthorizeEgress: func(ctx context.Context, input *awsec2.AuthorizeSecurityGroupEgressInput, opts []func(*awsec2.Options)) (*awsec2.AuthorizeSecurityGroupEgressOutput, error) { + var cidrIpv6 *string = nil + var cidrIpv4 *string = nil + var prefixListID *string = nil + var description *string = nil + var refSg *types.ReferencedSecurityGroup = nil + if input.IpPermissions[0].Ipv6Ranges != nil && len(input.IpPermissions[0].Ipv6Ranges) > 0 { + cidrIpv6 = input.IpPermissions[0].Ipv6Ranges[0].CidrIpv6 + description = input.IpPermissions[0].Ipv6Ranges[0].Description + } + if input.IpPermissions[0].IpRanges != nil && len(input.IpPermissions[0].IpRanges) > 0 { + cidrIpv4 = input.IpPermissions[0].IpRanges[0].CidrIp + description = input.IpPermissions[0].IpRanges[0].Description + } + if input.IpPermissions[0].PrefixListIds != nil && len(input.IpPermissions[0].PrefixListIds) > 0 { + prefixListID = input.IpPermissions[0].PrefixListIds[0].PrefixListId + description = input.IpPermissions[0].PrefixListIds[0].Description + } + if len(input.IpPermissions[0].UserIdGroupPairs) > 0 { + refSg = &types.ReferencedSecurityGroup{ + GroupId: input.IpPermissions[0].UserIdGroupPairs[0].GroupId, + } + description = input.IpPermissions[0].UserIdGroupPairs[0].Description + } + return &awsec2.AuthorizeSecurityGroupEgressOutput{ + Return: &trueValue, + SecurityGroupRules: []types.SecurityGroupRule{ + { + CidrIpv4: cidrIpv4, + CidrIpv6: cidrIpv6, + PrefixListId: prefixListID, + Description: description, + GroupId: input.GroupId, + SecurityGroupRuleId: &sgrIDEgress, + ReferencedGroupInfo: refSg, + FromPort: input.IpPermissions[0].FromPort, + ToPort: input.IpPermissions[0].ToPort, + IsEgress: &trueValue, + }, + }, + }, nil + }, + }, + cr: securityGroupRule(withSpec(manualv1alpha1.SecurityGroupRuleParameters{ + PrefixListID: &prefixListID, + FromPort: &fromPort, + ToPort: &toPort, + Type: &ingressTypeTest, + SecurityGroupID: &sgID, + Description: &description, + })), + }, + want: want{ + cr: securityGroupRule(withSpec(manualv1alpha1.SecurityGroupRuleParameters{ + SecurityGroupID: &sgID, + FromPort: &fromPort, + ToPort: &toPort, + Type: &ingressTypeTest, + PrefixListID: &prefixListID, + Description: &description, + }), withExternalName(sgrIDIngress)), + result: managed.ExternalCreation{}, + err: nil, + }, + }, "CreateIngressRefSG": { // Create a ingress sgr with a reference security group args: args{ @@ -805,6 +917,117 @@ func TestCreate(t *testing.T) { err: nil, }, }, + "CreateEgressPrefixListId": { + // Create an egress sgr with a prefix list id + args: args{ + sgr: &fake.MockSecurityGroupRuleClient{ + MockAuthorizeIngress: func(ctx context.Context, input *awsec2.AuthorizeSecurityGroupIngressInput, opts []func(*awsec2.Options)) (*awsec2.AuthorizeSecurityGroupIngressOutput, error) { + var cidrIpv6 *string = nil + var cidrIpv4 *string = nil + var prefixListID *string = nil + var description *string = nil + var refSg *types.ReferencedSecurityGroup = nil + if input.IpPermissions[0].Ipv6Ranges != nil && len(input.IpPermissions[0].Ipv6Ranges) > 0 { + cidrIpv6 = input.IpPermissions[0].Ipv6Ranges[0].CidrIpv6 + description = input.IpPermissions[0].Ipv6Ranges[0].Description + } + if input.IpPermissions[0].IpRanges != nil && len(input.IpPermissions[0].IpRanges) > 0 { + cidrIpv4 = input.IpPermissions[0].IpRanges[0].CidrIp + description = input.IpPermissions[0].IpRanges[0].Description + } + if input.IpPermissions[0].PrefixListIds != nil && len(input.IpPermissions[0].PrefixListIds) > 0 { + prefixListID = input.IpPermissions[0].PrefixListIds[0].PrefixListId + description = input.IpPermissions[0].PrefixListIds[0].Description + } + if len(input.IpPermissions[0].UserIdGroupPairs) > 0 { + refSg = &types.ReferencedSecurityGroup{ + GroupId: input.IpPermissions[0].UserIdGroupPairs[0].GroupId, + } + description = input.IpPermissions[0].UserIdGroupPairs[0].Description + } + return &awsec2.AuthorizeSecurityGroupIngressOutput{ + Return: &trueValue, + SecurityGroupRules: []types.SecurityGroupRule{ + { + CidrIpv4: cidrIpv4, + CidrIpv6: cidrIpv6, + PrefixListId: prefixListID, + Description: description, + GroupId: input.GroupId, + SecurityGroupRuleId: &sgrIDIngress, + ReferencedGroupInfo: refSg, + FromPort: input.IpPermissions[0].FromPort, + ToPort: input.IpPermissions[0].ToPort, + IsEgress: &falseValue, + }, + }, + }, nil + }, + MockAuthorizeEgress: func(ctx context.Context, input *awsec2.AuthorizeSecurityGroupEgressInput, opts []func(*awsec2.Options)) (*awsec2.AuthorizeSecurityGroupEgressOutput, error) { + var cidrIpv6 *string = nil + var cidrIpv4 *string = nil + var prefixListID *string = nil + var description *string = nil + var refSg *types.ReferencedSecurityGroup = nil + if input.IpPermissions[0].Ipv6Ranges != nil && len(input.IpPermissions[0].Ipv6Ranges) > 0 { + cidrIpv6 = input.IpPermissions[0].Ipv6Ranges[0].CidrIpv6 + description = input.IpPermissions[0].Ipv6Ranges[0].Description + } + if input.IpPermissions[0].IpRanges != nil && len(input.IpPermissions[0].IpRanges) > 0 { + cidrIpv4 = input.IpPermissions[0].IpRanges[0].CidrIp + description = input.IpPermissions[0].IpRanges[0].Description + } + if input.IpPermissions[0].PrefixListIds != nil && len(input.IpPermissions[0].PrefixListIds) > 0 { + prefixListID = input.IpPermissions[0].PrefixListIds[0].PrefixListId + description = input.IpPermissions[0].PrefixListIds[0].Description + } + if len(input.IpPermissions[0].UserIdGroupPairs) > 0 { + refSg = &types.ReferencedSecurityGroup{ + GroupId: input.IpPermissions[0].UserIdGroupPairs[0].GroupId, + } + description = input.IpPermissions[0].UserIdGroupPairs[0].Description + } + return &awsec2.AuthorizeSecurityGroupEgressOutput{ + Return: &trueValue, + SecurityGroupRules: []types.SecurityGroupRule{ + { + CidrIpv4: cidrIpv4, + CidrIpv6: cidrIpv6, + PrefixListId: prefixListID, + Description: description, + GroupId: input.GroupId, + SecurityGroupRuleId: &sgrIDEgress, + ReferencedGroupInfo: refSg, + FromPort: input.IpPermissions[0].FromPort, + ToPort: input.IpPermissions[0].ToPort, + IsEgress: &trueValue, + }, + }, + }, nil + }, + }, + cr: securityGroupRule(withSpec(manualv1alpha1.SecurityGroupRuleParameters{ + PrefixListID: &prefixListID, + FromPort: &fromPort, + ToPort: &toPort, + Type: &egressTypeTest, + SecurityGroupID: &sgID, + Description: &description, + })), + }, + want: want{ + cr: securityGroupRule(withSpec(manualv1alpha1.SecurityGroupRuleParameters{ + SecurityGroupID: &sgID, + FromPort: &fromPort, + ToPort: &toPort, + Type: &egressTypeTest, + PrefixListID: &prefixListID, + Description: &description, + }), withExternalName(sgrIDEgress)), + result: managed.ExternalCreation{}, + err: nil, + }, + }, "CreateEgressRefSG": { // Create a egress sgr with a reference security group args: args{