diff --git a/internal/common/validation/validation.go b/internal/common/validation/validation.go index 7956d28f..99d37b68 100644 --- a/internal/common/validation/validation.go +++ b/internal/common/validation/validation.go @@ -120,7 +120,7 @@ func ValidateResources(resources contentprovider.ResourcesMap) error { } if err := ValidateIsValidHTTPSURL(link); err != nil { - return err + return fmt.Errorf("failed to validate link: %w", err) } } @@ -128,13 +128,17 @@ func ValidateResources(resources contentprovider.ResourcesMap) error { } 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 diff --git a/internal/service/fileresolver/fileresolver.go b/internal/service/fileresolver/fileresolver.go index e0c2cbcb..ff3263a7 100644 --- a/internal/service/fileresolver/fileresolver.go +++ b/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) { diff --git a/internal/service/fileresolver/fileresolver_test.go b/internal/service/fileresolver/fileresolver_test.go index 794fe29a..613f6c44 100644 --- a/internal/service/fileresolver/fileresolver_test.go +++ b/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) { diff --git a/internal/service/moduleconfig/reader/moduleconfig_reader.go b/internal/service/moduleconfig/reader/moduleconfig_reader.go index 591dab96..6f9f45f7 100644 --- a/internal/service/moduleconfig/reader/moduleconfig_reader.go +++ b/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 { diff --git a/internal/service/moduleconfig/reader/moduleconfig_reader_test.go b/internal/service/moduleconfig/reader/moduleconfig_reader_test.go index e602d0f2..a2d15e87 100644 --- a/internal/service/moduleconfig/reader/moduleconfig_reader_test.go +++ b/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", diff --git a/tests/e2e/create/create_suite_test.go b/tests/e2e/create/create_suite_test.go index 9b169f89..7c5226e8 100644 --- a/tests/e2e/create/create_suite_test.go +++ b/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" diff --git a/tests/e2e/create/create_test.go b/tests/e2e/create/create_test.go index 77708571..74e888dc 100644 --- a/tests/e2e/create/create_test.go +++ b/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 diff --git a/tests/e2e/create/testdata/moduleconfig/invalid/defaultcr-fileref.yaml b/tests/e2e/create/testdata/moduleconfig/invalid/defaultcr-fileref.yaml new file mode 100644 index 00000000..d003571a --- /dev/null +++ b/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 diff --git a/tests/e2e/create/testdata/moduleconfig/invalid/manifest-fileref.yaml b/tests/e2e/create/testdata/moduleconfig/invalid/manifest-fileref.yaml new file mode 100644 index 00000000..98d90703 --- /dev/null +++ b/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 diff --git a/unit-test-coverage.yaml b/unit-test-coverage.yaml index e791aeab..933014e3 100644 --- a/unit-test-coverage.yaml +++ b/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