test: set clientOpts.port property by danbev · Pull Request #19767 · nodejs/node

danbev · 2018-04-03T08:57:51Z

Currently this test will overwrite the clientOpts object with the port,
instead of setting the port property on the clientOpts object which
looks like the original intent.

Doing this the test fails reporting that the fake-cnnic-root-cert has
expired. This is indeed true:

$ openssl x509 -in test/fixtures/keys/fake-cnnic-root-cert.pem \
-text -noout
Certificate:
        ...
        Validity
            Not Before: Jun  9 17:15:16 2015 GMT
            Not After : Mar 29 17:15:16 2018 GMT

This commit sets the errorCode to CERT_HAS_EXPIRED. I tried updating the
certificate using test/fixtures/keys/Makefile but then no error is
thrown and I'm currently looking into this.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
commit message follows commit guidelines

danbev · 2018-04-03T08:58:20Z

CI: https://ci.nodejs.org/job/node-test-pull-request/14010/

danbev · 2018-04-04T04:16:26Z

node-test-commit-linux » ubuntu1204-64 failure looks unrelated

console output:

05:02:44 make[1]: Leaving directory `/home/iojs/build/workspace/node-test-commit-linux/nodes/ubuntu1204-64'
05:02:44 make: *** [run-ci] Error 2
05:02:45 Build step 'Execute shell' marked build as failure
05:02:45 Run condition [Always] enabling perform for step [[]]
05:02:45 Run condition [Always] enabling perform for step [[]]
05:02:45 TAP Reports Processing: START
05:02:45 Looking for TAP results report in workspace using pattern: *.tap
05:02:45 Did not find any matching files. Setting build result to FAILURE.
05:02:45 Checking ^not ok
05:02:45 Jenkins Text Finder: File set '*.tap' is empty
05:02:45 Notifying upstream projects of job completion
05:02:46 Finished: FAILURE

node-test-commit-windows-fanned failure looks unrelated

console output:

 > git fetch --tags --progress binary_tmp@147.75.70.237:binary_tmp.git +refs/heads/jenkins-node-test-commit-windows-fanned-16879*:refs/remotes/jenkins_tmp/jenkins-node-test-commit-windows-fanned-16879* # timeout=30
ERROR: Error fetching remote repo 'jenkins_tmp'
hudson.plugins.git.GitException: Failed to fetch from binary_tmp@147.75.70.237:binary_tmp.git
	at hudson.plugins.git.GitSCM.fetchFrom(GitSCM.java:862)
	at hudson.plugins.git.GitSCM.retrieveChanges(GitSCM.java:1129)
	at hudson.plugins.git.GitSCM.checkout(GitSCM.java:1160)
	at hudson.scm.SCM.checkout(SCM.java:504)
	at hudson.model.AbstractProject.checkout(AbstractProject.java:1203)
	at hudson.model.AbstractBuild$AbstractBuildExecution.defaultCheckout(AbstractBuild.java:574)
	at jenkins.scm.SCMCheckoutStrategy.checkout(SCMCheckoutStrategy.java:86)
	at hudson.model.AbstractBuild$AbstractBuildExecution.run(AbstractBuild.java:499)
	at hudson.model.Run.execute(Run.java:1727)
	at hudson.matrix.MatrixRun.run(MatrixRun.java:146)
	at hudson.model.ResourceController.execute(ResourceController.java:97)
	at hudson.model.Executor.run(Executor.java:429)
Caused by: hudson.plugins.git.GitException: Command "git fetch --tags --progress binary_tmp@147.75.70.237:binary_tmp.git +refs/heads/jenkins-node-test-commit-windows-fanned-16879*:refs/remotes/jenkins_tmp/jenkins-node-test-commit-windows-fanned-16879*" returned status code 128:
stdout: 
stderr: ssh_exchange_identification: Connection closed by remote host
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.launchCommandIn(CliGitAPIImpl.java:1996)
	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.launchCommandWithCredentials(CliGitAPIImpl.java:1715)
	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.access$300(CliGitAPIImpl.java:72)
	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl$1.execute(CliGitAPIImpl.java:405)
	at org.jenkinsci.plugins.gitclient.RemoteGitImpl$CommandInvocationHandler$1.call(RemoteGitImpl.java:153)
	at org.jenkinsci.plugins.gitclient.RemoteGitImpl$CommandInvocationHandler$1.call(RemoteGitImpl.java:146)
	at hudson.remoting.UserRequest.perform(UserRequest.java:210)
	at hudson.remoting.UserRequest.perform(UserRequest.java:53)
	at hudson.remoting.Request$2.run(Request.java:364)
	at hudson.remoting.InterceptingExecutorService$1.call(InterceptingExecutorService.java:72)
	at java.util.concurrent.FutureTask.run(Unknown Source)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)
	at hudson.remoting.Engine$1$1.run(Engine.java:94)
	at java.lang.Thread.run(Unknown Source)
	Suppressed: hudson.remoting.Channel$CallSiteStackTrace: Remote call to JNLP4-connect connection from 40.118.135.231/40.118.135.231:1176
		at hudson.remoting.Channel.attachCallSiteStackTrace(Channel.java:1737)
		at hudson.remoting.UserResponse.retrieve(UserRequest.java:313)
		at hudson.remoting.Channel.call(Channel.java:952)
		at org.jenkinsci.plugins.gitclient.RemoteGitImpl$CommandInvocationHandler.execute(RemoteGitImpl.java:146)
		at sun.reflect.GeneratedMethodAccessor441.invoke(Unknown Source)
		at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
		at java.lang.reflect.Method.invoke(Method.java:498)
		at org.jenkinsci.plugins.gitclient.RemoteGitImpl$CommandInvocationHandler.invoke(RemoteGitImpl.java:132)
		at com.sun.proxy.$Proxy76.execute(Unknown Source)
		at hudson.plugins.git.GitSCM.fetchFrom(GitSCM.java:860)
		at hudson.plugins.git.GitSCM.retrieveChanges(GitSCM.java:1129)
		at hudson.plugins.git.GitSCM.checkout(GitSCM.java:1160)
		at hudson.scm.SCM.checkout(SCM.java:504)
		at hudson.model.AbstractProject.checkout(AbstractProject.java:1203)
		at hudson.model.AbstractBuild$AbstractBuildExecution.defaultCheckout(AbstractBuild.java:574)
		at jenkins.scm.SCMCheckoutStrategy.checkout(SCMCheckoutStrategy.java:86)
		at hudson.model.AbstractBuild$AbstractBuildExecution.run(AbstractBuild.java:499)
		at hudson.model.Run.execute(Run.java:1727)
		at hudson.matrix.MatrixRun.run(MatrixRun.java:146)
		at hudson.model.ResourceController.execute(ResourceController.java:97)
		at hudson.model.Executor.run(Executor.java:429)
ERROR: Error fetching remote repo 'jenkins_tmp'
TAP Reports Processing: START
Looking for TAP results report in workspace using pattern: *.tap
Did not find any matching files. Setting build result to FAILURE.
Checking ^not ok
Jenkins Text Finder: File set '*.tap' is empty
Notifying upstream projects of job completion
Finished: FAILURE

Trott · 2018-04-04T05:47:26Z

Re-running Linux: https://ci.nodejs.org/job/node-test-commit-linux/17598/

Re-running Windows: https://ci.nodejs.org/job/node-test-commit-windows-fanned/16905/

lpinca · 2018-04-06T13:35:13Z

@danbev is this still in progress?

danbev · 2018-04-06T13:50:45Z

@danbev is this still in progress?

Sort of, I still have to figure out how this test is supposed to work but not had time yet. But if it is fine to commit this as is I can follow up on it next week.

lpinca · 2018-04-06T13:53:06Z

No hurries, just wanted to know if the label was added by mistake.

Currently this test will overwrite the clientOpts object with the port, instead of setting the port property on the clientOpts object which looks like the original intent. Doing this the test fails reporting that the fake-cnnic-root-cert has expired. This is indeed true: $ openssl x509 -in test/fixtures/keys/fake-cnnic-root-cert.pem \ -text -noout Certificate: ... Validity Not Before: Jun 9 17:15:16 2015 GMT Not After : Mar 29 17:15:16 2018 GMT This commit sets the errorCode to CERT_HAS_EXPIRED. I tried updating the certificate using test/fixtures/keys/Makefile but then no error is thrown and I'm currently looking into this.

I looks like this test has not worked as expected since commit 2bc7841 ("test: use random ports where possible"). The test in that commit checked for `CERT_REVOKED` which was returned by CheckWhitelistedServerCert. CheckWhitelistedServerCert was later removed in commit dc87543 ("src: drop CNNIC+StartCom certificate whitelisting"). I'm suggesting that this test case be removed as I don't think it is valid anymore.

danbev · 2018-04-13T13:22:50Z

Rebased and updated CI: https://ci.nodejs.org/job/node-test-pull-request/14257/

Trott · 2018-04-13T21:14:13Z

Re-running Linux CI: https://ci.nodejs.org/job/node-test-commit-linux/17976/

Currently this test will overwrite the clientOpts object with the port, instead of setting the port property on the clientOpts object which looks like the original intent. Doing this the test fails reporting that the fake-cnnic-root-cert has expired. This is indeed true: $ openssl x509 -in test/fixtures/keys/fake-cnnic-root-cert.pem \ -text -noout Certificate: ... Validity Not Before: Jun 9 17:15:16 2015 GMT Not After : Mar 29 17:15:16 2018 GMT This commit sets the errorCode to CERT_HAS_EXPIRED. I tried updating the certificate using test/fixtures/keys/Makefile but then no error is thrown and I'm currently looking into this. PR-URL: #19767 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

I looks like this test has not worked as expected since commit 2bc7841 ("test: use random ports where possible"). The test in that commit checked for `CERT_REVOKED` which was returned by CheckWhitelistedServerCert. CheckWhitelistedServerCert was later removed in commit dc87543 ("src: drop CNNIC+StartCom certificate whitelisting"). I'm suggesting that this test case be removed as I don't think it is valid anymore. PR-URL: #19767 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

jasnell · 2018-04-14T19:08:34Z

Landed in 7a33c2c and 1d72434

Currently this test will overwrite the clientOpts object with the port, instead of setting the port property on the clientOpts object which looks like the original intent. Doing this the test fails reporting that the fake-cnnic-root-cert has expired. This is indeed true: $ openssl x509 -in test/fixtures/keys/fake-cnnic-root-cert.pem \ -text -noout Certificate: ... Validity Not Before: Jun 9 17:15:16 2015 GMT Not After : Mar 29 17:15:16 2018 GMT This commit sets the errorCode to CERT_HAS_EXPIRED. I tried updating the certificate using test/fixtures/keys/Makefile but then no error is thrown and I'm currently looking into this. PR-URL: #19767 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

I looks like this test has not worked as expected since commit 2bc7841 ("test: use random ports where possible"). The test in that commit checked for `CERT_REVOKED` which was returned by CheckWhitelistedServerCert. CheckWhitelistedServerCert was later removed in commit dc87543 ("src: drop CNNIC+StartCom certificate whitelisting"). I'm suggesting that this test case be removed as I don't think it is valid anymore. PR-URL: #19767 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

Currently this test will overwrite the clientOpts object with the port, instead of setting the port property on the clientOpts object which looks like the original intent. Doing this the test fails reporting that the fake-cnnic-root-cert has expired. This is indeed true: $ openssl x509 -in test/fixtures/keys/fake-cnnic-root-cert.pem \ -text -noout Certificate: ... Validity Not Before: Jun 9 17:15:16 2015 GMT Not After : Mar 29 17:15:16 2018 GMT This commit sets the errorCode to CERT_HAS_EXPIRED. I tried updating the certificate using test/fixtures/keys/Makefile but then no error is thrown and I'm currently looking into this. PR-URL: nodejs#19767 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

I looks like this test has not worked as expected since commit 2bc7841 ("test: use random ports where possible"). The test in that commit checked for `CERT_REVOKED` which was returned by CheckWhitelistedServerCert. CheckWhitelistedServerCert was later removed in commit dc87543 ("src: drop CNNIC+StartCom certificate whitelisting"). I'm suggesting that this test case be removed as I don't think it is valid anymore. PR-URL: nodejs#19767 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

Currently this test will overwrite the clientOpts object with the port, instead of setting the port property on the clientOpts object which looks like the original intent. Doing this the test fails reporting that the fake-cnnic-root-cert has expired. This is indeed true: $ openssl x509 -in test/fixtures/keys/fake-cnnic-root-cert.pem \ -text -noout Certificate: ... Validity Not Before: Jun 9 17:15:16 2015 GMT Not After : Mar 29 17:15:16 2018 GMT This commit sets the errorCode to CERT_HAS_EXPIRED. I tried updating the certificate using test/fixtures/keys/Makefile but then no error is thrown and I'm currently looking into this. Backport-PR-URL: #20776 PR-URL: #19767 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

I looks like this test has not worked as expected since commit 2bc7841 ("test: use random ports where possible"). The test in that commit checked for `CERT_REVOKED` which was returned by CheckWhitelistedServerCert. CheckWhitelistedServerCert was later removed in commit 6ee4228 ("src: drop CNNIC+StartCom certificate whitelisting"). I'm suggesting that this test case be removed as I don't think it is valid anymore. Backport-PR-URL: #20776 PR-URL: #19767 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

Currently this test will overwrite the clientOpts object with the port, instead of setting the port property on the clientOpts object which looks like the original intent. Doing this the test fails reporting that the fake-cnnic-root-cert has expired. This is indeed true: $ openssl x509 -in test/fixtures/keys/fake-cnnic-root-cert.pem \ -text -noout Certificate: ... Validity Not Before: Jun 9 17:15:16 2015 GMT Not After : Mar 29 17:15:16 2018 GMT This commit sets the errorCode to CERT_HAS_EXPIRED. I tried updating the certificate using test/fixtures/keys/Makefile but then no error is thrown and I'm currently looking into this. Backport-PR-URL: #20776 PR-URL: #19767 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

I looks like this test has not worked as expected since commit 2bc7841 ("test: use random ports where possible"). The test in that commit checked for `CERT_REVOKED` which was returned by CheckWhitelistedServerCert. CheckWhitelistedServerCert was later removed in commit 6ee4228 ("src: drop CNNIC+StartCom certificate whitelisting"). I'm suggesting that this test case be removed as I don't think it is valid anymore. Backport-PR-URL: #20776 PR-URL: #19767 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

Currently this test will overwrite the clientOpts object with the port, instead of setting the port property on the clientOpts object which looks like the original intent. Doing this the test fails reporting that the fake-cnnic-root-cert has expired. This is indeed true: $ openssl x509 -in test/fixtures/keys/fake-cnnic-root-cert.pem \ -text -noout Certificate: ... Validity Not Before: Jun 9 17:15:16 2015 GMT Not After : Mar 29 17:15:16 2018 GMT This commit sets the errorCode to CERT_HAS_EXPIRED. I tried updating the certificate using test/fixtures/keys/Makefile but then no error is thrown and I'm currently looking into this. Backport-PR-URL: #20776 PR-URL: #19767 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

I looks like this test has not worked as expected since commit 2bc7841 ("test: use random ports where possible"). The test in that commit checked for `CERT_REVOKED` which was returned by CheckWhitelistedServerCert. CheckWhitelistedServerCert was later removed in commit 6ee4228 ("src: drop CNNIC+StartCom certificate whitelisting"). I'm suggesting that this test case be removed as I don't think it is valid anymore. Backport-PR-URL: #20776 PR-URL: #19767 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

Currently this test will overwrite the clientOpts object with the port, instead of setting the port property on the clientOpts object which looks like the original intent. Doing this the test fails reporting that the fake-cnnic-root-cert has expired. This is indeed true: $ openssl x509 -in test/fixtures/keys/fake-cnnic-root-cert.pem \ -text -noout Certificate: ... Validity Not Before: Jun 9 17:15:16 2015 GMT Not After : Mar 29 17:15:16 2018 GMT This commit sets the errorCode to CERT_HAS_EXPIRED. I tried updating the certificate using test/fixtures/keys/Makefile but then no error is thrown and I'm currently looking into this. PR-URL: nodejs/node#19767 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

I looks like this test has not worked as expected since commit 2bc7841 ("test: use random ports where possible"). The test in that commit checked for `CERT_REVOKED` which was returned by CheckWhitelistedServerCert. CheckWhitelistedServerCert was later removed in commit 6ee4228 ("src: drop CNNIC+StartCom certificate whitelisting"). I'm suggesting that this test case be removed as I don't think it is valid anymore. PR-URL: nodejs/node#19767 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

danbev added the wip Issues and PRs that are still a work in progress. label Apr 3, 2018

nodejs-github-bot added the test Issues and PRs related to the tests. label Apr 3, 2018

cjihrig approved these changes Apr 3, 2018

View reviewed changes

jasnell approved these changes Apr 3, 2018

View reviewed changes

lpinca approved these changes Apr 4, 2018

View reviewed changes

danbev added 2 commits April 13, 2018 15:14

danbev force-pushed the test-tls-cnnic-whitelist_clientOpts branch from ee5fba2 to 5753046 Compare April 13, 2018 13:22

danbev removed the wip Issues and PRs that are still a work in progress. label Apr 13, 2018

jasnell closed this Apr 14, 2018

danbev mentioned this pull request May 16, 2018

[v8.x backport] backport of tls-cnnic-whitelist fixes #20776

Closed

2 tasks

MylesBorins mentioned this pull request Jul 9, 2018

v8.12.0 proposal #21593

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

test: set clientOpts.port property#19767

test: set clientOpts.port property#19767
danbev wants to merge 2 commits intonodejs:masterfrom
danbev:test-tls-cnnic-whitelist_clientOpts

danbev commented Apr 3, 2018

Uh oh!

danbev commented Apr 3, 2018

Uh oh!

danbev commented Apr 4, 2018

Uh oh!

Trott commented Apr 4, 2018

Uh oh!

lpinca commented Apr 6, 2018

Uh oh!

danbev commented Apr 6, 2018

Uh oh!

lpinca commented Apr 6, 2018

Uh oh!

danbev commented Apr 13, 2018

Uh oh!

Trott commented Apr 13, 2018

Uh oh!

jasnell commented Apr 14, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

danbev commented Apr 3, 2018

Checklist

Uh oh!

danbev commented Apr 3, 2018

Uh oh!

danbev commented Apr 4, 2018

Uh oh!

Trott commented Apr 4, 2018

Uh oh!

lpinca commented Apr 6, 2018

Uh oh!

danbev commented Apr 6, 2018

Uh oh!

lpinca commented Apr 6, 2018

Uh oh!

danbev commented Apr 13, 2018

Uh oh!

Trott commented Apr 13, 2018

Uh oh!

jasnell commented Apr 14, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants