Skip to content
This repository has been archived by the owner on Oct 30, 2024. It is now read-only.

Autofix Network Policy #155

Merged
merged 15 commits into from
Feb 22, 2019
Merged

Autofix Network Policy #155

merged 15 commits into from
Feb 22, 2019

Conversation

nschhina
Copy link
Contributor

@nschhina nschhina commented Feb 13, 2019

Description

Add autofix support for Network Policy auditing

For 🎩 try-

kubeaudit autofix -f  fixtures/namespace_missing_default_deny_netpol.yml

Add default egress/ingress deny network policy to namespace

#159 to be fixed later

Fixes (Autofix for Network Policies #156)

closes #156

Type of change
  • Bug fix 🐛
  • New feature ✨
How Has This Been Tested?
  • TestAllResourcesFixV1
  • TestExtraResourcesFixV1
  • TestExtraResourcesIngressFixV1
  • TestFixV1Beta2
  • TestFixV1Beta1
  • TestFixV1
Checklist:
  • I have 🎩 my changes (A 🎩 specifically includes pulling down changes, setting them up, and manually testing the changed features and potential side effects to make sure nothing is broken)
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • New and existing unit tests pass locally with my changes
  • The test coverage did not decrease

@nschhina nschhina changed the title Autofixnetwork policy Autofix Network Policy Feb 13, 2019
@nschhina nschhina added the WIP label Feb 13, 2019
Copy link
Contributor

@klautcomputing klautcomputing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have 2 nitpicks and the issue that comments are deleted here in fixtures/namespace_missing_default_deny_netpol.yml

-    - from:
-      - namespaceSelector:     # chooses all pods in namespaces labelled with team=operations
-          matchLabels:
-            team: operations
-        podSelector:           # chooses pods with type=monitoring
-          matchLabels:
-            type: monitoring
+  - from:
+    - namespaceSelector:
+        matchLabels:
+          team: operations
+      podSelector:
+        matchLabels:
+          type: monitoring

I am fine with fixing this issue in a different PR since it's about comments and not about autofixing NPs

@@ -159,6 +159,21 @@ func compareTextFiles(file1, file2 string) bool {
return false
}

f1stat, err := f1.Stat()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is this doesn't give you any idea where they diff. The other code gave you a diff, this doesn't anymore.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've moved the stat code to after the scan loop, now we'll get where the files don't match as well as it'd match length of both files at the end. Do you have a better way around this? One thing I thought of was to just set s1 to be the scanner of the larger file, but anyways if files dont match and we scan doesnt output an error but stat does we can imply that files don't match in length hence files missing some content after?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not the cleanest solution, but I think given how complicated a different solution would be this is I guess the best solution.

@nschhina
Copy link
Contributor Author

I have 2 nitpicks and the issue that comments are deleted here in fixtures/namespace_missing_default_deny_netpol.yml

-    - from:
-      - namespaceSelector:     # chooses all pods in namespaces labelled with team=operations
-          matchLabels:
-            team: operations
-        podSelector:           # chooses pods with type=monitoring
-          matchLabels:
-            type: monitoring
+  - from:
+    - namespaceSelector:
+        matchLabels:
+          team: operations
+      podSelector:
+        matchLabels:
+          type: monitoring

I am fine with fixing this issue in a different PR since it's about comments and not about autofixing NPs

I am aware of this bug, I think it's just that we don't have support for namespaceSelector or from in our comment slices yet, I'll open an issue on this so I don't forget about it

Copy link
Contributor

@klautcomputing klautcomputing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's open an issue for the comment thing and then merge this :)

@nschhina nschhina merged commit 2734ba1 into master Feb 22, 2019
@nschhina nschhina deleted the autofixnetworkPolicy branch February 25, 2019 18:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Autofix for Network Policies
2 participants