fix: allow --use-version artifact downloads without GitHub token#2212
Open
fix: allow --use-version artifact downloads without GitHub token#2212
Conversation
…nd other YAML functions (#2207) * updates * fix: derive stack name for locals so !terraform.state works (#2080) The 2-arg form of !terraform.state in locals failed with "stack is required" because extractLocalsFromRawYAML passed an empty string as the current stack. Add deriveStackNameForLocals() and computeStackFileName() to derive the stack name from the file path, vars, and atmos config before processing locals. Includes unit tests with mock StateGetter, integration tests for Go template conditionals with !env, a new locals-conditional fixture, and comprehensive architecture documentation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Update locals example to showcase all features concisely Add Sprig functions (pipe upper), multiple components (myapp-worker with suffixed full_name), and file-scoped isolation commentary. Update README with feature sections and try-it commands. Add TestExampleLocalsWorkerComponent test. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Update locals documentation with processing pipeline and YAML function details Add comprehensive documentation for locals processing pipeline, cross-component references with !terraform.state, environment variable conditionals with !env, and updated best practices. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * update docs * [autofix.ci] apply automated fixes * Fix broken link and add Gomplate/Atmos template function references in locals docs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Improve test coverage and address PR review feedback - Handle .yaml.tmpl/.yml.tmpl extensions in computeStackFileName (longest suffix first) - Add test cases for .yaml.tmpl, .yml.tmpl, no extension, and unknown extension - Use filepath.Join() for all test paths (cross-platform compliance) - Fix absible → ansible typo in locals docs - Fix misleading comment in dev.yaml example - Fix override semantics contradiction in fix doc Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Address PR review: fix test count, remove ansible from locals scopes - Update TestComputeStackFileName case count from 4 to 8 in fix doc - Remove ansible from locals pipeline/scope docs (only terraform/helmfile/packer supported) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
|
Important Cloud Posse Engineering Team Review RequiredThis pull request modifies files that require Cloud Posse's review. Please be patient, and a core maintainer will review your changes. To expedite this process, reach out to us on Slack in the |
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
The aws.github.io/aws-sdk-go-v2 site is no longer available (returns 404). Update the reference to point to the pkg.go.dev canonical documentation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe changes introduce optional GitHub token authentication for artifact downloads in the Go toolchain. Token retrieval is now non-blocking, allowing public repository flows to proceed without authentication. A new error builder improves HTTP failure diagnostics. Documentation is updated to reflect unauthenticated access capability. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
examples/locals/components/terraform/myapp/main.tf (1)
36-50: Outputs look fine for demo purposes.The outputs are straightforward passthrough values. For a mock component in the examples directory, this is adequate. If you want consistency with the variable definitions, you could add
descriptionattributes to the outputs, but it's optional for fixture code.💡 Optional: Add descriptions to outputs for consistency
output "name" { + description = "Application name" value = var.name } output "full_name" { + description = "Full application name (computed from locals)" value = var.full_name } output "deploy_target" { + description = "Deployment target (computed from conditional locals)" value = var.deploy_target } output "tags" { + description = "Resource tags (computed from locals)" value = var.tags }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/locals/components/terraform/myapp/main.tf` around lines 36 - 50, Outputs in main.tf are plain passthroughs and the reviewer suggested optionally adding descriptions for consistency; update the output blocks for name, full_name, deploy_target, and tags to include a description attribute (e.g., description = "Component name", "Full name of the component", "Deployment target", "Tags map/list") so each output block (output "name", output "full_name", output "deploy_target", output "tags") includes a descriptive string matching the intent of the corresponding variable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@examples/locals/components/terraform/myapp/main.tf`:
- Around line 36-50: Outputs in main.tf are plain passthroughs and the reviewer
suggested optionally adding descriptions for consistency; update the output
blocks for name, full_name, deploy_target, and tags to include a description
attribute (e.g., description = "Component name", "Full name of the component",
"Deployment target", "Tags map/list") so each output block (output "name",
output "full_name", output "deploy_target", output "tags") includes a
descriptive string matching the intent of the corresponding variable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e4b7cc25-304e-46f0-b61b-a425b0d3e8c9
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (23)
NOTICEdocs/fixes/2026-03-15-locals-terraform-state-missing-stack-context.mddocs/prd/auth-context-multi-identity.mdexamples/locals/README.mdexamples/locals/components/terraform/myapp/main.tfexamples/locals/stacks/deploy/dev.yamlexamples/locals/stacks/deploy/prod.yamlexamples/quick-start-advanced/Dockerfilego.modinternal/exec/stack_processor_utils.gointernal/exec/stack_processor_utils_test.gointernal/terraform_backend/terraform_backend_s3.gopkg/ai/analyze/analyze_test.gopkg/devcontainer/lifecycle_rebuild_test.gopkg/github/artifacts.gopkg/toolchain/pr_artifact.gopkg/toolchain/sha_artifact.gotests/cli_locals_test.gotests/fixtures/scenarios/locals-conditional/atmos.yamltests/fixtures/scenarios/locals-conditional/components/terraform/mock/main.tftests/fixtures/scenarios/locals-conditional/stacks/deploy/pr-empty.yamltests/fixtures/scenarios/locals-conditional/stacks/deploy/pr-set.yamlwebsite/docs/stacks/locals.mdx
💤 Files with no reviewable changes (1)
- internal/terraform_backend/terraform_backend_s3.go
Member
Author
|
@copilot this looks like a dirty merge update. It's showing thousands of lines changed, even though it was just a simple change. |
Contributor
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2212 +/- ##
==========================================
+ Coverage 76.83% 76.91% +0.07%
==========================================
Files 1001 1001
Lines 95361 95383 +22
==========================================
+ Hits 73274 73363 +89
+ Misses 17820 17746 -74
- Partials 4267 4274 +7
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Cover all HTTP error branches (401/403/429/5xx) with and without tokens, and verify downloadPRArtifact sends auth headers only when token is set. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
💥 This pull request now has conflicts. Could you fix it @osterman? 🙏 |
…act-fetch # Conflicts: # docs/prd/auth-context-multi-identity.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
what
--use-versionflagGetGitHubTokenOrError()gate with optionalGetGitHubToken()inInstallFromPR()andInstallFromSHA()Authorizationheader when token is unavailable indownloadPRArtifact()buildDownloadHTTPError()to distinguish auth failures from rate limitingwhy
references
atmos --use-version=2129fails with "authentication failed" when noGITHUB_TOKENis setSummary by CodeRabbit