From e10ce2345b71f91b95c5221f69782e340d93ad0d Mon Sep 17 00:00:00 2001 From: Mike Bryant Date: Thu, 13 Jul 2017 16:38:25 +0100 Subject: [PATCH] Add comments to iptables rules and ipsets Fixes #2621 --- npc/controller.go | 2 +- npc/controller_test.go | 2 +- npc/ipset/ipset.go | 8 ++++---- npc/namespace.go | 33 +++++++++++++++++++++------------ npc/rule.go | 8 ++++++++ npc/selector.go | 12 +++++++----- 6 files changed, 42 insertions(+), 23 deletions(-) diff --git a/npc/controller.go b/npc/controller.go index a0ad6c78..f6419f94 100644 --- a/npc/controller.go +++ b/npc/controller.go @@ -54,7 +54,7 @@ func (npc *controller) onNewNsSelector(selector *selector) error { for _, ns := range npc.nss { if ns.namespace != nil { if selector.matches(ns.namespace.ObjectMeta.Labels) { - if err := ns.ips.AddEntry(selector.spec.ipsetName, string(ns.allPods.ipsetName)); err != nil { + if err := selector.addEntry(string(ns.allPods.ipsetName), namespaceComment(ns)); err != nil { return err } } diff --git a/npc/controller_test.go b/npc/controller_test.go index 2c179dde..32fcedcc 100644 --- a/npc/controller_test.go +++ b/npc/controller_test.go @@ -39,7 +39,7 @@ func (i *mockIPSet) Create(ipsetName ipset.Name, ipsetType ipset.Type) error { return nil } -func (i *mockIPSet) AddEntry(ipsetName ipset.Name, entry string) error { +func (i *mockIPSet) AddEntry(ipsetName ipset.Name, entry string, comment string) error { log.Printf("adding entry %s to %s", entry, ipsetName) if _, ok := i.sets[entry]; !ok { return errors.Errorf("ipset %s does not exist", entry) diff --git a/npc/ipset/ipset.go b/npc/ipset/ipset.go index b88e05b4..965d278e 100644 --- a/npc/ipset/ipset.go +++ b/npc/ipset/ipset.go @@ -19,7 +19,7 @@ const ( type Interface interface { Create(ipsetName Name, ipsetType Type) error - AddEntry(ipsetName Name, entry string) error + AddEntry(ipsetName Name, entry string, comment string) error DelEntry(ipsetName Name, entry string) error Flush(ipsetName Name) error Destroy(ipsetName Name) error @@ -40,15 +40,15 @@ func New(logger *log.Logger) Interface { } func (i *ipset) Create(ipsetName Name, ipsetType Type) error { - return doExec("create", string(ipsetName), string(ipsetType)) + return doExec("create", string(ipsetName), string(ipsetType), "comment") } -func (i *ipset) AddEntry(ipsetName Name, entry string) error { +func (i *ipset) AddEntry(ipsetName Name, entry string, comment string) error { i.Logger.Printf("adding entry %s to %s", entry, ipsetName) if i.inc(ipsetName, entry) > 1 { // already in the set return nil } - return doExec("add", string(ipsetName), entry) + return doExec("add", string(ipsetName), entry, "comment", comment) } func (i *ipset) DelEntry(ipsetName Name, entry string) error { diff --git a/npc/namespace.go b/npc/namespace.go index e0cce644..222cdf02 100644 --- a/npc/namespace.go +++ b/npc/namespace.go @@ -2,6 +2,7 @@ package npc import ( "encoding/json" + "fmt" "k8s.io/client-go/pkg/api/unversioned" coreapi "k8s.io/client-go/pkg/api/v1" @@ -74,7 +75,7 @@ func (ns *ns) onNewPodSelector(selector *selector) error { for _, pod := range ns.pods { if hasIP(pod) { if selector.matches(pod.ObjectMeta.Labels) { - if err := selector.addEntry(pod.Status.PodIP); err != nil { + if err := selector.addEntry(pod.Status.PodIP, podComment(pod)); err != nil { return err } } @@ -98,9 +99,9 @@ func (ns *ns) addPod(obj *coreapi.Pod) error { } if ns.checkLocalPod(obj) { - ns.ips.AddEntry(LocalIpset, obj.Status.PodIP) + ns.ips.AddEntry(LocalIpset, obj.Status.PodIP, podComment(obj)) } - return ns.podSelectors.addToMatching(obj.ObjectMeta.Labels, obj.Status.PodIP) + return ns.podSelectors.addToMatching(obj.ObjectMeta.Labels, obj.Status.PodIP, podComment(obj)) } func (ns *ns) updatePod(oldObj, newObj *coreapi.Pod) error { @@ -120,9 +121,9 @@ func (ns *ns) updatePod(oldObj, newObj *coreapi.Pod) error { if !hasIP(oldObj) && hasIP(newObj) { if ns.checkLocalPod(newObj) { - ns.ips.AddEntry(LocalIpset, newObj.Status.PodIP) + ns.ips.AddEntry(LocalIpset, newObj.Status.PodIP, podComment(newObj)) } - return ns.podSelectors.addToMatching(newObj.ObjectMeta.Labels, newObj.Status.PodIP) + return ns.podSelectors.addToMatching(newObj.ObjectMeta.Labels, newObj.Status.PodIP, podComment(newObj)) } if !equals(oldObj.ObjectMeta.Labels, newObj.ObjectMeta.Labels) || @@ -140,7 +141,7 @@ func (ns *ns) updatePod(oldObj, newObj *coreapi.Pod) error { } } if newMatch { - if err := ps.addEntry(newObj.Status.PodIP); err != nil { + if err := ps.addEntry(newObj.Status.PodIP, podComment(newObj)); err != nil { return err } } @@ -246,18 +247,18 @@ func (ns *ns) deleteNetworkPolicy(obj *extnapi.NetworkPolicy) error { return nil } -func bypassRule(nsIpsetName ipset.Name) []string { - return []string{"-m", "set", "--match-set", string(nsIpsetName), "dst", "-j", "ACCEPT"} +func bypassRule(nsIpsetName ipset.Name, namespace string) []string { + return []string{"-m", "set", "--match-set", string(nsIpsetName), "dst", "-j", "ACCEPT", "-m", "comment", "--comment", "DefaultAllow isolation for namespace: " + namespace} } func (ns *ns) ensureBypassRule(nsIpsetName ipset.Name) error { common.Log.Debugf("ensuring rule for DefaultAllow in namespace: %s, set %s", ns.name, nsIpsetName) - return ns.ipt.Append(TableFilter, DefaultChain, bypassRule(ns.allPods.ipsetName)...) + return ns.ipt.Append(TableFilter, DefaultChain, bypassRule(ns.allPods.ipsetName, ns.name)...) } func (ns *ns) deleteBypassRule(nsIpsetName ipset.Name) error { common.Log.Debugf("removing default rule in namespace: %s, set %s", ns.name, nsIpsetName) - return ns.ipt.Delete(TableFilter, DefaultChain, bypassRule(ns.allPods.ipsetName)...) + return ns.ipt.Delete(TableFilter, DefaultChain, bypassRule(ns.allPods.ipsetName, ns.name)...) } func (ns *ns) addNamespace(obj *coreapi.Namespace) error { @@ -271,7 +272,7 @@ func (ns *ns) addNamespace(obj *coreapi.Namespace) error { } // Add namespace ipset to matching namespace selectors - return ns.nsSelectors.addToMatching(obj.ObjectMeta.Labels, string(ns.allPods.ipsetName)) + return ns.nsSelectors.addToMatching(obj.ObjectMeta.Labels, string(ns.allPods.ipsetName), namespaceComment(ns)) } func (ns *ns) updateNamespace(oldObj, newObj *coreapi.Namespace) error { @@ -309,7 +310,7 @@ func (ns *ns) updateNamespace(oldObj, newObj *coreapi.Namespace) error { } } if newMatch { - if err := selector.addEntry(string(ns.allPods.ipsetName)); err != nil { + if err := selector.addEntry(string(ns.allPods.ipsetName), namespaceComment(ns)); err != nil { return err } } @@ -368,3 +369,11 @@ func isDefaultDeny(namespace *coreapi.Namespace) bool { nnp.Ingress.Isolation != nil && *(nnp.Ingress.Isolation) == DefaultDeny } + +func namespaceComment(namespace *ns) string { + return "namespace: " + namespace.name +} + +func podComment(pod *coreapi.Pod) string { + return fmt.Sprintf("namespace: %s, pod: %s", pod.ObjectMeta.Namespace, pod.ObjectMeta.Name) +} diff --git a/npc/rule.go b/npc/rule.go index 07854424..5ed49129 100644 --- a/npc/rule.go +++ b/npc/rule.go @@ -1,6 +1,7 @@ package npc import ( + "fmt" "strings" "k8s.io/client-go/pkg/types" @@ -29,6 +30,13 @@ func newRuleSpec(proto *string, srcHost *selectorSpec, dstHost *selectorSpec, ds args = append(args, "--dport", *dstPort) } args = append(args, "-j", "ACCEPT") + srcComment := "" + if srcHost.nsName != "" { + srcComment = fmt.Sprintf("pods: namespace: %s, selector: %s", srcHost.nsName, srcHost.key) + } else { + srcComment = fmt.Sprintf("namespaces: selector: %s", srcHost.key) + } + args = append(args, "-m", "comment", "--comment", fmt.Sprintf("%s -> namespace: %s, key: %s", srcComment, dstHost.nsName, dstHost.key)) key := strings.Join(args, " ") return &ruleSpec{key, args} diff --git a/npc/selector.go b/npc/selector.go index ba897ad1..b1c065cb 100644 --- a/npc/selector.go +++ b/npc/selector.go @@ -15,6 +15,7 @@ type selectorSpec struct { ipsetType ipset.Type // type of ipset to provision ipsetName ipset.Name // generated ipset name + nsName string // Namespace name } func newSelectorSpec(json *unversioned.LabelSelector, nsName string, ipsetType ipset.Type) (*selectorSpec, error) { @@ -30,7 +31,8 @@ func newSelectorSpec(json *unversioned.LabelSelector, nsName string, ipsetType i // the shortname because you can specify the same selector in multiple // namespaces - we need those to map to distinct ipsets ipsetName: ipset.Name(IpsetNamePrefix + shortName(nsName+":"+key)), - ipsetType: ipsetType}, nil + ipsetType: ipsetType, + nsName: nsName}, nil } type selector struct { @@ -42,8 +44,8 @@ func (s *selector) matches(labelMap map[string]string) bool { return s.spec.selector.Matches(labels.Set(labelMap)) } -func (s *selector) addEntry(entry string) error { - return s.ips.AddEntry(s.spec.ipsetName, entry) +func (s *selector) addEntry(entry string, comment string) error { + return s.ips.AddEntry(s.spec.ipsetName, entry, comment) } func (s *selector) delEntry(entry string) error { @@ -67,10 +69,10 @@ func newSelectorSet(ips ipset.Interface, onNewSelector selectorFn) *selectorSet entries: make(map[string]*selector)} } -func (ss *selectorSet) addToMatching(labelMap map[string]string, entry string) error { +func (ss *selectorSet) addToMatching(labelMap map[string]string, entry string, comment string) error { for _, s := range ss.entries { if s.matches(labelMap) { - if err := s.addEntry(entry); err != nil { + if err := s.addEntry(entry, comment); err != nil { return err } }