✨ fix/enhance pinned-dependencies (#806)

* commit

* e2e tests

* typo

checks/pinned_dependencies.go

@@ -54,11 +54,11 @@ type gitHubActionWorkflowConfig struct {
 
 //nolint:gochecknoinits
 func init() {
-	registerCheck(CheckPinnedDependencies, FrozenDeps)
+	registerCheck(CheckPinnedDependencies, PinnedDependencies)
 }
 
-// FrozenDeps will check the repository if it contains frozen dependecies.
+// PinnedDependencies will check the repository if it contains frozen dependecies.
-func FrozenDeps(c *checker.CheckRequest) checker.CheckResult {
+func PinnedDependencies(c *checker.CheckRequest) checker.CheckResult {
 	// Lock file.
 	lockScore, lockErr := isPackageManagerLockFilePresent(c)
 	if lockErr != nil {
@@ -121,25 +121,63 @@ func maxScore(s1, s2 int) int {
 	return s2
 }
 
-func createReturnValues(r bool, infoMsg string, dl checker.DetailLogger, err error) (int, error) {
+type pinnedResult int
+
+const (
+	pinnedUndefined pinnedResult = iota
+	pinned
+	notPinned
+)
+
+func addPinnedResult(r *pinnedResult, to bool) {
+	// If the result is `notPinned`, we keep it.
+	// In other cases, we always update the result.
+	if *r == notPinned {
+		return
+	}
+
+	switch to {
+	case true:
+		*r = pinned
+	case false:
+		*r = notPinned
+	}
+}
+
+func dataAsResultPointer(data FileCbData) *pinnedResult {
+	pdata, ok := data.(*pinnedResult)
+	if !ok {
+		// This never happens.
+		panic("invalid type")
+	}
+	return pdata
+}
+
+func createReturnValues(r pinnedResult, infoMsg string, dl checker.DetailLogger, err error) (int, error) {
 	if err != nil {
 		return checker.InconclusiveResultScore, err
 	}
-	if !r {
+
+	switch r {
+	default:
+		panic("invalid value")
+	case pinned, pinnedUndefined:
+		dl.Info(infoMsg)
+		return checker.MaxResultScore, nil
+	case notPinned:
+		// No logging needed as it's done by the checks.
 		return checker.MinResultScore, nil
 	}
-
-	dl.Info(infoMsg)
-	return checker.MaxResultScore, nil
 }
 
 func isShellScriptFreeOfInsecureDownloads(c *checker.CheckRequest) (int, error) {
-	var r bool
+	var r pinnedResult
 	err := CheckFilesContent("*", false, c, validateShellScriptIsFreeOfInsecureDownloads, &r)
 	return createReturnForIsShellScriptFreeOfInsecureDownloads(r, c.Dlogger, err)
 }
 
-func createReturnForIsShellScriptFreeOfInsecureDownloads(r bool, dl checker.DetailLogger, err error) (int, error) {
+func createReturnForIsShellScriptFreeOfInsecureDownloads(r pinnedResult,
+	dl checker.DetailLogger, err error) (int, error) {
 	return createReturnValues(r,
 		"no insecure (unpinned) dependency downloads found in shell scripts",
 		dl, err)
@@ -147,18 +185,18 @@ func createReturnForIsShellScriptFreeOfInsecureDownloads(r bool, dl checker.Deta
 
 func testValidateShellScriptIsFreeOfInsecureDownloads(pathfn string,
 	content []byte, dl checker.DetailLogger) (int, error) {
-	var r bool
+	var r pinnedResult
 	_, err := validateShellScriptIsFreeOfInsecureDownloads(pathfn, content, dl, &r)
 	return createReturnForIsShellScriptFreeOfInsecureDownloads(r, dl, err)
 }
 
 func validateShellScriptIsFreeOfInsecureDownloads(pathfn string, content []byte,
 	dl checker.DetailLogger, data FileCbData) (bool, error) {
-	pdata := FileGetCbDataAsBoolPointer(data)
+	pdata := dataAsResultPointer(data)
 
 	// Validate the file type.
 	if !isShellScriptFile(pathfn, content) {
-		*pdata = true
+		addPinnedResult(pdata, true)
 		return true, nil
 	}
 
@@ -166,18 +204,20 @@ func validateShellScriptIsFreeOfInsecureDownloads(pathfn string, content []byte,
 	if err != nil {
 		return false, err
 	}
-	*pdata = r
+
+	addPinnedResult(pdata, r)
 	return true, nil
 }
 
 func isDockerfileFreeOfInsecureDownloads(c *checker.CheckRequest) (int, error) {
-	var r bool
+	var r pinnedResult
 	err := CheckFilesContent("*Dockerfile*", false, c, validateDockerfileIsFreeOfInsecureDownloads, &r)
 	return createReturnForIsDockerfileFreeOfInsecureDownloads(r, c.Dlogger, err)
 }
 
 // Create the result.
-func createReturnForIsDockerfileFreeOfInsecureDownloads(r bool, dl checker.DetailLogger, err error) (int, error) {
+func createReturnForIsDockerfileFreeOfInsecureDownloads(r pinnedResult,
+	dl checker.DetailLogger, err error) (int, error) {
 	return createReturnValues(r,
 		"no insecure (unpinned) dependency downloads found in Dockerfiles",
 		dl, err)
@@ -185,23 +225,23 @@ func createReturnForIsDockerfileFreeOfInsecureDownloads(r bool, dl checker.Detai
 
 func testValidateDockerfileIsFreeOfInsecureDownloads(pathfn string,
 	content []byte, dl checker.DetailLogger) (int, error) {
-	var r bool
+	var r pinnedResult
 	_, err := validateDockerfileIsFreeOfInsecureDownloads(pathfn, content, dl, &r)
 	return createReturnForIsDockerfileFreeOfInsecureDownloads(r, dl, err)
 }
 
 func validateDockerfileIsFreeOfInsecureDownloads(pathfn string, content []byte,
 	dl checker.DetailLogger, data FileCbData) (bool, error) {
-	pdata := FileGetCbDataAsBoolPointer(data)
+	pdata := dataAsResultPointer(data)
 
 	// Return early if this is a script, e.g. script_dockerfile_something.sh
 	if isShellScriptFile(pathfn, content) {
-		*pdata = true
+		addPinnedResult(pdata, true)
 		return true, nil
 	}
 
 	if !CheckFileContainsCommands(content, "#") {
-		*pdata = true
+		addPinnedResult(pdata, true)
 		return true, nil
 	}
 
@@ -244,25 +284,25 @@ func validateDockerfileIsFreeOfInsecureDownloads(pathfn string, content []byte,
 		return false, err
 	}
 
-	*pdata = r
+	addPinnedResult(pdata, r)
 	return true, nil
 }
 
 func isDockerfilePinned(c *checker.CheckRequest) (int, error) {
-	var r bool
+	var r pinnedResult
 	err := CheckFilesContent("*Dockerfile*", false, c, validateDockerfileIsPinned, &r)
 	return createReturnForIsDockerfilePinned(r, c.Dlogger, err)
 }
 
 // Create the result.
-func createReturnForIsDockerfilePinned(r bool, dl checker.DetailLogger, err error) (int, error) {
+func createReturnForIsDockerfilePinned(r pinnedResult, dl checker.DetailLogger, err error) (int, error) {
 	return createReturnValues(r,
 		"Dockerfile dependencies are pinned",
 		dl, err)
 }
 
 func testValidateDockerfileIsPinned(pathfn string, content []byte, dl checker.DetailLogger) (int, error) {
-	var r bool
+	var r pinnedResult
 	_, err := validateDockerfileIsPinned(pathfn, content, dl, &r)
 	return createReturnForIsDockerfilePinned(r, dl, err)
 }
@@ -273,15 +313,15 @@ func validateDockerfileIsPinned(pathfn string, content []byte,
 	// Dockerfile.aarch64, Dockerfile.template, Dockerfile_template, dockerfile, Dockerfile-name.template
 	// Templates may trigger false positives, e.g. FROM { NAME }.
 
-	pdata := FileGetCbDataAsBoolPointer(data)
+	pdata := dataAsResultPointer(data)
 	// Return early if this is a script, e.g. script_dockerfile_something.sh
 	if isShellScriptFile(pathfn, content) {
-		*pdata = true
+		addPinnedResult(pdata, true)
 		return true, nil
 	}
 
 	if !CheckFileContainsCommands(content, "#") {
-		*pdata = true
+		addPinnedResult(pdata, true)
 		return true, nil
 	}
 
@@ -351,18 +391,18 @@ func validateDockerfileIsPinned(pathfn string, content []byte,
 	// The file need not have a FROM statement,
 	// https://github.com/tensorflow/tensorflow/blob/master/tensorflow/tools/dockerfiles/partials/jupyter.partial.Dockerfile.
 
-	*pdata = ret
+	addPinnedResult(pdata, ret)
 	return true, nil
 }
 
 func isGitHubWorkflowScriptFreeOfInsecureDownloads(c *checker.CheckRequest) (int, error) {
-	var r bool
+	var r pinnedResult
 	err := CheckFilesContent(".github/workflows/*", false, c, validateGitHubWorkflowIsFreeOfInsecureDownloads, &r)
 	return createReturnForIsGitHubWorkflowScriptFreeOfInsecureDownloads(r, c.Dlogger, err)
 }
 
 // Create the result.
-func createReturnForIsGitHubWorkflowScriptFreeOfInsecureDownloads(r bool,
+func createReturnForIsGitHubWorkflowScriptFreeOfInsecureDownloads(r pinnedResult,
 	dl checker.DetailLogger, err error) (int, error) {
 	return createReturnValues(r,
 		"no insecure (unpinned) dependency downloads found in GitHub workflows",
@@ -371,17 +411,17 @@ func createReturnForIsGitHubWorkflowScriptFreeOfInsecureDownloads(r bool,
 
 func testValidateGitHubWorkflowScriptFreeOfInsecureDownloads(pathfn string,
 	content []byte, dl checker.DetailLogger) (int, error) {
-	var r bool
+	var r pinnedResult
 	_, err := validateGitHubWorkflowIsFreeOfInsecureDownloads(pathfn, content, dl, &r)
 	return createReturnForIsGitHubWorkflowScriptFreeOfInsecureDownloads(r, dl, err)
 }
 
 func validateGitHubWorkflowIsFreeOfInsecureDownloads(pathfn string, content []byte,
 	dl checker.DetailLogger, data FileCbData) (bool, error) {
-	pdata := FileGetCbDataAsBoolPointer(data)
+	pdata := dataAsResultPointer(data)
 
 	if !CheckFileContainsCommands(content, "#") {
-		*pdata = true
+		addPinnedResult(pdata, true)
 		return true, nil
 	}
 
@@ -433,26 +473,26 @@ func validateGitHubWorkflowIsFreeOfInsecureDownloads(pathfn string, content []by
 		}
 	}
 
-	*pdata = validated
+	addPinnedResult(pdata, validated)
 	return true, nil
 }
 
 // Check pinning of github actions in workflows.
 func isGitHubActionsWorkflowPinned(c *checker.CheckRequest) (int, error) {
-	var r bool
+	var r pinnedResult
 	err := CheckFilesContent(".github/workflows/*", true, c, validateGitHubActionWorkflow, &r)
 	return createReturnForIsGitHubActionsWorkflowPinned(r, c.Dlogger, err)
 }
 
 // Create the result.
-func createReturnForIsGitHubActionsWorkflowPinned(r bool, dl checker.DetailLogger, err error) (int, error) {
+func createReturnForIsGitHubActionsWorkflowPinned(r pinnedResult, dl checker.DetailLogger, err error) (int, error) {
 	return createReturnValues(r,
 		"GitHub actions are pinned",
 		dl, err)
 }
 
 func testIsGitHubActionsWorkflowPinned(pathfn string, content []byte, dl checker.DetailLogger) (int, error) {
-	var r bool
+	var r pinnedResult
 	_, err := validateGitHubActionWorkflow(pathfn, content, dl, &r)
 	return createReturnForIsGitHubActionsWorkflowPinned(r, dl, err)
 }
@@ -460,10 +500,10 @@ func testIsGitHubActionsWorkflowPinned(pathfn string, content []byte, dl checker
 // Check file content.
 func validateGitHubActionWorkflow(pathfn string, content []byte,
 	dl checker.DetailLogger, data FileCbData) (bool, error) {
-	pdata := FileGetCbDataAsBoolPointer(data)
+	pdata := dataAsResultPointer(data)
 
 	if !CheckFileContainsCommands(content, "#") {
-		*pdata = true
+		addPinnedResult(pdata, true)
 		return true, nil
 	}
 
@@ -494,18 +534,18 @@ func validateGitHubActionWorkflow(pathfn string, content []byte,
 		}
 	}
 
-	*pdata = ret
+	addPinnedResult(pdata, ret)
 	return true, nil
 }
 
 // Check presence of lock files thru validatePackageManagerFile().
 func isPackageManagerLockFilePresent(c *checker.CheckRequest) (int, error) {
-	var r bool
+	var r pinnedResult
 	err := CheckIfFileExists(CheckPinnedDependencies, c, validatePackageManagerFile, &r)
 	if err != nil {
 		return checker.InconclusiveResultScore, err
 	}
-	if !r {
+	if r != pinned {
 		c.Dlogger.Warn("no lock files detected for a package manager")
 		return checker.InconclusiveResultScore, nil
 	}
@@ -541,7 +581,8 @@ func validatePackageManagerFile(name string, dl checker.DetailLogger, data FileC
 	case "composer.lock":
 		dl.Info("composer lock file detected: %s", name)
 	}
-	pdata := FileGetCbDataAsBoolPointer(data)
+
-	*pdata = true
+	pdata := dataAsResultPointer(data)
-	return true, nil
+	addPinnedResult(pdata, true)
+	return false, nil
 }

e2e/pinned_dependencies_test.go

@@ -11,7 +11,7 @@
 // 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.
-
+//nolint:dupl
 package e2e
 
 import (
@@ -48,12 +48,12 @@ var _ = Describe("E2E TEST:"+checks.CheckPinnedDependencies, func() {
 			}
 			expected := scut.TestReturn{
 				Errors:        nil,
-				Score:         3,
+				Score:         checker.MinResultScore,
-				NumberOfWarn:  149,
+				NumberOfWarn:  154,
-				NumberOfInfo:  2,
+				NumberOfInfo:  0,
 				NumberOfDebug: 0,
 			}
-			result := checks.FrozenDeps(&req)
+			result := checks.PinnedDependencies(&req)
 			// UPGRADEv2: to remove.
 			// Old version.
 			Expect(result.Error).Should(BeNil())