enable ability to index new chart data for the same chart version (#556)

* enable ability to index new chart data for the same chart version

Previously, chart-repo skipped indexing any chart versions previous
indexed based on the repo, name and version. However, if a user modifies
a chart version without bumping a new version, the changes are not taken
into account. This changes chart-repo to skip based on the above AND the
digest of the chart package so that packages with a new digest are
correctly updated.

See also vmware-tanzu/kubeapps#659

fixes #555

Signed-off-by: Adnan Abdulhussein <[email protected]>

* update kubeapps/common dep to pull in fix for tests

Signed-off-by: Adnan Abdulhussein <[email protected]>

* add comments

Signed-off-by: Adnan Abdulhussein <[email protected]>

cmd/chart-repo/types.go

@@ -57,4 +57,5 @@ type chartFiles struct {
 	Readme string
 	Values string
 	Repo   repo
+	Digest string
 }

cmd/chart-repo/utils.go

@@ -315,7 +315,9 @@ func fetchAndImportFiles(dbSession datastore.Session, name string, r repo, cv ch
 	chartFilesID := fmt.Sprintf("%s/%s-%s", r.Name, name, cv.Version)
 	db, closer := dbSession.DB()
 	defer closer()
-	if err := db.C(chartFilesCollection).FindId(chartFilesID).One(&chartFiles{}); err == nil {
+
+	// Check if we already have indexed files for this chart version and digest
+	if err := db.C(chartFilesCollection).Find(bson.M{"_id": chartFilesID, "digest": cv.Digest}).One(&chartFiles{}); err == nil {
 		log.WithFields(log.Fields{"name": name, "version": cv.Version}).Debug("skipping existing files")
 		return nil
 	}
@@ -357,7 +359,7 @@ func fetchAndImportFiles(dbSession datastore.Session, name string, r repo, cv ch
 		return err
 	}
 
-	chartFiles := chartFiles{ID: chartFilesID, Repo: r}
+	chartFiles := chartFiles{ID: chartFilesID, Repo: r, Digest: cv.Digest}
 	if v, ok := files[readmeFileName]; ok {
 		chartFiles.Readme = v
 	} else {
@@ -369,7 +371,9 @@ func fetchAndImportFiles(dbSession datastore.Session, name string, r repo, cv ch
 		log.WithFields(log.Fields{"name": name, "version": cv.Version}).Info("values.yaml not found")
 	}
 
-	db.C(chartFilesCollection).Insert(chartFiles)
+	// inserts the chart files if not already indexed, or updates the existing
+	// entry if digest has changed
+	db.C(chartFilesCollection).UpsertId(chartFilesID, chartFiles)
 
 	return nil
 }

cmd/chart-repo/utils_test.go

@@ -346,7 +346,8 @@ func Test_fetchAndImportFiles(t *testing.T) {
 		netClient = &goodTarballClient{c: charts[0], skipValues: true, skipReadme: true}
 		m := mock.Mock{}
 		m.On("One", mock.Anything).Return(errors.New("return an error when checking if files already exists to force fetching"))
-		m.On("Insert", chartFiles{fmt.Sprintf("%s/%s-%s", charts[0].Repo.Name, charts[0].Name, cv.Version), "", "", charts[0].Repo})
+		chartFilesID := fmt.Sprintf("%s/%s-%s", charts[0].Repo.Name, charts[0].Name, cv.Version)
+		m.On("UpsertId", chartFilesID, chartFiles{chartFilesID, "", "", charts[0].Repo, cv.Digest})
 		dbSession := mockstore.NewMockSession(&m)
 		err := fetchAndImportFiles(dbSession, charts[0].Name, charts[0].Repo, cv)
 		assert.NoErr(t, err)
@@ -357,7 +358,8 @@ func Test_fetchAndImportFiles(t *testing.T) {
 		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"))
-		m.On("Insert", chartFiles{fmt.Sprintf("%s/%s-%s", charts[0].Repo.Name, charts[0].Name, cv.Version), testChartReadme, testChartValues, charts[0].Repo})
+		chartFilesID := fmt.Sprintf("%s/%s-%s", charts[0].Repo.Name, charts[0].Name, cv.Version)
+		m.On("UpsertId", chartFilesID, chartFiles{chartFilesID, testChartReadme, testChartValues, charts[0].Repo, cv.Digest})
 		dbSession := mockstore.NewMockSession(&m)
 		err := fetchAndImportFiles(dbSession, charts[0].Name, charts[0].Repo, cv)
 		assert.NoErr(t, err)
@@ -368,7 +370,8 @@ func Test_fetchAndImportFiles(t *testing.T) {
 		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"))
-		m.On("Insert", chartFiles{fmt.Sprintf("%s/%s-%s", charts[0].Repo.Name, charts[0].Name, cv.Version), testChartReadme, testChartValues, charts[0].Repo})
+		chartFilesID := fmt.Sprintf("%s/%s-%s", charts[0].Repo.Name, charts[0].Name, cv.Version)
+		m.On("UpsertId", chartFilesID, chartFiles{chartFilesID, testChartReadme, testChartValues, charts[0].Repo, cv.Digest})
 		dbSession := mockstore.NewMockSession(&m)
 		err := fetchAndImportFiles(dbSession, charts[0].Name, charts[0].Repo, cv)
 		assert.NoErr(t, err)
@@ -382,7 +385,7 @@ func Test_fetchAndImportFiles(t *testing.T) {
 		dbSession := mockstore.NewMockSession(&m)
 		err := fetchAndImportFiles(dbSession, charts[0].Name, charts[0].Repo, cv)
 		assert.NoErr(t, err)
-		m.AssertNotCalled(t, "Insert", mock.Anything)
+		m.AssertNotCalled(t, "UpsertId", mock.Anything, mock.Anything)
 	})
 }