Skip to content

Docs: make ToServices selectors work for in-cluster services too #35506

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 31, 2024
Merged

Docs: make ToServices selectors work for in-cluster services too #35506

merged 1 commit into from
Oct 31, 2024

Conversation

chaunceyjiang
Copy link
Member

@chaunceyjiang chaunceyjiang commented Oct 23, 2024 •
edited
Loading

Part of #34208

Env

# kubectl get pods -o wide           
NAME                              READY   STATUS    RESTARTS   AGE   IP             NODE                 NOMINATED NODE   READINESS GATES
details-v1-6448f9bdc8-p7b7l       1/1     Running   0          18d   10.244.2.208   kind-worker2         <none>           <none>
nginx-77b4fdf86c-968pb            1/1     Running   0          10d   10.244.2.219   kind-worker2         <none>           <none>
nginx-77b4fdf86c-bfj9w            1/1     Running   0          10d   10.244.0.32    kind-control-plane   <none>           <none>
nginx-77b4fdf86c-wg8dx            1/1     Running   0          10d   10.244.1.69    kind-worker          <none>           <none>
productpage-v1-65b8499c86-jbq8b   1/1     Running   0          18d   10.244.1.13    kind-worker          <none>           <none>
ratings-v1-56687d6766-c2pv5       1/1     Running   0          18d   10.244.2.173   kind-worker2         <none>           <none>
reviews-v1-5c785db578-c4p6v       1/1     Running   0          18d   10.244.2.13    kind-worker2         <none>           <none>
reviews-v2-6d8c88978b-6cc9k       1/1     Running   0          18d   10.244.2.212   kind-worker2         <none>           <none>
reviews-v3-678b968858-2kkwb       1/1     Running   0          18d   10.244.1.95    kind-worker          <none>           <none>

Access

# kubectl exec -it nginx-77b4fdf86c-968pb -- curl 10.244.2.208:9080/details/1
{"id":1,"author":"William Shakespeare","year":1595,"type":"paperback","pages":200,"publisher":"PublisherA","language":"English","ISBN-10":"1234567890","ISBN-13":"123-1234567890"}

#
# kubectl exec -it nginx-77b4fdf86c-968pb -- curl 10.244.1.13:9080                      
<!DOCTYPE html>
<html>
  <head>
    <title>Simple Bookstore App</title>
<meta charset="utf-8">
<meta http-equiv="X-UA-Compatible" content="IE=edge">
<meta name="viewport" content="width=device-width, initial-scale=1">

<!-- Latest compiled and minified CSS -->
<link rel="stylesheet" href="static/bootstrap/css/bootstrap.min.css">

<!-- Optional theme -->
<link rel="stylesheet" href="static/bootstrap/css/bootstrap-theme.min.css">

  </head>
  <body>
    
    
<p>
    <h3>Hello! This is a simple bookstore application consisting of three services as shown below</h3>
</p>
......
....

Test

# cat to_services.yaml             
apiVersion: "cilium.io/v2"
kind: CiliumNetworkPolicy
metadata:
  name: "l4-rule"
spec:
  endpointSelector:
    matchLabels:
      app: nginx
  egress:
    - toServices:
      - k8sServiceSelector:
          selector:
            matchLabels:
              app: "details"
      toPorts:
      - ports:
        - port: "9080"
          protocol: TCP
#  k apply -f to_services.yaml 
ciliumnetworkpolicy.cilium.io/l4-rule created

Access

# kubectl exec -it nginx-77b4fdf86c-968pb -- curl 10.244.2.208:9080/details/1
{"id":1,"author":"William Shakespeare","year":1595,"type":"paperback","pages":200,"publisher":"PublisherA","language":"English","ISBN-10":"1234567890","ISBN-13":"123-1234567890"}#                                                                                                                         

# kubectl exec -it nginx-77b4fdf86c-968pb -- curl 10.244.1.13:9080           
curl: (28) Failed to connect to 10.244.1.13 port 9080 after 129288 ms: Couldn't connect to server
command terminated with exit code 28

hubble


hubble observe -f --to-port 9080
Oct 29 06:46:44.262: default/nginx-77b4fdf86c-968pb:59604 (ID:7824) <> default/productpage-v1-65b8499c86-jbq8b:9080 (ID:9909) from-endpoint FORWARDED (TCP Flags: SYN)
Oct 29 06:46:44.262: default/nginx-77b4fdf86c-968pb:59604 (ID:7824) <> default/productpage-v1-65b8499c86-jbq8b:9080 (ID:9909) policy-verdict:none EGRESS DENIED (TCP Flags: SYN)
Oct 29 06:46:44.262: default/nginx-77b4fdf86c-968pb:59604 (ID:7824) <> default/productpage-v1-65b8499c86-jbq8b:9080 (ID:9909) Policy denied DROPPED (TCP Flags: SYN)
Oct 29 06:46:45.273: default/nginx-77b4fdf86c-968pb:59604 (ID:7824) <> default/productpage-v1-65b8499c86-jbq8b:9080 (ID:9909) from-endpoint FORWARDED (TCP Flags: SYN)
Oct 29 06:46:45.273: default/nginx-77b4fdf86c-968pb:59604 (ID:7824) <> default/productpage-v1-65b8499c86-jbq8b:9080 (ID:9909) policy-verdict:none EGRESS DENIED (TCP Flags: SYN)

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Oct 23, 2024
@chaunceyjiang chaunceyjiang marked this pull request as ready for review October 23, 2024 12:25
@chaunceyjiang chaunceyjiang requested review from a team as code owners October 23, 2024 12:25
qmonnet
qmonnet reviewed Oct 23, 2024
View reviewed changes
@qmonnet qmonnet added area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. release-note/misc This PR makes changes that have no direct user impact. labels Oct 23, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Oct 23, 2024
@qmonnet
Copy link
Member

qmonnet commented Oct 23, 2024

/test

@joestringer
Copy link
Member

@chaunceyjiang please avoid something like this in the PR description:

```release-note
None
```

This will cause a release note to be created that just says:

None (#35506)

Instead, it is better to just completely remove the ```release-note ... ``` section. Thanks!

joestringer
joestringer reviewed Oct 23, 2024
View reviewed changes
@joestringer joestringer requested a review from a team October 23, 2024 18:16
doniacld
doniacld approved these changes Oct 28, 2024
View reviewed changes
Copy link
Contributor

@doniacld doniacld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@chaunceyjiang
Copy link
Member Author

/ci-e2e-upgrade

joestringer
joestringer approved these changes Oct 29, 2024
View reviewed changes
Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for demonstrating! One small nit, I think that rather than using a service labels bar:bar it should be something at least a little bit more realistic like serviceName: myservice (which also matches the other example file in examples/policies/l3/service).

With that change I think it should be good to go in.

qmonnet
qmonnet requested changes Oct 30, 2024
View reviewed changes
@chaunceyjiang
Copy link
Member Author

/test

@joestringer joestringer enabled auto-merge October 30, 2024 16:28
@joestringer joestringer disabled auto-merge October 30, 2024 16:29
@joestringer joestringer mentioned this pull request Oct 30, 2024
Closed
@joestringer
Copy link
Member

I don't quite understand why, but several of the tests are failing with the errors in #35666 . I don't see any evidence of the same failure affecting the main branch. Is it possible this PR somehow triggers those failures? Several of the failures have recurred multiple times when retriggering the workflow test runs.

@chaunceyjiang
policy: make ToServices selectors work for in-cluster services too
125bf8f 
Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com>
@chaunceyjiang
Copy link
Member Author

/test

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Oct 31, 2024
@julianwiedmann julianwiedmann added the sig/policy Impacts whether traffic is allowed or denied based on user-defined policies. label Oct 31, 2024
@julianwiedmann julianwiedmann added this pull request to the merge queue Oct 31, 2024
@chaunceyjiang
Copy link
Member Author

I don't quite understand why, but several of the tests are failing with the errors in #35666 . I don't see any evidence of the same failure affecting the main branch. Is it possible this PR somehow triggers those failures? Several of the failures have recurred multiple times when retriggering the workflow test runs.

@joestringer I just rebased the main branch and all the tests passed the check.

Merged via the queue into cilium:main with commit d8cf2b1 Oct 31, 2024
109 checks passed
@chaunceyjiang chaunceyjiang deleted the docs_to_service branch October 31, 2024 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joestringer joestringer joestringer approved these changes

@qmonnet qmonnet qmonnet approved these changes

+1 more reviewer

@doniacld doniacld doniacld approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact. sig/policy Impacts whether traffic is allowed or denied based on user-defined policies.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@chaunceyjiang @qmonnet @joestringer @doniacld @julianwiedmann