chore: revert default Firecracker version to v1.12#2478
Conversation
The v1.14 default landed in #2415 but we are not ready to roll it out yet. Switch the infra-wide default back to v1.12.1_210cbac while we keep v1.14 available behind the feature flag.
PR SummaryMedium Risk Overview Reviewed by Cursor Bugbot for commit 39ac213. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
LGTM — straightforward one-line revert of the default Firecracker version from v1.14 back to v1.12.
Extended reasoning...
Overview
Single-line change in packages/shared/pkg/featureflags/flags.go that reassigns DefaultFirecrackerVersion from DefaultFirecackerV1_14Version to DefaultFirecackerV1_12Version. The change is a deliberate revert of the v1.14 default introduced in #2415, with v1.14 remaining accessible via FirecrackerVersionMap and per-build feature flag overrides.
Security risks
None. This is a version string constant change with no auth, crypto, or permissions implications. The version strings themselves are hardcoded and unchanged.
Level of scrutiny
Low. This is a config-level revert to a known-good state — the exact inverse of a previously merged PR. The intent is unambiguous and the blast radius is limited to the default Firecracker binary selected for new builds when no explicit override is set.
Other factors
The only finding is a pre-existing typo in the constant identifiers (Firecacker vs Firecracker), noted as a nit in the inline comments. The typo predates this PR and has no runtime impact since only the string values are used externally. It does not block approval of this revert.
| DefaultFirecackerV1_10Version = "v1.10.1_30cbb07" | ||
| DefaultFirecackerV1_12Version = "v1.12.1_210cbac" | ||
| DefaultFirecackerV1_14Version = "v1.14.1_458ca91" | ||
| DefaultFirecrackerVersion = DefaultFirecackerV1_14Version | ||
| DefaultFirecrackerVersion = DefaultFirecackerV1_12Version |
There was a problem hiding this comment.
🟡 Three exported constants use the misspelling 'Firecacker' (missing 'r'): DefaultFirecackerV1_10Version, DefaultFirecackerV1_12Version, and DefaultFirecackerV1_14Version, while DefaultFirecrackerVersion is spelled correctly. Since this PR already touches line 245 where these constants are referenced, this would be a low-risk opportunity to correct the naming.
Extended reasoning...
The typo: Three exported Go constants in the const block at lines 242-245 use Firecacker (dropping the first r) while the product name is Firecracker. The affected identifiers are DefaultFirecackerV1_10Version, DefaultFirecackerV1_12Version, and DefaultFirecackerV1_14Version. The fourth constant in the same block, DefaultFirecrackerVersion, is spelled correctly, creating an inconsistency within a four-line const block.
How it manifests: This is a naming inconsistency in the exported API surface. Any caller that tries to reference these constants by name, or any developer adding a new versioned constant who follows the existing pattern, will encounter or perpetuate the typo. IDE autocomplete and grep searches for Firecracker will miss these identifiers.
Why existing code does not prevent it: Go does not enforce any naming conventions at compile time. The string values (e.g. v1.12.1_210cbac) are spelled correctly, so runtime behavior is unaffected. The typo is purely in the Go identifier names.
Scope of impact: A codebase-wide grep confirms these three constants are only referenced within flags.go itself -- in the FirecrackerVersionMap var and the DefaultFirecrackerVersion assignment on line 245. No external callers use them by name, so renaming is a non-breaking change within this package.
Step-by-step proof:
- Open packages/shared/pkg/featureflags/flags.go, lines 242-248.
- DefaultFirecackerV1_10Version -- count the letters: F-i-r-e-c-a-c-k-e-r = Firecacker, missing the first r after e.
- Same for DefaultFirecackerV1_12Version and DefaultFirecackerV1_14Version.
- Compare against DefaultFirecrackerVersion on line 245: F-i-r-e-c-r-a-c-k-e-r = correct spelling.
- FirecrackerVersionMap on lines 247-251 references all three misspelled constants as values.
- This PR changes line 245 from DefaultFirecackerV1_14Version to DefaultFirecackerV1_12Version, directly touching the misspelled identifier -- making this PR an appropriate moment to fix the names.
How to fix: Rename DefaultFirecackerV1_10Version to DefaultFirecrackerV1_10Version, DefaultFirecackerV1_12Version to DefaultFirecrackerV1_12Version, and DefaultFirecackerV1_14Version to DefaultFirecrackerV1_14Version throughout flags.go. Since all usages are in this single file, the change is mechanical and safe.
Summary
featureflags.DefaultFirecrackerVersionback toDefaultFirecackerV1_12Version(v1.12.1_210cbac).firecracker-versionsflag and per-build overrides.Test plan
v1.12.1_210cbacwhen no explicit--firecracker/DEFAULT_FIRECRACKER_VERSIONis set.FirecrackerVersionMap/ feature flag overrides.