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

[cloud plugin] Add serverless projectName to configuration and contract #166330

Merged

Conversation

pgayvallet
Copy link
Contributor

Summary

Part of #166182
Similar to #161728

Add the serverless.project_name config setting to the cloud plugin, and expose the serverless.projectName info from the cloud plugin's API.

@pgayvallet pgayvallet added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes v8.11.0 labels Sep 13, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core (Team:Core)

@@ -230,6 +230,7 @@ export default function ({ getService }: PluginFunctionalProviderContext) {
'xpack.cloud.projects_url (any)', // It's a string (any because schema.conditional)
// can't be used to infer urls or customer id from the outside
'xpack.cloud.serverless.project_id (string)',
'xpack.cloud.serverless.project_name (string)',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@elastic/kibana-security I don't think this is a problem given it's only exposed on authenticated page and given we're going to expose the info in the Kibana navigation header, but still pointing this out.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good, considering that we're already exposing project_id which is more important.

Just curious, if we ever allow user to rename the project, does it mean we'll have to restart Kibana to apply the config change? Feels a bit heavy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The way the project controller works, changing the name from the Cloud UI will trigger an update of the deployment, yes. But there's isn't really ways around this until Cloud exposes an API for Kibana to get the info less passively

Comment on lines +75 to +79
/**
* The serverless project name.
* Will always be present if `isServerlessEnabled` is `true`
*/
projectName?: string;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Technically, will only be present once the PR to inject the info from the controller is merged, but I don't think we need to have this in the comments.

Comment on lines +36 to +43
schema.object(
{
project_id: schema.string(),
project_name: schema.maybe(schema.string()),
},
// avoid future chicken-and-egg situation with the component populating the config
{ unknowns: 'ignore' }
)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For future additions, this will allow to inject the data from the controller before having the version of Kibana knowing of the new setting is promoted to production

Copy link
Contributor

@TinaHeiligers TinaHeiligers left a comment

Choose a reason for hiding this comment

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

LGTM

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
cloud 5.4KB 5.6KB +164.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Copy link
Member

@azasypkin azasypkin left a comment

Choose a reason for hiding this comment

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

LGTM, just left one non-blocking question.

@@ -230,6 +230,7 @@ export default function ({ getService }: PluginFunctionalProviderContext) {
'xpack.cloud.projects_url (any)', // It's a string (any because schema.conditional)
// can't be used to infer urls or customer id from the outside
'xpack.cloud.serverless.project_id (string)',
'xpack.cloud.serverless.project_name (string)',
Copy link
Member

Choose a reason for hiding this comment

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

Sounds good, considering that we're already exposing project_id which is more important.

Just curious, if we ever allow user to rename the project, does it mean we'll have to restart Kibana to apply the config change? Feels a bit heavy.

@pgayvallet pgayvallet merged commit b1bfe92 into elastic:main Sep 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v8.11.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants