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

feat: Support spec.info #90

Merged
merged 28 commits into from
Nov 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
3ca5019
Add e2e testcase
lindnerby Oct 17, 2024
660f017
Add testcase for missing
lindnerby Oct 17, 2024
82eee19
Merge branch 'main' into spec-info
lindnerby Oct 24, 2024
254ba60
delete unused field
lindnerby Oct 24, 2024
4f57fb6
add info
lindnerby Oct 24, 2024
e4ab025
fix merge fail in tests
lindnerby Oct 24, 2024
b740472
add to scaffold
lindnerby Oct 29, 2024
83b197f
add validation for info fields
lindnerby Oct 29, 2024
42f094c
remove info parent in moduleconfig
lindnerby Oct 30, 2024
6ef084b
fix validate
lindnerby Oct 30, 2024
4264d6d
Merge branch 'main' into spec-info
lindnerby Oct 30, 2024
040f4af
fix tests
lindnerby Oct 30, 2024
46ac668
lint yamls
lindnerby Oct 30, 2024
988d8fc
fix e2e testdata
lindnerby Oct 30, 2024
1345581
share marshalling and unmarshalling logic
lindnerby Oct 31, 2024
110189b
remove generated module-configs
lindnerby Oct 31, 2024
38bb423
make docs
lindnerby Oct 31, 2024
38d4c4c
delete unneeded testdata
lindnerby Nov 4, 2024
d6f69d8
Merge branch 'main' into spec-info
lindnerby Nov 4, 2024
9207f2c
add e2e testcases
lindnerby Nov 4, 2024
0d03363
add more testcases
lindnerby Nov 4, 2024
30831b0
adapt unit-test-
lindnerby Nov 4, 2024
b337b40
fix assertion
lindnerby Nov 4, 2024
1743ceb
fix assertion
lindnerby Nov 4, 2024
30867fc
fix assertion
lindnerby Nov 4, 2024
8d63535
add different error for parsing errors of Icons and Resources
lindnerby Nov 4, 2024
65221f7
add unit tests for Icons
lindnerby Nov 4, 2024
c337605
add e2e for malformed icons entries
lindnerby Nov 4, 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
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
Loading