Skip to content

Commit f88c15c

Browse files
bshafferbshaffer
authored
fix: storage samples and tests (GoogleCloudPlatform#1082)
1 parent d578285 commit f88c15c

File tree

10 files changed

+175
-187
lines changed

10 files changed

+175
-187
lines changed

.github/renovate.json renamed to renovate.json

File renamed without changes.

‎storage/src/remove_bucket_conditional_iam_binding.php‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
*
4141
* @return void
4242
*/
43-
function remove_bucket_conditional_iam_binding($bucketName, $role, $members, $title, $description, $expression)
43+
function remove_bucket_conditional_iam_binding($bucketName, $role, $title, $description, $expression)
4444
{
4545
$storage = new StorageClient();
4646
$bucket = $storage->bucket($bucketName);
@@ -51,11 +51,11 @@ function remove_bucket_conditional_iam_binding($bucketName, $role, $members, $ti
5151

5252
$key_of_conditional_binding = null;
5353
foreach ($policy['bindings'] as $key => $binding) {
54-
if ($binding['role'] == $role && $binding['condition'] != null) {
54+
if ($binding['role'] == $role && isset($binding['condition'])) {
5555
$condition = $binding['condition'];
5656
if ($condition['title'] == $title
57-
&& $condition['description'] == description
58-
&& $condition['expression'] == expression) {
57+
&& $condition['description'] == $description
58+
&& $condition['expression'] == $expression) {
5959
$key_of_conditional_binding = $key;
6060
break;
6161
}
@@ -64,6 +64,9 @@ function remove_bucket_conditional_iam_binding($bucketName, $role, $members, $ti
6464

6565
if ($key_of_conditional_binding != null) {
6666
unset($policy['bindings'][$key_of_conditional_binding]);
67+
// Ensure array keys are sequential, otherwise JSON encodes
68+
// the array as an object, which fails when calling the API.
69+
$policy['bindings'] = array_values($policy['bindings']);
6770
$bucket->iam()->setPolicy($policy);
6871
print('Conditional Binding was removed.' . PHP_EOL);
6972
} else {

‎storage/src/remove_bucket_iam_member.php‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,18 @@ function remove_bucket_iam_member($bucketName, $role, $member)
5050
if ($key !== false) {
5151
unset($binding['members'][$key]);
5252

53-
// If the last member is removed from the binding, clean up
54-
// the binding.
53+
// If the last member is removed from the binding, clean up the
54+
// binding.
5555
if (count($binding['members']) == 0) {
5656
unset($policy['bindings'][$i]);
57+
// Ensure array keys are sequential, otherwise JSON encodes
58+
// the array as an object, which fails when calling the API.
59+
$policy['bindings'] = array_values($policy['bindings']);
60+
} else {
61+
// Ensure array keys are sequential, otherwise JSON encodes
62+
// the array as an object, which fails when calling the API.
63+
$binding['members'] = array_values($binding['members']);
64+
$policy['bindings'][$i] = $binding;
5765
}
5866

5967
$iam->setPolicy($policy);

‎storage/storage.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@
318318
if (!$expression) {
319319
throw new InvalidArgumentException('Must provide expression as an option.');
320320
}
321-
remove_bucket_conditional_iam_binding($bucket_name, $role, $title, $description, $expression);
321+
remove_bucket_conditional_iam_binding($bucketName, $role, $title, $description, $expression);
322322
} else {
323323
view_bucket_iam_members($bucketName);
324324
}

‎storage/test/IamCommandTest.php‎

Lines changed: 34 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -34,17 +34,18 @@ class IamCommandTest extends TestCase
3434
protected static $storage;
3535
protected static $user;
3636
protected static $bucket;
37+
private static $role = 'roles/storage.objectViewer';
3738
private static $commandFile = __DIR__ . '/../storage.php';
3839

3940
public static function setUpBeforeClass()
4041
{
4142
self::$storage = new StorageClient();
4243
self::$user = self::requireEnv('GOOGLE_IAM_USER');
4344
self::$bucket = self::requireEnv('GOOGLE_STORAGE_BUCKET');
44-
self::cleanUpIam();
45+
self::setUpIam();
4546
}
4647

47-
private static function cleanUpIam()
48+
private static function setUpIam()
4849
{
4950
$bucket = self::$storage->bucket(self::$bucket);
5051

@@ -59,11 +60,10 @@ private static function cleanUpIam()
5960
$iam = $bucket->iam();
6061

6162
$policy = $iam->policy(['requestedPolicyVersion' => 3]);
62-
$roles = ['roles/storage.objectViewer', 'roles/storage.objectCreator'];
6363

6464
foreach ($policy['bindings'] as $i => $binding) {
6565
if (
66-
in_array($binding['role'], $roles) &&
66+
$binding['role'] == self::$role &&
6767
in_array(self::$user, $binding['members'])
6868
) {
6969
unset($policy['bindings'][$i]);
@@ -75,16 +75,15 @@ private static function cleanUpIam()
7575

7676
public function testAddBucketIamMember()
7777
{
78-
$role = 'roles/storage.objectViewer';
7978
$output = $this->runCommand('iam', [
8079
'bucket' => self::$bucket,
81-
'--role' => $role,
80+
'--role' => self::$role,
8281
'--add-member' => [self::$user],
8382
]);
8483

8584
$outputString = sprintf(
8685
'Added the following member(s) to role %s for bucket %s
87-
%s', $role, self::$bucket, self::$user);
86+
%s', self::$role, self::$bucket, self::$user);
8887

8988
$this->assertStringContainsString($outputString, $output);
9089

@@ -93,7 +92,7 @@ public function testAddBucketIamMember()
9392
'requestedPolicyVersion' => 3
9493
]);
9594
foreach ($policy['bindings'] as $binding) {
96-
if ($binding['role'] == $role) {
95+
if ($binding['role'] == self::$role) {
9796
$foundRoleMember = in_array(self::$user, $binding['members']);
9897
break;
9998
}
@@ -103,14 +102,13 @@ public function testAddBucketIamMember()
103102

104103
public function testAddBucketConditionalIamBinding()
105104
{
106-
$role = 'roles/storage.objectCreator';
107105
$title = 'always true';
108106
$description = 'this condition is always true';
109107
$expression = '1 < 2';
110108

111109
$output = $this->runCommand('iam', [
112110
'bucket' => self::$bucket,
113-
'--role' => $role,
111+
'--role' => self::$role,
114112
'--add-member' => [self::$user],
115113
'--title' => $title,
116114
'--description' => $description,
@@ -124,7 +122,7 @@ public function testAddBucketConditionalIamBinding()
124122
Title: %s
125123
Description: %s
126124
Expression: %s
127-
', $role, self::$bucket, self::$user, $title, $description, $expression);
125+
', self::$role, self::$bucket, self::$user, $title, $description, $expression);
128126

129127
$this->assertEquals($outputString, $output);
130128

@@ -133,14 +131,16 @@ public function testAddBucketConditionalIamBinding()
133131
'requestedPolicyVersion' => 3
134132
]);
135133
foreach ($policy['bindings'] as $binding) {
136-
if ($binding['role'] == $role) {
137-
$foundBinding =
138-
in_array(self::$user, $binding['members']) &&
134+
if ($binding['role'] == self::$role) {
135+
if (in_array(self::$user, $binding['members']) &&
139136
isset($binding['condition']) &&
140137
$binding['condition']['title'] == $title &&
141138
$binding['condition']['description'] == $description &&
142-
$binding['condition']['expression'] == $expression;
143-
break;
139+
$binding['condition']['expression'] == $expression
140+
) {
141+
$foundBinding = true;
142+
break;
143+
}
144144
}
145145
}
146146
$this->assertTrue($foundBinding);
@@ -168,7 +168,7 @@ public function testListIamMembers()
168168
$this->assertRegexp($binding, $output);
169169

170170
$bindingWithCondition = sprintf(
171-
'Role: roles/storage.objectCreator
171+
'Role: roles/storage.objectViewer
172172
Members:
173173
%s
174174
with condition:
@@ -186,28 +186,30 @@ public function testListIamMembers()
186186
*/
187187
public function testRemoveBucketIamMember()
188188
{
189-
$role = 'roles/storage.objectViewer';
190189
$output = $this->runCommand('iam', [
191190
'bucket' => self::$bucket,
192-
'--role' => 'roles/storage.objectViewer',
191+
'--role' => self::$role,
193192
'--remove-member' => self::$user,
194193
]);
195194

196-
$outputString = sprintf(
195+
$expected = sprintf(
197196
'User %s removed from role %s for bucket %s',
198197
self::$user,
199-
$role,
198+
self::$role,
200199
self::$bucket
201200
);
202201

203-
$this->assertStringContainsString($outputString, $output);
202+
$this->assertStringContainsString($expected, $output);
204203

205204
$foundRoleMember = false;
206205
$policy = self::$storage->bucket(self::$bucket)->iam()->policy([
207206
'requestedPolicyVersion' => 3
208207
]);
209208
foreach ($policy['bindings'] as $binding) {
210-
if ($binding['role'] == $role) {
209+
if (
210+
$binding['role'] == self::$role
211+
&& empty($binding['condition'])
212+
) {
211213
$foundRoleMember = in_array(self::$user, $binding['members']);
212214
break;
213215
}
@@ -221,29 +223,32 @@ public function testRemoveBucketIamMember()
221223
*/
222224
public function testRemoveBucketConditionalIamBinding()
223225
{
224-
$role = 'roles/storage.objectViewer';
225226
$title = 'always true';
226227
$description = 'this condition is always true';
227228
$expression = '1 < 2';
228229
$output = $this->runCommand('iam', [
229230
'bucket' => self::$bucket,
230-
'--role' => 'roles/storage.objectViewer',
231+
'--role' => self::$role,
231232
'--remove-binding' => true,
232233
'--title' => $title,
233234
'--description' => $description,
234235
'--expression' => $expression
235236
]);
236237

237-
$outputString = sprintf('Conditional Binding was removed.');
238-
239-
$this->assertStringContainsString($outputString, $output);
238+
$this->assertStringContainsString(
239+
'Conditional Binding was removed.',
240+
$output
241+
);
240242

241243
$foundBinding = false;
242244
$policy = self::$storage->bucket(self::$bucket)->iam()->policy([
243245
'requestedPolicyVersion' => 3
244246
]);
245247
foreach ($policy['bindings'] as $binding) {
246-
if ($binding['role'] == $role && $binding['condition'] != null) {
248+
if (
249+
$binding['role'] == self::$role
250+
&& isset($binding['condition'])
251+
) {
247252
$condition = $binding['condition'];
248253
if ($condition['title'] == $title
249254
&& $condition['description'] == $description

‎storage/test/ObjectAclCommandTest.php‎

Lines changed: 36 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -18,86 +18,74 @@
1818
namespace Google\Cloud\Samples\Storage\Tests;
1919

2020
use Google\Cloud\Core\Exception\NotFoundException;
21-
use Google\Cloud\Samples\Storage\ObjectAclCommand;
2221
use Google\Cloud\Storage\StorageClient;
22+
use Google\Cloud\TestUtils\ExecuteCommandTrait;
2323
use Google\Cloud\TestUtils\TestTrait;
24-
use Symfony\Component\Console\Tester\CommandTester;
2524
use PHPUnit\Framework\TestCase;
2625

2726
/**
2827
* Unit Tests for ObjectAclCommand.
2928
*/
3029
class ObjectAclCommandTest extends TestCase
3130
{
32-
use TestTrait;
31+
use TestTrait, ExecuteCommandTrait;
3332

34-
protected $commandTester;
35-
protected $storage;
33+
private static $storage;
34+
private static $bucketName;
35+
private static $commandFile = __DIR__ . '/../storage.php';
3636

37-
public function setUp()
37+
public static function setUpBeforeClass()
3838
{
39-
$application = require __DIR__ . '/../storage.php';
40-
$this->commandTester = new CommandTester($application->get('object-acl'));
41-
$this->storage = new StorageClient();
39+
self::$storage = new StorageClient();
40+
self::$bucketName = sprintf(
41+
'%s-legacy',
42+
self::requireEnv('GOOGLE_STORAGE_BUCKET')
43+
);
4244
}
4345

4446
public function testObjectAcl()
4547
{
46-
$bucketName = $this->requireEnv('GOOGLE_STORAGE_BUCKET');
4748
$objectName = $this->requireEnv('GOOGLE_STORAGE_OBJECT');
4849

49-
$this->commandTester->execute(
50-
[
51-
'bucket' => $bucketName,
52-
'object' => $objectName,
53-
],
54-
['interactive' => false]
55-
);
50+
$output = $this->runCommand('object-acl', [
51+
'bucket' => self::$bucketName,
52+
'object' => $objectName,
53+
]);
5654

57-
$this->expectOutputRegex("/: OWNER/");
55+
$this->assertContains(': OWNER', $output);
5856
}
5957

6058
public function testManageObjectAcl()
6159
{
62-
$bucketName = $this->requireEnv('GOOGLE_STORAGE_BUCKET');
6360
$objectName = $this->requireEnv('GOOGLE_STORAGE_OBJECT');
6461

65-
$bucket = $this->storage->bucket($bucketName);
62+
$bucket = self::$storage->bucket(self::$bucketName);
6663
$object = $bucket->object($objectName);
6764
$acl = $object->acl();
6865

69-
$this->commandTester->execute(
70-
[
71-
'bucket' => $bucketName,
72-
'object' => $objectName,
73-
'--entity' => 'allAuthenticatedUsers',
74-
'--create' => true,
75-
],
76-
['interactive' => false]
77-
);
66+
$output = $this->runCommand('object-acl', [
67+
'bucket' => self::$bucketName,
68+
'object' => $objectName,
69+
'--entity' => 'allAuthenticatedUsers',
70+
'--create' => true,
71+
]);
7872

7973
$aclInfo = $acl->get(['entity' => 'allAuthenticatedUsers']);
8074
$this->assertArrayHasKey('role', $aclInfo);
8175
$this->assertEquals('READER', $aclInfo['role']);
8276

83-
$this->commandTester->execute(
84-
[
85-
'bucket' => $bucketName,
86-
'object' => $objectName,
87-
'--entity' => 'allAuthenticatedUsers',
88-
],
89-
['interactive' => false]
90-
);
77+
$output .= $this->runCommand('object-acl', [
78+
'bucket' => self::$bucketName,
79+
'object' => $objectName,
80+
'--entity' => 'allAuthenticatedUsers',
81+
]);
9182

92-
$this->commandTester->execute(
93-
[
94-
'bucket' => $bucketName,
95-
'object' => $objectName,
96-
'--entity' => 'allAuthenticatedUsers',
97-
'--delete' => true,
98-
],
99-
['interactive' => false]
100-
);
83+
$output .= $this->runCommand('object-acl', [
84+
'bucket' => self::$bucketName,
85+
'object' => $objectName,
86+
'--entity' => 'allAuthenticatedUsers',
87+
'--delete' => true,
88+
]);
10189

10290
try {
10391
$acl->get(['entity' => 'allAuthenticatedUsers']);
@@ -106,13 +94,13 @@ public function testManageObjectAcl()
10694
$this->assertTrue(true);
10795
}
10896

109-
$objectUrl = sprintf('gs://%s/%s', $bucketName, $objectName);
97+
$objectUrl = sprintf('gs://%s/%s', self::$bucketName, $objectName);
11098
$outputString = <<<EOF
11199
Added allAuthenticatedUsers (READER) to $objectUrl ACL
112100
allAuthenticatedUsers: READER
113101
Deleted allAuthenticatedUsers from $objectUrl ACL
114102
115103
EOF;
116-
$this->expectOutputString($outputString);
104+
$this->assertEquals($output, $outputString);
117105
}
118106
}

0 commit comments

Comments
 (0)