aws-samples / aws-samples/appmod-blueprints
Tech debt: provider orchestration is shell-in-YAML, with silent-failure traps
- Dominant language
- Shell
- Stars
- 105
- Forks
- 62
- Avg merge
- 11h 17m
- Merged PRs (30d)
- 76
Description
## Problem
The cluster-provider Taskfiles have grown into large programs written as shell embedded in YAML. `cluster-providers/kind-kro-ack/Taskfile.yaml` is ~1,900 lines with 53 tasks; `kind-crossplane/Taskfile.yaml` is ~1,600 with 45. Much of that is multi-line `bash` inside `cmds:` blocks doing control flow, polling loops, JSON assembly and AWS orchestration.
This is hard to read, effectively untestable, and it fails in ways that are specific to the embedding rather than to the logic. Three concrete instances found while implementing the `domainResolver` contract (PR on `feat/cluster-provider-parity-cloudfront`):
**1. `errexit` + `&&` silently aborts with no diagnostics.** These Taskfiles set `set: [errexit, nounset, pipefail]`. Under go-task's mvdan/sh interpreter, a failing `[ ... ] && cmd` as the last statement of a block makes the enclosing compound return non-zero and aborts the script. Observed: a `domain:resolve` implementation exited 1 *before printing any of its six diagnostic lines*, so the operator saw only `task: Failed to run task ...: exit status 1`. The same script ran correctly under `bash -e`. Debugging required tracing with `set -x` inside the task.
**2. Background jobs are killed.** `( ... ) &` inside a `cmds:` block is terminated when the task body exits — verified that the redirect target was never even created. `install:phase1-observability-bg` therefore never ran its 30+ minute AMP seed on either provider; only a fast-failing child ever produced log output, which is why it looked like it worked. Fixed with a double-fork (`sh -c '... &'`), which is not obvious and needs a comment explaining it.
**3. Template-time vs run-time variable evaluation is invisible.** go-task evaluates global `vars:` with `sh:` once at parse time. A task that updates the underlying config cannot change `{{.VAR}}` for the rest of the process, so consumers silently use stale values. Working around this required running a phase as a nested `task` process purely to force a re-parse.
None of these are logic bugs. They are all artifacts of writing a program in shell-inside-YAML, and each one failed *silently*, which is the worst property for provisioning code.
## Additional smells
- `yq '.domain'` returns the literal string `"null"` for a missing key. Non-empty, so it defeats `[ -z ... ]` guards; an `ingress_domain_name: "null"` could reach the cluster secret. Every config read needs `// ""` plus a `"null"` check by hand.
- JSON is assembled by string concatenation in shell, then patched with `jq`. See the `METADATA=` line in `secrets-manager:seed` — a single ~1,200-character line.
- No unit tests exist for any of this. The only validation is running a full install against AWS (1-2 hours, real spend).
- `shellcheck` cannot see the code, because it lives inside YAML string blocks.
## Options
**A. Extract logic into real scripts, keep Taskfile as a thin dispatcher.** Tasks become one-liners calling `scripts/*.sh` with explicit arguments. Immediate wins: `shellcheck` works, functions become testable with `bats`, no YAML-embedding surprises. Smallest change, keeps the existing toolchain.
**B. Rewrite the orchestration in a richer language (Node/TypeScript, Python, or Go).** Real data structures, real error handling, real unit tests, an actual AWS SDK instead of `aws` CLI plus `jq`. Taskfile keeps only the user-facing entry points. Larger effort; adds a runtime dependency, though Node and Python are already present on the workshop IDE and Node is already required by the Backstage build.
**C. Targeted hardening only.** Ban `&&` short-circuits, add a lint that greps for the dangerous patterns, document the three traps above. Cheapest, but leaves the class of bug in place.
## Recommendation
A as an incremental step that pays off immediately and can be done piecemeal, with B evaluated for the genuinely complex orchestration (domain resolution, secret seeding, cluster-secret assembly). C alone is insufficient — trap 1 and 2 both failed silently in shipped code and were only found by deliberate probing.
Whatever is chosen, the acceptance bar should include: the extracted logic is `shellcheck`-clean or type-checked, has unit tests that run without AWS, and no silent-failure path (every abort prints why).
## Notes
Raised while implementing the `domainResolver` extension point. That PR works around all three traps rather than fixing the underlying approach, and adds comments at each site explaining the workaround so they are not "simplified" back later. This issue is the follow-up for doing it properly.
Contributor guide
Research direction
Start by comparing cluster-providers/kind-kro-ack/Taskfile.yaml and cluster-providers/kind-crossplane/Taskfile.yaml, focusing on the embedded orchestration blocks and the existing workarounds. Evaluate extracting those blocks into scripts/*.sh with unit tests using bats, and check the result with shellcheck without AWS. Done means the extracted logic is testable without AWS, shellcheck-clean, and aborts provide diagnostics.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- aws, bash, shell
- Domain
- cloud, devops, infrastructure
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100