-
Notifications
You must be signed in to change notification settings - Fork 8.5k
[Synthetics] Clean up extra synthetics package policies !! #235200
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
[Synthetics] Clean up extra synthetics package policies !! #235200
Conversation
ada5a86 to
46acf7d
Compare
| encryptedSavedObjects: EncryptedSavedObjectsPluginStart; | ||
| }) { | ||
| const { privateLocationAPI } = this.syntheticsMonitorClient; | ||
| const privateConfigs: Array<{ |
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.
@shahzad31 As far as I understand you fixed synthetics policies being created in multiple places in this PR. With this change you undo the fix. Could it be that you pushed this change by mistake in order to create a broken environment with duplicated 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 reapplied the fix in this commit. This change fixes Synthetics API Tests SyncGlobalParamsSpaces number of package policies matches number of monitors failing tests. Will look into the rest ones
|
@elasticmachine merge upstream |
csr
left a comment
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.
AppEx QA codeowner changes LGTM 👍
|
Pinging @elastic/obs-ux-management-team (Team:obs-ux-management) |
|
Pinging @elastic/fleet (Team:Fleet) |
|
@shahzad31 I tested it and here are the results: Before
After
I confirm that the extra policy was successfully deleted. Is it ok though that only one of the automatically generated Another observation I have is that the 2nd space (test) doesn't list any policies under Fleet > Agent policies (this is also on main, nothing to do with the clean up task)
|
| user?: AuthenticatedUser; | ||
| skipUnassignFromAgentPolicies?: boolean; | ||
| force?: boolean; | ||
| spaceIds?: string[]; |
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 am wondering if we could use the same pattern as getByIds
ids: Array<
| string
| {
id: string;
/**
* The space id associated with the agent policy. When using an un-scoped SO client, the
* space id can be set to `*` which will look for that agent policy across all spaces
*/
spaceId?: string;
}
>,
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 updated types
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.
Looks like this ask is in reference to the getByIds definition on AgentPolicyService
https://github.com/elastic/kibana/blob/main/x-pack/platform/plugins/shared/fleet/server/services/agent_policy.ts#L719-L726
That said, the PackagePolicyClientImpl.getByIDs follows a different pattern, which this PR adopts, using the WithSpaceIdsOption interface, which is also used in other parts of this client.
65d2cc4 to
03955be
Compare
x-pack/solutions/observability/plugins/synthetics/server/routes/tasks/trigger_task_run.ts
Outdated
Show resolved
Hide resolved
💔 Build Failed
Failed CI StepsHistory
|
...utions/observability/plugins/synthetics/server/tasks/sync_private_locations_monitors_task.ts
Show resolved
Hide resolved
...utions/observability/plugins/synthetics/server/tasks/sync_private_locations_monitors_task.ts
Show resolved
Hide resolved
dominiqueclarke
left a comment
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 🚢
MichelLosier
left a comment
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.
Looks good!
With regards to this ask: #235200 (review)
Looks like the changes in this PR are consistent with other parts of the client, if we wanted to adopt later the pattern that was suggested, the changes here would not prevent that. So not a blocker for ✅
|
Starting backport for target branches: 8.19, 9.1 |
💔 All backports failed
Manual backportTo create the backport manually run: Questions ?Please refer to the Backport tool documentation |
💔 Some backports could not be created
Note: Successful backport PRs will be merged automatically after passing CI. Manual backportTo create the backport manually run: Questions ?Please refer to the Backport tool documentation |
…35200) ## Summary Clean up extra synthetics package policies created via elastic#235021 !! ### Clean up There are three basic tests for clean up 1. SpaceIds being added in a processor where there should only be one 2. Multiple package policies exists for same configId and locationId 3. ConfigId is missing altogether from monitors for a package policy The bug meant multiple spaceIds were being added in processor and also if multiple package policies exists for a locationId and configId, that also passes the test, so we clean up those policies. ### Testing Revert changes in PR elastic#235021, it will create extra package policies once you have monitors in multiple places, switching to this branch should clean up those. --------- Co-authored-by: kibanamachine <[email protected]> (cherry picked from commit ced929a) # Conflicts: # x-pack/solutions/observability/plugins/synthetics/server/tasks/sync_private_locations_monitors_task.test.ts # x-pack/solutions/observability/plugins/synthetics/server/tasks/sync_private_locations_monitors_task.ts # x-pack/solutions/observability/test/api_integration_deployment_agnostic/services/synthetics_private_location.ts
…35200) ## Summary Clean up extra synthetics package policies created via elastic#235021 !! ### Clean up There are three basic tests for clean up 1. SpaceIds being added in a processor where there should only be one 2. Multiple package policies exists for same configId and locationId 3. ConfigId is missing altogether from monitors for a package policy The bug meant multiple spaceIds were being added in processor and also if multiple package policies exists for a locationId and configId, that also passes the test, so we clean up those policies. ### Testing Revert changes in PR elastic#235021, it will create extra package policies once you have monitors in multiple places, switching to this branch should clean up those. --------- Co-authored-by: kibanamachine <[email protected]> (cherry picked from commit ced929a) # Conflicts: # x-pack/platform/plugins/shared/fleet/server/services/package_policy.ts # x-pack/platform/plugins/shared/fleet/server/services/package_policy_service.ts # x-pack/solutions/observability/plugins/synthetics/server/tasks/sync_private_locations_monitors_task.test.ts # x-pack/solutions/observability/plugins/synthetics/server/tasks/sync_private_locations_monitors_task.ts # x-pack/solutions/observability/test/api_integration_deployment_agnostic/apis/synthetics/index.ts # x-pack/solutions/observability/test/api_integration_deployment_agnostic/services/synthetics_private_location.ts
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. 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 |
…35200) ## Summary Clean up extra synthetics package policies created via elastic#235021 !! ### Clean up There are three basic tests for clean up 1. SpaceIds being added in a processor where there should only be one 2. Multiple package policies exists for same configId and locationId 3. ConfigId is missing altogether from monitors for a package policy The bug meant multiple spaceIds were being added in processor and also if multiple package policies exists for a locationId and configId, that also passes the test, so we clean up those policies. ### Testing Revert changes in PR elastic#235021, it will create extra package policies once you have monitors in multiple places, switching to this branch should clean up those. --------- Co-authored-by: kibanamachine <[email protected]> (cherry picked from commit ced929a) # Conflicts: # x-pack/platform/plugins/shared/fleet/server/services/package_policy.ts # x-pack/platform/plugins/shared/fleet/server/services/package_policy_service.ts # x-pack/solutions/observability/plugins/synthetics/server/tasks/sync_private_locations_monitors_task.test.ts # x-pack/solutions/observability/plugins/synthetics/server/tasks/sync_private_locations_monitors_task.ts # x-pack/solutions/observability/test/api_integration_deployment_agnostic/apis/synthetics/index.ts # x-pack/solutions/observability/test/api_integration_deployment_agnostic/services/synthetics_private_location.ts
…5200) (#237032) # Backport This will backport the following commits from `main` to `9.1`: - [[Synthetics] Clean up extra synthetics package policies !! (#235200)](#235200) <!--- Backport version: 10.0.2 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Shahzad","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-09-30T19:46:07Z","message":"[Synthetics] Clean up extra synthetics package policies !! (#235200)\n\n## Summary\n\nClean up extra synthetics package policies created via\nhttps://github.com//pull/235021 !!\n\n### Clean up\n\nThere are three basic tests for clean up\n\n1. SpaceIds being added in a processor where there should only be one \n2. Multiple package policies exists for same configId and locationId\n3. ConfigId is missing altogether from monitors for a package policy \n\nThe bug meant multiple spaceIds were being added in processor and also\nif multiple package policies exists for a locationId and configId, that\nalso passes the test, so we clean up those policies.\n\n\n### Testing \n\nRevert changes in PR #235021, it\nwill create extra package policies once you have monitors in multiple\nplaces, switching to this branch should clean up those.\n\n---------\n\nCo-authored-by: kibanamachine <[email protected]>","sha":"ced929a22bff204846cad448d3603f517cdd240f","branchLabelMapping":{"^v9.2.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Fleet","Team:obs-ux-management","backport:version","author:obs-ux-management","v9.2.0","v8.19.5","v9.1.5"],"title":"[Synthetics] Clean up extra synthetics package policies !!","number":235200,"url":"https://github.com/elastic/kibana/pull/235200","mergeCommit":{"message":"[Synthetics] Clean up extra synthetics package policies !! (#235200)\n\n## Summary\n\nClean up extra synthetics package policies created via\nhttps://github.com//pull/235021 !!\n\n### Clean up\n\nThere are three basic tests for clean up\n\n1. SpaceIds being added in a processor where there should only be one \n2. Multiple package policies exists for same configId and locationId\n3. ConfigId is missing altogether from monitors for a package policy \n\nThe bug meant multiple spaceIds were being added in processor and also\nif multiple package policies exists for a locationId and configId, that\nalso passes the test, so we clean up those policies.\n\n\n### Testing \n\nRevert changes in PR #235021, it\nwill create extra package policies once you have monitors in multiple\nplaces, switching to this branch should clean up those.\n\n---------\n\nCo-authored-by: kibanamachine <[email protected]>","sha":"ced929a22bff204846cad448d3603f517cdd240f"}},"sourceBranch":"main","suggestedTargetBranches":["8.19","9.1"],"targetPullRequestStates":[{"branch":"main","label":"v9.2.0","branchLabelMappingKey":"^v9.2.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/235200","number":235200,"mergeCommit":{"message":"[Synthetics] Clean up extra synthetics package policies !! (#235200)\n\n## Summary\n\nClean up extra synthetics package policies created via\nhttps://github.com//pull/235021 !!\n\n### Clean up\n\nThere are three basic tests for clean up\n\n1. SpaceIds being added in a processor where there should only be one \n2. Multiple package policies exists for same configId and locationId\n3. ConfigId is missing altogether from monitors for a package policy \n\nThe bug meant multiple spaceIds were being added in processor and also\nif multiple package policies exists for a locationId and configId, that\nalso passes the test, so we clean up those policies.\n\n\n### Testing \n\nRevert changes in PR #235021, it\nwill create extra package policies once you have monitors in multiple\nplaces, switching to this branch should clean up those.\n\n---------\n\nCo-authored-by: kibanamachine <[email protected]>","sha":"ced929a22bff204846cad448d3603f517cdd240f"}},{"branch":"8.19","label":"v8.19.5","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"9.1","label":"v9.1.5","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> --------- Co-authored-by: Shahzad <[email protected]>
…35200) (#237050) # Backport This will backport the following commits from `main` to `8.19`: - [[Synthetics] Clean up extra synthetics package policies !! (#235200)](#235200) <!--- Backport version: 10.0.2 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Shahzad","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-09-30T19:46:07Z","message":"[Synthetics] Clean up extra synthetics package policies !! (#235200)\n\n## Summary\n\nClean up extra synthetics package policies created via\nhttps://github.com//pull/235021 !!\n\n### Clean up\n\nThere are three basic tests for clean up\n\n1. SpaceIds being added in a processor where there should only be one \n2. Multiple package policies exists for same configId and locationId\n3. ConfigId is missing altogether from monitors for a package policy \n\nThe bug meant multiple spaceIds were being added in processor and also\nif multiple package policies exists for a locationId and configId, that\nalso passes the test, so we clean up those policies.\n\n\n### Testing \n\nRevert changes in PR #235021, it\nwill create extra package policies once you have monitors in multiple\nplaces, switching to this branch should clean up those.\n\n---------\n\nCo-authored-by: kibanamachine <[email protected]>","sha":"ced929a22bff204846cad448d3603f517cdd240f","branchLabelMapping":{"^v9.2.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Fleet","Team:obs-ux-management","backport:version","author:obs-ux-management","v9.2.0","v8.19.5","v9.1.5"],"title":"[Synthetics] Clean up extra synthetics package policies !!","number":235200,"url":"https://github.com/elastic/kibana/pull/235200","mergeCommit":{"message":"[Synthetics] Clean up extra synthetics package policies !! (#235200)\n\n## Summary\n\nClean up extra synthetics package policies created via\nhttps://github.com//pull/235021 !!\n\n### Clean up\n\nThere are three basic tests for clean up\n\n1. SpaceIds being added in a processor where there should only be one \n2. Multiple package policies exists for same configId and locationId\n3. ConfigId is missing altogether from monitors for a package policy \n\nThe bug meant multiple spaceIds were being added in processor and also\nif multiple package policies exists for a locationId and configId, that\nalso passes the test, so we clean up those policies.\n\n\n### Testing \n\nRevert changes in PR #235021, it\nwill create extra package policies once you have monitors in multiple\nplaces, switching to this branch should clean up those.\n\n---------\n\nCo-authored-by: kibanamachine <[email protected]>","sha":"ced929a22bff204846cad448d3603f517cdd240f"}},"sourceBranch":"main","suggestedTargetBranches":["8.19"],"targetPullRequestStates":[{"branch":"main","label":"v9.2.0","branchLabelMappingKey":"^v9.2.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/235200","number":235200,"mergeCommit":{"message":"[Synthetics] Clean up extra synthetics package policies !! (#235200)\n\n## Summary\n\nClean up extra synthetics package policies created via\nhttps://github.com//pull/235021 !!\n\n### Clean up\n\nThere are three basic tests for clean up\n\n1. SpaceIds being added in a processor where there should only be one \n2. Multiple package policies exists for same configId and locationId\n3. ConfigId is missing altogether from monitors for a package policy \n\nThe bug meant multiple spaceIds were being added in processor and also\nif multiple package policies exists for a locationId and configId, that\nalso passes the test, so we clean up those policies.\n\n\n### Testing \n\nRevert changes in PR #235021, it\nwill create extra package policies once you have monitors in multiple\nplaces, switching to this branch should clean up those.\n\n---------\n\nCo-authored-by: kibanamachine <[email protected]>","sha":"ced929a22bff204846cad448d3603f517cdd240f"}},{"branch":"8.19","label":"v8.19.5","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"9.1","label":"v9.1.5","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/237032","number":237032,"state":"OPEN"}]}] BACKPORT--> --------- Co-authored-by: Shahzad <[email protected]>
…35200) ## Summary Clean up extra synthetics package policies created via elastic#235021 !! ### Clean up There are three basic tests for clean up 1. SpaceIds being added in a processor where there should only be one 2. Multiple package policies exists for same configId and locationId 3. ConfigId is missing altogether from monitors for a package policy The bug meant multiple spaceIds were being added in processor and also if multiple package policies exists for a locationId and configId, that also passes the test, so we clean up those policies. ### Testing Revert changes in PR elastic#235021, it will create extra package policies once you have monitors in multiple places, switching to this branch should clean up those. --------- Co-authored-by: kibanamachine <[email protected]>



Summary
Clean up extra synthetics package policies created via #235021 !!
Clean up
There are three basic tests for clean up
The bug meant multiple spaceIds were being added in processor and also if multiple package policies exists for a locationId and configId, that also passes the test, so we clean up those policies.
Testing
Revert changes in PR #235021, it will create extra package policies once you have monitors in multiple places, switching to this branch should clean up those.