feat: Add GitHub Actions format to atmos auth env#1984
feat: Add GitHub Actions format to atmos auth env#1984
Conversation
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughReplaces per-format CLI output with a unified pkg/env.Output API, adds --format=github and --output-file (auto-detects $GITHUB_ENV), centralizes format/output handling via Viper, updates completions/tests/snapshots, and adds docs and roadmap entries for GitHub Actions output. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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)
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 |
✨ Finishing touches
🧪 Generate unit tests (beta)
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}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@cmd/auth_env_test.go`:
- Around line 388-555: Add a RunE‑level test that uses t.Setenv("GITHUB_ENV",
filePath) to verify auto‑detect and append behavior: create a temp file with
initial content, call the CLI entrypoint (invoke the command's RunE or root
command Execute so the code path that would call
writeAuthEnvToFile/formatAuthGitHub runs), then read the file and assert it was
appended to (not overwritten) and that multiline values are formatted the same
as writeAuthEnvToFile when using formatAuthGitHub; use t.Setenv for
setup/teardown and reuse writeAuthEnvToFile and formatAuthGitHub as references
to expected formatting.
In `@cmd/auth_env.go`:
- Around line 21-30: The default file mode constant defaultAuthFileMode
currently uses 0o644 which leaves created auth files world-readable; change
defaultAuthFileMode to an owner-only permission (e.g., 0o600) and update its
comment to reflect owner-only access so newly created credential files are not
world-readable; ensure any code that relies on defaultAuthFileMode (e.g., file
creation or write routines) continues to use this constant.
- Around line 228-239: In formatAuthGitHub, the heredoc delimiter
ATMOS_EOF_<key> can collide with the value; change the logic to generate a
delimiter that is guaranteed not to appear in the value (e.g., append a
random/unique token or UUID to the delimiter and retry if
strings.Contains(value, delimiter) is true) and then use that safe delimiter for
both the opening marker and closing marker; update the heredoc construction in
formatAuthGitHub to use the computed safe delimiter and ensure the containment
check uses that same delimiter before writing the heredoc.
- Around line 40-48: The current code reads flags directly using
cmd.Flags().GetString for "format" and "output" which bypasses Viper bindings
and ignores ATMOS_AUTH_ENV_* env overrides; replace those reads with
viper.GetString("auth_env_format") and viper.GetString("auth_env_output")
(preserving the validation against SupportedFormats and the error return using
errUtils.ErrInvalidArgumentError) so the values bound in init() are honored
(refer to the format/output variables and the SupportedFormats check in the
auth_env command).
🧹 Nitpick comments (1)
cmd/auth_env.go (1)
272-288: Consider migrating new flag bindings to StandardParser.This command still wires flags via
viper.BindEnv/BindPFlag. The repo convention isflags.NewStandardParser()to avoid manual binding and keep precedence behavior consistent. As per coding guidelines, follow the standard parser pattern for cmd flags.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmd/auth_env.go (1)
277-287: Refactor to useflags.NewStandardParser()instead of direct Viper binds.The direct
viper.BindEnv()andviper.BindPFlag()calls in init() violate the mandatory StandardParser pattern. Refactor the command to define flags viaStandardParserwith.WithStringFlag(), register them viaparser.RegisterFlags(), and bind them in RunE viaparser.BindFlagsToViper()(seecmd/toolchain/get.gofor the pattern).
🤖 Fix all issues with AI agents
In `@cmd/auth_env.go`:
- Around line 21-27: The constant defaultAuthFileMode currently sets new
credential file permissions to 0o644 which allows world-readable access; change
defaultAuthFileMode in cmd/auth_env.go to 0o600 (owner read/write only) so
exported credentials are created with restrictive permissions suitable for
secret material.
- Around line 251-258: The file I/O error returns in cmd/auth_env.go should use
static sentinel errors from errors/errors.go instead of raw string literals; add
two new sentinel errors (e.g., ErrFileOpen and ErrFileWrite) to
errors/errors.go, import that package into cmd/auth_env.go, and replace the
current fmt.Errorf calls (the open error and the f.WriteString error) to wrap
the sentinel with the underlying error and context using %w (for example:
fmt.Errorf("%w: failed to open file '%s': %v", errors.ErrFileOpen, filePath,
err) and similarly for ErrFileWrite), keeping the existing message/context and
preserving error wrapping so callers can check errors.Is(err,
errors.ErrFileOpen) or errors.Is(err, errors.ErrFileWrite).
- Around line 236-239: The heredoc currently uses a fixed delimiter
"ATMOS_EOF_<key>" which can collide if value contains that line; change the
logic in the block handling multiline values (where variables key, value, sb are
used) to generate a unique delimiter not present in value by e.g. appending a
random or incremental suffix and looping until strings.Contains(value,
delimiter) is false, then use that delimiter in the fmt.Sprintf write instead of
the fixed "ATMOS_EOF_%s"; ensure the new delimiter variable replaces the
existing hard-coded delimiter in the sb.WriteString call.
|
💥 This pull request now has conflicts. Could you fix it @osterman? 🙏 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
website/docs/cli/commands/auth/auth-env.mdx (1)
275-316: AddATMOS_AUTH_ENV_OUTPUT_FILEto the output-file precedence explanation.The docs don't mention that
ATMOS_AUTH_ENV_OUTPUT_FILEtakes precedence over$GITHUB_ENV. The actual behavior is:--output-fileflag >ATMOS_AUTH_ENV_OUTPUT_FILEenv var >$GITHUB_ENV(for--format=githubonly) > stdout. Update the--output-filedescription to clarify this full precedence chain.
🧹 Nitpick comments (1)
pkg/env/output.go (1)
31-58: Consider removing perf.Track from trivial option creators.These option functions are trivial closures that just assign a field. Per repository learnings,
perf.Trackoverhead may exceed the operation cost for such trivial accessors. That said, if linting enforces tracking on all public functions, this is acceptable.
Add --format=github and --output flag to enable direct output to $GITHUB_ENV in GitHub Actions workflows. The github format outputs KEY=value pairs and handles multiline values with heredoc syntax. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
- Add changelog blog post for --format=github feature - Update CI/CD roadmap initiative with new milestone - Increase progress from 35% to 40% Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Update documentation for `atmos auth env` to include: - New `--format=github` option for GitHub Actions integration - New `--output` flag for explicit file output - Updated CI/CD workflow example showing --format=github usage - New ATMOS_AUTH_ENV_OUTPUT environment variable Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Change file permissions from 0o644 to 0o600 for credential files (security) - Use Viper for flag values to honor ATMOS_AUTH_ENV_* env var bindings - Add heredoc delimiter collision protection for multiline values - Add ErrWriteFile sentinel error and wrap file I/O errors with sentinels - Add comprehensive tests for GITHUB_ENV auto-detect and delimiter collision Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Refactor cmd/auth_env.go to use pkg/env package for env var formatting - Use env.ParseFormat() and env.FormatData() instead of custom formatters - Use pkg/github/actions.GetEnvPath() for auto-detecting $GITHUB_ENV - Rename --output flag to --output-file for consistency - Update Viper bindings to auth_env_output_file / ATMOS_AUTH_ENV_OUTPUT_FILE - Handle JSON format separately since pkg/env doesn't support it - Update tests and documentation Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add pkg/env/output.go with unified Output() function that handles all format/destination branching for environment variable output - Add WithFileMode, WithAtmosConfig, and WithFormatOptions options - Simplify cmd/auth_env.go from ~229 to ~156 lines using env.Output() - Simplify cmd/env/env.go from ~227 to ~172 lines using env.Output() - Rename --output to --output-file in atmos env for consistency with atmos auth env - Update ATMOS_ENV_OUTPUT to ATMOS_ENV_OUTPUT_FILE Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The fmt.Print() call is intentional stdout output for shell evaluation (e.g., eval $(atmos env)), not accidental logging of sensitive data. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Update golden snapshots after shellescape.Quote() behavior change - Simple alphanumeric values no longer quoted (mock-identity instead of 'mock-identity') - Update error pattern from "invalid argument" to "unsupported format" Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Windows uses ACLs instead of Unix-style permissions, so file mode assertions always fail. Skip these checks on Windows while still testing the core file writing functionality. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Fix blog post flag name from --output to --output-file - Strengthen JSON test to verify content, not just file existence Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Update pkg/env/output.go writeToFileWithMode to use errUtils.Build with ErrOpenFile and ErrWriteFile sentinels - Update pkg/env/file.go WriteToFile to use the same pattern - Update tests to use assert.ErrorIs() instead of string matching This enables callers to use errors.Is() for robust error checking. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
osterman
force-pushed
the
osterman/auth-env-github-format
branch
from
e8aa11d to
33ab708
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@website/docs/cli/commands/auth/auth-env.mdx`:
- Around line 99-112: Update the documentation text to use the proper platform
capitalization "GitHub Actions" (capital "G" and "H") wherever the platform is
referenced in this section; specifically edit the sentence that currently says
"In GitHub Actions, use `--format=github`..." to ensure "GitHub Actions" is
capitalized while leaving the flag `--format=github` and the commands `atmos
auth env` and `--login` unchanged.
🧹 Nitpick comments (4)
cmd/env/env_test.go (1)
293-309: JSON validation improved.The test now reads file content and verifies keys/values are present. Previous review suggested using
json.Unmarshalfor stricter validation - current approach withassert.Containsis simpler but less rigorous.Consider unmarshalling for full round-trip validation:
♻️ Optional enhancement
// Verify JSON structure matches input. - for key, value := range tt.envVars { - assert.Contains(t, string(content), `"`+key+`"`) - assert.Contains(t, string(content), `"`+value+`"`) - } + var got map[string]string + require.NoError(t, json.Unmarshal(content, &got)) + assert.Equal(t, tt.envVars, got)cmd/env/env.go (1)
20-24: Consider derivingSupportedFormatsfrompkg/env.You're maintaining a separate format list here while
pkg/env.SupportedFormatsexists. Similar pattern is used incmd/auth_env.go(lines 138-144) where formats are built dynamically. This could drift.♻️ Suggested approach
// SupportedFormats lists all supported output formats. var SupportedFormats = append([]string{"json"}, envfmt.FormatStrings()...)Or derive directly from
envfmt.SupportedFormatsplus "json" likecmd/auth_env.godoes.pkg/env/output_test.go (1)
20-33: Consider checking pipe errors.The
os.Pipe()error is silently ignored. While rare, pipe creation can fail. Also, the buffer read (line 32) discards errors.♻️ Safer capture
- r, w, _ := os.Pipe() + r, w, err := os.Pipe() + require.NoError(t, err)- n, _ := r.Read(buf) + n, err := r.Read(buf) + require.NoError(t, err)pkg/env/output.go (1)
31-58:perf.Trackin trivial option functions is unnecessary overhead.Per coding guidelines, trivial getters/setters should skip performance tracking. These option functions are simple closure factories.
♻️ Remove perf.Track from option functions
func WithFileMode(mode os.FileMode) OutputOption { - defer perf.Track(nil, "env.WithFileMode")() - return func(c *outputConfig) { c.fileMode = mode } } func WithAtmosConfig(cfg *schema.AtmosConfiguration) OutputOption { - defer perf.Track(nil, "env.WithAtmosConfig")() - return func(c *outputConfig) { c.atmosConfig = cfg } } func WithFormatOptions(opts ...Option) OutputOption { - defer perf.Track(nil, "env.WithFormatOptions")() - return func(c *outputConfig) { c.formatOpts = opts } }
Add "in GitHub Actions" to clarify that subsequent steps refers to GitHub Actions workflow steps. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
💥 This pull request now has conflicts. Could you fix it @osterman? 🙏 |
1 participant
what
--format=githuboption toatmos auth envcommand--outputflag for explicit file output (appends mode)$GITHUB_ENVenvironment variable when using github formatKEY=valueand multiline values with heredoc syntaxwhy
This eliminates the need for complex shell pipelines like
atmos auth env ... | grep "^export " | sed 's/^export //' >> $GITHUB_ENV. Users can now directly output to GitHub Actions$GITHUB_ENVfile with proper formatting and multiline value handling.references
Closes issue related to GitHub Actions integration workflow simplification.
Summary by CodeRabbit
New Features
Behavior Changes
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.