Feature/cstackex-01: Primary Storage pool creation by sandeeplocharla · Pull Request #16 · NetApp/cloudstack

sandeeplocharla · 2025-11-04T04:41:27Z

Description

This PR includes changes for Primary storage pool creation

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)
Build/CI
Test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Primary Storage Pool 'SanPrimary' creation has been tested from Management Server

How did you try to break this feature and the system with this change?

… to work

Copilot

Pull Request Overview

This PR implements primary storage pool creation functionality for NetApp ONTAP integration in CloudStack. The changes migrate from Spring Cloud OpenFeign to standard OpenFeign, refactor dependency injection patterns, and add core infrastructure for creating and managing ONTAP storage pools.

Key Changes:

Migration from Spring Cloud OpenFeign to core OpenFeign library with manual client initialization
Implementation of primary storage pool creation workflow including SVM validation and volume creation
Refactoring of dependency injection from Spring @Inject to constructor-based instantiation

Reviewed Changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
pom.xml	Replaces Spring Cloud OpenFeign dependencies with core OpenFeign libraries (feign-core, feign-httpclient, feign-jackson)
FeignClientFactory.java	New factory class for creating Feign clients with custom configuration
FeignConfiguration.java	Migrated from Spring @bean annotations to factory methods for Feign components
All FeignClient interfaces	Converted from Spring annotations (@FeignClient, @RequestMapping) to core Feign annotations (@RequestLine, @headers)
StorageStrategy.java	Replaced @Inject dependencies with constructor-based initialization using FeignClientFactory
UnifiedSANStrategy.java, UnifiedNASStrategy.java	Added constructor-based initialization and setter methods for OntapStorage
StorageProviderFactory.java	Converted from Spring @component to static factory pattern
OntapPrimaryDatastoreLifecycle.java	Implemented primary storage creation logic with URL parsing and validation
OntapStorage.java	Fixed setter methods to properly assign instance variables
Utility.java	Removed unused methods and Spring dependencies
Constants.java	Added SIZE constant, removed unused API endpoint constants

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-04T04:43:35Z

...torage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java

            s_logger.info("Successfully connected to ONTAP cluster and validated ONTAP details provided");
        } catch (Exception e) {
-           throw new CloudRuntimeException("Failed to connect to ONTAP cluster: " + e.getMessage());
+            throw new CloudRuntimeException("Failed to connect to ONTAP cluster: " + e);


The error message loses the original exception's specific message. Consider using 'e.getMessage()' instead of 'e' to avoid printing the full stack trace in the message, or wrap the exception properly with 'new CloudRuntimeException("Failed to connect to ONTAP cluster: " + e.getMessage(), e)' to preserve the cause.

Suggested change

throw new CloudRuntimeException("Failed to connect to ONTAP cluster: " + e);

throw new CloudRuntimeException("Failed to connect to ONTAP cluster: " + e.getMessage(), e);

Copilot · 2025-11-04T04:43:35Z

...torage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java

+        s_logger.info("Attempting to connect to ONTAP cluster at " + storage.getManagementLIF() + " and validate SVM " +
+                storage.getSvmName() + ", username " + storage.getUsername() + ", password " + storage.getPassword() + ", protocol " + storage.getProtocol());


Logging sensitive credentials (username and password) in plaintext is a security risk. Remove password from the log statement and consider masking or omitting the username as well.

Suggested change

s_logger.info("Attempting to connect to ONTAP cluster at " + storage.getManagementLIF() + " and validate SVM " +

storage.getSvmName() + ", username " + storage.getUsername() + ", password " + storage.getPassword() + ", protocol " + storage.getProtocol());

// Mask username except first character, if present

String maskedUsername = storage.getUsername() != null && !storage.getUsername().isEmpty()

? storage.getUsername().charAt(0) + "***"

: "";

s_logger.info("Attempting to connect to ONTAP cluster at " + storage.getManagementLIF() +

" and validate SVM " + storage.getSvmName() +

", username " + maskedUsername +

", protocol " + storage.getProtocol());

Copilot · 2025-11-04T04:43:36Z

...ge/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java

-                        @PathVariable(name="igroup.uuid", required=true) String igroupUUID);
-
+    // LUN Operation APIs
+    @RequestLine("POST /")


The RequestLine is incomplete - it uses '/' as the path, but based on the previous implementation this should include the full API path like 'POST /api/storage/luns'. Without the full path in the annotation, the API calls will fail.

Added TODO, for developers who are going to use this file

...ap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java

Copilot · 2025-11-04T04:43:36Z

...orage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/FeignConfiguration.java

+                    return null;
+                }
+                String json = null;
+                try (var bodyStream = response.body().asInputStream()) {


[nitpick] Using 'var' for type inference is valid in Java 11+, but may reduce code readability. Consider explicitly declaring the type as 'InputStream bodyStream' for clarity.

Suggested change

try (var bodyStream = response.body().asInputStream()) {

try (InputStream bodyStream = response.body().asInputStream()) {

plugins/storage/volume/ontap/pom.xml

...ge/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java

...ap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java

Copilot · 2025-11-04T04:43:37Z

...torage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java

+            try {
+                s_logger.info("Fetching the SVM details...");
+                if (svmFeignClient == null) {
+                    throw new CloudRuntimeException("SVM Feign client is not initialized.");
+                }
+                Map<String, Object> queryParams = Map.of("name", svmName, "fields", "aggregates,state");
+                OntapResponse<Svm> svms = svmFeignClient.getSvmResponse(queryParams, authHeader);
+                if (svms != null && svms.getRecords() != null && !svms.getRecords().isEmpty()) {
+                    svm = svms.getRecords().get(0);
+                } else {
+                    throw new CloudRuntimeException("No SVM found on the ONTAP cluster by the name" + svmName + ".");
+                }
+            } catch (FeignException.FeignClientException e) {
+                throw new CloudRuntimeException("Failed to fetch SVM details: " + e.getMessage());
            }


This catch block only handles FeignException.FeignClientException, but line 129 has a broader catch block for all exceptions. The inner try-catch adds unnecessary nesting and the outer catch will handle FeignException already. Consider removing the inner try-catch block.

Suggested change

try {

s_logger.info("Fetching the SVM details...");

if (svmFeignClient == null) {

throw new CloudRuntimeException("SVM Feign client is not initialized.");

}

Map<String, Object> queryParams = Map.of("name", svmName, "fields", "aggregates,state");

OntapResponse<Svm> svms = svmFeignClient.getSvmResponse(queryParams, authHeader);

if (svms != null && svms.getRecords() != null && !svms.getRecords().isEmpty()) {

svm = svms.getRecords().get(0);

} else {

throw new CloudRuntimeException("No SVM found on the ONTAP cluster by the name" + svmName + ".");

}

} catch (FeignException.FeignClientException e) {

throw new CloudRuntimeException("Failed to fetch SVM details: " + e.getMessage());

}

s_logger.info("Fetching the SVM details...");

if (svmFeignClient == null) {

throw new CloudRuntimeException("SVM Feign client is not initialized.");

}

Map<String, Object> queryParams = Map.of("name", svmName, "fields", "aggregates,state");

OntapResponse<Svm> svms = svmFeignClient.getSvmResponse(queryParams, authHeader);

if (svms != null && svms.getRecords() != null && !svms.getRecords().isEmpty()) {

svm = svms.getRecords().get(0);

} else {

throw new CloudRuntimeException("No SVM found on the ONTAP cluster by the name" + svmName + ".");

}

sandeeplocharla · 2025-11-04T06:59:52Z

...me/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java


    }
+
+    public StorageStrategy getStrategyByStoragePoolDetails(Map<String, String> details) {


make it 'private'

sandeeplocharla · 2025-11-04T07:05:43Z

...ge/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java

+    @Headers({"Authorization: {authHeader}", "return_records: {returnRecords}"})
+    OntapResponse<Lun> createLun(@Param("authHeader") String authHeader,
+                                @Param("returnRecords") boolean returnRecords,
+                                @Param("lun") Lun lun);


TODO: All feign clients should be revisited for proper implementation according to ontap api spec

Done
Developer now has to only take care of the URLs in SANFeignClient and NASFeignClient files. Will try to make those changes also in the next PR, if its not already taken care of

sandeeplocharla · 2025-11-04T07:07:43Z

...orage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/OntapStorage.java

-    private  String password;
-    private  String managementLIF;
-    private  String svmName;
+    private String username;


make them all final

sandeeplocharla · 2025-11-04T07:08:18Z

...me/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java

    private static final Logger s_logger = LogManager.getLogger(OntapPrimaryDatastoreDriver.class);

-    @Inject private Utility utils;
+    private Utility utils;


make it 'final'

sandeeplocharla · 2025-11-04T07:13:12Z

plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Svm.java

 import java.util.List;
 import java.util.Objects;

 @JsonInclude(JsonInclude.Include.NON_NULL)


Add Unknown properties to all models

its ignore unknown propoerties

sandeeplocharla · 2025-11-04T07:18:33Z

...orage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/FeignConfiguration.java

-    @Bean
-    public Client client(ApacheHttpClientFactory httpClientFactory) {
+    public FeignConfiguration() {
+        this.objectMapper = new ObjectMapper();


Check out if json mapper could be used here to ignore unknown properties

sandeeplocharla · 2025-11-04T07:23:03Z

...ap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java

+        Long clusterId = (Long)dsInfos.get("clusterId");
+        String storagePoolName = (String)dsInfos.get("name");
+        String providerName = (String)dsInfos.get("providerName");
+        Long capacityBytes = (Long)dsInfos.get("capacityBytes");


Reuse this for Volume size

sandeeplocharla · 2025-11-04T07:23:37Z

...ap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java

+        String storagePoolName = (String)dsInfos.get("name");
+        String providerName = (String)dsInfos.get("providerName");
+        Long capacityBytes = (Long)dsInfos.get("capacityBytes");
+        String tags = (String)dsInfos.get("tags");


Add validation on this (for NPE)

Not necessary, as cloudstack provides 'tags' key in dsInfos map, while the value could be 'null', if in case tags are not set.

sandeeplocharla · 2025-11-04T07:26:01Z

...ap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java

        @SuppressWarnings("unchecked")
        Map<String, String> details = (Map<String, String>)dsInfos.get("details");
        // Validations
+        if (capacityBytes == null || capacityBytes <= 0) {


Add Ontap related check here

sandeeplocharla · 2025-11-04T07:35:51Z

...ap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java

        }

+        // Get ONTAP details from the URL
+        Map<String, String> storageDetails = Map.of(


Use hashset from here and check below if the key exists in it before setting, else throw an exception

Done, explaination on current implementation has been provided for another comment on the same file

sandeeplocharla · 2025-11-04T07:37:16Z

...ap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java

        // TODO: scheme could be 'custom' in our case and we might have to ask 'protocol' separately to the user
-        ProtocolType protocol = ProtocolType.valueOf(details.get(Constants.PROTOCOL).toLowerCase());
+        String path = "";
+        ProtocolType protocol = ProtocolType.valueOf(details.get(Constants.PROTOCOL));


Add NFS3 in the Protocols enum

...me/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java

...ume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/AggregateFeignClient.java

...olume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/ClusterFeignClient.java

rajiv-jain-netapp · 2025-11-04T10:44:56Z

...ge/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java

+
+    @RequestLine("POST /{uuid}/igroups")
+    @Headers({"Authorization: {authHeader}", "return_records: {returnRecords}"})
+    OntapResponse<Igroup> addNestedIgroups(@Param("authHeader") String authHeader,


we will not have nested igroups here

pending to be addressed

...ge/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SvmFeignClient.java

rajiv-jain-netapp · 2025-11-04T10:54:56Z

...ap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java

@@ -160,6 +200,8 @@
        parameters.setName(storagePoolName);
        parameters.setProviderName(providerName);
        parameters.setManaged(true);


why is this hardcoded, shall we assign this based on the check box value coming from CS UI form?

Whenever, user chooses ONTAP storage provider, any entity created in cloudstack via ONTAP provider should be managed by us. So, by default irrespective of whether user chooses to be managed or not, this should be 'true'. I've checked other vendors implementation also doing the same.
One thing that we could do is to check if 'managed' field is set to 'null' by the user, callout via an exception to draw user's attention to enable it.
Let me know if we should be doing this?

rajiv-jain-netapp · 2025-11-04T10:56:39Z

...olume/ontap/src/main/java/org/apache/cloudstack/storage/provider/StorageProviderFactory.java

        ProtocolType protocol = ontapStorage.getProtocol();
        s_logger.info("Initializing StorageProviderFactory with protocol: " + protocol);
        switch (protocol) {
            case NFS:


lets add NFS3 even here

rajiv-jain-netapp · 2025-11-04T10:59:21Z

...torage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java

    public boolean connect() {
-        s_logger.info("Attempting to connect to ONTAP cluster at " + storage.getManagementLIF());
+        s_logger.info("Attempting to connect to ONTAP cluster at " + storage.getManagementLIF() + " and validate SVM " +
+                storage.getSvmName() + ", username " + storage.getUsername() + ", password " + storage.getPassword() + ", protocol " + storage.getProtocol());


do not print username and password in logs

rajiv-jain-netapp · 2025-11-04T11:01:01Z

...torage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java

-                throw new CloudRuntimeException("No SVM found on the ONTAP cluster by the name" + svmName + ".");
+            try {
+                s_logger.info("Fetching the SVM details...");
+                if (svmFeignClient == null) {


This initialization is happening in the constructor of this class, do we see any chances of not getting this initialized?

No this came in from earlier code where we were using spring-feign, will don't need this

rajiv-jain-netapp · 2025-11-04T11:02:45Z

...torage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java

    abstract AccessGroup updateAccessGroup(AccessGroup accessGroup);
-
-    /**
-     * Method encapsulates the behavior based on the opted protocol in subclasses


Any reasons for removing these methods?

Reverted them, thankyou
Copilot has removed them due to a comment, I've added for my reference in the code

suryag1201 · 2025-11-04T04:53:16Z

plugins/storage/volume/ontap/pom.xml

+        <dependency>
+            <groupId>com.fasterxml.jackson.core</groupId>
+            <artifactId>jackson-databind</artifactId>
+            <version>2.15.2</version>


Put this version inside property tag

Done, thankyou

suryag1201 · 2025-11-04T04:53:30Z

plugins/storage/volume/ontap/pom.xml

+        <dependency>
+            <groupId>org.apache.httpcomponents</groupId>
+            <artifactId>httpclient</artifactId>
+            <version>4.5.14</version>


Done, thankyou

suryag1201 · 2025-11-04T15:25:34Z

...me/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java

+    private Utility utils;
    @Inject private StoragePoolDetailsDao storagePoolDetailsDao;
    @Inject private PrimaryDataStoreDao storagePoolDao;
+    public OntapPrimaryDatastoreDriver() {


can also create utility method as static, so we need not to instantiate utility class here

Done, thankyou

suryag1201 · 2025-11-04T15:30:06Z

...torage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java

+        this.utils = new Utility();
+        // Initialize FeignClientFactory and create clients
+        this.feignClientFactory = new FeignClientFactory();
+        this.volumeFeignClient = feignClientFactory.createClient(VolumeFeignClient.class, baseURL);


we can also create the feign client based on the protocol

Not currently required in StorageStrategy also any protocol specific feign calls must be made in the respective classes

suryag1201 · 2025-11-04T16:01:12Z

...torage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java

+                if (svmFeignClient == null) {
+                    throw new CloudRuntimeException("SVM Feign client is not initialized.");
+                }
+                Map<String, Object> queryParams = Map.of("name", svmName, "fields", "aggregates,state");


pass these values as a constant

Copilot

Pull Request Overview

Copilot reviewed 42 out of 42 changed files in this pull request and generated 6 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-05T03:28:57Z

...ge/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java

+    @RequestLine("POST /")
+    @Headers({"Authorization: {authHeader}", "return_records: {returnRecords}"})
+    OntapResponse<Lun> createLun(@Param("authHeader") String authHeader,
+                                @Param("returnRecords") boolean returnRecords,
+                                Lun lun);


The @RequestLine annotation specifies only POST / without the full API path. This should include the complete endpoint path like POST /api/storage/luns to match the ONTAP API structure, similar to other Feign clients in this PR.

Its a TODO task to be taken up when using these methods in implementation

...ge/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java

...ap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java

Copilot · 2025-11-05T03:28:58Z

...orage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/FeignConfiguration.java

-                logger.info("Body: {}", template.requestBody().asString());
+            public void encode(Object object, Type bodyType, feign.RequestTemplate template) throws EncodeException {
+                if (object == null) {
+                    template.body((byte[]) null, StandardCharsets.UTF_8);


Casting null to (byte[]) is unnecessary. The method accepts a null value directly, so this cast can be removed for cleaner code.

Suggested change

template.body((byte[]) null, StandardCharsets.UTF_8);

template.body(null, StandardCharsets.UTF_8);

...ap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java

Copilot · 2025-11-05T03:28:58Z

...torage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java

            s_logger.info("Successfully connected to ONTAP cluster and validated ONTAP details provided");
        } catch (Exception e) {
-           throw new CloudRuntimeException("Failed to connect to ONTAP cluster: " + e.getMessage());
+            throw new CloudRuntimeException("Failed to connect to ONTAP cluster: " + e.getMessage(), e);


The error message only includes the exception message but loses important context about which SVM connection failed. Consider including the SVM name and management LIF in the error message for better troubleshooting: \"Failed to connect to ONTAP cluster at \" + storage.getManagementLIF() + \" for SVM \" + storage.getSvmName() + \": \" + e.getMessage()

Suggested change

throw new CloudRuntimeException("Failed to connect to ONTAP cluster: " + e.getMessage(), e);

throw new CloudRuntimeException("Failed to connect to ONTAP cluster at " + storage.getManagementLIF() + " for SVM " + storage.getSvmName() + ": " + e.getMessage(), e);

The details are already being logged earlier in the code

rajiv-jain-netapp

Multiple comments are pending

rajiv-jain-netapp · 2025-11-05T03:37:02Z

...me/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java

-    @Inject private Utility utils;
    @Inject private StoragePoolDetailsDao storagePoolDetailsDao;
    @Inject private PrimaryDataStoreDao storagePoolDao;
+    public OntapPrimaryDatastoreDriver() {}


Why do we need this constructor?

Not needed. Removed

...olume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/ClusterFeignClient.java

rajiv-jain-netapp · 2025-11-05T03:45:46Z

...ge/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java

+
+    @RequestLine("POST /{uuid}/igroups")
+    @Headers({"Authorization: {authHeader}", "return_records: {returnRecords}"})
+    OntapResponse<Igroup> addNestedIgroups(@Param("authHeader") String authHeader,


pending to be addressed

suryag1201 · 2025-11-05T08:17:00Z

...ap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java

                break;
            case ISCSI:
                parameters.setType(Storage.StoragePoolType.Iscsi);
+                path = "iqn.1992-08.com.netapp:" + details.get(Constants.SVM_NAME) + "." + storagePoolName;


can put this in constant

Locharla, Sandeep added 2 commits November 3, 2025 11:22

CSTACKEX-1: Feign changes and fixes for getting storage pool creation…

ec1707b

… to work

CSTACKEX-01: Create Primary Storage pool changes with working code

b855306

Copilot AI review requested due to automatic review settings November 4, 2025 04:41

sandeeplocharla self-assigned this Nov 4, 2025

sandeeplocharla requested review from piyush5netapp, rajiv-jain-netapp and suryag1201 November 4, 2025 04:42

Copilot AI reviewed Nov 4, 2025

View reviewed changes

sandeeplocharla commented Nov 4, 2025

View reviewed changes

rajiv-jain-netapp reviewed Nov 4, 2025

View reviewed changes

suryag1201 requested changes Nov 4, 2025

View reviewed changes

suryag1201 reviewed Nov 4, 2025

View reviewed changes

Locharla, Sandeep added 2 commits November 5, 2025 08:09

CSTACKEX-01: Addressed all review comments and updated some code

fd67a41

CSTACKEX-01: Made some changes to fix some errors seen during testing

43c3684

Copilot AI review requested due to automatic review settings November 5, 2025 03:27

Copilot AI reviewed Nov 5, 2025

View reviewed changes

rajiv-jain-netapp reviewed Nov 5, 2025

View reviewed changes

CSTACKEX-01: Addressed additional comments

80b24ca

sandeeplocharla requested review from rajiv-jain-netapp and suryag1201 November 5, 2025 05:28

rajiv-jain-netapp approved these changes Nov 5, 2025

View reviewed changes

suryag1201 reviewed Nov 5, 2025

View reviewed changes

suryag1201 approved these changes Nov 5, 2025

View reviewed changes

sandeeplocharla merged commit 1b0c7f7 into main Nov 5, 2025
13 of 16 checks passed

	throw new CloudRuntimeException("Failed to connect to ONTAP cluster: " + e);
	throw new CloudRuntimeException("Failed to connect to ONTAP cluster: " + e.getMessage(), e);

		s_logger.info("Attempting to connect to ONTAP cluster at " + storage.getManagementLIF() + " and validate SVM " +
		storage.getSvmName() + ", username " + storage.getUsername() + ", password " + storage.getPassword() + ", protocol " + storage.getProtocol());

-        s_logger.info("Attempting to connect to ONTAP cluster at " + storage.getManagementLIF() + " and validate SVM " +
-                storage.getSvmName() + ", username " + storage.getUsername() + ", password " + storage.getPassword() + ", protocol " + storage.getProtocol());
+        // Mask username except first character, if present
+        String maskedUsername = storage.getUsername() != null && !storage.getUsername().isEmpty()
+                ? storage.getUsername().charAt(0) + "***"
+                : "";
+        s_logger.info("Attempting to connect to ONTAP cluster at " + storage.getManagementLIF() +
+                " and validate SVM " + storage.getSvmName() +
+                ", username " + maskedUsername +
+                ", protocol " + storage.getProtocol());

	try (var bodyStream = response.body().asInputStream()) {
	try (InputStream bodyStream = response.body().asInputStream()) {


		}

		public StorageStrategy getStrategyByStoragePoolDetails(Map<String, String> details) {

	template.body((byte[]) null, StandardCharsets.UTF_8);
	template.body(null, StandardCharsets.UTF_8);

Conversation

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

Uh oh!

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Nov 4, 2025

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!

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!

sandeeplocharla commented Nov 4, 2025 •

edited

Loading