From d5ccbcf9aa1ab894c155fa8a981c2093a0e54822 Mon Sep 17 00:00:00 2001 From: Olof Mattsson Date: Thu, 28 May 2026 14:41:45 +0200 Subject: [PATCH 1/2] =?UTF-8?q?test(live):=20expand=20suite=20=E2=80=94=20?= =?UTF-8?q?apikey=20CRUD,=20user=20lifecycle,=20template=20versions,=20cle?= =?UTF-8?q?anup=20policies,=20cluster=20CRUD?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds five new endpoint-group live tests covering the highest-blast-radius surfaces still missing from cli/test/live/. All tests are wire-shape focused (no real workloads created), follow the existing helpers/cleanup conventions in this package, and run cleanly under the CI api-only flow introduced in #99. New files: apikey_live_test.go - Create → list → revoke cycle against the calling user (whoami). - Locks the raw_key contract: sk_-prefixed, returned once, never in list. Was implicitly relied on by the CI bootstrap but never asserted. user_live_test.go - Register a throwaway user, list (admin-only path), disable, enable, reset-password, delete. Never operates on admin — locking out the caller would break the rest of the suite. template_versions_live_test.go - Publishes the same template twice (description-only change in between) to materialise two version snapshots, then exercises list → get → diff with shape assertions on left/right/chart_diffs. cleanup_policy_live_test.go - Full admin CRUD plus a dry-run execution. The condition "idle_days:9999" deliberately matches nothing so the run never mutates a real instance. cluster_lifecycle_live_test.go - Stub-cluster create → get → update → delete. IsDefault stays false so the test never disrupts requireCluster() for other tests. Exercises registry_* + image_pull_secret_name fields (the registry_password drift in PR #95 is the canonical example of why this surface needs a live test). Bonus findings surfaced during local validation against rancher-desktop (left as commented follow-ups, not blockers for this PR): - stackctl's UpdateTemplateRequest.Name is `omitempty` but the backend rejects PUT with "name is required" when omitted. Either drop the omitempty or relax the backend. - stackctl's CreateTemplateRequest has no `version` field, so the template-level Version is unsettable through the CLI — the version snapshot's `version` round-trips empty as a result. - Backend rejects kubeconfig_data unless KUBECONFIG_ENCRYPTION_KEY is configured (the CI compose env doesn't set it); kubeconfig_path works without that prerequisite. Verified locally against rancher-desktop k8s-stack-manager: full live suite passes (21 passed, 2 skipped by design, 0 failed). Co-Authored-By: Claude Opus 4.7 --- cli/test/live/apikey_live_test.go | 76 +++++++++++++ cli/test/live/cleanup_policy_live_test.go | 106 +++++++++++++++++++ cli/test/live/cluster_lifecycle_live_test.go | 89 ++++++++++++++++ cli/test/live/template_versions_live_test.go | 102 ++++++++++++++++++ cli/test/live/user_live_test.go | 92 ++++++++++++++++ 5 files changed, 465 insertions(+) create mode 100644 cli/test/live/apikey_live_test.go create mode 100644 cli/test/live/cleanup_policy_live_test.go create mode 100644 cli/test/live/cluster_lifecycle_live_test.go create mode 100644 cli/test/live/template_versions_live_test.go create mode 100644 cli/test/live/user_live_test.go diff --git a/cli/test/live/apikey_live_test.go b/cli/test/live/apikey_live_test.go new file mode 100644 index 0000000..12823c5 --- /dev/null +++ b/cli/test/live/apikey_live_test.go @@ -0,0 +1,76 @@ +//go:build live + +package live + +import ( + "testing" + "time" + + "github.com/omattsson/stackctl/cli/pkg/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestLiveAPIKey_CRUD locks the API-key wire contract end-to-end: +// create → list → revoke. Catches drift on the highest-blast-radius +// surface — a field-name regression here would mask every other +// authenticated test (CI mints its key via this exact flow). +// +// We scope every key to the calling user (whoami) and revoke it on +// cleanup so the test leaves no debris in the user's keyring. +func TestLiveAPIKey_CRUD(t *testing.T) { + c := newLiveClient(t) + login(t, c) + + me, err := c.Whoami() + require.NoError(t, err, "whoami") + require.NotEmpty(t, me.ID, "whoami must return a populated user id") + + prefix := liveResourcePrefix() + days := 1 + + // Create + created, err := c.CreateAPIKey(me.ID, &types.CreateAPIKeyRequest{ + Name: prefix + "-apikey", + ExpiresInDays: &days, + }) + require.NoError(t, err, "create api key") + require.NotEmpty(t, created.ID, "created key must have an ID") + require.NotEmpty(t, created.RawKey, "raw_key must be populated on create (this is the only time it's returned)") + assert.Truef(t, len(created.RawKey) > len("sk_"), "raw_key %q must be longer than the sk_ prefix", created.RawKey) + assert.Equal(t, "sk_", created.RawKey[:3], "raw_key must carry the sk_ prefix") + assert.NotEmpty(t, created.Prefix, "prefix must be set so the key shows up in list") + require.NotNil(t, created.ExpiresAt, "expires_at must echo back on create when expires_in_days was set") + assert.True(t, created.ExpiresAt.After(time.Now()), "expires_at must be in the future") + + // Always best-effort revoke so a failed assertion doesn't leave the + // key around the calling user's keyring. + t.Cleanup(func() { + _ = c.DeleteAPIKey(me.ID, created.ID) + }) + + // List — the new key must be visible. Raw key MUST NOT come back on list. + keys, err := c.ListAPIKeys(me.ID) + require.NoError(t, err, "list api keys") + var found *types.APIKey + for i := range keys { + if keys[i].ID == created.ID { + found = &keys[i] + break + } + } + require.NotNilf(t, found, "newly-created key %s must appear in list", created.ID) + assert.Equal(t, created.Prefix, found.Prefix, "prefix must match between create and list responses") + assert.Equal(t, me.ID, found.UserID, "list entry must echo back the owning user id") + + // Revoke (explicit — cleanup is the safety net). + require.NoError(t, c.DeleteAPIKey(me.ID, created.ID), "revoke api key") + + // Confirm gone. + after, err := c.ListAPIKeys(me.ID) + require.NoError(t, err, "list api keys after revoke") + for _, k := range after { + assert.NotEqualf(t, created.ID, k.ID, + "revoked key %s must not appear in list", created.ID) + } +} diff --git a/cli/test/live/cleanup_policy_live_test.go b/cli/test/live/cleanup_policy_live_test.go new file mode 100644 index 0000000..776ba16 --- /dev/null +++ b/cli/test/live/cleanup_policy_live_test.go @@ -0,0 +1,106 @@ +//go:build live + +package live + +import ( + "testing" + + "github.com/omattsson/stackctl/cli/pkg/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestLiveCleanupPolicy_CRUDAndDryRun exercises the cleanup-policy +// wire contract end-to-end: create → list → update → run --dry-run → +// delete. Admin-gated on the backend; the CI suite runs as the +// env-seeded admin so all five verbs are reachable. +// +// The run is always invoked with dry_run=true so we never mutate stack +// instances — only the response wire shape ([]CleanupResult) is checked. +// Empty results are fine: the seed backend rarely has stacks matching +// "idle_days:9999", which is the deliberately-impossible condition. +func TestLiveCleanupPolicy_CRUDAndDryRun(t *testing.T) { + c := newLiveClient(t) + login(t, c) + + cluster := requireCluster(t, c) + prefix := liveResourcePrefix() + + // 1. Create — use the "stop" action and an idle_days condition that + // will never match in CI (no stack has been idle for 9999 days), so + // run --dry-run is guaranteed to return an empty result set. + created, err := c.CreateCleanupPolicy(&types.CreateCleanupPolicyRequest{ + Name: prefix + "-cleanup", + ClusterID: cluster.ID, + Action: "stop", + Condition: "idle_days:9999", + Schedule: "0 3 * * *", + Enabled: false, + DryRun: true, + }) + require.NoError(t, err, "create cleanup policy") + require.NotEmpty(t, created.ID, "created policy must have an ID") + assert.Equal(t, cluster.ID, created.ClusterID, "policy must echo cluster_id") + assert.Equal(t, "stop", created.Action, "policy must echo action") + assert.Equal(t, "idle_days:9999", created.Condition, "policy must echo condition") + assert.False(t, created.Enabled, "fresh policy must echo enabled=false") + assert.True(t, created.DryRun, "fresh policy must echo dry_run=true") + + // Always best-effort delete so a failed assertion doesn't leave the + // policy in the cluster's schedule. + t.Cleanup(func() { + _ = c.DeleteCleanupPolicy(created.ID) + }) + + // 2. List — newly-created policy must be visible. + policies, err := c.ListCleanupPolicies() + require.NoError(t, err, "list cleanup policies") + var found *types.CleanupPolicy + for i := range policies { + if policies[i].ID == created.ID { + found = &policies[i] + break + } + } + require.NotNilf(t, found, "newly-created cleanup policy %s must appear in list", created.ID) + assert.Equal(t, created.Name, found.Name, "list entry must echo name") + + // 3. Update — flip the enabled flag. UpdateCleanupPolicy is a full PUT + // so we must re-send every field (the type comment in types.go calls + // this out explicitly). + updated, err := c.UpdateCleanupPolicy(created.ID, &types.UpdateCleanupPolicyRequest{ + Name: created.Name, + ClusterID: created.ClusterID, + Action: created.Action, + Condition: created.Condition, + Schedule: created.Schedule, + Enabled: true, + DryRun: created.DryRun, + }) + require.NoError(t, err, "update cleanup policy") + assert.True(t, updated.Enabled, "enabled flag must round-trip through PUT") + + // 4. Run with dry_run=true — wire-shape assertion only. Backend will + // return an empty slice when nothing matches; that's fine. What + // matters is that the response decodes into []CleanupResult without + // dropping fields. + results, err := c.RunCleanupPolicy(created.ID, true) + require.NoError(t, err, "run cleanup policy (dry-run)") + require.NotNil(t, results, "results slice must be non-nil (may be empty)") + for i, r := range results { + // On a real match each entry must populate the action + + // status fields. Status MUST be one of the documented values. + assert.NotEmptyf(t, r.InstanceID, "results[%d].instance_id must be set", i) + assert.Containsf(t, []string{"success", "error", "dry_run"}, r.Status, + "results[%d].status %q must be one of the documented values", i, r.Status) + } + + // 5. Delete (explicit — cleanup is the safety net). + require.NoError(t, c.DeleteCleanupPolicy(created.ID), "delete cleanup policy") + after, err := c.ListCleanupPolicies() + require.NoError(t, err, "list cleanup policies after delete") + for _, p := range after { + assert.NotEqualf(t, created.ID, p.ID, + "deleted policy %s must not appear in list", created.ID) + } +} diff --git a/cli/test/live/cluster_lifecycle_live_test.go b/cli/test/live/cluster_lifecycle_live_test.go new file mode 100644 index 0000000..8eab634 --- /dev/null +++ b/cli/test/live/cluster_lifecycle_live_test.go @@ -0,0 +1,89 @@ +//go:build live + +package live + +import ( + "testing" + + "github.com/omattsson/stackctl/cli/pkg/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestLiveCluster_CreateUpdateDelete locks the cluster-CRUD wire +// contract end-to-end: create → get → update → delete. The shaped +// fields (registry_password, image_pull_secret_name, max_namespaces, +// etc.) drifted twice during the epic #59 work — this catches that +// class of regression on the lifecycle path. +// +// IsDefault is deliberately set to false on both create and update — +// flipping the default would disrupt every other test that resolves +// the default cluster via requireCluster(). +func TestLiveCluster_CreateUpdateDelete(t *testing.T) { + c := newLiveClient(t) + login(t, c) + + prefix := liveResourcePrefix() + maxNS := 5 + + // 1. Create — registry fields are exercised because the + // registry_password drift in PR #95 is the canonical example of why + // this surface needs a live test. + // Backend requires api_server_url AND one of {kubeconfig_data, + // kubeconfig_path, use_in_cluster}. We use kubeconfig_path because + // kubeconfig_data is rejected unless KUBECONFIG_ENCRYPTION_KEY is + // configured (often not set in dev/CI). The path string is only + // validated for non-emptiness on create — it isn't dereferenced + // until a deploy actually runs, which we never trigger here. + created, err := c.CreateCluster(&types.CreateClusterRequest{ + Name: prefix + "-cluster", + Description: "live-test stub cluster", + APIServerURL: "https://ci-stub-cluster.invalid:6443", + KubeconfigPath: "/dev/null/ci-stub-kubeconfig", + Region: "ci-region", + MaxNamespaces: maxNS, + IsDefault: false, + UseInCluster: false, + RegistryURL: "registry.example.com", + RegistryUsername: "ci-test", + RegistryPassword: "ci-test-password", + ImagePullSecretName: "ci-test-pull-secret", + }) + require.NoError(t, err, "create cluster") + require.NotEmpty(t, created.ID, "created cluster must have an ID") + assert.Equal(t, prefix+"-cluster", created.Name, "name must round-trip") + + // Always best-effort delete so a failed assertion leaves no debris. + t.Cleanup(func() { + _ = c.DeleteCluster(created.ID) + }) + + // 2. Get — round-trip every field that has historically drifted. + // We can't assert registry_password because the backend treats it + // as write-only (json:"-" on the read path). + got, err := c.GetCluster(created.ID) + require.NoError(t, err, "get cluster by ID") + assert.Equal(t, created.ID, got.ID, "id must round-trip via GET") + assert.Equal(t, "live-test stub cluster", got.Description, "description must round-trip via GET") + assert.False(t, got.IsDefault, "fresh non-default cluster must echo is_default=false") + + // 3. Update — flip the description, leave the rest alone. The CLI + // uses pointer fields on UpdateClusterRequest so unset fields are + // not sent. We re-send the registry fields to confirm the + // registry_password update path doesn't 400 on an unchanged value. + newDesc := "live-test stub cluster (updated)" + pw := "ci-test-password" // intentionally unchanged + updated, err := c.UpdateCluster(created.ID, &types.UpdateClusterRequest{ + Description: &newDesc, + RegistryPassword: &pw, + }) + require.NoError(t, err, "update cluster") + assert.Equal(t, newDesc, updated.Description, "description must round-trip through PUT") + + // 4. Delete (explicit — cleanup is the safety net). + require.NoError(t, c.DeleteCluster(created.ID), "delete cluster") + + // Confirm gone — GET should 404. + _, err = c.GetCluster(created.ID) + require.Error(t, err, "GetCluster on deleted cluster must fail") +} diff --git a/cli/test/live/template_versions_live_test.go b/cli/test/live/template_versions_live_test.go new file mode 100644 index 0000000..ab4dce9 --- /dev/null +++ b/cli/test/live/template_versions_live_test.go @@ -0,0 +1,102 @@ +//go:build live + +package live + +import ( + "testing" + + "github.com/omattsson/stackctl/cli/pkg/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestLiveTemplate_VersionsListGetDiff exercises the template-version +// wire contract: list → get → diff. Each publish creates a new version +// snapshot server-side, so we publish twice (with a description change +// in between) to guarantee at least two diffable versions. +// +// Coverage rationale: the versions endpoints carry a custom envelope +// (TemplateVersionDetail wraps TemplateVersion + Snapshot; the diff +// response is left/right/chart_diffs) — wire-shape regressions on any +// of these would silently break `template versions diff` in the CLI. +func TestLiveTemplate_VersionsListGetDiff(t *testing.T) { + c := newLiveClient(t) + login(t, c) + + prefix := liveResourcePrefix() + + // Create a throwaway template with one chart so the snapshot is + // non-empty. Note: stackctl's CreateTemplateRequest has no `version` + // field, so the template-level Version stays empty and the diff + // response's left.version / right.version come back empty too. We + // assert on the snapshot's template name (always populated) rather + // than .version, and call out the gap below. + tmpl, err := c.CreateTemplate(&types.CreateTemplateRequest{ + Name: prefix + "-versions", + Description: "live-test versions fixture (v1)", + Charts: []types.ChartConfig{ + {ChartName: "noop-a", RepoURL: "", ChartVersion: "0.1.0"}, + }, + }) + require.NoError(t, err, "create template") + require.NotEmpty(t, tmpl.ID, "created template must have an ID") + deleteTemplateIfExists(t, c, tmpl.ID) + + // First publish → version 1. + _, err = c.PublishTemplate(tmpl.ID) + require.NoError(t, err, "publish template (v1)") + + // Update description so the second snapshot differs from the first. + // Backend rejects PUT /api/v1/templates/:id with `name is required` + // when Name is empty (the stackctl type has json:"name,omitempty" but + // the backend treats it as required). Echo the existing name to keep + // it a description-only change. Worth a follow-up: tighten stackctl's + // UpdateTemplateRequest to drop the omitempty on Name, or relax the + // backend. + _, err = c.UpdateTemplate(tmpl.ID, &types.UpdateTemplateRequest{ + Name: tmpl.Name, + Description: "live-test versions fixture (v2)", + }) + require.NoError(t, err, "update template") + + // Second publish → version 2. + _, err = c.PublishTemplate(tmpl.ID) + require.NoError(t, err, "publish template (v2)") + + // 1. List — must return both snapshots. + versions, err := c.ListTemplateVersions(tmpl.ID) + require.NoError(t, err, "list template versions") + require.GreaterOrEqual(t, len(versions), 2, + "two publishes must produce at least two version rows (got %d)", len(versions)) + for i, v := range versions { + assert.Equal(t, tmpl.ID, v.TemplateID, "versions[%d].template_id must echo the template", i) + assert.NotEmpty(t, v.ID, "versions[%d].id must be populated", i) + assert.False(t, v.CreatedAt.IsZero(), "versions[%d].created_at must decode", i) + } + + // 2. Get version detail — TemplateVersionDetail = TemplateVersion + Snapshot. + left := versions[len(versions)-1] // oldest (list is typically newest-first) + right := versions[0] // newest + detail, err := c.GetTemplateVersion(tmpl.ID, right.ID) + require.NoError(t, err, "get template version detail") + assert.Equal(t, right.ID, detail.ID, "detail must echo the requested version id") + assert.NotEmpty(t, detail.Snapshot.Template.Name, "snapshot.template.name must decode") + assert.Len(t, detail.Snapshot.Charts, 1, "snapshot must echo the one inline chart") + assert.Equal(t, "noop-a", detail.Snapshot.Charts[0].ChartName, + "snapshot chart name must round-trip") + + // 3. Diff between the two snapshots — the description-only change is + // captured at the template level, not in chart_diffs, but the chart_diffs + // slice must decode (empty or not). What matters is the left/right + // envelope wire shape. + diff, err := c.DiffTemplateVersions(tmpl.ID, left.ID, right.ID) + require.NoError(t, err, "diff template versions") + require.NotNil(t, diff, "diff response must not be nil") + // diff.{left,right}.version mirrors the template-level Version which + // stackctl can't set today (CreateTemplateRequest has no version + // field — a follow-up gap). Assert on the snapshot.template.name + // instead, which always round-trips. + assert.NotEmpty(t, diff.Left.Snapshot.Template.Name, "diff.left.snapshot.template.name must decode") + assert.NotEmpty(t, diff.Right.Snapshot.Template.Name, "diff.right.snapshot.template.name must decode") + assert.NotNil(t, diff.ChartDiffs, "chart_diffs must be a populated slice, even if empty") +} diff --git a/cli/test/live/user_live_test.go b/cli/test/live/user_live_test.go new file mode 100644 index 0000000..f74dfa6 --- /dev/null +++ b/cli/test/live/user_live_test.go @@ -0,0 +1,92 @@ +//go:build live + +package live + +import ( + "testing" + + "github.com/omattsson/stackctl/cli/pkg/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestLiveUser_RegisterListDisableEnable creates a throwaway user via +// /auth/register, exercises the admin user-management endpoints +// against it, then deletes it. Never operates on the calling user +// (admin) — locking the admin out would break the rest of the suite. +// +// The backend requires SELF_REGISTRATION=true for the register +// endpoint to be open without admin credentials; the CI compose +// stack ships with that default. We never assert role/serviceaccount +// because /auth/register forces role=user server-side. +func TestLiveUser_RegisterListDisableEnable(t *testing.T) { + c := newLiveClient(t) + login(t, c) + + prefix := liveResourcePrefix() + username := prefix + "-user" + + // 1. Register + created, err := c.Register(&types.RegisterRequest{ + Username: username, + Password: "ci-throwaway-password-1", + DisplayName: "CI throwaway", + }) + require.NoError(t, err, "register throwaway user") + require.NotEmpty(t, created.ID, "registered user must have an ID") + assert.Equal(t, username, created.Username, "response must echo the username") + + // Always delete on cleanup so the suite doesn't accumulate users. + t.Cleanup(func() { + _ = c.DeleteUser(created.ID) + }) + + // 2. List — registered user must be visible to the admin caller. + users, err := c.ListUsers() + require.NoError(t, err, "list users (admin)") + var found *types.User + for i := range users { + if users[i].ID == created.ID { + found = &users[i] + break + } + } + require.NotNilf(t, found, "newly-registered user %s must appear in list", username) + assert.False(t, found.Disabled, "freshly-registered user must not be disabled") + + // 3. Disable → re-list and verify the flag flipped. + require.NoError(t, c.DisableUser(created.ID), "disable user") + after, err := c.ListUsers() + require.NoError(t, err, "list users after disable") + for _, u := range after { + if u.ID == created.ID { + assert.True(t, u.Disabled, "user must be marked disabled after DisableUser") + } + } + + // 4. Enable → re-list and verify the flag flipped back. + require.NoError(t, c.EnableUser(created.ID), "enable user") + after2, err := c.ListUsers() + require.NoError(t, err, "list users after enable") + for _, u := range after2 { + if u.ID == created.ID { + assert.False(t, u.Disabled, "user must be re-enabled after EnableUser") + } + } + + // 5. ResetUserPassword — admin path. We don't try to authenticate as the + // new user, just verify the endpoint returns 204 for a local-provider + // account. A rejection would surface a 400 (length) or 403 (auth) — both + // would fail this assertion. + require.NoError(t, c.ResetUserPassword(created.ID, "ci-rotated-password-2"), + "reset user password") + + // 6. Explicit delete (cleanup is the safety net) — confirms gone. + require.NoError(t, c.DeleteUser(created.ID), "delete throwaway user") + final, err := c.ListUsers() + require.NoError(t, err, "list users after delete") + for _, u := range final { + assert.NotEqualf(t, created.ID, u.ID, + "deleted user %s must not appear in list", created.ID) + } +} From 31716957600d6db40a3ca53da4d9a1b46d77701c Mon Sep 17 00:00:00 2001 From: Olof Mattsson Date: Thu, 28 May 2026 15:15:47 +0200 Subject: [PATCH 2/2] =?UTF-8?q?test(live):=20address=20review=20=E2=80=94?= =?UTF-8?q?=20guard=20raw=5Fkey=20slice=20+=20find-first=20user=20lookups?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two correctness fixes from CodeRabbit on PR #100: - apikey_live_test.go: promote the raw_key length check from assert.Truef to require.Truef. A failing length check followed by created.RawKey[:3] would panic instead of failing cleanly. - user_live_test.go: after DisableUser/EnableUser, look up the created user in the list response first (find-first pattern, matched by the other live tests) and require.NotNil before asserting on the flag. The previous range-and-skip would silently pass if the user was missing from the response. The companion table-driven + t.Parallel() refactor suggestion for cleanup_policy_live_test.go is deliberately skipped — same reason as on PR #99: every other *_live_test.go file in this package uses ad-hoc subtests and runs serially against a shared backend by design. Co-Authored-By: Claude Opus 4.7 --- cli/test/live/apikey_live_test.go | 3 ++- cli/test/live/user_live_test.go | 24 +++++++++++++++++------- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/cli/test/live/apikey_live_test.go b/cli/test/live/apikey_live_test.go index 12823c5..710469a 100644 --- a/cli/test/live/apikey_live_test.go +++ b/cli/test/live/apikey_live_test.go @@ -37,7 +37,8 @@ func TestLiveAPIKey_CRUD(t *testing.T) { require.NoError(t, err, "create api key") require.NotEmpty(t, created.ID, "created key must have an ID") require.NotEmpty(t, created.RawKey, "raw_key must be populated on create (this is the only time it's returned)") - assert.Truef(t, len(created.RawKey) > len("sk_"), "raw_key %q must be longer than the sk_ prefix", created.RawKey) + // require (not assert) so a short response doesn't panic the slice below. + require.Truef(t, len(created.RawKey) > len("sk_"), "raw_key %q must be longer than the sk_ prefix", created.RawKey) assert.Equal(t, "sk_", created.RawKey[:3], "raw_key must carry the sk_ prefix") assert.NotEmpty(t, created.Prefix, "prefix must be set so the key shows up in list") require.NotNil(t, created.ExpiresAt, "expires_at must echo back on create when expires_in_days was set") diff --git a/cli/test/live/user_live_test.go b/cli/test/live/user_live_test.go index f74dfa6..346f7ff 100644 --- a/cli/test/live/user_live_test.go +++ b/cli/test/live/user_live_test.go @@ -54,25 +54,35 @@ func TestLiveUser_RegisterListDisableEnable(t *testing.T) { require.NotNilf(t, found, "newly-registered user %s must appear in list", username) assert.False(t, found.Disabled, "freshly-registered user must not be disabled") - // 3. Disable → re-list and verify the flag flipped. + // 3. Disable → re-list and verify the flag flipped. Find-first so a + // missing user in the response fails the test rather than silently + // no-op'ing the assertion. require.NoError(t, c.DisableUser(created.ID), "disable user") after, err := c.ListUsers() require.NoError(t, err, "list users after disable") - for _, u := range after { - if u.ID == created.ID { - assert.True(t, u.Disabled, "user must be marked disabled after DisableUser") + var disabled *types.User + for i := range after { + if after[i].ID == created.ID { + disabled = &after[i] + break } } + require.NotNilf(t, disabled, "user %s must still be in list after DisableUser", created.ID) + assert.True(t, disabled.Disabled, "user must be marked disabled after DisableUser") // 4. Enable → re-list and verify the flag flipped back. require.NoError(t, c.EnableUser(created.ID), "enable user") after2, err := c.ListUsers() require.NoError(t, err, "list users after enable") - for _, u := range after2 { - if u.ID == created.ID { - assert.False(t, u.Disabled, "user must be re-enabled after EnableUser") + var enabled *types.User + for i := range after2 { + if after2[i].ID == created.ID { + enabled = &after2[i] + break } } + require.NotNilf(t, enabled, "user %s must still be in list after EnableUser", created.ID) + assert.False(t, enabled.Disabled, "user must be re-enabled after EnableUser") // 5. ResetUserPassword — admin path. We don't try to authenticate as the // new user, just verify the endpoint returns 204 for a local-provider