DXCDT-282: Support suspended log streams #725
Conversation
test/e2e/e2e_recordings.md
Outdated
| - `access_denied: {"error":"access_denied","error_description":"Unauthorized"}` - The client ID/secret pair provided through `AUTH0_E2E_CLIENT_ID` and `AUTH0_E2E_CLIENT_SECRET` respectively is not valid, double-check their correct values | ||
| - `APIError: Nock: Disallowed net connect for "auth0-deploy-cli-e2e.us.auth0.com:443/oauth/token"` - Recordings already exist for this test, will need to remove the correlating recordings file and try again. Re-recording the entire file is necessary even if HTTP request changes are additive. |
There was a problem hiding this comment.
While re-recording the E2E tests, I came against these errors. I seem to always forget how to go through this process so any documentation is helpful to jog my memory.
| - name: Suspended DD Log Stream | ||
| sink: | ||
| datadogApiKey: some-sensitive-api-key | ||
| datadogRegion: us | ||
| type: datadog |
There was a problem hiding this comment.
The omission of status is key here. This simulates what a suspend log stream will look like when exported from Auth0 tenant to local configuration files.
| logStreams: [ | ||
| { | ||
| id: 'log-stream-1', | ||
| name: 'Splunky', | ||
| type: 'splunk', | ||
| status: 'paused', | ||
| sink: { | ||
| splunkDomain: 'test-splunk.com', | ||
| splunkPort: '8089', | ||
| splunkToken: '7b838bd0-028e-4d78-a82c-3564a2007770', | ||
| splunkSecure: false, | ||
| }, | ||
| }, | ||
| { | ||
| id: 'log-stream-2', | ||
| name: 'HTTP Log Stream', | ||
| type: 'http', | ||
| status: 'active', | ||
| sink: { | ||
| httpAuthorization: '_VALUE_NOT_SHOWN_', // secret obfuscated | ||
| httpContentFormat: 'JSONLINES', | ||
| httpContentType: 'application/json', | ||
| httpEndpoint: 'https://example.com/test', | ||
| }, | ||
| }, | ||
| { | ||
| id: 'log-stream-3', | ||
| name: 'Suspended HTTP Log Stream', | ||
| type: 'http', | ||
| // status property omitted if suspended | ||
| sink: { | ||
| httpAuthorization: '_VALUE_NOT_SHOWN_', // secret obfuscated | ||
| httpContentFormat: 'JSONLINES', | ||
| httpContentType: 'application/json', | ||
| httpEndpoint: 'https://suspended.com/logs', | ||
| }, | ||
| }, | ||
| ], |
There was a problem hiding this comment.
The difference between the output of the function and the mockLogStreams input is enough where I'd prefer to not write cryptic code to generate the expected value. Although more lines of code, it's clearer what the output is IMO.
| delete value.id; // Not expecting ID in PATCH payload | ||
| delete value.type; // Not expecting type in PATCH payload | ||
| //@ts-ignore because it's actually ok for status property to be omitted in PATCH payload | ||
| if (value?.status === 'suspended') delete value.status; // Not expecting status in PATCH payload if suspended |
There was a problem hiding this comment.
This is the major addition here. I'm not in love with this test, it's more testing the implementation than behavior. But at least it's an extra assertion to make 🤷♂️ . This is why I add it to the E2E test for more security.
Codecov ReportBase: 83.53% // Head: 83.49% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #725 +/- ##
==========================================
- Coverage 83.53% 83.49% -0.05%
==========================================
Files 114 114
Lines 3340 3344 +4
Branches 615 617 +2
==========================================
+ Hits 2790 2792 +2
- Misses 323 324 +1
- Partials 227 228 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| logStreams: { | ||
| getAll: (arg0: SharedPaginationParams) => Promise<LogStream[]>; | ||
| create: (arg0: { id: string }) => Promise<LogStream>; | ||
| update: (arg0: {}, arg1: Partial<LogStream>) => Promise<LogStream>; | ||
| delete: (arg0: Asset) => Promise<void>; | ||
| }; |
There was a problem hiding this comment.
This applies stricter type the Auth0 Management API when working with log streams. This is important because I do a lot of checking against the status field so want to make sure it's correct.
| const nonSuspendedLogStreams = logStreams.filter( | ||
| (logStream: LogStream) => logStream.status !== 'suspended' | ||
| ); |
There was a problem hiding this comment.
Removing this line allows us to unignore suspended log streams.
…to DXCDT-282-support-suspended-log-streams
…hub.com/auth0/auth0-deploy-cli into DXCDT-282-support-suspended-log-streams
Co-authored-by: Rita Zerrizuela <zeta@widcket.com>
🔧 Changes
Log streams were added with #495 which initially ignored log streams that were in a suspended status. This was mostly because the behavior of these log streams were slightly different and unknown. Ignoring these log streams caused a couple issues but most notably, if a target tenant has a log stream that has been suspended and applying configuration to it, the Deploy CLI will think that the suspended log stream does not exist and attempt to create a new one. With strict limits on the number of log streams, this is likely to be problematic.
This time around, the behavior of suspended log streams is better understood:
In addition to adding suspended log streams support and updating the tests, I expanded the documentation around HTTP recordings and some basic troubleshooting instructions.
📚 References
🔬 Testing
Added a couple of unit tests, but they're not great and mostly test the implementation. To really ensure functionality, the existing E2E tests add partial support. However, it is impossible to directly put a log stream into a suspended state, so we are not able to test this against real remote tenants.
📝 Checklist