Skip to content

Commit 2b7cf85

Browse files
randomizedcoderclaude
authored andcommitted
cleanup: lint fixes, tests, and go 1.26.4 bump on top of #276
review-driven follow-ups to PR #276 (CE-1579): - table-driven tests for the helpers added in #276: TestValueOrDash, TestFormatTimestamp (precision tiers + boundary cases), TestCreateCmd_FlagValidation, expanded TestBuildTemplateEndpointGQLInput, TestModelVersionHash edge cases. - benchmark + correctness check for cmd/serverless/create.go::randomString with two alternative implementations; data confirms keeping the current form (call site is one-shot per CLI invocation). - lint fixes in #276-touched files: 6× 200 -> http.StatusOK in api/model.go, sprintfQuotedString in addModelToRepo.go, two PR-introduced err shadows in create.go and addModelToRepo.go, removed an unjustified //nolint:gosec. - math/rand -> math/rand/v2 for randomString with a documented nolint explaining why crypto/rand is not warranted (display-only template-name uniqueness suffix, not a token). - range over int in randomString. - go.mod bump to 1.26.4, drop explicit toolchain directive. - dep bumps: google/uuid 1.4->1.6, x/crypto 0.50->0.53, x/mod 0.34->0.37, and transitive x/net x/sys x/term x/text. - ci.yml + release.yml setup-go bumped to 1.26.4. build + go vet + full test suite all pass. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 840e220 commit 2b7cf85

10 files changed

Lines changed: 541 additions & 57 deletions

File tree

.github/workflows/ci.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,10 @@ jobs:
55
runs-on: ubuntu-latest
66
steps:
77
- uses: actions/checkout@v4
8-
- name: Setup Go 1.26.3
8+
- name: Setup Go 1.26.4
99
uses: actions/setup-go@v5
1010
with:
11-
go-version: 1.26.3
11+
go-version: 1.26.4
1212
# You can test your matrix by printing the current Go version
1313
- name: Display Go version
1414
run: go version

.github/workflows/release.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ jobs:
3434
- name: Set up Go
3535
uses: actions/setup-go@v5
3636
with:
37-
go-version: "1.26.3"
37+
go-version: "1.26.4"
3838

3939
- name: Run GoReleaser
4040
uses: goreleaser/goreleaser-action@v6

api/model.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,7 @@ func AddModelToRepo(input *AddModelToRepoInput) (*Model, error) {
290290
if err != nil {
291291
return nil, err
292292
}
293-
if res.StatusCode != 200 {
293+
if res.StatusCode != http.StatusOK {
294294
return nil, fmt.Errorf("statuscode %d: %s", res.StatusCode, string(rawData))
295295
}
296296

@@ -374,7 +374,7 @@ query %s {
374374
if err != nil {
375375
return nil, err
376376
}
377-
if res.StatusCode != 200 {
377+
if res.StatusCode != http.StatusOK {
378378
return nil, fmt.Errorf("statuscode %d: %s", res.StatusCode, string(rawData))
379379
}
380380

@@ -492,7 +492,7 @@ func GetModel(input *GetModelInput) (*Model, error) {
492492
if err != nil {
493493
return nil, err
494494
}
495-
if res.StatusCode != 200 {
495+
if res.StatusCode != http.StatusOK {
496496
return nil, fmt.Errorf("statuscode %d: %s", res.StatusCode, string(rawData))
497497
}
498498

@@ -580,7 +580,7 @@ func RemoveModel(input *RemoveModelInput) (*ModelRepoMutationResult, error) {
580580
if err != nil {
581581
return nil, err
582582
}
583-
if res.StatusCode != 200 {
583+
if res.StatusCode != http.StatusOK {
584584
return nil, fmt.Errorf("statuscode %d: %s", res.StatusCode, string(rawData))
585585
}
586586

@@ -714,7 +714,7 @@ func CreateModelRepoUpload(input *CreateModelRepoUploadInput) (*ModelRepoMutatio
714714
if err != nil {
715715
return nil, err
716716
}
717-
if res.StatusCode != 200 {
717+
if res.StatusCode != http.StatusOK {
718718
return nil, fmt.Errorf("statuscode %d: %s", res.StatusCode, string(rawData))
719719
}
720720

@@ -863,7 +863,7 @@ func UpdateModelVersionStatus(hash, status string) (*ModelVersion, error) {
863863
if err != nil {
864864
return nil, err
865865
}
866-
if res.StatusCode != 200 {
866+
if res.StatusCode != http.StatusOK {
867867
return nil, fmt.Errorf("statuscode %d: %s", res.StatusCode, string(rawData))
868868
}
869869

cmd/model/addModelToRepo.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ func runAddModel(cmd *cobra.Command, args []string) {
207207
uploadInput.Name = addModelName
208208

209209
if len(modelFiles) > 0 {
210-
err := uploadModelFiles(modelFiles, uploadInput)
210+
err = uploadModelFiles(modelFiles, uploadInput)
211211
cobra.CheckErr(err)
212212
return
213213
}
@@ -400,7 +400,7 @@ func completeModelUpload(upload *api.ModelRepoUpload, artifactPath string) error
400400
err = fmt.Errorf("upload part %d missing ETag", part.PartNumber)
401401
return
402402
}
403-
completed = append(completed, completedPart{PartNumber: part.PartNumber, ETag: fmt.Sprintf("\"%s\"", etag)})
403+
completed = append(completed, completedPart{PartNumber: part.PartNumber, ETag: fmt.Sprintf("%q", etag)})
404404
}()
405405
if err != nil {
406406
return err

cmd/model/getModels_test.go

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
package model
22

33
import (
4+
"strconv"
45
"testing"
6+
"time"
57

68
"github.com/runpod/runpodctl/api"
79
)
@@ -22,6 +24,23 @@ func TestModelVersionHash(t *testing.T) {
2224
}},
2325
want: "hash-1",
2426
},
27+
{
28+
name: "skips nil version entries",
29+
model: &api.Model{Versions: []*api.ModelVersion{
30+
nil,
31+
{Hash: "hash-after-nil"},
32+
}},
33+
want: "hash-after-nil",
34+
},
35+
{
36+
name: "all hashes blank or whitespace",
37+
model: &api.Model{Versions: []*api.ModelVersion{
38+
{Hash: ""},
39+
{Hash: " "},
40+
{Hash: "\t\n"},
41+
}},
42+
want: "",
43+
},
2544
{
2645
name: "no versions",
2746
model: &api.Model{},
@@ -42,3 +61,94 @@ func TestModelVersionHash(t *testing.T) {
4261
})
4362
}
4463
}
64+
65+
func TestValueOrDash(t *testing.T) {
66+
tests := []struct {
67+
name string
68+
in string
69+
want string
70+
}{
71+
{name: "empty -> dash", in: "", want: "-"},
72+
{name: "whitespace only -> dash", in: " ", want: "-"},
73+
{name: "tabs and newlines -> dash", in: "\t\n ", want: "-"},
74+
{name: "value is preserved", in: "hello", want: "hello"},
75+
{name: "value with surrounding whitespace is trimmed", in: " hi ", want: "hi"},
76+
{name: "internal whitespace preserved", in: "a b c", want: "a b c"},
77+
{name: "single char", in: "x", want: "x"},
78+
{name: "unicode preserved", in: "café", want: "café"},
79+
}
80+
81+
for _, tt := range tests {
82+
t.Run(tt.name, func(t *testing.T) {
83+
if got := valueOrDash(tt.in); got != tt.want {
84+
t.Fatalf("valueOrDash(%q) = %q, want %q", tt.in, got, tt.want)
85+
}
86+
})
87+
}
88+
}
89+
90+
func TestFormatTimestamp(t *testing.T) {
91+
// Reference epoch values for 2021-01-01T00:00:00Z (chosen for stable RFC3339
92+
// formatting across precision tiers, no fractional seconds).
93+
const (
94+
secs = int64(1609459200) // 10 digits
95+
millis = int64(1609459200000) // 13 digits
96+
micros = int64(1609459200000000) // 16 digits
97+
nanos = int64(1609459200000000000) // 19 digits
98+
want = "2021-01-01T00:00:00Z"
99+
)
100+
101+
tests := []struct {
102+
name string
103+
in string
104+
want string
105+
}{
106+
// positive: each precision tier should round to the same RFC3339 second.
107+
{name: "seconds (10 digits)", in: "1609459200", want: want},
108+
{name: "milliseconds (13 digits)", in: "1609459200000", want: want},
109+
{name: "microseconds (16 digits)", in: "1609459200000000", want: want},
110+
{name: "nanoseconds (19 digits)", in: "1609459200000000000", want: want},
111+
112+
// positive: whitespace is trimmed before parsing.
113+
{name: "trims surrounding whitespace", in: " 1609459200 ", want: want},
114+
115+
// negative: empty / blank renders as dash.
116+
{name: "empty -> dash", in: "", want: "-"},
117+
{name: "whitespace only -> dash", in: " ", want: "-"},
118+
119+
// negative: non-numeric is passed through unchanged (ISO strings from
120+
// the API should not be mangled).
121+
{name: "ISO 8601 passthrough", in: "2024-06-10T12:00:00Z", want: "2024-06-10T12:00:00Z"},
122+
{name: "garbage passthrough", in: "not-a-number", want: "not-a-number"},
123+
124+
// corner: epoch zero across precisions.
125+
{name: "epoch zero seconds", in: "0", want: "1970-01-01T00:00:00Z"},
126+
127+
// corner: negative timestamp (pre-epoch).
128+
{name: "negative seconds (pre-epoch)", in: "-1", want: "1969-12-31T23:59:59Z"},
129+
130+
// boundary: default-branch path. A length-11 millisecond timestamp
131+
// (e.g. 99999999999 ms) is < 1e12 so the default branch treats it as
132+
// seconds — documenting the current (lossy) behavior so a future change
133+
// of the heuristic is intentional.
134+
{name: "11-digit value falls through default as seconds", in: "10000000000", want: time.Unix(10000000000, 0).UTC().Format(time.RFC3339)},
135+
{name: "12-digit value above default threshold treated as ms", in: "1000000000001", want: time.Unix(1000000000, 1*int64(time.Millisecond)).UTC().Format(time.RFC3339)},
136+
}
137+
138+
for _, tt := range tests {
139+
t.Run(tt.name, func(t *testing.T) {
140+
if got := formatTimestamp(tt.in); got != tt.want {
141+
t.Fatalf("formatTimestamp(%q) = %q, want %q", tt.in, got, tt.want)
142+
}
143+
})
144+
}
145+
146+
// Sanity-check the precision-tier constants stay aligned: every tier must
147+
// resolve to the same RFC3339 instant when fed through formatTimestamp.
148+
for _, v := range []int64{secs, millis, micros, nanos} {
149+
got := formatTimestamp(strconv.FormatInt(v, 10))
150+
if got != want {
151+
t.Fatalf("precision tier %d produced %q, want %q", v, got, want)
152+
}
153+
}
154+
}

cmd/serverless/create.go

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ package serverless
33
import (
44
"encoding/json"
55
"fmt"
6-
"math/rand"
6+
"math/rand/v2"
77
"strings"
88

99
"github.com/runpod/runpodctl/internal/api"
@@ -183,7 +183,6 @@ func runCreate(cmd *cobra.Command, args []string) error {
183183
endpointName = listing.Title
184184
}
185185

186-
//nolint:gosec
187186
templateName := fmt.Sprintf("%s__template__%s", endpointName, randomString(7))
188187

189188
gqlReq := &api.EndpointCreateGQLInput{
@@ -273,14 +272,14 @@ func runCreate(cmd *cobra.Command, args []string) error {
273272

274273
if len(createModelReferences) > 0 {
275274
gqlReq := buildTemplateEndpointGQLInput(req, gpuTypeID, createDataCenterIDs, createModelReferences)
276-
endpoint, err := client.CreateEndpointGQL(gqlReq)
277-
if err != nil {
278-
output.Error(err)
279-
return fmt.Errorf("failed to create endpoint: %w", err)
275+
gqlEndpoint, gqlErr := client.CreateEndpointGQL(gqlReq)
276+
if gqlErr != nil {
277+
output.Error(gqlErr)
278+
return fmt.Errorf("failed to create endpoint: %w", gqlErr)
280279
}
281280

282281
format := output.ParseFormat(cmd.Flag("output").Value.String())
283-
return output.Print(endpoint, &output.Config{Format: format})
282+
return output.Print(gqlEndpoint, &output.Config{Format: format})
284283
}
285284

286285
endpoint, err := client.CreateEndpoint(req)
@@ -319,11 +318,16 @@ func buildTemplateEndpointGQLInput(req *api.EndpointCreateRequest, gpuTypeID, lo
319318
}
320319
}
321320

321+
// randomString returns an n-character lowercase-alphanumeric suffix.
322+
// The suffix is used only to disambiguate generated template names; it is not
323+
// a token, secret, or anything an attacker benefits from predicting. gosec
324+
// G404 flags math/rand/v2 generically, so the directive below is a deliberate
325+
// suppression rather than a missed crypto/rand call.
322326
func randomString(n int) string {
323327
const letters = "abcdefghijklmnopqrstuvwxyz0123456789"
324328
b := make([]byte, n)
325-
for i := range b {
326-
b[i] = letters[rand.Intn(len(letters))] //nolint:gosec
329+
for i := range n {
330+
b[i] = letters[rand.IntN(len(letters))] //nolint:gosec // non-crypto: template-name uniqueness suffix, not a token
327331
}
328332
return string(b)
329333
}

0 commit comments

Comments
 (0)