From 49a59bda5ebdc8beb0a85af25c7be46b270ad4fe Mon Sep 17 00:00:00 2001 From: miagilepner Date: Thu, 25 Jan 2024 14:45:44 +0100 Subject: [PATCH] Fix api/ and sdk/ package tests (#25067) * fix * left in incorrectly * don't print generate commands * handle line breaks * remove -e --- .github/workflows/test-go.yml | 4 +- Makefile | 14 +- api/client_test.go | 8 +- sdk/framework/openapi.go | 30 ++-- sdk/framework/openapi_test.go | 16 +- sdk/framework/testdata/operations.json | 140 ++++++++++-------- sdk/framework/testdata/operations_list.json | 65 +++++--- .../clientcountutil/clientcountutil_test.go | 6 +- sdk/helper/pluginutil/run_config_test.go | 9 +- sdk/helper/testcluster/replication.go | 2 +- sdk/plugin/grpc_backend_test.go | 2 +- 11 files changed, 178 insertions(+), 118 deletions(-) diff --git a/.github/workflows/test-go.yml b/.github/workflows/test-go.yml index 199a479660..e0002d731a 100644 --- a/.github/workflows/test-go.yml +++ b/.github/workflows/test-go.yml @@ -141,7 +141,7 @@ jobs: run: | # testonly tests need additional build tag though let's exclude them anyway for clarity ( - go list ./... | grep -v "_binary" | grep -v "vault/integ" | grep -v "testonly" | gotestsum tool ci-matrix --debug \ + make all-packages | grep -v "_binary" | grep -v "vault/integ" | grep -v "testonly" | gotestsum tool ci-matrix --debug \ --partitions "${{ inputs.total-runners }}" \ --timing-files 'test-results/go-test/*.json' > matrix.json ) @@ -166,7 +166,7 @@ jobs: if: inputs.binary-tests id: list-binary-tests run: | - LIST="$(go list ./... | grep "_binary" | xargs)" + LIST="$(make all-packages | grep "_binary" | xargs)" echo "list=$LIST" >> "$GITHUB_OUTPUT" - name: Build complete matrix id: build diff --git a/Makefile b/Makefile index 50c6e92a7a..c345c5fed0 100644 --- a/Makefile +++ b/Makefile @@ -2,7 +2,11 @@ # Be sure to place this BEFORE `include` directives, if any. THIS_FILE := $(lastword $(MAKEFILE_LIST)) -TEST?=$$($(GO_CMD) list ./... | grep -v /vendor/ | grep -v /integ) +MAIN_PACKAGES=$$($(GO_CMD) list ./... | grep -v vendor/ ) +SDK_PACKAGES=$$(cd $(CURDIR)/sdk && $(GO_CMD) list ./... | grep -v vendor/ ) +API_PACKAGES=$$(cd $(CURDIR)/api && $(GO_CMD) list ./... | grep -v vendor/ ) +ALL_PACKAGES=$(MAIN_PACKAGES) $(SDK_PACKAGES) $(API_PACKAGES) +TEST=$$(echo $(ALL_PACKAGES) | grep -v integ/ ) TEST_TIMEOUT?=45m EXTENDED_TEST_TIMEOUT=60m INTEG_TEST_TIMEOUT=120m @@ -156,7 +160,9 @@ protolint: prep check-tools-external # dependency. prep: check-go-version @echo "==> Running go generate..." - @GOARCH= GOOS= $(GO_CMD) generate $$($(GO_CMD) list ./... | grep -v /vendor/) + @GOARCH= GOOS= $(GO_CMD) generate $(MAIN_PACKAGES) + @GOARCH= GOOS= cd api && $(GO_CMD) generate $(API_PACKAGES) + @GOARCH= GOOS= cd sdk && $(GO_CMD) generate $(SDK_PACKAGES) @if [ -d .git/hooks ]; then cp .hooks/* .git/hooks/; fi # bootstrap the build by generating any necessary code and downloading additional tools that may @@ -369,3 +375,7 @@ ci-copywriteheaders: .PHONY: all bin default prep test vet bootstrap fmt fmtcheck mysql-database-plugin mysql-legacy-database-plugin cassandra-database-plugin influxdb-database-plugin postgresql-database-plugin mssql-database-plugin hana-database-plugin mongodb-database-plugin ember-dist ember-dist-dev static-dist static-dist-dev assetcheck check-vault-in-path packages build build-ci semgrep semgrep-ci vet-codechecker ci-vet-codechecker clean dev .NOTPARALLEL: ember-dist ember-dist-dev + +.PHONY: all-packages +all-packages: + @echo $(ALL_PACKAGES) | tr ' ' '\n' diff --git a/api/client_test.go b/api/client_test.go index 7a55fe8c43..a3693a2fbc 100644 --- a/api/client_test.go +++ b/api/client_test.go @@ -86,7 +86,7 @@ func TestClientDefaultHttpClient_unixSocket(t *testing.T) { if client.addr.Scheme != "http" { t.Fatalf("bad: %s", client.addr.Scheme) } - if client.addr.Host != "/var/run/vault.sock" { + if client.addr.Host != "localhost" { t.Fatalf("bad: %s", client.addr.Host) } } @@ -111,8 +111,8 @@ func TestClientSetAddress(t *testing.T) { if client.addr.Scheme != "http" { t.Fatalf("bad: expected: 'http' actual: %q", client.addr.Scheme) } - if client.addr.Host != "/var/run/vault.sock" { - t.Fatalf("bad: expected: '/var/run/vault.sock' actual: %q", client.addr.Host) + if client.addr.Host != "localhost" { + t.Fatalf("bad: expected: 'localhost' actual: %q", client.addr.Host) } if client.addr.Path != "" { t.Fatalf("bad: expected '' actual: %q", client.addr.Path) @@ -1521,7 +1521,7 @@ func TestParseAddressWithUnixSocket(t *testing.T) { if u.Scheme != "http" { t.Fatal("Scheme not changed to http") } - if u.Host != "/var/run/vault.sock" { + if u.Host != "localhost" { t.Fatal("Host not changed to socket name") } if u.Path != "" { diff --git a/sdk/framework/openapi.go b/sdk/framework/openapi.go index 6e76f8edc6..82e7f5fb64 100644 --- a/sdk/framework/openapi.go +++ b/sdk/framework/openapi.go @@ -1174,24 +1174,26 @@ func hyphenatedToTitleCase(in string) string { // cleanedResponse is identical to logical.Response but with nulls // removed from from JSON encoding type cleanedResponse struct { - Secret *logical.Secret `json:"secret,omitempty"` - Auth *logical.Auth `json:"auth,omitempty"` - Data map[string]interface{} `json:"data,omitempty"` - Redirect string `json:"redirect,omitempty"` - Warnings []string `json:"warnings,omitempty"` - WrapInfo *wrapping.ResponseWrapInfo `json:"wrap_info,omitempty"` - Headers map[string][]string `json:"headers,omitempty"` + Secret *logical.Secret `json:"secret,omitempty"` + Auth *logical.Auth `json:"auth,omitempty"` + Data map[string]interface{} `json:"data,omitempty"` + Redirect string `json:"redirect,omitempty"` + Warnings []string `json:"warnings,omitempty"` + WrapInfo *wrapping.ResponseWrapInfo `json:"wrap_info,omitempty"` + Headers map[string][]string `json:"headers,omitempty"` + MountType string `json:"mount_type,omitempty"` } func cleanResponse(resp *logical.Response) *cleanedResponse { return &cleanedResponse{ - Secret: resp.Secret, - Auth: resp.Auth, - Data: resp.Data, - Redirect: resp.Redirect, - Warnings: resp.Warnings, - WrapInfo: resp.WrapInfo, - Headers: resp.Headers, + Secret: resp.Secret, + Auth: resp.Auth, + Data: resp.Data, + Redirect: resp.Redirect, + Warnings: resp.Warnings, + WrapInfo: resp.WrapInfo, + Headers: resp.Headers, + MountType: resp.MountType, } } diff --git a/sdk/framework/openapi_test.go b/sdk/framework/openapi_test.go index 0b6185b4d6..4cb94342ff 100644 --- a/sdk/framework/openapi_test.go +++ b/sdk/framework/openapi_test.go @@ -655,13 +655,14 @@ func TestOpenAPI_CleanResponse(t *testing.T) { // logical.Response. This will fail if logical.Response changes without a corresponding // change to cleanResponse() orig = &logical.Response{ - Secret: new(logical.Secret), - Auth: new(logical.Auth), - Data: map[string]interface{}{"foo": 42}, - Redirect: "foo", - Warnings: []string{"foo"}, - WrapInfo: &wrapping.ResponseWrapInfo{Token: "foo"}, - Headers: map[string][]string{"foo": {"bar"}}, + Secret: new(logical.Secret), + Auth: new(logical.Auth), + Data: map[string]interface{}{"foo": 42}, + Redirect: "foo", + Warnings: []string{"foo"}, + WrapInfo: &wrapping.ResponseWrapInfo{Token: "foo"}, + Headers: map[string][]string{"foo": {"bar"}}, + MountType: "mount", } origJSON := mustJSONMarshal(t, orig) @@ -900,7 +901,6 @@ func testPath(t *testing.T, path *Path, sp *logical.Paths, expectedJSON string) if err != nil { t.Fatal(err) } - // Compare json by first decoding, then comparing with a deep equality check. var expected, actual interface{} if err := jsonutil.DecodeJSON(docJSON, &actual); err != nil { diff --git a/sdk/framework/testdata/operations.json b/sdk/framework/testdata/operations.json index 91bd64d270..8e9ec9b8d0 100644 --- a/sdk/framework/testdata/operations.json +++ b/sdk/framework/testdata/operations.json @@ -12,20 +12,7 @@ "paths": { "/foo/{id}": { "description": "Synopsis", - "x-vault-createSupported": true, - "x-vault-sudo": true, - "x-vault-displayAttrs": { - "navigation": true - }, "parameters": [ - { - "name": "format", - "description": "a query param", - "in": "query", - "schema": { - "type": "string" - } - }, { "name": "id", "description": "id path parameter", @@ -36,13 +23,28 @@ "required": true } ], + "x-vault-sudo": true, + "x-vault-createSupported": true, + "x-vault-displayAttrs": { + "navigation": true + }, "get": { + "summary": "My Summary", + "description": "My Description", "operationId": "kv-read-foo-id", "tags": [ "secrets" ], - "summary": "My Summary", - "description": "My Description", + "parameters": [ + { + "name": "format", + "description": "a query param", + "in": "query", + "schema": { + "type": "string" + } + } + ], "responses": { "200": { "description": "OK" @@ -50,12 +52,12 @@ } }, "post": { + "summary": "Update Summary", + "description": "Update Description", "operationId": "kv-write-foo-id", "tags": [ "secrets" ], - "summary": "Update Summary", - "description": "Update Description", "requestBody": { "required": true, "content": { @@ -75,19 +77,7 @@ }, "/foo/{id}/": { "description": "Synopsis", - "x-vault-sudo": true, - "x-vault-displayAttrs": { - "navigation": true - }, "parameters": [ - { - "name": "format", - "description": "a query param", - "in": "query", - "schema": { - "type": "string" - } - }, { "name": "id", "description": "id path parameter", @@ -98,32 +88,51 @@ "required": true } ], + "x-vault-sudo": true, + "x-vault-displayAttrs": { + "navigation": true + }, "get": { + "summary": "List Summary", + "description": "List Description", "operationId": "kv-list-foo-id", "tags": [ "secrets" ], - "summary": "List Summary", - "description": "List Description", - "responses": { - "200": { - "description": "OK" - } - }, "parameters": [ + { + "name": "format", + "description": "a query param", + "in": "query", + "schema": { + "type": "string" + } + }, { "name": "list", "description": "Must be set to `true`", - "required": true, "in": "query", "schema": { "type": "string", "enum": [ "true" ] + }, + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/StandardListResponse" + } + } } } - ] + } } } }, @@ -131,17 +140,7 @@ "schemas": { "KvWriteFooIdRequest": { "type": "object", - "required": [ - "age" - ], "properties": { - "flavors": { - "type": "array", - "description": "the flavors", - "items": { - "type": "string" - } - }, "age": { "type": "integer", "description": "the age", @@ -152,16 +151,32 @@ ], "x-vault-displayAttrs": { "name": "Age", + "value": 7, "sensitive": true, - "group": "Some Group", - "value": 7 + "group": "Some Group" } }, + "flavors": { + "type": "array", + "description": "the flavors", + "items": { + "type": "string" + } + }, + "format": { + "type": "string", + "description": "a query param" + }, + "maximum": { + "type": "integer", + "description": "a maximum value", + "format": "int64" + }, "name": { "type": "string", "description": "the name", - "default": "Larry", - "pattern": "\\w([\\w-.]*\\w)?" + "pattern": "\\w([\\w-.]*\\w)?", + "default": "Larry" }, "x-abc-token": { "type": "string", @@ -171,14 +186,23 @@ "b", "c" ] - }, - "maximum": { - "type": "integer", - "description": "a maximum value", - "format": "int64" + } + }, + "required": [ + "age" + ] + }, + "StandardListResponse": { + "type": "object", + "properties": { + "keys": { + "type": "array", + "items": { + "type": "string" + } } } } } } -} +} \ No newline at end of file diff --git a/sdk/framework/testdata/operations_list.json b/sdk/framework/testdata/operations_list.json index d7bc50187c..feb7b2ccba 100644 --- a/sdk/framework/testdata/operations_list.json +++ b/sdk/framework/testdata/operations_list.json @@ -12,19 +12,7 @@ "paths": { "/foo/{id}/": { "description": "Synopsis", - "x-vault-sudo": true, - "x-vault-displayAttrs": { - "navigation": true - }, "parameters": [ - { - "name": "format", - "description": "a query param", - "in": "query", - "schema": { - "type": "string" - } - }, { "name": "id", "description": "id path parameter", @@ -35,36 +23,67 @@ "required": true } ], + "x-vault-sudo": true, + "x-vault-displayAttrs": { + "navigation": true + }, "get": { + "summary": "List Summary", + "description": "List Description", "operationId": "kv-list-foo-id", "tags": [ "secrets" ], - "summary": "List Summary", - "description": "List Description", - "responses": { - "200": { - "description": "OK" - } - }, "parameters": [ + { + "name": "format", + "description": "a query param", + "in": "query", + "schema": { + "type": "string" + } + }, { "name": "list", "description": "Must be set to `true`", - "required": true, "in": "query", "schema": { "type": "string", "enum": [ "true" ] + }, + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/StandardListResponse" + } + } } } - ] + } } } }, "components": { - "schemas": {} + "schemas": { + "StandardListResponse": { + "type": "object", + "properties": { + "keys": { + "type": "array", + "items": { + "type": "string" + } + } + } + } + } } -} +} \ No newline at end of file diff --git a/sdk/helper/clientcountutil/clientcountutil_test.go b/sdk/helper/clientcountutil/clientcountutil_test.go index 6d97655317..6a5b224bc6 100644 --- a/sdk/helper/clientcountutil/clientcountutil_test.go +++ b/sdk/helper/clientcountutil/clientcountutil_test.go @@ -5,6 +5,7 @@ package clientcountutil import ( "context" + "encoding/json" "io" "net/http" "net/http/httptest" @@ -119,7 +120,10 @@ func TestWrite(t *testing.T) { require.NoError(t, err) body, err := io.ReadAll(r.Body) require.NoError(t, err) - require.JSONEq(t, `{"write":["WRITE_ENTITIES"],"data":[{"monthsAgo":3,"all":{"clients":[{"count":1}]}},{"monthsAgo":2,"segments":{"segments":[{"segmentIndex":2,"clients":{"clients":[{"count":1,"repeated":true}]}}]}},{"currentMonth":true}]}`, string(body)) + raw := map[string]string{} + err = json.Unmarshal(body, &raw) + require.NoError(t, err) + require.JSONEq(t, `{"write":["WRITE_ENTITIES"],"data":[{"monthsAgo":3,"all":{"clients":[{"count":1}]}},{"monthsAgo":2,"segments":{"segments":[{"segmentIndex":2,"clients":{"clients":[{"count":1,"repeated":true}]}}]}},{"currentMonth":true}]}`, raw["input"]) })) defer ts.Close() diff --git a/sdk/helper/pluginutil/run_config_test.go b/sdk/helper/pluginutil/run_config_test.go index 25a950725c..dcd59dfa7b 100644 --- a/sdk/helper/pluginutil/run_config_test.go +++ b/sdk/helper/pluginutil/run_config_test.go @@ -322,7 +322,7 @@ func TestMakeConfig(t *testing.T) { responseWrapInfoTimes: 0, mlockEnabled: false, - mlockEnabledTimes: 1, + mlockEnabledTimes: 2, expectedConfig: &plugin.ClientConfig{ HandshakeConfig: plugin.HandshakeConfig{ @@ -341,9 +341,10 @@ func TestMakeConfig(t *testing.T) { plugin.ProtocolNetRPC, plugin.ProtocolGRPC, }, - Logger: hclog.NewNullLogger(), - AutoMTLS: true, - SkipHostEnv: true, + Logger: hclog.NewNullLogger(), + AutoMTLS: true, + SkipHostEnv: true, + GRPCBrokerMultiplex: true, UnixSocketConfig: &plugin.UnixSocketConfig{ Group: strconv.Itoa(os.Getgid()), }, diff --git a/sdk/helper/testcluster/replication.go b/sdk/helper/testcluster/replication.go index 6a541884a7..6f99581574 100644 --- a/sdk/helper/testcluster/replication.go +++ b/sdk/helper/testcluster/replication.go @@ -258,7 +258,7 @@ func WaitForPerfReplicationWorking(ctx context.Context, pri, sec VaultCluster) e "bar": 1, }) if err != nil { - return fmt.Errorf("unable to write KV on primary", "path", path) + return fmt.Errorf("unable to write KV on primary, path=%s", path) } for ctx.Err() == nil { diff --git a/sdk/plugin/grpc_backend_test.go b/sdk/plugin/grpc_backend_test.go index 01a6ea609f..880f09930f 100644 --- a/sdk/plugin/grpc_backend_test.go +++ b/sdk/plugin/grpc_backend_test.go @@ -177,7 +177,7 @@ func testGRPCBackend(t *testing.T) (logical.Backend, func()) { }), }, } - client, _ := gplugin.TestPluginGRPCConn(t, pluginMap) + client, _ := gplugin.TestPluginGRPCConn(t, false, pluginMap) cleanup := func() { client.Close() }