Skip to content

Commit 032f2f2

Browse files
authored
fix: update grpc handling of IAM Policy etag to account for base64 encoding (#2499)
Etags returned by the JSON api are base64 encoded, and IAM Policy is currently modeled around this for its public api. Update GrpcConversions to base64 decode/encode appropriately.
1 parent 09043c5 commit 032f2f2

File tree

5 files changed

+22
-12
lines changed

5 files changed

+22
-12
lines changed

google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcConversions.java

+9-2
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,13 @@
6060
import com.google.storage.v2.Owner;
6161
import com.google.type.Date;
6262
import com.google.type.Expr;
63+
import java.nio.charset.StandardCharsets;
6364
import java.time.Duration;
6465
import java.time.Instant;
6566
import java.time.LocalDate;
6667
import java.time.OffsetDateTime;
6768
import java.time.ZoneOffset;
69+
import java.util.Base64;
6870
import java.util.Collections;
6971
import java.util.HashMap;
7072
import java.util.List;
@@ -118,6 +120,11 @@ final class GrpcConversions {
118120
hierarchicalNamespaceCodec =
119121
Codec.of(this::hierarchicalNamespaceEncode, this::hierarchicalNamespaceDecode);
120122

123+
private final Codec<ByteString, String> byteStringB64StringCodec =
124+
Codec.of(
125+
bs -> Base64.getEncoder().encodeToString(bs.toByteArray()),
126+
s -> ByteString.copyFrom(Base64.getDecoder().decode(s.getBytes(StandardCharsets.UTF_8))));
127+
121128
@VisibleForTesting
122129
final Codec<OffsetDateTime, Timestamp> timestampCodec =
123130
Codec.of(
@@ -1048,7 +1055,7 @@ private NotificationInfo notificationDecode(NotificationConfig from) {
10481055

10491056
private com.google.iam.v1.Policy policyEncode(Policy from) {
10501057
com.google.iam.v1.Policy.Builder to = com.google.iam.v1.Policy.newBuilder();
1051-
ifNonNull(from.getEtag(), ByteString::copyFromUtf8, to::setEtag);
1058+
ifNonNull(from.getEtag(), byteStringB64StringCodec::decode, to::setEtag);
10521059
ifNonNull(from.getVersion(), to::setVersion);
10531060
from.getBindingsList().stream().map(bindingCodec::encode).forEach(to::addBindings);
10541061
return to.build();
@@ -1058,7 +1065,7 @@ private Policy policyDecode(com.google.iam.v1.Policy from) {
10581065
Policy.Builder to = Policy.newBuilder();
10591066
ByteString etag = from.getEtag();
10601067
if (!etag.isEmpty()) {
1061-
to.setEtag(etag.toStringUtf8());
1068+
to.setEtag(byteStringB64StringCodec.encode(etag));
10621069
}
10631070
to.setVersion(from.getVersion());
10641071
List<com.google.iam.v1.Binding> bindingsList = from.getBindingsList();

google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcRetryAlgorithmManager.java

+6-2
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import com.google.iam.v1.GetIamPolicyRequest;
2121
import com.google.iam.v1.SetIamPolicyRequest;
2222
import com.google.iam.v1.TestIamPermissionsRequest;
23+
import com.google.protobuf.ByteString;
2324
import com.google.storage.v2.BidiWriteObjectRequest;
2425
import com.google.storage.v2.ComposeObjectRequest;
2526
import com.google.storage.v2.CreateBucketRequest;
@@ -168,8 +169,11 @@ public ResultRetryAlgorithm<?> getFor(RewriteObjectRequest req) {
168169
}
169170

170171
public ResultRetryAlgorithm<?> getFor(SetIamPolicyRequest req) {
171-
// TODO: etag
172-
return retryStrategy.getNonidempotentHandler();
172+
if (req.getPolicy().getEtag().equals(ByteString.empty())) {
173+
return retryStrategy.getNonidempotentHandler();
174+
} else {
175+
return retryStrategy.getIdempotentHandler();
176+
}
173177
}
174178

175179
public ResultRetryAlgorithm<?> getFor(StartResumableWriteRequest req) {

google-cloud-storage/src/test/java/com/google/cloud/storage/conformance/retry/CtxFunctions.java

+4
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,10 @@ static final class Local {
136136
static final class Rpc {
137137
static final CtxFunction createEmptyBlob =
138138
(ctx, c) -> ctx.map(state -> state.with(ctx.getStorage().create(state.getBlobInfo())));
139+
static final CtxFunction bucketIamPolicy =
140+
(ctx, c) ->
141+
ctx.map(
142+
state -> state.with(ctx.getStorage().getIamPolicy(state.getBucket().getName())));
139143
}
140144

141145
static final class ResourceSetup {

google-cloud-storage/src/test/java/com/google/cloud/storage/conformance/retry/ITRetryConformanceTest.java

+1-7
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import static com.google.cloud.storage.PackagePrivateMethodWorkarounds.blobCopyWithStorage;
2020
import static com.google.cloud.storage.PackagePrivateMethodWorkarounds.bucketCopyWithStorage;
2121
import static com.google.cloud.storage.conformance.retry.Ctx.ctx;
22-
import static com.google.cloud.storage.conformance.retry.ITRetryConformanceTest.RetryTestCaseResolver.lift;
2322
import static com.google.cloud.storage.conformance.retry.State.empty;
2423
import static com.google.common.truth.Truth.assertThat;
2524
import static java.util.Objects.requireNonNull;
@@ -168,12 +167,7 @@ public ImmutableList<?> parameters() {
168167
.setMappings(new RpcMethodMappings())
169168
.setProjectId("conformance-tests")
170169
.setHost(testBench.getBaseUri().replaceAll("https?://", ""))
171-
.setTestAllowFilter(
172-
RetryTestCaseResolver.includeAll()
173-
.and(
174-
(lift(trc -> trc.getTransport() == Transport.GRPC)
175-
.and((m, trc) -> m == RpcMethod.storage.buckets.setIamPolicy))
176-
.negate()))
170+
.setTestAllowFilter(RetryTestCaseResolver.includeAll())
177171
.build();
178172

179173
List<RetryTestCase> retryTestCases;

google-cloud-storage/src/test/java/com/google/cloud/storage/conformance/retry/RpcMethodMappings.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -685,6 +685,7 @@ private static void setIamPolicy(ArrayList<RpcMethodMapping> a) {
685685
a.add(
686686
RpcMethodMapping.newBuilder(240, buckets.setIamPolicy)
687687
.withApplicable(TestRetryConformance::isPreconditionsProvided)
688+
.withSetup(ResourceSetup.defaultSetup.andThen(Rpc.bucketIamPolicy))
688689
.withTest(
689690
(ctx, c) ->
690691
ctx.map(
@@ -694,7 +695,7 @@ private static void setIamPolicy(ArrayList<RpcMethodMapping> a) {
694695
.setIamPolicy(
695696
state.getBucket().getName(),
696697
Policy.newBuilder()
697-
.setEtag("h??")
698+
.setEtag(state.getPolicy().getEtag())
698699
.setVersion(3)
699700
.setBindings(
700701
ImmutableList.of(

0 commit comments

Comments
 (0)