fix: Do not add configured prefix for `!store.get` calls

[ohaibbq]

what

All stores (besides Azure KeyVault) incorrectly added the store's prefix to calls to GetKey. This updates them to match the expected behavior (according to the Atmos documentation)

Exact Key Matching: The key you provide must match exactly how it is stored in the backend, including any required prefix, path, or normalization specific to that store

why

This updates the implementations to match the documentation. In the interim, we've configured multiple stores, one prefixed, one unprefixed in order to fetch arbitrary keys from GSM.

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Store implementations for Artifactory, AWS SSM Parameter Store, Google Secret Manager, and Redis now retrieve configuration and secrets using exact key names instead of automatically applying a prefix transformation.

• Tests

  • Added and updated test cases across all stores to verify keys are resolved using exact values without prefix manipulation.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai[bot]]

📝 Walkthrough

Walkthrough

The PR removes prefix-based key transformation from GetKey methods across multiple store backends (Artifactory, AWS SSM Parameter Store, Google Secret Manager, Redis). Each backend now uses provided keys directly rather than prepending the store's configured prefix. Corresponding test cases are updated to reflect this behavior change.

Changes

Cohort / File(s)	Summary
Artifactory Store pkg/store/artifactory_store.go	GetKey now uses the provided key directly without prepending s.prefix; filePath := key instead of conditionally prefixing with s.prefix
Artifactory Store Tests pkg/store/artifactory_store_test.go	New test suite TestArtifactoryStore_GetKey added covering successful retrieval without prefix, prefix handling (key used as-is), key with .json extension, download errors, and no-files scenarios
AWS SSM Parameter Store pkg/store/aws_ssm_param_store.go	GetKey now uses the provided key directly without prefix transformation; parameter name no longer conditionally prepended with s.prefix
AWS SSM Parameter Store Tests pkg/store/aws_ssm_param_store_test.go	All mock GetParameterInput Name field expectations updated across multiple test cases (successful_get, successful_get_slice, successful_get_map, parameter_not_found, etc.) to remove "/test-prefix" prefix
Google Secret Manager Store pkg/store/google_secret_manager_store.go	GetKey now uses the provided key directly as secret name without prefix transformation; descriptive comment added about accessing arbitrary keys
Google Secret Manager Store Tests pkg/store/google_secret_manager_store_test.go	New test case added to verify key without prefix handling; new test function TestGSMStore_GetKey_WithPrefix ensures key is used verbatim without store prefix injection
Redis Store pkg/store/redis_store.go	GetKey no longer prefixes the Redis key with s.prefix; now uses the provided key directly as Redis key; prefix-based namespacing removed
Redis Store Tests pkg/store/redis_store_test.go	Test expectation for GetKey changed from asserting prefixed key (myapp:) to using the raw key (tt.key) without prefix addition

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

• Areas requiring attention:
  • Verify that removing prefix logic from GetKey doesn't break existing usage patterns across codebase
  • Confirm test coverage adequately validates the new unprefixed behavior in each store backend
  • Check for any implicit dependencies on prefix-based key scoping in calling code

Possibly related PRs

• cloudposse/atmos#1352: Both PRs modify store-level GetKey functionality across multiple backends (Artifactory, SSM, Google Secret Manager, Redis) with related YAML store handling changes.
• cloudposse/atmos#1009: Modifies aws_ssm_param_store.go key-construction logic alongside the main PR's changes to the same file.
• cloudposse/atmos#1017: Both PRs modify artifactory_store key-generation logic and GetKey behavior in the same file.

Suggested labels

patch

Suggested reviewers

• osterman
• aknysh

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: removing prefix addition from store.get calls across multiple store implementations.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

• [ ] 📝 Generate docstrings

🧪 Generate unit tests (beta)

• [ ] Create PR with unit tests
• [ ] Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai[bot]]

📝 Walkthrough

Walkthrough

A consistent refactor across store backends (Artifactory, AWS SSM, Google Secret Manager, Redis) removes automatic key prefixing from GetKey methods, allowing keys to be accessed as-is rather than transformed with store-level prefixes. Implementation and test files updated to reflect this behavioral change.

Changes

Cohort / File(s)	Summary
Artifactory Store pkg/store/artifactory_store.go, pkg/store/artifactory_store_test.go	GetKey no longer prefixes keys with repository/prefix; uses keys as-is with only .json extension appended if missing. New test suite verifies key usage without prefix transformation across multiple scenarios.
AWS SSM Parameter Store pkg/store/aws_ssm_param_store.go, pkg/store/aws_ssm_param_store_test.go	GetKey stops applying store prefix to parameter names; parameter names now taken directly from provided key with leading slash added if missing. Test expectations updated from prefixed paths to direct paths.
Google Secret Manager Store pkg/store/google_secret_manager_store.go, pkg/store/google_secret_manager_store_test.go	GetKey removes optional prefix transformation; secret names now always use raw keys directly. Test cases added to verify keys are used verbatim without automatic prefixing.
Redis Store pkg/store/redis_store.go, pkg/store/redis_store_test.go	GetKey logic removes key prefixing with store prefix; Redis keys now use raw key provided without transformation. Test expectations changed from prefixed keys to raw keys.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Review consistency of prefix-removal pattern across all four store backends to ensure uniform semantic change
• Verify test cases in each backend properly validate the new key-as-is behavior
• Confirm no edge cases or error handling paths are inadvertently affected by removal of prefix logic

Possibly related PRs

• cloudposse/atmos#1352 — Directly related: changes GetKey behavior across the same backends (Artifactory, SSM, GSM, Redis) by removing automatic prefixing and updates corresponding tests.
• cloudposse/atmos#1009 — Related to AWS SSM parameter store: modifies key-construction logic in aws_ssm_param_store while this PR removes prefixing from the same component.

Suggested labels

patch

Suggested reviewers

• osterman
• aknysh

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: removing automatic prefix addition from store.get calls across multiple store implementations.

✨ Finishing touches

• [ ] 📝 Generate docstrings

🧪 Generate unit tests (beta)

• [ ] Create PR with unit tests
• [ ] Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 8f1fe665eae9f4d3705d9f9710e8e1818b7e3a2a and 4be744935c0ab1ad018507ac75f5c622bfe80cd5.

📒 Files selected for processing (8)

• pkg/store/artifactory_store.go (1 hunks)
• pkg/store/artifactory_store_test.go (1 hunks)
• pkg/store/aws_ssm_param_store.go (1 hunks)
• pkg/store/aws_ssm_param_store_test.go (9 hunks)
• pkg/store/google_secret_manager_store.go (1 hunks)
• pkg/store/google_secret_manager_store_test.go (2 hunks)
• pkg/store/redis_store.go (1 hunks)
• pkg/store/redis_store_test.go (1 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

**/*.go

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.go: Use functional options pattern to avoid functions with many parameters. Define Option functions that modify a Config struct and pass variadic options to New functions. Use context.Context for cancellation signals, deadlines/timeouts, and request-scoped values (trace IDs). Context should be first parameter in functions that accept it. DO NOT use context for configuration, dependencies, or avoiding proper function parameters. All comments must end with periods. This is enforced by the godot linter. NEVER delete existing comments without a very strong reason. Preserve helpful comments, update them to match code changes, refactor for clarity, and add context when modifying code. Only remove factually incorrect, duplicated, or outdated comments. Organize imports in three groups separated by blank lines (Go stdlib, 3rd-party excluding cloudposse/atmos, Atmos packages), sorted alphabetically. Maintain aliases: cfg, log, u, errUtils. Add defer perf.Track(atmosConfig, "pkg.FuncName")() + blank line to all public functions. Use nil if no atmosConfig parameter. All errors MUST be wrapped using static errors defined in errors/errors.go. Use errors.Join for combining multiple errors, fmt.Errorf with %w for adding string context, error builder for complex errors, and errors.Is() for error checking. NEVER use dynamic errors directly. Use go.uber.org/mock/mockgen with //go:generate directives for mock generation. Never create manual mocks. Use viper.BindEnv with ATMOS_ prefix for environment variables. Use colors from pkg/ui/theme/colors.go for theme consistency. Ensure Linux/macOS/Windows compatibility. Use SDKs over binaries. Use filepath.Join(), not hardcoded path separators. Small focused files (<600 lines). One cmd/impl per file. Co-locate tests. Never use //revive:disable:file-length-limit.

**/*.go: Use Viper for managing configuration, environment variables, and flags in CLI commands Use interfaces for external dependencies to facilitate mocking and consider us...

Files:

• pkg/store/aws_ssm_param_store.go
• pkg/store/aws_ssm_param_store_test.go
• pkg/store/redis_store_test.go
• pkg/store/google_secret_manager_store.go
• pkg/store/google_secret_manager_store_test.go
• pkg/store/artifactory_store_test.go
• pkg/store/redis_store.go
• pkg/store/artifactory_store.go

**/*_test.go

📄 CodeRabbit inference engine (CLAUDE.md)

**/*_test.go: Prefer unit tests with mocks over integration tests. Use interfaces + dependency injection for testability. Generate mocks with go.uber.org/mock/mockgen. Use table-driven tests for comprehensive coverage. Target >80% code coverage. Test behavior, not implementation. Never test stub functions. Avoid tautological tests. Make code testable with DI to avoid os.Exit and external systems. Use assert.ErrorIs(err, ErrSentinel) for stdlib/our errors.

**/*_test.go: Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages Use table-driven tests for testing multiple scenarios in Go Include integration tests for command flows and test CLI end-to-end when possible with test fixtures

Files:

• pkg/store/aws_ssm_param_store_test.go
• pkg/store/redis_store_test.go
• pkg/store/google_secret_manager_store_test.go
• pkg/store/artifactory_store_test.go

🧠 Learnings (5)

📓 Common learnings

Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.287Z
Learning: Follow multi-provider registry pattern: 1) Define interface in dedicated package, 2) Implement per provider, 3) Register implementations, 4) Generate mocks. See pkg/store/ for reference (AWS SSM, Azure Key Vault, Google Secret Manager).

📚 Learning: 2025-11-24T17:35:20.287Z

Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.287Z
Learning: Use interface-driven design with dependency injection for testability. Generate mocks with go.uber.org/mock/mockgen using //go:generate directives.

Applied to files:

• pkg/store/aws_ssm_param_store_test.go
• pkg/store/redis_store_test.go

📚 Learning: 2025-11-24T17:35:37.181Z

Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.181Z
Learning: Applies to **/*_test.go : Use table-driven tests for testing multiple scenarios in Go

Applied to files:

• pkg/store/google_secret_manager_store_test.go
• pkg/store/artifactory_store_test.go

📚 Learning: 2025-11-24T17:35:20.287Z

Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.287Z
Learning: Applies to **/*_test.go : Test behavior, not implementation. Never test stub functions. Avoid tautological tests. Make code testable with DI to avoid os.Exit and external systems. Use assert.ErrorIs(err, ErrSentinel) for stdlib/our errors.

Applied to files:

• pkg/store/google_secret_manager_store_test.go
• pkg/store/artifactory_store_test.go

📚 Learning: 2025-11-24T17:35:37.181Z

Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.181Z
Learning: Applies to **/*_test.go : Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages

Applied to files:

• pkg/store/artifactory_store_test.go

🧬 Code graph analysis (2)

pkg/store/google_secret_manager_store_test.go (1)

pkg/store/google_secret_manager_store.go (1)

• GSMStore (34-40)

pkg/store/artifactory_store_test.go (1)

pkg/store/artifactory_store.go (1)

• ArtifactoryStore (20-25)

⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Summary

🔇 Additional comments (9)

pkg/store/redis_store_test.go (1)

427-428: LGTM!

The test now correctly verifies that GetKey uses the raw key without applying the store prefix, aligning with the documented behavior.

pkg/store/redis_store.go (1)

161-163: LGTM!

The implementation correctly uses the key as-is without prefix transformation, with clear documentation explaining the rationale.

pkg/store/artifactory_store.go (1)

296-298: LGTM!

The implementation correctly uses the key as-is without prefix transformation, consistent with the documented behavior.

pkg/store/artifactory_store_test.go (1)

349-438: LGTM!

Comprehensive test coverage for the GetKey behavior change. The table-driven approach tests both success and error scenarios, with explicit verification that the prefix is not applied to keys.

pkg/store/google_secret_manager_store.go (1)

295-297: LGTM!

The implementation correctly uses the key as-is without prefix transformation, consistent with the documented behavior across all stores.

pkg/store/aws_ssm_param_store_test.go (1)

722-722: LGTM!

All mock expectations correctly updated to reflect that GetKey uses keys as-provided without applying the store prefix. The changes are consistent across all test cases.

Also applies to: 736-736, 750-750, 790-790, 805-805, 820-820, 835-835, 848-848, 874-874

pkg/store/aws_ssm_param_store.go (1)

265-272: LGTM!

The implementation correctly uses the key as-is without prefix transformation. The leading slash normalization is appropriate for SSM's requirements.

pkg/store/google_secret_manager_store_test.go (2)

682-689: LGTM!

The new test case explicitly verifies that keys are used exactly as provided, reinforcing the behavioral change.

---

736-768: LGTM!

Excellent test that explicitly verifies the prefix is not applied to GetKey operations, even when the store is configured with a prefix. This directly validates the fix.

[!TIP]

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

• Provide your own instructions using the high_level_summary_instructions setting.
• Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
• Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
4. 📊 Contributor Summary — Include a Markdown table showing contributions: | Contributor | Lines Added | Lines Removed | Files Changed |
5. ✔️ Additional Notes — Add any extra reviewer context. Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov[bot]]

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 73.17%. Comparing base (1cb9d02) to head (32a341c).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1814      +/-   ##
==========================================
+ Coverage   73.09%   73.17%   +0.08%     
==========================================
  Files         550      550              
  Lines       53208    53200       -8     
==========================================
+ Hits        38893    38930      +37     
+ Misses      11457    11412      -45     
  Partials     2858     2858              

Flag	Coverage Δ
unittests	73.17% <ø> (+0.08%)	:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
pkg/store/artifactory_store.go	59.64% <ø> (+11.67%)	:arrow_up:
pkg/store/aws_ssm_param_store.go	83.44% <ø> (+1.13%)	:arrow_up:
pkg/store/google_secret_manager_store.go	69.32% <ø> (+0.84%)	:arrow_up:
pkg/store/redis_store.go	68.23% <ø> (-0.74%)	:arrow_down:

... and 4 files with indirect coverage changes

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[osterman]

Windows tests are failing:

--- FAIL: TestArtifactoryStore_GetKey (0.00s)
    --- FAIL: TestArtifactoryStore_GetKey/successful_retrieval_without_prefix (0.00s)
panic: 
	
	mock: Unexpected Method Call
	-----------------------------
	
	DownloadFiles(services.DownloadParams)
			0: services.DownloadParams{CommonParams:(*utils.CommonParams)(0xc000285180), Symlink:false, ValidateSymlink:false, Flat:false, Explode:false, BypassArchiveInspection:false, MinSplitSize:5120, SplitCount:3, PublicGpgKey:"", SkipChecksum:false, Sha256:"", Size:(*int64)(nil)}
	
	The closest call I have is: 
	
	DownloadFiles(mock.argumentMatcher)
			0: mock.argumentMatcher{fn:reflect.Value{typ_:(*abi.Type)(0x7ff612a84080), ptr:(unsafe.Pointer)(0x7ff612e58c68), flag:0x13}}
	
	
	Diff: 0: FAIL:  (services.DownloadParams={0xc000285180 false false false false false 5120 3  false  <nil>}) not matched by func(services.DownloadParams) bool
	at: [D:/a/atmos/atmos/pkg/store/artifactory_store_test.go:35 D:/a/atmos/atmos/pkg/store/artifactory_store.go:326 D:/a/atmos/atmos/pkg/store/artifactory_store_test.go:423]
	 [recovered, repanicked]

goroutine 76 [running]:
testing.tRunner.func1.2({0x7ff612a5e8a0, 0xc00046f9b0})
	C:/hostedtoolcache/windows/go/1.25.0/x64/src/testing/testing.go:1872 +0x239
testing.tRunner.func1()
	C:/hostedtoolcache/windows/go/1.25.0/x64/src/testing/testing.go:1875 +0x35b
panic({0x7ff612a5e8a0?, 0xc00046f9b0?})
	C:/hostedtoolcache/windows/go/1.25.0/x64/src/runtime/panic.go:783 +0x132
github.com/stretchr/testify/mock.(*Mock).fail(0xc0000db540, {0x7ff612e47144?, 0x4?}, {0xc0000db6d0?, 0x1?, 0x1?})
	C:/Users/runneradmin/go/pkg/mod/github.com/stretchr/[email protected]/mock/mock.go:359 +0x125
github.com/stretchr/testify/mock.(*Mock).MethodCalled(0xc0000db540, {0x7ff61343efe0, 0xd}, {0xc00046f7e0, 0x1, 0x1})
	C:/Users/runneradmin/go/pkg/mod/github.com/stretchr/[email protected]/mock/mock.go:519 +0x5c7
github.com/stretchr/testify/mock.(*Mock).Called(0xc0000db540, {0xc00046f7e0, 0x1, 0x1})
	C:/Users/runneradmin/go/pkg/mod/github.com/stretchr/[email protected]/mock/mock.go:491 +0x125
github.com/cloudposse/atmos/pkg/store.(*MockArtifactoryClient).DownloadFiles(0xc0000db540, {0xc0000db590, 0x1, 0x13?})
	D:/a/atmos/atmos/pkg/store/artifactory_store_test.go:35 +0xb1
github.com/cloudposse/atmos/pkg/store.(*ArtifactoryStore).GetKey(0xc0004b1f28, {0x7ff612dae897?, 0x13?})
	D:/a/atmos/atmos/pkg/store/artifactory_store.go:326 +0x418
github.com/cloudposse/atmos/pkg/store.TestArtifactoryStore_GetKey.func5(0xc0003bec40)
	D:/a/atmos/atmos/pkg/store/artifactory_store_test.go:423 +0x105
testing.tRunner(0xc0003bec40, 0xc00032d420)
	C:/hostedtoolcache/windows/go/1.25.0/x64/src/testing/testing.go:1934 +0xc3
created by testing.(*T).Run in goroutine 75
	C:/hostedtoolcache/windows/go/1.25.0/x64/src/testing/testing.go:1997 +0x44b

[ohaibbq]

@aknysh looks like this was closed by mistake due to the mention of the pr in #1816 - can you reopen?

[aknysh]

@ohaibbq thanks for the PR, it looks good. The windows tests are failing b/c of this (please update):

Windows Test Failure Explanation

  The test failure is caused by cross-platform path handling in artifactory_store.go:310:

  repoPath := filepath.Join(s.repoName, filePath)

  Problem

  filepath.Join uses the OS-specific path separator:
  - Linux/macOS: / (forward slash)
  - Windows: \ (backslash)

  The test expects:
  params.Pattern == "test-repo/config/app-settings.json"

  But on Windows, filepath.Join("test-repo", "config/app-settings.json") produces:
  "test-repo\config/app-settings.json"

  This causes the mock matcher to fail because the pattern doesn't match.

  Solution

  The PR author should use path.Join instead of filepath.Join for URL/remote paths, or manually construct the path with /:

  // Option 1: Use path.Join (always uses forward slashes)
  import "path"
  repoPath := path.Join(s.repoName, filePath)

  // Option 2: Manual string concatenation
  repoPath := s.repoName + "/" + filePath

use path.Join (Option 1)