feat(cli): add DENO_CERT environment variable #6370
Conversation
|
|
|
Thanks for the patch @linde12 - we need a test of this... You should be able to copy and modify "cafile_fetch" in cli/tests/integration_tests.rs for this env var case. |
|
Thanks for the tip @ry, i was wondering how i would test this. I will get back to this and add a test in a day or two if that is ok, im currently celebrating a holiday! :-) |
|
Thanks - have a happy holiday! BTW we won't be able to release this feature until 1.2.0, which is scheduled for July 13 - so no rush. |
|
Thanks! Plenty of time then hehe, but hopefully i'll have time this weekend. Thanks for letting me know👍 |
|
Not sure if we want to test the precedence also, but i added a test based upon cafile_fetch |
|
CC @ry what do you think? 😃 |
bartlomieju
added
cli
| let deno_dir = TempDir::new().expect("tempdir fail"); | ||
| let module_url = | ||
| Url::parse("http://localhost:4545/cli/tests/cafile_url_imports.ts") | ||
| .unwrap(); |
There was a problem hiding this comment.
I think this URL should be https://localhost:5545/cli/tests/cafile_info.ts
I see that cafile_fetch below is also using a http URL rather than https. I think that is an error.
What do you think?
There was a problem hiding this comment.
@ry Nice catch! The weird thing is that test is actually failing if i remove the DENO_CERT env (i tested with and without it before submitting the PR) so something weird is happening? Could it be that the connection is being upgraded to HTTPS? 🤷
I changed to https://localhost:5545/cli/tests/cafile_url_imports.ts and it works, or did you specifically want cafile_info.ts? Also, want me to update the cafile_fetch test in the same PR?
- make cafile_env_fetch target https url - don't assert on stdout, just assert status
|
Tests were killed with SIGABRT and AFAIK there is no way to manually retrigger? |
@linde12 that might have been an intermittent failure. I've merged latest code to your branch let's see if it happens again. |
|
@bartlomieju Great, seems to be passing now👍 Can we merge or is there anything you want me to change before? |
|
Is the |
Add the ability to specify CA path in environment variable
DENO_CERT. Running deno with the--certarg will take precedence overDENO_CERTSee #6355