(NOT FOR SUBMISSION) Proof of concept of Storage conformance tests by jskeet · Pull Request #2954 · googleapis/google-cloud-dotnet

jskeet · 2019-03-18T17:59:55Z

test.proto describes the expected format of all JSON files
signing.json is an example of some Signing V4 tests
validation is a small amount of code to validate that all the JSON files are valid
.travis.yml runs the validation code

This is a quick and dirty way of doing the validation; other languages may well be more suitable.

frankyn

Thanks @jskeet for putting this together.

I think this is a good idea and I agree it should be moved into a conformance-tests repository.

Question: What is the experience for a failure (bad type or bad JSON format) and is it easy to debug based on test logs?

I had one related comment that can be addressed later if not in this PR.

frankyn · 2019-03-20T16:39:55Z

storage/v1/signing.json

+      "headers": {
+        "X-Goog-Encryption-Algorithm": {
+          "values": [
+            "ignored"


Slightly outside of scope comment:
Per discussion in bug:128647687.
The goal is to allow these three headers. However, it's not available right now.
Given it's not available, can you update this conformance test to not ignore X-Goog-Encryption-Algorithm? This will keep v2 and v4 consistent in this case.
Libraries should block until bug ref is fixed though.

I'll update the one that's actually being used in my repo, along with my implementation.

(I can update this one as well later on if that becomes useful, of course - but it'll be just as easy to do in the "real thing" when we have that.)

When you say "real thing" do you mean the library implementation?

jskeet · 2019-03-21T09:50:12Z

Question: What is the experience for a failure (bad type or bad JSON format) and is it easy to debug based on test logs?

I've just added a commit with an invalid piece of JSON. Here's the relevant part of the Travis log

$ dotnet run -p storage/v1/validation -- storage/v1/*.json
Validating storage/v1/signing.json
Error: InvalidProtocolBufferException: Unknown field: desription
The command "dotnet run -p storage/v1/validation -- storage/v1/*.json" exited with 1.

(If there are multiple files, it will validate all of them even if there's an error in the first one - but it'll only show the first error per file.)

frankyn

Small nits based on conversation and two colons are highlighted red.

Overall LGTM and follows our discussion this morning. Thanks @jskeet.

frankyn · 2019-03-26T18:35:50Z

storage/v1/signing.json

+      "expectedUrl": "https://storage.googleapis.com/test-bucket/test-object?X-Goog-Algorithm=GOOG4-RSA-SHA256\u0026X-Goog-Credential=test-iam-credentials%40dummy-project-id.iam.gserviceaccount.com%2F20190201%2Fauto%2Fstorage%2Fgoog4_request\u0026X-Goog-Date=20190201T090000Z\u0026X-Goog-Expires=10\u0026X-Goog-SignedHeaders=bar%3Bfoo%3Bhost\u0026X-Goog-Signature=68ecd3b008328ed30d91e2fe37444ed7b9b03f28ed4424555b5161980531ef87db1c3a5bc0265aad5640af30f96014c94fb2dba7479c41bfe1c020eb90c0c6d387d4dd09d4a5df8b60ea50eb6b01cdd786a1e37020f5f95eb8f9b6cd3f65a1f8a8a65c9fcb61ea662959efd9cd73b683f8d8804ef4d6d9b2852419b013368842731359d7f9e6d1139032ceca75d5e67cee5fd0192ea2125e5f2955d38d3d50cf116f3a52e6a62de77f6207f5b95aaa1d7d0f8a46de89ea72e7ea30f21286318d7eba0142232b0deb3a1dc9e1e812a981c66b5ffda3c6b01a8a9d113155792309fd53a3acfd054ca7776e8eec28c26480cd1e3c812f67f91d14217f39a606669d",
+      "headers": {
+        "BAR": "BAR-value"
+        "foo": "foo-value"


Why is this red?

Because I failed to put a comma at the end of line 60. Doh!

frankyn · 2019-03-26T18:36:11Z

storage/v1/test.proto

@@ -0,0 +1,19 @@
+syntax = "proto3";


Google Copyright.

frankyn · 2019-03-26T18:36:24Z

storage/v1/validation/Program.cs

@@ -0,0 +1,29 @@
+using Google.Protobuf;


Google Copyright.

frankyn · 2019-03-26T18:36:36Z

storage/v1/signing.json

+      "expectedUrl": "https://storage.googleapis.com/test-bucket/?X-Goog-Algorithm=GOOG4-RSA-SHA256\u0026X-Goog-Credential=test-iam-credentials%40dummy-project-id.iam.gserviceaccount.com%2F20190201%2Fauto%2Fstorage%2Fgoog4_request\u0026X-Goog-Date=20190201T090000Z\u0026X-Goog-Expires=10\u0026X-Goog-SignedHeaders=host\u0026X-Goog-Signature=2a1d342f11ddf0c90c669b9ba89ab5099f94049a86351cacbc85845fd5a8b31e1f9c8d484926c19fbd6930da6c8d3049ca8ebcfeefb7b02e53137755d36f97baab479414528b2802f10d94541facb888edf886d91ba124e60cb3801464f61aadc575fc921c99cf8c52e281f7bc0d3e740f529201c469c8e52775b6433687e0c0dca1c6b874614c3c3d09599be1e192c40ad6827416e387bf6e88a5f501f1d8225bce498d134599d0dfe30c9c833c244d3f90cf9595b9f8175658b788ee5c4a90b575fde5e83c645772250c7098373ca754b39d0fc1ebca2f50261a015931541c9827920eba67a1c41613853a1bd23299a1f9f5d583c0feb05ea2f792ba390d27"
+    }
+  ]
+}


Add a newline.

- test.proto describes the expected format of all JSON files - signing.json is an example of some Signing V4 tests - validation is a small amount of code to validate that all the JSON files are valid - .travis.yml runs the validation code This is a quick and dirty way of doing the validation; other languages may well be more suitable.

- Use a single value per header - Remove multi-value-header tests - Add "Header value with multiple inline values" test - Update customer-supplied encryption key test to not ignore headers

jskeet · 2019-04-18T06:17:20Z

This is now a PR in its own repo.

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 18, 2019

jskeet force-pushed the storage-conformance branch 2 times, most recently from fc42af5 to 4e9f6a8 Compare March 18, 2019 18:03

frankyn suggested changes Mar 20, 2019

View reviewed changes

frankyn mentioned this pull request Mar 21, 2019

(storage) Add V4 signing support googleapis/google-cloud-java#4692

Merged

yoshi-automation added the 🚨 This issue needs some love. label Mar 25, 2019

frankyn suggested changes Mar 26, 2019

View reviewed changes

jskeet added 5 commits March 28, 2019 07:29

Change to make a test deliberately invalid

22b531c

Fix deliberate typo

5cfde4a

Update conformance tests

387df71

- Use a single value per header - Remove multi-value-header tests - Add "Header value with multiple inline values" test - Update customer-supplied encryption key test to not ignore headers

Amend list objects test

d9ee6a1

jskeet force-pushed the storage-conformance branch from a1aafb2 to d9ee6a1 Compare March 28, 2019 07:29

jskeet added needs work This is a pull request that needs a little love. and removed 🚨 This issue needs some love. labels Apr 1, 2019

Update proto to use a timestamp instead of a string

536c0ed

jskeet force-pushed the storage-conformance branch from a4c72d7 to 536c0ed Compare April 1, 2019 06:40

jskeet added 2 commits April 1, 2019 07:44

Modify signing test data to use timestamps properly

7717191

Fix missing commas

b49ea48

jskeet force-pushed the storage-conformance branch from fadd59e to b49ea48 Compare April 1, 2019 07:26

jskeet closed this Apr 18, 2019

Conversation

jskeet commented Mar 18, 2019

Uh oh!

frankyn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jskeet commented Mar 21, 2019

Uh oh!

frankyn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jskeet commented Apr 18, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants