From 6bbd9d108536b34f412819e8b97558b01cc0c2af Mon Sep 17 00:00:00 2001 From: root Date: Mon, 26 Feb 2024 16:21:07 -0800 Subject: [PATCH 01/20] sg name field in crd --- .../v1alpha1/zz_generated.deepcopy.go | 1 - .../v1beta1/securitygrouppolicy_types.go | 10 ++ .../v1beta1/zz_generated.deepcopy.go | 22 +++- .../bases/vpcresources.k8s.aws_cninodes.yaml | 25 ++-- ...sources.k8s.aws_securitygrouppolicies.yaml | 115 +++++++++++------- config/rbac/role.yaml | 2 - config/webhook/manifests.yaml | 16 ++- 7 files changed, 122 insertions(+), 69 deletions(-) diff --git a/apis/vpcresources/v1alpha1/zz_generated.deepcopy.go b/apis/vpcresources/v1alpha1/zz_generated.deepcopy.go index 3fbda7f0..f30ae4fe 100644 --- a/apis/vpcresources/v1alpha1/zz_generated.deepcopy.go +++ b/apis/vpcresources/v1alpha1/zz_generated.deepcopy.go @@ -1,5 +1,4 @@ //go:build !ignore_autogenerated -// +build !ignore_autogenerated // Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. // diff --git a/apis/vpcresources/v1beta1/securitygrouppolicy_types.go b/apis/vpcresources/v1beta1/securitygrouppolicy_types.go index b929dcf5..122d4532 100644 --- a/apis/vpcresources/v1beta1/securitygrouppolicy_types.go +++ b/apis/vpcresources/v1beta1/securitygrouppolicy_types.go @@ -24,6 +24,15 @@ type SecurityGroupPolicySpec struct { PodSelector *metav1.LabelSelector `json:"podSelector,omitempty"` ServiceAccountSelector *metav1.LabelSelector `json:"serviceAccountSelector,omitempty"` SecurityGroups GroupIds `json:"securityGroups,omitempty"` + SecurityGroupNames GroupNames `json:"securityGroupNames,omitempty"` +} + +// GroupNames contains the list of security group names that will be applied to the network interface of the pod matching the criteria. +type GroupNames struct { + // Groups is the list of EC2 Security Group Names that need to be applied to the ENI of a Pod. + // +kubebuilder:validation:MinItems=1 + // +kubebuilder:validation:MaxItems=5 + GroupNames []string `json:"groupNames,omitempty"` } // GroupIds contains the list of security groups that will be applied to the network interface of the pod matching the criteria. @@ -45,6 +54,7 @@ type ServiceAccountSelector struct { // +kubebuilder:object:root=true // +kubebuilder:printcolumn:name="Security-Group-Ids",type=string,JSONPath=`.spec.securityGroups.groupIds`,description="The security group IDs to apply to the elastic network interface of pods that match this policy" +// +kubebuilder:printcolumn:name="Security-Group-Names",type=string,JSONPath=`.spec.securityGroups.groupNames`,description="The security group names to apply to the elastic network interface of pods that match this policy" // +kubebuilder:resource:shortName=sgp // Custom Resource Definition for applying security groups to pods diff --git a/apis/vpcresources/v1beta1/zz_generated.deepcopy.go b/apis/vpcresources/v1beta1/zz_generated.deepcopy.go index d3910ca5..0d2fdbba 100644 --- a/apis/vpcresources/v1beta1/zz_generated.deepcopy.go +++ b/apis/vpcresources/v1beta1/zz_generated.deepcopy.go @@ -1,5 +1,4 @@ //go:build !ignore_autogenerated -// +build !ignore_autogenerated // Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. // @@ -43,6 +42,26 @@ func (in *GroupIds) DeepCopy() *GroupIds { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *GroupNames) DeepCopyInto(out *GroupNames) { + *out = *in + if in.GroupNames != nil { + in, out := &in.GroupNames, &out.GroupNames + *out = make([]string, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GroupNames. +func (in *GroupNames) DeepCopy() *GroupNames { + if in == nil { + return nil + } + out := new(GroupNames) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SecurityGroupPolicy) DeepCopyInto(out *SecurityGroupPolicy) { *out = *in @@ -115,6 +134,7 @@ func (in *SecurityGroupPolicySpec) DeepCopyInto(out *SecurityGroupPolicySpec) { (*in).DeepCopyInto(*out) } in.SecurityGroups.DeepCopyInto(&out.SecurityGroups) + in.SecurityGroupNames.DeepCopyInto(&out.SecurityGroupNames) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SecurityGroupPolicySpec. diff --git a/config/crd/bases/vpcresources.k8s.aws_cninodes.yaml b/config/crd/bases/vpcresources.k8s.aws_cninodes.yaml index 393a50ab..e692b586 100644 --- a/config/crd/bases/vpcresources.k8s.aws_cninodes.yaml +++ b/config/crd/bases/vpcresources.k8s.aws_cninodes.yaml @@ -3,8 +3,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.9.0 - creationTimestamp: null + controller-gen.kubebuilder.io/version: v0.14.0 name: cninodes.vpcresources.k8s.aws spec: group: vpcresources.k8s.aws @@ -27,20 +26,26 @@ spec: openAPIV3Schema: properties: apiVersion: - description: 'APIVersion defines the versioned schema of this representation - of an object. Servers should convert recognized schemas to the latest - internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources type: string kind: - description: 'Kind is a string value representing the REST resource this - object represents. Servers may infer this from the endpoint the client - submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds type: string metadata: type: object spec: - description: 'Important: Run "make" to regenerate code after modifying - this file CNINodeSpec defines the desired state of CNINode' + description: |- + Important: Run "make" to regenerate code after modifying this file + CNINodeSpec defines the desired state of CNINode properties: features: items: diff --git a/config/crd/bases/vpcresources.k8s.aws_securitygrouppolicies.yaml b/config/crd/bases/vpcresources.k8s.aws_securitygrouppolicies.yaml index 64d4aac0..672f1e3b 100644 --- a/config/crd/bases/vpcresources.k8s.aws_securitygrouppolicies.yaml +++ b/config/crd/bases/vpcresources.k8s.aws_securitygrouppolicies.yaml @@ -3,8 +3,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.9.0 - creationTimestamp: null + controller-gen.kubebuilder.io/version: v0.14.0 name: securitygrouppolicies.vpcresources.k8s.aws spec: group: vpcresources.k8s.aws @@ -23,20 +22,30 @@ spec: jsonPath: .spec.securityGroups.groupIds name: Security-Group-Ids type: string + - description: The security group names to apply to the elastic network interface + of pods that match this policy + jsonPath: .spec.securityGroups.groupNames + name: Security-Group-Names + type: string name: v1beta1 schema: openAPIV3Schema: description: Custom Resource Definition for applying security groups to pods properties: apiVersion: - description: 'APIVersion defines the versioned schema of this representation - of an object. Servers should convert recognized schemas to the latest - internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources type: string kind: - description: 'Kind is a string value representing the REST resource this - object represents. Servers may infer this from the endpoint the client - submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds type: string metadata: type: object @@ -44,33 +53,33 @@ spec: description: SecurityGroupPolicySpec defines the desired state of SecurityGroupPolicy properties: podSelector: - description: A label selector is a label query over a set of resources. - The result of matchLabels and matchExpressions are ANDed. An empty - label selector matches all objects. A null label selector matches - no objects. + description: |- + A label selector is a label query over a set of resources. The result of matchLabels and + matchExpressions are ANDed. An empty label selector matches all objects. A null + label selector matches no objects. properties: matchExpressions: description: matchExpressions is a list of label selector requirements. The requirements are ANDed. items: - description: A label selector requirement is a selector that - contains values, a key, and an operator that relates the key - and values. + description: |- + A label selector requirement is a selector that contains values, a key, and an operator that + relates the key and values. properties: key: description: key is the label key that the selector applies to. type: string operator: - description: operator represents a key's relationship to - a set of values. Valid operators are In, NotIn, Exists - and DoesNotExist. + description: |- + operator represents a key's relationship to a set of values. + Valid operators are In, NotIn, Exists and DoesNotExist. type: string values: - description: values is an array of string values. If the - operator is In or NotIn, the values array must be non-empty. - If the operator is Exists or DoesNotExist, the values - array must be empty. This array is replaced during a strategic + description: |- + values is an array of string values. If the operator is In or NotIn, + the values array must be non-empty. If the operator is Exists or DoesNotExist, + the values array must be empty. This array is replaced during a strategic merge patch. items: type: string @@ -83,13 +92,27 @@ spec: matchLabels: additionalProperties: type: string - description: matchLabels is a map of {key,value} pairs. A single - {key,value} in the matchLabels map is equivalent to an element - of matchExpressions, whose key field is "key", the operator - is "In", and the values array contains only "value". The requirements - are ANDed. + description: |- + matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels + map is equivalent to an element of matchExpressions, whose key field is "key", the + operator is "In", and the values array contains only "value". The requirements are ANDed. type: object type: object + x-kubernetes-map-type: atomic + securityGroupNames: + description: GroupNames contains the list of security group names + that will be applied to the network interface of the pod matching + the criteria. + properties: + groupNames: + description: Groups is the list of EC2 Security Group Names that + need to be applied to the ENI of a Pod. + items: + type: string + maxItems: 5 + minItems: 1 + type: array + type: object securityGroups: description: GroupIds contains the list of security groups that will be applied to the network interface of the pod matching the criteria. @@ -104,33 +127,33 @@ spec: type: array type: object serviceAccountSelector: - description: A label selector is a label query over a set of resources. - The result of matchLabels and matchExpressions are ANDed. An empty - label selector matches all objects. A null label selector matches - no objects. + description: |- + A label selector is a label query over a set of resources. The result of matchLabels and + matchExpressions are ANDed. An empty label selector matches all objects. A null + label selector matches no objects. properties: matchExpressions: description: matchExpressions is a list of label selector requirements. The requirements are ANDed. items: - description: A label selector requirement is a selector that - contains values, a key, and an operator that relates the key - and values. + description: |- + A label selector requirement is a selector that contains values, a key, and an operator that + relates the key and values. properties: key: description: key is the label key that the selector applies to. type: string operator: - description: operator represents a key's relationship to - a set of values. Valid operators are In, NotIn, Exists - and DoesNotExist. + description: |- + operator represents a key's relationship to a set of values. + Valid operators are In, NotIn, Exists and DoesNotExist. type: string values: - description: values is an array of string values. If the - operator is In or NotIn, the values array must be non-empty. - If the operator is Exists or DoesNotExist, the values - array must be empty. This array is replaced during a strategic + description: |- + values is an array of string values. If the operator is In or NotIn, + the values array must be non-empty. If the operator is Exists or DoesNotExist, + the values array must be empty. This array is replaced during a strategic merge patch. items: type: string @@ -143,13 +166,13 @@ spec: matchLabels: additionalProperties: type: string - description: matchLabels is a map of {key,value} pairs. A single - {key,value} in the matchLabels map is equivalent to an element - of matchExpressions, whose key field is "key", the operator - is "In", and the values array contains only "value". The requirements - are ANDed. + description: |- + matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels + map is equivalent to an element of matchExpressions, whose key field is "key", the + operator is "In", and the values array contains only "value". The requirements are ANDed. type: object type: object + x-kubernetes-map-type: atomic type: object type: object served: true diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index b292d57b..70d80de7 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -2,7 +2,6 @@ apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: - creationTimestamp: null name: controller-role rules: - apiGroups: @@ -74,7 +73,6 @@ rules: apiVersion: rbac.authorization.k8s.io/v1 kind: Role metadata: - creationTimestamp: null name: controller-role namespace: kube-system rules: diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index 78f57a87..cf8e7b86 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -2,7 +2,6 @@ apiVersion: admissionregistration.k8s.io/v1 kind: MutatingWebhookConfiguration metadata: - creationTimestamp: null name: mutating-webhook-configuration webhooks: - admissionReviewVersions: @@ -29,7 +28,6 @@ webhooks: apiVersion: admissionregistration.k8s.io/v1 kind: ValidatingWebhookConfiguration metadata: - creationTimestamp: null name: validating-webhook-configuration webhooks: - admissionReviewVersions: @@ -38,20 +36,19 @@ webhooks: service: name: webhook-service namespace: system - path: /validate-v1-pod + path: /validate-v1-node failurePolicy: Ignore matchPolicy: Equivalent - name: vpod.vpc.k8s.aws + name: vnode.vpc.k8s.aws rules: - apiGroups: - "" apiVersions: - v1 operations: - - CREATE - UPDATE resources: - - pods + - nodes sideEffects: None - admissionReviewVersions: - v1 @@ -59,17 +56,18 @@ webhooks: service: name: webhook-service namespace: system - path: /validate-v1-node + path: /validate-v1-pod failurePolicy: Ignore matchPolicy: Equivalent - name: vnode.vpc.k8s.aws + name: vpod.vpc.k8s.aws rules: - apiGroups: - "" apiVersions: - v1 operations: + - CREATE - UPDATE resources: - - nodes + - pods sideEffects: None From 830d0f889dbd9c02753467b00efe945ee2a8fe28 Mon Sep 17 00:00:00 2001 From: GnatorX Date: Mon, 26 Feb 2024 16:55:49 -0800 Subject: [PATCH 02/20] add implementation --- main.go | 6 ++- pkg/aws/ec2/api/helper.go | 72 +++++++++++++++++++++++++++++++++- pkg/aws/ec2/api/helper_test.go | 64 +++++++++++++++++++++++++++++- pkg/aws/ec2/api/wrapper.go | 38 ++++++++++++++++-- pkg/utils/helper.go | 23 +++++++++-- pkg/utils/httpClient.go | 2 +- pkg/utils/httpClient_test.go | 2 +- 7 files changed, 194 insertions(+), 13 deletions(-) diff --git a/main.go b/main.go index 5b867f29..f6c28844 100644 --- a/main.go +++ b/main.go @@ -107,6 +107,8 @@ func main() { var healthCheckTimeout int var enableWindowsPrefixDelegation bool var region string + var workerNodeVpcId string + flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.") @@ -141,6 +143,8 @@ func main() { flag.BoolVar(&enableWindowsPrefixDelegation, "enable-windows-prefix-delegation", false, "Enable the feature flag for Windows prefix delegation") flag.StringVar(®ion, "aws-region", "", "The aws region of the k8s cluster") + flag.StringVar(&workerNodeVpcId, "worker-node-vpc-id", "", "The VPC ID for the worker nodes,"+ + "to be used when using with security group names") flag.Parse() @@ -267,7 +271,7 @@ func main() { if err != nil { setupLog.Error(err, "unable to create ec2 wrapper") } - ec2APIHelper := ec2API.NewEC2APIHelper(ec2Wrapper, clusterName) + ec2APIHelper := ec2API.NewEC2APIHelper(ec2Wrapper, clusterName, workerNodeVpcId) sgpAPI := utils.NewSecurityGroupForPodsAPI( mgr.GetClient(), diff --git a/pkg/aws/ec2/api/helper.go b/pkg/aws/ec2/api/helper.go index 3a6cb3ea..1030ddc3 100644 --- a/pkg/aws/ec2/api/helper.go +++ b/pkg/aws/ec2/api/helper.go @@ -61,16 +61,38 @@ var ( type ec2APIHelper struct { ec2Wrapper EC2Wrapper + workerNodeVpcId string + securityGroupNameToIdMap map[string]string } -func NewEC2APIHelper(ec2Wrapper EC2Wrapper, clusterName string) EC2APIHelper { +func NewEC2APIHelper(ec2Wrapper EC2Wrapper, clusterName, workerNodeVpcId string) EC2APIHelper { // Set the key and value of the cluster name tag which will be used to tag all the network interfaces created by // the controller clusterNameTag = &ec2.Tag{ Key: aws.String(fmt.Sprintf(config.ClusterNameTagKeyFormat, clusterName)), Value: aws.String(config.ClusterNameTagValue), } - return &ec2APIHelper{ec2Wrapper: ec2Wrapper} + return &ec2APIHelper{ + ec2Wrapper: ec2Wrapper, + workerNodeVpcId: workerNodeVpcId, + securityGroupNameToIdMap: make(map[string]string), + } +} + +// For unit testing +func newEC2APIHelper(ec2Wrapper EC2Wrapper, clusterName, workerNodeVpcId string, + securityGroupNameToIdMap map[string]string) EC2APIHelper { + // Set the key and value of the cluster name tag which will be used to tag all the network interfaces created by + // the controller + clusterNameTag = &ec2.Tag{ + Key: aws.String(fmt.Sprintf(config.ClusterNameTagKeyFormat, clusterName)), + Value: aws.String(config.ClusterNameTagValue), + } + return &ec2APIHelper{ + ec2Wrapper: ec2Wrapper, + workerNodeVpcId: workerNodeVpcId, + securityGroupNameToIdMap: securityGroupNameToIdMap, + } } type EC2APIHelper interface { @@ -93,6 +115,7 @@ type EC2APIHelper interface { GetInstanceDetails(instanceId *string) (*ec2.Instance, error) AssignIPv4ResourcesAndWaitTillReady(eniID string, resourceType config.ResourceType, count int) ([]string, error) UnassignIPv4Resources(eniID string, resourceType config.ResourceType, resources []string) error + GetSecurityGroupIdsForSecurityGroupNames(securityGroupNames []string) ([]string, error) } // CreateNetworkInterface creates a new network interface @@ -617,3 +640,48 @@ func (h *ec2APIHelper) DetachAndDeleteNetworkInterface(attachmentID *string, nwI } return nil } + +func (h *ec2APIHelper) GetSecurityGroupIdsForSecurityGroupNames(securityGroupNames []string) ([]string, error) { + sgIds := make([]string, 0) + if len(securityGroupNames) < 1 { + return sgIds, nil + } + sgNamesNotInCache := make([]string, 0) + for _, sgName := range securityGroupNames { + if sgId, ok := h.securityGroupNameToIdMap[sgName]; !ok { + sgNamesNotInCache = append(sgNamesNotInCache, sgName) + } else { + sgIds = append(sgIds, sgId) + } + } + if len(sgIds) == len(securityGroupNames) { + //All SGNames found in cache + return sgIds, nil + } + filters := []*ec2.Filter{ + { + Name: aws.String("group-name"), + Values: aws.StringSlice(securityGroupNames), + }, + { + Name: aws.String("vpc-id"), + Values: aws.StringSlice([]string{h.workerNodeVpcId}), + }, + } + input := &ec2.DescribeSecurityGroupsInput{Filters: filters} + for { + output, err := h.ec2Wrapper.DescribeSecurityGroups(input) + if err != nil { + return nil, err + } + for _, sg := range output.SecurityGroups { + h.securityGroupNameToIdMap[*sg.GroupName] = *sg.GroupId + sgIds = append(sgIds, *sg.GroupId) + } + if output.NextToken == nil { + break + } + input.NextToken = output.NextToken + } + return sgIds, nil +} \ No newline at end of file diff --git a/pkg/aws/ec2/api/helper_test.go b/pkg/aws/ec2/api/helper_test.go index 971e8211..f9afd01e 100644 --- a/pkg/aws/ec2/api/helper_test.go +++ b/pkg/aws/ec2/api/helper_test.go @@ -30,6 +30,7 @@ import ( var ( clusterName = "cluster-name" + workerNodeVpcId = "vpc-abcdef" // instance id of EC2 worker node instanceId = "i-00000000000000000" instanceType = "m5.xlarge" @@ -58,6 +59,10 @@ var ( securityGroup2 = "sg-00000000000000002" securityGroups = []string{securityGroup1, securityGroup2} + securityGroupName1 = "db-sg" + securityGroupName2 = "redis-sg" + securityGroupNames = []string{securityGroupName1, securityGroupName2} + tags = []*ec2.Tag{ { Key: aws.String("mock-key"), @@ -323,6 +328,32 @@ var ( } maxRetryOnError = 3 + + + describeSecurityGroupInput = &ec2.DescribeSecurityGroupsInput{ + Filters: []*ec2.Filter{ + { + Name: aws.String("group-name"), + Values: aws.StringSlice(securityGroupNames), + }, + { + Name: aws.String("vpc-id"), + Values: aws.StringSlice([]string{workerNodeVpcId}), + }, + }, + } + describeSecurityGroupOutput = &ec2.DescribeSecurityGroupsOutput{ + SecurityGroups: []*ec2.SecurityGroup{ + { + GroupId: aws.String(securityGroup1), + GroupName: aws.String(securityGroupName1), + }, + { + GroupId: aws.String(securityGroup2), + GroupName: aws.String(securityGroupName2), + }, + }, + } ) // getMockWrapper returns the Mock EC2Wrapper along with the EC2APIHelper with mock EC2Wrapper set up @@ -351,7 +382,7 @@ func getMockWrapper(ctrl *gomock.Controller) (EC2APIHelper, *mock_api.MockEC2Wra } mockWrapper := mock_api.NewMockEC2Wrapper(ctrl) - ec2ApiHelper := NewEC2APIHelper(mockWrapper, clusterName) + ec2ApiHelper := NewEC2APIHelper(mockWrapper, clusterName, workerNodeVpcId) return ec2ApiHelper, mockWrapper } @@ -1191,3 +1222,34 @@ func TestEc2APIHelper_GetBranchNetworkInterface_PaginatedResults(t *testing.T) { assert.NoError(t, err) assert.ElementsMatch(t, []*ec2.NetworkInterface{&networkInterface1, &networkInterface2}, branchInterfaces) } + +func TestEc2APIHelper_GetSecurityGroupIdsForSecurityGroupNames_EmptyCache(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + ec2ApiHelper, mockWrapper := getMockWrapper(ctrl) + + mockWrapper.EXPECT().DescribeSecurityGroups(describeSecurityGroupInput).Return(describeSecurityGroupOutput, nil) + + securityGroupIds, err := ec2ApiHelper.GetSecurityGroupIdsForSecurityGroupNames(securityGroupNames) + + assert.Nil(t, err) + assert.Equal(t, securityGroupIds[0], securityGroup1) + assert.Equal(t, securityGroupIds[1], securityGroup2) + assert.Equal(t, len(securityGroupIds), 2) +} + +func TestEc2APIHelper_GetSecurityGroupIdsForSecurityGroupNames_FullCache(t *testing.T) { + securityGroupNameToIdMap := map[string]string{ + securityGroupName1: securityGroup1, + securityGroupName2: securityGroup2, + } + ec2ApiHelper := newEC2APIHelper(nil, clusterName, workerNodeVpcId, securityGroupNameToIdMap) + + securityGroupIds, err := ec2ApiHelper.GetSecurityGroupIdsForSecurityGroupNames(securityGroupNames) + + assert.Nil(t, err) + assert.Equal(t, securityGroupIds[0], securityGroup1) + assert.Equal(t, securityGroupIds[1], securityGroup2) + assert.Equal(t, len(securityGroupIds), 2) +} \ No newline at end of file diff --git a/pkg/aws/ec2/api/wrapper.go b/pkg/aws/ec2/api/wrapper.go index bcf4cc74..11f863ec 100644 --- a/pkg/aws/ec2/api/wrapper.go +++ b/pkg/aws/ec2/api/wrapper.go @@ -20,7 +20,7 @@ import ( "time" "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/config" - "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/utils" + "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/httpclient" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/credentials" @@ -58,6 +58,7 @@ type EC2Wrapper interface { DescribeTrunkInterfaceAssociations(input *ec2.DescribeTrunkInterfaceAssociationsInput) (*ec2.DescribeTrunkInterfaceAssociationsOutput, error) ModifyNetworkInterfaceAttribute(input *ec2.ModifyNetworkInterfaceAttributeInput) (*ec2.ModifyNetworkInterfaceAttributeOutput, error) CreateNetworkInterfacePermission(input *ec2.CreateNetworkInterfacePermissionInput) (*ec2.CreateNetworkInterfacePermissionOutput, error) + DescribeSecurityGroups(input *ec2.DescribeSecurityGroupsInput) (*ec2.DescribeSecurityGroupsOutput, error) } var ( @@ -307,6 +308,20 @@ var ( }, ) + ec2DescribeSecurityGroupsCallCnt = prometheus.NewCounter( + prometheus.CounterOpts{ + Name: "ec2_describe_security_groups_api_call_count", + Help: "The number of calls made to describe security groups", + }, + ) + + ec2DescribeSecurityGroupsErrCnt = prometheus.NewCounter( + prometheus.CounterOpts{ + Name: "ec2_describe_security_groups_api_err_count", + Help: "The number of errors encountered while making calls to describe security groups", + }, + ) + prometheusRegistered = false ) @@ -344,6 +359,8 @@ func prometheusRegister() { ec2describeTrunkInterfaceAssociationAPIErrCnt, ec2modifyNetworkInterfaceAttributeAPICallCnt, ec2modifyNetworkInterfaceAttributeAPIErrCnt, + ec2DescribeSecurityGroupsCallCnt, + ec2DescribeSecurityGroupsErrCnt, ec2APICallLatencies, vpcCniLeakedENICleanupCnt, vpcrcLeakedENICleanupCnt, @@ -431,7 +448,7 @@ func (e *ec2Wrapper) getInstanceSession() (instanceSession *session.Session, err } func (e *ec2Wrapper) getInstanceServiceClient(qps int, burst int, instanceSession *session.Session) (*ec2.EC2, error) { - instanceClient, err := utils.NewRateLimitedClient(qps, burst) + instanceClient, err := httpclient.NewRateLimitedClient(qps, burst) if err != nil { return nil, fmt.Errorf("failed to create reate limited client with %d qps and %d burst: %v", qps, burst, err) @@ -448,7 +465,7 @@ func (e *ec2Wrapper) getClientUsingAssumedRole(instanceRegion, roleARN, clusterN injectUserAgent(&userStsSession.Handlers) // Create a rate limited http client for the - client, err := utils.NewRateLimitedClient(qps, burst) + client, err := httpclient.NewRateLimitedClient(qps, burst) if err != nil { return nil, fmt.Errorf("failed to create reate limited client with %d qps and %d burst: %v", qps, burst, err) } @@ -788,3 +805,18 @@ func (e *ec2Wrapper) CreateNetworkInterfacePermission(input *ec2.CreateNetworkIn return output, err } + +func (e *ec2Wrapper) DescribeSecurityGroups(input *ec2.DescribeSecurityGroupsInput) (*ec2.DescribeSecurityGroupsOutput, error) { + output, err := e.userServiceClient.DescribeSecurityGroups(input) + + // Metric Update + ec2APICallCnt.Inc() + ec2DescribeSecurityGroupsCallCnt.Inc() + + if err != nil { + ec2APIErrCnt.Inc() + ec2DescribeSecurityGroupsErrCnt.Inc() + } + + return output, err +} \ No newline at end of file diff --git a/pkg/utils/helper.go b/pkg/utils/helper.go index d6b6eeac..de3faad9 100644 --- a/pkg/utils/helper.go +++ b/pkg/utils/helper.go @@ -59,6 +59,7 @@ type SecurityGroupForPodsAPI interface { type SecurityGroupForPods struct { Client client.Client Log logr.Logger + Ec2Helper api.EC2APIHelper } // NewSecurityGroupForPodsAPI returns the SecurityGroupForPod APIs for common operations on objects @@ -102,7 +103,12 @@ func (s *SecurityGroupForPods) GetMatchingSecurityGroupForPods(pod *corev1.Pod) return nil, err } - sgList := s.filterPodSecurityGroups(sgpList, pod, sa) + sgList, err := s.filterPodSecurityGroups(sgpList, pod, sa) + if err != nil { + helperLog.Error(err, "Failed in associating pod to security groups") + return nil, err + } + if len(sgList) > 0 { helperLog.V(1).Info("Pod matched a SecurityGroupPolicy and will get the following Security Groups:", "Security Groups", sgList) @@ -113,13 +119,15 @@ func (s *SecurityGroupForPods) GetMatchingSecurityGroupForPods(pod *corev1.Pod) func (s *SecurityGroupForPods) filterPodSecurityGroups( sgpList *vpcresourcesv1beta1.SecurityGroupPolicyList, pod *corev1.Pod, - sa *corev1.ServiceAccount) []string { + sa *corev1.ServiceAccount) ([]string, error) { var sgList []string + var sgNameList []string sgpLogger := s.Log.WithValues("Pod name", pod.Name, "Pod namespace", pod.Namespace) for _, sgp := range sgpList.Items { hasPodSelector := sgp.Spec.PodSelector != nil hasSASelector := sgp.Spec.ServiceAccountSelector != nil - hasSecurityGroup := sgp.Spec.SecurityGroups.Groups != nil && len(sgp.Spec.SecurityGroups.Groups) > 0 + hasSecurityGroup := (sgp.Spec.SecurityGroups.Groups != nil && len(sgp.Spec.SecurityGroups.Groups) > 0) || + (sgp.Spec.SecurityGroupNames.GroupNames != nil && len(sgp.Spec.SecurityGroupNames.GroupNames) > 0) if (!hasPodSelector && !hasSASelector) || !hasSecurityGroup { sgpLogger.Info( @@ -155,11 +163,18 @@ func (s *SecurityGroupForPods) filterPodSecurityGroups( continue } + sgNameList = append(sgNameList, sgp.Spec.SecurityGroupNames.GroupNames...) sgList = append(sgList, sgp.Spec.SecurityGroups.Groups...) } + sgNameList = RemoveDuplicatedSg(sgNameList) + sgIdsForNames, err := s.Ec2Helper.GetSecurityGroupIdsForSecurityGroupNames(sgNameList) + if err != nil { + return nil, err + } + sgList = append(sgList, sgIdsForNames...) sgList = RemoveDuplicatedSg(sgList) - return sgList + return sgList, nil } // DeconstructIPsFromPrefix deconstructs a IPv4 prefix into a list of /32 IPv4 addresses diff --git a/pkg/utils/httpClient.go b/pkg/utils/httpClient.go index fcd68d6f..80b3d389 100644 --- a/pkg/utils/httpClient.go +++ b/pkg/utils/httpClient.go @@ -11,7 +11,7 @@ // express or implied. See the License for the specific language governing // permissions and limitations under the License. -package utils +package httpclient import ( "fmt" diff --git a/pkg/utils/httpClient_test.go b/pkg/utils/httpClient_test.go index 53695732..30dd993e 100644 --- a/pkg/utils/httpClient_test.go +++ b/pkg/utils/httpClient_test.go @@ -11,7 +11,7 @@ // express or implied. See the License for the specific language governing // permissions and limitations under the License. -package utils +package httpclient import ( "context" From 38059b85b0434c59a6d137654b005d69f423d2fc Mon Sep 17 00:00:00 2001 From: Garvin Pang Date: Tue, 26 Mar 2024 15:18:19 -0700 Subject: [PATCH 03/20] latest --- main.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/main.go b/main.go index 14f2a013..bd8dcc05 100644 --- a/main.go +++ b/main.go @@ -144,7 +144,6 @@ func main() { flag.StringVar(®ion, "aws-region", "", "The aws region of the k8s cluster") flag.StringVar(&vpcID, "vpc-id", "", "The vpc-id where EKS cluster is deployed") - flag.Parse() // Dev mode logging disabled by default, to enable set the enableDevLogging argument @@ -275,7 +274,7 @@ func main() { if err != nil { setupLog.Error(err, "unable to create ec2 wrapper") } - ec2APIHelper := ec2API.NewEC2APIHelper(ec2Wrapper, clusterName, workerNodeVpcId) + ec2APIHelper := ec2API.NewEC2APIHelper(ec2Wrapper, clusterName, vpcID) sgpAPI := utils.NewSecurityGroupForPodsAPI( mgr.GetClient(), From c58a0420352defbaee06d3d6bdbabb2169bb9451 Mon Sep 17 00:00:00 2001 From: Garvin Pang Date: Tue, 26 Mar 2024 15:23:45 -0700 Subject: [PATCH 04/20] add pkg --- pkg/httpclient/httpClient.go | 49 +++++++++++ pkg/httpclient/httpClient_test.go | 142 ++++++++++++++++++++++++++++++ 2 files changed, 191 insertions(+) create mode 100644 pkg/httpclient/httpClient.go create mode 100644 pkg/httpclient/httpClient_test.go diff --git a/pkg/httpclient/httpClient.go b/pkg/httpclient/httpClient.go new file mode 100644 index 00000000..80b3d389 --- /dev/null +++ b/pkg/httpclient/httpClient.go @@ -0,0 +1,49 @@ +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +package httpclient + +import ( + "fmt" + "net/http" + + "golang.org/x/time/rate" +) + +// NewRateLimitedClient returns a new HTTP client with rate limiter. +func NewRateLimitedClient(qps int, burst int) (*http.Client, error) { + if qps == 0 { + return http.DefaultClient, nil + } + if burst < 1 { + return nil, fmt.Errorf("burst expected >0, got %d", burst) + } + return &http.Client{ + Transport: &rateLimitedRoundTripper{ + rt: http.DefaultTransport, + rl: rate.NewLimiter(rate.Limit(qps), burst), + }, + }, nil +} + +type rateLimitedRoundTripper struct { + rt http.RoundTripper + rl *rate.Limiter +} + +func (rr *rateLimitedRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + if err := rr.rl.Wait(req.Context()); err != nil { + return nil, err + } + return rr.rt.RoundTrip(req) +} diff --git a/pkg/httpclient/httpClient_test.go b/pkg/httpclient/httpClient_test.go new file mode 100644 index 00000000..30dd993e --- /dev/null +++ b/pkg/httpclient/httpClient_test.go @@ -0,0 +1,142 @@ +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +package httpclient + +import ( + "context" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" +) + +func TestNewRateLimitedClient(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/test", testHandler) + + ts := httptest.NewServer(mux) + defer ts.Close() + + u := ts.URL + "/test" + + // requests are to be throttled if qps+burst < reqs + // estimated time: reqs / (qps+burst) seconds + tbs := []struct { + ctxTimeout time.Duration + qps int + burst int + requests int // concurrent requests + err string + }{ + { + qps: 1, + burst: 1, + requests: 4, + }, + { + qps: 8, + burst: 2, + requests: 20, + }, + { + // 20 concurrent ec2 API requests should exceed 1 QPS before 10ms + // thus rate limiter returns an error + ctxTimeout: 10 * time.Millisecond, + qps: 1, + burst: 1, + requests: 20, + err: `context deadline`, + // "Wait(n=1) would exceed context deadline" for requests before timeout + // "context deadline exceeded" for requests after timeout + }, + } + for idx, tt := range tbs { + cli, err := NewRateLimitedClient(tt.qps, tt.burst) + if err != nil { + t.Fatalf("#%d: failed to create a new client (%v)", idx, err) + } + + start := time.Now() + + errc := make(chan error, tt.requests) + for i := 0; i < tt.requests; i++ { + go func() { + var ctx context.Context + if tt.ctxTimeout > 0 { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(context.TODO(), tt.ctxTimeout) + defer cancel() + } else { + ctx = context.TODO() + } + req, err := http.NewRequest(http.MethodGet, u, nil) + if err != nil { + errc <- err + return + } + _, err = cli.Do(req.WithContext(ctx)) + errc <- err + }() + } + + failed := false + for i := 0; i < tt.requests; i++ { + err = <-errc + switch { + case tt.err == "": // expects no error + if err != nil { + t.Errorf("#%d-%d: unexpected error %v", idx, i, err) + } + case tt.err != "": // expects error + if err == nil { + // this means that the request did not get throttled. + continue + } + if !strings.Contains(err.Error(), tt.err) && + // TODO: why does this happen even when ctx is not canceled + // ref. https://github.com/golang/go/issues/36848 + !strings.Contains(err.Error(), "i/o timeout") { + t.Errorf("#%d-%d: expected %q, got %v", idx, i, tt.err, err) + } + failed = true + } + } + + if tt.err != "" && !failed { + t.Fatalf("#%d: expected failure %q, got no error", idx, tt.err) + } + + if tt.err == "" { + took := time.Since(start).Round(time.Second) + expectedTook := time.Duration(0) + if tt.qps+tt.burst < tt.requests { + expectedTook = (time.Duration(tt.requests/(tt.qps)) * time.Second) + } + if expectedTook > 0 && took > expectedTook { + t.Fatalf("with rate limit, requests expected took %v, got %v", expectedTook, took) + } + } + } +} + +func testHandler(w http.ResponseWriter, req *http.Request) { + switch req.Method { + case "GET": + fmt.Fprint(w, `test`) + default: + http.Error(w, "Method Not Allowed", 405) + } +} From 57e66e84723c4310320bb2f81ec9713cf5ebaca6 Mon Sep 17 00:00:00 2001 From: Garvin Pang Date: Tue, 26 Mar 2024 15:34:10 -0700 Subject: [PATCH 05/20] fix toolchain for now --- hack/toolchain.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hack/toolchain.sh b/hack/toolchain.sh index ce5ca130..2cca3a7e 100755 --- a/hack/toolchain.sh +++ b/hack/toolchain.sh @@ -10,7 +10,7 @@ main() { } tools() { - go install sigs.k8s.io/controller-runtime/tools/setup-envtest@latest + go install sigs.k8s.io/controller-runtime/tools/setup-envtest@c7e1dc9b go install sigs.k8s.io/controller-tools/cmd/controller-gen@v0.9.0 go install github.com/google/ko@latest From eb3f84f33d22928d7468db5b86cc5e843cc5b3d8 Mon Sep 17 00:00:00 2001 From: Garvin Pang Date: Tue, 26 Mar 2024 16:02:02 -0700 Subject: [PATCH 06/20] refactor --- pkg/aws/ec2/api/helper.go | 26 ++++++- pkg/aws/ec2/api/wrapper.go | 8 +- pkg/utils/helper.go | 30 +------- pkg/utils/helper_test.go | 129 +++++++++++++++++++++++++------ pkg/utils/httpClient.go | 49 ------------ pkg/utils/httpClient_test.go | 142 ----------------------------------- pkg/utils/setup_test.go | 51 ++++++++++--- 7 files changed, 177 insertions(+), 258 deletions(-) delete mode 100644 pkg/utils/httpClient.go delete mode 100644 pkg/utils/httpClient_test.go diff --git a/pkg/aws/ec2/api/helper.go b/pkg/aws/ec2/api/helper.go index 84e9ab38..cd9bb457 100644 --- a/pkg/aws/ec2/api/helper.go +++ b/pkg/aws/ec2/api/helper.go @@ -18,6 +18,7 @@ import ( "time" "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/arn" "github.com/aws/aws-sdk-go/service/ec2" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/util/retry" @@ -60,7 +61,7 @@ var ( ) type ec2APIHelper struct { - ec2Wrapper EC2Wrapper + ec2Wrapper EC2Wrapper workerNodeVpcId string securityGroupNameToIdMap map[string]string } @@ -661,4 +662,25 @@ func (h *ec2APIHelper) GetSecurityGroupIdsForSecurityGroupNames(securityGroupNam input.NextToken = output.NextToken } return sgIds, nil -} \ No newline at end of file +} + +// GetSourceAcctAndArn constructs source acct and arn and return them for use +func GetSourceAcctAndArn(roleARN, region, clusterName string) (string, string, error) { + // ARN format (https://docs.aws.amazon.com/IAM/latest/UserGuide/reference-arns.html) + // arn:partition:service:region:account-id:resource-type/resource-id + // IAM format, region is always blank + // arn:aws:iam::account:role/role-name-with-path + if !arn.IsARN(roleARN) { + return "", "", fmt.Errorf("incorrect ARN format for role %s", roleARN) + } else if region == "" { + return "", "", nil + } + + parsedArn, err := arn.Parse(roleARN) + if err != nil { + return "", "", err + } + + sourceArn := fmt.Sprintf("arn:%s:eks:%s:%s:cluster/%s", parsedArn.Partition, region, parsedArn.AccountID, clusterName) + return parsedArn.AccountID, sourceArn, nil +} diff --git a/pkg/aws/ec2/api/wrapper.go b/pkg/aws/ec2/api/wrapper.go index 629bf4ed..901429a7 100644 --- a/pkg/aws/ec2/api/wrapper.go +++ b/pkg/aws/ec2/api/wrapper.go @@ -23,7 +23,6 @@ import ( "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/httpclient" "k8s.io/apimachinery/pkg/util/wait" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/credentials" "github.com/aws/aws-sdk-go/aws/credentials/stscreds" @@ -322,6 +321,9 @@ var ( prometheus.CounterOpts{ Name: "ec2_describe_security_groups_api_err_count", Help: "The number of errors encountered while making calls to describe security groups", + }, + ) + ec2DescribeNetworkInterfacesPagesAPICallCnt = prometheus.NewCounter( prometheus.CounterOpts{ Name: "ec2_describe_network_interfaces_pages_api_call_count", @@ -497,7 +499,7 @@ func (e *ec2Wrapper) getClientUsingAssumedRole(instanceRegion, roleARN, clusterN roleARN = strings.Trim(roleARN, "\"") - sourceAcct, sourceArn, err := utils.GetSourceAcctAndArn(roleARN, region, clusterName) + sourceAcct, sourceArn, err := GetSourceAcctAndArn(roleARN, region, clusterName) if err != nil { return nil, err } @@ -867,4 +869,4 @@ func (e *ec2Wrapper) DescribeSecurityGroups(input *ec2.DescribeSecurityGroupsInp } return output, err -} \ No newline at end of file +} diff --git a/pkg/utils/helper.go b/pkg/utils/helper.go index de3faad9..45789469 100644 --- a/pkg/utils/helper.go +++ b/pkg/utils/helper.go @@ -20,10 +20,9 @@ import ( "strings" vpcresourcesv1beta1 "github.com/aws/amazon-vpc-resource-controller-k8s/apis/vpcresources/v1beta1" + "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/aws/ec2/api" "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/aws/vpc" - "github.com/aws/aws-sdk-go/aws/arn" - "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" @@ -57,8 +56,8 @@ type SecurityGroupForPodsAPI interface { } type SecurityGroupForPods struct { - Client client.Client - Log logr.Logger + Client client.Client + Log logr.Logger Ec2Helper api.EC2APIHelper } @@ -127,7 +126,7 @@ func (s *SecurityGroupForPods) filterPodSecurityGroups( hasPodSelector := sgp.Spec.PodSelector != nil hasSASelector := sgp.Spec.ServiceAccountSelector != nil hasSecurityGroup := (sgp.Spec.SecurityGroups.Groups != nil && len(sgp.Spec.SecurityGroups.Groups) > 0) || - (sgp.Spec.SecurityGroupNames.GroupNames != nil && len(sgp.Spec.SecurityGroupNames.GroupNames) > 0) + (sgp.Spec.SecurityGroupNames.GroupNames != nil && len(sgp.Spec.SecurityGroupNames.GroupNames) > 0) if (!hasPodSelector && !hasSASelector) || !hasSecurityGroup { sgpLogger.Info( @@ -226,24 +225,3 @@ func IsNitroInstance(instanceType string) (bool, error) { } return false, nil } - -// GetSourceAcctAndArn constructs source acct and arn and return them for use -func GetSourceAcctAndArn(roleARN, region, clusterName string) (string, string, error) { - // ARN format (https://docs.aws.amazon.com/IAM/latest/UserGuide/reference-arns.html) - // arn:partition:service:region:account-id:resource-type/resource-id - // IAM format, region is always blank - // arn:aws:iam::account:role/role-name-with-path - if !arn.IsARN(roleARN) { - return "", "", fmt.Errorf("incorrect ARN format for role %s", roleARN) - } else if region == "" { - return "", "", nil - } - - parsedArn, err := arn.Parse(roleARN) - if err != nil { - return "", "", err - } - - sourceArn := fmt.Sprintf("arn:%s:eks:%s:%s:cluster/%s", parsedArn.Partition, region, parsedArn.AccountID, clusterName) - return parsedArn.AccountID, sourceArn, nil -} diff --git a/pkg/utils/helper_test.go b/pkg/utils/helper_test.go index 34a4e4d2..eb088065 100644 --- a/pkg/utils/helper_test.go +++ b/pkg/utils/helper_test.go @@ -16,6 +16,7 @@ package utils import ( "testing" + "github.com/golang/mock/gomock" "github.com/samber/lo" "github.com/stretchr/testify/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -60,22 +61,76 @@ func TestCanInjectENI_CombinedSelectors(t *testing.T) { } // Combined SA selector and PodSelector - sgs := helper.filterPodSecurityGroups(sgpList, testPod, testSA) + sgs, err := helper.filterPodSecurityGroups(sgpList, testPod, testSA) + assert.Nil(t, err) assert.True(t, isEverySecurityGroupIncluded(sgs)) } // TestCanInjectENI_CombinedSelectors tests SGP with Pod selector. func TestCanInjectENI_PodSelectors(t *testing.T) { - // PodSelector alone - securityGroupPolicyPod := NewSecurityGroupPolicyPodSelector( - "test", "test_namespace", testSecurityGroupsOne) - sgpList := &vpcresourcesv1beta1.SecurityGroupPolicyList{ - TypeMeta: metav1.TypeMeta{}, - ListMeta: metav1.ListMeta{}, - Items: []vpcresourcesv1beta1.SecurityGroupPolicy{securityGroupPolicyPod}, + ctrl := gomock.NewController(t) + defer ctrl.Finish() + sgp, ec2WrapperMock := createHelper(ctrl) + + var sgNames, sgIds []string + for key, value := range testSecurityGroupNamesToIdMap { + sgNames = append(sgNames, key) + sgIds = append(sgIds, value) + } + + var tests = []struct { + testName string + sgpListInput *vpcresourcesv1beta1.SecurityGroupPolicyList + mockSecurityGroupIdsToReturn []string + expectedSgIds []string + }{ + { + testName: "OnlySecurityGroupIds", + sgpListInput: &vpcresourcesv1beta1.SecurityGroupPolicyList{ + TypeMeta: metav1.TypeMeta{}, + ListMeta: metav1.ListMeta{}, + Items: []vpcresourcesv1beta1.SecurityGroupPolicy{NewSecurityGroupPolicyPodSelector( + "test", "test_namespace", testSecurityGroupsOne, nil)}, + }, + mockSecurityGroupIdsToReturn: nil, + expectedSgIds: testSecurityGroupsOne, + }, + { + testName: "OnlySecurityGroupNames", + sgpListInput: &vpcresourcesv1beta1.SecurityGroupPolicyList{ + TypeMeta: metav1.TypeMeta{}, + ListMeta: metav1.ListMeta{}, + Items: []vpcresourcesv1beta1.SecurityGroupPolicy{NewSecurityGroupPolicyPodSelector( + "test", "test_namespace", nil, sgNames)}, + }, + mockSecurityGroupIdsToReturn: sgIds, + expectedSgIds: sgIds, + }, + { + testName: "BothSecurityGroupIdsAndSecurityGroupNames", + sgpListInput: &vpcresourcesv1beta1.SecurityGroupPolicyList{ + TypeMeta: metav1.TypeMeta{}, + ListMeta: metav1.ListMeta{}, + Items: []vpcresourcesv1beta1.SecurityGroupPolicy{NewSecurityGroupPolicyPodSelector( + "test", "test_namespace", testSecurityGroupsOne, sgNames)}, + }, + mockSecurityGroupIdsToReturn: sgIds, + expectedSgIds: append(sgIds, testSecurityGroupsOne...), + }, + } + + for _, test := range tests { + ec2WrapperMock.EXPECT().GetSecurityGroupIdsForSecurityGroupNames(test.sgpListInput.Items[0]. + Spec.SecurityGroupNames.GroupNames).Return(test.mockSecurityGroupIdsToReturn, nil) + sgIds, err := sgp.filterPodSecurityGroups(test.sgpListInput, testPod, testSA) + if err != nil { + t.Fatalf("Test %s failed", test.testName) + } + if !isEverySecurityGroupIncluded2(test.expectedSgIds, sgIds) { + t.Fatalf("Test Name: %s Failed. Expected SGs %v got %v. Test %s failed", test.testName, test.expectedSgIds, sgIds, + test.testName) + } } - sgs := helper.filterPodSecurityGroups(sgpList, testPod, testSA) - assert.True(t, isEverySecurityGroupIncluded(sgs)) } // TestCanInjectENI_SASelectors tests SGP with SA selector. @@ -88,7 +143,8 @@ func TestCanInjectENI_SASelectors(t *testing.T) { ListMeta: metav1.ListMeta{}, Items: []vpcresourcesv1beta1.SecurityGroupPolicy{securityGroupPolicySa}, } - sgs := helper.filterPodSecurityGroups(sgpList, testPod, testSA) + sgs, err := helper.filterPodSecurityGroups(sgpList, testPod, testSA) + assert.Nil(t, err) assert.True(t, isEverySecurityGroupIncluded(sgs)) } @@ -97,7 +153,7 @@ func TestCanInjectENI_Multi_SGPs(t *testing.T) { securityGroupPolicySa := NewSecurityGroupPolicySaSelector( name, namespace, []string{"sg-00001"}) securityGroupPolicyPod := NewSecurityGroupPolicyPodSelector( - name, namespace, []string{"sg-00002"}) + name, namespace, []string{"sg-00002"}, nil) sgsList := []vpcresourcesv1beta1.SecurityGroupPolicy{ securityGroupPolicySa, securityGroupPolicyPod} @@ -106,7 +162,8 @@ func TestCanInjectENI_Multi_SGPs(t *testing.T) { ListMeta: metav1.ListMeta{}, Items: sgsList, } - sgs := helper.filterPodSecurityGroups(sgpList, testPod, testSA) + sgs, err := helper.filterPodSecurityGroups(sgpList, testPod, testSA) + assert.Nil(t, err) assert.True(t, isEverySecurityGroupIncluded(sgs)) } @@ -120,7 +177,8 @@ func TestCanInjectENI_EmptyPodSelector(t *testing.T) { ListMeta: metav1.ListMeta{}, Items: []vpcresourcesv1beta1.SecurityGroupPolicy{securityGroupPolicyEmptyPodSelector}, } - sgs := helper.filterPodSecurityGroups(sgpList, testPod, testSA) + sgs, err := helper.filterPodSecurityGroups(sgpList, testPod, testSA) + assert.Nil(t, err) assert.True(t, isEverySecurityGroupIncluded(sgs)) } @@ -134,7 +192,8 @@ func TestCanInjectENI_EmptySASelector(t *testing.T) { ListMeta: metav1.ListMeta{}, Items: []vpcresourcesv1beta1.SecurityGroupPolicy{securityGroupPolicyEmptySaSelector}, } - sgs := helper.filterPodSecurityGroups(sgpList, testPod, testSA) + sgs, err := helper.filterPodSecurityGroups(sgpList, testPod, testSA) + assert.Nil(t, err) assert.True(t, isEverySecurityGroupIncluded(sgs)) } @@ -148,7 +207,8 @@ func TestCanInjectENI_MatchLabelSASelector(t *testing.T) { ListMeta: metav1.ListMeta{}, Items: []vpcresourcesv1beta1.SecurityGroupPolicy{securityGroupPolicyEmptySaSelector}, } - sgs := helper.filterPodSecurityGroups(sgpList, testPod, testSA) + sgs, err := helper.filterPodSecurityGroups(sgpList, testPod, testSA) + assert.Nil(t, err) assert.True(t, isEverySecurityGroupIncluded(sgs)) } @@ -162,7 +222,8 @@ func TestCanInjectENI_MatchExpressionsSASelector(t *testing.T) { ListMeta: metav1.ListMeta{}, Items: []vpcresourcesv1beta1.SecurityGroupPolicy{securityGroupPolicyEmptySaSelector}, } - sgs := helper.filterPodSecurityGroups(sgpList, testPod, testSA) + sgs, err := helper.filterPodSecurityGroups(sgpList, testPod, testSA) + assert.Nil(t, err) assert.True(t, isEverySecurityGroupIncluded(sgs)) } @@ -176,7 +237,8 @@ func TestCanInjectENI_MatchLabelPodSelector(t *testing.T) { ListMeta: metav1.ListMeta{}, Items: []vpcresourcesv1beta1.SecurityGroupPolicy{securityGroupPolicyEmptySaSelector}, } - sgs := helper.filterPodSecurityGroups(sgpList, testPod, testSA) + sgs, err := helper.filterPodSecurityGroups(sgpList, testPod, testSA) + assert.Nil(t, err) assert.True(t, isEverySecurityGroupIncluded(sgs)) } @@ -190,7 +252,8 @@ func TestCanInjectENI_MatchExpressionsPodSelector(t *testing.T) { ListMeta: metav1.ListMeta{}, Items: []vpcresourcesv1beta1.SecurityGroupPolicy{securityGroupPolicyEmptySaSelector}, } - sgs := helper.filterPodSecurityGroups(sgpList, testPod, testSA) + sgs, err := helper.filterPodSecurityGroups(sgpList, testPod, testSA) + assert.Nil(t, err) assert.True(t, isEverySecurityGroupIncluded(sgs)) } @@ -206,21 +269,23 @@ func TestCanInjectENI_MismatchedSASelector(t *testing.T) { } mismatchedSa := testSA.DeepCopy() mismatchedSa.Labels["environment"] = "dev" - sgs := helper.filterPodSecurityGroups(sgpList, testPod, mismatchedSa) + sgs, err := helper.filterPodSecurityGroups(sgpList, testPod, mismatchedSa) + assert.Nil(t, err) assert.True(t, len(sgs) == 0) } // TestEmptySecurityGroupInSGP tests empty security group groupids in SGP. func TestEmptySecurityGroupInSGP(t *testing.T) { securityGroupPolicyPod := NewSecurityGroupPolicyPodSelector( - "test", "test_namespace", testSecurityGroupsOne) + "test", "test_namespace", testSecurityGroupsOne, nil) securityGroupPolicyPod.Spec.SecurityGroups.Groups = []string{} sgpList := &vpcresourcesv1beta1.SecurityGroupPolicyList{ TypeMeta: metav1.TypeMeta{}, ListMeta: metav1.ListMeta{}, Items: []vpcresourcesv1beta1.SecurityGroupPolicy{securityGroupPolicyPod}, } - sgs := helper.filterPodSecurityGroups(sgpList, testPod, testSA) + sgs, err := helper.filterPodSecurityGroups(sgpList, testPod, testSA) + assert.Nil(t, err) assert.True(t, len(sgs) == 0) } @@ -249,6 +314,19 @@ func isEverySecurityGroupIncluded(retrievedSgs []string) bool { return true } +func isEverySecurityGroupIncluded2(expectedSgs, retrievedSgs []string) bool { + if len(expectedSgs) != len(retrievedSgs) { + return false + } + + for _, s := range retrievedSgs { + if !Include(s, expectedSgs) { + return false + } + } + return true +} + // NewSecurityGroupPolicyCombined creates a test SGP with both pod and SA selectors. func NewSecurityGroupPolicyCombined( name string, namespace string, securityGroups []string) vpcresourcesv1beta1.SecurityGroupPolicy { @@ -285,7 +363,7 @@ func NewSecurityGroupPolicyCombined( // NewSecurityGroupPolicyPodSelector creates a test SGP with only pod selector. func NewSecurityGroupPolicyPodSelector( - name string, namespace string, securityGroups []string) vpcresourcesv1beta1.SecurityGroupPolicy { + name string, namespace string, securityGroupIds []string, securityGroupNames []string) vpcresourcesv1beta1.SecurityGroupPolicy { sgp := vpcresourcesv1beta1.SecurityGroupPolicy{ TypeMeta: metav1.TypeMeta{}, ObjectMeta: metav1.ObjectMeta{ @@ -302,7 +380,10 @@ func NewSecurityGroupPolicyPodSelector( }}, }, SecurityGroups: vpcresourcesv1beta1.GroupIds{ - Groups: securityGroups, + Groups: securityGroupIds, + }, + SecurityGroupNames: vpcresourcesv1beta1.GroupNames{ + GroupNames: securityGroupNames, }, }, } diff --git a/pkg/utils/httpClient.go b/pkg/utils/httpClient.go deleted file mode 100644 index 80b3d389..00000000 --- a/pkg/utils/httpClient.go +++ /dev/null @@ -1,49 +0,0 @@ -// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"). You may -// not use this file except in compliance with the License. A copy of the -// License is located at -// -// http://aws.amazon.com/apache2.0/ -// -// or in the "license" file accompanying this file. This file is distributed -// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either -// express or implied. See the License for the specific language governing -// permissions and limitations under the License. - -package httpclient - -import ( - "fmt" - "net/http" - - "golang.org/x/time/rate" -) - -// NewRateLimitedClient returns a new HTTP client with rate limiter. -func NewRateLimitedClient(qps int, burst int) (*http.Client, error) { - if qps == 0 { - return http.DefaultClient, nil - } - if burst < 1 { - return nil, fmt.Errorf("burst expected >0, got %d", burst) - } - return &http.Client{ - Transport: &rateLimitedRoundTripper{ - rt: http.DefaultTransport, - rl: rate.NewLimiter(rate.Limit(qps), burst), - }, - }, nil -} - -type rateLimitedRoundTripper struct { - rt http.RoundTripper - rl *rate.Limiter -} - -func (rr *rateLimitedRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { - if err := rr.rl.Wait(req.Context()); err != nil { - return nil, err - } - return rr.rt.RoundTrip(req) -} diff --git a/pkg/utils/httpClient_test.go b/pkg/utils/httpClient_test.go deleted file mode 100644 index 30dd993e..00000000 --- a/pkg/utils/httpClient_test.go +++ /dev/null @@ -1,142 +0,0 @@ -// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"). You may -// not use this file except in compliance with the License. A copy of the -// License is located at -// -// http://aws.amazon.com/apache2.0/ -// -// or in the "license" file accompanying this file. This file is distributed -// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either -// express or implied. See the License for the specific language governing -// permissions and limitations under the License. - -package httpclient - -import ( - "context" - "fmt" - "net/http" - "net/http/httptest" - "strings" - "testing" - "time" -) - -func TestNewRateLimitedClient(t *testing.T) { - mux := http.NewServeMux() - mux.HandleFunc("/test", testHandler) - - ts := httptest.NewServer(mux) - defer ts.Close() - - u := ts.URL + "/test" - - // requests are to be throttled if qps+burst < reqs - // estimated time: reqs / (qps+burst) seconds - tbs := []struct { - ctxTimeout time.Duration - qps int - burst int - requests int // concurrent requests - err string - }{ - { - qps: 1, - burst: 1, - requests: 4, - }, - { - qps: 8, - burst: 2, - requests: 20, - }, - { - // 20 concurrent ec2 API requests should exceed 1 QPS before 10ms - // thus rate limiter returns an error - ctxTimeout: 10 * time.Millisecond, - qps: 1, - burst: 1, - requests: 20, - err: `context deadline`, - // "Wait(n=1) would exceed context deadline" for requests before timeout - // "context deadline exceeded" for requests after timeout - }, - } - for idx, tt := range tbs { - cli, err := NewRateLimitedClient(tt.qps, tt.burst) - if err != nil { - t.Fatalf("#%d: failed to create a new client (%v)", idx, err) - } - - start := time.Now() - - errc := make(chan error, tt.requests) - for i := 0; i < tt.requests; i++ { - go func() { - var ctx context.Context - if tt.ctxTimeout > 0 { - var cancel context.CancelFunc - ctx, cancel = context.WithTimeout(context.TODO(), tt.ctxTimeout) - defer cancel() - } else { - ctx = context.TODO() - } - req, err := http.NewRequest(http.MethodGet, u, nil) - if err != nil { - errc <- err - return - } - _, err = cli.Do(req.WithContext(ctx)) - errc <- err - }() - } - - failed := false - for i := 0; i < tt.requests; i++ { - err = <-errc - switch { - case tt.err == "": // expects no error - if err != nil { - t.Errorf("#%d-%d: unexpected error %v", idx, i, err) - } - case tt.err != "": // expects error - if err == nil { - // this means that the request did not get throttled. - continue - } - if !strings.Contains(err.Error(), tt.err) && - // TODO: why does this happen even when ctx is not canceled - // ref. https://github.com/golang/go/issues/36848 - !strings.Contains(err.Error(), "i/o timeout") { - t.Errorf("#%d-%d: expected %q, got %v", idx, i, tt.err, err) - } - failed = true - } - } - - if tt.err != "" && !failed { - t.Fatalf("#%d: expected failure %q, got no error", idx, tt.err) - } - - if tt.err == "" { - took := time.Since(start).Round(time.Second) - expectedTook := time.Duration(0) - if tt.qps+tt.burst < tt.requests { - expectedTook = (time.Duration(tt.requests/(tt.qps)) * time.Second) - } - if expectedTook > 0 && took > expectedTook { - t.Fatalf("with rate limit, requests expected took %v, got %v", expectedTook, took) - } - } - } -} - -func testHandler(w http.ResponseWriter, req *http.Request) { - switch req.Method { - case "GET": - fmt.Fprint(w, `test`) - default: - http.Error(w, "Method Not Allowed", 405) - } -} diff --git a/pkg/utils/setup_test.go b/pkg/utils/setup_test.go index aaf65034..0f65aeae 100644 --- a/pkg/utils/setup_test.go +++ b/pkg/utils/setup_test.go @@ -14,6 +14,8 @@ package utils import ( + mock_api "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/aws/ec2/api" + "github.com/golang/mock/gomock" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -23,21 +25,32 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" vpcresourcesv1beta1 "github.com/aws/amazon-vpc-resource-controller-k8s/apis/vpcresources/v1beta1" + "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/aws/ec2/api" ) var ( - testSA *corev1.ServiceAccount - testPod *corev1.Pod - testScheme *runtime.Scheme - testClient client.Client - testSecurityGroupsOne []string - testSecurityGroupsTwo []string - helper SecurityGroupForPods - name string - namespace string - saName string + testSA *corev1.ServiceAccount + testPod *corev1.Pod + testScheme *runtime.Scheme + testClient client.Client + testSecurityGroupsOne []string + testSecurityGroupsTwo []string + testSecurityGroupNamesToIdMap map[string]string + helper SecurityGroupForPods + name string + namespace string + saName string ) +// TODO: Use the mocks and actually correctly mock this +type FakeEc2Helper struct { + api.EC2APIHelper +} + +func (f *FakeEc2Helper) GetSecurityGroupIdsForSecurityGroupNames(securityGroupNames []string) ([]string, error) { + return nil, nil +} + func init() { name = "test" namespace = "test_namespace" @@ -50,6 +63,10 @@ func init() { testSecurityGroupsOne = []string{"sg-00001", "sg-00002"} testSecurityGroupsTwo = []string{"sg-00003", "sg-00004"} + testSecurityGroupNamesToIdMap = map[string]string{ + "db-sg": "sg-aaaa", + "cache-sg": "sg-bbbb", + } testClient = fake.NewClientBuilder().WithScheme(testScheme).WithObjects( NewPod(name, saName, namespace), NewPodNotForENI(name+"_NoENI", saName, namespace), @@ -60,11 +77,21 @@ func init() { ).Build() helper = SecurityGroupForPods{ - Client: testClient, - Log: ctrl.Log.WithName("testLog"), + Client: testClient, + Log: ctrl.Log.WithName("testLog"), + Ec2Helper: &FakeEc2Helper{}, } } +func createHelper(mockCtrl *gomock.Controller) (SecurityGroupForPods, *mock_api.MockEC2APIHelper) { + mockWrapper := mock_api.NewMockEC2APIHelper(mockCtrl) + return SecurityGroupForPods{ + Client: testClient, + Log: ctrl.Log.WithName("testLog"), + Ec2Helper: mockWrapper, + }, mockWrapper +} + // NewSecurityGroupPolicyOne creates a test security group policy's pointer. func NewSecurityGroupPolicyOne(name string, namespace string, securityGroups []string) *vpcresourcesv1beta1.SecurityGroupPolicy { sgp := &vpcresourcesv1beta1.SecurityGroupPolicy{ From eb8485e533912a12b917c8de0aacb06329b7891e Mon Sep 17 00:00:00 2001 From: Garvin Pang Date: Tue, 26 Mar 2024 16:44:24 -0700 Subject: [PATCH 07/20] fix make file to gen mocks --- Makefile | 1 + .../v1alpha1/zz_generated.deepcopy.go | 1 + .../v1beta1/zz_generated.deepcopy.go | 1 + .../bases/vpcresources.k8s.aws_cninodes.yaml | 25 ++--- ...sources.k8s.aws_securitygrouppolicies.yaml | 96 +++++++++---------- config/rbac/role.yaml | 2 + config/webhook/manifests.yaml | 16 ++-- .../pkg/aws/ec2/api/mock_ec2_apihelper.go | 28 +++--- .../pkg/aws/ec2/api/mock_ec2_wrapper.go | 28 +++--- .../pkg/aws/ec2/mock_instance.go | 13 --- .../pkg/condition/mock_condition.go | 13 --- .../pkg/handler/mock_handler.go | 13 --- .../pkg/k8s/mock_k8swrapper.go | 13 --- .../pkg/k8s/pod/mock_podapiwrapper.go | 13 --- .../pkg/node/manager/mock_manager.go | 13 --- .../pkg/node/mock_node.go | 13 --- .../pkg/pool/mock_pool.go | 13 --- .../provider/branch/cooldown/mock_cooldown.go | 13 --- .../pkg/provider/branch/trunk/mock_trunk.go | 13 --- .../pkg/provider/ip/eni/mock_eni.go | 13 --- .../pkg/provider/mock_provider.go | 13 --- .../pkg/resource/mock_resources.go | 13 --- .../pkg/utils/mock_k8shelper.go | 13 --- .../pkg/worker/mock_worker.go | 13 --- pkg/aws/ec2/api/helper_test.go | 48 +++++++++- pkg/utils/helper_test.go | 45 +-------- scripts/gen_mocks.sh | 34 +++---- 27 files changed, 163 insertions(+), 357 deletions(-) diff --git a/Makefile b/Makefile index 7f5e38aa..d7b27725 100644 --- a/Makefile +++ b/Makefile @@ -32,6 +32,7 @@ verify: go generate ./... go vet ./... go fmt ./... + ./scripts/gen_mocks.sh controller-gen crd rbac:roleName=controller-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases controller-gen object:headerFile="scripts/templates/boilerplate.go.txt" paths="./..." @git diff --quiet ||\ diff --git a/apis/vpcresources/v1alpha1/zz_generated.deepcopy.go b/apis/vpcresources/v1alpha1/zz_generated.deepcopy.go index f30ae4fe..3fbda7f0 100644 --- a/apis/vpcresources/v1alpha1/zz_generated.deepcopy.go +++ b/apis/vpcresources/v1alpha1/zz_generated.deepcopy.go @@ -1,4 +1,5 @@ //go:build !ignore_autogenerated +// +build !ignore_autogenerated // Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. // diff --git a/apis/vpcresources/v1beta1/zz_generated.deepcopy.go b/apis/vpcresources/v1beta1/zz_generated.deepcopy.go index 0d2fdbba..db53c2ec 100644 --- a/apis/vpcresources/v1beta1/zz_generated.deepcopy.go +++ b/apis/vpcresources/v1beta1/zz_generated.deepcopy.go @@ -1,4 +1,5 @@ //go:build !ignore_autogenerated +// +build !ignore_autogenerated // Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. // diff --git a/config/crd/bases/vpcresources.k8s.aws_cninodes.yaml b/config/crd/bases/vpcresources.k8s.aws_cninodes.yaml index e692b586..393a50ab 100644 --- a/config/crd/bases/vpcresources.k8s.aws_cninodes.yaml +++ b/config/crd/bases/vpcresources.k8s.aws_cninodes.yaml @@ -3,7 +3,8 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.14.0 + controller-gen.kubebuilder.io/version: v0.9.0 + creationTimestamp: null name: cninodes.vpcresources.k8s.aws spec: group: vpcresources.k8s.aws @@ -26,26 +27,20 @@ spec: openAPIV3Schema: properties: apiVersion: - description: |- - APIVersion defines the versioned schema of this representation of an object. - Servers should convert recognized schemas to the latest internal value, and - may reject unrecognized values. - More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + description: 'APIVersion defines the versioned schema of this representation + of an object. Servers should convert recognized schemas to the latest + internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' type: string kind: - description: |- - Kind is a string value representing the REST resource this object represents. - Servers may infer this from the endpoint the client submits requests to. - Cannot be updated. - In CamelCase. - More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + description: 'Kind is a string value representing the REST resource this + object represents. Servers may infer this from the endpoint the client + submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' type: string metadata: type: object spec: - description: |- - Important: Run "make" to regenerate code after modifying this file - CNINodeSpec defines the desired state of CNINode + description: 'Important: Run "make" to regenerate code after modifying + this file CNINodeSpec defines the desired state of CNINode' properties: features: items: diff --git a/config/crd/bases/vpcresources.k8s.aws_securitygrouppolicies.yaml b/config/crd/bases/vpcresources.k8s.aws_securitygrouppolicies.yaml index 672f1e3b..b1aab7db 100644 --- a/config/crd/bases/vpcresources.k8s.aws_securitygrouppolicies.yaml +++ b/config/crd/bases/vpcresources.k8s.aws_securitygrouppolicies.yaml @@ -3,7 +3,8 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.14.0 + controller-gen.kubebuilder.io/version: v0.9.0 + creationTimestamp: null name: securitygrouppolicies.vpcresources.k8s.aws spec: group: vpcresources.k8s.aws @@ -33,19 +34,14 @@ spec: description: Custom Resource Definition for applying security groups to pods properties: apiVersion: - description: |- - APIVersion defines the versioned schema of this representation of an object. - Servers should convert recognized schemas to the latest internal value, and - may reject unrecognized values. - More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + description: 'APIVersion defines the versioned schema of this representation + of an object. Servers should convert recognized schemas to the latest + internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' type: string kind: - description: |- - Kind is a string value representing the REST resource this object represents. - Servers may infer this from the endpoint the client submits requests to. - Cannot be updated. - In CamelCase. - More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + description: 'Kind is a string value representing the REST resource this + object represents. Servers may infer this from the endpoint the client + submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' type: string metadata: type: object @@ -53,33 +49,33 @@ spec: description: SecurityGroupPolicySpec defines the desired state of SecurityGroupPolicy properties: podSelector: - description: |- - A label selector is a label query over a set of resources. The result of matchLabels and - matchExpressions are ANDed. An empty label selector matches all objects. A null - label selector matches no objects. + description: A label selector is a label query over a set of resources. + The result of matchLabels and matchExpressions are ANDed. An empty + label selector matches all objects. A null label selector matches + no objects. properties: matchExpressions: description: matchExpressions is a list of label selector requirements. The requirements are ANDed. items: - description: |- - A label selector requirement is a selector that contains values, a key, and an operator that - relates the key and values. + description: A label selector requirement is a selector that + contains values, a key, and an operator that relates the key + and values. properties: key: description: key is the label key that the selector applies to. type: string operator: - description: |- - operator represents a key's relationship to a set of values. - Valid operators are In, NotIn, Exists and DoesNotExist. + description: operator represents a key's relationship to + a set of values. Valid operators are In, NotIn, Exists + and DoesNotExist. type: string values: - description: |- - values is an array of string values. If the operator is In or NotIn, - the values array must be non-empty. If the operator is Exists or DoesNotExist, - the values array must be empty. This array is replaced during a strategic + description: values is an array of string values. If the + operator is In or NotIn, the values array must be non-empty. + If the operator is Exists or DoesNotExist, the values + array must be empty. This array is replaced during a strategic merge patch. items: type: string @@ -92,13 +88,13 @@ spec: matchLabels: additionalProperties: type: string - description: |- - matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels - map is equivalent to an element of matchExpressions, whose key field is "key", the - operator is "In", and the values array contains only "value". The requirements are ANDed. + description: matchLabels is a map of {key,value} pairs. A single + {key,value} in the matchLabels map is equivalent to an element + of matchExpressions, whose key field is "key", the operator + is "In", and the values array contains only "value". The requirements + are ANDed. type: object type: object - x-kubernetes-map-type: atomic securityGroupNames: description: GroupNames contains the list of security group names that will be applied to the network interface of the pod matching @@ -127,33 +123,33 @@ spec: type: array type: object serviceAccountSelector: - description: |- - A label selector is a label query over a set of resources. The result of matchLabels and - matchExpressions are ANDed. An empty label selector matches all objects. A null - label selector matches no objects. + description: A label selector is a label query over a set of resources. + The result of matchLabels and matchExpressions are ANDed. An empty + label selector matches all objects. A null label selector matches + no objects. properties: matchExpressions: description: matchExpressions is a list of label selector requirements. The requirements are ANDed. items: - description: |- - A label selector requirement is a selector that contains values, a key, and an operator that - relates the key and values. + description: A label selector requirement is a selector that + contains values, a key, and an operator that relates the key + and values. properties: key: description: key is the label key that the selector applies to. type: string operator: - description: |- - operator represents a key's relationship to a set of values. - Valid operators are In, NotIn, Exists and DoesNotExist. + description: operator represents a key's relationship to + a set of values. Valid operators are In, NotIn, Exists + and DoesNotExist. type: string values: - description: |- - values is an array of string values. If the operator is In or NotIn, - the values array must be non-empty. If the operator is Exists or DoesNotExist, - the values array must be empty. This array is replaced during a strategic + description: values is an array of string values. If the + operator is In or NotIn, the values array must be non-empty. + If the operator is Exists or DoesNotExist, the values + array must be empty. This array is replaced during a strategic merge patch. items: type: string @@ -166,13 +162,13 @@ spec: matchLabels: additionalProperties: type: string - description: |- - matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels - map is equivalent to an element of matchExpressions, whose key field is "key", the - operator is "In", and the values array contains only "value". The requirements are ANDed. + description: matchLabels is a map of {key,value} pairs. A single + {key,value} in the matchLabels map is equivalent to an element + of matchExpressions, whose key field is "key", the operator + is "In", and the values array contains only "value". The requirements + are ANDed. type: object type: object - x-kubernetes-map-type: atomic type: object type: object served: true diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 70d80de7..b292d57b 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -2,6 +2,7 @@ apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: + creationTimestamp: null name: controller-role rules: - apiGroups: @@ -73,6 +74,7 @@ rules: apiVersion: rbac.authorization.k8s.io/v1 kind: Role metadata: + creationTimestamp: null name: controller-role namespace: kube-system rules: diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index cf8e7b86..78f57a87 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -2,6 +2,7 @@ apiVersion: admissionregistration.k8s.io/v1 kind: MutatingWebhookConfiguration metadata: + creationTimestamp: null name: mutating-webhook-configuration webhooks: - admissionReviewVersions: @@ -28,6 +29,7 @@ webhooks: apiVersion: admissionregistration.k8s.io/v1 kind: ValidatingWebhookConfiguration metadata: + creationTimestamp: null name: validating-webhook-configuration webhooks: - admissionReviewVersions: @@ -36,19 +38,20 @@ webhooks: service: name: webhook-service namespace: system - path: /validate-v1-node + path: /validate-v1-pod failurePolicy: Ignore matchPolicy: Equivalent - name: vnode.vpc.k8s.aws + name: vpod.vpc.k8s.aws rules: - apiGroups: - "" apiVersions: - v1 operations: + - CREATE - UPDATE resources: - - nodes + - pods sideEffects: None - admissionReviewVersions: - v1 @@ -56,18 +59,17 @@ webhooks: service: name: webhook-service namespace: system - path: /validate-v1-pod + path: /validate-v1-node failurePolicy: Ignore matchPolicy: Equivalent - name: vpod.vpc.k8s.aws + name: vnode.vpc.k8s.aws rules: - apiGroups: - "" apiVersions: - v1 operations: - - CREATE - UPDATE resources: - - pods + - nodes sideEffects: None diff --git a/mocks/amazon-vcp-resource-controller-k8s/pkg/aws/ec2/api/mock_ec2_apihelper.go b/mocks/amazon-vcp-resource-controller-k8s/pkg/aws/ec2/api/mock_ec2_apihelper.go index 19f7e104..8d287821 100644 --- a/mocks/amazon-vcp-resource-controller-k8s/pkg/aws/ec2/api/mock_ec2_apihelper.go +++ b/mocks/amazon-vcp-resource-controller-k8s/pkg/aws/ec2/api/mock_ec2_apihelper.go @@ -1,16 +1,3 @@ -// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"). You may -// not use this file except in compliance with the License. A copy of the -// License is located at -// -// http://aws.amazon.com/apache2.0/ -// -// or in the "license" file accompanying this file. This file is distributed -// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either -// express or implied. See the License for the specific language governing -// permissions and limitations under the License. - // Code generated by MockGen. DO NOT EDIT. // Source: github.com/aws/amazon-vpc-resource-controller-k8s/pkg/aws/ec2/api (interfaces: EC2APIHelper) @@ -240,6 +227,21 @@ func (mr *MockEC2APIHelperMockRecorder) GetInstanceNetworkInterface(arg0 interfa return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetInstanceNetworkInterface", reflect.TypeOf((*MockEC2APIHelper)(nil).GetInstanceNetworkInterface), arg0) } +// GetSecurityGroupIdsForSecurityGroupNames mocks base method. +func (m *MockEC2APIHelper) GetSecurityGroupIdsForSecurityGroupNames(arg0 []string) ([]string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetSecurityGroupIdsForSecurityGroupNames", arg0) + ret0, _ := ret[0].([]string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetSecurityGroupIdsForSecurityGroupNames indicates an expected call of GetSecurityGroupIdsForSecurityGroupNames. +func (mr *MockEC2APIHelperMockRecorder) GetSecurityGroupIdsForSecurityGroupNames(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSecurityGroupIdsForSecurityGroupNames", reflect.TypeOf((*MockEC2APIHelper)(nil).GetSecurityGroupIdsForSecurityGroupNames), arg0) +} + // GetSubnet mocks base method. func (m *MockEC2APIHelper) GetSubnet(arg0 *string) (*ec2.Subnet, error) { m.ctrl.T.Helper() diff --git a/mocks/amazon-vcp-resource-controller-k8s/pkg/aws/ec2/api/mock_ec2_wrapper.go b/mocks/amazon-vcp-resource-controller-k8s/pkg/aws/ec2/api/mock_ec2_wrapper.go index d89a5b9d..f39830bc 100644 --- a/mocks/amazon-vcp-resource-controller-k8s/pkg/aws/ec2/api/mock_ec2_wrapper.go +++ b/mocks/amazon-vcp-resource-controller-k8s/pkg/aws/ec2/api/mock_ec2_wrapper.go @@ -1,16 +1,3 @@ -// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"). You may -// not use this file except in compliance with the License. A copy of the -// License is located at -// -// http://aws.amazon.com/apache2.0/ -// -// or in the "license" file accompanying this file. This file is distributed -// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either -// express or implied. See the License for the specific language governing -// permissions and limitations under the License. - // Code generated by MockGen. DO NOT EDIT. // Source: github.com/aws/amazon-vpc-resource-controller-k8s/pkg/aws/ec2/api (interfaces: EC2Wrapper) @@ -197,6 +184,21 @@ func (mr *MockEC2WrapperMockRecorder) DescribeNetworkInterfacesPages(arg0 interf return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DescribeNetworkInterfacesPages", reflect.TypeOf((*MockEC2Wrapper)(nil).DescribeNetworkInterfacesPages), arg0) } +// DescribeSecurityGroups mocks base method. +func (m *MockEC2Wrapper) DescribeSecurityGroups(arg0 *ec2.DescribeSecurityGroupsInput) (*ec2.DescribeSecurityGroupsOutput, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DescribeSecurityGroups", arg0) + ret0, _ := ret[0].(*ec2.DescribeSecurityGroupsOutput) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// DescribeSecurityGroups indicates an expected call of DescribeSecurityGroups. +func (mr *MockEC2WrapperMockRecorder) DescribeSecurityGroups(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DescribeSecurityGroups", reflect.TypeOf((*MockEC2Wrapper)(nil).DescribeSecurityGroups), arg0) +} + // DescribeSubnets mocks base method. func (m *MockEC2Wrapper) DescribeSubnets(arg0 *ec2.DescribeSubnetsInput) (*ec2.DescribeSubnetsOutput, error) { m.ctrl.T.Helper() diff --git a/mocks/amazon-vcp-resource-controller-k8s/pkg/aws/ec2/mock_instance.go b/mocks/amazon-vcp-resource-controller-k8s/pkg/aws/ec2/mock_instance.go index d287cff4..53f71af7 100644 --- a/mocks/amazon-vcp-resource-controller-k8s/pkg/aws/ec2/mock_instance.go +++ b/mocks/amazon-vcp-resource-controller-k8s/pkg/aws/ec2/mock_instance.go @@ -1,16 +1,3 @@ -// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"). You may -// not use this file except in compliance with the License. A copy of the -// License is located at -// -// http://aws.amazon.com/apache2.0/ -// -// or in the "license" file accompanying this file. This file is distributed -// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either -// express or implied. See the License for the specific language governing -// permissions and limitations under the License. - // Code generated by MockGen. DO NOT EDIT. // Source: github.com/aws/amazon-vpc-resource-controller-k8s/pkg/aws/ec2 (interfaces: EC2Instance) diff --git a/mocks/amazon-vcp-resource-controller-k8s/pkg/condition/mock_condition.go b/mocks/amazon-vcp-resource-controller-k8s/pkg/condition/mock_condition.go index f2771980..4d6f08b4 100644 --- a/mocks/amazon-vcp-resource-controller-k8s/pkg/condition/mock_condition.go +++ b/mocks/amazon-vcp-resource-controller-k8s/pkg/condition/mock_condition.go @@ -1,16 +1,3 @@ -// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"). You may -// not use this file except in compliance with the License. A copy of the -// License is located at -// -// http://aws.amazon.com/apache2.0/ -// -// or in the "license" file accompanying this file. This file is distributed -// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either -// express or implied. See the License for the specific language governing -// permissions and limitations under the License. - // Code generated by MockGen. DO NOT EDIT. // Source: github.com/aws/amazon-vpc-resource-controller-k8s/pkg/condition (interfaces: Conditions) diff --git a/mocks/amazon-vcp-resource-controller-k8s/pkg/handler/mock_handler.go b/mocks/amazon-vcp-resource-controller-k8s/pkg/handler/mock_handler.go index 8ad52745..9570956b 100644 --- a/mocks/amazon-vcp-resource-controller-k8s/pkg/handler/mock_handler.go +++ b/mocks/amazon-vcp-resource-controller-k8s/pkg/handler/mock_handler.go @@ -1,16 +1,3 @@ -// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"). You may -// not use this file except in compliance with the License. A copy of the -// License is located at -// -// http://aws.amazon.com/apache2.0/ -// -// or in the "license" file accompanying this file. This file is distributed -// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either -// express or implied. See the License for the specific language governing -// permissions and limitations under the License. - // Code generated by MockGen. DO NOT EDIT. // Source: github.com/aws/amazon-vpc-resource-controller-k8s/pkg/handler (interfaces: Handler) diff --git a/mocks/amazon-vcp-resource-controller-k8s/pkg/k8s/mock_k8swrapper.go b/mocks/amazon-vcp-resource-controller-k8s/pkg/k8s/mock_k8swrapper.go index 9b376ede..07b6a4fe 100644 --- a/mocks/amazon-vcp-resource-controller-k8s/pkg/k8s/mock_k8swrapper.go +++ b/mocks/amazon-vcp-resource-controller-k8s/pkg/k8s/mock_k8swrapper.go @@ -1,16 +1,3 @@ -// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"). You may -// not use this file except in compliance with the License. A copy of the -// License is located at -// -// http://aws.amazon.com/apache2.0/ -// -// or in the "license" file accompanying this file. This file is distributed -// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either -// express or implied. See the License for the specific language governing -// permissions and limitations under the License. - // Code generated by MockGen. DO NOT EDIT. // Source: github.com/aws/amazon-vpc-resource-controller-k8s/pkg/k8s (interfaces: K8sWrapper) diff --git a/mocks/amazon-vcp-resource-controller-k8s/pkg/k8s/pod/mock_podapiwrapper.go b/mocks/amazon-vcp-resource-controller-k8s/pkg/k8s/pod/mock_podapiwrapper.go index a1ef9652..da1e79ba 100644 --- a/mocks/amazon-vcp-resource-controller-k8s/pkg/k8s/pod/mock_podapiwrapper.go +++ b/mocks/amazon-vcp-resource-controller-k8s/pkg/k8s/pod/mock_podapiwrapper.go @@ -1,16 +1,3 @@ -// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"). You may -// not use this file except in compliance with the License. A copy of the -// License is located at -// -// http://aws.amazon.com/apache2.0/ -// -// or in the "license" file accompanying this file. This file is distributed -// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either -// express or implied. See the License for the specific language governing -// permissions and limitations under the License. - // Code generated by MockGen. DO NOT EDIT. // Source: github.com/aws/amazon-vpc-resource-controller-k8s/pkg/k8s/pod (interfaces: PodClientAPIWrapper) diff --git a/mocks/amazon-vcp-resource-controller-k8s/pkg/node/manager/mock_manager.go b/mocks/amazon-vcp-resource-controller-k8s/pkg/node/manager/mock_manager.go index 092caf34..643e803f 100644 --- a/mocks/amazon-vcp-resource-controller-k8s/pkg/node/manager/mock_manager.go +++ b/mocks/amazon-vcp-resource-controller-k8s/pkg/node/manager/mock_manager.go @@ -1,16 +1,3 @@ -// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"). You may -// not use this file except in compliance with the License. A copy of the -// License is located at -// -// http://aws.amazon.com/apache2.0/ -// -// or in the "license" file accompanying this file. This file is distributed -// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either -// express or implied. See the License for the specific language governing -// permissions and limitations under the License. - // Code generated by MockGen. DO NOT EDIT. // Source: github.com/aws/amazon-vpc-resource-controller-k8s/pkg/node/manager (interfaces: Manager) diff --git a/mocks/amazon-vcp-resource-controller-k8s/pkg/node/mock_node.go b/mocks/amazon-vcp-resource-controller-k8s/pkg/node/mock_node.go index 59879491..0c96420e 100644 --- a/mocks/amazon-vcp-resource-controller-k8s/pkg/node/mock_node.go +++ b/mocks/amazon-vcp-resource-controller-k8s/pkg/node/mock_node.go @@ -1,16 +1,3 @@ -// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"). You may -// not use this file except in compliance with the License. A copy of the -// License is located at -// -// http://aws.amazon.com/apache2.0/ -// -// or in the "license" file accompanying this file. This file is distributed -// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either -// express or implied. See the License for the specific language governing -// permissions and limitations under the License. - // Code generated by MockGen. DO NOT EDIT. // Source: github.com/aws/amazon-vpc-resource-controller-k8s/pkg/node (interfaces: Node) diff --git a/mocks/amazon-vcp-resource-controller-k8s/pkg/pool/mock_pool.go b/mocks/amazon-vcp-resource-controller-k8s/pkg/pool/mock_pool.go index b0654a51..c5c29888 100644 --- a/mocks/amazon-vcp-resource-controller-k8s/pkg/pool/mock_pool.go +++ b/mocks/amazon-vcp-resource-controller-k8s/pkg/pool/mock_pool.go @@ -1,16 +1,3 @@ -// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"). You may -// not use this file except in compliance with the License. A copy of the -// License is located at -// -// http://aws.amazon.com/apache2.0/ -// -// or in the "license" file accompanying this file. This file is distributed -// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either -// express or implied. See the License for the specific language governing -// permissions and limitations under the License. - // Code generated by MockGen. DO NOT EDIT. // Source: github.com/aws/amazon-vpc-resource-controller-k8s/pkg/pool (interfaces: Pool) diff --git a/mocks/amazon-vcp-resource-controller-k8s/pkg/provider/branch/cooldown/mock_cooldown.go b/mocks/amazon-vcp-resource-controller-k8s/pkg/provider/branch/cooldown/mock_cooldown.go index ba1f1427..d4883986 100644 --- a/mocks/amazon-vcp-resource-controller-k8s/pkg/provider/branch/cooldown/mock_cooldown.go +++ b/mocks/amazon-vcp-resource-controller-k8s/pkg/provider/branch/cooldown/mock_cooldown.go @@ -1,16 +1,3 @@ -// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"). You may -// not use this file except in compliance with the License. A copy of the -// License is located at -// -// http://aws.amazon.com/apache2.0/ -// -// or in the "license" file accompanying this file. This file is distributed -// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either -// express or implied. See the License for the specific language governing -// permissions and limitations under the License. - // Code generated by MockGen. DO NOT EDIT. // Source: github.com/aws/amazon-vpc-resource-controller-k8s/pkg/provider/branch/cooldown (interfaces: CoolDown) diff --git a/mocks/amazon-vcp-resource-controller-k8s/pkg/provider/branch/trunk/mock_trunk.go b/mocks/amazon-vcp-resource-controller-k8s/pkg/provider/branch/trunk/mock_trunk.go index ac4b1c73..ef08dbf5 100644 --- a/mocks/amazon-vcp-resource-controller-k8s/pkg/provider/branch/trunk/mock_trunk.go +++ b/mocks/amazon-vcp-resource-controller-k8s/pkg/provider/branch/trunk/mock_trunk.go @@ -1,16 +1,3 @@ -// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"). You may -// not use this file except in compliance with the License. A copy of the -// License is located at -// -// http://aws.amazon.com/apache2.0/ -// -// or in the "license" file accompanying this file. This file is distributed -// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either -// express or implied. See the License for the specific language governing -// permissions and limitations under the License. - // Code generated by MockGen. DO NOT EDIT. // Source: github.com/aws/amazon-vpc-resource-controller-k8s/pkg/provider/branch/trunk (interfaces: TrunkENI) diff --git a/mocks/amazon-vcp-resource-controller-k8s/pkg/provider/ip/eni/mock_eni.go b/mocks/amazon-vcp-resource-controller-k8s/pkg/provider/ip/eni/mock_eni.go index 9cf897f8..b63526fa 100644 --- a/mocks/amazon-vcp-resource-controller-k8s/pkg/provider/ip/eni/mock_eni.go +++ b/mocks/amazon-vcp-resource-controller-k8s/pkg/provider/ip/eni/mock_eni.go @@ -1,16 +1,3 @@ -// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"). You may -// not use this file except in compliance with the License. A copy of the -// License is located at -// -// http://aws.amazon.com/apache2.0/ -// -// or in the "license" file accompanying this file. This file is distributed -// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either -// express or implied. See the License for the specific language governing -// permissions and limitations under the License. - // Code generated by MockGen. DO NOT EDIT. // Source: github.com/aws/amazon-vpc-resource-controller-k8s/pkg/provider/ip/eni (interfaces: ENIManager) diff --git a/mocks/amazon-vcp-resource-controller-k8s/pkg/provider/mock_provider.go b/mocks/amazon-vcp-resource-controller-k8s/pkg/provider/mock_provider.go index 0046284b..487180b1 100644 --- a/mocks/amazon-vcp-resource-controller-k8s/pkg/provider/mock_provider.go +++ b/mocks/amazon-vcp-resource-controller-k8s/pkg/provider/mock_provider.go @@ -1,16 +1,3 @@ -// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"). You may -// not use this file except in compliance with the License. A copy of the -// License is located at -// -// http://aws.amazon.com/apache2.0/ -// -// or in the "license" file accompanying this file. This file is distributed -// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either -// express or implied. See the License for the specific language governing -// permissions and limitations under the License. - // Code generated by MockGen. DO NOT EDIT. // Source: github.com/aws/amazon-vpc-resource-controller-k8s/pkg/provider (interfaces: ResourceProvider) diff --git a/mocks/amazon-vcp-resource-controller-k8s/pkg/resource/mock_resources.go b/mocks/amazon-vcp-resource-controller-k8s/pkg/resource/mock_resources.go index 328c2218..2867d39e 100644 --- a/mocks/amazon-vcp-resource-controller-k8s/pkg/resource/mock_resources.go +++ b/mocks/amazon-vcp-resource-controller-k8s/pkg/resource/mock_resources.go @@ -1,16 +1,3 @@ -// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"). You may -// not use this file except in compliance with the License. A copy of the -// License is located at -// -// http://aws.amazon.com/apache2.0/ -// -// or in the "license" file accompanying this file. This file is distributed -// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either -// express or implied. See the License for the specific language governing -// permissions and limitations under the License. - // Code generated by MockGen. DO NOT EDIT. // Source: github.com/aws/amazon-vpc-resource-controller-k8s/pkg/resource (interfaces: ResourceManager) diff --git a/mocks/amazon-vcp-resource-controller-k8s/pkg/utils/mock_k8shelper.go b/mocks/amazon-vcp-resource-controller-k8s/pkg/utils/mock_k8shelper.go index 1e6f3c27..b7217716 100644 --- a/mocks/amazon-vcp-resource-controller-k8s/pkg/utils/mock_k8shelper.go +++ b/mocks/amazon-vcp-resource-controller-k8s/pkg/utils/mock_k8shelper.go @@ -1,16 +1,3 @@ -// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"). You may -// not use this file except in compliance with the License. A copy of the -// License is located at -// -// http://aws.amazon.com/apache2.0/ -// -// or in the "license" file accompanying this file. This file is distributed -// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either -// express or implied. See the License for the specific language governing -// permissions and limitations under the License. - // Code generated by MockGen. DO NOT EDIT. // Source: github.com/aws/amazon-vpc-resource-controller-k8s/pkg/utils (interfaces: SecurityGroupForPodsAPI) diff --git a/mocks/amazon-vcp-resource-controller-k8s/pkg/worker/mock_worker.go b/mocks/amazon-vcp-resource-controller-k8s/pkg/worker/mock_worker.go index 831e6364..d14e3d95 100644 --- a/mocks/amazon-vcp-resource-controller-k8s/pkg/worker/mock_worker.go +++ b/mocks/amazon-vcp-resource-controller-k8s/pkg/worker/mock_worker.go @@ -1,16 +1,3 @@ -// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"). You may -// not use this file except in compliance with the License. A copy of the -// License is located at -// -// http://aws.amazon.com/apache2.0/ -// -// or in the "license" file accompanying this file. This file is distributed -// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either -// express or implied. See the License for the specific language governing -// permissions and limitations under the License. - // Code generated by MockGen. DO NOT EDIT. // Source: github.com/aws/amazon-vpc-resource-controller-k8s/pkg/worker (interfaces: Worker) diff --git a/pkg/aws/ec2/api/helper_test.go b/pkg/aws/ec2/api/helper_test.go index 6b0adb69..e0febd2a 100644 --- a/pkg/aws/ec2/api/helper_test.go +++ b/pkg/aws/ec2/api/helper_test.go @@ -29,7 +29,7 @@ import ( ) var ( - clusterName = "cluster-name" + clusterName = "cluster-name" workerNodeVpcId = "vpc-abcdef" // instance id of EC2 worker node instanceId = "i-00000000000000000" @@ -322,7 +322,6 @@ var ( maxRetryOnError = 3 - describeSecurityGroupInput = &ec2.DescribeSecurityGroupsInput{ Filters: []*ec2.Filter{ { @@ -1244,4 +1243,47 @@ func TestEc2APIHelper_GetSecurityGroupIdsForSecurityGroupNames_FullCache(t *test assert.Equal(t, securityGroupIds[0], securityGroup1) assert.Equal(t, securityGroupIds[1], securityGroup2) assert.Equal(t, len(securityGroupIds), 2) -} \ No newline at end of file +} + +// TestGetSourceAcctAndArn tests that generating account ID and cluster ARN +func TestGetSourceAcctAndArn(t *testing.T) { + accountID := "123456789876" + clusterName := "test-cluster" + region := "us-west-2" + clusterARN := "arn:aws:eks:us-west-2:123456789876:cluster/test-cluster" + + roleARN := "arn:aws:iam::123456789876:role/test-cluster" + + // test correct inputs + acct, arn, err := GetSourceAcctAndArn(roleARN, region, clusterName) + assert.NoError(t, err, "no error should be returned with accurate role arn") + assert.Equal(t, accountID, acct, "correct account ID should be retrieved") + assert.Equal(t, clusterARN, arn, "correct cluster arn should be retrieved") + + region = "us-gov-west-1" + roleARN = "arn:aws-us-gov:iam::123456789876:role/test-cluster" + clusterARN = "arn:aws-us-gov:eks:us-gov-west-1:123456789876:cluster/test-cluster" + acct, arn, err = GetSourceAcctAndArn(roleARN, region, clusterName) + assert.NoError(t, err, "no error should be returned with accurate aws-us-gov partition role arn") + assert.Equal(t, accountID, acct, "correct account ID should be retrieved") + assert.Equal(t, clusterARN, arn, "correct gov partition cluster arn should be retrieved") + + // test error handling + roleARN = "arn:aws:iam::123456789876" + _, _, err = GetSourceAcctAndArn(roleARN, region, clusterName) + assert.Error(t, err, "error should be returned with inaccurate role arn is given") +} + +// TestGetSourceAcctAndArn_NoRegion test empty region +func TestGetSourceAcctAndArn_NoRegion(t *testing.T) { + clusterName := "test-cluster" + region := "" + + roleARN := "arn:aws:iam::123456789876:role/test-cluster" + + // test correct inputs + acct, arn, err := GetSourceAcctAndArn(roleARN, region, clusterName) + assert.NoError(t, err, "no error should be returned with accurate role arn") + assert.Equal(t, "", acct, "correct account ID should be retrieved") + assert.Equal(t, "", arn, "correct cluster arn should be retrieved") +} diff --git a/pkg/utils/helper_test.go b/pkg/utils/helper_test.go index eb088065..e7e4b60e 100644 --- a/pkg/utils/helper_test.go +++ b/pkg/utils/helper_test.go @@ -320,7 +320,7 @@ func isEverySecurityGroupIncluded2(expectedSgs, retrievedSgs []string) bool { } for _, s := range retrievedSgs { - if !Include(s, expectedSgs) { + if !lo.Contains(expectedSgs, s) { return false } } @@ -612,46 +612,3 @@ func TestIsNitroInstance_NonNitro(t *testing.T) { assert.NoError(t, err) assert.False(t, isNitro) } - -// TestGetSourceAcctAndArn tests that generating account ID and cluster ARN -func TestGetSourceAcctAndArn(t *testing.T) { - accountID := "123456789876" - clusterName := "test-cluster" - region := "us-west-2" - clusterARN := "arn:aws:eks:us-west-2:123456789876:cluster/test-cluster" - - roleARN := "arn:aws:iam::123456789876:role/test-cluster" - - // test correct inputs - acct, arn, err := GetSourceAcctAndArn(roleARN, region, clusterName) - assert.NoError(t, err, "no error should be returned with accurate role arn") - assert.Equal(t, accountID, acct, "correct account ID should be retrieved") - assert.Equal(t, clusterARN, arn, "correct cluster arn should be retrieved") - - region = "us-gov-west-1" - roleARN = "arn:aws-us-gov:iam::123456789876:role/test-cluster" - clusterARN = "arn:aws-us-gov:eks:us-gov-west-1:123456789876:cluster/test-cluster" - acct, arn, err = GetSourceAcctAndArn(roleARN, region, clusterName) - assert.NoError(t, err, "no error should be returned with accurate aws-us-gov partition role arn") - assert.Equal(t, accountID, acct, "correct account ID should be retrieved") - assert.Equal(t, clusterARN, arn, "correct gov partition cluster arn should be retrieved") - - // test error handling - roleARN = "arn:aws:iam::123456789876" - _, _, err = GetSourceAcctAndArn(roleARN, region, clusterName) - assert.Error(t, err, "error should be returned with inaccurate role arn is given") -} - -// TestGetSourceAcctAndArn_NoRegion test empty region -func TestGetSourceAcctAndArn_NoRegion(t *testing.T) { - clusterName := "test-cluster" - region := "" - - roleARN := "arn:aws:iam::123456789876:role/test-cluster" - - // test correct inputs - acct, arn, err := GetSourceAcctAndArn(roleARN, region, clusterName) - assert.NoError(t, err, "no error should be returned with accurate role arn") - assert.Equal(t, "", acct, "correct account ID should be retrieved") - assert.Equal(t, "", arn, "correct cluster arn should be retrieved") -} diff --git a/scripts/gen_mocks.sh b/scripts/gen_mocks.sh index c0b65e9d..11b67f44 100755 --- a/scripts/gen_mocks.sh +++ b/scripts/gen_mocks.sh @@ -1,28 +1,28 @@ alias mockgen='mockgen -copyright_file=templates/copyright.txt' # package aws mocks -mockgen -destination=../mocks/amazon-vcp-resource-controller-k8s/pkg/aws/ec2/api/mock_ec2_wrapper.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/aws/ec2/api EC2Wrapper -mockgen -destination=../mocks/amazon-vcp-resource-controller-k8s/pkg/aws/ec2/api/mock_ec2_apihelper.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/aws/ec2/api EC2APIHelper -mockgen -destination=../mocks/amazon-vcp-resource-controller-k8s/pkg/aws/ec2/mock_instance.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/aws/ec2 EC2Instance +mockgen -destination=mocks/amazon-vcp-resource-controller-k8s/pkg/aws/ec2/api/mock_ec2_wrapper.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/aws/ec2/api EC2Wrapper +mockgen -destination=mocks/amazon-vcp-resource-controller-k8s/pkg/aws/ec2/api/mock_ec2_apihelper.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/aws/ec2/api EC2APIHelper +mockgen -destination=mocks/amazon-vcp-resource-controller-k8s/pkg/aws/ec2/mock_instance.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/aws/ec2 EC2Instance # package k8s mocks -mockgen -destination=../mocks/amazon-vcp-resource-controller-k8s/pkg/k8s/mock_k8swrapper.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/k8s K8sWrapper -mockgen -destination=../mocks/amazon-vcp-resource-controller-k8s/pkg/k8s/pod/mock_podapiwrapper.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/k8s/pod PodClientAPIWrapper +mockgen -destination=mocks/amazon-vcp-resource-controller-k8s/pkg/k8s/mock_k8swrapper.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/k8s K8sWrapper +mockgen -destination=mocks/amazon-vcp-resource-controller-k8s/pkg/k8s/pod/mock_podapiwrapper.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/k8s/pod PodClientAPIWrapper # package worker mocks -mockgen -destination=../mocks/amazon-vcp-resource-controller-k8s/pkg/worker/mock_worker.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/worker Worker +mockgen -destination=mocks/amazon-vcp-resource-controller-k8s/pkg/worker/mock_worker.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/worker Worker # package handler mocks -mockgen -destination=../mocks/amazon-vcp-resource-controller-k8s/pkg/handler/mock_handler.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/handler Handler +mockgen -destination=mocks/amazon-vcp-resource-controller-k8s/pkg/handler/mock_handler.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/handler Handler # package provider mocks -mockgen -destination=../mocks/amazon-vcp-resource-controller-k8s/pkg/provider/mock_provider.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/provider ResourceProvider -mockgen -destination=../mocks/amazon-vcp-resource-controller-k8s/pkg/provider/branch/trunk/mock_trunk.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/provider/branch/trunk TrunkENI -mockgen -destination=../mocks/amazon-vcp-resource-controller-k8s/pkg/provider/branch/cooldown/mock_cooldown.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/provider/branch/cooldown CoolDown -mockgen -destination=../mocks/amazon-vcp-resource-controller-k8s/pkg/provider/ip/eni/mock_eni.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/provider/ip/eni ENIManager +mockgen -destination=mocks/amazon-vcp-resource-controller-k8s/pkg/provider/mock_provider.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/provider ResourceProvider +mockgen -destination=mocks/amazon-vcp-resource-controller-k8s/pkg/provider/branch/trunk/mock_trunk.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/provider/branch/trunk TrunkENI +mockgen -destination=mocks/amazon-vcp-resource-controller-k8s/pkg/provider/branch/cooldown/mock_cooldown.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/provider/branch/cooldown CoolDown +mockgen -destination=mocks/amazon-vcp-resource-controller-k8s/pkg/provider/ip/eni/mock_eni.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/provider/ip/eni ENIManager # package node mocks -mockgen -destination=../mocks/amazon-vcp-resource-controller-k8s/pkg/node/manager/mock_manager.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/node/manager Manager -mockgen -destination=../mocks/amazon-vcp-resource-controller-k8s/pkg/node/mock_node.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/node Node +mockgen -destination=mocks/amazon-vcp-resource-controller-k8s/pkg/node/manager/mock_manager.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/node/manager Manager +mockgen -destination=mocks/amazon-vcp-resource-controller-k8s/pkg/node/mock_node.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/node Node # package utils mocks -mockgen -destination=../mocks/amazon-vcp-resource-controller-k8s/pkg/utils/mock_k8shelper.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/utils SecurityGroupForPodsAPI +mockgen -destination=mocks/amazon-vcp-resource-controller-k8s/pkg/utils/mock_k8shelper.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/utils SecurityGroupForPodsAPI # package pool mocks -mockgen -destination=../mocks/amazon-vcp-resource-controller-k8s/pkg/pool/mock_pool.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/pool Pool +mockgen -destination=mocks/amazon-vcp-resource-controller-k8s/pkg/pool/mock_pool.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/pool Pool # package resource mocks -mockgen -destination=../mocks/amazon-vcp-resource-controller-k8s/pkg/resource/mock_resources.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/resource ResourceManager +mockgen -destination=mocks/amazon-vcp-resource-controller-k8s/pkg/resource/mock_resources.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/resource ResourceManager # package condition mocks -mockgen -destination=../mocks/amazon-vcp-resource-controller-k8s/pkg/condition/mock_condition.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/condition Conditions +mockgen -destination=mocks/amazon-vcp-resource-controller-k8s/pkg/condition/mock_condition.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/condition Conditions From 702614eab40e26b067d4c54a5efdaa4a42d09040 Mon Sep 17 00:00:00 2001 From: Garvin Pang Date: Tue, 26 Mar 2024 17:09:51 -0700 Subject: [PATCH 08/20] revert --- Makefile | 1 - scripts/gen_mocks.sh | 34 +++++++++++++++++----------------- 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/Makefile b/Makefile index d7b27725..7f5e38aa 100644 --- a/Makefile +++ b/Makefile @@ -32,7 +32,6 @@ verify: go generate ./... go vet ./... go fmt ./... - ./scripts/gen_mocks.sh controller-gen crd rbac:roleName=controller-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases controller-gen object:headerFile="scripts/templates/boilerplate.go.txt" paths="./..." @git diff --quiet ||\ diff --git a/scripts/gen_mocks.sh b/scripts/gen_mocks.sh index 11b67f44..c0b65e9d 100755 --- a/scripts/gen_mocks.sh +++ b/scripts/gen_mocks.sh @@ -1,28 +1,28 @@ alias mockgen='mockgen -copyright_file=templates/copyright.txt' # package aws mocks -mockgen -destination=mocks/amazon-vcp-resource-controller-k8s/pkg/aws/ec2/api/mock_ec2_wrapper.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/aws/ec2/api EC2Wrapper -mockgen -destination=mocks/amazon-vcp-resource-controller-k8s/pkg/aws/ec2/api/mock_ec2_apihelper.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/aws/ec2/api EC2APIHelper -mockgen -destination=mocks/amazon-vcp-resource-controller-k8s/pkg/aws/ec2/mock_instance.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/aws/ec2 EC2Instance +mockgen -destination=../mocks/amazon-vcp-resource-controller-k8s/pkg/aws/ec2/api/mock_ec2_wrapper.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/aws/ec2/api EC2Wrapper +mockgen -destination=../mocks/amazon-vcp-resource-controller-k8s/pkg/aws/ec2/api/mock_ec2_apihelper.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/aws/ec2/api EC2APIHelper +mockgen -destination=../mocks/amazon-vcp-resource-controller-k8s/pkg/aws/ec2/mock_instance.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/aws/ec2 EC2Instance # package k8s mocks -mockgen -destination=mocks/amazon-vcp-resource-controller-k8s/pkg/k8s/mock_k8swrapper.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/k8s K8sWrapper -mockgen -destination=mocks/amazon-vcp-resource-controller-k8s/pkg/k8s/pod/mock_podapiwrapper.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/k8s/pod PodClientAPIWrapper +mockgen -destination=../mocks/amazon-vcp-resource-controller-k8s/pkg/k8s/mock_k8swrapper.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/k8s K8sWrapper +mockgen -destination=../mocks/amazon-vcp-resource-controller-k8s/pkg/k8s/pod/mock_podapiwrapper.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/k8s/pod PodClientAPIWrapper # package worker mocks -mockgen -destination=mocks/amazon-vcp-resource-controller-k8s/pkg/worker/mock_worker.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/worker Worker +mockgen -destination=../mocks/amazon-vcp-resource-controller-k8s/pkg/worker/mock_worker.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/worker Worker # package handler mocks -mockgen -destination=mocks/amazon-vcp-resource-controller-k8s/pkg/handler/mock_handler.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/handler Handler +mockgen -destination=../mocks/amazon-vcp-resource-controller-k8s/pkg/handler/mock_handler.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/handler Handler # package provider mocks -mockgen -destination=mocks/amazon-vcp-resource-controller-k8s/pkg/provider/mock_provider.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/provider ResourceProvider -mockgen -destination=mocks/amazon-vcp-resource-controller-k8s/pkg/provider/branch/trunk/mock_trunk.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/provider/branch/trunk TrunkENI -mockgen -destination=mocks/amazon-vcp-resource-controller-k8s/pkg/provider/branch/cooldown/mock_cooldown.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/provider/branch/cooldown CoolDown -mockgen -destination=mocks/amazon-vcp-resource-controller-k8s/pkg/provider/ip/eni/mock_eni.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/provider/ip/eni ENIManager +mockgen -destination=../mocks/amazon-vcp-resource-controller-k8s/pkg/provider/mock_provider.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/provider ResourceProvider +mockgen -destination=../mocks/amazon-vcp-resource-controller-k8s/pkg/provider/branch/trunk/mock_trunk.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/provider/branch/trunk TrunkENI +mockgen -destination=../mocks/amazon-vcp-resource-controller-k8s/pkg/provider/branch/cooldown/mock_cooldown.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/provider/branch/cooldown CoolDown +mockgen -destination=../mocks/amazon-vcp-resource-controller-k8s/pkg/provider/ip/eni/mock_eni.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/provider/ip/eni ENIManager # package node mocks -mockgen -destination=mocks/amazon-vcp-resource-controller-k8s/pkg/node/manager/mock_manager.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/node/manager Manager -mockgen -destination=mocks/amazon-vcp-resource-controller-k8s/pkg/node/mock_node.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/node Node +mockgen -destination=../mocks/amazon-vcp-resource-controller-k8s/pkg/node/manager/mock_manager.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/node/manager Manager +mockgen -destination=../mocks/amazon-vcp-resource-controller-k8s/pkg/node/mock_node.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/node Node # package utils mocks -mockgen -destination=mocks/amazon-vcp-resource-controller-k8s/pkg/utils/mock_k8shelper.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/utils SecurityGroupForPodsAPI +mockgen -destination=../mocks/amazon-vcp-resource-controller-k8s/pkg/utils/mock_k8shelper.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/utils SecurityGroupForPodsAPI # package pool mocks -mockgen -destination=mocks/amazon-vcp-resource-controller-k8s/pkg/pool/mock_pool.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/pool Pool +mockgen -destination=../mocks/amazon-vcp-resource-controller-k8s/pkg/pool/mock_pool.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/pool Pool # package resource mocks -mockgen -destination=mocks/amazon-vcp-resource-controller-k8s/pkg/resource/mock_resources.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/resource ResourceManager +mockgen -destination=../mocks/amazon-vcp-resource-controller-k8s/pkg/resource/mock_resources.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/resource ResourceManager # package condition mocks -mockgen -destination=mocks/amazon-vcp-resource-controller-k8s/pkg/condition/mock_condition.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/condition Conditions +mockgen -destination=../mocks/amazon-vcp-resource-controller-k8s/pkg/condition/mock_condition.go github.com/aws/amazon-vpc-resource-controller-k8s/pkg/condition Conditions From 6735aa75efa1d542229a5a94a8ebf9e61290f4bb Mon Sep 17 00:00:00 2001 From: GnatorX Date: Thu, 28 Mar 2024 13:59:15 -0700 Subject: [PATCH 09/20] Add validation --- pkg/aws/ec2/api/helper.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/pkg/aws/ec2/api/helper.go b/pkg/aws/ec2/api/helper.go index cd9bb457..1ebe5ef7 100644 --- a/pkg/aws/ec2/api/helper.go +++ b/pkg/aws/ec2/api/helper.go @@ -639,14 +639,15 @@ func (h *ec2APIHelper) GetSecurityGroupIdsForSecurityGroupNames(securityGroupNam filters := []*ec2.Filter{ { Name: aws.String("group-name"), - Values: aws.StringSlice(securityGroupNames), + Values: aws.StringSlice(sgNamesNotInCache), }, { Name: aws.String("vpc-id"), Values: aws.StringSlice([]string{h.workerNodeVpcId}), }, } - input := &ec2.DescribeSecurityGroupsInput{Filters: filters} + input := &ec2.DescribeSecurityGroupsInput{Filters: filters} + foundSgNames := map[string]bool{} for { output, err := h.ec2Wrapper.DescribeSecurityGroups(input) if err != nil { @@ -661,6 +662,16 @@ func (h *ec2APIHelper) GetSecurityGroupIdsForSecurityGroupNames(securityGroupNam } input.NextToken = output.NextToken } + var missingSgNames []string + for _, sgName := range sgNamesNotInCache { + if _, ok := foundSgNames[sgName]; !ok { + missingSgNames = append(missingSgNames, sgName) + } + } + if len(missingSgNames) > 0 { + sort.Strings(missingSgNames) + return nil, fmt.Errorf("security groups %s not found", missingSgNames) + } return sgIds, nil } From 9fbbd1b6f994a21d6cb40885696797a09318fe9e Mon Sep 17 00:00:00 2001 From: GnatorX Date: Thu, 28 Mar 2024 14:00:41 -0700 Subject: [PATCH 10/20] Update helper_test.go --- pkg/aws/ec2/api/helper_test.go | 39 ++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/pkg/aws/ec2/api/helper_test.go b/pkg/aws/ec2/api/helper_test.go index e0febd2a..b164aaa0 100644 --- a/pkg/aws/ec2/api/helper_test.go +++ b/pkg/aws/ec2/api/helper_test.go @@ -1287,3 +1287,42 @@ func TestGetSourceAcctAndArn_NoRegion(t *testing.T) { assert.Equal(t, "", acct, "correct account ID should be retrieved") assert.Equal(t, "", arn, "correct cluster arn should be retrieved") } + +func TestEc2APIHelper_GetSecurityGroupIdsForSecurityGroupNames_MissingGroup(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + securityGroupNames := []string{securityGroupName1, securityGroupName2, securityGroupName3} + + describeSecurityGroupInput := &ec2.DescribeSecurityGroupsInput{ + Filters: []*ec2.Filter{ + { + Name: aws.String("group-name"), + Values: aws.StringSlice(securityGroupNames), + }, + { + Name: aws.String("vpc-id"), + Values: aws.StringSlice([]string{workerNodeVpcId}), + }, + }, + } + + describeSecurityGroupOutput := &ec2.DescribeSecurityGroupsOutput{ + SecurityGroups: []*ec2.SecurityGroup{ + { + GroupId: aws.String(securityGroup1), + GroupName: aws.String(securityGroupName1), + }, + }, + } + + ec2ApiHelper, mockWrapper := getMockWrapper(ctrl) + + mockWrapper.EXPECT().DescribeSecurityGroups(describeSecurityGroupInput).Return(describeSecurityGroupOutput, nil) + + securityGroupIds, err := ec2ApiHelper.GetSecurityGroupIdsForSecurityGroupNames(securityGroupNames) + + assert.Nil(t, securityGroupIds) + assert.NotNil(t, err) + assert.ErrorContains(t, err, fmt.Sprintf("security groups [%s %s] not found", securityGroupName2, securityGroupName3)) +} From 04b028b4fc13855b25f98304b34e14c383bc2b59 Mon Sep 17 00:00:00 2001 From: GnatorX Date: Thu, 28 Mar 2024 14:04:08 -0700 Subject: [PATCH 11/20] fix import --- pkg/aws/ec2/api/helper.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/aws/ec2/api/helper.go b/pkg/aws/ec2/api/helper.go index 1ebe5ef7..04c9dcfa 100644 --- a/pkg/aws/ec2/api/helper.go +++ b/pkg/aws/ec2/api/helper.go @@ -15,6 +15,7 @@ package api import ( "fmt" + "sort" "time" "github.com/aws/aws-sdk-go/aws" From 4360caa9991b515bb003c277ccbea69022cae1e1 Mon Sep 17 00:00:00 2001 From: GnatorX Date: Thu, 28 Mar 2024 14:09:24 -0700 Subject: [PATCH 12/20] add metrics and account filter --- pkg/aws/ec2/api/wrapper.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pkg/aws/ec2/api/wrapper.go b/pkg/aws/ec2/api/wrapper.go index 901429a7..be916a29 100644 --- a/pkg/aws/ec2/api/wrapper.go +++ b/pkg/aws/ec2/api/wrapper.go @@ -857,7 +857,14 @@ func (e *ec2Wrapper) CreateNetworkInterfacePermission(input *ec2.CreateNetworkIn } func (e *ec2Wrapper) DescribeSecurityGroups(input *ec2.DescribeSecurityGroupsInput) (*ec2.DescribeSecurityGroupsOutput, error) { + accountIdFilter := ec2.Filter{ + Name: aws.String("owner-id"), + Values: aws.StringSlice([]string{e.accountID}), + } + start := time.Now() + input.Filters = append(input.Filters, &accountIdFilter) output, err := e.userServiceClient.DescribeSecurityGroups(input) + ec2APICallLatencies.WithLabelValues("describe_security_groups").Observe(timeSinceMs(start)) // Metric Update ec2APICallCnt.Inc() From b0b5546ccc39e4bbc26c4f8061601d2334bce009 Mon Sep 17 00:00:00 2001 From: GnatorX Date: Thu, 28 Mar 2024 14:10:31 -0700 Subject: [PATCH 13/20] sg test --- pkg/aws/ec2/api/helper_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/aws/ec2/api/helper_test.go b/pkg/aws/ec2/api/helper_test.go index b164aaa0..7442d43a 100644 --- a/pkg/aws/ec2/api/helper_test.go +++ b/pkg/aws/ec2/api/helper_test.go @@ -61,6 +61,7 @@ var ( securityGroupName1 = "db-sg" securityGroupName2 = "redis-sg" + securityGroupName3 = "web-sg" securityGroupNames = []string{securityGroupName1, securityGroupName2} tags = []*ec2.Tag{ From f82aeeff44c9e5a3373d15921afa9f1edce0ac5e Mon Sep 17 00:00:00 2001 From: GnatorX Date: Thu, 28 Mar 2024 14:18:03 -0700 Subject: [PATCH 14/20] lint --- pkg/aws/ec2/api/helper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/aws/ec2/api/helper.go b/pkg/aws/ec2/api/helper.go index 04c9dcfa..102e0e2a 100644 --- a/pkg/aws/ec2/api/helper.go +++ b/pkg/aws/ec2/api/helper.go @@ -647,7 +647,7 @@ func (h *ec2APIHelper) GetSecurityGroupIdsForSecurityGroupNames(securityGroupNam Values: aws.StringSlice([]string{h.workerNodeVpcId}), }, } - input := &ec2.DescribeSecurityGroupsInput{Filters: filters} + input := &ec2.DescribeSecurityGroupsInput{Filters: filters} foundSgNames := map[string]bool{} for { output, err := h.ec2Wrapper.DescribeSecurityGroups(input) From 25bc6c3051b5407b0bc6e87bc60d407f19dff035 Mon Sep 17 00:00:00 2001 From: GnatorX Date: Thu, 28 Mar 2024 15:05:01 -0700 Subject: [PATCH 15/20] fix copy miss --- pkg/aws/ec2/api/helper.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/aws/ec2/api/helper.go b/pkg/aws/ec2/api/helper.go index 102e0e2a..4f219ebf 100644 --- a/pkg/aws/ec2/api/helper.go +++ b/pkg/aws/ec2/api/helper.go @@ -657,6 +657,7 @@ func (h *ec2APIHelper) GetSecurityGroupIdsForSecurityGroupNames(securityGroupNam for _, sg := range output.SecurityGroups { h.securityGroupNameToIdMap[*sg.GroupName] = *sg.GroupId sgIds = append(sgIds, *sg.GroupId) + foundSgNames[*sg.GroupName] = true } if output.NextToken == nil { break From 670926939661adb068386ec57c8407fb54437dc4 Mon Sep 17 00:00:00 2001 From: Garvin Pang Date: Mon, 15 Apr 2024 13:45:58 -0700 Subject: [PATCH 16/20] PR changes: Move to getsgforVPC, move SG names into SG, unique items validation --- .../v1beta1/securitygrouppolicy_types.go | 15 ++++------ .../v1beta1/zz_generated.deepcopy.go | 22 ++------------ ...sources.k8s.aws_securitygrouppolicies.yaml | 24 +++++++-------- .../pkg/aws/ec2/api/mock_ec2_wrapper.go | 30 +++++++++---------- pkg/aws/ec2/api/helper.go | 12 +++----- pkg/aws/ec2/api/helper_test.go | 22 +++++++------- pkg/aws/ec2/api/wrapper.go | 30 ++++++++++--------- pkg/utils/helper.go | 4 +-- pkg/utils/helper_test.go | 6 ++-- 9 files changed, 68 insertions(+), 97 deletions(-) diff --git a/apis/vpcresources/v1beta1/securitygrouppolicy_types.go b/apis/vpcresources/v1beta1/securitygrouppolicy_types.go index 122d4532..5ebf786b 100644 --- a/apis/vpcresources/v1beta1/securitygrouppolicy_types.go +++ b/apis/vpcresources/v1beta1/securitygrouppolicy_types.go @@ -24,15 +24,6 @@ type SecurityGroupPolicySpec struct { PodSelector *metav1.LabelSelector `json:"podSelector,omitempty"` ServiceAccountSelector *metav1.LabelSelector `json:"serviceAccountSelector,omitempty"` SecurityGroups GroupIds `json:"securityGroups,omitempty"` - SecurityGroupNames GroupNames `json:"securityGroupNames,omitempty"` -} - -// GroupNames contains the list of security group names that will be applied to the network interface of the pod matching the criteria. -type GroupNames struct { - // Groups is the list of EC2 Security Group Names that need to be applied to the ENI of a Pod. - // +kubebuilder:validation:MinItems=1 - // +kubebuilder:validation:MaxItems=5 - GroupNames []string `json:"groupNames,omitempty"` } // GroupIds contains the list of security groups that will be applied to the network interface of the pod matching the criteria. @@ -40,7 +31,13 @@ type GroupIds struct { // Groups is the list of EC2 Security Groups Ids that need to be applied to the ENI of a Pod. // +kubebuilder:validation:MinItems=1 // +kubebuilder:validation:MaxItems=5 + // +kubebuilder:validation:UniqueItems=true Groups []string `json:"groupIds,omitempty"` + // GroupNames is the list of EC2 Security Group Names that need to be applied to the ENI of a Pod. + // +kubebuilder:validation:MinItems=0 + // +kubebuilder:validation:MaxItems=5 + // +kubebuilder:validation:UniqueItems=true + GroupNames []string `json:"groupNames,omitempty"` } // ServiceAccountSelector contains the selection criteria for matching pod with service account that matches the label selector diff --git a/apis/vpcresources/v1beta1/zz_generated.deepcopy.go b/apis/vpcresources/v1beta1/zz_generated.deepcopy.go index db53c2ec..d17853df 100644 --- a/apis/vpcresources/v1beta1/zz_generated.deepcopy.go +++ b/apis/vpcresources/v1beta1/zz_generated.deepcopy.go @@ -31,21 +31,6 @@ func (in *GroupIds) DeepCopyInto(out *GroupIds) { *out = make([]string, len(*in)) copy(*out, *in) } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GroupIds. -func (in *GroupIds) DeepCopy() *GroupIds { - if in == nil { - return nil - } - out := new(GroupIds) - in.DeepCopyInto(out) - return out -} - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *GroupNames) DeepCopyInto(out *GroupNames) { - *out = *in if in.GroupNames != nil { in, out := &in.GroupNames, &out.GroupNames *out = make([]string, len(*in)) @@ -53,12 +38,12 @@ func (in *GroupNames) DeepCopyInto(out *GroupNames) { } } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GroupNames. -func (in *GroupNames) DeepCopy() *GroupNames { +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GroupIds. +func (in *GroupIds) DeepCopy() *GroupIds { if in == nil { return nil } - out := new(GroupNames) + out := new(GroupIds) in.DeepCopyInto(out) return out } @@ -135,7 +120,6 @@ func (in *SecurityGroupPolicySpec) DeepCopyInto(out *SecurityGroupPolicySpec) { (*in).DeepCopyInto(*out) } in.SecurityGroups.DeepCopyInto(&out.SecurityGroups) - in.SecurityGroupNames.DeepCopyInto(&out.SecurityGroupNames) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SecurityGroupPolicySpec. diff --git a/config/crd/bases/vpcresources.k8s.aws_securitygrouppolicies.yaml b/config/crd/bases/vpcresources.k8s.aws_securitygrouppolicies.yaml index b1aab7db..aadf77d6 100644 --- a/config/crd/bases/vpcresources.k8s.aws_securitygrouppolicies.yaml +++ b/config/crd/bases/vpcresources.k8s.aws_securitygrouppolicies.yaml @@ -95,20 +95,6 @@ spec: are ANDed. type: object type: object - securityGroupNames: - description: GroupNames contains the list of security group names - that will be applied to the network interface of the pod matching - the criteria. - properties: - groupNames: - description: Groups is the list of EC2 Security Group Names that - need to be applied to the ENI of a Pod. - items: - type: string - maxItems: 5 - minItems: 1 - type: array - type: object securityGroups: description: GroupIds contains the list of security groups that will be applied to the network interface of the pod matching the criteria. @@ -121,6 +107,16 @@ spec: maxItems: 5 minItems: 1 type: array + uniqueItems: true + groupNames: + description: GroupNames is the list of EC2 Security Group Names + that need to be applied to the ENI of a Pod. + items: + type: string + maxItems: 5 + minItems: 0 + type: array + uniqueItems: true type: object serviceAccountSelector: description: A label selector is a label query over a set of resources. diff --git a/mocks/amazon-vcp-resource-controller-k8s/pkg/aws/ec2/api/mock_ec2_wrapper.go b/mocks/amazon-vcp-resource-controller-k8s/pkg/aws/ec2/api/mock_ec2_wrapper.go index f39830bc..d653e736 100644 --- a/mocks/amazon-vcp-resource-controller-k8s/pkg/aws/ec2/api/mock_ec2_wrapper.go +++ b/mocks/amazon-vcp-resource-controller-k8s/pkg/aws/ec2/api/mock_ec2_wrapper.go @@ -184,21 +184,6 @@ func (mr *MockEC2WrapperMockRecorder) DescribeNetworkInterfacesPages(arg0 interf return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DescribeNetworkInterfacesPages", reflect.TypeOf((*MockEC2Wrapper)(nil).DescribeNetworkInterfacesPages), arg0) } -// DescribeSecurityGroups mocks base method. -func (m *MockEC2Wrapper) DescribeSecurityGroups(arg0 *ec2.DescribeSecurityGroupsInput) (*ec2.DescribeSecurityGroupsOutput, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DescribeSecurityGroups", arg0) - ret0, _ := ret[0].(*ec2.DescribeSecurityGroupsOutput) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// DescribeSecurityGroups indicates an expected call of DescribeSecurityGroups. -func (mr *MockEC2WrapperMockRecorder) DescribeSecurityGroups(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DescribeSecurityGroups", reflect.TypeOf((*MockEC2Wrapper)(nil).DescribeSecurityGroups), arg0) -} - // DescribeSubnets mocks base method. func (m *MockEC2Wrapper) DescribeSubnets(arg0 *ec2.DescribeSubnetsInput) (*ec2.DescribeSubnetsOutput, error) { m.ctrl.T.Helper() @@ -244,6 +229,21 @@ func (mr *MockEC2WrapperMockRecorder) DetachNetworkInterface(arg0 interface{}) * return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DetachNetworkInterface", reflect.TypeOf((*MockEC2Wrapper)(nil).DetachNetworkInterface), arg0) } +// GetSecurityGroupsForVpc mocks base method. +func (m *MockEC2Wrapper) GetSecurityGroupsForVpc(arg0 *ec2.GetSecurityGroupsForVpcInput) (*ec2.GetSecurityGroupsForVpcOutput, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetSecurityGroupsForVpc", arg0) + ret0, _ := ret[0].(*ec2.GetSecurityGroupsForVpcOutput) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetSecurityGroupsForVpc indicates an expected call of GetSecurityGroupsForVpc. +func (mr *MockEC2WrapperMockRecorder) GetSecurityGroupsForVpc(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSecurityGroupsForVpc", reflect.TypeOf((*MockEC2Wrapper)(nil).GetSecurityGroupsForVpc), arg0) +} + // ModifyNetworkInterfaceAttribute mocks base method. func (m *MockEC2Wrapper) ModifyNetworkInterfaceAttribute(arg0 *ec2.ModifyNetworkInterfaceAttributeInput) (*ec2.ModifyNetworkInterfaceAttributeOutput, error) { m.ctrl.T.Helper() diff --git a/pkg/aws/ec2/api/helper.go b/pkg/aws/ec2/api/helper.go index 4f219ebf..fd97b972 100644 --- a/pkg/aws/ec2/api/helper.go +++ b/pkg/aws/ec2/api/helper.go @@ -126,7 +126,7 @@ func (h *ec2APIHelper) CreateNetworkInterface(description *string, subnetId *str eniDescription := CreateENIDescriptionPrefix + *description var ec2SecurityGroups []*string - if securityGroups != nil && len(securityGroups) != 0 { + if len(securityGroups) != 0 { // Only add security groups if there are one or more security group provided, otherwise API call will fail instead // of creating the interface with default security groups ec2SecurityGroups = aws.StringSlice(securityGroups) @@ -642,19 +642,15 @@ func (h *ec2APIHelper) GetSecurityGroupIdsForSecurityGroupNames(securityGroupNam Name: aws.String("group-name"), Values: aws.StringSlice(sgNamesNotInCache), }, - { - Name: aws.String("vpc-id"), - Values: aws.StringSlice([]string{h.workerNodeVpcId}), - }, } - input := &ec2.DescribeSecurityGroupsInput{Filters: filters} + input := &ec2.GetSecurityGroupsForVpcInput{VpcId: &h.workerNodeVpcId, Filters: filters} foundSgNames := map[string]bool{} for { - output, err := h.ec2Wrapper.DescribeSecurityGroups(input) + output, err := h.ec2Wrapper.GetSecurityGroupsForVpc(input) if err != nil { return nil, err } - for _, sg := range output.SecurityGroups { + for _, sg := range output.SecurityGroupForVpcs { h.securityGroupNameToIdMap[*sg.GroupName] = *sg.GroupId sgIds = append(sgIds, *sg.GroupId) foundSgNames[*sg.GroupName] = true diff --git a/pkg/aws/ec2/api/helper_test.go b/pkg/aws/ec2/api/helper_test.go index 7442d43a..2fae8a77 100644 --- a/pkg/aws/ec2/api/helper_test.go +++ b/pkg/aws/ec2/api/helper_test.go @@ -323,20 +323,17 @@ var ( maxRetryOnError = 3 - describeSecurityGroupInput = &ec2.DescribeSecurityGroupsInput{ + getSecurityGroupForVpcInput = &ec2.GetSecurityGroupsForVpcInput{ Filters: []*ec2.Filter{ { Name: aws.String("group-name"), Values: aws.StringSlice(securityGroupNames), }, - { - Name: aws.String("vpc-id"), - Values: aws.StringSlice([]string{workerNodeVpcId}), - }, }, + VpcId: &workerNodeVpcId, } - describeSecurityGroupOutput = &ec2.DescribeSecurityGroupsOutput{ - SecurityGroups: []*ec2.SecurityGroup{ + getSecurityGroupForVpcOutput = &ec2.GetSecurityGroupsForVpcOutput{ + SecurityGroupForVpcs: []*ec2.SecurityGroupForVpc{ { GroupId: aws.String(securityGroup1), GroupName: aws.String(securityGroupName1), @@ -1221,7 +1218,7 @@ func TestEc2APIHelper_GetSecurityGroupIdsForSecurityGroupNames_EmptyCache(t *tes ec2ApiHelper, mockWrapper := getMockWrapper(ctrl) - mockWrapper.EXPECT().DescribeSecurityGroups(describeSecurityGroupInput).Return(describeSecurityGroupOutput, nil) + mockWrapper.EXPECT().GetSecurityGroupsForVpc(getSecurityGroupForVpcInput).Return(getSecurityGroupForVpcOutput, nil) securityGroupIds, err := ec2ApiHelper.GetSecurityGroupIdsForSecurityGroupNames(securityGroupNames) @@ -1295,7 +1292,7 @@ func TestEc2APIHelper_GetSecurityGroupIdsForSecurityGroupNames_MissingGroup(t *t securityGroupNames := []string{securityGroupName1, securityGroupName2, securityGroupName3} - describeSecurityGroupInput := &ec2.DescribeSecurityGroupsInput{ + getSecurityGroupsForVpcInput := &ec2.GetSecurityGroupsForVpcInput{ Filters: []*ec2.Filter{ { Name: aws.String("group-name"), @@ -1306,10 +1303,11 @@ func TestEc2APIHelper_GetSecurityGroupIdsForSecurityGroupNames_MissingGroup(t *t Values: aws.StringSlice([]string{workerNodeVpcId}), }, }, + VpcId: &workerNodeVpcId, } - describeSecurityGroupOutput := &ec2.DescribeSecurityGroupsOutput{ - SecurityGroups: []*ec2.SecurityGroup{ + getSecurityGroupsForVpcOutput := &ec2.GetSecurityGroupsForVpcOutput{ + SecurityGroupForVpcs: []*ec2.SecurityGroupForVpc{ { GroupId: aws.String(securityGroup1), GroupName: aws.String(securityGroupName1), @@ -1319,7 +1317,7 @@ func TestEc2APIHelper_GetSecurityGroupIdsForSecurityGroupNames_MissingGroup(t *t ec2ApiHelper, mockWrapper := getMockWrapper(ctrl) - mockWrapper.EXPECT().DescribeSecurityGroups(describeSecurityGroupInput).Return(describeSecurityGroupOutput, nil) + mockWrapper.EXPECT().GetSecurityGroupsForVpc(getSecurityGroupsForVpcInput).Return(getSecurityGroupsForVpcOutput, nil) securityGroupIds, err := ec2ApiHelper.GetSecurityGroupIdsForSecurityGroupNames(securityGroupNames) diff --git a/pkg/aws/ec2/api/wrapper.go b/pkg/aws/ec2/api/wrapper.go index be916a29..9f5fd776 100644 --- a/pkg/aws/ec2/api/wrapper.go +++ b/pkg/aws/ec2/api/wrapper.go @@ -60,7 +60,7 @@ type EC2Wrapper interface { DescribeTrunkInterfaceAssociations(input *ec2.DescribeTrunkInterfaceAssociationsInput) (*ec2.DescribeTrunkInterfaceAssociationsOutput, error) ModifyNetworkInterfaceAttribute(input *ec2.ModifyNetworkInterfaceAttributeInput) (*ec2.ModifyNetworkInterfaceAttributeOutput, error) CreateNetworkInterfacePermission(input *ec2.CreateNetworkInterfacePermissionInput) (*ec2.CreateNetworkInterfacePermissionOutput, error) - DescribeSecurityGroups(input *ec2.DescribeSecurityGroupsInput) (*ec2.DescribeSecurityGroupsOutput, error) + GetSecurityGroupsForVpc(input *ec2.GetSecurityGroupsForVpcInput) (*ec2.GetSecurityGroupsForVpcOutput, error) } var ( @@ -310,17 +310,17 @@ var ( }, ) - ec2DescribeSecurityGroupsCallCnt = prometheus.NewCounter( + ec2GetSecurityGroupsForVpcCallCnt = prometheus.NewCounter( prometheus.CounterOpts{ - Name: "ec2_describe_security_groups_api_call_count", - Help: "The number of calls made to describe security groups", + Name: "ec2_get_security_groups_for_vpc_api_call_count", + Help: "The number of calls made to get security groups for vpc", }, ) - ec2DescribeSecurityGroupsErrCnt = prometheus.NewCounter( + ec2GetSecurityGroupsForVpcErrCnt = prometheus.NewCounter( prometheus.CounterOpts{ - Name: "ec2_describe_security_groups_api_err_count", - Help: "The number of errors encountered while making calls to describe security groups", + Name: "ec2_get_security_groups_for_vpc_api_err_count", + Help: "The number of errors encountered while making calls to get security groups for vpc", }, ) @@ -374,8 +374,8 @@ func prometheusRegister() { ec2describeTrunkInterfaceAssociationAPIErrCnt, ec2modifyNetworkInterfaceAttributeAPICallCnt, ec2modifyNetworkInterfaceAttributeAPIErrCnt, - ec2DescribeSecurityGroupsCallCnt, - ec2DescribeSecurityGroupsErrCnt, + ec2GetSecurityGroupsForVpcCallCnt, + ec2GetSecurityGroupsForVpcErrCnt, ec2APICallLatencies, vpccniAvailableENICnt, vpcrcAvailableENICnt, @@ -856,23 +856,25 @@ func (e *ec2Wrapper) CreateNetworkInterfacePermission(input *ec2.CreateNetworkIn return output, err } -func (e *ec2Wrapper) DescribeSecurityGroups(input *ec2.DescribeSecurityGroupsInput) (*ec2.DescribeSecurityGroupsOutput, error) { +func (e *ec2Wrapper) GetSecurityGroupsForVpc(input *ec2.GetSecurityGroupsForVpcInput) (*ec2.GetSecurityGroupsForVpcOutput, error) { accountIdFilter := ec2.Filter{ Name: aws.String("owner-id"), Values: aws.StringSlice([]string{e.accountID}), } + start := time.Now() input.Filters = append(input.Filters, &accountIdFilter) - output, err := e.userServiceClient.DescribeSecurityGroups(input) - ec2APICallLatencies.WithLabelValues("describe_security_groups").Observe(timeSinceMs(start)) + // e.userServiceClient.Get + output, err := e.userServiceClient.GetSecurityGroupsForVpc(input) + ec2APICallLatencies.WithLabelValues("get_security_groups_for_vpc").Observe(timeSinceMs(start)) // Metric Update ec2APICallCnt.Inc() - ec2DescribeSecurityGroupsCallCnt.Inc() + ec2GetSecurityGroupsForVpcCallCnt.Inc() if err != nil { ec2APIErrCnt.Inc() - ec2DescribeSecurityGroupsErrCnt.Inc() + ec2GetSecurityGroupsForVpcErrCnt.Inc() } return output, err diff --git a/pkg/utils/helper.go b/pkg/utils/helper.go index 45789469..0fbd3a67 100644 --- a/pkg/utils/helper.go +++ b/pkg/utils/helper.go @@ -126,7 +126,7 @@ func (s *SecurityGroupForPods) filterPodSecurityGroups( hasPodSelector := sgp.Spec.PodSelector != nil hasSASelector := sgp.Spec.ServiceAccountSelector != nil hasSecurityGroup := (sgp.Spec.SecurityGroups.Groups != nil && len(sgp.Spec.SecurityGroups.Groups) > 0) || - (sgp.Spec.SecurityGroupNames.GroupNames != nil && len(sgp.Spec.SecurityGroupNames.GroupNames) > 0) + (sgp.Spec.SecurityGroups.GroupNames != nil && len(sgp.Spec.SecurityGroups.GroupNames) > 0) if (!hasPodSelector && !hasSASelector) || !hasSecurityGroup { sgpLogger.Info( @@ -162,7 +162,7 @@ func (s *SecurityGroupForPods) filterPodSecurityGroups( continue } - sgNameList = append(sgNameList, sgp.Spec.SecurityGroupNames.GroupNames...) + sgNameList = append(sgNameList, sgp.Spec.SecurityGroups.GroupNames...) sgList = append(sgList, sgp.Spec.SecurityGroups.Groups...) } diff --git a/pkg/utils/helper_test.go b/pkg/utils/helper_test.go index e7e4b60e..ba93ed80 100644 --- a/pkg/utils/helper_test.go +++ b/pkg/utils/helper_test.go @@ -121,7 +121,7 @@ func TestCanInjectENI_PodSelectors(t *testing.T) { for _, test := range tests { ec2WrapperMock.EXPECT().GetSecurityGroupIdsForSecurityGroupNames(test.sgpListInput.Items[0]. - Spec.SecurityGroupNames.GroupNames).Return(test.mockSecurityGroupIdsToReturn, nil) + Spec.SecurityGroups.GroupNames).Return(test.mockSecurityGroupIdsToReturn, nil) sgIds, err := sgp.filterPodSecurityGroups(test.sgpListInput, testPod, testSA) if err != nil { t.Fatalf("Test %s failed", test.testName) @@ -380,9 +380,7 @@ func NewSecurityGroupPolicyPodSelector( }}, }, SecurityGroups: vpcresourcesv1beta1.GroupIds{ - Groups: securityGroupIds, - }, - SecurityGroupNames: vpcresourcesv1beta1.GroupNames{ + Groups: securityGroupIds, GroupNames: securityGroupNames, }, }, From 4fb7bdcd7a8781ce20b667bb6c10d39df65187b3 Mon Sep 17 00:00:00 2001 From: Garvin Pang Date: Mon, 15 Apr 2024 13:57:19 -0700 Subject: [PATCH 17/20] vuln check --- go.mod | 6 +++--- go.sum | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/go.mod b/go.mod index e24aab79..7090bb96 100644 --- a/go.mod +++ b/go.mod @@ -64,10 +64,10 @@ require ( github.com/spf13/pflag v1.0.5 // indirect go.uber.org/multierr v1.11.0 // indirect golang.org/x/exp v0.0.0-20230315142452-642cacee5cc0 - golang.org/x/net v0.21.0 // indirect + golang.org/x/net v0.23.0 // indirect golang.org/x/oauth2 v0.17.0 // indirect - golang.org/x/sys v0.17.0 // indirect - golang.org/x/term v0.17.0 // indirect + golang.org/x/sys v0.18.0 // indirect + golang.org/x/term v0.18.0 // indirect golang.org/x/text v0.14.0 // indirect golang.org/x/tools v0.16.1 // indirect google.golang.org/appengine v1.6.7 // indirect diff --git a/go.sum b/go.sum index a4e345f2..5303fbe7 100644 --- a/go.sum +++ b/go.sum @@ -150,8 +150,8 @@ golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLL golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU= golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM= -golang.org/x/net v0.21.0 h1:AQyQV4dYCvJ7vGmJyKki9+PBdyvhkSd8EIx/qb0AYv4= -golang.org/x/net v0.21.0/go.mod h1:bIjVDfnllIU7BJ2DNgfnXvpSvtn8VRwhlsaeUTyUS44= +golang.org/x/net v0.23.0 h1:7EYJ93RZ9vYSZAIb2x3lnuvqO5zneoD6IvWjuhfxjTs= +golang.org/x/net v0.23.0/go.mod h1:JKghWKKOSdJwpW2GEx0Ja7fmaKnMsbu+MWVZTokSYmg= golang.org/x/oauth2 v0.17.0 h1:6m3ZPmLEFdVxKKWnKq4VqZ60gutO35zm+zrAHVmHyDQ= golang.org/x/oauth2 v0.17.0/go.mod h1:OzPDGQiuQMguemayvdylqddI7qcD9lnSDb+1FiwQ5HA= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= @@ -165,11 +165,11 @@ golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.17.0 h1:25cE3gD+tdBA7lp7QfhuV+rJiE9YXTcS3VG1SqssI/Y= -golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.18.0 h1:DBdB3niSjOA/O0blCZBqDefyWNYveAYMNF1Wum0DYQ4= +golang.org/x/sys v0.18.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= -golang.org/x/term v0.17.0 h1:mkTF7LCd6WGJNL3K1Ad7kwxNfYAW6a8a8QqtMblp/4U= -golang.org/x/term v0.17.0/go.mod h1:lLRBjIVuehSbZlaOtGMbcMncT+aqLLLmKrsjNrUguwk= +golang.org/x/term v0.18.0 h1:FcHjZXDMxI8mM3nwhX9HlKop4C0YQvCVCdwYl2wOtE8= +golang.org/x/term v0.18.0/go.mod h1:ILwASektA3OnRv7amZ1xhE/KTR+u50pbXfZ03+6Nx58= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= From 8dfb5d444e712f412d82f7ce4fb49d50c09eccde Mon Sep 17 00:00:00 2001 From: Garvin Pang Date: Mon, 15 Apr 2024 14:01:45 -0700 Subject: [PATCH 18/20] unused --- pkg/aws/ec2/api/wrapper.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/aws/ec2/api/wrapper.go b/pkg/aws/ec2/api/wrapper.go index 31b6eca8..7361db1a 100644 --- a/pkg/aws/ec2/api/wrapper.go +++ b/pkg/aws/ec2/api/wrapper.go @@ -21,7 +21,6 @@ import ( "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/config" "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/httpclient" - "k8s.io/apimachinery/pkg/util/wait" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/credentials" From e9d523176c8da8640f880ef49dc07d10f442fd85 Mon Sep 17 00:00:00 2001 From: Garvin Pang Date: Mon, 15 Apr 2024 14:10:54 -0700 Subject: [PATCH 19/20] fix unit test --- pkg/aws/ec2/api/helper_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pkg/aws/ec2/api/helper_test.go b/pkg/aws/ec2/api/helper_test.go index 8fcb9035..e7fe501c 100644 --- a/pkg/aws/ec2/api/helper_test.go +++ b/pkg/aws/ec2/api/helper_test.go @@ -1318,10 +1318,6 @@ func TestEc2APIHelper_GetSecurityGroupIdsForSecurityGroupNames_MissingGroup(t *t Name: aws.String("group-name"), Values: aws.StringSlice(securityGroupNames), }, - { - Name: aws.String("vpc-id"), - Values: aws.StringSlice([]string{workerNodeVpcId}), - }, }, VpcId: &workerNodeVpcId, } From 617f995c5b4469ab3829e32e304b17df685c2d03 Mon Sep 17 00:00:00 2001 From: Garvin Pang Date: Mon, 15 Apr 2024 14:19:46 -0700 Subject: [PATCH 20/20] remove sg name list dedup --- pkg/utils/helper.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/utils/helper.go b/pkg/utils/helper.go index 0fbd3a67..36ea5517 100644 --- a/pkg/utils/helper.go +++ b/pkg/utils/helper.go @@ -166,7 +166,6 @@ func (s *SecurityGroupForPods) filterPodSecurityGroups( sgList = append(sgList, sgp.Spec.SecurityGroups.Groups...) } - sgNameList = RemoveDuplicatedSg(sgNameList) sgIdsForNames, err := s.Ec2Helper.GetSecurityGroupIdsForSecurityGroupNames(sgNameList) if err != nil { return nil, err