test: stabilize Terraform cache coverage (#2620)

* test: stabilize terraform cache coverage

* [autocommit] formatting fixes

* fix: avoid windows current-user root prompt in CI

* test: run registry cache acceptance across platforms

* Fix PR review follow-ups

* ci: run registry cache acceptance before broad suite

* ci: match registry cache test cgo settings

* fix: wrap sha pinning comment api calls

---------

Co-authored-by: atmos-pro[bot] <173522224+atmos-pro[bot]@users.noreply.github.com>
Co-authored-by: Andriy Knysh <aknysh@users.noreply.github.com>

Author: 3 people

.github/actions/verify-sha-pinning/action.yml

@@ -39,6 +39,29 @@ runs:
           const path = require('path');
 
           const workflowDir = process.env.WORKFLOW_DIR;
+          const tagShaCache = new Map();
+          const nonRetryableStatuses = new Set([400, 401, 403, 404, 422]);
+
+          const sleep = ms => new Promise(resolve => setTimeout(resolve, ms));
+
+          async function withRetries(operation, label, maxAttempts = 3) {
+            let lastErr;
+            for (let attempt = 1; attempt <= maxAttempts; attempt++) {
+              try {
+                return await operation();
+              } catch (err) {
+                if (nonRetryableStatuses.has(err.status)) {
+                  throw err;
+                }
+                lastErr = err;
+                if (attempt < maxAttempts) {
+                  core.warning(`${label} failed on attempt ${attempt}/${maxAttempts}: ${err.message}; retrying`);
+                  await sleep(250 * attempt);
+                }
+              }
+            }
+            throw lastErr;
+          }
 
           // Find all workflow files
           const files = fs.readdirSync(workflowDir)
@@ -91,28 +114,44 @@ runs:
 
           // Resolve a tag to its commit SHA, handling annotated tags
           async function resolveTagSha(owner, repo, tag) {
+            const cacheKey = `${owner}/${repo}@${tag}`;
+            if (tagShaCache.has(cacheKey)) {
+              return tagShaCache.get(cacheKey);
+            }
+
             try {
-              const ref = await github.rest.git.getRef({
-                owner,
-                repo,
-                ref: `tags/${tag}`,
-              });
+              const ref = await withRetries(
+                () => github.rest.git.getRef({
+                  owner,
+                  repo,
+                  ref: `tags/${tag}`,
+                }),
+                `Resolving ${cacheKey}`
+              );
 
               const obj = ref.data.object;
+              let sha;
 
               // Lightweight tag — points directly to a commit
               if (obj.type === 'commit') {
-                return obj.sha;
+                sha = obj.sha;
+                tagShaCache.set(cacheKey, sha);
+                return sha;
               }
 
               // Annotated tag — dereference to get the commit
               if (obj.type === 'tag') {
-                const tagObj = await github.rest.git.getTag({
-                  owner,
-                  repo,
-                  tag_sha: obj.sha,
-                });
-                return tagObj.data.object.sha;
+                const tagObj = await withRetries(
+                  () => github.rest.git.getTag({
+                    owner,
+                    repo,
+                    tag_sha: obj.sha,
+                  }),
+                  `Dereferencing ${cacheKey}`
+                );
+                sha = tagObj.data.object.sha;
+                tagShaCache.set(cacheKey, sha);
+                return sha;
               }
 
               throw new Error(`Unexpected ref object type: ${obj.type}`);
@@ -130,7 +169,10 @@ runs:
 
             // Check if the pinned SHA exists in this repo at all
             try {
-              await github.rest.repos.getCommit({ owner, repo, ref: pinnedSha });
+              await withRetries(
+                () => github.rest.repos.getCommit({ owner, repo, ref: pinnedSha }),
+                `Checking ${owner}/${repo}@${pinnedSha}`
+              );
               details.existsInRepo = true;
             } catch (err) {
               details.existsInRepo = (err.status !== 404 && err.status !== 422);
@@ -139,9 +181,12 @@ runs:
             // Find which tags (if any) point to this SHA
             if (details.existsInRepo) {
               try {
-                const tags = await github.paginate(github.rest.repos.listTags, {
-                  owner, repo, per_page: 100,
-                });
+                const tags = await withRetries(
+                  () => github.paginate(github.rest.repos.listTags, {
+                    owner, repo, per_page: 100,
+                  }),
+                  `Listing tags for ${owner}/${repo}`
+                );
                 details.matchingTags = tags
                   .filter(t => t.commit.sha === pinnedSha)
                   .map(t => t.name);
@@ -238,6 +283,28 @@ runs:
         github-token: ${{ inputs.github-token }}
         script: |
           const marker = '<!-- verify-sha-pinning -->';
+          const nonRetryableStatuses = new Set([400, 401, 403, 404, 422]);
+          const sleep = ms => new Promise(resolve => setTimeout(resolve, ms));
+
+          async function withRetries(operation, label, maxAttempts = 3) {
+            let lastErr;
+            for (let attempt = 1; attempt <= maxAttempts; attempt++) {
+              try {
+                return await operation();
+              } catch (err) {
+                if (nonRetryableStatuses.has(err.status)) {
+                  throw err;
+                }
+                lastErr = err;
+                if (attempt < maxAttempts) {
+                  core.warning(`${label} failed on attempt ${attempt}/${maxAttempts}: ${err.message}; retrying`);
+                  await sleep(250 * attempt);
+                }
+              }
+            }
+            throw lastErr;
+          }
+
           const prNumber = context.issue.number;
           if (!prNumber) {
             core.info('Not a PR context — skipping comment');
@@ -251,11 +318,14 @@ runs:
           const status = process.env.STATUS || 'pass';
 
           // Find existing comment
-          const { data: comments } = await github.rest.issues.listComments({
-            owner: context.repo.owner,
-            repo: context.repo.repo,
-            issue_number: prNumber,
-          });
+          const { data: comments } = await withRetries(
+            () => github.rest.issues.listComments({
+              owner: context.repo.owner,
+              repo: context.repo.repo,
+              issue_number: prNumber,
+            }),
+            `Listing comments for PR #${prNumber}`
+          );
           const existing = comments.find(c => c.body?.includes(marker));
 
           let body;
@@ -308,19 +378,25 @@ runs:
           }
 
           if (existing) {
-            await github.rest.issues.updateComment({
-              owner: context.repo.owner,
-              repo: context.repo.repo,
-              comment_id: existing.id,
-              body,
-            });
+            await withRetries(
+              () => github.rest.issues.updateComment({
+                owner: context.repo.owner,
+                repo: context.repo.repo,
+                comment_id: existing.id,
+                body,
+              }),
+              `Updating comment #${existing.id}`
+            );
             core.info(`Updated existing comment #${existing.id}`);
           } else {
-            await github.rest.issues.createComment({
-              owner: context.repo.owner,
-              repo: context.repo.repo,
-              issue_number: prNumber,
-              body,
-            });
+            await withRetries(
+              () => github.rest.issues.createComment({
+                owner: context.repo.owner,
+                repo: context.repo.repo,
+                issue_number: prNumber,
+                body,
+              }),
+              `Creating comment for PR #${prNumber}`
+            );
             core.info('Created new PR comment');
           }

.github/workflows/test.yml

@@ -179,12 +179,12 @@ jobs:
         with:
           version: ${{ env.PACKER_VERSION }}
 
-      # Dogfood `atmos ci cache`: cache ~/.cache/atmos (where the toolchain
-      # installs helm/helmfile) via the recommended composite action. Atmos
-      # derives the key/paths from atmos.yaml (ci.cache) and native actions/cache
-      # does the storage — restore now, save automatically in its post step. No
-      # runtime token needed. continue-on-error so cache issues never fail the
-      # job — the cache is a pure accelerator.
+      # Dogfood `atmos ci cache`: cache the configured Atmos cache root via the
+      # recommended composite action. Do not export ATMOS_XDG_CACHE_HOME or
+      # TF_PLUGIN_CACHE_DIR for the full acceptance job: many tests assert XDG
+      # defaults and Terraform's plugin cache is not safe for shared concurrent
+      # use. This cache step restores/saves toolchain bits only; it must stay a
+      # pure accelerator.
       - name: Cache Atmos toolchain
         if: ${{ ! ( matrix.flavor.target == 'windows' && github.event.pull_request.draft  ) }}
         continue-on-error: true
@@ -230,6 +230,17 @@ jobs:
         run: |
           make deps
 
+      - name: Terraform registry cache acceptance test
+        timeout-minutes: 10
+        if: matrix.flavor.target == 'linux' || matrix.flavor.target == 'macos' || (matrix.flavor.target == 'windows' && ! github.event.pull_request.draft)
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          PACKER_GITHUB_API_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          ATMOS_TEST_SKIP_PRECONDITION_CHECKS: true
+          ATMOS_TEST_TERRAFORM_REGISTRY_CACHE: 1
+          CGO_ENABLED: 0
+        run: go test ./tests -run '^TestTerraformRegistryCache$' -count=1 -timeout 10m -v
+
       # Enable this after merging test-cases
       # Only seems to work with remote schema files
       #- name: Validate YAML Schema for Test Cases

atmos.yaml

@@ -408,19 +408,19 @@ docs:
         indent_level: 2
 
 # CI build cache (dogfooded in .github/workflows/test.yml). The cache archives
-# the Atmos XDG cache root (~/.cache/atmos), where `atmos toolchain install`
-# places helm/helmfile, so CI restores them instead of re-downloading. The key
-# is defined here (once) rather than in the workflow because Atmos supplies
-# {{.OS}}/{{.Arch}}, avoiding any GitHub Actions context juggling.
+# the Atmos XDG cache root (~/.cache/atmos), where CI keeps toolchain installs,
+# and Terraform's provider plugin cache. The key is defined here (once) rather
+# than in the workflow because Atmos supplies {{.OS}}/{{.Arch}}, avoiding any
+# GitHub Actions context juggling.
 #
 # The cache is purely an accelerator: the workflow always installs exact pinned
 # versions, so it stays correct no matter what the cache already holds. The
-# trailing token (v1) is a manual cache-bust — bump it to start from a clean
+# trailing version token is a manual cache-bust — bump it to start from a clean
 # cache. restore_keys lets a bumped token still seed from the previous blob.
 ci:
   cache:
     enabled: true
-    key: 'atmos-toolchain-{{.OS}}-{{.Arch}}-v1'
+    key: 'atmos-toolchain-{{.OS}}-{{.Arch}}-v2'
     restore_keys:
       - 'atmos-toolchain-{{.OS}}-{{.Arch}}-'
 

go.mod

@@ -415,6 +415,8 @@ func setEnv(v *viper.Viper) {
 	// Terraform plugin cache configuration.
 	bindEnv(v, "components.terraform.plugin_cache", "ATMOS_COMPONENTS_TERRAFORM_PLUGIN_CACHE")
 	bindEnv(v, "components.terraform.plugin_cache_dir", "ATMOS_COMPONENTS_TERRAFORM_PLUGIN_CACHE_DIR")
+	bindEnv(v, "components.terraform.cache.enabled", "ATMOS_COMPONENTS_TERRAFORM_CACHE_ENABLED")
+	bindEnv(v, "components.terraform.cache.location", "ATMOS_COMPONENTS_TERRAFORM_CACHE_LOCATION")
 	bindEnv(v, "components.terraform.auto_provision_workdir_for_outputs", "ATMOS_COMPONENTS_TERRAFORM_AUTO_PROVISION_WORKDIR_FOR_OUTPUTS")
 
 	bindEnv(v, "settings.github_token", "GITHUB_TOKEN")

pkg/config/load.go

@@ -1,12 +1,15 @@
 package config
 
 import (
+	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/cloudposse/atmos/pkg/config/homedir"
 	"github.com/cloudposse/atmos/pkg/schema"
+	tfcache "github.com/cloudposse/atmos/pkg/terraform/cache"
 )
 
 // TestViperBindEnv_PluginCache tests that plugin cache env vars are properly bound
@@ -72,6 +75,83 @@ func TestViperBindEnv_PluginCache(t *testing.T) {
 	}
 }
 
+// TestViperBindEnv_TerraformRegistryCache tests that registry cache env vars are
+// bound via Viper and populate the nested components.terraform.cache config.
+func TestViperBindEnv_TerraformRegistryCache(t *testing.T) {
+	customLocation := filepath.Join(t.TempDir(), "terraform-registry-cache")
+
+	tests := []struct {
+		name             string
+		envEnabled       string
+		envLocation      string
+		expectedEnabled  bool
+		expectedLocation string
+	}{
+		{
+			name:            "registry cache enabled via env var",
+			envEnabled:      "true",
+			expectedEnabled: true,
+		},
+		{
+			name:            "registry cache disabled via env var",
+			envEnabled:      "false",
+			expectedEnabled: false,
+		},
+		{
+			name:             "registry cache with custom location",
+			envLocation:      customLocation,
+			expectedEnabled:  false,
+			expectedLocation: customLocation,
+		},
+		{
+			name:             "registry cache enabled with custom location",
+			envEnabled:       "true",
+			envLocation:      customLocation,
+			expectedEnabled:  true,
+			expectedLocation: customLocation,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Setenv("ATMOS_COMPONENTS_TERRAFORM_CACHE_ENABLED", "false")
+			t.Setenv("ATMOS_COMPONENTS_TERRAFORM_CACHE_LOCATION", "")
+
+			if tt.envEnabled != "" {
+				t.Setenv("ATMOS_COMPONENTS_TERRAFORM_CACHE_ENABLED", tt.envEnabled)
+			}
+			if tt.envLocation != "" {
+				t.Setenv("ATMOS_COMPONENTS_TERRAFORM_CACHE_LOCATION", tt.envLocation)
+			}
+
+			config, err := LoadConfig(&schema.ConfigAndStacksInfo{})
+			require.NoError(t, err)
+
+			require.NotNil(t, config.Components.Terraform.Cache)
+			assert.Equal(t, tt.expectedEnabled, config.Components.Terraform.Cache.Enabled)
+			assert.Equal(t, tt.expectedLocation, config.Components.Terraform.Cache.Location)
+		})
+	}
+}
+
+func TestViperBindEnv_TerraformRegistryCacheLocationExpands(t *testing.T) {
+	home := t.TempDir()
+	t.Setenv("HOME", home)
+	t.Setenv("USERPROFILE", home)
+	homedir.Reset()
+	t.Cleanup(homedir.Reset)
+
+	t.Setenv("ATMOS_COMPONENTS_TERRAFORM_CACHE_LOCATION", "~/terraform-registry-cache")
+
+	config, err := LoadConfig(&schema.ConfigAndStacksInfo{})
+	require.NoError(t, err)
+	require.NotNil(t, config.Components.Terraform.Cache)
+
+	root, err := tfcache.ResolveRoot(&config)
+	require.NoError(t, err)
+	assert.Equal(t, filepath.Join(home, "terraform-registry-cache"), root)
+}
+
 func TestDefaultConfig_PluginCache(t *testing.T) {
 	// Verify that the default config has plugin cache enabled.
 	assert.True(t, defaultCliConfig.Components.Terraform.PluginCache)