iam: implement doCreateIdentity for GCP #142

SriramGuduri · 2025-11-12T10:13:44Z

Summary

< Provide a brief description of the changes in this PR >

Some conventions to follow

add the module name as a prefix
- for example: add a prefix: docstore: for document store module, blobstore for Blob Store module
for a test only PR, add test:
for a perf improvement only PR, add perf:
for a refactoring only PR, add "refactor:"

salesforce-cla · 2025-11-12T10:13:50Z

Thanks for the contribution! Unfortunately we can't verify the commit author(s): Sriram Guduri <s***@s***.i***.s***.com>. One possible solution is to add that email to your GitHub account. Alternatively you can change your commits to another email and force push the change. After getting your commits associated with your GitHub account, sign the Salesforce Inc. Contributor License Agreement and this Pull Request will be revalidated.

codecov-commenter · 2025-11-12T10:26:20Z

Codecov Report

❌ Patch coverage is 67.61905% with 34 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.88%. Comparing base (5cfcc7b) to head (b8fcdf3).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...ava/com/salesforce/multicloudj/iam/gcp/GcpIam.java	67.01%	30 Missing and 2 partials ⚠️
...m/salesforce/multicloudj/iam/client/IamClient.java	33.33%	2 Missing ⚠️

❌ Your patch status has failed because the patch coverage (67.61%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@             Coverage Diff              @@
##               main     #142      +/-   ##
============================================
- Coverage     83.11%   82.88%   -0.24%     
- Complexity       36       51      +15     
============================================
  Files           148      149       +1     
  Lines          7648     7747      +99     
  Branches        891      899       +8     
============================================
+ Hits           6357     6421      +64     
- Misses          858      891      +33     
- Partials        433      435       +2

Flag	Coverage Δ
unittests	`82.88% <67.61%> (-0.24%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

salesforce-cla · 2025-11-12T10:31:56Z

Thanks for the contribution! Before we can merge this, we need @SriramGuduri to sign the Salesforce Inc. Contributor License Agreement.

sandeepvinayak · 2025-11-16T12:00:07Z

iam/iam-gcp/src/main/java/com/salesforce/multicloudj/iam/gcp/GcpIam.java

+import java.util.Optional;
+import java.util.stream.Collectors;
+
+@SuppressWarnings("rawtypes")


I think we don't need this. Also noticed that in AbstractIAM, we don't need the generic type T

public abstract class AbstractIam<T extends AbstractIam<T>> implements Provider, Identity

It's un-used.

Addressed this in the following commits.

sandeepvinayak · 2025-11-16T12:00:28Z

iam/iam-gcp/src/main/java/com/salesforce/multicloudj/iam/gcp/GcpIam.java

+
+@SuppressWarnings("rawtypes")
+@AutoService(AbstractIam.class)
+public class GcpIam extends AbstractIam<GcpIam> {


so, this will be public class GcpIam extends AbstractIam

Addressed this in the following commits.

sandeepvinayak · 2025-11-16T12:01:54Z

iam/iam-gcp/src/main/java/com/salesforce/multicloudj/iam/gcp/GcpIam.java

+    public GcpIam(Builder builder) {
+        super(builder);
+        try {
+            this.iamClient = IAMClient.create();


we should get the client from the builder, not create it here.

Good catch. Addressed this in the following commits.

sandeepvinayak · 2025-11-16T12:06:31Z

iam/iam-gcp/src/main/java/com/salesforce/multicloudj/iam/gcp/GcpIam.java

+                try {
+                    policy = iamClient.getIamPolicy(getRequest);
+                } catch (ApiException e) {
+                    // If policy doesn't exist, create a new one


but we have not idea why the exception occurred, maybe it was just 5xx. Let's try to narrow down

thinking aloud, since we are creating new identity here, what are the scenarios where policy would and would not exists? should be a single scenario

but we have not idea why the exception occurred, maybe it was just 5xx. Let's try to narrow down

The Idea is if getIamPolicy gives a 404 NOT FOUND error, create a new one. But yeah ApiException could be anything from a 5xx to a 4xx. I'll narrow down the check to 404 http status.

thinking aloud, since we are creating new identity here, what are the scenarios where policy would and would not exists? should be a single scenario

The scenario I can think of here was if identity already existing with a Policy attached to it. But seems like GCP doesn't restrict Display Names to be unique. This might not be a valid scenario to start with. I'll validate this once I have the GCP access and course correct.

…ype' issue

sandeepvinayak · 2025-11-17T21:19:00Z

iam/iam-gcp/src/main/java/com/salesforce/multicloudj/iam/gcp/GcpIam.java

+    public GcpIam(Builder builder, IAMClient iamClient) {
+        super(builder);
+        if (iamClient == null) {
+            throw new InvalidArgumentException("IAMClient cannot be null");
+        }
+        this.iamClient = iamClient;
+    }


we don't need this constructor now since we are building the iamClient in the builder if not present.

I'll address this in following commits.

sandeepvinayak · 2025-11-17T21:20:32Z

iam/iam-gcp/src/test/java/com/salesforce/multicloudj/iam/gcp/GcpIamIT.java

+/**
+ * Integration tests for GcpIam.
+ * 
+ * <p>This test class follows the conformance testing pattern used across the MultiCloudJ project.


this gen ai comments be modified or removed, it's implicit that it should follow the project patterns

My bad :-P. I'll remove them.

sandeepvinayak · 2025-11-17T21:22:16Z

iam/iam-gcp/src/test/java/com/salesforce/multicloudj/iam/gcp/GcpIamIT.java

+ * natively support. Therefore:
+ * <ul>
+ *   <li>Recording mode (-Drecord) works by connecting directly to real GCP APIs</li>
+ *   <li>Replay mode is not yet implemented and will fail</li>


we should try the rest client until we have the grpc enabled and avoid skipping the tests.
grpc is supported by wiremock here https://github.com/wiremock/wiremock-grpc-extension

I was facing issue with http json transport earlier. I had a sync up with Yamini on this. I'll address this in the following commits.

sandeepvinayak · 2025-11-17T21:23:03Z

iam/iam-gcp/src/test/java/com/salesforce/multicloudj/iam/gcp/GcpIamIT.java

+                    // Replay path - inject mock credentials
+                    // Note: Since WireMock doesn't support gRPC, replay mode is not yet implemented.


Acknowledged. I'll address this in the following commits.

sandeepvinayak · 2025-11-17T21:25:44Z

iam/iam-client/src/test/java/com/salesforce/multicloudj/iam/client/AbstractIamIT.java

+        /**
+         * Indicates whether the provider supports creating identities.
+         * 
+         * @return true if createIdentity is supported
+         */
+        boolean supportsCreateIdentity();
+
+        /**
+         * Indicates whether the provider supports creating identities with trust configuration.
+         * 
+         * @return true if createIdentity with trust config is supported
+         */
+        boolean supportsCreateIdentityWithTrustConfig();
+
+        /**
+         * Indicates whether the provider supports getting identity details.
+         * 
+         * @return true if getIdentity is supported
+         */
+        boolean supportsGetIdentity();
+
+        /**
+         * Indicates whether the provider supports deleting identities.
+         * 
+         * @return true if deleteIdentity is supported
+         */
+        boolean supportsDeleteIdentity();
+
+        /**
+         * Indicates whether the provider supports attaching inline policies.
+         * 
+         * @return true if attachInlinePolicy is supported
+         */
+        boolean supportsAttachInlinePolicy();
+
+        /**
+         * Indicates whether the provider supports getting inline policy details.
+         * 
+         * @return true if getInlinePolicyDetails is supported
+         */
+        boolean supportsGetInlinePolicyDetails();
+
+        /**
+         * Indicates whether the provider supports listing attached policies.
+         * 
+         * @return true if getAttachedPolicies is supported
+         */
+        boolean supportsGetAttachedPolicies();
+
+        /**
+         * Indicates whether the provider supports removing policies.
+         * 
+         * @return true if removePolicy is supported
+         */
+        boolean supportsRemovePolicy();


let's not add this for each func here, instead the tests can be skipped with Assumptions inside the @test.

Acknowledged. I'll address this in the following commits.

sandeepvinayak · 2025-11-17T21:28:06Z

iam/iam-gcp/src/main/java/com/salesforce/multicloudj/iam/gcp/GcpIam.java

+        public Builder withCredentials(GoogleCredentials credentials) {
+            this.credentials = credentials;
+            return self();
+        }


how will this be used ? the user never supply the GoogleCredentials directly

I was referring to one of the services that supplied credentials to the builder. I'll clean this up.

sandeepvinayak · 2025-11-17T21:29:23Z

iam/iam-client/src/test/java/com/salesforce/multicloudj/iam/client/AbstractIamIT.java

+         * 
+         * @return list of WireMock extension class names
+         */
+        List<String> getWiremockExtensions();


let's add when we need it. not only interfaces need to define this extensions

salesforce-cla bot added the cla:missing label Nov 12, 2025

SriramGuduri force-pushed the W-20111589/iamCreateIdentityGCP branch 2 times, most recently from 10d3d06 to 022911e Compare November 12, 2025 10:20

SriramGuduri force-pushed the W-20111589/iamCreateIdentityGCP branch from 022911e to fa1d1dc Compare November 12, 2025 10:31

SriramGuduri force-pushed the W-20111589/iamCreateIdentityGCP branch from fa1d1dc to 6285df8 Compare November 14, 2025 05:41

salesforce-cla bot added cla:signed and removed cla:missing labels Nov 14, 2025

sandeepvinayak reviewed Nov 16, 2025

View reviewed changes

SriramGuduri added 7 commits November 17, 2025 12:19

added GcpIam module with createIdentity Feature

a9ee41c

added conformance tests

17c8b9c

resolved invalid transport: httpjson issue

74e17e2

resolve google oAuth dependency issue

c341cd2

resolve 'method does not override or implement a method from a supert…

1aa8ea9

…ype' issue

address review comments: remove genericType from AbstractIam

3269c61

update version

b8fcdf3

SriramGuduri force-pushed the W-20111589/iamCreateIdentityGCP branch from 8f7a02b to b8fcdf3 Compare November 17, 2025 06:51

sandeepvinayak reviewed Nov 17, 2025

View reviewed changes

		// Replay path - inject mock credentials
		// Note: Since WireMock doesn't support gRPC, replay mode is not yet implemented.

iam: implement doCreateIdentity for GCP #142

Are you sure you want to change the base?

iam: implement doCreateIdentity for GCP #142

Uh oh!

Conversation

SriramGuduri commented Nov 12, 2025

Summary

Some conventions to follow

Uh oh!

salesforce-cla bot commented Nov 12, 2025

Uh oh!

codecov-commenter commented Nov 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

salesforce-cla bot commented Nov 12, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SriramGuduri Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SriramGuduri Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SriramGuduri Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov-commenter commented Nov 12, 2025 •

edited

Loading

SriramGuduri Nov 18, 2025 •

edited

Loading

SriramGuduri Nov 18, 2025 •

edited

Loading

SriramGuduri Nov 18, 2025 •

edited

Loading