-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Changes from all commits
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 |
---|---|---|
|
@@ -72,6 +72,11 @@ export interface CloudStart { | |
* Will always be present if `isServerlessEnabled` is `true` | ||
*/ | ||
projectId?: string; | ||
/** | ||
* The serverless project name. | ||
* Will always be present if `isServerlessEnabled` is `true` | ||
*/ | ||
projectName?: string; | ||
Comment on lines
+75
to
+79
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. 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. |
||
}; | ||
} | ||
|
||
|
@@ -172,5 +177,10 @@ export interface CloudSetup { | |
* Will always be present if `isServerlessEnabled` is `true` | ||
*/ | ||
projectId?: string; | ||
/** | ||
* The serverless project name. | ||
* Will always be present if `isServerlessEnabled` is `true` | ||
*/ | ||
projectName?: string; | ||
}; | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
|
||
import { config } from './config'; | ||
|
||
describe('Cloud plugin config', () => { | ||
it('evicts unknown properties under the `serverless` structure', () => { | ||
const output = config.schema.validate({ | ||
serverless: { | ||
project_id: 'project_id', | ||
unknown_prop: 'some unknown prop', | ||
}, | ||
}); | ||
|
||
expect(output.serverless).toEqual({ | ||
project_id: 'project_id', | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,9 +33,14 @@ const configSchema = schema.object({ | |
trial_end_date: schema.maybe(schema.string()), | ||
is_elastic_staff_owned: schema.maybe(schema.boolean()), | ||
serverless: schema.maybe( | ||
schema.object({ | ||
project_id: schema.string(), | ||
}) | ||
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' } | ||
) | ||
Comment on lines
+36
to
+43
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. 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 |
||
), | ||
}); | ||
|
||
|
@@ -57,6 +62,7 @@ export const config: PluginConfigDescriptor<CloudConfigType> = { | |
is_elastic_staff_owned: true, | ||
serverless: { | ||
project_id: true, | ||
project_name: true, | ||
}, | ||
}, | ||
schema: configSchema, | ||
|
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.
@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.
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.
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.
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.
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