Conversation
libs/api/__generated__/Api.ts
Outdated
| /** Mapping of which Fleet roles are conferred by each Silo role | ||
|
|
||
| The default is that no Fleet roles are conferred by any Silo roles unless there's a corresponding entry in this map. */ | ||
| mappedFleetRoles?: {} |
There was a problem hiding this comment.
This is not correct. Need to fix the generator and/or the OpenAPI spec.
|
@benjaminleonard @paryhin This is going to need kind of an interesting control in the form. The API post looks like this: mapped_fleet_roles: {
viewer: [],
collaborator: [],
admin: ['admin'],
}The key is the fleet role, and the list is the silo roles that give you that fleet role. So in this example, we're saying
However, that is almost certainly the wrong way to do it because the vast majority of the mapped_fleet_roles: {
viewer: ['admin'],
collaborator: ['admin'],
admin: ['admin'],
}
mapped_fleet_roles: {
viewer: ['admin'],
collaborator: [],
admin: ['admin'],
}
mapped_fleet_roles: {
viewer: [],
collaborator: ['admin'],
admin: ['admin'],
}A better approach would be to pick a much smaller number of options (maybe as small as 3 or 6 or 9) that make sense and provide a UI for picking among those. We would treat the fact that the underlying representation actually has 512 possible states as an implementation detail. The most obvious option is Thinking about the above example of states that don't make sense, you can extrapolate a rule that says it only makes sense to check at most one cell in a given column, because if you ever have multiple checks in a column, it's equivalent to only checking the bottom one.
This rule cuts the number of states substantially — each column has 4 states (off + 3 roles) and there are 3 of them, so we have
In this case the rule would be something like This one also probably doesn't make sense (because all silo viewers and collaborators are also viewers)
This implies a rule of thumb Note that because you can also create a silo through the API and CLI, on the silo view side we'd still have to make all 512 possible states displayable, maybe with a grid like above. @davepacheco when writing the feature, did you have in mind a few configurations that would be most likely to be used? |
|
@askfongjojo might want to weigh in, but the configurations I'd expect are, in decreasing order of importance:
Only the first of these seems really important right now. I expect much of this could change with user feedback. As you said, many combinations don't make a lot of sense (e.g., |
|
Ok, great. So I'm thinking we can start with either:
|
|
design HEEEEEELP 
|
|
I think two checkboxes are fine for now, but let's decrease the spacing between them. Also, let's make the text more user-friendly, perhaps something like: Grant fleet admin permissions to a silo admin. / Grant fleet viewer permissions to a silo viewer. |
|
The two options mocked up seem adequate for now. We can always add more based on field feedback. The applications of the fleet roles I can think of, supporting what @davepacheco already mentioned, are:
Down the road, the bigger needs might actually be splitting the |
|
That's better. It still feels a little lonely down there. Maybe we could use more text or some kind of heading that groups these checkboxes together with the admin group name field under the category of "authz stuff". But it's tolerable as-is IMO. 
Displaying in silo detailThere is no silo detail! Right now the silo link goes to IDPs list. Soon there will be an update endpoint for silos (oxidecomputer/omicron#3400), which means there will be a need for an edit form. So I'm thinking the silo link should go to a detail page or an edit form instead, and there we can show the state of the two checkboxes (or, like I said above, we probably have to show the entire roles-to-roles map).
|
david-crespo
changed the title
david-crespo
changed the title
david-crespo
force-pushed
the
fleet-role-map
branch
from
10e1d2c to
7d45c82
Compare
|
Hey @benjaminleonard @paryhin, check out this horrible thing I did! Like I said at the end here, because you can still set this mapping arbitrarily through the API and CLI, we might have to display the full mapping rather than this condensed version. 
|
|
Obviously this is too large but this would be a more precise way to show it. Ew. After thinking more about it, I think the best thing would be to default to showing the two checkboxes-ish display that matches the form, and then we can detect whether we also need a "show more" button that shows this table. Or maybe we always have the button that lets you see the full mapping. 
|
|
I thought about it for 3 hours and came up with a much less offensive idea: list the pairs. This is nice because it scales trivially to include "non-standard" combinations set through the API without making the standard case uglier. 
|
| export function VpcPage() { | ||
| const vpcSelector = useVpcSelector() | ||
| const { data: vpc } = useApiQuery('vpcView', toPathQuery('vpc', vpcSelector)) | ||
| invariant(vpc, 'VPC must be prefetched in loader') |
There was a problem hiding this comment.
unrelated, just noticed this was missing
|
Unfortunately, the Design
What I have
|
4 participants
Uh oh!
There was an error while loading. Please reload this page.