Directly download commercial ip geolocation databases from providers #110844

joegallo · 2024-07-12T19:39:05Z

Adds a feature for downloading commercial ip geolocation databases directly from different providers and exposing those databases to the geoip processor -- at present the only supported provider is MaxMind.

…10553)

elastic#110675) Co-authored-by: Joe Gallo <[email protected]>

…elastic#110666) (elastic#110679)

Some cluster state customs have singular nouns and some have plural, and I think plural is more natural for this case.

…f no database metadata is present

to work around a race condition on the first run (when there is no next run already scheduled).

From ingest.geoip.downloader.maxmind.default.license_key to just ingest.geoip.downloader.maxmind.license_key.

…arch into enterprise-downloader

In this version Maxmind is very much a hardcoded configuration -- I think we might want a NamedWriteable here rather than merely a Writeable, but this allows us to keep making progress.

…terprise-downloader

...main/java/org/elasticsearch/ingest/geoip/direct/TransportGetDatabaseConfigurationAction.java

jakelandis · 2024-07-17T19:29:49Z

...eoip/src/main/java/org/elasticsearch/ingest/geoip/EnterpriseGeoIpDownloaderTaskExecutor.java

+        try {
+            this.cachedSecureSettings = extractSecureSettings(settings, List.of(MAXMIND_LICENSE_KEY_SETTING));
+        } catch (GeneralSecurityException e) {
+            logger.error("Keystore exception while reloading enterprise geoip download task executor", e);


pretty sure that you want to rethrow this (or wrap with better type and rethrow) so that the REST reload API also know that this failed.

That makes sense, yeah. 95a820b

jakelandis · 2024-07-17T19:34:21Z

...eoip/src/main/java/org/elasticsearch/ingest/geoip/EnterpriseGeoIpDownloaderTaskExecutor.java

+
+            @Override
+            public InputStream getFile(String setting) {
+                throw new IllegalStateException("A NotificationService setting cannot be File.");


NotificationService ?
Also, perhaps UnsupportedOperationException instead of IllegalStateException ?

Agreed, 572991a

jakelandis · 2024-07-17T19:55:07Z

...eoip/src/main/java/org/elasticsearch/ingest/geoip/EnterpriseGeoIpDownloaderTaskExecutor.java

+        // get the secure settings out
+        final SecureSettings sourceSecureSettings = Settings.builder().put(source, true).getSecureSettings();
+        // filter and cache them...
+        final Map<String, Tuple<SecureString, byte[]>> cache = new HashMap<>();


super nitpick (feel free to ignore)

I think Records reads a lot nicer than Tuple's and are only 1 line of code difference (assuming you don't need to backport to 7.x).

Cache is bit misleading since the SecureSettings returned here is what is cached, this is the just the innerMap ?

I am in the business of saying yes to things: 12655ed and 31af5d6

jakelandis · 2024-07-17T20:13:05Z

...eoip/src/main/java/org/elasticsearch/ingest/geoip/EnterpriseGeoIpDownloaderTaskExecutor.java

+    /**
+     * Extracts the {@link SecureSettings}` out of the passed in {@link Settings} object. The {@code Setting} argument has to have the
+     * {@code SecureSettings} open/available. Normally {@code SecureSettings} are available only under specific callstacks (eg. during node
+     * initialization or during a `reload` call). The returned copy can be reused freely as it will never be closed (this is a bit of
+     * cheating, but it is necessary in this specific circumstance). Only works for secure settings of type string (not file).
+     *
+     * @param source               A {@code Settings} object with its {@code SecureSettings} open/available.
+     * @param securePluginSettings The list of settings to copy.
+     * @return A copy of the {@code SecureSettings} of the passed in {@code Settings} argument.
+     */
+    private static SecureSettings extractSecureSettings(Settings source, List<Setting<?>> securePluginSettings)
+        throws GeneralSecurityException {
+        // get the secure settings out
+        final SecureSettings sourceSecureSettings = Settings.builder().put(source, true).getSecureSettings();
+        // filter and cache them...
+        final Map<String, Tuple<SecureString, byte[]>> cache = new HashMap<>();
+        if (sourceSecureSettings != null && securePluginSettings != null) {
+            for (final String settingKey : sourceSecureSettings.getSettingNames()) {
+                for (final Setting<?> secureSetting : securePluginSettings) {
+                    if (secureSetting.match(settingKey)) {
+                        cache.put(
+                            settingKey,
+                            new Tuple<>(sourceSecureSettings.getString(settingKey), sourceSecureSettings.getSHA256Digest(settingKey))
+                        );
+                    }
+                }
+            }
+        }


only reviewed the secure settings bits, LGTM pending the outcome of the error handling (see comment above)

elasticsearchmachine · 2024-07-17T21:11:47Z

Pinging @elastic/es-data-management (Team:Data Management)

dakrone

LGTM, thanks for working through this with me

jakelandis

SecureSetting bits LGTM

…lastic#110844) Co-authored-by: Keith Massey <[email protected]>

…viders (#110844) (#111077) Co-authored-by: Keith Massey <[email protected]>

…lastic#110844) Co-authored-by: Keith Massey <[email protected]>

joegallo and others added 27 commits July 8, 2024 10:42

A small tidiness refactor of the GeoIpTaskState's Metadata (elastic#1…

64fbdf4

…10553)

Adding a unit test for GeoIpDownloader.cleanDatabases (elastic#110650) (

a86555c

elastic#110675) Co-authored-by: Joe Gallo <[email protected]>

Removing the use of Stream::peek from GeoIpDownloader::cleanDatabases (…

f9e0607

…elastic#110666) (elastic#110679)

WIP squashed history of dev work so far

c62f63b

Rename this class

87bba46

Tweak these constants to better shape the state

8f86909

Some cluster state customs have singular nouns and some have plural, and I think plural is more natural for this case.

Make it possible to override the endpoint for tests

29f9db2

Log and skip if there are no credentials

a0e009e

Updating EnterpriseGeoIpDownloaderLicenseListener to not start task i…

fc8dd0c

…f no database metadata is present

Schedule the next run at the very beginning of the run

4ae4171

to work around a race condition on the first run (when there is no next run already scheduled).

Guard these contains operations for the no-change case

8c447c6

Adding EnterpriseGeoIpTaskStateSerializationTests

18ec1e4

Remove the conflict between the two geoip downloaders

ec64555

Rename this setting and the associated constant

7110f42

From ingest.geoip.downloader.maxmind.default.license_key to just ingest.geoip.downloader.maxmind.license_key.

Address todos and comments, and tidy up logging

3102a7a

Bump to Priority.NORMAL and passed hallway review

dd72464

Merge branch 'enterprise-downloader' of github.com:joegallo/elasticse…

b5e44b2

…arch into enterprise-downloader

Adding IngestGeoIpMetadataTests (and fixing a bug it found)

102db55

This reads slightly better in terms of intent

e921255

Switch from a setting to a field for the account_id

8adcabc

In this version Maxmind is very much a hardcoded configuration -- I think we might want a NamedWriteable here rather than merely a Writeable, but this allows us to keep making progress.

Adding yaml rest tests

528a0a4

Adjust the new NodeFeature slightly

ad55ec3

Collapse the two versions and add an alias

1d05673

beginnings of a unit test for EnterpriseGeoIpDownloaderTests

8d5518a

Override the default maxmind endpoint for the yaml tests

38aa680

Merge remote-tracking branch 'masseyke/enterprise-downloader' into en…

36e3060

…terprise-downloader

Merge branch 'main' into enterprise-downloader

b4ef611

joegallo added >feature :Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP Team:Data Management Meta label for data/management team labels Jul 12, 2024

dakrone reviewed Jul 17, 2024

View reviewed changes

...main/java/org/elasticsearch/ingest/geoip/direct/TransportGetDatabaseConfigurationAction.java Outdated Show resolved Hide resolved

joegallo added 5 commits July 17, 2024 11:14

Expand javadoc slightly

3f3a7c0

Don't handle the empty state case specially

90a749c

No treble

9f54396

Extend the fixture to cover an edge case

e9e2c1c

Merge branch 'main' into enterprise-downloader

234a166

jakelandis reviewed Jul 17, 2024

View reviewed changes

joegallo added 5 commits July 17, 2024 16:23

Switch exception types and messages

572991a

Rename inner variable

31af5d6

Use a record instead of a Tuple

12655ed

Wrap and rethrow rather than logging

95a820b

Merge branch 'main' into enterprise-downloader

22c93bd

joegallo requested review from jakelandis and dakrone July 17, 2024 21:10

joegallo marked this pull request as ready for review July 17, 2024 21:11

joegallo requested a review from a team as a code owner July 17, 2024 21:11

dakrone approved these changes Jul 17, 2024

View reviewed changes

jakelandis reviewed Jul 17, 2024

View reviewed changes

jakelandis approved these changes Jul 17, 2024

View reviewed changes

joegallo merged commit 27e7601 into elastic:main Jul 18, 2024
15 checks passed

joegallo deleted the enterprise-downloader branch July 18, 2024 00:55

joegallo mentioned this pull request Jul 18, 2024

Add an 8.15 backport version for the ENTERPRISE_GEOIP_DOWNLOADER #111069

Merged

joegallo added a commit to joegallo/elasticsearch that referenced this pull request Jul 18, 2024

Directly download commercial ip geolocation databases from providers (e…

05057a8

…lastic#110844) Co-authored-by: Keith Massey <[email protected]>

joegallo mentioned this pull request Jul 18, 2024

[8.15] Directly download commercial ip geolocation databases from providers (#110844) #111077

Merged

joegallo added a commit that referenced this pull request Jul 19, 2024

[8.15] Directly download commercial ip geolocation databases from pro…

ff9a8eb

…viders (#110844) (#111077) Co-authored-by: Keith Massey <[email protected]>

ioanatia pushed a commit to ioanatia/elasticsearch that referenced this pull request Jul 22, 2024

Directly download commercial ip geolocation databases from providers (e…

27a3422

…lastic#110844) Co-authored-by: Keith Massey <[email protected]>

salvatore-campagna pushed a commit to salvatore-campagna/elasticsearch that referenced this pull request Jul 23, 2024

Directly download commercial ip geolocation databases from providers (e…

c1384ce

…lastic#110844) Co-authored-by: Keith Massey <[email protected]>

masseyke mentioned this pull request Aug 7, 2024

Ingest download databases docs #111688

Merged

lkts mentioned this pull request Aug 13, 2024

Fix references to logsdb index mode in release highlights lkts/elasticsearch#1

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Directly download commercial ip geolocation databases from providers #110844

Directly download commercial ip geolocation databases from providers #110844

joegallo commented Jul 12, 2024 •

edited

Loading

jakelandis Jul 17, 2024

joegallo Jul 17, 2024

jakelandis Jul 17, 2024

joegallo Jul 17, 2024

jakelandis Jul 17, 2024

joegallo Jul 17, 2024

jakelandis Jul 17, 2024

elasticsearchmachine commented Jul 17, 2024

dakrone left a comment

jakelandis left a comment

Directly download commercial ip geolocation databases from providers #110844

Directly download commercial ip geolocation databases from providers #110844

Conversation

joegallo commented Jul 12, 2024 • edited Loading

jakelandis Jul 17, 2024

Choose a reason for hiding this comment

joegallo Jul 17, 2024

Choose a reason for hiding this comment

jakelandis Jul 17, 2024

Choose a reason for hiding this comment

joegallo Jul 17, 2024

Choose a reason for hiding this comment

jakelandis Jul 17, 2024

Choose a reason for hiding this comment

joegallo Jul 17, 2024

Choose a reason for hiding this comment

jakelandis Jul 17, 2024

Choose a reason for hiding this comment

elasticsearchmachine commented Jul 17, 2024

dakrone left a comment

Choose a reason for hiding this comment

jakelandis left a comment

Choose a reason for hiding this comment

joegallo commented Jul 12, 2024 •

edited

Loading