From 5a948847a338f68f90a8f09ed1f1c61c0cf9c06e Mon Sep 17 00:00:00 2001 From: Tanner Frisch Date: Tue, 18 Jul 2023 14:43:13 -0400 Subject: [PATCH 1/7] feat: add unit tests Signed-off-by: Dave Dykstra <2129743+DrDaveD@users.noreply.github.com> --- client/push.go | 9 +- client/push_test.go | 1008 ++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 963 insertions(+), 54 deletions(-) diff --git a/client/push.go b/client/push.go index d6104b5..89b25bd 100644 --- a/client/push.go +++ b/client/push.go @@ -1,4 +1,4 @@ -// Copyright (c) 2018-2020, Sylabs Inc. All rights reserved. +// Copyright (c) 2018-2023, Sylabs Inc. All rights reserved. // This software is licensed under a 3-clause BSD license. Please consult the // LICENSE.md file distributed with the sources of this project regarding your // rights to use or distribute this software. @@ -48,7 +48,7 @@ type defaultUploadCallback struct { r io.Reader } -func (c *defaultUploadCallback) InitUpload(s int64, r io.Reader) { +func (c *defaultUploadCallback) InitUpload(_ int64, r io.Reader) { c.r = r } @@ -278,7 +278,10 @@ func (c *Client) postFile(ctx context.Context, fileSize int64, imageID string, c c.Logger.Logf("postFile calling %s", postURL) // Make an upload request - req, _ := c.newRequest(ctx, http.MethodPost, postURL, "", callback.GetReader()) + req, err := c.newRequest(ctx, http.MethodPost, postURL, "", callback.GetReader()) + if err != nil { + return nil, err + } // Content length is required by the API req.ContentLength = fileSize res, err := c.HTTPClient.Do(req) diff --git a/client/push_test.go b/client/push_test.go index 7369c43..2a1ea50 100644 --- a/client/push_test.go +++ b/client/push_test.go @@ -1,4 +1,4 @@ -// Copyright (c) 2018-2022, Sylabs Inc. All rights reserved. +// Copyright (c) 2018-2023, Sylabs Inc. All rights reserved. // This software is licensed under a 3-clause BSD license. Please consult the // LICENSE.md file distributed with the sources of this project regarding your // rights to use or distribute this software. @@ -11,7 +11,10 @@ import ( "fmt" "net/http" "net/http/httptest" + "net/url" "os" + "runtime" + "strings" "testing" jsonresp "github.com/sylabs/json-resp" @@ -21,6 +24,7 @@ const ( testQuotaUsageBytes int64 = 64 * 1024 * 1024 testQuotaTotalBytes int64 = 1024 * 1024 * 1024 testContainerURL = "/library/entity/collection/container" + testPayload = "testtesttesttest" ) func Test_postFile(t *testing.T) { @@ -154,12 +158,12 @@ func (m *v2ImageUploadMockService) MockImageFileEndpoint(w http.ResponseWriter, m.initCalled = true } -func (m *v2ImageUploadMockService) MockS3PresignedURLPUTEndpoint(w http.ResponseWriter, r *http.Request) { +func (m *v2ImageUploadMockService) MockS3PresignedURLPUTEndpoint(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusOK) m.putCalled = true } -func (m *v2ImageUploadMockService) MockImageFileCompleteEndpoint(w http.ResponseWriter, r *http.Request) { +func (m *v2ImageUploadMockService) MockImageFileCompleteEndpoint(w http.ResponseWriter, _ *http.Request) { response := UploadImageComplete{ Quota: QuotaResponse{ QuotaTotalBytes: testQuotaTotalBytes, @@ -175,87 +179,989 @@ func (m *v2ImageUploadMockService) MockImageFileCompleteEndpoint(w http.Response m.completeCalled = true } -func Test_legacyPostFileV2(t *testing.T) { +func mockS3Server(t *testing.T, statusCode int) *httptest.Server { + t.Helper() + + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if statusCode != http.StatusOK { + w.WriteHeader(statusCode) + return + } + w.Header().Set("ETag", "thisisasampleetag") + })) +} + +func initClient(t *testing.T, url string) *Client { + t.Helper() + + c, err := NewClient(&Config{BaseURL: url, AuthToken: testToken, Logger: &stdLogger{}}) + if err != nil { + t.Fatalf("error initializing client: %v", err) + } + + return c +} + +func commonHandler(t *testing.T, code int, w http.ResponseWriter) { + t.Helper() + + if code != http.StatusOK { + if code != http.StatusInternalServerError { + w.WriteHeader(code) + return + } + _, err := w.Write([]byte("'?")) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + return + } +} + +func uploadImageHelperHandler(t *testing.T, code int, w http.ResponseWriter) { + t.Helper() + + if code != http.StatusOK { + w.WriteHeader(code) + return + } +} + +func Test_UploadImageBadPath(t *testing.T) { + s3Server := mockS3Server(t, http.StatusOK) + defer s3Server.Close() + tests := []struct { - name string - imageRef string - testFile string + name string + path string + expectError bool }{ + {"badPath", "\n", true}, + {"pathError", "library://entity/collection/container:te,st", true}, + {"test", "entity/collection/container", false}, + } + + for _, tt := range tests { + tt := tt + + t.Run(tt.name, func(t *testing.T) { + h := http.NewServeMux() + h.HandleFunc("/v1/imagefile/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + commonHandler(t, http.StatusOK, w) + })) + + h.HandleFunc("/v1/tags/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + resp := TagMap{"test": "testValue"} + + if err := json.NewEncoder(w).Encode(&resp); err != nil { + t.Fatalf("Error encoding JSON response: %v", err) + } + })) + + h.HandleFunc("/v1/entities/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + uploadImageHelperHandler(t, http.StatusOK, w) + resp := Entity{ID: "testID"} + + if err := json.NewEncoder(w).Encode(&resp); err != nil { + t.Fatalf("Error encoding JSON response: %v", err) + } + })) + + h.HandleFunc("/v1/collections/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + uploadImageHelperHandler(t, http.StatusOK, w) + resp := Collection{ + ID: "testID", + } + if err := json.NewEncoder(w).Encode(&resp); err != nil { + t.Fatalf("Error encoding JSON response: %v", err) + } + })) + + h.HandleFunc("/v1/containers/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + uploadImageHelperHandler(t, http.StatusOK, w) + + resp := Container{ID: "test"} + + if err := json.NewEncoder(w).Encode(&resp); err != nil { + t.Fatalf("Error encoding JSON response: %v", err) + } + })) + + h.HandleFunc("/v1/images/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + uploadImageHelperHandler(t, http.StatusOK, w) + + commonHandler(t, http.StatusOK, w) + + resp := Image{ID: "testID"} + + if err := json.NewEncoder(w).Encode(&resp); err != nil { + t.Fatalf("Error encoding JSON response: %v", err) + } + })) + + libraryServer := httptest.NewServer(h) + defer libraryServer.Close() + + c := initClient(t, libraryServer.URL) + + r := strings.NewReader(testPayload) + + etag, err := c.UploadImage( + context.Background(), + r, + tt.path, + runtime.GOARCH, + []string{"tag"}, + "test", + &defaultUploadCallback{r: r}, + ) + if (err == nil) && tt.expectError { + t.Fatal("unexpected success") + } + + _ = etag + }) + } +} + +type testCodesStruct struct { + entity int + collection int + container int + image int +} + +func Test_UploadImage(t *testing.T) { + tests := []struct { + name string + statusCode int + s3StatusCode int + expectError bool + codes testCodesStruct + }{ + { + "statusOK", http.StatusOK, http.StatusOK, false, + testCodesStruct{http.StatusOK, http.StatusOK, http.StatusOK, http.StatusOK}, + }, + { + "badRequest", http.StatusBadRequest, http.StatusBadRequest, true, + testCodesStruct{http.StatusOK, http.StatusOK, http.StatusOK, http.StatusOK}, + }, + { + "internalServerError", http.StatusInternalServerError, http.StatusInternalServerError, true, + testCodesStruct{http.StatusOK, http.StatusOK, http.StatusOK, http.StatusOK}, + }, + { + "entityNotFound", http.StatusOK, http.StatusOK, true, + testCodesStruct{http.StatusNotFound, http.StatusOK, http.StatusOK, http.StatusOK}, + }, + { + "entityBadRequest", http.StatusOK, http.StatusOK, true, + testCodesStruct{http.StatusBadRequest, http.StatusOK, http.StatusOK, http.StatusOK}, + }, + { + "collectionNotFound", http.StatusOK, http.StatusOK, true, + testCodesStruct{http.StatusOK, http.StatusNotFound, http.StatusOK, http.StatusOK}, + }, + { + "collectionBadRequest", http.StatusOK, http.StatusOK, true, + testCodesStruct{http.StatusOK, http.StatusBadRequest, http.StatusOK, http.StatusOK}, + }, + { + "containerNotFound", http.StatusOK, http.StatusOK, true, + testCodesStruct{http.StatusOK, http.StatusOK, http.StatusNotFound, http.StatusOK}, + }, + { + "containerBadRequest", http.StatusOK, http.StatusOK, true, + testCodesStruct{http.StatusOK, http.StatusOK, http.StatusBadRequest, http.StatusOK}, + }, + { + "imageNotFound", http.StatusOK, http.StatusOK, true, + testCodesStruct{http.StatusOK, http.StatusOK, http.StatusOK, http.StatusNotFound}, + }, { - name: "Basic", - imageRef: "5cb9c34d7d960d82f5f5bc55", - testFile: "test_data/test_sha256", + "imageBadRequest", http.StatusOK, http.StatusOK, true, + testCodesStruct{http.StatusOK, http.StatusOK, http.StatusOK, http.StatusBadRequest}, }, } for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { - m := v2ImageUploadMockService{ - t: t, + s3Server := mockS3Server(t, tt.s3StatusCode) + defer s3Server.Close() + + h := http.NewServeMux() + h.HandleFunc("/v1/imagefile/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + commonHandler(t, tt.statusCode, w) + })) + + h.HandleFunc("/v1/tags/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + resp := TagMap{"test": "testValue"} + + if err := json.NewEncoder(w).Encode(&resp); err != nil { + t.Fatalf("Error encoding JSON response: %v", err) + } + })) + + h.HandleFunc("/v1/entities/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + uploadImageHelperHandler(t, tt.codes.entity, w) + resp := Entity{ID: "testID"} + + if err := json.NewEncoder(w).Encode(&resp); err != nil { + t.Fatalf("Error encoding JSON response: %v", err) + } + })) + + h.HandleFunc("/v1/collections/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + uploadImageHelperHandler(t, tt.codes.collection, w) + resp := Collection{ + ID: "testID", + } + if err := json.NewEncoder(w).Encode(&resp); err != nil { + t.Fatalf("Error encoding JSON response: %v", err) + } + })) + + h.HandleFunc("/v1/containers/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + uploadImageHelperHandler(t, tt.codes.container, w) + + resp := Container{ID: "test"} + + if err := json.NewEncoder(w).Encode(&resp); err != nil { + t.Fatalf("Error encoding JSON response: %v", err) + } + })) + + h.HandleFunc("/v1/images/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + uploadImageHelperHandler(t, tt.codes.image, w) + + commonHandler(t, tt.statusCode, w) + + resp := Image{ID: "testID"} + + if err := json.NewEncoder(w).Encode(&resp); err != nil { + t.Fatalf("Error encoding JSON response: %v", err) + } + })) + + libraryServer := httptest.NewServer(h) + defer libraryServer.Close() + + c := initClient(t, libraryServer.URL) + + r := strings.NewReader(testPayload) + + etag, err := c.UploadImage( + context.Background(), + r, + "entity/collection/container", + runtime.GOARCH, + []string{"tag"}, + "test", + &defaultUploadCallback{r: r}, + ) + if (err != nil) != tt.expectError { + t.Fatalf("unexpected error: %v", err) } - m.Run() - defer m.Stop() + _ = etag + }) + } +} - c, err := NewClient(&Config{AuthToken: testToken, BaseURL: m.baseURI}) - if err != nil { - t.Errorf("Error initializing client: %v", err) +func Test_postFileWrapper(t *testing.T) { + s3Server := mockS3Server(t, http.StatusOK) + defer s3Server.Close() + + h := http.NewServeMux() + h.HandleFunc("/v1/imagefile/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) + + libraryServer := httptest.NewServer(h) + defer libraryServer.Close() + + c := initClient(t, libraryServer.URL) + + r := strings.NewReader(testPayload) + + etag, err := c.postFileWrapper( + context.Background(), + r, + int64(len(testPayload)), + "xxx", + &defaultUploadCallback{r: r}, + map[string]string{"sha256sum": "xxx"}, + ) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + _ = etag +} + +func Test_postFileV2(t *testing.T) { + s3Server := mockS3Server(t, http.StatusOK) + defer s3Server.Close() + + tests := []struct { + name string + size int64 + expectError bool + }{ + {"basic", int64(len(testPayload)), false}, + {"invalid", minimumPartSize + 1, true}, + } + + for _, tt := range tests { + tt := tt + + t.Run(tt.name, func(t *testing.T) { + h := http.NewServeMux() + h.HandleFunc("/v2/imagefile/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case strings.HasSuffix(r.URL.String(), "/_multipart"): + switch r.Method { + case http.MethodPost: + resp := MultipartUploadStartResponse{ + Data: MultipartUpload{ + UploadID: "xxx", + TotalParts: 1, + PartSize: 123, + Options: map[string]string{OptionS3Compliant: "true"}, + }, + } + + if err := json.NewEncoder(w).Encode(&resp); err != nil { + t.Fatalf("Error encoding JSON response: %v", err) + } + case http.MethodPut: + resp := UploadImagePartResponse{Data: UploadImagePart{PresignedURL: s3Server.URL}} + + if err := json.NewEncoder(w).Encode(&resp); err != nil { + t.Fatalf("Error encoding JSON response: %v", err) + } + } + case strings.HasSuffix(r.URL.String(), "/_multipart_complete"): + resp := CompleteMultipartUploadResponse{Data: UploadImageComplete{}} + + if err := json.NewEncoder(w).Encode(&resp); err != nil { + t.Fatalf("Error encoding JSON response: %v", err) + } + default: + resp := UploadImageResponse{Data: UploadImage{UploadURL: s3Server.URL}} + + if err := json.NewEncoder(w).Encode(&resp); err != nil { + t.Fatalf("Error encoding JSON response: %v", err) + } + } + })) + + libraryServer := httptest.NewServer(h) + defer libraryServer.Close() + + c := initClient(t, libraryServer.URL) + + r := strings.NewReader(testPayload) + + etag, err := c.postFileV2( + context.Background(), + r, + tt.size, + "xxx", + &defaultUploadCallback{r: r}, + map[string]string{"sha256sum": "xxx"}, + ) + if (err != nil) != tt.expectError { + t.Fatalf("unexpected error: %v", err) } - f, err := os.Open(tt.testFile) - if err != nil { - t.Errorf("Error opening file %s for reading: %v", tt.testFile, err) + + _ = etag + }) + } +} + +func Test_postFileV2Multipart(t *testing.T) { + tests := []struct { + name string + statusCode int + s3StatusCode int + expectError bool + }{ + {"statusOK", http.StatusOK, http.StatusOK, false}, + {"internalServerError", http.StatusInternalServerError, http.StatusInternalServerError, true}, + {"okBadRequest", http.StatusOK, http.StatusBadRequest, true}, + {"badRequestOK", http.StatusBadRequest, http.StatusOK, true}, + {"internalOK", http.StatusInternalServerError, http.StatusOK, true}, + {"okInternal", http.StatusOK, http.StatusInternalServerError, true}, + {"internalBadRequest", http.StatusInternalServerError, http.StatusBadRequest, true}, + {"badRequestInternal", http.StatusBadRequest, http.StatusInternalServerError, true}, + } + + for _, tt := range tests { + tt := tt + + t.Run(tt.name, func(t *testing.T) { + s3Server := mockS3Server(t, tt.s3StatusCode) + defer s3Server.Close() + + h := http.NewServeMux() + h.HandleFunc("/v2/imagefile/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + commonHandler(t, tt.statusCode, w) + if strings.HasSuffix(r.URL.String(), "/_multipart") { + switch r.Method { + case http.MethodPost: + resp := MultipartUploadStartResponse{ + Data: MultipartUpload{ + UploadID: "xxx", + TotalParts: 1, + PartSize: 123456, + Options: map[string]string{OptionS3Compliant: "true"}, + }, + } + + if err := json.NewEncoder(w).Encode(&resp); err != nil { + t.Fatalf("Error encoding JSON response: %v", err) + } + case http.MethodPut: + resp := UploadImagePartResponse{Data: UploadImagePart{PresignedURL: s3Server.URL}} + + if err := json.NewEncoder(w).Encode(&resp); err != nil { + t.Fatalf("Error encoding JSON response: %v", err) + } + } + } else if strings.HasSuffix(r.URL.String(), "/_multipart_complete") { + resp := CompleteMultipartUploadResponse{Data: UploadImageComplete{}} + + if err := json.NewEncoder(w).Encode(&resp); err != nil { + t.Fatalf("Error encoding JSON response: %v", err) + } + } + })) + + libraryServer := httptest.NewServer(h) + defer libraryServer.Close() + + c := initClient(t, libraryServer.URL) + + r := strings.NewReader(testPayload) + + etag, err := c.postFileV2Multipart( + context.Background(), + r, + int64(len(testPayload)), + "xxx", + &defaultUploadCallback{r: r}, + ) + if (err != nil) != tt.expectError { + t.Fatalf("unexpected error: %v", err) } - defer f.Close() - fi, err := f.Stat() - if err != nil { - t.Errorf("Error stats for file %s: %v", tt.testFile, err) + _ = etag + }) + } +} + +func Test_getPartSize(t *testing.T) { + tests := []struct { + name string + bytesRemaining int64 + partSize int64 + want int64 + }{ + {"moreBytesThanParts", 2, 1, 1}, + {"morePartsThanBytes", 1, 2, 1}, + } + + for _, tt := range tests { + tt := tt + + t.Run(tt.name, func(t *testing.T) { + if got, want := getPartSize(tt.bytesRemaining, tt.partSize), tt.want; got != want { + t.Fatalf("got: %v, want: %v", got, want) } - fileSize := fi.Size() + }) + } +} - // calculate sha256 checksum - sha256checksum, _, err := sha256sum(f) - if err != nil { - t.Fatalf("error calculating sha256 checksum: %v", err) +func Test_startMultipartUpload(t *testing.T) { + s3Server := mockS3Server(t, http.StatusOK) + defer s3Server.Close() + + tests := []struct { + name string + statusCode int + expectError bool + }{ + {"success", http.StatusOK, false}, + {"badRequest", http.StatusBadRequest, true}, + {"internalServerError", http.StatusInternalServerError, true}, + } + + for _, tt := range tests { + tt := tt + + t.Run(tt.name, func(t *testing.T) { + h := http.NewServeMux() + h.HandleFunc("/v2/imagefile/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + commonHandler(t, tt.statusCode, w) + + if strings.HasSuffix(r.URL.String(), "/_multipart") { + resp := MultipartUploadStartResponse{ + Data: MultipartUpload{ + UploadID: "testUploadID", + TotalParts: 1, + PartSize: 123456, + Options: map[string]string{OptionS3Compliant: "true"}, + }, + } + + if err := json.NewEncoder(w).Encode(&resp); err != nil { + t.Fatalf("Error encoding JSON response: %v", err) + } + } + })) + + libraryServer := httptest.NewServer(h) + defer libraryServer.Close() + + c := initClient(t, libraryServer.URL) + + multi, err := c.startMultipartUpload(context.Background(), 0, "testID") + if (err != nil) != tt.expectError { + t.Fatalf("error uploading part: %v", err) } - _, err = f.Seek(0, 0) - if err != nil { - t.Fatalf("unexpected error seeking in sample data file: %v", err) + _ = multi + }) + } +} + +func Test_remoteSHA256ChecksumSupport(t *testing.T) { + tests := []struct { + name string + value string + want bool + }{ + {"basic", "x-amz-content-sha256", true}, + {"NotMatch", "x-amz-content-sha256x", false}, + {"NoValue", "", false}, + } + + for _, tt := range tests { + tt := tt + + t.Run(tt.name, func(t *testing.T) { + q := url.Values{} + + q.Set("X-Amz-SignedHeaders", tt.value) + + u := &url.URL{RawQuery: q.Encode()} + if got, want := remoteSHA256ChecksumSupport(u), tt.want; got != want { + t.Fatalf("got: %v, want: %v", got, want) } + }) + } +} - callback := &defaultUploadCallback{r: f} +// func Test_legacyPostFileV2(t *testing.T) { +// tests := []struct { +// name string +// imageRef string +// testFile string +// }{ +// { +// name: "Basic", +// imageRef: "5cb9c34d7d960d82f5f5bc55", +// testFile: "test_data/test_sha256", +// }, +// } - // include sha256 checksum in metadata - resp, err := c.legacyPostFileV2(context.Background(), fileSize, tt.imageRef, callback, map[string]string{ - "sha256sum": sha256checksum, - }) - if err != nil { +// for _, tt := range tests { +// tt := tt + +// t.Run(tt.name, func(t *testing.T) { +// m := v2ImageUploadMockService{ +// t: t, +// } + +// m.Run() +// defer m.Stop() + +// c := initClient(t, m.baseURI) +// f, err := os.Open(tt.testFile) +// if err != nil { +// t.Errorf("Error opening file %s for reading: %v", tt.testFile, err) +// } +// defer f.Close() + +// fi, err := f.Stat() +// if err != nil { +// t.Errorf("Error stats for file %s: %v", tt.testFile, err) +// } +// fileSize := fi.Size() + +// // calculate sha256 checksum +// sha256checksum, _, err := sha256sum(f) +// if err != nil { +// t.Fatalf("error calculating sha256 checksum: %v", err) +// } + +// _, err = f.Seek(0, 0) +// if err != nil { +// t.Fatalf("unexpected error seeking in sample data file: %v", err) +// } + +// callback := &defaultUploadCallback{r: f} + +// // include sha256 checksum in metadata +// resp, err := c.legacyPostFileV2(context.Background(), fileSize, tt.imageRef, callback, map[string]string{ +// "sha256sum": sha256checksum, +// }) +// if err != nil { +// t.Fatalf("unexpected error: %v", err) +// } + +// if got, want := resp.Quota.QuotaUsageBytes, testQuotaUsageBytes; got != want { +// t.Errorf("got quota usage %v, want %v", got, want) +// } + +// if got, want := resp.Quota.QuotaTotalBytes, testQuotaTotalBytes; got != want { +// t.Errorf("got quota total %v, want %v", got, want) +// } + +// if got, want := resp.ContainerURL, testContainerURL; got != want { +// t.Errorf("got container URL %v, want %v", got, want) +// } + +// if !m.initCalled { +// t.Errorf("init image upload request was not made") +// } + +// if !m.putCalled { +// t.Errorf("file PUT request was not made") +// } + +// if !m.completeCalled { +// t.Errorf("image upload complete request was not made") +// } +// }) +// } +// } + +func Test_legacyPostFileV2URL(t *testing.T) { + s3Server := mockS3Server(t, http.StatusOK) + defer s3Server.Close() + + tests := []struct { + name string + url string + expectError bool + }{ + {"basic", s3Server.URL, false}, + {"emptyURL", "", true}, + {"parseURLError", "\n", true}, + {"unsupported", "test", true}, + } + + for _, tt := range tests { + tt := tt + + t.Run(tt.name, func(t *testing.T) { + h := http.NewServeMux() + h.HandleFunc("/v2/imagefile/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + commonHandler(t, http.StatusOK, w) + + resp := UploadImageResponse{Data: UploadImage{UploadURL: tt.url}} + + if err := json.NewEncoder(w).Encode(&resp); err != nil { + t.Fatalf("Error encoding JSON response: %v", err) + } + })) + + libraryServer := httptest.NewServer(h) + defer libraryServer.Close() + + c := initClient(t, libraryServer.URL) + + r := strings.NewReader(testPayload) + + etag, err := c.legacyPostFileV2( + context.Background(), + 0, + "xxx", + &defaultUploadCallback{r: r}, + map[string]string{"sha256sum": "xxx"}, + ) + if (err != nil) != tt.expectError { t.Fatalf("unexpected error: %v", err) } - if got, want := resp.Quota.QuotaUsageBytes, testQuotaUsageBytes; got != want { - t.Errorf("got quota usage %v, want %v", got, want) - } + _ = etag + }) + } +} + +func Test_legacyPostFileV2(t *testing.T) { + tests := []struct { + name string + s3StatusCode int + statusCode int + expectError bool + }{ + {"basic", http.StatusOK, http.StatusOK, false}, + {"badRequest", http.StatusBadRequest, http.StatusBadRequest, true}, + {"internalServerError", http.StatusInternalServerError, http.StatusInternalServerError, true}, + {"okBadRequest", http.StatusOK, http.StatusBadRequest, true}, + {"badRequestOK", http.StatusBadRequest, http.StatusOK, true}, + {"internalOK", http.StatusInternalServerError, http.StatusOK, true}, + {"okInternal", http.StatusOK, http.StatusInternalServerError, true}, + {"internalBadRequest", http.StatusInternalServerError, http.StatusBadRequest, true}, + {"badRequestInternal", http.StatusBadRequest, http.StatusInternalServerError, true}, + } + + for _, tt := range tests { + tt := tt + + t.Run(tt.name, func(t *testing.T) { + s3Server := mockS3Server(t, tt.s3StatusCode) + defer s3Server.Close() + + h := http.NewServeMux() + h.HandleFunc("/v2/imagefile/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + commonHandler(t, tt.statusCode, w) + + resp := UploadImageResponse{Data: UploadImage{UploadURL: s3Server.URL}} - if got, want := resp.Quota.QuotaTotalBytes, testQuotaTotalBytes; got != want { - t.Errorf("got quota total %v, want %v", got, want) + if err := json.NewEncoder(w).Encode(&resp); err != nil { + t.Fatalf("Error encoding JSON response: %v", err) + } + })) + + libraryServer := httptest.NewServer(h) + defer libraryServer.Close() + + c := initClient(t, libraryServer.URL) + + r := strings.NewReader(testPayload) + + etag, err := c.legacyPostFileV2( + context.Background(), + 0, + "xxx", + &defaultUploadCallback{r: r}, + map[string]string{"sha256sum": "xxx"}, + ) + if (err != nil) != tt.expectError { + t.Fatalf("unexpected error: %v", err) } - if got, want := resp.ContainerURL, testContainerURL; got != want { - t.Errorf("got container URL %v, want %v", got, want) + _ = etag + }) + } +} + +func Test_Test_multipartUploadPartBadSize(t *testing.T) { + s3Server := mockS3Server(t, http.StatusOK) + defer s3Server.Close() + + h := http.NewServeMux() + h.HandleFunc("/v2/imagefile/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + commonHandler(t, http.StatusOK, w) + + if strings.HasSuffix(r.URL.String(), "/_multipart") { + resp := UploadImagePartResponse{Data: UploadImagePart{PresignedURL: s3Server.URL}} + + if err := json.NewEncoder(w).Encode(&resp); err != nil { + t.Fatalf("Error encoding JSON response: %v", err) } + } + })) + + libraryServer := httptest.NewServer(h) + defer libraryServer.Close() + + c := initClient(t, libraryServer.URL) + + r := strings.NewReader(testPayload) + + etag, err := c.multipartUploadPart( + context.Background(), + 0, + &uploadManager{ + Source: r, + Size: int64(len(testPayload)) + 1, + ImageID: "testImageID", + UploadID: "testUploadID", + }, + &defaultUploadCallback{r: r}, + true, + ) + if err == nil { + t.Fatal("unexpected success") + } + + _ = etag +} + +func Test_multipartUploadPart(t *testing.T) { + tests := []struct { + name string + statusCode int + s3StatusCode int + expectError bool + }{ + {"statusOK", http.StatusOK, http.StatusOK, false}, + {"badRequest", http.StatusBadRequest, http.StatusBadRequest, true}, + {"internalServerError", http.StatusInternalServerError, http.StatusInternalServerError, true}, + {"okBadRequest", http.StatusOK, http.StatusBadRequest, true}, + {"badRequestOK", http.StatusBadRequest, http.StatusOK, true}, + {"internalOK", http.StatusInternalServerError, http.StatusOK, true}, + {"okInternal", http.StatusOK, http.StatusInternalServerError, true}, + {"internalBadRequest", http.StatusInternalServerError, http.StatusBadRequest, true}, + {"badRequestInternal", http.StatusBadRequest, http.StatusInternalServerError, true}, + } + + for _, tt := range tests { + tt := tt + + t.Run(tt.name, func(t *testing.T) { + s3Server := mockS3Server(t, tt.s3StatusCode) + defer s3Server.Close() + + h := http.NewServeMux() + h.HandleFunc("/v2/imagefile/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + commonHandler(t, tt.statusCode, w) - if !m.initCalled { - t.Errorf("init image upload request was not made") + if strings.HasSuffix(r.URL.String(), "/_multipart") { + resp := UploadImagePartResponse{Data: UploadImagePart{PresignedURL: s3Server.URL}} + + if err := json.NewEncoder(w).Encode(&resp); err != nil { + t.Fatalf("Error encoding JSON response: %v", err) + } + } + })) + + libraryServer := httptest.NewServer(h) + defer libraryServer.Close() + + c := initClient(t, libraryServer.URL) + + r := strings.NewReader(testPayload) + + etag, err := c.multipartUploadPart( + context.Background(), + 0, + &uploadManager{ + Source: r, + Size: int64(len(testPayload)), + ImageID: "testImageID", + UploadID: "testUploadID", + }, + &defaultUploadCallback{r: r}, + true, + ) + if (err != nil) != tt.expectError { + t.Fatalf("error uploading part: %v", err) } - if !m.putCalled { - t.Errorf("file PUT request was not made") + _ = etag + }) + } +} + +func Test_completeMultipartUpload(t *testing.T) { + s3Server := mockS3Server(t, http.StatusOK) + defer s3Server.Close() + + tests := []struct { + name string + statusCode int + expectError bool + }{ + {"statusOK", http.StatusOK, false}, + {"badRequest", http.StatusBadRequest, true}, + {"internalServerError", http.StatusInternalServerError, true}, + } + + for _, tt := range tests { + tt := tt + + t.Run(tt.name, func(t *testing.T) { + h := http.NewServeMux() + h.HandleFunc("/v2/imagefile/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + commonHandler(t, tt.statusCode, w) + + if strings.HasSuffix(r.URL.String(), "/_multipart_complete") { + resp := CompleteMultipartUploadResponse{Data: UploadImageComplete{ContainerURL: s3Server.URL}} + + if err := json.NewEncoder(w).Encode(&resp); err != nil { + t.Fatalf("Error encoding JSON response: %v", err) + } + } + })) + + libraryServer := httptest.NewServer(h) + defer libraryServer.Close() + + c := initClient(t, libraryServer.URL) + + etag, err := c.completeMultipartUpload( + context.Background(), + &[]CompletedPart{{PartNumber: 0, Token: "xxx"}}, + &uploadManager{ + Source: strings.NewReader(testPayload), + Size: int64(len(testPayload)), + ImageID: "testImageID", + UploadID: "testUploadID", + }) + if (err != nil) != tt.expectError { + t.Fatalf("error uploading part: %v", err) } - if !m.completeCalled { - t.Errorf("image upload complete request was not made") + _ = etag + }) + } +} + +func Test_abortMultipartUpload(t *testing.T) { + s3Server := mockS3Server(t, http.StatusOK) + defer s3Server.Close() + + tests := []struct { + name string + statusCode int + expectError bool + }{ + {"statusOK", http.StatusOK, false}, + {"badRequest", http.StatusBadRequest, true}, + {"internalServerError", http.StatusInternalServerError, true}, + } + + for _, tt := range tests { + tt := tt + + t.Run(tt.name, func(t *testing.T) { + h := http.NewServeMux() + h.HandleFunc("/v2/imagefile/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + uploadImageHelperHandler(t, tt.statusCode, w) + })) + + libraryServer := httptest.NewServer(h) + defer libraryServer.Close() + + c := initClient(t, libraryServer.URL) + + err := c.abortMultipartUpload( + context.Background(), + &uploadManager{ + Source: strings.NewReader(testPayload), + Size: int64(len(testPayload)), + ImageID: "testImageID", + UploadID: "testUploadID", + }) + if (err != nil) != tt.expectError { + t.Fatalf("unexpected error: %v", err) } }) } From a88f0a6bf80e60171e3b397f5a1a27c8a9767b05 Mon Sep 17 00:00:00 2001 From: Tanner Frisch Date: Tue, 18 Jul 2023 14:48:50 -0400 Subject: [PATCH 2/7] fix: linting issues Signed-off-by: Dave Dykstra <2129743+DrDaveD@users.noreply.github.com> --- client/downloader_test.go | 2 + client/oci.go | 8 +- client/pull.go | 2 +- client/push_test.go | 213 ++++++++++++++------------------------ 4 files changed, 84 insertions(+), 141 deletions(-) diff --git a/client/downloader_test.go b/client/downloader_test.go index 01a1dc3..23c5d28 100644 --- a/client/downloader_test.go +++ b/client/downloader_test.go @@ -50,6 +50,8 @@ func (l *stdLogger) Logf(f string, v ...interface{}) { } func parseRangeHeader(t *testing.T, val string) (int64, int64) { + t.Helper() + if val == "" { return 0, 0 } diff --git a/client/oci.go b/client/oci.go index 645318b..79cb7ce 100644 --- a/client/oci.go +++ b/client/oci.go @@ -115,7 +115,7 @@ type basicCredentials struct { password string } -func (c basicCredentials) ModifyRequest(r *http.Request, opts ...modifyRequestOption) error { +func (c basicCredentials) ModifyRequest(r *http.Request, _ ...modifyRequestOption) error { r.SetBasicAuth(c.username, c.password) return nil } @@ -124,7 +124,7 @@ type bearerTokenCredentials struct { authToken string } -func (c bearerTokenCredentials) ModifyRequest(r *http.Request, opts ...modifyRequestOption) error { +func (c bearerTokenCredentials) ModifyRequest(r *http.Request, _ ...modifyRequestOption) error { if c.authToken != "" { r.Header.Set("Authorization", fmt.Sprintf("Bearer %v", c.authToken)) } @@ -403,7 +403,7 @@ func parseAuthHeader(authenticateHeader string) (authHeader, error) { type noneCreds struct{} -func (c *noneCreds) ModifyRequest(r *http.Request, opts ...modifyRequestOption) error { +func (c *noneCreds) ModifyRequest(r *http.Request, _ ...modifyRequestOption) error { r.Header.Set("Authorization", "none") return nil @@ -690,7 +690,7 @@ func (e *unexpectedImageDigest) Error() string { return fmt.Sprintf("unexpected image digest: %v != %v", e.got, e.want) } -func (c *Client) ociUploadImage(ctx context.Context, r io.Reader, size int64, name, arch string, tags []string, description, hash string, callback UploadCallback) error { +func (c *Client) ociUploadImage(ctx context.Context, r io.Reader, size int64, name, _ string, tags []string, description, hash string, callback UploadCallback) error { reg, creds, name, err := c.newOCIRegistry(ctx, name, []accessType{accessTypePull, accessTypePush}) if err != nil { return err diff --git a/client/pull.go b/client/pull.go index 680f9c6..9979408 100644 --- a/client/pull.go +++ b/client/pull.go @@ -279,7 +279,7 @@ func parseContentLengthHeader(val string) (int64, error) { } // download implements a simple, single stream downloader -func (c *Client) download(ctx context.Context, w io.WriterAt, r io.Reader, size int64, pb ProgressBar) error { +func (c *Client) download(_ context.Context, w io.WriterAt, r io.Reader, size int64, pb ProgressBar) error { pb.Init(size) defer pb.Wait() diff --git a/client/push_test.go b/client/push_test.go index 2a1ea50..725f969 100644 --- a/client/push_test.go +++ b/client/push_test.go @@ -764,174 +764,115 @@ func Test_remoteSHA256ChecksumSupport(t *testing.T) { } } -// func Test_legacyPostFileV2(t *testing.T) { -// tests := []struct { -// name string -// imageRef string -// testFile string -// }{ -// { -// name: "Basic", -// imageRef: "5cb9c34d7d960d82f5f5bc55", -// testFile: "test_data/test_sha256", -// }, -// } - -// for _, tt := range tests { -// tt := tt - -// t.Run(tt.name, func(t *testing.T) { -// m := v2ImageUploadMockService{ -// t: t, -// } - -// m.Run() -// defer m.Stop() - -// c := initClient(t, m.baseURI) -// f, err := os.Open(tt.testFile) -// if err != nil { -// t.Errorf("Error opening file %s for reading: %v", tt.testFile, err) -// } -// defer f.Close() - -// fi, err := f.Stat() -// if err != nil { -// t.Errorf("Error stats for file %s: %v", tt.testFile, err) -// } -// fileSize := fi.Size() - -// // calculate sha256 checksum -// sha256checksum, _, err := sha256sum(f) -// if err != nil { -// t.Fatalf("error calculating sha256 checksum: %v", err) -// } - -// _, err = f.Seek(0, 0) -// if err != nil { -// t.Fatalf("unexpected error seeking in sample data file: %v", err) -// } - -// callback := &defaultUploadCallback{r: f} - -// // include sha256 checksum in metadata -// resp, err := c.legacyPostFileV2(context.Background(), fileSize, tt.imageRef, callback, map[string]string{ -// "sha256sum": sha256checksum, -// }) -// if err != nil { -// t.Fatalf("unexpected error: %v", err) -// } - -// if got, want := resp.Quota.QuotaUsageBytes, testQuotaUsageBytes; got != want { -// t.Errorf("got quota usage %v, want %v", got, want) -// } - -// if got, want := resp.Quota.QuotaTotalBytes, testQuotaTotalBytes; got != want { -// t.Errorf("got quota total %v, want %v", got, want) -// } - -// if got, want := resp.ContainerURL, testContainerURL; got != want { -// t.Errorf("got container URL %v, want %v", got, want) -// } - -// if !m.initCalled { -// t.Errorf("init image upload request was not made") -// } - -// if !m.putCalled { -// t.Errorf("file PUT request was not made") -// } - -// if !m.completeCalled { -// t.Errorf("image upload complete request was not made") -// } -// }) -// } -// } - -func Test_legacyPostFileV2URL(t *testing.T) { - s3Server := mockS3Server(t, http.StatusOK) - defer s3Server.Close() - +func Test_legacyPostFileV2(t *testing.T) { tests := []struct { - name string - url string - expectError bool + name string + imageRef string + testFile string }{ - {"basic", s3Server.URL, false}, - {"emptyURL", "", true}, - {"parseURLError", "\n", true}, - {"unsupported", "test", true}, + { + name: "Basic", + imageRef: "5cb9c34d7d960d82f5f5bc55", + testFile: "test_data/test_sha256", + }, } for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - h := http.NewServeMux() - h.HandleFunc("/v2/imagefile/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - commonHandler(t, http.StatusOK, w) + m := v2ImageUploadMockService{ + t: t, + } - resp := UploadImageResponse{Data: UploadImage{UploadURL: tt.url}} + m.Run() + defer m.Stop() - if err := json.NewEncoder(w).Encode(&resp); err != nil { - t.Fatalf("Error encoding JSON response: %v", err) - } - })) + c := initClient(t, m.baseURI) + f, err := os.Open(tt.testFile) + if err != nil { + t.Errorf("Error opening file %s for reading: %v", tt.testFile, err) + } + defer f.Close() - libraryServer := httptest.NewServer(h) - defer libraryServer.Close() + fi, err := f.Stat() + if err != nil { + t.Errorf("Error stats for file %s: %v", tt.testFile, err) + } + fileSize := fi.Size() - c := initClient(t, libraryServer.URL) + // calculate sha256 checksum + sha256checksum, _, err := sha256sum(f) + if err != nil { + t.Fatalf("error calculating sha256 checksum: %v", err) + } - r := strings.NewReader(testPayload) + _, err = f.Seek(0, 0) + if err != nil { + t.Fatalf("unexpected error seeking in sample data file: %v", err) + } - etag, err := c.legacyPostFileV2( - context.Background(), - 0, - "xxx", - &defaultUploadCallback{r: r}, - map[string]string{"sha256sum": "xxx"}, - ) - if (err != nil) != tt.expectError { + callback := &defaultUploadCallback{r: f} + + // include sha256 checksum in metadata + resp, err := c.legacyPostFileV2(context.Background(), fileSize, tt.imageRef, callback, map[string]string{ + "sha256sum": sha256checksum, + }) + if err != nil { t.Fatalf("unexpected error: %v", err) } - _ = etag + if got, want := resp.Quota.QuotaUsageBytes, testQuotaUsageBytes; got != want { + t.Errorf("got quota usage %v, want %v", got, want) + } + + if got, want := resp.Quota.QuotaTotalBytes, testQuotaTotalBytes; got != want { + t.Errorf("got quota total %v, want %v", got, want) + } + + if got, want := resp.ContainerURL, testContainerURL; got != want { + t.Errorf("got container URL %v, want %v", got, want) + } + + if !m.initCalled { + t.Errorf("init image upload request was not made") + } + + if !m.putCalled { + t.Errorf("file PUT request was not made") + } + + if !m.completeCalled { + t.Errorf("image upload complete request was not made") + } }) } } -func Test_legacyPostFileV2(t *testing.T) { +func Test_legacyPostFileV2URL(t *testing.T) { + s3Server := mockS3Server(t, http.StatusOK) + defer s3Server.Close() + tests := []struct { - name string - s3StatusCode int - statusCode int - expectError bool + name string + url string + expectError bool }{ - {"basic", http.StatusOK, http.StatusOK, false}, - {"badRequest", http.StatusBadRequest, http.StatusBadRequest, true}, - {"internalServerError", http.StatusInternalServerError, http.StatusInternalServerError, true}, - {"okBadRequest", http.StatusOK, http.StatusBadRequest, true}, - {"badRequestOK", http.StatusBadRequest, http.StatusOK, true}, - {"internalOK", http.StatusInternalServerError, http.StatusOK, true}, - {"okInternal", http.StatusOK, http.StatusInternalServerError, true}, - {"internalBadRequest", http.StatusInternalServerError, http.StatusBadRequest, true}, - {"badRequestInternal", http.StatusBadRequest, http.StatusInternalServerError, true}, + {"basic", s3Server.URL, false}, + {"emptyURL", "", true}, + {"parseURLError", "\n", true}, + {"unsupported", "test", true}, } for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - s3Server := mockS3Server(t, tt.s3StatusCode) - defer s3Server.Close() - h := http.NewServeMux() h.HandleFunc("/v2/imagefile/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - commonHandler(t, tt.statusCode, w) + commonHandler(t, http.StatusOK, w) - resp := UploadImageResponse{Data: UploadImage{UploadURL: s3Server.URL}} + resp := UploadImageResponse{Data: UploadImage{UploadURL: tt.url}} if err := json.NewEncoder(w).Encode(&resp); err != nil { t.Fatalf("Error encoding JSON response: %v", err) From 782a6438d286b8409eaecd369dd7ac077cf88166 Mon Sep 17 00:00:00 2001 From: Tanner Frisch Date: Tue, 18 Jul 2023 15:17:56 -0400 Subject: [PATCH 3/7] fix: remove unnecessary test struct var Signed-off-by: Dave Dykstra <2129743+DrDaveD@users.noreply.github.com> --- client/push_test.go | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/client/push_test.go b/client/push_test.go index 725f969..37d2c0e 100644 --- a/client/push_test.go +++ b/client/push_test.go @@ -242,8 +242,6 @@ func Test_UploadImageBadPath(t *testing.T) { } for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { h := http.NewServeMux() h.HandleFunc("/v1/imagefile/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -386,8 +384,6 @@ func Test_UploadImage(t *testing.T) { } for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { s3Server := mockS3Server(t, tt.s3StatusCode) defer s3Server.Close() @@ -514,8 +510,6 @@ func Test_postFileV2(t *testing.T) { } for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { h := http.NewServeMux() h.HandleFunc("/v2/imagefile/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -599,8 +593,6 @@ func Test_postFileV2Multipart(t *testing.T) { } for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { s3Server := mockS3Server(t, tt.s3StatusCode) defer s3Server.Close() @@ -674,8 +666,6 @@ func Test_getPartSize(t *testing.T) { } for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { if got, want := getPartSize(tt.bytesRemaining, tt.partSize), tt.want; got != want { t.Fatalf("got: %v, want: %v", got, want) @@ -699,8 +689,6 @@ func Test_startMultipartUpload(t *testing.T) { } for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { h := http.NewServeMux() h.HandleFunc("/v2/imagefile/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -749,8 +737,6 @@ func Test_remoteSHA256ChecksumSupport(t *testing.T) { } for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { q := url.Values{} @@ -778,8 +764,6 @@ func Test_legacyPostFileV2(t *testing.T) { } for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { m := v2ImageUploadMockService{ t: t, @@ -865,8 +849,6 @@ func Test_legacyPostFileV2URL(t *testing.T) { } for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { h := http.NewServeMux() h.HandleFunc("/v2/imagefile/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -964,8 +946,6 @@ func Test_multipartUploadPart(t *testing.T) { } for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { s3Server := mockS3Server(t, tt.s3StatusCode) defer s3Server.Close() @@ -1026,8 +1006,6 @@ func Test_completeMultipartUpload(t *testing.T) { } for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { h := http.NewServeMux() h.HandleFunc("/v2/imagefile/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -1080,8 +1058,6 @@ func Test_abortMultipartUpload(t *testing.T) { } for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { h := http.NewServeMux() h.HandleFunc("/v2/imagefile/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { From f4bacd06723f8b7efa0d79095282cc7541020c72 Mon Sep 17 00:00:00 2001 From: Tanner Frisch Date: Mon, 24 Jul 2023 15:48:56 -0400 Subject: [PATCH 4/7] fix: Removed dynamic error decls Signed-off-by: Dave Dykstra <2129743+DrDaveD@users.noreply.github.com> --- .golangci.yml | 2 +- client/api.go | 61 +++++++++++++++++++++++--------------------- client/api_test.go | 19 +++++++------- client/client.go | 2 +- client/delete.go | 4 ++- client/downloader.go | 13 +++++++--- client/oci.go | 22 ++++++++-------- client/pull.go | 20 ++++++++------- client/push.go | 53 +++++++++++++++++++++----------------- client/ref_test.go | 7 ++--- client/restclient.go | 16 ++++++------ client/search.go | 11 +++++--- 12 files changed, 127 insertions(+), 103 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 246434f..70dab23 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -6,7 +6,6 @@ linters: - containedctx - contextcheck - decorder - - depguard - dogsled - errcheck - errchkjson @@ -14,6 +13,7 @@ linters: - goconst - gocritic - gocyclo + - goerr113 - gofumpt - goimports - goprintffuncname diff --git a/client/api.go b/client/api.go index 8df2cbd..5bfcaba 100644 --- a/client/api.go +++ b/client/api.go @@ -1,4 +1,4 @@ -// Copyright (c) 2018, Sylabs Inc. All rights reserved. +// Copyright (c) 2018-2023, Sylabs Inc. All rights reserved. // This software is licensed under a 3-clause BSD license. Please consult the // LICENSE.md file distributed with the sources of this project regarding your // rights to use or distribute this software. @@ -9,6 +9,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "net/http" "net/url" @@ -16,6 +17,8 @@ import ( jsonresp "github.com/sylabs/json-resp" ) +var errHTTP = errors.New("http error") + // getEntity returns the specified entity; returns ErrNotFound if entity is not // found, otherwise error func (c *Client) getEntity(ctx context.Context, entityRef string) (*Entity, error) { @@ -25,7 +28,7 @@ func (c *Client) getEntity(ctx context.Context, entityRef string) (*Entity, erro } var res EntityResponse if err := json.Unmarshal(entJSON, &res); err != nil { - return nil, fmt.Errorf("error decoding entity: %v", err) + return nil, fmt.Errorf("error decoding entity: %w", err) } return &res.Data, nil } @@ -39,7 +42,7 @@ func (c *Client) getCollection(ctx context.Context, collectionRef string) (*Coll } var res CollectionResponse if err := json.Unmarshal(colJSON, &res); err != nil { - return nil, fmt.Errorf("error decoding collection: %v", err) + return nil, fmt.Errorf("error decoding collection: %w", err) } return &res.Data, nil } @@ -53,7 +56,7 @@ func (c *Client) getContainer(ctx context.Context, containerRef string) (*Contai } var res ContainerResponse if err := json.Unmarshal(conJSON, &res); err != nil { - return nil, fmt.Errorf("error decoding container: %v", err) + return nil, fmt.Errorf("error decoding container: %w", err) } return &res.Data, nil } @@ -70,7 +73,7 @@ func (c *Client) createEntity(ctx context.Context, name string) (*Entity, error) } var res EntityResponse if err := json.Unmarshal(entJSON, &res); err != nil { - return nil, fmt.Errorf("error decoding entity: %v", err) + return nil, fmt.Errorf("error decoding entity: %w", err) } return &res.Data, nil } @@ -88,7 +91,7 @@ func (c *Client) createCollection(ctx context.Context, name string, entityID str } var res CollectionResponse if err := json.Unmarshal(colJSON, &res); err != nil { - return nil, fmt.Errorf("error decoding collection: %v", err) + return nil, fmt.Errorf("error decoding collection: %w", err) } return &res.Data, nil } @@ -106,7 +109,7 @@ func (c *Client) createContainer(ctx context.Context, name string, collectionID } var res ContainerResponse if err := json.Unmarshal(conJSON, &res); err != nil { - return nil, fmt.Errorf("error decoding container: %v", err) + return nil, fmt.Errorf("error decoding container: %w", err) } return &res.Data, nil } @@ -124,7 +127,7 @@ func (c *Client) createImage(ctx context.Context, hash string, containerID strin } var res ImageResponse if err := json.Unmarshal(imgJSON, &res); err != nil { - return nil, fmt.Errorf("error decoding image: %v", err) + return nil, fmt.Errorf("error decoding image: %w", err) } return &res.Data, nil } @@ -162,24 +165,24 @@ func (c *Client) getTags(ctx context.Context, containerID string) (TagMap, error c.Logger.Logf("getTags calling %s", url) req, err := c.newRequest(ctx, http.MethodGet, url, "", nil) if err != nil { - return nil, fmt.Errorf("error creating request to server:\n\t%v", err) + return nil, fmt.Errorf("error creating request to server:\n\t%w", err) } res, err := c.HTTPClient.Do(req) if err != nil { - return nil, fmt.Errorf("error making request to server:\n\t%v", err) + return nil, fmt.Errorf("error making request to server:\n\t%w", err) } defer res.Body.Close() if res.StatusCode != http.StatusOK { err := jsonresp.ReadError(res.Body) if err != nil { - return nil, fmt.Errorf("creation did not succeed: %v", err) + return nil, fmt.Errorf("creation did not succeed: %w", err) } - return nil, fmt.Errorf("unexpected http status code: %d", res.StatusCode) + return nil, fmt.Errorf("%w: unexpected http status code: %d", errHTTP, res.StatusCode) } var tagRes TagsResponse err = json.NewDecoder(res.Body).Decode(&tagRes) if err != nil { - return nil, fmt.Errorf("error decoding tags: %v", err) + return nil, fmt.Errorf("error decoding tags: %w", err) } return tagRes.Data, nil } @@ -190,23 +193,23 @@ func (c *Client) setTag(ctx context.Context, containerID string, t ImageTag) err c.Logger.Logf("setTag calling %s", url) s, err := json.Marshal(t) if err != nil { - return fmt.Errorf("error encoding object to JSON:\n\t%v", err) + return fmt.Errorf("error encoding object to JSON:\n\t%w", err) } req, err := c.newRequest(ctx, http.MethodPost, url, "", bytes.NewBuffer(s)) if err != nil { - return fmt.Errorf("error creating POST request:\n\t%v", err) + return fmt.Errorf("error creating POST request:\n\t%w", err) } res, err := c.HTTPClient.Do(req) if err != nil { - return fmt.Errorf("error making request to server:\n\t%v", err) + return fmt.Errorf("error making request to server:\n\t%w", err) } defer res.Body.Close() if res.StatusCode != http.StatusOK { err := jsonresp.ReadError(res.Body) if err != nil { - return fmt.Errorf("creation did not succeed: %v", err) + return fmt.Errorf("creation did not succeed: %w", err) } - return fmt.Errorf("creation did not succeed: http status code: %d", res.StatusCode) + return fmt.Errorf("%w: creation did not succeed: http status code: %d", errHTTP, res.StatusCode) } return nil } @@ -245,24 +248,24 @@ func (c *Client) getTagsV2(ctx context.Context, containerID string) (ArchTagMap, c.Logger.Logf("getTagsV2 calling %s", url) req, err := c.newRequest(ctx, http.MethodGet, url, "", nil) if err != nil { - return nil, fmt.Errorf("error creating request to server:\n\t%v", err) + return nil, fmt.Errorf("error creating request to server:\n\t%w", err) } res, err := c.HTTPClient.Do(req) if err != nil { - return nil, fmt.Errorf("error making request to server:\n\t%v", err) + return nil, fmt.Errorf("error making request to server:\n\t%w", err) } defer res.Body.Close() if res.StatusCode != http.StatusOK { err := jsonresp.ReadError(res.Body) if err != nil { - return nil, fmt.Errorf("creation did not succeed: %v", err) + return nil, fmt.Errorf("creation did not succeed: %w", err) } - return nil, fmt.Errorf("unexpected http status code: %d", res.StatusCode) + return nil, fmt.Errorf("%w: unexpected http status code: %d", errHTTP, res.StatusCode) } var tagRes ArchTagsResponse err = json.NewDecoder(res.Body).Decode(&tagRes) if err != nil { - return nil, fmt.Errorf("error decoding tags: %v", err) + return nil, fmt.Errorf("error decoding tags: %w", err) } return tagRes.Data, nil } @@ -273,23 +276,23 @@ func (c *Client) setTagV2(ctx context.Context, containerID string, t ArchImageTa c.Logger.Logf("setTag calling %s", url) s, err := json.Marshal(t) if err != nil { - return fmt.Errorf("error encoding object to JSON:\n\t%v", err) + return fmt.Errorf("error encoding object to JSON:\n\t%w", err) } req, err := c.newRequest(ctx, http.MethodPost, url, "", bytes.NewBuffer(s)) if err != nil { - return fmt.Errorf("error creating POST request:\n\t%v", err) + return fmt.Errorf("error creating POST request:\n\t%w", err) } res, err := c.HTTPClient.Do(req) if err != nil { - return fmt.Errorf("error making request to server:\n\t%v", err) + return fmt.Errorf("error making request to server:\n\t%w", err) } defer res.Body.Close() if res.StatusCode != http.StatusOK { err := jsonresp.ReadError(res.Body) if err != nil { - return fmt.Errorf("creation did not succeed: %v", err) + return fmt.Errorf("creation did not succeed: %w", err) } - return fmt.Errorf("creation did not succeed: http status code: %d", res.StatusCode) + return fmt.Errorf("%w: creation did not succeed: http status code: %d", errHTTP, res.StatusCode) } return nil } @@ -310,7 +313,7 @@ func (c *Client) GetImage(ctx context.Context, arch string, imageRef string) (*I } var res ImageResponse if err := json.Unmarshal(imgJSON, &res); err != nil { - return nil, fmt.Errorf("error decoding image: %v", err) + return nil, fmt.Errorf("error decoding image: %w", err) } return &res.Data, nil } diff --git a/client/api_test.go b/client/api_test.go index 60dba3d..8f9be4f 100644 --- a/client/api_test.go +++ b/client/api_test.go @@ -1,4 +1,4 @@ -// Copyright (c) 2018, Sylabs Inc. All rights reserved. +// Copyright (c) 2018-2023, Sylabs Inc. All rights reserved. // This software is licensed under a 3-clause BSD license. Please consult the // LICENSE.md file distributed with the sources of this project regarding your // rights to use or distribute this software. @@ -8,6 +8,7 @@ package client import ( "context" "encoding/json" + "errors" "net/http" "net/http/httptest" "reflect" @@ -261,8 +262,8 @@ func Test_getEntity(t *testing.T) { if err == nil && tt.expectError { t.Errorf("Unexpected success. Expected error.") } - if err != nil && err == ErrNotFound && tt.expectFound { - t.Errorf("Got found %v - expected %v", err != ErrNotFound, tt.expectFound) + if err != nil && errors.Is(err, ErrNotFound) && tt.expectFound { + t.Errorf("Got found %v - expected %v", !errors.Is(err, ErrNotFound), tt.expectFound) } if !reflect.DeepEqual(entity, tt.expectEntity) { t.Errorf("Got entity %v - expected %v", entity, tt.expectEntity) @@ -341,8 +342,8 @@ func Test_getCollection(t *testing.T) { if err == nil && tt.expectError { t.Errorf("Unexpected success. Expected error.") } - if err != nil && err == ErrNotFound && tt.expectFound { - t.Errorf("Got found %v - expected %v", err != ErrNotFound, tt.expectFound) + if err != nil && errors.Is(err, ErrNotFound) && tt.expectFound { + t.Errorf("Got found %v - expected %v", !errors.Is(err, ErrNotFound), tt.expectFound) } if !reflect.DeepEqual(collection, tt.expectCollection) { t.Errorf("Got entity %v - expected %v", collection, tt.expectCollection) @@ -421,8 +422,8 @@ func Test_getContainer(t *testing.T) { if err == nil && tt.expectError { t.Errorf("Unexpected success. Expected error.") } - if err != nil && err != ErrNotFound && tt.expectFound { - t.Errorf("Got found %v - expected %v", err != ErrNotFound, tt.expectFound) + if err != nil && !errors.Is(err, ErrNotFound) && tt.expectFound { + t.Errorf("Got found %v - expected %v", !errors.Is(err, ErrNotFound), tt.expectFound) } if !reflect.DeepEqual(container, tt.expectContainer) { t.Errorf("Got container %v - expected %v", container, tt.expectContainer) @@ -505,8 +506,8 @@ func Test_getImage(t *testing.T) { if err == nil && tt.expectError { t.Errorf("Unexpected success. Expected error.") } - if err != nil && err != ErrNotFound && tt.expectFound { - t.Errorf("Got found %v - expected %v", err != ErrNotFound, tt.expectFound) + if err != nil && !errors.Is(err, ErrNotFound) && tt.expectFound { + t.Errorf("Got found %v - expected %v", !errors.Is(err, ErrNotFound), tt.expectFound) } if !reflect.DeepEqual(image, tt.expectImage) { t.Errorf("Got image %v - expected %v", image, tt.expectImage) diff --git a/client/client.go b/client/client.go index 2a6c13c..80f92d0 100644 --- a/client/client.go +++ b/client/client.go @@ -81,7 +81,7 @@ func NewClient(cfg *Config) (*Client, error) { return nil, err } if baseURL.Scheme != "http" && baseURL.Scheme != "https" { - return nil, fmt.Errorf("unsupported protocol scheme %q", baseURL.Scheme) + return nil, fmt.Errorf("%w: unsupported protocol scheme %q", errHTTP, baseURL.Scheme) } c := &Client{ diff --git a/client/delete.go b/client/delete.go index 54c90fc..812ea20 100644 --- a/client/delete.go +++ b/client/delete.go @@ -6,10 +6,12 @@ import ( "net/url" ) +var errImageRefArchRequired = errors.New("imageRef and arch are required") + // DeleteImage deletes requested imageRef. func (c *Client) DeleteImage(ctx context.Context, imageRef, arch string) error { if imageRef == "" || arch == "" { - return errors.New("imageRef and arch are required") + return errImageRefArchRequired } _, err := c.doDeleteRequest(ctx, "v1/images/"+imageRef+"?arch="+url.QueryEscape(arch)) diff --git a/client/downloader.go b/client/downloader.go index 64af725..44ff8c3 100644 --- a/client/downloader.go +++ b/client/downloader.go @@ -1,4 +1,4 @@ -// Copyright (c) 2021-2022, Sylabs Inc. All rights reserved. +// Copyright (c) 2021-2023, Sylabs Inc. All rights reserved. // This software is licensed under a 3-clause BSD license. Please consult the // LICENSE.md file distributed with the sources of this project regarding your // rights to use or distribute this software. @@ -17,6 +17,11 @@ import ( "golang.org/x/sync/errgroup" ) +var ( + errBadRequest = errors.New("bad request") + errUnexpectedMalformedValue = errors.New("unexpected/malformed value") +) + // filePartDescriptor defines one part of multipart download. type filePartDescriptor struct { start int64 @@ -46,7 +51,7 @@ func minInt64(a, b int64) int64 { // Download performs download of contents at url by writing 'size' bytes to 'dst' using credentials 'c'. func (c *Client) multipartDownload(ctx context.Context, u string, creds credentials, w io.WriterAt, size int64, spec *Downloader, pb ProgressBar) error { if size <= 0 { - return fmt.Errorf("invalid image size (%v)", size) + return fmt.Errorf("%w: invalid image size (%v)", errBadRequest, size) } // Initialize the progress bar using passed size @@ -131,13 +136,13 @@ func parseContentRange(val string) (int64, error) { e := strings.Split(val, " ") if !strings.EqualFold(e[0], "bytes") { - return 0, errors.New("unexpected/malformed value") + return 0, errUnexpectedMalformedValue } rangeElems := strings.Split(e[1], "/") if len(rangeElems) != 2 { - return 0, errors.New("unexpected/malformed value") + return 0, errUnexpectedMalformedValue } return strconv.ParseInt(rangeElems[1], 10, 0) diff --git a/client/oci.go b/client/oci.go index 79cb7ce..0d5900b 100644 --- a/client/oci.go +++ b/client/oci.go @@ -25,6 +25,8 @@ import ( v1 "github.com/opencontainers/image-spec/specs-go/v1" ) +var errOCIServer = errors.New("OCI server error") + const mediaTypeSIFLayer = "application/vnd.sylabs.sif.layer.v1.sif" // ociRegistryAuth uses Cloud Library endpoint to determine if artifact can be pulled @@ -88,7 +90,7 @@ func (c *Client) ociRegistryAuth(ctx context.Context, name string, accessTypes [ endpoint, err := url.Parse(ociArtifactSpec.RegistryURI) if err != nil { - return nil, nil, "", fmt.Errorf("malformed OCI registry URI %v: %v", ociArtifactSpec.RegistryURI, err) + return nil, nil, "", fmt.Errorf("malformed OCI registry URI %v: %w", ociArtifactSpec.RegistryURI, err) } return endpoint, &bearerTokenCredentials{authToken: ociArtifactSpec.Token}, name, nil } @@ -176,7 +178,7 @@ func (r *ociRegistry) getManifestFromIndex(idx v1.Index, arch string) (digest.Di } // If we make it here, no matching OS/architecture was found. - return "", fmt.Errorf("no matching OS/architecture (%v) found", arch) + return "", fmt.Errorf("%w: no matching OS/architecture (%v) found", errOCIServer, arch) } func (r *ociRegistry) getImageManifest(ctx context.Context, creds credentials, name, tag, arch string) (digest.Digest, v1.Manifest, error) { @@ -200,12 +202,12 @@ func (r *ociRegistry) getImageDetails(ctx context.Context, creds credentials, na } if got, want := m.Config.MediaType, mediaTypeSIFConfig; got != want { - return v1.Descriptor{}, fmt.Errorf("unexpected media type error (got %v, want %v)", got, want) + return v1.Descriptor{}, fmt.Errorf("%w: unexpected media type error (got %v, want %v)", errOCIServer, got, want) } // There should always be exactly one layer (the image blob). if n := len(m.Layers); n != 1 { - return v1.Descriptor{}, fmt.Errorf("unexpected # of layers: %v", n) + return v1.Descriptor{}, fmt.Errorf("%w: unexpected # of layers: %v", errOCIServer, n) } // If architecture was supplied, ensure the image config matches. @@ -453,7 +455,7 @@ func (r *ociRegistry) doRequestWithCredentials(req *http.Request, creds credenti return nil, ErrUnauthorized } - return nil, fmt.Errorf("unexpected http status %v", res.StatusCode) + return nil, fmt.Errorf("%w: unexpected http status %v", errHTTP, res.StatusCode) } return res, nil @@ -502,7 +504,7 @@ func (r *ociRegistry) doRequest(req *http.Request, creds credentials, opts ...mo return nil, ErrUnauthorized } - return nil, fmt.Errorf("unexpected http status %v", code) + return nil, fmt.Errorf("%w: unexpected http status %v", errHTTP, code) } return res, nil @@ -747,7 +749,7 @@ func (c *Client) ociUploadImage(ctx context.Context, r io.Reader, size int64, na id = imageDigest if _, err := io.Copy(sifHeader, io.LimitReader(r, sifHeaderSize)); err != nil { - return fmt.Errorf("error reading local SIF file header: %v", err) + return fmt.Errorf("error reading local SIF file header: %w", err) } } @@ -785,7 +787,7 @@ func (c *Client) ociUploadImage(ctx context.Context, r io.Reader, size int64, na c.Logger.Logf("Tag: %v", ref) if _, err := reg.uploadManifest(ctx, creds, name, ref, idx, v1.MediaTypeImageIndex); err != nil { - return fmt.Errorf("error uploading index") + return fmt.Errorf("error uploading index: %w", err) } } @@ -797,7 +799,7 @@ func (r *ociRegistry) existingImageBlob(ctx context.Context, creds credentials, req, err := http.NewRequestWithContext(ctx, http.MethodHead, u.String(), nil) if err != nil { - return false, fmt.Errorf("error checking for existing layer: %v", err) + return false, fmt.Errorf("error checking for existing layer: %w", err) } res, err := r.doRequest(req, creds) @@ -923,7 +925,7 @@ func (r *ociRegistry) openUploadBlobSession(ctx context.Context, creds credentia // Strip prefix from Authorization header parts := strings.SplitN(req.Header.Get("Authorization"), " ", 2) if len(parts) != 2 { - return nil, nil, fmt.Errorf("malformed Authorization header (%v)", req.Header.Get("Authorization")) + return nil, nil, fmt.Errorf("%w malformed Authorization header (%v)", errHTTP, req.Header.Get("Authorization")) } return u, &bearerTokenCredentials{authToken: parts[1]}, nil diff --git a/client/pull.go b/client/pull.go index 9979408..b02d1a2 100644 --- a/client/pull.go +++ b/client/pull.go @@ -19,6 +19,8 @@ import ( jsonresp "github.com/sylabs/json-resp" ) +var errRequestedImageNotFound = fmt.Errorf("requested image was not found in the library") + // DownloadImage will retrieve an image from the Container Library, saving it // into the specified io.Writer. The timeout value for this operation is set // within the context. It is recommended to use a large value (ie. 1800 seconds) @@ -30,7 +32,7 @@ func (c *Client) DownloadImage(ctx context.Context, w io.Writer, arch, path, tag } if strings.Contains(path, ":") { - return fmt.Errorf("malformed image path: %s", path) + return fmt.Errorf("%w: malformed image path: %s", errBadRequest, path) } if tag == "" { @@ -55,18 +57,18 @@ func (c *Client) DownloadImage(ctx context.Context, w io.Writer, arch, path, tag defer res.Body.Close() if res.StatusCode == http.StatusNotFound { - return fmt.Errorf("requested image was not found in the library") + return errRequestedImageNotFound } if res.StatusCode != http.StatusOK { err := jsonresp.ReadError(res.Body) if err != nil { - return fmt.Errorf("download did not succeed: %v", err) + return fmt.Errorf("download did not succeed: %w", err) } if res.StatusCode == http.StatusUnauthorized { return ErrUnauthorized } - return fmt.Errorf("unexpected http status code: %d", res.StatusCode) + return fmt.Errorf("%w: unexpected http status code: %d", errHTTP, res.StatusCode) } c.Logger.Logf("OK response received, beginning body download") @@ -152,7 +154,7 @@ func (c *Client) ConcurrentDownloadImage(ctx context.Context, dst *os.File, arch } if strings.Contains(path, ":") { - return fmt.Errorf("malformed image path: %s", path) + return fmt.Errorf("%w: malformed image path: %s", errBadRequest, path) } name := strings.TrimPrefix(path, "/") @@ -194,7 +196,7 @@ func (c *Client) legacyDownloadImage(ctx context.Context, arch, name, tag string } maxRedir := 10 if len(via) >= maxRedir { - return fmt.Errorf("stopped after %d redirects", maxRedir) + return fmt.Errorf("%w: stopped after %d redirects", errHTTP, maxRedir) } return nil }, @@ -214,7 +216,7 @@ func (c *Client) legacyDownloadImage(ctx context.Context, arch, name, tag string defer res.Body.Close() if res.StatusCode == http.StatusNotFound { - return fmt.Errorf("requested image was not found in the library") + return errRequestedImageNotFound } if res.StatusCode == http.StatusOK { @@ -234,7 +236,7 @@ func (c *Client) legacyDownloadImage(ctx context.Context, arch, name, tag string if res.StatusCode == http.StatusUnauthorized { return ErrUnauthorized } - return fmt.Errorf("unexpected http status %d", res.StatusCode) + return fmt.Errorf("%w: unexpected http status %d", errHTTP, res.StatusCode) } // Get image metadata to determine image size @@ -273,7 +275,7 @@ func parseContentLengthHeader(val string) (int64, error) { } size, err := strconv.ParseInt(val, 10, 64) if err != nil { - return -1, fmt.Errorf("parsing Content-Length header %v: %v", val, err) + return -1, fmt.Errorf("parsing Content-Length header %v: %w", val, err) } return size, nil } diff --git a/client/push.go b/client/push.go index 89b25bd..e2154bb 100644 --- a/client/push.go +++ b/client/push.go @@ -19,6 +19,11 @@ import ( "golang.org/x/sync/errgroup" ) +var ( + errGettingPresignedURL = errors.New("error getting presigned URL") + errParsingPresignedURL = errors.New("error parsing presigned URL") +) + const ( // minimumPartSize is the minimum size of a part in a multipart upload; // this liberty is taken by defining this value on the client-side to @@ -83,7 +88,7 @@ func calculateChecksums(r io.Reader) (string, string, int64, error) { md5checksum, fileSize, err = md5sum(tr) if err != nil { - return fmt.Errorf("error calculating MD5 checksum: %v", err) + return fmt.Errorf("error calculating MD5 checksum: %w", err) } return nil }) @@ -93,7 +98,7 @@ func calculateChecksums(r io.Reader) (string, string, int64, error) { var err error sha256checksum, _, err = sha256sum(pr) if err != nil { - return fmt.Errorf("error calculating SHA checksum: %v", err) + return fmt.Errorf("error calculating SHA checksum: %w", err) } return nil }) @@ -108,23 +113,23 @@ func calculateChecksums(r io.Reader) (string, string, int64, error) { // prevent timeout when uploading large images. func (c *Client) UploadImage(ctx context.Context, r io.ReadSeeker, path, arch string, tags []string, description string, callback UploadCallback) (*UploadImageComplete, error) { if !IsLibraryPushRef(path) { - return nil, fmt.Errorf("malformed image path: %s", path) + return nil, fmt.Errorf("%w: malformed image path: %s", errBadRequest, path) } entityName, collectionName, containerName, parsedTags := ParseLibraryPath(path) if len(parsedTags) != 0 { - return nil, fmt.Errorf("malformed image path: %s", path) + return nil, fmt.Errorf("%w: malformed image path: %s", errBadRequest, path) } // calculate sha256 and md5 checksums md5Checksum, imageHash, fileSize, err := calculateChecksums(r) if err != nil { - return nil, fmt.Errorf("error calculating checksums: %v", err) + return nil, fmt.Errorf("error calculating checksums: %w", err) } // rollback to top of file if _, err = r.Seek(0, io.SeekStart); err != nil { - return nil, fmt.Errorf("error seeking to start stream: %v", err) + return nil, fmt.Errorf("error seeking to start stream: %w", err) } c.Logger.Logf("Image hash computed as %s", imageHash) @@ -141,7 +146,7 @@ func (c *Client) UploadImage(ctx context.Context, r io.ReadSeeker, path, arch st // Find or create entity entity, err := c.getEntity(ctx, entityName) if err != nil { - if err != ErrNotFound { + if !errors.Is(err, ErrNotFound) { return nil, err } c.Logger.Logf("Entity %s does not exist in library - creating it.", entityName) @@ -155,7 +160,7 @@ func (c *Client) UploadImage(ctx context.Context, r io.ReadSeeker, path, arch st qualifiedCollectionName := fmt.Sprintf("%s/%s", entityName, collectionName) collection, err := c.getCollection(ctx, qualifiedCollectionName) if err != nil { - if err != ErrNotFound { + if !errors.Is(err, ErrNotFound) { return nil, err } // create collection @@ -170,7 +175,7 @@ func (c *Client) UploadImage(ctx context.Context, r io.ReadSeeker, path, arch st computedName := fmt.Sprintf("%s/%s", qualifiedCollectionName, containerName) container, err := c.getContainer(ctx, computedName) if err != nil { - if err != ErrNotFound { + if !errors.Is(err, ErrNotFound) { return nil, err } // Create container @@ -184,7 +189,7 @@ func (c *Client) UploadImage(ctx context.Context, r io.ReadSeeker, path, arch st // Find or create image image, err := c.GetImage(ctx, arch, computedName+":"+"sha256."+imageHash) if err != nil { - if err != ErrNotFound { + if !errors.Is(err, ErrNotFound) { return nil, err } // Create image @@ -286,14 +291,14 @@ func (c *Client) postFile(ctx context.Context, fileSize int64, imageID string, c req.ContentLength = fileSize res, err := c.HTTPClient.Do(req) if err != nil { - return nil, fmt.Errorf("error uploading file to server: %s", err.Error()) + return nil, fmt.Errorf("error uploading file to server: %w", err) } defer res.Body.Close() if res.StatusCode != http.StatusOK { if err := jsonresp.ReadError(res.Body); err != nil { - return nil, fmt.Errorf("sending file did not succeed: %v", err) + return nil, fmt.Errorf("sending file did not succeed: %w", err) } - return nil, fmt.Errorf("sending file did not succeed: http status code %d", res.StatusCode) + return nil, fmt.Errorf("%w: sending file did not succeed: http status code %d", errHTTP, res.StatusCode) } return nil, nil } @@ -314,7 +319,7 @@ func (c *Client) postFileV2(ctx context.Context, r io.ReadSeeker, fileSize int64 if err != nil { // if the error is anything other than ErrNotFound, fallback to legacy (single part) // uploader. - if err != ErrNotFound { + if !errors.Is(err, ErrNotFound) { return nil, err } // fallthrough to legacy (single part) uploader @@ -475,12 +480,12 @@ func (c *Client) legacyPostFileV2(ctx context.Context, fileSize int64, imageID s // upload (PUT) directly to S3 presigned URL provided above presignedURL := res.Data.UploadURL if presignedURL == "" { - return nil, fmt.Errorf("error getting presigned URL") + return nil, errGettingPresignedURL } parsedURL, err := url.Parse(presignedURL) if err != nil { - return nil, fmt.Errorf("error parsing presigned URL") + return nil, errParsingPresignedURL } // parse presigned URL to determine if we need to send sha256 checksum @@ -488,7 +493,7 @@ func (c *Client) legacyPostFileV2(ctx context.Context, fileSize int64, imageID s req, err := http.NewRequestWithContext(ctx, http.MethodPut, presignedURL, callback.GetReader()) if err != nil { - return nil, fmt.Errorf("error creating request: %v", err) + return nil, fmt.Errorf("error creating request: %w", err) } req.ContentLength = fileSize @@ -501,18 +506,18 @@ func (c *Client) legacyPostFileV2(ctx context.Context, fileSize int64, imageID s resp, err := c.HTTPClient.Do(req) callback.Finish() if err != nil { - return nil, fmt.Errorf("error uploading image: %v", err) + return nil, fmt.Errorf("error uploading image: %w", err) } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("error uploading image: HTTP status %d", resp.StatusCode) + return nil, fmt.Errorf("%w: error uploading image: HTTP status %d", errHTTP, resp.StatusCode) } // send (PUT) image upload completion objJSON, err = c.apiUpdate(ctx, postURL+"/_complete", UploadImageCompleteRequest{}) if err != nil { - return nil, fmt.Errorf("error sending upload complete request: %v", err) + return nil, fmt.Errorf("error sending upload complete request: %w", err) } if len(objJSON) == 0 { @@ -522,7 +527,7 @@ func (c *Client) legacyPostFileV2(ctx context.Context, fileSize int64, imageID s var uploadResp UploadImageCompleteResponse if err := json.Unmarshal(objJSON, &uploadResp); err != nil { - return nil, fmt.Errorf("error decoding upload response: %v", err) + return nil, fmt.Errorf("error decoding upload response: %w", err) } return &uploadResp.Data, nil } @@ -576,7 +581,7 @@ func (c *Client) multipartUploadPart(ctx context.Context, partNumber int, m *upl // send request to S3 req, err := http.NewRequestWithContext(ctx, http.MethodPut, res.Data.PresignedURL, io.LimitReader(callback.GetReader(), m.Size)) if err != nil { - return "", fmt.Errorf("error creating request: %v", err) + return "", fmt.Errorf("error creating request: %w", err) } // add headers to be signed @@ -587,7 +592,7 @@ func (c *Client) multipartUploadPart(ctx context.Context, partNumber int, m *upl resp, err := c.HTTPClient.Do(req) if err != nil { - c.Logger.Logf("Failure uploading to presigned URL: %v", err) + c.Logger.Logf("Failure uploading to presigned URL: %w", err) return "", err } defer resp.Body.Close() @@ -595,7 +600,7 @@ func (c *Client) multipartUploadPart(ctx context.Context, partNumber int, m *upl // process response from S3 if resp.StatusCode != http.StatusOK { c.Logger.Logf("Object store returned an error: %d", resp.StatusCode) - return "", fmt.Errorf("object store returned an error: %d", resp.StatusCode) + return "", fmt.Errorf("%w: object store returned an error: %d", errHTTP, resp.StatusCode) } etag := resp.Header.Get("ETag") diff --git a/client/ref_test.go b/client/ref_test.go index 06db924..f684256 100644 --- a/client/ref_test.go +++ b/client/ref_test.go @@ -1,4 +1,4 @@ -// Copyright (c) 2018-2022, Sylabs Inc. All rights reserved. +// Copyright (c) 2018-2023, Sylabs Inc. All rights reserved. // This software is licensed under a 3-clause BSD license. Please consult the // LICENSE.md file distributed with the sources of this project regarding your // rights to use or distribute this software. @@ -6,6 +6,7 @@ package client import ( + "errors" "reflect" "testing" ) @@ -150,7 +151,7 @@ func TestParse(t *testing.T) { t.Fatalf("got err %v, want %v", err, tt.wantErr) } if tt.wantErrSpecific != nil { - if got, want := err, tt.wantErrSpecific; got != want { + if got, want := err, tt.wantErrSpecific; !errors.Is(got, want) { t.Fatalf("got err %v, want %v", got, want) } } @@ -312,7 +313,7 @@ func TestParseAmbiguous(t *testing.T) { t.Fatalf("got err %v, want %v", err, tt.wantErr) } if tt.wantErrSpecific != nil { - if got, want := err, tt.wantErrSpecific; got != want { + if got, want := err, tt.wantErrSpecific; !errors.Is(got, want) { t.Fatalf("got err %v, want %v", got, want) } } diff --git a/client/restclient.go b/client/restclient.go index 2ee1839..847d85d 100644 --- a/client/restclient.go +++ b/client/restclient.go @@ -1,4 +1,4 @@ -// Copyright (c) 2018-2019, Sylabs Inc. All rights reserved. +// Copyright (c) 2018-2023, Sylabs Inc. All rights reserved. // This software is licensed under a 3-clause BSD license. Please consult the // LICENSE.md file distributed with the sources of this project regarding your // rights to use or distribute this software. @@ -59,7 +59,7 @@ func (c *Client) commonRequestHandler(ctx context.Context, method string, path s if method != "GET" && method != "DELETE" { s, err := json.Marshal(o) if err != nil { - return []byte{}, fmt.Errorf("error encoding object to JSON:\n\t%v", err) + return []byte{}, fmt.Errorf("error encoding object to JSON:\n\t%w", err) } payload = bytes.NewBuffer(s) } @@ -67,17 +67,17 @@ func (c *Client) commonRequestHandler(ctx context.Context, method string, path s // split url containing query into component pieces (path and raw query) u, err := url.Parse(path) if err != nil { - return []byte{}, fmt.Errorf("error parsing url:\n\t%v", err) + return []byte{}, fmt.Errorf("error parsing url:\n\t%w", err) } req, err := c.newRequest(ctx, method, u.Path, u.RawQuery, payload) if err != nil { - return []byte{}, fmt.Errorf("error creating %s request:\n\t%v", method, err) + return []byte{}, fmt.Errorf("error creating %s request:\n\t%w", method, err) } res, err := c.HTTPClient.Do(req) if err != nil { - return []byte{}, fmt.Errorf("error making request to server:\n\t%v", err) + return []byte{}, fmt.Errorf("error making request to server:\n\t%w", err) } defer res.Body.Close() @@ -88,13 +88,13 @@ func (c *Client) commonRequestHandler(ctx context.Context, method string, path s if !isValidStatusCode(res.StatusCode, acceptedStatusCodes) { err := jsonresp.ReadError(res.Body) if err != nil { - return []byte{}, fmt.Errorf("request did not succeed: %v", err) + return []byte{}, fmt.Errorf("request did not succeed: %w", err) } - return []byte{}, fmt.Errorf("request did not succeed: http status code: %d", res.StatusCode) + return []byte{}, fmt.Errorf("%w: request did not succeed: http status code: %d", errHTTP, res.StatusCode) } objJSON, err = io.ReadAll(res.Body) if err != nil { - return []byte{}, fmt.Errorf("error reading response from server:\n\t%v", err) + return []byte{}, fmt.Errorf("error reading response from server:\n\t%w", err) } return objJSON, nil } diff --git a/client/search.go b/client/search.go index f48b38f..8d538bb 100644 --- a/client/search.go +++ b/client/search.go @@ -1,4 +1,4 @@ -// Copyright (c) 2018, Sylabs Inc. All rights reserved. +// Copyright (c) 2018-2023, Sylabs Inc. All rights reserved. // This software is licensed under a 3-clause BSD license. Please consult the // LICENSE.md file distributed with the sources of this project regarding your // rights to use or distribute this software. @@ -8,10 +8,13 @@ package client import ( "context" "encoding/json" + "errors" "fmt" "net/url" ) +var errQueryValueMustBeSpecified = errors.New("search query ('value') must be specified") + // Search performs a library search, returning any matching collections, // containers, entities, or images. // @@ -41,11 +44,11 @@ func (c *Client) Search(ctx context.Context, args map[string]string) (*SearchRes // "value" is minimally required in "args" value, ok := args["value"] if !ok { - return nil, fmt.Errorf("search query ('value') must be specified") + return nil, errQueryValueMustBeSpecified } if len(value) < 3 { - return nil, fmt.Errorf("bad query '%s'. You must search for at least 3 characters", value) + return nil, fmt.Errorf("%w: bad query '%s'. You must search for at least 3 characters", errBadRequest, value) } v := url.Values{} @@ -60,7 +63,7 @@ func (c *Client) Search(ctx context.Context, args map[string]string) (*SearchRes var res SearchResponse if err := json.Unmarshal(resJSON, &res); err != nil { - return nil, fmt.Errorf("error decoding results: %v", err) + return nil, fmt.Errorf("error decoding results: %w", err) } return &res.Data, nil From c47350c51c106fe665da494f48cb4681c3675509 Mon Sep 17 00:00:00 2001 From: Tanner Frisch Date: Tue, 25 Jul 2023 11:08:16 -0400 Subject: [PATCH 5/7] fix: Resolve suggested changes Signed-off-by: Dave Dykstra <2129743+DrDaveD@users.noreply.github.com> --- client/api.go | 3 --- client/client.go | 4 ---- client/delete.go | 3 --- client/downloader.go | 6 ------ client/errors.go | 36 ++++++++++++++++++++++++++++++++++++ client/oci.go | 20 +++----------------- client/push.go | 5 ----- client/ref.go | 16 ---------------- client/restclient.go | 4 ---- client/search.go | 3 --- 10 files changed, 39 insertions(+), 61 deletions(-) create mode 100644 client/errors.go diff --git a/client/api.go b/client/api.go index 5bfcaba..646736f 100644 --- a/client/api.go +++ b/client/api.go @@ -9,7 +9,6 @@ import ( "bytes" "context" "encoding/json" - "errors" "fmt" "net/http" "net/url" @@ -17,8 +16,6 @@ import ( jsonresp "github.com/sylabs/json-resp" ) -var errHTTP = errors.New("http error") - // getEntity returns the specified entity; returns ErrNotFound if entity is not // found, otherwise error func (c *Client) getEntity(ctx context.Context, entityRef string) (*Entity, error) { diff --git a/client/client.go b/client/client.go index 80f92d0..b4fbb02 100644 --- a/client/client.go +++ b/client/client.go @@ -7,7 +7,6 @@ package client import ( "context" - "errors" "fmt" "io" "net/http" @@ -17,9 +16,6 @@ import ( "github.com/go-log/log" ) -// ErrUnauthorized represents HTTP status "401 Unauthorized" -var ErrUnauthorized = errors.New("unauthorized") - // Config contains the client configuration. type Config struct { // Base URL of the service. diff --git a/client/delete.go b/client/delete.go index 812ea20..a83eb6c 100644 --- a/client/delete.go +++ b/client/delete.go @@ -2,12 +2,9 @@ package client import ( "context" - "errors" "net/url" ) -var errImageRefArchRequired = errors.New("imageRef and arch are required") - // DeleteImage deletes requested imageRef. func (c *Client) DeleteImage(ctx context.Context, imageRef, arch string) error { if imageRef == "" || arch == "" { diff --git a/client/downloader.go b/client/downloader.go index 44ff8c3..b0fd125 100644 --- a/client/downloader.go +++ b/client/downloader.go @@ -7,7 +7,6 @@ package client import ( "context" - "errors" "fmt" "io" "net/http" @@ -17,11 +16,6 @@ import ( "golang.org/x/sync/errgroup" ) -var ( - errBadRequest = errors.New("bad request") - errUnexpectedMalformedValue = errors.New("unexpected/malformed value") -) - // filePartDescriptor defines one part of multipart download. type filePartDescriptor struct { start int64 diff --git a/client/errors.go b/client/errors.go new file mode 100644 index 0000000..11eb1f2 --- /dev/null +++ b/client/errors.go @@ -0,0 +1,36 @@ +package client + +import "errors" + +var ( + errHTTP = errors.New("http error") + // ErrUnauthorized represents HTTP status "401 Unauthorized" + ErrUnauthorized = errors.New("unauthorized") + errImageRefArchRequired = errors.New("imageRef and arch are required") + errBadRequest = errors.New("bad request") + errUnexpectedMalformedValue = errors.New("unexpected/malformed value") + errOCIRegistry = errors.New("OCI registry error") + errArchNotSpecified = errors.New("architecture not specified") + errInvalidAuthHeader = errors.New("invalid auth header") + errResetHTTPBody = errors.New("unable to reset HTTP request body") + errArchitectureNotPresent = errors.New("architecture not present") + errDigestNotVerified = errors.New("digest not verified") + errOCIDownloadNotSupported = errors.New("not supported") + errGettingPresignedURL = errors.New("error getting presigned URL") + errParsingPresignedURL = errors.New("error parsing presigned URL") + // ErrRefSchemeNotValid represents a ref with an invalid scheme. + ErrRefSchemeNotValid = errors.New("library: ref scheme not valid") + // ErrRefUserNotPermitted represents a ref with an invalid user. + ErrRefUserNotPermitted = errors.New("library: user not permitted in ref") + // ErrRefQueryNotPermitted represents a ref with an invalid query. + ErrRefQueryNotPermitted = errors.New("library: query not permitted in ref") + // ErrRefFragmentNotPermitted represents a ref with an invalid fragment. + ErrRefFragmentNotPermitted = errors.New("library: fragment not permitted in ref") + // ErrRefPathNotValid represents a ref with an invalid path. + ErrRefPathNotValid = errors.New("library: ref path not valid") + // ErrRefTagsNotValid represents a ref with invalid tags. + ErrRefTagsNotValid = errors.New("library: ref tags not valid") + // ErrNotFound is returned by when a resource is not found (http status 404) + ErrNotFound = errors.New("not found") + errQueryValueMustBeSpecified = errors.New("search query ('value') must be specified") +) diff --git a/client/oci.go b/client/oci.go index 0d5900b..873731a 100644 --- a/client/oci.go +++ b/client/oci.go @@ -25,8 +25,6 @@ import ( v1 "github.com/opencontainers/image-spec/specs-go/v1" ) -var errOCIServer = errors.New("OCI server error") - const mediaTypeSIFLayer = "application/vnd.sylabs.sif.layer.v1.sif" // ociRegistryAuth uses Cloud Library endpoint to determine if artifact can be pulled @@ -152,8 +150,6 @@ type ociRegistry struct { logger log.Logger } -var errArchNotSpecified = errors.New("architecture not specified") - func (r *ociRegistry) getManifestFromIndex(idx v1.Index, arch string) (digest.Digest, error) { // If arch not supplied, return single manifest or error. if arch == "" { @@ -178,7 +174,7 @@ func (r *ociRegistry) getManifestFromIndex(idx v1.Index, arch string) (digest.Di } // If we make it here, no matching OS/architecture was found. - return "", fmt.Errorf("%w: no matching OS/architecture (%v) found", errOCIServer, arch) + return "", fmt.Errorf("%w: no matching OS/architecture (%v) found", errOCIRegistry, arch) } func (r *ociRegistry) getImageManifest(ctx context.Context, creds credentials, name, tag, arch string) (digest.Digest, v1.Manifest, error) { @@ -202,12 +198,12 @@ func (r *ociRegistry) getImageDetails(ctx context.Context, creds credentials, na } if got, want := m.Config.MediaType, mediaTypeSIFConfig; got != want { - return v1.Descriptor{}, fmt.Errorf("%w: unexpected media type error (got %v, want %v)", errOCIServer, got, want) + return v1.Descriptor{}, fmt.Errorf("%w: unexpected media type error (got %v, want %v)", errOCIRegistry, got, want) } // There should always be exactly one layer (the image blob). if n := len(m.Layers); n != 1 { - return v1.Descriptor{}, fmt.Errorf("%w: unexpected # of layers: %v", errOCIServer, n) + return v1.Descriptor{}, fmt.Errorf("%w: unexpected # of layers: %v", errOCIRegistry, n) } // If architecture was supplied, ensure the image config matches. @@ -336,8 +332,6 @@ func parsePairs(value string) map[string]string { return m } -var errInvalidAuthHeader = errors.New("invalid auth header") - type authHeader struct { at authType realm string @@ -430,8 +424,6 @@ func withHTTPClient(client *http.Client) modifyRequestOption { } } -var errResetHTTPBody = errors.New("unable to reset HTTP request body") - func (r *ociRegistry) doRequestWithCredentials(req *http.Request, creds credentials, opts ...modifyRequestOption) (*http.Response, error) { opts = append(opts, withUserAgent(r.userAgent), @@ -589,8 +581,6 @@ func (r *ociRegistry) downloadBlob(ctx context.Context, creds credentials, name return io.Copy(w, res.Body) } -var errArchitectureNotPresent = errors.New("architecture not present") - // validateImageConfig validates ic, and returns an error when ic is invalid. func validateImageConfig(ic imageConfig) error { if ic.Architecture == "" { @@ -618,8 +608,6 @@ func (e *unexpectedArchitectureError) Is(target error) bool { (e.want == t.want || t.want == "") } -var errDigestNotVerified = errors.New("digest not verified") - func (r *ociRegistry) getImageConfig(ctx context.Context, creds credentials, name string, d digest.Digest) (imageConfig, error) { var b bytes.Buffer if _, err := r.downloadBlob(ctx, creds, name, d, "", &b); err != nil { @@ -642,8 +630,6 @@ func (r *ociRegistry) getImageConfig(ctx context.Context, creds credentials, nam return ic, nil } -var errOCIDownloadNotSupported = errors.New("not supported") - // newOCIRegistry returns *ociRegistry, credentials for that registry, and the (optionally) remapped image name func (c *Client) newOCIRegistry(ctx context.Context, name string, accessTypes []accessType) (*ociRegistry, *bearerTokenCredentials, string, error) { // Attempt to obtain (direct) OCI registry auth token diff --git a/client/push.go b/client/push.go index e2154bb..db75c1c 100644 --- a/client/push.go +++ b/client/push.go @@ -19,11 +19,6 @@ import ( "golang.org/x/sync/errgroup" ) -var ( - errGettingPresignedURL = errors.New("error getting presigned URL") - errParsingPresignedURL = errors.New("error parsing presigned URL") -) - const ( // minimumPartSize is the minimum size of a part in a multipart upload; // this liberty is taken by defining this value on the client-side to diff --git a/client/ref.go b/client/ref.go index 5084331..fae961a 100644 --- a/client/ref.go +++ b/client/ref.go @@ -6,7 +6,6 @@ package client import ( - "errors" "net/url" "strings" ) @@ -14,21 +13,6 @@ import ( // Scheme is the required scheme for Library URIs. const Scheme = "library" -var ( - // ErrRefSchemeNotValid represents a ref with an invalid scheme. - ErrRefSchemeNotValid = errors.New("library: ref scheme not valid") - // ErrRefUserNotPermitted represents a ref with an invalid user. - ErrRefUserNotPermitted = errors.New("library: user not permitted in ref") - // ErrRefQueryNotPermitted represents a ref with an invalid query. - ErrRefQueryNotPermitted = errors.New("library: query not permitted in ref") - // ErrRefFragmentNotPermitted represents a ref with an invalid fragment. - ErrRefFragmentNotPermitted = errors.New("library: fragment not permitted in ref") - // ErrRefPathNotValid represents a ref with an invalid path. - ErrRefPathNotValid = errors.New("library: ref path not valid") - // ErrRefTagsNotValid represents a ref with invalid tags. - ErrRefTagsNotValid = errors.New("library: ref tags not valid") -) - // A Ref represents a parsed Library URI. // // The general form represented is: diff --git a/client/restclient.go b/client/restclient.go index 847d85d..ae0602e 100644 --- a/client/restclient.go +++ b/client/restclient.go @@ -9,7 +9,6 @@ import ( "bytes" "context" "encoding/json" - "errors" "fmt" "io" "net/http" @@ -18,9 +17,6 @@ import ( jsonresp "github.com/sylabs/json-resp" ) -// ErrNotFound is returned by when a resource is not found (http status 404) -var ErrNotFound = errors.New("not found") - func (c *Client) apiGet(ctx context.Context, path string) (objJSON []byte, err error) { c.Logger.Logf("apiGet calling %s", path) return c.doGETRequest(ctx, path) diff --git a/client/search.go b/client/search.go index 8d538bb..3406b91 100644 --- a/client/search.go +++ b/client/search.go @@ -8,13 +8,10 @@ package client import ( "context" "encoding/json" - "errors" "fmt" "net/url" ) -var errQueryValueMustBeSpecified = errors.New("search query ('value') must be specified") - // Search performs a library search, returning any matching collections, // containers, entities, or images. // From 75fa7a0d6f82d43464fea48a851e6b087033c5f7 Mon Sep 17 00:00:00 2001 From: Dave Dykstra <2129743+DrDaveD@users.noreply.github.com> Date: Fri, 29 Dec 2023 10:09:51 -0600 Subject: [PATCH 6/7] update CI to golang 1.20 Signed-off-by: Dave Dykstra <2129743+DrDaveD@users.noreply.github.com> --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0a58aef..71b775d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,7 +22,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v1 with: - go-version: '1.19.x' + go-version: '1.20.x' - name: Check go.mod and go.sum are tidy run: | From 35402370c95480ec5c76b4663ae9584621ef9742 Mon Sep 17 00:00:00 2001 From: Dave Dykstra <2129743+DrDaveD@users.noreply.github.com> Date: Fri, 29 Dec 2023 12:12:17 -0600 Subject: [PATCH 7/7] fix more linting errors Signed-off-by: Dave Dykstra <2129743+DrDaveD@users.noreply.github.com> --- .github/workflows/ci.yml | 4 +++- client/client.go | 2 +- client/errors.go | 2 ++ client/util.go | 2 +- 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 71b775d..aa02e62 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -35,7 +35,9 @@ jobs: - name: Install Lint uses: golangci/golangci-lint-action@v2 with: - version: v1.51 + version: v1.54 + skip-pkg-cache: true + skip-build-cache: true - name: Run Lint run: | diff --git a/client/client.go b/client/client.go index b4fbb02..7b088d4 100644 --- a/client/client.go +++ b/client/client.go @@ -62,7 +62,7 @@ func NewClient(cfg *Config) (*Client, error) { } if bu == "" { - return nil, fmt.Errorf("no BaseURL supplied") + return nil, errNoBaseURL } // If baseURL has a path component, ensure it is terminated with a separator, to prevent diff --git a/client/errors.go b/client/errors.go index 11eb1f2..694ee11 100644 --- a/client/errors.go +++ b/client/errors.go @@ -18,6 +18,8 @@ var ( errOCIDownloadNotSupported = errors.New("not supported") errGettingPresignedURL = errors.New("error getting presigned URL") errParsingPresignedURL = errors.New("error parsing presigned URL") + errNoBaseURL = errors.New("no BaseURL supplied") + // ErrRefSchemeNotValid represents a ref with an invalid scheme. ErrRefSchemeNotValid = errors.New("library: ref scheme not valid") // ErrRefUserNotPermitted represents a ref with an invalid user. diff --git a/client/util.go b/client/util.go index 7d8a850..87220d7 100644 --- a/client/util.go +++ b/client/util.go @@ -78,7 +78,7 @@ func ParseLibraryPath(libraryRef string) (entity string, collection string, cont collection = "" container = "" tags = []string{} - return + return entity, collection, container, tags } if strings.Contains(container, ":") {