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

ClusterManager is created with incorrect parameters #52860

Closed
pgayvallet opened this issue Dec 12, 2019 · 4 comments · Fixed by #53263
Closed

ClusterManager is created with incorrect parameters #52860

pgayvallet opened this issue Dec 12, 2019 · 4 comments · Fixed by #53263
Assignees
Labels
bug Fixes for quality problems that affect the customer experience Feature:New Platform Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc

Comments

@pgayvallet
Copy link
Contributor

In the LegacyService, we are creating the cluster manager by passing the actual legacy configuration

require('../../../cli/cluster/cluster_manager').create(
this.coreContext.env.cliArgs,
config,
await basePathProxy$.toPromise()
);

However the function actually expects the raw settings instead

export default class ClusterManager {
static create(opts, settings = {}, basePathProxy) {
return new ClusterManager(
opts,
Config.withDefaultSchema(settings),
basePathProxy
);
}

From #52251 (comment):

The fix is (well, should be) easy

require('../../../cli/cluster/cluster_manager').create(
      this.coreContext.env.cliArgs,
--    config,
++    config.get(),
      await basePathProxy$.toPromise()
    );

However the legacy service tests are a mess, and are not properly mocking with correct object types (the config in test is actually a Record and not a LegacyConfig ). Already had to fix that in #52060, so would be easier to fix in a separate PR once it lands.

@pgayvallet pgayvallet added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Feature:New Platform labels Dec 12, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform (Team:Platform)

@mshustov mshustov added the bug Fixes for quality problems that affect the customer experience label Dec 12, 2019
@pgayvallet
Copy link
Contributor Author

config is a legacy config here. config.get() returns the whole configuration when no key is provided

get(key) {
if (!key) {
return clone(this[vals]);
}
const value = _.get(this[vals], key);
if (value === undefined) {
if (!this.has(key)) {
throw new Error('Unknown config key: ' + key);
}
}
return clone(value);
}

@mshustov
Copy link
Contributor

So many configs 🙈

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Feature:New Platform Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants