Issue #340 - create application/project events for audit #440
Conversation
One thought about this: is it necessary that we dedicate one of these fields as a username? Can't it just be part of the message (e.g. "'admin' synchronized app") |
|
It make sense to me. Updated. |
server/project/project.go
Outdated
| func NewServer(ns string, appclientset appclientset.Interface, enf *rbac.Enforcer, projectLock *util.KeyLock) *Server { | ||
| return &Server{enf: enf, appclientset: appclientset, ns: ns, projectLock: projectLock} | ||
| func NewServer(ns string, kubeclientset kubernetes.Interface, appclientset appclientset.Interface, enf *rbac.Enforcer, projectLock *util.KeyLock) *Server { | ||
| auditLogger := argo.NewAuditLogger(ns, kubeclientset, "argo-server") |
There was a problem hiding this comment.
This should be argocd-server.
util/session/sessionmanager.go
Outdated
| case SessionManagerClaimsIssuer: | ||
| return mapClaims["sub"].(string) | ||
| default: | ||
| return mapClaims["email"].(string) |
There was a problem hiding this comment.
This is a potential panic. While email is a standard claims, it is not mandatory. We will panic here if it's absent.
|
Thanks for review @jessesuen . PTAL |
server/application/application.go
Outdated
| @@ -66,6 +68,7 @@ func NewServer( | |||
| appComparator: controller.NewAppStateManager(db, appclientset, repoClientset, namespace), | |||
| enf: enf, | |||
| projectLock: projectLock, | |||
| auditLogger: argo.NewAuditLogger(namespace, kubeclientset, "argo-server"), | |||
|
One more change to use argocd-server instead of argo-server. I don't think it needs to be addressed in this change, but can you file a new issue for us to use constants for things like |
PR implements following changes:
PR has one questionable change: k8s event don't have suitable field to store username. I've chosen to use
ReportingInstancefield for username. All fields seems equally bad for this purpose. Please let me know if you have better idea.