Skip to content

Commit

Permalink
Address CRs
Browse files Browse the repository at this point in the history
  • Loading branch information
kimlisa committed Oct 23, 2024
1 parent efa1921 commit c7802cb
Show file tree
Hide file tree
Showing 5 changed files with 117 additions and 122 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import { Option } from 'shared/components/Select';
import { FieldCheckbox } from 'shared/components/FieldCheckbox';
import { mergeRefs } from 'shared/libs/mergeRefs';
import { RequestableResourceKind } from 'shared/components/AccessRequests/NewRequest/resource';
import { TextSelectCopyMulti } from 'shared/components/TextSelectCopy';

import { CreateRequest } from '../../Shared/types';
import { AssumeStartTime } from '../../AssumeStartTime/AssumeStartTime';
Expand All @@ -66,8 +67,6 @@ import { CrossIcon } from './CrossIcon';

import type { TransitionStatus } from 'react-transition-group';
import type { AccessRequest } from 'shared/services/accessRequests';
import { P } from 'design/Text/Text';
import { TextSelectCopyMulti } from 'shared/components/TextSelectCopy';

export const RequestCheckoutWithSlider = forwardRef<
HTMLDivElement,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,123 +114,126 @@ export function AccessRequestCheckout() {
setShowCheckout(false);
}

const filteredData = pendingAccessRequests.filter(
d => !isKubeClusterWithNamespaces(d, pendingAccessRequests)
);
const pendingAccessRequestsWithoutParentResource =
pendingAccessRequests.filter(
d => !isKubeClusterWithNamespaces(d, pendingAccessRequests)
);

const numAddedResources = filteredData.length;
const numAddedResources = pendingAccessRequestsWithoutParentResource.length;

// We should rather detect how much space we have,
// but for simplicity we only count items.
const moreToShow = Math.max(
filteredData.length - MAX_RESOURCES_IN_BAR_TO_SHOW,
pendingAccessRequestsWithoutParentResource.length -
MAX_RESOURCES_IN_BAR_TO_SHOW,
0
);
return (
<>
{filteredData.length > 0 && !isCollapsed() && (
<Box
px={3}
py={2}
css={`
border-top: 1px solid
${props => props.theme.colors.spotBackground[1]};
`}
>
<Flex
justifyContent="space-between"
alignItems="center"
{pendingAccessRequestsWithoutParentResource.length > 0 &&
!isCollapsed() && (
<Box
px={3}
py={2}
css={`
gap: ${props => props.theme.space[1]}px;
border-top: 1px solid
${props => props.theme.colors.spotBackground[1]};
`}
>
<Flex flexDirection="column" minWidth={0}>
<Text mb={1}>
{numAddedResources}{' '}
{pluralize(
numAddedResources,
isRoleRequest ? 'role' : 'resource'
)}{' '}
added to access request:
</Text>
<Flex gap={1} flexWrap="wrap">
{filteredData
.slice(0, MAX_RESOURCES_IN_BAR_TO_SHOW)
.map(c => {
let resource = {
name: c.subResourceName
? `${c.id}/${c.subResourceName}`
: c.name,
key: `${c.clusterName}-${c.kind}-${c.id}-${c.subResourceName}`,
Icon: undefined,
};
switch (c.kind) {
case 'app':
case 'saml_idp_service_provider':
resource.Icon = Icon.Application;
break;
case 'node':
resource.Icon = Icon.Server;
break;
case 'db':
resource.Icon = Icon.Database;
break;
case 'kube_cluster':
case 'namespace':
resource.Icon = Icon.Kubernetes;
break;
case 'role':
break;
default:
c satisfies never;
}
return resource;
})
.map(c => (
<Label
kind="secondary"
key={c.key}
css={`
display: flex;
align-items: center;
min-width: 0;
gap: ${props => props.theme.space[1]}px;
`}
>
{c.Icon && <c.Icon size={15} />}
<span
<Flex
justifyContent="space-between"
alignItems="center"
css={`
gap: ${props => props.theme.space[1]}px;
`}
>
<Flex flexDirection="column" minWidth={0}>
<Text mb={1}>
{numAddedResources}{' '}
{pluralize(
numAddedResources,
isRoleRequest ? 'role' : 'resource'
)}{' '}
added to access request:
</Text>
<Flex gap={1} flexWrap="wrap">
{pendingAccessRequestsWithoutParentResource
.slice(0, MAX_RESOURCES_IN_BAR_TO_SHOW)
.map(c => {
let resource = {
name: c.subResourceName
? `${c.id}/${c.subResourceName}`
: c.name,
key: `${c.clusterName}-${c.kind}-${c.id}-${c.subResourceName}`,
Icon: undefined,
};
switch (c.kind) {
case 'app':
case 'saml_idp_service_provider':
resource.Icon = Icon.Application;
break;
case 'node':
resource.Icon = Icon.Server;
break;
case 'db':
resource.Icon = Icon.Database;
break;
case 'kube_cluster':
case 'namespace':
resource.Icon = Icon.Kubernetes;
break;
case 'role':
break;
default:
c satisfies never;
}
return resource;
})
.map(c => (
<Label
kind="secondary"
key={c.key}
css={`
text-overflow: ellipsis;
white-space: nowrap;
overflow: hidden;
display: flex;
align-items: center;
min-width: 0;
gap: ${props => props.theme.space[1]}px;
`}
>
{c.name}
</span>
</Label>
))}
{!!moreToShow && (
<Label kind="secondary">+ {moreToShow} more</Label>
)}
{c.Icon && <c.Icon size={15} />}
<span
css={`
text-overflow: ellipsis;
white-space: nowrap;
overflow: hidden;
`}
>
{c.name}
</span>
</Label>
))}
{!!moreToShow && (
<Label kind="secondary">+ {moreToShow} more</Label>
)}
</Flex>
</Flex>
<Flex gap={3}>
<ButtonPrimary
onClick={() => setShowCheckout(!showCheckout)}
textTransform="none"
css={`
white-space: nowrap;
`}
>
Proceed to request
</ButtonPrimary>
<ButtonIcon onClick={collapseBar}>
<Icon.ChevronDown size="medium" />
</ButtonIcon>
</Flex>
</Flex>
<Flex gap={3}>
<ButtonPrimary
onClick={() => setShowCheckout(!showCheckout)}
textTransform="none"
css={`
white-space: nowrap;
`}
>
Proceed to request
</ButtonPrimary>
<ButtonIcon onClick={collapseBar}>
<Icon.ChevronDown size="medium" />
</ButtonIcon>
</Flex>
</Flex>
</Box>
)}
</Box>
)}
{assumedRequests.map(request => (
<AssumedRolesBar key={request.id} assumedRolesRequest={request} />
))}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ test('fetching requestable roles for servers uses UUID, not hostname', async ()
);
});

test('fetching requestable roles kube_cluster resource without namespace request', async () => {
test('fetching requestable roles for a kube_cluster resource without specifying a namespace', async () => {
const kube = makeKube();
const cluster = makeRootCluster();
const appContext = new MockAppContext();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,10 +163,6 @@ export default function useAccessRequestCheckout() {
/**
*
* @param pendingRequest holds a list or map of resources to process
* @param excludeSubResourceParentResource when true, resources that have
* subresources, will be excluded from the returned list. eg:
* if a kube_cluster resource has a list of namespaces (subresources),
* then if this flag is true, kube_cluster will be excluded from the result.
*/
function getPendingAccessRequestsPerResource(
pendingRequest: PendingAccessRequest
Expand Down Expand Up @@ -293,19 +289,11 @@ export default function useAccessRequestCheckout() {
resourceIds: pendingAccessRequestsWithoutParentResource
.filter(d => d.kind !== 'role')
.map(d => {
if (d.kind === 'namespace') {
return {
name: d.id,
kind: d.kind,
clusterName: d.clusterName,
subResourceName: d.subResourceName,
};
}
return {
name: d.id,
clusterName: d.clusterName,
kind: d.kind,
subResourceName: '',
subResourceName: d.subResourceName || '',
};
}),
roles: pendingAccessRequestsWithoutParentResource
Expand Down Expand Up @@ -345,19 +333,19 @@ export default function useAccessRequestCheckout() {
async function performDryRun() {
let teletermAccessRequest: TeletermAccessRequest;

// reset any create attempts between dry runs.
setCreateRequestAttempt({ status: '' });

try {
const { accessRequest } = await prepareAndCreateRequest({
dryRun: true,
maxDuration: getDryRunMaxDuration(),
});
teletermAccessRequest = accessRequest;
} catch {
setCreateRequestAttempt({ status: '' });
return;
}

setCreateRequestAttempt({ status: '' });

const accessRequest = makeUiAccessRequest(teletermAccessRequest);
onDryRunChange(accessRequest);
}
Expand Down Expand Up @@ -440,7 +428,7 @@ export default function useAccessRequestCheckout() {
isCollapsed,
assumedRequests: getAssumedRequests(),
toggleResource,
data: pendingAccessRequests,
pendingAccessRequests,
shouldShowClusterNameColumn,
createRequest,
reset,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,9 +211,14 @@ export default function useNewRequest(rootCluster: Cluster) {
return;
}

/**
* This should never happen but just a safeguard.
* This function is used in the "unified resources" view,
* where a user can click on a "request access" button.
* Selecting kube_cluster's namespace is not available in this view
* (instead it is rendered in the "request checkout" view).
*/
if (kind === 'namespace') {
// It is not possible to request a kube namespace through this function.
// The type should be corrected.
return;
}

Expand Down

0 comments on commit c7802cb

Please sign in to comment.