-
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
[Ingest Manager] support multiple kibana urls #75712
[Ingest Manager] support multiple kibana urls #75712
Conversation
21e41f1
to
543f456
Compare
ebe2dfb
to
c71e883
Compare
Pinging @elastic/ingest-management (Team:Ingest Management) |
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.
Tested and it work as expected
There is maybe just one think that we can improve (can be done in a follow up PR)
I think the config yaml flyout in the policy details should show the standalone config and not the full one, (it's here https://github.com/neptunian/kibana/blob/72731-allow-multiple-kibana-urls/x-pack/plugins/ingest_manager/public/applications/ingest_manager/sections/agent_policy/components/agent_policy_yaml_flyout.tsx/#L36)
…cies when revision is bumped
@@ -186,6 +196,11 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { | |||
type: 'elasticsearch', | |||
}, | |||
}, | |||
fleet: { |
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.
These look fine to me. @paul-tavares do we need to make changes in the UI code that creates the policy?
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.
@jonathan-buttner no, I don't think this impacts the Endpoint Integration Policy because its outside of the Integration input
- that's the only one we manipulate in the Endpoint Policy Details page.
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.
Security changes look good.
…ana_urls: string[] and add migration
}, | ||
Settings | ||
> = (settingsDoc) => { | ||
settingsDoc.attributes.kibana_urls = [settingsDoc.attributes.kibana_url]; |
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.
is it possible for the old settingsDoc to have undefined
for settingsDoc.attributes.kibana_url
? if so, we may need to wrap this line in a conditional so that we don't set [undefined]
as a value for the new doc
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.
It's only possible for there to be no settings if they haven't started kibana. Settings along with the kibana_url gets created during setup and they cannot remove it through the settings api
...nager/public/applications/ingest_manager/components/enrollment_instructions/manual/index.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/ingest_manager/public/applications/ingest_manager/components/settings_flyout.tsx
Outdated
Show resolved
Hide resolved
GLOBAL_SETTINGS_SAVED_OBJECT_TYPE, | ||
newData | ||
); | ||
const defaultSettings = createDefaultSettings(); |
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.
@nchaulet since during setup we create the Settings saved object with default attributes, I moved those into a function and they will be added when the saved object (if missing for some reason) is created here as well.
💚 Build SucceededBuild metrics@kbn/optimizer bundle module count
async chunks size
page load bundle size
distributable file count
History
To update your PR or re-run it, just comment with: |
* let the user specify multiple kibana urls * add validation to kibana urls so paths and protocols cannot differ * update i18n message * only send the first url to the instructions * udpate all agent configs' revision when settings is updated * fix jest test * update endpoint full agent policy test * fix type * dont add settings if standalone mode * fix ui not handling errors from /{agentPolicyId}/full endpoint * fix formatted message id * only return needed fields * fill in updated_by and updated_at attributes of the ingest-agent-policies when revision is bumped * throw error if kibana_urls not set and update tests * change ingest_manager_settings SO attribute kibana_url: string to kibana_urls: string[] and add migration * leave instructions single kibana url * make kibana_url and other attributes created during setup required, fix types
* let the user specify multiple kibana urls * add validation to kibana urls so paths and protocols cannot differ * update i18n message * only send the first url to the instructions * udpate all agent configs' revision when settings is updated * fix jest test * update endpoint full agent policy test * fix type * dont add settings if standalone mode * fix ui not handling errors from /{agentPolicyId}/full endpoint * fix formatted message id * only return needed fields * fill in updated_by and updated_at attributes of the ingest-agent-policies when revision is bumped * throw error if kibana_urls not set and update tests * change ingest_manager_settings SO attribute kibana_url: string to kibana_urls: string[] and add migration * leave instructions single kibana url * make kibana_url and other attributes created during setup required, fix types
Hi @EricDavisX We have validated the ticket by setting up 03 different 7.10.0-SNAPSHOT Kibana cloud environments. Steps Followed:
Observations:
Moreover, we have created and executed 05 testcases under Support multiple kibana urls TestRun. Queries:
As per #75712 description under Test section and our understanding, error message should be displayed for the same. Please share your feedback.
(ii) On executing the POST /ingest_manager/settings ES query under Management->Dev Tools->Console, following error message is displayed. However as per #75712 description under Test section, all agent policy saved objects revision should be updated on executing PUT ES query . Please share your feedback.
Please share your feedback.
(vi) Enroll the agent on Windows10 using above command. Observation: Please let us know if it is the expected behavior.
(a)Error is displayed on adding following string url setting in kibana.yml file on cloud environment: (b) Error is displayed on adding following string array of urls setting in kibana yml file on cloud environment:- (c) Error is displayed on adding suggested host.0 setting in kibana yml file on cloud environment::- Please let us know if anything is missing from our end. |
@rahulgupta-qasource I'm sending you a DM, lets talk testing in a new test ticket instead of this closed pr. Thanks for kicking this off! |
#72731
Support letting the user have multiple kibana urls in their global Settings and notify the agent when they change.
Changes
/settings
endpoint that paths and protocol or a list of kibana urls cannot differingest_manager_settings
saved object'skibana_url
attribute is now an array of strings typePUT /api/ingest_manager/settings
was changed from allowing only a string url to allowing only an array of string urls. GET always returnskibana_url
as an array of strings.xpack.ingestManager.fleet.kibana.host
now accepts both a string url or an array of string urls.PUT /ingest_manager/settings
, regardless of what was updated, will bump all agent configuration saved objects revision, triggering a CONFIG_CHANGE action/{agentPolicyId}/full endpoint
Before:
After:
Test
xpack.ingestManager.fleet.kibana.host: ["http://localhost:5601/", "https://localhost:5603/"]
PUT /ingest_manager/settings
endpoint, all agent policy saved objects revision should be updated. If an agent is running, notice CONFIG_CHANGE action after a settings update. Also try to to updatekibana_url
with mismatched paths or protocol through this endpoint. It should fail.Note
kibana_url
but we should probably change it tokibana_urls
. Would like to do this change and SO migration in a separate PR.