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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
