generated from salesforce/oss-template
-
Notifications
You must be signed in to change notification settings - Fork 11
iam: getInlinePolicyDetails API(GCP) #163
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
Open
Yaminik1996
wants to merge
2
commits into
salesforce:main
Choose a base branch
from
Yaminik1996:W-20111602/iamPolicyAPIsGCP
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -125,14 +125,17 @@ public void attachInlinePolicy(PolicyDocument policyDocument, String tenantId, S | |
| * Retrieves the details of a specific inline policy attached to an identity. | ||
| * | ||
| * @param identityName the name of the identity | ||
| * @param policyName the name of the policy | ||
| * @param policyName the name of the policy. This parameter is optional and subject to cloud semantics. | ||
| * Some cloud providers may not support named policies, in which case this parameter may be ignored. | ||
| * @param roleName the role name. This parameter is optional and subject to cloud semantics. Some cloud providers | ||
| * may require this parameter to identify the policy, while others may not use it. | ||
| * @param tenantId the tenant ID | ||
| * @param region the region | ||
| * @return the policy document details as a string | ||
| */ | ||
| public String getInlinePolicyDetails(String identityName, String policyName, String tenantId, String region) { | ||
| public String getInlinePolicyDetails(String identityName, String policyName, String roleName, String tenantId, String region) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the reason for adding this roleName? Is it for aws and ali?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is for GCP. GCP APIs need roleName and AWS APIs need tenantId. |
||
| try { | ||
| return this.iam.getInlinePolicyDetails(identityName, policyName, tenantId, region); | ||
| return this.iam.getInlinePolicyDetails(identityName, policyName, roleName, tenantId, region); | ||
| } catch (Throwable t) { | ||
| Class<? extends SubstrateSdkException> exception = this.iam.getException(t); | ||
| ExceptionHandler.handleAndPropagate(exception, t); | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,7 @@ | |
| import com.google.iam.v1.Policy; | ||
| import com.google.iam.v1.SetIamPolicyRequest; | ||
|
|
||
| import com.salesforce.multicloudj.common.exceptions.InvalidArgumentException; | ||
| import com.salesforce.multicloudj.common.exceptions.SubstrateSdkException; | ||
| import com.salesforce.multicloudj.common.exceptions.UnknownException; | ||
| import com.salesforce.multicloudj.common.gcp.CommonErrorCodeMapping; | ||
|
|
@@ -24,6 +25,7 @@ | |
| import com.salesforce.multicloudj.iam.model.Statement; | ||
| import com.salesforce.multicloudj.iam.model.TrustConfiguration; | ||
|
|
||
| import com.fasterxml.jackson.core.JsonProcessingException; | ||
| import com.fasterxml.jackson.databind.ObjectMapper; | ||
|
|
||
| import java.io.IOException; | ||
|
|
@@ -262,12 +264,24 @@ private Policy addBinding(Policy policy, String role, String member) { | |
| } | ||
|
|
||
| /** | ||
| * Retrieves the details of a specific inline policy (role) attached to an IAM member. | ||
| * Retrieves the details of a specific inline policy attached to an IAM member. | ||
| * In GCP, this retrieves the role binding information and converts it back to a PolicyDocument format. | ||
| * | ||
| * <p><strong>GCP-specific behavior:</strong> | ||
| * <ul> | ||
| * <li><strong>policyName:</strong> Not used. GCP IAM does not support named policies. This parameter | ||
| * is ignored but kept for interface compatibility.</li> | ||
| * <li><strong>roleName:</strong> Required. Must be a valid GCP IAM role name (e.g., "roles/iam.serviceAccountUser", | ||
| * "roles/storage.objectViewer"). This is used to identify the role binding to retrieve.</li> | ||
| * </ul> | ||
| * | ||
| * @param identityName the IAM member (e.g., "serviceAccount:[email protected]", | ||
| * "user:[email protected]", "group:[email protected]") | ||
| * @param policyName the role name (e.g., "roles/iam.serviceAccountUser") | ||
| * @param policyName the name of the policy. This parameter is optional and subject to cloud semantics. | ||
| * In GCP, named policies are not supported, so this parameter is not used but kept for interface compatibility. | ||
| * @param roleName the role name. This parameter is optional and subject to cloud semantics. | ||
| * In GCP, this parameter is required and must be a valid GCP IAM role name (e.g., "roles/iam.serviceAccountUser"). | ||
| * Must not be null or empty. | ||
| * @param tenantId the resource name that owns the IAM policy. Examples include: | ||
| * "organizations/123456789012", | ||
| * "folders/987654321098", | ||
|
|
@@ -276,9 +290,14 @@ private Policy addBinding(Policy policy, String role, String member) { | |
| * Can be any GCP resource that supports IAM policies. | ||
| * @param region the region (optional for GCP) | ||
| * @return the policy document as a JSON string, or null if the policy doesn't exist | ||
| * @throws InvalidArgumentException if roleName is null or empty | ||
| */ | ||
| @Override | ||
| protected String doGetInlinePolicyDetails(String identityName, String policyName, String tenantId, String region) { | ||
| protected String doGetInlinePolicyDetails(String identityName, String policyName, String roleName, String tenantId, String region) { | ||
| if (roleName == null || roleName.trim().isEmpty()) { | ||
| throw new InvalidArgumentException("roleName is required for GCP IAM"); | ||
| } | ||
|
|
||
| // Get the current IAM policy for the resource | ||
| GetIamPolicyRequest getRequest = GetIamPolicyRequest.newBuilder() | ||
| .setResource(tenantId) | ||
|
|
@@ -291,7 +310,7 @@ protected String doGetInlinePolicyDetails(String identityName, String policyName | |
|
|
||
| // Find the binding for the specified role | ||
| Optional<Binding> binding = policy.getBindingsList().stream() | ||
| .filter(b -> b.getRole().equals(policyName)) | ||
| .filter(b -> b.getRole().equals(roleName)) | ||
| .findFirst(); | ||
|
|
||
| // Check if the service account is a member of this binding | ||
|
|
@@ -305,7 +324,7 @@ protected String doGetInlinePolicyDetails(String identityName, String policyName | |
| .version("") | ||
| .statement(Statement.builder() | ||
| .effect(EFFECT_ALLOW) | ||
| .action(policyName) | ||
| .action(roleName) | ||
| .build()) | ||
| .build(); | ||
|
|
||
|
|
@@ -323,7 +342,7 @@ protected String doGetInlinePolicyDetails(String identityName, String policyName | |
| private String toJsonString(PolicyDocument policyDocument) { | ||
| try { | ||
| return new ObjectMapper().writeValueAsString(policyDocument); | ||
| } catch (Exception e) { | ||
| } catch (JsonProcessingException e) { | ||
| throw new SubstrateSdkException("Failed to serialize policy document to JSON", e); | ||
| } | ||
| } | ||
|
|
@@ -398,9 +417,16 @@ protected void doRemovePolicy(String identityName, String policyName, String ten | |
| return; | ||
| } | ||
|
|
||
| // Remove the binding - we know a change will occur because of the check above | ||
| // Remove the binding | ||
| Policy updatedPolicy = removeBinding(policy, policyName, identityName); | ||
|
|
||
| // Only make the remote call if the policy actually changed | ||
| if (policy.getBindingsCount() == updatedPolicy.getBindingsCount() | ||
| && policy.getBindingsList().equals(updatedPolicy.getBindingsList())) { | ||
| // Policy didn't change, skip the remote call | ||
| return; | ||
| } | ||
|
|
||
| // Set the updated policy back to the resource | ||
| SetIamPolicyRequest setRequest = SetIamPolicyRequest.newBuilder() | ||
| .setResource(tenantId) | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to update our AwsIam for this function signature change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no AwsIam as of now, but yes when it's created this should be the signature.