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

Clarification of prerequisite? #166

Closed
Guitarkalle opened this issue Dec 19, 2023 · 7 comments
Closed

Clarification of prerequisite? #166

Guitarkalle opened this issue Dec 19, 2023 · 7 comments
Labels
needs investigation question Further information is requested

Comments

@Guitarkalle
Copy link

Guitarkalle commented Dec 19, 2023

What happened:
I'm wondering what this requirement really means?

For any of your Kubernetes services, the service port must be the same as the container port. If you're using named ports, use the same name in the service spec too.

Source: https://docs.aws.amazon.com/eks/latest/userguide/cni-network-policy.html#cni-network-policy-considerations (bullet point aws/amazon-vpc-cni-k8s#5)

So port and targetPort cannot be different in the services? What happens if they are, what are the implications of this?
I tested enabling this feature in the CNI and it seems to work OK even if some of my services have different target ports.

Environment:

  • Kubernetes version (use kubectl version): 1.27
  • CNI Version: 1.15.3-eksbuild..1
  • OS (e.g: cat /etc/os-release): Amazon linux2
  • Kernel (e.g. uname -a): 5.10
@Guitarkalle Guitarkalle added needs investigation question Further information is requested labels Dec 19, 2023
@jdn5126
Copy link
Contributor

jdn5126 commented Dec 19, 2023

@Guitarkalle please link to the documentation snippet you are referring to

@Guitarkalle
Copy link
Author

Sorry.. Added the link to the original post

@jdn5126
Copy link
Contributor

jdn5126 commented Dec 19, 2023

Ah, this is a Network Policy question. Moving to Network Policy agent repo

@jdn5126 jdn5126 transferred this issue from aws/amazon-vpc-cni-k8s Dec 19, 2023
@jdn5126
Copy link
Contributor

jdn5126 commented Dec 27, 2023

@Guitarkalle I did some digging here, and the service prerequisite that you are referring to is required to make sure that the service IP shows up in the PolicyEndpoint that gets generated for the configured NetworkPolicy.

So when you define a NetworkPolicy to match against a pod selector, the controller reconciles that pod against any service IPs that may need to be added: https://github.com/aws/amazon-network-policy-controller-k8s/blob/main/pkg/resolvers/endpoints.go#L89

In order for the match to happen, the controller validates a few things: https://github.com/aws/amazon-network-policy-controller-k8s/blob/main/pkg/resolvers/endpoints.go#L368:

  1. namespace
  2. that the policy selector and service selector are the same
  3. that the port names and numbers match

So the implication of the ports not matching is that the Service IP may be missing from the PolicyEndpoint object. The upstream Kubernetes documentation in this space can be vague, so I think the AWS doc is trying to be explicit and strict to prevent unexpected behavior.

Hopefully this answers your question, but please let me know if any part is still confusing.

@Guitarkalle
Copy link
Author

Guitarkalle commented Jan 3, 2024

Hello @jdn5126 Thanks for the reply.

A few more questions if you don't mind. Reading the code, this is only an issue if your networkpolicies are filtering using port I guess? If you omit ports in the networkpolicy the "matching service port" code doesn't run?

Maybe I am misunderstanding but in this code: https://github.com/aws/amazon-network-policy-controller-k8s/blob/b2055b4430aaee064375c44ab3c5e63021c97cf1/pkg/k8s/service_utils.go#L13
it seems to me that it would work since it then returns the servicePort?
If we have a servicePort 443 and containerPort 8443, the networkPolicy should define port 8443 I think since networkPolicies are constructs attached to pods? If using named ports the logic for matching in the pod manifest seems to work also to me. Maybe as you say, this is only written as a requirement to reduce any potential issues since the policy agent works before DNAT? But to me it seems like the risk is low.

So the implication of the ports not matching is that the Service IP may be missing from the PolicyEndpoint

The implication of this is that traffic could be blocked I guess since the service IP would not be allowed? In general it feels that many third party services could cause issues here since it's not that uncommon to have different serviceport and targetport as well as named ports?

Is there a way on EKS to view the network policy agent logs to find if any of these issues exist?

@jdn5126
Copy link
Contributor

jdn5126 commented Jan 3, 2024

@Guitarkalle Your understanding seems correct to me. I think this is written as a "requirement" just to reduce complexity and the opportunity for confusion.

The implication of this is that traffic could be blocked I guess since the service IP would not be allowed? In general it feels that many third party services could cause issues here since it's not that uncommon to have different serviceport and targetport as well as named ports?

Correct, traffic would be blocked if the (service IP, service port) is not in the allow-list for the policy endpoint. I am not super familiar here, so I will do some more digging.

As for logs, they are written to /var/log/aws-routed-eni/network-policy-agent.log on the node by default. You can enable individual flow logs to see policy ACCEPT or DENY by enabling policy event logs (https://github.com/aws/aws-network-policy-agent?tab=readme-ov-file#enable-policy-event-logs). Note that these logs impact performance, so they should only be enabled while debugging

@Guitarkalle
Copy link
Author

Thanks for the answers. I will close this one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs investigation question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants