Skip to content

Commit c50cdbd

Browse files
malancasmalancas
authored
Merge pull request cli#10943 from malancas/gh-attestation-tuf-client-retry
Add retry logic when fetching TUF content in `gh attestation` commands
2 parents 2cdee00 + 23382f4 commit c50cdbd

13 files changed

Lines changed: 115 additions & 92 deletions

‎go.mod‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ require (
1010
github.com/Netflix/go-expect v0.0.0-20220104043353-73e0943537d2
1111
github.com/briandowns/spinner v1.18.1
1212
github.com/cenkalti/backoff/v4 v4.3.0
13+
github.com/cenkalti/backoff/v5 v5.0.2
1314
github.com/charmbracelet/glamour v0.9.2-0.20250319212134-549f544650e3
1415
github.com/charmbracelet/huh v0.7.0
1516
github.com/charmbracelet/lipgloss v1.1.1-0.20250319133953-166f707985bc
@@ -48,6 +49,7 @@ require (
4849
github.com/spf13/cobra v1.9.1
4950
github.com/spf13/pflag v1.0.6
5051
github.com/stretchr/testify v1.10.0
52+
github.com/theupdateframework/go-tuf/v2 v2.1.1
5153
github.com/yuin/goldmark v1.7.12
5254
github.com/zalando/go-keyring v0.2.5
5355
golang.org/x/crypto v0.38.0
@@ -73,7 +75,6 @@ require (
7375
github.com/aymerick/douceur v0.2.0 // indirect
7476
github.com/blang/semver v3.5.1+incompatible // indirect
7577
github.com/catppuccin/go v0.3.0 // indirect
76-
github.com/cenkalti/backoff/v5 v5.0.2 // indirect
7778
github.com/charmbracelet/bubbles v0.21.0 // indirect
7879
github.com/charmbracelet/bubbletea v1.3.4 // indirect
7980
github.com/charmbracelet/colorprofile v0.2.3-0.20250311203215-f60798e515dc // indirect
@@ -167,7 +168,6 @@ require (
167168
github.com/stretchr/objx v0.5.2 // indirect
168169
github.com/subosito/gotenv v1.6.0 // indirect
169170
github.com/theupdateframework/go-tuf v0.7.0 // indirect
170-
github.com/theupdateframework/go-tuf/v2 v2.1.1 // indirect
171171
github.com/thlib/go-timezone-local v0.0.0-20210907160436-ef149e42d28e // indirect
172172
github.com/titanous/rocacheck v0.0.0-20171023193734-afe73141d399 // indirect
173173
github.com/transparency-dev/merkle v0.0.2 // indirect

‎pkg/cmd/attestation/inspect/inspect.go‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,13 +77,18 @@ func NewInspectCmd(f *cmdutil.Factory, runF func(*Options) error) *cobra.Command
7777
opts.Hostname, _ = ghauth.DefaultHost()
7878
}
7979

80-
err := auth.IsHostSupported(opts.Hostname)
80+
if err := auth.IsHostSupported(opts.Hostname); err != nil {
81+
return err
82+
}
83+
84+
hc, err := f.HttpClient()
8185
if err != nil {
8286
return err
8387
}
8488

8589
config := verification.SigstoreConfig{
86-
Logger: opts.Logger,
90+
HttpClient: hc,
91+
Logger: opts.Logger,
8792
}
8893

8994
if ghauth.IsTenancy(opts.Hostname) {

‎pkg/cmd/attestation/inspect/inspect_integration_test.go‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
//go:build integration
2+
3+
package inspect
4+
5+
import (
6+
"bytes"
7+
"fmt"
8+
"net/http"
9+
"strings"
10+
"testing"
11+
12+
"github.com/cli/cli/v2/pkg/cmdutil"
13+
"github.com/cli/cli/v2/pkg/iostreams"
14+
"github.com/stretchr/testify/assert"
15+
)
16+
17+
func TestNewInspectCmd_PrintOutputJSONFormat(t *testing.T) {
18+
testIO, _, _, _ := iostreams.Test()
19+
f := &cmdutil.Factory{
20+
IOStreams: testIO,
21+
HttpClient: func() (*http.Client, error) {
22+
return http.DefaultClient, nil
23+
},
24+
}
25+
26+
t.Run("Print output in JSON format", func(t *testing.T) {
27+
var opts *Options
28+
cmd := NewInspectCmd(f, func(o *Options) error {
29+
opts = o
30+
return nil
31+
})
32+
33+
argv := strings.Split(fmt.Sprintf("%s --format json", bundlePath), " ")
34+
cmd.SetArgs(argv)
35+
cmd.SetIn(&bytes.Buffer{})
36+
cmd.SetOut(&bytes.Buffer{})
37+
cmd.SetErr(&bytes.Buffer{})
38+
_, err := cmd.ExecuteC()
39+
assert.NoError(t, err)
40+
41+
assert.Equal(t, bundlePath, opts.BundlePath)
42+
assert.NotNil(t, opts.Logger)
43+
assert.NotNil(t, opts.exporter)
44+
})
45+
}

‎pkg/cmd/attestation/inspect/inspect_test.go‎

Lines changed: 1 addition & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,7 @@
11
package inspect
22

33
import (
4-
"bytes"
54
"encoding/json"
6-
"fmt"
7-
"net/http"
8-
"strings"
95
"testing"
106

117
"github.com/cli/cli/v2/pkg/cmd/attestation/artifact/oci"
@@ -14,7 +10,6 @@ import (
1410
"github.com/cli/cli/v2/pkg/cmd/attestation/verification"
1511
"github.com/cli/cli/v2/pkg/cmdutil"
1612

17-
"github.com/cli/cli/v2/pkg/httpmock"
1813
"github.com/cli/cli/v2/pkg/iostreams"
1914

2015
"github.com/stretchr/testify/assert"
@@ -26,66 +21,7 @@ const (
2621
SigstoreSanRegex = "^https://github.com/sigstore/sigstore-js/"
2722
)
2823

29-
var (
30-
bundlePath = test.NormalizeRelativePath("../test/data/sigstore-js-2.1.0-bundle.json")
31-
)
32-
33-
func TestNewInspectCmd(t *testing.T) {
34-
testIO, _, _, _ := iostreams.Test()
35-
f := &cmdutil.Factory{
36-
IOStreams: testIO,
37-
HttpClient: func() (*http.Client, error) {
38-
reg := &httpmock.Registry{}
39-
client := &http.Client{}
40-
httpmock.ReplaceTripper(client, reg)
41-
return client, nil
42-
},
43-
}
44-
45-
testcases := []struct {
46-
name string
47-
cli string
48-
wants Options
49-
wantsErr bool
50-
wantsExporter bool
51-
}{
52-
{
53-
name: "Prints output in JSON format",
54-
cli: fmt.Sprintf("%s --format json", bundlePath),
55-
wants: Options{
56-
BundlePath: bundlePath,
57-
SigstoreVerifier: verification.NewMockSigstoreVerifier(t),
58-
},
59-
wantsExporter: true,
60-
},
61-
}
62-
63-
for _, tc := range testcases {
64-
t.Run(tc.name, func(t *testing.T) {
65-
var opts *Options
66-
cmd := NewInspectCmd(f, func(o *Options) error {
67-
opts = o
68-
return nil
69-
})
70-
71-
argv := strings.Split(tc.cli, " ")
72-
cmd.SetArgs(argv)
73-
cmd.SetIn(&bytes.Buffer{})
74-
cmd.SetOut(&bytes.Buffer{})
75-
cmd.SetErr(&bytes.Buffer{})
76-
_, err := cmd.ExecuteC()
77-
if tc.wantsErr {
78-
assert.Error(t, err)
79-
return
80-
}
81-
assert.NoError(t, err)
82-
83-
assert.Equal(t, tc.wants.BundlePath, opts.BundlePath)
84-
assert.NotNil(t, opts.Logger)
85-
assert.Equal(t, tc.wantsExporter, opts.exporter != nil)
86-
})
87-
}
88-
}
24+
var bundlePath = test.NormalizeRelativePath("../test/data/sigstore-js-2.1.0-bundle.json")
8925

9026
func TestRunInspect(t *testing.T) {
9127
opts := Options{

‎pkg/cmd/attestation/trustedroot/trustedroot.go‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"bytes"
55
"encoding/json"
66
"fmt"
7+
"net/http"
78
"os"
89

910
"github.com/cli/cli/v2/pkg/cmd/attestation/api"
@@ -68,6 +69,10 @@ func NewTrustedRootCmd(f *cmdutil.Factory, runF func(*Options) error) *cobra.Com
6869
return err
6970
}
7071

72+
hc, err := f.HttpClient()
73+
if err != nil {
74+
return err
75+
}
7176
if ghauth.IsTenancy(opts.Hostname) {
7277
c, err := f.Config()
7378
if err != nil {
@@ -77,11 +82,6 @@ func NewTrustedRootCmd(f *cmdutil.Factory, runF func(*Options) error) *cobra.Com
7782
if !c.Authentication().HasActiveToken(opts.Hostname) {
7883
return fmt.Errorf("not authenticated with %s", opts.Hostname)
7984
}
80-
81-
hc, err := f.HttpClient()
82-
if err != nil {
83-
return err
84-
}
8585
logger := io.NewHandler(f.IOStreams)
8686
apiClient := api.NewLiveClient(hc, opts.Hostname, logger)
8787
td, err := apiClient.GetTrustDomain()
@@ -95,7 +95,7 @@ func NewTrustedRootCmd(f *cmdutil.Factory, runF func(*Options) error) *cobra.Com
9595
return runF(opts)
9696
}
9797

98-
if err := getTrustedRoot(tuf.New, opts); err != nil {
98+
if err := getTrustedRoot(tuf.New, opts, hc); err != nil {
9999
return fmt.Errorf("Failed to verify the TUF repository: %w", err)
100100
}
101101

@@ -118,11 +118,11 @@ type tufConfig struct {
118118
targets []string
119119
}
120120

121-
func getTrustedRoot(makeTUF tufClientInstantiator, opts *Options) error {
121+
func getTrustedRoot(makeTUF tufClientInstantiator, opts *Options, hc *http.Client) error {
122122
var tufOptions []tufConfig
123123
var defaultTR = "trusted_root.json"
124124

125-
tufOpt := verification.DefaultOptionsWithCacheSetting(o.None[string]())
125+
tufOpt := verification.DefaultOptionsWithCacheSetting(o.None[string](), hc)
126126
// Disable local caching, so we get up-to-date response from TUF repository
127127
tufOpt.CacheValidity = 0
128128

@@ -151,7 +151,7 @@ func getTrustedRoot(makeTUF tufClientInstantiator, opts *Options) error {
151151
targets: []string{defaultTR},
152152
})
153153

154-
tufOpt = verification.GitHubTUFOptions(o.None[string]())
154+
tufOpt = verification.GitHubTUFOptions(o.None[string](), hc)
155155
tufOpt.CacheValidity = 0
156156
tufOptions = append(tufOptions, tufConfig{
157157
tufOptions: tufOpt,

‎pkg/cmd/attestation/trustedroot/trustedroot_test.go‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,12 @@ func TestNewTrustedRootCmd(t *testing.T) {
2828
Config: func() (gh.Config, error) {
2929
return &ghmock.ConfigMock{}, nil
3030
},
31+
HttpClient: func() (*http.Client, error) {
32+
reg := &httpmock.Registry{}
33+
client := &http.Client{}
34+
httpmock.ReplaceTripper(client, reg)
35+
return client, nil
36+
},
3137
}
3238

3339
testcases := []struct {
@@ -113,6 +119,7 @@ func TestNewTrustedRootWithTenancy(t *testing.T) {
113119
},
114120
}, nil
115121
},
122+
HttpClient: httpClientFunc,
116123
}
117124

118125
cmd := NewTrustedRootCmd(f, func(_ *Options) error {
@@ -171,15 +178,19 @@ func TestGetTrustedRoot(t *testing.T) {
171178
TufRootPath: root,
172179
}
173180

181+
reg := &httpmock.Registry{}
182+
client := &http.Client{}
183+
httpmock.ReplaceTripper(client, reg)
184+
174185
t.Run("failed to create TUF root", func(t *testing.T) {
175-
err := getTrustedRoot(newTUFErrClient, opts)
186+
err := getTrustedRoot(newTUFErrClient, opts, client)
176187
require.Error(t, err)
177188
require.ErrorContains(t, err, "failed to create TUF client")
178189
})
179190

180191
t.Run("fails because the root cannot be found", func(t *testing.T) {
181192
opts.TufRootPath = test.NormalizeRelativePath("./does/not/exist/root.json")
182-
err := getTrustedRoot(tuf.New, opts)
193+
err := getTrustedRoot(tuf.New, opts, client)
183194
require.Error(t, err)
184195
require.ErrorContains(t, err, "failed to read root file")
185196
})

‎pkg/cmd/attestation/verification/sigstore.go‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"crypto/x509"
77
"errors"
88
"fmt"
9+
"net/http"
910
"os"
1011

1112
"github.com/cli/cli/v2/pkg/cmd/attestation/api"
@@ -33,6 +34,7 @@ type SigstoreConfig struct {
3334
TrustedRoot string
3435
Logger *io.Handler
3536
NoPublicGood bool
37+
HttpClient *http.Client
3638
// If tenancy mode is not used, trust domain is empty
3739
TrustDomain string
3840
// TUFMetadataDir
@@ -71,13 +73,13 @@ func NewLiveSigstoreVerifier(config SigstoreConfig) (*LiveSigstoreVerifier, erro
7173
return liveVerifier, nil
7274
}
7375
if !config.NoPublicGood {
74-
publicGoodVerifier, err := newPublicGoodVerifier(config.TUFMetadataDir)
76+
publicGoodVerifier, err := newPublicGoodVerifier(config.TUFMetadataDir, config.HttpClient)
7577
if err != nil {
7678
return nil, err
7779
}
7880
liveVerifier.PublicGood = publicGoodVerifier
7981
}
80-
github, err := newGitHubVerifier(config.TrustDomain, config.TUFMetadataDir)
82+
github, err := newGitHubVerifier(config.TrustDomain, config.TUFMetadataDir, config.HttpClient)
8183
if err != nil {
8284
return nil, err
8385
}
@@ -314,10 +316,10 @@ func newCustomVerifier(trustedRoot *root.TrustedRoot) (*verify.Verifier, error)
314316
return gv, nil
315317
}
316318

317-
func newGitHubVerifier(trustDomain string, tufMetadataDir o.Option[string]) (*verify.Verifier, error) {
319+
func newGitHubVerifier(trustDomain string, tufMetadataDir o.Option[string], hc *http.Client) (*verify.Verifier, error) {
318320
var tr string
319321

320-
opts := GitHubTUFOptions(tufMetadataDir)
322+
opts := GitHubTUFOptions(tufMetadataDir, hc)
321323
client, err := tuf.New(opts)
322324
if err != nil {
323325
return nil, fmt.Errorf("failed to create TUF client: %v", err)
@@ -348,8 +350,8 @@ func newGitHubVerifierWithTrustedRoot(trustedRoot *root.TrustedRoot) (*verify.Ve
348350
return gv, nil
349351
}
350352

351-
func newPublicGoodVerifier(tufMetadataDir o.Option[string]) (*verify.Verifier, error) {
352-
opts := DefaultOptionsWithCacheSetting(tufMetadataDir)
353+
func newPublicGoodVerifier(tufMetadataDir o.Option[string], hc *http.Client) (*verify.Verifier, error) {
354+
opts := DefaultOptionsWithCacheSetting(tufMetadataDir, hc)
353355
client, err := tuf.New(opts)
354356
if err != nil {
355357
return nil, fmt.Errorf("failed to create TUF client: %v", err)

‎pkg/cmd/attestation/verification/sigstore_integration_test.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
package verification
44

55
import (
6+
"net/http"
67
"testing"
78

89
"github.com/cli/cli/v2/pkg/cmd/attestation/api"
@@ -51,6 +52,7 @@ func TestLiveSigstoreVerifier(t *testing.T) {
5152
for _, tc := range testcases {
5253
t.Run(tc.name, func(t *testing.T) {
5354
verifier, err := NewLiveSigstoreVerifier(SigstoreConfig{
55+
HttpClient: http.DefaultClient,
5456
Logger: io.NewTestHandler(),
5557
TUFMetadataDir: o.Some(t.TempDir()),
5658
})
@@ -71,6 +73,7 @@ func TestLiveSigstoreVerifier(t *testing.T) {
7173

7274
t.Run("with 2/3 verified attestations", func(t *testing.T) {
7375
verifier, err := NewLiveSigstoreVerifier(SigstoreConfig{
76+
HttpClient: http.DefaultClient,
7477
Logger: io.NewTestHandler(),
7578
TUFMetadataDir: o.Some(t.TempDir()),
7679
})
@@ -89,6 +92,7 @@ func TestLiveSigstoreVerifier(t *testing.T) {
8992

9093
t.Run("fail with 0/2 verified attestations", func(t *testing.T) {
9194
verifier, err := NewLiveSigstoreVerifier(SigstoreConfig{
95+
HttpClient: http.DefaultClient,
9296
Logger: io.NewTestHandler(),
9397
TUFMetadataDir: o.Some(t.TempDir()),
9498
})
@@ -114,6 +118,7 @@ func TestLiveSigstoreVerifier(t *testing.T) {
114118
attestations := getAttestationsFor(t, "../test/data/github_provenance_demo-0.0.12-py3-none-any-bundle.jsonl")
115119

116120
verifier, err := NewLiveSigstoreVerifier(SigstoreConfig{
121+
HttpClient: http.DefaultClient,
117122
Logger: io.NewTestHandler(),
118123
TUFMetadataDir: o.Some(t.TempDir()),
119124
})
@@ -128,6 +133,7 @@ func TestLiveSigstoreVerifier(t *testing.T) {
128133
attestations := getAttestationsFor(t, "../test/data/sigstore-js-2.1.0_with_2_bundles.jsonl")
129134

130135
verifier, err := NewLiveSigstoreVerifier(SigstoreConfig{
136+
HttpClient: http.DefaultClient,
131137
Logger: io.NewTestHandler(),
132138
TrustedRoot: test.NormalizeRelativePath("../test/data/trusted_root.json"),
133139
TUFMetadataDir: o.Some(t.TempDir()),

0 commit comments

Comments
 (0)