Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Remove ResourceName #87

Merged
merged 5 commits into from
Oct 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion cmd/modulectl/create/long.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion docs/gen-docs/modulectl_create.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion internal/service/contentprovider/moduleconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -147,6 +146,7 @@ func (*fileExistsStub) ReadFile(_ string) ([]byte, error) {
Kind: "Deployment",
},
},
Resources: contentprovider.ResourcesMap{},
}

return yaml.Marshal(moduleConfig)
Expand Down
50 changes: 27 additions & 23 deletions internal/service/moduleconfig/reader/moduleconfig_reader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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 {
Expand Down Expand Up @@ -363,29 +368,25 @@ 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",
Version: "v1alpha3",
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",
Expand All @@ -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) {
Expand Down
6 changes: 2 additions & 4 deletions internal/service/templategenerator/templategenerator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -150,7 +148,7 @@ func (s *Service) GenerateModuleTemplate(
}

mtData := moduleTemplateData{
ResourceName: moduleConfig.ResourceName,
ResourceName: moduleTemplateName,
Namespace: moduleConfig.Namespace,
Descriptor: cva,
Channel: moduleConfig.Channel,
Expand Down
64 changes: 32 additions & 32 deletions internal/service/templategenerator/templategenerator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -59,15 +59,16 @@ 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")
require.Contains(t, mockFS.writtenTemplate, "example.com/component")
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) {
Expand All @@ -87,20 +88,20 @@ 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) {
mockFS := &mockFileSystem{}
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",
Expand All @@ -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")
Expand All @@ -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",
Expand All @@ -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")
Expand All @@ -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{
Expand All @@ -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")
Expand Down
1 change: 0 additions & 1 deletion scaffold-create-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion tests/e2e/create/create_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
14 changes: 0 additions & 14 deletions tests/e2e/create/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down

This file was deleted.

1 change: 0 additions & 1 deletion tests/e2e/scaffold/scaffold_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
Loading