Skip to content

Commit

Permalink
feat: Support spec.info (#90)
Browse files Browse the repository at this point in the history
* Add e2e testcase

* Add testcase for missing

* delete unused field

* add info

* fix merge fail in tests

* add to scaffold

* add validation for info fields

* remove info parent in moduleconfig

* fix validate

* fix tests

* lint yamls

* fix e2e testdata

* share marshalling and unmarshalling logic

* remove generated module-configs

* make docs

* delete unneeded testdata

* add e2e testcases

* add more testcases

* adapt unit-test-

* fix assertion

* fix assertion

* fix assertion

* add different error for parsing errors of Icons and Resources

* add unit tests for Icons

* add e2e for malformed icons entries
  • Loading branch information
lindnerby authored Nov 4, 2024
1 parent 5bd654a commit 17493ed
Show file tree
Hide file tree
Showing 45 changed files with 765 additions and 161 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/create-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ jobs:
run: ./.github/scripts/upload_artifacts.sh ${{ github.event.inputs.tag }}
publish-release:
runs-on: ubuntu-latest
needs: [ validate-release, draft-release, create-artifacts ]
needs: [validate-release, draft-release, create-artifacts]
steps:
- name: Checkout code
uses: actions/checkout@v4
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/test-unit-coverage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: Verify Unit Test Coverage

on:
pull_request:
branches: [ "main" ]
branches: ["main"]

env:
coverage_guard: ${{ github.workspace }}/scripts/coverage-metrics/bin/utils/unit-test-coverage/coverage_guard.py
Expand Down
4 changes: 2 additions & 2 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ linters-settings:
- name: line-length-limit
severity: warning
disabled: true
arguments: [ 120 ]
arguments: [120]
funlen:
statements: 50
lines: 80
Expand Down Expand Up @@ -102,7 +102,7 @@ issues:
- funlen # Table driven unit and integration tests exceed function length by design
- maintidx # Table driven unit and integration tests exceed maintainability index by design
- path: tests/e2e/
linters: [ gci ] # Disable gci due to the test utilities dot import.
linters: [gci] # Disable gci due to the test utilities dot import.
- linters:
- importas
text: has alias "" which is not part of config # Ignore false positives that emerged due to https://github.com/julz/importas/issues/15.
Expand Down
6 changes: 5 additions & 1 deletion cmd/modulectl/create/long.txt
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@ The module config file is a YAML file used to configure the following attributes
- name: a string, required, the name of the module
- version: a string, required, the version of the module
- manifest: a string, required, reference to the manifest, must be a URL

- repository: a string, required, reference to the repository, must be a URL
- documentation: a string, required, reference to the documentation, must be a URL
- icons: a map with string keys and values, required, icons used for UI
- name: a string, required, the name of the icon
link: a URL, required, the link to the icon
- 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
- internal: a boolean, optional, default=false, determines whether the ModuleTemplate CR should have the internal flag or not
Expand Down
6 changes: 5 additions & 1 deletion docs/gen-docs/modulectl_create.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@ The module config file is a YAML file used to configure the following attributes
- name: a string, required, the name of the module
- version: a string, required, the version of the module
- manifest: a string, required, reference to the manifest, must be a URL

- repository: a string, required, reference to the repository, must be a URL
- documentation: a string, required, reference to the documentation, must be a URL
- icons: a map with string keys and values, required, icons used for UI
- name: a string, required, the name of the icon
link: a URL, required, the link to the icon
- 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
- internal: a boolean, optional, default=false, determines whether the ModuleTemplate CR should have the internal flag or not
Expand Down
5 changes: 2 additions & 3 deletions internal/common/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"github.com/Masterminds/semver/v3"

commonerrors "github.com/kyma-project/modulectl/internal/common/errors"
"github.com/kyma-project/modulectl/internal/service/contentprovider"
)

const (
Expand Down Expand Up @@ -81,8 +80,8 @@ func ValidateNamespace(namespace string) error {
return nil
}

func ValidateResources(resources contentprovider.ResourcesMap) error {
for name, link := range resources {
func ValidateMapEntries(nameLinkMap map[string]string) error {
for name, link := range nameLinkMap {
if name == "" {
return fmt.Errorf("%w: name must not be empty", commonerrors.ErrInvalidOption)
}
Expand Down
17 changes: 8 additions & 9 deletions internal/common/validation/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"testing"

"github.com/kyma-project/modulectl/internal/common/validation"
"github.com/kyma-project/modulectl/internal/service/contentprovider"
)

func TestValidateModuleName(t *testing.T) {
Expand Down Expand Up @@ -228,47 +227,47 @@ func TestValidateGvk(t *testing.T) {
}
}

func TestValidateResources(t *testing.T) {
func TestValidateMapEntries(t *testing.T) {
tests := []struct {
name string
resources contentprovider.ResourcesMap
resources map[string]string
wantErr bool
}{
{
name: "valid resources",
resources: contentprovider.ResourcesMap{
resources: map[string]string{
"first": "https://github.com/kyma-project/template-operator/releases/download/1.0.1/template-operator.yaml",
"second": "https://github.com/kyma-project/template-operator/releases/download/1.0.1/template-operator.yaml",
},
wantErr: false,
},
{
name: "empty name",
resources: contentprovider.ResourcesMap{
resources: map[string]string{
"": "https://github.com/kyma-project/template-operator/releases/download/1.0.1/template-operator.yaml",
},
wantErr: true,
},
{
name: "empty link",
resources: contentprovider.ResourcesMap{
resources: map[string]string{
"first": "",
},
wantErr: true,
},
{
name: "non-https schema",
resources: contentprovider.ResourcesMap{
resources: map[string]string{
"first": "http://github.com/kyma-project/template-operator/releases/download/1.0.1/template-operator.yaml",
},
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if err := validation.ValidateResources(tt.resources); (err != nil) != tt.wantErr {
if err := validation.ValidateMapEntries(tt.resources); (err != nil) != tt.wantErr {
fmt.Println(err.Error())
t.Errorf("ValidateResources() error = %v, wantErr %v", err, tt.wantErr)
t.Errorf("ValidateMapEntries() error = %v, wantErr %v", err, tt.wantErr)
}
})
}
Expand Down
94 changes: 67 additions & 27 deletions internal/service/contentprovider/moduleconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
"github.com/kyma-project/modulectl/internal/common/types"
)

var ErrDuplicateResourceNames = errors.New("resources contain duplicate entries")
var ErrDuplicateMapEntries = errors.New("map contains duplicate entries")

type ModuleConfigProvider struct {
yamlConverter ObjectToYAMLConverter
Expand Down Expand Up @@ -66,16 +66,13 @@ func (s *ModuleConfigProvider) validateArgs(args types.KeyValueArgs) error {
return nil
}

type Manager struct {
Name string `yaml:"name" comment:"required, the name of the manager"`
Namespace string `yaml:"namespace" comment:"optional, the path to the manager"`
metav1.GroupVersionKind `yaml:",inline" comment:"required, the GVK of the manager"`
}

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"`
Manifest string `yaml:"manifest" comment:"required, relative path or remote URL to the manifests"`
Repository string `yaml:"repository" comment:"required, link to the repository"`
Documentation string `yaml:"documentation" comment:"required, link to documentation"`
Icons Icons `yaml:"icons,omitempty" comment:"required, list of icons to represent the module in the UI"`
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"`
Namespace string `yaml:"namespace" comment:"optional, default=kcp-system, the namespace where the ModuleTemplate will be deployed"`
Expand All @@ -86,38 +83,81 @@ type ModuleConfig struct {
Annotations map[string]string `yaml:"annotations" comment:"optional, additional annotations for the ModuleTemplate"`
AssociatedResources []*metav1.GroupVersionKind `yaml:"associatedResources" comment:"optional, GVK of the resources which are associated with the module and have to be deleted with module deletion"`
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"`
Resources Resources `yaml:"resources,omitempty" comment:"optional, additional resources of the ModuleTemplate that may be fetched"`
}

type resource struct {
Name string `yaml:"name"`
Link string `yaml:"link"`
type Manager struct {
Name string `yaml:"name" comment:"required, the name of the manager"`
Namespace string `yaml:"namespace" comment:"optional, the path to the manager"`
metav1.GroupVersionKind `yaml:",inline" comment:"required, the GVK of the manager"`
}

// Icons represents a map of icon names to links.
type Icons map[string]string

// UnmarshalYAML unmarshals Icons from YAML format.
func (i *Icons) UnmarshalYAML(unmarshal func(interface{}) error) error {
dataMap, err := unmarshalToMap(unmarshal)
if err != nil {
return fmt.Errorf("failed to unmarshal Icons: %w", err)
}
*i = dataMap
return nil
}

type ResourcesMap map[string]string
// MarshalYAML marshals Icons to YAML format.
func (i *Icons) MarshalYAML() (interface{}, error) {
return marshalFromMap(*i)
}

func (rm *ResourcesMap) UnmarshalYAML(unmarshal func(interface{}) error) error {
resources := []resource{}
if err := unmarshal(&resources); err != nil {
return err
// Resources represents a map of resource names to links.
type Resources map[string]string

// UnmarshalYAML unmarshals Resources from YAML format.
func (rm *Resources) UnmarshalYAML(unmarshal func(interface{}) error) error {
dataMap, err := unmarshalToMap(unmarshal)
if err != nil {
return fmt.Errorf("failed to unmarshal Resources: %w", err)
}
*rm = dataMap
return nil
}

*rm = make(map[string]string)
for _, resource := range resources {
(*rm)[resource.Name] = resource.Link
// MarshalYAML marshals Resources to YAML format.
func (rm *Resources) MarshalYAML() (interface{}, error) {
return marshalFromMap(*rm)
}

func unmarshalToMap(unmarshal func(interface{}) error) (map[string]string, error) {
var items []nameLinkItem
if err := unmarshal(&items); err == nil {
resultMap := make(map[string]string)
for _, item := range items {
if _, exists := resultMap[item.Name]; exists {
return nil, ErrDuplicateMapEntries
}
resultMap[item.Name] = item.Link
}
return resultMap, nil
}

if len(resources) > len(*rm) {
return ErrDuplicateResourceNames
resultMap := make(map[string]string)
if err := unmarshal(&resultMap); err != nil {
return nil, err
}

return nil
return resultMap, nil
}

func (rm ResourcesMap) MarshalYAML() (interface{}, error) {
resources := []resource{}
for name, link := range rm {
resources = append(resources, resource{Name: name, Link: link})
func marshalFromMap(dataMap map[string]string) (interface{}, error) {
items := make([]nameLinkItem, 0, len(dataMap))
for name, link := range dataMap {
items = append(items, nameLinkItem{Name: name, Link: link})
}
return resources, nil
return items, nil
}

type nameLinkItem struct {
Name string `yaml:"name"`
Link string `yaml:"link"`
}
83 changes: 81 additions & 2 deletions internal/service/contentprovider/moduleconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,85 @@ func Test_ModuleConfig_GetDefaultContent_ReturnsConvertedContent(t *testing.T) {
assert.Equal(t, mcConvertedContent, result)
}

func Test_ModuleConfig_Unmarshal_Icons_Success(t *testing.T) {
moduleConfigData := `
icons:
- name: icon1
link: https://example.com/icon1
- name: icon2
link: https://example.com/icon2
`

moduleConfig := &contentprovider.ModuleConfig{}
err := yaml.Unmarshal([]byte(moduleConfigData), moduleConfig)

require.NoError(t, err)
assert.Len(t, moduleConfig.Icons, 2)
assert.Equal(t, "https://example.com/icon1", moduleConfig.Icons["icon1"])
assert.Equal(t, "https://example.com/icon2", moduleConfig.Icons["icon2"])
}

func Test_ModuleConfig_Unmarshal_Icons_Success_Ignoring_Unknown_Fields(t *testing.T) {
moduleConfigData := `
icons:
- name: icon
link: https://example.com/icon
unknown: something
`

moduleConfig := &contentprovider.ModuleConfig{}
err := yaml.Unmarshal([]byte(moduleConfigData), moduleConfig)

require.NoError(t, err)
assert.Len(t, moduleConfig.Icons, 1)
assert.Equal(t, "https://example.com/icon", moduleConfig.Icons["icon"])
}

func Test_ModuleConfig_Unmarshal_Icons_FailOnDuplicateNames(t *testing.T) {
moduleConfigData := `
icons:
- name: icon1
link: https://example.com/icon1
- name: icon1
link: https://example.com/icon2
`

moduleConfig := &contentprovider.ModuleConfig{}
err := yaml.Unmarshal([]byte(moduleConfigData), moduleConfig)

require.Error(t, err)
assert.Equal(t, "failed to unmarshal Icons: map contains duplicate entries", err.Error())
}

func Test_ModuleConfig_Marshal_Icons_Success(t *testing.T) {
// parse the expected config
expectedModuleConfigData := `
icons:
- name: icon1
link: https://example.com/icon1
- name: icon2
link: https://example.com/icon2
`
expectedModuleConfig := &contentprovider.ModuleConfig{}
err := yaml.Unmarshal([]byte(expectedModuleConfigData), expectedModuleConfig)
require.NoError(t, err)

moduleConfig := &contentprovider.ModuleConfig{
Icons: contentprovider.Icons{
"icon1": "https://example.com/icon1",
"icon2": "https://example.com/icon2",
},
}
marshalledModuleConfigData, err := yaml.Marshal(moduleConfig)
require.NoError(t, err)

marshalledModuleConfig := &contentprovider.ModuleConfig{}
err = yaml.Unmarshal(marshalledModuleConfigData, marshalledModuleConfig)

require.NoError(t, err)
assert.Equal(t, expectedModuleConfig.Icons, marshalledModuleConfig.Icons)
}

func Test_ModuleConfig_Unmarshall_Resources_Success(t *testing.T) {
moduleConfigData := `
resources:
Expand Down Expand Up @@ -162,7 +241,7 @@ resources:
err := yaml.Unmarshal([]byte(moduleConfigData), moduleConfig)

require.Error(t, err)
assert.Equal(t, "resources contain duplicate entries", err.Error())
assert.Equal(t, "failed to unmarshal Resources: map contains duplicate entries", err.Error())
}

func Test_ModuleConfig_Marshall_Resources_Success(t *testing.T) {
Expand All @@ -180,7 +259,7 @@ resources:

// round trip a module config (marshal and unmarshal)
moduleConfig := &contentprovider.ModuleConfig{
Resources: contentprovider.ResourcesMap{
Resources: contentprovider.Resources{
"resource1": "https://example.com/resource1",
"resource2": "https://example.com/resource2",
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ func (*fileExistsStub) ReadFile(_ string) ([]byte, error) {
Kind: "Deployment",
},
},
Resources: contentprovider.ResourcesMap{},
Resources: contentprovider.Resources{},
}

return yaml.Marshal(moduleConfig)
Expand Down
Loading

0 comments on commit 17493ed

Please sign in to comment.