Commit b69113d
Support explicit CI git checkout and bundle Docker CLI (#2812)
* feat(ci): support explicit git clone checkout
* fix(docs): satisfy skill editorconfig
* chore(ci): validate editorconfig before commit
* fix(ci): install atmos for pre-commit validation
* fix(ci): use validate-capable atmos release
* test(docker): expect bundled docker cli
* fix(ci): mirror Docker Hub pulls through Google
* fix(test): resolve CI toolchain cache binaries
* fix(pre-commit): remove Go build hook
* test(git-clone): cover CI-mode resolution edge cases
Patch coverage was 82.71% (below the 85% target) with the gaps
concentrated in parseCloneFlags/resolveCICloneMode's flag-parse-error
path, the unset-ATMOS_CI-and-unset-flag default path, and root.go's
gitCloneBootstrapCIMode explicit --ci=true and invalid-ATMOS_CI branches.
* fix(tests): close TOCTOU race in Terraform/OpenTofu binary resolution
TestExecuteTerraform_Version and TestYamlFuncTerraformOutput each resolved
the tofu/terraform binary via RequireTofu/RequireTerraform, then made a
second, independent exec.LookPath call before isolating the binary. On the
Windows acceptance job's concurrent packages, another test can mutate the
shared toolchain cache directory between those two lookups, turning a
binary that just existed into a spurious "not found" failure.
RequireTerraformPath/RequireTofuPath now resolve the path once and hand it
back directly, removing the redundant lookup at each call site.
* fix(tests): fail loudly instead of returning an empty path on lookup miss
requireExecutablePath (backing RequireTerraformPath/RequireTofuPath) silently
returned "" when ATMOS_TEST_SKIP_PRECONDITION_CHECKS=true (set for every CI
acceptance job) and the binary wasn't found, instead of surfacing the miss.
Callers that need the resolved path then crashed on the empty string with a
confusing "open : file not found" error rather than a clear diagnostic.
RequireExecutable is decoupled back to its original full-bypass behavior
(a true no-op when checks are disabled) since its callers only need a
boolean gate, not a path -- only the path-returning variants now hard-fail
on a genuine lookup miss.
* fix(test): isolate toolchain install-path from the real shared cache dir
Several pkg/toolchain tests (TestRunInstall_WithLatestKeyword,
TestRunInstall_WithCanonicalFormat, TestRunInstall_WithValidToolSpec,
TestRunInstall_WithSetAsDefault, TestRunInstallWithNoArgs,
TestRunInstall_Reinstall, TestRunUninstall, TestRunUninstall_InvalidToolSpecFormat,
TestUninstallAllVersionsOfTool) called SetAtmosConfig() without setting
Toolchain.InstallPath. NewInstaller()'s GetInstallPath() falls back to the
real, shared XDG toolchain cache directory whenever InstallPath is empty --
the exact directory CI's "atmos toolchain install --default" step populates
and that the whole acceptance suite depends on for hashicorp/terraform and
opentofu/opentofu binaries for the rest of the run.
TestRunInstall_WithLatestKeyword and TestRunInstall_WithCanonicalFormat in
particular perform real network installs of real "terraform" binaries into
that shared path, racing with every other concurrently-running package's
test process reading tofu/terraform off PATH -- almost certainly the root
cause of the intermittent "'tofu' not found in PATH" hard failures seen on
the Windows acceptance job (round 3 of this investigation had already ruled
out a stale-PATH-visibility theory for this specific failure instance).
Fixed by pointing Toolchain.InstallPath at each test's own t.TempDir(),
matching the isolation pattern already used by the rest of the toolchain
test suite.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(test): isolate remaining toolchain-path leak, widen resolution retry
TestBuildToolRow/TestBuildToolRow_NotInstalled resolved NewInstaller()'s
binDir to the real shared toolchain cache dir (no InstallPath override),
same class of issue fixed in fdbb83f -- read-only here, but still worth
isolating for hygiene and to rule them out as a contributor.
The Windows tofu-not-found flake recurred in CI even after that fix, so
widen requireExecutablePath's retry window from 2s to 15s as a defensive
backstop: some residual contention under the acceptance job's concurrent
package load isn't yet fully identified, and a longer bounded poll is safer
than risking another silent failure while that's tracked down.
* test(ci): add lookup forensics for the Windows toolchain-vanishing flake
The Windows acceptance job has a recurring failure where an installed
toolchain binary (tofu/terraform) vanishes from PATH mid-suite for the
full retry window. Root-cause candidates are all writer-side windows in
the shared toolchain cache (uninstall RemoveAll, onedir rename-aside,
in-place flat-install extraction) but four rounds of static analysis
couldn't pin which one fires in CI.
requireExecutablePath now dumps per-PATH-entry forensics on failure:
whether each toolchain dir exists, its contents, target presence, and
whether the installer's ".lock" sibling is held. The next CI failure
becomes direct evidence of the mechanism (dir deleted vs tree renamed
aside vs writer mid-flight vs PATH corruption) instead of another guess.
* fix(test): stop telemetry test deleting the shared Atmos cache root
Root cause of the recurring Windows acceptance flake, confirmed by the
lookup forensics added in a68e195: the failure dump showed EVERY
toolchain directory (terraform, opentofu, helm, helmfile) gone at once
-- the whole <cache>/atmos tree had been deleted mid-run.
TestPrintTelemetryDisclosureOnlyOnce "cleaned up" by os.RemoveAll on
filepath.Dir(cfg.GetCacheFilePath()) -- the real shared <cache>/atmos
root -- twice (setup + defer). Ever since the toolchain install root
moved underneath that same root (<cache>/atmos/toolchain, #2579), this
deleted every CI-provisioned tool out from under the concurrently
running package test binaries. Whichever terraform/tofu-dependent test
happened to run during the window failed with "executable file not
found in %PATH%"; the tests package healed itself via TestMain
re-provisioning, which is why only internal/exec kept failing.
All four disclosure tests now isolate the cache via a per-test
ATMOS_XDG_CACHE_HOME/XDG_CACHE_HOME redirect instead of deleting
anything shared. This also fixes the sibling tests that "cleaned"
./.atmos -- a directory the disclosure code never used -- while
actually polluting the real user-level cache.yaml.
* docs(fixes): record telemetry-test shared-cache-root fix
* refactor(cmd): remove hand-rolled argv parsing for CI git-clone bootstrap
cmd/root.go hand-parsed os.Args to decide whether a config-init failure
was a no-argument `atmos git clone` bootstrapping in an empty CI
workspace, duplicating cmd/git's own --ci/ATMOS_CI resolution
(resolveCICloneMode) with real drift: root's literal "--ci=true"/
"--ci=false" string matching missed values pflag's ParseBool accepts
(e.g. --ci=TRUE, ATMOS_CI=1).
Replace ~120 lines of argv sniffing (isCIGitCloneBootstrapRequested,
gitCloneBootstrapCIMode, stripRootFlagsForBootstrapCheck, and friends)
with cmd/git.CICloneBootstrapRequested(cmd, args): a Cobra-identity +
already-parsed-flag check, callable only from PersistentPreRun onward
(where Cobra has parsed the target command's flags), delegating the
actual --ci/ATMOS_CI precedence to the single implementation
(resolveCICloneMode) cmd/git's own RunE already uses.
This also fixes a load-bearing bug uncovered while tracing the old
code: the pre-dispatch call site (handleConfigInitErrorWithArgs, run
from Execute() before RootCmd.ExecuteC()) was the ONLY place that set
CI.Enabled on the config cmd/git actually reads (the package-level
`atmosConfig`, aliased into cmd/git via SetAtmosConfig before
PersistentPreRun runs) -- PersistentPreRun's own bootstrap branch wrote
to a local `tmpConfig` that is discarded at the end of the function, so
for the canonical empty-workspace (cfg.NotFound) case it never did
anything. The pre-dispatch call site is deleted entirely; the load-bearing
write is added directly to PersistentPreRun's NotFound branch instead,
which now writes both the package-level atmosConfig (load-bearing) and
the local tmpConfig (used by the rest of this invocation).
Malformed (as opposed to missing) atmos.yaml encountered during CI
bootstrap continues to be tolerated via PersistentPreRun's existing
non-NotFound bootstrap branch, now also routed through the shared
helper.
* fix(ci): exclude docs.docker.com from link check (connection resets)
CI's Check Markdown Links job failed on docs/prd/ecr-authentication.md
(unrelated to this PR's diff) with "Connection reset by peer" against
docs.docker.com. The URL is live (curl: 301 -> 200 outside CI); this
matches the repo's existing precedent for CDN-fronted hosts that
intermittently reset connections from CI runners (taskfile.dev,
geminicli.com, otelic.com, etc.).
* fix(test): address CodeRabbit findings on PR #2812
Fix three CodeRabbit-flagged issues:
- pkg/toolchain/install_test.go, list_test.go: several tests set
Toolchain.InstallPath to a per-test t.TempDir() (needed to keep real
installs off the shared XDG toolchain cache) but only partially
restored atmosConfig afterward, leaving InstallPath pointed at a
directory that TempDir cleanup had already deleted. Save/restore the
full prior atmosConfig instead of reconstructing a partial one.
- cmd/root_test.go: TestApplyCIGitCloneBootstrap_AllowsBootstrap and
TestApplyCIGitCloneBootstrap_NoCIProviderDetected didn't pin
ATMOS_CI, so an ambient ATMOS_CI=false in the developer/CI
environment could make resolveCICloneMode resolve to "disabled" and
fail the test non-deterministically. Pin ATMOS_CI explicitly,
mirroring the existing guard in cmd/git/bootstrap_test.go.
- .github/workflows/test.yml: the Docker Hub mirror configuration step
used jq's `unique` to append mirror.gcr.io to registry-mirrors, which
sorts the array and can silently reorder any pre-existing mirrors
(Docker tries mirrors in listed order). Only append when absent,
preserving existing order.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(git): use CI context branch name, not raw ref, in no-arg checkout
Add a real (non-stubbed) native-ci job cloning a local fixture repo to
exercise the CI git-clone bootstrap path end-to-end, which immediately
surfaced a pre-existing bug: runCICheckout passed the raw ref (e.g.
refs/heads/main) as `git clone --branch`, which rejects full ref paths.
Use the CI context's already-parsed branch name instead, and document
the config-init tolerance mechanism in docs/prd/git-ops.md.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* revert(ci): drop the ci-bootstrap-clone native-ci job
The synthetic fixture-repo redirection (overriding GITHUB_SERVER_URL/
GITHUB_REPOSITORY at the step level) worked in a local shell but not in
a real Actions run: GitHub Actions silently ignores GITHUB_-prefixed
env overrides, so the job actually cloned the real cloudposse/atmos
repo and failed a later assertion meant for the fixture. That's too
curve-fitted to the local repro to be a real test of native CI in CI,
and not fixable by adjusting the script further, so drop it rather than
keep a job that looks like coverage but silently tests the wrong thing.
The runCICheckout branch/ref fix itself is unaffected — it was found
and verified via a local (non-CI) reproduction and is covered by the
corrected unit test.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* docs(fix-log): fix missing comma per CodeRabbit review
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(test): recognize packer-only manifest error in build tolerance check
TestPackerBuildCmdWithDirectoryTemplate failed on Windows CI (job
91001781247) with "Failed loading manifest ... EOF" -- text packer's
own manifest post-processor produces, never Atmos. The packerRan
heuristic didn't recognize this specific string, so it fell through to
a real test failure even though packer clearly ran and failed for
environmental (credential) reasons, same as the test already tolerates
for other packer error shapes.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: Andriy Knysh <aknysh@users.noreply.github.com>1 parent 0c77c7b commit b69113d
25 files changed
Lines changed: 775 additions & 198 deletions
File tree
- .github/workflows
- agent-skills/skills/atmos-modernization
- cmd
- git
- docs
- fixes
- prd
- pkg/toolchain
- website/docs
- cli/commands/git
- integrations/github-actions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
5 | 5 | | |
6 | 6 | | |
7 | 7 | | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
8 | 11 | | |
9 | 12 | | |
10 | 13 | | |
| |||
35 | 38 | | |
36 | 39 | | |
37 | 40 | | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
38 | 46 | | |
39 | 47 | | |
40 | 48 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
591 | 591 | | |
592 | 592 | | |
593 | 593 | | |
| 594 | + | |
| 595 | + | |
| 596 | + | |
| 597 | + | |
| 598 | + | |
| 599 | + | |
| 600 | + | |
| 601 | + | |
| 602 | + | |
| 603 | + | |
| 604 | + | |
| 605 | + | |
| 606 | + | |
| 607 | + | |
| 608 | + | |
| 609 | + | |
594 | 610 | | |
595 | 611 | | |
596 | 612 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
10 | 10 | | |
11 | 11 | | |
12 | 12 | | |
13 | | - | |
14 | | - | |
15 | | - | |
16 | | - | |
17 | 13 | | |
18 | 14 | | |
19 | 15 | | |
| |||
67 | 63 | | |
68 | 64 | | |
69 | 65 | | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
70 | 77 | | |
71 | 78 | | |
72 | 79 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
19 | 19 | | |
20 | 20 | | |
21 | 21 | | |
22 | | - | |
| 22 | + | |
23 | 23 | | |
24 | 24 | | |
25 | 25 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
19 | 19 | | |
20 | 20 | | |
21 | 21 | | |
| 22 | + | |
22 | 23 | | |
23 | 24 | | |
24 | 25 | | |
| |||
88 | 89 | | |
89 | 90 | | |
90 | 91 | | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
91 | 95 | | |
92 | 96 | | |
93 | 97 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
12 | 12 | | |
13 | 13 | | |
14 | 14 | | |
15 | | - | |
| 15 | + | |
16 | 16 | | |
17 | 17 | | |
18 | 18 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
0 commit comments