From 276c162d919663038a629ad800ebf98fc562c736 Mon Sep 17 00:00:00 2001 From: Michael Nelson Date: Tue, 22 Sep 2020 13:12:00 +1000 Subject: [PATCH] Remove mongodb support from asset svcs and re-write postgres tests. (#2048) * Remove mongodb support from asset svcs and re-write postgres tests. * Remove database type from app repo controller. * Remove database-type from assetsvc. * Remove references to mongodb in docs. * Fix sort criteria. --- .../templates/apprepository-deployment.yaml | 1 - .../apprepository-jobs-postupgrade.yaml | 1 - .../templates/assetsvc-deployment.yaml | 1 - cmd/apprepository-controller/controller.go | 1 - .../controller_test.go | 62 ++-- cmd/apprepository-controller/main.go | 6 +- cmd/asset-syncer/delete.go | 2 +- cmd/asset-syncer/invalidate-cache.go | 2 +- cmd/asset-syncer/main.go | 2 - cmd/asset-syncer/mongodb_db_test.go | 178 ----------- cmd/asset-syncer/mongodb_utils.go | 152 --------- cmd/asset-syncer/mongodb_utils_test.go | 140 -------- cmd/asset-syncer/postgresql_utils.go | 16 +- cmd/asset-syncer/postgresql_utils_test.go | 235 +++++++------- cmd/asset-syncer/sync.go | 2 +- cmd/asset-syncer/utils.go | 12 +- cmd/asset-syncer/utils_test.go | 202 ++++++------ cmd/assetsvc/handler.go | 2 +- cmd/assetsvc/handler_test.go | 301 +++++++++++------- cmd/assetsvc/main.go | 4 +- cmd/assetsvc/main_test.go | 213 ++++++++----- cmd/assetsvc/mongodb_utils.go | 172 ---------- cmd/assetsvc/postgresql_utils.go | 2 +- cmd/assetsvc/postgresql_utils_test.go | 161 ++++------ cmd/assetsvc/utils.go | 10 +- docs/developer/README.md | 2 +- docs/developer/asset-syncer.md | 9 - docs/developer/assetsvc.md | 10 +- go.mod | 2 +- go.sum | 2 + pkg/dbutils/dbutilstest/mgtest/mgtest.go | 63 ---- pkg/dbutils/mongodb_utils.go | 96 ------ pkg/dbutils/postgresql_utils.go | 4 +- script/cluster-openshift.mk | 50 +-- script/e2e-test.sh | 4 - 35 files changed, 682 insertions(+), 1440 deletions(-) delete mode 100644 cmd/asset-syncer/mongodb_db_test.go delete mode 100644 cmd/asset-syncer/mongodb_utils.go delete mode 100644 cmd/asset-syncer/mongodb_utils_test.go delete mode 100644 cmd/assetsvc/mongodb_utils.go delete mode 100644 pkg/dbutils/dbutilstest/mgtest/mgtest.go delete mode 100644 pkg/dbutils/mongodb_utils.go diff --git a/chart/kubeapps/templates/apprepository-deployment.yaml b/chart/kubeapps/templates/apprepository-deployment.yaml index a5aa16f4f2b..8c80ed1d79c 100644 --- a/chart/kubeapps/templates/apprepository-deployment.yaml +++ b/chart/kubeapps/templates/apprepository-deployment.yaml @@ -47,7 +47,6 @@ spec: - --namespace={{ .Release.Namespace }} - --database-secret-name={{ .Values.postgresql.existingSecret }} - --database-secret-key=postgresql-password - - --database-type=postgresql - --database-url={{ template "kubeapps.postgresql.fullname" . }}:5432 - --database-user=postgres - --database-name=assets diff --git a/chart/kubeapps/templates/apprepository-jobs-postupgrade.yaml b/chart/kubeapps/templates/apprepository-jobs-postupgrade.yaml index 1779eb58d0d..7660db552be 100644 --- a/chart/kubeapps/templates/apprepository-jobs-postupgrade.yaml +++ b/chart/kubeapps/templates/apprepository-jobs-postupgrade.yaml @@ -44,7 +44,6 @@ spec: - /asset-syncer args: - invalidate-cache - - --database-type=postgresql - --database-url={{ template "kubeapps.postgresql.fullname" . }}:5432 - --database-user=postgres - --database-name=assets diff --git a/chart/kubeapps/templates/assetsvc-deployment.yaml b/chart/kubeapps/templates/assetsvc-deployment.yaml index f60f1aaa439..fafa8f5db6c 100644 --- a/chart/kubeapps/templates/assetsvc-deployment.yaml +++ b/chart/kubeapps/templates/assetsvc-deployment.yaml @@ -40,7 +40,6 @@ spec: command: - /assetsvc args: - - --database-type=postgresql - --database-user=postgres - --database-name=assets - --database-url={{ template "kubeapps.postgresql.fullname" . }}-headless:5432 diff --git a/cmd/apprepository-controller/controller.go b/cmd/apprepository-controller/controller.go index a5a9cc7e3b1..506378f612a 100644 --- a/cmd/apprepository-controller/controller.go +++ b/cmd/apprepository-controller/controller.go @@ -614,7 +614,6 @@ func apprepoCleanupJobArgs(repoName, repoNamespace string) []string { func dbFlags() []string { return []string{ - "--database-type=" + dbType, "--database-url=" + dbURL, "--database-user=" + dbUser, "--database-name=" + dbName, diff --git a/cmd/apprepository-controller/controller_test.go b/cmd/apprepository-controller/controller_test.go index bd164364dcc..0006def0266 100644 --- a/cmd/apprepository-controller/controller_test.go +++ b/cmd/apprepository-controller/controller_test.go @@ -13,10 +13,10 @@ import ( ) func Test_newCronJob(t *testing.T) { - dbURL = "mongodb.kubeapps" + dbURL = "postgresql.kubeapps" dbName = "assets" dbUser = "admin" - dbSecretName = "mongodb" + dbSecretName = "postgresql" const kubeappsNamespace = "kubeapps" tests := []struct { name string @@ -84,8 +84,7 @@ func Test_newCronJob(t *testing.T) { Command: []string{"/chart-repo"}, Args: []string{ "sync", - "--database-type=mongodb", - "--database-url=mongodb.kubeapps", + "--database-url=postgresql.kubeapps", "--database-user=admin", "--database-name=assets", "--namespace=kubeapps", @@ -96,7 +95,7 @@ func Test_newCronJob(t *testing.T) { { Name: "DB_PASSWORD", ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "mongodb"}, Key: "mongodb-root-password"}}, + SecretKeyRef: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "postgresql"}, Key: "postgresql-root-password"}}, }, }, VolumeMounts: nil, @@ -175,8 +174,7 @@ func Test_newCronJob(t *testing.T) { Command: []string{"/chart-repo"}, Args: []string{ "sync", - "--database-type=mongodb", - "--database-url=mongodb.kubeapps", + "--database-url=postgresql.kubeapps", "--database-user=admin", "--database-name=assets", "--user-agent-comment=kubeapps/v2.3", @@ -188,7 +186,7 @@ func Test_newCronJob(t *testing.T) { { Name: "DB_PASSWORD", ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "mongodb"}, Key: "mongodb-root-password"}}, + SecretKeyRef: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "postgresql"}, Key: "postgresql-root-password"}}, }, { Name: "AUTHORIZATION_HEADER", @@ -263,8 +261,7 @@ func Test_newCronJob(t *testing.T) { Command: []string{"/chart-repo"}, Args: []string{ "sync", - "--database-type=mongodb", - "--database-url=mongodb.kubeapps", + "--database-url=postgresql.kubeapps", "--database-user=admin", "--database-name=assets", "--user-agent-comment=kubeapps/v2.3", @@ -276,7 +273,7 @@ func Test_newCronJob(t *testing.T) { { Name: "DB_PASSWORD", ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "mongodb"}, Key: "mongodb-root-password"}}, + SecretKeyRef: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "postgresql"}, Key: "postgresql-root-password"}}, }, { Name: "AUTHORIZATION_HEADER", @@ -318,10 +315,10 @@ func Test_newCronJob(t *testing.T) { } func Test_newSyncJob(t *testing.T) { - dbURL = "mongodb.kubeapps" + dbURL = "postgresql.kubeapps" dbName = "assets" dbUser = "admin" - dbSecretName = "mongodb" + dbSecretName = "postgresql" const kubeappsNamespace = "kubeapps" tests := []struct { name string @@ -381,8 +378,7 @@ func Test_newSyncJob(t *testing.T) { Command: []string{"/chart-repo"}, Args: []string{ "sync", - "--database-type=mongodb", - "--database-url=mongodb.kubeapps", + "--database-url=postgresql.kubeapps", "--database-user=admin", "--database-name=assets", "--namespace=kubeapps", @@ -393,7 +389,7 @@ func Test_newSyncJob(t *testing.T) { { Name: "DB_PASSWORD", ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "mongodb"}, Key: "mongodb-root-password"}}, + SecretKeyRef: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "postgresql"}, Key: "postgresql-root-password"}}, }, }, VolumeMounts: nil, @@ -448,8 +444,7 @@ func Test_newSyncJob(t *testing.T) { Command: []string{"/chart-repo"}, Args: []string{ "sync", - "--database-type=mongodb", - "--database-url=mongodb.kubeapps", + "--database-url=postgresql.kubeapps", "--database-user=admin", "--database-name=assets", "--namespace=my-other-namespace", @@ -460,7 +455,7 @@ func Test_newSyncJob(t *testing.T) { { Name: "DB_PASSWORD", ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "mongodb"}, Key: "mongodb-root-password"}}, + SecretKeyRef: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "postgresql"}, Key: "postgresql-root-password"}}, }, }, VolumeMounts: nil, @@ -529,8 +524,7 @@ func Test_newSyncJob(t *testing.T) { Command: []string{"/chart-repo"}, Args: []string{ "sync", - "--database-type=mongodb", - "--database-url=mongodb.kubeapps", + "--database-url=postgresql.kubeapps", "--database-user=admin", "--database-name=assets", "--user-agent-comment=kubeapps/v2.3", @@ -542,7 +536,7 @@ func Test_newSyncJob(t *testing.T) { { Name: "DB_PASSWORD", ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "mongodb"}, Key: "mongodb-root-password"}}, + SecretKeyRef: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "postgresql"}, Key: "postgresql-root-password"}}, }, { Name: "AUTHORIZATION_HEADER", @@ -617,8 +611,7 @@ func Test_newSyncJob(t *testing.T) { Command: []string{"/chart-repo"}, Args: []string{ "sync", - "--database-type=mongodb", - "--database-url=mongodb.kubeapps", + "--database-url=postgresql.kubeapps", "--database-user=admin", "--database-name=assets", "--namespace=kubeapps", @@ -629,7 +622,7 @@ func Test_newSyncJob(t *testing.T) { { Name: "DB_PASSWORD", ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "mongodb"}, Key: "mongodb-root-password"}}, + SecretKeyRef: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "postgresql"}, Key: "postgresql-root-password"}}, }, }, VolumeMounts: []corev1.VolumeMount{{ @@ -716,8 +709,7 @@ func Test_newSyncJob(t *testing.T) { Command: []string{"/chart-repo"}, Args: []string{ "sync", - "--database-type=mongodb", - "--database-url=mongodb.kubeapps", + "--database-url=postgresql.kubeapps", "--database-user=admin", "--database-name=assets", "--namespace=kubeapps", @@ -728,7 +720,7 @@ func Test_newSyncJob(t *testing.T) { { Name: "DB_PASSWORD", ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "mongodb"}, Key: "mongodb-root-password"}}, + SecretKeyRef: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "postgresql"}, Key: "postgresql-root-password"}}, }, { Name: "AUTHORIZATION_HEADER", @@ -833,8 +825,7 @@ func Test_newSyncJob(t *testing.T) { Command: []string{"/chart-repo"}, Args: []string{ "sync", - "--database-type=mongodb", - "--database-url=mongodb.kubeapps", + "--database-url=postgresql.kubeapps", "--database-user=admin", "--database-name=assets", "--namespace=kubeapps", @@ -846,7 +837,7 @@ func Test_newSyncJob(t *testing.T) { { Name: "DB_PASSWORD", ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "mongodb"}, Key: "mongodb-root-password"}}, + SecretKeyRef: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "postgresql"}, Key: "postgresql-root-password"}}, }, }, VolumeMounts: []corev1.VolumeMount{{Name: "foo", MountPath: "/bar"}}, @@ -877,10 +868,10 @@ func Test_newSyncJob(t *testing.T) { } func Test_newCleanupJob(t *testing.T) { - dbURL = "mongodb.kubeapps" + dbURL = "postgresql.kubeapps" dbName = "assets" dbUser = "admin" - dbSecretName = "mongodb" + dbSecretName = "postgresql" const kubeappsNamespace = "kubeapps" tests := []struct { @@ -912,8 +903,7 @@ func Test_newCleanupJob(t *testing.T) { "delete", "my-charts", "--namespace=kubeapps", - "--database-type=mongodb", - "--database-url=mongodb.kubeapps", + "--database-url=postgresql.kubeapps", "--database-user=admin", "--database-name=assets", }, @@ -921,7 +911,7 @@ func Test_newCleanupJob(t *testing.T) { { Name: "DB_PASSWORD", ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "mongodb"}, Key: "mongodb-root-password"}}, + SecretKeyRef: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "postgresql"}, Key: "postgresql-root-password"}}, }, }, }, diff --git a/cmd/apprepository-controller/main.go b/cmd/apprepository-controller/main.go index ee7be317c8e..424337f9f8d 100644 --- a/cmd/apprepository-controller/main.go +++ b/cmd/apprepository-controller/main.go @@ -36,7 +36,6 @@ var ( repoSyncImage string repoSyncCommand string namespace string - dbType string dbURL string dbUser string dbName string @@ -89,12 +88,11 @@ func init() { flag.StringVar(&repoSyncCommand, "repo-sync-cmd", "/chart-repo", "command used to sync/delete repos for repo-sync-image") flag.StringVar(&namespace, "namespace", "kubeapps", "Namespace to discover AppRepository resources") flag.BoolVar(&reposPerNamespace, "repos-per-namespace", true, "UNUSED: This flag will be removed in a future release.") - flag.StringVar(&dbType, "database-type", "mongodb", "Database type. Allowed values: mongodb, postgresql") flag.StringVar(&dbURL, "database-url", "localhost", "Database URL") flag.StringVar(&dbUser, "database-user", "root", "Database user") flag.StringVar(&dbName, "database-name", "charts", "Database name") - flag.StringVar(&dbSecretName, "database-secret-name", "mongodb", "Kubernetes secret name for database credentials") - flag.StringVar(&dbSecretKey, "database-secret-key", "mongodb-root-password", "Kubernetes secret key used for database credentials") + flag.StringVar(&dbSecretName, "database-secret-name", "kubeapps-db", "Kubernetes secret name for database credentials") + flag.StringVar(&dbSecretKey, "database-secret-key", "postgresql-root-password", "Kubernetes secret key used for database credentials") flag.StringVar(&userAgentComment, "user-agent-comment", "", "UserAgent comment used during outbound requests") flag.StringVar(&crontab, "crontab", "*/10 * * * *", "CronTab to specify schedule") } diff --git a/cmd/asset-syncer/delete.go b/cmd/asset-syncer/delete.go index cf3c3f7f7c0..6f841ac1610 100644 --- a/cmd/asset-syncer/delete.go +++ b/cmd/asset-syncer/delete.go @@ -41,7 +41,7 @@ var deleteCmd = &cobra.Command{ dbConfig := datastore.Config{URL: databaseURL, Database: databaseName, Username: databaseUser, Password: databasePassword} kubeappsNamespace := os.Getenv("POD_NAMESPACE") - manager, err := newManager(databaseType, dbConfig, kubeappsNamespace) + manager, err := newManager(dbConfig, kubeappsNamespace) if err != nil { logrus.Fatal(err) } diff --git a/cmd/asset-syncer/invalidate-cache.go b/cmd/asset-syncer/invalidate-cache.go index 40ee5de7f9b..6f844f111b8 100644 --- a/cmd/asset-syncer/invalidate-cache.go +++ b/cmd/asset-syncer/invalidate-cache.go @@ -40,7 +40,7 @@ var invalidateCacheCmd = &cobra.Command{ dbConfig := datastore.Config{URL: databaseURL, Database: databaseName, Username: databaseUser, Password: databasePassword} kubeappsNamespace := os.Getenv("POD_NAMESPACE") - manager, err := newManager(databaseType, dbConfig, kubeappsNamespace) + manager, err := newManager(dbConfig, kubeappsNamespace) if err != nil { logrus.Fatal(err) } diff --git a/cmd/asset-syncer/main.go b/cmd/asset-syncer/main.go index 9933bbf92d3..d7b3c54cb60 100644 --- a/cmd/asset-syncer/main.go +++ b/cmd/asset-syncer/main.go @@ -23,7 +23,6 @@ import ( ) var ( - databaseType string databaseURL string databaseName string databaseUser string @@ -48,7 +47,6 @@ func main() { } func init() { - rootCmd.PersistentFlags().StringVar(&databaseType, "database-type", "mongodb", "Database to use. Choice: mongodb, postgresql") rootCmd.PersistentFlags().StringVar(&databaseURL, "database-url", "localhost", "Database URL") rootCmd.PersistentFlags().StringVar(&databaseName, "database-name", "charts", "Name of the database to use") rootCmd.PersistentFlags().StringVar(&databaseUser, "database-user", "", "Database user") diff --git a/cmd/asset-syncer/mongodb_db_test.go b/cmd/asset-syncer/mongodb_db_test.go deleted file mode 100644 index c0cfc416803..00000000000 --- a/cmd/asset-syncer/mongodb_db_test.go +++ /dev/null @@ -1,178 +0,0 @@ -/* -Copyright (c) 2020 Bitnami - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -// Currently these tests will be skipped entirely unless the -// ENABLE_MONGO_INTEGRATION_TESTS env var is set. -// Run the local mongodb with -// docker run --publish 27017:27017 -e MONGODB_ROOT_PASSWORD=testpassword -e ALLOW_EMPTY_PASSWORD=yes bitnami/mongodb:4.2.3-debian-10-r31 -// in another terminal. -package main - -import ( - "errors" - "testing" - - "github.com/google/go-cmp/cmp" - "github.com/google/go-cmp/cmp/cmpopts" - "github.com/kubeapps/kubeapps/pkg/chart/models" - "github.com/kubeapps/kubeapps/pkg/dbutils" - "github.com/kubeapps/kubeapps/pkg/dbutils/dbutilstest/mgtest" -) - -func getInitializedMongoManager(t *testing.T) (*mongodbAssetManager, func()) { - manager, cleanup := mgtest.GetInitializedManager(t) - return &mongodbAssetManager{manager}, cleanup -} - -func TestMongoImportCharts(t *testing.T) { - mgtest.SkipIfNoDB(t) - - repo := models.Repo{ - Name: "repo-name", - Namespace: "repo-namespace", - } - repoSameNameOtherNamespace := models.Repo{ - Name: "repo-name", - Namespace: "other-namespace", - } - - testCases := []struct { - name string - existingCharts []models.Chart - charts []models.Chart - expectedCharts []models.Chart - expectedError error - }{ - { - name: "it inserts the charts", - charts: []models.Chart{ - models.Chart{Name: "my-chart1", Repo: &repo, ID: "foo/bar:123"}, - models.Chart{Name: "my-chart2", Repo: &repo, ID: "foo/bar:456"}, - }, - expectedCharts: []models.Chart{ - models.Chart{Name: "my-chart1", Repo: &repo, ID: "foo/bar:123"}, - models.Chart{Name: "my-chart2", Repo: &repo, ID: "foo/bar:456"}, - }, - }, - { - name: "it errors if asked to insert a chart in a different namespace", - charts: []models.Chart{ - models.Chart{Name: "my-chart1", Repo: &repo, ID: "foo/bar:123"}, - models.Chart{Name: "my-chart2", Repo: &repo, ID: "foo/bar:456"}, - models.Chart{Name: "my-chart1", Repo: &repoSameNameOtherNamespace, ID: "foo/bar:123"}, - }, - expectedError: ErrRepoMismatch, - }, - { - name: "it updates existing charts in the chart namespace", - existingCharts: []models.Chart{ - models.Chart{Name: "my-chart1", Repo: &repo, ID: "foo/bar:123", Description: "Old description"}, - }, - charts: []models.Chart{ - models.Chart{Name: "my-chart1", Repo: &repo, ID: "foo/bar:123", Description: "New description"}, - models.Chart{Name: "my-chart2", Repo: &repo, ID: "foo/bar:456"}, - }, - expectedCharts: []models.Chart{ - models.Chart{Name: "my-chart1", Repo: &repo, ID: "foo/bar:123", Description: "New description"}, - models.Chart{Name: "my-chart2", Repo: &repo, ID: "foo/bar:456"}, - }, - }, - { - name: "it removes charts that are not included in the import", - existingCharts: []models.Chart{ - models.Chart{Name: "my-chart-old", Repo: &repo, ID: "foo/old:123"}, - }, - charts: []models.Chart{ - models.Chart{Name: "my-chart1", Repo: &repo, ID: "foo/bar:123", Description: "New description"}, - models.Chart{Name: "my-chart2", Repo: &repo, ID: "foo/bar:456"}, - }, - expectedCharts: []models.Chart{ - models.Chart{Name: "my-chart1", Repo: &repo, ID: "foo/bar:123", Description: "New description"}, - models.Chart{Name: "my-chart2", Repo: &repo, ID: "foo/bar:456"}, - }, - }, - { - name: "it does not remove charts from other namespaces", - existingCharts: []models.Chart{ - models.Chart{Name: "my-chart-old", Repo: &repoSameNameOtherNamespace, ID: "foo/other:123"}, - }, - charts: []models.Chart{ - models.Chart{Name: "my-chart1", Repo: &repo, ID: "foo/bar:123", Description: "New description"}, - models.Chart{Name: "my-chart2", Repo: &repo, ID: "foo/bar:456"}, - }, - expectedCharts: []models.Chart{ - models.Chart{Name: "my-chart-old", Repo: &repoSameNameOtherNamespace, ID: "foo/other:123"}, - models.Chart{Name: "my-chart1", Repo: &repo, ID: "foo/bar:123", Description: "New description"}, - models.Chart{Name: "my-chart2", Repo: &repo, ID: "foo/bar:456"}, - }, - }, - { - name: "it does not remove charts from other namespaces even if they have the same repo name", - existingCharts: []models.Chart{ - models.Chart{Name: "my-chart-old", Repo: &repoSameNameOtherNamespace, ID: "foo/bar:123"}, - }, - charts: []models.Chart{ - models.Chart{Name: "my-chart1", Repo: &repo, ID: "foo/bar:123", Description: "New description"}, - models.Chart{Name: "my-chart2", Repo: &repo, ID: "foo/bar:456"}, - }, - expectedCharts: []models.Chart{ - models.Chart{Name: "my-chart-old", Repo: &repoSameNameOtherNamespace, ID: "foo/bar:123"}, - models.Chart{Name: "my-chart1", Repo: &repo, ID: "foo/bar:123", Description: "New description"}, - models.Chart{Name: "my-chart2", Repo: &repo, ID: "foo/bar:456"}, - }, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - manager, cleanup := getInitializedMongoManager(t) - defer cleanup() - if len(tc.existingCharts) > 0 { - err := manager.importCharts(tc.existingCharts, *tc.existingCharts[0].Repo) - if err != nil { - t.Fatalf("%+v", err) - } - } - - err := manager.importCharts(tc.charts, repo) - if tc.expectedError != nil { - if got, want := err, tc.expectedError; !errors.Is(got, want) { - t.Fatalf("got: %+v, want: %+v", got, want) - } - } else if err != nil { - t.Fatalf("%+v", err) - } - - opts := cmpopts.EquateEmpty() - if got, want := getAllCharts(t, manager), tc.expectedCharts; !cmp.Equal(want, got, opts) { - t.Errorf("mismatch (-want +got):\n%s", cmp.Diff(want, got, opts)) - } - }) - } -} - -func getAllCharts(t *testing.T, manager *mongodbAssetManager) []models.Chart { - var result []models.Chart - db, closer := manager.DBSession.DB() - defer closer() - - coll := db.C(dbutils.ChartCollection) - err := coll.Find(nil).Sort("repo.name", "repo.namespace", "id").All(&result) - if err != nil { - t.Fatalf("%+v", err) - } - return result -} diff --git a/cmd/asset-syncer/mongodb_utils.go b/cmd/asset-syncer/mongodb_utils.go deleted file mode 100644 index 62bb3507a0d..00000000000 --- a/cmd/asset-syncer/mongodb_utils.go +++ /dev/null @@ -1,152 +0,0 @@ -/* -Copyright (c) 2018 The Helm Authors - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package main - -import ( - "fmt" - "time" - - "github.com/globalsign/mgo/bson" - "github.com/kubeapps/common/datastore" - "github.com/kubeapps/kubeapps/pkg/chart/models" - "github.com/kubeapps/kubeapps/pkg/dbutils" -) - -var ErrRepoMismatch = fmt.Errorf("chart repository did not match import repository") - -type mongodbAssetManager struct { - *dbutils.MongodbAssetManager -} - -func newMongoDBManager(config datastore.Config, kubeappsNamespace string) assetManager { - m := dbutils.NewMongoDBManager(config, kubeappsNamespace) - return &mongodbAssetManager{m} -} - -// Syncing is performed in the following steps: -// 1. Update database to match chart metadata from index -// 2. Concurrently process icons for charts (concurrently) -// 3. Concurrently process the README and values.yaml for the latest chart version of each chart -// 4. Concurrently process READMEs and values.yaml for historic chart versions -// -// 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 (m *mongodbAssetManager) Sync(repo models.Repo, charts []models.Chart) error { - err := m.InitCollections() - if err != nil { - return err - } - - return m.importCharts(charts, repo) -} - -func (m *mongodbAssetManager) RepoAlreadyProcessed(repo models.Repo, checksum string) bool { - db, closer := m.DBSession.DB() - defer closer() - lastCheck := &models.RepoCheck{} - err := db.C(dbutils.RepositoryCollection).Find(bson.M{"name": repo.Name, "namespace": repo.Namespace}).One(lastCheck) - return err == nil && checksum == lastCheck.Checksum -} - -func (m *mongodbAssetManager) UpdateLastCheck(repoNamespace, repoName, checksum string, now time.Time) error { - db, closer := m.DBSession.DB() - defer closer() - _, err := db.C(dbutils.RepositoryCollection).Upsert(bson.M{"name": repoName, "namespace": repoNamespace}, bson.M{"$set": bson.M{"last_update": now, "checksum": checksum}}) - return err -} - -func (m *mongodbAssetManager) Delete(repo models.Repo) error { - db, closer := m.DBSession.DB() - defer closer() - _, err := db.C(dbutils.ChartCollection).RemoveAll(bson.M{ - "repo.name": repo.Name, - "repo.namespace": repo.Namespace, - }) - if err != nil { - return err - } - - _, err = db.C(dbutils.ChartFilesCollection).RemoveAll(bson.M{ - "repo.name": repo.Name, - "repo.namespace": repo.Namespace, - }) - if err != nil { - return err - } - - _, err = db.C(dbutils.RepositoryCollection).RemoveAll(bson.M{ - "name": repo.Name, - "namespace": repo.Namespace, - }) - return err -} - -func (m *mongodbAssetManager) importCharts(charts []models.Chart, repo models.Repo) error { - var pairs []interface{} - var chartIDs []string - for _, c := range charts { - if c.Repo == nil || c.Repo.Namespace != repo.Namespace || c.Repo.Name != repo.Name { - return fmt.Errorf("%w: chart repo: %+v, import repo: %+v", ErrRepoMismatch, c.Repo, repo) - } - chartIDs = append(chartIDs, c.ID) - // charts to upsert - pair of selector, chart - // Mongodb generates the unique _id, we rely on the compound unique index on chart_id and repo. - pairs = append(pairs, bson.M{"chart_id": c.ID, "repo.name": repo.Name, "repo.namespace": repo.Namespace}, bson.M{"$set": c}) - } - - db, closer := m.DBSession.DB() - defer closer() - bulk := db.C(dbutils.ChartCollection).Bulk() - - // Upsert pairs of selectors, charts - bulk.Upsert(pairs...) - - // Remove charts no longer existing in index - bulk.RemoveAll(bson.M{ - "chart_id": bson.M{ - "$nin": chartIDs, - }, - "repo.name": repo.Name, - "repo.namespace": repo.Namespace, - }) - - _, err := bulk.Run() - return err -} - -func (m *mongodbAssetManager) updateIcon(repo models.Repo, data []byte, contentType, ID string) error { - db, closer := m.DBSession.DB() - defer closer() - _, err := db.C(dbutils.ChartCollection).Upsert(bson.M{"chart_id": ID, "repo.name": repo.Name, "repo.namespace": repo.Namespace}, bson.M{"$set": bson.M{"raw_icon": data, "icon_content_type": contentType}}) - return err -} - -func (m *mongodbAssetManager) filesExist(repo models.Repo, chartFilesID, digest string) bool { - db, closer := m.DBSession.DB() - defer closer() - err := db.C(dbutils.ChartFilesCollection).Find(bson.M{"file_id": chartFilesID, "repo.name": repo.Name, "repo.namespace": repo.Namespace, "digest": digest}).One(&models.ChartFiles{}) - return err == nil -} - -func (m *mongodbAssetManager) insertFiles(chartId string, files models.ChartFiles) error { - db, closer := m.DBSession.DB() - defer closer() - - _, err := db.C(dbutils.ChartFilesCollection).Upsert(bson.M{"file_id": files.ID, "repo.name": files.Repo.Name, "repo.namespace": files.Repo.Namespace}, files) - return err -} diff --git a/cmd/asset-syncer/mongodb_utils_test.go b/cmd/asset-syncer/mongodb_utils_test.go deleted file mode 100644 index 8bca758a446..00000000000 --- a/cmd/asset-syncer/mongodb_utils_test.go +++ /dev/null @@ -1,140 +0,0 @@ -/* -Copyright (c) 2018 The Helm Authors - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package main - -import ( - "testing" - "time" - - "github.com/arschles/assert" - "github.com/globalsign/mgo/bson" - "github.com/kubeapps/common/datastore" - "github.com/kubeapps/common/datastore/mockstore" - "github.com/kubeapps/kubeapps/pkg/chart/models" - "github.com/kubeapps/kubeapps/pkg/dbutils" - "github.com/kubeapps/kubeapps/pkg/dbutils/dbutilstest" - "github.com/stretchr/testify/mock" -) - -func getMockManager(m *mock.Mock) *mongodbAssetManager { - dbSession := mockstore.NewMockSession(m) - man := dbutils.NewMongoDBManager(datastore.Config{}, dbutilstest.KubeappsTestNamespace) - man.DBSession = dbSession - return &mongodbAssetManager{man} -} - -func Test_importCharts(t *testing.T) { - m := &mock.Mock{} - // Ensure Upsert func is called with some arguments - m.On("Upsert", mock.Anything) - m.On("RemoveAll", mock.Anything) - index, _ := parseRepoIndex([]byte(validRepoIndexYAML)) - repo := models.Repo{ - Name: "repo-name", - Namespace: "repo-namespace", - URL: "http://testrepo.example.com", - } - charts := chartsFromIndex(index, &repo) - manager := getMockManager(m) - manager.importCharts(charts, repo) - - m.AssertExpectations(t) - // The Bulk Upsert method takes an array that consists of a selector followed by an interface to upsert. - // So for x charts to upsert, there should be x*2 elements (each chart has it's own selector) - // e.g. [selector1, chart1, selector2, chart2, ...] - args := m.Calls[0].Arguments.Get(0).([]interface{}) - assert.Equal(t, len(args), len(charts)*2, "number of selector, chart pairs to upsert") - for i := 0; i < len(args); i += 2 { - m := args[i+1].(bson.M) - c := m["$set"].(models.Chart) - assert.Equal(t, args[i], bson.M{"chart_id": "repo-name/" + c.Name, "repo.name": "repo-name", "repo.namespace": "repo-namespace"}, "selector") - } -} - -func Test_DeleteRepo(t *testing.T) { - m := &mock.Mock{} - repo := models.Repo{Name: "repo-name", Namespace: "repo-namespace"} - m.On("RemoveAll", bson.M{ - "repo.name": repo.Name, - "repo.namespace": repo.Namespace, - }) - m.On("RemoveAll", bson.M{ - "name": repo.Name, - "namespace": repo.Namespace, - }) - manager := getMockManager(m) - err := manager.Delete(repo) - if err != nil { - t.Errorf("failed to delete chart repo test: %v", err) - } - m.AssertExpectations(t) -} - -func Test_emptyChartRepo(t *testing.T) { - r := &models.Repo{Name: "testRepo", URL: "https://my.examplerepo.com"} - i, err := parseRepoIndex(emptyRepoIndexYAMLBytes) - assert.NoErr(t, err) - charts := chartsFromIndex(i, r) - assert.Equal(t, len(charts), 0, "charts") -} - -func Test_repoAlreadyProcessed(t *testing.T) { - tests := []struct { - name string - checksum string - mockedLastCheck models.RepoCheck - processed bool - }{ - {"not processed yet", "bar", models.RepoCheck{}, false}, - {"already processed", "bar", models.RepoCheck{Checksum: "bar"}, true}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - m := &mock.Mock{} - repo := &models.RepoCheck{} - m.On("One", repo).Run(func(args mock.Arguments) { - *args.Get(0).(*models.RepoCheck) = tt.mockedLastCheck - }).Return(nil) - manager := getMockManager(m) - res := manager.RepoAlreadyProcessed(models.Repo{Namespace: "repo-namespace", Name: "repo-name"}, tt.checksum) - if res != tt.processed { - t.Errorf("Expected alreadyProcessed to be %v got %v", tt.processed, res) - } - }) - } -} - -func Test_updateLastCheck(t *testing.T) { - m := &mock.Mock{} - const ( - repoNamespace = "repoNamespace" - repoName = "foo" - checksum = "bar" - ) - now := time.Now() - m.On("Upsert", bson.M{"name": repoName, "namespace": repoNamespace}, bson.M{"$set": bson.M{"last_update": now, "checksum": checksum}}).Return(nil) - manager := getMockManager(m) - err := manager.UpdateLastCheck(repoNamespace, repoName, checksum, now) - m.AssertExpectations(t) - if err != nil { - t.Errorf("Unexpected error %v", err) - } - if len(m.Calls) != 1 { - t.Errorf("Expected one call got %d", len(m.Calls)) - } -} diff --git a/cmd/asset-syncer/postgresql_utils.go b/cmd/asset-syncer/postgresql_utils.go index 158dc111b65..f9ebfe33098 100644 --- a/cmd/asset-syncer/postgresql_utils.go +++ b/cmd/asset-syncer/postgresql_utils.go @@ -28,6 +28,7 @@ import ( "github.com/kubeapps/kubeapps/pkg/chart/models" "github.com/kubeapps/kubeapps/pkg/dbutils" _ "github.com/lib/pq" + log "github.com/sirupsen/logrus" ) var ErrMultipleRows = fmt.Errorf("more than one row returned in query result") @@ -76,6 +77,7 @@ func (m *postgresAssetManager) RepoAlreadyProcessed(repo models.Repo, repoChecks row := m.DB.QueryRow(fmt.Sprintf("SELECT checksum FROM %s WHERE name = $1 AND namespace = $2", dbutils.RepositoryTable), repo.Name, repo.Namespace) if row != nil { err := row.Scan(&lastChecksum) + log.Errorf("lastChecksum: %+v", lastChecksum) return err == nil && lastChecksum == repoChecksum } return false @@ -160,13 +162,13 @@ func (m *postgresAssetManager) filesExist(repo models.Repo, chartFilesID, digest var exists bool err := m.DB.QueryRow( fmt.Sprintf(` -SELECT EXISTS( - SELECT 1 FROM %s - WHERE chart_files_id = $1 AND - repo_name = $2 AND - repo_namespace = $3 AND - info ->> 'Digest' = $4 - )`, dbutils.ChartFilesTable), + SELECT EXISTS( + SELECT 1 FROM %s + WHERE chart_files_id = $1 AND + repo_name = $2 AND + repo_namespace = $3 AND + info ->> 'Digest' = $4 + )`, dbutils.ChartFilesTable), chartFilesID, repo.Name, repo.Namespace, digest).Scan(&exists) return err == nil && exists } diff --git a/cmd/asset-syncer/postgresql_utils_test.go b/cmd/asset-syncer/postgresql_utils_test.go index 30feea3eb00..9e2c8f5af76 100644 --- a/cmd/asset-syncer/postgresql_utils_test.go +++ b/cmd/asset-syncer/postgresql_utils_test.go @@ -2,173 +2,182 @@ package main import ( "database/sql" + "errors" "testing" "time" "github.com/DATA-DOG/go-sqlmock" - "github.com/kubeapps/common/datastore" "github.com/kubeapps/kubeapps/pkg/chart/models" "github.com/kubeapps/kubeapps/pkg/dbutils" - "github.com/kubeapps/kubeapps/pkg/dbutils/dbutilstest" - "github.com/stretchr/testify/mock" ) -type mockDB struct { - *mock.Mock -} - -func (d *mockDB) Query(query string, args ...interface{}) (*sql.Rows, error) { - d.Called(query, args) - return nil, nil -} - -func (d *mockDB) Begin() (*sql.Tx, error) { - d.Called() - return nil, nil -} - -func (d *mockDB) QueryRow(query string, args ...interface{}) *sql.Row { - d.Called(query, args) - return nil -} +func getMockManager(t *testing.T) (*postgresAssetManager, sqlmock.Sqlmock, func()) { + db, mock, err := sqlmock.New() + if err != nil { + t.Fatalf("%+v", err) + } -func (d *mockDB) Close() error { - return nil -} + pgManager := &postgresAssetManager{&dbutils.PostgresAssetManager{DB: db}} -func (d *mockDB) Exec(query string, args ...interface{}) (sql.Result, error) { - return nil, nil + return pgManager, mock, func() { db.Close() } } func Test_DeletePGRepo(t *testing.T) { + pgManager, mock, cleanup := getMockManager(t) + defer cleanup() + repo := models.Repo{Name: "testrepo", Namespace: "testnamespace"} - m := &mockDB{&mock.Mock{}} - m.On("Query", "DELETE FROM repos WHERE name = $1 AND namespace = $2", []interface{}{repo.Name, repo.Namespace}) + mock.ExpectQuery(`DELETE FROM repos WHERE name = \$1 AND namespace = \$2`). + WithArgs(repo.Name, repo.Namespace). + WillReturnRows(sqlmock.NewRows([]string{"ID"}).AddRow(1)) - man, _ := dbutils.NewPGManager(datastore.Config{URL: "localhost:4123"}, dbutilstest.KubeappsTestNamespace) - man.DB = m - pgManager := &postgresAssetManager{man} err := pgManager.Delete(repo) if err != nil { t.Errorf("failed to delete chart repo test: %v", err) } - m.AssertExpectations(t) } -func Test_PGRepoAlreadyPropcessed(t *testing.T) { - m := &mockDB{&mock.Mock{}} - man, _ := dbutils.NewPGManager(datastore.Config{URL: "localhost:4123"}, dbutilstest.KubeappsTestNamespace) - man.DB = m - pgManager := &postgresAssetManager{man} - m.On("QueryRow", "SELECT checksum FROM repos WHERE name = $1 AND namespace = $2", []interface{}{"foo", "repo-namespace"}) - pgManager.RepoAlreadyProcessed(models.Repo{Namespace: "repo-namespace", Name: "foo"}, "123") - m.AssertExpectations(t) +func Test_PGRepoAlreadyProcessed(t *testing.T) { + pgManager, mock, cleanup := getMockManager(t) + defer cleanup() + + mock.ExpectQuery("SELECT checksum FROM repos *"). + WithArgs("foo", "repo-namespace"). + WillReturnRows(sqlmock.NewRows([]string{"checksum"}).AddRow("123")) + + if got, want := pgManager.RepoAlreadyProcessed(models.Repo{Namespace: "repo-namespace", Name: "foo"}, "123"), true; got != want { + t.Errorf("got: %t, want: %t", got, want) + } } func Test_PGUpdateLastCheck(t *testing.T) { - m := &mockDB{&mock.Mock{}} const ( repoNamespace = "repoNamespace" repoName = "foo" checksum = "bar" ) + + pgManager, mock, cleanup := getMockManager(t) + defer cleanup() + now := time.Now() - man, _ := dbutils.NewPGManager(datastore.Config{URL: "localhost:4123"}, dbutilstest.KubeappsTestNamespace) - man.DB = m - pgManager := &postgresAssetManager{man} - expectedQuery := `INSERT INTO repos (namespace, name, checksum, last_update) - VALUES ($1, $2, $3, $4) - ON CONFLICT (namespace, name) - DO UPDATE SET last_update = $4, checksum = $3 - ` - m.On("Query", expectedQuery, []interface{}{repoNamespace, repoName, checksum, now.String()}) - pgManager.UpdateLastCheck(repoNamespace, repoName, checksum, now) - m.AssertExpectations(t) + mock.ExpectQuery("INSERT INTO repos *"). + WithArgs(repoNamespace, repoName, checksum, now.String()). + WillReturnRows(sqlmock.NewRows([]string{"ID"}).AddRow("3")) + + err := pgManager.UpdateLastCheck(repoNamespace, repoName, checksum, now) + if err != nil { + t.Errorf("%+v", err) + } } func Test_PGremoveMissingCharts(t *testing.T) { + pgManager, mock, cleanup := getMockManager(t) + defer cleanup() + repo := models.Repo{Name: "repo"} charts := []models.Chart{{ID: "foo", Repo: &repo}, {ID: "bar"}} - m := &mockDB{&mock.Mock{}} - man, _ := dbutils.NewPGManager(datastore.Config{URL: "localhost:4123"}, dbutilstest.KubeappsTestNamespace) - man.DB = m - pgManager := &postgresAssetManager{man} - m.On("Query", "DELETE FROM charts WHERE chart_id NOT IN ('foo', 'bar') AND repo_name = $1 AND repo_namespace = $2", []interface{}{repo.Name, repo.Namespace}) - pgManager.removeMissingCharts(repo, charts) - m.AssertExpectations(t) + mock.ExpectQuery(`^DELETE FROM charts WHERE chart_id NOT IN \('foo', 'bar'\) AND repo_name = \$1 AND repo_namespace = \$2`). + WithArgs(repo.Name, repo.Namespace). + WillReturnRows(sqlmock.NewRows([]string{"ID"}).AddRow(1).AddRow(2)) + + err := pgManager.removeMissingCharts(repo, charts) + if err != nil { + t.Errorf("%+v", err) + } } func Test_PGupdateIcon(t *testing.T) { data := []byte("foo") contentType := "image/png" id := "stable/wordpress" - m := &mockDB{&mock.Mock{}} - man, _ := dbutils.NewPGManager(datastore.Config{URL: "localhost:4123"}, dbutilstest.KubeappsTestNamespace) - man.DB = m - pgManager := &postgresAssetManager{man} - m.On( - "Query", - `UPDATE charts SET info = info || '{"raw_icon": "Zm9v", "icon_content_type": "image/png"}' WHERE chart_id = $1 AND repo_namespace = $2 AND repo_name = $3 RETURNING ID`, - []interface{}{"stable/wordpress", "repo-namespace", "repo-name"}, - ) - err := pgManager.updateIcon(models.Repo{Namespace: "repo-namespace", Name: "repo-name"}, data, contentType, id) - if err != nil { - t.Errorf("Failed to update icon") - } - m.AssertExpectations(t) + + t.Run("one icon only", func(t *testing.T) { + pgManager, mock, cleanup := getMockManager(t) + defer cleanup() + mock.ExpectQuery(`^UPDATE charts SET info = info *`). + WithArgs("stable/wordpress", "repo-namespace", "repo-name"). + WillReturnRows(sqlmock.NewRows([]string{"ID"}).AddRow(1)) + + err := pgManager.updateIcon(models.Repo{Namespace: "repo-namespace", Name: "repo-name"}, data, contentType, id) + + if err != nil { + t.Errorf("%+v", err) + } + }) + + t.Run("no rows returned", func(t *testing.T) { + pgManager, mock, cleanup := getMockManager(t) + defer cleanup() + mock.ExpectQuery(`^UPDATE charts SET info = info *`). + WithArgs("stable/wordpress", "repo-namespace", "repo-name"). + WillReturnRows(sqlmock.NewRows([]string{"ID"})) + + err := pgManager.updateIcon(models.Repo{Namespace: "repo-namespace", Name: "repo-name"}, data, contentType, id) + + if got, want := err, sql.ErrNoRows; got != want { + t.Errorf("got: %+v, want: %+v", got, want) + } + }) + + t.Run("more than one icon", func(t *testing.T) { + pgManager, mock, cleanup := getMockManager(t) + defer cleanup() + mock.ExpectQuery(`^UPDATE charts SET info = info *`). + WithArgs("stable/wordpress", "repo-namespace", "repo-name"). + WillReturnRows(sqlmock.NewRows([]string{"ID"}).AddRow(1).AddRow(2)) + + err := pgManager.updateIcon(models.Repo{Namespace: "repo-namespace", Name: "repo-name"}, data, contentType, id) + + if got, want := errors.Is(err, ErrMultipleRows), true; got != want { + t.Errorf("got: %t, want: %t", got, want) + } + }) } func Test_PGfilesExist(t *testing.T) { - db, mock, err := sqlmock.New() - if err != nil { - t.Errorf("Unexpected error %v", err) - } + pgManager, mock, cleanup := getMockManager(t) + defer cleanup() + + const ( + id = "stable/wordpress" + digest = "foo" + ) + repo := models.Repo{Namespace: "namespace", Name: "repo-name"} + rows := sqlmock.NewRows([]string{"info"}).AddRow(`true`) mock.ExpectQuery(`^SELECT EXISTS\( - SELECT 1 FROM files - WHERE chart_files_id = \$1 AND - repo_name = \$2 AND - repo_namespace = \$3 AND - info ->> 'Digest' = \$4 - \)$`).WillReturnRows(rows) - id := "stable/wordpress" - digest := "foo" - man := &dbutils.PostgresAssetManager{DB: db} - pgManager := &postgresAssetManager{man} - exists := pgManager.filesExist(models.Repo{Namespace: "namespace", Name: "repo-name"}, id, digest) + SELECT 1 FROM files + WHERE chart_files_id = \$1 AND + repo_name = \$2 AND + repo_namespace = \$3 AND + info ->> 'Digest' = \$4 + \)$`). + WithArgs(id, repo.Name, repo.Namespace, digest). + WillReturnRows(rows) + + exists := pgManager.filesExist(repo, id, digest) if exists != true { t.Errorf("Failed to check if file exists") } - err = mock.ExpectationsWereMet() - if err != nil { - t.Errorf("err %v", err) - } } func Test_PGinsertFiles(t *testing.T) { + pgManager, mock, cleanup := getMockManager(t) + defer cleanup() const ( - namespace = "my-namespace" - repoName = "my-repo" - chartId = repoName + "/wordpress" - filesId = chartId + "-2.1.3" + namespace = "my-namespace" + repoName = "my-repo" + chartID string = repoName + "/wordpress" + filesID string = chartID + "-2.1.3" ) - files := models.ChartFiles{ID: filesId, Readme: "foo", Values: "bar", Repo: &models.Repo{Namespace: namespace, Name: repoName}} - m := &mockDB{&mock.Mock{}} - man, _ := dbutils.NewPGManager(datastore.Config{URL: "localhost:4123"}, dbutilstest.KubeappsTestNamespace) - man.DB = m - pgManager := &postgresAssetManager{man} - m.On( - "Query", - `INSERT INTO files (chart_id, repo_name, repo_namespace, chart_files_ID, info) - VALUES ($1, $2, $3, $4, $5) - ON CONFLICT (repo_namespace, chart_files_ID) - DO UPDATE SET info = $5 - `, - []interface{}{chartId, repoName, namespace, filesId, files}, - ) - err := pgManager.insertFiles(chartId, files) + files := models.ChartFiles{ID: filesID, Readme: "foo", Values: "bar", Repo: &models.Repo{Namespace: namespace, Name: repoName}} + mock.ExpectQuery(`INSERT INTO files \(chart_id, repo_name, repo_namespace, chart_files_ID, info\)*`). + WithArgs(chartID, repoName, namespace, filesID, files). + WillReturnRows(sqlmock.NewRows([]string{"ID"}).AddRow("3")) + + err := pgManager.insertFiles(chartID, files) if err != nil { - t.Errorf("Failed to insert files: %+v", err) + t.Errorf("%+v", err) } - m.AssertExpectations(t) } diff --git a/cmd/asset-syncer/sync.go b/cmd/asset-syncer/sync.go index 4c123ee86fc..a9450cb9e9d 100644 --- a/cmd/asset-syncer/sync.go +++ b/cmd/asset-syncer/sync.go @@ -42,7 +42,7 @@ var syncCmd = &cobra.Command{ dbConfig := datastore.Config{URL: databaseURL, Database: databaseName, Username: databaseUser, Password: databasePassword} kubeappsNamespace := os.Getenv("POD_NAMESPACE") - manager, err := newManager(databaseType, dbConfig, kubeappsNamespace) + manager, err := newManager(dbConfig, kubeappsNamespace) if err != nil { logrus.Fatal(err) } diff --git a/cmd/asset-syncer/utils.go b/cmd/asset-syncer/utils.go index c4d91669fbc..a0e67593541 100644 --- a/cmd/asset-syncer/utils.go +++ b/cmd/asset-syncer/utils.go @@ -31,6 +31,7 @@ import ( "net/url" "os" "path" + "sort" "strings" "sync" "time" @@ -87,14 +88,8 @@ type assetManager interface { insertFiles(chartId string, files models.ChartFiles) error } -func newManager(databaseType string, config datastore.Config, kubeappsNamespace string) (assetManager, error) { - if databaseType == "mongodb" { - return newMongoDBManager(config, kubeappsNamespace), nil - } else if databaseType == "postgresql" { - return newPGManager(config, kubeappsNamespace) - } else { - return nil, fmt.Errorf("Unsupported database type %s", databaseType) - } +func newManager(config datastore.Config, kubeappsNamespace string) (assetManager, error) { + return newPGManager(config, kubeappsNamespace) } func getSha256(src []byte) (string, error) { @@ -184,6 +179,7 @@ func chartsFromIndex(index *helmrepo.IndexFile, r *models.Repo) []models.Chart { } charts = append(charts, newChart(entry, r)) } + sort.Slice(charts, func(i, j int) bool { return charts[i].ID < charts[j].ID }) return charts } diff --git a/cmd/asset-syncer/utils_test.go b/cmd/asset-syncer/utils_test.go index 89f68711a44..d9a23ba7999 100644 --- a/cmd/asset-syncer/utils_test.go +++ b/cmd/asset-syncer/utils_test.go @@ -21,7 +21,7 @@ import ( "bytes" "compress/gzip" "crypto/rand" - "errors" + "encoding/base64" "fmt" "image" "image/color" @@ -34,13 +34,12 @@ import ( "strings" "testing" + "github.com/DATA-DOG/go-sqlmock" "github.com/arschles/assert" "github.com/disintegration/imaging" - "github.com/globalsign/mgo/bson" "github.com/kubeapps/common/datastore" "github.com/kubeapps/kubeapps/pkg/chart/models" log "github.com/sirupsen/logrus" - "github.com/stretchr/testify/mock" ) var validRepoIndexYAMLBytes, _ = ioutil.ReadFile("testdata/valid-index.yaml") @@ -104,6 +103,10 @@ func iconBytes() []byte { return b.Bytes() } +func iconB64() string { + return base64.StdEncoding.EncodeToString(iconBytes()) +} + func (h *goodIconClient) Do(req *http.Request) (*http.Response, error) { w := httptest.NewRecorder() w.Write(iconBytes()) @@ -473,20 +476,18 @@ func Test_getSha256(t *testing.T) { func Test_newManager(t *testing.T) { tests := []struct { name string - database string dbName string dbURL string dbUser string dbPass string expectedManager string }{ - {"mongodb database", "mongodb", "charts", "example.com", "admin", "root", "&{{example.com charts admin root 0} }"}, - {"postgresql database", "postgresql", "assets", "example.com:44124", "postgres", "root", "&{host=example.com port=44124 user=postgres password=root dbname=assets sslmode=disable }"}, + {"postgresql database", "assets", "example.com:44124", "postgres", "root", "&{host=example.com port=44124 user=postgres password=root dbname=assets sslmode=disable }"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { config := datastore.Config{URL: tt.dbURL, Database: tt.dbName, Username: tt.dbUser, Password: tt.dbPass} - _, err := newManager(tt.database, config, "kubeapps") + _, err := newManager(config, "kubeapps") assert.NoErr(t, err) }) } @@ -494,61 +495,64 @@ func Test_newManager(t *testing.T) { } func Test_fetchAndImportIcon(t *testing.T) { - r := &models.RepoInternal{Name: "test", Namespace: "repo-namespace"} + repo := &models.RepoInternal{Name: "test", Namespace: "repo-namespace"} t.Run("no icon", func(t *testing.T) { - m := &mock.Mock{} + pgManager, _, cleanup := getMockManager(t) + defer cleanup() c := models.Chart{ID: "test/acs-engine-autoscaler"} - manager := getMockManager(m) - fImporter := fileImporter{manager} - assert.NoErr(t, fImporter.fetchAndImportIcon(c, r)) + fImporter := fileImporter{pgManager} + assert.NoErr(t, fImporter.fetchAndImportIcon(c, repo)) }) index, _ := parseRepoIndex([]byte(validRepoIndexYAML)) charts := chartsFromIndex(index, &models.Repo{Name: "test", Namespace: "repo-namespace", URL: "http://testrepo.com"}) t.Run("failed download", func(t *testing.T) { + pgManager, _, cleanup := getMockManager(t) + defer cleanup() netClient = &badHTTPClient{} - c := charts[0] - m := &mock.Mock{} - manager := getMockManager(m) - fImporter := fileImporter{manager} - assert.Err(t, fmt.Errorf("500 %s", c.Icon), fImporter.fetchAndImportIcon(c, r)) + fImporter := fileImporter{pgManager} + assert.Err(t, fmt.Errorf("500 %s", charts[0].Icon), fImporter.fetchAndImportIcon(charts[0], repo)) }) t.Run("bad icon", func(t *testing.T) { + pgManager, _, cleanup := getMockManager(t) + defer cleanup() netClient = &badIconClient{} c := charts[0] - m := &mock.Mock{} - manager := getMockManager(m) - fImporter := fileImporter{manager} - assert.Err(t, image.ErrFormat, fImporter.fetchAndImportIcon(c, r)) + fImporter := fileImporter{pgManager} + assert.Err(t, image.ErrFormat, fImporter.fetchAndImportIcon(c, repo)) }) t.Run("valid icon", func(t *testing.T) { + pgManager, mock, cleanup := getMockManager(t) + defer cleanup() netClient = &goodIconClient{} - c := charts[0] - m := &mock.Mock{} - m.On("Upsert", bson.M{"chart_id": c.ID, "repo.name": c.Repo.Name, "repo.namespace": c.Repo.Namespace}, bson.M{"$set": bson.M{"raw_icon": iconBytes(), "icon_content_type": "image/png"}}).Return(nil) - manager := getMockManager(m) - fImporter := fileImporter{manager} - assert.NoErr(t, fImporter.fetchAndImportIcon(c, r)) - m.AssertExpectations(t) + + mock.ExpectQuery("UPDATE charts SET info *"). + WithArgs("test/acs-engine-autoscaler", "repo-namespace", "test"). + WillReturnRows(sqlmock.NewRows([]string{"ID"}).AddRow(1)) + + fImporter := fileImporter{pgManager} + assert.NoErr(t, fImporter.fetchAndImportIcon(charts[0], repo)) }) t.Run("valid SVG icon", func(t *testing.T) { + pgManager, mock, cleanup := getMockManager(t) + defer cleanup() netClient = &svgIconClient{} c := models.Chart{ ID: "foo", Icon: "https://foo/bar/logo.svg", - Repo: &models.Repo{Name: r.Name, Namespace: r.Namespace}, + Repo: &models.Repo{Name: repo.Name, Namespace: repo.Namespace}, } - m := &mock.Mock{} - m.On("Upsert", bson.M{"chart_id": c.ID, "repo.name": c.Repo.Name, "repo.namespace": c.Repo.Namespace}, bson.M{"$set": bson.M{"raw_icon": []byte("foo"), "icon_content_type": "image/svg"}}).Return(nil) - manager := getMockManager(m) - fImporter := fileImporter{manager} - assert.NoErr(t, fImporter.fetchAndImportIcon(c, r)) - m.AssertExpectations(t) + mock.ExpectQuery("UPDATE charts SET info *"). + WithArgs("foo", "repo-namespace", "test"). + WillReturnRows(sqlmock.NewRows([]string{"ID"}).AddRow(1)) + + fImporter := fileImporter{pgManager} + assert.NoErr(t, fImporter.fetchAndImportIcon(c, repo)) }) } @@ -556,87 +560,107 @@ func Test_fetchAndImportFiles(t *testing.T) { index, _ := parseRepoIndex([]byte(validRepoIndexYAML)) repo := &models.RepoInternal{Name: "test", Namespace: "repo-namespace", URL: "http://testrepo.com"} charts := chartsFromIndex(index, &models.Repo{Name: repo.Name, Namespace: repo.Namespace, URL: repo.URL}) - cv := charts[0].ChartVersions[0] + chartVersion := charts[0].ChartVersions[0] + chartID := fmt.Sprintf("%s/%s", charts[0].Repo.Name, charts[0].Name) + chartFilesID := fmt.Sprintf("%s-%s", chartID, chartVersion.Version) + chartFiles := models.ChartFiles{ + ID: chartFilesID, + Readme: testChartReadme, + Values: testChartValues, + Schema: testChartSchema, + Repo: charts[0].Repo, + Digest: chartVersion.Digest, + } t.Run("http error", func(t *testing.T) { - m := mock.Mock{} - m.On("One", mock.Anything).Return(errors.New("return an error when checking if readme already exists to force fetching")) + pgManager, mock, cleanup := getMockManager(t) + defer cleanup() + + mock.ExpectQuery("SELECT EXISTS*"). + WithArgs(chartFilesID, repo.Name, repo.Namespace). + WillReturnRows(sqlmock.NewRows([]string{"ID"}).AddRow(1)) netClient = &badHTTPClient{} - manager := getMockManager(&m) - fImporter := fileImporter{manager} - assert.Err(t, io.EOF, fImporter.fetchAndImportFiles(charts[0].Name, repo, cv)) + fImporter := fileImporter{pgManager} + assert.Err(t, io.EOF, fImporter.fetchAndImportFiles(charts[0].Name, repo, chartVersion)) }) t.Run("file not found", func(t *testing.T) { - netClient = &goodTarballClient{c: charts[0], skipValues: true, skipReadme: true, skipSchema: true} - m := mock.Mock{} - m.On("One", mock.Anything).Return(errors.New("return an error when checking if files already exists to force fetching")) - chartFilesID := fmt.Sprintf("%s/%s-%s", charts[0].Repo.Name, charts[0].Name, cv.Version) - m.On("Upsert", bson.M{"file_id": chartFilesID, "repo.name": repo.Name, "repo.namespace": repo.Namespace}, models.ChartFiles{ + pgManager, mock, cleanup := getMockManager(t) + defer cleanup() + + files := models.ChartFiles{ ID: chartFilesID, Readme: "", Values: "", Schema: "", Repo: charts[0].Repo, - Digest: cv.Digest, - }) + Digest: chartVersion.Digest, + } + + // file does not exist (no rows returned) so insertion goes ahead. + mock.ExpectQuery(`SELECT EXISTS*`). + WithArgs(chartFilesID, repo.Name, repo.Namespace, chartVersion.Digest). + WillReturnRows(sqlmock.NewRows([]string{"info"})) + mock.ExpectQuery("INSERT INTO files *"). + WithArgs(chartID, repo.Name, repo.Namespace, chartFilesID, files). + WillReturnRows(sqlmock.NewRows([]string{"ID"}).AddRow("3")) + + netClient = &goodTarballClient{c: charts[0], skipValues: true, skipReadme: true, skipSchema: true} - manager := getMockManager(&m) - fImporter := fileImporter{manager} - err := fImporter.fetchAndImportFiles(charts[0].Name, repo, cv) + fImporter := fileImporter{pgManager} + err := fImporter.fetchAndImportFiles(charts[0].Name, repo, chartVersion) assert.NoErr(t, err) - m.AssertExpectations(t) }) t.Run("authenticated request", func(t *testing.T) { + pgManager, mock, cleanup := getMockManager(t) + defer cleanup() + + // file does not exist (no rows returned) so insertion goes ahead. + mock.ExpectQuery(`SELECT EXISTS*`). + WithArgs(chartFilesID, repo.Name, repo.Namespace, chartVersion.Digest). + WillReturnRows(sqlmock.NewRows([]string{"info"})) + mock.ExpectQuery("INSERT INTO files *"). + WithArgs(chartID, repo.Name, repo.Namespace, chartFilesID, chartFiles). + WillReturnRows(sqlmock.NewRows([]string{"ID"}).AddRow("3")) + netClient = &authenticatedTarballClient{c: charts[0]} - m := mock.Mock{} - m.On("One", mock.Anything).Return(errors.New("return an error when checking if files already exists to force fetching")) - chartFilesID := fmt.Sprintf("%s/%s-%s", charts[0].Repo.Name, charts[0].Name, cv.Version) - m.On("Upsert", bson.M{"file_id": chartFilesID, "repo.name": repo.Name, "repo.namespace": repo.Namespace}, models.ChartFiles{ - ID: chartFilesID, - Readme: testChartReadme, - Values: testChartValues, - Schema: testChartSchema, - Repo: charts[0].Repo, - Digest: cv.Digest, - }) - manager := getMockManager(&m) - fImporter := fileImporter{manager} + + fImporter := fileImporter{pgManager} + r := &models.RepoInternal{Name: repo.Name, Namespace: repo.Namespace, URL: repo.URL, AuthorizationHeader: "Bearer ThisSecretAccessTokenAuthenticatesTheClient"} - err := fImporter.fetchAndImportFiles(charts[0].Name, r, cv) + err := fImporter.fetchAndImportFiles(charts[0].Name, r, chartVersion) assert.NoErr(t, err) - m.AssertExpectations(t) }) t.Run("valid tarball", func(t *testing.T) { + pgManager, mock, cleanup := getMockManager(t) + defer cleanup() + + mock.ExpectQuery(`SELECT EXISTS*`). + WithArgs(chartFilesID, repo.Name, repo.Namespace, chartVersion.Digest). + WillReturnRows(sqlmock.NewRows([]string{"info"})) + mock.ExpectQuery("INSERT INTO files *"). + WithArgs(chartID, repo.Name, repo.Namespace, chartFilesID, chartFiles). + WillReturnRows(sqlmock.NewRows([]string{"ID"}).AddRow("3")) + netClient = &goodTarballClient{c: charts[0]} - m := mock.Mock{} - m.On("One", mock.Anything).Return(errors.New("return an error when checking if files already exists to force fetching")) - chartFilesID := fmt.Sprintf("%s/%s-%s", charts[0].Repo.Name, charts[0].Name, cv.Version) - m.On("Upsert", bson.M{"file_id": chartFilesID, "repo.name": repo.Name, "repo.namespace": repo.Namespace}, models.ChartFiles{ - ID: chartFilesID, - Readme: testChartReadme, - Values: testChartValues, - Schema: testChartSchema, - Repo: charts[0].Repo, - Digest: cv.Digest, - }) - manager := getMockManager(&m) - fImporter := fileImporter{manager} - err := fImporter.fetchAndImportFiles(charts[0].Name, repo, cv) + fImporter := fileImporter{pgManager} + + err := fImporter.fetchAndImportFiles(charts[0].Name, repo, chartVersion) assert.NoErr(t, err) - m.AssertExpectations(t) }) t.Run("file exists", func(t *testing.T) { - m := mock.Mock{} - // don't return an error when checking if files already exists - m.On("One", mock.Anything).Return(nil) - manager := getMockManager(&m) - fImporter := fileImporter{manager} - err := fImporter.fetchAndImportFiles(charts[0].Name, repo, cv) + pgManager, mock, cleanup := getMockManager(t) + defer cleanup() + + mock.ExpectQuery(`SELECT EXISTS*`). + WithArgs(chartFilesID, repo.Name, repo.Namespace, chartVersion.Digest). + WillReturnRows(sqlmock.NewRows([]string{"info"}).AddRow(`true`)) + + fImporter := fileImporter{pgManager} + err := fImporter.fetchAndImportFiles(charts[0].Name, repo, chartVersion) assert.NoErr(t, err) - m.AssertNotCalled(t, "UpsertId", mock.Anything, mock.Anything) }) } diff --git a/cmd/assetsvc/handler.go b/cmd/assetsvc/handler.go index 2bb82ffa1e0..c3378ef7f33 100644 --- a/cmd/assetsvc/handler.go +++ b/cmd/assetsvc/handler.go @@ -182,7 +182,7 @@ func getChartIcon(w http.ResponseWriter, req *http.Request, params Params) { return } - if chart.RawIcon == nil { + if len(chart.RawIcon) == 0 { http.NotFound(w, req) return } diff --git a/cmd/assetsvc/handler_test.go b/cmd/assetsvc/handler_test.go index c6bf1422e5d..458a2c1a572 100644 --- a/cmd/assetsvc/handler_test.go +++ b/cmd/assetsvc/handler_test.go @@ -18,6 +18,7 @@ package main import ( "bytes" + "database/sql/driver" "encoding/json" "errors" "image/color" @@ -26,13 +27,11 @@ import ( "strings" "testing" + "github.com/DATA-DOG/go-sqlmock" "github.com/disintegration/imaging" - "github.com/kubeapps/common/datastore" - "github.com/kubeapps/common/datastore/mockstore" "github.com/kubeapps/kubeapps/pkg/chart/models" "github.com/kubeapps/kubeapps/pkg/dbutils" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/mock" ) type bodyAPIListResponse struct { @@ -48,11 +47,12 @@ var chartsList []*models.Chart var cc count const ( - testChartReadme = "# Quickstart\n\n```bash\nhelm install my-repo/my-chart\n```" - testChartValues = "image:\n registry: docker.io\n repository: my-repo/my-chart\n tag: 0.1.0" - testChartSchema = `{"properties": {"type": "object"}}` - namespace = "kubeapps-namespace" - testRepoName = "my-repo" + testChartReadme = "# Quickstart\n\n```bash\nhelm install my-repo/my-chart\n```" + testChartValues = "image:\n registry: docker.io\n repository: my-repo/my-chart\n tag: 0.1.0" + testChartSchema = `{"properties": {"type": "object"}}` + namespace = "namespace" + kubeappsNamespace = "kubeapps-namespace" + testRepoName = "my-repo" ) var testRepo *models.Repo = &models.Repo{Name: testRepoName, Namespace: namespace} @@ -64,11 +64,18 @@ func iconBytes() []byte { return b.Bytes() } -func getMockManager(m *mock.Mock) *mongodbAssetManager { - dbSession := mockstore.NewMockSession(m) - man := dbutils.NewMongoDBManager(datastore.Config{}, "kubeapps") - man.DBSession = dbSession - return &mongodbAssetManager{man} +func setMockManager(t *testing.T) (sqlmock.Sqlmock, func()) { + db, mock, err := sqlmock.New() + if err != nil { + t.Fatalf("%+v", err) + } + + // TODO(absoludity): Let's not use globals for storing state like this. + origManager := manager + + manager = &postgresAssetManager{&dbutils.PostgresAssetManager{DB: db, KubeappsNamespace: kubeappsNamespace}} + + return mock, func() { db.Close(); manager = origManager } } func Test_chartAttributes(t *testing.T) { @@ -255,45 +262,46 @@ func Test_newChartVersionListResponse(t *testing.T) { func Test_listCharts(t *testing.T) { tests := []struct { name string - query string charts []*models.Chart meta meta }{ - {"no charts", "", []*models.Chart{}, meta{1}}, - {"one chart", "", []*models.Chart{ + {"no charts", []*models.Chart{}, meta{1}}, + {"one chart", []*models.Chart{ {Repo: testRepo, ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.0.1", Digest: "123"}}}, }, meta{1}}, - {"two charts", "", []*models.Chart{ + {"two charts", []*models.Chart{ {Repo: testRepo, ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.0.1", Digest: "123"}}}, {Repo: testRepo, ID: "stable/dokuwiki", ChartVersions: []models.ChartVersion{{Version: "1.2.3", Digest: "1234"}, {Version: "1.2.2", Digest: "12345"}}}, }, meta{1}}, - // Pagination tests - {"four charts with pagination", "?size=2", []*models.Chart{ + {"four charts", []*models.Chart{ {Repo: testRepo, ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.0.1", Digest: "123"}}}, {Repo: testRepo, ID: "stable/dokuwiki", ChartVersions: []models.ChartVersion{{Version: "1.2.3", Digest: "1234"}}}, {Repo: testRepo, ID: "stable/drupal", ChartVersions: []models.ChartVersion{{Version: "1.2.3", Digest: "12345"}}}, {Repo: testRepo, ID: "stable/wordpress", ChartVersions: []models.ChartVersion{{Version: "1.2.3", Digest: "123456"}}}, - }, meta{2}}, + }, meta{1}}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - var m mock.Mock - manager = getMockManager(&m) - m.On("All", &chartsList).Run(func(args mock.Arguments) { - *args.Get(0).(*[]*models.Chart) = tt.charts - }) - if tt.query != "" { - m.On("One", &cc).Run(func(args mock.Arguments) { - *args.Get(0).(*count) = count{len(tt.charts)} - }) + mock, cleanup := setMockManager(t) + defer cleanup() + + rows := sqlmock.NewRows([]string{"info"}) + for _, chart := range tt.charts { + chartJSON, err := json.Marshal(chart) + if err != nil { + t.Fatalf("%+v", err) + } + rows.AddRow(string(chartJSON)) } + mock.ExpectQuery("SELECT info FROM"). + WithArgs(namespace, kubeappsNamespace). + WillReturnRows(rows) w := httptest.NewRecorder() - req := httptest.NewRequest("GET", "/charts"+tt.query, nil) + req := httptest.NewRequest("GET", "/charts", nil) listCharts(w, req, Params{"namespace": namespace}) - m.AssertExpectations(t) assert.Equal(t, http.StatusOK, w.Code) var b bodyAPIListResponse @@ -335,32 +343,39 @@ func Test_listRepoCharts(t *testing.T) { {Repo: testRepo, ID: "stable/dokuwiki", ChartVersions: []models.ChartVersion{{Version: "1.2.3", Digest: "1234"}}}, {Repo: testRepo, ID: "stable/drupal", ChartVersions: []models.ChartVersion{{Version: "1.2.3", Digest: "12345"}}}, {Repo: testRepo, ID: "stable/wordpress", ChartVersions: []models.ChartVersion{{Version: "1.2.3", Digest: "123456"}}}, - }, meta{2}}, + }, meta{1}}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - var m mock.Mock - manager = getMockManager(&m) - - m.On("All", &chartsList).Run(func(args mock.Arguments) { - *args.Get(0).(*[]*models.Chart) = tt.charts - }) - if tt.query != "" { - m.On("One", &cc).Run(func(args mock.Arguments) { - *args.Get(0).(*count) = count{len(tt.charts)} - }) + mock, cleanup := setMockManager(t) + defer cleanup() + + rows := sqlmock.NewRows([]string{"info"}) + for _, chart := range tt.charts { + chartJSON, err := json.Marshal(chart) + if err != nil { + t.Fatalf("%+v", err) + } + rows.AddRow(string(chartJSON)) + } + expectedParams := []driver.Value{namespace, kubeappsNamespace} + if tt.repo != "" { + expectedParams = append(expectedParams, tt.repo) } + mock.ExpectQuery("SELECT info FROM charts"). + WithArgs(expectedParams...). + WillReturnRows(rows) w := httptest.NewRecorder() req := httptest.NewRequest("GET", "/charts/"+tt.repo+tt.query, nil) params := Params{ - "repo": "my-repo", + "repo": "my-repo", + "namespace": namespace, } listCharts(w, req, params) - m.AssertExpectations(t) assert.Equal(t, http.StatusOK, w.Code) var b bodyAPIListResponse @@ -406,15 +421,19 @@ func Test_getChart(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - var m mock.Mock - manager = getMockManager(&m) + mock, cleanup := setMockManager(t) + defer cleanup() + mockQuery := mock.ExpectQuery("SELECT info FROM charts"). + WithArgs(namespace, tt.chart.ID) if tt.err != nil { - m.On("One", mock.Anything).Return(tt.err) + mockQuery.WillReturnError(tt.err) } else { - m.On("One", &models.Chart{}).Return(nil).Run(func(args mock.Arguments) { - *args.Get(0).(*models.Chart) = tt.chart - }) + chartJSON, err := json.Marshal(tt.chart) + if err != nil { + t.Fatalf("%+v", err) + } + mockQuery.WillReturnRows(sqlmock.NewRows([]string{"info"}).AddRow(chartJSON)) } w := httptest.NewRecorder() @@ -428,7 +447,6 @@ func Test_getChart(t *testing.T) { getChart(w, req, params) - m.AssertExpectations(t) assert.Equal(t, tt.wantCode, w.Code) if tt.wantCode == http.StatusOK { var b bodyAPIResponse @@ -471,28 +489,33 @@ func Test_listChartVersions(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - var m mock.Mock - manager = getMockManager(&m) + mock, cleanup := setMockManager(t) + defer cleanup() + + mockQuery := mock.ExpectQuery("SELECT info FROM charts"). + WithArgs(namespace, tt.chart.ID) if tt.err != nil { - m.On("One", mock.Anything).Return(tt.err) + mockQuery.WillReturnError(tt.err) } else { - m.On("One", &models.Chart{}).Return(nil).Run(func(args mock.Arguments) { - *args.Get(0).(*models.Chart) = tt.chart - }) + chartJSON, err := json.Marshal(tt.chart) + if err != nil { + t.Fatalf("%+v", err) + } + mockQuery.WillReturnRows(sqlmock.NewRows([]string{"info"}).AddRow(chartJSON)) } w := httptest.NewRecorder() req := httptest.NewRequest("GET", "/charts/"+tt.chart.ID+"/versions", nil) parts := strings.Split(tt.chart.ID, "/") params := Params{ + "namespace": namespace, "repo": parts[0], "chartName": parts[1], } listChartVersions(w, req, params) - m.AssertExpectations(t) assert.Equal(t, tt.wantCode, w.Code) if tt.wantCode == http.StatusOK { var b bodyAPIListResponse @@ -537,21 +560,27 @@ func Test_getChartVersion(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - var m mock.Mock - manager = getMockManager(&m) + mock, cleanup := setMockManager(t) + defer cleanup() + + mockQuery := mock.ExpectQuery("SELECT info FROM charts"). + WithArgs(namespace, tt.chart.ID) if tt.err != nil { - m.On("One", mock.Anything).Return(tt.err) + mockQuery.WillReturnError(tt.err) } else { - m.On("One", &models.Chart{}).Return(nil).Run(func(args mock.Arguments) { - *args.Get(0).(*models.Chart) = tt.chart - }) + chartJSON, err := json.Marshal(tt.chart) + if err != nil { + t.Fatalf("%+v", err) + } + mockQuery.WillReturnRows(sqlmock.NewRows([]string{"info"}).AddRow(chartJSON)) } w := httptest.NewRecorder() req := httptest.NewRequest("GET", "/charts/"+tt.chart.ID+"/versions/"+tt.chart.ChartVersions[0].Version, nil) parts := strings.Split(tt.chart.ID, "/") params := Params{ + "namespace": namespace, "repo": parts[0], "chartName": parts[1], "version": tt.chart.ChartVersions[0].Version, @@ -559,7 +588,6 @@ func Test_getChartVersion(t *testing.T) { getChartVersion(w, req, params) - m.AssertExpectations(t) assert.Equal(t, tt.wantCode, w.Code) if tt.wantCode == http.StatusOK { var b bodyAPIResponse @@ -607,15 +635,20 @@ func Test_getChartIcon(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - var m mock.Mock - manager = getMockManager(&m) + mock, cleanup := setMockManager(t) + defer cleanup() + + mockQuery := mock.ExpectQuery("SELECT info FROM charts"). + WithArgs(namespace, tt.chart.ID) if tt.err != nil { - m.On("One", mock.Anything).Return(tt.err) + mockQuery.WillReturnError(tt.err) } else { - m.On("One", &models.Chart{}).Return(nil).Run(func(args mock.Arguments) { - *args.Get(0).(*models.Chart) = tt.chart - }) + chartJSON, err := json.Marshal(tt.chart) + if err != nil { + t.Fatalf("%+v", err) + } + mockQuery.WillReturnRows(sqlmock.NewRows([]string{"info"}).AddRow(chartJSON)) } w := httptest.NewRecorder() @@ -624,11 +657,11 @@ func Test_getChartIcon(t *testing.T) { params := Params{ "repo": parts[0], "chartName": parts[1], + "namespace": namespace, } getChartIcon(w, req, params) - m.AssertExpectations(t) assert.Equal(t, tt.wantCode, w.Code, "http status code should match") if tt.wantCode == http.StatusOK { assert.Equal(t, w.Body.Bytes(), tt.chart.RawIcon, "raw icon data should match") @@ -671,15 +704,20 @@ func Test_getChartVersionReadme(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - var m mock.Mock - manager = getMockManager(&m) + mock, cleanup := setMockManager(t) + defer cleanup() + + mockQuery := mock.ExpectQuery("SELECT info FROM files"). + WithArgs(namespace, tt.files.ID+"-0.1.0") if tt.err != nil { - m.On("One", mock.Anything).Return(tt.err) + mockQuery.WillReturnError(tt.err) } else { - m.On("One", &models.ChartFiles{}).Return(nil).Run(func(args mock.Arguments) { - *args.Get(0).(*models.ChartFiles) = tt.files - }) + filesJSON, err := json.Marshal(tt.files) + if err != nil { + t.Fatalf("%+v", err) + } + mockQuery.WillReturnRows(sqlmock.NewRows([]string{"info"}).AddRow(filesJSON)) } w := httptest.NewRecorder() @@ -689,11 +727,11 @@ func Test_getChartVersionReadme(t *testing.T) { "repo": parts[0], "chartName": parts[1], "version": "0.1.0", + "namespace": namespace, } getChartVersionReadme(w, req, params) - m.AssertExpectations(t) assert.Equal(t, tt.wantCode, w.Code, "http status code should match") if tt.wantCode == http.StatusOK { assert.Equal(t, string(w.Body.Bytes()), tt.files.Readme, "content of the readme should match") @@ -735,15 +773,20 @@ func Test_getChartVersionValues(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - var m mock.Mock - manager = getMockManager(&m) + mock, cleanup := setMockManager(t) + defer cleanup() + + mockQuery := mock.ExpectQuery("SELECT info FROM files"). + WithArgs(namespace, tt.files.ID+"-"+tt.version) if tt.err != nil { - m.On("One", mock.Anything).Return(tt.err) + mockQuery.WillReturnError(tt.err) } else { - m.On("One", &models.ChartFiles{}).Return(nil).Run(func(args mock.Arguments) { - *args.Get(0).(*models.ChartFiles) = tt.files - }) + filesJSON, err := json.Marshal(tt.files) + if err != nil { + t.Fatalf("%+v", err) + } + mockQuery.WillReturnRows(sqlmock.NewRows([]string{"info"}).AddRow(filesJSON)) } w := httptest.NewRecorder() @@ -752,12 +795,12 @@ func Test_getChartVersionValues(t *testing.T) { params := Params{ "repo": parts[0], "chartName": parts[1], - "version": "0.1.0", + "version": tt.version, + "namespace": namespace, } getChartVersionValues(w, req, params) - m.AssertExpectations(t) assert.Equal(t, tt.wantCode, w.Code, "http status code should match") if tt.wantCode == http.StatusOK { assert.Equal(t, string(w.Body.Bytes()), tt.files.Values, "content of values.yaml should match") @@ -799,15 +842,20 @@ func Test_getChartVersionSchema(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - var m mock.Mock - manager = getMockManager(&m) + mock, cleanup := setMockManager(t) + defer cleanup() + + mockQuery := mock.ExpectQuery("SELECT info FROM files"). + WithArgs(namespace, tt.files.ID+"-"+tt.version) if tt.err != nil { - m.On("One", mock.Anything).Return(tt.err) + mockQuery.WillReturnError(tt.err) } else { - m.On("One", &models.ChartFiles{}).Return(nil).Run(func(args mock.Arguments) { - *args.Get(0).(*models.ChartFiles) = tt.files - }) + filesJSON, err := json.Marshal(tt.files) + if err != nil { + t.Fatalf("%+v", err) + } + mockQuery.WillReturnRows(sqlmock.NewRows([]string{"info"}).AddRow(filesJSON)) } w := httptest.NewRecorder() @@ -816,12 +864,12 @@ func Test_getChartVersionSchema(t *testing.T) { params := Params{ "repo": parts[0], "chartName": parts[1], - "version": "0.1.0", + "version": tt.version, + "namespace": namespace, } getChartVersionSchema(w, req, params) - m.AssertExpectations(t) assert.Equal(t, tt.wantCode, w.Code, "http status code should match") if tt.wantCode == http.StatusOK { assert.Equal(t, string(w.Body.Bytes()), tt.files.Schema, "content of values.schema.json should match") @@ -841,22 +889,27 @@ func Test_findLatestChart(t *testing.T) { models.ChartVersion{Version: "0.0.1", AppVersion: "0.1.0"}, }, } - charts := []*models.Chart{chart} reqVersion := "1.0.0" reqAppVersion := "0.1.0" - var m mock.Mock - manager = getMockManager(&m) - m.On("All", &chartsList).Run(func(args mock.Arguments) { - *args.Get(0).(*[]*models.Chart) = charts - }) + mock, cleanup := setMockManager(t) + defer cleanup() + + chartJSON, err := json.Marshal(chart) + if err != nil { + t.Fatalf("%+v", err) + } + mock.ExpectQuery("SELECT info FROM charts WHERE info*"). + WithArgs("foo", "namespace", kubeappsNamespace). + WillReturnRows(sqlmock.NewRows([]string{"info"}).AddRow(chartJSON)) w := httptest.NewRecorder() req := httptest.NewRequest("GET", "/charts?name="+chart.Name+"&version="+reqVersion+"&appversion="+reqAppVersion, nil) params := Params{ - "name": chart.Name, + "chartName": chart.Name, "version": reqVersion, "appversion": reqAppVersion, + "namespace": namespace, } listChartsWithFilters(w, req, params) @@ -880,18 +933,28 @@ func Test_findLatestChart(t *testing.T) { reqVersion := "1.0.0" reqAppVersion := "0.1.0" - var m mock.Mock - manager = getMockManager(&m) - m.On("All", &chartsList).Run(func(args mock.Arguments) { - *args.Get(0).(*[]*models.Chart) = charts - }) + mock, cleanup := setMockManager(t) + defer cleanup() + + rows := sqlmock.NewRows([]string{"info"}) + for _, chart := range charts { + chartJSON, err := json.Marshal(chart) + if err != nil { + t.Fatalf("%+v", err) + } + rows.AddRow(string(chartJSON)) + } + mock.ExpectQuery("SELECT info FROM charts WHERE info*"). + WithArgs("foo", "namespace", kubeappsNamespace). + WillReturnRows(rows) w := httptest.NewRecorder() req := httptest.NewRequest("GET", "/charts?name="+charts[0].Name+"&version="+reqVersion+"&appversion="+reqAppVersion, nil) params := Params{ - "name": charts[0].Name, + "chartName": charts[0].Name, "version": reqVersion, "appversion": reqAppVersion, + "namespace": namespace, } listChartsWithFilters(w, req, params) @@ -916,18 +979,28 @@ func Test_findLatestChart(t *testing.T) { reqVersion := "1.0.0" reqAppVersion := "0.1.0" - var m mock.Mock - manager = getMockManager(&m) - m.On("All", &chartsList).Run(func(args mock.Arguments) { - *args.Get(0).(*[]*models.Chart) = charts - }) + mock, cleanup := setMockManager(t) + defer cleanup() + + rows := sqlmock.NewRows([]string{"info"}) + for _, chart := range charts { + chartJSON, err := json.Marshal(chart) + if err != nil { + t.Fatalf("%+v", err) + } + rows.AddRow(string(chartJSON)) + } + mock.ExpectQuery("SELECT info FROM charts WHERE info*"). + WithArgs("foo", "namespace", kubeappsNamespace). + WillReturnRows(rows) w := httptest.NewRecorder() req := httptest.NewRequest("GET", "/charts?showDuplicates=true&name="+charts[0].Name+"&version="+reqVersion+"&appversion="+reqAppVersion, nil) params := Params{ - "name": charts[0].Name, + "chartName": charts[0].Name, "version": reqVersion, "appversion": reqAppVersion, + "namespace": namespace, } listChartsWithFilters(w, req, params) diff --git a/cmd/assetsvc/main.go b/cmd/assetsvc/main.go index 74249928a05..7cfb3f28371 100644 --- a/cmd/assetsvc/main.go +++ b/cmd/assetsvc/main.go @@ -30,6 +30,7 @@ import ( const pathPrefix = "/v1" +// TODO(absoludity): Let's not use globals for storing state like this. var manager assetManager func setupRoutes() http.Handler { @@ -68,7 +69,6 @@ func main() { dbURL := flag.String("database-url", "localhost", "Database URL") dbName := flag.String("database-name", "charts", "Database database") dbUsername := flag.String("database-user", "", "Database user") - dbType := flag.String("database-type", "mongodb", "Database type") dbPassword := os.Getenv("DB_PASSWORD") flag.Parse() @@ -77,7 +77,7 @@ func main() { kubeappsNamespace := os.Getenv("POD_NAMESPACE") var err error - manager, err = newManager(*dbType, dbConfig, kubeappsNamespace) + manager, err = newManager("postgresql", dbConfig, kubeappsNamespace) if err != nil { log.Fatal(err) } diff --git a/cmd/assetsvc/main_test.go b/cmd/assetsvc/main_test.go index c9bf5f9227e..0d3a46e020e 100644 --- a/cmd/assetsvc/main_test.go +++ b/cmd/assetsvc/main_test.go @@ -23,15 +23,15 @@ import ( "net/http/httptest" "testing" + "github.com/DATA-DOG/go-sqlmock" "github.com/kubeapps/kubeapps/pkg/chart/models" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/mock" ) // tests the GET /live endpoint func Test_GetLive(t *testing.T) { - var m mock.Mock - manager = getMockManager(&m) + _, cleanup := setMockManager(t) + defer cleanup() ts := httptest.NewServer(setupRoutes()) defer ts.Close() @@ -44,8 +44,8 @@ func Test_GetLive(t *testing.T) { // tests the GET /ready endpoint func Test_GetReady(t *testing.T) { - var m mock.Mock - manager = getMockManager(&m) + _, cleanup := setMockManager(t) + defer cleanup() ts := httptest.NewServer(setupRoutes()) defer ts.Close() @@ -77,17 +77,25 @@ func Test_GetCharts(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - var m mock.Mock - manager = getMockManager(&m) - m.On("All", &chartsList).Run(func(args mock.Arguments) { - *args.Get(0).(*[]*models.Chart) = tt.charts - }) + mock, cleanup := setMockManager(t) + defer cleanup() + + rows := sqlmock.NewRows([]string{"info"}) + for _, chart := range tt.charts { + chartJSON, err := json.Marshal(chart) + if err != nil { + t.Fatalf("%+v", err) + } + rows.AddRow(string(chartJSON)) + } + mock.ExpectQuery("SELECT info FROM charts WHERE *"). + WithArgs("my-namespace", kubeappsNamespace). + WillReturnRows(rows) - res, err := http.Get(ts.URL + pathPrefix + "/clusters/default/namespaces/kubeapps/charts") + res, err := http.Get(ts.URL + pathPrefix + "/clusters/default/namespaces/my-namespace/charts") assert.NoError(t, err) defer res.Body.Close() - m.AssertExpectations(t) assert.Equal(t, res.StatusCode, http.StatusOK, "http status code should match") var b bodyAPIListResponse @@ -119,17 +127,25 @@ func Test_GetChartsInRepo(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - var m mock.Mock - manager = getMockManager(&m) - m.On("All", &chartsList).Run(func(args mock.Arguments) { - *args.Get(0).(*[]*models.Chart) = tt.charts - }) + mock, cleanup := setMockManager(t) + defer cleanup() + + rows := sqlmock.NewRows([]string{"info"}) + for _, chart := range tt.charts { + chartJSON, err := json.Marshal(chart) + if err != nil { + t.Fatalf("%+v", err) + } + rows.AddRow(string(chartJSON)) + } + mock.ExpectQuery("SELECT info FROM charts WHERE *"). + WithArgs("my-namespace", kubeappsNamespace, tt.repo). + WillReturnRows(rows) - res, err := http.Get(ts.URL + pathPrefix + "/clusters/default/namespaces/kubeapps/charts/" + tt.repo) + res, err := http.Get(ts.URL + pathPrefix + "/clusters/default/namespaces/my-namespace/charts/" + tt.repo) assert.NoError(t, err) defer res.Body.Close() - m.AssertExpectations(t) assert.Equal(t, res.StatusCode, http.StatusOK, "http status code should match") var b bodyAPIListResponse @@ -172,21 +188,26 @@ func Test_GetChartInRepo(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - var m mock.Mock - manager = getMockManager(&m) + mock, cleanup := setMockManager(t) + defer cleanup() + + mockQuery := mock.ExpectQuery("SELECT info FROM charts WHERE *"). + WithArgs("my-namespace", tt.chart.ID) + if tt.err != nil { - m.On("One", mock.Anything).Return(tt.err) + mockQuery.WillReturnError(tt.err) } else { - m.On("One", &models.Chart{}).Return(nil).Run(func(args mock.Arguments) { - *args.Get(0).(*models.Chart) = tt.chart - }) + chartJSON, err := json.Marshal(tt.chart) + if err != nil { + t.Fatalf("%+v", err) + } + mockQuery.WillReturnRows(sqlmock.NewRows([]string{"info"}).AddRow(chartJSON)) } - res, err := http.Get(ts.URL + pathPrefix + "/clusters/default/namespaces/kubeapps/charts/" + tt.chart.ID) + res, err := http.Get(ts.URL + pathPrefix + "/clusters/default/namespaces/my-namespace/charts/" + tt.chart.ID) assert.NoError(t, err) defer res.Body.Close() - m.AssertExpectations(t) assert.Equal(t, res.StatusCode, tt.wantCode, "http status code should match") }) } @@ -225,21 +246,26 @@ func Test_ListChartVersions(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - var m mock.Mock - manager = getMockManager(&m) + mock, cleanup := setMockManager(t) + defer cleanup() + + mockQuery := mock.ExpectQuery("SELECT info FROM charts WHERE *"). + WithArgs("my-namespace", tt.chart.ID) + if tt.err != nil { - m.On("One", mock.Anything).Return(tt.err) + mockQuery.WillReturnError(tt.err) } else { - m.On("One", &models.Chart{}).Return(nil).Run(func(args mock.Arguments) { - *args.Get(0).(*models.Chart) = tt.chart - }) + chartJSON, err := json.Marshal(tt.chart) + if err != nil { + t.Fatalf("%+v", err) + } + mockQuery.WillReturnRows(sqlmock.NewRows([]string{"info"}).AddRow(chartJSON)) } - res, err := http.Get(ts.URL + pathPrefix + "/clusters/default/namespaces/kubeapps/charts/" + tt.chart.ID + "/versions") + res, err := http.Get(ts.URL + pathPrefix + "/clusters/default/namespaces/my-namespace/charts/" + tt.chart.ID + "/versions") assert.NoError(t, err) defer res.Body.Close() - m.AssertExpectations(t) assert.Equal(t, res.StatusCode, tt.wantCode, "http status code should match") }) } @@ -278,21 +304,26 @@ func Test_GetChartVersion(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - var m mock.Mock - manager = getMockManager(&m) + mock, cleanup := setMockManager(t) + defer cleanup() + + mockQuery := mock.ExpectQuery("SELECT info FROM charts WHERE *"). + WithArgs("my-namespace", tt.chart.ID) + if tt.err != nil { - m.On("One", mock.Anything).Return(tt.err) + mockQuery.WillReturnError(tt.err) } else { - m.On("One", &models.Chart{}).Return(nil).Run(func(args mock.Arguments) { - *args.Get(0).(*models.Chart) = tt.chart - }) + chartJSON, err := json.Marshal(tt.chart) + if err != nil { + t.Fatalf("%+v", err) + } + mockQuery.WillReturnRows(sqlmock.NewRows([]string{"info"}).AddRow(chartJSON)) } - res, err := http.Get(ts.URL + pathPrefix + "/clusters/default/namespaces/kubeapps/charts/" + tt.chart.ID + "/versions/" + tt.chart.ChartVersions[0].Version) + res, err := http.Get(ts.URL + pathPrefix + "/clusters/default/namespaces/my-namespace/charts/" + tt.chart.ID + "/versions/" + tt.chart.ChartVersions[0].Version) assert.NoError(t, err) defer res.Body.Close() - m.AssertExpectations(t) assert.Equal(t, res.StatusCode, tt.wantCode, "http status code should match") }) } @@ -333,27 +364,28 @@ func Test_GetChartIcon(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - var m mock.Mock - manager = getMockManager(&m) + mock, cleanup := setMockManager(t) + defer cleanup() + + mockQuery := mock.ExpectQuery("SELECT info FROM charts WHERE *"). + WithArgs("my-namespace", tt.chart.ID) + if tt.err != nil { - m.On("One", mock.Anything).Return(tt.err) + mockQuery.WillReturnError(tt.err) } else { - m.On("One", &models.Chart{}).Return(nil).Run(func(args mock.Arguments) { - *args.Get(0).(*models.Chart) = tt.chart - }) + chartJSON, err := json.Marshal(tt.chart) + if err != nil { + t.Fatalf("%+v", err) + } + mockQuery.WillReturnRows(sqlmock.NewRows([]string{"info"}).AddRow(chartJSON)) } - for _, path := range []string{ - "/clusters/default/namespaces/kubeapps/assets/", - "/ns/kubeapps/assets/", - } { - res, err := http.Get(ts.URL + pathPrefix + path + tt.chart.ID + "/logo") - assert.NoError(t, err) - defer res.Body.Close() + path := "/clusters/default/namespaces/my-namespace/assets/" + res, err := http.Get(ts.URL + pathPrefix + path + tt.chart.ID + "/logo") + assert.NoError(t, err) + defer res.Body.Close() - m.AssertExpectations(t) - assert.Equal(t, res.StatusCode, tt.wantCode, "http status code should match") - } + assert.Equal(t, res.StatusCode, tt.wantCode, "http status code should match") }) } } @@ -395,21 +427,26 @@ func Test_GetChartReadme(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - var m mock.Mock - manager = getMockManager(&m) + mock, cleanup := setMockManager(t) + defer cleanup() + + mockQuery := mock.ExpectQuery("SELECT info FROM files"). + WithArgs("my-namespace", tt.files.ID+"-"+tt.version) + if tt.err != nil { - m.On("One", mock.Anything).Return(tt.err) + mockQuery.WillReturnError(tt.err) } else { - m.On("One", &models.ChartFiles{}).Return(nil).Run(func(args mock.Arguments) { - *args.Get(0).(*models.ChartFiles) = tt.files - }) + filesJSON, err := json.Marshal(tt.files) + if err != nil { + t.Fatalf("%+v", err) + } + mockQuery.WillReturnRows(sqlmock.NewRows([]string{"info"}).AddRow(filesJSON)) } - res, err := http.Get(ts.URL + pathPrefix + "/clusters/default/namespaces/kubeapps/assets/" + tt.files.ID + "/versions/" + tt.version + "/README.md") + res, err := http.Get(ts.URL + pathPrefix + "/clusters/default/namespaces/my-namespace/assets/" + tt.files.ID + "/versions/" + tt.version + "/README.md") assert.NoError(t, err) defer res.Body.Close() - m.AssertExpectations(t) assert.Equal(t, tt.wantCode, res.StatusCode, "http status code should match") }) } @@ -452,21 +489,26 @@ func Test_GetChartValues(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - var m mock.Mock - manager = getMockManager(&m) + mock, cleanup := setMockManager(t) + defer cleanup() + + mockQuery := mock.ExpectQuery("SELECT info FROM files"). + WithArgs("my-namespace", tt.files.ID+"-"+tt.version) + if tt.err != nil { - m.On("One", mock.Anything).Return(tt.err) + mockQuery.WillReturnError(tt.err) } else { - m.On("One", &models.ChartFiles{}).Return(nil).Run(func(args mock.Arguments) { - *args.Get(0).(*models.ChartFiles) = tt.files - }) + filesJSON, err := json.Marshal(tt.files) + if err != nil { + t.Fatalf("%+v", err) + } + mockQuery.WillReturnRows(sqlmock.NewRows([]string{"info"}).AddRow(filesJSON)) } - res, err := http.Get(ts.URL + pathPrefix + "/clusters/default/namespaces/kubeapps/assets/" + tt.files.ID + "/versions/" + tt.version + "/values.yaml") + res, err := http.Get(ts.URL + pathPrefix + "/clusters/default/namespaces/my-namespace/assets/" + tt.files.ID + "/versions/" + tt.version + "/values.yaml") assert.NoError(t, err) defer res.Body.Close() - m.AssertExpectations(t) assert.Equal(t, res.StatusCode, tt.wantCode, "http status code should match") }) } @@ -509,21 +551,26 @@ func Test_GetChartSchema(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - var m mock.Mock - manager = getMockManager(&m) + mock, cleanup := setMockManager(t) + defer cleanup() + + mockQuery := mock.ExpectQuery("SELECT info FROM files"). + WithArgs("my-namespace", tt.files.ID+"-"+tt.version) + if tt.err != nil { - m.On("One", mock.Anything).Return(tt.err) + mockQuery.WillReturnError(tt.err) } else { - m.On("One", &models.ChartFiles{}).Return(nil).Run(func(args mock.Arguments) { - *args.Get(0).(*models.ChartFiles) = tt.files - }) + filesJSON, err := json.Marshal(tt.files) + if err != nil { + t.Fatalf("%+v", err) + } + mockQuery.WillReturnRows(sqlmock.NewRows([]string{"info"}).AddRow(filesJSON)) } - res, err := http.Get(ts.URL + pathPrefix + "/clusters/default/namespaces/kubeapps/assets/" + tt.files.ID + "/versions/" + tt.version + "/values.schema.json") + res, err := http.Get(ts.URL + pathPrefix + "/clusters/default/namespaces/my-namespace/assets/" + tt.files.ID + "/versions/" + tt.version + "/values.schema.json") assert.NoError(t, err) defer res.Body.Close() - m.AssertExpectations(t) assert.Equal(t, res.StatusCode, tt.wantCode, "http status code should match") }) } diff --git a/cmd/assetsvc/mongodb_utils.go b/cmd/assetsvc/mongodb_utils.go deleted file mode 100644 index a4087e56538..00000000000 --- a/cmd/assetsvc/mongodb_utils.go +++ /dev/null @@ -1,172 +0,0 @@ -/* -Copyright (c) 2019 Bitnami - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package main - -import ( - "math" - - "github.com/globalsign/mgo/bson" - "github.com/kubeapps/common/datastore" - "github.com/kubeapps/kubeapps/pkg/chart/models" - "github.com/kubeapps/kubeapps/pkg/dbutils" -) - -type mongodbAssetManager struct { - *dbutils.MongodbAssetManager -} - -func newMongoDBManager(config datastore.Config, kubeappsNamespace string) assetManager { - m := dbutils.NewMongoDBManager(config, kubeappsNamespace) - return &mongodbAssetManager{m} -} - -func (m *mongodbAssetManager) getPaginatedChartList(namespace, repo string, pageNumber, pageSize int, showDuplicates bool) ([]*models.Chart, int, error) { - db, closer := m.DBSession.DB() - defer closer() - var charts []*models.Chart - - c := db.C(chartCollection) - pipeline := []bson.M{} - matcher := bson.M{} - if namespace != dbutils.AllNamespaces { - matcher["repo.namespace"] = bson.M{"$in": []string{namespace, m.KubeappsNamespace}} - } - if repo != "" { - matcher["repo.name"] = repo - } - if len(matcher) > 0 { - pipeline = append(pipeline, bson.M{"$match": matcher}) - } - - if !showDuplicates { - // We should query unique charts - pipeline = append(pipeline, - // Add a new field to store the latest version - bson.M{"$addFields": bson.M{"firstChartVersion": bson.M{"$arrayElemAt": []interface{}{"$chartversions", 0}}}}, - // Group by unique digest for the latest version (remove duplicates) - bson.M{"$group": bson.M{"_id": "$firstChartVersion.digest", "chart": bson.M{"$first": "$$ROOT"}}}, - // Restore original object struct - bson.M{"$replaceRoot": bson.M{"newRoot": "$chart"}}, - ) - } - - // Order by name - pipeline = append(pipeline, bson.M{"$sort": bson.M{"name": 1}}) - - totalPages := 1 - if pageSize != 0 { - // If a pageSize is given, returns only the the specified number of charts and - // the number of pages - countPipeline := append(pipeline, bson.M{"$count": "count"}) - cc := count{} - err := c.Pipe(countPipeline).One(&cc) - if err != nil { - return charts, 0, err - } - totalPages = int(math.Ceil(float64(cc.Count) / float64(pageSize))) - - // If the page number is out of range, return the last one - if pageNumber > totalPages { - pageNumber = totalPages - } - - pipeline = append(pipeline, - bson.M{"$skip": pageSize * (pageNumber - 1)}, - bson.M{"$limit": pageSize}, - ) - } - err := c.Pipe(pipeline).All(&charts) - if err != nil { - return charts, 0, err - } - - return charts, totalPages, nil -} - -func (m *mongodbAssetManager) getChart(namespace, chartID string) (models.Chart, error) { - db, closer := m.DBSession.DB() - defer closer() - var chart models.Chart - err := db.C(chartCollection).Find(bson.M{"repo.namespace": namespace, "chart_id": chartID}).One(&chart) - return chart, err -} - -func (m *mongodbAssetManager) getChartVersion(namespace, chartID, version string) (models.Chart, error) { - db, closer := m.DBSession.DB() - defer closer() - var chart models.Chart - err := db.C(chartCollection).Find(bson.M{ - "repo.namespace": namespace, - "chart_id": chartID, - "chartversions": bson.M{"$elemMatch": bson.M{"version": version}}, - }).Select(bson.M{ - "name": 1, "repo": 1, "description": 1, "home": 1, "keywords": 1, "maintainers": 1, "sources": 1, - "chartversions.$": 1, - }).One(&chart) - return chart, err -} - -func (m *mongodbAssetManager) getChartFiles(namespace, filesID string) (models.ChartFiles, error) { - db, closer := m.DBSession.DB() - defer closer() - var files models.ChartFiles - err := db.C(filesCollection).Find(bson.M{"repo.namespace": namespace, "file_id": filesID}).One(&files) - return files, err -} - -func (m *mongodbAssetManager) getChartsWithFilters(namespace, name, version, appVersion string) ([]*models.Chart, error) { - db, closer := m.DBSession.DB() - defer closer() - var charts []*models.Chart - matcher := bson.M{ - "repo.namespace": namespace, - "name": name, - "chartversions": bson.M{ - "$elemMatch": bson.M{"version": version, "appversion": appVersion}, - }, - } - if namespace != dbutils.AllNamespaces { - matcher["repo.namespace"] = bson.M{"$in": []string{namespace, m.KubeappsNamespace}} - } - - err := db.C(chartCollection).Find(matcher).Select(bson.M{ - "name": 1, "repo": 1, - "chartversions": bson.M{"$slice": 1}, - }).All(&charts) - return charts, err -} - -func (m *mongodbAssetManager) searchCharts(query, repo string) ([]*models.Chart, error) { - db, closer := m.DBSession.DB() - defer closer() - var charts []*models.Chart - conditions := bson.M{ - "$or": []bson.M{ - {"name": bson.M{"$regex": query}}, - {"description": bson.M{"$regex": query}}, - {"repo.name": bson.M{"$regex": query}}, - {"keywords": bson.M{"$elemMatch": bson.M{"$regex": query}}}, - {"sources": bson.M{"$elemMatch": bson.M{"$regex": query}}}, - {"maintainers": bson.M{"$elemMatch": bson.M{"name": bson.M{"$regex": query}}}}, - }, - } - if repo != "" { - conditions["repo.name"] = repo - } - err := db.C(chartCollection).Find(conditions).All(&charts) - return charts, err -} diff --git a/cmd/assetsvc/postgresql_utils.go b/cmd/assetsvc/postgresql_utils.go index 8a44281c891..443932f4af8 100644 --- a/cmd/assetsvc/postgresql_utils.go +++ b/cmd/assetsvc/postgresql_utils.go @@ -71,7 +71,7 @@ func (m *postgresAssetManager) getPaginatedChartList(namespace, repo string, pag dbQuery := fmt.Sprintf("SELECT info FROM %s %s ORDER BY info ->> 'name' ASC", dbutils.ChartTable, repoQuery) charts, err := m.QueryAllCharts(dbQuery, queryParams...) if err != nil { - return nil, 0, nil + return nil, 0, err } if !showDuplicates { // Group by unique digest for the latest version (remove duplicates) diff --git a/cmd/assetsvc/postgresql_utils_test.go b/cmd/assetsvc/postgresql_utils_test.go index 1dd3e16da26..dfe80c19f4c 100644 --- a/cmd/assetsvc/postgresql_utils_test.go +++ b/cmd/assetsvc/postgresql_utils_test.go @@ -17,73 +17,31 @@ limitations under the License. package main import ( + "database/sql/driver" "encoding/base64" - "fmt" + "encoding/json" "testing" + "github.com/DATA-DOG/go-sqlmock" "github.com/google/go-cmp/cmp" - "github.com/kubeapps/common/datastore" "github.com/kubeapps/kubeapps/pkg/chart/models" "github.com/kubeapps/kubeapps/pkg/dbutils" - "github.com/stretchr/testify/mock" ) -type fakePGManager struct { - *mock.Mock -} - -func (f *fakePGManager) Init() error { - return nil -} - -func (f *fakePGManager) Close() error { - return nil -} - -func (f *fakePGManager) QueryOne(target interface{}, query string, args ...interface{}) error { - f.Called(target, query, args) - return nil -} - -var chartsResponse []*models.Chart - -func (f *fakePGManager) QueryAllCharts(query string, args ...interface{}) ([]*models.Chart, error) { - f.Called(query, args) - return chartsResponse, nil -} - -func (f *fakePGManager) InvalidateCache() error { - return nil -} - -func (f *fakePGManager) InitTables() error { - return nil -} - -func (f *fakePGManager) EnsureRepoExists(namespace, name string) (int, error) { - return 0, nil -} - -func (f *fakePGManager) GetDB() dbutils.PostgresDB { - return nil -} - -func (f *fakePGManager) GetKubeappsNamespace() string { - return "kubeapps" -} - -func Test_NewPGManager(t *testing.T) { - config := datastore.Config{URL: "10.11.12.13:5432"} - _, err := newPGManager(config, "kubeapps") +func getMockManager(t *testing.T) (*postgresAssetManager, sqlmock.Sqlmock, func()) { + db, mock, err := sqlmock.New() if err != nil { - t.Errorf("Found error %v", err) + t.Fatalf("%+v", err) } + + pgManager := &postgresAssetManager{&dbutils.PostgresAssetManager{DB: db, KubeappsNamespace: "kubeapps"}} + + return pgManager, mock, func() { db.Close() } } func Test_PGgetChart(t *testing.T) { - m := &mock.Mock{} - fpg := &fakePGManager{m} - pg := postgresAssetManager{fpg} + pgManager, mock, cleanup := getMockManager(t) + defer cleanup() icon := []byte("test") iconB64 := base64.StdEncoding.EncodeToString(icon) @@ -91,11 +49,15 @@ func Test_PGgetChart(t *testing.T) { Chart: models.Chart{ID: "foo"}, RawIcon: iconB64, } - m.On("QueryOne", &models.ChartIconString{}, "SELECT info FROM charts WHERE repo_namespace = $1 AND chart_id = $2", []interface{}{"namespace", "foo"}).Run(func(args mock.Arguments) { - *args.Get(0).(*models.ChartIconString) = dbChart - }) + dbChartJSON, err := json.Marshal(dbChart) + if err != nil { + t.Fatalf("%+v", err) + } + mock.ExpectQuery("SELECT info FROM charts*"). + WithArgs("namespace", "foo"). + WillReturnRows(sqlmock.NewRows([]string{"info"}).AddRow(string(dbChartJSON))) - chart, err := pg.getChart("namespace", "foo") + chart, err := pgManager.getChart("namespace", "foo") if err != nil { t.Errorf("Found error %v", err) } @@ -109,9 +71,8 @@ func Test_PGgetChart(t *testing.T) { } func Test_PGgetChartVersion(t *testing.T) { - m := &mock.Mock{} - fpg := &fakePGManager{m} - pg := postgresAssetManager{fpg} + pgManager, mock, cleanup := getMockManager(t) + defer cleanup() dbChart := models.Chart{ ID: "foo", @@ -120,11 +81,15 @@ func Test_PGgetChartVersion(t *testing.T) { {Version: "2.0.0"}, }, } - m.On("QueryOne", &models.Chart{}, "SELECT info FROM charts WHERE repo_namespace = $1 AND chart_id = $2", []interface{}{"namespace", "foo"}).Run(func(args mock.Arguments) { - *args.Get(0).(*models.Chart) = dbChart - }) + dbChartJSON, err := json.Marshal(dbChart) + if err != nil { + t.Fatalf("%+v", err) + } + mock.ExpectQuery("SELECT info FROM charts*"). + WithArgs("namespace", "foo"). + WillReturnRows(sqlmock.NewRows([]string{"info"}).AddRow(string(dbChartJSON))) - chart, err := pg.getChartVersion("namespace", "foo", "1.0.0") + chart, err := pgManager.getChartVersion("namespace", "foo", "1.0.0") if err != nil { t.Errorf("Found error %v", err) } @@ -140,16 +105,19 @@ func Test_PGgetChartVersion(t *testing.T) { } func Test_getChartFiles(t *testing.T) { - m := &mock.Mock{} - fpg := &fakePGManager{m} - pg := postgresAssetManager{fpg} + pgManager, mock, cleanup := getMockManager(t) + defer cleanup() expectedFiles := models.ChartFiles{ID: "foo"} - m.On("QueryOne", &models.ChartFiles{}, "SELECT info FROM files WHERE repo_namespace = $1 AND chart_files_id = $2", []interface{}{"namespace", "foo"}).Run(func(args mock.Arguments) { - *args.Get(0).(*models.ChartFiles) = expectedFiles - }) + filesJSON, err := json.Marshal(expectedFiles) + if err != nil { + t.Fatalf("%+v", err) + } + mock.ExpectQuery("SELECT info FROM files*"). + WithArgs("namespace", "foo"). + WillReturnRows(sqlmock.NewRows([]string{"info"}).AddRow(string(filesJSON))) - files, err := pg.getChartFiles("namespace", "foo") + files, err := pgManager.getChartFiles("namespace", "foo") if err != nil { t.Errorf("Found error %v", err) } @@ -158,10 +126,9 @@ func Test_getChartFiles(t *testing.T) { } } -func Test_getChartWithFilters(t *testing.T) { - m := &mock.Mock{} - fpg := &fakePGManager{m} - pg := postgresAssetManager{fpg} +func Test_getChartsWithFilters(t *testing.T) { + pgManager, mock, cleanup := getMockManager(t) + defer cleanup() dbChart := models.Chart{ Name: "foo", @@ -170,10 +137,16 @@ func Test_getChartWithFilters(t *testing.T) { {Version: "1.0.0", AppVersion: "1.0.1"}, }, } - chartsResponse = []*models.Chart{&dbChart} - m.On("QueryAllCharts", "SELECT info FROM charts WHERE info ->> 'name' = $1 AND (repo_namespace = $2 OR repo_namespace = $3) ORDER BY info ->> 'ID' ASC", []interface{}{"foo", "namespace", "kubeapps"}) + dbChartJSON, err := json.Marshal(dbChart) + if err != nil { + t.Fatalf("%+v", err) + } - charts, err := pg.getChartsWithFilters("namespace", "foo", "1.0.0", "1.0.1") + mock.ExpectQuery("SELECT info FROM charts WHERE info*"). + WithArgs("foo", "namespace", "kubeapps"). + WillReturnRows(sqlmock.NewRows([]string{"info"}).AddRow(dbChartJSON)) + + charts, err := pgManager.getChartsWithFilters("namespace", "foo", "1.0.0", "1.0.1") if err != nil { t.Errorf("Found error %v", err) } @@ -229,22 +202,28 @@ func Test_getPaginatedChartList(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - m := &mock.Mock{} - fpg := &fakePGManager{m} - pg := postgresAssetManager{fpg} - - chartsResponse = availableCharts - expectedQuery := "WHERE (repo_namespace = $1 OR repo_namespace = $2)" - expectedParams := []interface{}{"other-namespace", "kubeapps"} + pgManager, mock, cleanup := getMockManager(t) + defer cleanup() + + rows := sqlmock.NewRows([]string{"info"}) + for _, chart := range availableCharts { + chartJSON, err := json.Marshal(chart) + if err != nil { + t.Fatalf("%+v", err) + } + rows.AddRow(string(chartJSON)) + } + expectedParams := []driver.Value{"other-namespace", "kubeapps"} if tt.repo != "" { - expectedQuery = expectedQuery + " AND repo_name = $3" expectedParams = append(expectedParams, "bitnami") } - expectedQuery = fmt.Sprintf("SELECT info FROM %s %s ORDER BY info ->> 'name' ASC", dbutils.ChartTable, expectedQuery) - m.On("QueryAllCharts", expectedQuery, expectedParams) - charts, totalPages, err := pg.getPaginatedChartList(tt.namespace, tt.repo, tt.pageNumber, tt.pageSize, tt.showDuplicates) + mock.ExpectQuery("SELECT info FROM *"). + WithArgs(expectedParams...). + WillReturnRows(rows) + + charts, totalPages, err := pgManager.getPaginatedChartList(tt.namespace, tt.repo, tt.pageNumber, tt.pageSize, tt.showDuplicates) if err != nil { - t.Errorf("Found error %v", err) + t.Fatalf("Found error %v", err) } if totalPages != tt.expectedTotalPages { t.Errorf("Unexpected number of pages, got %d expecting %d", totalPages, tt.expectedTotalPages) diff --git a/cmd/assetsvc/utils.go b/cmd/assetsvc/utils.go index 301f0d8c9fb..b064147605a 100644 --- a/cmd/assetsvc/utils.go +++ b/cmd/assetsvc/utils.go @@ -17,8 +17,6 @@ limitations under the License. package main import ( - "fmt" - "github.com/kubeapps/common/datastore" "github.com/kubeapps/kubeapps/pkg/chart/models" ) @@ -34,11 +32,5 @@ type assetManager interface { } func newManager(databaseType string, config datastore.Config, kubeappsNamespace string) (assetManager, error) { - if databaseType == "mongodb" { - return newMongoDBManager(config, kubeappsNamespace), nil - } else if databaseType == "postgresql" { - return newPGManager(config, kubeappsNamespace) - } else { - return nil, fmt.Errorf("Unsupported database type %s", databaseType) - } + return newPGManager(config, kubeappsNamespace) } diff --git a/docs/developer/README.md b/docs/developer/README.md index 58fcac21b54..c28a9ca8611 100644 --- a/docs/developer/README.md +++ b/docs/developer/README.md @@ -8,7 +8,7 @@ Please refer to the [Kubeapps Dashboard Developer Guide](dashboard.md) for the d ### assetsvc -The `assetsvc` component is a micro-service that creates an API endpoint for accessing the metadata for charts in Helm chart repositories that's populated in a MongoDB server. +The `assetsvc` component is a micro-service that creates an API endpoint for accessing the metadata for charts in Helm chart repositories that's populated in a Postgresql server. Please refer to the [Kubeapps assetsvc Developer Guide](assetsvc.md) for the developer setup. diff --git a/docs/developer/asset-syncer.md b/docs/developer/asset-syncer.md index cfbc3a7734f..e55280b0eea 100644 --- a/docs/developer/asset-syncer.md +++ b/docs/developer/asset-syncer.md @@ -43,15 +43,6 @@ This builds the `asset-syncer` Docker image. ### Running in development -When using MongoDB: - -```bash -export DB_PASSWORD=$(kubectl get secret --namespace kubeapps kubeapps-mongodb -o go-template='{{index .data "mongodb-root-password" | base64decode}}') -telepresence --namespace kubeapps --docker-run -e DB_PASSWORD=$DB_PASSWORD --rm -ti kubeapps/asset-syncer /asset-syncer sync --database-user=root --database-url=kubeapps-mongodb --database-type=mongodb --database-name=charts stable https://kubernetes-charts.storage.googleapis.com -``` - -When using PostgreSQL: - ```bash export DB_PASSWORD=$(kubectl get secret --namespace kubeapps kubeapps-db -o go-template='{{index .data "postgresql-password" | base64decode}}') telepresence --namespace kubeapps --docker-run -e DB_PASSWORD=$DB_PASSWORD --rm -ti kubeapps/asset-syncer /asset-syncer sync --database-user=postgres --database-url=kubeapps-postgresql:5432 --database-type=postgresql --database-name=assets stable https://kubernetes-charts.storage.googleapis.com diff --git a/docs/developer/assetsvc.md b/docs/developer/assetsvc.md index d23f4a59a0e..0ec6bf7df54 100644 --- a/docs/developer/assetsvc.md +++ b/docs/developer/assetsvc.md @@ -1,6 +1,6 @@ # Kubeapps assetsvc Developer Guide -The `assetsvc` component is a micro-service that creates an API endpoint for accessing the metadata for charts in Helm chart repositories that's populated in a MongoDB server. +The `assetsvc` component is a micro-service that creates an API endpoint for accessing the metadata for charts in Helm chart repositories that's populated in a Postgresql server. ## Prerequisites @@ -45,14 +45,6 @@ This builds the `assetsvc` Docker image. #### Option 1: Using Telepresence (recommended) -When using MongoDB: - -```bash -telepresence --swap-deployment kubeapps-internal-assetsvc --namespace kubeapps --expose 8080:8080 --docker-run --rm -ti kubeapps/assetsvc /assetsvc --database-user=root --database-url=kubeapps-mongodb --database-type=mongodb --database-name=charts -``` - -When using PostgreSQL: - ```bash telepresence --swap-deployment kubeapps-internal-assetsvc --namespace kubeapps --expose 8080:8080 --docker-run --rm -ti kubeapps/assetsvc /assetsvc --database-user=postgres --database-url=kubeapps-postgresql:5432 --database-type=postgresql --database-name=assets ``` diff --git a/go.mod b/go.mod index 10924bd7a52..abcb8f9b49e 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,7 @@ go 1.13 replace github.com/docker/docker => github.com/docker/docker v0.0.0-20190731150326-928381b2215c require ( - github.com/DATA-DOG/go-sqlmock v1.4.1 + github.com/DATA-DOG/go-sqlmock v1.5.0 github.com/MakeNowJust/heredoc v0.0.0-20171113091838-e9091a26100e // indirect github.com/Masterminds/semver v1.5.0 // indirect github.com/Masterminds/sprig v2.22.0+incompatible // indirect diff --git a/go.sum b/go.sum index 44d11cc9d0c..27a573e2cb3 100644 --- a/go.sum +++ b/go.sum @@ -19,6 +19,8 @@ github.com/DATA-DOG/go-sqlmock v1.3.3 h1:CWUqKXe0s8A2z6qCgkP4Kru7wC11YoAnoupUKFD github.com/DATA-DOG/go-sqlmock v1.3.3/go.mod h1:f/Ixk793poVmq4qj/V1dPUg2JEAKC73Q5eFN3EC/SaM= github.com/DATA-DOG/go-sqlmock v1.4.1 h1:ThlnYciV1iM/V0OSF/dtkqWb6xo5qITT1TJBG1MRDJM= github.com/DATA-DOG/go-sqlmock v1.4.1/go.mod h1:f/Ixk793poVmq4qj/V1dPUg2JEAKC73Q5eFN3EC/SaM= +github.com/DATA-DOG/go-sqlmock v1.5.0 h1:Shsta01QNfFxHCfpW6YH2STWB0MudeXXEWMr20OEh60= +github.com/DATA-DOG/go-sqlmock v1.5.0/go.mod h1:f/Ixk793poVmq4qj/V1dPUg2JEAKC73Q5eFN3EC/SaM= github.com/MakeNowJust/heredoc v0.0.0-20170808103936-bb23615498cd/go.mod h1:64YHyfSL2R96J44Nlwm39UHepQbyR5q10x7iYa1ks2E= github.com/MakeNowJust/heredoc v0.0.0-20171113091838-e9091a26100e h1:eb0Pzkt15Bm7f2FFYv7sjY7NPFi3cPkS3tv1CcrFBWA= github.com/MakeNowJust/heredoc v0.0.0-20171113091838-e9091a26100e/go.mod h1:64YHyfSL2R96J44Nlwm39UHepQbyR5q10x7iYa1ks2E= diff --git a/pkg/dbutils/dbutilstest/mgtest/mgtest.go b/pkg/dbutils/dbutilstest/mgtest/mgtest.go deleted file mode 100644 index b990701e7b0..00000000000 --- a/pkg/dbutils/dbutilstest/mgtest/mgtest.go +++ /dev/null @@ -1,63 +0,0 @@ -/* -Copyright (c) 2020 Bitnami - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package mgtest - -import ( - "testing" - - "github.com/kubeapps/common/datastore" - "github.com/kubeapps/kubeapps/pkg/dbutils" - "github.com/kubeapps/kubeapps/pkg/dbutils/dbutilstest" -) - -const ( - // EnvvarMongoTests enables tests that run against a local mongo db - EnvvarMongoTests = "ENABLE_MONGO_INTEGRATION_TESTS" -) - -func SkipIfNoDB(t *testing.T) { - if !dbutilstest.IsEnvVarTrue(t, EnvvarMongoTests) { - t.Skipf("skipping mongodb tests as %q not set to be true", EnvvarMongoTests) - } -} - -func OpenTestManager(t *testing.T) *dbutils.MongodbAssetManager { - manager := dbutils.NewMongoDBManager(datastore.Config{ - URL: "localhost:27017", - Username: "root", - Password: "testpassword", - }, dbutilstest.KubeappsTestNamespace) - - err := manager.Init() - if err != nil { - t.Fatalf("%+v", err) - } - return manager -} - -// GetInitializedManager returns an initialized mongodb manager ready for testing. -func GetInitializedManager(t *testing.T) (*dbutils.MongodbAssetManager, func()) { - manager := OpenTestManager(t) - cleanup := func() { manager.Close() } - - err := manager.InvalidateCache() - if err != nil { - t.Fatalf("%+v", err) - } - - return manager, cleanup -} diff --git a/pkg/dbutils/mongodb_utils.go b/pkg/dbutils/mongodb_utils.go deleted file mode 100644 index 5ba8f02aad0..00000000000 --- a/pkg/dbutils/mongodb_utils.go +++ /dev/null @@ -1,96 +0,0 @@ -/* -Copyright (c) 2019 Bitnami - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package dbutils - -import ( - "fmt" - - "github.com/globalsign/mgo" - "github.com/kubeapps/common/datastore" -) - -const ( - ChartCollection = "charts" - RepositoryCollection = "repos" - ChartFilesCollection = "files" -) - -// MongodbAssetManager struct containing mongodb info -type MongodbAssetManager struct { - mongoConfig datastore.Config - DBSession datastore.Session - KubeappsNamespace string -} - -// NewMongoDBManager creates an asset manager for MongoDB -func NewMongoDBManager(config datastore.Config, kubeappsNamespace string) *MongodbAssetManager { - return &MongodbAssetManager{config, nil, kubeappsNamespace} -} - -// Init creates dbsession -func (m *MongodbAssetManager) Init() error { - dbSession, err := datastore.NewSession(m.mongoConfig) - if err != nil { - return fmt.Errorf("Can't connect to mongoDB: %v", err) - } - m.DBSession = dbSession - return nil -} - -// Close (no-op) -func (m *MongodbAssetManager) Close() error { - return nil -} - -// InitCollections ensure indexes of the different collections -func (m *MongodbAssetManager) InitCollections() error { - db, closer := m.DBSession.DB() - defer closer() - - err := db.C(ChartCollection).EnsureIndex(mgo.Index{ - Key: []string{"chart_id", "repo.namespace", "repo.name"}, - Unique: true, - DropDups: true, - Background: false, - }) - if err != nil { - return err - } - return db.C(ChartFilesCollection).EnsureIndex(mgo.Index{ - Key: []string{"file_id", "repo.namespace", "repo.name"}, - Background: false, - }) -} - -// InvalidateCache drops the different collections and initialize them again -func (m *MongodbAssetManager) InvalidateCache() error { - db, closer := m.DBSession.DB() - defer closer() - - err := db.C(ChartCollection).DropCollection() - // We ignore "ns not found" which relates to an operation on a non-existent collection. - if err != nil && err.Error() != "ns not found" { - return err - } - - err = m.InitCollections() - if err != nil { - return err - } - - return m.DBSession.Fsync(false) -} diff --git a/pkg/dbutils/postgresql_utils.go b/pkg/dbutils/postgresql_utils.go index 4db5903026d..af18312265e 100644 --- a/pkg/dbutils/postgresql_utils.go +++ b/pkg/dbutils/postgresql_utils.go @@ -62,7 +62,7 @@ type PostgresAssetManagerIface interface { type PostgresAssetManager struct { connStr string DB PostgresDB - kubeappsNamespace string + KubeappsNamespace string } // NewPGManager creates an asset manager for PG @@ -219,5 +219,5 @@ func (m *PostgresAssetManager) GetDB() PostgresDB { } func (m *PostgresAssetManager) GetKubeappsNamespace() string { - return m.kubeappsNamespace + return m.KubeappsNamespace } diff --git a/script/cluster-openshift.mk b/script/cluster-openshift.mk index dd1dd38b3d5..8b14959d0e6 100644 --- a/script/cluster-openshift.mk +++ b/script/cluster-openshift.mk @@ -3,67 +3,25 @@ # 1) helm installed # 2) minishift installed and a cluster started (which automatically updates your KUBECONFIG). # 3) the `oc` cli setup (ie. you've run `eval $(minishift oc-env)`) and are authed as `system:admin` -TILLER_NAMESPACE ?= tiller KUBEAPPS_NAMESPACE ?= kubeapps -MONGODB_CHART_VERSION = $(strip $(shell cat chart/kubeapps/requirements.lock | grep version | cut --delimiter=":" -f2)) - -devel/openshift-tiller-project-created: - oc new-project ${TILLER_NAMESPACE} - touch $@ - -# TODO: The following three targets create enable tiller to create CRDs -# cluster-wide as well as app repositories in the TILLER_NAMESPACE. They should -# not be specific to OpenShift, but the default for any development -# environment, though the importance of this will vanish with Helm3. -devel/openshift-tiller-with-crd-rbac.yaml: devel/openshift-tiller-project-created - @oc process -f ./docs/user/manifests/openshift-tiller-with-crd-rbac.yaml \ - -p TILLER_NAMESPACE="${TILLER_NAMESPACE}" \ - -o yaml \ - > $@ - -devel/openshift-tiller-with-apprepository-rbac.yaml: devel/openshift-tiller-with-crd-rbac.yaml - @oc process -f ./docs/user/manifests/openshift-tiller-with-apprepository-rbac.yaml \ - -p TILLER_NAMESPACE="${TILLER_NAMESPACE}" \ - -p KUBEAPPS_NAMESPACE="${KUBEAPPS_NAMESPACE}" \ - -o yaml \ - > $@ - -openshift-install-tiller: devel/openshift-tiller-with-crd-rbac.yaml devel/openshift-tiller-with-apprepository-rbac.yaml devel/openshift-kubeapps-project-created - kubectl --namespace ${TILLER_NAMESPACE} apply -f devel/openshift-tiller-with-crd-rbac.yaml --wait=true - kubectl --namespace ${KUBEAPPS_NAMESPACE} apply -f devel/openshift-tiller-with-apprepository-rbac.yaml - helm init --tiller-namespace ${TILLER_NAMESPACE} --service-account tiller --wait - -devel/openshift-kubeapps-project-created: devel/openshift-tiller-project-created +devel/openshift-kubeapps-project-created: oc new-project ${KUBEAPPS_NAMESPACE} - oc policy add-role-to-user edit "system:serviceaccount:${TILLER_NAMESPACE}:tiller" touch $@ chart/kubeapps/charts/mongodb-${MONGODB_CHART_VERSION}.tgz: helm dep build ./chart/kubeapps -devel/openshift-kubeapps-installed: openshift-install-tiller chart/kubeapps/charts/mongodb-${MONGODB_CHART_VERSION}.tgz +devel/openshift-kubeapps-installed: @oc project ${KUBEAPPS_NAMESPACE} - helm --tiller-namespace=${TILLER_NAMESPACE} install ./chart/kubeapps -n ${KUBEAPPS_NAMESPACE} \ - --set tillerProxy.host=tiller-deploy.${TILLER_NAMESPACE}:44134 \ + helm install ./chart/kubeapps -n ${KUBEAPPS_NAMESPACE} \ --values ./docs/user/manifests/kubeapps-local-dev-values.yaml -# Due to openshift having multiple secrets for the service account, the code is slightly different from -# that at /~https://github.com/kubeapps/kubeapps/blob/master/docs/user/getting-started.md#on-linuxmacos -# TODO: update this target to use a kubeapps user, rather than tiller service account. -openshift-tiller-token: - @kubectl get secret -n "${TILLER_NAMESPACE}" \ - $(shell kubectl get serviceaccount -n "${TILLER_NAMESPACE}" tiller -o jsonpath='{range .secrets[*]}{.name}{"\n"}{end}' | grep tiller-token) \ - -o go-template='{{.data.token | base64decode}}' && echo - openshift-kubeapps: devel/openshift-kubeapps-installed openshift-kubeapps-reset: oc delete project ${KUBEAPPS_NAMESPACE} || true - oc delete project ${TILLER_NAMESPACE} || true - oc delete -f devel/openshift-tiller-with-crd-rbac.yaml || true - oc delete -f devel/openshift-tiller-with-apprepository-rbac.yaml || true oc delete customresourcedefinition apprepositories.kubeapps.com || true rm devel/openshift-* || true -.PHONY: openshift-install-tiller openshift-kubeapps openshift-kubeapps-reset +.PHONY: openshift-kubeapps openshift-kubeapps-reset diff --git a/script/e2e-test.sh b/script/e2e-test.sh index 1c19ee1e981..2a6e79b44fc 100755 --- a/script/e2e-test.sh +++ b/script/e2e-test.sh @@ -137,7 +137,6 @@ installOrUpgradeKubeapps() { helm upgrade --install kubeapps-ci --namespace kubeapps "${chartSource}" \ ${invalidateCacheFlag} \ "${img_flags[@]}" \ - "${db_flags[@]}" \ --set featureFlags.ui=clarity \ --set featureFlags.operators=true } @@ -152,9 +151,6 @@ info "IMAGE_REPO_SUFFIX: $IMG_MODIFIER" info "Cluster Version: $(kubectl version -o json | jq -r '.serverVersion.gitVersion')" info "Kubectl Version: $(kubectl version -o json | jq -r '.clientVersion.gitVersion')" -db_flags=("--set" "mongodb.enabled=true" "--set" "postgresql.enabled=false") -[[ "${KUBEAPPS_DB:-}" == "postgresql" ]] && db_flags=("--set" "mongodb.enabled=false" "--set" "postgresql.enabled=true") - # Use dev images or Bitnami if testing the latest release image_prefix="kubeapps/" [[ -n "${TEST_LATEST_RELEASE:-}" ]] && image_prefix="bitnami/kubeapps-"