Skip to content

Conversation

@stu-elastic
Copy link
Contributor

Adds support in the scripting fields API for the ip mapped type,
including the runtime script type.

Adds a new value object, IPAddress, to avoid exposing Java's
InetAddress. InetAddress may cause name resolution if whitelisted
improperly.

field('ip'), implemented by IpDocValuesField exposes:
IPAddress get(IPAddress)
IPAddress get(int, IPAddress)
Iterator<IPAddress> iterator()
List asStrings()
String asString(String)
String asString(int, String)

IPAddress exposes:
boolean isV4()
boolean isV6()
String toString()

Refs: #79105

Adds support in the scripting fields API for the `ip` mapped type,
including the runtime script type.

Adds a new value object, `IPAddress`, to avoid exposing Java's
`InetAddress`. `InetAddress` may cause name resolution if whitelisted
improperly.

`field('ip')`, implemented by `IpDocValuesField` exposes:
  `IPAddress get(IPAddress)`
  `IPAddress get(int, IPAddress)`
  `Iterator<IPAddress> iterator()`
  `List asStrings()`
  `String asString(String)`
  `String asString(int, String)`

`IPAddress` exposes:
  `boolean isV4()`
  `boolean isV6()`
  `String toString()`

Refs: elastic#79105
@stu-elastic stu-elastic added :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >enhancement labels Dec 6, 2021
@stu-elastic stu-elastic marked this pull request as ready for review December 6, 2021 22:42
@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Dec 6, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@elasticsearchmachine
Copy link
Collaborator

Hi @stu-elastic, I've created a changelog YAML for you.

Copy link
Contributor

@jdconrad jdconrad left a comment

Choose a reason for hiding this comment

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

This looks good! Please do not commit before addressing the plumbing comment. It's also a huge bummer about how there's two different ways we store IP addresses. I wonder if this is something that can be addressed in the future.

@Override
public DocValuesField<?> getScriptField(String name) {
return new DelegateDocValuesField(new Strings(new IpSupplier(getBytesValues())), name);
return new IpDocValuesField(getBytesValues(), name);
Copy link
Contributor

Choose a reason for hiding this comment

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

This should take in a ToScriptField and follow our standard plumbing pattern to this point. I think there's a real possibility this may be required for source fallback.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gotcha, will fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, done.

return builder.apply(lower, upper);
}

public static final class IpScriptDocValues extends ScriptDocValues<String> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just for clarity, this all can be deleted because we use the other version now for doc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved it to IpDocValuesField.SortedSetIpSupplier. Seemed to make more sense there.

}

public boolean isV4() {
return address instanceof Inet4Address;
Copy link
Contributor

Choose a reason for hiding this comment

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

Big thumbs up for this utility method along with the v6. Annoying that the Java API is like this. (Nothing actionable, just like this solution.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I had to provide these.

Without these two methods there would be no way for users to determine the version of address.

@stu-elastic stu-elastic merged commit bf4861c into elastic:master Dec 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >enhancement Team:Core/Infra Meta label for core/infra team v8.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants