From 57c8576c623c1bee4d093b3a0f7976387bcf8d3f Mon Sep 17 00:00:00 2001 From: ianhundere <138915+ianhundere@users.noreply.github.com> Date: Thu, 12 Dec 2024 12:35:29 -0500 Subject: [PATCH] fix: changes cloudkms flag to gcpkms and makes azure/gcp flags more descriptive. --- cmd/certificate_maker/certificate_maker.go | 15 ++++++-- .../certificate_maker_test.go | 38 +++++++++---------- go.mod | 2 +- go.sum | 4 +- pkg/certmaker/certmaker.go | 11 +++--- pkg/certmaker/certmaker_test.go | 20 +++++----- 6 files changed, 49 insertions(+), 41 deletions(-) diff --git a/cmd/certificate_maker/certificate_maker.go b/cmd/certificate_maker/certificate_maker.go index 696cbaf3..4cae6bd1 100644 --- a/cmd/certificate_maker/certificate_maker.go +++ b/cmd/certificate_maker/certificate_maker.go @@ -84,11 +84,11 @@ func init() { rootCmd.AddCommand(createCmd) - createCmd.Flags().StringVar(&kmsType, "kms-type", "", "KMS provider type (awskms, cloudkms, azurekms)") + createCmd.Flags().StringVar(&kmsType, "kms-type", "", "KMS provider type (awskms, gcpkms, azurekms)") createCmd.Flags().StringVar(&kmsRegion, "kms-region", "", "KMS region") createCmd.Flags().StringVar(&kmsKeyID, "kms-key-id", "", "KMS key identifier") - createCmd.Flags().StringVar(&kmsTenantID, "kms-tenant-id", "", "Azure KMS tenant ID") - createCmd.Flags().StringVar(&kmsCredsFile, "kms-credentials-file", "", "Path to credentials file (for Google Cloud KMS)") + createCmd.Flags().StringVar(&kmsTenantID, "azure-tenant-id", "", "Azure KMS tenant ID") + createCmd.Flags().StringVar(&kmsCredsFile, "gcpkms-credentials-file", "", "Path to credentials file for GCP KMS") createCmd.Flags().StringVar(&rootTemplatePath, "root-template", "pkg/certmaker/templates/root-template.json", "Path to root certificate template") createCmd.Flags().StringVar(&leafTemplatePath, "leaf-template", @@ -118,8 +118,15 @@ func runCreate(_ *cobra.Command, _ []string) error { // Handle KMS provider options switch config.Type { - case "cloudkms": + case "gcpkms": if credsFile := getConfigValue(kmsCredsFile, "KMS_CREDENTIALS_FILE"); credsFile != "" { + // Check if credentials file exists before trying to use it + if _, err := os.Stat(credsFile); err != nil { + if os.IsNotExist(err) { + return fmt.Errorf("failed to initialize KMS: credentials file not found: %s", credsFile) + } + return fmt.Errorf("failed to initialize KMS: error accessing credentials file: %w", err) + } config.Options["credentials-file"] = credsFile } case "azurekms": diff --git a/cmd/certificate_maker/certificate_maker_test.go b/cmd/certificate_maker/certificate_maker_test.go index bb9aceb3..9ae4a91d 100644 --- a/cmd/certificate_maker/certificate_maker_test.go +++ b/cmd/certificate_maker/certificate_maker_test.go @@ -176,15 +176,15 @@ func TestRunCreate(t *testing.T) { { 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", + "--kms-type", "gcpkms", + "--root-key-id", "projects/test-project/locations/global/keyRings/test-ring/cryptoKeys/test-key/cryptoKeyVersions/1", + "--leaf-key-id", "projects/test-project/locations/global/keyRings/test-ring/cryptoKeys/leaf-key/cryptoKeyVersions/1", + "--gcpkms-credentials-file", "/nonexistent/credentials.json", "--root-template", rootTmplPath, "--leaf-template", leafTmplPath, }, wantError: true, - errMsg: "credentials file not found", + errMsg: "failed to initialize KMS: credentials file not found", }, { name: "Azure KMS without tenant ID", @@ -214,20 +214,20 @@ func TestRunCreate(t *testing.T) { } // 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.Flags().StringVar(&kmsType, "kms-type", "", "KMS provider type (awskms, gcpkms, azurekms)") + cmd.Flags().StringVar(&kmsRegion, "kms-region", "", "KMS region") + cmd.Flags().StringVar(&kmsKeyID, "kms-key-id", "", "KMS key identifier") + cmd.Flags().StringVar(&kmsTenantID, "azure-tenant-id", "", "Azure KMS tenant ID") + cmd.Flags().StringVar(&kmsCredsFile, "gcpkms-credentials-file", "", "Path to credentials file for GCP KMS") + cmd.Flags().StringVar(&rootKeyID, "root-key-id", "", "KMS key identifier for root certificate") + cmd.Flags().StringVar(&leafKeyID, "leaf-key-id", "", "KMS key identifier for leaf certificate") + cmd.Flags().StringVar(&rootTemplatePath, "root-template", "", "Path to root certificate template") + cmd.Flags().StringVar(&leafTemplatePath, "leaf-template", "", "Path to leaf certificate template") + cmd.Flags().StringVar(&rootCertPath, "root-cert", "root.pem", "Output path for root certificate") + cmd.Flags().StringVar(&leafCertPath, "leaf-cert", "leaf.pem", "Output path for leaf certificate") + cmd.Flags().StringVar(&intermediateKeyID, "intermediate-key-id", "", "KMS key identifier for intermediate certificate") + cmd.Flags().StringVar(&intermediateTemplate, "intermediate-template", "", "Path to intermediate certificate template") + cmd.Flags().StringVar(&intermediateCert, "intermediate-cert", "intermediate.pem", "Output path for intermediate certificate") cmd.SetArgs(tt.args) err := cmd.Execute() diff --git a/go.mod b/go.mod index fef09a7d..04b8a8ff 100644 --- a/go.mod +++ b/go.mod @@ -48,7 +48,7 @@ require ( cloud.google.com/go/compute/metadata v0.5.2 // indirect cloud.google.com/go/iam v1.2.2 // indirect cloud.google.com/go/kms v1.20.1 // indirect - cloud.google.com/go/longrunning v0.6.1 // indirect + cloud.google.com/go/longrunning v0.6.2 // indirect dario.cat/mergo v1.0.1 // indirect filippo.io/edwards25519 v1.1.0 // indirect github.com/Azure/azure-sdk-for-go/sdk/azcore v1.16.0 // indirect diff --git a/go.sum b/go.sum index f686b40b..6e127993 100644 --- a/go.sum +++ b/go.sum @@ -170,8 +170,6 @@ github.com/golang/groupcache v0.0.0-20200121045136-8c9f03a8e57e/go.mod h1:cIg4er github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da h1:oI5xCqsCo564l8iNU+DwB5epxmsaqB+rhGL0m5jtYqE= github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc= github.com/golang/mock v1.1.1/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfbm0A= -github.com/golang/mock v1.6.0 h1:ErTB+efbowRARo13NNdxyJji2egdxLGQhRaY+DUumQc= -github.com/golang/mock v1.6.0/go.mod h1:p6yTPP+5HYm5mzsMV8JkE6ZKdX+/wYM6Hr+LicevLPs= github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.4.0-rc.1/go.mod h1:ceaxUfeHdC40wWswd/P6IGgMaK3YpKi5j83Wpe3EHw8= @@ -387,6 +385,8 @@ go.step.sm/crypto v0.55.0 h1:575Q7NahuM/ZRxUVN1GkO2e1aDYQJqIIg+nbfOajQJk= go.step.sm/crypto v0.55.0/go.mod h1:MgEmD1lgwsuzZwTgI0GwKapHjKVEQLVggSvHuf3bYnU= go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE= +go.uber.org/mock v0.5.0 h1:KAMbZvZPyBPWgD14IrIQ38QCyjwpvVVV6K/bHl1IwQU= +go.uber.org/mock v0.5.0/go.mod h1:ge71pBPLYDk7QIi1LupWxdAykm7KIEFchiOqd6z7qMM= go.uber.org/multierr v1.10.0 h1:S0h4aNzvfcFsC3dRF1jLoaov7oRaKqRGC/pUEJ2yvPQ= go.uber.org/multierr v1.10.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y= go.uber.org/zap v1.27.0 h1:aJMhYGrd5QSmlpLMr2MftRKl7t8J8PTZPA732ud/XR8= diff --git a/pkg/certmaker/certmaker.go b/pkg/certmaker/certmaker.go index 5448b50a..c8bb4be9 100644 --- a/pkg/certmaker/certmaker.go +++ b/pkg/certmaker/certmaker.go @@ -64,7 +64,8 @@ func InitKMS(ctx context.Context, config KMSConfig) (apiv1.KeyManager, error) { case "awskms": opts.URI = fmt.Sprintf("awskms:///%s?region=%s", keyID, config.Region) return awskms.New(ctx, opts) - case "cloudkms": + case "gcpkms": + opts.Type = apiv1.Type("cloudkms") opts.URI = fmt.Sprintf("cloudkms:%s", keyID) if credFile, ok := config.Options["credentials-file"]; ok { if _, err := os.Stat(credFile); err != nil { @@ -77,7 +78,7 @@ func InitKMS(ctx context.Context, config KMSConfig) (apiv1.KeyManager, error) { } km, err := cloudkms.New(ctx, opts) if err != nil { - return nil, fmt.Errorf("failed to initialize Cloud KMS: %w", err) + return nil, fmt.Errorf("failed to initialize GCP KMS: %w", err) } return km, nil case "azurekms": @@ -242,17 +243,17 @@ func ValidateKMSConfig(config KMSConfig) error { return err } - case "cloudkms": + case "gcpkms": // GCP KMS validation validateGCPKeyID := func(keyID, keyType string) error { if keyID == "" { return nil } if !strings.HasPrefix(keyID, "projects/") { - return fmt.Errorf("cloudkms %s must start with 'projects/'", keyType) + return fmt.Errorf("gcpkms %s must start with 'projects/'", keyType) } if !strings.Contains(keyID, "/locations/") || !strings.Contains(keyID, "/keyRings/") { - return fmt.Errorf("invalid cloudkms key format for %s: %s", keyType, keyID) + return fmt.Errorf("invalid gcpkms key format for %s: %s", keyType, keyID) } return nil } diff --git a/pkg/certmaker/certmaker_test.go b/pkg/certmaker/certmaker_test.go index b6e0eb55..3def594b 100644 --- a/pkg/certmaker/certmaker_test.go +++ b/pkg/certmaker/certmaker_test.go @@ -147,35 +147,35 @@ func TestValidateKMSConfig(t *testing.T) { { name: "GCP KMS invalid root key ID", config: KMSConfig{ - Type: "cloudkms", + Type: "gcpkms", RootKeyID: "invalid-key-id", }, - wantError: "cloudkms RootKeyID must start with 'projects/'", + wantError: "gcpkms RootKeyID must start with 'projects/'", }, { name: "GCP KMS invalid intermediate key ID", config: KMSConfig{ - Type: "cloudkms", + Type: "gcpkms", RootKeyID: "projects/my-project/locations/global/keyRings/my-keyring/cryptoKeys/my-key", IntermediateKeyID: "invalid-key-id", }, - wantError: "cloudkms IntermediateKeyID must start with 'projects/'", + wantError: "gcpkms IntermediateKeyID must start with 'projects/'", }, { name: "GCP KMS invalid leaf key ID", config: KMSConfig{ - Type: "cloudkms", + Type: "gcpkms", LeafKeyID: "invalid-key-id", }, - wantError: "cloudkms LeafKeyID must start with 'projects/'", + wantError: "gcpkms LeafKeyID must start with 'projects/'", }, { name: "GCP KMS missing required parts", config: KMSConfig{ - Type: "cloudkms", + Type: "gcpkms", RootKeyID: "projects/my-project", }, - wantError: "invalid cloudkms key format", + wantError: "invalid gcpkms key format", }, { name: "Azure KMS missing tenant ID", @@ -261,7 +261,7 @@ func TestValidateKMSConfig(t *testing.T) { LeafKeyID: "alias/my-key", }, { - Type: "cloudkms", + Type: "gcpkms", RootKeyID: "projects/my-project/locations/global/keyRings/my-keyring/cryptoKeys/my-key", }, { @@ -636,7 +636,7 @@ func TestInitKMS(t *testing.T) { { name: "GCP KMS", config: KMSConfig{ - Type: "cloudkms", + Type: "gcpkms", RootKeyID: "test-key", Options: map[string]string{ "credentials-file": "/path/to/credentials.json",