From 67a3e6ea13a2df129a7f2a6e01e607ec1ba4d594 Mon Sep 17 00:00:00 2001 From: Sean Kane <68240067+seankane-msft@users.noreply.github.com> Date: Tue, 7 Dec 2021 10:35:51 -0500 Subject: [PATCH] [Internal] Variable storage (#16375) * adding variables API * update changelog, fixing body * checking error value on stop * adding Close to response bodies, doc comments * fixing close on Stop method * updating for release --- sdk/internal/CHANGELOG.md | 3 +- sdk/internal/recording/recording.go | 58 +++++++++++++++++++++++- sdk/internal/recording/recording_test.go | 49 +++++++++++++++----- sdk/internal/recording/sanitizer.go | 1 + 4 files changed, 96 insertions(+), 15 deletions(-) diff --git a/sdk/internal/CHANGELOG.md b/sdk/internal/CHANGELOG.md index 51d757092dd8..945e6a76e902 100644 --- a/sdk/internal/CHANGELOG.md +++ b/sdk/internal/CHANGELOG.md @@ -1,12 +1,13 @@ # Release History -## 0.8.3 (2021-11-30) +## 0.8.3 (2021-12-07) ### Features Added * If `AZURE_RECORD_MODE` is not set, default to `playback` * If `PROXY_CERT` is not set, try to find it based on the `GOPATH` environment variable and the path to `eng/common/testproxy/dotnet-devcert.crt` * Adds `NewRecordingHTTPClient()` method which returns an `azcore.Transporter` interface that routes requests to the test proxy [#16221](https://github.com/Azure/azure-sdk-for-go/pull/16221). * Adds the `SetBodilessMatcher` method [#16256](https://github.com/Azure/azure-sdk-for-go/pull/16256) +* Added variables storage to the `Stop` function. Pass in a `map[string]interface{}` to the `Stop` method options and the values can be retrieved with the `GetVariables(t *testing.T)` function [#16375](https://github.com/Azure/azure-sdk-for-go/pull/16375). ### Breaking Changes * Renames `ResetSanitizers` to `ResetProxy` [#16256](https://github.com/Azure/azure-sdk-for-go/pull/16256) diff --git a/sdk/internal/recording/recording.go b/sdk/internal/recording/recording.go index 418211de1c3b..b8e8363cf2c2 100644 --- a/sdk/internal/recording/recording.go +++ b/sdk/internal/recording/recording.go @@ -10,6 +10,7 @@ import ( "bytes" "crypto/tls" "crypto/x509" + "encoding/json" "errors" "fmt" "io/ioutil" @@ -492,6 +493,7 @@ const ( type recordedTest struct { recordingId string liveOnly bool + variables map[string]interface{} } var testSuite = map[string]recordedTest{} @@ -505,6 +507,7 @@ var client = http.Client{ type RecordingOptions struct { UseHTTPS bool GroupForReplace string + Variables map[string]interface{} } func defaultOptions() *RecordingOptions { @@ -548,6 +551,7 @@ func getTestId(pathToRecordings string, t *testing.T) string { return path.Join(pathToRecordings, "recordings", t.Name()+".json") } +// Start tells the test proxy to begin accepting requests for a given test func Start(t *testing.T, pathToRecordings string, options *RecordingOptions) error { if options == nil { options = defaultOptions() @@ -580,20 +584,42 @@ func Start(t *testing.T, pathToRecordings string, options *RecordingOptions) err recId := resp.Header.Get(IDHeader) if recId == "" { b, err := ioutil.ReadAll(resp.Body) + defer resp.Body.Close() if err != nil { return err } return fmt.Errorf("Recording ID was not returned by the response. Response body: %s", b) } + + // Unmarshal any variables returned by the proxy + var m map[string]interface{} + body, err := ioutil.ReadAll(resp.Body) + defer resp.Body.Close() + if err != nil { + return err + } + if len(body) > 0 { + err = json.Unmarshal(body, &m) + if err != nil { + return err + } + } + if val, ok := testSuite[t.Name()]; ok { val.recordingId = recId + val.variables = m testSuite[t.Name()] = val } else { - testSuite[t.Name()] = recordedTest{recordingId: recId, liveOnly: false} + testSuite[t.Name()] = recordedTest{ + recordingId: recId, + liveOnly: false, + variables: m, + } } return nil } +// Stop tells the test proxy to stop accepting requests for a given test func Stop(t *testing.T, options *RecordingOptions) error { if options == nil { options = defaultOptions() @@ -614,13 +640,32 @@ func Stop(t *testing.T, options *RecordingOptions) error { if err != nil { return err } + if len(options.Variables) > 0 { + req.Header.Set("Content-Type", "application/json") + marshalled, err := json.Marshal(options.Variables) + if err != nil { + return err + } + req.Body = ioutil.NopCloser(bytes.NewReader(marshalled)) + req.ContentLength = int64(len(marshalled)) + } + var recTest recordedTest var ok bool if recTest, ok = testSuite[t.Name()]; !ok { return errors.New("Recording ID was never set. Did you call StartRecording?") } req.Header.Set("x-recording-id", recTest.recordingId) - _, err = client.Do(req) + resp, err := client.Do(req) + if resp.StatusCode != 200 { + b, err := ioutil.ReadAll(resp.Body) + defer resp.Body.Close() + if err == nil { + return fmt.Errorf("proxy did not stop the recording properly: %s", string(b)) + } + return fmt.Errorf("proxy did not stop the recording properly: %s", err.Error()) + } + _ = resp.Body.Close() return err } @@ -687,6 +732,7 @@ func (c RecordingHTTPClient) Do(req *http.Request) (*http.Response, error) { return c.defaultClient.Do(req) } +// NewRecordingHTTPClient returns a type that implements `azcore.Transporter`. This will automatically route tests on the `Do` call. func NewRecordingHTTPClient(t *testing.T, options *RecordingOptions) (*RecordingHTTPClient, error) { if options == nil { options = &RecordingOptions{UseHTTPS: true} @@ -721,3 +767,11 @@ func IsLiveOnly(t *testing.T) bool { } return false } + +// GetVariables returns access to the variables stored by the test proxy for a specific test +func GetVariables(t *testing.T) map[string]interface{} { + if s, ok := testSuite[t.Name()]; ok { + return s.variables + } + return nil +} diff --git a/sdk/internal/recording/recording_test.go b/sdk/internal/recording/recording_test.go index b75f577b0599..ea75e52dff38 100644 --- a/sdk/internal/recording/recording_test.go +++ b/sdk/internal/recording/recording_test.go @@ -569,22 +569,47 @@ func TestFindProxyCertLocation(t *testing.T) { require.Contains(t, location, filepath.Join("eng", "common", "testproxy", "dotnet-devcert.crt")) } -func TestModeNotSet(t *testing.T) { - proxyMode, _ := os.LookupEnv("AZURE_RECORD_MODE") - defer os.Setenv("AZURE_RECORD_MODE", proxyMode) +func TestVariables(t *testing.T) { + temp := recordMode + recordMode = RecordingMode + defer func() { recordMode = temp }() - os.Unsetenv("AZURE_RECORD_MODE") - require.Equal(t, PlaybackMode, GetRecordMode()) -} + err := Start(t, packagePath, nil) + require.NoError(t, err) + + client, err := NewRecordingHTTPClient(t, nil) + require.NoError(t, err) -func TestModeNotSetStartStop(t *testing.T) { - proxyMode, _ := os.LookupEnv("AZURE_RECORD_MODE") - defer os.Setenv("AZURE_RECORD_MODE", proxyMode) + req, err := http.NewRequest("POST", "https://azsdkengsys.azurecr.io/acr/v1/some_registry/_tags", nil) + require.NoError(t, err) - os.Unsetenv("AZURE_RECORD_MODE") - err := Start(t, packagePath, nil) + resp, err := client.Do(req) + require.NoError(t, err) + require.NotNil(t, resp) + + require.NotNil(t, GetRecordingId(t)) + + err = Stop(t, &RecordingOptions{Variables: map[string]interface{}{"key1": "value1", "key2": "1"}}) + require.NoError(t, err) + + recordMode = PlaybackMode + err = Start(t, packagePath, nil) require.NoError(t, err) - require.Equal(t, PlaybackMode, GetRecordMode()) + + variables := GetVariables(t) + require.Equal(t, variables["key1"], "value1") + require.Equal(t, variables["key2"], "1") + err = Stop(t, nil) require.NoError(t, err) + + // Make sure the file is there + jsonFile, err := os.Open(fmt.Sprintf("./testdata/recordings/%s.json", t.Name())) + require.NoError(t, err) + defer func() { + err = jsonFile.Close() + require.NoError(t, err) + err = os.Remove(jsonFile.Name()) + require.NoError(t, err) + }() } diff --git a/sdk/internal/recording/sanitizer.go b/sdk/internal/recording/sanitizer.go index 2074726b691b..b765a36aa18d 100644 --- a/sdk/internal/recording/sanitizer.go +++ b/sdk/internal/recording/sanitizer.go @@ -102,6 +102,7 @@ func handleProxyResponse(resp *http.Response, err error) error { } body, err := ioutil.ReadAll(resp.Body) + defer resp.Body.Close() if err != nil { return err }