-
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
[UII] Restrict non-local ES output types for agentless integrations and policies #207296
Conversation
Pinging @elastic/fleet (Team:Fleet) |
…t --include-path /api/status --include-path /api/alerting/rule/ --include-path /api/alerting/rules --include-path /api/actions --include-path /api/security/role --include-path /api/spaces --include-path /api/fleet --include-path /api/dashboards --update'
…into feat/agentless-outputs
@@ -17,10 +17,7 @@ export const doesAgentPolicyAlreadyIncludePackage = ( | |||
agentPolicy: AgentPolicy, | |||
packageName: string | |||
): boolean => { | |||
if (!agentPolicy.package_policies) { |
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.
I think we may want to keep that one, sometime we retrieve agent policy without package_policies
so to be sure we pass the correct agent policy. In an ideal world it could be great to have two types one for AgentPolicy one for AgentPolicyWithPackagePolicies to avoid this kind of dynamic check
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.
I reverted this change, it was trying to fix an issue I found with switching between agent-based/agentless and new hosts/existing hosts, but I fixed it elsewhere
package_policies: [ | ||
{ | ||
package: { name: 'nginx' }, | ||
}, | ||
], | ||
} as any); | ||
|
||
expect(res).toContain('elasticsearch'); | ||
expect(res).toContain('logstash'); | ||
expect(res).toHaveLength(4); |
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.
nit: could be nice to test it contains all the expected
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.
updated
@@ -406,6 +406,15 @@ export const CreatePackagePolicySinglePage: CreatePackagePolicyParams = ({ | |||
handleSetupTechnologyChange(value); | |||
// agentless doesn't need system integration | |||
setWithSysMonitoring(value === SetupTechnology.AGENT_BASED); | |||
|
|||
// reset selected output if switching to agentless |
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 is not confusing for the user? especially as the order of these inputs in the UI is selecting output first before agentless right?
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.
I removed this and moved the resetting logic elsewhere that only triggers if the selected output is not compatible
const internalSoClientWithoutSpaceExtension = | ||
appContextService.getInternalUserSOClientWithoutSpaceExtension(); | ||
|
||
const agentlessPolicies = await agentPolicyService.list(internalSoClientWithoutSpaceExtension, { |
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.
shouldn't we query policies from all spaces like this? https://github.com/elastic/kibana/blob/main/x-pack/platform/plugins/shared/fleet/server/services/agent_policy.ts#L927-L938
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.
I checked with Nicolas and when we query with SO client directly, it should be using namespaces: ['*']
, but when using our services it should be with spaceId: '*'
here I am using agentPolicyService
with a non-scoped soClient so the query looks correct to me
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 property is named namespaces in the saved object, but we expose it and the rest of kibana (and documentation) expose it to the end user as space id, and to add to the confusion we have another concept of namespace in fleet policies
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.
LGTM 🚀
@@ -62,7 +62,6 @@ export const useAgentless = () => { | |||
export function useSetupTechnology({ | |||
setNewAgentPolicy, | |||
newAgentPolicy, | |||
updateAgentPolicies, |
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/cloud-security-posture probably this file and the related test file is what triggered codeowners review. I removed the triggering of updateAgentPolicies
, triggering setNewAgentPolicy
suffices here from my testing. it was causing issues when switching between agent-based/agentless while also switching between new hosts/existing hosts
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.
Thanks for the additional context!
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.
LGTM
@@ -62,7 +62,6 @@ export const useAgentless = () => { | |||
export function useSetupTechnology({ | |||
setNewAgentPolicy, | |||
newAgentPolicy, | |||
updateAgentPolicies, |
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.
Thanks for the additional context!
Starting backport for target branches: 8.x https://github.com/elastic/kibana/actions/runs/12940778494 |
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]Async chunks
Page load bundle
History
cc @jen-huang |
💔 All backports failed
Manual backportTo create the backport manually run:
Questions ?Please refer to the Backport tool documentation |
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
…nd policies (elastic#207296) ## Summary Resolves elastic#202090. Incidentally also fixes and unskips the failing test reported in elastic#203346. This PR makes it so that: - Agentless agent policies and package policies (those with `supports_agentless: true` set) cannot be directly set to use a non-local ES output for integration data - This restriction applies to UI and API level - When a non-local ES output would be updated to be the global Fleet default integration data output, existing agentless policies without an explicit output set will have their output directly set to the current default output ID - This is the same mechanism used today to ensure that Fleet Server and Synthetics integrations do not accidentally have their output set to non-ES as well ## Testing 1. Apply [patch](https://gist.github.com/jen-huang/dfc3e02ceb63976ad54bd1f50c524cb4) to skip actual agentless creation 2. Create different types of outputs in addition to default local ES 3. Enable beta integrations 4. Add an agentless integration, for example Box Connector, observe that outputs list only shows ES outputs: <img width="1422" alt="image" src="https://github.com/user-attachments/assets/72e43220-702f-4bb7-8e37-8be69aa4e6ea" /> 5. Switch to agent-based setup technology, observe that outputs list now shows all outputs 6. Create the agentless integration, go to its agent policy 7. Observe that outputs list only enables ES outputs: <img width="1425" alt="image" src="https://github.com/user-attachments/assets/3bc5985f-07bf-407a-8b62-4248b28904a5" /> 8. Play around with setting global default outputs, it should be not possible to get into a state where an agentless policy is using a non-local ES output ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md) - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <[email protected]> (cherry picked from commit d4cc532) # Conflicts: # oas_docs/bundle.json # oas_docs/bundle.serverless.json # oas_docs/output/kibana.yaml # x-pack/platform/plugins/shared/fleet/server/services/agent_policy.ts # x-pack/platform/plugins/shared/fleet/server/services/output.test.ts # x-pack/platform/plugins/shared/fleet/server/types/models/agent_policy.ts # x-pack/test/fleet_api_integration/apis/agent_policy/agent_policy.ts
…ions and policies (#207296) (#208131) # Backport This will backport the following commits from `main` to `8.x`: - [[UII] Restrict non-local ES output types for agentless integrations and policies (#207296)](#207296) <!--- Backport version: 9.6.4 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Jen Huang","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-01-24T00:32:42Z","message":"[UII] Restrict non-local ES output types for agentless integrations and policies (#207296)\n\n## Summary\r\n\r\nResolves #202090.\r\nIncidentally also fixes and unskips the failing test reported in\r\n#203346.\r\n\r\nThis PR makes it so that:\r\n\r\n- Agentless agent policies and package policies (those with\r\n`supports_agentless: true` set) cannot be directly set to use a\r\nnon-local ES output for integration data\r\n - This restriction applies to UI and API level\r\n- When a non-local ES output would be updated to be the global Fleet\r\ndefault integration data output, existing agentless policies without an\r\nexplicit output set will have their output directly set to the current\r\ndefault output ID\r\n- This is the same mechanism used today to ensure that Fleet Server and\r\nSynthetics integrations do not accidentally have their output set to\r\nnon-ES as well\r\n\r\n## Testing\r\n1. Apply\r\n[patch](https://gist.github.com/jen-huang/dfc3e02ceb63976ad54bd1f50c524cb4)\r\nto skip actual agentless creation\r\n2. Create different types of outputs in addition to default local ES\r\n3. Enable beta integrations\r\n4. Add an agentless integration, for example Box Connector, observe that\r\noutputs list only shows ES outputs:\r\n<img width=\"1422\" alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/72e43220-702f-4bb7-8e37-8be69aa4e6ea\"\r\n/>\r\n5. Switch to agent-based setup technology, observe that outputs list now\r\nshows all outputs\r\n6. Create the agentless integration, go to its agent policy\r\n7. Observe that outputs list only enables ES outputs:\r\n<img width=\"1425\" alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/3bc5985f-07bf-407a-8b62-4248b28904a5\"\r\n/>\r\n8. Play around with setting global default outputs, it should be not\r\npossible to get into a state where an agentless policy is using a\r\nnon-local ES output\r\n\r\n### Checklist\r\n\r\nCheck the PR satisfies following conditions. \r\n\r\nReviewers should verify this PR satisfies this list as well.\r\n\r\n- [x] Any text added follows [EUI's writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\r\nsentence case text and includes [i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n- [x] The PR description includes the appropriate Release Notes section,\r\nand the correct `release_note:*` label is applied per the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <[email protected]>","sha":"d4cc532919d97d6c36e0b12bdf08f5b0cd417cf5","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Fleet","v9.0.0","backport:prev-minor"],"title":"[UII] Restrict non-local ES output types for agentless integrations and policies","number":207296,"url":"https://github.com/elastic/kibana/pull/207296","mergeCommit":{"message":"[UII] Restrict non-local ES output types for agentless integrations and policies (#207296)\n\n## Summary\r\n\r\nResolves #202090.\r\nIncidentally also fixes and unskips the failing test reported in\r\n#203346.\r\n\r\nThis PR makes it so that:\r\n\r\n- Agentless agent policies and package policies (those with\r\n`supports_agentless: true` set) cannot be directly set to use a\r\nnon-local ES output for integration data\r\n - This restriction applies to UI and API level\r\n- When a non-local ES output would be updated to be the global Fleet\r\ndefault integration data output, existing agentless policies without an\r\nexplicit output set will have their output directly set to the current\r\ndefault output ID\r\n- This is the same mechanism used today to ensure that Fleet Server and\r\nSynthetics integrations do not accidentally have their output set to\r\nnon-ES as well\r\n\r\n## Testing\r\n1. Apply\r\n[patch](https://gist.github.com/jen-huang/dfc3e02ceb63976ad54bd1f50c524cb4)\r\nto skip actual agentless creation\r\n2. Create different types of outputs in addition to default local ES\r\n3. Enable beta integrations\r\n4. Add an agentless integration, for example Box Connector, observe that\r\noutputs list only shows ES outputs:\r\n<img width=\"1422\" alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/72e43220-702f-4bb7-8e37-8be69aa4e6ea\"\r\n/>\r\n5. Switch to agent-based setup technology, observe that outputs list now\r\nshows all outputs\r\n6. Create the agentless integration, go to its agent policy\r\n7. Observe that outputs list only enables ES outputs:\r\n<img width=\"1425\" alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/3bc5985f-07bf-407a-8b62-4248b28904a5\"\r\n/>\r\n8. Play around with setting global default outputs, it should be not\r\npossible to get into a state where an agentless policy is using a\r\nnon-local ES output\r\n\r\n### Checklist\r\n\r\nCheck the PR satisfies following conditions. \r\n\r\nReviewers should verify this PR satisfies this list as well.\r\n\r\n- [x] Any text added follows [EUI's writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\r\nsentence case text and includes [i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n- [x] The PR description includes the appropriate Release Notes section,\r\nand the correct `release_note:*` label is applied per the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <[email protected]>","sha":"d4cc532919d97d6c36e0b12bdf08f5b0cd417cf5"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/207296","number":207296,"mergeCommit":{"message":"[UII] Restrict non-local ES output types for agentless integrations and policies (#207296)\n\n## Summary\r\n\r\nResolves #202090.\r\nIncidentally also fixes and unskips the failing test reported in\r\n#203346.\r\n\r\nThis PR makes it so that:\r\n\r\n- Agentless agent policies and package policies (those with\r\n`supports_agentless: true` set) cannot be directly set to use a\r\nnon-local ES output for integration data\r\n - This restriction applies to UI and API level\r\n- When a non-local ES output would be updated to be the global Fleet\r\ndefault integration data output, existing agentless policies without an\r\nexplicit output set will have their output directly set to the current\r\ndefault output ID\r\n- This is the same mechanism used today to ensure that Fleet Server and\r\nSynthetics integrations do not accidentally have their output set to\r\nnon-ES as well\r\n\r\n## Testing\r\n1. Apply\r\n[patch](https://gist.github.com/jen-huang/dfc3e02ceb63976ad54bd1f50c524cb4)\r\nto skip actual agentless creation\r\n2. Create different types of outputs in addition to default local ES\r\n3. Enable beta integrations\r\n4. Add an agentless integration, for example Box Connector, observe that\r\noutputs list only shows ES outputs:\r\n<img width=\"1422\" alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/72e43220-702f-4bb7-8e37-8be69aa4e6ea\"\r\n/>\r\n5. Switch to agent-based setup technology, observe that outputs list now\r\nshows all outputs\r\n6. Create the agentless integration, go to its agent policy\r\n7. Observe that outputs list only enables ES outputs:\r\n<img width=\"1425\" alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/3bc5985f-07bf-407a-8b62-4248b28904a5\"\r\n/>\r\n8. Play around with setting global default outputs, it should be not\r\npossible to get into a state where an agentless policy is using a\r\nnon-local ES output\r\n\r\n### Checklist\r\n\r\nCheck the PR satisfies following conditions. \r\n\r\nReviewers should verify this PR satisfies this list as well.\r\n\r\n- [x] Any text added follows [EUI's writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\r\nsentence case text and includes [i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n- [x] The PR description includes the appropriate Release Notes section,\r\nand the correct `release_note:*` label is applied per the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <[email protected]>","sha":"d4cc532919d97d6c36e0b12bdf08f5b0cd417cf5"}}]}] BACKPORT--> --------- Co-authored-by: kibanamachine <[email protected]>
…nd policies (elastic#207296) ## Summary Resolves elastic#202090. Incidentally also fixes and unskips the failing test reported in elastic#203346. This PR makes it so that: - Agentless agent policies and package policies (those with `supports_agentless: true` set) cannot be directly set to use a non-local ES output for integration data - This restriction applies to UI and API level - When a non-local ES output would be updated to be the global Fleet default integration data output, existing agentless policies without an explicit output set will have their output directly set to the current default output ID - This is the same mechanism used today to ensure that Fleet Server and Synthetics integrations do not accidentally have their output set to non-ES as well ## Testing 1. Apply [patch](https://gist.github.com/jen-huang/dfc3e02ceb63976ad54bd1f50c524cb4) to skip actual agentless creation 2. Create different types of outputs in addition to default local ES 3. Enable beta integrations 4. Add an agentless integration, for example Box Connector, observe that outputs list only shows ES outputs: <img width="1422" alt="image" src="https://github.com/user-attachments/assets/72e43220-702f-4bb7-8e37-8be69aa4e6ea" /> 5. Switch to agent-based setup technology, observe that outputs list now shows all outputs 6. Create the agentless integration, go to its agent policy 7. Observe that outputs list only enables ES outputs: <img width="1425" alt="image" src="https://github.com/user-attachments/assets/3bc5985f-07bf-407a-8b62-4248b28904a5" /> 8. Play around with setting global default outputs, it should be not possible to get into a state where an agentless policy is using a non-local ES output ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md) - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <[email protected]>
Summary
Resolves #202090.
Incidentally also fixes and unskips the failing test reported in #203346.
This PR makes it so that:
supports_agentless: true
set) cannot be directly set to use a non-local ES output for integration dataTesting
Checklist
Check the PR satisfies following conditions.
Reviewers should verify this PR satisfies this list as well.
release_note:*
label is applied per the guidelines