Skip to content

Commit

Permalink
feat: Security must be a local file, not from repository (#100)
Browse files Browse the repository at this point in the history
* feat: Security must be a local file, not from repository
  • Loading branch information
c-pius authored Nov 21, 2024
1 parent 04ed116 commit 1230ee5
Show file tree
Hide file tree
Showing 14 changed files with 150 additions and 41 deletions.
3 changes: 2 additions & 1 deletion cmd/modulectl/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,8 @@ func buildModuleService() (*create.Service, error) {
if err != nil {
return nil, fmt.Errorf("failed to create git sources service: %w", err)
}
securityConfigService, err := componentdescriptor.NewSecurityConfigService(gitService)

securityConfigService, err := componentdescriptor.NewSecurityConfigService(fileSystemUtil)
if err != nil {
return nil, fmt.Errorf("failed to create security config service: %w", err)
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/modulectl/create/long.txt
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ The module config file is a YAML file used to configure the following attributes
- mandatory: a boolean, optional, default=false, indicates whether the module is mandatory to be installed on all clusters
- 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
- security: a string, optional, reference to a YAML file containing the security scanners config, must be a local file path
- labels: a map with string keys and values, optional, additional labels for the generated ModuleTemplate CR
- annotations: a map with string keys and values, optional, additional annotations for the generated ModuleTemplate CR
- manager: # an object, optional, module resource that indicates the installation readiness of the module
Expand Down
2 changes: 1 addition & 1 deletion docs/gen-docs/modulectl_create.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ The module config file is a YAML file used to configure the following attributes
- mandatory: a boolean, optional, default=false, indicates whether the module is mandatory to be installed on all clusters
- 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
- security: a string, optional, reference to a YAML file containing the security scanners config, must be a local file path
- labels: a map with string keys and values, optional, additional labels for the generated ModuleTemplate CR
- annotations: a map with string keys and values, optional, additional annotations for the generated ModuleTemplate CR
- manager: # an object, optional, module resource that indicates the installation readiness of the module
Expand Down
34 changes: 22 additions & 12 deletions internal/service/componentdescriptor/securityconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,37 +35,47 @@ const (
refLabel = "git.kyma-project.io/ref"
)

var ErrSecurityConfigFileDoesNotExist = errors.New("security config file does not exist")

type FileReader interface {
FileExists(path string) (bool, error)
ReadFile(path string) ([]byte, error)
}

type SecurityConfigService struct {
gitService GitService
fileReader FileReader
}

func NewSecurityConfigService(gitService GitService) (*SecurityConfigService, error) {
if gitService == nil {
return nil, fmt.Errorf("%w: gitService must not be nil", commonerrors.ErrInvalidArg)
func NewSecurityConfigService(fileReader FileReader) (*SecurityConfigService, error) {
if fileReader == nil {
return nil, fmt.Errorf("%w: fileReader must not be nil", commonerrors.ErrInvalidArg)
}

return &SecurityConfigService{
gitService: gitService,
fileReader: fileReader,
}, nil
}

func (s *SecurityConfigService) ParseSecurityConfigData(gitRepoURL, securityConfigFile string) (
func (s *SecurityConfigService) ParseSecurityConfigData(securityConfigFile string) (
*contentprovider.SecurityScanConfig,
error,
) {
latestCommit, err := s.gitService.GetLatestCommit(gitRepoURL)
exists, err := s.fileReader.FileExists(securityConfigFile)
if err != nil {
return nil, fmt.Errorf("failed to get latest commit: %w", err)
return nil, fmt.Errorf("failed to check if security config file exists: %w", err)
}
if !exists {
return nil, ErrSecurityConfigFileDoesNotExist
}

securityConfigContent, err := s.gitService.GetRemoteGitFileContent(gitRepoURL, latestCommit, securityConfigFile)
securityConfigContent, err := s.fileReader.ReadFile(securityConfigFile)
if err != nil {
return nil, fmt.Errorf("failed to get security config content: %w", err)
return nil, fmt.Errorf("failed to read security config file: %w", err)
}

securityConfig := &contentprovider.SecurityScanConfig{}
if err := yaml.Unmarshal([]byte(securityConfigContent), securityConfig); err != nil {
return nil, fmt.Errorf("failed to parse security config file: %w", err)
if err := yaml.Unmarshal(securityConfigContent, securityConfig); err != nil {
return nil, fmt.Errorf("failed to unmarshal security config file: %w", err)
}

return securityConfig, nil
Expand Down
77 changes: 59 additions & 18 deletions internal/service/componentdescriptor/securityconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@ import (
"github.com/kyma-project/modulectl/internal/service/contentprovider"
)

func Test_NewSecurityConfigService_ReturnsErrorOnNilFileReader(t *testing.T) {
securityConfigService, err := componentdescriptor.NewSecurityConfigService(nil)
require.ErrorContains(t, err, "fileReader must not be nil")
require.Nil(t, securityConfigService)
}

func Test_GetImageNameAndTag(t *testing.T) {
tests := []struct {
name string
Expand Down Expand Up @@ -152,11 +158,10 @@ func Test_AppendSecurityLabelsToSources_ReturnCorrectLabels(t *testing.T) {
}

func TestSecurityConfigService_ParseSecurityConfigData_ReturnsCorrectData(t *testing.T) {
securityConfigService, err := componentdescriptor.NewSecurityConfigService(&gitServiceSecurityConfigStub{})
securityConfigService, err := componentdescriptor.NewSecurityConfigService(&fileReaderStub{})
require.NoError(t, err)

returned, err := securityConfigService.ParseSecurityConfigData("https://github.com/kyma-project/template-operator",
"sec-scanners-config.yaml")
returned, err := securityConfigService.ParseSecurityConfigData("sec-scanners-config.yaml")
require.NoError(t, err)

require.Equal(t, securityConfig.RcTag, returned.RcTag)
Expand All @@ -166,18 +171,39 @@ func TestSecurityConfigService_ParseSecurityConfigData_ReturnsCorrectData(t *tes
require.Equal(t, securityConfig.WhiteSource.Language, returned.WhiteSource.Language)
}

func TestSecurityConfigService_ParseSecurityConfigData_ReturnErrOnRemoteFileReadingError(t *testing.T) {
securityConfigService, err := componentdescriptor.NewSecurityConfigService(&gitServiceNoRemoteFile{})
func TestSecurityConfigService_ParseSecurityConfigData_ReturnErrOnFileExistenceCheckError(t *testing.T) {
securityConfigService, err := componentdescriptor.NewSecurityConfigService(&fileReaderFileExistsErrorStub{})
require.NoError(t, err)

_, err = securityConfigService.ParseSecurityConfigData("testFile")
require.ErrorContains(t, err, "failed to check if security config file exists")
}

func TestSecurityConfigService_ParseSecurityConfigData_ReturnErrOnFileReadingError(t *testing.T) {
securityConfigService, err := componentdescriptor.NewSecurityConfigService(&fileReaderReadFileErrorStub{})
require.NoError(t, err)

_, err = securityConfigService.ParseSecurityConfigData("testUrl", "testFile")
require.ErrorContains(t, err, "failed to get security config content")
_, err = securityConfigService.ParseSecurityConfigData("testFile")
require.ErrorContains(t, err, "failed to read security config file")
}

type gitServiceSecurityConfigStub struct{}
func TestSecurityConfigService_ParseSecurityConfigData_ReturnErrOnFileDoesNotExist(t *testing.T) {
securityConfigService, err := componentdescriptor.NewSecurityConfigService(&fileReaderFileExistsFalseStub{})
require.NoError(t, err)

_, err = securityConfigService.ParseSecurityConfigData("testFile")
require.ErrorContains(t, err, "security config file does not exist")
}

type fileReaderStub struct{}

func (*fileReaderStub) FileExists(path string) (bool, error) {
return true, nil
}

func (g *gitServiceSecurityConfigStub) GetLatestCommit(_ string) (string, error) {
return "latestCommit", nil
func (*fileReaderStub) ReadFile(path string) ([]byte, error) {
securityConfigBytes, _ := yaml.Marshal(securityConfig)
return securityConfigBytes, nil
}

var securityConfig = contentprovider.SecurityScanConfig{
Expand All @@ -190,17 +216,32 @@ var securityConfig = contentprovider.SecurityScanConfig{
},
}

func (g *gitServiceSecurityConfigStub) GetRemoteGitFileContent(_, _, _ string) (string, error) {
securityConfigBytes, _ := yaml.Marshal(securityConfig)
return string(securityConfigBytes), nil
type fileReaderFileExistsErrorStub struct{}

func (*fileReaderFileExistsErrorStub) FileExists(path string) (bool, error) {
return false, errors.New("error while checking file existence")
}

func (*fileReaderFileExistsErrorStub) ReadFile(path string) ([]byte, error) {
return nil, errors.New("error while reading file")
}

type fileReaderReadFileErrorStub struct{}

func (*fileReaderReadFileErrorStub) FileExists(path string) (bool, error) {
return true, nil
}

func (*fileReaderReadFileErrorStub) ReadFile(path string) ([]byte, error) {
return nil, errors.New("error while reading file")
}

type gitServiceNoRemoteFile struct{}
type fileReaderFileExistsFalseStub struct{}

func (*gitServiceNoRemoteFile) GetLatestCommit(_ string) (string, error) {
return "latestCommit", nil
func (*fileReaderFileExistsFalseStub) FileExists(path string) (bool, error) {
return false, nil
}

func (*gitServiceNoRemoteFile) GetRemoteGitFileContent(_, _, _ string) (string, error) {
return "", errors.New("error")
func (*fileReaderFileExistsFalseStub) ReadFile(path string) ([]byte, error) {
return nil, nil
}
6 changes: 3 additions & 3 deletions internal/service/create/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ type FileResolver interface {
}

type SecurityConfigService interface {
ParseSecurityConfigData(gitRepoURL, securityConfigFile string) (*contentprovider.SecurityScanConfig, error)
ParseSecurityConfigData(securityConfigFile string) (*contentprovider.SecurityScanConfig, error)
AppendSecurityScanConfig(descriptor *compdesc.ComponentDescriptor,
securityConfig contentprovider.SecurityScanConfig) error
}
Expand Down Expand Up @@ -246,9 +246,9 @@ func (s *Service) pushImgAndCreateTemplate(archive *comparch.ComponentArchive, m

func (s *Service) configureSecScannerConf(descriptor *compdesc.ComponentDescriptor, moduleConfig *contentprovider.ModuleConfig, opts Options) error {
opts.Out.Write("- Configuring security scanners config\n")
securityConfig, err := s.securityConfigService.ParseSecurityConfigData(moduleConfig.Repository, moduleConfig.Security)
securityConfig, err := s.securityConfigService.ParseSecurityConfigData(moduleConfig.Security)
if err != nil {
return fmt.Errorf("%w: failed to parse security config data", err)
return fmt.Errorf("failed to parse security config data: %w", err)
}

if err = s.securityConfigService.AppendSecurityScanConfig(descriptor, *securityConfig); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion internal/service/create/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ func (*gitSourcesServiceStub) AddGitSources(_ *compdesc.ComponentDescriptor,

type securityConfigServiceStub struct{}

func (*securityConfigServiceStub) ParseSecurityConfigData(_, _ string) (*contentprovider.SecurityScanConfig, error) {
func (*securityConfigServiceStub) ParseSecurityConfigData(_ string) (*contentprovider.SecurityScanConfig, error) {
return &contentprovider.SecurityScanConfig{}, nil
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ func Test_ParseModuleConfig_Returns_CorrectModuleConfig(t *testing.T) {
require.Equal(t, "https://example.com/path/to/defaultCR", result.DefaultCR)
require.Equal(t, "https://example.com/path/to/repository", result.Repository)
require.Equal(t, "https://example.com/path/to/documentation", result.Documentation)
require.Equal(t, "path/to/securityConfig", result.Security)
require.Equal(t, contentprovider.Icons{
"module-icon": "https://example.com/path/to/some-icon",
}, result.Icons)
Expand Down
1 change: 1 addition & 0 deletions tests/e2e/create/create_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ const (
iconsWithoutName = invalidConfigs + "icons-without-name.yaml"
manifestFileref = invalidConfigs + "manifest-fileref.yaml"
defaultCRFileref = invalidConfigs + "defaultcr-fileref.yaml"
invalidSecurityConfig = invalidConfigs + "not-existing-security.yaml"

validConfigs = testdataDir + "valid/"
minimalConfig = validConfigs + "minimal.yaml"
Expand Down
38 changes: 36 additions & 2 deletions tests/e2e/create/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -486,13 +486,20 @@ var _ = Describe("Test 'create' command", Ordered, func() {
Expect(template.Name).To(Equal("template-operator-1.0.3"))

By("And descriptor.component.resources should be correct")
Expect(descriptor.Resources).To(HaveLen(2))
Expect(descriptor.Resources).To(HaveLen(3))
resource := descriptor.Resources[0]
Expect(resource.Name).To(Equal("template-operator"))
Expect(resource.Relation).To(Equal(ocmv1.ExternalRelation))
Expect(resource.Type).To(Equal("ociArtifact"))
Expect(resource.Version).To(Equal("1.0.1"))

resource = descriptor.Resources[1]
Expect(resource.Name).To(Equal("template-operator"))
Expect(resource.Relation).To(Equal(ocmv1.ExternalRelation))
Expect(resource.Type).To(Equal("ociArtifact"))
Expect(resource.Version).To(Equal("2.0.0"))

resource = descriptor.Resources[2]
Expect(resource.Name).To(Equal("raw-manifest"))
Expect(resource.Version).To(Equal("1.0.3"))

Expand All @@ -507,7 +514,15 @@ var _ = Describe("Test 'create' command", Ordered, func() {
By("And descriptor.component.resources[1].access should be correct")
resourceAccessSpec1, err := ocm.DefaultContext().AccessSpecForSpec(descriptor.Resources[1].Access)
Expect(err).ToNot(HaveOccurred())
localBlobAccessSpec, ok := resourceAccessSpec1.(*localblob.AccessSpec)
ociArtifactAccessSpec, ok = resourceAccessSpec1.(*ociartifact.AccessSpec)
Expect(ok).To(BeTrue())
Expect(ociArtifactAccessSpec.GetType()).To(Equal(ociartifact.Type))
Expect(ociArtifactAccessSpec.ImageReference).To(Equal("europe-docker.pkg.dev/kyma-project/prod/template-operator:2.0.0"))

By("And descriptor.component.resources[2].access should be correct")
resourceAccessSpec2, err := ocm.DefaultContext().AccessSpecForSpec(descriptor.Resources[2].Access)
Expect(err).ToNot(HaveOccurred())
localBlobAccessSpec, ok := resourceAccessSpec2.(*localblob.AccessSpec)
Expect(ok).To(BeTrue())
Expect(localBlobAccessSpec.GetType()).To(Equal(localblob.Type))
Expect(localBlobAccessSpec.LocalReference).To(ContainSubstring("sha256:"))
Expand Down Expand Up @@ -538,6 +553,25 @@ var _ = Describe("Test 'create' command", Ordered, func() {
})
})

Context("Given 'modulectl create' command", func() {
var cmd createCmd
It("When invoked with invalid module-config containing not existing security-scanner-config",
func() {
cmd = createCmd{
moduleConfigFile: invalidSecurityConfig,
registry: ociRegistry,
insecure: true,
output: templateOutputPath,
}
})
It("Then the command should succeed", func() {
err := cmd.execute()

Expect(err).Should(HaveOccurred())
Expect(err.Error()).Should(ContainSubstring("failed to configure security scanners: failed to parse security config data: security config file does not exist"))
})
})

Context("Given 'modulectl create' command", func() {
var cmd createCmd
It("When invoked with valid module-config containing mandatory true and different version", func() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
name: kyma-project.io/module/template-operator
version: 1.0.3
manifest: https://github.com/kyma-project/template-operator/releases/download/1.0.1/template-operator.yaml
repository: https://github.com/kyma-project/template-operator
documentation: https://github.com/kyma-project/template-operator/blob/main/README.md
icons:
- name: module-icon
link: https://github.com/kyma-project/template-operator/blob/main/docs/assets/logo.png
security: ./testdata/sec-scanners-config/does-not-exist.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@ documentation: https://github.com/kyma-project/template-operator/blob/main/READM
icons:
- name: module-icon
link: https://github.com/kyma-project/template-operator/blob/main/docs/assets/logo.png
security: sec-scanners-config.yaml
security: ./testdata/sec-scanners-config/config.yaml
12 changes: 12 additions & 0 deletions tests/e2e/create/testdata/sec-scanners-config/config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
module-name: template-operator
rc-tag: 1.0.1
dev-branch: main
protecode:
- europe-docker.pkg.dev/kyma-project/prod/template-operator:1.0.1
- europe-docker.pkg.dev/kyma-project/prod/template-operator:2.0.0
whitesource:
language: golang-mod
subprojects: false
exclude:
- "**/test/**"
- "**/*_test.go"
2 changes: 1 addition & 1 deletion unit-test-coverage.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ packages:
internal/service/moduleconfig/generator: 100
internal/service/moduleconfig/reader: 76
internal/service/create: 43
internal/service/componentdescriptor: 78
internal/service/componentdescriptor: 80
internal/service/templategenerator: 85
internal/service/crdparser: 73
internal/service/registry: 52
Expand Down

0 comments on commit 1230ee5

Please sign in to comment.