Skip to content

feat: terragrunt filter + close discover git/kubectl/glab gaps - #13

Merged
maxkulish merged 4 commits into
masterfrom
feat/discover-coverage
Jul 1, 2026
Merged

maxkulish merged 4 commits into
masterfrom
feat/discover-coverage

Conversation

@maxkulish

Copy link
Copy Markdown
Owner

Motivation

Driven by a Linux user's rtk discover output: ~3.3K commands/report were flagged as "unhandled — open an issue?" that rtk actually handles but failed to detect, plus a genuinely missing terragrunt filter that the registry already promised.

What changed

feat(terragrunt) — new rtk terragrunt filter

Real Terragrunt 1.0.8 output is ANSI-colored and log-prefixed (<ts> LEVEL terraform: <content>), not the old time=level=msg= format. The filter strips ANSI + the log prefix, drops init/refresh/progress noise, and keeps the review essentials: resource action headers (# X will be …), in-place attribute changes (~ attr = old -> new), Plan: / Apply complete! / No changes summaries, Changes to Outputs / Outputs, and error boxes. 80–92% reduction.

Fixtures were captured from a real cloud-free null_resource project (tests/fixtures/terragrunt_*.txt); apply/no-change/update are synthesized because terragrunt apply -auto-approve is safety-gated.

feat(discover) — close git/kubectl/glab detection gaps

fix(kubectl) — surface errors + route global-flags-first

  • rtk kubectl <global flags> <subcommand> was failing clap parsing and falling back to raw (unfiltered). normalize_kubectl_argv reorders global flags after the subcommand so it routes through the filter. Behavior-preserving (kubectl accepts flags anywhere); unknown input still falls back to raw (no regression).
  • Doing that surfaced a latent bug: kubectl_pods/kubectl_services ignored the exit code and masked real failures (bad context, RBAC, expired SSO) as "No pods found" with exit 0. Now they check status.success(), print stderr, and propagate the real exit code.

Upstream analysis (rtk-ai/rtk @ v0.43.0)

  • terragrunt: absent entirely upstream — net-new here.
  • git -C / abs-paths: already fixed upstream → ported rather than reinvented.
  • kubectl global flags / $(which) literal / raw-fallback: still broken upstream → candidates to contribute back.

Testing

  • All 905 tests pass; cargo fmt clean; zero new clippy warnings in changed files; scripts/validate-docs.sh passes (60 modules).
  • Verified end-to-end via the hook path (rtk rewrite) and real execution (rtk terragrunt plan, rtk kubectl --context … get pods, rtk kubectl config get-contexts, rtk git describe).

Release impact

Two feats → Release Please will cut a minor bump. Docs (README/CLAUDE/ARCHITECTURE) updated in-PR.

maxkulish added 3 commits July 1, 2026 11:43
Wrapped and non-dedicated commands were reported as "unhandled" even
though rtk handles them, so the hook never rewrote them either:

- Normalize before classifying: strip git global opts (-C/-c, ported
  from upstream rtk-ai#163), absolute binary paths (upstream rtk-ai#485), kubectl
  global flags before the subcommand, $(which x)/backtick wrappers, and
  a leading backslash line-continuation. Rewrite preserves the flags.
- Classify git/kubectl passthrough subcommands (checkout, rebase,
  describe, config, ...) and glab (mr/ci/issue compact, rest
  passthrough) via a 0%-savings -> Passthrough status heuristic.
kubectl_pods/kubectl_services ignored the exit code, so a failed kubectl
(bad context, RBAC denial, expired SSO token) produced empty stdout that
parsed to "No pods found" with exit 0. Check status.success() first,
print stderr, and propagate the real exit code, matching the behavior
kubectl_get_generic already had.
…flags

- New `rtk terragrunt`: strip ANSI + the terragrunt log prefix
  (<ts> LEVEL terraform: ...), drop init/refresh/progress noise, and keep
  resource action headers, in-place attribute changes, Plan/Apply/No-changes
  summaries, Changes to Outputs / Outputs, and error boxes. 80-92%
  reduction; validated against real Terragrunt 1.0.8 (fixtures captured
  from a null_resource project, apply/no-change/update synthesized).
- main.rs: reorder 'rtk kubectl <global flags> <subcommand>' so clap routes
  it through the filter instead of falling back to raw (unfiltered).
- Docs: ARCHITECTURE (60 modules), CLAUDE.md, README.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a new terragrunt command and output filter to compact Terragrunt, Terraform, and OpenTofu execution logs. It also adds command normalization and passthrough tracking for git, kubectl, and glab subcommands, allowing global flags (like -C or --context) to be preserved or reordered correctly. Additionally, it improves error visibility for kubectl failures. The reviewer identified a high-severity issue in the Terragrunt filter where global deduplication would inadvertently strip identical attribute changes across different resources, and provided a code suggestion to restrict deduplication to error, warning, and box framing lines.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/terragrunt_cmd.rs Outdated
Comment on lines +219 to +220
let mut seen = std::collections::HashSet::new();
kept.retain(|line| seen.insert(line.clone()));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Global deduplication of all kept lines will inadvertently strip identical attribute changes from different resources (e.g., if multiple resources have the exact same ~ triggers = { or ~ "foo" = "bar" -> "CHANGED" lines). This makes the plan output incomplete and misleading for resources with duplicate changes.\n\nTo fix this while still deduplicating the duplicate error/warning boxes printed by Terragrunt, we should restrict deduplication to only error, warning, and box framing lines.

    let mut seen = std::collections::HashSet::new();\n    kept.retain(|line| {\n        let trimmed = line.trim();\n        if is_box_line(trimmed) || trimmed.starts_with("Error:") || trimmed.starts_with("Warning:") {\n            seen.insert(line.clone())\n        } else {\n            true\n        }\n    });

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, fixed in 9470998. You are right that global dedup would drop identical in-place changes shared by different resources (common when the same attribute changes across a fleet). Restricted dedup to error/warning/box lines, which are the only content Terragrunt actually prints twice (the STDERR box + the error occurred: footer).

Added two regression tests: test_identical_changes_across_resources_preserved (two resources with the same ~ instance_type change, both survive) and test_error_box_deduplicated_to_single_copy (box still collapses to one copy).

Global deduplication dropped identical in-place change lines shared by
different resources (e.g. the same instance_type bump across a fleet),
making plans incomplete. Restrict dedup to error/warning/box framing -
the only lines Terragrunt genuinely prints twice (STDERR + footer).

Addresses Gemini review on PR #13.
@maxkulish
maxkulish merged commit 5d9eb9c into master Jul 1, 2026
4 checks passed
@maxkulish
maxkulish deleted the feat/discover-coverage branch July 1, 2026 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant