Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

If a custom network is used and no security group is specified on ENIConfig, the VPC default security group is attached to the trunk ENI #189

Closed
hiraken-w opened this issue Mar 19, 2023 · 1 comment · Fixed by #221
Assignees
Labels
bug Something isn't working v1.1.6 Release candidate for v1.1.6

Comments

@hiraken-w
Copy link

Describe the Bug:

I verified this behavior using the following version.

  • EKS cluster version
    1.25
  • Platform version
    eks.1
  • Amazon VPC CNI Plugin
    v1.12.2

When trunk ENI is created, the attaching security groups are chosen based on whether the node is using custom network.

if i.newCustomNetworkingSubnetID != "" {
i.currentInstanceSecurityGroup = i.newCustomNetworkingSecurityGroup
// Only get the subnet CIDR block again if the subnet ID has changed
if i.newCustomNetworkingSubnetID != i.currentSubnetID {
customSubnet, err := ec2APIHelper.GetSubnet(&i.newCustomNetworkingSubnetID)
if err != nil {
return err
}
if customSubnet == nil || customSubnet.CidrBlock == nil {
return fmt.Errorf("failed to find subnet %s", i.newCustomNetworkingSubnetID)
}
i.currentSubnetID = i.newCustomNetworkingSubnetID
i.currentSubnetCIDRBlock = *customSubnet.CidrBlock
}
} else {
// Custom networking in not being used, point to the instance security group and
// subnet details
i.currentSubnetID = i.instanceSubnetID
i.currentSubnetCIDRBlock = i.instanceSubnetCidrBlock
i.currentInstanceSecurityGroup = i.instanceSecurityGroups
}

And if the node is using a custom network, the vpc resource controller uses ENIConfig's spec.securityGroups for trunk ENI. However, if this is not specified on ENIConfig, the default security group for VPC will be used as specification for the CreateNetworkInterface API. This differs in behavior from the following specifications.

https://github.com/awsdocs/amazon-eks-user-guide/blob/086ca837b806e2fe73702f2cc1c827c677207ebb/doc_source/cni-custom-network.md?plain=1#L293-L296

If no security group is specified on ENIConfig, I believe that the primary ENI's security group should also be used for trunk ENI.

Observed Behavior:

If a custom network is used and no security group is specified on ENIConfig, the VPC default security group is attached to the trunk ENI

Expected Behavior:

The primary ENI's security groups are used for trunk ENI

How to reproduce it (as minimally and precisely as possible):

  1. Execute the following command.
kubectl set env daemonset aws-node -n kube-system ENABLE_POD_ENI=true

cat <<EOF | kubectl apply -f -
---
apiVersion: crd.k8s.amazonaws.com/v1alpha1 
kind: ENIConfig
metadata:
  name: us-west-2a
spec:
  subnet: subnet-XXXXXXXX
---
apiVersion: crd.k8s.amazonaws.com/v1alpha1 
kind: ENIConfig
metadata:
  name: us-west-2b
spec:
  subnet: subnet-YYYYYYYY
---
apiVersion: crd.k8s.amazonaws.com/v1alpha1 
kind: ENIConfig
metadata:
  name: us-west-2c
spec:
  subnet: subnet-ZZZZZZZZ
EOF

kubectl set env daemonset aws-node -n kube-system AWS_VPC_K8S_CNI_CUSTOM_NETWORK_CFG=true
kubectl set env daemonset aws-node -n kube-system ENI_CONFIG_LABEL_DEF=topology.kubernetes.io/zone
  1. Create a new node.

  2. Check the trunk ENI's security group.

Additional Context:

none.

Environment:

  • Kubernetes version (use kubectl version): 1.25.6
  • CNI Version: v1.12.2
  • OS (Linux/Windows): Linux
@hiraken-w hiraken-w added the bug Something isn't working label Mar 19, 2023
@haouc
Copy link
Contributor

haouc commented Mar 20, 2023

@hiraken-w thanks for submitting this issue. We should fail the request if the security groups are missing in eniConfig, or doing what you said as the doc describes to use instance SG. Let me discuss with our CNI team and make a change accordingly since this is supporting custom networking feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working v1.1.6 Release candidate for v1.1.6
Projects
None yet
3 participants