mirror of
https://github.com/mattermost/mattermost.git
synced 2026-10-29 18:53:04 +08:00
Generate instead of hard-coding test passwords, enforce new minimum for FIPS, shard CI, fix FIPS builds (#35905)
* Replace hardcoded test passwords with model.NewTestPassword() Add model.NewTestPassword() utility that generates 14+ character passwords meeting complexity requirements for FIPS compliance. Replace all short hardcoded test passwords across the test suite with calls to this function. * Enforce FIPS compliance for passwords and HMAC keys FIPS OpenSSL requires HMAC keys to be at least 14 bytes. PBKDF2 uses the password as the HMAC key internally, so short passwords cause PKCS5_PBKDF2_HMAC to fail. - Add FIPSEnabled and PasswordFIPSMinimumLength build-tag constants - Raise the password minimum length floor to 14 when compiled with requirefips, applied in SetDefaults only when unset and validated independently in IsValid - Return ErrMismatchedHashAndPassword for too-short passwords in PBKDF2 CompareHashAndPassword rather than a cryptic OpenSSL error - Validate atmos/camo HMAC key length under FIPS and lengthen test keys accordingly - Adjust password validation tests to use PasswordFIPSMinimumLength so they work under both FIPS and non-FIPS builds * CI: shard FIPS test suite and extract merge template Run FIPS tests on PRs that touch go.mod or have 'fips' in the branch name. Shard FIPS tests across 4 runners matching the normal Postgres suite. Extract the test result merge logic into a reusable workflow template to deduplicate the normal and FIPS merge jobs. * more * Fix email test helper to respect FIPS minimum password length * Fix test helpers to respect FIPS minimum password length * Remove unnecessary "disable strict password requirements" blocks from test helpers * Fix CodeRabbit review comments on PR #35905 - Add server-test-merge-template.yml to server-ci.yml pull_request.paths so changes to the reusable merge workflow trigger Server CI validation - Skip merge-postgres-fips-test-results job when test-postgres-normal-fips was skipped, preventing failures due to missing artifacts - Set guest.Password on returned guest in CreateGuestAndClient helper to keep contract consistent with CreateUserWithClient - Use shared LowercaseLetters/UppercaseLetters/NUMBERS/PasswordFIPSMinimumLength constants in NewTestPassword() to avoid drift if FIPS floor changes https://claude.ai/code/session_01HmE9QkZM3cAoXn2J7XrK2f * Rename FIPS test artifact to match server-ci-report pattern The server-ci-report job searches for artifacts matching "*-test-logs", so rename from postgres-server-test-logs-fips to postgres-server-fips-test-logs to be included in the report. --------- Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
Jesse Hallam
@@ -14,6 +14,7 @@ on:
|
||||
- "server/**"
|
||||
- ".github/workflows/server-ci.yml"
|
||||
- ".github/workflows/server-test-template.yml"
|
||||
- ".github/workflows/server-test-merge-template.yml"
|
||||
- ".github/workflows/mmctl-test-template.yml"
|
||||
- "!server/build/Dockerfile.buildenv"
|
||||
- "!server/build/Dockerfile.buildenv-fips"
|
||||
@@ -32,6 +33,7 @@ jobs:
|
||||
runs-on: ubuntu-22.04
|
||||
outputs:
|
||||
version: ${{ steps.calculate.outputs.GO_VERSION }}
|
||||
gomod-changed: ${{ steps.changed-files.outputs.any_changed }}
|
||||
steps:
|
||||
- name: Checkout mattermost project
|
||||
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
|
||||
@@ -39,6 +41,12 @@ jobs:
|
||||
id: calculate
|
||||
working-directory: server/
|
||||
run: echo GO_VERSION=$(cat .go-version) >> "${GITHUB_OUTPUT}"
|
||||
- name: Check for go.mod changes
|
||||
id: changed-files
|
||||
uses: tj-actions/changed-files@22103cc46bda19c2b464ffe86db46df6922fd323 # v47.0.5
|
||||
with:
|
||||
files: |
|
||||
**/go.mod
|
||||
check-mocks:
|
||||
name: Check mocks
|
||||
needs: go
|
||||
@@ -225,89 +233,42 @@ jobs:
|
||||
name: Merge Postgres Test Results
|
||||
needs: test-postgres-normal
|
||||
if: always()
|
||||
runs-on: ubuntu-22.04
|
||||
steps:
|
||||
- name: Download all shard artifacts
|
||||
uses: actions/download-artifact@70fc10c6e5e1ce46ad2ea6f2b72d43f7d47b13c3 # v8.0.0
|
||||
with:
|
||||
pattern: postgres-server-test-logs-shard-*
|
||||
path: shards
|
||||
|
||||
- name: Setup Node for junit-report-merger
|
||||
uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f # v6.3.0
|
||||
with:
|
||||
node-version: 20
|
||||
|
||||
- name: Merge JUnit reports
|
||||
run: |
|
||||
npx junit-report-merger@7.0.0 merged-report.xml "shards/*/report.xml"
|
||||
|
||||
# Prepare files with expected names for the report workflow
|
||||
- name: Prepare merged artifact
|
||||
run: |
|
||||
mkdir -p merged
|
||||
cp merged-report.xml merged/report.xml
|
||||
# Find the first shard artifact that has test-name and pr-number
|
||||
for dir in shards/postgres-server-test-logs-shard-*/; do
|
||||
if [[ -f "${dir}test-name" ]]; then
|
||||
cp "${dir}test-name" merged/test-name
|
||||
cp "${dir}pr-number" merged/pr-number
|
||||
break
|
||||
fi
|
||||
done
|
||||
|
||||
# Upload merged artifact with the name the report workflow expects
|
||||
- name: Upload merged test logs
|
||||
uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0
|
||||
with:
|
||||
name: postgres-server-test-logs
|
||||
path: merged/
|
||||
|
||||
# Save merged timing data for future shard balancing.
|
||||
# Includes both JUnit XML (package-level) and gotestsum.json (per-test).
|
||||
- name: Prepare timing cache
|
||||
id: timing-prep
|
||||
run: |
|
||||
mkdir -p server
|
||||
# Only save timing cache if we have real data (merged report > 1KB).
|
||||
# Failed runs produce tiny/empty files that would overwrite good data.
|
||||
if [[ -f merged-report.xml && $(stat -c%s merged-report.xml) -gt 1024 ]]; then
|
||||
cp merged-report.xml server/prev-report.xml
|
||||
# Merge gotestsum.json (JSONL) from all shards for per-test timing
|
||||
cat shards/*/gotestsum.json > server/prev-gotestsum.json 2>/dev/null || true
|
||||
echo "has_timing=true" >> "$GITHUB_OUTPUT"
|
||||
else
|
||||
echo "Skipping timing cache — merged report too small or missing"
|
||||
echo "has_timing=false" >> "$GITHUB_OUTPUT"
|
||||
fi
|
||||
|
||||
- name: Save test timing cache
|
||||
# Only save timing on the default branch. PR branches restore from
|
||||
# master via restore-keys prefix match. Timing data is stable day-to-day
|
||||
# so this eliminates cache misses on first PR runs.
|
||||
if: steps.timing-prep.outputs.has_timing == 'true' && github.ref_name == github.event.repository.default_branch
|
||||
uses: actions/cache/save@5a3ec84eff668545956fd18022155c47e93e2684 # v4.2.3
|
||||
with:
|
||||
path: |
|
||||
server/prev-report.xml
|
||||
server/prev-gotestsum.json
|
||||
key: server-test-timing-master-${{ github.run_id }}
|
||||
uses: ./.github/workflows/server-test-merge-template.yml
|
||||
with:
|
||||
artifact-pattern: postgres-server-test-logs-shard-*
|
||||
artifact-name: postgres-server-test-logs
|
||||
save-timing-cache: true
|
||||
|
||||
test-postgres-normal-fips:
|
||||
# FIPS compile check already runs in enterprise CI for PRs.
|
||||
# Only run full FIPS tests on pushes to the default branch.
|
||||
if: github.event_name == 'push'
|
||||
name: Postgres (FIPS)
|
||||
# Always run on pushes to master/release branches.
|
||||
# For PRs, run when the branch name contains "fips" or any go.mod was changed.
|
||||
if: github.event_name == 'push' || contains(github.head_ref, 'fips') || needs.go.outputs.gomod-changed == 'true'
|
||||
name: Postgres FIPS (shard ${{ matrix.shard }})
|
||||
needs: go
|
||||
strategy:
|
||||
fail-fast: false
|
||||
matrix:
|
||||
shard: [0, 1, 2, 3]
|
||||
uses: ./.github/workflows/server-test-template.yml
|
||||
secrets: inherit
|
||||
with:
|
||||
name: Postgres
|
||||
name: "Postgres FIPS (shard ${{ matrix.shard }})"
|
||||
datasource: postgres://mmuser:mostest-fips-test@postgres:5432/mattermost_test?sslmode=disable&connect_timeout=10
|
||||
drivername: postgres
|
||||
logsartifact: postgres-server-test-logs
|
||||
logsartifact: "postgres-server-fips-test-logs-shard-${{ matrix.shard }}"
|
||||
go-version: ${{ needs.go.outputs.version }}
|
||||
fips-enabled: true
|
||||
shard-index: ${{ matrix.shard }}
|
||||
shard-total: 4
|
||||
merge-postgres-fips-test-results:
|
||||
name: Merge Postgres FIPS Test Results
|
||||
needs: test-postgres-normal-fips
|
||||
if: needs.test-postgres-normal-fips.result != 'skipped'
|
||||
uses: ./.github/workflows/server-test-merge-template.yml
|
||||
with:
|
||||
artifact-pattern: postgres-server-fips-test-logs-shard-*
|
||||
artifact-name: postgres-server-fips-test-logs
|
||||
|
||||
test-coverage:
|
||||
name: Generate Test Coverage
|
||||
# Disabled: Running out of memory and causing spurious failures.
|
||||
|
||||
@@ -0,0 +1,89 @@
|
||||
name: Server Test Merge Template
|
||||
|
||||
on:
|
||||
workflow_call:
|
||||
inputs:
|
||||
artifact-pattern:
|
||||
description: "Glob pattern to download shard artifacts"
|
||||
required: true
|
||||
type: string
|
||||
artifact-name:
|
||||
description: "Name for the merged output artifact"
|
||||
required: true
|
||||
type: string
|
||||
save-timing-cache:
|
||||
description: "Whether to save timing cache for future shard balancing"
|
||||
required: false
|
||||
type: boolean
|
||||
default: false
|
||||
|
||||
jobs:
|
||||
merge:
|
||||
name: Merge
|
||||
if: always()
|
||||
runs-on: ubuntu-22.04
|
||||
steps:
|
||||
- name: Download all shard artifacts
|
||||
uses: actions/download-artifact@70fc10c6e5e1ce46ad2ea6f2b72d43f7d47b13c3 # v8.0.0
|
||||
with:
|
||||
pattern: ${{ inputs.artifact-pattern }}
|
||||
path: shards
|
||||
|
||||
- name: Merge JUnit reports
|
||||
run: |
|
||||
python3 -c "
|
||||
import glob, sys
|
||||
from xml.etree import ElementTree as ET
|
||||
|
||||
root = ET.Element('testsuites')
|
||||
for path in sorted(glob.glob('shards/*/report.xml')):
|
||||
tree = ET.parse(path)
|
||||
r = tree.getroot()
|
||||
if r.tag == 'testsuites':
|
||||
root.extend(r)
|
||||
else:
|
||||
root.append(r)
|
||||
|
||||
ET.ElementTree(root).write('merged-report.xml', xml_declaration=True, encoding='UTF-8')
|
||||
"
|
||||
|
||||
- name: Prepare merged artifact
|
||||
run: |
|
||||
mkdir -p merged
|
||||
cp merged-report.xml merged/report.xml
|
||||
for dir in shards/*/; do
|
||||
if [[ -f "${dir}test-name" ]]; then
|
||||
cp "${dir}test-name" merged/test-name
|
||||
cp "${dir}pr-number" merged/pr-number
|
||||
break
|
||||
fi
|
||||
done
|
||||
|
||||
- name: Upload merged test logs
|
||||
uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0
|
||||
with:
|
||||
name: ${{ inputs.artifact-name }}
|
||||
path: merged/
|
||||
|
||||
- name: Prepare timing cache
|
||||
if: inputs.save-timing-cache
|
||||
id: timing-prep
|
||||
run: |
|
||||
mkdir -p server
|
||||
if [[ -f merged-report.xml && $(stat -c%s merged-report.xml) -gt 1024 ]]; then
|
||||
cp merged-report.xml server/prev-report.xml
|
||||
cat shards/*/gotestsum.json > server/prev-gotestsum.json 2>/dev/null || true
|
||||
echo "has_timing=true" >> "$GITHUB_OUTPUT"
|
||||
else
|
||||
echo "Skipping timing cache — merged report too small or missing"
|
||||
echo "has_timing=false" >> "$GITHUB_OUTPUT"
|
||||
fi
|
||||
|
||||
- name: Save test timing cache
|
||||
if: inputs.save-timing-cache && steps.timing-prep.outputs.has_timing == 'true' && github.ref_name == github.event.repository.default_branch
|
||||
uses: actions/cache/save@5a3ec84eff668545956fd18022155c47e93e2684 # v4.2.3
|
||||
with:
|
||||
path: |
|
||||
server/prev-report.xml
|
||||
server/prev-gotestsum.json
|
||||
key: server-test-timing-master-${{ github.run_id }}
|
||||
Reference in New Issue
Block a user