From c638e1c14c6fd0eeb1e2d502ec2d3907046ecc25 Mon Sep 17 00:00:00 2001 From: Nilekh Chaudhari <1626598+nilekhc@users.noreply.github.com> Date: Fri, 12 May 2023 15:05:15 -0700 Subject: [PATCH] fix: fixes version check for Artifact Hub generation (#339) Signed-off-by: Nilekh Chaudhari <1626598+nilekhc@users.noreply.github.com> --- scripts/artifacthub/hub.go | 56 +++++++++++++++++--- scripts/artifacthub/hub_test.go | 94 ++++++++++++++++++++++++++------- 2 files changed, 122 insertions(+), 28 deletions(-) diff --git a/scripts/artifacthub/hub.go b/scripts/artifacthub/hub.go index d74f0d412..c7e0140c9 100644 --- a/scripts/artifacthub/hub.go +++ b/scripts/artifacthub/hub.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "io/fs" + "net/http" "os" "path" "path/filepath" @@ -72,6 +73,19 @@ type ArtifactHubMetadata struct { } `yaml:"annotations,omitempty"` } +// HTTPClient is an interface that matches the methods needed from http.Client. +type HTTPClient interface { + Get(url string) (*http.Response, error) +} + +// DefaultClient is a default implementation of the HTTPClient interface. +type DefaultClient struct{} + +// Get is a method of DefaultClient that makes the actual HTTP GET request. +func (c DefaultClient) Get(url string) (*http.Response, error) { + return http.Get(url) +} + const ( // entryPoint is the directory entry point for artifact hub ahEntryPoint = "artifacthub" @@ -123,9 +137,11 @@ func main() { panic(err) } + githubSourceRelativePath := filepath.Join(entryPoint, entry.Name(), dir.Name(), "template.yaml") createVersionDirectory( rootDir, filepath.Join(entryPoint, entry.Name(), dir.Name()), + githubSourceRelativePath, constraintTemplate, ) } @@ -134,7 +150,7 @@ func main() { } } -func createVersionDirectory(rootDir, basePath string, constraintTemplate map[string]interface{}) { +func createVersionDirectory(rootDir, basePath, githubSourceRelativePath string, constraintTemplate map[string]interface{}) { version := fmt.Sprintf("%s", constraintTemplate["metadata"].(map[string]interface{})["annotations"].(map[string]interface{})["metadata.gatekeeper.sh/version"]) // create directory if not exists @@ -152,7 +168,7 @@ func createVersionDirectory(rootDir, basePath string, constraintTemplate map[str // create artifacthub-pkg.yml file first then copy rest of the files. This will avoid unnecessary diff if there is any error while generating or updating artifacthub-pkg.yml // add artifact hub metadata - addArtifactHubMetadata(filepath.Base(source), destination, ahBasePath, constraintTemplate) + addArtifactHubMetadata(filepath.Base(source), destination, ahBasePath, githubSourceRelativePath, constraintTemplate) // copy directory content err := copyDirectory(source, destination) @@ -162,7 +178,7 @@ func createVersionDirectory(rootDir, basePath string, constraintTemplate map[str } } -func addArtifactHubMetadata(sourceDirectory, destinationPath, ahBasePath string, constraintTemplate map[string]interface{}) { +func addArtifactHubMetadata(sourceDirectory, destinationPath, ahBasePath, githubSourceRelativePath string, constraintTemplate map[string]interface{}) { metadataFilePath := filepath.Join(destinationPath, "artifacthub-pkg.yml") constraintTemplateHash := getConstraintTemplateHash(constraintTemplate) @@ -199,7 +215,7 @@ func addArtifactHubMetadata(sourceDirectory, destinationPath, ahBasePath string, } } else { // when metadata file already exists, check version to make sure it's updated if constraint template is changed - err := checkVersion(artifactHubMetadata, constraintTemplate, constraintTemplateHash) + err := checkVersion(&DefaultClient{}, artifactHubMetadata, githubSourceRelativePath) if err != nil { panic(err) } @@ -221,11 +237,35 @@ func addArtifactHubMetadata(sourceDirectory, destinationPath, ahBasePath string, } } -func checkVersion(artifactHubMetadata *ArtifactHubMetadata, constraintTemplate map[string]interface{}, newConstraintTemplateHash string) error { - // compare hash - if artifactHubMetadata.Digest != newConstraintTemplateHash { +func checkVersion(httpClient HTTPClient, artifactHubMetadata *ArtifactHubMetadata, githubSourceRelativePath string) error { + // compare hash with template.yaml in github + githubTemplateURL := sourceURL + githubSourceRelativePath + resp, err := httpClient.Get(githubTemplateURL) + if err != nil { + return fmt.Errorf("error while getting constraint template from github: %v", err) + } + if resp.StatusCode == http.StatusNotFound { + fmt.Printf("constraint template %s not found in github. It is likely that constraint template is being updated locally and not merged to github yet.\n", githubSourceRelativePath) + + return nil + } + defer resp.Body.Close() + + githubConstraintTemplateBytes, err := io.ReadAll(resp.Body) + if err != nil { + return fmt.Errorf("error while reading constraint template from github") + } + + githubConstraintTemplate := make(map[string]interface{}) + err = yaml.Unmarshal(githubConstraintTemplateBytes, &githubConstraintTemplate) + if err != nil { + return fmt.Errorf("error while unmarshaling constraint template from github") + } + + githubConstraintTemplateHash := getConstraintTemplateHash(githubConstraintTemplate) + if artifactHubMetadata.Digest != githubConstraintTemplateHash { // compare version - if artifactHubMetadata.Version == constraintTemplate["metadata"].(map[string]interface{})["annotations"].(map[string]interface{})["metadata.gatekeeper.sh/version"].(string) { + if artifactHubMetadata.Version == githubConstraintTemplate["metadata"].(map[string]interface{})["annotations"].(map[string]interface{})["metadata.gatekeeper.sh/version"].(string) { // panic if version is same but hash is different return fmt.Errorf("looks like template.yaml is updated but the version is not. Please update the 'metadata.gatekeeper.sh/version' annotation in the template.yaml source") } diff --git a/scripts/artifacthub/hub_test.go b/scripts/artifacthub/hub_test.go index 470ed04e1..d080c6157 100644 --- a/scripts/artifacthub/hub_test.go +++ b/scripts/artifacthub/hub_test.go @@ -1,8 +1,14 @@ package main import ( + "errors" + "io" + "net/http" "os" + "strings" "testing" + + "gopkg.in/yaml.v3" ) const ( @@ -159,13 +165,25 @@ func TestCopyDirectory(t *testing.T) { } } +// MockClient is a mock implementation of the HTTPClient interface for testing. +type MockClient struct { + Resp *http.Response + Err error +} + +// Get is a method of MockClient that returns the pre-configured response and error. +func (c MockClient) Get(url string) (*http.Response, error) { + return c.Resp, c.Err +} + func TestCheckVersion(t *testing.T) { testCases := []struct { - name string - artifactHubMetadata *ArtifactHubMetadata - constraintTemplate map[string]interface{} - newConstraintTemplateHash string - expectedError bool + name string + artifactHubMetadata *ArtifactHubMetadata + httpStatus int + httpError error + githubConstraintTemplate map[string]interface{} + expectedErrorMessage string }{ { name: "invalid version", @@ -173,47 +191,83 @@ func TestCheckVersion(t *testing.T) { Digest: "invalid-digest", Version: "v1.0.0", }, - newConstraintTemplateHash: "some-random-hash", - constraintTemplate: map[string]interface{}{ + httpStatus: http.StatusOK, + githubConstraintTemplate: map[string]interface{}{ "metadata": map[string]interface{}{ "annotations": map[string]interface{}{ "metadata.gatekeeper.sh/version": "v1.0.0", }, }, }, - expectedError: true, + expectedErrorMessage: "looks like template.yaml is updated but the version is not. Please update the 'metadata.gatekeeper.sh/version' annotation in the template.yaml source", }, { name: "valid version with same digest", artifactHubMetadata: &ArtifactHubMetadata{ - Digest: "same-digest", + Digest: "600ca4d7048b1b64a5d80aaabf015eceef5d613c2f5a0a3d31e5360535d3c6e8", + }, + httpStatus: http.StatusOK, + githubConstraintTemplate: map[string]interface{}{ + "metadata": map[string]interface{}{ + "annotations": map[string]interface{}{ + "metadata.gatekeeper.sh/version": "v1.0.0", + }, + }, }, - newConstraintTemplateHash: "same-digest", - expectedError: false, }, { - name: "valid version with different digest", + name: "valid version bump with different digest", artifactHubMetadata: &ArtifactHubMetadata{ Digest: "digest", - Version: "v1.0.0", + Version: "v1.0.1", }, - newConstraintTemplateHash: "different-digest", - constraintTemplate: map[string]interface{}{ + httpStatus: http.StatusOK, + githubConstraintTemplate: map[string]interface{}{ "metadata": map[string]interface{}{ "annotations": map[string]interface{}{ - "metadata.gatekeeper.sh/version": "v1.0.1", + "metadata.gatekeeper.sh/version": "v1.0.0", }, }, }, - expectedError: false, + }, + { + name: "template not found", + httpStatus: http.StatusNotFound, + }, + { + name: "get template error", + httpStatus: http.StatusOK, + httpError: errors.New("fake error"), + expectedErrorMessage: "error while getting constraint template from github: fake error", }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - err := checkVersion(tc.artifactHubMetadata, tc.constraintTemplate, tc.newConstraintTemplateHash) - if err != nil && !tc.expectedError { - t.Errorf("expected error to be nil") + githubConstraintTemplateBytes, err := yaml.Marshal(tc.githubConstraintTemplate) + + // Create a mock client with a pre-configured response and error. + mockResp := &http.Response{ + StatusCode: tc.httpStatus, + Body: io.NopCloser(strings.NewReader(string(githubConstraintTemplateBytes))), + } + mockClient := MockClient{ + Resp: mockResp, + Err: tc.httpError, + } + + err = checkVersion(mockClient, tc.artifactHubMetadata, "path/to/constraint/template.yaml") + + if tc.expectedErrorMessage != "" { + if err == nil { + t.Errorf("Expected error '%s', but got no error", tc.expectedErrorMessage) + } else if err.Error() != tc.expectedErrorMessage { + t.Errorf("Expected error '%s', but got '%s'", tc.expectedErrorMessage, err.Error()) + } + } else { + if err != nil { + t.Errorf("Expected no error, but got '%s'", err.Error()) + } } }) }