74 Commits

Author SHA1 Message Date
Xiang Liu
492e0d126b fix(NPC): make code more understandable 2022-03-15 12:06:22 -05:00
Aaron U'Ren
a9f0084665
Revert "feat(metrics): add more iptables sync metrics" (#1216)
This reverts commit 22b031beaa3393f8f02812242a9f637ce525b4eb.

@MikeSpreitzer pointed out that these metrics are already present in the
histogram type as *_count and *_sum and these two added metrics just add
duplicates. I've also proved out in my own environments that these
metric values are identical to the ones already carried in the
histogram.
2021-12-10 23:26:19 +05:30
Aaron U'Ren
419c078c60 feat(.golangci.yml): enable unparam linter and remediate 2021-09-11 16:20:07 -05:00
Aaron U'Ren
1d90e215e9 feat(.golangci.yml): enable stylecheck linter and remediate 2021-09-11 16:20:07 -05:00
Aaron U'Ren
85f28411dc feat(.golangci.yml): enable long lines linter and remediate 2021-09-11 16:20:07 -05:00
Aaron U'Ren
6208bfac46 feat(.golangci.yml): enable gomnd and remediate 2021-09-11 16:20:07 -05:00
Aaron U'Ren
f52fddddee feat(.golangci.yml): enable gocritic and remediate 2021-09-11 16:20:07 -05:00
Aaron U'Ren
d6ccc22519 feat(.golangci.yml): enable goconst and remediate 2021-09-11 16:20:07 -05:00
Aaron U'Ren
dc1960333d
fix(NPC): don't rely on exit code for chain check (#1157)
Don't use the exit code of NewChain() to decide if the chain exists or
not as it doesn't appear to be consistent between nftables and legacy
iptables implementations.
2021-08-18 23:06:02 +05:30
Aaron U'Ren
bffdc729cc
fix(npc): ordering of firewall / service rules (#1144) 2021-08-10 03:59:17 +05:30
Aaron U'Ren
1a8c354882 fix(NPC): Cleanup() function overhaul
Use existing cleanupStale*() methods to cleanup NPC based iptables and
ipsets. This provides a more consistent method of cleanup, consolidates
the logic, and updates it for all of the changes NPC has gone through.
2021-08-05 16:39:28 -05:00
Aaron U'Ren
9bc55dc1fa fix(NPC): missed ipset locking
Somehow I missed adding ipset locking around cleanupStaleIPSets()
2021-08-05 16:39:28 -05:00
Aaron U'Ren
031d75265b feat(NPC): minor performance improvement
Don't continue the loop if we've already matched.
2021-08-05 16:39:28 -05:00
Aaron U'Ren
e9be04ef2f
fix: add nil checking to ipsetMutex cleanup actions (#1129) 2021-07-20 01:22:48 +05:30
Murali Reddy
c8f7daf7ce fix lint errors 2021-06-28 12:45:43 -05:00
Murali Reddy
4c8cfc9c27 bug fix 2021-06-28 12:45:43 -05:00
Murali Reddy
93fe004ce6 bug fixes 2021-06-28 12:45:43 -05:00
Murali Reddy
d684ec0c65 add logic to explicitly ACCEPT traffic from/to the pod if its
permitted by applicable network policies. If there are no network
policies then by default ACCEPT the pod traffic
2021-06-28 12:45:43 -05:00
Ricardo Katz
21473edf05
Add support for kubernetes endport field (#1080) 2021-06-17 21:44:32 +05:30
Aaron U'Ren
fa8d69edd8 fix: add locking around ipset invocations 2021-06-01 10:42:08 -05:00
Aaron U'Ren
45b7fd1d94 fix(NPC): parse NodePorts as unsigned ints
Also separates logic so that it can be tested more easily, and adds unit
tests to make sure there is no regression.

Fixes #1083
2021-05-17 15:33:13 -05:00
Aaron U'Ren
be01f317c7 fact: other misc cleanups 2021-04-14 16:23:59 -05:00
Aaron U'Ren
53cfbe30eb fix: return early when we might be holding nil references 2021-04-14 16:23:59 -05:00
Aaron U'Ren
4efa5ccc48 fact: remove function parameters that are never referenced 2021-04-14 16:23:59 -05:00
Aaron U'Ren
96675e620b fix: don't capitalize error messages
It is standard practice in Go to not capitalize error messages:
https://github.com/golang/go/wiki/CodeReviewComments#error-strings
2021-04-14 16:23:59 -05:00
Aaron U'Ren
e9c77d0a35 fix(comments): misspellings and bad doc strings 2021-04-14 16:23:59 -05:00
Manuel Rüger
7d47aefe7d Replace github.com/golang/glog with k8s.io/klog/v2
glog is effectively unmaintained and the kubernetes ecosystem is mainly
using its fork klog

Fixes: #1051
2021-04-11 13:16:03 -05:00
Aaron U'Ren
22b031beaa feat(metrics): add more iptables sync metrics 2021-03-18 09:21:22 -05:00
Murali Reddy
afd866c0de use ipset save and restore to modify ipset to reduce exec calls 2021-03-18 09:21:22 -05:00
Murali Reddy
888cac9193 use iptables-save and iptables-restore commands to consolidate
individual iptables command that are run during full network
policies sync
2021-03-18 09:21:22 -05:00
Murali Reddy
e16f2077dd
npc code restructuring (#1007) 2020-12-16 18:40:00 +05:30
Murali Reddy
46e903aa13
remove deprecated netpol beta API support (#1001)
* remove deprecated netpol beta API support

* removing unused function
2020-11-26 21:24:32 +05:30
Aaron U'Ren
5a5e835d0f
fix(network_policy): mask mark reset on FW marks (#992)
Don't resent all marks, only the mark that we originally set as part of
the firewall rules so that we don't affect other systems like hostPort
and other elements of the nat chain that may apply their own marks.
2020-09-26 03:04:27 +05:30
Murali Reddy
3b992e4c45 fix build break due to go linter errros 2020-09-04 00:04:22 +05:30
Aaron U'Ren
7cd5235b69 fix(network_policy): missed gofmt on #970 2020-08-27 01:13:48 +02:00
Aaron U'Ren
827ce5510a
Permit ExternalIP on input (#970)
* fact(network_policy): validate ClusterIP CIDR

Ensure that --service-cluster-ip-range is a valid CIDR while controller
is starting up.

* fix(network_policy): parse/validate NodePort

Validate the NodePort range that is passed and allow for it to be
specified with hyphens which is what the previous example used to show
and is more cohesive with the way NodePort ranges are specified when
passed to the kube-apiserver.

* test(network_policy): add tests for input validation

* feat(network_policy): permit ExternalIP on input

fixes #934

* fix(network_policy): ensure pos with index offset

Because iptables list function now appears to be returning -N and -P
items in the chain results, we need to account for them when taking into
consideration the rule position.

* fix(network_policy): add uuid to comments on ensure

iptables list is now no longer keeping the position of parameters which
means that we can't compare string to string. In absence of a better way
to handle this, this adds a UUID to the comment string which can then be
looked for when determining what position a rule occupies.
2020-08-25 21:27:35 +05:30
Murali Reddy
a33089d292
[testing] run go linters (#943)
* run go linters for static code checking

* fix(lint): fix all goimports linting errors

* fix(lint): fix all golint errors

* fix(lint): fix all spelling errors

Co-authored-by: Aaron U'Ren <aauren@gmail.com>
2020-07-28 23:52:41 +05:30
Murali Reddy
1bec864a8a avoide listing a chain if the rule already exists 2020-06-15 16:34:48 +05:30
Murali Reddy
e858e2626f
change ACCEPT to RETURN with mark when a netpol is matched so that we run through (#915)
all the applicable network policies for a flow
2020-06-09 22:31:15 +05:30
Murali Reddy
4d6b0b818f
whitelist traffic to cluster IP and node ports in INPUT chain to bypass netwrok policy enforcement (#914)
* whitelist traffic to cluster IP and node ports in INPUT chain to bypass
netwrok policy enforcement

Fixes #905

* fix unit test failure

* ensure netpol firewall rules are configured after service proxy firewall rules
2020-06-09 16:36:31 +05:30
Murali Reddy
210dc3d526
avoids adding kube-router specific rules to enforce network policies in (#909)
built in chain used in fitler table

- adds one level of indirection
  INPUT   -> KUBE-ROUTER-INPUT
  FORWARD -> KUBE-ROUTER-FORWARD
  OUTPUT  -> KUBE-ROUTER-OUTPUT

- easy to troubleshoot with namesake custom chains

Fixes #185
2020-06-08 21:21:57 +05:30
刘旭
8f5c95942e
full sync when namespace labels change (#917) 2020-06-04 10:20:04 +05:30
Manuel Rüger
12674d5f8b
Add golangci-lint support (#895)
* Makefile: Add lint using golangci-lint

* build/travis-test.sh: Run lint step

* metrics_controller: Lint

pkg/metrics/metrics_controller.go:150:2: `mu` is unused (structcheck)
        mu          sync.Mutex
        ^
pkg/metrics/metrics_controller.go:151:2: `nodeIP` is unused (structcheck)
        nodeIP      net.IP
        ^

* network_service_graceful: Lint

pkg/controllers/proxy/network_service_graceful.go:21:6: `gracefulQueueItem` is unused (deadcode)
type gracefulQueueItem struct {
     ^
pkg/controllers/proxy/network_service_graceful.go:22:2: `added` is unused (structcheck)
        added   time.Time
        ^
pkg/controllers/proxy/network_service_graceful.go:23:2: `service` is unused (structcheck)
        service *ipvs.Service
        ^

* network_services_controller_test: Lint

pkg/controllers/proxy/network_services_controller_test.go:80:6: func `logf` is unused (unused)

* ecmp_vip: Lint

pkg/controllers/routing/ecmp_vip.go:208:4: S1023: redundant `return` statement (gosimple)
                        return
                        ^

* bgp_peers: Lint

pkg/controllers/routing/bgp_peers.go:331:4: S1023: redundant `return` statement (gosimple)
                        return
                        ^

* bgp_policies: Lint

pkg/controllers/routing/bgp_policies.go:80:3: S1011: should replace loop with `externalBgpPeers = append(externalBgpPeers, nrc.nodePeerRouters...)` (gosimple)
                for _, peer := range nrc.nodePeerRouters {
                ^
pkg/controllers/routing/bgp_policies.go:23:20: ineffectual assignment to `err` (ineffassign)
        podCidrPrefixSet, err := table.NewPrefixSet(config.PrefixSet{
                          ^
pkg/controllers/routing/bgp_policies.go:42:22: ineffectual assignment to `err` (ineffassign)
        clusterIPPrefixSet, err := table.NewPrefixSet(config.PrefixSet{
                            ^
pkg/controllers/routing/bgp_policies.go:33:30: Error return value of `nrc.bgpServer.AddDefinedSet` is not checked (errcheck)
                nrc.bgpServer.AddDefinedSet(podCidrPrefixSet)
                                           ^
pkg/controllers/routing/bgp_policies.go:48:30: Error return value of `nrc.bgpServer.AddDefinedSet` is not checked (errcheck)
                nrc.bgpServer.AddDefinedSet(clusterIPPrefixSet)
                                           ^
pkg/controllers/routing/bgp_policies.go:69:31: Error return value of `nrc.bgpServer.AddDefinedSet` is not checked (errcheck)
                        nrc.bgpServer.AddDefinedSet(iBGPPeerNS)
                                                   ^
pkg/controllers/routing/bgp_policies.go:108:31: Error return value of `nrc.bgpServer.AddDefinedSet` is not checked (errcheck)
                        nrc.bgpServer.AddDefinedSet(ns)
                                                   ^
pkg/controllers/routing/bgp_policies.go:120:30: Error return value of `nrc.bgpServer.AddDefinedSet` is not checked (errcheck)
                nrc.bgpServer.AddDefinedSet(ns)
                                           ^
                                                   ^

* network_policy_controller: Lint

pkg/controllers/netpol/network_policy_controller.go:35:2: `networkPolicyAnnotation` is unused (deadcode)
        networkPolicyAnnotation      = "net.beta.kubernetes.io/network-policy"
        ^
pkg/controllers/netpol/network_policy_controller.go:1047:4: SA9003: empty branch (staticcheck)
                        if err != nil {
                        ^
pkg/controllers/netpol/network_policy_controller.go:969:10: SA4006: this value of `err` is never used (staticcheck)
        chains, err := iptablesCmdHandler.ListChains("filter")
                ^
pkg/controllers/netpol/network_policy_controller.go:1568:4: SA4006: this value of `err` is never used (staticcheck)
                        err = iptablesCmdHandler.Delete("filter", "FORWARD", strconv.Itoa(i-realRuleNo))
                        ^
pkg/controllers/netpol/network_policy_controller.go:1584:4: SA4006: this value of `err` is never used (staticcheck)
                        err = iptablesCmdHandler.Delete("filter", "OUTPUT", strconv.Itoa(i-realRuleNo))
                        ^

* network_services_controller: Lint

pkg/controllers/proxy/network_services_controller.go:66:2: `h` is unused (deadcode)
        h      *ipvs.Handle
        ^
pkg/controllers/proxy/network_services_controller.go:879:23: SA1019: client.NewEnvClient is deprecated: use NewClientWithOpts(FromEnv)  (staticcheck)
        dockerClient, err := client.NewEnvClient()
                             ^
pkg/controllers/proxy/network_services_controller.go:944:5: unreachable: unreachable code (govet)
                                glog.V(3).Infof("Waiting for tunnel interface %s to come up in the pod, retrying", KUBE_TUNNEL_IF)
                                ^
pkg/controllers/proxy/network_services_controller.go:1289:5: S1002: should omit comparison to bool constant, can be simplified to `!hasHairpinChain` (gosimple)
        if hasHairpinChain != true {
           ^
pkg/controllers/proxy/network_services_controller.go:1237:43: S1019: should use make(map[string][]string) instead (gosimple)
        rulesNeeded := make(map[string][]string, 0)
                                                 ^
pkg/controllers/proxy/network_services_controller.go:1111:4: S1023: redundant break statement (gosimple)
                        break
                        ^
pkg/controllers/proxy/network_services_controller.go:1114:4: S1023: redundant break statement (gosimple)
                        break
                        ^
pkg/controllers/proxy/network_services_controller.go:1117:4: S1023: redundant break statement (gosimple)
                        break
                        ^
pkg/controllers/proxy/network_services_controller.go:445:21: Error return value of `nsc.publishMetrics` is not checked (errcheck)
                nsc.publishMetrics(nsc.serviceMap)
                                  ^
pkg/controllers/proxy/network_services_controller.go:1609:9: Error return value of `h.Write` is not checked (errcheck)
        h.Write([]byte(ip + "-" + protocol + "-" + port))
               ^
pkg/controllers/proxy/network_services_controller.go:912:13: Error return value of `netns.Set` is not checked (errcheck)
                        netns.Set(hostNetworkNamespaceHandle)
                                 ^
pkg/controllers/proxy/network_services_controller.go:926:13: Error return value of `netns.Set` is not checked (errcheck)
                        netns.Set(hostNetworkNamespaceHandle)
                                 ^
pkg/controllers/proxy/network_services_controller.go:950:13: Error return value of `netns.Set` is not checked (errcheck)
                        netns.Set(hostNetworkNamespaceHandle)
                                 ^
pkg/controllers/proxy/network_services_controller.go:641:9: SA4006: this value of `err` is never used (staticcheck)
        addrs, err := getAllLocalIPs()
               ^

* network_routes_controller: Lint

pkg/controllers/routing/network_routes_controller.go:340:2: S1000: should use for range instead of for { select {} } (gosimple)
        for {
        ^
pkg/controllers/routing/network_routes_controller.go:757:22: Error return value of `nrc.bgpServer.Stop` is not checked (errcheck)
                        nrc.bgpServer.Stop()
                                          ^
pkg/controllers/routing/network_routes_controller.go:770:22: Error return value of `nrc.bgpServer.Stop` is not checked (errcheck)
                        nrc.bgpServer.Stop()
                                          ^
pkg/controllers/routing/network_routes_controller.go:782:23: Error return value of `nrc.bgpServer.Stop` is not checked (errcheck)
                                nrc.bgpServer.Stop()
                                                  ^
pkg/controllers/routing/network_routes_controller.go:717:12: Error return value of `g.Serve` is not checked (errcheck)
        go g.Serve()

* ipset: Lint

pkg/utils/ipset.go:243:23: Error return value of `entry.Set.Parent.Save` is not checked (errcheck)
        entry.Set.Parent.Save()
                             ^

* pkg/cmd/kube-router: Lint

pkg/cmd/kube-router.go:214:26: SA1006: printf-style function with dynamic format string and no further arguments should use print-style function instead (staticcheck)
                fmt.Fprintf(os.Stderr, output)
                                       ^
pkg/cmd/kube-router.go:184:15: SA1017: the channel used with signal.Notify should be buffered (staticcheck)
        signal.Notify(ch, syscall.SIGINT, syscall.SIGTERM)
                     ^
pkg/cmd/kube-router.go:94:17: Error return value of `hc.RunServer` is not checked (errcheck)
        go hc.RunServer(stopCh, &wg)
                       ^
pkg/cmd/kube-router.go:112:16: Error return value of `hc.RunCheck` is not checked (errcheck)
        go hc.RunCheck(healthChan, stopCh, &wg)
                      ^
pkg/cmd/kube-router.go:121:12: Error return value of `mc.Run` is not checked (errcheck)
                go mc.Run(healthChan, stopCh, &wg)
                         ^

* cmd/kube-router/kube-router: Lint

cmd/kube-router/kube-router.go:31:24: Error return value of `flag.CommandLine.Parse` is not checked (errcheck)
        flag.CommandLine.Parse([]string{})
                              ^
cmd/kube-router/kube-router.go:33:10: Error return value of `flag.Set` is not checked (errcheck)
        flag.Set("logtostderr", "true")
                ^
cmd/kube-router/kube-router.go:34:10: Error return value of `flag.Set` is not checked (errcheck)
        flag.Set("v", config.VLevel)
                ^
cmd/kube-router/kube-router.go:62:27: SA1006: printf-style function with dynamic format string and no further arguments should use print-style function instead (staticcheck)
                        fmt.Fprintf(os.Stdout, http.ListenAndServe("0.0.0.0:6060", nil).Error())
                                               ^

* kube-router_test: Lint

cmd/kube-router/kube-router_test.go:21:10: Error return value of `io.Copy` is not checked (errcheck)
                io.Copy(stderrBuf, stderrR)
                       ^
cmd/kube-router/kube-router_test.go:40:17: Error return value of `docBuf.ReadFrom` is not checked (errcheck)
        docBuf.ReadFrom(docF)
                       ^

* service_endpoints_sync: Lint

pkg/controllers/proxy/service_endpoints_sync.go:460:2: ineffectual assignment to `ipvsSvcs` (ineffassign)
        ipvsSvcs, err := nsc.ln.ipvsGetServices()
        ^
pkg/controllers/proxy/service_endpoints_sync.go:311:5: SA4006: this value of `err` is never used (staticcheck)
                                err = nsc.ln.ipAddrDel(dummyVipInterface, externalIP)
                                ^

* node: Lint

pkg/utils/node.go:19:16: SA1019: clientset.Core is deprecated: please explicitly pick a version if possible.  (staticcheck)
                node, err := clientset.Core().Nodes().Get(nodeName, metav1.GetOptions{})
                             ^
pkg/utils/node.go:27:15: SA1019: clientset.Core is deprecated: please explicitly pick a version if possible.  (staticcheck)
        node, err := clientset.Core().Nodes().Get(hostName, metav1.GetOptions{})
                     ^
pkg/utils/node.go:34:15: SA1019: clientset.Core is deprecated: please explicitly pick a version if possible.  (staticcheck)
                node, err = clientset.Core().Nodes().Get(hostnameOverride, metav1.GetOptions{})
                            ^

* aws: Lint

pkg/controllers/routing/aws.go:31:8: SA4006: this value of `err` is never used (staticcheck)
                URL, err := url.Parse(providerID)
                     ^

* health_controller: Lint

pkg/healthcheck/health_controller.go:54:10: Error return value of `w.Write` is not checked (errcheck)
                w.Write([]byte("OK\n"))
                       ^
pkg/healthcheck/health_controller.go:68:10: Error return value of `w.Write` is not checked (errcheck)
                w.Write([]byte("Unhealthy"))
                       ^
pkg/healthcheck/health_controller.go:159:2: S1000: should use a simple channel send/receive instead of `select` with a single case (gosimple)
        select {
        ^

* network_routes_controller_test: Lint

pkg/controllers/routing/network_routes_controller_test.go:1113:37: Error return value of `testcase.nrc.bgpServer.Stop` is not checked (errcheck)
                        defer testcase.nrc.bgpServer.Stop()
                                                         ^
pkg/controllers/routing/network_routes_controller_test.go:1314:37: Error return value of `testcase.nrc.bgpServer.Stop` is not checked (errcheck)
                        defer testcase.nrc.bgpServer.Stop()
                                                         ^
pkg/controllers/routing/network_routes_controller_test.go:2327:37: Error return value of `testcase.nrc.bgpServer.Stop` is not checked (errcheck)
                        defer testcase.nrc.bgpServer.Stop()
                                                         ^

* .golangci.yml: Increase timeout

Default is 1m, increase to 5m otherwise travis might fail

* Makefile: Update golangci-lint to 1.27.0

* kube-router_test.go: defer waitgroup

Co-authored-by: Aaron U'Ren <aauren@users.noreply.github.com>

* network_routes_controller: Incorporate review

* bgp_policies: Incorporate review

* network_routes_controller: Incorporate review

* bgp_policies: Log error instead

* network_services_controller: Incorporate review

Co-authored-by: Aaron U'Ren <aauren@users.noreply.github.com>
2020-06-03 22:29:06 +02:00
Aaron U'Ren
837554bf1a
Fix Memory Consumption in network_policy_controller (#902)
* feat(gitignore): don't track intellij files

* fact(network_policy): networkPoliciesInfo -> stack

Take networkPoliciesInfo off of the npc struct and convert it to a stack
variable that is easy to cleanup.

* fix(network_policy): k8s obj memory accumulation

Kubernetes informers will block on handler execution and will then begin
to accumulate cached Kubernetes object information into the heap. This
change moves the full sync logic into it's own goroutine where full
syncs are triggered and gated via writing to a single item channel.

This ensures that:
- Syncs will only happen one at a time (as they are full syncs and we
  can't process multiple at once)
- Sync requests are only ever delayed and never lost as they will be
  added to the request channel
- After we make a sync request we return fast to ensure that the handler
  execution returns fast and that we don't block the Kubernetes
  informers

* fact(network_policy): rework readyForUpdates

Now that we are better managing requests for full syncs we no longer
need to manage readyForUpdates on the npc controller. We already enforce
not blocking the handlers and a single sync execution chain, whether it
comes from the controller in the form of a periodic sync or whether it
comes from a Kubernetes informer, either way the result is a
non-blocking, single thread of execution, full sync.

* fix(network_policy): address PR feedback
2020-05-20 16:15:57 +05:30
Aaron U'Ren
856c7d762a fix(network_policy): cleanup all primary chains
Cleanup all primary chains, and not just FORWARD and INPUT.

fixes #896
2020-05-12 16:47:17 -05:00
Aaron U'Ren
02058ece97 doc(network_policy_controller): add comments 2020-05-12 16:03:09 -05:00
Murali Reddy
361d6feb15
outbound traffic from pod should be intercepted in filter table INPUT chain (#891)
(pod's traffic that is destined to node's local ip). with out this fix even
with network policy to drop all egress traffic, pod can reach host IP's.
Pod's can access any service hosted in host network as well
2020-05-04 16:42:51 +05:30
Murali Reddy
1af329c480
nflog the packet that will be dropped by network policy enforcement (#889)
* nflog the packet that will be dropped by network policy enforcement
that can be further by read by ulogd

Fixes #505

* addressing review comments
2020-05-02 15:45:11 +05:30
Murali Reddy
7777b9a825
use Spec.PolicyTypes for the type of network policy (#883) 2020-04-23 08:19:26 +05:30
Filinto Duran
3e67159579
Update selectors to allow matchexpressions as well as matlabels (#881)
--------------------------------------------
Copyright: Sony Interactive Entertainment Inc.

Co-authored-by: Author Name <Filinto.Duran@sony.com>
2020-04-21 16:17:00 +05:30