Skip to content

Commit

Permalink
fixes for hazelcast-aws Github issues hazelcast#29 and hazelcast#22 (h…
Browse files Browse the repository at this point in the history
…azelcast#30)

* fixes for hazelcast-aws Github issues hazelcast#29 and hazelcast#22

* fixes for checkstyle errors

* fixes for checkstyle errors

* extended strategy factory tests

* removed Configuration to AwsConfig

* Configuration fixes, we now allow w/out any creds, or iam-role defined in config file.

* missing default value for timeout added
  • Loading branch information
Baris authored and emrahkocaman committed Jun 29, 2017
1 parent b8e1bb8 commit b04f49f
Show file tree
Hide file tree
Showing 9 changed files with 109 additions and 26 deletions.
4 changes: 2 additions & 2 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>

<jdk.version>1.6</jdk.version>
<hazelcast.version>3.7.1</hazelcast.version>
<hazelcast.version>3.8.2</hazelcast.version>

<junit.version>4.12</junit.version>
<mockito.version>1.10.19</mockito.version>
Expand Down Expand Up @@ -492,7 +492,7 @@
</property>
</javaApiLinks>
<excludePackageNames>
*.impl:*.internal:*.operations:*.proxy:*.util:com.hazelcast.aws.security:
*.impl:*.internal:*.operations:*.proxy:*.util:com.hazelcast.aws.security:com:hazelcast:aws:
*.handlermigration:*.client.connection.nio:*.client.console:*.buildutils:
*.client.protocol.generator:*.cluster.client:*.concurrent:*.collection:
*.nio.ascii:*.nio.ssl:*.nio.tcp:*.partition.client:*.transaction.client:
Expand Down
37 changes: 32 additions & 5 deletions src/main/java/com/hazelcast/aws/AwsDiscoveryStrategy.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.hazelcast.spi.discovery.DiscoveryNode;
import com.hazelcast.spi.discovery.DiscoveryStrategy;
import com.hazelcast.spi.discovery.SimpleDiscoveryNode;
import com.hazelcast.util.StringUtil;

import java.util.ArrayList;
import java.util.Collections;
Expand Down Expand Up @@ -66,18 +67,24 @@ public AwsDiscoveryStrategy(Map<String, Comparable> properties) {
private AwsConfig getAwsConfig() throws IllegalArgumentException {
final AwsConfig config = new AwsConfig()
.setEnabled(true)
.setAccessKey(getOrNull(ACCESS_KEY))
.setSecretKey(getOrNull(SECRET_KEY))
.setSecurityGroupName(getOrNull(SECURITY_GROUP_NAME))
.setTagKey(getOrNull(TAG_KEY))
.setTagValue(getOrNull(TAG_VALUE))
.setIamRole(getOrNull(IAM_ROLE));

final Integer timeout = getOrNull(CONNECTION_TIMEOUT_SECONDS.getDefinition());
if (timeout != null) {
config.setConnectionTimeoutSeconds(timeout);
String property = getOrNull(ACCESS_KEY);
if (property != null) {
config.setAccessKey(property);
}

property = getOrNull(SECRET_KEY);
if (property != null) {
config.setSecretKey(property);
}

final Integer timeout = getOrDefault(CONNECTION_TIMEOUT_SECONDS.getDefinition(), 10);
config.setConnectionTimeoutSeconds(timeout);

final String region = getOrNull(REGION);
if (region != null) {
config.setRegion(region);
Expand All @@ -87,9 +94,29 @@ private AwsConfig getAwsConfig() throws IllegalArgumentException {
if (hostHeader != null) {
config.setHostHeader(hostHeader);
}

reviewConfiguration(config);
return config;
}

private void reviewConfiguration(AwsConfig config) {
if (StringUtil.isNullOrEmptyAfterTrim(config.getSecretKey())
|| StringUtil.isNullOrEmptyAfterTrim(config.getAccessKey())) {

if (!StringUtil.isNullOrEmptyAfterTrim(config.getIamRole())) {
getLogger().info("Describe instances will be queried with iam-role, "
+ "please make sure given iam-role have ec2:DescribeInstances policy attached.");
} else {
getLogger().warning("Describe instances will be queried with iam-role assigned to EC2 instance, "
+ "please make sure given iam-role have ec2:DescribeInstances policy attached.");
}
} else {
if (!StringUtil.isNullOrEmptyAfterTrim(config.getIamRole())) {
getLogger().info("No need to define iam-role, when access and secret keys are configured!");
}
}
}

@Override
public Iterable<DiscoveryNode> discoverNodes() {
try {
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/com/hazelcast/aws/AwsProperties.java
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,11 @@ public enum AwsProperties {
/**
* Access key of your account on EC2
*/
ACCESS_KEY("access-key", STRING, false),
ACCESS_KEY("access-key", STRING, true),
/**
* Secret key of your account on EC2
*/
SECRET_KEY("secret-key", STRING, false),
SECRET_KEY("secret-key", STRING, true),
/**
* The region where your members are running. Default value is us-east-1. You need to specify this if the region is other
* than the default one.
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/com/hazelcast/aws/impl/DescribeInstances.java
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ public Map<String, String> execute() throws Exception {
attributes.put("X-Amz-Signature", signature);
try {
stream = callService(endpoint);
response = CloudyUtility.unmarshalTheResponse(stream, awsConfig);
response = CloudyUtility.unmarshalTheResponse(stream);
return response;
} finally {
closeResource(stream);
Expand All @@ -283,6 +283,7 @@ private InputStream callService(String endpoint) throws Exception {
URL url = new URL("https", endpoint, -1, "/?" + query);
HttpURLConnection httpConnection = (HttpURLConnection) (url.openConnection());
httpConnection.setRequestMethod(Constants.GET);
httpConnection.setConnectTimeout(awsConfig.getConnectionTimeoutSeconds());
httpConnection.setDoOutput(false);
httpConnection.connect();
return httpConnection.getInputStream();
Expand Down
4 changes: 1 addition & 3 deletions src/main/java/com/hazelcast/aws/utility/CloudyUtility.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

package com.hazelcast.aws.utility;

import com.hazelcast.config.AwsConfig;
import com.hazelcast.logging.ILogger;
import com.hazelcast.logging.Logger;
import org.w3c.dom.Document;
Expand Down Expand Up @@ -53,10 +52,9 @@ private CloudyUtility() {
* If there is an exception while unmarshalling the response, returns an empty map.
*
* @param stream the response XML stream
* @param awsConfig the AWS configuration for filtering the returned addresses
* @return map from private to public IP or empty map in case of exceptions
*/
public static Map<String, String> unmarshalTheResponse(InputStream stream, AwsConfig awsConfig) {
public static Map<String, String> unmarshalTheResponse(InputStream stream) {
DocumentBuilder builder;
try {
DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ public class TcpIpJoinerOverAWS extends TcpIpJoiner {
public TcpIpJoinerOverAWS(Node node) {
super(node);
logger = node.getLogger(getClass());

AwsConfig awsConfig = node.getConfig().getNetworkConfig().getJoin().getAwsConfig();
aws = new AWSClient(awsConfig);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,34 +47,34 @@
@Category(QuickTest.class)
public class AwsDiscoveryStrategyFactoryTest extends HazelcastTestSupport {


@Test(expected = InvalidConfigurationException.class)
public void missingAccessKey() throws Exception {
public void hostHeaderMalformed() throws Exception {
final Map<String, Comparable> props = new HashMap<String, Comparable>();
props.put("access-key", "test-value");
props.put("secret-key", "test-value");
props.put("host-header", "test-value");
createStrategy(props);
}

@Test(expected = InvalidConfigurationException.class)
public void missingSecretKey() throws Exception {
@Test
public void testMinimalOk() throws Exception {
final Map<String, Comparable> props = new HashMap<String, Comparable>();
props.put("access-key", "test-value");
createStrategy(props);
}

@Test(expected = InvalidConfigurationException.class)
public void hostHeaderMalformed() throws Exception {
@Test
public void testOnlyGivenRoleOk(){
final Map<String, Comparable> props = new HashMap<String, Comparable>();
props.put("access-key", "test-value");
props.put("secret-key", "test-value");
props.put("host-header", "test-value");
props.put("iam-role", "test-value");
createStrategy(props);
}

@Test
public void testMinimalOk() throws Exception {
public void testOnlyAccessAndSecretKeyOk(){
final Map<String, Comparable> props = new HashMap<String, Comparable>();
props.put("access-key", "test-value");
props.put("secret-key", "test-value");
props.put("access-key", "test-value");
createStrategy(props);
}

Expand All @@ -96,8 +96,17 @@ public void testFull() throws Exception {

@Test
public void parseAndCreateDiscoveryStrategyPasses() {
final AwsDiscoveryStrategyFactory factory = new AwsDiscoveryStrategyFactory();
final Config config = createConfig("test-aws-config.xml");
validateConfig(config);
}

@Test
public void parseMissingCredentialsConfigPasses(){
final Config config = createConfig("missing-creds-aws-config.xml");
validateConfig(config);
}

private void validateConfig(final Config config){
final DiscoveryConfig discoveryConfig = config.getNetworkConfig().getJoin().getDiscoveryConfig();
final DiscoveryServiceSettings settings = new DiscoveryServiceSettings().setDiscoveryConfig(discoveryConfig);
final DefaultDiscoveryService service = new DefaultDiscoveryService(settings);
Expand Down Expand Up @@ -146,6 +155,7 @@ public void parseDiscoveryStrategyConfigPasses() {
assertEquals("5702", providerProperties.get("hz-port"));
}


private static DiscoveryStrategy createStrategy(Map<String, Comparable> props) {
final AwsDiscoveryStrategyFactory factory = new AwsDiscoveryStrategyFactory();
return factory.newDiscoveryStrategy(null, null, props);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public void testUnmarshalling() throws IOException {
awsConfig1.setAccessKey("some-access-key");
awsConfig1.setSecretKey("some-secret-key");

Map<String, String> result = CloudyUtility.unmarshalTheResponse(is, awsConfig);
Map<String, String> result = CloudyUtility.unmarshalTheResponse(is);
assertEquals(2, result.size());
}

Expand Down
46 changes: 46 additions & 0 deletions src/test/resources/missing-creds-aws-config.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
~ Copyright (c) 2008-2017, Hazelcast, Inc. All Rights Reserved.
~
~ Licensed under the Apache License, Version 2.0 (the "License");
~ you may not use this file except in compliance with the License.
~ You may obtain a copy of the License at
~
~ http://www.apache.org/licenses/LICENSE-2.0
~
~ Unless required by applicable law or agreed to in writing, software
~ distributed under the License is distributed on an "AS IS" BASIS,
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
~ See the License for the specific language governing permissions and
~ limitations under the License.
-->

<hazelcast xsi:schemaLocation="http://www.hazelcast.com/schema/config hazelcast-config-3.7.xsd"
xmlns="http://www.hazelcast.com/schema/config"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">

<properties>
<property name="hazelcast.discovery.enabled">true</property>
</properties>

<network>
<join>
<multicast enabled="false"/>
<tcp-ip enabled="false"/>
<aws enabled="false"/>
<discovery-strategies>
<discovery-strategy enabled="true" class="com.hazelcast.aws.AwsDiscoveryStrategy">
<properties>
<property name="region">test-region</property>
<property name="host-header">ec2.test-host-header</property>
<property name="security-group-name">test-security-group-name</property>
<property name="tag-key">test-tag-key</property>
<property name="tag-value">test-tag-value</property>
<property name="connection-timeout-seconds">10</property>
</properties>
</discovery-strategy>
</discovery-strategies>
</join>
</network>

</hazelcast>

0 comments on commit b04f49f

Please sign in to comment.