Skip to content

Specifies version on get iam calls#1745

Merged
joehan merged 7 commits intomasterfrom
jh-policy-version
Oct 28, 2019
Merged

Specifies version on get iam calls#1745
joehan merged 7 commits intomasterfrom
jh-policy-version

Conversation

@joehan
Copy link
Copy Markdown
Member

@joehan joehan commented Oct 23, 2019

Description

Specifies schema version=3 on get and set IAM calls.

Scenarios Tested

I created and updated an instance of storage-resize-images, and confirmed that the IAM calls worked and set version=3.

@googlebot googlebot added the cla: yes Manual indication that this has passed CLA. label Oct 23, 2019
@coveralls
Copy link
Copy Markdown

coveralls commented Oct 23, 2019

Coverage Status

Coverage increased (+0.04%) to 66.173% when pulling 4759c93 on jh-policy-version into deebbdf on master.

return member === `serviceAccount:${serviceAccountEmail}`;
});
});
policy.version = 3;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can getIamPolicy return a .version on its own? What would overwriting it here do?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it's a closed loop get-write-set, then the get with v3 will return version: 3.

outside mods that are a "blind set" can be done without a version: 3

I verified this with the IAM person (Ajay).

So I think we can remove version: 3 being forced here.

Copy link
Copy Markdown

@tstirrat tstirrat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably fine as is, but the version = 3 is redundant.

Copy link
Copy Markdown
Contributor

@bkendall bkendall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a small note to the changelog so we don't forget we did this:

* Specify version when dealing with IAM Policies related to Extensions.
@bkendall
Copy link
Copy Markdown
Contributor

maybe change the title to "set" not "get and set"?

@joehan joehan changed the title Specifies version on get and set iam calls Oct 28, 2019
@joehan joehan requested a review from bkendall October 28, 2019 20:15
@joehan joehan merged commit b44fb92 into master Oct 28, 2019
@joehan joehan deleted the jh-policy-version branch October 31, 2019 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Manual indication that this has passed CLA.

5 participants