[GitHub] [cassandra] smiklosovic commented on a diff in pull request #2403: CASSANDRA-16555 trunk heavy wip

["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