Skip to content

Commit e6145c8

Browse files
tpodowdtpodowd
committed
Fix timeout typo for CloudianClient connections
- The timeout parameter was using the port so instead of timing out in 10 seconds, it was using 19443 seconds. - Added tests to use real connections instead of mocking and added line tests to try catch other issues. - Noticed that HyperStore and AWS IAM services use return different errorcodes. This will be fixed in HyperStore so handle both errorcodes.
1 parent 40c0366 commit e6145c8

File tree

3 files changed

+247
-5
lines changed

3 files changed

+247
-5
lines changed

‎plugins/storage/object/cloudian/pom.xml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,5 +60,11 @@
6060
<groupId>com.amazonaws</groupId>
6161
<artifactId>aws-java-sdk-s3</artifactId>
6262
</dependency>
63+
<dependency>
64+
<groupId>com.github.tomakehurst</groupId>
65+
<artifactId>wiremock-standalone</artifactId>
66+
<version>${cs.wiremock.version}</version>
67+
<scope>test</scope>
68+
</dependency>
6369
</dependencies>
6470
</project>

‎plugins/storage/object/cloudian/src/main/java/org/apache/cloudstack/storage/datastore/util/CloudianHyperStoreUtil.java‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ public class CloudianHyperStoreUtil {
6565
public static final String KEY_IAM_SECRET_KEY = "hs_IAMSecretKey";
6666

6767
public static final int DEFAULT_ADMIN_PORT = 19443;
68-
public static final int DEFAULT_ADMIN_TIMEOUT = 10;
68+
public static final int DEFAULT_ADMIN_TIMEOUT_SECONDS = 10;
6969

7070
public static final String IAM_USER_USERNAME = "CloudStack";
7171
public static final String IAM_USER_POLICY_NAME = "CloudStackPolicy";
@@ -97,6 +97,15 @@ public class CloudianHyperStoreUtil {
9797
IAM_USER_POLICY = sb.toString();
9898
}
9999

100+
/**
101+
* This method is solely for test purposes so that we can mock the timeout.
102+
*
103+
* @returns the timeout in seconds
104+
*/
105+
protected static int getAdminTimeoutSeconds() {
106+
return DEFAULT_ADMIN_TIMEOUT_SECONDS;
107+
}
108+
100109
/**
101110
* Get a connection to the Cloudian HyperStore ADMIN API Service.
102111
* @param url the url of the ADMIN API service
@@ -115,7 +124,7 @@ public static CloudianClient getCloudianClient(String url, String user, String p
115124
if (port == -1) {
116125
port = DEFAULT_ADMIN_PORT;
117126
}
118-
return new CloudianClient(host, port, scheme, user, pass, validateSSL, DEFAULT_ADMIN_PORT);
127+
return new CloudianClient(host, port, scheme, user, pass, validateSSL, getAdminTimeoutSeconds());
119128
} catch (MalformedURLException e) {
120129
throw new CloudRuntimeException(e);
121130
} catch (KeyStoreException | NoSuchAlgorithmException | KeyManagementException e) {
@@ -194,8 +203,8 @@ public static void validateS3Url(String s3Url) {
194203
* Test the IAMUrl to confirm it behaves like an IAM Service.
195204
*
196205
* The method uses bad credentials and looks for the particular error from IAM
197-
* that says InvalidAccessKeyId was used. The method quietly returns if
198-
* we connect and get the expected error back.
206+
* that says InvalidAccessKeyId or InvalidClientTokenId was used. The method quietly
207+
* returns if we connect and get the expected error back.
199208
*
200209
* @param iamUrl the url to check
201210
*
@@ -206,7 +215,7 @@ public static void validateIAMUrl(String iamUrl) {
206215
AmazonIdentityManagement iamClient = CloudianHyperStoreUtil.getIAMClient(iamUrl, "unknown", "unknown");
207216
iamClient.listAccessKeys();
208217
} catch (AmazonServiceException e) {
209-
if (StringUtils.compareIgnoreCase(e.getErrorCode(), "InvalidAccessKeyId") != 0) {
218+
if (! StringUtils.equalsAnyIgnoreCase(e.getErrorCode(), "InvalidAccessKeyId", "InvalidClientTokenId")) {
210219
throw new CloudRuntimeException("Unexpected response from IAM Endpoint.", e);
211220
}
212221
}

‎plugins/storage/object/cloudian/src/test/java/org/apache/cloudstack/storage/datastore/util/CloudianHyperStoreUtilTest.java‎

Lines changed: 227 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,227 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
// SPDX-License-Identifier: Apache-2.0
18+
package org.apache.cloudstack.storage.datastore.util;
19+
20+
import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
21+
import static com.github.tomakehurst.wiremock.client.WireMock.get;
22+
import static com.github.tomakehurst.wiremock.client.WireMock.post;
23+
import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
24+
import static org.junit.Assert.assertEquals;
25+
import static org.junit.Assert.assertNotNull;
26+
import static org.junit.Assert.assertThrows;
27+
import static org.junit.Assert.assertTrue;
28+
import static org.mockito.ArgumentMatchers.anyBoolean;
29+
import static org.mockito.ArgumentMatchers.anyString;
30+
31+
import org.apache.cloudstack.api.ApiErrorCode;
32+
import org.apache.cloudstack.api.ServerApiException;
33+
import org.apache.cloudstack.cloudian.client.CloudianClient;
34+
import org.junit.After;
35+
import org.junit.Before;
36+
import org.junit.Rule;
37+
import org.junit.Test;
38+
import org.mockito.MockedStatic;
39+
import org.mockito.Mockito;
40+
import org.mockito.MockitoAnnotations;
41+
42+
import com.cloud.utils.exception.CloudRuntimeException;
43+
import com.github.tomakehurst.wiremock.junit.WireMockRule;
44+
45+
public class CloudianHyperStoreUtilTest {
46+
private final int port = 18081;
47+
48+
@Rule
49+
public WireMockRule wireMockRule = new WireMockRule(port);
50+
51+
private AutoCloseable closeable;
52+
53+
@Before
54+
public void setUp() {
55+
closeable = MockitoAnnotations.openMocks(this);
56+
}
57+
58+
@After
59+
public void tearDown() throws Exception {
60+
closeable.close();
61+
}
62+
63+
@Test
64+
public void testGetCloudianClientBadUrl() {
65+
String url = "bad://bad-url";
66+
CloudRuntimeException thrown = assertThrows(CloudRuntimeException.class, () -> CloudianHyperStoreUtil.getCloudianClient(url, "", "", false));
67+
assertNotNull(thrown);
68+
assertTrue(thrown.getMessage().contains("unknown protocol"));
69+
}
70+
71+
@Test
72+
public void testGetCloudianClient() {
73+
String url = "https://localhost:98765";
74+
CloudianClient cc = CloudianHyperStoreUtil.getCloudianClient(url, "", "", false);
75+
assertNotNull(cc);
76+
}
77+
78+
@Test
79+
public void testGetCloudianClientNoPort() {
80+
String url = "https://localhost";
81+
CloudianClient cc = CloudianHyperStoreUtil.getCloudianClient(url, "", "", false);
82+
assertNotNull(cc);
83+
}
84+
85+
@Test
86+
public void testGetCloudianClientToGetServerVersion() {
87+
final String expect = "8.1 Compiled: 2023-11-11 16:30";
88+
wireMockRule.stubFor(get(urlEqualTo("/system/version"))
89+
.willReturn(aResponse()
90+
.withStatus(200)
91+
.withBody(expect)));
92+
93+
// Get a connection and try using it
94+
String url = String.format("http://localhost:%d", port);
95+
CloudianClient cc = CloudianHyperStoreUtil.getCloudianClient(url, "u", "p", false);
96+
String version = cc.getServerVersion();
97+
assertEquals(expect, version);
98+
}
99+
100+
@Test
101+
public void testGetCloudianClientShortenedTimeout() {
102+
// Response delayed 3 seconds. We should never get it.
103+
final String expect = "8.1 Compiled: 2023-11-11 16:30";
104+
wireMockRule.stubFor(get(urlEqualTo("/system/version"))
105+
.willReturn(aResponse()
106+
.withStatus(200)
107+
.withFixedDelay(3000) // 3 second delay.
108+
.withBody(expect)));
109+
110+
try (MockedStatic<CloudianHyperStoreUtil> mockStatic = Mockito.mockStatic(CloudianHyperStoreUtil.class)) {
111+
// Force a shorter 1 second timeout for testing so as not to hold up unit tests.
112+
mockStatic.when(() -> CloudianHyperStoreUtil.getAdminTimeoutSeconds()).thenReturn(1);
113+
mockStatic.when(() -> CloudianHyperStoreUtil.getCloudianClient(anyString(), anyString(), anyString(), anyBoolean())).thenCallRealMethod();
114+
115+
// Get a connection and try using it but it should timeout
116+
String url = String.format("http://localhost:%d", port);
117+
CloudianClient cc = CloudianHyperStoreUtil.getCloudianClient(url, "u", "p", false);
118+
long before = System.currentTimeMillis();
119+
ServerApiException thrown = assertThrows(ServerApiException.class, () -> cc.getServerVersion());
120+
long after = System.currentTimeMillis();
121+
assertNotNull(thrown);
122+
assertEquals(ApiErrorCode.RESOURCE_UNAVAILABLE_ERROR, thrown.getErrorCode());
123+
assertTrue((after - before) >= 1000); // should timeout after 1 second.
124+
}
125+
}
126+
127+
@Test
128+
public void testValidateS3UrlGood() {
129+
// Mock an AWS S3 invalid access key response.
130+
StringBuilder ERR_XML = new StringBuilder("<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n");
131+
ERR_XML.append("<Error>\n");
132+
ERR_XML.append(" <Code>InvalidAccessKeyId</Code>\n");
133+
ERR_XML.append(" <Message>The AWS Access Key Id you provided does not exist in our records.</Message>\n");
134+
ERR_XML.append(" <AWSAccessKeyId>unknown</AWSAccessKeyId>\n");
135+
ERR_XML.append(" <HostId>12345=</HostId>\n");
136+
ERR_XML.append("</Error>\n");
137+
138+
wireMockRule.stubFor(get(urlEqualTo("/"))
139+
.willReturn(aResponse()
140+
.withStatus(403)
141+
.withHeader("content-type", "application/xml")
142+
.withBody(ERR_XML.toString())));
143+
144+
// Test: validates the AmazonS3 client returned by CloudianHyperStoreUtil.getS3Client()
145+
// which is called indirectly via the validateS3Url() method can connect to the
146+
// remote port and handles the access key error as the expected s3 response.
147+
String url = String.format("http://localhost:%d", port);
148+
CloudianHyperStoreUtil.validateS3Url(url);
149+
}
150+
151+
@Test
152+
public void testValidateS3UrlBadRequest() {
153+
wireMockRule.stubFor(get(urlEqualTo("/"))
154+
.willReturn(aResponse()
155+
.withStatus(400)
156+
.withHeader("content-type", "text/html")
157+
.withBody("<html><body>400 Bad Request</body></html>")));
158+
159+
String url = String.format("http://localhost:%d", port);
160+
CloudRuntimeException thrown = assertThrows(CloudRuntimeException.class, () -> CloudianHyperStoreUtil.validateS3Url(url));
161+
assertNotNull(thrown);
162+
}
163+
164+
@Test
165+
public void testValidateIAMUrlGoodInvalidClientTokenId() {
166+
// Mock an AWS IAM invalid access key response.
167+
StringBuilder ERR_XML = new StringBuilder();
168+
ERR_XML.append("<ErrorResponse xmlns=\"https://iam.amazonaws.com/doc/2010-05-08/\">\n");
169+
ERR_XML.append(" <Error>\n");
170+
ERR_XML.append(" <Type>Sender</Type>\n");
171+
ERR_XML.append(" <Code>InvalidClientTokenId</Code>\n");
172+
ERR_XML.append(" <Message>The security token included in the request is invalid.</Message>\n");
173+
ERR_XML.append(" </Error>\n");
174+
ERR_XML.append(" <RequestId>a2c47f7e-0196-4b45-af18-a9e99e4d9ed5</RequestId>\n");
175+
ERR_XML.append("</ErrorResponse>\n");
176+
177+
wireMockRule.stubFor(post(urlEqualTo("/"))
178+
.willReturn(aResponse()
179+
.withStatus(403)
180+
.withHeader("content-type", "text/xml")
181+
.withBody(ERR_XML.toString())));
182+
183+
// Test: validates the AmazonIdentityManagement client returned by CloudianHyperStoreUtil.getIAMClient()
184+
// which is called indirectly via the validateIAMUrl() method can connect to the
185+
// remote port and handles the access key error as the expected s3 response.
186+
String url = String.format("http://localhost:%d", port);
187+
CloudianHyperStoreUtil.validateIAMUrl(url);
188+
}
189+
190+
@Test
191+
public void testValidateIAMUrlGoodInvalidAccessKeyId() {
192+
// Mock HyperStore IAM invalid access key current response.
193+
StringBuilder ERR_XML = new StringBuilder("<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n");
194+
ERR_XML.append("<ErrorResponse xmlns=\"https://iam.amazonaws.com/doc/2010-05-08/\">\n");
195+
ERR_XML.append(" <Error>\n");
196+
ERR_XML.append(" <Code>InvalidAccessKeyId</Code>\n");
197+
ERR_XML.append(" <Message>The Access Key Id you provided does not exist in our records.</Message>\n");
198+
ERR_XML.append(" </Error>\n");
199+
ERR_XML.append(" <RequestId>a2c47f7e-0196-4b45-af18-a9e99e4d9ed5</RequestId>\n");
200+
ERR_XML.append("</ErrorResponse>\n");
201+
202+
wireMockRule.stubFor(post(urlEqualTo("/"))
203+
.willReturn(aResponse()
204+
.withStatus(403)
205+
.withHeader("content-type", "application/xml;charset=UTF-8")
206+
.withBody(ERR_XML.toString())));
207+
208+
// Test: validates the AmazonIdentityManagement client returned by CloudianHyperStoreUtil.getIAMClient()
209+
// which is called indirectly via the validateIAMUrl() method can connect to the
210+
// remote port and handles the access key error as the expected s3 response.
211+
String url = String.format("http://localhost:%d", port);
212+
CloudianHyperStoreUtil.validateIAMUrl(url);
213+
}
214+
215+
@Test
216+
public void testValidateIAMUrlBadRequest() {
217+
wireMockRule.stubFor(post(urlEqualTo("/"))
218+
.willReturn(aResponse()
219+
.withStatus(400)
220+
.withHeader("content-type", "text/html")
221+
.withBody("<html><body>400 Bad Request</body></html>")));
222+
223+
String url = String.format("http://localhost:%d", port);
224+
CloudRuntimeException thrown = assertThrows(CloudRuntimeException.class, () -> CloudianHyperStoreUtil.validateIAMUrl(url));
225+
assertNotNull(thrown);
226+
}
227+
}

0 commit comments

Comments
 (0)