Skip to content
This repository was archived by the owner on Jul 26, 2022. It is now read-only.

Commit ac9ff93

Browse files
varditnvarditn
authoredSep 5, 2019
Allow non packaged charts (#12)
* allow non packaged charts * Improve test units
1 parent 9615c37 commit ac9ff93

File tree

5 files changed

+172
-79
lines changed

5 files changed

+172
-79
lines changed
 

‎.gitignore

+1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ armador
77
*.dll
88
*.so
99
*.dylib
10+
*.out
1011

1112
# Test binary, build with `go test -c`
1213
*.test

‎internal/armador/chart.go

+40-57
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,42 @@ func (chart *Chart) processGit(cmd commands.Command, dirs commands.Dirs) error {
5959
return nil
6060
}
6161

62+
func digestFields(fields map[interface{}]interface{}) (retChart Chart) {
63+
retChart = Chart{}
64+
for name, vals := range fields {
65+
retChart.Name = name.(string)
66+
vals := vals.(map[interface{}]interface{})
67+
r, ok := vals["repo"].(string)
68+
if ok {
69+
retChart.Repo = r
70+
}
71+
v, ok := vals["version"].(string)
72+
if ok {
73+
retChart.Version = v
74+
}
75+
c, ok := vals["pathToChart"].(string)
76+
if ok {
77+
retChart.PathToChart = c
78+
}
79+
80+
p, ok := vals["packaged"].(bool)
81+
if ok {
82+
retChart.Packaged = p
83+
} else {
84+
retChart.Packaged = true
85+
}
86+
87+
var files []string
88+
overrideFiles, ok := vals["overrideValueFiles"].([]interface{})
89+
if ok {
90+
for _, f := range overrideFiles {
91+
files = append(files, f.(string))
92+
}
93+
}
94+
retChart.OverrideValueFiles = files
95+
}
96+
return retChart
97+
}
6298
func readFileToViper(configName, configPath string) (*viper.Viper, error) {
6399
new := viper.New()
64100
new.SetConfigName(configName) // name of config file (without extension)
@@ -90,29 +126,13 @@ func (chart *Chart) unmarshalArmadorConfig(vip *viper.Viper) {
90126
}
91127
chart.OverrideValueFiles = ovf
92128

93-
// TODO: add ability for non-pacakaged charts to be dependencies (ie: use same logic as in `getPrereqCharts()`)
94129
deps, ok := vip.Get("dependencies").([]interface{})
95130
if !ok {
96131
chart.Dependencies = []Chart{}
97132
}
98133
for _, d := range deps {
99-
dep, ok := d.(map[interface{}]interface{})
100-
if !ok {
101-
continue
102-
}
103-
for k, v := range dep {
104-
name := k.(string)
105-
extra := v.(map[interface{}]interface{})
106-
r, ok := extra["repo"].(string)
107-
if !ok {
108-
r = ""
109-
}
110-
v, ok := extra["version"].(string)
111-
if !ok {
112-
v = ""
113-
}
114-
chart.Dependencies = append(chart.Dependencies, Chart{Name: name, Repo: r, Version: v})
115-
}
134+
dep, _ := d.(map[interface{}]interface{})
135+
chart.Dependencies = append(chart.Dependencies, digestFields(dep))
116136
}
117137
}
118138

@@ -126,45 +146,8 @@ func getPrereqCharts() (retCharts []Chart, err error) {
126146

127147
prereqList := viper.Get("prereqCharts").([]interface{})
128148
for _, chart := range prereqList {
129-
for name, vals := range chart.(map[interface{}]interface{}) {
130-
vals := vals.(map[interface{}]interface{})
131-
nextChart := Chart{
132-
Name: name.(string),
133-
}
134-
135-
r, ok := vals["repo"].(string)
136-
if ok {
137-
nextChart.Repo = r
138-
}
139-
140-
v, ok := vals["version"].(string)
141-
if ok {
142-
nextChart.Version = v
143-
}
144-
145-
c, ok := vals["pathToChart"].(string)
146-
if ok {
147-
nextChart.PathToChart = c
148-
}
149-
150-
p, ok := vals["packaged"].(bool)
151-
if ok {
152-
nextChart.Packaged = p
153-
} else {
154-
nextChart.Packaged = true
155-
}
156-
157-
files := []string{}
158-
overrideFiles, ok := vals["overrideValueFiles"].([]interface{})
159-
if ok {
160-
for _, f := range overrideFiles {
161-
files = append(files, f.(string))
162-
}
163-
}
164-
nextChart.OverrideValueFiles = files
165-
166-
retCharts = append(retCharts, nextChart)
167-
}
149+
preq, _ := chart.(map[interface{}]interface{})
150+
retCharts = append(retCharts, digestFields(preq))
168151
}
169152

170153
return retCharts, err

‎internal/armador/chartList.go

+4-5
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,8 @@ func (charts *ChartList) processCharts(cmd commands.Command, depList map[string]
5252
if _, ok := filterDuplicates[n]; ok {
5353
continue
5454
}
55-
5655
chart := (*charts)[n]
56+
5757
// sync chart values with dependency info
5858
if dep.Name != "" && chart.Name == "" {
5959
chart.Name = dep.Name
@@ -62,11 +62,10 @@ func (charts *ChartList) processCharts(cmd commands.Command, depList map[string]
6262
if dep.Repo != "" {
6363
chart.Repo = dep.Repo
6464
}
65-
chart.Packaged = true
66-
if !dep.Packaged && dep.Repo != "" && dep.PathToChart != "" {
67-
chart.Packaged = false
68-
}
65+
chart.Packaged = dep.Packaged
66+
chart.PathToChart = dep.PathToChart
6967
}
68+
7069
if chart.ChartPath == "" {
7170
if chart.Packaged {
7271
chart.processHelm(cmd, dirs)

‎internal/armador/chartList_test.go

+116-13
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"reflect"
55
"testing"
66

7+
"github.com/go-test/deep"
78
"github.com/travelaudience/armador/internal/commands"
89
)
910

@@ -167,7 +168,6 @@ func TestChartList_processCharts(t *testing.T) {
167168
Tmp: commands.TmpDirs{Root: "tp/tmp/", Extracted: "tp/tmp/extracted", Hold: "tp/tmp/hold"},
168169
Cache: commands.CacheDirs{Root: "tp/cache/", Charts: "tp/cache/charts"},
169170
}
170-
filterDuplicates := make(map[string]struct{})
171171
cmd := commands.CmdMock{}
172172
tests := []struct {
173173
name string
@@ -184,46 +184,149 @@ func TestChartList_processCharts(t *testing.T) {
184184
{
185185
name: "no-dependecy",
186186
charts: &ChartList{
187-
"first-chart": Chart{Name: "first-chart", Repo: "stable", ChartPath: "../testData"},
188-
"secondchart": Chart{Name: "secondchart", Repo: "stable", ChartPath: "../testData"},
187+
"first-chart": Chart{Name: "first-chart", Repo: "stable", ChartPath: "../testData", Packaged: false},
188+
"secondchart": Chart{Name: "secondchart", Repo: "stable", ChartPath: "../testData", Packaged: false},
189189
},
190190
expectedCharts: &ChartList{
191-
"first-chart": Chart{Name: "first-chart", Repo: "stable", ChartPath: "../testData"},
192-
"secondchart": Chart{Name: "secondchart", Repo: "stable", ChartPath: "../testData"},
191+
"first-chart": Chart{Name: "first-chart", Repo: "stable", ChartPath: "../testData", Packaged: false},
192+
"secondchart": Chart{Name: "secondchart", Repo: "stable", ChartPath: "../testData", Packaged: false},
193193
},
194194
wantErr: false,
195195
},
196196
{
197197
name: "simple-dependecy",
198198
charts: &ChartList{
199-
"first-chart": Chart{Name: "first-chart", Repo: "stable", ChartPath: "../testData",
200-
Dependencies: []Chart{Chart{Name: "first-dep", Repo: "github", PathToChart: "helm/first-dep"}},
199+
"first-chart": Chart{Name: "first-chart", Repo: "stable", ChartPath: "../testData", Packaged: false,
200+
Dependencies: []Chart{Chart{Name: "first-dep", Repo: "github", PathToChart: "helm/first-dep", Packaged: false}},
201+
},
202+
"secondchart": Chart{Name: "secondchart", Repo: "stable", ChartPath: "../testData", Packaged: false},
203+
},
204+
expectedCharts: &ChartList{
205+
"first-chart": Chart{Name: "first-chart", Repo: "stable", ChartPath: "../testData", Packaged: false,
206+
Dependencies: []Chart{Chart{Name: "first-dep", Repo: "github", PathToChart: "helm/first-dep", Packaged: false}},
207+
},
208+
"first-dep": Chart{Name: "first-dep", Repo: "github", Packaged: false,
209+
ChartPath: "tp/cache/charts/first-dep/helm/first-dep", PathToChart: "helm/first-dep",
210+
},
211+
"secondchart": Chart{Name: "secondchart", Repo: "stable", ChartPath: "../testData", Packaged: false},
212+
},
213+
wantErr: false,
214+
},
215+
{
216+
name: "non-packaged-dependecy",
217+
charts: &ChartList{
218+
"first-chart": Chart{
219+
Name: "first-chart",
220+
Repo: "stable",
221+
ChartPath: "../testData",
222+
Dependencies: []Chart{Chart{Name: "first-dep", Repo: "github", Version: "1.0.0", Packaged: false, PathToChart: "helm/first-dep"}},
223+
Packaged: false,
224+
PathToChart: "helm/first-chart",
201225
},
202226
"secondchart": Chart{Name: "secondchart", Repo: "stable", ChartPath: "../testData"},
203227
},
204228
expectedCharts: &ChartList{
205-
"first-chart": Chart{Name: "first-chart", Repo: "stable", ChartPath: "../testData",
206-
Dependencies: []Chart{Chart{Name: "first-dep", Repo: "github", PathToChart: "helm/first-dep"}},
229+
"first-chart": Chart{
230+
Name: "first-chart",
231+
Repo: "stable",
232+
ChartPath: "../testData",
233+
Dependencies: []Chart{Chart{Name: "first-dep", Repo: "github", Version: "1.0.0", Packaged: false, PathToChart: "helm/first-dep"}},
234+
Packaged: false,
235+
PathToChart: "helm/first-chart",
207236
},
208-
"first-dep": Chart{Name: "first-dep", Repo: "github",
209-
ChartPath: "tp/cache/charts/first-dep", Packaged: false,
237+
"first-dep": Chart{Name: "first-dep", Repo: "github", Version: "1.0.0",
238+
ChartPath: "tp/cache/charts/first-dep/helm/first-dep", Packaged: false, PathToChart: "helm/first-dep",
210239
},
211240
"secondchart": Chart{Name: "secondchart", Repo: "stable", ChartPath: "../testData"},
212241
},
213242
wantErr: false,
214243
},
244+
{
245+
// TODO: Some of these charts should be changed to `packaged: true` once this will be fixed: https://github.com/travelaudience/armador/issues/19
246+
name: "duplicate-dependecy",
247+
charts: &ChartList{
248+
"first-chart": Chart{
249+
Name: "first-chart",
250+
Repo: "stable",
251+
ChartPath: "../basic",
252+
Dependencies: []Chart{
253+
Chart{
254+
Name: "second-chart",
255+
Repo: "stable",
256+
Packaged: false,
257+
},
258+
Chart{
259+
Name: "third-chart",
260+
Repo: "stable",
261+
Packaged: false,
262+
},
263+
Chart{
264+
Name: "dep-chart",
265+
Repo: "incubator",
266+
Packaged: false,
267+
},
268+
},
269+
Packaged: false,
270+
},
271+
},
272+
expectedCharts: &ChartList{
273+
"dep-chart": Chart{
274+
Name: "dep-chart",
275+
Repo: "incubator",
276+
ChartPath: "tp/cache/charts/dep-chart",
277+
Packaged: false,
278+
},
279+
"first-chart": Chart{
280+
Name: "first-chart",
281+
Repo: "stable",
282+
ChartPath: "../basic",
283+
Dependencies: []Chart{
284+
Chart{
285+
Name: "second-chart",
286+
Repo: "stable",
287+
Packaged: false,
288+
},
289+
Chart{
290+
Name: "third-chart",
291+
Repo: "stable",
292+
Packaged: false,
293+
},
294+
Chart{
295+
Name: "dep-chart",
296+
Repo: "incubator",
297+
Packaged: false,
298+
},
299+
},
300+
Packaged: false,
301+
},
302+
"second-chart": Chart{
303+
Name: "second-chart",
304+
Repo: "stable",
305+
ChartPath: "tp/cache/charts/second-chart",
306+
Packaged: false,
307+
},
308+
"third-chart": Chart{
309+
Name: "third-chart",
310+
Repo: "stable",
311+
ChartPath: "tp/cache/charts/third-chart",
312+
Packaged: false,
313+
},
314+
},
315+
wantErr: false,
316+
},
215317
}
216318
for _, tt := range tests {
217319
t.Run(tt.name, func(t *testing.T) {
218320
depList := tt.charts.flattenInitialChartsMap()
321+
filterDuplicates := make(map[string]struct{})
219322
err := tt.charts.processCharts(cmd, depList, dirs, filterDuplicates)
220323
if err != nil && !tt.wantErr {
221324
t.Errorf("unexpected test error for %s: %v", tt.name, err)
222325
} else if err == nil && tt.wantErr {
223326
t.Errorf("expected error, no error received for %s", tt.name)
224327
}
225-
if !reflect.DeepEqual(tt.expectedCharts, tt.charts) {
226-
t.Errorf("%s test failed. \nGot: %v \nExpected: %v", tt.name, tt.charts, tt.expectedCharts)
328+
if diff := deep.Equal(tt.charts, tt.expectedCharts); diff != nil {
329+
t.Errorf("%s test failed.\nDiff: %s \nGot: %v \nExpected: %v", tt.name, diff, tt.charts, tt.expectedCharts)
227330
}
228331

229332
})

‎internal/armador/chart_test.go

+11-4
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
package armador
22

33
import (
4-
"reflect"
54
"testing"
5+
6+
"github.com/go-test/deep"
67
)
78

89
func TestChart_parseArmadorFile(t *testing.T) {
@@ -22,7 +23,7 @@ func TestChart_parseArmadorFile(t *testing.T) {
2223
name: "basic-file",
2324
chart: Chart{Name: "parsed-example", Repo: "stable", ChartPath: "../testData/basic", OverrideValueFiles: nil},
2425
expected: Chart{Name: "parsed-example", Repo: "stable", ChartPath: "../testData/basic", OverrideValueFiles: []string{"values-test.yaml"},
25-
Dependencies: []Chart{Chart{Name: "dep-chart", Repo: "test-stable", Version: "3.5.4"}},
26+
Dependencies: []Chart{Chart{Name: "dep-chart", Repo: "test-stable", Version: "3.5.4", Packaged: true}},
2627
},
2728
wantErr: false,
2829
},
@@ -32,6 +33,12 @@ func TestChart_parseArmadorFile(t *testing.T) {
3233
expected: Chart{Name: "parsed-example", Repo: "stable", ChartPath: "../testData/invalid"},
3334
wantErr: true,
3435
},
36+
{
37+
name: "non-packaged-chart",
38+
chart: Chart{Name: "first", Repo: "stable", Packaged: false, PathToChart: "./chart"},
39+
expected: Chart{Name: "first", Repo: "stable", Packaged: false, PathToChart: "./chart"},
40+
wantErr: false,
41+
},
3542
}
3643
for _, tt := range cases {
3744
t.Run(tt.name, func(t *testing.T) {
@@ -41,8 +48,8 @@ func TestChart_parseArmadorFile(t *testing.T) {
4148
} else if err == nil && tt.wantErr {
4249
t.Errorf("expected error, no error received for %s", tt.name)
4350
}
44-
if !reflect.DeepEqual(tt.chart, tt.expected) {
45-
t.Errorf("%s test failed. \nGot: %v \nExpected: %v", tt.name, tt.chart, tt.expected)
51+
if diff := deep.Equal(tt.chart, tt.expected); diff != nil {
52+
t.Errorf("%s test failed.\nDiff: %s \nGot: \n%++v \nExpected: \n%++v", tt.name, diff, tt.chart, tt.expected)
4653
}
4754
})
4855
}

0 commit comments

Comments
 (0)
This repository has been archived.