From 75e992de36aff898c45df2bed525b46d23f66b5d Mon Sep 17 00:00:00 2001 From: Chris Cotter Date: Tue, 21 Mar 2023 19:58:13 -0400 Subject: [PATCH] chore(storage): add idempotency header (#7602) This additional header will allow idempotent retries on certain requests which are non-idempotent currently. --- storage/invoke.go | 5 +++++ storage/invoke_test.go | 23 ++++++++++++++--------- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/storage/invoke.go b/storage/invoke.go index 810d64285d0c..6fde482404ef 100644 --- a/storage/invoke.go +++ b/storage/invoke.go @@ -76,9 +76,14 @@ func setRetryHeaderHTTP(req interface{ Header() http.Header }) func(string, int) return } header := req.Header() + // TODO(b/274504690): Consider dropping gccl-invocation-id key since it + // duplicates the X-Goog-Gcs-Idempotency-Token header (added in v1.31.0). invocationHeader := fmt.Sprintf("gccl-invocation-id/%v gccl-attempt-count/%v", invocationID, attempts) xGoogHeader := strings.Join([]string{invocationHeader, xGoogDefaultHeader}, " ") header.Set("x-goog-api-client", xGoogHeader) + // Also use the invocationID for the idempotency token header, which will + // enable idempotent retries for more operations. + header.Set("x-goog-gcs-idempotency-token", invocationID) } } diff --git a/storage/invoke_test.go b/storage/invoke_test.go index bb27526bcb10..48f19bc0f33f 100644 --- a/storage/invoke_test.go +++ b/storage/invoke_test.go @@ -169,10 +169,11 @@ func TestInvoke(t *testing.T) { t.Run(test.desc, func(s *testing.T) { counter := 0 req := &fakeApiaryRequest{header: http.Header{}} - var initialHeader string + var initialClientHeader, initialIdempotencyHeader string call := func() error { if counter == 0 { - initialHeader = req.Header()["X-Goog-Api-Client"][0] + initialClientHeader = req.Header()["X-Goog-Api-Client"][0] + initialIdempotencyHeader = req.Header()["X-Goog-Gcs-Idempotency-Token"][0] } counter++ if counter <= test.count { @@ -186,22 +187,26 @@ func TestInvoke(t *testing.T) { } else if !test.expectFinalErr && got != test.initialErr { s.Errorf("got %v, want %v", got, test.initialErr) } - gotHeader := req.Header()["X-Goog-Api-Client"][0] + gotClientHeader := req.Header()["X-Goog-Api-Client"][0] + gotIdempotencyHeader := req.Header()["X-Goog-Gcs-Idempotency-Token"][0] wantAttempts := 1 + test.count if !test.expectFinalErr { wantAttempts = 1 } - wantHeader := strings.ReplaceAll(initialHeader, "gccl-attempt-count/1", fmt.Sprintf("gccl-attempt-count/%v", wantAttempts)) - if gotHeader != wantHeader { - t.Errorf("case %q, retry header:\ngot %v\nwant %v", test.desc, gotHeader, wantHeader) + wantClientHeader := strings.ReplaceAll(initialClientHeader, "gccl-attempt-count/1", fmt.Sprintf("gccl-attempt-count/%v", wantAttempts)) + if gotClientHeader != wantClientHeader { + t.Errorf("case %q, retry header:\ngot %v\nwant %v", test.desc, gotClientHeader, wantClientHeader) } - wantHeaderFormat := "gccl-invocation-id/.{36} gccl-attempt-count/[0-9]+ gl-go/.* gccl/" - match, err := regexp.MatchString(wantHeaderFormat, gotHeader) + wantClientHeaderFormat := "gccl-invocation-id/.{36} gccl-attempt-count/[0-9]+ gl-go/.* gccl/" + match, err := regexp.MatchString(wantClientHeaderFormat, gotClientHeader) if err != nil { s.Fatalf("compiling regexp: %v", err) } if !match { - s.Errorf("X-Goog-Api-Client header has wrong format\ngot %v\nwant regex matching %v", gotHeader, wantHeaderFormat) + s.Errorf("X-Goog-Api-Client header has wrong format\ngot %v\nwant regex matching %v", gotClientHeader, wantClientHeaderFormat) + } + if gotIdempotencyHeader != initialIdempotencyHeader { + t.Errorf("case %q, idempotency header:\ngot %v\nwant %v", test.desc, gotIdempotencyHeader, initialIdempotencyHeader) } }) }