test: add script to create 0-dns-cert.pem#11579

Closed

shigeki wants to merge 3 commits intonodejs:masterfrom

shigeki:create_0-dns_cert

Contributor

shigeki commented Feb 27, 2017 •

edited by gibfahn

Loading

0-dns-cert.pem and 0-dns-key.pem were stored in test/fixtures/key
directory, but the cert file cannot be created with the openssl
command via Makefile. make clean removes them but we could not re-create them.

This added a script to create it with using asn1.js and
asn1.js-rfc5280 and moved them out of key directory and put into
test/fixtures/0-dns.

The domains listed in the cert were also changed into example.com and
example.org to show the use for only testing.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
commit message follows commit guidelines

Affected core subsystem(s)

test, tls


test: add script to create 0-dns-cert.pem

0-dns-cert.pem  and 0-dns-key.pem were stored in `test/fixtures/key`
directory, but the cert file cannot be created with the openssl
command via Makefile.

Added a script to create it with using  `asn1.js` and
`asn1.js-rfc5280` and moved them out of key directory and put into
`test/fixtures/0-dns`.

The domains listed in the cert were also changed into example.com and
example.org to show the use for only testing.

nodejs-github-bot added the test label

shigeki added the tls label

shigeki assigned indutny

Contributor Author

shigeki commented Feb 27, 2017

CI is running on https://ci.nodejs.org/job/node-test-pull-request/6602/ .

Contributor Author

shigeki commented Feb 27, 2017

To be the fix of #10228.

jasnell approved these changes

View reviewed changes

Contributor Author

shigeki commented Feb 28, 2017

The ci jobs was somehow removed.
A new job is https://ci.nodejs.org/job/node-test-pull-request/6623/ and all is green.

sam-github suggested changes

View reviewed changes

test/fixtures/0-dns/README.md Outdated

Contributor

sam-github Feb 28, 2017

"evil"?

Contributor Author

shigeki Mar 1, 2017

Fixed

test/fixtures/0-dns/createCert.js Outdated

Contributor

sam-github Feb 28, 2017

you can wrap to 80 columns, and the backslash isn't necessary at end of line

Contributor Author

shigeki Mar 1, 2017

Fixed

test/fixtures/0-dns/createCert.js Outdated

Contributor

sam-github Feb 28, 2017

sort requires

Contributor Author

shigeki Mar 1, 2017

Fixed

test/fixtures/0-dns/createCert.js Outdated

Contributor

sam-github Feb 28, 2017

js files would be named create-cert.js

Contributor Author

shigeki Mar 1, 2017

Done

test/fixtures/0-dns/createCert.js Outdated

Contributor

sam-github Feb 28, 2017

inconsistent casing, sometimes snake_case, sometimes camelCase, it looks like test/fixtures doesn't get linted

Contributor Author

shigeki Mar 1, 2017

id_at_commonName is named after the ASN.1 notation in RFC5280 which would come from OID name.

test/fixtures/0-dns/createCert.js Outdated

Contributor

sam-github Feb 28, 2017

this isn't generally how node indents object literals

Contributor Author

shigeki Mar 1, 2017

Fixed

test/fixtures/0-dns/createCert.js Outdated

Contributor

sam-github Feb 28, 2017

lacking before closing } here and a couple lines onwards

Contributor Author

shigeki Mar 1, 2017

Fixed

test/parallel/test-tls-0-dns-altname.js Outdated

Contributor

sam-github Feb 28, 2017

Good to have a test description, it should be a capitalized and period terminated sentence.

Contributor Author

shigeki Mar 1, 2017

Fixed.

Contributor

sam-github commented Feb 28, 2017

Nice use of asn1 to craft the cert. I think the js code should follow the same coding conventions we use elswhere, even though test/fixtures seems to be exempted from linting at the moment.


fixup

fd05b71

shigeki force-pushed the create_0-dns_cert branch from 0920c23 to fd05b71 Compare

March 1, 2017 05:22

Contributor Author

shigeki commented Mar 1, 2017

I made eslint for test/fixtures/0-dns/create-cert.js and only one error of missing common module appeared.

$ ./node tools/eslint/bin/eslint.js --rulesdir=tools/eslint-rules test/fixtures/0-dns/create-cert.js --no-ignore

/home/sotsu/github/node/test/fixtures/0-dns/create-cert.js
  1:1  error  Mandatory module "common" must be loaded  required-modules

✖ 1 problem (1 error, 0 warnings)

I think it is not necessary for the script in fixtures.

gibfahn mentioned this pull request

parallel/test-debug-port-numbers - intermittent failure #11619

Closed

sam-github suggested changes

View reviewed changes

Contributor

sam-github left a comment

Its still a mixture of snake_case and camelCase for vars, but basically LGTM

test/fixtures/0-dns/0-dns-cert.pem Outdated

Contributor

sam-github Mar 1, 2017

missing newline

Contributor Author

shigeki Mar 2, 2017

Done


add newline to pem

68cd928

Contributor Author

shigeki commented Mar 2, 2017 •

edited

Loading

camelCase vars are used to follow ASN.1 notation in ~~RFC5289~~ RFC5280.

Contributor Author

shigeki commented Mar 2, 2017

@indutny Please take a look of this if you have time for you are the author of asn.1 and asn1.js-rfc5280.

sam-github approved these changes

View reviewed changes

Contributor Author

shigeki commented Mar 7, 2017

I will land this tomorrow if there are no any comments.

shigeki unassigned indutny

Contributor Author

shigeki commented Mar 10, 2017

Landed in dacaaa5. Thanks.

shigeki closed this

shigeki added a commit that referenced this pull request


test: add script to create 0-dns-cert.pem

dacaaa5

0-dns-cert.pem  and 0-dns-key.pem were stored in `test/fixtures/key`
directory, but the cert file cannot be created with the openssl
command via Makefile.

Added a script to create it with using  `asn1.js` and
`asn1.js-rfc5280` and moved them out of key directory and put into
`test/fixtures/0-dns`.

The domains listed in the cert were also changed into example.com and
example.org to show the use for only testing.

Fixes: #10228
PR-URL: #11579
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>

italoacasas pushed a commit to italoacasas/node that referenced this pull request


test: add script to create 0-dns-cert.pem

c01c7a4

0-dns-cert.pem  and 0-dns-key.pem were stored in `test/fixtures/key`
directory, but the cert file cannot be created with the openssl
command via Makefile.

Added a script to create it with using  `asn1.js` and
`asn1.js-rfc5280` and moved them out of key directory and put into
`test/fixtures/0-dns`.

The domains listed in the cert were also changed into example.com and
example.org to show the use for only testing.

Fixes: nodejs#10228
PR-URL: nodejs#11579
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>

italoacasas mentioned this pull request

7.7.3 Release Proposal #11831

Merged

jungx098 pushed a commit to jungx098/node that referenced this pull request


test: add script to create 0-dns-cert.pem

f76a167

0-dns-cert.pem  and 0-dns-key.pem were stored in `test/fixtures/key`
directory, but the cert file cannot be created with the openssl
command via Makefile.

Added a script to create it with using  `asn1.js` and
`asn1.js-rfc5280` and moved them out of key directory and put into
`test/fixtures/0-dns`.

The domains listed in the cert were also changed into example.com and
example.org to show the use for only testing.

Fixes: nodejs#10228
PR-URL: nodejs#11579
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>

MylesBorins added the land-on-v6.x label

MylesBorins pushed a commit that referenced this pull request


test: add script to create 0-dns-cert.pem

60da9ae

0-dns-cert.pem  and 0-dns-key.pem were stored in `test/fixtures/key`
directory, but the cert file cannot be created with the openssl
command via Makefile.

Added a script to create it with using  `asn1.js` and
`asn1.js-rfc5280` and moved them out of key directory and put into
`test/fixtures/0-dns`.

The domains listed in the cert were also changed into example.com and
example.org to show the use for only testing.

Fixes: #10228
PR-URL: #11579
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>

MylesBorins pushed a commit that referenced this pull request


test: add script to create 0-dns-cert.pem

aa98150

0-dns-cert.pem  and 0-dns-key.pem were stored in `test/fixtures/key`
directory, but the cert file cannot be created with the openssl
command via Makefile.

Added a script to create it with using  `asn1.js` and
`asn1.js-rfc5280` and moved them out of key directory and put into
`test/fixtures/0-dns`.

The domains listed in the cert were also changed into example.com and
example.org to show the use for only testing.

Fixes: #10228
PR-URL: #11579
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>

MylesBorins mentioned this pull request

v6.10.3 proposal #12498

Merged

andrew749 pushed a commit to michielbaird/node that referenced this pull request


test: add script to create 0-dns-cert.pem

039c984

0-dns-cert.pem  and 0-dns-key.pem were stored in `test/fixtures/key`
directory, but the cert file cannot be created with the openssl
command via Makefile.

Added a script to create it with using  `asn1.js` and
`asn1.js-rfc5280` and moved them out of key directory and put into
`test/fixtures/0-dns`.

The domains listed in the cert were also changed into example.com and
example.org to show the use for only testing.

Fixes: nodejs/node#10228
PR-URL: nodejs/node#11579
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels