address PR comments

Author: johnstcn

Makefile

@@ -8,6 +8,8 @@ gen:
 
 build: terraform-provider-coder
 
+# Builds the provider. Note that as coder/coder is based on
+# alpine, we need to disable cgo.
 terraform-provider-coder: provider/*.go main.go
 	CGO_ENABLED=0 go build .
 

integration/integration_test.go

@@ -5,10 +5,10 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"net"
 	"os"
 	"path/filepath"
 	"runtime"
+	"strconv"
 	"strings"
 	"testing"
 	"time"
@@ -21,58 +21,102 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+// TestIntegration performs an integration test against an ephemeral Coder deployment.
+// For each directory containing a `main.tf` under `/integration`, performs the following:
+//   - Pushes the template to a temporary Coder instance running in Docker
+//   - Creates a workspace from the template. Templates here are expected to create a
+//     local_file resource containing JSON that can be marshalled as a map[string]string
+//   - Fetches the content of the JSON file created and compares it against the expected output.
+//
+// NOTE: all interfaces to this Coder deployment are performed without github.com/coder/coder/v2/codersdk
+// in order to avoid a circular dependency.
 func TestIntegration(t *testing.T) {
 	if os.Getenv("TF_ACC") == "1" {
 		t.Skip("Skipping integration tests during tf acceptance tests")
 	}
-	ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
+
+	timeoutStr := os.Getenv("TIMEOUT_MINS")
+	if timeoutStr == "" {
+		timeoutStr = "10"
+	}
+	timeoutMins, err := strconv.Atoi(timeoutStr)
+	require.NoError(t, err, "invalid value specified for timeout")
+	ctx, cancel := context.WithTimeout(context.Background(), time.Duration(timeoutMins)*time.Minute)
 	t.Cleanup(cancel)
 
+	// Given: we have an existing Coder deployment running locally
 	ctrID := setup(ctx, t)
 
-	t.Run("test-data-sources", func(t *testing.T) {
-		// Import an example template
-		_, rc := execContainer(ctx, t, ctrID, `coder templates push test-data-source --directory /src/integration/test-data-source/ --var output_path=/tmp/test-data-sources.json --yes`)
-		require.Equal(t, 0, rc)
-		// Create a workspace
-		_, rc = execContainer(ctx, t, ctrID, `coder create test-data-source -t test-data-source --yes`)
-		require.Equal(t, 0, rc)
-		// Fetch the output created by the template
-		out, rc := execContainer(ctx, t, ctrID, `cat /tmp/test-data-sources.json`)
-		require.Equal(t, 0, rc)
-		m := make(map[string]string)
-		require.NoError(t, json.NewDecoder(strings.NewReader(out)).Decode(&m))
-		assert.Equal(t, runtime.GOARCH, m["provisioner.arch"])
-		assert.NotEmpty(t, m["provisioner.id"])
-		assert.Equal(t, runtime.GOOS, m["provisioner.os"])
-		assert.NotEmpty(t, m["workspace.access_port"])
-		assert.NotEmpty(t, m["workspace.access_url"])
-		assert.NotEmpty(t, m["workspace.id"])
-		assert.Equal(t, "test-data-source", m["workspace.name"])
-		assert.Equal(t, "testing", m["workspace.owner"])
-		assert.Equal(t, "[email protected]", m["workspace.owner_email"])
-		assert.NotEmpty(t, m["workspace.owner_id"])
-		assert.Equal(t, "default", m["workspace.owner_name"])
-		// assert.NotEmpty(t, m["workspace.owner_oidc_access_token"]) // TODO: need a test OIDC integration
-		assert.NotEmpty(t, m["workspace.owner_session_token"])
-		assert.Equal(t, "1", m["workspace.start_count"])
-		assert.NotEmpty(t, m["workspace.template_id"])
-		assert.Equal(t, "test-data-source", m["workspace.template_name"])
-		assert.NotEmpty(t, m["workspace.template_version"])
-		assert.Equal(t, "start", m["workspace.transition"])
-		assert.Equal(t, "[email protected]", m["workspace_owner.email"])
-		assert.Equal(t, "default", m["workspace_owner.full_name"])
-		assert.NotEmpty(t, m["workspace_owner.groups"])
-		assert.NotEmpty(t, m["workspace_owner.id"])
-		assert.Equal(t, "testing", m["workspace_owner.name"])
-		// assert.NotEmpty(t, m["workspace_owner.oidc_access_token"]) // TODO: test OIDC integration
-		assert.NotEmpty(t, m["workspace_owner.session_token"])
-		assert.NotEmpty(t, m["workspace_owner.ssh_private_key"])
-		assert.NotEmpty(t, m["workspace_owner.ssh_public_key"])
-	})
+	for _, tt := range []struct {
+		// Name of the folder under `integration/` containing a test template
+		templateName string
+		// map of string to regex to be passed to assertOutput()
+		expectedOutput map[string]string
+	}{
+		{
+			templateName: "test-data-source",
+			expectedOutput: map[string]string{
+				"provisioner.arch":                  runtime.GOARCH,
+				"provisioner.id":                    `[a-zA-Z0-9-]+`,
+				"provisioner.os":                    runtime.GOOS,
+				"workspace.access_port":             `\d+`,
+				"workspace.access_url":              `https?://\D+:\d+`,
+				"workspace.id":                      `[a-zA-z0-9-]+`,
+				"workspace.name":                    `test-data-source`,
+				"workspace.owner":                   `testing`,
+				"workspace.owner_email":             `testing@coder\.com`,
+				"workspace.owner_groups":            `\[\]`,
+				"workspace.owner_id":                `[a-zA-Z0-9]+`,
+				"workspace.owner_name":              `default`,
+				"workspace.owner_oidc_access_token": `^$`, // TODO: need a test OIDC integration
+				"workspace.owner_session_token":     `[a-zA-Z0-9-]+`,
+				"workspace.start_count":             `1`,
+				"workspace.template_id":             `[a-zA-Z0-9-]+`,
+				"workspace.template_name":           `test-data-source`,
+				"workspace.template_version":        `.+`,
+				"workspace.transition":              `start`,
+				"workspace_owner.email":             `testing@coder\.com`,
+				"workspace_owner.full_name":         `default`,
+				"workspace_owner.groups":            `\[\]`,
+				"workspace_owner.id":                `[a-zA-Z0-9-]+`,
+				"workspace_owner.name":              `testing`,
+				"workspace_owner.oidc_access_token": `^$`, // TODO: test OIDC integration
+				"workspace_owner.session_token":     `.+`,
+				"workspace_owner.ssh_private_key":   `^$`, // Depends on coder/coder#13366
+				"workspace_owner.ssh_public_key":    `^$`, // Depends on coder/coder#13366
+			},
+		},
+	} {
+		t.Run(tt.templateName, func(t *testing.T) {
+			// Import named template
+			_, rc := execContainer(ctx, t, ctrID, fmt.Sprintf(`coder templates push %s --directory /src/integration/%s --var output_path=/tmp/%s.json --yes`, tt.templateName, tt.templateName, tt.templateName))
+			require.Equal(t, 0, rc)
+			// Create a workspace
+			_, rc = execContainer(ctx, t, ctrID, fmt.Sprintf(`coder create %s -t %s --yes`, tt.templateName, tt.templateName))
+			require.Equal(t, 0, rc)
+			// Fetch the output created by the template
+			out, rc := execContainer(ctx, t, ctrID, fmt.Sprintf(`cat /tmp/%s.json`, tt.templateName))
+			require.Equal(t, 0, rc)
+			actual := make(map[string]string)
+			require.NoError(t, json.NewDecoder(strings.NewReader(out)).Decode(&actual))
+			assertOutput(t, tt.expectedOutput, actual)
+		})
+	}
 }
 
 func setup(ctx context.Context, t *testing.T) string {
+	var (
+		// For this test to work, we pass in a custom terraformrc to use
+		// the locally built version of the provider.
+		testTerraformrc = `provider_installation {
+		dev_overrides {
+		  "coder/coder" = "/src"
+		}
+		  direct{}
+	  }`
+		localURL = "http://localhost:3000"
+	)
+
 	coderImg := os.Getenv("CODER_IMAGE")
 	if coderImg == "" {
 		coderImg = "ghcr.io/coder/coder"
@@ -92,45 +136,39 @@ func setup(ctx context.Context, t *testing.T) string {
 		t.Fatalf("not found: %q - please build the provider first", binPath)
 	}
 	tmpDir := t.TempDir()
+	// Create a terraformrc to point to our freshly built provider!
 	tfrcPath := filepath.Join(tmpDir, "integration.tfrc")
-	tfrc := `provider_installation {
-  dev_overrides {
-    "coder/coder" = "/src"
-  }
-	direct{}
-}`
-	err = os.WriteFile(tfrcPath, []byte(tfrc), 0o644)
+	err = os.WriteFile(tfrcPath, []byte(testTerraformrc), 0o644)
 	require.NoError(t, err, "write terraformrc to tempdir")
 
 	cli, err := client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation())
 	require.NoError(t, err, "init docker client")
 
-	p := randomPort(t)
-	t.Logf("random port is %d\n", p)
-
 	srcPath, err := filepath.Abs("..")
 	require.NoError(t, err, "get abs path of parent")
 	t.Logf("src path is %s\n", srcPath)
 
+	// Stand up a temporary Coder instance
 	ctr, err := cli.ContainerCreate(ctx, &container.Config{
 		Image: coderImg + ":" + coderVersion,
 		Env: []string{
-			fmt.Sprintf("CODER_ACCESS_URL=http://host.docker.internal:%d", p),
-			"CODER_IN_MEMORY=true",
-			"CODER_TELEMETRY_ENABLE=false",
-			"TF_CLI_CONFIG_FILE=/tmp/integration.tfrc",
+			"CODER_ACCESS_URL=" + localURL,             // Set explicitly to avoid creating try.coder.app URLs.
+			"CODER_IN_MEMORY=true",                     // We don't necessarily care about real persistence here.
+			"CODER_TELEMETRY_ENABLE=false",             // Avoid creating noise.
+			"TF_CLI_CONFIG_FILE=/tmp/integration.tfrc", // Our custom tfrc from above.
 		},
 		Labels: map[string]string{},
 	}, &container.HostConfig{
 		Binds: []string{
-			tfrcPath + ":/tmp/integration.tfrc",
-			srcPath + ":/src",
+			tfrcPath + ":/tmp/integration.tfrc", // Custom tfrc from above.
+			srcPath + ":/src",                   // Bind-mount in the repo with the built binary and templates.
 		},
 	}, nil, nil, "")
 	require.NoError(t, err, "create test deployment")
 
 	t.Logf("created container %s\n", ctr.ID)
-	t.Cleanup(func() {
+	t.Cleanup(func() { // Make sure we clean up after ourselves.
+		// TODO: also have this execute if you Ctrl+C!
 		t.Logf("stopping container %s\n", ctr.ID)
 		_ = cli.ContainerRemove(ctx, ctr.ID, container.RemoveOptions{
 			Force: true,
@@ -141,24 +179,39 @@ func setup(ctx context.Context, t *testing.T) string {
 	require.NoError(t, err, "start container")
 	t.Logf("started container %s\n", ctr.ID)
 
-	// Perform first time setup
+	// nolint:gosec // For testing only.
 	var (
 		testEmail    = "[email protected]"
 		testPassword = "InsecurePassw0rd!"
 		testUsername = "testing"
 	)
 
 	// Wait for container to come up
-	execContainer(ctx, t, ctr.ID, `until curl -s --fail http://localhost:3000/healthz; do sleep 1; done`)
+waitLoop:
+	for {
+		select {
+		case <-ctx.Done():
+			t.Fatalf("coder failed to become ready in time")
+		default:
+			_, rc := execContainer(ctx, t, ctr.ID, fmt.Sprintf(`curl -s --fail %s/api/v2/buildinfo`, localURL))
+			if rc == 0 {
+				break waitLoop
+			}
+			t.Logf("not ready yet...")
+			<-time.After(time.Second)
+		}
+	}
 	// Perform first time setup
-	execContainer(ctx, t, ctr.ID, fmt.Sprintf(`coder login http://localhost:3000 --first-user-email=%q --first-user-password=%q --first-user-trial=false --first-user-username=%q`, testEmail, testPassword, testUsername))
+	_, rc := execContainer(ctx, t, ctr.ID, fmt.Sprintf(`coder login %s --first-user-email=%q --first-user-password=%q --first-user-trial=false --first-user-username=%q`, localURL, testEmail, testPassword, testUsername))
+	require.Equal(t, 0, rc, "failed to perform first-time setup")
 	return ctr.ID
 }
 
 // execContainer executes the given command in the given container and returns
-// the output.
+// the output and the exit code of the command.
 func execContainer(ctx context.Context, t *testing.T, containerID, command string) (string, int) {
 	t.Helper()
+	t.Logf("exec container cmd: %q", command)
 	cli, err := client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation())
 	require.NoError(t, err, "connect to docker")
 	defer cli.Close()
@@ -182,13 +235,18 @@ func execContainer(ctx context.Context, t *testing.T, containerID, command strin
 	return out, execResp.ExitCode
 }
 
-func randomPort(t *testing.T) int {
-	l, err := net.Listen("tcp", "127.0.0.1:0")
-	require.NoError(t, err, "listen on random port")
-	defer func() {
-		_ = l.Close()
-	}()
-	tcpAddr, valid := l.Addr().(*net.TCPAddr)
-	require.True(t, valid, "net.Listen did not return a *net.TCPAddr")
-	return tcpAddr.Port
+// assertOutput asserts that, for each key-value pair in expected:
+// 1. actual[k] as a regex matches expected[k], and
+// 2. the set of keys of expected are not a subset of actual.
+func assertOutput(t *testing.T, expected, actual map[string]string) {
+	t.Helper()
+
+	for expectedKey, expectedValExpr := range expected {
+		actualVal := actual[expectedKey]
+		assert.Regexp(t, expectedValExpr, actualVal)
+	}
+	for actualKey := range actual {
+		_, ok := expected[actualKey]
+		assert.True(t, ok, "unexpected field in actual %q=%q", actualKey, actual[actualKey])
+	}
 }