Merge pull request #45360 from diogoasouza/update-oci-list-of-charts-…

…when-a-chart-or-tag-is-deleted removing deleted tags from the helm index file
rancher · May 8, 2024 · 4964f37 · 4964f37
2 parents e768a5e + cfb5844
commit 4964f37
Show file tree

Hide file tree

Showing 2 changed files with 84 additions and 6 deletions.
diff --git a/pkg/catalogv2/oci/oci.go b/pkg/catalogv2/oci/oci.go
@@ -127,7 +127,9 @@ func GenerateIndex(URL string, credentialSecret *corev1.Secret,
 
 	// Loop over all the tags and find the latest version
 	tagsFunc := func(tags []string) error {
+		existingTags := make(map[string]bool)
 		for i := len(tags) - 1; i >= 0; i-- {
+			existingTags[tags[i]] = true
 			// Check if the tag is a valid semver version or not. If yes, then proceed.
 			semverTag, err := version.NewVersion(tags[i])
 			if err != nil {
@@ -151,26 +153,35 @@ func GenerateIndex(URL string, credentialSecret *corev1.Secret,
 				indexFile.Entries[chartName] = append(indexFile.Entries[chartName], chartVersion)
 			}
 		}
+		var updatedChartVersions []*repo.ChartVersion
+
+		// Only keep the versions that are also present in the /tags/list call
+		for _, chartVersion := range indexFile.Entries[chartName] {
+			if existingTags[chartVersion.Version] {
+				updatedChartVersions = append(updatedChartVersions, chartVersion)
+			}
+		}
+		indexFile.Entries[chartName] = updatedChartVersions
 		return nil
 	}
 
 	// Loop over all the repositories and fetch the tags
 	repositoriesFunc := func(repositories []string) error {
+		existingCharts := make(map[string]bool)
 		for _, repository := range repositories {
 			logrus.Debugf("found repository %s for OCI clusterrepo URL %s", repository, URL)
-
 			// Storing the user provided repository that can be an oras repository or a sub repository.
 			userProvidedRepository := ociClient.repository
 
 			// Work on the oci repositories that match with the userProvidedRepository
 			if _, found := strings.CutPrefix(repository, ociClient.repository); found {
 				ociClient.repository = repository
-
 				orasRepository, err := ociClient.GetOrasRepository()
 				if err != nil {
 					return fmt.Errorf("failed to create an oras repository for url %s: %w", URL, err)
 				}
 				chartName = ociClient.repository[strings.LastIndex(ociClient.repository, "/")+1:]
+				existingCharts[chartName] = true
 				maxTag = nil
 
 				// call tags to get the max tag and update the indexFile
@@ -191,6 +202,13 @@ func GenerateIndex(URL string, credentialSecret *corev1.Secret,
 			}
 			ociClient.repository = userProvidedRepository
 		}
+
+		// Only keep the charts that are also present in the /_catalog call
+		for chartName := range indexFile.Entries {
+			if !existingCharts[chartName] {
+				delete(indexFile.Entries, chartName)
+			}
+		}
 		return nil
 	}
 

diff --git a/pkg/catalogv2/oci/oci_test.go b/pkg/catalogv2/oci/oci_test.go
@@ -111,6 +111,36 @@ func TestGenerateIndex(t *testing.T) {
 			},
 		},
 	}
+
+	indexFile3 := repo.NewIndexFile()
+	indexFile3.Entries["testingchart"] = repo.ChartVersions{
+		&repo.ChartVersion{
+			Metadata: &chart.Metadata{
+				Name:    "testingchart",
+				Version: "1.0.0",
+			},
+			Digest: "digest",
+		},
+	}
+	indexFile4 := repo.NewIndexFile()
+	indexFile4.Entries["anotherchart"] = repo.ChartVersions{
+		&repo.ChartVersion{
+			Metadata: &chart.Metadata{
+				Name:    "anotherchart",
+				Version: "1.0.0",
+			},
+			Digest: "digest",
+		},
+	}
+	indexFile4.Entries["anotherchartagain"] = repo.ChartVersions{
+		&repo.ChartVersion{
+			Metadata: &chart.Metadata{
+				Name:    "anotherchartagain",
+				Version: "1.0.0",
+			},
+			Digest: "digest",
+		},
+	}
 	one := 1
 	two := 2
 
@@ -119,6 +149,7 @@ func TestGenerateIndex(t *testing.T) {
 		indexFile       *repo.IndexFile
 		expectedErrMsg  string
 		numberOfEntries *int
+		numberOfCharts  *int
 		secret          *corev1.Secret
 		url             string
 		urlPath         string
@@ -129,6 +160,7 @@ func TestGenerateIndex(t *testing.T) {
 			"failed to create an OCI client for url",
 			nil,
 			nil,
+			nil,
 			"invalidUrl//",
 			"",
 		},
@@ -138,6 +170,7 @@ func TestGenerateIndex(t *testing.T) {
 			"failed to create an OCI client for url",
 			nil,
 			nil,
+			nil,
 			"http://github.com/rancher/charts",
 			"",
 		},
@@ -147,6 +180,7 @@ func TestGenerateIndex(t *testing.T) {
 			repo.NewIndexFile(),
 			"",
 			&one,
+			&one,
 			nil,
 			"",
 			"testingchart:0.1.0",
@@ -156,6 +190,7 @@ func TestGenerateIndex(t *testing.T) {
 			repo.NewIndexFile(),
 			"",
 			&two,
+			&one,
 			nil,
 			"",
 			"testingchart",
@@ -165,6 +200,7 @@ func TestGenerateIndex(t *testing.T) {
 			repo.NewIndexFile(),
 			"",
 			&two,
+			&one,
 			nil,
 			"",
 			"",
@@ -174,10 +210,31 @@ func TestGenerateIndex(t *testing.T) {
 			indexFile,
 			"",
 			&one,
+			&one,
 			nil,
 			"",
 			"testingchart:0.1.0",
 		},
+		{
+			"Index file should not have versions that aren't present in the response of /tags/list",
+			indexFile3,
+			"",
+			&two,
+			&one,
+			nil,
+			"",
+			"",
+		},
+		{
+			"Index file should not have repositories that aren't present in the response of /_catalog",
+			indexFile4,
+			"",
+			&two,
+			&one,
+			nil,
+			"",
+			"",
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -190,14 +247,17 @@ func TestGenerateIndex(t *testing.T) {
 			repoSpec := v1.RepoSpec{InsecurePlainHTTP: true, InsecureSkipTLSverify: true}
 			i, err := GenerateIndex(u, nil, repoSpec, v1.RepoStatus{}, tt.indexFile)
 			if tt.expectedErrMsg != "" {
-				assert.Contains(t, err.Error(), tt.expectedErrMsg)
+				assert.Contains(t, err.Error(), tt.expectedErrMsg, "wrong error")
+			}
+			if tt.numberOfCharts != nil {
+				assert.Equal(t, len(i.Entries), *tt.numberOfCharts, "number of charts don't match")
 			}
 			if tt.numberOfEntries != nil {
-				assert.Equal(t, len(i.Entries["testingchart"]), *tt.numberOfEntries)
+				assert.Equal(t, len(i.Entries["testingchart"]), *tt.numberOfEntries, "number of entries don't match")
 				i.SortEntries()
-				assert.NotEmpty(t, i.Entries["testingchart"][0].Digest)
+				assert.NotEmpty(t, i.Entries["testingchart"][0].Digest, "wrong digest for the first entry")
 				if *tt.numberOfEntries > 1 {
-					assert.Empty(t, i.Entries["testingchart"][1].Digest)
+					assert.Empty(t, i.Entries["testingchart"][1].Digest, "wrong digest for the second entry")
 				}
 			}
 		})