fix: adds further test coverage.

Signed-off-by: ianhundere <[email protected]>

cmd/certificate_maker/certificate_maker_test.go

@@ -0,0 +1,290 @@
+// Copyright 2024 The Sigstore Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// 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.
+//
+
+package main
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/spf13/cobra"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestGetConfigValue(t *testing.T) {
+	tests := []struct {
+		name      string
+		flagValue string
+		envVar    string
+		envValue  string
+		want      string
+	}{
+		{
+			name:      "flag value takes precedence",
+			flagValue: "flag-value",
+			envVar:    "TEST_ENV",
+			envValue:  "env-value",
+			want:      "flag-value",
+		},
+		{
+			name:      "env value used when flag empty",
+			flagValue: "",
+			envVar:    "TEST_ENV",
+			envValue:  "env-value",
+			want:      "env-value",
+		},
+		{
+			name:      "empty when both unset",
+			flagValue: "",
+			envVar:    "TEST_ENV",
+			envValue:  "",
+			want:      "",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if tt.envValue != "" {
+				os.Setenv(tt.envVar, tt.envValue)
+				defer os.Unsetenv(tt.envVar)
+			}
+			got := getConfigValue(tt.flagValue, tt.envVar)
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}
+
+func TestInitLogger(t *testing.T) {
+	logger := initLogger()
+	require.NotNil(t, logger)
+}
+
+func TestRunCreate(t *testing.T) {
+	tmpDir, err := os.MkdirTemp("", "cert-test-*")
+	require.NoError(t, err)
+	defer os.RemoveAll(tmpDir)
+
+	// Create test template files
+	rootTemplate := `{
+		"subject": {
+			"commonName": "Test Root CA"
+		},
+		"issuer": {
+			"commonName": "Test Root CA"
+		},
+		"notBefore": "2024-01-01T00:00:00Z",
+		"notAfter": "2025-01-01T00:00:00Z",
+		"keyUsage": ["certSign", "crlSign"],
+		"basicConstraints": {
+			"isCA": true,
+			"maxPathLen": 1
+		}
+	}`
+
+	leafTemplate := `{
+		"subject": {
+			"commonName": "Test Leaf"
+		},
+		"notBefore": "2024-01-01T00:00:00Z",
+		"notAfter": "2025-01-01T00:00:00Z",
+		"keyUsage": ["digitalSignature"],
+		"extKeyUsage": ["CodeSigning"],
+		"basicConstraints": {
+			"isCA": false
+		}
+	}`
+
+	rootTmplPath := filepath.Join(tmpDir, "root-template.json")
+	leafTmplPath := filepath.Join(tmpDir, "leaf-template.json")
+	err = os.WriteFile(rootTmplPath, []byte(rootTemplate), 0600)
+	require.NoError(t, err)
+	err = os.WriteFile(leafTmplPath, []byte(leafTemplate), 0600)
+	require.NoError(t, err)
+
+	tests := []struct {
+		name      string
+		args      []string
+		envVars   map[string]string
+		wantError bool
+		errMsg    string
+	}{
+		{
+			name: "missing KMS type",
+			args: []string{
+				"--kms-region", "us-west-2",
+				"--root-key-id", "test-root-key",
+				"--leaf-key-id", "test-leaf-key",
+				"--root-template", rootTmplPath,
+				"--leaf-template", leafTmplPath,
+			},
+			wantError: true,
+			errMsg:    "KMS type cannot be empty",
+		},
+		{
+			name: "invalid KMS type",
+			args: []string{
+				"--kms-type", "invalid",
+				"--kms-region", "us-west-2",
+				"--root-key-id", "arn:aws:kms:us-west-2:123456789012:key/test-key",
+				"--leaf-key-id", "arn:aws:kms:us-west-2:123456789012:key/test-key",
+				"--root-template", rootTmplPath,
+				"--leaf-template", leafTmplPath,
+			},
+			wantError: true,
+			errMsg:    "unsupported KMS type",
+		},
+		{
+			name: "missing root template",
+			args: []string{
+				"--kms-type", "awskms",
+				"--kms-region", "us-west-2",
+				"--root-key-id", "arn:aws:kms:us-west-2:123456789012:key/test-key",
+				"--leaf-key-id", "arn:aws:kms:us-west-2:123456789012:key/test-key",
+				"--root-template", "nonexistent.json",
+				"--leaf-template", leafTmplPath,
+			},
+			wantError: true,
+			errMsg:    "template not found",
+		},
+		{
+			name: "missing leaf template",
+			args: []string{
+				"--kms-type", "awskms",
+				"--kms-region", "us-west-2",
+				"--root-key-id", "arn:aws:kms:us-west-2:123456789012:key/test-key",
+				"--leaf-key-id", "arn:aws:kms:us-west-2:123456789012:key/test-key",
+				"--root-template", rootTmplPath,
+				"--leaf-template", "nonexistent.json",
+			},
+			wantError: true,
+			errMsg:    "template not found",
+		},
+		{
+			name: "GCP KMS with credentials file",
+			args: []string{
+				"--kms-type", "cloudkms",
+				"--root-key-id", "projects/test-project/locations/global/keyRings/test-ring/cryptoKeys/test-key",
+				"--leaf-key-id", "projects/test-project/locations/global/keyRings/test-ring/cryptoKeys/leaf-key",
+				"--kms-credentials-file", "/nonexistent/credentials.json",
+				"--root-template", rootTmplPath,
+				"--leaf-template", leafTmplPath,
+			},
+			wantError: true,
+			errMsg:    "credentials file not found",
+		},
+		{
+			name: "Azure KMS without tenant ID",
+			args: []string{
+				"--kms-type", "azurekms",
+				"--root-key-id", "azurekms:name=test-key;vault=test-vault",
+				"--leaf-key-id", "azurekms:name=leaf-key;vault=test-vault",
+				"--root-template", rootTmplPath,
+				"--leaf-template", leafTmplPath,
+			},
+			wantError: true,
+			errMsg:    "tenant-id is required",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Set environment variables
+			for k, v := range tt.envVars {
+				os.Setenv(k, v)
+				defer os.Unsetenv(k)
+			}
+
+			cmd := &cobra.Command{
+				Use:  "test",
+				RunE: runCreate,
+			}
+
+			// Add all flags that runCreate expects
+			cmd.Flags().StringVar(&kmsType, "kms-type", "", "")
+			cmd.Flags().StringVar(&kmsRegion, "kms-region", "", "")
+			cmd.Flags().StringVar(&kmsKeyID, "kms-key-id", "", "")
+			cmd.Flags().StringVar(&kmsTenantID, "kms-tenant-id", "", "")
+			cmd.Flags().StringVar(&kmsCredsFile, "kms-credentials-file", "", "")
+			cmd.Flags().StringVar(&rootTemplatePath, "root-template", "", "")
+			cmd.Flags().StringVar(&leafTemplatePath, "leaf-template", "", "")
+			cmd.Flags().StringVar(&rootKeyID, "root-key-id", "", "")
+			cmd.Flags().StringVar(&leafKeyID, "leaf-key-id", "", "")
+			cmd.Flags().StringVar(&rootCertPath, "root-cert", "root.pem", "")
+			cmd.Flags().StringVar(&leafCertPath, "leaf-cert", "leaf.pem", "")
+			cmd.Flags().StringVar(&intermediateKeyID, "intermediate-key-id", "", "")
+			cmd.Flags().StringVar(&intermediateTemplate, "intermediate-template", "", "")
+			cmd.Flags().StringVar(&intermediateCert, "intermediate-cert", "", "")
+
+			cmd.SetArgs(tt.args)
+			err := cmd.Execute()
+
+			if tt.wantError {
+				require.Error(t, err)
+				assert.Contains(t, err.Error(), tt.errMsg)
+			} else {
+				require.NoError(t, err)
+			}
+		})
+	}
+}
+
+func TestCreateCommand(t *testing.T) {
+	// Create a test command
+	cmd := &cobra.Command{
+		Use: "test",
+		RunE: func(cmd *cobra.Command, args []string) error {
+			return nil
+		},
+	}
+
+	// Add flags
+	cmd.Flags().StringVar(&kmsType, "kms-type", "", "KMS type")
+	cmd.Flags().StringVar(&kmsRegion, "kms-region", "", "KMS region")
+	cmd.Flags().StringVar(&rootKeyID, "root-key-id", "", "Root key ID")
+	cmd.Flags().StringVar(&leafKeyID, "leaf-key-id", "", "Leaf key ID")
+
+	// Test missing required flags
+	err := cmd.Execute()
+	require.NoError(t, err) // No required flags set yet
+
+	// Test flag parsing
+	err = cmd.ParseFlags([]string{
+		"--kms-type", "awskms",
+		"--kms-region", "us-west-2",
+		"--root-key-id", "arn:aws:kms:us-west-2:123456789012:key/1234abcd-12ab-34cd-56ef-1234567890ab",
+		"--leaf-key-id", "arn:aws:kms:us-west-2:123456789012:key/9876fedc-ba98-7654-3210-fedcba987654",
+	})
+	require.NoError(t, err)
+
+	// Verify flag values
+	assert.Equal(t, "awskms", kmsType)
+	assert.Equal(t, "us-west-2", kmsRegion)
+	assert.Equal(t, "arn:aws:kms:us-west-2:123456789012:key/1234abcd-12ab-34cd-56ef-1234567890ab", rootKeyID)
+	assert.Equal(t, "arn:aws:kms:us-west-2:123456789012:key/9876fedc-ba98-7654-3210-fedcba987654", leafKeyID)
+}
+
+func TestRootCommand(t *testing.T) {
+	// Test help output
+	rootCmd.SetArgs([]string{"--help"})
+	err := rootCmd.Execute()
+	require.NoError(t, err)
+
+	// Test unknown command
+	rootCmd.SetArgs([]string{"unknown"})
+	err = rootCmd.Execute()
+	require.Error(t, err)
+}

pkg/certmaker/certmaker.go

@@ -69,10 +69,7 @@ func InitKMS(ctx context.Context, config KMSConfig) (apiv1.KeyManager, error) {
 		opts.URI = fmt.Sprintf("cloudkms:%s", keyID)
 		if credFile, ok := config.Options["credentials-file"]; ok {
 			if _, err := os.Stat(credFile); err != nil {
-				if os.IsNotExist(err) {
-					return nil, fmt.Errorf("credentials file not found: %s", credFile)
-				}
-				return nil, fmt.Errorf("error accessing credentials file: %w", err)
+				return nil, fmt.Errorf("failed to initialize Cloud KMS: credentials file not found: %s", credFile)
 			}
 			opts.URI += fmt.Sprintf("?credentials-file=%s", credFile)
 		}
@@ -269,6 +266,9 @@ func ValidateKMSConfig(config KMSConfig) error {
 
 	case "azurekms":
 		// Azure KMS validation
+		if config.Options == nil {
+			return fmt.Errorf("options map is required for Azure KMS")
+		}
 		if config.Options["tenant-id"] == "" {
 			return fmt.Errorf("tenant-id is required for Azure KMS")
 		}
@@ -281,7 +281,7 @@ func ValidateKMSConfig(config KMSConfig) error {
 				return fmt.Errorf("azurekms %s must start with 'azurekms:name='", keyType)
 			}
 			if !strings.Contains(keyID, ";vault=") {
-				return fmt.Errorf("azurekms %s must contain ';vault=' parameter", keyType)
+				return fmt.Errorf("vault name is required for Azure Key Vault")
 			}
 			return nil
 		}