HTTP-level tests for puma-dev server via goroutine #233

nonrational · 2020-02-26T19:56:02Z

This PR adds end-to-end HTTP level tests for puma-dev server.

Purpose

Alright folks. This is the one you've been waiting for. We can now run a puma-dev server in tests and throw HTTP requests against it to ensure proper behavior. 💥

This is a big one, so I'm gonna take you through it commit by commit.

Changes, Commit-by-Commit

a1644d2 ensure temp dirs are cleaned up, more testing helpers

This fixes an existing bug in the tests file that didn't correctly clean up some temp directories.

Sidenote: Deferred return is an awesome pattern.

a9dcd8a end-to-end ssl cert test, pull create_cert out of setup_cert for separate use

SetupOurCert generates a puma-dev CA cert if one doesn't already exist and trusts in in the macOS keychain. I extracted the cert generation piece into a separate function, so we can test it and call it as part of running the server as part of tests.

I also made it more explicit that if the cert is already present, we'll return early and assume that it's already trusted. This "feature" ends up being important in tests, because you can't exercise the "trust" branch on TravisCI. It requires an interactive password dialog to unlock the macOS keychain. But, if we pre-generate the cert, we can "trick" the code into thinking the cert has already been trusted previously.

763a40a test puma-dev server w/ rack app, use retry to wait for socket & poll /events

This is the big one. The basic structure is

Symlink a tiny rack app into ~/.gotest-puma-dev
Generate a dummy (but valid) CA for the length of the test run (that will never be trusted). This allows us to skip the "trust" branch that would require user interaction on macOS.
Run go main() with flags to set the app directory to ~/.gotest-puma-dev
Attempt a TCP connection to localhost:9280. Once it returns, you know the server is up.
Given that it takes some handful of seconds for the server to be ready & listening on 9280, I pulled in avast/retry-go to retry the connection rather than opting for a sleep.
Run 4 subtests inside TestMainPumaDev to verify that
puma-dev /status returns json
rack app boots responds correctly
restart.txt kills the rack app (I was working on Running puma-dev in background will not pickup changes to file after restart. #190 simultaneously, so this seemed like a good thing to add)
unknown app returns ... unknown app.

72d7fe8 clean up gitignore, ensure we don't commit a .ruby-version for dummy rack app
There were some outdated entried in .gitignore that I removed. And, since I needed to install puma, I needed to set a ruby version but don't want to enforce a particular version.

5f58924 set up travis environment to run rack app. skip bundler for now

Speaking of puma, need to install it for Travis and ensure that /etc/resolver is writable. We won't use it in our tests (because we can just use Host: foo), but we need it for puma-dev to boot.

bc49df2 add makefile shortcut for macOS tests that require interactivity keychain unlock

Before I started on the SSL refactor, I wrote a test to exercise the whole thing but it can only be run by manually because it involves unlocking your login keychain via a system prompt.

IMPORTANT NOTE: Since running this test generates AND TRUSTS a new CA cert, it would be irresponsible to leave it lying around in your keychain. Running this test will delete any and all puma-dev CA certs in your keychain. You'll need to run setup again in order to get a new one. But, nobody should be running this test themselves unless they're actively developing related code for puma-dev.

b6006bd add circleci config to test catalina

TravisCI still hasn't released a macOS 10.15 build environment, so I'm trying out Circle's on my fork. I will revert this commit before merging.

Conclusion

You have reached The End. Congratulations. 🎉

…rate use

… /events

…rack app

…hain unlock

Makefile

nonrational · 2020-02-26T20:00:39Z

cmd/puma-dev/main_test.go

+func generateLivePumaDevCertIfNotExist(t *testing.T) {
+	liveSupportPath := homedir.MustExpand(dev.SupportDir)
+	liveCertPath := filepath.Join(liveSupportPath, "cert.pem")
+	liveKeyPath := filepath.Join(liveSupportPath, "key.pem")


we need a valid cert to exist at this path before we start main(). this cert won't be trusted, but it will be "valid".

nonrational · 2020-02-26T20:01:27Z

cmd/puma-dev/main_test.go

+	}
+}
+
+func getUrlWithHost(t *testing.T, url string, host string) string {


we don't use DNS in tests, so we set the Host header. h/t @tubbo for giving me the idea.

nonrational · 2020-02-26T20:03:14Z

dev/devtest/testutils.go

-		if err := os.MkdirAll(path, 0755); err != nil {
-			panic(err)
-		}
+		panic(err)


it's more complicated to have this helper make the path if it doesn't exist, so just bail out if you didn't create the directory in the first place.

nonrational · 2020-02-26T20:04:03Z

dev/ssl.go

@@ -98,7 +83,36 @@ func SetupOurCert() error {

 	keyOut.Close()

-	return TrustCert(certPath)
+	return nil


this diff is a bit hard to read, but I just refactored out most of the SetupOurCert function into GeneratePumaDevCertificateAuthority

nonrational · 2020-02-26T20:05:50Z

dev/ssl_darwin.go

+	stderr, readPipeErr := addTrustedCertCommand.StderrPipe()
+	if readPipeErr != nil {
+		return readPipeErr
+	}


wanted to be able to examine the output if this fails, so takes a few extra lines to feed a pipe to the cmd before running it.

nonrational · 2020-02-26T20:07:17Z

dev/ssl_darwin_test.go

+		return fmt.Sprintf(`for sha in $(security find-certificate -a -c "Puma-dev CA" -Z | awk '/SHA-1/ {print $3}'); do %s; done`, subCommand)
+	}
+
+	if err := exec.Command("sh", "-c", forEachPumaDevKeychainCertSha("security delete-certificate -Z $sha")).Run(); err != nil {


given there might be a few certs with the same common name (because you ran setup several times or something) we need to find all the hashes and delete them that way.

nonrational · 2020-02-26T20:11:19Z

dev/ssl_darwin_test.go

+}
+
+func TestSetupOurCert_ensureNotWorldReadable(t *testing.T) {
+	t.Skip("not implemented yet - https://github.com/puma/puma-dev/issues/215")


I'm not sure exactly what the attack chain looks like, but if this CA cert and key remain unprivileged user-readable then it stands to reason that a piece of malicious software could read it, sign a new cert for www.whereyoukeepyoursecrets.com with it, hijack your DNS, and you'd be in the 💩right quick and likely never know it.

nonrational · 2020-02-26T20:11:50Z

dev/ssl_linux_test.go

+		assert.Nil(t, err)
+	})
+
+	assert.Regexp(t, "^! Add /does/not/exist to your browser to trust CA\\n$", stdOut)


hooray code coverage. 🙃

nonrational · 2020-02-26T20:12:10Z

dev/ssl_test.go

+		assert.Fail(t, err.Error())
+	}
+
+	_, err := tls.LoadX509KeyPair(testCertPath, testKeyPath)


being able to load the certificate vouches for its "validity".

evanphx

Yaaay!

longer names mean don't do this unless you know what you're doing. :D

This reverts commit b6006bd.

nonrational added 7 commits February 26, 2020 09:46

ensure temp dirs are cleaned up, more testing helpers

a1644d2

end-to-end ssl cert test, pull create_cert out of setup_cert for sepa…

a9dcd8a

…rate use

test puma-dev server w/ rack app, use retry to wait for socket & poll…

763a40a

… /events

clean up gitignore, ensure we don't commit a .ruby-version for dummy …

72d7fe8

…rack app

set up travis environment to run rack app. skip bundler for now

5f58924

add makefile shortcut for macOS tests that require interactivity keyc…

bc49df2

…hain unlock

add circleci config to test catalina

b6006bd

nonrational requested a review from evanphx February 26, 2020 19:56

nonrational commented Feb 26, 2020

View reviewed changes

evanphx approved these changes Feb 26, 2020

View reviewed changes

nonrational added 2 commits February 26, 2020 16:31

Update Makefile

62ed4ff

longer names mean don't do this unless you know what you're doing. :D

Revert "add circleci config to test catalina"

d78ce2c

This reverts commit b6006bd.

nonrational merged commit 8907643 into puma:master Feb 26, 2020

nonrational deleted the end-to-end-test branch February 26, 2020 21:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HTTP-level tests for puma-dev server via goroutine #233

HTTP-level tests for puma-dev server via goroutine #233

nonrational commented Feb 26, 2020 •

edited

nonrational Feb 26, 2020

nonrational Feb 26, 2020

nonrational Feb 26, 2020

nonrational Feb 26, 2020

nonrational Feb 26, 2020

nonrational Feb 26, 2020

nonrational Feb 26, 2020

nonrational Feb 26, 2020

nonrational Feb 26, 2020

evanphx left a comment

HTTP-level tests for puma-dev server via goroutine #233

HTTP-level tests for puma-dev server via goroutine #233

Conversation

nonrational commented Feb 26, 2020 • edited

Purpose

Changes, Commit-by-Commit

Conclusion

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evanphx left a comment

Choose a reason for hiding this comment

nonrational commented Feb 26, 2020 •

edited