-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor IAM-related classes to be ready to support more services #1224
Conversation
- Remove resourcemanager's Policy class - Rename core's IamPolicy to Policy - Add Role class to hold string values for roles - Add Policy.Marshaller and Policy.DefaultMarshaller classes to convert Policy to/from gRPC protos - Add PolicyMarshaller to resourcemanager to convert Policy to/from resourcemanager's protos - Update READMEs, javadoc and examples
@@ -37,69 +44,127 @@ | |||
* a set of identities to a role, where the identities can be user accounts, Google groups, Google |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Looks good (only skimmed the tests though). Few comments/suggestions. I am not convinced that we should put the Policy under iam sub-package unless we expect to have more kinds of policies in the future but fine either way. |
LGTM |
b3c94f6
to
462910e
Compare
Comments fixed, PTAL
I was considering to put: |
LGTM (one comment was not addressed, your call). |
@aozarov Another thing we might want to discuss is wether to store |
My preference would be that as long as we wrap (and not expose) the underlying protobuf we should return it as String (which is also what we did for the api-client based Services). |
…#1224) [![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [com.google.cloud:google-cloud-core](https://togithub.com/googleapis/java-core) | `2.8.9` -> `2.8.10` | [![age](https://badges.renovateapi.com/packages/maven/com.google.cloud:google-cloud-core/2.8.10/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/maven/com.google.cloud:google-cloud-core/2.8.10/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/maven/com.google.cloud:google-cloud-core/2.8.10/compatibility-slim/2.8.9)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/maven/com.google.cloud:google-cloud-core/2.8.10/confidence-slim/2.8.9)](https://docs.renovatebot.com/merge-confidence/) | --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Renovate will not automatically rebase this PR, because other commits have been found. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox. ⚠ **Warning**: custom changes will be lost. --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/java-automl). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzMi4xNzQuMSIsInVwZGF0ZWRJblZlciI6IjMyLjE3NC4xIn0=-->
🤖 I have created a release *beep* *boop* --- ## [2.3.6](googleapis/java-automl@v2.3.5...v2.3.6) (2022-08-25) ### Dependencies * update dependency com.google.cloud:google-cloud-core to v2.8.10 ([#1224](googleapis/java-automl#1224)) ([731ed9f](googleapis/java-automl@731ed9f)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
This PR does the following:
Policy
classIamPolicy
toPolicy
Role
class to hold string values for rolesPolicy.Marshaller
andPolicy.DefaultMarshaller
classes to convertPolicy
to/from gRPC protosPolicyMarshaller
to resourcemanager to convertPolicy
to/from resourcemanager's protos@aozarov @ajkannan do you mind having a look? Also, should we add an
iam
package?