Skip to content

Commit

Permalink
Adds some checks to confirm actions, and include Update Broker from D…
Browse files Browse the repository at this point in the history
…B action
  • Loading branch information
dakota002 committed Sep 12, 2024
1 parent ec31f5f commit bc8e81f
Show file tree
Hide file tree
Showing 4 changed files with 138 additions and 62 deletions.
26 changes: 8 additions & 18 deletions app/lib/kafka.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { tables } from '@architect/functions'
import { paginateScan } from '@aws-sdk/lib-dynamodb'

Check warning on line 9 in app/lib/kafka.server.ts

View check run for this annotation

Codecov / codecov/patch

app/lib/kafka.server.ts#L8-L9

Added lines #L8 - L9 were not covered by tests
import type { DynamoDBDocument } from '@aws-sdk/lib-dynamodb'
import crypto from 'crypto'

Check warning on line 11 in app/lib/kafka.server.ts

View check run for this annotation

Codecov / codecov/patch

app/lib/kafka.server.ts#L11

Added line #L11 was not covered by tests
import type { AclFilter } from 'gcn-kafka'
import { Kafka } from 'gcn-kafka'
import type { AclEntry } from 'kafkajs'
import {

Check warning on line 15 in app/lib/kafka.server.ts

View check run for this annotation

Codecov / codecov/patch

app/lib/kafka.server.ts#L15

Added line #L15 was not covered by tests
Expand Down Expand Up @@ -81,17 +82,6 @@ if (process.env.ARC_SANDBOX) {
}
}

/**
* AclEntry already contains definitions for the following:
*
* principal: string --> 'User:{cognito_group_name}'
* host: string --> '*'
* operation: AclOperationTypes --> Read,Write, etc from enum
* permissionType: AclPermissionTypes --> Allow, Deny, etc from enum
* resourceType: AclResourceTypes --> TOPIC, etc
* resourceName: string --> name of topic: 'gcn.notices.burstcube'
* resourcePatternType: ResourcePatternTypes --> PREFIXED or LITERAL
*/
export type KafkaACL = AclEntry & {
aclId?: string
}
Expand Down Expand Up @@ -139,9 +129,9 @@ export async function createKafkaACL(
resourceName,
principal: `User:${group}`,
host: '*',
operation, // Read, write, etc
permissionType, // Allow, deny etc
resourcePatternType: 3, // LITERAL | PREFIXED
operation,
permissionType,
resourcePatternType: 3,
resourceType,
}
})
Expand All @@ -150,9 +140,9 @@ export async function createKafkaACL(
resourceName,
principal: `User:${group}`,
host: '*',
operation, // Read, write, etc
operation,
permissionType,
resourcePatternType: 3, // LITERAL | PREFIX
resourcePatternType: 3,
resourceType,
}
})
Expand Down Expand Up @@ -280,13 +270,13 @@ export async function getAclsFromBrokers() {
export async function deleteKafkaACL(user: User, aclIds: string[]) {
validateUser(user)
const db = await tables()
const acls = await Promise.all(
const acls: KafkaACL[] = await Promise.all(
aclIds.map((aclId) => db.kafka_acls.get({ aclId }))

Check warning on line 274 in app/lib/kafka.server.ts

View check run for this annotation

Codecov / codecov/patch

app/lib/kafka.server.ts#L270-L274

Added lines #L270 - L274 were not covered by tests
)

const adminClient = adminKafka.admin()
await adminClient.connect()
await adminClient.deleteAcls({ filters: acls })
await adminClient.deleteAcls({ filters: acls as AclFilter[] })
await adminClient.disconnect()

Check warning on line 280 in app/lib/kafka.server.ts

View check run for this annotation

Codecov / codecov/patch

app/lib/kafka.server.ts#L277-L280

Added lines #L277 - L280 were not covered by tests

await Promise.all(
Expand Down
147 changes: 110 additions & 37 deletions app/routes/admin.kafka._index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
ModalToggleButton,
TextInput,
} from '@trussworks/react-uswds'
import { groupBy, sortBy } from 'lodash'
import { useEffect, useRef, useState } from 'react'

Check warning on line 23 in app/routes/admin.kafka._index.tsx

View check run for this annotation

Codecov / codecov/patch

app/routes/admin.kafka._index.tsx#L22-L23

Added lines #L22 - L23 were not covered by tests

import { getUser } from './_auth/user.server'
Expand All @@ -43,7 +44,13 @@ export async function loader({ request }: LoaderFunctionArgs) {
if (!user || !user.groups.includes(adminGroup))
throw new Response(null, { status: 403 })
const { aclFilter } = Object.fromEntries(new URL(request.url).searchParams)
const dynamoDbAclData = await getKafkaACLsFromDynamoDB(user, aclFilter)
const dynamoDbAclData = groupBy(

Check warning on line 47 in app/routes/admin.kafka._index.tsx

View check run for this annotation

Codecov / codecov/patch

app/routes/admin.kafka._index.tsx#L45-L47

Added lines #L45 - L47 were not covered by tests
sortBy(await getKafkaACLsFromDynamoDB(user, aclFilter), [
'resourceName',
'principal',
]),
'resourceName'
)
const latestSync = await getLastSyncDate()
return { dynamoDbAclData, latestSync }

Check warning on line 55 in app/routes/admin.kafka._index.tsx

View check run for this annotation

Codecov / codecov/patch

app/routes/admin.kafka._index.tsx#L54-L55

Added lines #L54 - L55 were not covered by tests
}
Expand All @@ -54,6 +61,7 @@ export async function action({ request }: ActionFunctionArgs) {
throw new Response(null, { status: 403 })
const data = await request.formData()
const intent = getFormDataString(data, 'intent')

Check warning on line 63 in app/routes/admin.kafka._index.tsx

View check run for this annotation

Codecov / codecov/patch

app/routes/admin.kafka._index.tsx#L61-L63

Added lines #L61 - L63 were not covered by tests

if (intent === 'migrateFromBroker') {
await updateDbFromBrokers(user)
return null

Check warning on line 67 in app/routes/admin.kafka._index.tsx

View check run for this annotation

Codecov / codecov/patch

app/routes/admin.kafka._index.tsx#L66-L67

Added lines #L66 - L67 were not covered by tests
Expand All @@ -64,11 +72,11 @@ export async function action({ request }: ActionFunctionArgs) {
return null

Check warning on line 72 in app/routes/admin.kafka._index.tsx

View check run for this annotation

Codecov / codecov/patch

app/routes/admin.kafka._index.tsx#L71-L72

Added lines #L71 - L72 were not covered by tests
}

const aclId = getFormDataString(data, 'aclId')
const promises = []

Check warning on line 75 in app/routes/admin.kafka._index.tsx

View check run for this annotation

Codecov / codecov/patch

app/routes/admin.kafka._index.tsx#L75

Added line #L75 was not covered by tests

switch (intent) {
case 'delete':
const aclId = getFormDataString(data, 'aclId')

Check warning on line 79 in app/routes/admin.kafka._index.tsx

View check run for this annotation

Codecov / codecov/patch

app/routes/admin.kafka._index.tsx#L79

Added line #L79 was not covered by tests
if (!aclId) throw new Response(null, { status: 400 })
promises.push(deleteKafkaACL(user, [aclId]))
break

Check warning on line 82 in app/routes/admin.kafka._index.tsx

View check run for this annotation

Codecov / codecov/patch

app/routes/admin.kafka._index.tsx#L81-L82

Added lines #L81 - L82 were not covered by tests
Expand All @@ -78,8 +86,8 @@ export async function action({ request }: ActionFunctionArgs) {
data,
'userClientType'
) as UserClientType
const permissionTypeString = getFormDataString(data, 'permissionType')
const group = getFormDataString(data, 'group')
const permissionTypeString = getFormDataString(data, 'permissionType')
const includePrefixed = getFormDataString(data, 'includePrefixed')
const resourceTypeString = getFormDataString(data, 'resourceType')

Check warning on line 92 in app/routes/admin.kafka._index.tsx

View check run for this annotation

Codecov / codecov/patch

app/routes/admin.kafka._index.tsx#L89-L92

Added lines #L89 - L92 were not covered by tests

Expand Down Expand Up @@ -122,6 +130,7 @@ export default function Index() {
const updateFetcher = useFetcher<typeof action>()
const aclFetcher = useFetcher<typeof loader>()
const brokerFromDbFetcher = useFetcher()
const ref = useRef<ModalRef>(null)

Check warning on line 133 in app/routes/admin.kafka._index.tsx

View check run for this annotation

Codecov / codecov/patch

app/routes/admin.kafka._index.tsx#L127-L133

Added lines #L127 - L133 were not covered by tests

useEffect(() => {

Check warning on line 135 in app/routes/admin.kafka._index.tsx

View check run for this annotation

Codecov / codecov/patch

app/routes/admin.kafka._index.tsx#L135

Added line #L135 was not covered by tests
setAclData(aclFetcher.data?.dynamoDbAclData ?? aclData)
Expand All @@ -140,7 +149,6 @@ export default function Index() {
data from topics, create or delete topics, manage consumer groups, and
perform administrative tasks.
</p>

<updateFetcher.Form method="POST" action="/admin/kafka">
<Button
type="submit"
Expand All @@ -159,36 +167,35 @@ export default function Index() {
</span>
)}
</updateFetcher.Form>
{latestSync && (
{latestSync ? (
<p>
Last synced by {latestSync.syncedBy}{' '}
<TimeAgo time={latestSync.syncedOn} />
</p>
) : (
<br />
)}
<ModalToggleButton
opener
disabled={
updateFetcher.state !== 'idle' || brokerFromDbFetcher.state !== 'idle'
}
modalRef={ref}
type="button"
>
Update Broker from DB
</ModalToggleButton>
{brokerFromDbFetcher.state !== 'idle' && (
<span className="text-middle">
<Spinner /> Updating...
</span>
)}

<brokerFromDbFetcher.Form>
<Button
type="submit"
name="intent"
value="migrateFromDB"
disabled={
updateFetcher.state !== 'idle' ||
brokerFromDbFetcher.state !== 'idle'
}
>
Update Broker from DB
</Button>
{brokerFromDbFetcher.state !== 'idle' && (
<span className="text-middle">
<Spinner /> Updating...
</span>
)}
</brokerFromDbFetcher.Form>

{aclData && (
<>
<aclFetcher.Form method="GET">
<Label htmlFor="aclFilter">Filter ({aclData.length} results)</Label>
<Label htmlFor="aclFilter">
Filter ({Object.keys(aclData).length} results)
</Label>
<TextInput id="aclFilter" name="aclFilter" type="text" />
<Button
type="submit"
Expand All @@ -204,14 +211,50 @@ export default function Index() {
)}
</aclFetcher.Form>
<SegmentedCards>
{aclData
.sort((a, b) => a.resourceName.localeCompare(b.resourceName))
.map((x, index) => (
<KafkaAclCard key={index} acl={x} />
{Object.keys(aclData)
.sort((a, b) => a.localeCompare(b))
.flatMap((key) => (
<span key={key}>

Check warning on line 217 in app/routes/admin.kafka._index.tsx

View check run for this annotation

Codecov / codecov/patch

app/routes/admin.kafka._index.tsx#L215-L217

Added lines #L215 - L217 were not covered by tests
<h3>Resource: {key}</h3>
{aclData[key].map((acl, index) => (
<KafkaAclCard key={`${key}-${index}`} acl={acl} />

Check warning on line 220 in app/routes/admin.kafka._index.tsx

View check run for this annotation

Codecov / codecov/patch

app/routes/admin.kafka._index.tsx#L219-L220

Added lines #L219 - L220 were not covered by tests
))}
</span>
))}
</SegmentedCards>
</>
)}
<Modal
id="modal-update"
ref={ref}
aria-labelledby="modal-update-heading"
aria-describedby="modal-update-description"
renderToPortal={false} // FIXME: https://github.com/trussworks/react-uswds/pull/1890#issuecomment-1023730448
>
<brokerFromDbFetcher.Form method="POST" action="/admin/kafka">
<ModalHeading id="modal-update-heading">Confirm Update</ModalHeading>
<p id="modal-update-description">
This will affect some_number of ACLs currently defined on the
broker. If you want to maintain the ACLs defined on the brokers,
click cancel to close this window then click the Pull ACLs from
Broker button.
</p>
<p>This action cannot be undone.</p>
<ModalFooter>
<ModalToggleButton modalRef={ref} closer outline>
Cancel
</ModalToggleButton>
<Button
data-close-modal
type="submit"
name="intent"
value="migrateFromDB"
>
Confirm
</Button>
</ModalFooter>
</brokerFromDbFetcher.Form>
</Modal>
</>
)
}
Expand All @@ -221,26 +264,56 @@ function KafkaAclCard({ acl }: { acl: KafkaACL }) {
const fetcher = useFetcher()
const disabled = fetcher.state !== 'idle'

Check warning on line 265 in app/routes/admin.kafka._index.tsx

View check run for this annotation

Codecov / codecov/patch

app/routes/admin.kafka._index.tsx#L262-L265

Added lines #L262 - L265 were not covered by tests

// TODO: These maps can probably be refactored, since they are
// just inverting the enum from kafka, but importing them
// directly here causes some errors. Same for mapping them to
// dropdowns
const permissionMap: { [key: number]: string } = {

Check warning on line 271 in app/routes/admin.kafka._index.tsx

View check run for this annotation

Codecov / codecov/patch

app/routes/admin.kafka._index.tsx#L271

Added line #L271 was not covered by tests
2: 'Deny',
3: 'Allow',
}

const operationMap: { [key: number]: string } = {

Check warning on line 276 in app/routes/admin.kafka._index.tsx

View check run for this annotation

Codecov / codecov/patch

app/routes/admin.kafka._index.tsx#L276

Added line #L276 was not covered by tests
0: 'Unknown',
1: 'Any',
2: 'All',
3: 'Read',
4: 'Write',
5: 'Create',
6: 'Delete',
7: 'Alter',
8: 'Describe',
9: 'Cluster Action',
10: 'Describe Configs',
11: 'Alter Configs',
12: 'Idempotent Write',
}

const resourceTypeMap: { [key: number]: string } = {

Check warning on line 292 in app/routes/admin.kafka._index.tsx

View check run for this annotation

Codecov / codecov/patch

app/routes/admin.kafka._index.tsx#L292

Added line #L292 was not covered by tests
0: 'Unknown',
1: 'Any',
2: 'Topic',
3: 'Group',
4: 'Cluster',
5: 'Transactional Id',
6: 'Delegation Token',
}

return (

Check warning on line 302 in app/routes/admin.kafka._index.tsx

View check run for this annotation

Codecov / codecov/patch

app/routes/admin.kafka._index.tsx#L302

Added line #L302 was not covered by tests
<>
<Grid row style={disabled ? { opacity: '50%' } : undefined}>
<div className="tablet:grid-col flex-fill">
<div className="tablet:grid-col flex-fill margin-y-1">
<div>
<strong>Type:</strong> {resourceTypeMap[acl.resourceType]}
</div>
<div>
<strong>Group:</strong> {acl.principal}
</div>
{/* <div>
<strong>Client Type:</strong> {acl.userClientType}
</div> */}
<div>
<strong>Permission:</strong> {permissionMap[acl.permissionType]}
</div>
<div>
<strong>Resource:</strong> {acl.resourceName}
<strong>Operation:</strong> {operationMap[acl.operation]}
</div>
</div>
<div className="tablet:grid-col flex-auto margin-y-auto">
Expand Down Expand Up @@ -272,8 +345,8 @@ function KafkaAclCard({ acl }: { acl: KafkaACL }) {
Delete Kafka ACL
</ModalHeading>
<p id="modal-delete-description">
This will delete the DynamoDB entry and associated "read","create",
"write", and "describe" ACLs. Do you wish to continue?
This will delete the DynamoDB entry and remove the ACL from the
broker. Do you wish to continue?
</p>
<ModalFooter>
<ModalToggleButton modalRef={ref} closer outline>
Expand Down
11 changes: 8 additions & 3 deletions app/routes/admin.kafka.edit.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,20 +39,25 @@ function KafkaAclForm({ groups }: { groups: string[] }) {
<h1>Create Kafka ACLs</h1>
<Form method="POST" action="/admin/kafka">
<Label htmlFor="name">
Topic
Resource Name
<span title="required" className="usa-label--required">
*
</span>
</Label>
<TextInput
autoFocus
id="topicName"
name="topicName"
id="resourceName"
name="resourceName"
type="text"
autoCapitalize="off"
autoCorrect="off"
required
/>
<Label htmlFor="resourceType">Resource Type</Label>
<Select id="resourceType" name="resourceType">
<option value="2">Topic</option>
<option value="3">Group</option>
</Select>
<Label htmlFor="userClientType">Client Type</Label>
<Select id="userClientType" name="userClientType">
<option value="producer">Producer</option>
Expand Down
16 changes: 12 additions & 4 deletions sandbox-seed.json
Original file line number Diff line number Diff line change
Expand Up @@ -5160,16 +5160,24 @@
{
"aclId": "12345678-abcd-1234-abcd-1234abcd1234",
"resourceName": "test_topic_created_from_website",
"cognitoGroup": "gcn.nasa.gov/kafka-gcn-test-consumer",
"prefixed": false,
"permissionType": "3"
"permissionType": 3,
"operation": 3,
"host": "*",
"principal": "some-user-group",
"resourcePatternType": 3,
"resourceType": 2
},
{
"aclId": "62fe8590-42e4-4917-afea-db6a0a84079a",
"resourceName": "test_topic_created_from_website",
"cognitoGroup": "gcn.nasa.gov/kafka-gcn-test-producer",
"prefixed": false,
"permissionType": "3"
"permissionType": 3,
"operation": 4,
"host": "*",
"principal": "some-user-group",
"resourcePatternType": 3,
"resourceType": 2
}
]
}

0 comments on commit bc8e81f

Please sign in to comment.