[release-0.4] fix pod alerts (take 2) (#138)

* fix pod alerts

* fix review wording comments

* fix e2e - wait for guard-service as well

* fix e2e - wait also after setting TLS

* fix e2e - typo nit in comment

* fix e2e ready

Co-authored-by: David Hadas <david.hadas@gmail.com>

Author: knative-prow-robot

.github/workflows/e2e.yaml

@@ -71,6 +71,7 @@ jobs:
       run: |
         echo Waiting for Pods to become ready.
         kubectl wait pod --timeout 300s --for=condition=Ready -n knative-serving -l "app.kubernetes.io/name=knative-serving"
+        kubectl wait pod --timeout 300s --for=condition=Ready -n knative-serving -l "app=guard-service"
         # For debugging.
         kubectl get pods --all-namespaces
 
@@ -113,6 +114,13 @@ jobs:
         URL=`kn service list|head -2|tail -1|awk '{print $2}'`   
         echo "SERVICE_URL=$URL" >> $GITHUB_ENV
 
+    - name: Wait for Ready2
+      run: |
+        echo Waiting for Pods to become ready.
+        kubectl wait pod --timeout 300s --for=condition=Ready -n knative-serving -l "app=guard-service"
+        # For debugging.
+        kubectl get pods --all-namespaces
+
     - name: Run e2e Tests With TLS
       run: |
         ./test/e2e/e2e-tests.sh $SERVICE_URL "httptest2"

pkg/apis/guard/v1alpha1/pod.go

@@ -115,10 +115,17 @@ func IpNetFromProc(protocol string) (ips []net.IP) {
 		return
 	}
 
+	ipMap := make(map[string]bool)
 	ips = make([]net.IP, 0)
+
 	ip, data := nextRemoteIp(data)
 	for data != nil {
-		ips = append(ips, ip)
+		ipStr := ip.String()
+		if _, ok := ipMap[ipStr]; !ok {
+			// New IP address
+			ipMap[ipStr] = true
+			ips = append(ips, ip)
+		}
 		ip, data = nextRemoteIp(data)
 	}
 	return ips

pkg/guard-gate/gate.go

@@ -78,6 +78,7 @@ func (p *plug) ApproveRequest(req *http.Request) (*http.Request, error) {
 
 	if p.gateState.shouldBlock() && (s.hasAlert() || p.gateState.hasAlert()) {
 		p.gateState.addStat("BlockOnRequest")
+		pi.Log.Debugf("Request blocked")
 		cancelFunction()
 		return nil, errSecurity
 	}
@@ -107,8 +108,9 @@ func (p *plug) ApproveResponse(req *http.Request, resp *http.Response) (*http.Re
 	s.screenResponseBody(resp)
 	s.screenEnvelop()
 	if p.gateState.shouldBlock() && (s.hasAlert() || p.gateState.hasAlert()) {
-		s.cancel()
 		p.gateState.addStat("BlockOnResponse")
+		pi.Log.Debugf("Response blocked")
+		s.cancel()
 		return nil, errSecurity
 	}
 

pkg/guard-gate/session.go

@@ -96,7 +96,7 @@ func (s *session) sessionEventLoop(ctx context.Context) {
 
 		// Should we alert?
 		if s.gateState.hasAlert() {
-			logAlert("Pod has an alert")
+			s.gateState.logAlert()
 			s.gateState.addStat("BlockOnPod")
 			return
 		}
@@ -133,8 +133,8 @@ func (s *session) sessionEventLoop(ctx context.Context) {
 			s.screenEnvelop()
 			s.screenPod()
 			if s.gateState.shouldBlock() && (s.hasAlert() || s.gateState.hasAlert()) {
+				pi.Log.Debugf("Request processing canceled during sessionTicker")
 				s.cancel()
-				pi.Log.Debugf("Session Canceled")
 				return
 			}
 			pi.Log.Debugf("Session Tick")

pkg/guard-gate/state.go

@@ -40,6 +40,7 @@ type gateState struct {
 	pod        spec.PodProfile         // pod profile
 	srv        *gateClient             // maintainer of the pile, include client to the guard-service & kubeApi
 	certPool   *x509.CertPool          // rootCAs
+	prevAlert  string                  // previous gate alert
 }
 
 func (gs *gateState) init(cancelFunc context.CancelFunc, monitorPod bool, guardServiceUrl string, sid string, ns string, useCm bool) {
@@ -96,7 +97,7 @@ func (gs *gateState) loadConfig() {
 	}
 	criteria.Prepare()
 	gs.criteria = criteria
-	pi.Log.Infof("Loading Guardian  - Active %t Auto %t", gs.criteria.Active, gs.ctrl.Auto)
+	pi.Log.Infof("Loading Guardian  - Active %t Auto %t Block %t", gs.criteria.Active, gs.ctrl.Auto, gs.ctrl.Block)
 }
 
 // flushPile is called periodically to send the pile to the guard-service
@@ -131,14 +132,24 @@ func (gs *gateState) profileAndDecidePod() {
 		if decision != nil {
 			gs.addStat("PodAlert")
 			gs.alert = decision.String("Pod  -> ")
-
-			logAlert(gs.alert)
-			// terminate the reverse proxy
-			gs.cancelFunc()
+			gs.logAlert()
+			if gs.shouldBlock() {
+				// Terminate the reverse proxy since all requests will block from now on
+				pi.Log.Infof("Terminating")
+				gs.cancelFunc()
+			}
 		}
 	}
 }
 
+func (gs *gateState) logAlert() {
+	if gs.prevAlert == gs.alert {
+		return
+	}
+	gs.prevAlert = gs.alert
+	logAlert(gs.alert)
+}
+
 // if pod is monitored, copy its profile to the session profile
 func (gs *gateState) copyPodProfile(pp *spec.PodProfile) {
 	if !gs.monitorPod {