Skip to content
This repository has been archived by the owner on Jul 16, 2021. It is now read-only.

Adding filter options to chart-repo #658

Merged
merged 7 commits into from
Mar 4, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 3 additions & 0 deletions chart/monocular/templates/_helpers.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ spec:
- --mongo-url={{ template "mongodb.fullname" $global }}
- --mongo-user=root
{{- end }}
{{- range $repo.args }}
- {{ . }}
{{- end }}
- {{ $repo.name }}
- {{ $repo.url }}
command:
Expand Down
7 changes: 5 additions & 2 deletions chart/monocular/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ sync:
# source: my-repository-source
# schedule: "*/5 * * * *"
# successfulJobsHistoryLimit: 1
# args:
# - --filter-name=name # sync if chart is named
# - --filter-annotation=annotation # sync if annotation exists
# - --filter-annotation=annotation=value # sync if annotation has value
# Uncomment these properties to set HTTP proxy for chart synchronization jobs
# httpProxy:
# httpsProxy:
Expand Down Expand Up @@ -154,9 +158,8 @@ mongodb:
# ref: https://docs.mongodb.com/manual/reference/connection-string/
global:
mongoUrl:

# This parameter will allow to set securityContext: { "runAsUser": 1001} by example
# this is usefull to set a user different to 0 (root)
# this is usefull to set a user different to 0 (root)
# It allows to be launch in a Kubernetes with PodSecurityPolicy with a policy set runAsNonRoot
securityContext: {}

Expand Down
5 changes: 5 additions & 0 deletions cmd/chart-repo/chart_repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,17 @@ func main() {

func init() {
cmds := []*cobra.Command{syncCmd, deleteCmd}
filterAnnotations := []string{}
filterNames := []string{}

for _, cmd := range cmds {
rootCmd.AddCommand(cmd)
cmd.Flags().String("mongo-url", "localhost", "MongoDB URL (see https://godoc.org/github.com/globalsign/mgo#Dial for format)")
cmd.Flags().String("mongo-database", "charts", "MongoDB database")
cmd.Flags().String("mongo-user", "", "MongoDB user")
cmd.Flags().StringSliceVar(&filterAnnotations, "filter-annotation", []string{}, "Filter by charts that match any of these annotations")
cmd.Flags().StringSliceVar(&filterNames, "filter-name", []string{}, "Filter by charts that match these names")

// see version.go
cmd.Flags().StringVarP(&userAgentComment, "user-agent-comment", "", "", "UserAgent comment used during outbound requests")
cmd.Flags().Bool("debug", false, "verbose logging")
Expand Down
24 changes: 23 additions & 1 deletion cmd/chart-repo/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package main

import (
"os"
"strings"

"github.com/kubeapps/common/datastore"
"github.com/sirupsen/logrus"
Expand Down Expand Up @@ -46,6 +47,27 @@ var syncCmd = &cobra.Command{
if err != nil {
logrus.Fatal(err)
}

filter := new(filters)
filter.Annotations = make(map[string]string)
filterAnnotationsStrings, err := cmd.Flags().GetStringSlice("filter-annotation")
if err != nil {
logrus.Fatal(err)
}
for _, a := range filterAnnotationsStrings {
kv := strings.Split(a, "=")
if len(kv) == 2 {
filter.Annotations[kv[0]] = kv[1]
} else {
filter.Annotations[a] = ""
}
}
filterNammesStrings, err := cmd.Flags().GetStringSlice("filter-name")
if err != nil {
logrus.Fatal(err)
}
filter.Names = filterNammesStrings

mongoPW := os.Getenv("MONGO_PASSWORD")
debug, err := cmd.Flags().GetBool("debug")
if err != nil {
Expand All @@ -61,7 +83,7 @@ var syncCmd = &cobra.Command{
}

authorizationHeader := os.Getenv("AUTHORIZATION_HEADER")
if err = syncRepo(dbSession, args[0], args[1], authorizationHeader); err != nil {
if err = syncRepo(dbSession, args[0], args[1], authorizationHeader, filter); err != nil {
logrus.Fatalf("Can't add chart repository to database: %v", err)
}

Expand Down
50 changes: 50 additions & 0 deletions cmd/chart-repo/testdata/valid-index.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,53 @@ entries:
urls:
- https://kubernetes-charts.storage.googleapis.com/acs-engine-autoscaler-2.1.1.tgz
version: 2.1.1
nginx-ingress:
- apiVersion: v1
appVersion: 0.28.0
created: 2020-02-14T00:58:48.780420335Z
description: An nginx Ingress controller that uses ConfigMap to store the nginx
configuration.
digest: c170639916a16e33a570923cebefe06066558a266f28a9b2c04d98357f984427
engine: gotpl
home: https://github.com/kubernetes/ingress-nginx
icon: https://upload.wikimedia.org/wikipedia/commons/thumb/c/c5/Nginx_logo.svg/500px-Nginx_logo.svg.png
keywords:
- ingress
- nginx
kubeVersion: '>=1.10.0-0'
maintainers:
- name: ChiefAlexander
- email: Trevor.G.Wood@gmail.com
name: taharah
name: nginx-ingress
sources:
- https://github.com/kubernetes/ingress-nginx
urls:
- https://kubernetes-charts.storage.googleapis.com/nginx-ingress-1.30.2.tgz
version: 1.30.2
- apiVersion: v1
appVersion: 0.28.0
created: 2020-02-13T21:29:23.810801158Z
description: An nginx Ingress controller that uses ConfigMap to store the nginx
configuration.
digest: be955d4e77599468d63d61d1dc471fc488e36a3fca2263efd639f17260db1968
engine: gotpl
home: https://github.com/kubernetes/ingress-nginx
icon: https://upload.wikimedia.org/wikipedia/commons/thumb/c/c5/Nginx_logo.svg/500px-Nginx_logo.svg.png
keywords:
- ingress
- nginx
kubeVersion: '>=1.10.0-0'
maintainers:
- name: ChiefAlexander
- email: Trevor.G.Wood@gmail.com
name: taharah
name: nginx-ingress
sources:
- https://github.com/kubernetes/ingress-nginx
urls:
- https://kubernetes-charts.storage.googleapis.com/nginx-ingress-1.30.1.tgz
version: 1.30.1
wordpress:
- appVersion: 4.9.1
created: 2017-12-06T18:48:59.644981487Z
Expand All @@ -42,6 +89,9 @@ entries:
urls:
- https://kubernetes-charts.storage.googleapis.com/wordpress-0.7.5.tgz
version: 0.7.5
annotations:
sync: "true"
sync-by-name-only: "true"
- appVersion: 4.9.0
created: 2017-12-01T11:49:00.136950565Z
description: Web publishing platform for building blogs and websites.
Expand Down
5 changes: 5 additions & 0 deletions cmd/chart-repo/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,8 @@ type repoCheck struct {
LastUpdate time.Time `bson:"last_update"`
Checksum string `bson:"checksum"`
}

type filters struct {
Annotations map[string]string
Names []string
}
41 changes: 38 additions & 3 deletions cmd/chart-repo/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"net/url"
"os"
"path"
"path/filepath"
"strings"
"sync"
"time"
Expand Down Expand Up @@ -86,7 +87,7 @@ func init() {
// These steps are processed in this way to ensure relevant chart data is
// imported into the database as fast as possible. E.g. we want all icons for
// charts before fetching readmes for each chart and version pair.
func syncRepo(dbSession datastore.Session, repoName, repoURL string, authorizationHeader string) error {
func syncRepo(dbSession datastore.Session, repoName, repoURL string, authorizationHeader string, filter *filters) error {
url, err := parseRepoURL(repoURL)
if err != nil {
log.WithFields(log.Fields{"url": repoURL}).WithError(err).Error("failed to parse URL")
Expand Down Expand Up @@ -115,7 +116,7 @@ func syncRepo(dbSession datastore.Session, repoName, repoURL string, authorizati
return err
}

charts := chartsFromIndex(index, r)
charts := chartsFromIndex(index, r, filter)
if len(charts) == 0 {
return errors.New("no charts in repository index")
}
Expand Down Expand Up @@ -271,13 +272,22 @@ func parseRepoIndex(body []byte) (*helmrepo.IndexFile, error) {
return &index, nil
}

func chartsFromIndex(index *helmrepo.IndexFile, r repo) []chart {
func chartsFromIndex(index *helmrepo.IndexFile, r repo, filter *filters) []chart {
var charts []chart
for _, entry := range index.Entries {
if entry[0].GetDeprecated() {
log.WithFields(log.Fields{"name": entry[0].GetName()}).Info("skipping deprecated chart")
continue
}

if len(filter.Annotations) > 0 ||
len(filter.Names) > 0 {
if !filterEntry(entry[0], filter) {
log.WithFields(log.Fields{"name": entry[0].GetName()}).Info("skipping chart as filters did not match")
continue
}
}

charts = append(charts, newChart(entry, r))
}
return charts
Expand Down Expand Up @@ -536,3 +546,28 @@ func initNetClient(additionalCA string) (*http.Client, error) {
},
}, nil
}

// return true if entry matches any filter
func filterEntry(entry *helmrepo.ChartVersion, filter *filters) bool {
if len(filter.Annotations) > 0 {
for a, av := range filter.Annotations {
if v, ok := entry.Annotations[a]; ok {
if len(av) == 0 {
return true
} else if v == av {
return true
}
}
}
}

if len(filter.Names) > 0 {
for _, n := range filter.Names {
matched, _ := filepath.Match(n, entry.Name)
if matched {
return true
}
}
}
return false
}
95 changes: 84 additions & 11 deletions cmd/chart-repo/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ func Test_syncURLInvalidity(t *testing.T) {
dbSession := mockstore.NewMockSession(&m)
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := syncRepo(dbSession, "test", tt.repoURL, "")
err := syncRepo(dbSession, "test", tt.repoURL, "", new(filters))
assert.ExistsErr(t, err, tt.name)
})
}
Expand Down Expand Up @@ -277,25 +277,98 @@ func Test_parseRepoIndex(t *testing.T) {
t.Run("valid", func(t *testing.T) {
index, err := parseRepoIndex([]byte(validRepoIndexYAML))
assert.NoErr(t, err)
assert.Equal(t, len(index.Entries), 2, "number of charts")
assert.Equal(t, len(index.Entries), 3, "number of charts")
assert.Equal(t, index.Entries["acs-engine-autoscaler"][0].GetName(), "acs-engine-autoscaler", "chart version populated")
})
}

func Test_chartsFromIndex(t *testing.T) {
r := repo{Name: "test", URL: "http://testrepo.com"}
index, _ := parseRepoIndex([]byte(validRepoIndexYAML))
charts := chartsFromIndex(index, r)
assert.Equal(t, len(charts), 2, "number of charts")

charts := chartsFromIndex(index, r, new(filters))
assert.Equal(t, len(charts), 3, "number of charts")
indexWithDeprecated := validRepoIndexYAML + `
deprecated-chart:
- name: deprecated-chart
deprecated: true`
index2, err := parseRepoIndex([]byte(indexWithDeprecated))
assert.NoErr(t, err)
charts = chartsFromIndex(index2, r)
assert.Equal(t, len(charts), 2, "number of charts")
charts = chartsFromIndex(index2, r, new(filters))
assert.Equal(t, len(charts), 3, "number of charts")
}

func Test_chartsFromIndexFilterByName(t *testing.T) {
r := repo{Name: "test", URL: "http://testrepo.com"}
index, _ := parseRepoIndex([]byte(validRepoIndexYAML))
filter := new(filters)
filter.Names = append(filter.Names, "wordpress")
filter.Names = append(filter.Names, "not-found")
charts := chartsFromIndex(index, r, filter)
assert.Equal(t, len(charts), 1, "number of charts")
}

func Test_chartsFromIndexFilterByNameGlobbedSingleChar(t *testing.T) {
r := repo{Name: "test", URL: "http://testrepo.com"}
index, _ := parseRepoIndex([]byte(validRepoIndexYAML))
filter := new(filters)
filter.Names = append(filter.Names, "word?ress")
filter.Names = append(filter.Names, "not-found")
charts := chartsFromIndex(index, r, filter)
assert.Equal(t, len(charts), 1, "number of charts")
}

func Test_chartsFromIndexFilterByNameGlobbedWildcard(t *testing.T) {
r := repo{Name: "test", URL: "http://testrepo.com"}
index, _ := parseRepoIndex([]byte(validRepoIndexYAML))
filter := new(filters)
filter.Names = append(filter.Names, "word*")
filter.Names = append(filter.Names, "not-found")
charts := chartsFromIndex(index, r, filter)
assert.Equal(t, len(charts), 1, "number of charts")
}

func Test_chartsFromIndexFilterByAnnotationWithValue(t *testing.T) {
r := repo{Name: "test", URL: "http://testrepo.com"}
index, _ := parseRepoIndex([]byte(validRepoIndexYAML))
filter := new(filters)
filter.Annotations = make(map[string]string)
filter.Annotations["sync"] = "true"
filter.Annotations["not-found"] = "missing"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the expectation here that it only needs to match one of the annotations and not all of the annotations? I would have assumed that all the annotations need to match for the entry to be selected (I believe this is also how kubectl label filters work).

Copy link
Author

@gregsidelinger gregsidelinger Feb 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point I did not think about that. My intend was if any of them matched it synced. That is actually how I just rewrote the code as a function.

// return true if entry matches any filter
func filterEntry( entry *helmrepo.ChartVersion, filter *filters) bool {

charts := chartsFromIndex(index, r, filter)
assert.Equal(t, len(charts), 1, "number of charts")
}

func Test_chartsFromIndexFilterByAnnotation(t *testing.T) {
r := repo{Name: "test", URL: "http://testrepo.com"}
index, _ := parseRepoIndex([]byte(validRepoIndexYAML))
// filter on annotation
filter := new(filters)
filter.Annotations = make(map[string]string)
filter.Annotations["sync-by-name-only"] = ""
charts := chartsFromIndex(index, r, filter)
assert.Equal(t, len(charts), 1, "number of charts")
}

func Test_chartsFromIndexFilterByAnnotationDuplicateMatches(t *testing.T) {
r := repo{Name: "test", URL: "http://testrepo.com"}
index, _ := parseRepoIndex([]byte(validRepoIndexYAML))
filter := new(filters)
filter.Annotations = make(map[string]string)
filter.Annotations["sync"] = "true"
filter.Annotations["sync-by-name-only"] = ""
charts := chartsFromIndex(index, r, filter)
assert.Equal(t, len(charts), 1, "number of charts")
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's add a test for both an annotation and name filter

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


func Test_chartsFromIndexFilterByAnnotationAndName(t *testing.T) {
r := repo{Name: "test", URL: "http://testrepo.com"}
index, _ := parseRepoIndex([]byte(validRepoIndexYAML))
filter := new(filters)
filter.Annotations = make(map[string]string)
filter.Annotations["sync"] = "true"
filter.Names = append(filter.Names, "wordpress")
charts := chartsFromIndex(index, r, filter)
assert.Equal(t, len(charts), 1, "number of charts")
}

func Test_newChart(t *testing.T) {
Expand All @@ -316,7 +389,7 @@ func Test_importCharts(t *testing.T) {
m.On("RemoveAll", mock.Anything)
dbSession := mockstore.NewMockSession(m)
index, _ := parseRepoIndex([]byte(validRepoIndexYAML))
charts := chartsFromIndex(index, repo{Name: "test", URL: "http://testrepo.com"})
charts := chartsFromIndex(index, repo{Name: "test", URL: "http://testrepo.com"}, new(filters))
importCharts(dbSession, charts)

m.AssertExpectations(t)
Expand Down Expand Up @@ -357,7 +430,7 @@ func Test_fetchAndImportIcon(t *testing.T) {
})

index, _ := parseRepoIndex([]byte(validRepoIndexYAML))
charts := chartsFromIndex(index, repo{Name: "test", URL: "http://testrepo.com"})
charts := chartsFromIndex(index, repo{Name: "test", URL: "http://testrepo.com"}, new(filters))

t.Run("failed download", func(t *testing.T) {
netClient = &badHTTPClient{}
Expand Down Expand Up @@ -402,7 +475,7 @@ func Test_fetchAndImportIcon(t *testing.T) {

func Test_fetchAndImportFiles(t *testing.T) {
index, _ := parseRepoIndex([]byte(validRepoIndexYAML))
charts := chartsFromIndex(index, repo{Name: "test", URL: "http://testrepo.com", AuthorizationHeader: "Bearer ThisSecretAccessTokenAuthenticatesTheClient1s"})
charts := chartsFromIndex(index, repo{Name: "test", URL: "http://testrepo.com", AuthorizationHeader: "Bearer ThisSecretAccessTokenAuthenticatesTheClient1s"}, new(filters))
cv := charts[0].ChartVersions[0]

t.Run("http error", func(t *testing.T) {
Expand Down Expand Up @@ -622,7 +695,7 @@ func Test_emptyChartRepo(t *testing.T) {
m := mock.Mock{}
m.On("One", &repoCheck{}).Return(nil)
dbSession := mockstore.NewMockSession(&m)
err := syncRepo(dbSession, "testRepo", "https://my.examplerepo.com", "")
err := syncRepo(dbSession, "testRepo", "https://my.examplerepo.com", "", new(filters))
assert.ExistsErr(t, err, "Failed Request")
}

Expand Down