fix(app): fixes lint issues and test cases

Signed-off-by: Mohit Nagaraj <[email protected]>

Author: mohit-nagaraj

CONTRIBUTING.md

@@ -52,11 +52,11 @@ Or you may configure your IDE, for example, Visual Studio Code to automatically
 
 
 #### Tests
-Users can now test their code on their local machine against the CI checks implemented using `make run-tests`.
+Users can now test their code on their local machine against the CI checks implemented using `make test`.
 
 To test code changes on your local machine, run the following command:
 ```
-make run-tests
+make test
 ```
 
 #### Building Docker image

broker/nats/nats.go

@@ -1,6 +1,7 @@
 package nats
 
 import (
+	"encoding/json"
 	"log"
 	"strings"
 	"sync"
@@ -25,8 +26,8 @@ type Options struct {
 
 // Nats will implement Nats subscribe and publish functionality
 type Nats struct {
-	ec *nats.EncodedConn
-	wg *sync.WaitGroup
+  conn *nats.Conn
+  wg   *sync.WaitGroup
 }
 
 // New - constructor
@@ -57,55 +58,66 @@ func New(opts Options) (broker.Handler, error) {
 		return nil, ErrConnect(err)
 	}
 
-	ec, err := nats.NewEncodedConn(nc, nats.JSON_ENCODER)
-	if err != nil {
-		return nil, ErrEncodedConn(err)
-	}
-
-	return &Nats{ec: ec}, nil
+	return &Nats{conn: nc, wg: &sync.WaitGroup{}}, nil
 }
+
 func (n *Nats) ConnectedEndpoints() (endpoints []string) {
-	for _, server := range n.ec.Conn.Servers() {
+	for _, server := range n.conn.Servers() {
 		endpoints = append(endpoints, strings.TrimPrefix(server, "nats://"))
 	}
 	return
 }
 
 func (n *Nats) Info() string {
-	if n.ec == nil || n.ec.Conn == nil {
+	if n.conn == nil {
 		return broker.NotConnected
 	}
-	return n.ec.Conn.Opts.Name
+	return n.conn.Opts.Name
 }
 
 func (n *Nats) CloseConnection() {
-	n.ec.Close()
+    if n.conn != nil {
+        if err := n.conn.Drain(); err != nil {
+            log.Printf("nats: drain error: %v", err)
+        }
+        n.conn.Close()
+    }
 }
 
+
 // Publish - to publish messages
 func (n *Nats) Publish(subject string, message *broker.Message) error {
-	err := n.ec.Publish(subject, message)
+	b, err := json.Marshal(message)
 	if err != nil {
 		return ErrPublish(err)
 	}
-	return nil
+	return n.conn.Publish(subject, b)
 }
 
 // PublishWithChannel - to publish messages with channel
 func (n *Nats) PublishWithChannel(subject string, msgch chan *broker.Message) error {
-	err := n.ec.BindSendChan(subject, msgch)
-	if err != nil {
-		return ErrPublish(err)
-	}
-	return nil
+    go func() {
+        for msg := range msgch {
+            b, err := json.Marshal(msg)
+            if err != nil {
+                log.Printf("nats: JSON marshal error: %v", err)
+                continue
+            }
+            if err := n.conn.Publish(subject, b); err != nil {
+                log.Printf("nats: publish error for subject %s: %v", subject, err)
+            }
+        }
+    }()
+    return nil
 }
 
+
 // Subscribe - for subscribing messages
 // TODO Ques: Do we want to unsubscribe
 // TODO will the method-user just subsribe, how will it handle the received messages?
 func (n *Nats) Subscribe(subject, queue string, message []byte) error {
 	n.wg.Add(1)
-	_, err := n.ec.QueueSubscribe(subject, queue, func(msg *nats.Msg) {
+	_, err := n.conn.QueueSubscribe(subject, queue, func(msg *nats.Msg) {
 		message = msg.Data
 		n.wg.Done()
 	})
@@ -119,7 +131,12 @@ func (n *Nats) Subscribe(subject, queue string, message []byte) error {
 
 // SubscribeWithChannel will publish all the messages received to the given channel
 func (n *Nats) SubscribeWithChannel(subject, queue string, msgch chan *broker.Message) error {
-	_, err := n.ec.BindRecvQueueChan(subject, queue, msgch)
+	_, err := n.conn.QueueSubscribe(subject, queue, func(m *nats.Msg) {
+    	var msg broker.Message
+    	if err := json.Unmarshal(m.Data, &msg); err == nil {
+      		msgch <- &msg
+    	}
+  	})
 	if err != nil {
 		return ErrQueueSubscribe(err)
 	}
@@ -153,8 +170,5 @@ func (in *Nats) DeepCopyObject() broker.Handler {
 // Check if the connection object is empty
 func (in *Nats) IsEmpty() bool {
 	empty := &Nats{}
-	if in == nil || *in == *empty {
-		return true
-	}
-	return false
+	return in == nil || *in == *empty
 }

converter/tests/helm_test.go

@@ -139,7 +139,9 @@ func extractManifestFromChart(chartData []byte) (bool, string) {
 		}
 		if strings.HasSuffix(hdr.Name, "templates/manifest.yaml") {
 			buf := new(bytes.Buffer)
-			io.Copy(buf, tr)
+			if _, err := io.Copy(buf, tr); err != nil {
+    			return false, ""
+			}
 			return true, buf.String()
 		}
 	}

files/error.go

@@ -67,12 +67,12 @@ func ErrUnsupportedExtensionForOperation(operation string, fileName string, file
 	return errors.New(ErrUnsupportedExtensionForOperationCode, errors.Critical, sdescription, ldescription, probableCause, remedy)
 }
 
-func ErrUnsupportedExtension(fileName string, fileExt string, supportedExtensionsMap map[string]bool) error {
-	supportedExtensions := slices.Collect(maps.Keys(supportedExtensionsMap))
+func ErrUnsupportedExtension(fileName string, fileExt string, supportedExtensions []string) error {
+	extList := strings.Join(supportedExtensions, ", ")
 
 	sdescription := []string{
 		fmt.Sprintf("The file '%s' has an unsupported extension: '%s'.", fileName, fileExt),
-		fmt.Sprintf("Supported file extensions are: %s.", strings.Join(supportedExtensions, ", ")),
+		fmt.Sprintf("Supported file extensions are: %s.", extList),
 	}
 
 	ldescription := []string{
@@ -380,7 +380,7 @@ func ErrInvalidModel(operation string, filename string, err error) error {
 		// check prefix as random numeric suffix is appended to archive during file handling (eg: .tar becomes .tar263831)
 		return ErrInvalidModelArchive(filename, err)
 	default:
-		supportedExtensions := slices.Collect(maps.Keys(ValidIacExtensions))
+		supportedExtensions := slices.Clone(ValidIacExtensions)
 		supportedExtensions = slices.DeleteFunc(supportedExtensions, func(ext string) bool {
 			return ext == ".zip"
 		})

files/sanitization.go

@@ -6,6 +6,7 @@ import (
 	"bytes"
 	"compress/gzip"
 	"encoding/json"
+	"slices"
 
 	"fmt"
 	"io"
@@ -25,24 +26,23 @@ type SanitizedFile struct {
 	ExtractedContentPath string
 }
 
-var ValidIacExtensions = map[string]bool{
-	".yml":     true,
-	".yaml":    true,
-	".json":    true,
-	".tar":     true,
-	".tar.gz":  true,
-	".tar.tgz": true,
-	".zip":     true,
-	".gz":      true,
-	".tgz":     true,
+var ValidIacExtensions = []string{
+	".yaml",
+	".tar",
+	".tar.gz",
+	".tgz",
+	".zip",
+	".json",
+	".yml",
 }
 
-func SanitizeFile(data []byte, fileName string, tempDir string, validExts map[string]bool) (SanitizedFile, error) {
+func SanitizeFile(data []byte, fileName string, tempDir string, validExts []string) (SanitizedFile, error) {
 
 	ext := filepath.Ext(fileName)
+	// Handle .gz special case
+	ext2 := filepath.Ext(strings.TrimSuffix(fileName, ".gz"))
 
-	// 1. Check if file has supported  extension
-	if !validExts[ext] && !validExts[filepath.Ext(strings.TrimSuffix(fileName, ".gz"))] {
+	if !slices.Contains(validExts, ext) && !slices.Contains(validExts, ext2) {
 		return SanitizedFile{}, ErrUnsupportedExtension(fileName, ext, validExts)
 	}
 	switch ext {

files/tests/sanitization_test.go

@@ -5,7 +5,6 @@ import (
 	"path/filepath"
 	"testing"
 
-	"github.com/meshery/meshkit/errors"
 	"github.com/meshery/meshkit/files"
 	coreV1 "github.com/meshery/schemas/models/v1alpha1/core"
 )
@@ -16,7 +15,7 @@ func TestSanitizeFile(t *testing.T) {
 		filePath        string
 		expectedExt     string
 		expectError     bool
-		expectedErrCode string
+		expectedErrMsg  string
 		expectedContent map[string]interface{}
 		expectedType    coreV1.IaCFileTypes
 	}{
@@ -43,10 +42,10 @@ func TestSanitizeFile(t *testing.T) {
 			expectedType: "",
 		},
 		{
-			name:            "Unsupported extension",
-			filePath:        "./samples/valid.txt",
-			expectError:     true,
-			expectedErrCode: files.ErrUnsupportedExtensionCode,
+			name:           "Unsupported extension",
+			filePath:       "./samples/valid.txt",
+			expectError:    true,
+			expectedErrMsg: "The file 'valid.txt' could not be processed because the extension '.txt' is not supported by the system..The system is designed to handle files with the following extensions: .yaml, .tar, .tar.gz, .tgz, .zip, .json, .yml.",
 		},
 		{
 			name:        "Valid compressed extension",
@@ -86,14 +85,14 @@ func TestSanitizeFile(t *testing.T) {
 			name:         "Can Identify Kubernetes Manifest",
 			filePath:     "./samples/valid_manifest.yml",
 			expectedExt:  ".yml",
-expectedType: coreV1.K8sManifest,
+			expectedType: coreV1.K8sManifest,
 		},
 
 		{
 			name:         "Can Identify Kubernetes Manifest With Crds",
 			filePath:     "./samples/manifest-with-crds.yml",
 			expectedExt:  ".yml",
-expectedType: coreV1.K8sManifest,
+			expectedType: coreV1.K8sManifest,
 		},
 
 		{
@@ -136,18 +135,20 @@ expectedType: coreV1.K8sManifest,
 		// },
 	}
 
-	validExts := map[string]bool{
-		".json":   true,
-		".yml":    true,
-		".yaml":   true,
-		".tar":    true,
-		".tar.gz": true,
-		".tgz":    true,
-		".zip":    true,
+	validExts := []string{
+		".yaml",
+		".tar",
+		".tar.gz",
+		".tgz",
+		".zip",
+		".json",
+		".yml",
 	}
 
-	tempDir, _ := os.MkdirTemp(" ", "temp-tests")
+	tempDir, _ := os.MkdirTemp("", "temp-tests")
+	os.Setenv("MESHERY_CONTENT_PATH", tempDir)
 	defer os.RemoveAll(tempDir)
+	defer os.Unsetenv("MESHERY_CONTENT_PATH")
 	// tempDir := "./temp"
 
 	for _, tc := range testCases {
@@ -166,8 +167,8 @@ expectedType: coreV1.K8sManifest,
 			if tc.expectError {
 				if err == nil {
 					t.Error("Expected error, got nil")
-				} else if tc.expectedErrCode != "" && errors.GetCode(err) != tc.expectedErrCode {
-					t.Errorf("Expected error message %q, got %q", tc.expectedErrCode, err.Error())
+				} else if tc.expectedErrMsg != "" && err.Error() != tc.expectedErrMsg {
+					t.Errorf("Expected error message %q, got %q", tc.expectedErrMsg, err.Error())
 				}
 				return
 			}

generators/artifacthub/package.go

@@ -49,7 +49,6 @@ func (pkg AhPackage) GetName() string {
 func (pkg AhPackage) GenerateComponents(group string) ([]_component.ComponentDefinition, error) {
 	components := make([]_component.ComponentDefinition, 0)
 	// TODO: Move this to the configuration
-
 	if pkg.ChartUrl == "" {
 		return components, ErrChartUrlEmpty(pkg.Name, "ArtifactHub")
 	}
@@ -107,15 +106,29 @@ func (pkg *AhPackage) UpdatePackageData() error {
 	if pkgEntry == nil || !ok {
 		return ErrGetChartUrl(fmt.Errorf("Cannot extract chartUrl from repository helm index"))
 	}
-	urls, ok := pkgEntry.([]interface{})[0].(map[interface{}]interface{})["urls"]
-	if urls == nil || !ok {
+	urlField, ok := pkgEntry.([]interface{})[0].(map[interface{}]interface{})["urls"]
+	if urlField == nil || !ok {
 		return ErrGetChartUrl(fmt.Errorf("Cannot extract chartUrl from repository helm index"))
 	}
-	chartUrl, ok := urls.([]interface{})[0].(string)
-	if !ok || chartUrl == "" {
+	var chartUrl string
+	for i, u := range urlField.([]interface{}) {
+		s, _ := u.(string)
+		if s == "" {
+			continue
+		}
+		if i == 0 {
+			chartUrl = s
+		}
+		if strings.HasPrefix(s, "http") {
+			chartUrl = s
+			break
+		}
+	}
+	if chartUrl == "" {
 		return ErrGetChartUrl(fmt.Errorf("Cannot extract chartUrl from repository helm index"))
 	}
-	if !strings.HasPrefix(chartUrl, "http") {
+
+	if !strings.HasPrefix(chartUrl, "http") && !strings.HasPrefix(chartUrl, "oci://") {
 		if !strings.HasSuffix(pkg.RepoUrl, "/") {
 			pkg.RepoUrl = pkg.RepoUrl + "/"
 		}

generators/artifacthub/package_manager.go

@@ -22,7 +22,9 @@ func (ahpm ArtifactHubPackageManager) GetPackage() (models.Package, error) {
 	}
 	// update package information
 	for i, ap := range pkgs {
-		_ = ap.UpdatePackageData()
+		if err := ap.UpdatePackageData(); err != nil {
+			return nil, err
+		}
 		pkgs[i] = ap
 	}
 

generators/artifacthub/package_test.go

@@ -17,7 +17,7 @@ func TestGetChartUrl(t *testing.T) {
 	}{
 		// these might change in the future, so the tests have to be changed as well when the urls change
 		// because the urls will change with every new version update to the package
-		{AhPackage{Name: "consul", Repository: "bitnami", Organization: "", RepoUrl: "https://charts.bitnami.com/bitnami"}, "https://charts.bitnami.com/bitnami/consul"},
+		{AhPackage{Name: "consul", Repository: "bitnami", Organization: "", RepoUrl: "https://charts.bitnami.com/bitnami"}, "oci://registry-1.docker.io/bitnamicharts/consul"},
 		{AhPackage{Name: "crossplane-types", Repository: "crossplane", Organization: "", RepoUrl: "https://charts.crossplane.io/master"}, "https://charts.crossplane.io/master/crossplane-types-0.13.0-rc.191.g3a18fb7.tgz"},
 	}
 	for _, tt := range tests {

generators/github/package_test.go

@@ -41,7 +41,7 @@ func TestGenerateCompFromGitHub(t *testing.T) {
 		{ // Source pointing to a directly downloadable file (not a repo per se)
 			ghPackageManager: GitHubPackageManager{
 				PackageName: "k8s-config-connector",
-				SourceURL:   "https://raw.githubusercontent.com/GoogleCloudPlatform/k8s-config-connector/master/crds/alloydb_v1beta1_alloydbbackup.yaml/1.113.0",
+				SourceURL:   "https://raw.githubusercontent.com/GoogleCloudPlatform/k8s-config-connector/master/crds/alloydb_v1beta1_alloydbbackup.yaml",
 			},
 			want: 1,
 		},
@@ -56,7 +56,7 @@ func TestGenerateCompFromGitHub(t *testing.T) {
 		{ // Source pointing to a zip containing manifests but no CRDs
 			ghPackageManager: GitHubPackageManager{
 				PackageName: "acm-controller",
-				SourceURL:   "https://github.com/MUzairS15/WASM-filters/raw/main/test.tar.gz/v0.7.12",
+				SourceURL:   "https://github.com/MUzairS15/WASM-filters/raw/main/test.tar.gz",
 			},
 			want: 0,
 		},