refactor workflow inputs to use string/bool vals instead of pointers

Signed-off-by: linus-sun <[email protected]>

cmd/verifier/main.go

@@ -83,7 +83,12 @@ func main() {
 		log.Fatal(err)
 	}
 
-	err = rekor.RunConsistencyCheck(interval, rekorClient, verifier, logInfoFile, monitoredVals, outputIdentitiesFile, once)
+	err = rekor.VerifyConsistencyCheckInputs(interval, logInfoFile, outputIdentitiesFile, once)
+	if err != nil {
+		log.Fatal(err)
+	}
+
+	err = rekor.RunConsistencyCheck(*interval, rekorClient, verifier, *logInfoFile, monitoredVals, *outputIdentitiesFile, *once)
 	if err != nil {
 		log.Fatalf("%v", err)
 	}

pkg/rekor/identity.go

@@ -342,7 +342,7 @@ func oidMatchesPolicy(cert *x509.Certificate, oid asn1.ObjectIdentifier, extensi
 }
 
 // writeIdentitiesBetweenCheckpoints monitors for given identities between two checkpoints and writes any found identities to file.
-func writeIdentitiesBetweenCheckpoints(logInfo *models.LogInfo, prevCheckpoint *util.SignedCheckpoint, checkpoint *util.SignedCheckpoint, monitoredValues identity.MonitoredValues, rekorClient *client.Rekor, outputIdentitiesFile *string) error {
+func writeIdentitiesBetweenCheckpoints(logInfo *models.LogInfo, prevCheckpoint *util.SignedCheckpoint, checkpoint *util.SignedCheckpoint, monitoredValues identity.MonitoredValues, rekorClient *client.Rekor, outputIdentitiesFile string) error {
 	// Get log size of inactive shards
 	totalSize := 0
 	for _, s := range logInfo.InactiveShards {
@@ -366,7 +366,7 @@ func writeIdentitiesBetweenCheckpoints(logInfo *models.LogInfo, prevCheckpoint *
 			for _, idEntry := range idEntries {
 				fmt.Fprintf(os.Stderr, "Found %s\n", idEntry.String())
 
-				if err := file.WriteIdentity(*outputIdentitiesFile, idEntry); err != nil {
+				if err := file.WriteIdentity(outputIdentitiesFile, idEntry); err != nil {
 					return fmt.Errorf("failed to write entry: %v", err)
 				}
 			}

pkg/rekor/verifier.go

@@ -18,6 +18,7 @@ import (
 	"context"
 	"crypto"
 	"encoding/hex"
+	"errors"
 	"fmt"
 	"os"
 	"time"
@@ -65,9 +66,9 @@ func verifyLatestCheckpointSignature(logInfo *models.LogInfo, verifier signature
 
 // verifyCheckpointConsistency reads and verifies the consistency of the previous latest checkpoint from a log info file against the current up-to-date checkpoint.
 // If it successfully fetches and verifies the consistency between these two checkpoints, it returns the previous checkpoint; otherwise, it returns an error.
-func verifyCheckpointConsistency(logInfoFile *string, checkpoint *util.SignedCheckpoint, treeID string, rekorClient *client.Rekor, verifier signature.Verifier) (*util.SignedCheckpoint, error) {
+func verifyCheckpointConsistency(logInfoFile string, checkpoint *util.SignedCheckpoint, treeID string, rekorClient *client.Rekor, verifier signature.Verifier) (*util.SignedCheckpoint, error) {
 	var prevCheckpoint *util.SignedCheckpoint
-	prevCheckpoint, err := file.ReadLatestCheckpoint(*logInfoFile)
+	prevCheckpoint, err := file.ReadLatestCheckpoint(logInfoFile)
 	if err != nil {
 		return nil, fmt.Errorf("reading checkpoint log: %v", err)
 	}
@@ -82,10 +83,26 @@ func verifyCheckpointConsistency(logInfoFile *string, checkpoint *util.SignedChe
 	return prevCheckpoint, nil
 }
 
+// VerifyConsistencyCheckInputs verifies that the input flag values to the rekor-monitor workflow are not nil.
+func VerifyConsistencyCheckInputs(interval *time.Duration, logInfoFile *string, outputIdentitiesFile *string, once *bool) error {
+	if interval == nil {
+		return errors.New("--interval flag equal to nil")
+	}
+	if logInfoFile == nil {
+		return errors.New("--file flag equal to nil")
+	}
+	if outputIdentitiesFile == nil {
+		return errors.New("--output-identities equal to nil")
+	}
+	if once == nil {
+		return errors.New("--once equal to nil")
+	}
+	return nil
+}
+
 // RunConsistencyCheck periodically verifies the root hash consistency of a Rekor log.
-// TODO: RunConsistencyCheck should take in string/bool flags directly instead of pointers and check that flags are being set correctly.
-func RunConsistencyCheck(interval *time.Duration, rekorClient *client.Rekor, verifier signature.Verifier, logInfoFile *string, mvs identity.MonitoredValues, outputIdentitiesFile *string, once *bool) error {
-	ticker := time.NewTicker(*interval)
+func RunConsistencyCheck(interval time.Duration, rekorClient *client.Rekor, verifier signature.Verifier, logInfoFile string, mvs identity.MonitoredValues, outputIdentitiesFile string, once bool) error {
+	ticker := time.NewTicker(interval)
 	defer ticker.Stop()
 
 	// Loop will:
@@ -104,7 +121,7 @@ func RunConsistencyCheck(interval *time.Duration, rekorClient *client.Rekor, ver
 			return fmt.Errorf("failed to verify signature of latest checkpoint: %v", err)
 		}
 
-		fi, err := os.Stat(*logInfoFile)
+		fi, err := os.Stat(logInfoFile)
 		// File containing previous checkpoints exists
 		var prevCheckpoint *util.SignedCheckpoint
 		if err == nil && fi.Size() != 0 {
@@ -117,7 +134,7 @@ func RunConsistencyCheck(interval *time.Duration, rekorClient *client.Rekor, ver
 
 		// Write if there was no stored checkpoint or the sizes differ
 		if prevCheckpoint == nil || prevCheckpoint.Size != checkpoint.Size {
-			if err := file.WriteCheckpoint(checkpoint, *logInfoFile); err != nil {
+			if err := file.WriteCheckpoint(checkpoint, logInfoFile); err != nil {
 				// TODO: Once the consistency check and identity search are split into separate tasks, this should hard fail.
 				// Temporarily skipping this to allow this job to succeed, remediating the issue noted here: https://github.com/sigstore/rekor-monitor/issues/271
 				fmt.Fprintf(os.Stderr, "failed to write checkpoint: %v", err)
@@ -134,11 +151,11 @@ func RunConsistencyCheck(interval *time.Duration, rekorClient *client.Rekor, ver
 		// TODO: Switch to writing checkpoints to GitHub so that the history is preserved. Then we only need
 		// to persist the last checkpoint.
 		// Delete old checkpoints to avoid the log growing indefinitely
-		if err := file.DeleteOldCheckpoints(*logInfoFile); err != nil {
+		if err := file.DeleteOldCheckpoints(logInfoFile); err != nil {
 			return fmt.Errorf("failed to delete old checkpoints: %v", err)
 		}
 
-		if *once {
+		if once {
 			return nil
 		}
 	}

pkg/rekor/verifier_test.go

@@ -19,7 +19,9 @@ import (
 	"crypto/ecdsa"
 	"crypto/elliptic"
 	"crypto/rand"
+	"errors"
 	"testing"
+	"time"
 
 	"github.com/sigstore/rekor-monitor/pkg/rekor/mock"
 	"github.com/sigstore/rekor/pkg/generated/client"
@@ -47,3 +49,72 @@ func TestGetLogVerifier(t *testing.T) {
 		t.Fatalf("expected equal keys: %v", err)
 	}
 }
+
+func TestVerifyConsistencyCheckInputs(t *testing.T) {
+	interval := 5 * time.Minute
+	logInfoFile := "./test/example_log_info_file_path.txt"
+	outputIdentitiesFile := "./test/example_output_identities_file.txt"
+	once := true
+	verifyConsistencyCheckInputTests := map[string]struct {
+		interval             *time.Duration
+		logInfoFile          *string
+		outputIdentitiesFile *string
+		once                 *bool
+		expectedError        error
+	}{
+		"successful verification": {
+			interval:             &interval,
+			logInfoFile:          &logInfoFile,
+			outputIdentitiesFile: &outputIdentitiesFile,
+			once:                 &once,
+			expectedError:        nil,
+		},
+		"fail --interval verification": {
+			interval:             nil,
+			logInfoFile:          &logInfoFile,
+			outputIdentitiesFile: &outputIdentitiesFile,
+			once:                 &once,
+			expectedError:        errors.New("--interval flag equal to nil"),
+		},
+		"fail --file verification": {
+			interval:             &interval,
+			logInfoFile:          nil,
+			outputIdentitiesFile: &outputIdentitiesFile,
+			once:                 &once,
+			expectedError:        errors.New("--file flag equal to nil"),
+		},
+		"fail --output-identities verification": {
+			interval:             &interval,
+			logInfoFile:          &logInfoFile,
+			outputIdentitiesFile: nil,
+			once:                 &once,
+			expectedError:        errors.New("--output-identities flag equal to nil"),
+		},
+		"fail --once verification": {
+			interval:             &interval,
+			logInfoFile:          &logInfoFile,
+			outputIdentitiesFile: &outputIdentitiesFile,
+			once:                 nil,
+			expectedError:        errors.New("--once flag equal to nil"),
+		},
+		"empty case": {
+			interval:             nil,
+			logInfoFile:          nil,
+			outputIdentitiesFile: nil,
+			once:                 nil,
+			expectedError:        errors.New("--interval flag equal to nil"),
+		},
+	}
+
+	for _, verifyConsistencyCheckInputTestCase := range verifyConsistencyCheckInputTests {
+		interval := verifyConsistencyCheckInputTestCase.interval
+		logInfoFile := verifyConsistencyCheckInputTestCase.logInfoFile
+		outputIdentitiesFile := verifyConsistencyCheckInputTestCase.outputIdentitiesFile
+		once := verifyConsistencyCheckInputTestCase.once
+		expectedError := verifyConsistencyCheckInputTestCase.expectedError
+		err := VerifyConsistencyCheckInputs(interval, logInfoFile, outputIdentitiesFile, once)
+		if err != expectedError {
+			t.Errorf("expected error %v, received error %v", expectedError, err)
+		}
+	}
+}