Improved version taking into account the last PR comments

Author: chrissgyulev

charts-syncer.yaml

@@ -4,11 +4,6 @@
 
 # source includes relevant information about the source chart repository
 source:
-  # Dependencies located in repos from this list will be considered as trusted, and also synced.
-  # The entry format is the same as "repo" (see below)
-  trustedSourceDeps:
-  - kind: HELM
-    url: https://grafana.github.io/helm-charts
   repo:
     # Kind specify the chart repository kind. Valid values are HELM, CHARTMUSEUM, and HARBOR
     kind: HELM
@@ -27,9 +22,7 @@ source:
     # chartsIndex: my-oci-registry.io/my-project/my-custom-index:prod
 # target includes relevant information about the target chart repository
 target:
-  # In case there is a need to mirror dependencies (from trustedSourceDeps list, see above) - this must be set to true
-  replaceDependencyRepo: true
-# repoName is used to modify the README of the chart. Default value: `myrepo`
+  # repoName is used to modify the README of the chart. Default value: `myrepo`
   repoName: myrepo
   # containerRegistry is used to update the image registry section of the values.yaml file
   # NOTE: If containerRegistry is not set (or not present), the registry sections won't be updated

examples/sync-deps.yaml

@@ -4,33 +4,35 @@
 
 # source includes relevant information about the source chart repository
 source:
-  # Dependencies located in repos from this list will be considered as trusted, and also synced.
-  # The entry format is the same as "repo" (see below)
-  trustedSourceDeps:
-  - kind: HELM
-    url: https://grafana.github.io/helm-charts
+  # Optional: Dependencies located in repos from this list will be considered as trusted and maintained as is
+  # if there is a need to work with external repositories different from the source - they must be included here
+  ignoreTrustedRepos:
+    - kind: HELM
+      url: https://grafana.github.io/helm-charts
   repo:
     # Kind specify the chart repository kind. Valid values are HELM, CHARTMUSEUM, and HARBOR
     kind: HELM
     # url is the url of the chart repository
-    url: https://prometheus-community.github.io/helm-charts # local test source repo
+    url: https://prometheus-community.github.io/helm-charts  # local test source repo
+
 # target includes relevant information about the target chart repository
 target:
-  # In case there is a need to mirror dependencies (from trustedSourceDeps list, see above) - this must be set to true
-  replaceDependencyRepo: true
+  # repoName is used to modify the README of the chart. Default value: `myrepo`
+  repoName: myrepo
+
+  # Optional: Dependencies located in repos from this list will be considered as trusted and also synced to the target.
+  # This setting takes precedence of source.ignoreTrustedRepos for the same entry.
+  # If there is a need to work with external repositories different from the source - they must be included here
+  syncTrustedRepos:
+    - kind: HELM
+      url: https://grafana.github.io/helm-charts
   repo:
     # Kind specify the chart repository kind. Valid values are HELM, CHARTMUSEUM, and HARBOR
     kind: LOCAL
-    path: localrepo
-# charts is an OPTIONAL list to specify a subset of charts to be synchronized
-# It is mandatory if the source repo is OCI and not autodiscovery is supported in that repository
-# More info here https://github.com/bitnami-labs/charts-syncer#charts-index-for-oci-based-repositories
+    # url is the url of the chart repository
+    path: localrepo # local test target repo
 charts:
   - kube-prometheus-stack
-# opt-out counterpart of "charts" property that explicit list the Helm charts to be skipped
-# either "charts" or "skipCharts" can be used at once
-# skipCharts:
-#  - mariadb
 
 # Whether to also relocate the container images referenced by the Helm Chart
 # Note that this requires the Helm Chart to be compatible with relok8s tool by containing a .relok8s-images.yaml file

internal/chart/dependency.go

@@ -4,16 +4,15 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
-	"io/ioutil"
-	"net/url"
-	"os"
-	"path"
-
 	"github.com/juju/errors"
 	"github.com/mkmik/multierror"
 	"helm.sh/helm/v3/pkg/chart"
 	"helm.sh/helm/v3/pkg/provenance"
+	"io/ioutil"
 	"k8s.io/klog"
+	"net/url"
+	"os"
+	"path"
 	"sigs.k8s.io/yaml"
 
 	"github.com/bitnami-labs/charts-syncer/api"
@@ -65,7 +64,7 @@ func GetChartLock(chartPath string) (*chart.Lock, error) {
 	return lock, nil
 }
 
-// GetChartDependencies returns the chart chart.Dependencies from a chart in tgz format.
+// GetChartDependencies returns the chart dependencies from a chart in tgz format.
 func GetChartDependencies(filepath string, name string) ([]*chart.Dependency, error) {
 	// Create temporary working directory
 	chartPath, err := ioutil.TempDir("", "charts-syncer")
@@ -112,9 +111,9 @@ func GetLockAPIVersion(chartPath string) (string, error) {
 
 // BuildDependencies updates the chart dependencies and their repository references in the provided chart path
 //
-// It reads the lock file to download the versions from the target
-// chart repository (it assumes all charts are stored in a single repo).
-func BuildDependencies(chartPath string, r client.ChartsReader, sourceRepo, targetRepo *api.Repo, replaceDependencyRepo bool) error {
+// It reads the lock file to download the versions from the target chart repository
+func BuildDependencies(chartPath string, r client.ChartsReader, sourceRepo, targetRepo *api.Repo, t map[uint32]client.ChartsReaderWriter, syncTrusted, ignoreTrusted []*api.Repo) error {
+
 	// Build deps manually for OCI as helm does not support it yet
 	if err := os.RemoveAll(path.Join(chartPath, "charts")); err != nil {
 		return errors.Trace(err)
@@ -138,13 +137,14 @@ func BuildDependencies(chartPath string, r client.ChartsReader, sourceRepo, targ
 	if apiVersion == "" {
 		return nil
 	}
+
 	switch apiVersion {
 	case APIV1:
-		if err := updateRequirementsFile(chartPath, lock, sourceRepo, targetRepo, replaceDependencyRepo); err != nil {
+		if err := updateRequirementsFile(chartPath, lock, sourceRepo, targetRepo, syncTrusted, ignoreTrusted); err != nil {
 			return errors.Trace(err)
 		}
 	case APIV2:
-		if err := updateChartMetadataFile(chartPath, lock, sourceRepo, targetRepo, replaceDependencyRepo); err != nil {
+		if err := updateChartMetadataFile(chartPath, lock, sourceRepo, targetRepo, syncTrusted, ignoreTrusted); err != nil {
 			return errors.Trace(err)
 		}
 	default:
@@ -158,7 +158,22 @@ func BuildDependencies(chartPath string, r client.ChartsReader, sourceRepo, targ
 			id := fmt.Sprintf("%s-%s", dep.Name, dep.Version)
 			klog.V(4).Infof("Building %q chart dependency", id)
 
-			depTgz, err := r.Fetch(dep.Name, dep.Version)
+			var repoClient client.ChartsReader = nil
+
+			depRepo := api.Repo{
+				Url: dep.Repository,
+			}
+
+			//if the repo is trusted and won't be synced - we download the dependency from it (source)
+			if utils.ShouldIgnoreRepo(depRepo, syncTrusted, ignoreTrusted) {
+				repoClient = t[utils.GetRepoLocationId(dep.Repository)]
+			} else {
+				//otherwise we download it from the destination repo
+				repoClient = r
+			}
+
+			depTgz, err := repoClient.Fetch(dep.Name, dep.Version)
+
 			if err != nil {
 				klog.Warningf("Failed fetching %q chart. The dependencies processing will remain incomplete.", id)
 				errs = multierror.Append(errs, errors.Annotatef(err, "fetching %q chart", id))
@@ -179,7 +194,7 @@ func BuildDependencies(chartPath string, r client.ChartsReader, sourceRepo, targ
 
 // updateChartMetadataFile updates the dependencies in Chart.yaml
 // For helm v3 dependency management
-func updateChartMetadataFile(chartPath string, lock *chart.Lock, sourceRepo, targetRepo *api.Repo, replaceDependencyRepo bool) error {
+func updateChartMetadataFile(chartPath string, lock *chart.Lock, sourceRepo, targetRepo *api.Repo, syncTrusted, ignoreTrusted []*api.Repo) error {
 	chartFile := path.Join(chartPath, ChartFilename)
 	chartYamlContent, err := ioutil.ReadFile(chartFile)
 	if err != nil {
@@ -191,8 +206,15 @@ func updateChartMetadataFile(chartPath string, lock *chart.Lock, sourceRepo, tar
 		return errors.Annotatef(err, "error unmarshaling %s file", chartFile)
 	}
 	for _, dep := range chartMetadata.Dependencies {
-		// Maybe there are dependencies from other chart repos. In this case we don't want to replace
-		// the repository.
+		// Maybe there are dependencies from other chart repos. We replace them or not depending on what we have in
+		// source.ignoreTrustedRepos and target.syncTrustedRepos (the logic can be found in utils.ShouldIgnoreRepo)
+		r := api.Repo{
+			Url: dep.Repository,
+		}
+
+		//ignore repo means don't replace it, don't ignore - means "replace it" - use negation to achieve it
+		replaceDependencyRepo := !utils.ShouldIgnoreRepo(r, syncTrusted, ignoreTrusted)
+
 		if dep.Repository == sourceRepo.GetUrl() || replaceDependencyRepo {
 			repoUrl, err := getDependencyRepoURL(targetRepo)
 			if err != nil {
@@ -206,15 +228,15 @@ func updateChartMetadataFile(chartPath string, lock *chart.Lock, sourceRepo, tar
 	if err := writeChartFile(dest, chartMetadata); err != nil {
 		return errors.Trace(err)
 	}
-	if err := updateLockFile(chartPath, lock, chartMetadata.Dependencies, sourceRepo, targetRepo, false, replaceDependencyRepo); err != nil {
+	if err := updateLockFile(chartPath, lock, chartMetadata.Dependencies, sourceRepo, targetRepo, false, syncTrusted, ignoreTrusted); err != nil {
 		return errors.Trace(err)
 	}
 	return nil
 }
 
 // updateRequirementsFile returns the full list of dependencies and the list of missing dependencies.
 // For helm v2 dependency management
-func updateRequirementsFile(chartPath string, lock *chart.Lock, sourceRepo, targetRepo *api.Repo, replaceDependencyRepo bool) error {
+func updateRequirementsFile(chartPath string, lock *chart.Lock, sourceRepo, targetRepo *api.Repo, syncTrusted, ignoreTrusted []*api.Repo) error {
 	requirementsFile := path.Join(chartPath, RequirementsFilename)
 	requirements, err := ioutil.ReadFile(requirementsFile)
 	if err != nil {
@@ -227,8 +249,15 @@ func updateRequirementsFile(chartPath string, lock *chart.Lock, sourceRepo, targ
 		return errors.Annotatef(err, "error unmarshaling %s file", requirementsFile)
 	}
 	for _, dep := range deps.Dependencies {
-		// Maybe there are dependencies from other chart repos. In this case we don't want to replace
-		// the repository.
+		// Maybe there are dependencies from other chart repos. We replace them or not depending on what we have in
+		// source.ignoreTrustedRepos and target.syncTrustedRepos (the logic can be found in utils.ShouldIgnoreRepo)
+		r := api.Repo{
+			Url: dep.Repository,
+		}
+
+		//ignore repo means don't replace it, don't ignore - means "replace it" - use negation to achieve it
+		replaceDependencyRepo := !utils.ShouldIgnoreRepo(r, syncTrusted, ignoreTrusted)
+
 		// For example, old charts pointing to helm/charts repo
 		if dep.Repository == sourceRepo.GetUrl() || replaceDependencyRepo {
 			repoUrl, err := getDependencyRepoURL(targetRepo)
@@ -243,15 +272,25 @@ func updateRequirementsFile(chartPath string, lock *chart.Lock, sourceRepo, targ
 	if err := writeChartFile(dest, deps); err != nil {
 		return errors.Trace(err)
 	}
-	if err := updateLockFile(chartPath, lock, deps.Dependencies, sourceRepo, targetRepo, true, replaceDependencyRepo); err != nil {
+	if err := updateLockFile(chartPath, lock, deps.Dependencies, sourceRepo, targetRepo, true, syncTrusted, ignoreTrusted); err != nil {
 		return errors.Trace(err)
 	}
 	return nil
 }
 
 // updateLockFile updates the lock file with the new registry
-func updateLockFile(chartPath string, lock *chart.Lock, deps []*chart.Dependency, sourceRepo *api.Repo, targetRepo *api.Repo, legacyLockfile, replaceDependencyRepo bool) error {
+func updateLockFile(chartPath string, lock *chart.Lock, deps []*chart.Dependency, sourceRepo *api.Repo, targetRepo *api.Repo, legacyLockfile bool, syncTrusted, ignoreTrusted []*api.Repo) error {
 	for _, dep := range lock.Dependencies {
+
+		// Maybe there are dependencies from other chart repos. We replace them or not depending on what we have in
+		// source.ignoreTrustedRepos and target.syncTrustedRepos (the logic can be found in utils.ShouldIgnoreRepo)
+		r := api.Repo{
+			Url: dep.Repository,
+		}
+
+		//ignore repo means don't replace it, don't ignore - means "replace it" - use negation to achieve it
+		replaceDependencyRepo := !utils.ShouldIgnoreRepo(r, syncTrusted, ignoreTrusted)
+
 		if dep.Repository == sourceRepo.GetUrl() || replaceDependencyRepo {
 			repoUrl, err := getDependencyRepoURL(targetRepo)
 			if err != nil {

internal/chart/dependency_test.go

@@ -89,7 +89,10 @@ func TestUpdateRequirementsFile(t *testing.T) {
 
 	chartPath := newChartPath(t, "../../testdata/kafka-10.3.3.tgz", "kafka")
 	requirementsFile := path.Join(chartPath, RequirementsFilename)
-	if err := updateRequirementsFile(chartPath, lock, source.GetRepo(), target.GetRepo(), false); err != nil {
+
+	var ignoreTrusted, syncTrusted []*api.Repo
+
+	if err := updateRequirementsFile(chartPath, lock, source.GetRepo(), target.GetRepo(), syncTrusted, ignoreTrusted); err != nil {
 		t.Fatal(err)
 	}
 
@@ -163,7 +166,9 @@ func TestUpdateChartMetadataFile(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if err := updateChartMetadataFile(chartPath, lock, source.GetRepo(), target.GetRepo(), false); err != nil {
+	var ignoreTrusted, syncTrusted []*api.Repo
+
+	if err := updateChartMetadataFile(chartPath, lock, source.GetRepo(), target.GetRepo(), syncTrusted, ignoreTrusted); err != nil {
 		t.Fatal(err)
 	}
 

internal/utils/utils.go

@@ -6,6 +6,7 @@ import (
 	"crypto/sha1"
 	"crypto/tls"
 	"fmt"
+	"hash/fnv"
 	"io"
 	"io/ioutil"
 	"net"
@@ -445,3 +446,43 @@ func FetchAndCache(name, version string, cache cache.Cacher, fopts ...FetchOptio
 
 	return cache.Path(id), nil
 }
+
+func ShouldIgnoreRepo(repo api.Repo, syncTrusted, ignoreTrusted []*api.Repo) bool {
+
+	repoLocationId := GetRepoLocationId(GetRepoLocation(&repo))
+
+	for _, trRepo := range syncTrusted {
+		if GetRepoLocationId(GetRepoLocation(trRepo)) == repoLocationId {
+			return false
+		}
+	}
+
+	for _, ignoreTrRepo := range ignoreTrusted {
+		if GetRepoLocationId(GetRepoLocation(ignoreTrRepo)) == repoLocationId {
+			return true
+		}
+	}
+
+	return false
+}
+
+// GetRepoLocationId returns a unique id for a repo based on the repo url or path
+func GetRepoLocationId(l string) uint32 {
+	h := fnv.New32a()
+
+	//@todo trim whitespaces from the values used ?!
+	h.Write([]byte(strings.ToLower(l)))
+
+	return h.Sum32()
+}
+
+// GetRepoLocation returns the repo url or path
+func GetRepoLocation(repo *api.Repo) string {
+	if repo.Url != "" {
+		//remote repo
+		return repo.Url
+	}
+
+	//local repo
+	return repo.Path
+}

pkg/syncer/index.go

@@ -2,6 +2,7 @@ package syncer
 
 import (
 	"fmt"
+	"github.com/bitnami-labs/charts-syncer/api"
 	"sort"
 	"time"
 
@@ -20,6 +21,7 @@ type Chart struct {
 	Name         string
 	Version      string
 	Dependencies []string
+	Repo         api.Repo
 
 	TgzPath string
 }
@@ -161,7 +163,6 @@ func (s *Syncer) processVersion(name, version string, publishingThreshold time.T
 		klog.V(5).Infof("Skipping %q chart: Already indexed", id)
 		return nil
 	}
-
 	if err := s.loadChart(name, version, "", false); err != nil {
 		klog.Errorf("unable to load %q chart: %v", id, err)
 		return err
@@ -199,15 +200,17 @@ func (s *Syncer) loadChart(name string, version string, repository string, isDep
 		return nil
 	}
 
-	//main source repo client
-	client := s.cli.src
+	var tgz string
+	var err error
+
+	repoKey := utils.GetRepoLocationId(repository)
 
-	//in case of dependency - switch to deps client, but only if there is a valid entry for the current repo
-	if isDep && len(s.cli.deps) > 0 && s.cli.deps[repository] != nil {
-		client = s.cli.deps[repository]
+	if isDep && s.cli.trusted[repoKey] != nil {
+		tgz, err = s.cli.trusted[repoKey].Fetch(name, version)
+	} else {
+		tgz, err = s.cli.src.Fetch(name, version)
 	}
 
-	tgz, err := client.Fetch(name, version)
 	if err != nil {
 		return errors.Trace(err)
 	}
@@ -216,6 +219,7 @@ func (s *Syncer) loadChart(name string, version string, repository string, isDep
 		Name:    name,
 		Version: version,
 		TgzPath: tgz,
+		Repo:    api.Repo{Url: repository},
 	}
 
 	if !s.skipDependencies {