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.

Sign up for GitHub

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

Jump to bottom

[cloud plugin] Add serverless projectId to configuration and contract #161728

Merged
merged 7 commits into from
Jul 14, 2023
Merged

[cloud plugin] Add serverless projectId to configuration and contract #161728

merged 7 commits into from
Jul 14, 2023

Conversation

pgayvallet
Copy link
Contributor

@pgayvallet pgayvallet commented Jul 12, 2023 •
edited
Loading

Summary

Fix #161652

Add the serverless.projectId config setting to the cloud plugin, and expose the isCloudServerless and serverless.projectId 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 release_note:skip Skip the PR/issue when compiling release notes v8.10.0 labels Jul 12, 2023
@pgayvallet pgayvallet marked this pull request as ready for review July 12, 2023 12:26
@pgayvallet pgayvallet requested review from a team as code owners July 12, 2023 12:26
@pgayvallet pgayvallet requested a review from a team July 12, 2023 12:26
@pgayvallet pgayvallet requested a review from a team as a code owner July 12, 2023 12:26
@elasticmachine
Copy link
Contributor

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

@botelastic botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Jul 13, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

azasypkin
azasypkin approved these changes Jul 14, 2023
View reviewed changes
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!

test/plugin_functional/test_suites/core_plugins/rendering.ts
@@ -219,6 +219,7 @@ export default function ({ getService }: PluginFunctionalProviderContext) {
'xpack.cloud.organization_url (string)',
'xpack.cloud.billing_url (string)',
'xpack.cloud.profile_url (string)',
'xpack.cloud.serverless.project_id (string)',
Copy link
Member

Choose a reason for hiding this comment

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

optional nit: it'd be great to leave a comment saying that it doesn't include anything sensitive, just in case we have to conduct an audit of this file at some point.

gsoldevila
gsoldevila reviewed Jul 14, 2023
View reviewed changes
x-pack/plugins/cloud/server/plugin.ts
* The serverless projectId.
* Will always be present if `isServerlessEnabled` is `true`
*/
projectId?: string;
Copy link
Contributor

@gsoldevila gsoldevila Jul 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

NIT I wonder if it should be the whole 'serverless' section that is optional instead.
Does it make sense to have a serverless: {} empty section?

Could the fact that the "serverless" property is defined be enough to determine that "serverless is enabled"? This way we would get rid of the isServerlessEnabled flag.

Serverless does not sound like something you can enable / disable, either you are or not on serverless, WDYT?

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 now it would make more sense. The thing is, we will likely add more serverless attributes to this section, and given their presence may be optional, it could complicate the thing (the same reason why the props are each individually nullable for ESS-related props)

gsoldevila
gsoldevila approved these changes Jul 14, 2023
View reviewed changes
Copy link
Contributor

@gsoldevila gsoldevila left a comment

Choose a reason for hiding this comment

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

LGTM!

@pgayvallet pgayvallet enabled auto-merge (squash) July 14, 2023 11:03
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #22 / Alerting builtin alertTypes es_query rule runs correctly and populates recovery context for searchSource search type

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
cloud 12 13 +1

Page load bundle

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

id before after diff
cloud 4.8KB 5.2KB +406.0B
Unknown metric groups

API count

id before after diff
cloud 54 61 +7

ESLint disabled line counts

id before after diff
enterpriseSearch 14 16 +2
securitySolution 411 415 +4
total +6

Total ESLint disabled count

id before after diff
enterpriseSearch 15 17 +2
securitySolution 490 494 +4
total +6

History

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

@pgayvallet pgayvallet merged commit a9786df into elastic:main Jul 14, 2023
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Jul 14, 2023
@gsoldevila gsoldevila mentioned this pull request Aug 10, 2023
Merged
This was referenced Sep 13, 2023
Closed
Merged
pgayvallet added a commit that referenced this pull request Sep 14, 2023
@pgayvallet
[cloud plugin] Add serverless projectName to configuration and contra…
b1bfe92 
…ct (#166330)

## 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sphilipse sphilipse sphilipse approved these changes

@juliaElastic juliaElastic juliaElastic approved these changes

@gsoldevila gsoldevila gsoldevila approved these changes

@azasypkin azasypkin azasypkin approved these changes

Assignees
No one assigned
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 Team:Fleet Team label for Observability Data Collection Fleet team v8.10.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add serverless projectId to cloud plugin's configuration
8 participants
@pgayvallet @elasticmachine @kibana-ci @azasypkin @gsoldevila @juliaElastic @sphilipse @kibanamachine