From 209f9dc87125b1540a3be9b786bb3672425ea4f8 Mon Sep 17 00:00:00 2001 From: Kelly Deng Date: Wed, 17 Jun 2020 17:50:27 -0400 Subject: [PATCH] added more edge cases for storage disk partition size field --- data_fetch.go | 208 +++++++++++++++++++++++++++++++++++++++++++- grpc_server.go | 10 ++- grpc_server_test.go | 71 ++++++++++++--- 3 files changed, 269 insertions(+), 20 deletions(-) diff --git a/data_fetch.go b/data_fetch.go index ffab088f..ace27eb1 100644 --- a/data_fetch.go +++ b/data_fetch.go @@ -136,6 +136,26 @@ const ( } } } +` + cacherPartitionSizeStringLeadingZeros = ` + { + "id": "8978e7d4-1a55-4845-8a66-a5259236b104", + "instance": { + "storage": { + "disks": [ + { + "partitions": [ + { + "size": "007", + "label": "BIOS", + "number": 1 + } + ] + } + ] + } + } + } ` cacherPartitionSizeWhitespace = ` { @@ -146,7 +166,67 @@ const ( { "partitions": [ { - "size": " 1234 ", + "size": " \t 1234\n ", + "label": "BIOS", + "number": 1 + } + ] + } + ] + } + } + } +` + cacherPartitionSizeInterceptingWhitespace = ` + { + "id": "8978e7d4-1a55-4845-8a66-a5259236b104", + "instance": { + "storage": { + "disks": [ + { + "partitions": [ + { + "size": "12\tmb", + "label": "BIOS", + "number": 1 + } + ] + } + ] + } + } + } +` + cacherPartitionSizeBLower = ` + { + "id": "8978e7d4-1a55-4845-8a66-a5259236b104", + "instance": { + "storage": { + "disks": [ + { + "partitions": [ + { + "size": "1000000b", + "label": "BIOS", + "number": 1 + } + ] + } + ] + } + } + } +` + cacherPartitionSizeBUpper = ` + { + "id": "8978e7d4-1a55-4845-8a66-a5259236b104", + "instance": { + "storage": { + "disks": [ + { + "partitions": [ + { + "size": "1000000B", "label": "BIOS", "number": 1 } @@ -177,7 +257,27 @@ const ( } } ` - cacherPartitionSizeKB = ` + cacherPartitionSizeKBLower = ` + { + "id": "8978e7d4-1a55-4845-8a66-a5259236b104", + "instance": { + "storage": { + "disks": [ + { + "partitions": [ + { + "size": "24kb", + "label": "BIOS", + "number": 1 + } + ] + } + ] + } + } + } +` + cacherPartitionSizeKBUpper = ` { "id": "8978e7d4-1a55-4845-8a66-a5259236b104", "instance": { @@ -186,7 +286,27 @@ const ( { "partitions": [ { - "size": "25Kb", + "size": "24KB", + "label": "BIOS", + "number": 1 + } + ] + } + ] + } + } + } +` + cacherPartitionSizeKBMixed = ` + { + "id": "8978e7d4-1a55-4845-8a66-a5259236b104", + "instance": { + "storage": { + "disks": [ + { + "partitions": [ + { + "size": "24Kb", "label": "BIOS", "number": 1 } @@ -216,6 +336,26 @@ const ( } } } +` + cacherPartitionSizeTB = ` + { + "id": "8978e7d4-1a55-4845-8a66-a5259236b104", + "instance": { + "storage": { + "disks": [ + { + "partitions": [ + { + "size": "2TB", + "label": "BIOS", + "number": 1 + } + ] + } + ] + } + } + } ` cacherPartitionSizeInvalidSuffix = ` { @@ -226,7 +366,7 @@ const ( { "partitions": [ { - "size": "3kmgt", + "size": "3kmgtb", "label": "BIOS", "number": 1 } @@ -256,6 +396,66 @@ const ( } } } +` + cacherPartitionSizeInvalidIntertwined2 = ` + { + "id": "8978e7d4-1a55-4845-8a66-a5259236b104", + "instance": { + "storage": { + "disks": [ + { + "partitions": [ + { + "size": "k123b", + "label": "BIOS", + "number": 1 + } + ] + } + ] + } + } + } +` + cacherPartitionSizeEmpty = ` + { + "id": "8978e7d4-1a55-4845-8a66-a5259236b104", + "instance": { + "storage": { + "disks": [ + { + "partitions": [ + { + "size": "", + "label": "BIOS", + "number": 1 + } + ] + } + ] + } + } + } +` + cacherPartitionSizeReversedPlacement = ` + { + "id": "8978e7d4-1a55-4845-8a66-a5259236b104", + "instance": { + "storage": { + "disks": [ + { + "partitions": [ + { + "size": "b10", + "label": "BIOS", + "number": 1 + } + ] + } + ] + } + } + } ` tinkerbellDataModel = ` { diff --git a/grpc_server.go b/grpc_server.go index b030c28b..3d529e0b 100644 --- a/grpc_server.go +++ b/grpc_server.go @@ -135,6 +135,7 @@ type storage struct { type intOrString int +// UnmarshalJSON implements the json.Unmarshaler interface for custom unmarshalling of IntOrString func (ios *intOrString) UnmarshalJSON(b []byte) error { if b[0] != '"' { return json.Unmarshal(b, (*int)(ios)) @@ -153,9 +154,13 @@ func (ios *intOrString) UnmarshalJSON(b []byte) error { // converts a string of "" (e.g. "123kb") into its equivalent size in bytes func convertSuffix(s string) (int, error) { + if s == "" { // return 0 if empty string + return 0, nil + } + suffixes := map[string]float64{"": 0, "b": 0, "k": 1, "kb": 1, "m": 2, "mb": 2, "g": 3, "gb": 3, "t": 4, "tb": 4} s = strings.ToLower(s) // converts string s to all lowercase - i := strings.TrimFunc(s, func(r rune) bool { // trims from string s of anything not a number (e.g., "123 kb" -> "123" and "12k3b" -> "12k3") + i := strings.TrimFunc(s, func(r rune) bool { // trims both ends of string s of anything not a number (e.g., "123 kb" -> "123" and "12k3b" -> "12k3") return !unicode.IsNumber(r) }) size, err := strconv.Atoi(i) // convert number portion of string s into an int @@ -163,7 +168,7 @@ func convertSuffix(s string) (int, error) { return -1, err } - suf := strings.TrimFunc(s, func(r rune) bool { // trims from string s of anything not a letter (e.g., "123 kb" -> "kb") + suf := strings.TrimFunc(s, func(r rune) bool { // trims both ends of string s of anything not a letter (e.g., "123 kb" -> "kb") return !unicode.IsLetter(r) }) @@ -193,6 +198,7 @@ func exportHardware(hw []byte) ([]byte, error) { return json.Marshal(exported) } +// UnmarshalJSON implements the json.Unmarshaler interface for custom unmarshalling of exportedHardwareCacher func (eh *exportedHardwareCacher) UnmarshalJSON(b []byte) error { type ehj exportedHardwareCacher var tmp ehj diff --git a/grpc_server_test.go b/grpc_server_test.go index 531db0e8..5b855467 100644 --- a/grpc_server_test.go +++ b/grpc_server_test.go @@ -51,7 +51,7 @@ func TestGetByIPCacher(t *testing.T) { if err.Error() != test.error { t.Fatalf("unexpected error in getByIP, want: %v, got: %v\n", test.error, err.Error()) } - return + continue } hw := exportedHardwareCacher{} err = json.Unmarshal(ehw, &hw) @@ -75,6 +75,7 @@ func TestGetByIPCacher(t *testing.T) { if hw.Instance.Storage.Disks[0].WipeTable != test.wipeTable { t.Fatalf("unexpected storage disk wipe table, want: %v, got: %v\n", test.wipeTable, hw.Instance.Storage.Disks[0].WipeTable) } + t.Log("want:", test.partitionSize, " got:", hw.Instance.Storage.Disks[0].Paritions[0].Size) if int(hw.Instance.Storage.Disks[0].Paritions[0].Size) != test.partitionSize { t.Fatalf("unexpected storage disk partition size, want: %v, got: %v\n", test.partitionSize, hw.Instance.Storage.Disks[0].Paritions[0].Size) } @@ -113,7 +114,7 @@ func TestGetByIPTinkerbell(t *testing.T) { if err.Error() != test.error { t.Fatalf("unexpected error in getByIP, want: %v, got: %v\n", test.error, err.Error()) } - return + continue } hw := exportedHardwareTinkerbell{} err = json.Unmarshal(ehw, &hw) @@ -183,38 +184,80 @@ var cacherGrpcTests = map[string]struct { planSlug: "t1.small.x86", json: cacherDataModel, }, - "cacher_partition_size_int": { + "cacher_partition_size_int": { // 4096 partitionSize: 4096, json: cacherPartitionSizeInt, }, - "cacher_partition_size_string": { + "cacher_partition_size_string": { // "3333" partitionSize: 3333, json: cacherPartitionSizeString, }, - "cacher_partition_size_whitespace": { + "cacher_partition_size_string_leading_zeros": { // "007" + partitionSize: 7, + json: cacherPartitionSizeStringLeadingZeros, + }, + "cacher_partition_size_whitespace": { // " \t 1234\n " partitionSize: 1234, json: cacherPartitionSizeWhitespace, }, - "cacher_partition_size_k": { + "cacher_partition_size_intercepting_whitespace": { // "12\tmb" + partitionSize: 12582912, + json: cacherPartitionSizeInterceptingWhitespace, + }, + "cacher_partition_size_b_lower": { // "1000000b" + partitionSize: 1000000, + json: cacherPartitionSizeBLower, + }, + "cacher_partition_size_b_upper": { // "1000000B" + partitionSize: 1000000, + json: cacherPartitionSizeBUpper, + }, + "cacher_partition_size_k": { // "24K" partitionSize: 24576, json: cacherPartitionSizeK, }, - "cacher_partition_size_kb": { - partitionSize: 25600, - json: cacherPartitionSizeKB, + "cacher_partition_size_kb_lower": { // "24kb" + partitionSize: 24576, + json: cacherPartitionSizeKBLower, + }, + "cacher_partition_size_kb_upper": { // "24KB" + partitionSize: 24576, + json: cacherPartitionSizeKBUpper, + }, + "cacher_partition_size_kb_mixed": { // "24Kb" + partitionSize: 24576, + json: cacherPartitionSizeKBMixed, }, - "cacher_partition_size_m": { + "cacher_partition_size_m": { // "3m" partitionSize: 3145728, json: cacherPartitionSizeM, }, - "cacher_partition_size_invalid_suffix": { + "cacher_partition_size_t": { // "2TB" + partitionSize: 2199023255552, + json: cacherPartitionSizeTB, + }, + "cacher_partition_size_invalid_suffix": { // "3kmgtb" partitionSize: -1, json: cacherPartitionSizeInvalidSuffix, error: "invalid suffix", }, - "cacher_partition_size_invalid_intertwined": { - json: cacherPartitionSizeInvalidIntertwined, - error: `strconv.Atoi: parsing "12kb3": invalid syntax`, + "cacher_partition_size_invalid_intertwined": { // "12kb3" + partitionSize: -1, + json: cacherPartitionSizeInvalidIntertwined, + error: `strconv.Atoi: parsing "12kb3": invalid syntax`, + }, + "cacher_partition_size_invalid_intertwined_2": { // "k123b" + partitionSize: -1, + json: cacherPartitionSizeInvalidIntertwined2, + error: "invalid suffix", + }, + "cacher_partition_size_empty": { // "" + partitionSize: 0, + json: cacherPartitionSizeEmpty, + }, + "cacher_partition_size_reverse_placement": { // "b10" // (kdeng3849) should we allow this? + partitionSize: 10, + json: cacherPartitionSizeReversedPlacement, }, }