Adds HumioUsers CRD #711
Adds HumioUsers CRD #711
Conversation
schofield
force-pushed
the
grant/issue-635
branch
3 times, most recently
from
584db1a
to
0f5512e
Compare
schofield
force-pushed
the
grant/issue-635
branch
from
0f5512e
to
1063fdd
Compare
|
I fixed the last remaining bug. This is ready for review. |
api/v1alpha1/humiouser_types.go
Outdated
| Picture string `json:"picture,omitempty"` | ||
| // IsRoot is the root setting for the user | ||
| IsRoot bool `json:"isRoot,omitempty"` | ||
| // CreatedAt is date when the user was created |
There was a problem hiding this comment.
Do we need this as part of the spec? I can't think of a reason one would want to set this.
There was a problem hiding this comment.
I have seen people use it and it is configurable so I think it should be in the spec.
There was a problem hiding this comment.
What controls will there be around the IsRoot field? Could anyone push a YAML doc to the API server and give themselves root privileges?
There was a problem hiding this comment.
I agree that CreatedAt does seem very strange to add (at least to the Spec of the CRD). The go client involved using Users().Add(...) doesn't use it (same for Users().Update(...). The only use of it that I see is to reflect back the CreatedAt timestamp for a user as for when it was created. This does not make sense in HumioUsers.Spec though. If we really want to ensure our k8s CR's store that data, then we can store it in HumioUsers.Status if we really have to, but I'm not sure we really need it. It definitely doesn't belong in the Spec though, since users cannot specify that themselves, but is given by the user upon user-creation.
There was a problem hiding this comment.
That makes sense, I'll remove it.
There was a problem hiding this comment.
What controls will there be around the
IsRootfield? Could anyone push a YAML doc to the API server and give themselves root privileges?
Any user that has the ability to create the resource could change the attribute yes but they could also change the cluster resource and change the authentication.
schofield
force-pushed
the
grant/issue-635
branch
from
1063fdd
to
e917e93
Compare
schofield
force-pushed
the
grant/issue-635
branch
from
e917e93
to
8b9bf0e
Compare
api/v1alpha1/humiouser_types.go
Outdated
| Picture string `json:"picture,omitempty"` | ||
| // IsRoot is the root setting for the user | ||
| IsRoot bool `json:"isRoot,omitempty"` | ||
| // CreatedAt is date when the user was created |
There was a problem hiding this comment.
I agree that CreatedAt does seem very strange to add (at least to the Spec of the CRD). The go client involved using Users().Add(...) doesn't use it (same for Users().Update(...). The only use of it that I see is to reflect back the CreatedAt timestamp for a user as for when it was created. This does not make sense in HumioUsers.Spec though. If we really want to ensure our k8s CR's store that data, then we can store it in HumioUsers.Status if we really have to, but I'm not sure we really need it. It definitely doesn't belong in the Spec though, since users cannot specify that themselves, but is given by the user upon user-creation.
Co-authored-by: Mike Rostermund <[email protected]>
Co-authored-by: Mike Rostermund <[email protected]>
Co-authored-by: Mike Rostermund <[email protected]>
Co-authored-by: Mike Rostermund <[email protected]>
Co-authored-by: Mike Rostermund <[email protected]>
Currently a work in progress, soliciting feedback.