From 375ed075b48b21ea639db2312a8018aa8593d364 Mon Sep 17 00:00:00 2001 From: "Adam D. Cornett" Date: Mon, 29 Apr 2024 13:26:10 -0700 Subject: [PATCH] reordering submit flow to create test results as first action, then linking to an imageID later in the flow to be able to fail fast and and not have incomplete results attached to an image Signed-off-by: Adam D. Cornett --- internal/pyxis/pyxis.go | 38 ++++++++++++++++++++++++++++++ internal/pyxis/pyxis_suite_test.go | 4 +++- internal/pyxis/pyxis_test.go | 1 + internal/pyxis/submit.go | 34 ++++++++++++++++---------- internal/pyxis/submit_test.go | 12 ++++++++++ 5 files changed, 76 insertions(+), 13 deletions(-) diff --git a/internal/pyxis/pyxis.go b/internal/pyxis/pyxis.go index 217e3f5d..5c4ce53e 100644 --- a/internal/pyxis/pyxis.go +++ b/internal/pyxis/pyxis.go @@ -445,6 +445,44 @@ func (p *pyxisClient) createTestResults(ctx context.Context, testResults *TestRe return &newTestResults, nil } +func (p *pyxisClient) updateTestResults(ctx context.Context, testResults *TestResults) (*TestResults, error) { + b, err := json.Marshal(testResults) + if err != nil { + return nil, fmt.Errorf("could not marshal test results: %w", err) + } + + req, err := p.newRequestWithAPIToken(ctx, http.MethodPatch, p.getPyxisURL(fmt.Sprintf("projects/certification/test-results/id/%s", testResults.ID)), bytes.NewReader(b)) + if err != nil { + return nil, fmt.Errorf("could not create new request: %w", err) + } + + resp, err := p.Client.Do(req) + if err != nil { + return nil, fmt.Errorf("could not update test results in pyxis: %w", err) + } + + defer resp.Body.Close() + + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("could not read body: %w", err) + } + + if ok := checkStatus(resp.StatusCode); !ok { + return nil, fmt.Errorf( + "status code: %d: body: %s", + resp.StatusCode, + string(body)) + } + + newTestResults := TestResults{} + if err := json.Unmarshal(body, &newTestResults); err != nil { + return nil, fmt.Errorf("could not unmarshal body: %s: %w", string(body), err) + } + + return &newTestResults, nil +} + func (p *pyxisClient) createArtifact(ctx context.Context, artifact *Artifact) (*Artifact, error) { logger := logr.FromContextOrDiscard(ctx) diff --git a/internal/pyxis/pyxis_suite_test.go b/internal/pyxis/pyxis_suite_test.go index 3db263ae..d94fd602 100644 --- a/internal/pyxis/pyxis_suite_test.go +++ b/internal/pyxis/pyxis_suite_test.go @@ -157,8 +157,10 @@ func pyxisTestResultsHandler(ctx context.Context) http.HandlerFunc { switch { case request.Header["X-Api-Key"][0] == "my-bad-testresults-api-token": response.WriteHeader(http.StatusUnauthorized) + case request.Method == http.MethodPatch && request.Header["X-Api-Key"][0] == "my-bad-results-patch-api-token": + response.WriteHeader(http.StatusInternalServerError) default: - mustWrite(response, `{"image":"quay.io/awesome/image:latest","passed": true}`) + mustWrite(response, `{"_id": "54321","image": "quay.io/awesome/image:latest","passed": true}`) } } } diff --git a/internal/pyxis/pyxis_test.go b/internal/pyxis/pyxis_test.go index f987b56e..4151fcca 100644 --- a/internal/pyxis/pyxis_test.go +++ b/internal/pyxis/pyxis_test.go @@ -12,6 +12,7 @@ var _ = Describe("Pyxis", func() { ctx := context.Background() mux := http.NewServeMux() mux.HandleFunc("/query/", pyxisGraphqlFindImagesHandler(ctx)) + mux.HandleFunc("/api/v1/projects/certification/test-results/id/54321", pyxisTestResultsHandler(ctx)) pyxisClient := NewPyxisClient("my.pyxis.host/query/", "my-spiffy-api-token", "my-awesome-project-id", &http.Client{Transport: localRoundTripper{handler: mux}}) Context("Find Images", func() { diff --git a/internal/pyxis/submit.go b/internal/pyxis/submit.go index d87131c2..fd9d7119 100644 --- a/internal/pyxis/submit.go +++ b/internal/pyxis/submit.go @@ -15,8 +15,20 @@ var defaultRegistryAlias = "docker.io" func (p *pyxisClient) SubmitResults(ctx context.Context, certInput *CertificationInput) (*CertificationResults, error) { var err error - certProject := certInput.CertProject certImage := certInput.CertImage + // You must have an existing repository. + if len(certImage.Repositories) == 0 { + return nil, fmt.Errorf("certImage has not been properly populated") + } + + // Create the test results, so we can fail fast if version check throws error. + testResults := certInput.TestResults + testResults, err = p.createTestResults(ctx, testResults) + if err != nil { + return nil, fmt.Errorf("could not create test results: %v", err) + } + + certProject := certInput.CertProject // Submission effectively starts the certification process, so switch // the status to reflect this if needed. This only needs to be done for net new projects. @@ -28,11 +40,6 @@ func (p *pyxisClient) SubmitResults(ctx context.Context, certInput *Certificatio certProject.CertificationStatus = "In Progress" } - // You must have an existing repository. - if len(certImage.Repositories) == 0 { - return nil, fmt.Errorf("certImage has not been properly populated") - } - // Always set the project's metadata to match the image that we're certifying. These values will always be sent // to pyxis which has the validation rules on if the values can be updated, and will throw an exception if they // are not allowed to be updated, ie if the images/projects are already published. @@ -108,19 +115,22 @@ func (p *pyxisClient) SubmitResults(ctx context.Context, certInput *Certificatio } } - // Create the test results. - testResults := certInput.TestResults - testResults.ImageID = certImage.ID - testResults, err = p.createTestResults(ctx, testResults) + // Update the test results with the certification image id to link the results to the image. + updatedTestResults := &TestResults{ + ID: testResults.ID, + ImageID: certImage.ID, + } + + updatedTestResults, err = p.updateTestResults(ctx, updatedTestResults) if err != nil { - return nil, fmt.Errorf("could not create test results: %v", err) + return nil, fmt.Errorf("could not update test results: %v", err) } // Return the results with up-to-date information. return &CertificationResults{ CertProject: certProject, CertImage: certImage, - TestResults: testResults, + TestResults: updatedTestResults, }, nil } diff --git a/internal/pyxis/submit_test.go b/internal/pyxis/submit_test.go index ee3a1dd0..d38a5d6c 100644 --- a/internal/pyxis/submit_test.go +++ b/internal/pyxis/submit_test.go @@ -17,6 +17,8 @@ var _ = Describe("Pyxis Submit", func() { // These go from most explicit to least explicit. They will be check that way by the ServeMux. mux.HandleFunc("/api/v1/projects/certification/id/my-awesome-project-id/test-results", pyxisTestResultsHandler(ctx)) + mux.HandleFunc("/api/v1/projects/certification/id/my-image-project-id/test-results", pyxisTestResultsHandler(ctx)) + mux.HandleFunc("/api/v1/projects/certification/test-results/id/54321", pyxisTestResultsHandler(ctx)) mux.HandleFunc("/api/v1/projects/certification/id/my-image-project-id/images", pyxisImageHandler(ctx)) mux.HandleFunc("/api/v1/projects/certification/id/", pyxisProjectHandler(ctx)) mux.HandleFunc("/api/v1/images/id/updateImage", pyxisImageHandler(ctx)) @@ -101,6 +103,16 @@ var _ = Describe("Pyxis Submit", func() { Expect(certResults).To(BeNil()) }) }) + Context("and POST call to test-results is success and PATCH is a failure", func() { + JustBeforeEach(func() { + pyxisClient.APIToken = "my-bad-results-patch-api-token" + }) + It("should get an unknown error", func() { + certResults, err := pyxisClient.SubmitResults(ctx, &certInput) + Expect(err).To(HaveOccurred()) + Expect(certResults).To(BeNil()) + }) + }) }) Context("when an index.docker.io project is submitted", func() {