Skip to content

Commit 1f804a2

Browse files
aFlyBird0aFlyBird0
authored
Merge pull request #1265 from merico-dev/bugfix2-ht
fix: greedy matching problem of helm-installer plugin
2 parents 7196ad9 + d31c4fb commit 1f804a2

File tree

7 files changed

+76
-40
lines changed

7 files changed

+76
-40
lines changed

‎internal/pkg/plugin/helminstaller/create.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,13 @@ func Create(options configmanager.RawOptions) (statemanager.ResourceStatus, erro
1212
// Initialize Operator with Operations
1313
operator := &installer.Operator{
1414
PreExecuteOperations: installer.PreExecuteOperations{
15-
RenderDefaultConfig,
16-
RenderValuesYaml,
15+
renderDefaultConfig,
16+
renderValuesYaml,
1717
validate,
1818
},
1919
ExecuteOperations: helm.DefaultCreateOperations,
2020
TerminateOperations: helm.DefaultTerminateOperations,
21-
GetStatusOperation: IndexStatusGetterFunc(options),
21+
GetStatusOperation: indexStatusGetterFunc(options),
2222
}
2323

2424
// Execute all Operations in Operator

‎internal/pkg/plugin/helminstaller/defaults/defaultconfig.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,5 +5,7 @@ import (
55
"github.com/devstream-io/devstream/internal/pkg/plugin/installer/helm"
66
)
77

8-
var DefaultOptionsMap = make(map[string]*helm.Options)
9-
var StatusGetterFuncMap = make(map[string]installer.StatusGetterOperation)
8+
var (
9+
DefaultOptionsMap = make(map[string]*helm.Options)
10+
StatusGetterFuncMap = make(map[string]installer.StatusGetterOperation)
11+
)

‎internal/pkg/plugin/helminstaller/delete.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ func Delete(options configmanager.RawOptions) (bool, error) {
1010
// Initialize Operator with Operations
1111
operator := &installer.Operator{
1212
PreExecuteOperations: installer.PreExecuteOperations{
13-
RenderDefaultConfig,
14-
RenderValuesYaml,
13+
renderDefaultConfig,
14+
renderValuesYaml,
1515
validate,
1616
},
1717
ExecuteOperations: helm.DefaultDeleteOperations,

‎internal/pkg/plugin/helminstaller/helminstaller.go‎

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,14 @@ import (
1212
"github.com/devstream-io/devstream/pkg/util/types"
1313
)
1414

15-
func RenderDefaultConfig(options configmanager.RawOptions) (configmanager.RawOptions, error) {
15+
func renderDefaultConfig(options configmanager.RawOptions) (configmanager.RawOptions, error) {
1616
helmOptions, err := helm.NewOptions(options)
1717
if err != nil {
1818
return nil, err
1919
}
2020

2121
instanceID := helmOptions.InstanceID
22-
defaultHelmOptions := GetDefaultOptionsByInstanceID(instanceID)
22+
defaultHelmOptions := getDefaultOptionsByInstanceID(instanceID)
2323

2424
if defaultHelmOptions == nil {
2525
log.Debugf("Default config for %s wasn't found.", instanceID)
@@ -33,9 +33,9 @@ func RenderDefaultConfig(options configmanager.RawOptions) (configmanager.RawOpt
3333
return types.EncodeStruct(helmOptions)
3434
}
3535

36-
// RenderValuesYaml renders options.valuesYaml to options.chart.valuesYaml;
36+
// renderValuesYaml renders options.valuesYaml to options.chart.valuesYaml;
3737
// If options.valuesYaml don't contains ":", it should be a path like "./values.yaml", then read it and transfer to options.chart.valuesYaml
38-
func RenderValuesYaml(options configmanager.RawOptions) (configmanager.RawOptions, error) {
38+
func renderValuesYaml(options configmanager.RawOptions) (configmanager.RawOptions, error) {
3939
helmOptions, err := helm.NewOptions(options)
4040
if err != nil {
4141
return nil, err
@@ -64,15 +64,15 @@ func RenderValuesYaml(options configmanager.RawOptions) (configmanager.RawOption
6464
return types.EncodeStruct(helmOptions)
6565
}
6666

67-
func IndexStatusGetterFunc(options configmanager.RawOptions) installer.StatusGetterOperation {
67+
func indexStatusGetterFunc(options configmanager.RawOptions) installer.StatusGetterOperation {
6868
helmOptions, err := helm.NewOptions(options)
6969
if err != nil {
7070
// It's ok to return GetAllResourcesStatus here when err != nil.
7171
return helm.GetAllResourcesStatus
7272
}
7373

7474
instanceID := helmOptions.InstanceID
75-
statusGetterFunc := GetStatusGetterFuncByInstanceID(instanceID)
75+
statusGetterFunc := getStatusGetterFuncByInstanceID(instanceID)
7676

7777
if statusGetterFunc == nil {
7878
return helm.GetAllResourcesStatus
@@ -81,16 +81,36 @@ func IndexStatusGetterFunc(options configmanager.RawOptions) installer.StatusGet
8181
return statusGetterFunc
8282
}
8383

84-
func GetDefaultOptionsByInstanceID(instanceID string) *helm.Options {
85-
for name, options := range defaults.DefaultOptionsMap {
86-
if strings.Contains(instanceID, name+"-") {
87-
return options
84+
// e.g. instanceID="argocd-config-001", "argocd" and "argocd-config" both are supported helm charts,
85+
// then DefaultOptionsMap["argocd-config"] needs to be returned.
86+
func getDefaultOptionsByInstanceID(instanceID string) *helm.Options {
87+
// if string instanceID contains a name, that name is a matched name.
88+
// e.g. argocd-config-001 contains argocd and argocd-config, so the argocd and argocd-config both are matched name.
89+
var matchedNames = make([]string, 0)
90+
for name := range defaults.DefaultOptionsMap {
91+
if strings.HasPrefix(instanceID, name) {
92+
matchedNames = append(matchedNames, name)
8893
}
8994
}
90-
return nil
95+
96+
if len(matchedNames) == 1 {
97+
return defaults.DefaultOptionsMap[matchedNames[0]]
98+
}
99+
100+
return defaults.DefaultOptionsMap[getLongestMatchedName(matchedNames)]
101+
}
102+
103+
func getLongestMatchedName(nameList []string) string {
104+
var retStr string
105+
for _, name := range nameList {
106+
if len(name) > len(retStr) {
107+
retStr = name
108+
}
109+
}
110+
return retStr
91111
}
92112

93-
func GetStatusGetterFuncByInstanceID(instanceID string) installer.StatusGetterOperation {
113+
func getStatusGetterFuncByInstanceID(instanceID string) installer.StatusGetterOperation {
94114
for name, fn := range defaults.StatusGetterFuncMap {
95115
if strings.Contains(instanceID, name+"-") {
96116
return fn

‎internal/pkg/plugin/helminstaller/helminstaller_test.go‎

Lines changed: 29 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package helminstaller_test
1+
package helminstaller
22

33
import (
44
"os"
@@ -8,53 +8,52 @@ import (
88
. "github.com/onsi/gomega"
99

1010
"github.com/devstream-io/devstream/internal/pkg/configmanager"
11-
"github.com/devstream-io/devstream/internal/pkg/plugin/helminstaller"
1211
"github.com/devstream-io/devstream/internal/pkg/plugin/helminstaller/defaults"
1312
"github.com/devstream-io/devstream/internal/pkg/plugin/installer"
1413
"github.com/devstream-io/devstream/internal/pkg/plugin/installer/helm"
1514
"github.com/devstream-io/devstream/internal/pkg/statemanager"
1615
)
1716

1817
var _ = Describe("helm installer test", func() {
19-
Context("GetDefaultOptionsByInstanceID", func() {
18+
Context("getDefaultOptionsByInstanceID", func() {
2019
defaults.DefaultOptionsMap["foo"] = &helm.Options{
2120
ValuesYaml: "foo: bar",
2221
}
2322

2423
It("should exists", func() {
25-
opts := helminstaller.GetDefaultOptionsByInstanceID("foo-001")
24+
opts := getDefaultOptionsByInstanceID("foo-001")
2625
Expect(opts).NotTo(BeNil())
2726
Expect(opts.ValuesYaml).To(Equal("foo: bar"))
2827
})
2928

3029
It("should not exists", func() {
31-
optsNil := helminstaller.GetDefaultOptionsByInstanceID("fo-001")
30+
optsNil := getDefaultOptionsByInstanceID("fo-001")
3231
Expect(optsNil).To(BeNil())
3332
})
3433
})
3534

36-
Context("GetStatusGetterFuncByInstanceID", func() {
35+
Context("getStatusGetterFuncByInstanceID", func() {
3736
defaults.StatusGetterFuncMap = map[string]installer.StatusGetterOperation{
3837
"foo": func(options configmanager.RawOptions) (statemanager.ResourceStatus, error) {
3938
return nil, nil
4039
},
4140
}
4241

4342
It("should exists", func() {
44-
fn := helminstaller.GetStatusGetterFuncByInstanceID("foo-001")
43+
fn := getStatusGetterFuncByInstanceID("foo-001")
4544
Expect(fn).NotTo(BeNil())
4645
})
4746

4847
It("should not exists", func() {
49-
fn := helminstaller.GetStatusGetterFuncByInstanceID("fooo")
48+
fn := getStatusGetterFuncByInstanceID("fooo")
5049
Expect(fn).To(BeNil())
5150
})
5251
})
5352

54-
Context("RenderDefaultConfig", func() {
53+
Context("renderDefaultConfig", func() {
5554
opts := configmanager.RawOptions{}
5655
opts["instanceID"] = interface{}("argocd-001")
57-
newOpts, err := helminstaller.RenderDefaultConfig(opts)
56+
newOpts, err := renderDefaultConfig(opts)
5857
Expect(err).To(BeNil())
5958

6059
helmOpts, err := helm.NewOptions(newOpts)
@@ -64,11 +63,11 @@ var _ = Describe("helm installer test", func() {
6463
Expect(helmOpts.Repo.URL).To(Equal(defaults.DefaultConfigWithArgoCD.Repo.URL))
6564
})
6665

67-
Context("RenderValuesYaml", func() {
66+
Context("renderValuesYaml", func() {
6867
It("config with yaml", func() {
6968
opts := configmanager.RawOptions{}
7069
opts["valuesYaml"] = interface{}("foo: bar")
71-
newOpts, err := helminstaller.RenderValuesYaml(opts)
70+
newOpts, err := renderValuesYaml(opts)
7271
Expect(err).To(BeNil())
7372

7473
helmOpts, err := helm.NewOptions(newOpts)
@@ -83,7 +82,7 @@ var _ = Describe("helm installer test", func() {
8382

8483
opts := configmanager.RawOptions{}
8584
opts["valuesYaml"] = interface{}("./values.yaml")
86-
newOpts, err := helminstaller.RenderValuesYaml(opts)
85+
newOpts, err := renderValuesYaml(opts)
8786
Expect(err).To(BeNil())
8887

8988
helmOpts, err := helm.NewOptions(newOpts)
@@ -96,14 +95,29 @@ var _ = Describe("helm installer test", func() {
9695
})
9796
})
9897

99-
Context("IndexStatusGetterFunc", func() {
98+
Context("indexStatusGetterFunc", func() {
10099
opts1 := configmanager.RawOptions{
101100
"instanceID": interface{}("argocd-001"),
102101
}
103102

104103
defaults.StatusGetterFuncMap["argocd"] = defaults.GetArgoCDStatus
105104

106-
fn1 := helminstaller.IndexStatusGetterFunc(opts1)
105+
fn1 := indexStatusGetterFunc(opts1)
107106
Expect(reflect.ValueOf(fn1).Pointer()).To(Equal(reflect.ValueOf(defaults.GetArgoCDStatus).Pointer()))
108107
})
108+
109+
Context("getLongestMatchedName", func() {
110+
testList := []string{"abc", "abcd", "ab"}
111+
retStr := getLongestMatchedName(testList)
112+
Expect(retStr).To(Equal("abcd"))
113+
})
114+
115+
Context("getDefaultOptionsByInstanceID", func() {
116+
opt1 := getDefaultOptionsByInstanceID("argocd")
117+
opt2 := getDefaultOptionsByInstanceID("argocd-001")
118+
opt3 := getDefaultOptionsByInstanceID("argocd001")
119+
120+
Expect(opt1).To(Equal(opt2))
121+
Expect(opt2).To(Equal(opt3))
122+
})
109123
})

‎internal/pkg/plugin/helminstaller/read.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,11 @@ func Read(options configmanager.RawOptions) (statemanager.ResourceStatus, error)
1111
// Initialize Operator with Operations
1212
operator := &installer.Operator{
1313
PreExecuteOperations: installer.PreExecuteOperations{
14-
RenderDefaultConfig,
15-
RenderValuesYaml,
14+
renderDefaultConfig,
15+
renderValuesYaml,
1616
validate,
1717
},
18-
GetStatusOperation: IndexStatusGetterFunc(options),
18+
GetStatusOperation: indexStatusGetterFunc(options),
1919
}
2020

2121
// Execute all Operations in Operator

‎internal/pkg/plugin/helminstaller/update.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,13 @@ func Update(options configmanager.RawOptions) (statemanager.ResourceStatus, erro
1212
// Initialize Operator with Operations
1313
operator := &installer.Operator{
1414
PreExecuteOperations: installer.PreExecuteOperations{
15-
RenderDefaultConfig,
16-
RenderValuesYaml,
15+
renderDefaultConfig,
16+
renderValuesYaml,
1717
validate,
1818
},
1919
ExecuteOperations: helm.DefaultUpdateOperations,
2020
TerminateOperations: helm.DefaultTerminateOperations,
21-
GetStatusOperation: IndexStatusGetterFunc(options),
21+
GetStatusOperation: indexStatusGetterFunc(options),
2222
}
2323

2424
// Execute all Operations in Operator

0 commit comments

Comments
 (0)