Now checks for privileged system users instead of groups + Unit tests#1
Conversation
cdef51a to
b1cb810
Compare
wtripp180901
changed the title
wtripp180901
changed the title
e6b0519 to
b83f978
Compare
| // Returns true if user is a service account with correct privileges or a privileged internal K8s system user | ||
| func isPrivilegedSystemUser(user string, protectedNamespaces []string) bool { | ||
|
|
||
| systemAccountRegex, _ := regexp.Compile("system:.+") |
There was a problem hiding this comment.
I wonder if we can trim this list down somehow, it seems quite broad.
Lets get the full list from our test cluster of what we need to allow, and put that here, I think. Lets do a full sonaboy conformance test, and see what users we need in this allow list for that test to pass. Hopefully there are no arbitry ones 🤞
There was a problem hiding this comment.
sonobuoy fails trying to use its service account to create rolebindings in kube-system, but that's probably what we want?
JohnGarbutt
left a comment
JohnGarbutt
left a comment
There was a problem hiding this comment.
Looks really close thank you.
My main worry is being stricter on the systemAccountRegex.
A few formatting nits, and also add go fmt into the the CI job please.
I think we should also move to a two stage docker build, something like:
https://docs.docker.com/build/building/multi-stage/#name-your-build-stages
| deniedLogOutput = "Allowed" | ||
| } | ||
| if logLevel >= 1 && sar.Spec.NonResourceAttributes != nil { | ||
| fmt.Printf("%s non-resource request from \"%s\". Reason: %s\n", deniedLogOutput, sar.Spec.User, status.Reason) |
There was a problem hiding this comment.
Lets move this to the golang Log system, and try to use Println() to make sure they all go onto a new line:
https://pkg.go.dev/log
Its worth checking that we get what we expect to see inside Loki.
There was a problem hiding this comment.
I think the request has RemoteAddr (probably rubbish) and the Headers (e.g. X-Forwarded-For), which should tell us enough so we know which cluster has sent the request. Needs testing to see what works best inside Loki.
Key take away: be clear which cluster sent the request.
JohnGarbutt
left a comment
JohnGarbutt
left a comment
There was a problem hiding this comment.
Good work, thank you, lets cut a release with this in.
2 participants
No description provided.