diff --git a/go.mod b/go.mod index bbb40cc2a..e674998f8 100644 --- a/go.mod +++ b/go.mod @@ -44,10 +44,10 @@ require ( gopkg.in/square/go-jose.v2 v2.6.0 k8s.io/api v0.0.0-20230302120942-f6c2559ad4f4 k8s.io/apimachinery v0.0.0-20230302115847-76eb944e266d - k8s.io/client-go v0.0.0-20230303000145-3f4372de09cb + k8s.io/client-go v0.0.0-20230303065657-bfa72fd2d367 k8s.io/component-base v0.0.0-20230302013528-cdd3d3a94e73 k8s.io/klog/v2 v2.90.1 - k8s.io/kms v0.0.0-20230302204122-57de6be5f4a6 + k8s.io/kms v0.0.0-20230303123242-0b3f9dde5fcd k8s.io/kube-openapi v0.0.0-20230123231816-1cb3ae25d79a k8s.io/utils v0.0.0-20230209194617-a36077c30491 sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.1.1 @@ -126,7 +126,7 @@ require ( replace ( k8s.io/api => k8s.io/api v0.0.0-20230302120942-f6c2559ad4f4 k8s.io/apimachinery => k8s.io/apimachinery v0.0.0-20230302115847-76eb944e266d - k8s.io/client-go => k8s.io/client-go v0.0.0-20230303000145-3f4372de09cb + k8s.io/client-go => k8s.io/client-go v0.0.0-20230303065657-bfa72fd2d367 k8s.io/component-base => k8s.io/component-base v0.0.0-20230302013528-cdd3d3a94e73 - k8s.io/kms => k8s.io/kms v0.0.0-20230302204122-57de6be5f4a6 + k8s.io/kms => k8s.io/kms v0.0.0-20230303123242-0b3f9dde5fcd ) diff --git a/go.sum b/go.sum index 85eb0bff7..94bc3421c 100644 --- a/go.sum +++ b/go.sum @@ -878,14 +878,14 @@ k8s.io/api v0.0.0-20230302120942-f6c2559ad4f4 h1:qm5Xz7wiOrtVHPKAZsBByEd6k2WST6T k8s.io/api v0.0.0-20230302120942-f6c2559ad4f4/go.mod h1:cTD04/XhoraqP0GpFXtefYJYXBw6coqVSibz5Rzivkw= k8s.io/apimachinery v0.0.0-20230302115847-76eb944e266d h1:mridg1Zm6thnb5oTe+rOGnEUbPnjys9YHBFxlOf+GeA= k8s.io/apimachinery v0.0.0-20230302115847-76eb944e266d/go.mod h1:TO4higCGNMwebVSdb1XPJdXMU4kk+nmMY/cTMVCGa6M= -k8s.io/client-go v0.0.0-20230303000145-3f4372de09cb h1:f+duaMC7mXtKgrzzFQYIHMdyR0436E1X/r4JDU9CYYk= -k8s.io/client-go v0.0.0-20230303000145-3f4372de09cb/go.mod h1:4+y45xff2wn/bjSSXI+/SqdRZnYI4r8ndWJg7eOsyNg= +k8s.io/client-go v0.0.0-20230303065657-bfa72fd2d367 h1:y1rUzE1VGpnWqpVBHXCOp8BlmTtZ3bU6I8L5xAQd3gg= +k8s.io/client-go v0.0.0-20230303065657-bfa72fd2d367/go.mod h1:4+y45xff2wn/bjSSXI+/SqdRZnYI4r8ndWJg7eOsyNg= k8s.io/component-base v0.0.0-20230302013528-cdd3d3a94e73 h1:9aUdsRcbNOOttZL0FVKTuiZlVLCef4+aAKh7DAmArOw= k8s.io/component-base v0.0.0-20230302013528-cdd3d3a94e73/go.mod h1:OEU5Klnkrq4+Nh1Ir1vBDc2JL9FvvCcmjzrnR/eT7I4= k8s.io/klog/v2 v2.90.1 h1:m4bYOKall2MmOiRaR1J+We67Do7vm9KiQVlT96lnHUw= k8s.io/klog/v2 v2.90.1/go.mod h1:y1WjHnz7Dj687irZUWR/WLkLc5N1YHtjLdmgWjndZn0= -k8s.io/kms v0.0.0-20230302204122-57de6be5f4a6 h1:dC5jw8zGznaPcsoInDJEs999or8eS21PNFC67HV2wdI= -k8s.io/kms v0.0.0-20230302204122-57de6be5f4a6/go.mod h1:hp2A3BisYagCb7/4bWUf48fdBFflj12BBYAmH9XpWaI= +k8s.io/kms v0.0.0-20230303123242-0b3f9dde5fcd h1:0vh05+hg06/s6B0rPgOZ4ej5AH7GNp4qX6GLEa+pjVw= +k8s.io/kms v0.0.0-20230303123242-0b3f9dde5fcd/go.mod h1:BsJEqK1MWN8sYXv2fVg9sQ8DJcmCIGFo/WDDOokPOTQ= k8s.io/kube-openapi v0.0.0-20230123231816-1cb3ae25d79a h1:s6zvHjyDQX1NtVT88pvw2tddqhqY0Bz0Gbnn+yctsFU= k8s.io/kube-openapi v0.0.0-20230123231816-1cb3ae25d79a/go.mod h1:/BYxry62FuDzmI+i9B+X2pqfySRmSOW2ARmj5Zbqhj0= k8s.io/utils v0.0.0-20230209194617-a36077c30491 h1:r0BAOLElQnnFhE/ApUsg3iHdVYYPBjNSSOMowRZxxsY= diff --git a/pkg/server/options/encryptionconfig/config.go b/pkg/server/options/encryptionconfig/config.go index 3f73a3d54..631aa64df 100644 --- a/pkg/server/options/encryptionconfig/config.go +++ b/pkg/server/options/encryptionconfig/config.go @@ -347,7 +347,7 @@ func loadConfig(filepath string, reload bool) (*apiserverconfig.EncryptionConfig configObj, gvk, err := codecs.UniversalDecoder().Decode(data, nil, nil) if err != nil { - return nil, "", err + return nil, "", fmt.Errorf("error decoding encryption provider configuration file %q: %w", filepath, err) } config, ok := configObj.(*apiserverconfig.EncryptionConfiguration) if !ok { diff --git a/pkg/server/options/encryptionconfig/config_test.go b/pkg/server/options/encryptionconfig/config_test.go index d0f5ab381..3cbb8c411 100644 --- a/pkg/server/options/encryptionconfig/config_test.go +++ b/pkg/server/options/encryptionconfig/config_test.go @@ -214,6 +214,12 @@ func TestEncryptionProviderConfigCorrect(t *testing.T) { t.Fatalf("KMSCloseGracePeriod mismatch (-want +got):\n%s", cmp.Diff(expectedKMSCloseGracePeriod, aesGcmFirstEncryptionConfiguration.KMSCloseGracePeriod)) } + invalidConfigWithAesGcm := "testdata/invalid-configs/invalid-aes-gcm.yaml" + _, err = LoadEncryptionConfig(ctx, invalidConfigWithAesGcm, false) + if !strings.Contains(errString(err), "error while parsing file") { + t.Fatalf("should result in error while parsing configuration file: %s.\nThe file was:\n%s", err, invalidConfigWithAesGcm) + } + // Math for GracePeriod is explained at - https://github.com/kubernetes/kubernetes/blob/c9ed04762f94a319d7b1fb718dc345491a32bea6/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/config.go#L159-L163 expectedKMSCloseGracePeriod = 26 * time.Second correctConfigWithAesCbcFirst := "testdata/valid-configs/aes-cbc-first.yaml" @@ -307,9 +313,27 @@ func TestKMSMaxTimeout(t *testing.T) { testCases := []struct { name string + expectedErr string expectedTimeout time.Duration config apiserverconfig.EncryptionConfiguration }{ + { + name: "config with bad provider", + config: apiserverconfig.EncryptionConfiguration{ + Resources: []apiserverconfig.ResourceConfiguration{ + { + Resources: []string{"secrets"}, + Providers: []apiserverconfig.ProviderConfiguration{ + { + KMS: nil, + }, + }, + }, + }, + }, + expectedErr: "provider does not contain any of the expected providers: KMS, AESGCM, AESCBC, Secretbox, Identity", + expectedTimeout: 6 * time.Second, + }, { name: "default timeout", config: apiserverconfig.EncryptionConfiguration{ @@ -333,6 +357,7 @@ func TestKMSMaxTimeout(t *testing.T) { }, }, }, + expectedErr: "", expectedTimeout: 6 * time.Second, }, { @@ -375,6 +400,7 @@ func TestKMSMaxTimeout(t *testing.T) { }, }, }, + expectedErr: "", expectedTimeout: 12 * time.Second, }, { @@ -433,6 +459,7 @@ func TestKMSMaxTimeout(t *testing.T) { }, }, }, + expectedErr: "", expectedTimeout: 32 * time.Second, }, { @@ -491,6 +518,7 @@ func TestKMSMaxTimeout(t *testing.T) { }, }, }, + expectedErr: "", expectedTimeout: 15 * time.Second, }, } @@ -509,14 +537,21 @@ func TestKMSMaxTimeout(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) cancel() // cancel this upfront so the kms v2 checks do not block - _, _, kmsUsed, _ := getTransformerOverridesAndKMSPluginHealthzCheckers(ctx, &testCase.config) - if kmsUsed == nil { - t.Fatal("kmsUsed should not be nil") + _, _, kmsUsed, err := getTransformerOverridesAndKMSPluginHealthzCheckers(ctx, &testCase.config) + + if !strings.Contains(errString(err), testCase.expectedErr) { + t.Fatalf("expecting error calling prefixTransformersAndProbes, expected: %s, got: %s", testCase.expectedErr, errString(err)) + } + if len(testCase.expectedErr) == 0 { + if kmsUsed == nil { + t.Fatal("kmsUsed should not be nil") + } + + if kmsUsed.kmsTimeoutSum != testCase.expectedTimeout { + t.Fatalf("expected timeout %v, got %v", testCase.expectedTimeout, kmsUsed.kmsTimeoutSum) + } } - if kmsUsed.kmsTimeoutSum != testCase.expectedTimeout { - t.Fatalf("expected timeout %v, got %v", testCase.expectedTimeout, kmsUsed.kmsTimeoutSum) - } }) } } @@ -539,6 +574,30 @@ func TestKMSPluginHealthz(t *testing.T) { kmsv2 bool kmsv1 bool }{ + { + desc: "Invalid config file path", + config: "invalid/path", + want: nil, + wantErr: `error opening encryption provider configuration file "invalid/path"`, + }, + { + desc: "Empty config file content", + config: "testdata/invalid-configs/kms/invalid-content.yaml", + want: nil, + wantErr: `encryption provider configuration file "testdata/invalid-configs/kms/invalid-content.yaml" is empty`, + }, + { + desc: "Unable to decode", + config: "testdata/invalid-configs/kms/invalid-gvk.yaml", + want: nil, + wantErr: `error decoding encryption provider configuration file`, + }, + { + desc: "Unexpected config type", + config: "testdata/invalid-configs/kms/invalid-config-type.yaml", + want: nil, + wantErr: `no kind "EncryptionConfigurations" is registered for version "apiserver.config.k8s.io/v1"`, + }, { desc: "Install Healthz", config: "testdata/valid-configs/kms/default-timeout.yaml", @@ -593,7 +652,7 @@ func TestKMSPluginHealthz(t *testing.T) { for _, tt := range testCases { t.Run(tt.desc, func(t *testing.T) { config, _, err := loadConfig(tt.config, false) - if errStr := errString(err); errStr != tt.wantErr { + if errStr := errString(err); !strings.Contains(errStr, tt.wantErr) { t.Fatalf("unexpected error state got=%s want=%s", errStr, tt.wantErr) } if len(tt.wantErr) > 0 { @@ -837,7 +896,7 @@ func TestCBCKeyRotationWithoutOverlappingProviders(t *testing.T) { func testCBCKeyRotationWithProviders(t *testing.T, firstEncryptionConfig, firstPrefix, secondEncryptionConfig, secondPrefix string) { p := getTransformerFromEncryptionConfig(t, firstEncryptionConfig) - ctx := context.Background() + ctx := testContext(t) dataCtx := value.DefaultContext([]byte("authenticated_data")) out, err := p.TransformToStorage(ctx, []byte("firstvalue"), dataCtx) @@ -907,6 +966,7 @@ func getTransformerFromEncryptionConfig(t *testing.T, encryptionConfigPath strin func TestIsKMSv2ProviderHealthyError(t *testing.T) { testCases := []struct { desc string + expectedErr string statusResponse *kmsservice.StatusResponse }{ { @@ -914,12 +974,14 @@ func TestIsKMSv2ProviderHealthyError(t *testing.T) { statusResponse: &kmsservice.StatusResponse{ Healthz: "unhealthy", }, + expectedErr: "got unexpected healthz status: unhealthy, expected KMSv2 API version v2alpha1, got , expected KMSv2 KeyID to be set, got ", }, { desc: "version is not v2alpha1", statusResponse: &kmsservice.StatusResponse{ Version: "v1beta1", }, + expectedErr: "got unexpected healthz status: , expected KMSv2 API version v2alpha1, got v1beta1, expected KMSv2 KeyID to be set, got ", }, { desc: "missing keyID", @@ -927,13 +989,24 @@ func TestIsKMSv2ProviderHealthyError(t *testing.T) { Healthz: "ok", Version: "v2alpha1", }, + expectedErr: "expected KMSv2 KeyID to be set, got ", + }, + { + desc: "invalid long keyID", + statusResponse: &kmsservice.StatusResponse{ + Healthz: "ok", + Version: "v2alpha1", + KeyID: sampleInvalidKeyID, + }, + expectedErr: "expected KMSv2 KeyID to be set, got ", }, } for _, tt := range testCases { t.Run(tt.desc, func(t *testing.T) { - if err := isKMSv2ProviderHealthy("testplugin", tt.statusResponse); err == nil { - t.Fatalf("isKMSv2ProviderHealthy() should have returned an error") + err := isKMSv2ProviderHealthy("testplugin", tt.statusResponse) + if !strings.Contains(errString(err), tt.expectedErr) { + t.Errorf("expected err %q, got %q", tt.expectedErr, errString(err)) } }) } @@ -961,3 +1034,36 @@ func TestComputeEncryptionConfigHash(t *testing.T) { t.Errorf("expected hash %q but got %q", expect, sum) } } + +func TestGetCurrentKeyID(t *testing.T) { + ctx := testContext(t) + kmsv2Probe := &kmsv2PluginProbe{ + name: "foo", + ttl: 3 * time.Second, + } + testCases := []struct { + desc string + keyID string + expectedErr string + }{ + { + desc: "empty keyID", + keyID: "", + expectedErr: "got unexpected empty keyID", + }, + { + desc: "valid keyID", + keyID: "1", + expectedErr: "", + }, + } + for _, tt := range testCases { + t.Run(tt.desc, func(t *testing.T) { + kmsv2Probe.keyID.Store(&tt.keyID) + _, err := kmsv2Probe.getCurrentKeyID(ctx) + if errString(err) != tt.expectedErr { + t.Errorf("expected err %q, got %q", tt.expectedErr, errString(err)) + } + }) + } +} diff --git a/pkg/server/options/encryptionconfig/testdata/invalid-configs/invalid-aes-gcm.yaml b/pkg/server/options/encryptionconfig/testdata/invalid-configs/invalid-aes-gcm.yaml new file mode 100644 index 000000000..75bb54e58 --- /dev/null +++ b/pkg/server/options/encryptionconfig/testdata/invalid-configs/invalid-aes-gcm.yaml @@ -0,0 +1,8 @@ +kind: EncryptionConfiguration +apiVersion: apiserver.config.k8s.io/v1 +resources: + - resources: + - secrets + providers: + - aesgcm: + keys: diff --git a/pkg/server/options/encryptionconfig/testdata/invalid-configs/kms/invalid-config-type.yaml b/pkg/server/options/encryptionconfig/testdata/invalid-configs/kms/invalid-config-type.yaml new file mode 100644 index 000000000..b4b7f44b4 --- /dev/null +++ b/pkg/server/options/encryptionconfig/testdata/invalid-configs/kms/invalid-config-type.yaml @@ -0,0 +1,11 @@ +kind: EncryptionConfigurations +apiVersion: apiserver.config.k8s.io/v1 +resources: + - resources: + - secrets + providers: + - kms: + apiVersion: v2 + name: testproviderv2 + endpoint: unix:///tmp/testprovider.sock + timeout: 15s diff --git a/pkg/server/options/encryptionconfig/testdata/invalid-configs/kms/invalid-content.yaml b/pkg/server/options/encryptionconfig/testdata/invalid-configs/kms/invalid-content.yaml new file mode 100644 index 000000000..e69de29bb diff --git a/pkg/server/options/encryptionconfig/testdata/invalid-configs/kms/invalid-gvk.yaml b/pkg/server/options/encryptionconfig/testdata/invalid-configs/kms/invalid-gvk.yaml new file mode 100644 index 000000000..d1a2cc1c3 --- /dev/null +++ b/pkg/server/options/encryptionconfig/testdata/invalid-configs/kms/invalid-gvk.yaml @@ -0,0 +1,11 @@ +kinds: EncryptionConfiguration +apiVersion: apiserver.config.k8s.io/v1 +resources: + - resources: + - secrets + providers: + - kms: + apiVersion: v2 + name: testproviderv2 + endpoint: unix:///tmp/testprovider.sock + timeout: 15s diff --git a/pkg/storage/value/encrypt/envelope/metrics/metrics_test.go b/pkg/storage/value/encrypt/envelope/metrics/metrics_test.go index d9cbeeaeb..b72099c77 100644 --- a/pkg/storage/value/encrypt/envelope/metrics/metrics_test.go +++ b/pkg/storage/value/encrypt/envelope/metrics/metrics_test.go @@ -345,6 +345,51 @@ func TestRecordKeyIDLRUKey(t *testing.T) { } } +func TestRecordKeyIDFromStatus(t *testing.T) { + RegisterMetrics() + + cacheSize = 3 + registerLRUMetrics() + KeyIDHashStatusLastTimestampSeconds.Reset() + defer KeyIDHashStatusLastTimestampSeconds.Reset() + + var wg sync.WaitGroup + for i := 1; i < 100; i++ { + wg.Add(1) + go func() { + defer wg.Done() + keyID := rand.String(32) + key := metricLabels{ + providerName: rand.String(32), + keyIDHash: getHash(keyID), + } + RecordKeyIDFromStatus(key.providerName, keyID) + }() + } + wg.Wait() + + validMetrics := 0 + metricFamilies, err := legacyregistry.DefaultGatherer.Gather() + if err != nil { + t.Fatal(err) + } + for _, family := range metricFamilies { + if family.GetName() != "apiserver_envelope_encryption_key_id_hash_status_last_timestamp_seconds" { + continue + } + for _, metric := range family.GetMetric() { + if metric.Gauge.GetValue() == 0 { + t.Errorf("invalid metric seen: %s", metric.String()) + } else { + validMetrics++ + } + } + } + if validMetrics != cacheSize { + t.Fatalf("expected total valid metrics to be the same as cacheSize %d, got %d", cacheSize, validMetrics) + } +} + func TestRecordInvalidKeyIDFromStatus(t *testing.T) { testCases := []struct { desc string