[PRODENG-3471] Fix Windows MCR install failing on FIPS channels - #640
Conversation
DOCKER_VERSION was hardcoded to "latest" in the InstallMCR command. For FIPS channels, install.ps1 appends +fips to produce "latest+fips", but docker-latest+fips.zip is never published — only versioned artifacts (docker-29.2.1+fips.zip) exist. Without DOCKER_VERSION set, install.ps1 fetches the channel's index.json and calls getNumericallyHigherVersion, which returns the correct pinned version. This works for all channel types: versioned FIPS (stable-29.2.1/fips), rolling FIPS (stable/fips), and non-FIPS channels alike. Fixes PRODENG-3471
Exercises a ubuntu24 manager + windows_2022 worker cluster with MCR channel stable-29.2.1/fips. Both Linux (APT component ubuntu/dists/noble/stable-29.2.1/fips/) and Windows (win/static/stable-29.2.1/fips/index.json) have published artifacts for this channel. The test directly covers the regression: without the fix, the Windows installer would attempt docker-latest+fips.zip (which does not exist); with the fix it resolves docker-29.2.1.1+fips.zip from the channel index.
Triggered by the 'smoke-windows-fips' PR label (or 'smoke-test' to run all suites, or on push to main).
There was a problem hiding this comment.
Pull request overview
This PR fixes Windows MCR installation on FIPS channels by no longer forcing DOCKER_VERSION=latest, allowing the upstream install.ps1 script to resolve an appropriate versioned +fips artifact via the channel index.json.
Changes:
- Remove hardcoded
DOCKER_VERSION=latestfrom the Windows MCR install command so the installer script can select a valid versioned artifact. - Add a new FIPS-focused smoke test (
TestFIPSCluster) and wire it into Make and GitHub Actions as an on-demand smoke job.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
pkg/configurer/windows.go |
Drops DOCKER_VERSION from the Windows MCR install invocation so FIPS channels resolve to versioned artifacts. |
test/smoke/smoke_test.go |
Adds a smoke test covering an Ubuntu manager + Windows 2022 worker using a FIPS MCR channel. |
Makefile |
Adds a smoke-fips target to run the new FIPS smoke test. |
.github/workflows/smoke-tests.yaml |
Adds a smoke-fips workflow job gated by labels / pushes, invoking make smoke-fips. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ks up v0.1.6 The terraform-mirantis-modules/provision-aws module gained ubuntu_22.04_fips in v0.1.6 but the Terraform Registry has not yet indexed that release. Add it to lib_local_platform_definitions in examples/terraform/aws-simple/platform.tf as a temporary override with a TODO to remove once the module source is bumped to >= v0.1.6. Also wire Ubuntu22FIPS into test/platforms.go and update TestFIPSCluster to use it as the manager node.
|
The Written by AI: claude-sonnet-4-5 |
windows_2022 consistently fails to open port 5986 (WinRM HTTPS) within the retry window. Switching to windows_2025 to determine if the issue is AMI-specific.
e2fb317 to
4f2d630
Compare
Address Copilot review on #640: the cmd.exe install chain interpolated engineConfig.RepoURL and engineConfig.Channel into bare `set VAR=value` assignments. Use the atomic `set "VAR=value"` form so cmd metacharacters (&, |, ^, <, >) in either value cannot break the command or inject extra commands.
The smoke-fips Windows 2025 worker rejected every WinRM connection with "401 - invalid content type" (wrong Basic-auth credentials), so launchpad never reached the MCR install step under test. Root cause: Server 2025 uses EC2Launch v2, which sets a random Administrator password via its PreReady setAdminAccount task. The user-data script runs in the later UserData stage and must override it, but the legacy ADSI WinNT SetPassword silently failed on Server 2025 Core (user data runs with ErrorActionPreference=Continue), leaving the random password active. setAdminAccount cannot be used from user data (PreReady-only), so set the password in-script with the modern LocalAccounts API (Set-LocalUser / Enable-LocalUser) and -ErrorAction Stop to fail loudly if it does not apply. Verification pending a smoke-fips CI run (requires AWS provisioning).
|
Pushed two fixes to The Root cause: Server 2025 runs EC2Launch v2, which sets a random Administrator password via its Changes:
The Administrator-password fix is a strong candidate but unverified until this Written by AI: claude-opus-4-8 |
smoke-fips has failed twice now with an identical WinRM 401 on the Windows 2025 worker, even after the Administrator-password fix -- but terraform.Destroy tears the instance down at the end of every run, so there's never anything left to inspect. Add dumpConsoleOutputOnFailure, deferred after terraform.Destroy (defers run LIFO, so this fires first) and gated on t.Failed(): best-effort DescribeInstances + GetConsoleOutput for every instance tagged with this run's stack name, logged before the stack is destroyed. Note EC2 console output is not guaranteed to carry Windows user-data script output (EC2Launch v2 normally logs to agent.log instead); this is the cheapest diagnostic to try first, and an empty result is itself informative -- it would point at SSM/RDP into the box as the next step.
|
Ran `smoke-fips` 3 additional times on this exact commit (`7aae999`) after it failed 120/120 WinRM attempts on 2026-06-19. All 3 reruns today connected via WinRM on the first attempt, zero `401`s, full `--- PASS: TestFIPSCluster` (~19-21m each):
No code changed between the failing run and these -- same commit. 3/3 clean first-attempt connections rules out a persistent bug in the Set-LocalUser fix; the original 401 was most likely an AWS-side EC2Launch v2 issue (the Windows 2025 AMI resolves via `most_recent = true`, so a month-later run can pick up a newer/patched build) rather than a flaw in `7aae999`. Considering this fix validated. Still just needs the required human approval to merge. Written by AI: claude-sonnet-5 |
What
Remove hardcoded
DOCKER_VERSION=latestfrom the Windows MCR install command, lettinginstall.ps1resolve the version from the channel'sindex.json.Why
For FIPS channels,
install.ps1appends+fipstoDOCKER_VERSION, producingdocker-latest+fips.zip— a file that has never been published. Only versioned artifacts (docker-29.2.1+fips.zip) exist on FIPS channels.How
version := "latest"and theDOCKER_VERSIONenv var from theset … && powershellinstall command inInstallMCRDOCKER_VERSIONset,install.ps1fetches{repoURL}/win/static/{channel}/index.jsonand callsgetNumericallyHigherVersion, returning the correct pinned artifact (e.g.29.2.1.1+fips)Testing
go build ./...)docker-latest+fips.zipabsent,docker-29.2.1.1+fips.zippresent;index.jsonlists versioned entries thatgetNumericallyHigherVersioncorrectly selectsLinks
Checklist
Written by AI: claude-sonnet-4-5