Support more fields to the role editor #51458
Conversation
bl-nero
changed the title
bl-nero
force-pushed
the
bl-nero/role-editor-more-fields
branch
from
e946468 to
4ebf9db
Compare
bl-nero
added
the
no-changelog
| <Text typography="body3" mb={1}> | ||
| Labels | ||
| </Text> | ||
| <FieldSelectCreatable |
There was a problem hiding this comment.
the inputs placeholder says Select..., are we supposed to have a list of users to select? or maybe replace with Start typing a user and press enter?
| /> | ||
| </Box> | ||
| <LabelsInput | ||
| legend="Labels" |
There was a problem hiding this comment.
should we also add a tooltip explaining labels as well?
| /> | ||
| <LabelsInput | ||
| legend="Database Service Labels" | ||
| tooltipContent="The database service labels have no influence on which databases' access is controlled by this role. Instead, they control which database services are discoverable while enrolling a new database." |
There was a problem hiding this comment.
Instead, they control which database services are discoverable while enrolling a new database.
tbh, i didn't understand what this means 😢, can we simplify it? The ai doc says:
| @@ -112,6 +115,26 @@ const AccessRule = memo(function AccessRule({ | |||
| value={verbs} | |||
| onChange={v => setRule({ ...value, verbs: v })} | |||
| rule={precomputed(validation.fields.verbs)} | |||
| /> | |||
| <FieldInput | |||
| label="Condition" | |||
There was a problem hiding this comment.
personally i think it's okay to label it this way, but i'd also double check with product team because we try to teach users consistent wording
| Additional condition, expressed using the{' '} | ||
| <Text | ||
| as="a" | ||
| href="https://goteleport.com/docs/reference/predicate-language/" | ||
| target="_blank" | ||
| color={theme.colors.interactive.solid.accent.default} | ||
| > | ||
| Teleport predicate language | ||
| </Text> |
There was a problem hiding this comment.
what about something like: Optional condition further limiting the scope for this rule expressed using the ....
This PR adds the following fields:
whereclause in access rules. I took liberty to call it "Condition" in the UI, so please voice your opinion on whether this is a good choice. My line of reasoning is that without understanding the underlying YAML model, the name "Where" would be confusing.The intent behind this PR was to fully support the
accessrole in the rich UI, but it turned out that one more field was added to it, so we still miss the target. However, I added warnings to the Go codebase now, so hopefully, this will make it easier to prevent these situations in future.Requires #51457