-
Notifications
You must be signed in to change notification settings - Fork 25
WIP: 🕸️ Bug / Support AWS OpenSearch Auth #1081
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
base: main
Are you sure you want to change the base?
Changes from 16 commits
4685670
16daf53
bf529fc
d636aac
da47b8b
d4edc0f
7275919
eaa1762
ab084cf
0ee6553
f74aa4a
a61d27f
eb2f820
17070fd
897442d
439d760
5fa7b18
94db9a7
7393589
e39af2a
7f995d3
b97e8f2
ce4504d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,4 +16,5 @@ lerna-debug.log | |
| !.env.test | ||
| !**/.env.schema | ||
|
|
||
| /configs | ||
| /configs | ||
| /modules/graphql-router/src/admin/** | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,10 +7,9 @@ import enforceAccessControl, { getDefaultServerSideFilter } from '#accessControl | |
| import fallbackConfigs, { validateConfigs } from '#config/index.js'; | ||
| import downloadRoutes from '#download/index.js'; | ||
| import getGraphQLRoutes from '#graphqlRoutes.js'; | ||
| import { getIndexMapping } from '#searchClient/index.js'; | ||
| import { buildCatalogueIntrospectionBody } from '#introspection/buildCatalogueIntrospection.js'; | ||
| import resolveCatalogueFields from '#mapping/resolveCatalogueFields.js'; | ||
| import buildSearchClient, { type SearchClient } from '#searchClient/index.js'; | ||
| import buildSearchClient, { getIndexMapping, type SearchClient } from '#searchClient/index.js'; | ||
| import type { ArrangerBaseContext } from '#types.js'; | ||
| import { addContext } from '#utils/context.js'; | ||
| import { warnDeprecatedConfigsSource } from '#utils/noops.js'; | ||
|
|
@@ -56,10 +55,18 @@ const arrangerRouter = async <Context extends ArrangerBaseContext>({ | |
| try { | ||
| const aggregatedConfigs = mergeConfigs(fallbackConfigs, customConfigs); | ||
|
|
||
| const { enableAdmin, enableDebug, esHost, esPass, esUser, searchEngine, ...configs } = validateConfigs( | ||
| aggregatedConfigs, | ||
| customEsClient, | ||
| ); | ||
| const { | ||
| enableAdmin, | ||
| enableDebug, | ||
| esHost, | ||
| esPass, | ||
| esUser, | ||
| searchEngine, | ||
| searchEngineAuthType, | ||
| searchEngineAuthRegion, | ||
| searchEngineAuthService, | ||
| ...configs | ||
| } = validateConfigs(aggregatedConfigs, customEsClient); | ||
|
|
||
| warnDeprecatedConfigsSource({ configsSource, enableDebug: aggregatedConfigs.enableDebug }); | ||
|
|
||
|
|
@@ -69,14 +76,24 @@ const arrangerRouter = async <Context extends ArrangerBaseContext>({ | |
| const esClient = | ||
| customEsClient || | ||
| (await buildSearchClient({ | ||
| client: searchEngine, | ||
| clientType: searchEngine, | ||
| node: esHost, | ||
| password: esPass, | ||
| username: esUser, | ||
| auth: { | ||
| password: esPass, | ||
| username: esUser, | ||
| type: searchEngineAuthType, | ||
| region: searchEngineAuthRegion, | ||
| service: searchEngineAuthService, | ||
| }, | ||
| })); | ||
|
|
||
| if (!esClient) | ||
| throw new Error( | ||
| 'Failure with buildSearchClient while initializing Arranger Server. Unable to fetch index mappings.', | ||
| ); | ||
|
|
||
| const mappingFromIndex = await getIndexMapping({ | ||
| enableDebug, | ||
| enableDebug: !!enableDebug, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how is this change necessary?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a Type error, enableDebug at line 58 is
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alternately we could change the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sounds like a type problem in the function's definition, rather than something to be coerced in this manner 🤔 @joneubank thoughts on this? |
||
| searchClient: esClient, | ||
| esIndex: configs[configRootProperties.ES_INDEX], | ||
| }); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We aren't adding any code to consume these env variables, in fact we make no code changes to the server. Let's not add new env variables to our schema that are unused.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted get a start on documenting the Auth changes and new config values, then updated the client side without revisiting. Will revise.