Validate provided name for charts from HelmRepos

Following the rules described in
https://helm.sh/docs/chart_best_practices/conventions/#chart-names.

This guards against people following the wrong guidance of Artifactory,
that supports and promotes repository indexes with e.g. '/' in the
chart names.

In a future version this should be moved to a validation webhook, but
there are still discussions ongoing around the TLS certificates for
this.

Signed-off-by: Hidde Beydals <[email protected]>

Author: hiddeco

controllers/helmchart_controller.go

@@ -23,6 +23,7 @@ import (
 	"net/url"
 	"os"
 	"path"
+	"regexp"
 	"strings"
 	"time"
 
@@ -187,7 +188,8 @@ func (r *HelmChartReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 	}
 
 	// Emit an event if we did not have an artifact before, or the revision has changed
-	if chart.Status.Artifact == nil || reconciledChart.Status.Artifact.Revision != chart.Status.Artifact.Revision {
+	if (chart.Status.Artifact == nil && reconciledChart.Status.Artifact != nil) ||
+		reconciledChart.Status.Artifact.Revision != chart.Status.Artifact.Revision {
 		r.event(reconciledChart, events.EventSeverityInfo, sourcev1.HelmChartReadyMessage(reconciledChart))
 	}
 	r.recordReadiness(reconciledChart)
@@ -275,6 +277,12 @@ func (r *HelmChartReconciler) getSource(ctx context.Context, chart sourcev1.Helm
 
 func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
 	repository sourcev1.HelmRepository, chart sourcev1.HelmChart, force bool) (sourcev1.HelmChart, error) {
+	// TODO: move this to a validation webhook once the discussion around
+	//  certificates has settled: https://github.com/fluxcd/image-reflector-controller/issues/69
+	if err := validHelmChartName(chart.Spec.Chart); err != nil {
+		return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), nil
+	}
+
 	// Configure ChartRepository getter options
 	var clientOpts []getter.Option
 	if secret, err := r.getHelmRepositorySecret(ctx, &repository); err != nil {
@@ -877,3 +885,15 @@ func (r *HelmChartReconciler) requestsForBucketChange(obj handler.MapObject) []r
 	}
 	return reqs
 }
+
+// validHelmChartName returns an error if the given string is not a
+// valid Helm chart name; a valid name must be lower case letters
+// and numbers, words may be separated with dashes (-).
+// Ref: https://helm.sh/docs/chart_best_practices/conventions/#chart-names
+func validHelmChartName(s string) error {
+	chartFmt := regexp.MustCompile("^([-a-z0-9]*)$")
+	if !chartFmt.MatchString(s) {
+		return fmt.Errorf("invalid chart name %q, a valid name must be lower case letters and numbers and MAY be seperated with dashes (-)", s)
+	}
+	return nil
+}

controllers/helmchart_controller_test.go

@@ -26,6 +26,7 @@ import (
 	"path"
 	"path/filepath"
 	"strings"
+	"testing"
 	"time"
 
 	"github.com/fluxcd/pkg/apis/meta"
@@ -938,3 +939,26 @@ var _ = Describe("HelmChartReconciler", func() {
 		})
 	})
 })
+
+func Test_validHelmChartName(t *testing.T) {
+	tests := []struct {
+		name      string
+		chart     string
+		expectErr bool
+	}{
+		{"valid", "drupal", false},
+		{"valid dash", "nginx-lego", false},
+		{"valid dashes", "aws-cluster-autoscaler", false},
+		{"valid alphanum", "ng1nx-leg0", false},
+		{"invalid slash", "artifactory/invalid", true},
+		{"invalid dot", "in.valid", true},
+		{"invalid uppercase", "inValid", true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if err := validHelmChartName(tt.chart); (err != nil) != tt.expectErr {
+				t.Errorf("validHelmChartName() error = %v, expectErr %v", err, tt.expectErr)
+			}
+		})
+	}
+}