Merge #99500 #99701

99500: workload/kv: fully adopt hash-sharded syntax r=ajwerner a=ajwerner This syntax has been stable since 22.1. Epic: none Release note: None 99701: build: tweak openssl config for FIPS buils r=rickystewart a=rail Previously, for FIPS-enabled builds, the OpenSSL configs were untouched, and as a result, some of the configuration options were not FIPS-compliant. This PR tweaks the configs to be FIPS-compliant. Epic: DEVINF-478 Release note: None Co-authored-by: ajwerner <[email protected]> Co-authored-by: Rail Aliiev <[email protected]>
cockroachdb · Mar 27, 2023 · 3f113fc · 3f113fc
3 parents 97fee54 + eaffd81 + a149d38
commit 3f113fc
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 58 deletions.
diff --git a/build/deploy/Dockerfile b/build/deploy/Dockerfile
@@ -1,5 +1,5 @@
 FROM registry.access.redhat.com/ubi8/ubi-minimal
-ARG additional_packages
+ARG fips_enabled
 
 # For deployment, we need the following additionally installed:
 # tzdata - for time zone functions; reinstalled to replace the missing
@@ -8,8 +8,23 @@ ARG additional_packages
 # tar - used by kubectl cp
 RUN microdnf update -y \
     && rpm --erase --nodeps tzdata \
-    && microdnf install tzdata hostname tar gzip xz $additional_packages -y \
+    && microdnf install tzdata hostname tar gzip xz -y \
     && rm -rf /var/cache/yum
+# FIPS mode requires the `openssl` package installed. Also we need to temporarily
+# install the `crypto-policies-scripts` packege to tweak some configs. Because
+# `microdnf` doesn't support `autoremove`, we need to record the list of
+# packages before and after, and remove the installed ones afterward.
+RUN if [ "$fips_enabled" == "1" ]; then \
+    microdnf install -y openssl && \
+    rpm -qa | sort > /before.txt && \
+    microdnf install crypto-policies-scripts && \
+    fips-mode-setup --enable --no-bootcfg && \
+    rpm -qa | sort > /after.txt && \
+    microdnf remove -y $(comm -13 /before.txt /after.txt) && \
+    microdnf clean all && \
+    rm -rf /var/cache/yum /before.txt /after.txt; \
+    fi
+
 
 RUN mkdir /usr/local/lib/cockroach /cockroach /licenses /docker-entrypoint-initdb.d
 COPY cockroach.sh cockroach /cockroach/

diff --git a/build/teamcity/internal/release/process/make-and-publish-build-artifacts.sh b/build/teamcity/internal/release/process/make-and-publish-build-artifacts.sh
@@ -112,7 +112,7 @@ cp --recursive licenses "build/deploy-${platform_name}"
 mv build/deploy-${platform_name}/lib/* build/deploy-${platform_name}/
 rmdir build/deploy-${platform_name}/lib
 
-docker build --no-cache --pull --platform "linux/amd64" --tag="${gcr_tag_fips}" --build-arg additional_packages=openssl "build/deploy-${platform_name}"
+docker build --no-cache --pull --platform "linux/amd64" --tag="${gcr_tag_fips}" --build-arg fips_enabled=1 "build/deploy-${platform_name}"
 docker push "$gcr_tag_fips"
 
 tc_end_block "Make and push FIPS docker image"

diff --git a/build/teamcity/internal/release/process/publish-cockroach-release.sh b/build/teamcity/internal/release/process/publish-cockroach-release.sh
@@ -169,7 +169,7 @@ docker build \
   --platform="linux/amd64" \
   --tag="${dockerhub_tag_fips}" \
   --tag="${gcr_tag_fips}" \
-  --build-arg additional_packages=openssl \
+  --build-arg fips_enabled=1 \
   "build/deploy-${platform_name}"
 docker push "$gcr_tag_fips"
 docker push "$dockerhub_tag_fips"

diff --git a/pkg/workload/kv/kv.go b/pkg/workload/kv/kv.go
@@ -47,19 +47,13 @@ const (
 		v BYTES NOT NULL,
 		INDEX (v)
 	)`
-	// TODO(ajwerner): Change this to use the "easier" hash sharded index syntax once that
-	// is in.
 	shardedKvSchema = `(
-		k BIGINT NOT NULL,
-		v BYTES NOT NULL,
-		shard INT4 AS (mod(k, %d)) STORED CHECK (%s),
-		PRIMARY KEY (shard, k)
+		k BIGINT NOT NULL PRIMARY KEY USING HASH WITH (bucket_count = %d),
+		v BYTES NOT NULL
 	)`
 	shardedKvSchemaWithIndex = `(
-		k BIGINT NOT NULL,
+		k BIGINT NOT NULL PRIMARY KEY USING HASH WITH (bucket_count = %d,
 		v BYTES NOT NULL,
-		shard INT4 AS (mod(k, %d)) STORED CHECK (%s),
-		PRIMARY KEY (shard, k),
 		INDEX (v)
 	)`
 )
@@ -200,7 +194,7 @@ ALTER TABLE kv ADD COLUMN e enum_type NOT NULL AS ('v') STORED;`)
 				return errors.New("'sequential' and 'zipfian' cannot both be enabled")
 			}
 			if w.shards > 0 && !(w.sequential || w.zipfian) {
-				return errors.New("'shards' only work with 'sequential' or 'zipfian' key distributions")
+				return errors.New("'num-shards' only work with 'sequential' or 'zipfian' key distributions")
 			}
 			if w.readPercent+w.spanPercent+w.delPercent > 100 {
 				return errors.New("'read-percent', 'span-percent' and 'del-precent' combined exceed 100%")
@@ -277,16 +271,7 @@ func (w *kv) Tables() []workload.Table {
 		if w.secondaryIndex {
 			schema = shardedKvSchemaWithIndex
 		}
-		checkConstraint := strings.Builder{}
-		checkConstraint.WriteString(`shard IN (`)
-		for i := 0; i < w.shards; i++ {
-			if i != 0 {
-				checkConstraint.WriteString(",")
-			}
-			fmt.Fprintf(&checkConstraint, "%d", i)
-		}
-		checkConstraint.WriteString(")")
-		table.Schema = fmt.Sprintf(schema, w.shards, checkConstraint.String())
+		table.Schema = fmt.Sprintf(schema, w.shards)
 	} else {
 		if w.secondaryIndex {
 			table.Schema = kvSchemaWithIndex
@@ -372,15 +357,7 @@ func (w *kv) Ops(
 
 	// Read statement
 	var buf strings.Builder
-	if w.shards == 0 {
-		buf.WriteString(`SELECT k, v FROM kv WHERE k IN (`)
-		for i := 0; i < w.batchSize; i++ {
-			if i > 0 {
-				buf.WriteString(", ")
-			}
-			fmt.Fprintf(&buf, `$%d`, i+1)
-		}
-	} else if w.enum {
+	if w.enum {
 		buf.WriteString(`SELECT k, v, e FROM kv WHERE k IN (`)
 		for i := 0; i < w.batchSize; i++ {
 			if i > 0 {
@@ -389,17 +366,12 @@ func (w *kv) Ops(
 			fmt.Fprintf(&buf, `$%d`, i+1)
 		}
 	} else {
-		// TODO(ajwerner): We're currently manually plumbing down the computed shard column
-		// since the optimizer doesn't yet support deriving values of computed columns
-		// when all the columns they reference are available. See
-		// https://github.com/cockroachdb/cockroach/issues/39340#issuecomment-535338071
-		// for details. Remove this once that functionality is added.
-		buf.WriteString(`SELECT k, v FROM kv WHERE (shard, k) in (`)
+		buf.WriteString(`SELECT k, v FROM kv WHERE k IN (`)
 		for i := 0; i < w.batchSize; i++ {
 			if i > 0 {
 				buf.WriteString(", ")
 			}
-			fmt.Fprintf(&buf, `(mod($%d, %d), $%d)`, i+1, w.shards, i+1)
+			fmt.Fprintf(&buf, `$%d`, i+1)
 		}
 	}
 	buf.WriteString(`)`)
@@ -420,9 +392,6 @@ func (w *kv) Ops(
 	// Select for update statement
 	var sfuStmtStr string
 	if w.writesUseSelectForUpdate {
-		if w.shards != 0 {
-			return workload.QueryLoad{}, fmt.Errorf("select for update in kv requires shard=0")
-		}
 		buf.Reset()
 		buf.WriteString(`SELECT k, v FROM kv WHERE k IN (`)
 		for i := 0; i < w.batchSize; i++ {
@@ -450,22 +419,12 @@ func (w *kv) Ops(
 
 	// Del statement
 	buf.Reset()
-	if w.shards == 0 {
-		buf.WriteString(`DELETE FROM kv WHERE k IN (`)
-		for i := 0; i < w.batchSize; i++ {
-			if i > 0 {
-				buf.WriteString(", ")
-			}
-			fmt.Fprintf(&buf, `$%d`, i+1)
-		}
-	} else {
-		buf.WriteString(`DELETE FROM kv WHERE (shard, k) in (`)
-		for i := 0; i < w.batchSize; i++ {
-			if i > 0 {
-				buf.WriteString(", ")
-			}
-			fmt.Fprintf(&buf, `(mod($%d, %d), $%d)`, i+1, w.shards, i+1)
+	buf.WriteString(`DELETE FROM kv WHERE k IN (`)
+	for i := 0; i < w.batchSize; i++ {
+		if i > 0 {
+			buf.WriteString(", ")
 		}
+		fmt.Fprintf(&buf, `$%d`, i+1)
 	}
 	buf.WriteString(`)`)
 	delStmtStr := buf.String()