Address review comments

Signed-off-by: Marko Mudrinić <[email protected]>

pkg/cosign/env/env.go

@@ -26,6 +26,7 @@ type Variable string
 
 type VariableOpts struct {
 	Description string
+	Expects     string
 	Sensitive   bool
 }
 
@@ -46,36 +47,55 @@ var (
 	environmentVariables = map[Variable]VariableOpts{
 		VariableExperimental: {
 			Description: "enables experimental cosign features",
+			Expects:     "1 if experimental features should be enabled (0 by default)",
 			Sensitive:   false,
 		},
 		VariableDockerMediaTypes: {
 			Description: "to be used with registries that do not support OCI media types",
+			Expects:     "1 to fallback to legacy OCI media types equivalents (0 by default)",
 			Sensitive:   false,
 		},
 		VariablePassword: {
 			Description: "overrides password inputs with this value",
+			Expects:     "string with a password (asks on stdin by default)",
 			Sensitive:   true,
 		},
 		VariablePKCS11Pin: {
 			Description: "to be used if PKCS11 PIN is not provided",
+			Expects:     "string with a PIN",
 			Sensitive:   true,
 		},
 		VariablePKCS11ModulePath: {
 			Description: "is PKCS11 module-path",
+			Expects:     "string with a module-path",
 			Sensitive:   false,
 		},
 		VariableRepository: {
 			Description: "can be used to store signatures in an alternate location",
+			Expects:     "string with a repository",
 			Sensitive:   false,
 		},
 	}
 )
 
+func mustRegisterEnv(name Variable) {
+	if _, ok := environmentVariables[name]; !ok {
+		panic(fmt.Sprintf("environment variable %q is not registered in pkg/cosign/env", name.String()))
+	}
+	if !strings.HasPrefix(name.String(), "COSIGN_") {
+		panic(fmt.Sprintf("environment varialbe %q must start with COSIGN_ prefix", name.String()))
+	}
+}
+
 func Getenv(name Variable) string {
+	mustRegisterEnv(name)
+
 	return os.Getenv(name.String())
 }
 
 func LookupEnv(name Variable) (string, bool) {
+	mustRegisterEnv(name)
+
 	return os.LookupEnv(name.String())
 }
 
@@ -92,6 +112,7 @@ func PrintEnv(showDescription, showSensitive bool) {
 		// If showDescription is set, print description for that variable
 		if showDescription {
 			fmt.Printf("# %s %s\n", env.String(), opts.Description)
+			fmt.Printf("# Expects: %s\n", opts.Expects)
 		}
 
 		// If variable is sensitive, and we don't want to show sensitive values,

pkg/cosign/env/env_test.go

@@ -73,20 +73,26 @@ const (
 COSIGN_TEST2=""
 `
 	expectedPrintWithDescription = `# COSIGN_TEST1 is the first test variable
+# Expects: test1 value
 COSIGN_TEST1="abcd"
 # COSIGN_TEST2 is the second test variable
+# Expects: test2 value
 COSIGN_TEST2=""
 `
 
 	expectedPrintWithHiddenSensitive = `# COSIGN_TEST1 is the first test variable
+# Expects: test1 value
 COSIGN_TEST1="abcd"
 # COSIGN_TEST2 is the second test variable
+# Expects: test2 value
 COSIGN_TEST2="******"
 `
 
 	expectedPrintWithSensitive = `# COSIGN_TEST1 is the first test variable
+# Expects: test1 value
 COSIGN_TEST1="abcd"
 # COSIGN_TEST2 is the second test variable
+# Expects: test2 value
 COSIGN_TEST2="1234"
 `
 
@@ -96,6 +102,19 @@ COSIGN_TEST2="1234"
 )
 
 func TestPrintEnv(t *testing.T) {
+	variables := map[Variable]VariableOpts{
+		VariableTest1: {
+			Description: "is the first test variable",
+			Expects:     "test1 value",
+			Sensitive:   false,
+		},
+		VariableTest2: {
+			Description: "is the second test variable",
+			Expects:     "test2 value",
+			Sensitive:   true,
+		},
+	}
+
 	tests := []struct {
 		name                 string
 		prepareFn            func()
@@ -110,119 +129,65 @@ func TestPrintEnv(t *testing.T) {
 				os.Setenv("COSIGN_TEST1", "abcd")
 				os.Setenv("COSIGN_TEST2", "")
 			},
-			environmentVariables: map[Variable]VariableOpts{
-				VariableTest1: {
-					Description: "is the first test variable",
-					Sensitive:   false,
-				},
-				VariableTest2: {
-					Description: "is the second test variable",
-					Sensitive:   true,
-				},
-			},
-			showDescriptions:    false,
-			showSensitiveValues: false,
-			expectedOutput:      expectedPrintWithoutDescription,
+			environmentVariables: variables,
+			showDescriptions:     false,
+			showSensitiveValues:  false,
+			expectedOutput:       expectedPrintWithoutDescription,
 		},
 		{
 			name: "descriptions but sensitive variable is unset",
 			prepareFn: func() {
 				os.Setenv("COSIGN_TEST1", "abcd")
 				os.Setenv("COSIGN_TEST2", "")
 			},
-			environmentVariables: map[Variable]VariableOpts{
-				VariableTest1: {
-					Description: "is the first test variable",
-					Sensitive:   false,
-				},
-				VariableTest2: {
-					Description: "is the second test variable",
-					Sensitive:   true,
-				},
-			},
-			showDescriptions:    true,
-			showSensitiveValues: false,
-			expectedOutput:      expectedPrintWithDescription,
+			environmentVariables: variables,
+			showDescriptions:     true,
+			showSensitiveValues:  false,
+			expectedOutput:       expectedPrintWithDescription,
 		},
 		{
 			name: "sensitive variable is non-empty but show sensitive variables is disabled",
 			prepareFn: func() {
 				os.Setenv("COSIGN_TEST1", "abcd")
 				os.Setenv("COSIGN_TEST2", "1234")
 			},
-			environmentVariables: map[Variable]VariableOpts{
-				VariableTest1: {
-					Description: "is the first test variable",
-					Sensitive:   false,
-				},
-				VariableTest2: {
-					Description: "is the second test variable",
-					Sensitive:   true,
-				},
-			},
-			showDescriptions:    true,
-			showSensitiveValues: false,
-			expectedOutput:      expectedPrintWithHiddenSensitive,
+			environmentVariables: variables,
+			showDescriptions:     true,
+			showSensitiveValues:  false,
+			expectedOutput:       expectedPrintWithHiddenSensitive,
 		},
 		{
 			name: "sensitive variable is empty",
 			prepareFn: func() {
 				os.Setenv("COSIGN_TEST1", "abcd")
 				os.Setenv("COSIGN_TEST2", "")
 			},
-			environmentVariables: map[Variable]VariableOpts{
-				VariableTest1: {
-					Description: "is the first test variable",
-					Sensitive:   false,
-				},
-				VariableTest2: {
-					Description: "is the second test variable",
-					Sensitive:   true,
-				},
-			},
-			showDescriptions:    true,
-			showSensitiveValues: true,
-			expectedOutput:      expectedPrintWithDescription,
+			environmentVariables: variables,
+			showDescriptions:     true,
+			showSensitiveValues:  true,
+			expectedOutput:       expectedPrintWithDescription,
 		},
 		{
 			name: "sensitive variable is non-empty and show sensitive variables is enabled",
 			prepareFn: func() {
 				os.Setenv("COSIGN_TEST1", "abcd")
 				os.Setenv("COSIGN_TEST2", "1234")
 			},
-			environmentVariables: map[Variable]VariableOpts{
-				VariableTest1: {
-					Description: "is the first test variable",
-					Sensitive:   false,
-				},
-				VariableTest2: {
-					Description: "is the second test variable",
-					Sensitive:   true,
-				},
-			},
-			showDescriptions:    true,
-			showSensitiveValues: true,
-			expectedOutput:      expectedPrintWithSensitive,
+			environmentVariables: variables,
+			showDescriptions:     true,
+			showSensitiveValues:  true,
+			expectedOutput:       expectedPrintWithSensitive,
 		},
 		{
 			name: "sensitive variable is non-empty but show descriptions is disabled",
 			prepareFn: func() {
 				os.Setenv("COSIGN_TEST1", "abcd")
 				os.Setenv("COSIGN_TEST2", "1234")
 			},
-			environmentVariables: map[Variable]VariableOpts{
-				VariableTest1: {
-					Description: "is the first test variable",
-					Sensitive:   false,
-				},
-				VariableTest2: {
-					Description: "is the second test variable",
-					Sensitive:   true,
-				},
-			},
-			showDescriptions:    false,
-			showSensitiveValues: true,
-			expectedOutput:      expectedPrintSensitiveWithoutDescription,
+			environmentVariables: variables,
+			showDescriptions:     false,
+			showSensitiveValues:  true,
+			expectedOutput:       expectedPrintSensitiveWithoutDescription,
 		},
 	}