Copilot commented on code in PR #13060:
URL: https://github.com/apache/cloudstack/pull/13060#discussion_r3128737537
##########
plugins/storage/volume/flasharray/src/main/java/org/apache/cloudstack/storage/datastore/adapter/flasharray/FlashArrayAdapter.java:
##########
@@ -661,72 +664,117 @@ private void login() {
skipTlsValidation = true;
}
+ // Resolve the long-lived API token. Prefer a pre-minted api_token
(Purity REST 2.x flow);
+ // fall back to legacy username/password auth via Purity REST 1.x for
backward compatibility.
+ String apiToken = connectionDetails.get(ProviderAdapter.API_TOKEN_KEY);
+ if (apiToken != null && apiToken.isEmpty()) {
+ apiToken = null;
+ }
+ boolean usingLegacyUserPass = apiToken == null;
+ if (usingLegacyUserPass && (username == null || password == null)) {
+ throw new CloudRuntimeException("FlashArray adapter requires
either " + ProviderAdapter.API_TOKEN_KEY
+ + " (preferred) or both " +
ProviderAdapter.API_USERNAME_KEY + " and "
+ + ProviderAdapter.API_PASSWORD_KEY + " in the connection
details");
+ }
+
+ CloseableHttpClient client = getClient();
CloseableHttpResponse response = null;
try {
- HttpPost request = new HttpPost(url + "/" + apiLoginVersion +
"/auth/apitoken");
- // request.addHeader("Content-Type", "application/json");
- // request.addHeader("Accept", "application/json");
- ArrayList<NameValuePair> postParms = new
ArrayList<NameValuePair>();
- postParms.add(new BasicNameValuePair("username", username));
- postParms.add(new BasicNameValuePair("password", password));
- request.setEntity(new UrlEncodedFormEntity(postParms, "UTF-8"));
- CloseableHttpClient client = getClient();
- response = (CloseableHttpResponse) client.execute(request);
+ // Discover the latest supported API version from the array unless
one was explicitly configured.
+ // GET /api/api_version is unauthenticated and returns
{"version":["1.0",...,"2.36"]}.
+ if (!apiVersionExplicit) {
+ HttpGet vReq = new HttpGet(url + "/api_version");
+ CloseableHttpResponse vResp = null;
+ try {
+ vResp = (CloseableHttpResponse) client.execute(vReq);
+ if (vResp.getStatusLine().getStatusCode() == 200) {
+ JsonNode root =
mapper.readTree(vResp.getEntity().getContent());
+ JsonNode versions = root.get("version");
+ if (versions != null && versions.isArray() &&
versions.size() > 0) {
+ apiVersion = versions.get(versions.size() -
1).asText();
+ }
+ } else {
+ logger.warn("Unexpected HTTP " +
vResp.getStatusLine().getStatusCode()
+ + " from FlashArray [" + url + "]
/api_version, falling back to default "
+ + API_VERSION_DEFAULT);
+ }
+ } catch (Exception e) {
+ logger.warn("Failed to discover Purity REST API version
from " + url
+ + "/api_version, falling back to default " +
API_VERSION_DEFAULT, e);
+ } finally {
+ if (vResp != null) {
+ vResp.close();
+ }
+ }
+ }
- int statusCode = response.getStatusLine().getStatusCode();
- FlashArrayApiToken apitoken = null;
- if (statusCode == 200 | statusCode == 201) {
- apitoken = mapper.readValue(response.getEntity().getContent(),
FlashArrayApiToken.class);
- if (apitoken == null) {
+ if (usingLegacyUserPass) {
+ logger.warn("FlashArray adapter at [" + url + "] is using
deprecated username/password "
+ + "login against Purity REST 1.x. Replace with a
pre-minted "
+ + ProviderAdapter.API_TOKEN_KEY + " detail; the
username/password code path will be "
+ + "removed in a future release.");
+ HttpPost request = new HttpPost(url + "/" + apiLoginVersion +
"/auth/apitoken");
+ ArrayList<NameValuePair> postParms = new
ArrayList<NameValuePair>();
+ postParms.add(new BasicNameValuePair("username", username));
+ postParms.add(new BasicNameValuePair("password", password));
+ request.setEntity(new UrlEncodedFormEntity(postParms,
"UTF-8"));
+ response = (CloseableHttpResponse) client.execute(request);
+ int statusCode = response.getStatusLine().getStatusCode();
+ if (statusCode == 200 || statusCode == 201) {
+ FlashArrayApiToken legacyToken =
mapper.readValue(response.getEntity().getContent(),
+ FlashArrayApiToken.class);
+ if (legacyToken == null || legacyToken.getApiToken() ==
null) {
+ throw new CloudRuntimeException(
+ "Authentication responded successfully but no
api token was returned");
+ }
+ apiToken = legacyToken.getApiToken();
+ } else if (statusCode == 401 || statusCode == 403) {
+ throw new CloudRuntimeException(
+ "Authentication or Authorization to FlashArray ["
+ url + "] with user [" + username
+ + "] failed, unable to retrieve session
token");
+ } else {
throw new CloudRuntimeException(
- "Authentication responded successfully but no api
token was returned");
+ "Unexpected HTTP response code from FlashArray ["
+ url + "] - [" + statusCode
+ + "] - " +
response.getStatusLine().getReasonPhrase());
}
- } else if (statusCode == 401 || statusCode == 403) {
- throw new CloudRuntimeException(
- "Authentication or Authorization to FlashArray [" +
url + "] with user [" + username
- + "] failed, unable to retrieve session
token");
- } else {
- throw new CloudRuntimeException(
- "Unexpected HTTP response code from FlashArray [" +
url + "] - [" + statusCode
- + "] - " +
response.getStatusLine().getReasonPhrase());
+ response.close();
+ response = null;
Review Comment:
`response.close()` in the legacy username/password branch is not protected
by a try/catch. If closing the response throws, `login()` will fail even after
a successful auth/token exchange. Close it in a try/catch (or rely on the
existing outer `finally`) to avoid turning close errors into authentication
failures.
```suggestion
try {
response.close();
} catch (IOException e) {
logger.warn("Failed to close legacy authentication
response from FlashArray [" + url + "]", e);
} finally {
response = null;
}
```
##########
plugins/storage/volume/flasharray/src/main/java/org/apache/cloudstack/storage/datastore/adapter/flasharray/FlashArrayAdapter.java:
##########
@@ -661,72 +664,117 @@ private void login() {
skipTlsValidation = true;
}
+ // Resolve the long-lived API token. Prefer a pre-minted api_token
(Purity REST 2.x flow);
+ // fall back to legacy username/password auth via Purity REST 1.x for
backward compatibility.
+ String apiToken = connectionDetails.get(ProviderAdapter.API_TOKEN_KEY);
+ if (apiToken != null && apiToken.isEmpty()) {
+ apiToken = null;
+ }
+ boolean usingLegacyUserPass = apiToken == null;
+ if (usingLegacyUserPass && (username == null || password == null)) {
+ throw new CloudRuntimeException("FlashArray adapter requires
either " + ProviderAdapter.API_TOKEN_KEY
+ + " (preferred) or both " +
ProviderAdapter.API_USERNAME_KEY + " and "
+ + ProviderAdapter.API_PASSWORD_KEY + " in the connection
details");
+ }
+
+ CloseableHttpClient client = getClient();
CloseableHttpResponse response = null;
try {
- HttpPost request = new HttpPost(url + "/" + apiLoginVersion +
"/auth/apitoken");
- // request.addHeader("Content-Type", "application/json");
- // request.addHeader("Accept", "application/json");
- ArrayList<NameValuePair> postParms = new
ArrayList<NameValuePair>();
- postParms.add(new BasicNameValuePair("username", username));
- postParms.add(new BasicNameValuePair("password", password));
- request.setEntity(new UrlEncodedFormEntity(postParms, "UTF-8"));
- CloseableHttpClient client = getClient();
- response = (CloseableHttpResponse) client.execute(request);
+ // Discover the latest supported API version from the array unless
one was explicitly configured.
+ // GET /api/api_version is unauthenticated and returns
{"version":["1.0",...,"2.36"]}.
+ if (!apiVersionExplicit) {
+ HttpGet vReq = new HttpGet(url + "/api_version");
+ CloseableHttpResponse vResp = null;
+ try {
+ vResp = (CloseableHttpResponse) client.execute(vReq);
+ if (vResp.getStatusLine().getStatusCode() == 200) {
+ JsonNode root =
mapper.readTree(vResp.getEntity().getContent());
+ JsonNode versions = root.get("version");
+ if (versions != null && versions.isArray() &&
versions.size() > 0) {
+ apiVersion = versions.get(versions.size() -
1).asText();
+ }
+ } else {
+ logger.warn("Unexpected HTTP " +
vResp.getStatusLine().getStatusCode()
+ + " from FlashArray [" + url + "]
/api_version, falling back to default "
+ + API_VERSION_DEFAULT);
+ }
+ } catch (Exception e) {
+ logger.warn("Failed to discover Purity REST API version
from " + url
+ + "/api_version, falling back to default " +
API_VERSION_DEFAULT, e);
+ } finally {
+ if (vResp != null) {
+ vResp.close();
Review Comment:
`vResp.close()` is called in the inner `finally` without handling
`IOException`. A close failure would currently bubble up and fail the entire
login even though the version discovery response was already processed. Wrap
the close in its own try/catch and log at debug (similar to the outer
`response` close) to avoid spurious login failures.
```suggestion
try {
vResp.close();
} catch (IOException e) {
logger.debug("Unable to close /api_version
response", e);
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
