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.

Sign up for GitHub

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: Refactor file resolving #1

Merged
merged 5 commits into from
Oct 21, 2024
Merged

chore: Refactor file resolving #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
60cebf8
chore: Refactor file resolving
c-pius Oct 18, 2024
bcc8a52
fix unit tests
c-pius Oct 18, 2024
6996756
revert suite test changes
c-pius Oct 18, 2024
65254d9
increase fileresolver coverage
c-pius Oct 18, 2024
5284bf4
fix linting
c-pius Oct 18, 2024
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
16 changes: 14 additions & 2 deletions cmd/modulectl/cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/kyma-project/modulectl/internal/service/create"
"github.com/kyma-project/modulectl/internal/service/filegenerator"
"github.com/kyma-project/modulectl/internal/service/filegenerator/reusefilegenerator"
"github.com/kyma-project/modulectl/internal/service/fileresolver"
"github.com/kyma-project/modulectl/internal/service/git"
moduleconfiggenerator "github.com/kyma-project/modulectl/internal/service/moduleconfig/generator"
moduleconfigreader "github.com/kyma-project/modulectl/internal/service/moduleconfig/reader"
Expand Down Expand Up @@ -89,7 +90,17 @@ func buildModuleService() (*create.Service, error) {
fileSystemUtil := &filesystem.Util{}
tmpFileSystem := filesystem.NewTempFileSystem()

moduleConfigService, err := moduleconfigreader.NewService(fileSystemUtil, tmpFileSystem)
manifestFileResolver, err := fileresolver.NewFileResolver("kyma-module-manifest-*.yaml", tmpFileSystem)
if err != nil {
return nil, fmt.Errorf("failed to create manifest file resolver: %w", err)
}

defaultCRFileResolver, err := fileresolver.NewFileResolver("kyma-module-default-cr-*.yaml", tmpFileSystem)
if err != nil {
return nil, fmt.Errorf("failed to create default CR file resolver: %w", err)
}

moduleConfigService, err := moduleconfigreader.NewService(fileSystemUtil)
if err != nil {
return nil, fmt.Errorf("failed to create module config service: %w", err)
}
Expand Down Expand Up @@ -127,7 +138,8 @@ func buildModuleService() (*create.Service, error) {
return nil, fmt.Errorf("failed to create crd parser service: %w", err)
}
moduleService, err := create.NewService(moduleConfigService, gitSourcesService,
securityConfigService, componentArchiveService, registryService, moduleTemplateService, crdParserService)
securityConfigService, componentArchiveService, registryService, moduleTemplateService,
crdParserService, manifestFileResolver, defaultCRFileResolver, fileSystemUtil)
if err != nil {
return nil, fmt.Errorf("failed to create module service: %w", err)
}
Expand Down
43 changes: 21 additions & 22 deletions internal/service/contentprovider/moduleconfig.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,12 @@ func (s *ModuleConfigProvider) GetDefaultContent(args types.KeyValueArgs) (strin

func (s *ModuleConfigProvider) getModuleConfig(args types.KeyValueArgs) ModuleConfig {
return ModuleConfig{
Name: args[ArgModuleName],
Version: args[ArgModuleVersion],
Channel: args[ArgModuleChannel],
Manifest: args[ArgManifestFile],
Security: args[ArgSecurityConfigFile],
DefaultCRPath: args[ArgDefaultCRFile],
Name: args[ArgModuleName],
Version: args[ArgModuleVersion],
Channel: args[ArgModuleChannel],
Manifest: args[ArgManifestFile],
Security: args[ArgSecurityConfigFile],
DefaultCR: args[ArgDefaultCRFile],
}
}

Expand Down Expand Up @@ -80,22 +80,21 @@ type Manager struct {
}

type ModuleConfig struct {
Name string `yaml:"name" comment:"required, the name of the Module"`
Version string `yaml:"version" comment:"required, the version of the Module"`
Channel string `yaml:"channel" comment:"required, channel that should be used in the ModuleTemplate"`
Manifest string `yaml:"manifest" comment:"required, relative path or remote URL to the manifests"`
ManifestFilePath string `yaml:"-"` // ignore this field, will be filled programmatically
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"`
Beta bool `yaml:"beta" comment:"optional, default=false, determines whether the ModuleTemplate should have the beta flag or not"`
Labels map[string]string `yaml:"labels" comment:"optional, additional labels for the ModuleTemplate"`
Annotations map[string]string `yaml:"annotations" comment:"optional, additional annotations for the ModuleTemplate"`
Manager *Manager `yaml:"manager" comment:"optional, the module resource that can be used to indicate the installation readiness of the module. This is typically the manager deployment of the module"`
Resources ResourcesMap `yaml:"resources,omitempty" comment:"optional, additional resources of the ModuleTemplate that may be fetched"`
Name string `yaml:"name" comment:"required, the name of the Module"`
Version string `yaml:"version" comment:"required, the version of the Module"`
Channel string `yaml:"channel" comment:"required, channel that should be used in the ModuleTemplate"`
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"`
Beta bool `yaml:"beta" comment:"optional, default=false, determines whether the ModuleTemplate should have the beta flag or not"`
Labels map[string]string `yaml:"labels" comment:"optional, additional labels for the ModuleTemplate"`
Annotations map[string]string `yaml:"annotations" comment:"optional, additional annotations for the ModuleTemplate"`
Manager *Manager `yaml:"manager" comment:"optional, the module resource that can be used to indicate the installation readiness of the module. This is typically the manager deployment of the module"`
Resources ResourcesMap `yaml:"resources,omitempty" comment:"optional, additional resources of the ModuleTemplate that may be fetched"`
}

type resource struct {
Expand Down
65 changes: 55 additions & 10 deletions internal/service/create/create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,14 @@ import (

type ModuleConfigService interface {
ParseAndValidateModuleConfig(moduleConfigFile string) (*contentprovider.ModuleConfig, error)
GetDefaultCRData(defaultCRPath string) ([]byte, error)
}

type FileSystem interface {
ReadFile(path string) ([]byte, error)
}

type FileResolver interface {
Resolve(file string) (string, error)
CleanupTempFiles() []error
}

Expand Down Expand Up @@ -62,6 +69,9 @@ type Service struct {
registryService RegistryService
moduleTemplateService ModuleTemplateService
crdParserService CRDParserService
manifestFileResolver FileResolver
defaultCRFileResolver FileResolver
fileSystem FileSystem
}

func NewService(moduleConfigService ModuleConfigService,
Expand All @@ -71,6 +81,9 @@ func NewService(moduleConfigService ModuleConfigService,
registryService RegistryService,
moduleTemplateService ModuleTemplateService,
crdParserService CRDParserService,
manifestFileResolver FileResolver,
defaultCRFileResolver FileResolver,
fileSystem FileSystem,
) (*Service, error) {
if moduleConfigService == nil {
return nil, fmt.Errorf("%w: moduleConfigService must not be nil", commonerrors.ErrInvalidArg)
Expand Down Expand Up @@ -100,6 +113,18 @@ func NewService(moduleConfigService ModuleConfigService,
return nil, fmt.Errorf("%w: crdParserService must not be nil", commonerrors.ErrInvalidArg)
}

if manifestFileResolver == nil {
return nil, fmt.Errorf("%w: manifestFileResolver must not be nil", commonerrors.ErrInvalidArg)
}

if defaultCRFileResolver == nil {
return nil, fmt.Errorf("%w: defaultCRFileResolver must not be nil", commonerrors.ErrInvalidArg)
}

if fileSystem == nil {
return nil, fmt.Errorf("%w: fileSystem must not be nil", commonerrors.ErrInvalidArg)
}

return &Service{
moduleConfigService: moduleConfigService,
gitSourcesService: gitSourcesService,
Expand All @@ -108,6 +133,9 @@ func NewService(moduleConfigService ModuleConfigService,
registryService: registryService,
moduleTemplateService: moduleTemplateService,
crdParserService: crdParserService,
manifestFileResolver: manifestFileResolver,
defaultCRFileResolver: defaultCRFileResolver,
fileSystem: fileSystem,
}, nil
}

Expand All @@ -117,8 +145,12 @@ func (s *Service) Run(opts Options) error {
}

defer func() {
if err := s.moduleConfigService.CleanupTempFiles(); err != nil {
opts.Out.Write(fmt.Sprintf("failed to cleanup temporary files: %v\n", err))
if err := s.defaultCRFileResolver.CleanupTempFiles(); err != nil {
opts.Out.Write(fmt.Sprintf("failed to cleanup temporary default CR files: %v\n", err))
}

if err := s.manifestFileResolver.CleanupTempFiles(); err != nil {
opts.Out.Write(fmt.Sprintf("failed to cleanup temporary manifest files: %v\n", err))
}
}()

Expand All @@ -127,13 +159,26 @@ func (s *Service) Run(opts Options) error {
return fmt.Errorf("failed to parse module config: %w", err)
}

manifestFilePath, err := s.manifestFileResolver.Resolve(moduleConfig.Manifest)
if err != nil {
return fmt.Errorf("failed to resolve manifest file: %w", err)
}

defaultCRFilePath := moduleConfig.DefaultCR
if moduleConfig.DefaultCR != "" {
defaultCRFilePath, err = s.defaultCRFileResolver.Resolve(moduleConfig.DefaultCR)
if err != nil {
return fmt.Errorf("failed to resolve default CR file: %w", err)
}
}

descriptor, err := componentdescriptor.InitializeComponentDescriptor(moduleConfig.Name, moduleConfig.Version)
if err != nil {
return fmt.Errorf("failed to populate component descriptor metadata: %w", err)
}

moduleResources, err := componentdescriptor.GenerateModuleResources(moduleConfig.Version, moduleConfig.ManifestFilePath,
moduleConfig.DefaultCRPath, opts.RegistryCredSelector)
moduleResources, err := componentdescriptor.GenerateModuleResources(moduleConfig.Version, manifestFilePath,
defaultCRFilePath, opts.RegistryCredSelector)
if err != nil {
return fmt.Errorf("failed to generate module resources: %w", err)
}
Expand Down Expand Up @@ -162,14 +207,14 @@ func (s *Service) Run(opts Options) error {
}

if opts.RegistryURL != "" {
return s.pushImgAndCreateTemplate(archive, moduleConfig, opts)
return s.pushImgAndCreateTemplate(archive, moduleConfig, manifestFilePath, defaultCRFilePath, opts)
}
return nil
}

func (s *Service) pushImgAndCreateTemplate(archive *comparch.ComponentArchive, moduleConfig *contentprovider.ModuleConfig, opts Options) error {
func (s *Service) pushImgAndCreateTemplate(archive *comparch.ComponentArchive, moduleConfig *contentprovider.ModuleConfig, manifestFilePath, defaultCRFilePath string, opts Options) error {
opts.Out.Write("- Pushing component version\n")
isCRDClusterScoped, err := s.crdParserService.IsCRDClusterScoped(moduleConfig.DefaultCRPath, moduleConfig.ManifestFilePath)
isCRDClusterScoped, err := s.crdParserService.IsCRDClusterScoped(defaultCRFilePath, manifestFilePath)
if err != nil {
return fmt.Errorf("failed to determine if CRD is cluster scoped: %w", err)
}
Expand All @@ -185,8 +230,8 @@ func (s *Service) pushImgAndCreateTemplate(archive *comparch.ComponentArchive, m
}

var crData []byte
if moduleConfig.DefaultCRPath != "" {
crData, err = s.moduleConfigService.GetDefaultCRData(moduleConfig.DefaultCRPath)
if defaultCRFilePath != "" {
crData, err = s.fileSystem.ReadFile(defaultCRFilePath)
if err != nil {
return fmt.Errorf("%w: failed to get default CR data", err)
}
Expand Down
94 changes: 71 additions & 23 deletions internal/service/create/create_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,17 @@ import (

func Test_NewService_ReturnsError_WhenModuleConfigServiceIsNil(t *testing.T) {
_, err := create.NewService(nil, &gitSourcesServiceStub{}, &securityConfigServiceStub{},
&componentArchiveServiceStub{}, &registryServiceStub{}, &ModuleTemplateServiceStub{}, &CRDParserServiceStub{})
&componentArchiveServiceStub{}, &registryServiceStub{}, &ModuleTemplateServiceStub{}, &CRDParserServiceStub{},
&fileResolverStub{}, &fileResolverStub{}, &fileExistsStub{})

require.ErrorIs(t, err, commonerrors.ErrInvalidArg)
require.Contains(t, err.Error(), "moduleConfigService")
}

func Test_CreateModule_ReturnsError_WhenModuleConfigFileIsEmpty(t *testing.T) {
svc, err := create.NewService(&moduleConfigServiceStub{}, &gitSourcesServiceStub{}, &securityConfigServiceStub{},
&componentArchiveServiceStub{}, &registryServiceStub{}, &ModuleTemplateServiceStub{}, &CRDParserServiceStub{})
&componentArchiveServiceStub{}, &registryServiceStub{}, &ModuleTemplateServiceStub{}, &CRDParserServiceStub{},
&fileResolverStub{}, &fileResolverStub{}, &fileExistsStub{})
require.NoError(t, err)

opts := newCreateOptionsBuilder().withModuleConfigFile("").build()
Expand All @@ -41,7 +43,8 @@ func Test_CreateModule_ReturnsError_WhenModuleConfigFileIsEmpty(t *testing.T) {

func Test_CreateModule_ReturnsError_WhenOutIsNil(t *testing.T) {
svc, err := create.NewService(&moduleConfigServiceStub{}, &gitSourcesServiceStub{}, &securityConfigServiceStub{},
&componentArchiveServiceStub{}, &registryServiceStub{}, &ModuleTemplateServiceStub{}, &CRDParserServiceStub{})
&componentArchiveServiceStub{}, &registryServiceStub{}, &ModuleTemplateServiceStub{}, &CRDParserServiceStub{},
&fileResolverStub{}, &fileResolverStub{}, &fileExistsStub{})
require.NoError(t, err)

opts := newCreateOptionsBuilder().withOut(nil).build()
Expand All @@ -54,7 +57,8 @@ func Test_CreateModule_ReturnsError_WhenOutIsNil(t *testing.T) {

func Test_CreateModule_ReturnsError_WhenCredentialsIsInInvalidFormat(t *testing.T) {
svc, err := create.NewService(&moduleConfigServiceStub{}, &gitSourcesServiceStub{}, &securityConfigServiceStub{},
&componentArchiveServiceStub{}, &registryServiceStub{}, &ModuleTemplateServiceStub{}, &CRDParserServiceStub{})
&componentArchiveServiceStub{}, &registryServiceStub{}, &ModuleTemplateServiceStub{}, &CRDParserServiceStub{},
&fileResolverStub{}, &fileResolverStub{}, &fileExistsStub{})
require.NoError(t, err)

opts := newCreateOptionsBuilder().withCredentials("user").build()
Expand All @@ -67,7 +71,8 @@ func Test_CreateModule_ReturnsError_WhenCredentialsIsInInvalidFormat(t *testing.

func Test_CreateModule_ReturnsError_WhenTemplateOutputIsEmpty(t *testing.T) {
svc, err := create.NewService(&moduleConfigServiceStub{}, &gitSourcesServiceStub{}, &securityConfigServiceStub{},
&componentArchiveServiceStub{}, &registryServiceStub{}, &ModuleTemplateServiceStub{}, &CRDParserServiceStub{})
&componentArchiveServiceStub{}, &registryServiceStub{}, &ModuleTemplateServiceStub{}, &CRDParserServiceStub{},
&fileResolverStub{}, &fileResolverStub{}, &fileExistsStub{})
require.NoError(t, err)

opts := newCreateOptionsBuilder().withTemplateOutput("").build()
Expand All @@ -79,9 +84,9 @@ func Test_CreateModule_ReturnsError_WhenTemplateOutputIsEmpty(t *testing.T) {
}

func Test_CreateModule_ReturnsError_WhenParseAndValidateModuleConfigReturnsError(t *testing.T) {
svc, err := create.NewService(&moduleConfigServiceParseErrorStub{}, &gitSourcesServiceStub{},
&securityConfigServiceStub{}, &componentArchiveServiceStub{}, &registryServiceStub{},
&ModuleTemplateServiceStub{}, &CRDParserServiceStub{})
svc, err := create.NewService(&moduleConfigServiceParseErrorStub{}, &gitSourcesServiceStub{}, &securityConfigServiceStub{},
&componentArchiveServiceStub{}, &registryServiceStub{}, &ModuleTemplateServiceStub{}, &CRDParserServiceStub{},
&fileResolverStub{}, &fileResolverStub{}, &fileExistsStub{})
require.NoError(t, err)

opts := newCreateOptionsBuilder().build()
Expand All @@ -92,6 +97,34 @@ func Test_CreateModule_ReturnsError_WhenParseAndValidateModuleConfigReturnsError
require.Contains(t, err.Error(), "failed to read module config file")
}

func Test_CreateModule_ReturnsError_WhenResolvingManifestFilePathReturnsError(t *testing.T) {
svc, err := create.NewService(&moduleConfigServiceStub{}, &gitSourcesServiceStub{}, &securityConfigServiceStub{},
&componentArchiveServiceStub{}, &registryServiceStub{}, &ModuleTemplateServiceStub{}, &CRDParserServiceStub{},
&fileResolverErrorStub{}, &fileResolverStub{}, &fileExistsStub{})
require.NoError(t, err)

opts := newCreateOptionsBuilder().build()

err = svc.Run(opts)

require.Error(t, err)
require.Contains(t, err.Error(), "failed to resolve file")
}

func Test_CreateModule_ReturnsError_WhenResolvingDefaultCRFilePathReturnsError(t *testing.T) {
svc, err := create.NewService(&moduleConfigServiceStub{}, &gitSourcesServiceStub{}, &securityConfigServiceStub{},
&componentArchiveServiceStub{}, &registryServiceStub{}, &ModuleTemplateServiceStub{}, &CRDParserServiceStub{},
&fileResolverStub{}, &fileResolverErrorStub{}, &fileExistsStub{})
require.NoError(t, err)

opts := newCreateOptionsBuilder().build()

err = svc.Run(opts)

require.Error(t, err)
require.Contains(t, err.Error(), "failed to resolve file")
}

type createOptionsBuilder struct {
options create.Options
}
Expand Down Expand Up @@ -144,21 +177,44 @@ func (b *createOptionsBuilder) withCredentials(credentials string) *createOption
return b
}

// Test Stubs
type moduleConfigServiceStub struct{}
type fileExistsStub struct{}

func (*moduleConfigServiceStub) ParseAndValidateModuleConfig(_ string) (*contentprovider.ModuleConfig, error) {
return &contentprovider.ModuleConfig{}, nil
func (*fileExistsStub) FileExists(_ string) (bool, error) {
return true, nil
}

func (*moduleConfigServiceStub) GetDefaultCRData(_ string) ([]byte, error) {
return []byte{}, nil
func (*fileExistsStub) ReadFile(_ string) ([]byte, error) {
return nil, nil
}

type fileResolverStub struct{}

func (*fileResolverStub) Resolve(_ string) (string, error) {
return "/tmp/some-file.yaml", nil
}

func (*moduleConfigServiceStub) CleanupTempFiles() []error {
func (*fileResolverStub) CleanupTempFiles() []error {
return nil
}

type fileResolverErrorStub struct{}

func (*fileResolverErrorStub) Resolve(_ string) (string, error) {
return "", errors.New("failed to resolve file")
}

func (*fileResolverErrorStub) CleanupTempFiles() []error {
return []error{errors.New("failed to cleanup temp files")}
}

type moduleConfigServiceStub struct{}

func (*moduleConfigServiceStub) ParseAndValidateModuleConfig(_ string) (*contentprovider.ModuleConfig, error) {
return &contentprovider.ModuleConfig{
DefaultCR: "default-cr.yaml",
}, nil
}

type moduleConfigServiceParseErrorStub struct{}

func (*moduleConfigServiceParseErrorStub) ParseAndValidateModuleConfig(_ string) (*contentprovider.ModuleConfig,
Expand All @@ -167,14 +223,6 @@ func (*moduleConfigServiceParseErrorStub) ParseAndValidateModuleConfig(_ string)
return nil, errors.New("failed to read module config file")
}

func (*moduleConfigServiceParseErrorStub) GetDefaultCRData(_ string) ([]byte, error) {
return []byte{}, nil
}

func (*moduleConfigServiceParseErrorStub) CleanupTempFiles() []error {
return nil
}

type gitSourcesServiceStub struct{}

func (*gitSourcesServiceStub) AddGitSources(_ *compdesc.ComponentDescriptor,
Expand Down
Loading