From 18cf52b64713ded037b6c40ea5e07b49f68019e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Bourgois?= Date: Thu, 13 Feb 2020 14:22:19 +0100 Subject: [PATCH 1/4] wip: fix: properly handle maps in unmarshaller --- internal/args/args_test.go | 7 +++ internal/args/unmarshal_test.go | 32 ++++++++++++ .../instance/v1/instance_cli_test.go | 18 +++++++ ...st-image-create-create-image.cassette.yaml | 36 ++++++++++++++ ...st-image-create-create-image.stderr.golden | 49 +++++++++++++++++++ 5 files changed, 142 insertions(+) create mode 100644 internal/namespaces/instance/v1/testdata/test-image-create-create-image.cassette.yaml create mode 100644 internal/namespaces/instance/v1/testdata/test-image-create-create-image.stderr.golden diff --git a/internal/args/args_test.go b/internal/args/args_test.go index cd6018cfff..c05f734550 100644 --- a/internal/args/args_test.go +++ b/internal/args/args_test.go @@ -81,6 +81,13 @@ type Enum struct { Size Size } +type RecursiveWithMapOfRecursive struct { + ID int + Name string + Short string + Elements map[string]*RecursiveWithMapOfRecursive +} + func (c *CustomStruct) UnmarshalArgs(value string) error { c.value = strings.ToUpper(value) return nil diff --git a/internal/args/unmarshal_test.go b/internal/args/unmarshal_test.go index cc3845e4ae..be2e564bfa 100644 --- a/internal/args/unmarshal_test.go +++ b/internal/args/unmarshal_test.go @@ -331,6 +331,38 @@ func TestUnmarshalStruct(t *testing.T) { All: "all", }, })) + + t.Run("recursive-with-map-of-recursive-with-one-field-set", run(TestCase{ + args: []string{ + "name=coucou", + "elements.0.name=bob", + }, + expected: &RecursiveWithMapOfRecursive{ + Name: "coucou", + Elements: map[string]*RecursiveWithMapOfRecursive{ + "0": { + Name: "bob", + }, + }, + }, + })) + + t.Run("recursive-with-map-of-recursive-with-multiple-fields-set", run(TestCase{ + args: []string{ + "name=coucou", + "elements.0.id=1453", + "elements.0.name=bob", + }, + expected: &RecursiveWithMapOfRecursive{ + Name: "coucou", + Elements: map[string]*RecursiveWithMapOfRecursive{ + "0": { + ID: 1453, + Name: "bob", + }, + }, + }, + })) } func TestIsUmarshalableValue(t *testing.T) { diff --git a/internal/namespaces/instance/v1/instance_cli_test.go b/internal/namespaces/instance/v1/instance_cli_test.go index 54e528e17f..679693a72f 100644 --- a/internal/namespaces/instance/v1/instance_cli_test.go +++ b/internal/namespaces/instance/v1/instance_cli_test.go @@ -316,3 +316,21 @@ func Test_ServerUpdate(t *testing.T) { }, })) } + +// TODO: add proper test +func Test_ImageCreate(t *testing.T) { + t.Run(`Create image`, core.Test(&core.TestConfig{ + Commands: GetCommands(), + BeforeFunc: func(ctx *core.BeforeFuncCtx) error { + return nil + }, + Cmd: `scw instance -D image create name=foobar root-volume=6fd63468-b198-42cf-8462-81df850a7a8e arch=x86_64 extra-volumes.0.id=6fd63468-b198-42cf-8462-81df850a7a8e extra-volumes.0.name=foobar extra-volumes.0.volume-type=l_ssd`, + Check: core.TestCheckCombine( + core.TestCheckExitCode(1), + core.TestCheckGolden(), + ), + AfterFunc: func(ctx *core.AfterFuncCtx) error { + return nil + }, + })) +} diff --git a/internal/namespaces/instance/v1/testdata/test-image-create-create-image.cassette.yaml b/internal/namespaces/instance/v1/testdata/test-image-create-create-image.cassette.yaml new file mode 100644 index 0000000000..eba6897ff0 --- /dev/null +++ b/internal/namespaces/instance/v1/testdata/test-image-create-create-image.cassette.yaml @@ -0,0 +1,36 @@ +--- +version: 1 +interactions: +- request: + body: '{"name":"foobar","root_volume":"6fd63468-b198-42cf-8462-81df850a7a8e","arch":"x86_64","extra_volumes":{"0":{"volume_type":"l_ssd"}},"organization":"aba2d0d0-b01d-4d88-b322-935edc96d0fd"}' + form: {} + headers: + Content-Type: + - application/json + User-Agent: + - scaleway-sdk-go/v1.0.0-beta.5+dev (go1.13.1; darwin; amd64) cli-e2e-test + url: https://api.scaleway.com/instance/v1/zones/fr-par-1/images + method: POST + response: + body: '{"type": "invalid_request_error", "message": "Validation Error", "fields": + {"extra_volumes.0": ["extra keys not allowed"]}}' + headers: + Content-Length: + - "123" + Content-Security-Policy: + - default-src 'none'; frame-ancestors 'none' + Content-Type: + - application/json + Date: + - Thu, 13 Feb 2020 12:46:49 GMT + Server: + - scaleway_api + Strict-Transport-Security: + - max-age=63072000 + X-Content-Type-Options: + - nosniff + X-Frame-Options: + - DENY + status: 400 Bad Request + code: 400 + duration: "" diff --git a/internal/namespaces/instance/v1/testdata/test-image-create-create-image.stderr.golden b/internal/namespaces/instance/v1/testdata/test-image-create-create-image.stderr.golden new file mode 100644 index 0000000000..2dd452f60d --- /dev/null +++ b/internal/namespaces/instance/v1/testdata/test-image-create-create-image.stderr.golden @@ -0,0 +1,49 @@ +DEBUG: 2019/12/09 16:04:07 could not validate arg value for 'extra-volumes.{key}.id': invalid fieldName: ExtraVolumes.id +DEBUG: 2019/12/09 16:04:07 could not validate arg value for 'extra-volumes.{key}.name': invalid fieldName: ExtraVolumes.name +DEBUG: 2019/12/09 16:04:07 could not validate arg value for 'extra-volumes.{key}.size': invalid fieldName: ExtraVolumes.size +DEBUG: 2019/12/09 16:04:07 could not validate arg value for 'extra-volumes.{key}.volume-type': invalid fieldName: ExtraVolumes.volumeType +DEBUG: 2019/12/09 16:04:07 could not validate arg value for 'extra-volumes.{key}.organization': invalid fieldName: ExtraVolumes.organization +DEBUG: 2019/12/09 16:04:07 creating POST request on https://api.scaleway.com/instance/v1/zones/fr-par-1/images +DEBUG: 2019/12/09 16:04:07 +--------------- Scaleway SDK REQUEST 1 : --------------- +POST /instance/v1/zones/fr-par-1/images HTTP/1.1 +Host: api.scaleway.com +User-Agent: scaleway-sdk-go/v1.0.0-beta.5+dev (go1.13.1; darwin; amd64) cli-e2e-test +Content-Length: 186 +Content-Type: application/json +X-Auth-Token: ee418114-xxxx-xxxx-xxxx-xxxxxxxxxxxx +Accept-Encoding: gzip + +{"name":"foobar","root_volume":"6fd63468-b198-42cf-8462-81df850a7a8e","arch":"x86_64","extra_volumes":{"0":{"volume_type":"l_ssd"}},"organization":"aba2d0d0-b01d-4d88-b322-935edc96d0fd"} +--------------------------------------------------------- +DEBUG: 2019/12/09 16:04:07 +--------------- Scaleway SDK RESPONSE 1 : --------------- +HTTP/1.0 400 Bad Request +Connection: close +Content-Length: 123 +Content-Security-Policy: default-src 'none'; frame-ancestors 'none' +Content-Type: application/json +Date: Thu, 13 Feb 2020 12:46:49 GMT +Server: scaleway_api +Strict-Transport-Security: max-age=63072000 +X-Content-Type-Options: nosniff +X-Frame-Options: DENY + +{"type": "invalid_request_error", "message": "Validation Error", "fields": {"extra_volumes.0": ["extra keys not allowed"]}} +---------------------------------------------------------- +DEBUG: 2019/12/09 16:04:07 marshalling type '*scw.InvalidArgumentsError' +DEBUG: 2019/12/09 16:04:07 marshalling type '*core.CliError' +DEBUG: 2019/12/09 16:04:07 marshalling type '*errors.errorString' +DEBUG: 2019/12/09 16:04:07 marshalling type 'string' +DEBUG: 2019/12/09 16:04:07 marshalling type 'string' +DEBUG: 2019/12/09 16:04:07 marshalling type 'string' +DEBUG: 2019/12/09 16:04:07 marshalling type 'string' +Invalid arguments 'extra_volumes.0' + +Details: +- 'extra_volumes.0' does not respect constraint + +Hint: +Extra keys not allowed +DEBUG: 2019/12/09 16:04:07 skipping check version +DEBUG: 2019/12/09 16:04:07 skipping telemetry report From 9533f209db16d9ada275fcfb96d79ada95f1ab35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Bourgois?= Date: Thu, 13 Feb 2020 15:05:03 +0100 Subject: [PATCH 2/4] fix --- internal/args/unmarshal.go | 15 ++++-- .../instance/v1/instance_cli_test.go | 18 ------- ...st-image-create-create-image.cassette.yaml | 36 -------------- ...st-image-create-create-image.stderr.golden | 49 ------------------- 4 files changed, 11 insertions(+), 107 deletions(-) delete mode 100644 internal/namespaces/instance/v1/testdata/test-image-create-create-image.cassette.yaml delete mode 100644 internal/namespaces/instance/v1/testdata/test-image-create-create-image.stderr.golden diff --git a/internal/args/unmarshal.go b/internal/args/unmarshal.go index 5143dcb3c6..7dec290a38 100644 --- a/internal/args/unmarshal.go +++ b/internal/args/unmarshal.go @@ -213,10 +213,17 @@ func set(dest reflect.Value, argNameWords []string, value string) error { if len(argNameWords) == 0 { return &MissingMapKeyError{} } - // Create a new value call set and add result in the map - newValue := reflect.New(dest.Type().Elem()) - err := set(newValue.Elem(), argNameWords[1:], value) - dest.SetMapIndex(reflect.ValueOf(argNameWords[0]), newValue.Elem()) + + // Create a new value if it does not exist, then call set and add result in the map + mapKey := reflect.ValueOf(argNameWords[0]) + mapValue := dest.MapIndex(mapKey) + + if !mapValue.IsValid() { + mapValue = reflect.New(dest.Type().Elem()).Elem() + } + err := set(mapValue, argNameWords[1:], value) + dest.SetMapIndex(mapKey, mapValue) + return err case reflect.Struct: diff --git a/internal/namespaces/instance/v1/instance_cli_test.go b/internal/namespaces/instance/v1/instance_cli_test.go index 679693a72f..54e528e17f 100644 --- a/internal/namespaces/instance/v1/instance_cli_test.go +++ b/internal/namespaces/instance/v1/instance_cli_test.go @@ -316,21 +316,3 @@ func Test_ServerUpdate(t *testing.T) { }, })) } - -// TODO: add proper test -func Test_ImageCreate(t *testing.T) { - t.Run(`Create image`, core.Test(&core.TestConfig{ - Commands: GetCommands(), - BeforeFunc: func(ctx *core.BeforeFuncCtx) error { - return nil - }, - Cmd: `scw instance -D image create name=foobar root-volume=6fd63468-b198-42cf-8462-81df850a7a8e arch=x86_64 extra-volumes.0.id=6fd63468-b198-42cf-8462-81df850a7a8e extra-volumes.0.name=foobar extra-volumes.0.volume-type=l_ssd`, - Check: core.TestCheckCombine( - core.TestCheckExitCode(1), - core.TestCheckGolden(), - ), - AfterFunc: func(ctx *core.AfterFuncCtx) error { - return nil - }, - })) -} diff --git a/internal/namespaces/instance/v1/testdata/test-image-create-create-image.cassette.yaml b/internal/namespaces/instance/v1/testdata/test-image-create-create-image.cassette.yaml deleted file mode 100644 index eba6897ff0..0000000000 --- a/internal/namespaces/instance/v1/testdata/test-image-create-create-image.cassette.yaml +++ /dev/null @@ -1,36 +0,0 @@ ---- -version: 1 -interactions: -- request: - body: '{"name":"foobar","root_volume":"6fd63468-b198-42cf-8462-81df850a7a8e","arch":"x86_64","extra_volumes":{"0":{"volume_type":"l_ssd"}},"organization":"aba2d0d0-b01d-4d88-b322-935edc96d0fd"}' - form: {} - headers: - Content-Type: - - application/json - User-Agent: - - scaleway-sdk-go/v1.0.0-beta.5+dev (go1.13.1; darwin; amd64) cli-e2e-test - url: https://api.scaleway.com/instance/v1/zones/fr-par-1/images - method: POST - response: - body: '{"type": "invalid_request_error", "message": "Validation Error", "fields": - {"extra_volumes.0": ["extra keys not allowed"]}}' - headers: - Content-Length: - - "123" - Content-Security-Policy: - - default-src 'none'; frame-ancestors 'none' - Content-Type: - - application/json - Date: - - Thu, 13 Feb 2020 12:46:49 GMT - Server: - - scaleway_api - Strict-Transport-Security: - - max-age=63072000 - X-Content-Type-Options: - - nosniff - X-Frame-Options: - - DENY - status: 400 Bad Request - code: 400 - duration: "" diff --git a/internal/namespaces/instance/v1/testdata/test-image-create-create-image.stderr.golden b/internal/namespaces/instance/v1/testdata/test-image-create-create-image.stderr.golden deleted file mode 100644 index 2dd452f60d..0000000000 --- a/internal/namespaces/instance/v1/testdata/test-image-create-create-image.stderr.golden +++ /dev/null @@ -1,49 +0,0 @@ -DEBUG: 2019/12/09 16:04:07 could not validate arg value for 'extra-volumes.{key}.id': invalid fieldName: ExtraVolumes.id -DEBUG: 2019/12/09 16:04:07 could not validate arg value for 'extra-volumes.{key}.name': invalid fieldName: ExtraVolumes.name -DEBUG: 2019/12/09 16:04:07 could not validate arg value for 'extra-volumes.{key}.size': invalid fieldName: ExtraVolumes.size -DEBUG: 2019/12/09 16:04:07 could not validate arg value for 'extra-volumes.{key}.volume-type': invalid fieldName: ExtraVolumes.volumeType -DEBUG: 2019/12/09 16:04:07 could not validate arg value for 'extra-volumes.{key}.organization': invalid fieldName: ExtraVolumes.organization -DEBUG: 2019/12/09 16:04:07 creating POST request on https://api.scaleway.com/instance/v1/zones/fr-par-1/images -DEBUG: 2019/12/09 16:04:07 ---------------- Scaleway SDK REQUEST 1 : --------------- -POST /instance/v1/zones/fr-par-1/images HTTP/1.1 -Host: api.scaleway.com -User-Agent: scaleway-sdk-go/v1.0.0-beta.5+dev (go1.13.1; darwin; amd64) cli-e2e-test -Content-Length: 186 -Content-Type: application/json -X-Auth-Token: ee418114-xxxx-xxxx-xxxx-xxxxxxxxxxxx -Accept-Encoding: gzip - -{"name":"foobar","root_volume":"6fd63468-b198-42cf-8462-81df850a7a8e","arch":"x86_64","extra_volumes":{"0":{"volume_type":"l_ssd"}},"organization":"aba2d0d0-b01d-4d88-b322-935edc96d0fd"} ---------------------------------------------------------- -DEBUG: 2019/12/09 16:04:07 ---------------- Scaleway SDK RESPONSE 1 : --------------- -HTTP/1.0 400 Bad Request -Connection: close -Content-Length: 123 -Content-Security-Policy: default-src 'none'; frame-ancestors 'none' -Content-Type: application/json -Date: Thu, 13 Feb 2020 12:46:49 GMT -Server: scaleway_api -Strict-Transport-Security: max-age=63072000 -X-Content-Type-Options: nosniff -X-Frame-Options: DENY - -{"type": "invalid_request_error", "message": "Validation Error", "fields": {"extra_volumes.0": ["extra keys not allowed"]}} ----------------------------------------------------------- -DEBUG: 2019/12/09 16:04:07 marshalling type '*scw.InvalidArgumentsError' -DEBUG: 2019/12/09 16:04:07 marshalling type '*core.CliError' -DEBUG: 2019/12/09 16:04:07 marshalling type '*errors.errorString' -DEBUG: 2019/12/09 16:04:07 marshalling type 'string' -DEBUG: 2019/12/09 16:04:07 marshalling type 'string' -DEBUG: 2019/12/09 16:04:07 marshalling type 'string' -DEBUG: 2019/12/09 16:04:07 marshalling type 'string' -Invalid arguments 'extra_volumes.0' - -Details: -- 'extra_volumes.0' does not respect constraint - -Hint: -Extra keys not allowed -DEBUG: 2019/12/09 16:04:07 skipping check version -DEBUG: 2019/12/09 16:04:07 skipping telemetry report From 1a7a32221f5e33b58f6514bb4464df586ce3ba53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Bourgois?= Date: Thu, 13 Feb 2020 15:11:07 +0100 Subject: [PATCH 3/4] fix --- internal/namespaces/instance/v1/custom_server_create.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/internal/namespaces/instance/v1/custom_server_create.go b/internal/namespaces/instance/v1/custom_server_create.go index 6b834a6b40..22b1728893 100644 --- a/internal/namespaces/instance/v1/custom_server_create.go +++ b/internal/namespaces/instance/v1/custom_server_create.go @@ -259,8 +259,12 @@ func instanceServerCreateRun(ctx context.Context, argsI interface{}) (i interfac } // Validate root volume type and size. - if err := validateRootVolume(getImageResponse.Image.RootVolume.Size, volumes["0"]); err != nil { - return nil, err + if getImageResponse != nil { + if err := validateRootVolume(getImageResponse.Image.RootVolume.Size, volumes["0"]); err != nil { + return nil, err + } + } else { + logger.Warningf("skipping root volume validation") } // Validate total local volume sizes. From 2d3b18d63a1f35f6498d4831a91cd803b0ba2467 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Bourgois?= Date: Thu, 13 Feb 2020 15:24:12 +0100 Subject: [PATCH 4/4] address comment --- internal/args/unmarshal_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/internal/args/unmarshal_test.go b/internal/args/unmarshal_test.go index be2e564bfa..e34e56cacc 100644 --- a/internal/args/unmarshal_test.go +++ b/internal/args/unmarshal_test.go @@ -336,12 +336,18 @@ func TestUnmarshalStruct(t *testing.T) { args: []string{ "name=coucou", "elements.0.name=bob", + "elements.0.elements.plop.name=world", }, expected: &RecursiveWithMapOfRecursive{ Name: "coucou", Elements: map[string]*RecursiveWithMapOfRecursive{ "0": { Name: "bob", + Elements: map[string]*RecursiveWithMapOfRecursive{ + "plop": { + Name: "world", + }, + }, }, }, }, @@ -352,6 +358,8 @@ func TestUnmarshalStruct(t *testing.T) { "name=coucou", "elements.0.id=1453", "elements.0.name=bob", + "elements.0.elements.plop.name=world", + "elements.0.elements.plop.short=long", }, expected: &RecursiveWithMapOfRecursive{ Name: "coucou", @@ -359,6 +367,12 @@ func TestUnmarshalStruct(t *testing.T) { "0": { ID: 1453, Name: "bob", + Elements: map[string]*RecursiveWithMapOfRecursive{ + "plop": { + Name: "world", + Short: "long", + }, + }, }, }, },