From 9387a66c71fd85840cb199b468610b8fa950253f Mon Sep 17 00:00:00 2001 From: Monis Khan Date: Wed, 10 Jan 2024 14:48:30 -0500 Subject: [PATCH] Clean up encryption config reading and hashing logic This is a no-op change that makes the internal encryption config hash more specific to it use and explicitly marks it as unstable. Signed-off-by: Monis Khan --- .../server/options/encryptionconfig/config.go | 13 +++-------- .../options/encryptionconfig/config_test.go | 8 +++---- .../controller/controller_test.go | 22 +++++++++---------- 3 files changed, 18 insertions(+), 25 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/config.go b/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/config.go index 88dc6ea57a5..2dbf335b2bb 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/config.go +++ b/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/config.go @@ -24,7 +24,6 @@ import ( "encoding/base64" "errors" "fmt" - "io" "net/http" "os" "sync" @@ -522,15 +521,9 @@ func loadConfig(filepath string, reload bool) (*apiserver.EncryptionConfiguratio } func loadDataAndHash(filepath string) ([]byte, string, error) { - f, err := os.Open(filepath) + data, err := os.ReadFile(filepath) if err != nil { - return nil, "", fmt.Errorf("error opening encryption provider configuration file %q: %w", filepath, err) - } - defer f.Close() - - data, err := io.ReadAll(f) - if err != nil { - return nil, "", fmt.Errorf("could not read contents: %w", err) + return nil, "", fmt.Errorf("error reading encryption provider configuration file %q: %w", filepath, err) } if len(data) == 0 { return nil, "", fmt.Errorf("encryption provider configuration file %q is empty", filepath) @@ -902,7 +895,7 @@ func (u unionTransformers) TransformToStorage(ctx context.Context, data []byte, // We use a hash instead of the raw file contents when tracking changes to avoid holding any encryption keys in memory outside of their associated transformers. // This hash must be used in-memory and not externalized to the process because it has no cross-release stability guarantees. func computeEncryptionConfigHash(data []byte) string { - return fmt.Sprintf("%x", sha256.Sum256(data)) + return fmt.Sprintf("k8s:enc:unstable:1:%x", sha256.Sum256(data)) } var _ storagevalue.ResourceTransformers = &DynamicTransformers{} diff --git a/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/config_test.go b/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/config_test.go index 83b8492290b..5bd422a72bd 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/config_test.go +++ b/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/config_test.go @@ -725,7 +725,7 @@ func TestKMSPluginHealthz(t *testing.T) { desc: "Invalid config file path", config: "invalid/path", want: nil, - wantErr: `error opening encryption provider configuration file "invalid/path"`, + wantErr: `error reading encryption provider configuration file "invalid/path"`, }, { desc: "Empty config file content", @@ -1833,7 +1833,7 @@ func errString(err error) string { func TestComputeEncryptionConfigHash(t *testing.T) { // hash the empty string to be sure that sha256 is being used - expect := "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" + expect := "k8s:enc:unstable:1:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" sum := computeEncryptionConfigHash([]byte("")) if expect != sum { t.Errorf("expected hash %q but got %q", expect, sum) @@ -2197,12 +2197,12 @@ func TestGetEncryptionConfigHash(t *testing.T) { name: "missing file", filepath: "testdata/invalid-configs/kms/file-that-does-not-exist.yaml", wantHash: "", - wantErr: `error opening encryption provider configuration file "testdata/invalid-configs/kms/file-that-does-not-exist.yaml": open testdata/invalid-configs/kms/file-that-does-not-exist.yaml: no such file or directory`, + wantErr: `error reading encryption provider configuration file "testdata/invalid-configs/kms/file-that-does-not-exist.yaml": open testdata/invalid-configs/kms/file-that-does-not-exist.yaml: no such file or directory`, }, { name: "valid file", filepath: "testdata/valid-configs/secret-box-first.yaml", - wantHash: "c638c0327dbc3276dd1fcf3e67895d19ebca16b91ae0d19af24ef0759b8e0f66", + wantHash: "k8s:enc:unstable:1:c638c0327dbc3276dd1fcf3e67895d19ebca16b91ae0d19af24ef0759b8e0f66", wantErr: ``, }, } diff --git a/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/controller/controller_test.go b/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/controller/controller_test.go index 59af76df16f..da7948a2288 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/controller/controller_test.go +++ b/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/controller/controller_test.go @@ -67,7 +67,7 @@ apiserver_encryption_config_controller_automatic_reload_failures_total{apiserver }{ { name: "when invalid config is provided previous config shouldn't be changed", - wantECFileHash: "6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", + wantECFileHash: "k8s:enc:unstable:1:6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", wantLoadCalls: 1, wantHashCalls: 1, wantTransformerClosed: true, @@ -104,7 +104,7 @@ apiserver_encryption_config_controller_automatic_reload_failures_total{apiserver }, { name: "when same valid config is provided previous config shouldn't be changed", - wantECFileHash: "6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", + wantECFileHash: "k8s:enc:unstable:1:6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", wantLoadCalls: 1, wantHashCalls: 1, wantTransformerClosed: true, @@ -122,13 +122,13 @@ apiserver_encryption_config_controller_automatic_reload_failures_total{apiserver }, }, // hash of initial "testdata/ec_config.yaml" config file before reloading - EncryptionFileContentHash: "6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", + EncryptionFileContentHash: "k8s:enc:unstable:1:6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", }, nil }, }, { name: "when transformer's health check fails previous config shouldn't be changed", - wantECFileHash: "6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", + wantECFileHash: "k8s:enc:unstable:1:6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", wantLoadCalls: 1, wantHashCalls: 1, wantTransformerClosed: true, @@ -152,7 +152,7 @@ apiserver_encryption_config_controller_automatic_reload_failures_total{apiserver }, { name: "when multiple health checks are present previous config shouldn't be changed", - wantECFileHash: "6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", + wantECFileHash: "k8s:enc:unstable:1:6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", wantLoadCalls: 1, wantHashCalls: 1, wantTransformerClosed: true, @@ -179,7 +179,7 @@ apiserver_encryption_config_controller_automatic_reload_failures_total{apiserver }, { name: "when invalid health check URL is provided previous config shouldn't be changed", - wantECFileHash: "6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", + wantECFileHash: "k8s:enc:unstable:1:6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", wantLoadCalls: 1, wantHashCalls: 1, wantTransformerClosed: true, @@ -202,7 +202,7 @@ apiserver_encryption_config_controller_automatic_reload_failures_total{apiserver }, { name: "when config is not updated transformers are closed correctly", - wantECFileHash: "6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", + wantECFileHash: "k8s:enc:unstable:1:6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", wantLoadCalls: 1, wantHashCalls: 1, wantTransformerClosed: true, @@ -220,13 +220,13 @@ apiserver_encryption_config_controller_automatic_reload_failures_total{apiserver }, }, // hash of initial "testdata/ec_config.yaml" config file before reloading - EncryptionFileContentHash: "6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", + EncryptionFileContentHash: "k8s:enc:unstable:1:6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", }, nil }, }, { name: "when config hash is not updated transformers are closed correctly", - wantECFileHash: "6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", + wantECFileHash: "k8s:enc:unstable:1:6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", wantLoadCalls: 0, wantHashCalls: 1, wantTransformerClosed: true, @@ -234,7 +234,7 @@ apiserver_encryption_config_controller_automatic_reload_failures_total{apiserver wantAddRateLimitedCount: 0, mockGetEncryptionConfigHash: func(ctx context.Context, filepath string) (string, error) { // hash of initial "testdata/ec_config.yaml" config file before reloading - return "6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", nil + return "k8s:enc:unstable:1:6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", nil }, mockLoadEncryptionConfig: func(ctx context.Context, filepath string, reload bool, apiServerID string) (*encryptionconfig.EncryptionConfiguration, error) { return nil, fmt.Errorf("should not be called") @@ -242,7 +242,7 @@ apiserver_encryption_config_controller_automatic_reload_failures_total{apiserver }, { name: "when config hash errors transformers are closed correctly", - wantECFileHash: "6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", + wantECFileHash: "k8s:enc:unstable:1:6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", wantLoadCalls: 0, wantHashCalls: 1, wantTransformerClosed: true,