Record pod events for all server calls #122
Conversation
| @@ -96,15 +96,20 @@ func (k *KiamServer) GetPodCredentials(ctx context.Context, req *pb.GetPodCreden | |||
|
|
|||
| if !decision.IsAllowed() { | |||
| logger.WithField("policy.explanation", decision.Explanation()).Errorf("pod denied by policy") | |||
| k.recordEvent(pod, v1.EventTypeWarning, "KiamRoleForbidden", | |||
There was a problem hiding this comment.
In this event we should also be able to use decision.Explanation() to include more information about why it was rejected.
pkg/server/server.go
Outdated
| return nil, err | ||
| } | ||
|
|
||
| k.recordEvent(pod, v1.EventTypeNormal, "KiamCredentialIssued", "issued credentials") |
There was a problem hiding this comment.
I'd want to check our metrics but I have a feeling this could get called a lot. Does the Event API already guard against logging the same message over and over? Don't want to do something that DDoS the API server/etcd :)
There was a problem hiding this comment.
I suppose neither of these Normal events is necessary now I think about it. The fact the pod is running without issue shows things are normal.
There was a problem hiding this comment.
I guess if it's possible to only have credentials, or an error, then its safe to remove.
One upside to include is that if people don't see any errors, or successful messages they'll know that the annotation was misspelt/missing etc.
pkg/server/server.go
Outdated
|
|
||
| logger.WithField("pod.iam.role", role).Infof("found role") | ||
| k.eventRecorder.Event(ref, v1.EventTypeNormal, "KiamRoleFound", | ||
| fmt.Sprintf("Role: %q found for pod: %q", role, pod.Name)) | ||
| k.recordEvent(pod, v1.EventTypeNormal, "KiamRoleFound", |
There was a problem hiding this comment.
As above, I'd want to know we could do this without risk of DDoS'ing the API server and etcd- I think these endpoints will get called a lot.
Codecov Report
@@ Coverage Diff @@
## master #122 +/- ##
=========================================
+ Coverage 37.42% 38.32% +0.9%
=========================================
Files 21 21
Lines 799 801 +2
=========================================
+ Hits 299 307 +8
+ Misses 472 466 -6
Partials 28 28
|
|
Nice, thanks @Sambooo. Could you update the RBAC configuration in |
|
Then I think we're in a place to merge and close #14, happy days! |
Resolves #14