Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add support to run pipeline registration in secured ES clusters #867

Merged
merged 5 commits into from
Apr 30, 2024

Conversation

landryb
Copy link
Contributor

@landryb landryb commented Apr 26, 2024

i might be a paranoid but my ES clusters have security on and require authentication, so this is required to deploy https://geonetwork.github.io/geonetwork-ui/main/docs/guide/deploy.html#enabling-improved-search-fields

i know this might be ugly, and 'better' to take username/password from env vars .. feedback welcome.

also i failed to run npm run lint (because i haven't done a full build of the repo) so i suppose wrapping is wrong.

with that diff i'm able to register the pipeline and set it as the default one for a custom gn-records index.

$sudo -u tomcat node /data/gn-es-pipeline/register-es-pipelines.js register --host=http://127.0.0.1:9400 --username xxx --password xxx --records-index gn-records-dev
querying currently registered pipelines...

 > xpack_monitoring_6
   This pipeline upgrades documents from the older version of the Monitoring API to the newer version (7) by fixing breaking changes in those older documents before they are indexed from the older version (6).

 > geonetwork-ui
   GeoNetwork-UI pipeline

 > xpack_monitoring_7
   This is a placeholder pipeline for Monitoring API version 7 so that future versions may fix breaking changes.

adding geonetwork-ui pipeline...
geonetwork-ui pipeline was successfully registered!

setting geonetwork-ui as default pipeline...
geonetwork-ui pipeline was successfully set as default!

afaict for non-securised ES clusters, sending an empty Authorization: header doesnt seem to bother ES which replies anyway. I can make it conditional to authHeader being non-empty but i don't know the syntax of modern javascript to do that.

clear doesn't seem to work, because running it after register just says No geonetwork-ui pipelines found, exiting

cc @fgravin @jahow @f-necas for opinions

Copy link
Collaborator

@jahow jahow left a comment

Choose a reason for hiding this comment

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

Thank you, that makes a lot of sense! I suggested a small improvement because there's already a native function for converting to base64 (atob). Also could you please leave a note in the docs about this option? e.g. at the end of this chapter https://github.com/geonetwork/geonetwork-ui/blob/main/docs/guide/deploy.md#option-a-executing-a-node-script

Thank you!!

tools/pipelines/register-es-pipelines.js Outdated Show resolved Hide resolved
tools/pipelines/register-es-pipelines.js Outdated Show resolved Hide resolved
@jahow
Copy link
Collaborator

jahow commented Apr 26, 2024

PS: I've tested without authentication on the default docker composition and the empty Auth header does not cause any issue.

@landryb
Copy link
Contributor Author

landryb commented Apr 29, 2024

PS: I've tested without authentication on the default docker composition and the empty Auth header does not cause any issue.

thanks, i was mostly having concerns about potential weird mitm 'secure' proxies that might exist in cloud environments and choke on that...

as for atob, i had seen it on stackoverflow but i wasn't sure if it was part of the standard library - thanks for confirming.

@landryb
Copy link
Contributor Author

landryb commented Apr 29, 2024

ah, now i know why i didn't use btoa, https://developer.mozilla.org/en-US/docs/Web/API/btoa says it is available in browsers/for web workers, so wasn't sure about nodejs. But it's listed below in the compatible list, so using it is safe.

@landryb
Copy link
Contributor Author

landryb commented Apr 29, 2024

and now i've figured out why clear errors out, that was quite obvious...:

"type":"illegal_argument_exception",
"reason":"pipeline [geonetwork-ui] cannot be deleted because it is the default pipeline for 1 index(es) including [gn-records-dev]"

edit to be able to remove a pipeline, first it should be removed from the index settings via a PUT '{"index.default_pipeline":"_none"}'

@jahow
Copy link
Collaborator

jahow commented Apr 29, 2024

edit to be able to remove a pipeline, first it should be removed from the index settings via a PUT '{"index.default_pipeline":"_none"}'

Ah right, that makes sense. I wouldn't worry too much about this to be honest, the clear command shouldn't be that important anyways.

Copy link
Collaborator

@jahow jahow left a comment

Choose a reason for hiding this comment

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

@landryb I've added a commit to the branch because the formatting was failing, other than that this is good to merge! thanks!

@jahow jahow merged commit 61774f7 into geonetwork:main Apr 30, 2024
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants