[Policy Assistant] Add support for dynamic workload traffic via POLA cli

[gabrielggg]

Hey @huntergregory , as discussed, here is the pr for allowing dynamic traffic via cli. This is to finish #220 . And to allow #255 .

To test you cand do something like this:

Please check it out.

[k8s-ci-robot]

Hi @gabrielggg. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[netlify]

✅ Deploy Preview for kubernetes-sigs-network-policy-api ready!

Name	Link
🔨 Latest commit	21efeb4
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-network-policy-api/deploys/67322436d464980008f426e0
😎 Deploy Preview	https://deploy-preview-264--kubernetes-sigs-network-policy-api.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[huntergregory]

/ok-to-test

[huntergregory]

Thank you for the effort last minute @gabrielggg !

This looks great. If you could address the minor suggestions today that'd be amazing. Could you test each of the 5 workload types and when --port and --protocol are unspecified vs. specified?

Would be awesome if we could also support specifying a traffic.json with Pod labels etc. in case the user doesn't have a cluster and can't specify workload name. It also could be nice to support specifying workload in the traffic.json, but that is lower priority if we run out of time.

Let me know your thoughts! Thanks!

[gabrielggg]

hey @huntergregory, thanks for the review , i just committed some code to at least fix the conversations you opened, also tested the behavior when user doesn´t input port or protocol and tested different workload Types, the next commit will include support for the traffic.json file, but i think that will be ready for the night or maybe tomorrow.

[gabrielggg]

Hey @huntergregory , it's working now, and supporting cli arguments and also traffic.json file including native workloads on the file, I had to maneuver several scenarios to avoid null pointers, please check it out. Of course the code can be simplified but this was the fastest i could do for the first release!

This is the traffic, json file used for generating the image below.

https://github.com/gabrielggg/network-policy-api/blob/main/cmd/policy-assistant/examples/traffic.json

Some more testing with this last version:

[gabrielggg]

/retest

[huntergregory]

Thanks @gabrielggg that result looks amazing! I appreciate you going above and beyond supporting the whole traffic.json in this PR. I had minor suggestions for validating input. I also feel like having all these if-else statements could be bug prone 😕. Luckily, it looks like all the branches have shared logic that can be refactored into a couple functions? I added comments with outlines for much smaller code.

I still would love to release this change for the upcoming KubeCon, but I feel like I unfortunately can't do a just review of the code if there are all the if-else branches

[huntergregory]

/retest

[gabrielggg]

hey @huntergregory , thanks for the advice, i finished with the refactoring! Please check it out!

[huntergregory]

Thanks @gabrielggg, on the home stretch. I'd like to make one more change for the user experience. Good to merge after! We are planning to cut the release last minute on Monday after 12:00 PST if you could possibly update before then (the conference starts Tuesday). Would that work?

[gabrielggg]

Hey @huntergregory , thanks again for the review, i tackled all your suggestions and also squashed the commits. Please check it out!

[huntergregory]

Looks good at a glance. Adding approval to hopefully kick off the GitHub action to make sure it passes

/approve

Will add a LGTM hopefully tomorrow morning

[huntergregory]

/approve cancel

[huntergregory]

Trying the other way around. Lgtm first to kickoff GH action

/lgtm

[huntergregory]

Thanks @gabrielggg. Ready to approve but would prefer if we could fix the minor bug, and I'd recommend renaming the args, but neither is a blocker

[gabrielggg]

Hey @huntergregory , thanks again for the review, i made some changes based on your comments. Please check it out!

[huntergregory]

Thanks @gabrielggg ! This is a great feature

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gabrielggg, huntergregory

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• cmd/policy-assistant/OWNERS [huntergregory]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment