Merge branch 'main' into feat/#54-suppport-spec-associatedresources

internal/common/validation/validation.go

@@ -120,21 +120,25 @@ func ValidateResources(resources contentprovider.ResourcesMap) error {
 		}
 
 		if err := ValidateIsValidHTTPSURL(link); err != nil {
-			return err
+			return fmt.Errorf("failed to validate link: %w", err)
 		}
 	}
 
 	return nil
 }
 
 func ValidateIsValidHTTPSURL(input string) error {
+	if input == "" {
+		return fmt.Errorf("%w: must not be empty", commonerrors.ErrInvalidOption)
+	}
+
 	_url, err := url.Parse(input)
 	if err != nil {
-		return fmt.Errorf("%w: link %s is not a valid URL", commonerrors.ErrInvalidOption, input)
+		return fmt.Errorf("%w: '%s' is not a valid URL", commonerrors.ErrInvalidOption, input)
 	}
 
 	if _url.Scheme != "https" {
-		return fmt.Errorf("%w: link %s is not using https scheme", commonerrors.ErrInvalidOption, input)
+		return fmt.Errorf("%w: '%s' is not using https scheme", commonerrors.ErrInvalidOption, input)
 	}
 
 	return nil

internal/service/fileresolver/fileresolver.go

@@ -3,8 +3,6 @@ package fileresolver
 import (
 	"fmt"
 	"net/url"
-	"os"
-	"path/filepath"
 
 	commonerrors "github.com/kyma-project/modulectl/internal/common/errors"
 )
@@ -34,31 +32,17 @@ func NewFileResolver(filePattern string, tempFileSystem TempFileSystem) (*FileRe
 	}, nil
 }
 
-func (r *FileResolver) Resolve(file string) (string, error) {
-	if parsedURL, err := r.ParseURL(file); err == nil {
-		file, err = r.tempFileSystem.DownloadTempFile("", r.filePattern, parsedURL)
-		if err != nil {
-			return "", fmt.Errorf("failed to download file: %w", err)
-		}
-		return file, nil
+func (r *FileResolver) Resolve(fileName string) (string, error) {
+	parsedURL, err := r.ParseURL(fileName)
+	if err != nil {
+		return "", fmt.Errorf("failed to parse URL: %w", err)
 	}
 
-	if !filepath.IsAbs(file) {
-		// Get the current working directory
-		homeDir, err := os.Getwd()
-		if err != nil {
-			return "", fmt.Errorf("failed to get the current directory: %w", err)
-		}
-		// Get the relative path from the current directory
-		file = filepath.Join(homeDir, file)
-		file, err = filepath.Abs(file)
-		if err != nil {
-			return "", fmt.Errorf("failed to obtain absolute path to file: %w", err)
-		}
-		return file, nil
+	tempFilePath, err := r.tempFileSystem.DownloadTempFile("", r.filePattern, parsedURL)
+	if err != nil {
+		return "", fmt.Errorf("failed to download file: %w", err)
 	}
-
-	return file, nil
+	return tempFilePath, nil
 }
 
 func (r *FileResolver) ParseURL(urlString string) (*url.URL, error) {

internal/service/fileresolver/fileresolver_test.go

@@ -51,21 +51,22 @@ func Test_Resolve_Returns_Error_WhenFailingToDownload(t *testing.T) {
 	assert.Empty(t, result)
 }
 
-func Test_Resolve_Returns_CorrectPath_When_AbsolutePath(t *testing.T) {
+func Test_Resolve_Returns_Error_When_AbsolutePath(t *testing.T) {
 	resolver, _ := fileresolver.NewFileResolver(filePattern, &tmpfileSystemStub{})
 	result, err := resolver.Resolve("/path/to/manifest.yaml")
 
-	require.NoError(t, err)
-	assert.Equal(t, "/path/to/manifest.yaml", result)
+	require.Error(t, err)
+	assert.Empty(t, result)
+	assert.Equal(t, "failed to parse URL: failed to parse url /path/to/manifest.yaml: invalid argument", err.Error())
 }
 
-func Test_Resolve_Returns_CorrectPath_When_Relative(t *testing.T) {
+func Test_Resolve_Returns_Error_When_Relative(t *testing.T) {
 	resolver, _ := fileresolver.NewFileResolver(filePattern, &tmpfileSystemStub{})
 	result, err := resolver.Resolve("./path/to/manifest.yaml")
 
-	require.NoError(t, err)
-	assert.Contains(t, result, "/path/to/manifest.yaml")
-	assert.Equal(t, '/', rune(result[0]))
+	require.Error(t, err)
+	assert.Empty(t, result)
+	assert.Equal(t, "failed to parse URL: failed to parse url ./path/to/manifest.yaml: invalid argument", err.Error())
 }
 
 func TestService_ParseURL(t *testing.T) {

internal/service/moduleconfig/reader/moduleconfig_reader.go

@@ -67,8 +67,14 @@ func ValidateModuleConfig(moduleConfig *contentprovider.ModuleConfig) error {
 		return fmt.Errorf("failed to validate resources: %w", err)
 	}
 
-	if moduleConfig.Manifest == "" {
-		return fmt.Errorf("manifest must not be empty: %w", commonerrors.ErrInvalidOption)
+	if err := validation.ValidateIsValidHTTPSURL(moduleConfig.Manifest); err != nil {
+		return fmt.Errorf("failed to validate manifest: %w", err)
+	}
+
+	if moduleConfig.DefaultCR != "" {
+		if err := validation.ValidateIsValidHTTPSURL(moduleConfig.DefaultCR); err != nil {
+			return fmt.Errorf("failed to validate default CR: %w", err)
+		}
 	}
 
 	if err := validateAssociatedResources(moduleConfig.AssociatedResources); err != nil {

internal/service/moduleconfig/reader/moduleconfig_reader_test.go

@@ -32,8 +32,8 @@ func Test_ParseModuleConfig_Returns_CorrectModuleConfig(t *testing.T) {
 	require.Equal(t, "github.com/module-name", result.Name)
 	require.Equal(t, "0.0.1", result.Version)
 	require.Equal(t, "regular", result.Channel)
-	require.Equal(t, "path/to/manifests", result.Manifest)
-	require.Equal(t, "path/to/defaultCR", result.DefaultCR)
+	require.Equal(t, "https://example.com/path/to/manifests", result.Manifest)
+	require.Equal(t, "https://example.com/path/to/defaultCR", result.DefaultCR)
 	require.Equal(t, "module-name-0.0.1", result.ResourceName)
 	require.False(t, result.Mandatory)
 	require.Equal(t, "kcp-system", result.Namespace)
@@ -124,7 +124,7 @@ func Test_ValidateModuleConfig(t *testing.T) {
 				Namespace: "kcp-system",
 				Manifest:  "",
 			},
-			expectedError: fmt.Errorf("manifest must not be empty: %w", commonerrors.ErrInvalidOption),
+			expectedError: fmt.Errorf("failed to validate manifest: %w: must not be empty", commonerrors.ErrInvalidOption),
 		},
 		{
 			name: "invalid module resources - not a URL",
@@ -138,7 +138,7 @@ func Test_ValidateModuleConfig(t *testing.T) {
 					"key": "%% not a URL",
 				},
 			},
-			expectedError: fmt.Errorf("failed to validate resources: %w: link %%%% not a URL is not a valid URL", commonerrors.ErrInvalidOption),
+			expectedError: fmt.Errorf("failed to validate resources: failed to validate link: %w: '%%%% not a URL' is not a valid URL", commonerrors.ErrInvalidOption),
 		},
 		{
 			name: "invalid module resources - empty name",
@@ -168,6 +168,29 @@ func Test_ValidateModuleConfig(t *testing.T) {
 			},
 			expectedError: fmt.Errorf("failed to validate resources: %w: link must not be empty", commonerrors.ErrInvalidOption),
 		},
+		{
+			name: "manifest file path",
+			moduleConfig: &contentprovider.ModuleConfig{
+				Name:      "github.com/module-name",
+				Version:   "0.0.1",
+				Channel:   "regular",
+				Namespace: "kcp-system",
+				Manifest:  "./test",
+			},
+			expectedError: fmt.Errorf("failed to validate manifest: %w: './test' is not using https scheme", commonerrors.ErrInvalidOption),
+		},
+		{
+			name: "default CR file path",
+			moduleConfig: &contentprovider.ModuleConfig{
+				Name:      "github.com/module-name",
+				Version:   "0.0.1",
+				Channel:   "regular",
+				Namespace: "kcp-system",
+				Manifest:  "https://example.com/test",
+				DefaultCR: "/test",
+			},
+			expectedError: fmt.Errorf("failed to validate default CR: %w: '/test' is not using https scheme", commonerrors.ErrInvalidOption),
+		},
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
@@ -291,9 +314,9 @@ var expectedReturnedModuleConfig = contentprovider.ModuleConfig{
 	Name:         "github.com/module-name",
 	Version:      "0.0.1",
 	Channel:      "regular",
-	Manifest:     "path/to/manifests",
+	Manifest:     "https://example.com/path/to/manifests",
 	Mandatory:    false,
-	DefaultCR:    "path/to/defaultCR",
+	DefaultCR:    "https://example.com/path/to/defaultCR",
 	ResourceName: "module-name-0.0.1",
 	Namespace:    "kcp-system",
 	Security:     "path/to/securityConfig",

tests/e2e/create/create_suite_test.go

@@ -30,6 +30,8 @@ const (
 	nonHttpsResource      = invalidConfigs + "non-https-resource.yaml"
 	resourceWithoutLink   = invalidConfigs + "resource-without-link.yaml"
 	resourceWithoutName   = invalidConfigs + "resource-without-name.yaml"
+	manifestFileref       = invalidConfigs + "manifest-fileref.yaml"
+	defaultCRFileref      = invalidConfigs + "defaultcr-fileref.yaml"
 
 	validConfigs                 = testdataDir + "valid/"
 	minimalConfig                = validConfigs + "minimal.yaml"

tests/e2e/create/create_test.go

@@ -89,7 +89,7 @@ var _ = Describe("Test 'create' command", Ordered, func() {
 		It("Then the command should fail", func() {
 			err := cmd.execute()
 			Expect(err).Should(HaveOccurred())
-			Expect(err.Error()).Should(ContainSubstring("failed to parse module config: failed to validate module config: manifest must not be empty: invalid Option"))
+			Expect(err.Error()).Should(ContainSubstring("failed to parse module config: failed to validate module config: failed to validate manifest: invalid Option: must not be empty"))
 		})
 	})
 
@@ -131,7 +131,7 @@ var _ = Describe("Test 'create' command", Ordered, func() {
 		It("Then the command should fail", func() {
 			err := cmd.execute()
 			Expect(err).Should(HaveOccurred())
-			Expect(err.Error()).Should(ContainSubstring("failed to parse module config: failed to validate module config: failed to validate resources: invalid Option: link http://some.other/location/template-operator.yaml is not using https scheme"))
+			Expect(err.Error()).Should(ContainSubstring("failed to parse module config: failed to validate module config: failed to validate resources: failed to validate link: invalid Option: 'http://some.other/location/template-operator.yaml' is not using https scheme"))
 		})
 	})
 
@@ -633,6 +633,34 @@ var _ = Describe("Test 'create' command", Ordered, func() {
 			Expect(template.Spec.Resources[0].Link).To(Equal("https://some.other/location/template-operator.yaml"))
 		})
 	})
+
+	Context("Given 'modulectl create' command", func() {
+		var cmd createCmd
+		It("When invoked with manifest being a fileref", func() {
+			cmd = createCmd{
+				moduleConfigFile: manifestFileref,
+			}
+		})
+		It("Then the command should fail", func() {
+			err := cmd.execute()
+			Expect(err).Should(HaveOccurred())
+			Expect(err.Error()).Should(ContainSubstring("failed to parse module config: failed to validate module config: failed to validate manifest: invalid Option: './template-operator.yaml' is not using https scheme"))
+		})
+	})
+
+	Context("Given 'modulectl create' command", func() {
+		var cmd createCmd
+		It("When invoked with default CR being a fileref", func() {
+			cmd = createCmd{
+				moduleConfigFile: defaultCRFileref,
+			}
+		})
+		It("Then the command should fail", func() {
+			err := cmd.execute()
+			Expect(err).Should(HaveOccurred())
+			Expect(err.Error()).Should(ContainSubstring("failed to parse module config: failed to validate module config: failed to validate default CR: invalid Option: '/tmp/default-sample-cr.yaml' is not using https scheme"))
+		})
+	})
 })
 
 // Test helper functions

tests/e2e/create/testdata/moduleconfig/invalid/defaultcr-fileref.yaml

@@ -0,0 +1,5 @@
+name: kyma-project.io/module/template-operator
+channel: regular
+version: 1.0.0
+manifest: https://github.com/kyma-project/template-operator/releases/download/1.0.1/template-operator.yaml
+defaultCR: /tmp/default-sample-cr.yaml

tests/e2e/create/testdata/moduleconfig/invalid/manifest-fileref.yaml

@@ -0,0 +1,4 @@
+name: kyma-project.io/module/template-operator
+channel: regular
+version: 1.0.0
+manifest: ./template-operator.yaml

unit-test-coverage.yaml

@@ -1,15 +1,17 @@
 packages:
   cmd/modulectl/scaffold: 100
+  cmd/modulectl/create: 100
   internal/common/validation: 86
   internal/service/scaffold: 92
   internal/service/contentprovider: 98
   internal/service/filegenerator: 100
-  internal/service/fileresolver: 92
+  internal/service/filegenerator/reusefilegenerator: 94
+  internal/service/fileresolver: 100
   internal/service/moduleconfig/generator: 100
-  internal/service/moduleconfig/reader: 50
+  internal/service/moduleconfig/reader: 78
   internal/service/create: 43
   internal/service/componentdescriptor: 78
-  internal/service/templategenerator: 78
+  internal/service/templategenerator: 85
   internal/service/crdparser: 73
   internal/service/registry: 52
-  internal/service/componentarchive: 37
+  internal/service/componentarchive: 41