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 <mok@microsoft.com>
This commit is contained in:
		| @@ -24,7 +24,6 @@ import ( | |||||||
| 	"encoding/base64" | 	"encoding/base64" | ||||||
| 	"errors" | 	"errors" | ||||||
| 	"fmt" | 	"fmt" | ||||||
| 	"io" |  | ||||||
| 	"net/http" | 	"net/http" | ||||||
| 	"os" | 	"os" | ||||||
| 	"sync" | 	"sync" | ||||||
| @@ -522,15 +521,9 @@ func loadConfig(filepath string, reload bool) (*apiserver.EncryptionConfiguratio | |||||||
| } | } | ||||||
|  |  | ||||||
| func loadDataAndHash(filepath string) ([]byte, string, error) { | func loadDataAndHash(filepath string) ([]byte, string, error) { | ||||||
| 	f, err := os.Open(filepath) | 	data, err := os.ReadFile(filepath) | ||||||
| 	if err != nil { | 	if err != nil { | ||||||
| 		return nil, "", fmt.Errorf("error opening encryption provider configuration file %q: %w", filepath, err) | 		return nil, "", fmt.Errorf("error reading 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) |  | ||||||
| 	} | 	} | ||||||
| 	if len(data) == 0 { | 	if len(data) == 0 { | ||||||
| 		return nil, "", fmt.Errorf("encryption provider configuration file %q is empty", filepath) | 		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. | // 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. | // 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 { | 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{} | var _ storagevalue.ResourceTransformers = &DynamicTransformers{} | ||||||
|   | |||||||
| @@ -725,7 +725,7 @@ func TestKMSPluginHealthz(t *testing.T) { | |||||||
| 			desc:    "Invalid config file path", | 			desc:    "Invalid config file path", | ||||||
| 			config:  "invalid/path", | 			config:  "invalid/path", | ||||||
| 			want:    nil, | 			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", | 			desc:    "Empty config file content", | ||||||
| @@ -1833,7 +1833,7 @@ func errString(err error) string { | |||||||
|  |  | ||||||
| func TestComputeEncryptionConfigHash(t *testing.T) { | func TestComputeEncryptionConfigHash(t *testing.T) { | ||||||
| 	// hash the empty string to be sure that sha256 is being used | 	// hash the empty string to be sure that sha256 is being used | ||||||
| 	expect := "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" | 	expect := "k8s:enc:unstable:1:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" | ||||||
| 	sum := computeEncryptionConfigHash([]byte("")) | 	sum := computeEncryptionConfigHash([]byte("")) | ||||||
| 	if expect != sum { | 	if expect != sum { | ||||||
| 		t.Errorf("expected hash %q but got %q", expect, sum) | 		t.Errorf("expected hash %q but got %q", expect, sum) | ||||||
| @@ -2197,12 +2197,12 @@ func TestGetEncryptionConfigHash(t *testing.T) { | |||||||
| 			name:     "missing file", | 			name:     "missing file", | ||||||
| 			filepath: "testdata/invalid-configs/kms/file-that-does-not-exist.yaml", | 			filepath: "testdata/invalid-configs/kms/file-that-does-not-exist.yaml", | ||||||
| 			wantHash: "", | 			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", | 			name:     "valid file", | ||||||
| 			filepath: "testdata/valid-configs/secret-box-first.yaml", | 			filepath: "testdata/valid-configs/secret-box-first.yaml", | ||||||
| 			wantHash: "c638c0327dbc3276dd1fcf3e67895d19ebca16b91ae0d19af24ef0759b8e0f66", | 			wantHash: "k8s:enc:unstable:1:c638c0327dbc3276dd1fcf3e67895d19ebca16b91ae0d19af24ef0759b8e0f66", | ||||||
| 			wantErr:  ``, | 			wantErr:  ``, | ||||||
| 		}, | 		}, | ||||||
| 	} | 	} | ||||||
|   | |||||||
| @@ -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", | 			name:                    "when invalid config is provided previous config shouldn't be changed", | ||||||
| 			wantECFileHash:          "6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", | 			wantECFileHash:          "k8s:enc:unstable:1:6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", | ||||||
| 			wantLoadCalls:           1, | 			wantLoadCalls:           1, | ||||||
| 			wantHashCalls:           1, | 			wantHashCalls:           1, | ||||||
| 			wantTransformerClosed:   true, | 			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", | 			name:                    "when same valid config is provided previous config shouldn't be changed", | ||||||
| 			wantECFileHash:          "6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", | 			wantECFileHash:          "k8s:enc:unstable:1:6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", | ||||||
| 			wantLoadCalls:           1, | 			wantLoadCalls:           1, | ||||||
| 			wantHashCalls:           1, | 			wantHashCalls:           1, | ||||||
| 			wantTransformerClosed:   true, | 			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 | 					// hash of initial "testdata/ec_config.yaml" config file before reloading | ||||||
| 					EncryptionFileContentHash: "6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", | 					EncryptionFileContentHash: "k8s:enc:unstable:1:6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", | ||||||
| 				}, nil | 				}, nil | ||||||
| 			}, | 			}, | ||||||
| 		}, | 		}, | ||||||
| 		{ | 		{ | ||||||
| 			name:                    "when transformer's health check fails previous config shouldn't be changed", | 			name:                    "when transformer's health check fails previous config shouldn't be changed", | ||||||
| 			wantECFileHash:          "6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", | 			wantECFileHash:          "k8s:enc:unstable:1:6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", | ||||||
| 			wantLoadCalls:           1, | 			wantLoadCalls:           1, | ||||||
| 			wantHashCalls:           1, | 			wantHashCalls:           1, | ||||||
| 			wantTransformerClosed:   true, | 			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", | 			name:                    "when multiple health checks are present previous config shouldn't be changed", | ||||||
| 			wantECFileHash:          "6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", | 			wantECFileHash:          "k8s:enc:unstable:1:6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", | ||||||
| 			wantLoadCalls:           1, | 			wantLoadCalls:           1, | ||||||
| 			wantHashCalls:           1, | 			wantHashCalls:           1, | ||||||
| 			wantTransformerClosed:   true, | 			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", | 			name:                    "when invalid health check URL is provided previous config shouldn't be changed", | ||||||
| 			wantECFileHash:          "6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", | 			wantECFileHash:          "k8s:enc:unstable:1:6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", | ||||||
| 			wantLoadCalls:           1, | 			wantLoadCalls:           1, | ||||||
| 			wantHashCalls:           1, | 			wantHashCalls:           1, | ||||||
| 			wantTransformerClosed:   true, | 			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", | 			name:                    "when config is not updated transformers are closed correctly", | ||||||
| 			wantECFileHash:          "6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", | 			wantECFileHash:          "k8s:enc:unstable:1:6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", | ||||||
| 			wantLoadCalls:           1, | 			wantLoadCalls:           1, | ||||||
| 			wantHashCalls:           1, | 			wantHashCalls:           1, | ||||||
| 			wantTransformerClosed:   true, | 			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 | 					// hash of initial "testdata/ec_config.yaml" config file before reloading | ||||||
| 					EncryptionFileContentHash: "6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", | 					EncryptionFileContentHash: "k8s:enc:unstable:1:6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", | ||||||
| 				}, nil | 				}, nil | ||||||
| 			}, | 			}, | ||||||
| 		}, | 		}, | ||||||
| 		{ | 		{ | ||||||
| 			name:                    "when config hash is not updated transformers are closed correctly", | 			name:                    "when config hash is not updated transformers are closed correctly", | ||||||
| 			wantECFileHash:          "6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", | 			wantECFileHash:          "k8s:enc:unstable:1:6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", | ||||||
| 			wantLoadCalls:           0, | 			wantLoadCalls:           0, | ||||||
| 			wantHashCalls:           1, | 			wantHashCalls:           1, | ||||||
| 			wantTransformerClosed:   true, | 			wantTransformerClosed:   true, | ||||||
| @@ -234,7 +234,7 @@ apiserver_encryption_config_controller_automatic_reload_failures_total{apiserver | |||||||
| 			wantAddRateLimitedCount: 0, | 			wantAddRateLimitedCount: 0, | ||||||
| 			mockGetEncryptionConfigHash: func(ctx context.Context, filepath string) (string, error) { | 			mockGetEncryptionConfigHash: func(ctx context.Context, filepath string) (string, error) { | ||||||
| 				// hash of initial "testdata/ec_config.yaml" config file before reloading | 				// 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) { | 			mockLoadEncryptionConfig: func(ctx context.Context, filepath string, reload bool, apiServerID string) (*encryptionconfig.EncryptionConfiguration, error) { | ||||||
| 				return nil, fmt.Errorf("should not be called") | 				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", | 			name:                    "when config hash errors transformers are closed correctly", | ||||||
| 			wantECFileHash:          "6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", | 			wantECFileHash:          "k8s:enc:unstable:1:6bc9f4aa2e5587afbb96074e1809550cbc4de3cc3a35717dac8ff2800a147fd3", | ||||||
| 			wantLoadCalls:           0, | 			wantLoadCalls:           0, | ||||||
| 			wantHashCalls:           1, | 			wantHashCalls:           1, | ||||||
| 			wantTransformerClosed:   true, | 			wantTransformerClosed:   true, | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user
	 Monis Khan
					Monis Khan