Skip to content

Commit e65de76

Browse files
zregvartzregvart
authored
Merge pull request #1601 from zregvart/pr/cleanup-oci-client
Refactor everything relating to OCI client
2 parents c9f2df5 + 0c8f7b3 commit e65de76

23 files changed

Lines changed: 527 additions & 818 deletions

File tree

‎cmd/validate/common_test.go‎

Lines changed: 4 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -21,43 +21,19 @@ package validate
2121
import (
2222
"context"
2323

24-
"github.com/google/go-containerregistry/pkg/name"
2524
v1 "github.com/google/go-containerregistry/pkg/v1"
26-
"github.com/google/go-containerregistry/pkg/v1/remote"
25+
"github.com/google/go-containerregistry/pkg/v1/types"
2726
"github.com/spf13/cobra"
2827
"github.com/stretchr/testify/mock"
2928

3029
"github.com/enterprise-contract/ec-cli/cmd/root"
3130
"github.com/enterprise-contract/ec-cli/internal/evaluator"
31+
"github.com/enterprise-contract/ec-cli/internal/utils/oci/fake"
3232
)
3333

34-
type MockRemoteClient struct {
35-
mock.Mock
36-
}
37-
38-
func (m *MockRemoteClient) Get(ref name.Reference) (*remote.Descriptor, error) {
39-
args := m.Called(ref)
40-
result := args.Get(0)
41-
if result != nil {
42-
return result.(*remote.Descriptor), args.Error(1)
43-
}
44-
return nil, args.Error(1)
45-
}
46-
47-
func (m *MockRemoteClient) Index(ref name.Reference) (v1.ImageIndex, error) {
48-
args := m.Called(ref)
49-
result := args.Get(0)
50-
if result != nil {
51-
return args.Get(0).(v1.ImageIndex), args.Error(1)
52-
}
53-
return nil, args.Error(1)
54-
}
55-
56-
func commonMockClient(mockClient *MockRemoteClient) {
57-
imageManifestJson := `{"mediaType": "application/vnd.oci.image.manifest.v1+json"}`
58-
imageManifestJsonBytes := []byte(imageManifestJson)
34+
func commonMockClient(client *fake.FakeClient) {
5935
// TODO: Replace mock.Anything calls with specific values
60-
mockClient.On("Get", mock.Anything).Return(&remote.Descriptor{Manifest: imageManifestJsonBytes}, nil)
36+
client.On("Head", mock.Anything).Return(&v1.Descriptor{MediaType: types.OCIManifestSchema1}, nil)
6137
}
6238

6339
type mockEvaluator struct {

‎cmd/validate/image_integration_test.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,19 +34,20 @@ import (
3434
"github.com/stretchr/testify/mock"
3535
"github.com/stretchr/testify/require"
3636

37-
"github.com/enterprise-contract/ec-cli/internal/applicationsnapshot"
3837
"github.com/enterprise-contract/ec-cli/internal/evaluator"
3938
"github.com/enterprise-contract/ec-cli/internal/output"
4039
"github.com/enterprise-contract/ec-cli/internal/policy"
4140
"github.com/enterprise-contract/ec-cli/internal/policy/source"
4241
"github.com/enterprise-contract/ec-cli/internal/utils"
42+
"github.com/enterprise-contract/ec-cli/internal/utils/oci"
43+
"github.com/enterprise-contract/ec-cli/internal/utils/oci/fake"
4344
)
4445

4546
func TestEvaluatorLifecycle(t *testing.T) {
4647
ctx := utils.WithFS(context.Background(), afero.NewMemMapFs())
47-
mockRemoteClient := &MockRemoteClient{}
48-
commonMockClient(mockRemoteClient)
49-
ctx = context.WithValue(ctx, applicationsnapshot.RemoteClientKey{}, mockRemoteClient)
48+
client := fake.FakeClient{}
49+
commonMockClient(&client)
50+
ctx = oci.WithClient(ctx, &client)
5051

5152
noEvaluators := 100
5253

‎cmd/validate/image_test.go‎

Lines changed: 56 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ import (
3939
"github.com/enterprise-contract/ec-cli/internal/output"
4040
"github.com/enterprise-contract/ec-cli/internal/policy"
4141
"github.com/enterprise-contract/ec-cli/internal/utils"
42+
"github.com/enterprise-contract/ec-cli/internal/utils/oci"
43+
"github.com/enterprise-contract/ec-cli/internal/utils/oci/fake"
4244
)
4345

4446
type data struct {
@@ -229,9 +231,9 @@ func Test_determineInputSpec(t *testing.T) {
229231
},
230232
},
231233
}
232-
mockRemoteClient := &MockRemoteClient{}
233-
commonMockClient(mockRemoteClient)
234-
ctx := context.WithValue(context.Background(), applicationsnapshot.RemoteClientKey{}, mockRemoteClient)
234+
client := fake.FakeClient{}
235+
commonMockClient(&client)
236+
ctx := oci.WithClient(context.Background(), &client)
235237
for _, c := range cases {
236238
t.Run(c.name, func(t *testing.T) {
237239
s, err := applicationsnapshot.DetermineInputSpec(ctx, applicationsnapshot.Input{
@@ -287,10 +289,10 @@ func Test_ValidateImageCommand(t *testing.T) {
287289
validateImageCmd := validateImageCmd(validate)
288290
cmd := setUpCobra(validateImageCmd)
289291

290-
mockRemoteClient := &MockRemoteClient{}
291-
commonMockClient(mockRemoteClient)
292-
ctx := utils.WithFS(context.TODO(), afero.NewMemMapFs())
293-
ctx = context.WithValue(ctx, applicationsnapshot.RemoteClientKey{}, mockRemoteClient)
292+
client := fake.FakeClient{}
293+
commonMockClient(&client)
294+
ctx := utils.WithFS(context.Background(), afero.NewMemMapFs())
295+
ctx = oci.WithClient(ctx, &client)
294296
cmd.SetContext(ctx)
295297

296298
effectiveTimeTest := time.Now().UTC().Format(time.RFC3339Nano)
@@ -367,10 +369,10 @@ func Test_ValidateImageCommandImages(t *testing.T) {
367369
validateImageCmd := validateImageCmd(validate)
368370
cmd := setUpCobra(validateImageCmd)
369371

370-
mockRemoteClient := &MockRemoteClient{}
371-
commonMockClient(mockRemoteClient)
372-
ctx := utils.WithFS(context.TODO(), afero.NewMemMapFs())
373-
ctx = context.WithValue(ctx, applicationsnapshot.RemoteClientKey{}, mockRemoteClient)
372+
client := fake.FakeClient{}
373+
commonMockClient(&client)
374+
ctx := utils.WithFS(context.Background(), afero.NewMemMapFs())
375+
ctx = oci.WithClient(ctx, &client)
374376
cmd.SetContext(ctx)
375377

376378
effectiveTimeTest := time.Now().UTC().Format(time.RFC3339Nano)
@@ -467,10 +469,10 @@ func Test_ValidateImageCommandKeyless(t *testing.T) {
467469
})
468470
cmd := setUpCobra(validateImageCmd)
469471

470-
mockRemoteClient := &MockRemoteClient{}
471-
commonMockClient(mockRemoteClient)
472-
ctx := utils.WithFS(context.TODO(), afero.NewMemMapFs())
473-
ctx = context.WithValue(ctx, applicationsnapshot.RemoteClientKey{}, mockRemoteClient)
472+
client := fake.FakeClient{}
473+
commonMockClient(&client)
474+
ctx := utils.WithFS(context.Background(), afero.NewMemMapFs())
475+
ctx = oci.WithClient(ctx, &client)
474476
cmd.SetContext(ctx)
475477

476478
cmd.SetArgs(append(rootArgs, []string{
@@ -533,11 +535,11 @@ func Test_ValidateImageCommandYAMLPolicyFile(t *testing.T) {
533535
validateImageCmd := validateImageCmd(validate)
534536
cmd := setUpCobra(validateImageCmd)
535537

536-
mockRemoteClient := &MockRemoteClient{}
537-
commonMockClient(mockRemoteClient)
538+
client := fake.FakeClient{}
539+
commonMockClient(&client)
538540
fs := afero.NewMemMapFs()
539-
ctx := utils.WithFS(context.TODO(), fs)
540-
ctx = context.WithValue(ctx, applicationsnapshot.RemoteClientKey{}, mockRemoteClient)
541+
ctx := utils.WithFS(context.Background(), fs)
542+
ctx = oci.WithClient(ctx, &client)
541543
cmd.SetContext(ctx)
542544

543545
cases := []struct {
@@ -650,11 +652,11 @@ func Test_ValidateImageCommandJSONPolicyFile(t *testing.T) {
650652
validateImageCmd := validateImageCmd(validate)
651653
cmd := setUpCobra(validateImageCmd)
652654

653-
mockRemoteClient := &MockRemoteClient{}
654-
commonMockClient(mockRemoteClient)
655+
client := fake.FakeClient{}
656+
commonMockClient(&client)
655657
fs := afero.NewMemMapFs()
656-
ctx := utils.WithFS(context.TODO(), fs)
657-
ctx = context.WithValue(ctx, applicationsnapshot.RemoteClientKey{}, mockRemoteClient)
658+
ctx := utils.WithFS(context.Background(), fs)
659+
ctx = oci.WithClient(ctx, &client)
658660
cmd.SetContext(ctx)
659661

660662
testPolicyJSON := `sources:
@@ -731,10 +733,10 @@ func Test_ValidateImageCommandExtraData(t *testing.T) {
731733

732734
fs := afero.NewMemMapFs()
733735

734-
ctx := utils.WithFS(context.TODO(), fs)
735-
mockRemoteClient := &MockRemoteClient{}
736-
commonMockClient(mockRemoteClient)
737-
ctx = context.WithValue(ctx, applicationsnapshot.RemoteClientKey{}, mockRemoteClient)
736+
ctx := utils.WithFS(context.Background(), fs)
737+
client := fake.FakeClient{}
738+
commonMockClient(&client)
739+
ctx = oci.WithClient(ctx, &client)
738740

739741
cmd.SetContext(ctx)
740742

@@ -832,11 +834,11 @@ func Test_ValidateImageCommandEmptyPolicyFile(t *testing.T) {
832834
validateImageCmd := validateImageCmd(validate)
833835
cmd := setUpCobra(validateImageCmd)
834836

835-
mockRemoteClient := &MockRemoteClient{}
836-
commonMockClient(mockRemoteClient)
837+
client := fake.FakeClient{}
838+
commonMockClient(&client)
837839
fs := afero.NewMemMapFs()
838-
ctx := utils.WithFS(context.TODO(), fs)
839-
ctx = context.WithValue(ctx, applicationsnapshot.RemoteClientKey{}, mockRemoteClient)
840+
ctx := utils.WithFS(context.Background(), fs)
841+
ctx = oci.WithClient(ctx, &client)
840842
cmd.SetContext(ctx)
841843

842844
err := afero.WriteFile(fs, "/policy.yaml", []byte(nil), 0644)
@@ -930,10 +932,10 @@ func Test_ValidateErrorCommand(t *testing.T) {
930932
validateImageCmd := validateImageCmd(validate)
931933
cmd := setUpCobra(validateImageCmd)
932934

933-
mockRemoteClient := &MockRemoteClient{}
934-
commonMockClient(mockRemoteClient)
935-
ctx := utils.WithFS(context.TODO(), afero.NewMemMapFs())
936-
ctx = context.WithValue(ctx, applicationsnapshot.RemoteClientKey{}, mockRemoteClient)
935+
client := fake.FakeClient{}
936+
commonMockClient(&client)
937+
ctx := utils.WithFS(context.Background(), afero.NewMemMapFs())
938+
ctx = oci.WithClient(ctx, &client)
937939
cmd.SetContext(ctx)
938940

939941
cmd.SetArgs(append([]string{"validate", "image"}, c.args...))
@@ -975,10 +977,10 @@ func Test_FailureImageAccessibility(t *testing.T) {
975977
cmd := setUpCobra(validateImageCmd)
976978
cmd.SilenceUsage = true // The root command is set to prevent usage printouts when running the CLI directly. This setup is temporary workaround.
977979

978-
mockRemoteClient := &MockRemoteClient{}
979-
commonMockClient(mockRemoteClient)
980-
ctx := utils.WithFS(context.TODO(), afero.NewMemMapFs())
981-
ctx = context.WithValue(ctx, applicationsnapshot.RemoteClientKey{}, mockRemoteClient)
980+
client := fake.FakeClient{}
981+
commonMockClient(&client)
982+
ctx := utils.WithFS(context.Background(), afero.NewMemMapFs())
983+
ctx = oci.WithClient(ctx, &client)
982984
cmd.SetContext(ctx)
983985

984986
effectiveTimeTest := time.Now().UTC().Format(time.RFC3339Nano)
@@ -1045,10 +1047,10 @@ func Test_FailureOutput(t *testing.T) {
10451047
cmd := setUpCobra(validateImageCmd)
10461048
cmd.SilenceUsage = true // The root command is set to prevent usage printouts when running the CLI directly. This setup is temporary workaround.
10471049

1048-
mockRemoteClient := &MockRemoteClient{}
1049-
commonMockClient(mockRemoteClient)
1050-
ctx := utils.WithFS(context.TODO(), afero.NewMemMapFs())
1051-
ctx = context.WithValue(ctx, applicationsnapshot.RemoteClientKey{}, mockRemoteClient)
1050+
client := fake.FakeClient{}
1051+
commonMockClient(&client)
1052+
ctx := utils.WithFS(context.Background(), afero.NewMemMapFs())
1053+
ctx = oci.WithClient(ctx, &client)
10521054
cmd.SetContext(ctx)
10531055

10541056
effectiveTimeTest := time.Now().UTC().Format(time.RFC3339Nano)
@@ -1119,10 +1121,10 @@ func Test_WarningOutput(t *testing.T) {
11191121
validateImageCmd := validateImageCmd(validate)
11201122
cmd := setUpCobra(validateImageCmd)
11211123

1122-
mockRemoteClient := &MockRemoteClient{}
1123-
commonMockClient(mockRemoteClient)
1124-
ctx := utils.WithFS(context.TODO(), afero.NewMemMapFs())
1125-
ctx = context.WithValue(ctx, applicationsnapshot.RemoteClientKey{}, mockRemoteClient)
1124+
client := fake.FakeClient{}
1125+
commonMockClient(&client)
1126+
ctx := utils.WithFS(context.Background(), afero.NewMemMapFs())
1127+
ctx = oci.WithClient(ctx, &client)
11261128
cmd.SetContext(ctx)
11271129

11281130
effectiveTimeTest := time.Now().UTC().Format(time.RFC3339Nano)
@@ -1184,10 +1186,10 @@ func Test_FailureImageAccessibilityNonStrict(t *testing.T) {
11841186
cmd := setUpCobra(validateImageCmd)
11851187
cmd.SilenceUsage = true // The root command is set to prevent usage printouts when running the CLI directly. This setup is temporary workaround.
11861188

1187-
mockRemoteClient := &MockRemoteClient{}
1188-
commonMockClient(mockRemoteClient)
1189-
ctx := utils.WithFS(context.TODO(), afero.NewMemMapFs())
1190-
ctx = context.WithValue(ctx, applicationsnapshot.RemoteClientKey{}, mockRemoteClient)
1189+
client := fake.FakeClient{}
1190+
commonMockClient(&client)
1191+
ctx := utils.WithFS(context.Background(), afero.NewMemMapFs())
1192+
ctx = oci.WithClient(ctx, &client)
11911193
cmd.SetContext(ctx)
11921194

11931195
effectiveTimeTest := time.Now().UTC().Format(time.RFC3339Nano)
@@ -1272,10 +1274,9 @@ func TestValidateImageCommand_RunE(t *testing.T) {
12721274
cmd := setUpCobra(validateImageCmd)
12731275

12741276
ctx := utils.WithFS(context.Background(), afero.NewMemMapFs())
1275-
mockRemoteClient := &MockRemoteClient{}
1276-
commonMockClient(mockRemoteClient)
1277-
ctx = context.WithValue(ctx, applicationsnapshot.RemoteClientKey{}, mockRemoteClient)
1278-
1277+
client := fake.FakeClient{}
1278+
commonMockClient(&client)
1279+
ctx = oci.WithClient(ctx, &client)
12791280
cmd.SetContext(ctx)
12801281

12811282
effectiveTimeTest := time.Now().UTC().Format(time.RFC3339Nano)

‎internal/applicationsnapshot/input.go‎

Lines changed: 3 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,8 @@ import (
2020
"context"
2121
"errors"
2222
"fmt"
23-
"time"
2423

25-
"github.com/google/go-containerregistry/pkg/authn"
2624
"github.com/google/go-containerregistry/pkg/name"
27-
v1 "github.com/google/go-containerregistry/pkg/v1"
28-
"github.com/google/go-containerregistry/pkg/v1/remote"
2925
"github.com/hashicorp/go-multierror"
3026
app "github.com/redhat-appstudio/application-api/api/v1alpha1"
3127
log "github.com/sirupsen/logrus"
@@ -35,6 +31,7 @@ import (
3531

3632
"github.com/enterprise-contract/ec-cli/internal/kubernetes"
3733
"github.com/enterprise-contract/ec-cli/internal/utils"
34+
"github.com/enterprise-contract/ec-cli/internal/utils/oci"
3835
)
3936

4037
const unnamed = "Unnamed"
@@ -51,17 +48,6 @@ type snapshot struct {
5148
app.SnapshotSpec
5249
}
5350

54-
type client interface {
55-
Get(name.Reference) (*remote.Descriptor, error)
56-
Index(name.Reference) (v1.ImageIndex, error)
57-
}
58-
59-
type remoteClient struct{}
60-
61-
var _ client = (*remoteClient)(nil)
62-
63-
type RemoteClientKey struct{}
64-
6551
func (s *snapshot) merge(snap app.SnapshotSpec) {
6652
if s.Application == "" {
6753
s.Application = snap.Application
@@ -195,45 +181,8 @@ func readSnapshotSource(input []byte) (app.SnapshotSpec, error) {
195181
return file, nil
196182
}
197183

198-
func createRemoteOptions(ctx context.Context) []remote.Option {
199-
backoff := remote.Backoff{
200-
Duration: 1.0 * time.Second,
201-
Factor: 2.0,
202-
Jitter: 0.1,
203-
Steps: 3,
204-
}
205-
206-
return []remote.Option{
207-
remote.WithTransport(remote.DefaultTransport),
208-
remote.WithContext(ctx),
209-
remote.WithAuthFromKeychain(authn.DefaultKeychain),
210-
remote.WithRetryBackoff(backoff),
211-
}
212-
}
213-
214-
func (*remoteClient) Get(ref name.Reference) (*remote.Descriptor, error) {
215-
// TODO: remoteClient.Get should take a context and the actual remote options so it
216-
// can simply pass them through to remote.Get.
217-
return remote.Get(ref, createRemoteOptions(context.TODO())...)
218-
}
219-
220-
func (*remoteClient) Index(ref name.Reference) (v1.ImageIndex, error) {
221-
// TODO: remoteClient.Index should take a context and the actual remote options so it
222-
// can simply pass them through to remote.Index.
223-
return remote.Index(ref, createRemoteOptions(context.TODO())...)
224-
}
225-
226-
var defaultClient = &remoteClient{}
227-
228-
func determineClient(ctx context.Context) client {
229-
if cli, ok := ctx.Value(RemoteClientKey{}).(client); ok {
230-
return cli
231-
}
232-
return defaultClient
233-
}
234-
235184
func expandImageIndex(ctx context.Context, snap *app.SnapshotSpec) {
236-
client := determineClient(ctx)
185+
client := oci.NewClient(ctx)
237186
// For an image index, remove the original component and replace it with an expanded component with all its image manifests
238187
var components []app.SnapshotComponent
239188
// Do not raise an error if the image is inaccessible, it will be handled as a violation when evaluated against the policy
@@ -248,7 +197,7 @@ func expandImageIndex(ctx context.Context, snap *app.SnapshotSpec) {
248197
continue
249198
}
250199

251-
desc, err := client.Get(ref)
200+
desc, err := client.Head(ref)
252201
if err != nil {
253202
allErrors = multierror.Append(allErrors, fmt.Errorf("unable to fetch descriptior for container image %s: %w", ref, err))
254203
continue

0 commit comments

Comments
 (0)