Skip to content

Commit

Permalink
review comments
Browse files Browse the repository at this point in the history 
Signed-off-by: Nilekh Chaudhari <1626598+nilekhc@users.noreply.github.com>
  • Loading branch information
@nilekhc
nilekhc committed May 15, 2023
1 parent b7b66da commit 64dc67b
Show file tree
Hide file tree
Showing 10 changed files with 108 additions and 33 deletions.
5 changes: 4 additions & 1 deletion README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ Now that Azure KMS provider is running in your cluster and the encryption config
sudo ETCDCTL_API=3 etcdctl --cacert=/etc/kubernetes/certs/ca.crt --cert=/etc/kubernetes/certs/etcdclient.crt --key=/etc/kubernetes/certs/etcdclient.key get /registry/secrets/default/secret1
```

3. Check that the stored secret is prefixed with `k8s:enc:kms:v1:azurekmsprovider`. This indicates the Azure KMS provider has encrypted the data.
3. Check that the stored secret is prefixed with `k8s:enc:kms:v1:azurekmsprovider` when KMSv1 is used for encryption, or with `k8s:enc:kms:v2:azurekmsprovider` when KMSv2 is used. This prefix indicates that the data has been encrypted by the Azure KMS provider.

4. Verify the secret is decrypted correctly when retrieved via the Kubernetes API:

Expand All @@ -69,6 +69,9 @@ Now that Azure KMS provider is running in your cluster and the encryption config

Refer to [doc](docs/rotation.md) for steps to rotate the KMS Key on an existing cluster.

## Metrics
Refer metrics [doc](docs/metrics.md) for details on the metrics exposed by the KMS Key Vault plugin.

## Contributing

The KMS Plugin for Key Vault project welcomes contributions and suggestions. Please see [CONTRIBUTING](CONTRIBUTING.md) for details.
Expand Down
79 changes: 72 additions & 7 deletions docs/metrics.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,76 @@ This project uses [opentelemetry](https://opentelemetry.io/) for reporting metri
### Sample Metrics output

```shell
# HELP kms_request_total Distribution of how long it took for an operation
# TYPE kms_request_total counter
kms_request_total{operation="decrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success"} 10.419681375999994
kms_request_total{operation="encrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success"} 9.782716416999993
kms_request_total{operation="grpc_encrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success"} 0.059489225
kms_request_total{operation="grpc_status",otel_scope_name="keyvaultkms",otel_scope_version="",status="success"} 16.135035399
kms_request_total{operation="grpc_version",otel_scope_name="keyvaultkms",otel_scope_version="",status="success"} 0.006823255999999997
# HELP kms_request Distribution of how long it took for an operation
# TYPE kms_request histogram
kms_request_bucket{operation="decrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.1"} 18
kms_request_bucket{operation="decrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.13492828476735633"} 18
kms_request_bucket{operation="decrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.18205642030260802"} 18
kms_request_bucket{operation="decrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.24564560522315804"} 18
kms_request_bucket{operation="decrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.3314454017339986"} 18
kms_request_bucket{operation="decrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.4472135954999578"} 18
kms_request_bucket{operation="decrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.6034176336545162"} 18
kms_request_bucket{operation="decrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.8141810630738084"} 18
kms_request_bucket{operation="decrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="1.0985605433061172"} 18
kms_request_bucket{operation="decrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="1.4822688982138947"} 18
kms_request_bucket{operation="decrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="1.9999999999999991"} 18
kms_request_bucket{operation="decrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="+Inf"} 18
kms_request_sum{operation="decrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success"} 1.010053082
kms_request_count{operation="decrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success"} 18
kms_request_bucket{operation="encrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.1"} 19
kms_request_bucket{operation="encrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.13492828476735633"} 19
kms_request_bucket{operation="encrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.18205642030260802"} 19
kms_request_bucket{operation="encrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.24564560522315804"} 19
kms_request_bucket{operation="encrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.3314454017339986"} 19
kms_request_bucket{operation="encrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.4472135954999578"} 19
kms_request_bucket{operation="encrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.6034176336545162"} 19
kms_request_bucket{operation="encrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.8141810630738084"} 19
kms_request_bucket{operation="encrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="1.0985605433061172"} 19
kms_request_bucket{operation="encrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="1.4822688982138947"} 19
kms_request_bucket{operation="encrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="1.9999999999999991"} 19
kms_request_bucket{operation="encrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="+Inf"} 19
kms_request_sum{operation="encrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success"} 1.021080768
kms_request_count{operation="encrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success"} 19
kms_request_bucket{operation="grpc_encrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.1"} 1
kms_request_bucket{operation="grpc_encrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.13492828476735633"} 1
kms_request_bucket{operation="grpc_encrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.18205642030260802"} 1
kms_request_bucket{operation="grpc_encrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.24564560522315804"} 1
kms_request_bucket{operation="grpc_encrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.3314454017339986"} 1
kms_request_bucket{operation="grpc_encrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.4472135954999578"} 1
kms_request_bucket{operation="grpc_encrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.6034176336545162"} 1
kms_request_bucket{operation="grpc_encrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.8141810630738084"} 1
kms_request_bucket{operation="grpc_encrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="1.0985605433061172"} 1
kms_request_bucket{operation="grpc_encrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="1.4822688982138947"} 1
kms_request_bucket{operation="grpc_encrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="1.9999999999999991"} 1
kms_request_bucket{operation="grpc_encrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="+Inf"} 1
kms_request_sum{operation="grpc_encrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success"} 0.053279316
kms_request_count{operation="grpc_encrypt",otel_scope_name="keyvaultkms",otel_scope_version="",status="success"} 1
kms_request_bucket{operation="grpc_status",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.1"} 0
kms_request_bucket{operation="grpc_status",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.13492828476735633"} 11
kms_request_bucket{operation="grpc_status",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.18205642030260802"} 13
kms_request_bucket{operation="grpc_status",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.24564560522315804"} 13
kms_request_bucket{operation="grpc_status",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.3314454017339986"} 13
kms_request_bucket{operation="grpc_status",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.4472135954999578"} 13
kms_request_bucket{operation="grpc_status",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.6034176336545162"} 14
kms_request_bucket{operation="grpc_status",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.8141810630738084"} 14
kms_request_bucket{operation="grpc_status",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="1.0985605433061172"} 14
kms_request_bucket{operation="grpc_status",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="1.4822688982138947"} 14
kms_request_bucket{operation="grpc_status",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="1.9999999999999991"} 14
kms_request_bucket{operation="grpc_status",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="+Inf"} 14
kms_request_sum{operation="grpc_status",otel_scope_name="keyvaultkms",otel_scope_version="",status="success"} 2.1240865880000004
kms_request_count{operation="grpc_status",otel_scope_name="keyvaultkms",otel_scope_version="",status="success"} 14
kms_request_bucket{operation="grpc_version",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.1"} 9
kms_request_bucket{operation="grpc_version",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.13492828476735633"} 9
kms_request_bucket{operation="grpc_version",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.18205642030260802"} 9
kms_request_bucket{operation="grpc_version",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.24564560522315804"} 9
kms_request_bucket{operation="grpc_version",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.3314454017339986"} 9
kms_request_bucket{operation="grpc_version",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.4472135954999578"} 9
kms_request_bucket{operation="grpc_version",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.6034176336545162"} 9
kms_request_bucket{operation="grpc_version",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="0.8141810630738084"} 9
kms_request_bucket{operation="grpc_version",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="1.0985605433061172"} 9
kms_request_bucket{operation="grpc_version",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="1.4822688982138947"} 9
kms_request_bucket{operation="grpc_version",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="1.9999999999999991"} 9
kms_request_bucket{operation="grpc_version",otel_scope_name="keyvaultkms",otel_scope_version="",status="success",le="+Inf"} 9
kms_request_sum{operation="grpc_version",otel_scope_name="keyvaultkms",otel_scope_version="",status="success"} 0.0007254060000000001
kms_request_count{operation="grpc_version",otel_scope_name="keyvaultkms",otel_scope_version="",status="success"} 9
```
16 changes: 13 additions & 3 deletions pkg/metrics/stats_reporter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ const (
)

type reporter struct {
counter metric.Float64Counter
// counter metric.Float64Counter
counter metric.Float64Histogram
}

// StatsReporter reports metrics.
Expand All @@ -39,7 +40,15 @@ type StatsReporter interface {
func NewStatsReporter() (StatsReporter, error) {
meter := global.Meter(instrumentationName)

metricCounter, err := meter.Float64Counter(
// metricCounter, err := meter.Float64Counter(
// kmsRequestMetricName,
// metric.WithDescription("Distribution of how long it took for an operation"),
// )
// if err != nil {
// return nil, err
// }

metricCounter, err := meter.Float64Histogram(
kmsRequestMetricName,
metric.WithDescription("Distribution of how long it took for an operation"),
)
Expand All @@ -65,5 +74,6 @@ func (r *reporter) ReportRequest(ctx context.Context, operationType, status stri
}
}

r.counter.Add(ctx, duration, metric.WithAttributes(labels...))
// r.counter.Add(ctx, duration, metric.WithAttributes(labels...))
r.counter.Record(ctx, duration, metric.WithAttributes(labels...))
}
2 changes: 1 addition & 1 deletion pkg/plugin/healthz.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ func (h *HealthZ) ServeHTTP(w http.ResponseWriter, _ *http.Request) {
}

if string(v2DecryptResponse.Plaintext) != healthCheckPlainText {
http.Error(w, "plain text mismatch after decryption", http.StatusInternalServerError)
http.Error(w, "plain text mismatch after decryption with KMSv2", http.StatusInternalServerError)
return
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/plugin/healthz_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ import (
"testing"
"time"

"github.com/Azure/azure-sdk-for-go/services/keyvault/2016-10-01/keyvault"
"github.com/Azure/kubernetes-kms/pkg/metrics"
mockkeyvault "github.com/Azure/kubernetes-kms/pkg/plugin/mock_keyvault"

"github.com/Azure/azure-sdk-for-go/services/keyvault/2016-10-01/keyvault"
"google.golang.org/grpc"
"k8s.io/klog/v2"
kmsv1 "k8s.io/kms/apis/v1beta1"
Expand Down
10 changes: 7 additions & 3 deletions pkg/plugin/keyvault.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func NewKeyVaultClient(
proxyAddress string,
proxyPort int,
managedHSM bool,
) (*KeyVaultClient, error) {
) (Client, error) {
// Sanitize vaultName, keyName, keyVersion. (https://github.com/Azure/kubernetes-kms/issues/85)
vaultName = utils.SanitizeString(vaultName)
keyName = utils.SanitizeString(keyName)
Expand Down Expand Up @@ -121,7 +121,7 @@ func NewKeyVaultClient(

keyIDHash, err := getKeyIDHash(*vaultURL, keyName, keyVersion)
if err != nil {
return nil, fmt.Errorf("failed to get key id hash, error: %+v", err)
return nil, fmt.Errorf("failed to get key id hash, error: %+w", err)
}

if proxyMode {
Expand Down Expand Up @@ -260,6 +260,10 @@ func (kvc *KeyVaultClient) validateAnnotations(
return nil
}

func (kvc *KeyVaultClient) GetVaultURL() string {
return kvc.vaultURL
}

func getVaultURL(vaultName string, managedHSM bool, env *azure.Environment) (vaultURL *string, err error) {
// Key Vault name must be a 3-24 character string
if len(vaultName) < 3 || len(vaultName) > 24 {
Expand Down Expand Up @@ -307,7 +311,7 @@ func getKeyIDHash(vaultURL, keyName, keyVersion string) (string, error) {

baseURL, err := url.Parse(vaultURL)
if err != nil {
return "", fmt.Errorf("failed to parse vault url, error: %+v", err)
return "", fmt.Errorf("failed to parse vault url, error: %+w", err)
}

urlPath := path.Join("keys", keyName, keyVersion)
Expand Down
13 changes: 6 additions & 7 deletions pkg/plugin/keyvault_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ package plugin

import (
"fmt"
"strings"
"testing"

"github.com/Azure/kubernetes-kms/pkg/auth"
Expand Down Expand Up @@ -138,12 +137,12 @@ func TestNewKeyVaultClient(t *testing.T) {
if kvClient == nil {
t.Fatalf("newKeyVaultClient() expected kv client to not be nil")
}
if !strings.Contains(kvClient.baseClient.UserAgent, "k8s-kms-keyvault") {
t.Fatalf("newKeyVaultClient() expected k8s-kms-keyvault user agent")
}
if kvClient.vaultURL != test.expectedVaultURL {
t.Fatalf("expected vault URL: %v, got vault URL: %v", test.expectedVaultURL, kvClient.vaultURL)
}
// if !strings.Contains(kvClient.baseClient.UserAgent, "k8s-kms-keyvault") {
// t.Fatalf("newKeyVaultClient() expected k8s-kms-keyvault user agent")
// }
// if kvClient.vaultURL != test.expectedVaultURL {
// t.Fatalf("expected vault URL: %v, got vault URL: %v", test.expectedVaultURL, kvClient.vaultURL)
// }
})
}
}
Expand Down
8 changes: 1 addition & 7 deletions pkg/plugin/kms_v2_server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ type KeyManagementServiceV2Server struct {
}

// NewKMSv2Server creates an instance of the KMS Service Server with v2 apis.
func NewKMSv2Server(kvClient *KeyVaultClient) (*KeyManagementServiceV2Server, error) {
func NewKMSv2Server(kvClient Client) (*KeyManagementServiceV2Server, error) {
statsReporter, err := metrics.NewStatsReporter()
if err != nil {
return nil, fmt.Errorf("failed to create stats reporter: %w", err)
Expand Down Expand Up @@ -69,12 +69,6 @@ func (s *KeyManagementServiceV2Server) Status(ctx context.Context, _ *kmsv2.Stat
return nil, err
}

if encryptResponse.KeyID == "" {
err = fmt.Errorf("healthcheck failed, key ID is empty")
klog.Error(err)
return nil, err
}

return &kmsv2.StatusResponse{
Version: version.KMSv2APIVersion,
Healthz: "ok",
Expand Down
4 changes: 2 additions & 2 deletions pkg/plugin/kms_v2_server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ func TestV2Decrypt(t *testing.T) {
desc: "invalid algorithm failed to decrypt",
input: []byte("bar"),
output: []byte{},
err: fmt.Errorf("algorithm \"insecure-algorithm\" does not match expected algorithm \"RSA1_5\" used for encryption"),
err: fmt.Errorf("algorithm \"insecure-algorithm\" does not match expected algorithm \"RSAOAEP256\" used for encryption"),
annotations: map[string][]byte{
algorithmAnnotationKey: []byte("insecure-algorithm"),
versionAnnotationKey: []byte("1"),
Expand Down Expand Up @@ -147,7 +147,7 @@ func TestV2Decrypt(t *testing.T) {
t.Run(test.desc, func(t *testing.T) {
kvClient := &mockkeyvault.KeyVaultClient{
KeyID: "mock-key-id",
Algorithm: keyvault.RSA15,
Algorithm: keyvault.RSAOAEP256,
}
kvClient.SetDecryptResponse(test.output, test.err)

Expand Down
2 changes: 1 addition & 1 deletion pkg/plugin/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ type Config struct {
}

// NewKMSv1Server creates an instance of the KMS Service Server.
func NewKMSv1Server(kvClient *KeyVaultClient) (*KeyManagementServiceServer, error) {
func NewKMSv1Server(kvClient Client) (*KeyManagementServiceServer, error) {
statsReporter, err := metrics.NewStatsReporter()
if err != nil {
return nil, fmt.Errorf("failed to create stats reporter: %w", err)
Expand Down

0 comments on commit 64dc67b

Please sign in to comment.