From 46c159d3ff3578c501b02ee70e38c753d7915f30 Mon Sep 17 00:00:00 2001 From: Nesma Badr Date: Tue, 29 Oct 2024 16:19:51 +0100 Subject: [PATCH] chore: Remove ResourceName (#87) Remove ResourceName --- cmd/modulectl/create/long.txt | 1 - docs/gen-docs/modulectl_create.md | 1 - .../service/contentprovider/moduleconfig.go | 1 - .../generator/moduleconfig_generator_test.go | 26 ++++---- .../reader/moduleconfig_reader_test.go | 50 ++++++++------- .../templategenerator/templategenerator.go | 6 +- .../templategenerator_test.go | 64 +++++++++---------- scaffold-create-config.yaml | 1 - tests/e2e/create/create_suite_test.go | 1 - tests/e2e/create/create_test.go | 14 ---- .../invalid/empty-resource-name.yaml | 7 -- tests/e2e/scaffold/scaffold_suite_test.go | 1 - 12 files changed, 74 insertions(+), 99 deletions(-) delete mode 100644 tests/e2e/create/testdata/moduleconfig/invalid/empty-resource-name.yaml diff --git a/cmd/modulectl/create/long.txt b/cmd/modulectl/create/long.txt index f2f70774..4f0dbf91 100644 --- a/cmd/modulectl/create/long.txt +++ b/cmd/modulectl/create/long.txt @@ -18,7 +18,6 @@ The module config file is a YAML file used to configure the following attributes - defaultCR: a string, optional, reference to a YAML file containing the default CR for the module, must be a URL - mandatory: a boolean, optional, default=false, indicates whether the module is mandatory to be installed on all clusters -- resourceName: a string, optional, default={NAME}-{CHANNEL}, the name for the ModuleTemplate CR that will be created - internal: a boolean, optional, default=false, determines whether the ModuleTemplate CR should have the internal flag or not - beta: a boolean, optional, default=false, determines whether the ModuleTemplate CR should have the beta flag or not - security: a string, optional, name of the security scanners config file diff --git a/docs/gen-docs/modulectl_create.md b/docs/gen-docs/modulectl_create.md index cb0b429c..a02b2573 100644 --- a/docs/gen-docs/modulectl_create.md +++ b/docs/gen-docs/modulectl_create.md @@ -26,7 +26,6 @@ The module config file is a YAML file used to configure the following attributes - defaultCR: a string, optional, reference to a YAML file containing the default CR for the module, must be a URL - mandatory: a boolean, optional, default=false, indicates whether the module is mandatory to be installed on all clusters -- resourceName: a string, optional, default={NAME}-{CHANNEL}, the name for the ModuleTemplate CR that will be created - internal: a boolean, optional, default=false, determines whether the ModuleTemplate CR should have the internal flag or not - beta: a boolean, optional, default=false, determines whether the ModuleTemplate CR should have the beta flag or not - security: a string, optional, name of the security scanners config file diff --git a/internal/service/contentprovider/moduleconfig.go b/internal/service/contentprovider/moduleconfig.go index 31137255..a3c50bbd 100644 --- a/internal/service/contentprovider/moduleconfig.go +++ b/internal/service/contentprovider/moduleconfig.go @@ -86,7 +86,6 @@ type ModuleConfig struct { Manifest string `yaml:"manifest" comment:"required, relative path or remote URL to the manifests"` Mandatory bool `yaml:"mandatory" comment:"optional, default=false, indicates whether the module is mandatory to be installed on all clusters"` DefaultCR string `yaml:"defaultCR" comment:"optional, relative path or remote URL to a YAML file containing the default CR for the module"` - ResourceName string `yaml:"resourceName" comment:"optional, default={name}-{channel}, when channel is 'none', the default is {name}-{version}, the name for the ModuleTemplate that will be created"` Namespace string `yaml:"namespace" comment:"optional, default=kcp-system, the namespace where the ModuleTemplate will be deployed"` Security string `yaml:"security" comment:"optional, name of the security scanners config file"` Internal bool `yaml:"internal" comment:"optional, default=false, determines whether the ModuleTemplate should have the internal flag or not"` diff --git a/internal/service/moduleconfig/generator/moduleconfig_generator_test.go b/internal/service/moduleconfig/generator/moduleconfig_generator_test.go index bbfc056a..b91e1e23 100644 --- a/internal/service/moduleconfig/generator/moduleconfig_generator_test.go +++ b/internal/service/moduleconfig/generator/moduleconfig_generator_test.go @@ -118,19 +118,18 @@ func (*fileExistsStub) FileExists(_ string) (bool, error) { func (*fileExistsStub) ReadFile(_ string) ([]byte, error) { moduleConfig := contentprovider.ModuleConfig{ - Name: "module-name", - Version: "0.0.1", - Channel: "regular", - Manifest: "path/to/manifests", - Mandatory: false, - DefaultCR: "path/to/defaultCR", - ResourceName: "module-name-0.0.1", - Namespace: "kcp-system", - Security: "path/to/securityConfig", - Internal: false, - Beta: false, - Labels: map[string]string{"label1": "value1"}, - Annotations: map[string]string{"annotation1": "value1"}, + Name: "module-name", + Version: "0.0.1", + Channel: "regular", + Manifest: "path/to/manifests", + Mandatory: false, + DefaultCR: "path/to/defaultCR", + Namespace: "kcp-system", + Security: "path/to/securityConfig", + Internal: false, + Beta: false, + Labels: map[string]string{"label1": "value1"}, + Annotations: map[string]string{"annotation1": "value1"}, AssociatedResources: []*metav1.GroupVersionKind{ { Group: "networking.istio.io", @@ -147,6 +146,7 @@ func (*fileExistsStub) ReadFile(_ string) ([]byte, error) { Kind: "Deployment", }, }, + Resources: contentprovider.ResourcesMap{}, } return yaml.Marshal(moduleConfig) diff --git a/internal/service/moduleconfig/reader/moduleconfig_reader_test.go b/internal/service/moduleconfig/reader/moduleconfig_reader_test.go index 8e60a6f4..a9894d27 100644 --- a/internal/service/moduleconfig/reader/moduleconfig_reader_test.go +++ b/internal/service/moduleconfig/reader/moduleconfig_reader_test.go @@ -34,7 +34,6 @@ func Test_ParseModuleConfig_Returns_CorrectModuleConfig(t *testing.T) { require.Equal(t, "regular", result.Channel) 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) require.Equal(t, "path/to/securityConfig", result.Security) @@ -124,7 +123,8 @@ func Test_ValidateModuleConfig(t *testing.T) { Namespace: "kcp-system", Manifest: "", }, - expectedError: fmt.Errorf("failed to validate manifest: %w: must not be empty", 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,8 @@ func Test_ValidateModuleConfig(t *testing.T) { "key": "%% not a URL", }, }, - expectedError: fmt.Errorf("failed to validate resources: failed to validate link: %w: '%%%% 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", @@ -152,7 +153,8 @@ func Test_ValidateModuleConfig(t *testing.T) { "": "https://github.com/kyma-project/template-operator/releases/download/1.0.1/template-operator.yaml", }, }, - expectedError: fmt.Errorf("failed to validate resources: %w: name must not be empty", commonerrors.ErrInvalidOption), + expectedError: fmt.Errorf("failed to validate resources: %w: name must not be empty", + commonerrors.ErrInvalidOption), }, { name: "invalid module resources - empty link", @@ -166,7 +168,8 @@ func Test_ValidateModuleConfig(t *testing.T) { "name": "", }, }, - expectedError: fmt.Errorf("failed to validate resources: %w: link must not be empty", commonerrors.ErrInvalidOption), + expectedError: fmt.Errorf("failed to validate resources: %w: link must not be empty", + commonerrors.ErrInvalidOption), }, { name: "manifest file path", @@ -177,7 +180,8 @@ func Test_ValidateModuleConfig(t *testing.T) { Namespace: "kcp-system", Manifest: "./test", }, - expectedError: fmt.Errorf("failed to validate manifest: %w: './test' is not using https scheme", commonerrors.ErrInvalidOption), + expectedError: fmt.Errorf("failed to validate manifest: %w: './test' is not using https scheme", + commonerrors.ErrInvalidOption), }, { name: "default CR file path", @@ -189,7 +193,8 @@ func Test_ValidateModuleConfig(t *testing.T) { Manifest: "https://example.com/test", DefaultCR: "/test", }, - expectedError: fmt.Errorf("failed to validate default CR: %w: '/test' is not using https scheme", commonerrors.ErrInvalidOption), + expectedError: fmt.Errorf("failed to validate default CR: %w: '/test' is not using https scheme", + commonerrors.ErrInvalidOption), }, } for _, test := range tests { @@ -363,19 +368,18 @@ func (*fileExistsStub) FileExists(_ string) (bool, error) { } var expectedReturnedModuleConfig = contentprovider.ModuleConfig{ - Name: "github.com/module-name", - Version: "0.0.1", - Channel: "regular", - Manifest: "https://example.com/path/to/manifests", - Mandatory: false, - DefaultCR: "https://example.com/path/to/defaultCR", - ResourceName: "module-name-0.0.1", - Namespace: "kcp-system", - Security: "path/to/securityConfig", - Internal: false, - Beta: false, - Labels: map[string]string{"label1": "value1"}, - Annotations: map[string]string{"annotation1": "value1"}, + Name: "github.com/module-name", + Version: "0.0.1", + Channel: "regular", + Manifest: "https://example.com/path/to/manifests", + Mandatory: false, + DefaultCR: "https://example.com/path/to/defaultCR", + Namespace: "kcp-system", + Security: "path/to/securityConfig", + Internal: false, + Beta: false, + Labels: map[string]string{"label1": "value1"}, + Annotations: map[string]string{"annotation1": "value1"}, AssociatedResources: []*metav1.GroupVersionKind{ { Group: "networking.istio.io", @@ -383,9 +387,6 @@ var expectedReturnedModuleConfig = contentprovider.ModuleConfig{ Kind: "Gateway", }, }, - Resources: contentprovider.ResourcesMap{ - "rawManifest": "https://github.com/kyma-project/template-operator/releases/download/1.0.1/template-operator.yaml", - }, Manager: &contentprovider.Manager{ Name: "manager-name", Namespace: "manager-namespace", @@ -395,6 +396,9 @@ var expectedReturnedModuleConfig = contentprovider.ModuleConfig{ Kind: "Deployment", }, }, + Resources: contentprovider.ResourcesMap{ + "rawManifest": "https://github.com/kyma-project/template-operator/releases/download/1.0.1/template-operator.yaml", + }, } func (*fileExistsStub) ReadFile(_ string) ([]byte, error) { diff --git a/internal/service/templategenerator/templategenerator.go b/internal/service/templategenerator/templategenerator.go index b85510b1..4161de00 100644 --- a/internal/service/templategenerator/templategenerator.go +++ b/internal/service/templategenerator/templategenerator.go @@ -132,9 +132,7 @@ func (s *Service) GenerateModuleTemplate( } shortName := trimShortNameFromRef(ref) labels[shared.ModuleName] = shortName - if moduleConfig.ResourceName == "" { - moduleConfig.ResourceName = shortName + "-" + moduleConfig.Version - } + moduleTemplateName := shortName + "-" + moduleConfig.Version moduleTemplate, err := template.New("moduleTemplate").Funcs(template.FuncMap{ "yaml": yaml.Marshal, @@ -150,7 +148,7 @@ func (s *Service) GenerateModuleTemplate( } mtData := moduleTemplateData{ - ResourceName: moduleConfig.ResourceName, + ResourceName: moduleTemplateName, Namespace: moduleConfig.Namespace, Descriptor: cva, Channel: moduleConfig.Channel, diff --git a/internal/service/templategenerator/templategenerator_test.go b/internal/service/templategenerator/templategenerator_test.go index 04c6afa7..8702cd7b 100644 --- a/internal/service/templategenerator/templategenerator_test.go +++ b/internal/service/templategenerator/templategenerator_test.go @@ -43,14 +43,14 @@ func TestGenerateModuleTemplate_Success(t *testing.T) { svc, _ := templategenerator.NewService(mockFS) moduleConfig := &contentprovider.ModuleConfig{ - ResourceName: "test-resource", - Namespace: "default", - Channel: "stable", - Labels: map[string]string{"key": "value"}, - Annotations: map[string]string{"annotation": "value"}, - Mandatory: true, - Manifest: "https://github.com/kyma-project/template-operator/releases/download/1.0.1/template-operator.yaml", - Resources: contentprovider.ResourcesMap{"someResource": "https://some.other/location/template-operator.yaml"}, + Namespace: "default", + Version: "1.0.0", + Channel: "stable", + Labels: map[string]string{"key": "value"}, + Annotations: map[string]string{"annotation": "value"}, + Mandatory: true, + Manifest: "https://github.com/kyma-project/template-operator/releases/download/1.0.1/template-operator.yaml", + Resources: contentprovider.ResourcesMap{"someResource": "https://some.other/location/template-operator.yaml"}, } descriptor := testutils.CreateComponentDescriptor("example.com/component", "1.0.0") data := []byte("test-data") @@ -59,7 +59,7 @@ func TestGenerateModuleTemplate_Success(t *testing.T) { require.NoError(t, err) require.Equal(t, "output.yaml", mockFS.path) - require.Contains(t, mockFS.writtenTemplate, "test-resource") + require.Contains(t, mockFS.writtenTemplate, "component-1.0.0") require.Contains(t, mockFS.writtenTemplate, "default") require.Contains(t, mockFS.writtenTemplate, "stable") require.Contains(t, mockFS.writtenTemplate, "test-data") @@ -67,7 +67,8 @@ func TestGenerateModuleTemplate_Success(t *testing.T) { require.Contains(t, mockFS.writtenTemplate, "someResource") require.Contains(t, mockFS.writtenTemplate, "https://some.other/location/template-operator.yaml") require.Contains(t, mockFS.writtenTemplate, "rawManifest") - require.Contains(t, mockFS.writtenTemplate, "https://github.com/kyma-project/template-operator/releases/download/1.0.1/template-operator.yaml") + require.Contains(t, mockFS.writtenTemplate, + "https://github.com/kyma-project/template-operator/releases/download/1.0.1/template-operator.yaml") } func TestGenerateModuleTemplate_Success_With_Overwritten_RawManifest(t *testing.T) { @@ -87,7 +88,8 @@ func TestGenerateModuleTemplate_Success_With_Overwritten_RawManifest(t *testing. require.Equal(t, "output.yaml", mockFS.path) require.Contains(t, mockFS.writtenTemplate, "rawManifest") require.Contains(t, mockFS.writtenTemplate, "https://some.other/location/template-operator.yaml") - require.NotContains(t, mockFS.writtenTemplate, "https://github.com/kyma-project/template-operator/releases/download/1.0.1/template-operator.yaml") + require.NotContains(t, mockFS.writtenTemplate, + "https://github.com/kyma-project/template-operator/releases/download/1.0.1/template-operator.yaml") } func TestGenerateModuleTemplateWithAssociatedResources_Success(t *testing.T) { @@ -95,12 +97,11 @@ func TestGenerateModuleTemplateWithAssociatedResources_Success(t *testing.T) { svc, _ := templategenerator.NewService(mockFS) moduleConfig := &contentprovider.ModuleConfig{ - ResourceName: "test-resource", - Namespace: "default", - Channel: "stable", - Labels: map[string]string{"key": "value"}, - Annotations: map[string]string{"annotation": "value"}, - Mandatory: true, + Namespace: "default", + Channel: "stable", + Labels: map[string]string{"key": "value"}, + Annotations: map[string]string{"annotation": "value"}, + Mandatory: true, AssociatedResources: []*metav1.GroupVersionKind{ { Group: "networking.istio.io", @@ -116,7 +117,6 @@ func TestGenerateModuleTemplateWithAssociatedResources_Success(t *testing.T) { require.NoError(t, err) require.Equal(t, "output.yaml", mockFS.path) - require.Contains(t, mockFS.writtenTemplate, "test-resource") require.Contains(t, mockFS.writtenTemplate, "default") require.Contains(t, mockFS.writtenTemplate, "stable") require.Contains(t, mockFS.writtenTemplate, "test-data") @@ -132,12 +132,12 @@ func TestGenerateModuleTemplateWithManager_Success(t *testing.T) { svc, _ := templategenerator.NewService(mockFS) moduleConfig := &contentprovider.ModuleConfig{ - ResourceName: "test-resource", - Namespace: "default", - Channel: "stable", - Labels: map[string]string{"key": "value"}, - Annotations: map[string]string{"annotation": "value"}, - Mandatory: true, + Namespace: "default", + Channel: "stable", + Version: "1.0.0", + Labels: map[string]string{"key": "value"}, + Annotations: map[string]string{"annotation": "value"}, + Mandatory: true, Manager: &contentprovider.Manager{ Name: "manager-name", Namespace: "manager-ns", @@ -155,7 +155,7 @@ func TestGenerateModuleTemplateWithManager_Success(t *testing.T) { require.NoError(t, err) require.Equal(t, "output.yaml", mockFS.path) - require.Contains(t, mockFS.writtenTemplate, "test-resource") + require.Contains(t, mockFS.writtenTemplate, "component-1.0.0") require.Contains(t, mockFS.writtenTemplate, "default") require.Contains(t, mockFS.writtenTemplate, "stable") require.Contains(t, mockFS.writtenTemplate, "test-data") @@ -173,12 +173,12 @@ func TestGenerateModuleTemplateWithManagerWithoutNamespace_Success(t *testing.T) svc, _ := templategenerator.NewService(mockFS) moduleConfig := &contentprovider.ModuleConfig{ - ResourceName: "test-resource", - Namespace: "default", - Channel: "stable", - Labels: map[string]string{"key": "value"}, - Annotations: map[string]string{"annotation": "value"}, - Mandatory: true, + Namespace: "default", + Channel: "stable", + Version: "1.0.0", + Labels: map[string]string{"key": "value"}, + Annotations: map[string]string{"annotation": "value"}, + Mandatory: true, Manager: &contentprovider.Manager{ Name: "manager-name", GroupVersionKind: metav1.GroupVersionKind{ @@ -195,7 +195,7 @@ func TestGenerateModuleTemplateWithManagerWithoutNamespace_Success(t *testing.T) require.NoError(t, err) require.Equal(t, "output.yaml", mockFS.path) - require.Contains(t, mockFS.writtenTemplate, "test-resource") + require.Contains(t, mockFS.writtenTemplate, "component-1.0.0") require.Contains(t, mockFS.writtenTemplate, "default") require.Contains(t, mockFS.writtenTemplate, "stable") require.Contains(t, mockFS.writtenTemplate, "test-data") diff --git a/scaffold-create-config.yaml b/scaffold-create-config.yaml index cba04664..e74bf28e 100644 --- a/scaffold-create-config.yaml +++ b/scaffold-create-config.yaml @@ -4,7 +4,6 @@ channel: "regular" # required, channel that should be used in the ModuleTemplate manifest: "manifest.yaml" # required, relative path or remote URL to the manifests # mandatory: false # optional, default=false, indicates whether the module is mandatory to be installed on all clusters # defaultCR: "" # optional, relative path or remote URL to a YAML file containing the default CR for the module -# resourceName: "" # optional, default={name}-{channel}, when channel is 'none', the default is {name}-{version}, the name for the ModuleTemplate that will be created # namespace: "" # optional, default=kcp-system, the namespace where the ModuleTemplate will be deployed # security: "" # optional, name of the security scanners config file # internal: false # optional, default=false, determines whether the ModuleTemplate should have the internal flag or not diff --git a/tests/e2e/create/create_suite_test.go b/tests/e2e/create/create_suite_test.go index 7c5226e8..e0ccc6c9 100644 --- a/tests/e2e/create/create_suite_test.go +++ b/tests/e2e/create/create_suite_test.go @@ -22,7 +22,6 @@ const ( invalidConfigs = testdataDir + "invalid/" duplicateResources = invalidConfigs + "duplicate-resources.yaml" - emptyResourceName = invalidConfigs + "empty-resource-name.yaml" missingNameConfig = invalidConfigs + "missing-name.yaml" missingChannelConfig = invalidConfigs + "missing-channel.yaml" missingVersionConfig = invalidConfigs + "missing-version.yaml" diff --git a/tests/e2e/create/create_test.go b/tests/e2e/create/create_test.go index f63d5997..25f4ddaa 100644 --- a/tests/e2e/create/create_test.go +++ b/tests/e2e/create/create_test.go @@ -107,20 +107,6 @@ var _ = Describe("Test 'create' command", Ordered, func() { }) }) - Context("Given 'modulectl create' command", func() { - var cmd createCmd - It("When invoked with empty resource name", func() { - cmd = createCmd{ - moduleConfigFile: emptyResourceName, - } - }) - 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: name must not be empty")) - }) - }) - Context("Given 'modulectl create' command", func() { var cmd createCmd It("When invoked with non https resource", func() { diff --git a/tests/e2e/create/testdata/moduleconfig/invalid/empty-resource-name.yaml b/tests/e2e/create/testdata/moduleconfig/invalid/empty-resource-name.yaml deleted file mode 100644 index ac32b8d3..00000000 --- a/tests/e2e/create/testdata/moduleconfig/invalid/empty-resource-name.yaml +++ /dev/null @@ -1,7 +0,0 @@ -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 -resources: -- name: "" - link: http://some.other/location/template-operator.yaml diff --git a/tests/e2e/scaffold/scaffold_suite_test.go b/tests/e2e/scaffold/scaffold_suite_test.go index fc83177a..cdf04686 100644 --- a/tests/e2e/scaffold/scaffold_suite_test.go +++ b/tests/e2e/scaffold/scaffold_suite_test.go @@ -157,7 +157,6 @@ type moduleConfig struct { ManifestPath string `yaml:"manifest" comment:"required, relative path or remote URL to the manifests"` Mandatory bool `yaml:"mandatory" comment:"optional, default=false, indicates whether the module is mandatory to be installed on all clusters"` DefaultCRPath string `yaml:"defaultCR" comment:"optional, relative path or remote URL to a YAML file containing the default CR for the module"` - ResourceName string `yaml:"resourceName" comment:"optional, default={name}-{channel}, when channel is 'none', the default is {name}-{version}, the name for the ModuleTemplate that will be created"` Namespace string `yaml:"namespace" comment:"optional, default=kcp-system, the namespace where the ModuleTemplate will be deployed"` Security string `yaml:"security" comment:"optional, name of the security scanners config file"` Internal bool `yaml:"internal" comment:"optional, default=false, determines whether the ModuleTemplate should have the internal flag or not"`