You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "smiklosovic (via GitHub)" <gi...@apache.org> on 2023/06/09 08:03:31 UTC
[GitHub] [cassandra] smiklosovic opened a new pull request, #2403: CASSANDRA-16555 trunk heavy wip
smiklosovic opened a new pull request, #2403:
URL: https://github.com/apache/cassandra/pull/2403
Thanks for sending a pull request! Here are some tips if you're new here:
* Ensure you have added or run the [appropriate tests](https://cassandra.apache.org/_/development/testing.html) for your PR.
* Be sure to keep the PR description updated to reflect all changes.
* Write your PR title to summarize what this PR proposes.
* If possible, provide a concise example to reproduce the issue for a faster review.
* Read our [contributor guidelines](https://cassandra.apache.org/_/development/index.html)
* If you're making a documentation change, see our [guide to documentation contribution](https://cassandra.apache.org/_/development/documentation.html)
Commit messages should follow the following format:
```
<One sentence description, usually Jira title or CHANGES.txt summary>
<Optional lengthier description (context on patch)>
patch by <Authors>; reviewed by <Reviewers> for CASSANDRA-#####
Co-authored-by: Name1 <email1>
Co-authored-by: Name2 <email2>
```
The [Cassandra Jira](https://issues.apache.org/jira/projects/CASSANDRA/issues/)
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] smiklosovic commented on a diff in pull request #2403: CASSANDRA-16555 trunk heavy wip
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #2403:
URL: https://github.com/apache/cassandra/pull/2403#discussion_r1223987363
##########
src/java/org/apache/cassandra/locator/Ec2SnitchIMDSv2.java:
##########
@@ -17,53 +17,90 @@
*/
package org.apache.cassandra.locator;
+
import java.io.DataInputStream;
import java.io.FilterInputStream;
import java.io.IOException;
import java.net.HttpURLConnection;
import java.net.URL;
import java.nio.charset.StandardCharsets;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
-import org.apache.cassandra.exceptions.ConfigurationException;
-import org.apache.cassandra.io.util.FileUtils;
-import org.apache.cassandra.utils.Clock.Global;
+import com.google.common.base.Supplier;
+import com.google.common.base.Suppliers;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import org.apache.cassandra.io.util.FileUtils;
+
/**
- * An implementation of the Ec2Snitch which uses Instance Meta Data Service v2 (IMDSv2) which requires you
+ * An implementation of the Ec2Snitch which uses Instance Metadata Service v2 (IMDSv2) which requires you
* to get a session token first before calling the metaData service
*/
public class Ec2SnitchIMDSv2 extends Ec2Snitch
{
- protected static final Logger logger = LoggerFactory.getLogger(Ec2SnitchIMDSv2.class);
- private static final long REFRESH_TOKEN_TIME = 21600;
- private static final String AWS_EC2_METADATA_HEADER_TTL = "X-aws-ec2-metadata-token-ttl-seconds";
- private static final String TOKEN_TTL_SECONDS = String.valueOf(REFRESH_TOKEN_TIME);
- private static final String AWS_EC2_METADATA_HEADER = "X-aws-ec2-metadata-token";
+ private static final Logger logger = LoggerFactory.getLogger(Ec2SnitchIMDSv2.class);
+ private static final int MAX_TOKEN_TIME_IN_SECONDS = 21600;
+ private static final String AWS_EC2_METADATA_TOKEN_HEADER = "X-aws-ec2-metadata-token";
+ private static final String AWS_EC2_METADATA_TOKEN_TTL_SECONDS_HEADER = "X-aws-ec2-metadata-token-ttl-seconds";
private static final String TOKEN_ENDPOINT = "http://169.254.169.254/latest/api/token";
- private String myToken;
- private Long myLastTokenTime;
-
+ private Supplier<ApiCallResult> tokenSupplier;
+ private long tokenTTL;
- public Ec2SnitchIMDSv2() throws IOException, ConfigurationException
+ public Ec2SnitchIMDSv2() throws IOException
{
- super();
+ super(new SnitchProperties());
}
- @Override
- String awsApiCall(final String url) throws IOException, ConfigurationException
+ public Ec2SnitchIMDSv2(SnitchProperties snitchProperties) throws IOException
{
- // Populate the region and zone by introspection, fail if 404 on metadata
- if (myToken == null || myLastTokenTime == null
- || Global.currentTimeMillis() - myLastTokenTime > (REFRESH_TOKEN_TIME - 100))
+ super(snitchProperties);
+
+ String parsedTokenTTL = snitchProperties.get(AWS_EC2_METADATA_TOKEN_TTL_SECONDS_HEADER, Integer.toString(MAX_TOKEN_TIME_IN_SECONDS));
+
+ try
{
- getAndSetNewToken();
+ tokenTTL = Integer.parseInt(parsedTokenTTL);
+
+ if (tokenTTL > MAX_TOKEN_TIME_IN_SECONDS || tokenTTL < 1)
+ {
+ logger.info(String.format("property %s was set to %s which is more than maximum allowed range of (0, 21600]: %s, defaulting to %s",
+ AWS_EC2_METADATA_TOKEN_HEADER, tokenTTL, MAX_TOKEN_TIME_IN_SECONDS, MAX_TOKEN_TIME_IN_SECONDS));
+ tokenTTL = MAX_TOKEN_TIME_IN_SECONDS;
+ }
}
- final HttpURLConnection conn = (HttpURLConnection) new URL(url).openConnection();
- conn.setRequestProperty(AWS_EC2_METADATA_HEADER, myToken);
- return getContent(conn);
+ catch (NumberFormatException ex)
+ {
+ logger.error(String.format("Unable to parse integer from %s, value to parse: %s. Defaulting to %s",
+ AWS_EC2_METADATA_TOKEN_HEADER, parsedTokenTTL, MAX_TOKEN_TIME_IN_SECONDS));
+ tokenTTL = MAX_TOKEN_TIME_IN_SECONDS;
+ }
+
+ tokenSupplier = Suppliers.memoizeWithExpiration(this::getNewToken, Math.max(5, tokenTTL - 100), TimeUnit.SECONDS);
Review Comment:
5 is probably wrong, what I try to do here is that when we get a token, by the time we memorize it, some time already passed, no? So memorizing it with same timeout as TTL for that token does not make sense because it might happen that it would return invalid token. Being on the safe side of "tokenTTL - 100s" seems like a good idea (it was in the original PR too). I am not sure about the minimum there though ... what if TTL is ... 3? It would then hold invalid token for 2 seconds? (5-3) ... not sure what the logic should be here.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] smiklosovic commented on a diff in pull request #2403: CASSANDRA-16555 trunk
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #2403:
URL: https://github.com/apache/cassandra/pull/2403#discussion_r1243291317
##########
src/java/org/apache/cassandra/locator/AbstractCloudMetadataServiceConnector.java:
##########
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+
+package org.apache.cassandra.locator;
+
+import java.io.DataInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.HttpURLConnection;
+import java.net.URL;
+import java.util.Map;
+
+import com.google.common.collect.ImmutableMap;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+abstract class AbstractCloudMetadataServiceConnector
+{
+ protected String metadataServiceUrl;
Review Comment:
@jacek-lewandowski I would have to create a constructor in this abstract class to set it there because it complains that it might be uninitialized.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] smiklosovic commented on a diff in pull request #2403: CASSANDRA-16555 trunk
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #2403:
URL: https://github.com/apache/cassandra/pull/2403#discussion_r1243292929
##########
src/java/org/apache/cassandra/locator/AbstractCloudMetadataServiceConnector.java:
##########
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+
+package org.apache.cassandra.locator;
+
+import java.io.DataInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.HttpURLConnection;
+import java.net.URL;
+import java.util.Map;
+
+import com.google.common.collect.ImmutableMap;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+abstract class AbstractCloudMetadataServiceConnector
+{
+ protected String metadataServiceUrl;
Review Comment:
I did it like I just said above.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] jacek-lewandowski commented on a diff in pull request #2403: CASSANDRA-16555 trunk
Posted by "jacek-lewandowski (via GitHub)" <gi...@apache.org>.
jacek-lewandowski commented on code in PR #2403:
URL: https://github.com/apache/cassandra/pull/2403#discussion_r1243134753
##########
src/java/org/apache/cassandra/locator/AbstractCloudMetadataServiceConnector.java:
##########
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+
+package org.apache.cassandra.locator;
+
+import java.io.DataInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.HttpURLConnection;
+import java.net.URL;
+import java.util.Map;
+
+import com.google.common.collect.ImmutableMap;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+abstract class AbstractCloudMetadataServiceConnector
+{
+ protected String metadataServiceUrl;
Review Comment:
Can it be final?
##########
src/java/org/apache/cassandra/locator/Ec2MetadataServiceConnector.java:
##########
@@ -0,0 +1,218 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+
+package org.apache.cassandra.locator;
+
+import java.io.IOException;
+import java.time.Duration;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+import java.util.function.Function;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableMap;
+
+import org.apache.cassandra.exceptions.ConfigurationException;
+import org.apache.cassandra.utils.Clock;
+import org.apache.cassandra.utils.Pair;
+
+import static java.lang.String.format;
+import static java.util.Arrays.stream;
+import static java.util.stream.Collectors.joining;
+import static org.apache.cassandra.locator.Ec2MetadataServiceConnector.EC2MetadataType.v2;
+
+abstract class Ec2MetadataServiceConnector extends AbstractCloudMetadataServiceConnector
+{
+ static final String EC2_METADATA_TYPE_PROPERTY = "ec2_metadata_type";
+ static final String EC2_METADATA_URL_PROPERTY = "ec2_metadata_url";
+ static final String DEFAULT_EC2_METADATA_URL = "http://169.254.169.254";
+
+ Ec2MetadataServiceConnector(SnitchProperties properties)
+ {
+ metadataServiceUrl = properties.get(EC2_METADATA_URL_PROPERTY, DEFAULT_EC2_METADATA_URL);
+ }
+
+ enum EC2MetadataType
+ {
+ v1(props -> Ec2MetadataServiceConnector.V1Connector.create(props)),
+ v2(props -> Ec2MetadataServiceConnector.V2Connector.create(props));
+
+ private final Function<SnitchProperties, Ec2MetadataServiceConnector> creator;
+
+ EC2MetadataType(Function<SnitchProperties, Ec2MetadataServiceConnector> creator)
+ {
+ this.creator = creator;
+ }
+
+ Ec2MetadataServiceConnector create(SnitchProperties properties)
+ {
+ return creator.apply(properties);
+ }
+ }
+
+ static Ec2MetadataServiceConnector create(SnitchProperties props)
+ {
+ try
+ {
+ return EC2MetadataType.valueOf(props.get(EC2_METADATA_TYPE_PROPERTY, v2.name())).create(props);
+ }
+ catch (IllegalArgumentException ex)
+ {
+ throw new ConfigurationException(format("%s must be one of %s", EC2_METADATA_TYPE_PROPERTY,
+ stream(EC2MetadataType.values()).map(Enum::name).collect(joining(", "))));
+ }
+ }
+
+ static class V1Connector extends Ec2MetadataServiceConnector
+ {
+ static V1Connector create(SnitchProperties props)
+ {
+ return new V1Connector(props);
+ }
+
+ V1Connector(SnitchProperties props)
+ {
+ super(props);
+ }
+
+ @Override
+ public String toString()
+ {
+ return V1Connector.class.getName() + '{' + EC2_METADATA_URL_PROPERTY + '=' + metadataServiceUrl + '}';
Review Comment:
nit: maybe use `String.format`
##########
src/java/org/apache/cassandra/locator/Ec2MetadataServiceConnector.java:
##########
@@ -0,0 +1,218 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+
+package org.apache.cassandra.locator;
+
+import java.io.IOException;
+import java.time.Duration;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+import java.util.function.Function;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableMap;
+
+import org.apache.cassandra.exceptions.ConfigurationException;
+import org.apache.cassandra.utils.Clock;
+import org.apache.cassandra.utils.Pair;
+
+import static java.lang.String.format;
+import static java.util.Arrays.stream;
+import static java.util.stream.Collectors.joining;
+import static org.apache.cassandra.locator.Ec2MetadataServiceConnector.EC2MetadataType.v2;
+
+abstract class Ec2MetadataServiceConnector extends AbstractCloudMetadataServiceConnector
+{
+ static final String EC2_METADATA_TYPE_PROPERTY = "ec2_metadata_type";
+ static final String EC2_METADATA_URL_PROPERTY = "ec2_metadata_url";
+ static final String DEFAULT_EC2_METADATA_URL = "http://169.254.169.254";
+
+ Ec2MetadataServiceConnector(SnitchProperties properties)
+ {
+ metadataServiceUrl = properties.get(EC2_METADATA_URL_PROPERTY, DEFAULT_EC2_METADATA_URL);
+ }
+
+ enum EC2MetadataType
+ {
+ v1(props -> Ec2MetadataServiceConnector.V1Connector.create(props)),
+ v2(props -> Ec2MetadataServiceConnector.V2Connector.create(props));
+
+ private final Function<SnitchProperties, Ec2MetadataServiceConnector> creator;
+
+ EC2MetadataType(Function<SnitchProperties, Ec2MetadataServiceConnector> creator)
+ {
+ this.creator = creator;
+ }
+
+ Ec2MetadataServiceConnector create(SnitchProperties properties)
+ {
+ return creator.apply(properties);
+ }
+ }
+
+ static Ec2MetadataServiceConnector create(SnitchProperties props)
+ {
+ try
+ {
+ return EC2MetadataType.valueOf(props.get(EC2_METADATA_TYPE_PROPERTY, v2.name())).create(props);
+ }
+ catch (IllegalArgumentException ex)
+ {
+ throw new ConfigurationException(format("%s must be one of %s", EC2_METADATA_TYPE_PROPERTY,
+ stream(EC2MetadataType.values()).map(Enum::name).collect(joining(", "))));
+ }
+ }
+
+ static class V1Connector extends Ec2MetadataServiceConnector
+ {
+ static V1Connector create(SnitchProperties props)
+ {
+ return new V1Connector(props);
+ }
+
+ V1Connector(SnitchProperties props)
+ {
+ super(props);
+ }
+
+ @Override
+ public String toString()
+ {
+ return V1Connector.class.getName() + '{' + EC2_METADATA_URL_PROPERTY + '=' + metadataServiceUrl + '}';
+ }
+ }
+
+ static class V2Connector extends Ec2MetadataServiceConnector
+ {
+ @VisibleForTesting
Review Comment:
nit: perhaps we do not need to annotate static final fields with `@VisibleForTesting`
##########
test/unit/org/apache/cassandra/locator/Ec2ConnectorTest.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+
+package org.apache.cassandra.locator;
+
+import java.time.Duration;
+import java.util.Properties;
+
+import org.junit.Test;
+
+import org.apache.cassandra.exceptions.ConfigurationException;
+import org.apache.cassandra.locator.Ec2MetadataServiceConnector.EC2MetadataType;
+import org.apache.cassandra.locator.Ec2MetadataServiceConnector.V1Connector;
+import org.apache.cassandra.locator.Ec2MetadataServiceConnector.V2Connector;
+
+import static java.lang.String.format;
+import static java.util.Arrays.stream;
+import static java.util.stream.Collectors.joining;
+import static org.apache.cassandra.locator.Ec2MetadataServiceConnector.DEFAULT_EC2_METADATA_URL;
+import static org.apache.cassandra.locator.Ec2MetadataServiceConnector.EC2_METADATA_TYPE_PROPERTY;
+import static org.apache.cassandra.locator.Ec2MetadataServiceConnector.V2Connector.AWS_EC2_METADATA_TOKEN_TTL_SECONDS_HEADER_PROPERTY;
+import static org.apache.cassandra.locator.Ec2MetadataServiceConnector.V2Connector.MAX_TOKEN_TIME_IN_SECONDS;
+import static org.apache.cassandra.locator.Ec2MetadataServiceConnector.V2Connector.MIN_TOKEN_TIME_IN_SECONDS;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+public class Ec2ConnectorTest
+{
+ @Test
+ public void testV1Configuration()
+ {
+ Properties p = new Properties();
+ p.setProperty(EC2_METADATA_TYPE_PROPERTY, EC2MetadataType.v1.name());
+ Ec2MetadataServiceConnector ec2Connector = Ec2MetadataServiceConnector.create(new SnitchProperties(p));
+
+ assertTrue(ec2Connector instanceof V1Connector);
+
+ assertEquals(DEFAULT_EC2_METADATA_URL, ec2Connector.metadataServiceUrl);
+ }
+
+ @Test
+ public void testV2Configuration()
+ {
+ Ec2MetadataServiceConnector ec2Connector = Ec2MetadataServiceConnector.create(new SnitchProperties(new Properties()));
+
+ // v2 connector by default
+ assertTrue(ec2Connector instanceof V2Connector);
+ assertEquals(DEFAULT_EC2_METADATA_URL, ec2Connector.metadataServiceUrl);
+ assertEquals(Duration.ofSeconds(MAX_TOKEN_TIME_IN_SECONDS), ((V2Connector) ec2Connector).tokenTTL);
+ }
+
+ @Test
+ public void testInvalidConfiguration()
+ {
+ try
+ {
+ Properties p = new Properties();
+ p.setProperty(EC2_METADATA_TYPE_PROPERTY, "non-existent");
+
+ Ec2MetadataServiceConnector.create(new SnitchProperties(p));
+ fail("it should not be possible create a connector of type non-existent");
+ }
+ catch (ConfigurationException ex)
Review Comment:
Could you use AssertJ in tests? (see https://www.tabnine.com/code/java/methods/org.assertj.core.api.Assertions/assertThatExceptionOfType)
##########
test/unit/org/apache/cassandra/locator/Ec2V2ConnectorMockingTest.java:
##########
@@ -0,0 +1,174 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+
+package org.apache.cassandra.locator;
+
+import java.util.Properties;
+
+import org.junit.Rule;
+import org.junit.Test;
+
+import com.github.tomakehurst.wiremock.client.MappingBuilder;
+import com.github.tomakehurst.wiremock.junit.WireMockRule;
+import org.apache.cassandra.locator.Ec2MetadataServiceConnector.V2Connector;
+
+import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
+import static com.github.tomakehurst.wiremock.client.WireMock.equalTo;
+import static com.github.tomakehurst.wiremock.client.WireMock.get;
+import static com.github.tomakehurst.wiremock.client.WireMock.put;
+import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
+import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig;
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.apache.cassandra.locator.Ec2MetadataServiceConnector.EC2MetadataType.v2;
+import static org.apache.cassandra.locator.Ec2MetadataServiceConnector.EC2_METADATA_TYPE_PROPERTY;
+import static org.apache.cassandra.locator.Ec2MetadataServiceConnector.V1Connector.EC2_METADATA_URL_PROPERTY;
+import static org.apache.cassandra.locator.Ec2MetadataServiceConnector.V2Connector.AWS_EC2_METADATA_TOKEN_HEADER;
+import static org.apache.cassandra.locator.Ec2MetadataServiceConnector.V2Connector.AWS_EC2_METADATA_TOKEN_TTL_SECONDS_HEADER_PROPERTY;
+import static org.apache.cassandra.locator.Ec2MetadataServiceConnector.V2Connector.TOKEN_QUERY;
+import static org.apache.cassandra.locator.Ec2Snitch.ZONE_NAME_QUERY;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+
+public class Ec2V2ConnectorMockingTest
Review Comment:
maybe you merge these tests to Ec2ConnectorTest?
##########
src/java/org/apache/cassandra/locator/Ec2MetadataServiceConnector.java:
##########
@@ -0,0 +1,218 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+
+package org.apache.cassandra.locator;
+
+import java.io.IOException;
+import java.time.Duration;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+import java.util.function.Function;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableMap;
+
+import org.apache.cassandra.exceptions.ConfigurationException;
+import org.apache.cassandra.utils.Clock;
+import org.apache.cassandra.utils.Pair;
+
+import static java.lang.String.format;
+import static java.util.Arrays.stream;
+import static java.util.stream.Collectors.joining;
+import static org.apache.cassandra.locator.Ec2MetadataServiceConnector.EC2MetadataType.v2;
+
+abstract class Ec2MetadataServiceConnector extends AbstractCloudMetadataServiceConnector
+{
+ static final String EC2_METADATA_TYPE_PROPERTY = "ec2_metadata_type";
+ static final String EC2_METADATA_URL_PROPERTY = "ec2_metadata_url";
+ static final String DEFAULT_EC2_METADATA_URL = "http://169.254.169.254";
+
+ Ec2MetadataServiceConnector(SnitchProperties properties)
+ {
+ metadataServiceUrl = properties.get(EC2_METADATA_URL_PROPERTY, DEFAULT_EC2_METADATA_URL);
+ }
+
+ enum EC2MetadataType
+ {
+ v1(props -> Ec2MetadataServiceConnector.V1Connector.create(props)),
Review Comment:
Uppercase for enum names?
##########
test/unit/org/apache/cassandra/locator/Ec2SnitchTest.java:
##########
@@ -71,30 +124,60 @@ public static void setup() throws Exception
StorageService.instance.initServer(0);
}
- private class TestEC2Snitch extends Ec2Snitch
+ private static class TestEc2Snitch extends Ec2Snitch
{
- public TestEC2Snitch() throws IOException, ConfigurationException
+ public TestEc2Snitch() throws IOException, ConfigurationException
{
- super();
+ this(new SnitchProperties());
}
- public TestEC2Snitch(SnitchProperties props) throws IOException, ConfigurationException
+ public TestEc2Snitch(SnitchProperties props) throws IOException, ConfigurationException
Review Comment:
What do you think about refactoring this test a bit? We have separated a snitch from a connector so it would be enough to simply mock the connector and do not bother distinguishing between v1 and v2 at all. The whole test would be much simpler and easier to follow
##########
test/unit/org/apache/cassandra/locator/Ec2V2ConnectorMockingTest.java:
##########
@@ -0,0 +1,174 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+
+package org.apache.cassandra.locator;
+
+import java.util.Properties;
+
+import org.junit.Rule;
+import org.junit.Test;
+
+import com.github.tomakehurst.wiremock.client.MappingBuilder;
+import com.github.tomakehurst.wiremock.junit.WireMockRule;
+import org.apache.cassandra.locator.Ec2MetadataServiceConnector.V2Connector;
+
+import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
+import static com.github.tomakehurst.wiremock.client.WireMock.equalTo;
+import static com.github.tomakehurst.wiremock.client.WireMock.get;
+import static com.github.tomakehurst.wiremock.client.WireMock.put;
+import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
+import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig;
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.apache.cassandra.locator.Ec2MetadataServiceConnector.EC2MetadataType.v2;
+import static org.apache.cassandra.locator.Ec2MetadataServiceConnector.EC2_METADATA_TYPE_PROPERTY;
+import static org.apache.cassandra.locator.Ec2MetadataServiceConnector.V1Connector.EC2_METADATA_URL_PROPERTY;
+import static org.apache.cassandra.locator.Ec2MetadataServiceConnector.V2Connector.AWS_EC2_METADATA_TOKEN_HEADER;
+import static org.apache.cassandra.locator.Ec2MetadataServiceConnector.V2Connector.AWS_EC2_METADATA_TOKEN_TTL_SECONDS_HEADER_PROPERTY;
+import static org.apache.cassandra.locator.Ec2MetadataServiceConnector.V2Connector.TOKEN_QUERY;
+import static org.apache.cassandra.locator.Ec2Snitch.ZONE_NAME_QUERY;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+
+public class Ec2V2ConnectorMockingTest
+{
+ private static final String token = "thisismytoken";
+ private static final String az = "us-east-1a";
+
+ @Rule
+ public WireMockRule v2Service = new WireMockRule(wireMockConfig().bindAddress("127.0.0.1").port(8080));
+
+ @Test
+ public void testV2Connector() throws Throwable
+ {
+ v2Service.stubFor(tokenRequest(100, 200, token));
+ v2Service.stubFor(azRequest(az, 200, token));
+
+ assertEquals(az, getConnector(100).apiCall(ZONE_NAME_QUERY));
+ }
+
+ @Test
+ public void testV2ConnectorWhenUnauthorized() throws Throwable
+ {
+ String unauthorizedBody = "<?xml version=\"1.0\" encoding=\"iso-8859-1\"?>\n" +
+ "<!DOCTYPE html PUBLIC \"-//W3C//DTD XHTML 1.0 Transitional//EN\"\n" +
+ "\t\"http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd\">\n" +
+ "<html xmlns=\"http://www.w3.org/1999/xhtml\" xml:lang=\"en\" lang=\"en\">\n" +
+ " <head>\n" +
+ " <title>401 - Unauthorized</title>\n" +
+ " </head>\n" +
+ " <body>\n" +
+ " <h1>401 - Unauthorized</h1>\n" +
+ " </body>\n" +
+ "</html>\n";
+
+ v2Service.stubFor(tokenRequest(100, 200, token));
+
+ v2Service.stubFor(get(urlEqualTo(ZONE_NAME_QUERY)).withHeader(AWS_EC2_METADATA_TOKEN_HEADER, equalTo(token))
+ .willReturn(aResponse().withStatus(401)
+ .withStatusMessage("Unauthorized")
+ .withHeader("Content-Type", "text/html")
+ .withHeader("Content-Length", String.valueOf(unauthorizedBody.getBytes(UTF_8).length))));
+
+ V2Connector.HTTP_REQUEST_RETRIES = 0;
+ Ec2MetadataServiceConnector v2Connector = getConnector(100);
+
+ try
+ {
+ v2Connector.apiCall(ZONE_NAME_QUERY);
Review Comment:
nit: maybe AssertJ?
##########
src/java/org/apache/cassandra/locator/Ec2MetadataServiceConnector.java:
##########
@@ -0,0 +1,218 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+
+package org.apache.cassandra.locator;
+
+import java.io.IOException;
+import java.time.Duration;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+import java.util.function.Function;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableMap;
+
+import org.apache.cassandra.exceptions.ConfigurationException;
+import org.apache.cassandra.utils.Clock;
+import org.apache.cassandra.utils.Pair;
+
+import static java.lang.String.format;
+import static java.util.Arrays.stream;
+import static java.util.stream.Collectors.joining;
+import static org.apache.cassandra.locator.Ec2MetadataServiceConnector.EC2MetadataType.v2;
+
+abstract class Ec2MetadataServiceConnector extends AbstractCloudMetadataServiceConnector
+{
+ static final String EC2_METADATA_TYPE_PROPERTY = "ec2_metadata_type";
+ static final String EC2_METADATA_URL_PROPERTY = "ec2_metadata_url";
+ static final String DEFAULT_EC2_METADATA_URL = "http://169.254.169.254";
+
+ Ec2MetadataServiceConnector(SnitchProperties properties)
+ {
+ metadataServiceUrl = properties.get(EC2_METADATA_URL_PROPERTY, DEFAULT_EC2_METADATA_URL);
+ }
+
+ enum EC2MetadataType
+ {
+ v1(props -> Ec2MetadataServiceConnector.V1Connector.create(props)),
+ v2(props -> Ec2MetadataServiceConnector.V2Connector.create(props));
+
+ private final Function<SnitchProperties, Ec2MetadataServiceConnector> creator;
Review Comment:
maybe name it "provider" ?
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] smiklosovic commented on a diff in pull request #2403: CASSANDRA-16555 trunk heavy wip
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #2403:
URL: https://github.com/apache/cassandra/pull/2403#discussion_r1236929145
##########
src/java/org/apache/cassandra/locator/Ec2MetadataServiceConnector.java:
##########
@@ -0,0 +1,234 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+
+package org.apache.cassandra.locator;
+
+import java.io.IOException;
+import java.time.Duration;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+import java.util.stream.Collectors;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableMap;
+
+import org.apache.cassandra.exceptions.ConfigurationException;
+import org.apache.cassandra.utils.Clock;
+import org.apache.cassandra.utils.Pair;
+
+import static java.lang.String.format;
+
+public interface Ec2MetadataServiceConnector extends CloudApiCallable
+{
+ String EC2_METADATA_TYPE_PROPERTY = "ec2_metadata_type";
+ String DEFAULT_METADATA_SERVICE_URL = "http://169.254.169.254";
+
+ enum EC2MetadataType
+ {
+ V1("v1"),
+ V2("v2");
+
+ private final String name;
+
+ EC2MetadataType(String name)
+ {
+ this.name = name;
+ }
+
+ @Override
+ public String toString()
+ {
+ return name;
+ }
+ }
+
+ static Ec2MetadataServiceConnector create(SnitchProperties props)
+ {
+ try
+ {
+ EC2MetadataType type = EC2MetadataType.valueOf(props.get(EC2_METADATA_TYPE_PROPERTY, EC2MetadataType.V2.toString()).toUpperCase());
+
+ switch (type)
+ {
+ case V1:
+ return V1Connector.create(props);
Review Comment:
great idea.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] smiklosovic commented on a diff in pull request #2403: CASSANDRA-16555 trunk
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #2403:
URL: https://github.com/apache/cassandra/pull/2403#discussion_r1243317087
##########
test/unit/org/apache/cassandra/locator/Ec2ConnectorTest.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+
+package org.apache.cassandra.locator;
+
+import java.time.Duration;
+import java.util.Properties;
+
+import org.junit.Test;
+
+import org.apache.cassandra.exceptions.ConfigurationException;
+import org.apache.cassandra.locator.Ec2MetadataServiceConnector.EC2MetadataType;
+import org.apache.cassandra.locator.Ec2MetadataServiceConnector.V1Connector;
+import org.apache.cassandra.locator.Ec2MetadataServiceConnector.V2Connector;
+
+import static java.lang.String.format;
+import static java.util.Arrays.stream;
+import static java.util.stream.Collectors.joining;
+import static org.apache.cassandra.locator.Ec2MetadataServiceConnector.DEFAULT_EC2_METADATA_URL;
+import static org.apache.cassandra.locator.Ec2MetadataServiceConnector.EC2_METADATA_TYPE_PROPERTY;
+import static org.apache.cassandra.locator.Ec2MetadataServiceConnector.V2Connector.AWS_EC2_METADATA_TOKEN_TTL_SECONDS_HEADER_PROPERTY;
+import static org.apache.cassandra.locator.Ec2MetadataServiceConnector.V2Connector.MAX_TOKEN_TIME_IN_SECONDS;
+import static org.apache.cassandra.locator.Ec2MetadataServiceConnector.V2Connector.MIN_TOKEN_TIME_IN_SECONDS;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+public class Ec2ConnectorTest
+{
+ @Test
+ public void testV1Configuration()
+ {
+ Properties p = new Properties();
+ p.setProperty(EC2_METADATA_TYPE_PROPERTY, EC2MetadataType.v1.name());
+ Ec2MetadataServiceConnector ec2Connector = Ec2MetadataServiceConnector.create(new SnitchProperties(p));
+
+ assertTrue(ec2Connector instanceof V1Connector);
+
+ assertEquals(DEFAULT_EC2_METADATA_URL, ec2Connector.metadataServiceUrl);
+ }
+
+ @Test
+ public void testV2Configuration()
+ {
+ Ec2MetadataServiceConnector ec2Connector = Ec2MetadataServiceConnector.create(new SnitchProperties(new Properties()));
+
+ // v2 connector by default
+ assertTrue(ec2Connector instanceof V2Connector);
+ assertEquals(DEFAULT_EC2_METADATA_URL, ec2Connector.metadataServiceUrl);
+ assertEquals(Duration.ofSeconds(MAX_TOKEN_TIME_IN_SECONDS), ((V2Connector) ec2Connector).tokenTTL);
+ }
+
+ @Test
+ public void testInvalidConfiguration()
+ {
+ try
+ {
+ Properties p = new Properties();
+ p.setProperty(EC2_METADATA_TYPE_PROPERTY, "non-existent");
+
+ Ec2MetadataServiceConnector.create(new SnitchProperties(p));
+ fail("it should not be possible create a connector of type non-existent");
+ }
+ catch (ConfigurationException ex)
Review Comment:
rewritten
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] smiklosovic commented on a diff in pull request #2403: CASSANDRA-16555 trunk
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #2403:
URL: https://github.com/apache/cassandra/pull/2403#discussion_r1243300351
##########
src/java/org/apache/cassandra/locator/Ec2MetadataServiceConnector.java:
##########
@@ -0,0 +1,218 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+
+package org.apache.cassandra.locator;
+
+import java.io.IOException;
+import java.time.Duration;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+import java.util.function.Function;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableMap;
+
+import org.apache.cassandra.exceptions.ConfigurationException;
+import org.apache.cassandra.utils.Clock;
+import org.apache.cassandra.utils.Pair;
+
+import static java.lang.String.format;
+import static java.util.Arrays.stream;
+import static java.util.stream.Collectors.joining;
+import static org.apache.cassandra.locator.Ec2MetadataServiceConnector.EC2MetadataType.v2;
+
+abstract class Ec2MetadataServiceConnector extends AbstractCloudMetadataServiceConnector
+{
+ static final String EC2_METADATA_TYPE_PROPERTY = "ec2_metadata_type";
+ static final String EC2_METADATA_URL_PROPERTY = "ec2_metadata_url";
+ static final String DEFAULT_EC2_METADATA_URL = "http://169.254.169.254";
+
+ Ec2MetadataServiceConnector(SnitchProperties properties)
+ {
+ metadataServiceUrl = properties.get(EC2_METADATA_URL_PROPERTY, DEFAULT_EC2_METADATA_URL);
+ }
+
+ enum EC2MetadataType
+ {
+ v1(props -> Ec2MetadataServiceConnector.V1Connector.create(props)),
+ v2(props -> Ec2MetadataServiceConnector.V2Connector.create(props));
+
+ private final Function<SnitchProperties, Ec2MetadataServiceConnector> creator;
+
+ EC2MetadataType(Function<SnitchProperties, Ec2MetadataServiceConnector> creator)
+ {
+ this.creator = creator;
+ }
+
+ Ec2MetadataServiceConnector create(SnitchProperties properties)
+ {
+ return creator.apply(properties);
+ }
+ }
+
+ static Ec2MetadataServiceConnector create(SnitchProperties props)
+ {
+ try
+ {
+ return EC2MetadataType.valueOf(props.get(EC2_METADATA_TYPE_PROPERTY, v2.name())).create(props);
+ }
+ catch (IllegalArgumentException ex)
+ {
+ throw new ConfigurationException(format("%s must be one of %s", EC2_METADATA_TYPE_PROPERTY,
+ stream(EC2MetadataType.values()).map(Enum::name).collect(joining(", "))));
+ }
+ }
+
+ static class V1Connector extends Ec2MetadataServiceConnector
+ {
+ static V1Connector create(SnitchProperties props)
+ {
+ return new V1Connector(props);
+ }
+
+ V1Connector(SnitchProperties props)
+ {
+ super(props);
+ }
+
+ @Override
+ public String toString()
+ {
+ return V1Connector.class.getName() + '{' + EC2_METADATA_URL_PROPERTY + '=' + metadataServiceUrl + '}';
+ }
+ }
+
+ static class V2Connector extends Ec2MetadataServiceConnector
+ {
+ @VisibleForTesting
Review Comment:
I will gladly get rid of them.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] smiklosovic commented on a diff in pull request #2403: CASSANDRA-16555 trunk
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #2403:
URL: https://github.com/apache/cassandra/pull/2403#discussion_r1243332451
##########
test/unit/org/apache/cassandra/locator/Ec2V2ConnectorMockingTest.java:
##########
@@ -0,0 +1,174 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+
+package org.apache.cassandra.locator;
+
+import java.util.Properties;
+
+import org.junit.Rule;
+import org.junit.Test;
+
+import com.github.tomakehurst.wiremock.client.MappingBuilder;
+import com.github.tomakehurst.wiremock.junit.WireMockRule;
+import org.apache.cassandra.locator.Ec2MetadataServiceConnector.V2Connector;
+
+import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
+import static com.github.tomakehurst.wiremock.client.WireMock.equalTo;
+import static com.github.tomakehurst.wiremock.client.WireMock.get;
+import static com.github.tomakehurst.wiremock.client.WireMock.put;
+import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
+import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig;
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.apache.cassandra.locator.Ec2MetadataServiceConnector.EC2MetadataType.v2;
+import static org.apache.cassandra.locator.Ec2MetadataServiceConnector.EC2_METADATA_TYPE_PROPERTY;
+import static org.apache.cassandra.locator.Ec2MetadataServiceConnector.V1Connector.EC2_METADATA_URL_PROPERTY;
+import static org.apache.cassandra.locator.Ec2MetadataServiceConnector.V2Connector.AWS_EC2_METADATA_TOKEN_HEADER;
+import static org.apache.cassandra.locator.Ec2MetadataServiceConnector.V2Connector.AWS_EC2_METADATA_TOKEN_TTL_SECONDS_HEADER_PROPERTY;
+import static org.apache.cassandra.locator.Ec2MetadataServiceConnector.V2Connector.TOKEN_QUERY;
+import static org.apache.cassandra.locator.Ec2Snitch.ZONE_NAME_QUERY;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+
+public class Ec2V2ConnectorMockingTest
+{
+ private static final String token = "thisismytoken";
+ private static final String az = "us-east-1a";
+
+ @Rule
+ public WireMockRule v2Service = new WireMockRule(wireMockConfig().bindAddress("127.0.0.1").port(8080));
+
+ @Test
+ public void testV2Connector() throws Throwable
+ {
+ v2Service.stubFor(tokenRequest(100, 200, token));
+ v2Service.stubFor(azRequest(az, 200, token));
+
+ assertEquals(az, getConnector(100).apiCall(ZONE_NAME_QUERY));
+ }
+
+ @Test
+ public void testV2ConnectorWhenUnauthorized() throws Throwable
+ {
+ String unauthorizedBody = "<?xml version=\"1.0\" encoding=\"iso-8859-1\"?>\n" +
+ "<!DOCTYPE html PUBLIC \"-//W3C//DTD XHTML 1.0 Transitional//EN\"\n" +
+ "\t\"http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd\">\n" +
+ "<html xmlns=\"http://www.w3.org/1999/xhtml\" xml:lang=\"en\" lang=\"en\">\n" +
+ " <head>\n" +
+ " <title>401 - Unauthorized</title>\n" +
+ " </head>\n" +
+ " <body>\n" +
+ " <h1>401 - Unauthorized</h1>\n" +
+ " </body>\n" +
+ "</html>\n";
+
+ v2Service.stubFor(tokenRequest(100, 200, token));
+
+ v2Service.stubFor(get(urlEqualTo(ZONE_NAME_QUERY)).withHeader(AWS_EC2_METADATA_TOKEN_HEADER, equalTo(token))
+ .willReturn(aResponse().withStatus(401)
+ .withStatusMessage("Unauthorized")
+ .withHeader("Content-Type", "text/html")
+ .withHeader("Content-Length", String.valueOf(unauthorizedBody.getBytes(UTF_8).length))));
+
+ V2Connector.HTTP_REQUEST_RETRIES = 0;
+ Ec2MetadataServiceConnector v2Connector = getConnector(100);
+
+ try
+ {
+ v2Connector.apiCall(ZONE_NAME_QUERY);
Review Comment:
sure
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] smiklosovic commented on a diff in pull request #2403: CASSANDRA-16555 trunk heavy wip
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #2403:
URL: https://github.com/apache/cassandra/pull/2403#discussion_r1236953084
##########
.build/cassandra-build-deps-template.xml:
##########
@@ -123,5 +123,9 @@
<groupId>com.fasterxml.jackson.dataformat</groupId>
<artifactId>jackson-dataformat-yaml</artifactId>
</dependency>
+ <dependency>
+ <groupId>com.github.tomakehurst</groupId>
+ <artifactId>wiremock-jre8</artifactId>
Review Comment:
I run the tests with Java 11 and it just works. I think this is backward compatible. `jre11` probably can not be run with Java 8 but jre8 can be run with Java 11 just fine.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] jacek-lewandowski commented on a diff in pull request #2403: CASSANDRA-16555 trunk heavy wip
Posted by "jacek-lewandowski (via GitHub)" <gi...@apache.org>.
jacek-lewandowski commented on code in PR #2403:
URL: https://github.com/apache/cassandra/pull/2403#discussion_r1236894911
##########
src/java/org/apache/cassandra/locator/Ec2MetadataServiceConnector.java:
##########
@@ -0,0 +1,234 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+
+package org.apache.cassandra.locator;
+
+import java.io.IOException;
+import java.time.Duration;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+import java.util.stream.Collectors;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableMap;
+
+import org.apache.cassandra.exceptions.ConfigurationException;
+import org.apache.cassandra.utils.Clock;
+import org.apache.cassandra.utils.Pair;
+
+import static java.lang.String.format;
+
+public interface Ec2MetadataServiceConnector extends CloudApiCallable
+{
+ String EC2_METADATA_TYPE_PROPERTY = "ec2_metadata_type";
+ String DEFAULT_METADATA_SERVICE_URL = "http://169.254.169.254";
+
+ enum EC2MetadataType
+ {
+ V1("v1"),
+ V2("v2");
+
+ private final String name;
+
+ EC2MetadataType(String name)
+ {
+ this.name = name;
+ }
+
+ @Override
+ public String toString()
+ {
+ return name;
+ }
+ }
+
+ static Ec2MetadataServiceConnector create(SnitchProperties props)
+ {
+ try
+ {
+ EC2MetadataType type = EC2MetadataType.valueOf(props.get(EC2_METADATA_TYPE_PROPERTY, EC2MetadataType.V2.toString()).toUpperCase());
+
+ switch (type)
+ {
+ case V1:
+ return V1Connector.create(props);
+ case V2:
+ return V2Connector.create(props);
+ default:
+ throw new IllegalArgumentException();
+ }
+ }
+ catch (IllegalArgumentException ex)
+ {
+ throw new ConfigurationException(format("%s must be one of %s", EC2_METADATA_TYPE_PROPERTY,
+ Arrays.stream(EC2MetadataType.values())
+ .map(EC2MetadataType::toString)
+ .collect(Collectors.joining(", "))));
+ }
+ }
+
+ class V1Connector implements Ec2MetadataServiceConnector
+ {
+ @VisibleForTesting
+ static final String EC2_METADATA_URL_PROPERTY = "ec2_metadata_url";
+
+ private final String metadataServiceUrl;
+
+ static V1Connector create(SnitchProperties props)
+ {
+ String url = props.get(EC2_METADATA_URL_PROPERTY, DEFAULT_METADATA_SERVICE_URL);
+ return new V1Connector(url);
+ }
+
+ public V1Connector(String metadataServiceUrl)
+ {
+ this.metadataServiceUrl = metadataServiceUrl;
+ }
+
+ @Override
+ public String getApiUrl()
+ {
+ return metadataServiceUrl;
+ }
+
+ @Override
+ public String toString()
+ {
+ return "V1Connector{" + EC2_METADATA_URL_PROPERTY + '=' + getApiUrl() + '}';
Review Comment:
probably we should return full name, that is, including the information that it is EC2 connector
##########
.build/cassandra-build-deps-template.xml:
##########
@@ -123,5 +123,9 @@
<groupId>com.fasterxml.jackson.dataformat</groupId>
<artifactId>jackson-dataformat-yaml</artifactId>
</dependency>
+ <dependency>
+ <groupId>com.github.tomakehurst</groupId>
+ <artifactId>wiremock-jre8</artifactId>
Review Comment:
should whether it is `jre8` or `jre11` depend on which Java version we compile it against?
##########
src/java/org/apache/cassandra/locator/Ec2MetadataServiceConnector.java:
##########
@@ -0,0 +1,234 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+
+package org.apache.cassandra.locator;
+
+import java.io.IOException;
+import java.time.Duration;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+import java.util.stream.Collectors;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableMap;
+
+import org.apache.cassandra.exceptions.ConfigurationException;
+import org.apache.cassandra.utils.Clock;
+import org.apache.cassandra.utils.Pair;
+
+import static java.lang.String.format;
+
+public interface Ec2MetadataServiceConnector extends CloudApiCallable
+{
+ String EC2_METADATA_TYPE_PROPERTY = "ec2_metadata_type";
+ String DEFAULT_METADATA_SERVICE_URL = "http://169.254.169.254";
+
+ enum EC2MetadataType
+ {
+ V1("v1"),
+ V2("v2");
+
+ private final String name;
+
+ EC2MetadataType(String name)
+ {
+ this.name = name;
+ }
+
+ @Override
+ public String toString()
+ {
+ return name;
+ }
+ }
+
+ static Ec2MetadataServiceConnector create(SnitchProperties props)
+ {
+ try
+ {
+ EC2MetadataType type = EC2MetadataType.valueOf(props.get(EC2_METADATA_TYPE_PROPERTY, EC2MetadataType.V2.toString()).toUpperCase());
+
+ switch (type)
+ {
+ case V1:
+ return V1Connector.create(props);
+ case V2:
+ return V2Connector.create(props);
+ default:
+ throw new IllegalArgumentException();
+ }
+ }
+ catch (IllegalArgumentException ex)
+ {
+ throw new ConfigurationException(format("%s must be one of %s", EC2_METADATA_TYPE_PROPERTY,
+ Arrays.stream(EC2MetadataType.values())
+ .map(EC2MetadataType::toString)
+ .collect(Collectors.joining(", "))));
+ }
+ }
+
+ class V1Connector implements Ec2MetadataServiceConnector
+ {
+ @VisibleForTesting
+ static final String EC2_METADATA_URL_PROPERTY = "ec2_metadata_url";
+
+ private final String metadataServiceUrl;
+
+ static V1Connector create(SnitchProperties props)
+ {
+ String url = props.get(EC2_METADATA_URL_PROPERTY, DEFAULT_METADATA_SERVICE_URL);
+ return new V1Connector(url);
+ }
+
+ public V1Connector(String metadataServiceUrl)
+ {
+ this.metadataServiceUrl = metadataServiceUrl;
+ }
+
+ @Override
+ public String getApiUrl()
+ {
+ return metadataServiceUrl;
+ }
+
+ @Override
+ public String toString()
+ {
+ return "V1Connector{" + EC2_METADATA_URL_PROPERTY + '=' + getApiUrl() + '}';
+ }
+ }
+
+ class V2Connector extends V1Connector
+ {
+ private static final int MAX_TOKEN_TIME_IN_SECONDS = 21600;
+ private static final int MIN_TOKEN_TIME_IN_SECONDS = 30;
+
+ @VisibleForTesting
+ static int HTTP_REQUEST_RETRIES = 1;
+ @VisibleForTesting
+ static final String AWS_EC2_METADATA_TOKEN_TTL_SECONDS_HEADER_PROPERTY = "X-aws-ec2-metadata-token-ttl-seconds";
+
+ @VisibleForTesting
+ static final String AWS_EC2_METADATA_TOKEN_HEADER = "X-aws-ec2-metadata-token";
+ @VisibleForTesting
+ static final String TOKEN_QUERY = "/latest/api/token";
+
+ private Pair<String, Long> token;
+ private final Duration tokenTTL;
+
+ static V2Connector create(SnitchProperties props)
+ {
+ String url = props.get(EC2_METADATA_URL_PROPERTY, DEFAULT_METADATA_SERVICE_URL);
+
+ String tokenTTLString = props.get(AWS_EC2_METADATA_TOKEN_TTL_SECONDS_HEADER_PROPERTY,
+ Integer.toString(MAX_TOKEN_TIME_IN_SECONDS));
+
+ Duration tokenTTL;
+ try
+ {
+ tokenTTL = Duration.ofSeconds(Integer.parseInt(tokenTTLString));
+
+ if (tokenTTL.getSeconds() < MIN_TOKEN_TIME_IN_SECONDS || tokenTTL.getSeconds() > MAX_TOKEN_TIME_IN_SECONDS)
+ {
+ throw new ConfigurationException(format("property %s was set to %s which is not in allowed range of [%s..%s]",
+ AWS_EC2_METADATA_TOKEN_TTL_SECONDS_HEADER_PROPERTY,
+ tokenTTL,
+ MIN_TOKEN_TIME_IN_SECONDS,
+ MAX_TOKEN_TIME_IN_SECONDS));
+ }
+ }
+ catch (NumberFormatException ex)
+ {
+ throw new ConfigurationException(format("Unable to parse integer from %s, value to parse: %s",
+ AWS_EC2_METADATA_TOKEN_TTL_SECONDS_HEADER_PROPERTY, tokenTTLString));
+ }
+
+ return new V2Connector(url, tokenTTL);
+ }
+
+ public V2Connector(String metadataServiceUrl, Duration tokenTTL)
+ {
+ super(metadataServiceUrl);
+ this.tokenTTL = tokenTTL;
+ }
+
+ @Override
+ public String apiCall(String url, String query, String method, Map<String, String> extraHeaders, int expectedResponseCode) throws IOException
+ {
+ Map<String, String> headers = new HashMap<>(extraHeaders);
+ for (int retry = 0; retry <= HTTP_REQUEST_RETRIES; retry++)
+ {
+ String resolvedToken;
+ if (token != null && token.right > Clock.Global.currentTimeMillis())
+ resolvedToken = token.left;
+ else
+ resolvedToken = getToken();
+
+ try
+ {
+ headers.put(AWS_EC2_METADATA_TOKEN_HEADER, resolvedToken);
+ return super.apiCall(url, query, method, headers, expectedResponseCode);
+ }
+ catch (HttpException ex)
+ {
+ if (retry == HTTP_REQUEST_RETRIES)
+ throw ex;
+
+ if (ex.responseCode == 401) // invalidate token if it is 401
+ this.token = null;
+ }
+ }
+
+ throw new AssertionError();
+ }
+
+ @Override
+ public String toString()
+ {
+ return "V2Connector{" +
Review Comment:
same suggestion as for v1
##########
src/java/org/apache/cassandra/locator/CloudApiCallable.java:
##########
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+
+package org.apache.cassandra.locator;
+
+import java.io.DataInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.HttpURLConnection;
+import java.net.URL;
+import java.util.Map;
+
+import com.google.common.collect.ImmutableMap;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+public interface CloudApiCallable
Review Comment:
also, maybe a different name? something more like `CloudMetadataServiceConnector` (with `I` prefix if you decide to keep it an interface)?
##########
src/java/org/apache/cassandra/locator/CloudApiCallable.java:
##########
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+
+package org.apache.cassandra.locator;
+
+import java.io.DataInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.HttpURLConnection;
+import java.net.URL;
+import java.util.Map;
+
+import com.google.common.collect.ImmutableMap;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+public interface CloudApiCallable
Review Comment:
why not make it an abstract class and have url in a field?
##########
src/java/org/apache/cassandra/locator/Ec2MetadataServiceConnector.java:
##########
@@ -0,0 +1,234 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+
+package org.apache.cassandra.locator;
+
+import java.io.IOException;
+import java.time.Duration;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+import java.util.stream.Collectors;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableMap;
+
+import org.apache.cassandra.exceptions.ConfigurationException;
+import org.apache.cassandra.utils.Clock;
+import org.apache.cassandra.utils.Pair;
+
+import static java.lang.String.format;
+
+public interface Ec2MetadataServiceConnector extends CloudApiCallable
+{
+ String EC2_METADATA_TYPE_PROPERTY = "ec2_metadata_type";
+ String DEFAULT_METADATA_SERVICE_URL = "http://169.254.169.254";
+
+ enum EC2MetadataType
+ {
+ V1("v1"),
+ V2("v2");
+
+ private final String name;
+
+ EC2MetadataType(String name)
+ {
+ this.name = name;
+ }
+
+ @Override
+ public String toString()
+ {
+ return name;
+ }
+ }
+
+ static Ec2MetadataServiceConnector create(SnitchProperties props)
+ {
+ try
+ {
+ EC2MetadataType type = EC2MetadataType.valueOf(props.get(EC2_METADATA_TYPE_PROPERTY, EC2MetadataType.V2.toString()).toUpperCase());
+
+ switch (type)
+ {
+ case V1:
+ return V1Connector.create(props);
Review Comment:
well, provider could be included in enum, then we would just call `type.create(props)`
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] jacek-lewandowski commented on a diff in pull request #2403: CASSANDRA-16555 trunk heavy wip
Posted by "jacek-lewandowski (via GitHub)" <gi...@apache.org>.
jacek-lewandowski commented on code in PR #2403:
URL: https://github.com/apache/cassandra/pull/2403#discussion_r1226412606
##########
src/java/org/apache/cassandra/locator/Ec2SnitchIMDSv2.java:
##########
@@ -17,53 +17,90 @@
*/
package org.apache.cassandra.locator;
+
import java.io.DataInputStream;
import java.io.FilterInputStream;
import java.io.IOException;
import java.net.HttpURLConnection;
import java.net.URL;
import java.nio.charset.StandardCharsets;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
-import org.apache.cassandra.exceptions.ConfigurationException;
-import org.apache.cassandra.io.util.FileUtils;
-import org.apache.cassandra.utils.Clock.Global;
+import com.google.common.base.Supplier;
+import com.google.common.base.Suppliers;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import org.apache.cassandra.io.util.FileUtils;
+
/**
- * An implementation of the Ec2Snitch which uses Instance Meta Data Service v2 (IMDSv2) which requires you
+ * An implementation of the Ec2Snitch which uses Instance Metadata Service v2 (IMDSv2) which requires you
* to get a session token first before calling the metaData service
*/
public class Ec2SnitchIMDSv2 extends Ec2Snitch
{
- protected static final Logger logger = LoggerFactory.getLogger(Ec2SnitchIMDSv2.class);
- private static final long REFRESH_TOKEN_TIME = 21600;
- private static final String AWS_EC2_METADATA_HEADER_TTL = "X-aws-ec2-metadata-token-ttl-seconds";
- private static final String TOKEN_TTL_SECONDS = String.valueOf(REFRESH_TOKEN_TIME);
- private static final String AWS_EC2_METADATA_HEADER = "X-aws-ec2-metadata-token";
+ private static final Logger logger = LoggerFactory.getLogger(Ec2SnitchIMDSv2.class);
+ private static final int MAX_TOKEN_TIME_IN_SECONDS = 21600;
+ private static final String AWS_EC2_METADATA_TOKEN_HEADER = "X-aws-ec2-metadata-token";
+ private static final String AWS_EC2_METADATA_TOKEN_TTL_SECONDS_HEADER = "X-aws-ec2-metadata-token-ttl-seconds";
private static final String TOKEN_ENDPOINT = "http://169.254.169.254/latest/api/token";
- private String myToken;
- private Long myLastTokenTime;
-
+ private Supplier<ApiCallResult> tokenSupplier;
+ private long tokenTTL;
- public Ec2SnitchIMDSv2() throws IOException, ConfigurationException
+ public Ec2SnitchIMDSv2() throws IOException
{
- super();
+ super(new SnitchProperties());
}
- @Override
- String awsApiCall(final String url) throws IOException, ConfigurationException
+ public Ec2SnitchIMDSv2(SnitchProperties snitchProperties) throws IOException
{
- // Populate the region and zone by introspection, fail if 404 on metadata
- if (myToken == null || myLastTokenTime == null
- || Global.currentTimeMillis() - myLastTokenTime > (REFRESH_TOKEN_TIME - 100))
+ super(snitchProperties);
+
+ String parsedTokenTTL = snitchProperties.get(AWS_EC2_METADATA_TOKEN_TTL_SECONDS_HEADER, Integer.toString(MAX_TOKEN_TIME_IN_SECONDS));
+
+ try
{
- getAndSetNewToken();
+ tokenTTL = Integer.parseInt(parsedTokenTTL);
+
+ if (tokenTTL > MAX_TOKEN_TIME_IN_SECONDS || tokenTTL < 1)
+ {
+ logger.info(String.format("property %s was set to %s which is more than maximum allowed range of (0, 21600]: %s, defaulting to %s",
+ AWS_EC2_METADATA_TOKEN_HEADER, tokenTTL, MAX_TOKEN_TIME_IN_SECONDS, MAX_TOKEN_TIME_IN_SECONDS));
+ tokenTTL = MAX_TOKEN_TIME_IN_SECONDS;
+ }
}
- final HttpURLConnection conn = (HttpURLConnection) new URL(url).openConnection();
- conn.setRequestProperty(AWS_EC2_METADATA_HEADER, myToken);
- return getContent(conn);
+ catch (NumberFormatException ex)
+ {
+ logger.error(String.format("Unable to parse integer from %s, value to parse: %s. Defaulting to %s",
+ AWS_EC2_METADATA_TOKEN_HEADER, parsedTokenTTL, MAX_TOKEN_TIME_IN_SECONDS));
+ tokenTTL = MAX_TOKEN_TIME_IN_SECONDS;
+ }
+
+ tokenSupplier = Suppliers.memoizeWithExpiration(this::getNewToken, Math.max(5, tokenTTL - 100), TimeUnit.SECONDS);
Review Comment:
Well, it is memoized when it is retrieved. It is just a session token and a session can be expired externally as well. Therefore, we should simply recognize such a situation and simply retry
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] smiklosovic closed pull request #2403: CASSANDRA-16555 trunk
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic closed pull request #2403: CASSANDRA-16555 trunk
URL: https://github.com/apache/cassandra/pull/2403
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] smiklosovic commented on a diff in pull request #2403: CASSANDRA-16555 trunk
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #2403:
URL: https://github.com/apache/cassandra/pull/2403#discussion_r1243295121
##########
src/java/org/apache/cassandra/locator/Ec2MetadataServiceConnector.java:
##########
@@ -0,0 +1,218 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+
+package org.apache.cassandra.locator;
+
+import java.io.IOException;
+import java.time.Duration;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+import java.util.function.Function;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableMap;
+
+import org.apache.cassandra.exceptions.ConfigurationException;
+import org.apache.cassandra.utils.Clock;
+import org.apache.cassandra.utils.Pair;
+
+import static java.lang.String.format;
+import static java.util.Arrays.stream;
+import static java.util.stream.Collectors.joining;
+import static org.apache.cassandra.locator.Ec2MetadataServiceConnector.EC2MetadataType.v2;
+
+abstract class Ec2MetadataServiceConnector extends AbstractCloudMetadataServiceConnector
+{
+ static final String EC2_METADATA_TYPE_PROPERTY = "ec2_metadata_type";
+ static final String EC2_METADATA_URL_PROPERTY = "ec2_metadata_url";
+ static final String DEFAULT_EC2_METADATA_URL = "http://169.254.169.254";
+
+ Ec2MetadataServiceConnector(SnitchProperties properties)
+ {
+ metadataServiceUrl = properties.get(EC2_METADATA_URL_PROPERTY, DEFAULT_EC2_METADATA_URL);
+ }
+
+ enum EC2MetadataType
+ {
+ v1(props -> Ec2MetadataServiceConnector.V1Connector.create(props)),
Review Comment:
@jacek-lewandowski
I had it like that but I liked this more because I do not need to use "toUpperCase()" when I am parsing it from the file. In the file, it will be probably "v1" or "v2" intead of "V1" or "V2". Also in 'toString' it would look strange to have it uppercased so I would have to lowercase it again. And all of this just to have an enum in uppercase ...
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org