-
Notifications
You must be signed in to change notification settings - Fork 896
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GODRIVER-3168 Retry KMS requests on transient errors. #1887
base: master
Are you sure you want to change the base?
Changes from 5 commits
02a60a5
12a1530
00e69f5
701bd46
614aba5
893731f
f567dbf
12650cd
f5ccc05
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -553,6 +553,39 @@ functions: | |
KMS_MOCK_SERVERS_RUNNING: "true" | ||
args: [*task-runner, evg-test-kmip] | ||
|
||
start-kms-failpoint-server: | ||
- command: subprocess.exec | ||
params: | ||
working_dir: src/go.mongodb.org/mongo-driver | ||
binary: bash | ||
background: true | ||
include_expansions_in_env: ["AWS_ACCESS_KEY_ID", "AWS_SECRET_ACCESS_KEY", "AWS_SESSION_TOKEN", "DRIVERS_TOOLS"] | ||
# This cannot use task because it will hang on Windows. | ||
args: [etc/setup-encryption.sh] | ||
- command: subprocess.exec | ||
params: | ||
binary: python3 | ||
background: true | ||
args: ["-u", "${DRIVERS_TOOLS}/.evergreen/csfle/kms_failpoint_server.py", "--port", "9003"] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This step is already done by start-servers.sh. |
||
|
||
run-retry-kms-requests: | ||
- command: subprocess.exec | ||
type: test | ||
params: | ||
binary: "bash" | ||
env: | ||
GO_BUILD_TAGS: cse | ||
include_expansions_in_env: [AUTH, SSL, MONGODB_URI, TOPOLOGY, MONGO_GO_DRIVER_COMPRESSOR] | ||
args: [*task-runner, setup-test] | ||
- command: subprocess.exec | ||
type: test | ||
params: | ||
binary: "bash" | ||
env: | ||
KMS_FAILPOINT_CA_FILE: "${DRIVERS_TOOLS}/.evergreen/x509gen/ca.pem" | ||
KMS_FAILPOINT_SERVER_RUNNING: "true" | ||
args: [*task-runner, evg-test-retry-kms-requests] | ||
|
||
run-fuzz-tests: | ||
- command: subprocess.exec | ||
type: test | ||
|
@@ -1486,6 +1519,17 @@ tasks: | |
AUTH: "noauth" | ||
SSL: "nossl" | ||
|
||
- name: "test-retry-kms-requests" | ||
tags: ["retry-kms-requests"] | ||
commands: | ||
- func: bootstrap-mongo-orchestration | ||
vars: | ||
TOPOLOGY: "server" | ||
AUTH: "noauth" | ||
SSL: "nossl" | ||
- func: start-kms-failpoint-server | ||
- func: run-retry-kms-requests | ||
|
||
- name: "test-serverless" | ||
tags: ["serverless"] | ||
commands: | ||
|
@@ -2195,6 +2239,12 @@ buildvariants: | |
tasks: | ||
- name: ".kms-kmip" | ||
|
||
- matrix_name: "retry-kms-requests-test" | ||
matrix_spec: { version: ["7.0"], os-ssl-40: ["rhel87-64"] } | ||
display_name: "Retry KMS Requests ${os-ssl-40}" | ||
tasks: | ||
- name: ".retry-kms-requests" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of making a new matrix, can we roll this up into a general "KMS Test" that contains all ".kms-tls" and ".retry-kms-requests" tests? |
||
|
||
- matrix_name: "fuzz-test" | ||
matrix_spec: { version: ["5.0"], os-ssl-40: ["rhel87-64"] } | ||
display_name: "Fuzz ${version} ${os-ssl-40}" | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -14,6 +14,7 @@ import ( | |||||||||||||||||||||||||||||||
"context" | ||||||||||||||||||||||||||||||||
"crypto/tls" | ||||||||||||||||||||||||||||||||
"encoding/base64" | ||||||||||||||||||||||||||||||||
"encoding/json" | ||||||||||||||||||||||||||||||||
"fmt" | ||||||||||||||||||||||||||||||||
"io/ioutil" | ||||||||||||||||||||||||||||||||
"net" | ||||||||||||||||||||||||||||||||
|
@@ -30,6 +31,7 @@ import ( | |||||||||||||||||||||||||||||||
"go.mongodb.org/mongo-driver/v2/internal/handshake" | ||||||||||||||||||||||||||||||||
"go.mongodb.org/mongo-driver/v2/internal/integration/mtest" | ||||||||||||||||||||||||||||||||
"go.mongodb.org/mongo-driver/v2/internal/integtest" | ||||||||||||||||||||||||||||||||
"go.mongodb.org/mongo-driver/v2/internal/require" | ||||||||||||||||||||||||||||||||
"go.mongodb.org/mongo-driver/v2/mongo" | ||||||||||||||||||||||||||||||||
"go.mongodb.org/mongo-driver/v2/mongo/options" | ||||||||||||||||||||||||||||||||
"go.mongodb.org/mongo-driver/v2/mongo/writeconcern" | ||||||||||||||||||||||||||||||||
|
@@ -2918,7 +2920,7 @@ func TestClientSideEncryptionProse(t *testing.T) { | |||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
mt.RunOpts("22. range explicit encryption applies defaults", qeRunOpts22, func(mt *mtest.T) { | ||||||||||||||||||||||||||||||||
mt.RunOpts("23. range explicit encryption applies defaults", qeRunOpts22, func(mt *mtest.T) { | ||||||||||||||||||||||||||||||||
err := mt.Client.Database("keyvault").Collection("datakeys").Drop(context.Background()) | ||||||||||||||||||||||||||||||||
assert.Nil(mt, err, "error on Drop: %v", err) | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
|
@@ -2979,6 +2981,150 @@ func TestClientSideEncryptionProse(t *testing.T) { | |||||||||||||||||||||||||||||||
assert.Greater(t, len(payload.Data), len(payloadDefaults.Data), "the returned payload size is expected to be greater than %d", len(payloadDefaults.Data)) | ||||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
mt.RunOpts("24. kms retry tests", noClientOpts, func(mt *mtest.T) { | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Optional: Consider separating this into its own test function. |
||||||||||||||||||||||||||||||||
kmsTlsTestcase := os.Getenv("KMS_FAILPOINT_SERVER_RUNNING") | ||||||||||||||||||||||||||||||||
if kmsTlsTestcase == "" { | ||||||||||||||||||||||||||||||||
mt.Skipf("Skipping test as KMS_FAILPOINT_SERVER_RUNNING is not set") | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
mt.Parallel() | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
var tlsCfg *tls.Config | ||||||||||||||||||||||||||||||||
if tlsCAFile := os.Getenv("KMS_FAILPOINT_CA_FILE"); tlsCAFile == "" { | ||||||||||||||||||||||||||||||||
require.Fail(mt, "failed to load CA file") | ||||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||||
var err error | ||||||||||||||||||||||||||||||||
clientAndCATlsMap := map[string]interface{}{ | ||||||||||||||||||||||||||||||||
"tlsCAFile": tlsCAFile, | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
tlsCfg, err = options.BuildTLSConfig(clientAndCATlsMap) | ||||||||||||||||||||||||||||||||
require.Nil(mt, err, "BuildTLSConfig error: %v", err) | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since
Suggested change
|
||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
setFailPoint := func(failure string, count int) error { | ||||||||||||||||||||||||||||||||
url := fmt.Sprintf("https://localhost:9003/set_failpoint/%s", failure) | ||||||||||||||||||||||||||||||||
var payloadBuf bytes.Buffer | ||||||||||||||||||||||||||||||||
body := map[string]int{"count": count} | ||||||||||||||||||||||||||||||||
json.NewEncoder(&payloadBuf).Encode(body) | ||||||||||||||||||||||||||||||||
req, err := http.NewRequest(http.MethodPost, url, &payloadBuf) | ||||||||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||||||||
return err | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
client := &http.Client{ | ||||||||||||||||||||||||||||||||
Transport: &http.Transport{TLSClientConfig: tlsCfg}, | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
res, err := client.Do(req) | ||||||||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||||||||
return err | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
return res.Body.Close() | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
kmsProviders := map[string]map[string]interface{}{ | ||||||||||||||||||||||||||||||||
"aws": { | ||||||||||||||||||||||||||||||||
"accessKeyId": awsAccessKeyID, | ||||||||||||||||||||||||||||||||
"secretAccessKey": awsSecretAccessKey, | ||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||
"azure": { | ||||||||||||||||||||||||||||||||
"tenantId": azureTenantID, | ||||||||||||||||||||||||||||||||
"clientId": azureClientID, | ||||||||||||||||||||||||||||||||
"clientSecret": azureClientSecret, | ||||||||||||||||||||||||||||||||
"identityPlatformEndpoint": "127.0.0.1:9003", | ||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||
"gcp": { | ||||||||||||||||||||||||||||||||
"email": gcpEmail, | ||||||||||||||||||||||||||||||||
"privateKey": gcpPrivateKey, | ||||||||||||||||||||||||||||||||
"endpoint": "127.0.0.1:9003", | ||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
dataKeys := []struct { | ||||||||||||||||||||||||||||||||
provider string | ||||||||||||||||||||||||||||||||
masterKey interface{} | ||||||||||||||||||||||||||||||||
}{ | ||||||||||||||||||||||||||||||||
{"aws", bson.D{ | ||||||||||||||||||||||||||||||||
{"region", "foo"}, | ||||||||||||||||||||||||||||||||
{"key", "bar"}, | ||||||||||||||||||||||||||||||||
{"endpoint", "127.0.0.1:9003"}, | ||||||||||||||||||||||||||||||||
}}, | ||||||||||||||||||||||||||||||||
{"azure", bson.D{ | ||||||||||||||||||||||||||||||||
{"keyVaultEndpoint", "127.0.0.1:9003"}, | ||||||||||||||||||||||||||||||||
{"keyName", "foo"}, | ||||||||||||||||||||||||||||||||
}}, | ||||||||||||||||||||||||||||||||
{"gcp", bson.D{ | ||||||||||||||||||||||||||||||||
{"projectId", "foo"}, | ||||||||||||||||||||||||||||||||
{"location", "bar"}, | ||||||||||||||||||||||||||||||||
{"keyRing", "baz"}, | ||||||||||||||||||||||||||||||||
{"keyName", "qux"}, | ||||||||||||||||||||||||||||||||
{"endpoint", "127.0.0.1:9003"}, | ||||||||||||||||||||||||||||||||
}}, | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
testCases := []struct { | ||||||||||||||||||||||||||||||||
name string | ||||||||||||||||||||||||||||||||
failure string | ||||||||||||||||||||||||||||||||
}{ | ||||||||||||||||||||||||||||||||
{"Case 1: createDataKey and encrypt with TCP retry", "network"}, | ||||||||||||||||||||||||||||||||
{"Case 2: createDataKey and encrypt with HTTP retry", "http"}, | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
for _, tc := range testCases { | ||||||||||||||||||||||||||||||||
for _, dataKey := range dataKeys { | ||||||||||||||||||||||||||||||||
mt.Run(fmt.Sprintf("%s_%s", tc.name, dataKey.provider), func(mt *mtest.T) { | ||||||||||||||||||||||||||||||||
keyVaultClient, err := mongo.Connect(options.Client().ApplyURI(mtest.ClusterURI())) | ||||||||||||||||||||||||||||||||
require.NoError(mt, err, "error on Connect: %v", err) | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
ceo := options.ClientEncryption(). | ||||||||||||||||||||||||||||||||
SetKeyVaultNamespace(kvNamespace). | ||||||||||||||||||||||||||||||||
SetKmsProviders(kmsProviders). | ||||||||||||||||||||||||||||||||
SetTLSConfig(map[string]*tls.Config{dataKey.provider: tlsCfg}) | ||||||||||||||||||||||||||||||||
clientEncryption, err := mongo.NewClientEncryption(keyVaultClient, ceo) | ||||||||||||||||||||||||||||||||
require.NoError(mt, err, "error on NewClientEncryption: %v", err) | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
err = setFailPoint(tc.failure, 1) | ||||||||||||||||||||||||||||||||
require.NoError(mt, err, "mock server error: %v", err) | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
dkOpts := options.DataKey().SetMasterKey(dataKey.masterKey) | ||||||||||||||||||||||||||||||||
var keyID bson.Binary | ||||||||||||||||||||||||||||||||
keyID, err = clientEncryption.CreateDataKey(context.Background(), dataKey.provider, dkOpts) | ||||||||||||||||||||||||||||||||
require.NoError(mt, err, "error in CreateDataKey: %v", err) | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
err = setFailPoint(tc.failure, 1) | ||||||||||||||||||||||||||||||||
require.NoError(mt, err, "mock server error: %v", err) | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
testVal := bson.RawValue{Type: bson.TypeInt32, Value: bsoncore.AppendInt32(nil, 123)} | ||||||||||||||||||||||||||||||||
eo := options.Encrypt(). | ||||||||||||||||||||||||||||||||
SetKeyID(keyID). | ||||||||||||||||||||||||||||||||
SetAlgorithm("AEAD_AES_256_CBC_HMAC_SHA_512-Deterministic") | ||||||||||||||||||||||||||||||||
_, err = clientEncryption.Encrypt(context.Background(), testVal, eo) | ||||||||||||||||||||||||||||||||
assert.NoError(mt, err, "error in Encrypt: %v", err) | ||||||||||||||||||||||||||||||||
qingyang-hu marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
for _, dataKey := range dataKeys { | ||||||||||||||||||||||||||||||||
mt.Run(fmt.Sprintf("Case 3: createDataKey fails after too many retries_%s", dataKey.provider), func(mt *mtest.T) { | ||||||||||||||||||||||||||||||||
keyVaultClient, err := mongo.Connect(options.Client().ApplyURI(mtest.ClusterURI())) | ||||||||||||||||||||||||||||||||
require.NoError(mt, err, "error on Connect: %v", err) | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
ceo := options.ClientEncryption(). | ||||||||||||||||||||||||||||||||
SetKeyVaultNamespace(kvNamespace). | ||||||||||||||||||||||||||||||||
SetKmsProviders(kmsProviders). | ||||||||||||||||||||||||||||||||
SetTLSConfig(map[string]*tls.Config{dataKey.provider: tlsCfg}) | ||||||||||||||||||||||||||||||||
clientEncryption, err := mongo.NewClientEncryption(keyVaultClient, ceo) | ||||||||||||||||||||||||||||||||
require.NoError(mt, err, "error on NewClientEncryption: %v", err) | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
err = setFailPoint("network", 4) | ||||||||||||||||||||||||||||||||
require.NoError(mt, err, "mock server error: %v", err) | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
dkOpts := options.DataKey().SetMasterKey(dataKey.masterKey) | ||||||||||||||||||||||||||||||||
_, err = clientEncryption.CreateDataKey(context.Background(), dataKey.provider, dkOpts) | ||||||||||||||||||||||||||||||||
require.ErrorContains(mt, err, "KMS request failed after 3 retries due to a network error") | ||||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
func getWatcher(mt *mtest.T, streamType mongo.StreamType, cpt *cseProseTest) watcher { | ||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function seems like mostly a duplicate of
start-cse-servers
. Can we use that existing function instead of this one?