Dont allow cloud tenants to update certain cluster networking config fields #28634
Conversation
|
|
||
| func (a *ServerWithRoles) validateCloudNetworkConfigUpdate(newConfig, oldConfig types.ClusterNetworkingConfig) error { | ||
| if a.hasBuiltinRole(types.RoleAdmin) { | ||
| return nil |
There was a problem hiding this comment.
I'd argue that not having permission should return an error rather than nil. How can I know if the config is updated if this function returns nil and do nothing?
| } | ||
|
|
||
| if !modules.GetModules().Features().Cloud { | ||
| return nil |
There was a problem hiding this comment.
The same as above. IMO this should be an error
There was a problem hiding this comment.
I'm not sure I understand, for non cloud instances, its okay for non admin users to to update the config. This is the same way ValidateResource in modules.go works
There was a problem hiding this comment.
My bad, this is validate not set 🤦
But in this case, if you're an admin then we don't need to validate the values? Shouldn't this and if a.hasBuiltinRole(types.RoleAdmin) { check be done last not first?
There was a problem hiding this comment.
As far as I understand from the issue (#18829 (comment)) cluster_networking_config is going to be a dynamic resource and so we'll want admin users to be able to update it without checking the rules, I think
There was a problem hiding this comment.
This looks correct to me too
lxea
force-pushed
the
lxea/cluster_networking_config-cloud
branch
from
bf576e6
to
0b81a64
Compare
| } | ||
|
|
||
| if !modules.GetModules().Features().Cloud { | ||
| return nil |
There was a problem hiding this comment.
This looks correct to me too
lxea
force-pushed
the
lxea/cluster_networking_config-cloud
branch
from
f4b95d8
to
4040404
Compare
…fields (#41197) * Dont allow cloud tenants to update certain cluster networking config fields I'm bringing #28634 back from the dead, looks like it got nuked from orbit in a bad merge conflict resolution in #27873 Dont allow cloud tenants to update certain cluster networking fields * share validate impl * add godoc comment --------- Co-authored-by: Alex McGrath <alex.mcgrath@goteleport.com>
…onfig fields (#41247) * Dont allow cloud tenants to update certain cluster networking config fields I'm bringing #28634 back from the dead, looks like it got nuked from orbit in a bad merge conflict resolution in #27873 Dont allow cloud tenants to update certain cluster networking fields * share validate impl * add godoc comment --------- Co-authored-by: Alex McGrath <alex.mcgrath@goteleport.com>
Fix for #18829
This change allows cloud tenants to editing all fields of the cluster networking config except for those specified in #18829 --
The fields can be changed if the user editing them has the Admin role type