Skip to content

Prevent Namespace Race #463

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Dec 20, 2019
Merged

Prevent Namespace Race #463

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
108ac57
poll api-server version for a minute before panicking
jaer-tsun Dec 18, 2019
c92544f
always add namespace set, when adding nw policy
jaer-tsun Dec 20, 2019
53c4c6d
create the ns set in add pod, if add namespace has not been called yet
jaer-tsun Dec 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions npm/npm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,11 +136,21 @@ func NewNetworkPolicyManager(clientset *kubernetes.Clientset, informerFactory in
iptMgr := iptm.NewIptablesManager()
iptMgr.UninitNpmChains()

podInformer := informerFactory.Core().V1().Pods()
nsInformer := informerFactory.Core().V1().Namespaces()
npInformer := informerFactory.Networking().V1().NetworkPolicies()
var (
podInformer = informerFactory.Core().V1().Pods()
nsInformer = informerFactory.Core().V1().Namespaces()
npInformer = informerFactory.Networking().V1().NetworkPolicies()
serverVersion *version.Info
err error
)

serverVersion, err := clientset.ServerVersion()
for ticker, start := time.NewTicker(1 * time.Second).C, time.Now(); time.Since(start) < time.Minute * 1; {
<-ticker
serverVersion, err = clientset.ServerVersion()
if err == nil {
break
}
}
if err != nil {
log.Logf("Error: failed to retrieving kubernetes version")
panic(err.Error)
Expand Down
17 changes: 10 additions & 7 deletions npm/pod.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,16 @@ func (npMgr *NetworkPolicyManager) AddPod(podObj *corev1.Pod) error {

// Add the pod to ipset
ipsMgr := npMgr.nsMap[util.KubeAllNamespacesFlag].ipsMgr

// Add pod namespace if it doesn't exist
if _, exists := npMgr.nsMap[podNs]; !exists {
log.Printf("Creating set: %v, hashedSet: %v", podNs, util.GetHashedName(podNs))
if err = ipsMgr.CreateSet(podNs); err != nil {
log.Printf("Error creating ipset %s", podNs)
return err
}
}

// Add the pod to its namespace's ipset.
log.Printf("Adding pod %s to ipset %s", podIP, podNs)
if err = ipsMgr.AddToSet(podNs, podIP); err != nil {
Expand All @@ -60,13 +70,6 @@ func (npMgr *NetworkPolicyManager) AddPod(podObj *corev1.Pod) error {
}
}

ns, err := newNs(podNs)
if err != nil {
log.Errorf("Error: failed to create namespace %s", podNs)
return err
}
npMgr.nsMap[podNs] = ns

return nil
}

Expand Down
15 changes: 3 additions & 12 deletions npm/translatePolicy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,13 +155,8 @@ func translateIngress(ns string, targetSelector metav1.LabelSelector, rules []ne

labelsWithOps, _, _ := parseSelector(&targetSelector)
ops, labels := GetOperatorsAndLabels(labelsWithOps)
if len(ops) == 1 && len(labels) == 1 {
if ops[0] == "" && labels[0] == "" {
// targetSelector is empty. Select all pods within the namespace
labels[0] = "ns-" + ns
}
}
sets = append(sets, labels...)
sets = append(sets, "ns-" + ns)

targetSelectorIptEntrySpec := craftPartialIptEntrySpecFromOpsAndLabels(ns, ops, labels, util.IptablesDstFlag, false)
targetSelectorComment := craftPartialIptablesCommentFromSelector(ns, &targetSelector, false)
Expand Down Expand Up @@ -643,13 +638,9 @@ func translateEgress(ns string, targetSelector metav1.LabelSelector, rules []net

labelsWithOps, _, _ := parseSelector(&targetSelector)
ops, labels := GetOperatorsAndLabels(labelsWithOps)
if len(ops) == 1 && len(labels) == 1 {
if ops[0] == "" && labels[0] == "" {
// targetSelector is empty. Select all pods within the namespace
labels[0] = "ns-" + ns
}
}
sets = append(sets, labels...)
sets = append(sets, "ns-" + ns)

targetSelectorIptEntrySpec := craftPartialIptEntrySpecFromOpsAndLabels(ns, ops, labels, util.IptablesSrcFlag, false)
targetSelectorComment := craftPartialIptablesCommentFromSelector(ns, &targetSelector, false)
for _, rule := range rules {
Expand Down
17 changes: 17 additions & 0 deletions npm/translatePolicy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -607,6 +607,7 @@ func TestTranslateIngress(t *testing.T) {
expectedSets := []string{
"context:dev",
"testNotIn:frontend",
"ns-testnamespace",
"app:db",
"testIn:frontend",
"region:northpole",
Expand Down Expand Up @@ -884,6 +885,7 @@ func TestTranslateEgress(t *testing.T) {
expectedSets := []string{
"context:dev",
"testNotIn:frontend",
"ns-testnamespace",
"app:db",
"testIn:frontend",
"region:northpole",
Expand Down Expand Up @@ -1139,6 +1141,7 @@ func TestTranslatePolicy(t *testing.T) {

expectedSets = []string{
"app:backend",
"ns-testnamespace",
"app:frontend",
}
if !reflect.DeepEqual(sets, expectedSets) {
Expand Down Expand Up @@ -1263,6 +1266,7 @@ func TestTranslatePolicy(t *testing.T) {

expectedSets = []string{
"app:frontend",
"ns-testnamespace",
}
if !reflect.DeepEqual(sets, expectedSets) {
t.Errorf("translatedPolicy failed @ ALLOW-all-TO-app:frontend-FROM-all-namespaces-policy sets comparison")
Expand Down Expand Up @@ -1337,6 +1341,7 @@ func TestTranslatePolicy(t *testing.T) {

expectedSets = []string{
"app:frontend",
"ns-testnamespace",
}
if !reflect.DeepEqual(sets, expectedSets) {
t.Errorf("translatedPolicy failed @ ALLOW-none-TO-app:frontend-policy sets comparison")
Expand Down Expand Up @@ -1521,6 +1526,7 @@ func TestTranslatePolicy(t *testing.T) {
sets, lists, iptEntries = translatePolicy(allowAllNsToFrontendPolicy)
expectedSets = []string{
"app:frontend",
"ns-testnamespace",
}
if !reflect.DeepEqual(sets, expectedSets) {
t.Errorf("translatedPolicy failed @ ALLOW-all-namespaces-TO-app:frontend-policy sets comparison")
Expand Down Expand Up @@ -1666,6 +1672,7 @@ func TestTranslatePolicy(t *testing.T) {

expectedSets = []string{
"app:frontend",
"ns-testnamespace",
}
if !reflect.DeepEqual(sets, expectedSets) {
t.Errorf("translatedPolicy failed @ ALLOW-ns-namespace:dev-AND-!ns-namespace:test0-AND-!ns-namespace:test1-TO-app:frontend-policy sets comparison")
Expand Down Expand Up @@ -1827,6 +1834,7 @@ func TestTranslatePolicy(t *testing.T) {
"k0",
"k1:v0",
"k1:v1",
"ns-testnamespace",
}
if !reflect.DeepEqual(sets, expectedSets) {
t.Errorf("translatedPolicy failed @ AllOW-ALL-TO-k0-AND-k1:v0-AND-k1:v1-AND-app:frontend-policy sets comparison")
Expand Down Expand Up @@ -2031,6 +2039,7 @@ func TestTranslatePolicy(t *testing.T) {

expectedSets = []string{
"app:frontend",
"ns-testnamespace",
"app:backend",
}
if !reflect.DeepEqual(sets, expectedSets) {
Expand Down Expand Up @@ -2165,6 +2174,7 @@ func TestTranslatePolicy(t *testing.T) {

expectedSets = []string{
"app:backdoor",
"ns-dangerous",
}
if !reflect.DeepEqual(sets, expectedSets) {
t.Errorf("translatedPolicy failed @ ALLOW-ALL-TO-app:backdoor-policy sets comparison")
Expand Down Expand Up @@ -2251,6 +2261,7 @@ func TestTranslatePolicy(t *testing.T) {

expectedSets = []string{
"app:frontend",
"ns-testnamespace",
"app:backend",
}
if !reflect.DeepEqual(sets, expectedSets) {
Expand Down Expand Up @@ -2381,6 +2392,7 @@ func TestTranslatePolicy(t *testing.T) {
expectedSets = []string{
"app:k8s",
"team:aks",
"ns-acn",
"program:cni",
"team:acn",
"binary:cns",
Expand Down Expand Up @@ -2561,6 +2573,7 @@ func TestTranslatePolicy(t *testing.T) {

expectedSets = []string{
"app:backend",
"ns-testnamespace",
}
if !reflect.DeepEqual(sets, expectedSets) {
t.Errorf("translatedPolicy failed @ ALLOW-none-FROM-app:backend-policy sets comparison")
Expand Down Expand Up @@ -2612,6 +2625,7 @@ func TestTranslatePolicy(t *testing.T) {

expectedSets = []string{
"app:backend",
"ns-testnamespace",
}
if !reflect.DeepEqual(sets, expectedSets) {
t.Errorf("translatedPolicy failed @ ALLOW-all-FROM-app:backend-policy sets comparison")
Expand Down Expand Up @@ -2752,6 +2766,7 @@ func TestTranslatePolicy(t *testing.T) {

expectedSets = []string{
"app:frontend",
"ns-testnamespace",
}
if !reflect.DeepEqual(sets, expectedSets) {
t.Errorf("translatedPolicy failed @ ALLOW-ALL-FROM-app:frontend-TCP-PORT-53-OR-UDP-PORT-53-policy sets comparison")
Expand Down Expand Up @@ -2969,6 +2984,7 @@ func TestTranslatePolicy(t *testing.T) {

expectedSets = []string{
"role:db",
"ns-default",
"role:frontend",
}
if !reflect.DeepEqual(sets, expectedSets) {
Expand Down Expand Up @@ -3303,6 +3319,7 @@ func TestDropPrecedenceOverAllow(t *testing.T) {
expectedSets = []string{
"app:test",
"testIn:pod-A",
"ns-default",
"testIn:pod-B",
"testIn:pod-C",
}
Expand Down