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

support new SGPP standard mode #1907

Merged
merged 1 commit into from
Mar 31, 2022
Merged

support new SGPP standard mode #1907

merged 1 commit into from
Mar 31, 2022

Conversation

M00nF1sh
Copy link
Contributor

@M00nF1sh M00nF1sh commented Mar 4, 2022

What type of PR is this?
feature
cleanup

Which issue does this PR fix:
This PR supports calico network policy & NodeLocalDNS for Pods with SecurityGroup with a new traffic mode.

What does this PR do / Why do we need it:

  1. This PR introduces a new knob called POD_SECURITY_GROUP_ENFORCING_MODE, which decides the traffic mode "security group for pods" feature.
    The possible settings of POD_SECURITY_GROUP_ENFORCING_MODE are:
    • strict: all inbound/outbound traffic from pod with security group will be enforced by security group rules.
      • this is the default mode, and is the existing behavior before this change.
    • standard: the traffic of pod with security group behaves same as pods without security group, except that each pod occupies dedicated ENI
      • inbound traffic to pod with security group from another host will be enforced by security group rules.
      • outbound traffic from pod with security group to another host in same VPC will be enforced by security group rules.
      • outbound traffic from pod with security group to IP address outside VPC
        • if externalSNAT enabled, traffic won't be SNATed, thus will be enforced by security group rules.
        • if externalSNAT disabled, traffic will be SNATed via eth0, thus will only be enforced by security group associated with eth0.
      • inbound/bound traffic from another pod on same host or another service on same host(such as kubelet/nodeLocalDNS) won't be enforced by security group rules.
  2. This PR did several refactor to make original code more clean and maintainable.

If an issue # is not available please add repro steps and logs from IPAMD/CNI showing the issue:

Testing done on this change:

  1. rewrote test cases for driver.go
    • increases test coverage from 34% to 95%.
    • actually test API calls instead of abuse gomock.any
    • fixes old broken tests

Automation added to e2e:

Will this PR introduce any new dependencies?:

N/A

Will this break upgrades or downgrades. Has updating a running cluster been tested?:

Does this change require updates to the CNI daemonset config files to work?:

No, the default value of "POD_SECURITY_GROUP_ENFORCING_MODE" if not set is "standard".

Does this PR introduce any user-facing change?:

Introduce a new knob named POD_SECURITY_GROUP_ENFORCING_MODE to decide the traffic mode for security group for pods feature. 

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@M00nF1sh M00nF1sh requested a review from a team as a code owner March 4, 2022 21:47
@jayanthvn
Copy link
Contributor

@M00nF1sh - Can you please run "make format"?

@M00nF1sh M00nF1sh force-pushed the sgpp_enhance branch 2 times, most recently from a8c14e8 to 0be6f77 Compare March 7, 2022 20:38
@jayanthvn jayanthvn added this to the v1.11.1 milestone Mar 7, 2022
@jayanthvn jayanthvn modified the milestones: v1.11.1, v1.11.0 Mar 7, 2022
pkg/sgpp/utils.go Outdated Show resolved Hide resolved
@M00nF1sh M00nF1sh force-pushed the sgpp_enhance branch 5 times, most recently from af14a3d to c42c8f4 Compare March 9, 2022 19:25
README.md Show resolved Hide resolved
@M00nF1sh M00nF1sh force-pushed the sgpp_enhance branch 7 times, most recently from 70156a5 to 8e6c69c Compare March 22, 2022 23:20
@M00nF1sh M00nF1sh merged commit d43309b into aws:master Mar 31, 2022
@joebowbeer
Copy link

@M00nF1sh The description contains conflicting statements. I assume this one is wrong:

Does this change require updates to the CNI daemonset config files to work?:

No, the default value of "POD_SECURITY_GROUP_ENFORCING_MODE" if not set is "standard".

I assume default is strict to agree with previous statement in description.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants