Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support for auth enabled Marathon #4

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

support for auth enabled Marathon #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
728b318
added support for discovery with Marathon Auth
mudit1403 Jan 3, 2022
b55adc2
fixed MarathonClient.getInstance typo
mudit1403 Jan 4, 2022
43c1f3f
revert testing pom changes
mudit1403 Jan 4, 2022
3f724e7
refactored getHazelcastInstance in test
mudit1403 Jan 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/main/java/com/marathon/hazelcast/servicediscovery/MarathonDiscoveryConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,8 @@ public interface MarathonDiscoveryConfiguration {

PropertyDefinition PORT_INDEX = new SimplePropertyDefinition("port-index", PropertyTypeConverter.STRING);

PropertyDefinition MARATHON_USERNAME = new SimplePropertyDefinition("marathon-username", true, PropertyTypeConverter.STRING);

PropertyDefinition MARATHON_PASSWORD = new SimplePropertyDefinition("marathon-password", true, PropertyTypeConverter.STRING);

}
9 changes: 8 additions & 1 deletion src/main/java/com/marathon/hazelcast/servicediscovery/MarathonDiscoveryStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,23 @@ public class MarathonDiscoveryStrategy extends AbstractDiscoveryStrategy {

private int port;

private String marathonUsername;

private String marathonPassword;

private ILogger logger;

public MarathonDiscoveryStrategy(final ILogger logger, Map<String, Comparable> properties) {
super(logger, properties);
this.marathonEndpoint = getOrNull("discovery.marathon", MarathonDiscoveryConfiguration.MARATHON_ENDPOINT);
this.appId = getOrNull("discovery.marathon", MarathonDiscoveryConfiguration.APP_ID);
this.port = Integer.parseInt(getOrNull("discovery.marathon", MarathonDiscoveryConfiguration.PORT_INDEX));
this.marathonUsername = getOrNull("discovery.marathon", MarathonDiscoveryConfiguration.MARATHON_USERNAME);
this.marathonPassword = getOrNull("discovery.marathon", MarathonDiscoveryConfiguration.MARATHON_PASSWORD);
this.logger = logger;
try {
MarathonServiceDiscoveryHelper.start(marathonEndpoint, appId, port, logger);
MarathonServiceDiscoveryHelper.start(marathonEndpoint, appId, port, marathonUsername, marathonPassword,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this work if someone has upgraded the hazelcast marathon client but the marathon cluster isn't still not auth enabled?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, if marathon username or password is null or empty we do not make auth based call inside the start method

logger);
} catch (Exception e) {
logger.severe("Failed to start service discovery!", e);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it ok to just log the exception? What is the client expectation?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Service startup should not be impacted if marathon discovery fails so I guess that's why we are not rethrowing the Exception here.

}
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/com/marathon/hazelcast/servicediscovery/MarathonDiscoveryStrategyFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ public MarathonDiscoveryStrategyFactory() {
properties.add(MarathonDiscoveryConfiguration.APP_ID);
properties.add(MarathonDiscoveryConfiguration.PORT_INDEX);
properties.add(MarathonDiscoveryConfiguration.MARATHON_ENDPOINT);
properties.add(MarathonDiscoveryConfiguration.MARATHON_USERNAME);
properties.add(MarathonDiscoveryConfiguration.MARATHON_PASSWORD);
}

@Override
Expand Down
10 changes: 8 additions & 2 deletions src/main/java/com/marathon/hazelcast/servicediscovery/MarathonServiceDiscoveryHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,14 @@ public int getPort() {
}
}

public static void start(final String marathonEndpoint, final String appId, final int portIndex, final ILogger logger) throws Exception {
marathon = MarathonClient.getInstance(marathonEndpoint);
public static void start(final String marathonEndpoint, final String appId, final int portIndex,
final String marathonUsername, final String marathonPassword, final ILogger logger) throws Exception {
if (marathonUsername == null || marathonUsername.isEmpty() ||
marathonPassword == null || marathonPassword.isEmpty()) {
marathon = MarathonClient.getInstance(marathonEndpoint);
} else {
marathon = MarathonClient.getInstanceWithBasicAuth(marathonEndpoint, marathonUsername, marathonPassword);
}
log = logger;
MarathonPoller marathonPoller = new MarathonPoller(marathon, appId, portIndex);
scheduledExecutorService.scheduleAtFixedRate(marathonPoller, 0, 10, TimeUnit.SECONDS);
Expand Down
25 changes: 20 additions & 5 deletions src/test/java/com/marathon/hazelcast/servicediscovery/DiscoveryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public void testSingleMemberDiscovery() throws IOException, InterruptedException
.withStatus(200)
.withHeader("Content-Type", "application/json")
.withBody(mapper.writeValueAsBytes(response))));
HazelcastInstance hazelcast = getHazelcastInstance(5701);
HazelcastInstance hazelcast = getHazelcastInstanceWithoutBasicAuth(5701);
assertTrue(hazelcast.getCluster().getMembers().size() > 0);
hazelcast.shutdown();
}
Expand All @@ -84,17 +84,28 @@ public void testMultiMemberDiscovery() throws UnknownHostException, InterruptedE
.withHeader("Content-Type", "application/json")
.withBody(mapper.writeValueAsBytes(response))));

HazelcastInstance hazelcast1 = getHazelcastInstance(5701);
HazelcastInstance hazelcast2 = getHazelcastInstance(5702);
HazelcastInstance hazelcast3 = getHazelcastInstance(5703);
HazelcastInstance hazelcast1 = getHazelcastInstanceWithoutBasicAuth(5701);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's have test cases for both with and without auth

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, if you see here 87 and 88 line one's are Without Auth instances and 89th line one is With Auth. And in 91st line asserting that cluster members should be 3

HazelcastInstance hazelcast2 = getHazelcastInstanceWithoutBasicAuth(5702);
HazelcastInstance hazelcast3 = getHazelcastInstanceWithBasicAuth(5703);
assertTrue(hazelcast3.getCluster().getMembers().size() > 0);
assertTrue(hazelcast3.getCluster().getMembers().size() == 3);
hazelcast1.shutdown();
hazelcast2.shutdown();
hazelcast3.shutdown();
}

private HazelcastInstance getHazelcastInstance(int port) throws UnknownHostException, InterruptedException {
private HazelcastInstance getHazelcastInstanceWithoutBasicAuth(int port)
throws UnknownHostException, InterruptedException {
return getHazelcastInstance(port, false);
}

private HazelcastInstance getHazelcastInstanceWithBasicAuth(int port)
throws UnknownHostException, InterruptedException {
return getHazelcastInstance(port, true);
}

private HazelcastInstance getHazelcastInstance(int port, boolean withBasicAuthMarathon)
throws UnknownHostException, InterruptedException {
Config config = new Config();
config.setProperty("hazelcast.discovery.enabled", "true");
config.setProperty("hazelcast.discovery.public.ip.enabled", "true");
Expand All @@ -112,6 +123,10 @@ private HazelcastInstance getHazelcastInstance(int port) throws UnknownHostExcep
discoveryStrategyConfig.addProperty("marathon-endpoint", "http://localhost:8080");
discoveryStrategyConfig.addProperty("app-id", "test_app");
discoveryStrategyConfig.addProperty("port-index", "0");
if (withBasicAuthMarathon) {
discoveryStrategyConfig.addProperty("marathon-username", "username");
discoveryStrategyConfig.addProperty("marathon-password", "password");
}
discoveryConfig.addDiscoveryStrategyConfig(discoveryStrategyConfig);
Thread.sleep(2000);
return Hazelcast.newHazelcastInstance(config);
Expand Down