Merge pull request #168 from replicatedhq/divolgin/sc-87374/for-a-cha…

…rt-rendering-error-don-t-show-any

Don't show missing files if no chart was rendered

Makefile

@@ -50,7 +50,7 @@ build:
 
 .PHONY: test
 test:
-	go test ./pkg/... -tags "$(BUILDTAGS)"
+	go test -v ./pkg/... -tags "$(BUILDTAGS)"
 
 .PHONY: example
 example:

pkg/handlers/builders_lint.go

@@ -29,6 +29,7 @@ func LintBuildersRelease(c *gin.Context) {
 	log.Infof("Received builders lint request with content-length=%s, content-type=%s, client-ip=%s", c.GetHeader("content-length"), c.ContentType(), c.ClientIP())
 
 	specFiles := kots.SpecFiles{}
+	numChartsRendered := 0
 
 	// Include rendering errors in the lint results (even though pedantically they're not lint expressions)
 	var lintExpressions []kots.LintExpression
@@ -62,6 +63,7 @@ func LintBuildersRelease(c *gin.Context) {
 				continue
 			}
 
+			numChartsRendered += 1
 			specFiles = append(specFiles, files...)
 		}
 	} else if c.ContentType() == "application/gzip" {
@@ -75,21 +77,26 @@ func LintBuildersRelease(c *gin.Context) {
 			})
 		}
 
+		numChartsRendered += 1
 		specFiles = append(specFiles, files...)
 	} else {
 		c.JSON(http.StatusBadRequest, gin.H{"error": "content type must be application/gzip or application/tar"})
 		return
 	}
 
-	lint, err := kots.LintBuilders(c.Request.Context(), specFiles)
-	if err != nil {
-		log.Errorf("failed to lint builders charts: %v", err)
-		c.AbortWithError(http.StatusInternalServerError, err)
-		return
+	// Only lint if at least one chart was rendered, otherwise we get missing spec warnings/errors
+	if numChartsRendered > 0 {
+		lint, err := kots.LintBuilders(c.Request.Context(), specFiles)
+		if err != nil {
+			log.Errorf("failed to lint builders charts: %v", err)
+			c.AbortWithError(http.StatusInternalServerError, err)
+			return
+		}
+
+		lintExpressions = append(lintExpressions, lint...)
 	}
 
-	lintExpressions = append(lintExpressions, lint...)
-	response := LintReleaseResponse{}
+	response := LintBuildersReleaseResponse{}
 	response.Body.LintExpressions = lintExpressions
 
 	c.JSON(http.StatusOK, response.Body)

pkg/handlers/builders_lint_test.go

@@ -0,0 +1,164 @@
+package handlers
+
+import (
+	"archive/tar"
+	"embed"
+	"encoding/json"
+	"fmt"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/gin-gonic/gin"
+	"github.com/replicatedhq/kots-lint/pkg/kots"
+	"github.com/stretchr/testify/require"
+)
+
+func init() {
+	kots.InitOPALinting()
+}
+
+//go:embed test-data/*
+var testdata embed.FS
+
+func Test_LintBuildersRelease(t *testing.T) {
+
+	type resultType struct {
+		LintExpressions []kots.LintExpression `json:"lintExpressions"`
+	}
+
+	getTarReader := func(filesNames []string) io.Reader {
+		pipeReader, pipeWriter := io.Pipe()
+
+		go func() {
+			defer pipeWriter.Close()
+
+			tarWriter := tar.NewWriter(pipeWriter)
+			defer tarWriter.Close()
+
+			for _, fileName := range filesNames {
+				data, err := testdata.ReadFile(fmt.Sprintf("test-data/builders/%s", fileName))
+				if err != nil {
+					t.Fatalf("failed to open file: %v", err)
+				}
+
+				header := &tar.Header{
+					Name: fileName,
+					Mode: 0644,
+					Size: int64(len(data)),
+				}
+
+				tarWriter.WriteHeader(header)
+				tarWriter.Write(data)
+			}
+		}()
+
+		return pipeReader
+	}
+
+	tests := []struct {
+		name         string
+		chartReader  func() io.Reader
+		isValidChart bool
+		want         resultType
+		skip         bool
+	}{
+		{
+			skip: false,
+			name: "one valid chart without preflights",
+			chartReader: func() io.Reader {
+				return getTarReader([]string{"testchart-with-labels-16.2.2.tgz"})
+			},
+			isValidChart: true,
+			want: resultType{
+				LintExpressions: []kots.LintExpression{
+					{
+						Rule:      "preflight-spec",
+						Type:      "warn",
+						Message:   "Missing preflight spec",
+						Path:      "",
+						Positions: nil,
+					},
+				},
+			},
+		},
+		{
+			skip: false,
+			name: "one valid chart without preflights and one invalid chart",
+			chartReader: func() io.Reader {
+				return getTarReader([]string{"testchart-with-labels-16.2.2.tgz", "not-a-chart.tgz"})
+			},
+			isValidChart: true,
+			want: resultType{
+				LintExpressions: []kots.LintExpression{
+					{
+						Rule:      "rendering",
+						Type:      "error",
+						Message:   "load chart archive: EOF",
+						Path:      "not-a-chart.tgz",
+						Positions: nil,
+					},
+					{
+						Rule:      "preflight-spec",
+						Type:      "warn",
+						Message:   "Missing preflight spec",
+						Path:      "",
+						Positions: nil,
+					},
+				},
+			},
+		},
+		{
+			skip: false,
+			name: "one invalid chart",
+			chartReader: func() io.Reader {
+				return getTarReader([]string{"not-a-chart.tgz"})
+			},
+			isValidChart: true,
+			want: resultType{
+				LintExpressions: []kots.LintExpression{
+					{
+						Rule:      "rendering",
+						Type:      "error",
+						Message:   "load chart archive: EOF",
+						Path:      "not-a-chart.tgz",
+						Positions: nil,
+					},
+				},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		if tt.skip {
+			continue
+		}
+		t.Run(tt.name, func(t *testing.T) {
+			req := require.New(t)
+
+			clientRequest := &http.Request{
+				Body: io.NopCloser(tt.chartReader()),
+				Header: http.Header{
+					"Content-Type": []string{"application/tar"},
+				},
+			}
+			respWriter := httptest.NewRecorder()
+
+			c, _ := gin.CreateTestContext(respWriter)
+			c.Request = clientRequest
+
+			LintBuildersRelease(c)
+
+			req.Equal(http.StatusOK, respWriter.Result().StatusCode)
+
+			body, err := io.ReadAll(respWriter.Body)
+			req.NoError(err)
+
+			var got resultType
+			err = json.Unmarshal(body, &got)
+			req.NoError(err)
+			req.Equal(tt.want, got)
+		})
+	}
+}