Skip to content
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

Add --ignore-https-errors flag #361

Merged
merged 12 commits into from Aug 11, 2021
Merged

Conversation

andrewvc
Copy link
Contributor

@andrewvc andrewvc commented Aug 10, 2021

Adds the ability to ignore HTTPS errors via a flag. Checks one box in elastic/beats#27202

This is adding a flag, not a capability since if a user specifies the new option we want it to fail, rather than silently behave as though it were not set.

@apmmachine
Copy link
Collaborator

apmmachine commented Aug 10, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2021-08-11T14:46:14.383+0000

  • Duration: 14 min 53 sec

  • Commit: b372e14

Test stats 🧪

Test Results
Failed 0
Passed 124
Skipped 0
Total 124

Trends 🧪

Image of Build Times

Image of Tests

@andrewvc andrewvc changed the title Add ignoreHTTPSErrors capability Add ignoreHTTPSErrors flag Aug 10, 2021
@andrewvc andrewvc self-assigned this Aug 10, 2021
@andrewvc andrewvc added the enhancement New feature or request label Aug 10, 2021
@andrewvc andrewvc marked this pull request as ready for review August 10, 2021 22:56
examples/todos/package-lock.json Outdated Show resolved Hide resolved
__tests__/cli.test.ts Outdated Show resolved Hide resolved
__tests__/cli.test.ts Show resolved Hide resolved
__tests__/cli.test.ts Outdated Show resolved Hide resolved
__tests__/fixtures/selfsigned.cert Outdated Show resolved Hide resolved
src/parse_args.ts Outdated Show resolved Hide resolved
src/parse_args.ts Outdated Show resolved Hide resolved
@andrewvc
Copy link
Contributor Author

I removed the promise rejection on stderr. We should generally be able to consider exit status for determining error. If we want to watch for stderr messages we should make that explicit in the tests IMHO since stderr != a failed test where we want to short circuit the CLI Mock.

@andrewvc andrewvc changed the title Add ignoreHTTPSErrors flag Add --ignore-https-flag Aug 11, 2021
@andrewvc andrewvc changed the title Add --ignore-https-flag Add --ignore-https-errors flag Aug 11, 2021
@vigneshshanmugam
Copy link
Member

@andrewvc Sure, Agree with your thoughts. We can watch out for exitCode which should generally work, but it was easier to debug when something was written on stderr either from CLI or errors thrown from Playwright which we haven't caught. But for now I would resolve and reject the promise if exitCode is != 0.

@andrewvc
Copy link
Contributor Author

@vigneshshanmugam if you're OK with the current behavior can I get an LGTM?

try {
return JSON.parse(data);
} catch (e) {
throw `Error ${e} could not parse data '${data}'`;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Glad you did this 👍🏽

@andrewvc andrewvc merged commit a38aad8 into elastic:master Aug 11, 2021
@andrewvc andrewvc deleted the ignore-ssl-capability branch August 11, 2021 15:43
@andrewvc andrewvc mentioned this pull request Aug 17, 2021
6 tasks
andrewvc added a commit to elastic/beats that referenced this pull request Aug 17, 2021
Fixes #27202

NOTE: This requires a version of synthetics with elastic/synthetics#361

Adds an option to disable errors on invalid TLS certificates in heartbeat.

Rather than try to work with ssl.verification_mode: none this adds a new option. The reason being that the behavior here is just too different to share a configuration option.
mergify bot pushed a commit to elastic/beats that referenced this pull request Aug 17, 2021
Fixes #27202

NOTE: This requires a version of synthetics with elastic/synthetics#361

Adds an option to disable errors on invalid TLS certificates in heartbeat.

Rather than try to work with ssl.verification_mode: none this adds a new option. The reason being that the behavior here is just too different to share a configuration option.

(cherry picked from commit 6c603a8)
andrewvc added a commit to elastic/beats that referenced this pull request Aug 17, 2021
Fixes #27202

NOTE: This requires a version of synthetics with elastic/synthetics#361

Adds an option to disable errors on invalid TLS certificates in heartbeat.

Rather than try to work with ssl.verification_mode: none this adds a new option. The reason being that the behavior here is just too different to share a configuration option.

(cherry picked from commit 6c603a8)

Co-authored-by: Andrew Cholakian <andrew@andrewvc.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants