Run CI for #8049 - #8055
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This PR implements AWS IAM authentication for MSK and adds support for multiple Kafka broker addresses via comma-separated strings in the usage and usage-ingestor services. Feedback identifies a redundant type cast in the token provider utility, a typo in the changeset file, and recommends trimming whitespace when splitting broker addresses to ensure robust environment variable parsing.
| return (async () => { | ||
| const token = await generateAuthToken({ region }); | ||
| return { value: token.token }; | ||
| }) as () => Promise<{ value: string }>; |
There was a problem hiding this comment.
The type cast as () => Promise<{ value: string }> is redundant because the async arrow function's return type is already correctly inferred by TypeScript.
| return (async () => { | |
| const token = await generateAuthToken({ region }); | |
| return { value: token.token }; | |
| }) as () => Promise<{ value: string }>; | |
| return async () => { | |
| const token = await generateAuthToken({ region }); | |
| return { value: token.token }; | |
| }; |
|
|
||
| Added opt-in AWS IAM authentication on MSK for self-hosters deployed on AWS. When enabled, the | ||
| `usage` and `usage-ingestor` services authenticate to Kafka using AWS IAM (SigV4) via the | ||
| OAUTHBEARER SASL mechanism ??? no static username/password required. |
There was a problem hiding this comment.
There is a placeholder or typo (???) in the description. Please remove it and ensure the sentence is complete.
| OAUTHBEARER SASL mechanism ??? no static username/password required. | |
| OAUTHBEARER SASL mechanism; no static username/password required. |
| const kafka = new Kafka({ | ||
| clientId: 'usage-ingestor', | ||
| brokers: [config.kafka.connection.broker], | ||
| brokers: config.kafka.connection.broker.split(','), |
There was a problem hiding this comment.
It is safer to trim the broker addresses after splitting the string, as environment variables can sometimes contain accidental leading or trailing whitespace.
| brokers: config.kafka.connection.broker.split(','), | |
| brokers: config.kafka.connection.broker.split(',').map(broker => broker.trim()), |
| const kafka = new Kafka({ | ||
| clientId: 'usage', | ||
| brokers: [config.kafka.connection.broker], | ||
| brokers: config.kafka.connection.broker.split(','), |
There was a problem hiding this comment.
It is safer to trim the broker addresses after splitting the string, as environment variables can sometimes contain accidental leading or trailing whitespace.
| brokers: config.kafka.connection.broker.split(','), | |
| brokers: config.kafka.connection.broker.split(',').map(broker => broker.trim()), |
2 participants
Pushed locally in order to run CI for #8049