Add ShowResources to UIConfig #41064
Conversation
github-actions
bot
requested review from
kimlisa,
marcoandredinis,
rudream and
tigrato
|
The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with |
avatus
added
the
no-changelog
avatus
force-pushed
the
avatus/uiconfig
branch
2 times, most recently
from
361d080 to
0e67d81
Compare
api/client/webclient/webconfig.go
Outdated
| // DisableIncludeRequestableRequests will turn off the option in the web UI to request resources with the "includeRequestable" flag. | ||
| // This will make resource requests NOT include the "RequiresRequest" field and thus not display both types of resources | ||
| // to users ("have access" and "requires request") in the unified resources UI. | ||
| DisableIncludeRequestableRequests bool `json:"disableIncludeRequestableRequests,omitempty"` |
There was a problem hiding this comment.
Having a bool field that you set to true to disable is counter-intuitive and often causes confusion.
WDYT about instead having a string/enum instead?
show_resources: accessible # or 'all', defaults to accessible if unset
all, might be a bit confusing, as you still have to have the ability to request them. Maybe accessible or requestable are better names.
There was a problem hiding this comment.
Whats funny is I originally had this as "Allow" with default to true but thought "no, having to configure an "allow" to false is counter intuitive". Seems like its confusing all around!
I think show_resources might be a bit too ambiguous but can probably be cleared up with a comment. It needs to state that if this is set to accessible than you won't even be able to select "all" in the dropdown.
I'll fiddle with it and resort to show_resources if I don't think of a better option.
Thank you!
There was a problem hiding this comment.
Updated to show_resources with the valid values accessible_only and requestable (default)
avatus
changed the title
tool/tctl/common/collection.go
Outdated
| t := asciitable.MakeTable([]string{"Scrollback Lines"}) | ||
| t.AddRow([]string{string(c.uiconfig.GetScrollbackLines())}) | ||
| t.AddRow([]string{string(c.uiconfig.GetShowResources())}) | ||
| _, err := t.AsBuffer().WriteTo(w) | ||
| return trace.Wrap(err) |
There was a problem hiding this comment.
| t := asciitable.MakeTable([]string{"Scrollback Lines"}) | |
| t.AddRow([]string{string(c.uiconfig.GetScrollbackLines())}) | |
| t.AddRow([]string{string(c.uiconfig.GetShowResources())}) | |
| _, err := t.AsBuffer().WriteTo(w) | |
| return trace.Wrap(err) | |
| t := asciitable.MakeTable([]string{"Scrollback Lines", "Show Resources"}) | |
| t.AddRow([]string{string(c.uiconfig.GetScrollbackLines()), string(c.uiconfig.GetShowResources())}) | |
| _, err := t.AsBuffer().WriteTo(w) | |
| return trace.Wrap(err) |
Or some other wording
But the current output doesn't seem correct to me 🤔
There was a problem hiding this comment.
Updated.
Scrollback Lines Show Resources
---------------- ---------------
499 accessible_only
api/types/ui_config.go
Outdated
| // SetShowResources will set which resources should be shown in the unified resources UI | ||
| func (c *UIConfigV1) SetShowResources(value constants.ShowResources) { | ||
| c.Spec.ShowResources = value | ||
| } |
There was a problem hiding this comment.
Is this being used?
I didn't find a reference to it.
There was a problem hiding this comment.
This method or the value? The method I added just because I've seen the get/set for the other values in ui/other configs.
There was a problem hiding this comment.
Method.
I think we can remove it, but no big deal
This PR will allow admins to disable the option in the UI to
IncludeRequestablewhen listing unified resources. The upcoming unified resources pages will by default include both "has access" and "requestable" resources. This adds a bit of overhead because each page will need to double check RBAC to add the new flag to know if a resource is requestable.The UI will have a dropdown of
With this configuration, the UI will hide the "Has Access And Requestable" option and allow only access or requestable (similar to how the requests work today) as a backdoor for any customers who notice a performance hit.