roachtest: Add LDAP conn. latency test via roachtest

[sanchit-CRL]

No test previously existed to compute and monitor LDAP
connection latency

Created a roachtest which leverages the workload
to get the stats for LDAP connection latency
The test provisions an openLDAP service and it's user jdoe
which is authenticated on the CRDB via LDAP.
The test

• provisions openLDAP with TLS connection
• Creates a user named jdoe into CRDB
• Sets the HBA conf and custom CA into the cluster settings
• runs the workload binary to compute the connection latency

Epic: CRDB-40412
Fixes: #127358

Release note: None

[cockroach-teamcity]

This change is 

[souravcrl]

Reviewed 6 of 6 files at r1, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @pritesh-lahoti and @sanchit-CRL)

---

-- commits line 2 at r1:
nit: since the title is prefixed with a package name, we don't capitalize the title. The text that follows after colon(:) is not capitalized.

https://cockroachlabs.atlassian.net/wiki/spaces/CRDB/pages/73072807/Git+Commit+Messages#Commit-title

---

-- commits line 13 at r1:
nit: spell error

Suggestion:

  * Sets the HBA conf and custom CA into the cluster settings

---

-- commits line 17 at r1:
nit: we need to provide a release note at the end of the commit even if there is none, as that helps docs team to go through the commits for documentation.
https://cockroachlabs.atlassian.net/wiki/spaces/CRDB/pages/186548364/Release+notes#High-level-rules

Suggestion:

  Release Note: none

---

build/teamcity/cockroach/nightlies/roachtest_nightly_gce.sh line 10 at r1 (raw file):

source "$dir/teamcity-bazel-support.sh"  # For run_bazel

BAZEL_SUPPORT_EXTRA_DOCKER_ARGS="-e LITERAL_ARTIFACTS_DIR=$root/artifacts -e BUILD_VCS_NUMBER -e CLOUD -e COCKROACH_DEV_LICENSE -e TESTS -e COUNT -e GITHUB_API_TOKEN -e GITHUB_ORG -e GITHUB_REPO -e GOOGLE_EPHEMERAL_CREDENTIALS -e GOOGLE_KMS_KEY_A -e GOOGLE_KMS_KEY_B -e GOOGLE_CREDENTIALS_ASSUME_ROLE -e GOOGLE_SERVICE_ACCOUNT -e SLACK_TOKEN -e TC_BUILDTYPE_ID -e TC_BUILD_BRANCH -e TC_BUILD_ID -e TC_SERVER_URL -e SELECT_PROBABILITY -e COCKROACH_RANDOM_SEED -e ROACHTEST_ASSERTIONS_ENABLED_SEED -e ROACHTEST_FORCE_RUN_INVALID_RELEASE_BRANCH -e GRAFANA_SERVICE_ACCOUNT_JSON -e GRAFANA_SERVICE_ACCOUNT_AUDIENCE -e ARM_PROBABILITY -e USE_SPOT -e SELECTIVE_TESTS -e SFUSER -e SFPASSWORD -e SIDE_EYE_API_TOKEN -e COCKROACH_EA_PROBABILITY -e LDAP_TEST_USER_PASSWORD" \

nit: for SnowFlake I see we use both username and password credentials. It should be more secure to pass in the full credential here.

---

pkg/cmd/roachtest/tests/connection_latency.go line 170 at r1 (raw file):

		Cluster: r.MakeClusterSpec(numNodes+1,
			spec.WorkloadNode(), spec.GCEZones(regionUsCentral)),
		RequiresLicense:            false,

nit: this is required since LDAP feature is entirely behind cockroach enterprise license. Setting this to true sets the COCKROACH_DEV_LICENSE for the test.

---

pkg/cmd/roachtest/tests/connection_latency.go line 184 at r1 (raw file):

	ldapTestUserPassword, ok := os.LookupEnv("LDAP_TEST_USER_PASSWORD")
	if !ok {
		t.L().Printf("environment variable LDAP_TEST_USER_PASSWORD is not set")

nit: this should fail the test if environment variable is not set. We do this for github API token here:

cockroach/pkg/cmd/roachtest/tests/blocklist_test.go

Lines 80 to 82 in 0b52650

	if !ok {
	t.Fatalf("GitHub API token environment variable %s is not set", githubAPITokenEnv)
	}

---

pkg/cmd/roachtest/tests/connection_latency.go line 233 at r1 (raw file):

	// Currently the test runs for a single region, this is for future use
	// if the test has to be extended for multiple regions.
	if numZones > 1 {

This would probably need change to HBA as each connection for a different region will need to be mapped to a separate LDAP server. This seems beyond scope of current work, and can be left out I feel.

---

pkg/cmd/roachtest/testdata/ldap_authentication_hba_conf line 2 at r1 (raw file):

host    all           roachprod           0.0.0.0/0          password
host    all           all            all                 ldap ldapserver=crlcloud.dev ldapport=636 \"ldapbasedn=OU=AADDC Users,DC=crlcloud,DC=dev\" \"ldapbinddn=CN=Test User,OU=AADDC Users,DC=crlcloud,DC=dev\" ldapbindpasswd=~cockroachTest ldapsearchattribute=sAMAccountName \"ldapsearchfilter=(memberOf=CN=crl-crlcloud-dev-domain-sync-users,OU=AADDC Users,DC=crlcloud,DC=dev)\"

nit: we should also redact the ldap password and username from HBA conf. Effectively this could be set from the same environment variable used for the latency test.

[pritesh-lahoti]

Reviewed 6 of 6 files at r1, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @sanchit-CRL and @souravcrl)

---

pkg/cmd/roachtest/tests/connection_latency.go line 204 at r1 (raw file):

	urlTemplate := func(host string) string {
		return fmt.Sprintf("postgresql://testuser:%s@%s:{pgport:1}", ldapTestUserPassword, host)

nit: can move testuser to a var/constant and then use it here and at prepareSQLUserForLDAP(ctx, t, c, "testuser").

---

pkg/cmd/roachtest/tests/connection_latency.go line 233 at r1 (raw file):

Previously, souravcrl wrote…

This would probably need change to HBA as each connection for a different region will need to be mapped to a separate LDAP server. This seems beyond scope of current work, and can be left out I feel.

I agree.

---

pkg/cmd/roachtest/tests/connection_latency.go line 280 at r1 (raw file):

		c.Node(1), roachprod.PGURLOptions{})
	require.NoError(t, err)
	conn, err := pgx.Connect(ctx, pgURL[0])

Should we add require.NotEmpty(t, pgURL) before accessing pgURL[0]?

---

pkg/cmd/roachtest/tests/registry.go line 46 at r1 (raw file):

	registerClusterReplicationDisconnect(r)
	registerConnectionLatencyTest(r)
	registerLDAPConnectionLatencyTest(r)

nit: this may need to be arranged in lexicographical order.

[sanchit-CRL]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @csgourav, @pritesh-lahoti, @shailendra-patel, and @souravcrl)

---

pkg/cmd/roachtest/tests/connection_latency.go line 170 at r1 (raw file):

Previously, souravcrl wrote…

nit: this is required since LDAP feature is entirely behind cockroach enterprise license. Setting this to true sets the COCKROACH_DEV_LICENSE for the test.

the field has been removed from the roachtest spec now

---

pkg/cmd/roachtest/tests/connection_latency.go line 233 at r1 (raw file):

Previously, pritesh-lahoti wrote…

I agree.

Done.

---

pkg/cmd/roachtest/tests/connection_latency.go line 280 at r1 (raw file):

Previously, pritesh-lahoti wrote…

Should we add require.NotEmpty(t, pgURL) before accessing pgURL[0]?

Done.

---

pkg/cmd/roachtest/testdata/ldap_authentication_hba_conf line 2 at r1 (raw file):

Previously, souravcrl wrote…

nit: we should also redact the ldap password and username from HBA conf. Effectively this could be set from the same environment variable used for the latency test.

Not required anymore as the password is for a user created locally

[pritesh-lahoti]

Reviewed 7 of 8 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @csgourav, @shailendra-patel, @souravcrl, and @vidit-bhat)

[sanchit-CRL]

bors r+

[craig]

Build succeeded:

• linux_amd64_fips_build
• local_roachtest
• macos_arm64_build
• check_generated_code
• macos_amd64_build
• acceptance
• windows_build
• examples_orms
• linux_arm64_build
• docker_image_amd64
• unit_tests
• lint
• local_roachtest_fips
• linux_amd64_build