Support more than 5 SecurityGroups on one CRD

README.md

@@ -18,11 +18,7 @@ ENI Trunking is a private feature even though the APIs are publicly accessible u
 
 Please follow the [guide](https://docs.aws.amazon.com/eks/latest/userguide/security-groups-for-pods.html) for enabling Security Group for Pods on your EKS Cluster. 
 
-Note: The SecurityGroupPolicy CRD only supports up to 5 security groups per custom resource. If you need more than 5 security groups for a pod, please consider to use more than one custom resources. For example, you can have two custom resources to associate up to 10 security groups to a pod. Please be aware when you are doing so: 
-
-1, you need to request increasing the limit since the default limit is 5 security groups per interface and there is a hard limit of 16 currently.
-
-2, currently Fargate only allows up to 5 security groups. If you are using Fargate, you can only use up to 5 security groups per pod.
+Note: The SecurityGroupPolicy CRD only supports up to 16 security groups per custom resource. currently Fargate only allows up to 5 security groups. If you are using Fargate, you can only use up to 5 security groups per pod.
 
 ## Windows IPv4 Address Management
 

apis/vpcresources/v1beta1/securitygrouppolicy_types.go

@@ -30,7 +30,7 @@ type SecurityGroupPolicySpec struct {
 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:MaxItems=16
 	Groups []string `json:"groupIds,omitempty"`
 }
 

config/crd/bases/vpcresources.k8s.aws_securitygrouppolicies.yaml

@@ -99,7 +99,7 @@ spec:
                       need to be applied to the ENI of a Pod.
                     items:
                       type: string
-                    maxItems: 5
+                    maxItems: 16
                     minItems: 1
                     type: array
                 type: object

pkg/aws/ec2/api/helper.go

@@ -19,6 +19,7 @@ import (
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/aws/aws-sdk-go/service/servicequotas"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/util/retry"
 
@@ -94,6 +95,7 @@ type EC2APIHelper interface {
 	AssignIPv4ResourcesAndWaitTillReady(eniID string, resourceType config.ResourceType, count int) ([]string, error)
 	UnassignIPv4Resources(eniID string, resourceType config.ResourceType, resources []string) error
 	DisassociateTrunkInterface(associationID *string) error
+	GetSecurityGroupQuota(quotaCode, serviceCode *string) (int, error)
 }
 
 // CreateNetworkInterface creates a new network interface
@@ -630,3 +632,16 @@ func (h *ec2APIHelper) DisassociateTrunkInterface(associationID *string) error {
 	}
 	return h.ec2Wrapper.DisassociateTrunkInterface(input)
 }
+
+func (h *ec2APIHelper) GetSecurityGroupQuota(quotaCode, serviceCode *string) (int, error) {
+	input := &servicequotas.GetServiceQuotaInput{
+		QuotaCode:   quotaCode,
+		ServiceCode: serviceCode,
+	}
+	serviceQuotaOutput, err := h.ec2Wrapper.GetServiceQuota(input)
+	if err != nil {
+		// Default value of security groups per network interface
+		return 5, fmt.Errorf("failed to retrieve security group quota: %v", err)
+	}
+	return int(serviceQuotaOutput), nil
+}

pkg/aws/ec2/api/helper_test.go

@@ -23,6 +23,7 @@ import (
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/aws/aws-sdk-go/service/servicequotas"
 	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/assert"
 	"k8s.io/apimachinery/pkg/util/wait"
@@ -1185,3 +1186,46 @@ func TestEc2APIHelper_GetBranchNetworkInterface(t *testing.T) {
 	assert.NoError(t, err)
 	assert.ElementsMatch(t, []*ec2.NetworkInterface{&networkInterface1, &networkInterface2}, branchInterfaces)
 }
+
+// TestEc2APIHelper_GetSecurityGroupQuota tests the GetSecurityGroupQuota function
+func TestEc2APIHelper_GetSecurityGroupQuota(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	ec2ApiHelper, mockWrapper := getMockWrapper(ctrl)
+
+	quotaCode := "L-1234ABCD"
+	serviceCode := "ec2"
+
+	getServiceQuotaInput := &servicequotas.GetServiceQuotaInput{
+		QuotaCode:   &quotaCode,
+		ServiceCode: &serviceCode,
+	}
+
+	mockWrapper.EXPECT().GetServiceQuota(getServiceQuotaInput).Return(5, nil)
+
+	quota, err := ec2ApiHelper.GetSecurityGroupQuota(&quotaCode, &serviceCode)
+	assert.NoError(t, err)
+	assert.Equal(t, 5, quota)
+}
+
+// TestEc2APIHelper_GetSecurityGroupQuota_Error tests that error is returned if GetServiceQuota call fails
+func TestEc2APIHelper_GetSecurityGroupQuota_Error(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	ec2ApiHelper, mockWrapper := getMockWrapper(ctrl)
+
+	quotaCode := "L-1234ABCD"
+	serviceCode := "ec2"
+
+	getServiceQuotaInput := &servicequotas.GetServiceQuotaInput{
+		QuotaCode:   &quotaCode,
+		ServiceCode: &serviceCode,
+	}
+
+	mockWrapper.EXPECT().GetServiceQuota(getServiceQuotaInput).Return(5, mockError)
+
+	_, err := ec2ApiHelper.GetSecurityGroupQuota(&quotaCode, &serviceCode)
+	assert.Error(t, err)
+}

pkg/aws/ec2/api/wrapper.go

@@ -31,6 +31,7 @@ import (
 	"github.com/aws/aws-sdk-go/aws/request"
 	"github.com/aws/aws-sdk-go/aws/session"
 	"github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/aws/aws-sdk-go/service/servicequotas"
 	"github.com/aws/aws-sdk-go/service/sts"
 	"github.com/go-logr/logr"
 	"github.com/prometheus/client_golang/prometheus"
@@ -61,6 +62,7 @@ type EC2Wrapper interface {
 	ModifyNetworkInterfaceAttribute(input *ec2.ModifyNetworkInterfaceAttributeInput) (*ec2.ModifyNetworkInterfaceAttributeOutput, error)
 	CreateNetworkInterfacePermission(input *ec2.CreateNetworkInterfacePermissionInput) (*ec2.CreateNetworkInterfacePermissionOutput, error)
 	DisassociateTrunkInterface(input *ec2.DisassociateTrunkInterfaceInput) error
+	GetServiceQuota(input *servicequotas.GetServiceQuotaInput) (int, error)
 }
 
 var (
@@ -358,6 +360,19 @@ var (
 		},
 	)
 
+	ec2GetServiceQuotaAPICallCnt = prometheus.NewCounter(
+		prometheus.CounterOpts{
+			Name: "ec2_describe_network_interfaces_pages_api_err_count",
+			Help: "The number of calls made to get service quotas (paginated)",
+		},
+	)
+	ec2GetServiceQuotaAPIErrCnt = prometheus.NewCounter(
+		prometheus.CounterOpts{
+			Name: "ec2_get_service_quota_api_err_count",
+			Help: "The number of errors encountered while making call to get service quotas (paginated)",
+		},
+	)
+
 	prometheusRegistered = false
 )
 
@@ -403,6 +418,8 @@ func prometheusRegister() {
 			LeakedENIClusterCleanupCnt,
 			ec2DescribeNetworkInterfacesPagesAPICallCnt,
 			ec2DescribeNetworkInterfacesPagesAPIErrCnt,
+			ec2GetServiceQuotaAPICallCnt,
+			ec2GetServiceQuotaAPIErrCnt,
 		)
 
 		prometheusRegistered = true
@@ -413,6 +430,7 @@ type ec2Wrapper struct {
 	log                   logr.Logger
 	instanceServiceClient *ec2.EC2
 	userServiceClient     *ec2.EC2
+	serviceQuotasClient   *servicequotas.ServiceQuotas
 	accountID             string
 }
 
@@ -429,6 +447,10 @@ func NewEC2Wrapper(roleARN, clusterName, region string, log logr.Logger) (EC2Wra
 		return nil, err
 	}
 
+	// Initialize the service quotas client
+	serviceQuotasClient := servicequotas.New(instanceSession)
+	ec2Wrapper.serviceQuotasClient = serviceQuotasClient
+
 	// Role ARN is passed, assume the role ARN to make EC2 API Calls
 	if roleARN != "" {
 		// Create the instance service client with low QPS, it will be only used for associate branch to trunk calls
@@ -892,3 +914,20 @@ func (e *ec2Wrapper) DisassociateTrunkInterface(input *ec2.DisassociateTrunkInte
 	}
 	return err
 }
+
+func (e *ec2Wrapper) GetServiceQuota(input *servicequotas.GetServiceQuotaInput) (int, error) {
+	start := time.Now()
+	// Using the service quotas client
+	output, err := e.serviceQuotasClient.GetServiceQuota(input)
+	ec2APICallLatencies.WithLabelValues("get_service_quota").Observe(time.Since(start).Seconds() * 1e3) // milliseconds
+
+	// Metric Update
+	ec2APICallCnt.Inc()
+	ec2GetServiceQuotaAPICallCnt.Inc()
+
+	if err != nil {
+		ec2APIErrCnt.Inc()
+		ec2GetServiceQuotaAPIErrCnt.Inc()
+	}
+	return int(*output.Quota.Value), nil
+}

webhooks/core/pod_webhook.go

@@ -25,8 +25,10 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/healthz"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
+	"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/aws/ec2/api"
 	"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/condition"
 	"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/config"
+
 	rcHealthz "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/healthz"
 	"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/utils"
 )
@@ -41,10 +43,11 @@ const (
 
 // PodResourceInjector injects resources into Pods
 type PodMutationWebHook struct {
-	decoder   *admission.Decoder
-	SGPAPI    utils.SecurityGroupForPodsAPI
-	Log       logr.Logger
-	Condition condition.Conditions
+	decoder      *admission.Decoder
+	SGPAPI       utils.SecurityGroupForPodsAPI
+	Log          logr.Logger
+	Condition    condition.Conditions
+	ec2ApiHelper api.EC2APIHelper
 }
 
 func NewPodMutationWebHook(
@@ -53,12 +56,14 @@ func NewPodMutationWebHook(
 	condition condition.Conditions,
 	d *admission.Decoder,
 	healthzHandler *rcHealthz.HealthzHandler,
+	ec2ApiHelper api.EC2APIHelper,
 ) *PodMutationWebHook {
 	podWebhook := &PodMutationWebHook{
-		SGPAPI:    sgpAPI,
-		Log:       log,
-		Condition: condition,
-		decoder:   d,
+		SGPAPI:       sgpAPI,
+		Log:          log,
+		Condition:    condition,
+		decoder:      d,
+		ec2ApiHelper: ec2ApiHelper,
 	}
 	// add health check on subpath for pod mutation webhook
 	healthzHandler.AddControllersHealthCheckers(
@@ -149,6 +154,13 @@ func (i *PodMutationWebHook) HandleFargatePod(req admission.Request, pod *corev1
 			response = admission.Allowed("Fargate pod not matching any SGP")
 		}
 	default:
+		// If more than 5 SGs match for the Pod, deny the request
+		// Fargate only allows up to 5 security groups. If you are using Fargate, you can only use up to 5 security groups per pod
+		if len(sgList) > 5 {
+			log.Info("Denying request due to too many matching security groups",
+				"Security Groups", sgList)
+			return admission.Denied("Too many matching security groups, rejecting event")
+		}
 		// If more than 1 SG match for the Pod then add all matching SG to the Annotation
 		pod.Annotations[FargatePodSGAnnotationKey] = strings.Join(sgList, ",")
 		log.Info("annotating Fargate pod with matching security groups",
@@ -163,7 +175,6 @@ func (i *PodMutationWebHook) HandleFargatePod(req admission.Request, pod *corev1
 // Limit to the Pod when the Windows IPAM feature is enabled via ConfigMap
 func (i *PodMutationWebHook) HandleWindowsPod(req admission.Request, pod *corev1.Pod,
 	log logr.Logger) (response admission.Response) {
-
 	if !i.Condition.IsWindowsIPAMEnabled() {
 		return admission.Allowed("")
 	}
@@ -182,17 +193,34 @@ func (i *PodMutationWebHook) HandleWindowsPod(req admission.Request, pod *corev1
 // matches any SGP
 func (i *PodMutationWebHook) HandleLinuxPod(req admission.Request, pod *corev1.Pod,
 	log logr.Logger) (response admission.Response) {
-
 	sgList, err := i.SGPAPI.GetMatchingSecurityGroupForPods(pod)
 	if err != nil {
 		i.Log.Error(err, "failed to get matching SGP for Pods",
 			"namespace", pod.Namespace, "name", pod.Name)
 		return admission.Denied("Failed to get Matching SGP for Pods, rejecting event")
 	}
+
 	if len(sgList) == 0 {
 		return admission.Allowed("Pod didn't match any SGP")
 	}
 
+	quotaCode := "L-2AFB9258" // Security groups per network interface QuotaCode
+	serviceCode := "vpc"
+
+	securityGroupQuota, err := i.ec2ApiHelper.GetSecurityGroupQuota(&quotaCode, &serviceCode)
+	if err != nil {
+		i.Log.Error(err, "failed to get security group quota",
+			"namespace", pod.Namespace, "name", pod.Name)
+		return admission.Denied("Failed to get security group quota, rejecting event")
+	}
+
+	// Verify that the number of securitygroups matches sevicequtoa
+	if len(sgList) > securityGroupQuota {
+		log.Info("Denied the security groups to match quota",
+			"Original Security Groups", sgList, "Quota", securityGroupQuota)
+		return admission.Denied("Exceed security group quota, rejecting event")
+	}
+
 	log.Info("injecting resource to the first container of the pod", "resource name",
 		config.ResourceNamePodENI, "resource count", DefaultResourceLimit)