From 637da56975fca112686bfada3e94f3452a4259d7 Mon Sep 17 00:00:00 2001 From: "Charel Baum (external expert on behalf of DB InfraGO AG)" Date: Tue, 21 May 2024 12:57:54 +0200 Subject: [PATCH 1/2] feat(ec2): Add SecurityGroupRuleIDs to SG status Signed-off-by: Charel Baum (external expert on behalf of DB InfraGO AG) --- apis/ec2/v1beta1/securitygroup_types.go | 25 +++++++++ apis/ec2/v1beta1/zz_generated.deepcopy.go | 56 ++++++++++++++++++- .../ec2.aws.crossplane.io_securitygroups.yaml | 46 +++++++++++++++ pkg/clients/ec2/fake/securitygroup.go | 6 ++ pkg/clients/ec2/securitygroup.go | 23 +++++++- pkg/clients/ec2/securitygroup_test.go | 44 ++++++++++++--- .../ec2/securitygroup/controller.go | 13 ++++- .../ec2/securitygroup/controller_test.go | 42 +++++++++++++- 8 files changed, 242 insertions(+), 13 deletions(-) diff --git a/apis/ec2/v1beta1/securitygroup_types.go b/apis/ec2/v1beta1/securitygroup_types.go index d4ea2b1b37..327370e6bb 100644 --- a/apis/ec2/v1beta1/securitygroup_types.go +++ b/apis/ec2/v1beta1/securitygroup_types.go @@ -248,6 +248,31 @@ type SecurityGroupObservation struct { // SecurityGroupID is the ID of the SecurityGroup. SecurityGroupID string `json:"securityGroupID"` + + // IngressRules of the observed SecurityGroup. + IngressRules []SecurityGroupRuleObservation `json:"ingressRules,omitempty"` + + // EgressRules of the observed SecurityGroup. + EgressRules []SecurityGroupRuleObservation `json:"egressRules,omitempty"` +} + +type SecurityGroupRuleObservation struct { + // ID of the security group rule. + ID *string `json:"id,omitempty"` + + // CidrIpv4 range. + CidrIpv4 *string `json:"cidrIpv4,omitempty"` + + // CidrIpv6 range. + CidrIpv6 *string `json:"cidrIpv6,omitempty"` + + // The IP protocol name (tcp, udp, icmp, icmpv6) or number (see Protocol Numbers + // (http://www.iana.org/assignments/protocol-numbers/protocol-numbers.xhtml)). Use + // -1 to specify all protocols. + IpProtocol *string `json:"ipProtocol,omitempty"` + + // Description of this rule. + Description *string `json:"description,omitempty"` } // A SecurityGroupStatus represents the observed state of a SecurityGroup. diff --git a/apis/ec2/v1beta1/zz_generated.deepcopy.go b/apis/ec2/v1beta1/zz_generated.deepcopy.go index 02c479e69c..73b092130d 100644 --- a/apis/ec2/v1beta1/zz_generated.deepcopy.go +++ b/apis/ec2/v1beta1/zz_generated.deepcopy.go @@ -1031,6 +1031,20 @@ func (in *SecurityGroupList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SecurityGroupObservation) DeepCopyInto(out *SecurityGroupObservation) { *out = *in + if in.IngressRules != nil { + in, out := &in.IngressRules, &out.IngressRules + *out = make([]SecurityGroupRuleObservation, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + if in.EgressRules != nil { + in, out := &in.EgressRules, &out.EgressRules + *out = make([]SecurityGroupRuleObservation, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SecurityGroupObservation. @@ -1102,6 +1116,46 @@ func (in *SecurityGroupParameters) DeepCopy() *SecurityGroupParameters { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SecurityGroupRuleObservation) DeepCopyInto(out *SecurityGroupRuleObservation) { + *out = *in + if in.ID != nil { + in, out := &in.ID, &out.ID + *out = new(string) + **out = **in + } + if in.CidrIpv4 != nil { + in, out := &in.CidrIpv4, &out.CidrIpv4 + *out = new(string) + **out = **in + } + if in.CidrIpv6 != nil { + in, out := &in.CidrIpv6, &out.CidrIpv6 + *out = new(string) + **out = **in + } + if in.IpProtocol != nil { + in, out := &in.IpProtocol, &out.IpProtocol + *out = new(string) + **out = **in + } + if in.Description != nil { + in, out := &in.Description, &out.Description + *out = new(string) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SecurityGroupRuleObservation. +func (in *SecurityGroupRuleObservation) DeepCopy() *SecurityGroupRuleObservation { + if in == nil { + return nil + } + out := new(SecurityGroupRuleObservation) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SecurityGroupSpec) DeepCopyInto(out *SecurityGroupSpec) { *out = *in @@ -1123,7 +1177,7 @@ func (in *SecurityGroupSpec) DeepCopy() *SecurityGroupSpec { func (in *SecurityGroupStatus) DeepCopyInto(out *SecurityGroupStatus) { *out = *in in.ResourceStatus.DeepCopyInto(&out.ResourceStatus) - out.AtProvider = in.AtProvider + in.AtProvider.DeepCopyInto(&out.AtProvider) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SecurityGroupStatus. diff --git a/package/crds/ec2.aws.crossplane.io_securitygroups.yaml b/package/crds/ec2.aws.crossplane.io_securitygroups.yaml index 562709f347..c6ffbfa20f 100644 --- a/package/crds/ec2.aws.crossplane.io_securitygroups.yaml +++ b/package/crds/ec2.aws.crossplane.io_securitygroups.yaml @@ -965,6 +965,52 @@ spec: description: SecurityGroupObservation keeps the state for the external resource properties: + egressRules: + description: EgressRules of the observed SecurityGroup. + items: + properties: + cidrIpv4: + description: CidrIpv4 range. + type: string + cidrIpv6: + description: CidrIpv6 range. + type: string + description: + description: Description of this rule. + type: string + id: + description: ID of the security group rule. + type: string + ipProtocol: + description: The IP protocol name (tcp, udp, icmp, icmpv6) + or number (see Protocol Numbers (http://www.iana.org/assignments/protocol-numbers/protocol-numbers.xhtml)). + Use -1 to specify all protocols. + type: string + type: object + type: array + ingressRules: + description: IngressRules of the observed SecurityGroup. + items: + properties: + cidrIpv4: + description: CidrIpv4 range. + type: string + cidrIpv6: + description: CidrIpv6 range. + type: string + description: + description: Description of this rule. + type: string + id: + description: ID of the security group rule. + type: string + ipProtocol: + description: The IP protocol name (tcp, udp, icmp, icmpv6) + or number (see Protocol Numbers (http://www.iana.org/assignments/protocol-numbers/protocol-numbers.xhtml)). + Use -1 to specify all protocols. + type: string + type: object + type: array ownerId: description: The AWS account ID of the owner of the security group. type: string diff --git a/pkg/clients/ec2/fake/securitygroup.go b/pkg/clients/ec2/fake/securitygroup.go index bddaf42ffe..e2d966e09f 100644 --- a/pkg/clients/ec2/fake/securitygroup.go +++ b/pkg/clients/ec2/fake/securitygroup.go @@ -32,6 +32,7 @@ type MockSecurityGroupClient struct { MockCreate func(ctx context.Context, input *ec2.CreateSecurityGroupInput, opts []func(*ec2.Options)) (*ec2.CreateSecurityGroupOutput, error) MockDelete func(ctx context.Context, input *ec2.DeleteSecurityGroupInput, opts []func(*ec2.Options)) (*ec2.DeleteSecurityGroupOutput, error) MockDescribe func(ctx context.Context, input *ec2.DescribeSecurityGroupsInput, opts []func(*ec2.Options)) (*ec2.DescribeSecurityGroupsOutput, error) + MockDescribeRules func(ctx context.Context, input *ec2.DescribeSecurityGroupRulesInput, opts []func(*ec2.Options)) (*ec2.DescribeSecurityGroupRulesOutput, error) MockAuthorizeIngress func(ctx context.Context, input *ec2.AuthorizeSecurityGroupIngressInput, opts []func(*ec2.Options)) (*ec2.AuthorizeSecurityGroupIngressOutput, error) MockAuthorizeEgress func(ctx context.Context, input *ec2.AuthorizeSecurityGroupEgressInput, opts []func(*ec2.Options)) (*ec2.AuthorizeSecurityGroupEgressOutput, error) MockRevokeIngress func(ctx context.Context, input *ec2.RevokeSecurityGroupIngressInput, opts []func(*ec2.Options)) (*ec2.RevokeSecurityGroupIngressOutput, error) @@ -55,6 +56,11 @@ func (m *MockSecurityGroupClient) DescribeSecurityGroups(ctx context.Context, in return m.MockDescribe(ctx, input, opts) } +// DescribeSecurityGroups mocks DescribeSecurityGroups method +func (m *MockSecurityGroupClient) DescribeSecurityGroupRules(ctx context.Context, input *ec2.DescribeSecurityGroupRulesInput, opts ...func(*ec2.Options)) (*ec2.DescribeSecurityGroupRulesOutput, error) { + return m.MockDescribeRules(ctx, input, opts) +} + // AuthorizeSecurityGroupIngress mocks AuthorizeSecurityGroupIngress method func (m *MockSecurityGroupClient) AuthorizeSecurityGroupIngress(ctx context.Context, input *ec2.AuthorizeSecurityGroupIngressInput, opts ...func(*ec2.Options)) (*ec2.AuthorizeSecurityGroupIngressOutput, error) { return m.MockAuthorizeIngress(ctx, input, opts) diff --git a/pkg/clients/ec2/securitygroup.go b/pkg/clients/ec2/securitygroup.go index ad1da465d9..c936cbe52d 100644 --- a/pkg/clients/ec2/securitygroup.go +++ b/pkg/clients/ec2/securitygroup.go @@ -8,6 +8,7 @@ import ( "github.com/aws/aws-sdk-go-v2/service/ec2" ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types" "github.com/aws/smithy-go" + "k8s.io/utils/ptr" "github.com/crossplane-contrib/provider-aws/apis/ec2/v1beta1" "github.com/crossplane-contrib/provider-aws/pkg/utils/pointer" @@ -26,6 +27,7 @@ type SecurityGroupClient interface { CreateSecurityGroup(ctx context.Context, input *ec2.CreateSecurityGroupInput, opts ...func(*ec2.Options)) (*ec2.CreateSecurityGroupOutput, error) DeleteSecurityGroup(ctx context.Context, input *ec2.DeleteSecurityGroupInput, opts ...func(*ec2.Options)) (*ec2.DeleteSecurityGroupOutput, error) DescribeSecurityGroups(ctx context.Context, input *ec2.DescribeSecurityGroupsInput, opts ...func(*ec2.Options)) (*ec2.DescribeSecurityGroupsOutput, error) + DescribeSecurityGroupRules(ctx context.Context, input *ec2.DescribeSecurityGroupRulesInput, optFns ...func(*ec2.Options)) (*ec2.DescribeSecurityGroupRulesOutput, error) AuthorizeSecurityGroupIngress(ctx context.Context, input *ec2.AuthorizeSecurityGroupIngressInput, opts ...func(*ec2.Options)) (*ec2.AuthorizeSecurityGroupIngressOutput, error) AuthorizeSecurityGroupEgress(ctx context.Context, input *ec2.AuthorizeSecurityGroupEgressInput, opts ...func(*ec2.Options)) (*ec2.AuthorizeSecurityGroupEgressOutput, error) RevokeSecurityGroupIngress(ctx context.Context, input *ec2.RevokeSecurityGroupIngressInput, opts ...func(*ec2.Options)) (*ec2.RevokeSecurityGroupIngressOutput, error) @@ -98,10 +100,29 @@ func GenerateEC2Permissions(objectPerms []v1beta1.IPPermission) []ec2types.IpPer // GenerateSGObservation is used to produce v1beta1.SecurityGroupExternalStatus from // ec2types.SecurityGroup. -func GenerateSGObservation(sg ec2types.SecurityGroup) v1beta1.SecurityGroupObservation { +func GenerateSGObservation(sg ec2types.SecurityGroup, rules []ec2types.SecurityGroupRule) v1beta1.SecurityGroupObservation { + ingressRules := []v1beta1.SecurityGroupRuleObservation{} + egressRules := []v1beta1.SecurityGroupRuleObservation{} + + for _, r := range rules { + observedRule := v1beta1.SecurityGroupRuleObservation{ + ID: r.SecurityGroupRuleId, + CidrIpv4: r.CidrIpv4, + CidrIpv6: r.CidrIpv6, + IpProtocol: r.IpProtocol, + } + if ptr.Deref(r.IsEgress, false) { + egressRules = append(egressRules, observedRule) + } else { + ingressRules = append(ingressRules, observedRule) + } + } + return v1beta1.SecurityGroupObservation{ OwnerID: pointer.StringValue(sg.OwnerId), SecurityGroupID: pointer.StringValue(sg.GroupId), + IngressRules: ingressRules, + EgressRules: egressRules, } } diff --git a/pkg/clients/ec2/securitygroup_test.go b/pkg/clients/ec2/securitygroup_test.go index 986ef271ce..412143c97e 100644 --- a/pkg/clients/ec2/securitygroup_test.go +++ b/pkg/clients/ec2/securitygroup_test.go @@ -6,6 +6,7 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types" "github.com/google/go-cmp/cmp" + "k8s.io/utils/ptr" "github.com/crossplane-contrib/provider-aws/apis/ec2/v1beta1" ) @@ -126,14 +127,37 @@ func TestIsSGUpToDate(t *testing.T) { } func TestGenerateSGObservation(t *testing.T) { + type args struct { + sg ec2types.SecurityGroup + rules []ec2types.SecurityGroupRule + } + cases := map[string]struct { - in ec2types.SecurityGroup + in args out v1beta1.SecurityGroupObservation }{ "AllFilled": { - in: ec2types.SecurityGroup{ - OwnerId: aws.String(sgOwner), - GroupId: aws.String(sgID), + in: args{ + sg: ec2types.SecurityGroup{ + OwnerId: aws.String(sgOwner), + GroupId: aws.String(sgID), + }, + rules: []ec2types.SecurityGroupRule{ + { + CidrIpv4: ptr.To("10.0.0.16/32"), + Description: ptr.To("egress rule"), + GroupId: ptr.To("abcd"), + IpProtocol: ptr.To("tcp"), + IsEgress: ptr.To(true), + }, + { + CidrIpv4: ptr.To("10.0.100.16/16"), + Description: ptr.To("ingress rule"), + GroupId: ptr.To("efgh"), + IpProtocol: ptr.To("tcp"), + IsEgress: ptr.To(false), + }, + }, }, out: v1beta1.SecurityGroupObservation{ OwnerID: sgOwner, @@ -141,18 +165,22 @@ func TestGenerateSGObservation(t *testing.T) { }, }, "NoIpCount": { - in: ec2types.SecurityGroup{ - OwnerId: aws.String(sgOwner), + in: args{ + sg: ec2types.SecurityGroup{ + OwnerId: aws.String(sgOwner), + }, }, out: v1beta1.SecurityGroupObservation{ - OwnerID: sgOwner, + OwnerID: sgOwner, + IngressRules: []v1beta1.SecurityGroupRuleObservation{}, + EgressRules: []v1beta1.SecurityGroupRuleObservation{}, }, }, } for name, tc := range cases { t.Run(name, func(t *testing.T) { - r := GenerateSGObservation(tc.in) + r := GenerateSGObservation(tc.in.sg, tc.in.rules) if diff := cmp.Diff(r, tc.out); diff != "" { t.Errorf("GenerateNetworkObservation(...): -want, +got:\n%s", diff) } diff --git a/pkg/controller/ec2/securitygroup/controller.go b/pkg/controller/ec2/securitygroup/controller.go index d91580281c..aa9d5d8675 100644 --- a/pkg/controller/ec2/securitygroup/controller.go +++ b/pkg/controller/ec2/securitygroup/controller.go @@ -51,6 +51,7 @@ const ( errUnexpectedObject = "The managed resource is not an SecurityGroup resource" errDescribe = "failed to describe SecurityGroup" + errDescribeRules = "failed to describe SecurityGroupRules" errGetSecurityGroup = "failed to get SecurityGroup based on groupName" errMultipleItems = "retrieved multiple SecurityGroups for the given securityGroupId" errCreate = "failed to create the SecurityGroup resource" @@ -153,12 +154,22 @@ func (e *external) Observe(ctx context.Context, mgd resource.Managed) (managed.E return managed.ExternalObservation{}, errors.New(errMultipleItems) } + securityGroupRulesResponse, err := e.sg.DescribeSecurityGroupRules(ctx, &awsec2.DescribeSecurityGroupRulesInput{ + Filters: []awsec2types.Filter{ + awsec2types.Filter{Name: aws.String("group-id"), Values: []string{meta.GetExternalName(cr)}}, + }, + }) + if err != nil { + return managed.ExternalObservation{}, errorutils.Wrap(resource.Ignore(ec2.IsSecurityGroupNotFoundErr, err), errDescribeRules) + } + + observedRules := securityGroupRulesResponse.SecurityGroupRules observed := response.SecurityGroups[0] current := cr.Spec.ForProvider.DeepCopy() ec2.LateInitializeSG(&cr.Spec.ForProvider, &observed) - cr.Status.AtProvider = ec2.GenerateSGObservation(observed) + cr.Status.AtProvider = ec2.GenerateSGObservation(observed, observedRules) upToDate := ec2.IsSGUpToDate(cr.Spec.ForProvider, observed) // this is to make sure that the security group exists with the specified traffic rules. diff --git a/pkg/controller/ec2/securitygroup/controller_test.go b/pkg/controller/ec2/securitygroup/controller_test.go index d700e65561..ca2f71b721 100644 --- a/pkg/controller/ec2/securitygroup/controller_test.go +++ b/pkg/controller/ec2/securitygroup/controller_test.go @@ -134,6 +134,11 @@ func TestObserve(t *testing.T) { SecurityGroups: []awsec2types.SecurityGroup{{}}, }, nil }, + MockDescribeRules: func(ctx context.Context, input *awsec2.DescribeSecurityGroupRulesInput, opts []func(*awsec2.Options)) (*awsec2.DescribeSecurityGroupRulesOutput, error) { + return &awsec2.DescribeSecurityGroupRulesOutput{ + SecurityGroupRules: []awsec2types.SecurityGroupRule{}, + }, nil + }, }, cr: sg(withStatus(v1beta1.SecurityGroupObservation{ SecurityGroupID: sgID, @@ -141,8 +146,14 @@ func TestObserve(t *testing.T) { withExternalName(sgID)), }, want: want{ - cr: sg(withExternalName(sgID), - withConditions(xpv1.Available())), + cr: sg( + withExternalName(sgID), + withStatus(v1beta1.SecurityGroupObservation{ + IngressRules: []v1beta1.SecurityGroupRuleObservation{}, + EgressRules: []v1beta1.SecurityGroupRuleObservation{}, + }), + withConditions(xpv1.Available()), + ), result: managed.ExternalObservation{ ResourceExists: true, ResourceUpToDate: true, @@ -160,12 +171,19 @@ func TestObserve(t *testing.T) { SecurityGroups: []awsec2types.SecurityGroup{{GroupId: aws.String(sgID)}}, }, nil }, + MockDescribeRules: func(ctx context.Context, input *awsec2.DescribeSecurityGroupRulesInput, opts []func(*awsec2.Options)) (*awsec2.DescribeSecurityGroupRulesOutput, error) { + return &awsec2.DescribeSecurityGroupRulesOutput{ + SecurityGroupRules: []awsec2types.SecurityGroupRule{}, + }, nil + }, }, cr: sg(withExternalName("")), }, want: want{ cr: sg(withStatus(v1beta1.SecurityGroupObservation{ SecurityGroupID: sgID, + IngressRules: []v1beta1.SecurityGroupRuleObservation{}, + EgressRules: []v1beta1.SecurityGroupRuleObservation{}, }), withExternalName(sgID), withConditions(xpv1.Available())), @@ -186,6 +204,11 @@ func TestObserve(t *testing.T) { SecurityGroups: []awsec2types.SecurityGroup{}, }, nil }, + MockDescribeRules: func(ctx context.Context, input *awsec2.DescribeSecurityGroupRulesInput, opts []func(*awsec2.Options)) (*awsec2.DescribeSecurityGroupRulesOutput, error) { + return &awsec2.DescribeSecurityGroupRulesOutput{ + SecurityGroupRules: []awsec2types.SecurityGroupRule{}, + }, nil + }, }, cr: sg(), }, @@ -205,6 +228,11 @@ func TestObserve(t *testing.T) { MockDescribe: func(ctx context.Context, input *awsec2.DescribeSecurityGroupsInput, opts []func(*awsec2.Options)) (*awsec2.DescribeSecurityGroupsOutput, error) { return nil, errBoom }, + MockDescribeRules: func(ctx context.Context, input *awsec2.DescribeSecurityGroupRulesInput, opts []func(*awsec2.Options)) (*awsec2.DescribeSecurityGroupRulesOutput, error) { + return &awsec2.DescribeSecurityGroupRulesOutput{ + SecurityGroupRules: []awsec2types.SecurityGroupRule{}, + }, nil + }, }, cr: sg(), }, @@ -224,6 +252,11 @@ func TestObserve(t *testing.T) { SecurityGroups: []awsec2types.SecurityGroup{{}, {}}, }, nil }, + MockDescribeRules: func(ctx context.Context, input *awsec2.DescribeSecurityGroupRulesInput, opts []func(*awsec2.Options)) (*awsec2.DescribeSecurityGroupRulesOutput, error) { + return &awsec2.DescribeSecurityGroupRulesOutput{ + SecurityGroupRules: []awsec2types.SecurityGroupRule{}, + }, nil + }, }, cr: sg(withStatus(v1beta1.SecurityGroupObservation{ SecurityGroupID: sgID, @@ -244,6 +277,11 @@ func TestObserve(t *testing.T) { MockDescribe: func(ctx context.Context, input *awsec2.DescribeSecurityGroupsInput, opts []func(*awsec2.Options)) (*awsec2.DescribeSecurityGroupsOutput, error) { return nil, errBoom }, + MockDescribeRules: func(ctx context.Context, input *awsec2.DescribeSecurityGroupRulesInput, opts []func(*awsec2.Options)) (*awsec2.DescribeSecurityGroupRulesOutput, error) { + return &awsec2.DescribeSecurityGroupRulesOutput{ + SecurityGroupRules: []awsec2types.SecurityGroupRule{}, + }, nil + }, }, cr: sg(withStatus(v1beta1.SecurityGroupObservation{ SecurityGroupID: sgID, From d0f56f688768e2bef85f9c71550fb43c0c7c5f15 Mon Sep 17 00:00:00 2001 From: "Charel Baum (external expert on behalf of DB InfraGO AG)" Date: Tue, 21 May 2024 13:37:11 +0200 Subject: [PATCH 2/2] fix(ec2): unit-test Signed-off-by: Charel Baum (external expert on behalf of DB InfraGO AG) --- pkg/clients/ec2/fake/securitygroup.go | 12 +-- pkg/clients/ec2/securitygroup.go | 44 +++++----- pkg/clients/ec2/securitygroup_test.go | 84 +++++++++++-------- .../ec2/securitygroup/controller.go | 5 +- 4 files changed, 80 insertions(+), 65 deletions(-) diff --git a/pkg/clients/ec2/fake/securitygroup.go b/pkg/clients/ec2/fake/securitygroup.go index e2d966e09f..23b126e34e 100644 --- a/pkg/clients/ec2/fake/securitygroup.go +++ b/pkg/clients/ec2/fake/securitygroup.go @@ -32,7 +32,7 @@ type MockSecurityGroupClient struct { MockCreate func(ctx context.Context, input *ec2.CreateSecurityGroupInput, opts []func(*ec2.Options)) (*ec2.CreateSecurityGroupOutput, error) MockDelete func(ctx context.Context, input *ec2.DeleteSecurityGroupInput, opts []func(*ec2.Options)) (*ec2.DeleteSecurityGroupOutput, error) MockDescribe func(ctx context.Context, input *ec2.DescribeSecurityGroupsInput, opts []func(*ec2.Options)) (*ec2.DescribeSecurityGroupsOutput, error) - MockDescribeRules func(ctx context.Context, input *ec2.DescribeSecurityGroupRulesInput, opts []func(*ec2.Options)) (*ec2.DescribeSecurityGroupRulesOutput, error) + MockDescribeRules func(ctx context.Context, input *ec2.DescribeSecurityGroupRulesInput, opts []func(*ec2.Options)) (*ec2.DescribeSecurityGroupRulesOutput, error) MockAuthorizeIngress func(ctx context.Context, input *ec2.AuthorizeSecurityGroupIngressInput, opts []func(*ec2.Options)) (*ec2.AuthorizeSecurityGroupIngressOutput, error) MockAuthorizeEgress func(ctx context.Context, input *ec2.AuthorizeSecurityGroupEgressInput, opts []func(*ec2.Options)) (*ec2.AuthorizeSecurityGroupEgressOutput, error) MockRevokeIngress func(ctx context.Context, input *ec2.RevokeSecurityGroupIngressInput, opts []func(*ec2.Options)) (*ec2.RevokeSecurityGroupIngressOutput, error) @@ -56,11 +56,11 @@ func (m *MockSecurityGroupClient) DescribeSecurityGroups(ctx context.Context, in return m.MockDescribe(ctx, input, opts) } -// DescribeSecurityGroups mocks DescribeSecurityGroups method -func (m *MockSecurityGroupClient) DescribeSecurityGroupRules(ctx context.Context, input *ec2.DescribeSecurityGroupRulesInput, opts ...func(*ec2.Options)) (*ec2.DescribeSecurityGroupRulesOutput, error) { - return m.MockDescribeRules(ctx, input, opts) -} - +// DescribeSecurityGroups mocks DescribeSecurityGroups method +func (m *MockSecurityGroupClient) DescribeSecurityGroupRules(ctx context.Context, input *ec2.DescribeSecurityGroupRulesInput, opts ...func(*ec2.Options)) (*ec2.DescribeSecurityGroupRulesOutput, error) { + return m.MockDescribeRules(ctx, input, opts) +} + // AuthorizeSecurityGroupIngress mocks AuthorizeSecurityGroupIngress method func (m *MockSecurityGroupClient) AuthorizeSecurityGroupIngress(ctx context.Context, input *ec2.AuthorizeSecurityGroupIngressInput, opts ...func(*ec2.Options)) (*ec2.AuthorizeSecurityGroupIngressOutput, error) { return m.MockAuthorizeIngress(ctx, input, opts) diff --git a/pkg/clients/ec2/securitygroup.go b/pkg/clients/ec2/securitygroup.go index c936cbe52d..0562583bf2 100644 --- a/pkg/clients/ec2/securitygroup.go +++ b/pkg/clients/ec2/securitygroup.go @@ -8,7 +8,7 @@ import ( "github.com/aws/aws-sdk-go-v2/service/ec2" ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types" "github.com/aws/smithy-go" - "k8s.io/utils/ptr" + "k8s.io/utils/ptr" "github.com/crossplane-contrib/provider-aws/apis/ec2/v1beta1" "github.com/crossplane-contrib/provider-aws/pkg/utils/pointer" @@ -27,7 +27,7 @@ type SecurityGroupClient interface { CreateSecurityGroup(ctx context.Context, input *ec2.CreateSecurityGroupInput, opts ...func(*ec2.Options)) (*ec2.CreateSecurityGroupOutput, error) DeleteSecurityGroup(ctx context.Context, input *ec2.DeleteSecurityGroupInput, opts ...func(*ec2.Options)) (*ec2.DeleteSecurityGroupOutput, error) DescribeSecurityGroups(ctx context.Context, input *ec2.DescribeSecurityGroupsInput, opts ...func(*ec2.Options)) (*ec2.DescribeSecurityGroupsOutput, error) - DescribeSecurityGroupRules(ctx context.Context, input *ec2.DescribeSecurityGroupRulesInput, optFns ...func(*ec2.Options)) (*ec2.DescribeSecurityGroupRulesOutput, error) + DescribeSecurityGroupRules(ctx context.Context, input *ec2.DescribeSecurityGroupRulesInput, optFns ...func(*ec2.Options)) (*ec2.DescribeSecurityGroupRulesOutput, error) AuthorizeSecurityGroupIngress(ctx context.Context, input *ec2.AuthorizeSecurityGroupIngressInput, opts ...func(*ec2.Options)) (*ec2.AuthorizeSecurityGroupIngressOutput, error) AuthorizeSecurityGroupEgress(ctx context.Context, input *ec2.AuthorizeSecurityGroupEgressInput, opts ...func(*ec2.Options)) (*ec2.AuthorizeSecurityGroupEgressOutput, error) RevokeSecurityGroupIngress(ctx context.Context, input *ec2.RevokeSecurityGroupIngressInput, opts ...func(*ec2.Options)) (*ec2.RevokeSecurityGroupIngressOutput, error) @@ -100,29 +100,29 @@ func GenerateEC2Permissions(objectPerms []v1beta1.IPPermission) []ec2types.IpPer // GenerateSGObservation is used to produce v1beta1.SecurityGroupExternalStatus from // ec2types.SecurityGroup. -func GenerateSGObservation(sg ec2types.SecurityGroup, rules []ec2types.SecurityGroupRule) v1beta1.SecurityGroupObservation { - ingressRules := []v1beta1.SecurityGroupRuleObservation{} - egressRules := []v1beta1.SecurityGroupRuleObservation{} - - for _, r := range rules { - observedRule := v1beta1.SecurityGroupRuleObservation{ - ID: r.SecurityGroupRuleId, - CidrIpv4: r.CidrIpv4, - CidrIpv6: r.CidrIpv6, - IpProtocol: r.IpProtocol, - } - if ptr.Deref(r.IsEgress, false) { - egressRules = append(egressRules, observedRule) - } else { - ingressRules = append(ingressRules, observedRule) - } - } - +func GenerateSGObservation(sg ec2types.SecurityGroup, rules []ec2types.SecurityGroupRule) v1beta1.SecurityGroupObservation { + ingressRules := []v1beta1.SecurityGroupRuleObservation{} + egressRules := []v1beta1.SecurityGroupRuleObservation{} + + for _, r := range rules { + observedRule := v1beta1.SecurityGroupRuleObservation{ + ID: r.SecurityGroupRuleId, + CidrIpv4: r.CidrIpv4, + CidrIpv6: r.CidrIpv6, + IpProtocol: r.IpProtocol, + } + if ptr.Deref(r.IsEgress, false) { + egressRules = append(egressRules, observedRule) + } else { + ingressRules = append(ingressRules, observedRule) + } + } + return v1beta1.SecurityGroupObservation{ OwnerID: pointer.StringValue(sg.OwnerId), SecurityGroupID: pointer.StringValue(sg.GroupId), - IngressRules: ingressRules, - EgressRules: egressRules, + IngressRules: ingressRules, + EgressRules: egressRules, } } diff --git a/pkg/clients/ec2/securitygroup_test.go b/pkg/clients/ec2/securitygroup_test.go index 412143c97e..464cb84d75 100644 --- a/pkg/clients/ec2/securitygroup_test.go +++ b/pkg/clients/ec2/securitygroup_test.go @@ -6,7 +6,7 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types" "github.com/google/go-cmp/cmp" - "k8s.io/utils/ptr" + "k8s.io/utils/ptr" "github.com/crossplane-contrib/provider-aws/apis/ec2/v1beta1" ) @@ -127,60 +127,72 @@ func TestIsSGUpToDate(t *testing.T) { } func TestGenerateSGObservation(t *testing.T) { - type args struct { - sg ec2types.SecurityGroup - rules []ec2types.SecurityGroupRule - } - + type args struct { + sg ec2types.SecurityGroup + rules []ec2types.SecurityGroupRule + } + cases := map[string]struct { - in args + in args out v1beta1.SecurityGroupObservation }{ "AllFilled": { - in: args{ - sg: ec2types.SecurityGroup{ - OwnerId: aws.String(sgOwner), - GroupId: aws.String(sgID), - }, - rules: []ec2types.SecurityGroupRule{ - { - CidrIpv4: ptr.To("10.0.0.16/32"), - Description: ptr.To("egress rule"), - GroupId: ptr.To("abcd"), - IpProtocol: ptr.To("tcp"), - IsEgress: ptr.To(true), - }, - { - CidrIpv4: ptr.To("10.0.100.16/16"), - Description: ptr.To("ingress rule"), - GroupId: ptr.To("efgh"), - IpProtocol: ptr.To("tcp"), - IsEgress: ptr.To(false), - }, - }, + in: args{ + sg: ec2types.SecurityGroup{ + OwnerId: aws.String(sgOwner), + GroupId: aws.String(sgID), + }, + rules: []ec2types.SecurityGroupRule{ + { + CidrIpv4: ptr.To("10.0.0.16/32"), + Description: ptr.To("egress rule"), + GroupId: ptr.To("abcd"), + IpProtocol: ptr.To("tcp"), + IsEgress: ptr.To(true), + }, + { + CidrIpv4: ptr.To("10.0.100.16/16"), + Description: ptr.To("ingress rule"), + GroupId: ptr.To("efgh"), + IpProtocol: ptr.To("tcp"), + IsEgress: ptr.To(false), + }, + }, }, out: v1beta1.SecurityGroupObservation{ OwnerID: sgOwner, SecurityGroupID: sgID, + EgressRules: []v1beta1.SecurityGroupRuleObservation{ + { + CidrIpv4: ptr.To("10.0.0.16/32"), + IpProtocol: ptr.To("tcp"), + }, + }, + IngressRules: []v1beta1.SecurityGroupRuleObservation{ + { + CidrIpv4: ptr.To("10.0.100.16/16"), + IpProtocol: ptr.To("tcp"), + }, + }, }, }, "NoIpCount": { - in: args{ - sg: ec2types.SecurityGroup{ - OwnerId: aws.String(sgOwner), - }, + in: args{ + sg: ec2types.SecurityGroup{ + OwnerId: aws.String(sgOwner), + }, }, out: v1beta1.SecurityGroupObservation{ - OwnerID: sgOwner, - IngressRules: []v1beta1.SecurityGroupRuleObservation{}, - EgressRules: []v1beta1.SecurityGroupRuleObservation{}, + OwnerID: sgOwner, + IngressRules: []v1beta1.SecurityGroupRuleObservation{}, + EgressRules: []v1beta1.SecurityGroupRuleObservation{}, }, }, } for name, tc := range cases { t.Run(name, func(t *testing.T) { - r := GenerateSGObservation(tc.in.sg, tc.in.rules) + r := GenerateSGObservation(tc.in.sg, tc.in.rules) if diff := cmp.Diff(r, tc.out); diff != "" { t.Errorf("GenerateNetworkObservation(...): -want, +got:\n%s", diff) } diff --git a/pkg/controller/ec2/securitygroup/controller.go b/pkg/controller/ec2/securitygroup/controller.go index aa9d5d8675..6597e26f89 100644 --- a/pkg/controller/ec2/securitygroup/controller.go +++ b/pkg/controller/ec2/securitygroup/controller.go @@ -156,7 +156,10 @@ func (e *external) Observe(ctx context.Context, mgd resource.Managed) (managed.E securityGroupRulesResponse, err := e.sg.DescribeSecurityGroupRules(ctx, &awsec2.DescribeSecurityGroupRulesInput{ Filters: []awsec2types.Filter{ - awsec2types.Filter{Name: aws.String("group-id"), Values: []string{meta.GetExternalName(cr)}}, + { + Name: aws.String("group-id"), + Values: []string{meta.GetExternalName(cr)}, + }, }, }) if err != nil {