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

Fix: fail to get global helm config secret #867

Merged
merged 5 commits into from
Aug 2, 2023
Merged
Show file tree
Hide file tree
Changes from 3 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
50 changes: 31 additions & 19 deletions pkg/server/domain/service/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,17 @@
ListConfigDistributions(ctx context.Context, project string) ([]*config.Distribution, error)
}

var (
// GlobalConfigNamespace is the namespace for global config, global config should be seen by all projects
GlobalConfigNamespace = types.DefaultKubeVelaNS
// NoProject is the project name to pass when query global config
NoProject = ""
)

func isGlobal(project string) bool {
return project == NoProject
}

// NewConfigService returns a config use case
func NewConfigService() ConfigService {
return &configServiceImpl{}
Expand All @@ -62,8 +73,8 @@

// ListTemplates list the config templates
func (u *configServiceImpl) ListTemplates(ctx context.Context, project, scope string) ([]*apis.ConfigTemplate, error) {
listCtx := utils.WithProject(ctx, "")
queryTemplates, err := u.Factory.ListTemplates(listCtx, types.DefaultKubeVelaNS, scope)
listCtx := utils.WithProject(ctx, NoProject)
queryTemplates, err := u.Factory.ListTemplates(listCtx, GlobalConfigNamespace, scope)

Check warning on line 77 in pkg/server/domain/service/config.go

View check run for this annotation

Codecov / codecov/patch

pkg/server/domain/service/config.go#L76-L77

Added lines #L76 - L77 were not covered by tests
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -99,9 +110,9 @@
// GetTemplate detail a template
func (u *configServiceImpl) GetTemplate(ctx context.Context, tem config.NamespacedName) (*apis.ConfigTemplateDetail, error) {
if tem.Namespace == "" {
tem.Namespace = types.DefaultKubeVelaNS
tem.Namespace = GlobalConfigNamespace
}
getCtx := utils.WithProject(ctx, "")
getCtx := utils.WithProject(ctx, NoProject)
template, err := u.Factory.LoadTemplate(getCtx, tem.Name, tem.Namespace)
if err != nil {
if errors.Is(err, config.ErrTemplateNotFound) {
Expand All @@ -128,8 +139,8 @@
}

func (u *configServiceImpl) CreateConfig(ctx context.Context, project string, req apis.CreateConfigRequest) (*apis.Config, error) {
ns := types.DefaultKubeVelaNS
if project != "" {
ns := GlobalConfigNamespace
if !isGlobal(project) {
pro, err := u.ProjectService.GetProject(ctx, project)
if err != nil {
return nil, err
Expand All @@ -138,7 +149,7 @@
}
exist, err := u.Factory.IsExist(ctx, ns, req.Name)
if err != nil {
klog.Errorf("check config name is exist failure %s", err.Error())
klog.Errorf("check config name exist fail %s", err.Error())

Check warning on line 152 in pkg/server/domain/service/config.go

View check run for this annotation

Codecov / codecov/patch

pkg/server/domain/service/config.go#L152

Added line #L152 was not covered by tests
return nil, bcode.ErrConfigExist
}
if exist {
Expand All @@ -149,7 +160,7 @@
return nil, err
}
if req.Template.Namespace == "" {
req.Template.Namespace = types.DefaultKubeVelaNS
req.Template.Namespace = GlobalConfigNamespace
}
configItem, err := u.Factory.ParseConfig(ctx, config.NamespacedName(req.Template), config.Metadata{
NamespacedName: config.NamespacedName{Name: req.Name, Namespace: ns},
Expand All @@ -169,8 +180,8 @@
}

func (u *configServiceImpl) UpdateConfig(ctx context.Context, project string, name string, req apis.UpdateConfigRequest) (*apis.Config, error) {
ns := types.DefaultKubeVelaNS
if project != "" {
ns := GlobalConfigNamespace
if !isGlobal(project) {

Check warning on line 184 in pkg/server/domain/service/config.go

View check run for this annotation

Codecov / codecov/patch

pkg/server/domain/service/config.go#L183-L184

Added lines #L183 - L184 were not covered by tests
pro, err := u.ProjectService.GetProject(ctx, project)
if err != nil {
return nil, err
Expand Down Expand Up @@ -217,8 +228,8 @@
var list []*apis.Config
scope := ""
var projectNamespace string
listCtx := utils.WithProject(ctx, "")
if project != "" {
listCtx := utils.WithProject(ctx, NoProject)
if !isGlobal(project) {
scope = "project"
pro, err := u.ProjectService.GetProject(ctx, project)
if err != nil {
Expand All @@ -235,7 +246,7 @@
}
}

configs, err := u.Factory.ListConfigs(listCtx, types.DefaultKubeVelaNS, template, scope, true)
configs, err := u.Factory.ListConfigs(listCtx, GlobalConfigNamespace, template, scope, true)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -283,7 +294,7 @@
})
}

// ListDistributeConfigs list the all distributions
// ListConfigDistributions list the all distributions
func (u *configServiceImpl) ListConfigDistributions(ctx context.Context, project string) ([]*config.Distribution, error) {
pro, err := u.ProjectService.GetProject(ctx, project)
if err != nil {
Expand Down Expand Up @@ -324,8 +335,8 @@
}

func (u *configServiceImpl) GetConfig(ctx context.Context, project, name string) (*apis.Config, error) {
ns := types.DefaultKubeVelaNS
if project != "" {
ns := GlobalConfigNamespace
if !isGlobal(project) {
pro, err := u.ProjectService.GetProject(ctx, project)
if err != nil {
return nil, err
Expand All @@ -339,7 +350,8 @@
return nil, bcode.ErrSensitiveConfig
}
if errors.Is(err, config.ErrConfigNotFound) {
return nil, bcode.ErrConfigNotFound
// Try to get global config
return u.GetConfig(ctx, NoProject, name)
}
return nil, err
}
Expand All @@ -348,8 +360,8 @@
}

func (u *configServiceImpl) DeleteConfig(ctx context.Context, project, name string) error {
ns := types.DefaultKubeVelaNS
if project != "" {
ns := GlobalConfigNamespace
if !isGlobal(project) {
pro, err := u.ProjectService.GetProject(ctx, project)
if err != nil {
return err
Expand Down
102 changes: 99 additions & 3 deletions pkg/server/domain/service/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,69 @@ template: {
}
}
`
var (
helmTemplateName = "helm-repository"
helmTemplate = `
import (
"vela/config"
)

metadata: {
name: "helm-repository"
alias: "Helm Repository"
description: "Config information to authenticate helm chart repository"
sensitive: false
scope: "project"
}

template: {
output: {
apiVersion: "v1"
kind: "Secret"
metadata: {
name: context.name
namespace: context.namespace
labels: {
"config.oam.dev/catalog": "velacore-config"
"config.oam.dev/type": "helm-repository"
"config.oam.dev/multi-cluster": "true"
"config.oam.dev/sub-type": "helm"
}
}
type: "Opaque"
stringData: {
url: parameter.url
if parameter.username != _|_ {
username: parameter.username
}
if parameter.password != _|_ {
password: parameter.password
}

}
data: {
if parameter.caFile != _|_ {
caFile: parameter.caFile
}
}
}
// skip the validation here for config.#HelmRepository requires the repository actually exists which can be flaky in unit test
//validation: config.#HelmRepository & {
// $params: parameter
//}

parameter: {
// +usage=The public url of the helm chart repository.
url: string
// +usage=The username of basic auth repo.
username?: string
// +usage=The password of basic auth repo.
password?: string
// +usage=The ca certificate of helm repository. Please encode this data with base64.
caFile?: string
}
}`
)

var _ = Describe("Test config service", func() {
var factory config.Factory
Expand Down Expand Up @@ -180,9 +243,42 @@ var _ = Describe("Test config service", func() {
Expect(len(list)).To(Equal(0))
})

It("Test detail a config", func() {
_, err := configService.GetConfig(context.TODO(), "", "alibaba-test")
Expect(err).To(Equal(bcode.ErrSensitiveConfig))
Context("Test get config", func() {
It("Simple get", func() {
_, err := configService.GetConfig(context.TODO(), "", "alibaba-test")
Expect(err).To(Equal(bcode.ErrSensitiveConfig))
})

It("Get config in project and fall back to get global config", func() {
By("apply helm template")
tem, err := factory.ParseTemplate(helmTemplateName, []byte(helmTemplate))
Expect(err).To(BeNil())
Expect(factory.CreateOrUpdateConfigTemplate(context.Background(), types.DefaultKubeVelaNS, tem)).To(BeNil())

By("create a project")
_, err = projectService.CreateProject(context.TODO(), v1.CreateProjectRequest{Name: "some-project"})
Expect(err).To(BeNil())
defer func() {
Expect(projectService.DeleteProject(context.Background(), "some-project")).To(BeNil())
}()
By("create a common global config")
_, err = configService.CreateConfig(context.TODO(), NoProject, v1.CreateConfigRequest{
Name: "helm-test",
Template: v1.NamespacedName{
Name: helmTemplateName,
},
Properties: `{"username":"test","password":"test","url":"https://helm.kubevela.com/charts"}`,
})
Expect(err).To(BeNil())
defer func() {
Expect(configService.DeleteConfig(context.Background(), NoProject, "helm-test")).To(BeNil())
}()
By("try to get the config in project, should success")
config, err := configService.GetConfig(context.TODO(), "some-project", "helm-test")
Expect(err).To(BeNil())
Expect(config.Name).To(Equal("helm-test"))
})

})

It("Test delete a config", func() {
Expand Down
6 changes: 3 additions & 3 deletions pkg/server/domain/service/helm.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,10 +175,10 @@
url, ok := item.Properties["url"].(string)
if ok {
res = append(res, &v1.ChartRepoResponse{URL: url, SecretName: item.Name})
continue

Check warning on line 178 in pkg/server/domain/service/helm.go

View check run for this annotation

Codecov / codecov/patch

pkg/server/domain/service/helm.go#L178

Added line #L178 was not covered by tests
}
}
// Compatible with historical data
if url, ok := item.Secret.Data["url"]; ok {
} else if url, ok := item.Secret.Data["url"]; ok {
// Compatible with historical data
res = append(res, &v1.ChartRepoResponse{URL: string(url), SecretName: item.Name})
}
}
Expand Down
Loading