From 27ebfa8c561e758b60872b9bb081253036e559ff Mon Sep 17 00:00:00 2001 From: Thomas FREYSS Date: Thu, 23 Apr 2020 14:47:08 +0200 Subject: [PATCH 1/7] fix(*): remove bom in utf files when loading chart files (#6081) Removes the BOM prefix if present, in read files before processing the data. Affects the following pkg: - pkg/chart/loader: directory and archive loader - internal/ignore: when loading .helmignore file Signed-off-by: Thomas FREYSS --- internal/ignore/rules.go | 13 +++++- pkg/chart/loader/archive.go | 4 +- pkg/chart/loader/directory.go | 5 +++ pkg/chart/loader/load_test.go | 40 ++++++++++++++++++ .../loader/testdata/frobnitz_with_bom.tgz | Bin 0 -> 3523 bytes .../testdata/frobnitz_with_bom/.helmignore | 1 + .../testdata/frobnitz_with_bom/Chart.lock | 8 ++++ .../testdata/frobnitz_with_bom/Chart.yaml | 27 ++++++++++++ .../testdata/frobnitz_with_bom/INSTALL.txt | 1 + .../loader/testdata/frobnitz_with_bom/LICENSE | 1 + .../testdata/frobnitz_with_bom/README.md | 11 +++++ .../frobnitz_with_bom/charts/_ignore_me | 1 + .../charts/alpine/Chart.yaml | 5 +++ .../frobnitz_with_bom/charts/alpine/README.md | 9 ++++ .../charts/alpine/charts/mast1/Chart.yaml | 5 +++ .../charts/alpine/charts/mast1/values.yaml | 4 ++ .../charts/alpine/charts/mast2-0.1.0.tgz | Bin 0 -> 252 bytes .../charts/alpine/templates/alpine-pod.yaml | 14 ++++++ .../charts/alpine/values.yaml | 2 + .../charts/mariner-4.3.2.tgz | Bin 0 -> 967 bytes .../testdata/frobnitz_with_bom/docs/README.md | 1 + .../testdata/frobnitz_with_bom/icon.svg | 8 ++++ .../testdata/frobnitz_with_bom/ignore/me.txt | 0 .../frobnitz_with_bom/templates/template.tpl | 1 + .../testdata/frobnitz_with_bom/values.yaml | 6 +++ 25 files changed, 165 insertions(+), 2 deletions(-) create mode 100644 pkg/chart/loader/testdata/frobnitz_with_bom.tgz create mode 100644 pkg/chart/loader/testdata/frobnitz_with_bom/.helmignore create mode 100644 pkg/chart/loader/testdata/frobnitz_with_bom/Chart.lock create mode 100644 pkg/chart/loader/testdata/frobnitz_with_bom/Chart.yaml create mode 100644 pkg/chart/loader/testdata/frobnitz_with_bom/INSTALL.txt create mode 100644 pkg/chart/loader/testdata/frobnitz_with_bom/LICENSE create mode 100644 pkg/chart/loader/testdata/frobnitz_with_bom/README.md create mode 100644 pkg/chart/loader/testdata/frobnitz_with_bom/charts/_ignore_me create mode 100644 pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/Chart.yaml create mode 100644 pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/README.md create mode 100644 pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/charts/mast1/Chart.yaml create mode 100644 pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/charts/mast1/values.yaml create mode 100644 pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/charts/mast2-0.1.0.tgz create mode 100644 pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/templates/alpine-pod.yaml create mode 100644 pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/values.yaml create mode 100644 pkg/chart/loader/testdata/frobnitz_with_bom/charts/mariner-4.3.2.tgz create mode 100644 pkg/chart/loader/testdata/frobnitz_with_bom/docs/README.md create mode 100644 pkg/chart/loader/testdata/frobnitz_with_bom/icon.svg create mode 100644 pkg/chart/loader/testdata/frobnitz_with_bom/ignore/me.txt create mode 100644 pkg/chart/loader/testdata/frobnitz_with_bom/templates/template.tpl create mode 100644 pkg/chart/loader/testdata/frobnitz_with_bom/values.yaml diff --git a/internal/ignore/rules.go b/internal/ignore/rules.go index 9049aff0d..a80923baf 100644 --- a/internal/ignore/rules.go +++ b/internal/ignore/rules.go @@ -18,6 +18,7 @@ package ignore import ( "bufio" + "bytes" "io" "log" "os" @@ -65,8 +66,18 @@ func Parse(file io.Reader) (*Rules, error) { r := &Rules{patterns: []*pattern{}} s := bufio.NewScanner(file) + currentLine := 0 + utf8bom := []byte{0xEF, 0xBB, 0xBF} for s.Scan() { - if err := r.parseRule(s.Text()); err != nil { + scannedBytes := s.Bytes() + // We trim UTF8 BOM + if currentLine == 0 { + scannedBytes = bytes.TrimPrefix(scannedBytes, utf8bom) + } + line := string(scannedBytes) + currentLine++ + + if err := r.parseRule(line); err != nil { return r, err } } diff --git a/pkg/chart/loader/archive.go b/pkg/chart/loader/archive.go index 7e187a170..8b38cb89f 100644 --- a/pkg/chart/loader/archive.go +++ b/pkg/chart/loader/archive.go @@ -173,7 +173,9 @@ func LoadArchiveFiles(in io.Reader) ([]*BufferedFile, error) { return nil, err } - files = append(files, &BufferedFile{Name: n, Data: b.Bytes()}) + data := bytes.TrimPrefix(b.Bytes(), utf8bom) + + files = append(files, &BufferedFile{Name: n, Data: data}) b.Reset() } diff --git a/pkg/chart/loader/directory.go b/pkg/chart/loader/directory.go index a12c5158e..bbe543870 100644 --- a/pkg/chart/loader/directory.go +++ b/pkg/chart/loader/directory.go @@ -17,6 +17,7 @@ limitations under the License. package loader import ( + "bytes" "fmt" "io/ioutil" "os" @@ -30,6 +31,8 @@ import ( "helm.sh/helm/v3/pkg/chart" ) +var utf8bom = []byte{0xEF, 0xBB, 0xBF} + // DirLoader loads a chart from a directory type DirLoader string @@ -104,6 +107,8 @@ func LoadDir(dir string) (*chart.Chart, error) { return errors.Wrapf(err, "error reading %s", n) } + data = bytes.TrimPrefix(data, utf8bom) + files = append(files, &BufferedFile{Name: n, Data: data}) return nil } diff --git a/pkg/chart/loader/load_test.go b/pkg/chart/loader/load_test.go index 26513d359..3fa6bba0d 100644 --- a/pkg/chart/loader/load_test.go +++ b/pkg/chart/loader/load_test.go @@ -85,6 +85,38 @@ func TestLoadDirWithSymlink(t *testing.T) { verifyDependenciesLock(t, c) } +func TestLoadDirWithUTFBOM(t *testing.T) { + l, err := Loader("testdata/frobnitz_with_bom") + if err != nil { + t.Fatalf("Failed to load testdata: %s", err) + } + c, err := l.Load() + if err != nil { + t.Fatalf("Failed to load testdata: %s", err) + } + verifyFrobnitz(t, c) + verifyChart(t, c) + verifyDependencies(t, c) + verifyDependenciesLock(t, c) + verifyBomStripped(t, c.Files) +} + +func TestLoadArchiveWithUTFBOM(t *testing.T) { + l, err := Loader("testdata/frobnitz_with_bom.tgz") + if err != nil { + t.Fatalf("Failed to load testdata: %s", err) + } + c, err := l.Load() + if err != nil { + t.Fatalf("Failed to load testdata: %s", err) + } + verifyFrobnitz(t, c) + verifyChart(t, c) + verifyDependencies(t, c) + verifyDependenciesLock(t, c) + verifyBomStripped(t, c.Files) +} + func TestLoadV1(t *testing.T) { l, err := Loader("testdata/frobnitz.v1") if err != nil { @@ -465,3 +497,11 @@ func verifyChartFileAndTemplate(t *testing.T, c *chart.Chart, name string) { } } } + +func verifyBomStripped(t *testing.T, files []*chart.File) { + for _, file := range files { + if bytes.HasPrefix(file.Data, utf8bom) { + t.Errorf("Byte Order Mark still present in processed file %s", file.Name) + } + } +} diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom.tgz b/pkg/chart/loader/testdata/frobnitz_with_bom.tgz new file mode 100644 index 0000000000000000000000000000000000000000..be0cd027db34d392a875cf8151642f9922016dbd GIT binary patch literal 3523 zcmV;!4LtH6iwFpsexY6f17>n>Vs2@4dS7>GbZB2l` znyB~YxdYx5VfMWn1PB3&iueFgG=d@A)?MdMF5{FG{)R-w{q^;#sSRI4>wKcMw~Q(D5t@dOKi zA7!-`+)B<<`fn83E%ZMmB{eNBIT_~*crVotZ@BcY(W;T0Qmaxb{D8ts z_1_YIaQ)Y8DN8d^96jbvO|I-O_N{vddll(tl;3oR_>LlRN{?j%e|Fv3;H2(Vp zjr~7>TstE)F*Jz==xkxQcDaEBPcas0JpMhRCI6F$#3!btCVH_BkNj6Fn%jSBwZwm) z0P)|Y@W$+yxc)~$Oa8};_P(7k)84AX1OJsuWy|=lP9^c*7eM?ctW-K=IW)ybf;^?n zLYQGBaQ6!t2{|K6S$Q$J!BlXk z1%gDHgN?IT7z0Dvn`Gh`8*7BF8cjTJNEv94Q{pM$OwytaT-rzO|XDx3G{X%omY7TZ4=r_AR!ZE#RS*2Gl_&&Xd$7+NWmuW zhM=wJeTh%l@bVS75?5g2%?2O(#tjrbdahA{W`Y$I(5yrauEtfZD)CNtW(loE%kkf- zOK(O1+Tj1|)GEpU_XTdM|4iqyK&JWjSAjg)|HuGnHvc1OY5w;K-2OjjVr(=C3=lZ= zjxOzDXqILO+5!cF<_vI)XPD*)$no^E>` zp6q|ULSH}rYjnc+uh;05690XHKC=V-*8OGC6MpT_yp(I|Hu%rm*Cb8IC=QPas9e!g zE*rY@z}YiL{@3fl$KUr)4oMhwctFL(W9+cYPrd&7@VQucQrz6}pR6i=wfI7OW#=dR zl-zrtc6^sPYd)(k8v2F-3|d?H9;U1~b97$FKSwQ@U3~sXR(4Ic!?Eegz<*C4r|x>m zt||rr?GoFaT~&IvU+vNAeVx1X%}Bp@RQHolbm==PEh_f&>J_6O)jjjFpBDkmn}YS-*cw3 z@jJr|^B1b3Ws|CtP0PTEeA()Us))BgExwi;*e`x_yN?0~x9i%m=t@ce@nBbyyV$?0 zN`7t{{$$@b3$~`kc6w#~&Q7^I?`QqGO@H&ARkOg37tLSS_-*f(UDa!?HNHzi;B!@R zC7s`yIVgGXlzplx{rlL)Jd!zNzy;ZdJ3CG~Yc4smrTox)UnKV|sQg(ztTZ$r5rFF|=V;9}m_XP*lVf*QUz1}>X+1b{i>J`dw<{C%& z$ZNx%yFS0tv3p^MW6J_~{+iw6zhH_h*eQ|FG7EE2b$H52uYt`~^ zvE2*zC6Pt{JzVm)={wHV^hhWxKU}B@3i_da_#VT_x~wEe$+nlfZ@e73bniP?9Fa3; z7iL^sS>p(Oy`v#Eur}l8`Tm>c8l%gqHQ|@%oZK{?So%OropsFQQKx1q7f-IB>GHtv zsM=~vy1nZCi{C%J{o{?P+AAyLd%XAX?g{y7)!Oa{tM;#hpPpUa-8isn+LQ^E70btl z_KqKTsP_buvhZn)N3<#&3|#?>WUa^<`R4Q)!Kx=me@8v<~X?ZskK{c z_Z~aY>*DE66`xg9z=KCGUO!d>@wMumYp&1I>c2@g+o!Dgp!S0ek1hY<+>nUJ zA}>{ky-;>&UY$*IKyBN<^>l~Ay?ahg+g!G8rFFx?KABrCo>yP~c5cb~k*`Kv8NcLx zCUw``{YMW@I(_!EwXbCq_BdYh$imIfU!7L_Qqd(H~ z@yh46`}MmG{@--`cRHC~aRXX!|Lc?r$^Q2VJn7#X3xS^Oe~rF*{9muuO8dWj0=NC| z-tCc3a3BXF!;j#Rfp6m|OOEIU7#Xvfu#g~2h`#|N3sJf&5S4`sTS6en;vp&m-RI<4 zfTUPx6aq#lrx@Q8f`)kzedj2#A}d1z7CPf_KO|tr&sHw z{a-%8FVw%8;CN+QdqJ!9uTp4~YDxdTK&$lMMq|L^{IAwGoBvg6wG{vN32rw3iwbW# z{l_^Mc=o?qQBeGD*n5Cu+|O$p7^g)Hn;y(T4DZ|`2Xue zJpYl<(s>pYj?01@ZTZQF8=f4F(#!-e#0CT(To6crMw(!uxRu1&Ly-4QvB)iFgvCND z!ExeJA^K`Z-hRCU&Iy@?OduLyVm@iO@I6|=e^t0RxVL^S&=ddF`uhFHN1(&}Vi)H}S&J-){hsG+JRy8m>#WM`#V|bOo8JrN*a5=tiWc>xPe|pCl}#J#7>d zE!Y8mr}wTc{YcTx|4@bf`KY*>lKm58Kj8SYTPD9fI80+6^3}7&HTWQYRZ($T*F7J+ zri)+D{`h=t%!!h!Z#Mqp?3{NN2KvP=g=_B#8r!dKa^1XBL*EJ;5wc?A?r}9mPX#WG z$=$zymSW=ikmGM{=wH3;(^sdQEX6-@6h^H4uKXv>pV_Y?H=P(;-Ah)TXbkJFN|_&o z_XcD7f3jtQq4T513l+8z(@HxWnsoh{B|kg*e{rSo?{&|dFAp*vJQ0K*)N!O}Z>C?T zJN)AB5_sZ&Lrk~rb+p0%Qwsc-{J&oxzg+*#BI&KD!_)Y$Q>vQTe``h8bg=`*WbQW-Q`ECnKu76Z6TBGW30h*i3loto`e&nGC}wS$^6qbEcy zaS|p{jYl{)QM;DVBEbYqZlEl3j)u^Rg<%nh0@%bmWczyJe;ZxCCh)|6g+|kS{YRyh z`0o>l{-3bGDy+u}UD*}?!OZq>XBUvXvfndW@?SXEf^&H}-mAkC|FsS4zpmeZLzmL| zPoE%aV1b!-ANq;Lgfm0~V`NcC{{SUexJYyrjk(T0<>%+)`D&bDbL1+dH#thfoMN{W z5obSVX$3T8$-R-VG9n^EE~a9Qk0Ci*1oA18H$`JAtqwyIK9ytQU0*0N8bc-mkHw%| zQ9`&nCegKlL+Hm*a`BdsCTvF$#j`0E$%6t^0tW2jUsv%OMMLR2be&DnbTnom^6Z$r zX&D?ug_B|-O08Im#zQGAB!xnvgclNX7mcVb`R@&nuPIu;{)?;tjpYCN0=LqC{Z=X7 zdRu3KJ==fH&;ROmdMW-za0w`V9Evj@UhfyrdZ5E3^B z)JKRgaS#y?2D;KD|M70d&3^xbW{kPss>9>@pP~i-RT{m-e_sIc|2OwD6C{;`T)s#& x%AjG3BBk*~f&>W?BuJ1TL4pJc5+q2FAVGoz2@)hokRYKE{tus-sa61Z0036V1Lgn# literal 0 HcmV?d00001 diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/.helmignore b/pkg/chart/loader/testdata/frobnitz_with_bom/.helmignore new file mode 100644 index 000000000..7a4b92da2 --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_with_bom/.helmignore @@ -0,0 +1 @@ +ignore/ diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/Chart.lock b/pkg/chart/loader/testdata/frobnitz_with_bom/Chart.lock new file mode 100644 index 000000000..ed43b227f --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_with_bom/Chart.lock @@ -0,0 +1,8 @@ +dependencies: + - name: alpine + version: "0.1.0" + repository: https://example.com/charts + - name: mariner + version: "4.3.2" + repository: https://example.com/charts +digest: invalid diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/Chart.yaml b/pkg/chart/loader/testdata/frobnitz_with_bom/Chart.yaml new file mode 100644 index 000000000..21b21f0b5 --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_with_bom/Chart.yaml @@ -0,0 +1,27 @@ +apiVersion: v1 +name: frobnitz +description: This is a frobnitz. +version: "1.2.3" +keywords: + - frobnitz + - sprocket + - dodad +maintainers: + - name: The Helm Team + email: helm@example.com + - name: Someone Else + email: nobody@example.com +sources: + - https://example.com/foo/bar +home: http://example.com +icon: https://example.com/64x64.png +annotations: + extrakey: extravalue + anotherkey: anothervalue +dependencies: + - name: alpine + version: "0.1.0" + repository: https://example.com/charts + - name: mariner + version: "4.3.2" + repository: https://example.com/charts diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/INSTALL.txt b/pkg/chart/loader/testdata/frobnitz_with_bom/INSTALL.txt new file mode 100644 index 000000000..77c4e724a --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_with_bom/INSTALL.txt @@ -0,0 +1 @@ +This is an install document. The client may display this. diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/LICENSE b/pkg/chart/loader/testdata/frobnitz_with_bom/LICENSE new file mode 100644 index 000000000..c27b00bf2 --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_with_bom/LICENSE @@ -0,0 +1 @@ +LICENSE placeholder. diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/README.md b/pkg/chart/loader/testdata/frobnitz_with_bom/README.md new file mode 100644 index 000000000..e9c40031b --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_with_bom/README.md @@ -0,0 +1,11 @@ +# Frobnitz + +This is an example chart. + +## Usage + +This is an example. It has no usage. + +## Development + +For developer info, see the top-level repository. diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/charts/_ignore_me b/pkg/chart/loader/testdata/frobnitz_with_bom/charts/_ignore_me new file mode 100644 index 000000000..a7e3a38b7 --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_with_bom/charts/_ignore_me @@ -0,0 +1 @@ +This should be ignored by the loader, but may be included in a chart. diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/Chart.yaml b/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/Chart.yaml new file mode 100644 index 000000000..adb9853c6 --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/Chart.yaml @@ -0,0 +1,5 @@ +apiVersion: v1 +name: alpine +description: Deploy a basic Alpine Linux pod +version: 0.1.0 +home: https://helm.sh/helm diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/README.md b/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/README.md new file mode 100644 index 000000000..ea7526bee --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/README.md @@ -0,0 +1,9 @@ +This example was generated using the command `helm create alpine`. + +The `templates/` directory contains a very simple pod resource with a +couple of parameters. + +The `values.toml` file contains the default values for the +`alpine-pod.yaml` template. + +You can install this example using `helm install ./alpine`. diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/charts/mast1/Chart.yaml b/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/charts/mast1/Chart.yaml new file mode 100644 index 000000000..1ad84b346 --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/charts/mast1/Chart.yaml @@ -0,0 +1,5 @@ +apiVersion: v1 +name: mast1 +description: A Helm chart for Kubernetes +version: 0.1.0 +home: "" diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/charts/mast1/values.yaml b/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/charts/mast1/values.yaml new file mode 100644 index 000000000..f690d53c4 --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/charts/mast1/values.yaml @@ -0,0 +1,4 @@ +# Default values for mast1. +# This is a YAML-formatted file. +# Declare name/value pairs to be passed into your templates. +# name = "value" diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/charts/mast2-0.1.0.tgz b/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/charts/mast2-0.1.0.tgz new file mode 100644 index 0000000000000000000000000000000000000000..61cb62051110b55f3d08213dc81dcf0b1c2d8e53 GIT binary patch literal 252 zcmVDc zVQyr3R8em|NM&qo0PNJUs=_c72H?(liabH@pWIst-7YSIyL+rhEHrIN(t?QZE=F{y zgNRfS&$pa5Ly`mMk2OB%pV`*9knW7FlL-Joo@KED7*{C$d;N~z z37$S{+}wvSU9}|VtF|fRpv9Ve>8dWo|9?5B+RE}Y9CFh-x#(Bq8Vck^V=NUiPLCKa z8z5CF#JgK!4>;$4Fm+FUst4d+{(+nP|0&J+e}(;l^U4@w-{=?s0RR8vgVbLD3;+OM Cs&R<` literal 0 HcmV?d00001 diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/templates/alpine-pod.yaml b/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/templates/alpine-pod.yaml new file mode 100644 index 000000000..f3e662a28 --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/templates/alpine-pod.yaml @@ -0,0 +1,14 @@ +apiVersion: v1 +kind: Pod +metadata: + name: {{.Release.Name}}-{{.Chart.Name}} + labels: + app.kubernetes.io/managed-by: {{.Release.Service}} + app.kubernetes.io/name: {{.Chart.Name}} + helm.sh/chart: "{{.Chart.Name}}-{{.Chart.Version}}" +spec: + restartPolicy: {{default "Never" .restart_policy}} + containers: + - name: waiter + image: "alpine:3.9" + command: ["/bin/sleep","9000"] diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/values.yaml b/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/values.yaml new file mode 100644 index 000000000..6b7cb2596 --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/values.yaml @@ -0,0 +1,2 @@ +# The pod name +name: "my-alpine" diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/charts/mariner-4.3.2.tgz b/pkg/chart/loader/testdata/frobnitz_with_bom/charts/mariner-4.3.2.tgz new file mode 100644 index 0000000000000000000000000000000000000000..3190136b050e62c628b3c817fd963ac9dc4a9e25 GIT binary patch literal 967 zcmV;&133I2iwFR+9h6)E1MQb_y%a`Bd>#= zhbqf2w!b6}wZ9@rvIhtwzm?~C&+QLm+G2!l%`$_aUgS(@pdjdX3a%E}VXVc7`)dg( zL%IRN2}c1D3xoMi2w@WuWOMZ?5i&3FelBVyq_Ce_8fREdP%NDf<&d!wu3{&VUQNs{N%vK$Ha~k^gB2!0bO7r0ic0 zbqCp*X#j?=|H@GNONsuE)&Idbh>2MX(Z}|+=@*sLod*wS?A8Ugp#mMPuMO0NnZmos9_rr z3xpDL+otj~lRh?B4hHFTl+d5-8NBXmUXB~EyF^bx7m*+!*g>obczvGF|8xkWsHN8; z%#+wiWP{=2pC*98@$VNzzsll&G#D7&11-;D>HT0x|DV2?6}a~*p46@R|2l??e_8dX z@Bb>D3t~VC_*wjq2Dy!6J-_5ME%%J+xmsbK5a#qO>ZV?Wt`d|TDdrB^B&LqFr@lYdUA zs&4-JAg3&uc4dA0gv*bXU9QePa9^8wR{HovA)j@)JOAIkak0Jl)aKqA_3XLM#?H=V z-{tlG=xy^os=1Z><53;&+C4=zp|%rwv!)UyY=%k_t%Y|wNRQl`C6N_Z&S;S+~wb1=)2Uh_4I81`y>DO zoLPSt=btB&x{CUK`0DA&){efW_O36RxnsYZNT0r;JbTLR{H4M3C$8(Cee==aQ~Gbq p$`5v3?NB{4-j0XQHf literal 0 HcmV?d00001 diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/docs/README.md b/pkg/chart/loader/testdata/frobnitz_with_bom/docs/README.md new file mode 100644 index 000000000..816c3e431 --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_with_bom/docs/README.md @@ -0,0 +1 @@ +This is a placeholder for documentation. diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/icon.svg b/pkg/chart/loader/testdata/frobnitz_with_bom/icon.svg new file mode 100644 index 000000000..892130606 --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_with_bom/icon.svg @@ -0,0 +1,8 @@ + + + Example icon + + + diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/ignore/me.txt b/pkg/chart/loader/testdata/frobnitz_with_bom/ignore/me.txt new file mode 100644 index 000000000..e69de29bb diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/templates/template.tpl b/pkg/chart/loader/testdata/frobnitz_with_bom/templates/template.tpl new file mode 100644 index 000000000..bb29c5491 --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_with_bom/templates/template.tpl @@ -0,0 +1 @@ +Hello {{.Name | default "world"}} diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/values.yaml b/pkg/chart/loader/testdata/frobnitz_with_bom/values.yaml new file mode 100644 index 000000000..c24ceadf9 --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_with_bom/values.yaml @@ -0,0 +1,6 @@ +# A values file contains configuration. + +name: "Some Name" + +section: + name: "Name in a section" From c422e51ca13b9f213d5f7ed42c187d401ffec245 Mon Sep 17 00:00:00 2001 From: Thomas FREYSS Date: Fri, 24 Apr 2020 11:09:27 +0200 Subject: [PATCH 2/7] test: add test for bom test data integrity Signed-off-by: Thomas FREYSS --- pkg/chart/loader/load_test.go | 49 +++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/pkg/chart/loader/load_test.go b/pkg/chart/loader/load_test.go index 3fa6bba0d..40b86dec2 100644 --- a/pkg/chart/loader/load_test.go +++ b/pkg/chart/loader/load_test.go @@ -20,6 +20,7 @@ import ( "archive/tar" "bytes" "compress/gzip" + "io" "io/ioutil" "os" "path/filepath" @@ -85,6 +86,54 @@ func TestLoadDirWithSymlink(t *testing.T) { verifyDependenciesLock(t, c) } +func TestBomTestData(t *testing.T) { + testFiles := []string{"frobnitz_with_bom/.helmignore", "frobnitz_with_bom/templates/template.tpl", "frobnitz_with_bom/Chart.yaml"} + for _, file := range testFiles { + data, err := ioutil.ReadFile("testdata/" + file) + if err != nil || !bytes.HasPrefix(data, utf8bom) { + t.Errorf("Test file has no BOM or is invalid: testdata/%s", file) + } + } + + archive, err := ioutil.ReadFile("testdata/frobnitz_with_bom.tgz") + if err != nil { + t.Fatalf("Error reading archive frobnitz_with_bom.tgz: %s", err) + } + unzipped, err := gzip.NewReader(bytes.NewReader(archive)) + if err != nil { + t.Fatalf("Error reading archive frobnitz_with_bom.tgz: %s", err) + } + defer unzipped.Close() + for _, testFile := range testFiles { + data := make([]byte, 3) + err := unzipped.Reset(bytes.NewReader(archive)) + if err != nil { + t.Fatalf("Error reading archive frobnitz_with_bom.tgz: %s", err) + } + tr := tar.NewReader(unzipped) + for { + file, err := tr.Next() + if err == io.EOF { + break + } + if err != nil { + t.Fatalf("Error reading archive frobnitz_with_bom.tgz: %s", err) + } + if file != nil && strings.EqualFold(file.Name, testFile) { + _, err := tr.Read(data) + if err != nil { + t.Fatalf("Error reading archive frobnitz_with_bom.tgz: %s", err) + } else { + break + } + } + } + if !bytes.Equal(data, utf8bom) { + t.Fatalf("Test file has no BOM or is invalid: frobnitz_with_bom.tgz/%s", testFile) + } + } +} + func TestLoadDirWithUTFBOM(t *testing.T) { l, err := Loader("testdata/frobnitz_with_bom") if err != nil { From 6bc4a948be0ef1e32f6a605b655cc8cd5f0c2f06 Mon Sep 17 00:00:00 2001 From: Hu Shuai Date: Mon, 27 Apr 2020 14:19:02 +0800 Subject: [PATCH 3/7] Add unit test for pkg/chart/chart.go Signed-off-by: Hu Shuai --- pkg/chart/chart_test.go | 50 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/pkg/chart/chart_test.go b/pkg/chart/chart_test.go index 1b8669ac8..ef8cec3ad 100644 --- a/pkg/chart/chart_test.go +++ b/pkg/chart/chart_test.go @@ -159,3 +159,53 @@ func TestChartFullPath(t *testing.T) { is.Equal("foo/charts/", chrt1.ChartFullPath()) is.Equal("foo", chrt2.ChartFullPath()) } + +func TestCRDObjects(t *testing.T) { + chrt := Chart{ + Files: []*File{ + { + Name: "crds/foo.yaml", + Data: []byte("hello"), + }, + { + Name: "bar.yaml", + Data: []byte("hello"), + }, + { + Name: "crds/foo/bar/baz.yaml", + Data: []byte("hello"), + }, + { + Name: "crdsfoo/bar/baz.yaml", + Data: []byte("hello"), + }, + { + Name: "crds/README.md", + Data: []byte("# hello"), + }, + }, + } + + expected := []CRD{ + { + Name: "crds/foo.yaml", + Filename: "crds/foo.yaml", + File: &File{ + Name: "crds/foo.yaml", + Data: []byte("hello"), + }, + }, + { + Name: "crds/foo/bar/baz.yaml", + Filename: "crds/foo/bar/baz.yaml", + File: &File{ + Name: "crds/foo/bar/baz.yaml", + Data: []byte("hello"), + }, + }, + } + + is := assert.New(t) + crds := chrt.CRDObjects() + is.Equal(expected, crds) +} From f7d1bfd0063a0d43aaef0764222a819fe3472056 Mon Sep 17 00:00:00 2001 From: Bridget Kromhout Date: Mon, 27 Apr 2020 09:02:14 -0500 Subject: [PATCH 4/7] Adding PR template from dev-v2 branch Signed-off-by: Bridget Kromhout --- .github/pull_request_template.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 .github/pull_request_template.md diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 000000000..595b50218 --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,12 @@ + + +**What this PR does / why we need it**: + +**Special notes for your reviewer**: + +**If applicable**: +- [ ] this PR contains documentation +- [ ] this PR contains unit tests +- [ ] this PR has been tested for backwards compatibility From 0a318beba32626997abb6af915ebad9399430103 Mon Sep 17 00:00:00 2001 From: Bridget Kromhout Date: Mon, 27 Apr 2020 09:17:09 -0500 Subject: [PATCH 5/7] Updating CONTRIBUTING to match current practice Signed-off-by: Bridget Kromhout --- CONTRIBUTING.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 63780365e..a637f9255 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -184,9 +184,9 @@ contributing to Helm. All issue types follow the same general lifecycle. Differe ## How to Contribute a Patch -1. If you haven't already done so, sign a Contributor License Agreement (see details above). -2. Fork the desired repo, develop and test your code changes. -3. Submit a pull request. +1. Identify or create the related issue. +2. Fork the desired repo; develop and test your code changes. +3. Submit a pull request, making sure to sign your work and link the related issue. Coding conventions and standards are explained in the [official developer docs](https://helm.sh/docs/developers/). From cd50d0c3621ad91b3848f14b7ef3a8d6aa29d2c9 Mon Sep 17 00:00:00 2001 From: Anshul Verma Date: Tue, 28 Apr 2020 01:41:47 +0530 Subject: [PATCH 6/7] Fix : Prints empty list in json/yaml is no repositories are present (#7949) * Prints empty repolist in json/yaml if there are no repos and output format is given as json/yaml rather that printing the error msg "no repositories to show" Signed-off-by: Anshul Verma * Prints empty repolist in json/yaml if there are no repos and output format is given as json/yaml rather that printing the error msg "no repositories to show" Signed-off-by: Anshul Verma --- cmd/helm/repo_list.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/helm/repo_list.go b/cmd/helm/repo_list.go index 51b4f0d58..ed1c9573c 100644 --- a/cmd/helm/repo_list.go +++ b/cmd/helm/repo_list.go @@ -38,7 +38,7 @@ func newRepoListCmd(out io.Writer) *cobra.Command { Args: require.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { f, err := repo.LoadFile(settings.RepositoryConfig) - if isNotExist(err) || len(f.Repositories) == 0 { + if isNotExist(err) || (len(f.Repositories) == 0 && !(outfmt == output.JSON || outfmt == output.YAML)) { return errors.New("no repositories to show") } From 6fc93530569813580c1b6554a7154f4ec0fda0b4 Mon Sep 17 00:00:00 2001 From: Matt Butcher Date: Tue, 28 Apr 2020 17:12:14 -0600 Subject: [PATCH 7/7] feat: lint the names of templated resources (#8011) Signed-off-by: Matt Butcher --- pkg/lint/lint_test.go | 8 ++- pkg/lint/rules/template.go | 53 ++++++++++++++----- pkg/lint/rules/template_test.go | 29 ++++++++++ .../testdata/goodone/templates/goodone.yaml | 2 +- pkg/lint/rules/testdata/goodone/values.yaml | 2 +- 5 files changed, 78 insertions(+), 16 deletions(-) diff --git a/pkg/lint/lint_test.go b/pkg/lint/lint_test.go index 2c110009d..e7ff4cd7a 100644 --- a/pkg/lint/lint_test.go +++ b/pkg/lint/lint_test.go @@ -104,7 +104,10 @@ func TestBadValues(t *testing.T) { func TestGoodChart(t *testing.T) { m := All(goodChartDir, values, namespace, strict).Messages if len(m) != 0 { - t.Errorf("All failed but shouldn't have: %#v", m) + t.Error("All returned linter messages when it shouldn't have") + for i, msg := range m { + t.Logf("Message %d: %s", i, msg) + } } } @@ -130,6 +133,9 @@ func TestHelmCreateChart(t *testing.T) { m := All(createdChart, values, namespace, true).Messages if ll := len(m); ll != 1 { t.Errorf("All should have had exactly 1 error. Got %d", ll) + for i, msg := range m { + t.Logf("Message %d: %s", i, msg.Error()) + } } else if msg := m[0].Err.Error(); !strings.Contains(msg, "icon is recommended") { t.Errorf("Unexpected lint error: %s", msg) } diff --git a/pkg/lint/rules/template.go b/pkg/lint/rules/template.go index 3d388f81b..e27c6a345 100644 --- a/pkg/lint/rules/template.go +++ b/pkg/lint/rules/template.go @@ -17,9 +17,11 @@ limitations under the License. package rules import ( + "fmt" "os" "path/filepath" "regexp" + "strings" "github.com/pkg/errors" "sigs.k8s.io/yaml" @@ -35,6 +37,14 @@ var ( releaseTimeSearch = regexp.MustCompile(`\.Release\.Time`) ) +// validName is a regular expression for names. +// +// This is different than action.ValidName. It conforms to the regular expression +// `kubectl` says it uses, plus it disallows empty names. +// +// For details, see https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names +var validName = regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$`) + // Templates lints the templates in the Linter. func Templates(linter *support.Linter, values map[string]interface{}, namespace string, strict bool) { path := "templates/" @@ -57,7 +67,7 @@ func Templates(linter *support.Linter, values map[string]interface{}, namespace } options := chartutil.ReleaseOptions{ - Name: "testRelease", + Name: "test-release", Namespace: namespace, } @@ -111,14 +121,17 @@ func Templates(linter *support.Linter, values map[string]interface{}, namespace // linter.RunLinterRule(support.WarningSev, path, validateQuotes(string(preExecutedTemplate))) renderedContent := renderedContentMap[filepath.Join(chart.Name(), fileName)] - var yamlStruct K8sYamlStruct - // Even though K8sYamlStruct only defines Metadata namespace, an error in any other - // key will be raised as well - err := yaml.Unmarshal([]byte(renderedContent), &yamlStruct) - - // If YAML linting fails, we sill progress. So we don't capture the returned state - // on this linter run. - linter.RunLinterRule(support.ErrorSev, path, validateYamlContent(err)) + if strings.TrimSpace(renderedContent) != "" { + var yamlStruct K8sYamlStruct + // Even though K8sYamlStruct only defines a few fields, an error in any other + // key will be raised as well + err := yaml.Unmarshal([]byte(renderedContent), &yamlStruct) + + // If YAML linting fails, we sill progress. So we don't capture the returned state + // on this linter run. + linter.RunLinterRule(support.ErrorSev, path, validateYamlContent(err)) + linter.RunLinterRule(support.ErrorSev, path, validateMetadataName(&yamlStruct)) + } } } @@ -149,6 +162,15 @@ func validateYamlContent(err error) error { return errors.Wrap(err, "unable to parse YAML") } +func validateMetadataName(obj *K8sYamlStruct) error { + // This will return an error if the characters do not abide by the standard OR if the + // name is left empty. + if validName.MatchString(obj.Metadata.Name) { + return nil + } + return fmt.Errorf("object name does not conform to Kubernetes naming requirements: %q", obj.Metadata.Name) +} + func validateNoCRDHooks(manifest []byte) error { if crdHookSearch.Match(manifest) { return errors.New("manifest is a crd-install hook. This hook is no longer supported in v3 and all CRDs should also exist the crds/ directory at the top level of the chart") @@ -164,9 +186,14 @@ func validateNoReleaseTime(manifest []byte) error { } // K8sYamlStruct stubs a Kubernetes YAML file. -// Need to access for now to Namespace only +// +// DEPRECATED: In Helm 4, this will be made a private type, as it is for use only within +// the rules package. type K8sYamlStruct struct { - Metadata struct { - Namespace string - } + Metadata k8sYamlMetadata +} + +type k8sYamlMetadata struct { + Namespace string + Name string } diff --git a/pkg/lint/rules/template_test.go b/pkg/lint/rules/template_test.go index ddb46aba0..c924de0e7 100644 --- a/pkg/lint/rules/template_test.go +++ b/pkg/lint/rules/template_test.go @@ -101,3 +101,32 @@ func TestV3Fail(t *testing.T) { t.Errorf("Unexpected error: %s", res[2].Err) } } + +func TestValidateMetadataName(t *testing.T) { + names := map[string]bool{ + "": false, + "foo": true, + "foo.bar1234baz.seventyone": true, + "FOO": false, + "123baz": true, + "foo.BAR.baz": false, + "one-two": true, + "-two": false, + "one_two": false, + "a..b": false, + "%^&#$%*@^*@&#^": false, + } + for input, expectPass := range names { + obj := K8sYamlStruct{Metadata: k8sYamlMetadata{Name: input}} + if err := validateMetadataName(&obj); (err == nil) != expectPass { + st := "fail" + if expectPass { + st = "succeed" + } + t.Errorf("Expected %q to %s", input, st) + if err != nil { + t.Log(err) + } + } + } +} diff --git a/pkg/lint/rules/testdata/goodone/templates/goodone.yaml b/pkg/lint/rules/testdata/goodone/templates/goodone.yaml index 0e77f46f2..cd46f62c7 100644 --- a/pkg/lint/rules/testdata/goodone/templates/goodone.yaml +++ b/pkg/lint/rules/testdata/goodone/templates/goodone.yaml @@ -1,2 +1,2 @@ metadata: - name: {{.Values.name | default "foo" | title}} + name: {{ .Values.name | default "foo" | lower }} diff --git a/pkg/lint/rules/testdata/goodone/values.yaml b/pkg/lint/rules/testdata/goodone/values.yaml index fe9abd983..92c3d9bb9 100644 --- a/pkg/lint/rules/testdata/goodone/values.yaml +++ b/pkg/lint/rules/testdata/goodone/values.yaml @@ -1 +1 @@ -name: "goodone here" +name: "goodone-here"