-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Add an audit event reference generator #52007
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@ptgott - this PR will require admin approval to merge due to its size. Consider breaking it up into a series smaller changes. |
Amplify deployment status
|
I'd argue that the In fact, here's some recent commits that did update |
Thanks! I am planning to:
|
36c791c
to
f14b702
Compare
86ec6f6
to
01a930b
Compare
91c53fa
to
9e8f842
Compare
9e8f842
to
eb38037
Compare
eb38037
to
b590de1
Compare
7b5d43a
to
58bc991
Compare
58bc991
to
18dc744
Compare
18dc744
to
e72be95
Compare
e72be95
to
a0d8880
Compare
a0d8880
to
581b047
Compare
581b047
to
7223dd9
Compare
7223dd9
to
85b69ad
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about using TypeScript in this package? It should make maintenance way easier. I have a WIP on r7s/gen-event-ts
, I can spend some more time on it tomorrow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, so r7s/gen-event-ts
has TypeScript working, but at the end I realized there's little value in it since the stuff in the dist
directory is not typed. We could copy the types by hand based on what's in web/packages/teleport/src/Audit/fixtures/index.ts
and web/packages/teleport/src/services/audit/makeEvent.ts
.
However, the overall question is: how do we make sure that a person working on product code in web/packages/teleport
is not going to break docs/gen-event-reference
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the script was a part of the teleport package, perhaps we could skip the Vite step altogether and instead import makeEvent.ts
and the other file directly?
Then TypeScript should surface compilation errors if someone changes makeEvent.ts
in a way that's incompatible with the generator.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That sounds good to me! I've started reorganizing the code in this branch, which I've opened off of paul.gottschling/5044-audit-ts
. Would it make sense to change the new files to .ts
and execute the gen-event-reference.ts
entrypoint with tsx
to generate the reference?
@zmb3 I know you've had some concerns about locating docs generator code next to production code in relation to another project—do those concerns apply here as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to change the new files to
.ts
…
Yeah! I added some more types and commented on what still needs to be done in #55002.
…and execute the
gen-event-reference.ts
entrypoint withtsx
to generate the reference?
This is more tricky. I think there's no contraindications for having the docs gen code next to production code as long as the docs gen code doesn't bring new dependencies. 😏
So tsx and ts-node kind of out of the question. I tried to use native node --experimental-transform-types
, but the problem is that we use custom paths in tsconfig.json
which the native TypeScript support in Node.js doesn't understand.
Then I tried the tsconfig-paths package which we already use indirectly through Storybook. However, I wasn't able to get it to work.
I'll give Vite a try, this time it'd be used just to strip types from that index.ts file.
Edit: I managed to do this with Vite, more details in my draft PR (#55002).
Closes #5044 Closes #10350 One source of Teleport audit event data is the fixtures file we use for testing the audit event view within the Web UI. We usually update our test fixtures when we add an audit event, since we need to display new audit events as expected within the Web UI. This change adds a generator for the audit event reference documentation based on the test fixtures. Previous attempts to generate an audit event reference drew from the Teleport source, rather than test fixtures, using `AuditEvent` declarations (#13615) and naming conventions (#38344), but inconsistencies within the source meant that the resulting generator was inadequate. Implementation details: - Use vite to transpile the test fixture into a library that we can import into a short script. - Alphabetize reference sections by event type. - Ignore "Unknown" event descriptions, leaving these blank. - If one event type includes multiple possible codes, document each code in an H3 section. - Ignore instances of the same code beyond the first occurrence.
Responds to zmb3 feedback. When we add a new event, we need to edit the formatters in `makeEvent.ts` so the event appears in the Web UI. For any event formatter with no test fixture, include the event, type, code, and description in the reference guide using data from the formatter, but don't include an example. Also remove any test fixtures from the guide that don't correspond to a formatter. Also log events with no examples when running the generator so we can add test fixtures later on.
85b69ad
to
d41791f
Compare
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Closes #5044
Closes #10350
One source of Teleport audit event data is the fixtures file we use for testing the audit event view within the Web UI. We usually update our test fixtures when we add an audit event, since we need to display new audit events as expected within the Web UI. This change adds a generator for the audit event reference documentation based on the test fixtures.
When we add a new event, we need to edit the formatters in
makeEvent.ts
so the event appears in the Web UI. For any event formatter with no test fixture, include the event, type, code, and description in the reference guide using data from the formatter, but don't include an example. Also remove any test fixtures from the guide that don't correspond to a formatter.Also log events with no examples when running the generator so we can add test fixtures later on.
Implementation details: