internal/helm: various nitpicks

- Add some more documentation around chart builders
- Ensure correct indentation in some doc comments
- Provide example of using `errors.Is` for typed `BuildError`
- Mention "bytes" in file size limit errors
- Add missing copyright header

Signed-off-by: Hidde Beydals <hello@hidde.co>

internal/helm/chart/builder_local.go

@@ -34,16 +34,34 @@ type localChartBuilder struct {
 	dm *DependencyManager
 }
 
-// NewLocalBuilder returns a Builder capable of building a Helm
-// chart with a LocalReference. For chart references pointing to a
-// directory, the DependencyManager is used to resolve missing local and
-// remote dependencies.
+// NewLocalBuilder returns a Builder capable of building a Helm chart with a
+// LocalReference. For chart references pointing to a directory, the
+// DependencyManager is used to resolve missing local and remote dependencies.
 func NewLocalBuilder(dm *DependencyManager) Builder {
 	return &localChartBuilder{
 		dm: dm,
 	}
 }
 
+// Build attempts to build a Helm chart with the given LocalReference and
+// BuildOptions, writing it to p.
+// It returns a Build describing the produced (or from cache observed) chart
+// written to p, or a BuildError.
+//
+// The chart is loaded from the LocalReference.Path, and only packaged if the
+// version (including BuildOptions.VersionMetadata modifications) differs from
+// the current BuildOptions.CachedChart.
+//
+// BuildOptions.ValuesFiles changes are in this case not taken into account,
+// and BuildOptions.Force should be used to enforce a rebuild.
+//
+// If the LocalReference.Path refers to an already packaged chart, and no
+// packaging is required due to BuildOptions modifying the chart,
+// LocalReference.Path is copied to p.
+//
+// If the LocalReference.Path refers to a chart directory, dependencies are
+// confirmed to be present using the DependencyManager, while attempting to
+// resolve any missing.
 func (b *localChartBuilder) Build(ctx context.Context, ref Reference, p string, opts BuildOptions) (*Build, error) {
 	localRef, ok := ref.(LocalReference)
 	if !ok {
@@ -80,8 +98,8 @@ func (b *localChartBuilder) Build(ctx context.Context, ref Reference, p string,
 	}
 
 	// If all the following is true, we do not need to package the chart:
-	//	Chart version from metadata matches chart version for ref
-	//	BuildOptions.Force is False
+	// - Chart version from current metadata matches calculated version
+	// - BuildOptions.Force is False
 	if opts.CachedChart != "" && !opts.Force {
 		if curMeta, err = LoadChartMetadataFromArchive(opts.CachedChart); err == nil && result.Version == curMeta.Version {
 			result.Path = opts.CachedChart

internal/helm/chart/builder_remote.go

@@ -40,13 +40,27 @@ type remoteChartBuilder struct {
 }
 
 // NewRemoteBuilder returns a Builder capable of building a Helm
-// chart with a RemoteReference from the given Index.
+// chart with a RemoteReference in the given repository.ChartRepository.
 func NewRemoteBuilder(repository *repository.ChartRepository) Builder {
 	return &remoteChartBuilder{
 		remote: repository,
 	}
 }
 
+// Build attempts to build a Helm chart with the given RemoteReference and
+// BuildOptions, writing it to p.
+// It returns a Build describing the produced (or from cache observed) chart
+// written to p, or a BuildError.
+//
+// The latest version for the RemoteReference.Version is determined in the
+// repository.ChartRepository, only downloading it if the version (including
+// BuildOptions.VersionMetadata) differs from the current BuildOptions.CachedChart.
+// BuildOptions.ValuesFiles changes are in this case not taken into account,
+// and BuildOptions.Force should be used to enforce a rebuild.
+//
+// After downloading the chart, it is only packaged if required due to BuildOptions
+// modifying the chart, otherwise the exact data as retrieved from the repository
+// is written to p, after validating it to be a chart.
 func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, opts BuildOptions) (*Build, error) {
 	remoteRef, ok := ref.(RemoteReference)
 	if !ok {
@@ -88,8 +102,8 @@ func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, o
 	}
 
 	// If all the following is true, we do not need to download and/or build the chart:
-	//	Chart version from metadata matches chart version for ref
-	//  BuildOptions.Force is False
+	// - Chart version from current metadata matches calculated version
+	// - BuildOptions.Force is False
 	if opts.CachedChart != "" && !opts.Force {
 		if curMeta, err := LoadChartMetadataFromArchive(opts.CachedChart); err == nil && result.Version == curMeta.Version {
 			result.Path = opts.CachedChart

internal/helm/chart/errors.go

@@ -35,7 +35,7 @@ type BuildError struct {
 	Err    error
 }
 
-// Error returns Err as a string, prefixed with the Reason, if any.
+// Error returns Err as a string, prefixed with the Reason to provide context.
 func (e *BuildError) Error() string {
 	if e.Reason == nil {
 		return e.Err.Error()
@@ -43,7 +43,11 @@ func (e *BuildError) Error() string {
 	return fmt.Sprintf("%s: %s", e.Reason.Error(), e.Err.Error())
 }
 
-// Is returns true of the Reason or Err equals target.
+// Is returns true if the Reason or Err equals target.
+// It can be used to programmatically place an arbitrary Err in the
+// context of the Builder:
+//  err := &BuildError{Reason: ErrChartPull, Err: errors.New("arbitrary transport error")}
+//  errors.Is(err, ErrChartPull)
 func (e *BuildError) Is(target error) bool {
 	if e.Reason != nil && e.Reason == target {
 		return true

internal/helm/chart/metadata.go

@@ -118,7 +118,7 @@ func LoadChartMetadataFromDir(dir string) (*helmchart.Metadata, error) {
 			return nil, fmt.Errorf("'%s' is a directory", stat.Name())
 		}
 		if stat.Size() > helm.MaxChartFileSize {
-			return nil, fmt.Errorf("size of '%s' exceeds '%d' limit", stat.Name(), helm.MaxChartFileSize)
+			return nil, fmt.Errorf("size of '%s' exceeds '%d' bytes limit", stat.Name(), helm.MaxChartFileSize)
 		}
 	}
 
@@ -145,7 +145,7 @@ func LoadChartMetadataFromArchive(archive string) (*helmchart.Metadata, error) {
 		return nil, err
 	}
 	if stat.Size() > helm.MaxChartSize {
-		return nil, fmt.Errorf("size of chart '%s' exceeds '%d' limit", stat.Name(), helm.MaxChartSize)
+		return nil, fmt.Errorf("size of chart '%s' exceeds '%d' bytes limit", stat.Name(), helm.MaxChartSize)
 	}
 
 	f, err := os.Open(archive)

internal/helm/repository/chart_repository.go

@@ -244,7 +244,7 @@ func (r *ChartRepository) LoadFromFile(path string) error {
 		return err
 	}
 	if stat.Size() > helm.MaxIndexSize {
-		return fmt.Errorf("size of index '%s' exceeds '%d' limit", stat.Name(), helm.MaxIndexSize)
+		return fmt.Errorf("size of index '%s' exceeds '%d' bytes limit", stat.Name(), helm.MaxIndexSize)
 	}
 	b, err := os.ReadFile(path)
 	if err != nil {

internal/helm/repository/utils_test.go

@@ -1,3 +1,19 @@
+/*
+Copyright 2021 The Flux 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 repository
 
 import (