Skip to content

Commit 07530d2

Browse files
authored
feat: allow injecting a pre-configured AWS client into pkg/snipe (#124)
Snipe always resolved its AWS client via spawnaws.NewClientWithRegion, which loads config through the default credential chain — no way to point it at a custom endpoint. spawnaws.Client already has NewClientFromConfig, documented as "used in tests to point all SDK calls at an emulator such as Substrate", but Snipe had no way to reach it: acquirer.clientFor is unexported and Options had no field for injecting one. Adds Options.ClientFor: an optional override for the per-region client builder, mirroring acquirer's internal clientFor shape. Snipe still calls it once per distinct region and caches the result for the run, exactly like the default spawnaws.NewClientWithRegion path (which remains the zero value behavior). Adds pkg/snipe/snipe_substrate_test.go: - TestSnipe_AgainstSubstrate exercises Snipe's real request-building/ response-parsing/retry code against a Substrate-emulated EC2 endpoint, the test tier #113 describes losing when calque's Acquirer migrates to Snipe. - TestSnipe_ClientForOverridesDefault proves ClientFor is actually threaded through (not merely accepted and ignored) by asserting a sentinel error from a fake ClientFor surfaces from Snipe. Confirmed both new tests fail to compile without the Options.ClientFor field (go vet: "unknown field ClientFor in struct literal") and pass with it restored. Fixes #113
1 parent f364aac commit 07530d2

3 files changed

Lines changed: 148 additions & 9 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2626
ordinary PR CI: one asserts `.goreleaser.yaml`'s ldflag still names the
2727
`cmd.Version` variable this package actually declares, the other asserts
2828
the release workflow still invokes the guard script.
29+
- **`pkg/snipe.Options.ClientFor` — inject a pre-configured `*spawnaws.Client`
30+
builder, e.g. one pointed at a test emulator like Substrate** (#113).
31+
`Snipe` previously always resolved its AWS client via
32+
`spawnaws.NewClientWithRegion`, which loads config through the default AWS
33+
credential chain with no way to point it at a custom endpoint — so a
34+
consumer wanting to exercise `Snipe`'s real request-building/response-
35+
parsing/error-classification code against a fake EC2 endpoint (as calque's
36+
own `Acquirer` could, via `github.com/scttfrdmn/substrate/emulator`, before
37+
migrating to `Snipe`) had no way to do so. `Options.ClientFor`, if set,
38+
overrides the client-resolution function Snipe calls once per distinct
39+
region (cached for the run, same as the default); a caller sets it to
40+
something built via `spawnaws.NewClientFromConfig` with
41+
`config.WithBaseEndpoint` pointed at an emulator. Zero value is unchanged:
42+
the default AWS credential chain, same as before this field existed.
2943

3044
### Fixed
3145
- **`pkg/snipe.Result.AvailabilityZone` now reports the AZ the instance

‎pkg/snipe/snipe.go‎

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,20 @@ type Options struct {
153153
// id, in-region launch artifacts/SG/subnet, etc. Off by default; opt-in only.
154154
// A terminal failure on any target still stops immediately.
155155
Fallbacks []Target
156+
// ClientFor, if set, overrides how Snipe resolves the *spawnaws.Client for a
157+
// given region — the same shape as acquirer's internal clientFor (#113). By
158+
// default Snipe builds one via spawnaws.NewClientWithRegion, which loads AWS
159+
// config through the default credential chain with no way to point it at a
160+
// custom endpoint. A caller that wants Snipe's real request-building/
161+
// response-parsing/error-classification code exercised against a fake EC2
162+
// endpoint (e.g. github.com/scttfrdmn/substrate/emulator) sets ClientFor to
163+
// something built via spawnaws.NewClientFromConfig with
164+
// config.WithBaseEndpoint pointed at that endpoint — mirroring
165+
// NewClientFromConfig's own doc comment ("Used in tests to point all SDK
166+
// calls at an emulator such as Substrate"), which pkg/snipe had no way to
167+
// reach before this field existed. Snipe still calls this once per distinct
168+
// region and caches the result for the run, exactly like the default.
169+
ClientFor func(ctx context.Context, region string) (*spawnaws.Client, error)
156170
}
157171

158172
// Result is what Snipe returns on success — the launched instance's identity
@@ -188,21 +202,31 @@ type acquirer struct {
188202
sleep func(ctx context.Context, d time.Duration) error
189203
}
190204

191-
func newAcquirer() *acquirer {
192-
return &acquirer{clientFor: cachedClientFor(), provide: launcher.Provision, sleep: sleepCtx}
205+
// newAcquirer builds an *acquirer using build to construct a client for each
206+
// distinct region on first use, cached for the rest of the run — a
207+
// multi-region Snipe (Options.Fallbacks) can revisit the same region many
208+
// times across retry rounds. build defaults to spawnaws.NewClientWithRegion
209+
// (the real-AWS default-credential-chain path); a caller-supplied
210+
// Options.ClientFor (#113) is threaded in here instead, so a test can point
211+
// every client this acquirer ever builds at an emulator such as Substrate.
212+
func newAcquirer(build func(ctx context.Context, region string) (*spawnaws.Client, error)) *acquirer {
213+
if build == nil {
214+
build = spawnaws.NewClientWithRegion
215+
}
216+
return &acquirer{clientFor: cachedClientFor(build), provide: launcher.Provision, sleep: sleepCtx}
193217
}
194218

195-
// cachedClientFor returns a clientFor function that builds one
196-
// region-pinned *spawnaws.Client per distinct region on first use and reuses
197-
// it for the rest of the run — a multi-region Snipe (Options.Fallbacks) can
198-
// revisit the same region many times across retry rounds.
199-
func cachedClientFor() func(ctx context.Context, region string) (*spawnaws.Client, error) {
219+
// cachedClientFor wraps build so it is called at most once per distinct
220+
// region for the lifetime of the returned function, regardless of how many
221+
// times a multi-round, possibly-multi-region (Options.Fallbacks) Snipe run
222+
// asks for it.
223+
func cachedClientFor(build func(ctx context.Context, region string) (*spawnaws.Client, error)) func(ctx context.Context, region string) (*spawnaws.Client, error) {
200224
clients := make(map[string]*spawnaws.Client)
201225
return func(ctx context.Context, region string) (*spawnaws.Client, error) {
202226
if c, ok := clients[region]; ok {
203227
return c, nil
204228
}
205-
c, err := spawnaws.NewClientWithRegion(ctx, region)
229+
c, err := build(ctx, region)
206230
if err != nil {
207231
return nil, err
208232
}
@@ -227,7 +251,7 @@ func cachedClientFor() func(ctx context.Context, region string) (*spawnaws.Clien
227251
// wait. On success it returns a *Result carrying the launched InstanceID,
228252
// Region, AZ, and Subnet.
229253
func Snipe(ctx context.Context, target Target, opts Options) (*Result, error) {
230-
return snipeWith(ctx, newAcquirer(), target, opts)
254+
return snipeWith(ctx, newAcquirer(opts.ClientFor), target, opts)
231255
}
232256

233257
func snipeWith(ctx context.Context, l *acquirer, target Target, opts Options) (*Result, error) {

‎pkg/snipe/snipe_substrate_test.go‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
package snipe_test
2+
3+
import (
4+
"context"
5+
"strings"
6+
"testing"
7+
8+
awssdk "github.com/aws/aws-sdk-go-v2/aws"
9+
"github.com/aws/aws-sdk-go-v2/service/ssm"
10+
ssmtypes "github.com/aws/aws-sdk-go-v2/service/ssm/types"
11+
"github.com/spore-host/lagotto/pkg/snipe"
12+
"github.com/spore-host/lagotto/pkg/testutil"
13+
spawnaws "github.com/spore-host/spawn/pkg/aws"
14+
)
15+
16+
// TestSnipe_AgainstSubstrate is the #113 regression guard: it exercises
17+
// Snipe's REAL request-building/response-parsing/retry code against a fake
18+
// EC2-shaped endpoint (github.com/scttfrdmn/substrate/emulator), rather than
19+
// a hand-rolled fake `provide` function. Before Options.ClientFor existed,
20+
// Snipe always built its client via spawnaws.NewClientWithRegion — the
21+
// default AWS credential chain, with no way to point it at Substrate's
22+
// in-process test server — so this test tier was impossible for pkg/snipe
23+
// (see #113: this is exactly the tier calque's own Acquirer had, via
24+
// substrate_test.go, before migrating to Snipe).
25+
//
26+
// Mirrors the skip pattern already used for the equivalent spawn/pkg/launcher
27+
// test (TestProvision_EndToEnd): Substrate may not fully model IAM CreateRole
28+
// or RunInstances, so a failure naming those is a skip, not a failure.
29+
func TestSnipe_AgainstSubstrate(t *testing.T) {
30+
env := testutil.SubstrateServer(t)
31+
ctx := context.Background()
32+
33+
// Seed the SSM AMI parameters spawn's launcher reads for AMI auto-detection.
34+
ssmClient := ssm.NewFromConfig(env.AWSConfig)
35+
for name, val := range map[string]string{
36+
"/aws/service/ami-amazon-linux-latest/al2023-ami-kernel-default-x86_64": "ami-x86-standard",
37+
"/aws/service/ami-amazon-linux-latest/al2023-ami-kernel-default-arm64": "ami-arm-standard",
38+
} {
39+
if _, err := ssmClient.PutParameter(ctx, &ssm.PutParameterInput{
40+
Name: awssdk.String(name), Value: awssdk.String(val), Type: ssmtypes.ParameterTypeString,
41+
}); err != nil {
42+
t.Skipf("substrate SSM PutParameter unavailable: %v", err)
43+
}
44+
}
45+
46+
target := snipe.Target{
47+
InstanceType: "m7i.large",
48+
Region: "us-east-1",
49+
}
50+
opts := snipe.Options{
51+
// The whole point: point every client Snipe builds at Substrate's
52+
// in-process server instead of the real AWS default credential chain.
53+
ClientFor: func(context.Context, string) (*spawnaws.Client, error) {
54+
return spawnaws.NewClientFromConfig(env.AWSConfig), nil
55+
},
56+
}
57+
58+
result, err := snipe.Snipe(ctx, target, opts)
59+
if err != nil {
60+
if strings.Contains(err.Error(), "IAM") || strings.Contains(err.Error(), "launch") {
61+
t.Skipf("substrate does not fully model the launch path: %v", err)
62+
}
63+
t.Fatalf("Snipe: %v", err)
64+
}
65+
if result.InstanceID == "" {
66+
t.Error("Snipe returned empty InstanceID")
67+
}
68+
if result.Region != "us-east-1" {
69+
t.Errorf("Region = %q, want us-east-1", result.Region)
70+
}
71+
}
72+
73+
// TestSnipe_ClientForOverridesDefault verifies ClientFor is actually used
74+
// (not just accepted and ignored): a ClientFor that returns a recognizable
75+
// error must be the one Snipe surfaces, proving Snipe called it rather than
76+
// falling back to spawnaws.NewClientWithRegion's real-AWS default chain.
77+
func TestSnipe_ClientForOverridesDefault(t *testing.T) {
78+
wantErr := "sentinel-clientfor-error"
79+
opts := snipe.Options{
80+
ClientFor: func(context.Context, string) (*spawnaws.Client, error) {
81+
return nil, errSentinel{wantErr}
82+
},
83+
// A ClientFor error isn't a smithy API error, so it classifies as
84+
// FailureUnknown and would otherwise retry (with real sleeps) up to
85+
// MaxConsecutiveUnknown times before giving up. Cap at 1 attempt and
86+
// use a near-zero interval so this test asserts the error, not the
87+
// retry/backoff behavior (already covered elsewhere).
88+
MaxConsecutiveUnknown: 1,
89+
}
90+
_, err := snipe.Snipe(context.Background(), snipe.Target{
91+
InstanceType: "m7i.large",
92+
Region: "us-east-1",
93+
}, opts)
94+
if err == nil || !strings.Contains(err.Error(), wantErr) {
95+
t.Errorf("Snipe error = %v, want it to contain %q (proving ClientFor was actually called)", err, wantErr)
96+
}
97+
}
98+
99+
type errSentinel struct{ msg string }
100+
101+
func (e errSentinel) Error() string { return e.msg }

0 commit comments

Comments
 (0)