feat: Adds support for HNS L4WFPProxyPolicy

[davinci26]

Fixes #1002

Allow the cni plugin to marshall and apply L4WFPProxyPolicy
to Windows endpoints.

Tested on Kubernetes v1.19 with AKS-engine and docker runtime

Signed-off-by: Sotiris Nanopoulos sonanopo@microsoft.com

Reason for Change:

Issue Fixed:

Requirements:

• uses conventional commit messages
• includes documentation
• adds unit tests

Notes:

[davinci26]

Some comments:

I see that the other policies do not have unit tests so I did not add any tests but I am happy to add tests as needed. Similar for the documentation.

I have verified this change with Kubernetes + docker, let me know if you want me to verify this with Kubernetes + containerd as well.

In general I am looking for your guidance and I am happy to do any validation/changes needed to get this PR merged

[rbtr]

first pass lgtm so I kicked off the CI
@davinci26 actually this week we are doing a quality week focused on unit tests, if you are up to UT this scenario that would be much appreciated 😊
k8s x containerd validation would be good. does this functionality deserve a full e2e scenario?

[rbtr]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[davinci26]

if you are up to UT this scenario that would be much appreciated

I can try to do that.

k8s x containerd validation would be good

I will validate that today

does this functionality deserve a full e2e scenario?

I dont think this is is needed as this is mostly a dev solution.

Is the errors in the CI due to my change?

[rbtr]

negative, it's a flaky test 👍🏼

[davinci26]

@rbtr as I was working through the containerd + kubernetes integration I realized that I need to update the version of github.com/Microsoft/hcsshim to v0.8.22 from v0.8.10 to pick up the api change.

Making this change is making the diff 300+ files where all the files are in the vendor path. Does this seem correct?

Also I have added unit tests but you won't be able to run them in CI because they require GOOS=windows go test -v ./network/policy to be invoked but I think you have a draft PR for that so they will become available

See #758

[rbtr]

hey @matmerr maybe you could bump your #961 to 0.8.22 and we can merge that before this?

[davinci26]

Ok lets not merge this until #961 goes in (I still need to push my tests, I have them locally)

[matmerr]

hey @matmerr maybe you could bump your #961 to 0.8.22 and we can merge that before this?

would like to have @ashvindeodhar's eyes on this as well as #961

[ashvindeodhar]

lgtm

[davinci26]

Rebased and added a unit test for my feature as discussed. Please review if the test is in the correct direction, currently I invoke it with GOOS=windows go test -v ./network/policy but this mean that the test will not be invoked in the CI

[rbtr]

Thanks for updating 👍🏼
I am personally not a fan of ginkgo, but it is used for tests elsewhere in our repo already so 🤷🏼‍♂️

@ashvindeodhar can you re-review please?

[rbtr]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[davinci26]

@ashvindeodhar can you please take a look at this PR

[davinci26]

Am I going to be too annoying if I also ask to carve out a release? I want to ingest this change into aks-engine

[rbtr]

You want a new Windows CNI release, right?

[davinci26]

I want to push a pr similar to this https://github.com/Azure/aks-engine/pull/4656/files so I would need release 1.4.13

[rbtr]

@davinci26 v1.4.13 is released, includes your change