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.

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] Fixes partial updates for params and params viewing #195866

Merged
merged 19 commits into from
Oct 25, 2024

Conversation

shahzad31
Copy link
Contributor

@shahzad31 shahzad31 commented Oct 11, 2024

Summary

Fixes #167781

In docs we says that only key/value pairs are required, but in actual edit, that means rest of the data was being lost on edits

Allow partial updates to params edit API !!

This PR makes sure prev objects is fetched and merged with new data hence allowing partial updates !!

We are also allowing the ability to view value of the secret once it's saved via API !!

Value is hidden

Param value will not be visible unless user is super_user or kibana_admin, though user can assign new value.

Releae notes

Only user with asuper_user or kibana_admin will be able to view existing param values !!

@shahzad31 shahzad31 requested a review from a team as a code owner October 11, 2024 09:07
@botelastic botelastic bot added ci:project-deploy-observability Create an Observability project Team:obs-ux-management Observability Management User Experience Team labels Oct 11, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-management-team (Team:obs-ux-management)

@shahzad31 shahzad31 changed the title [Synthetics] Fixes partial updates !! [Synthetics] Fixes partial updates for params !! Oct 11, 2024
@shahzad31 shahzad31 added backport:prev-major Backport to (8.x, 8.17, 8.16) the previous major branch and other branches in development release_note:skip Skip the PR/issue when compiling release notes labels Oct 11, 2024
Copy link
Contributor

@dominiqueclarke dominiqueclarke left a comment

Choose a reason for hiding this comment

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

Could you add a single api integration test for this? Otherwise, LGTM.

Copy link
Contributor

@justinkambic justinkambic left a comment

Choose a reason for hiding this comment

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

Leaving a code review first, will smoke test the feature and respond again.

@@ -277,5 +336,22 @@ export default function ({ getService }: FtrProviderContext) {
expect(getResponse.body[0].namespaces).eql(['*']);
assertHas(getResponse.body[0], testParam);
});

it('should not return values for non admin user', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why aren't we allowing viewing for Synthetics editors any longer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we are also fixing this PR #167781

@shahzad31 shahzad31 changed the title [Synthetics] Fixes partial updates for params !! [Synthetics] Fixes partial updates for params and params viewing Oct 21, 2024
Copy link
Contributor

@dominiqueclarke dominiqueclarke left a comment

Choose a reason for hiding this comment

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

LGTM

@elasticmachine
Copy link
Contributor

⏳ Build in-progress

  • Buildkite Build
  • Commit: 2a80569
  • Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-195866-2a80569a4a66

History

@shahzad31 shahzad31 merged commit 0ff9a8a into elastic:main Oct 25, 2024
27 checks passed
@shahzad31 shahzad31 deleted the params-edit branch October 25, 2024 15:06
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.15, 8.16, 8.x

https://github.com/elastic/kibana/actions/runs/11520738897

@shahzad31 shahzad31 added backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) and removed v9.0.0 backport:prev-major Backport to (8.x, 8.17, 8.16) the previous major branch and other branches in development labels Oct 25, 2024
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11520744579

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 25, 2024
…stic#195866)

## Summary

Fixes elastic#167781

In docs we says that only key/value pairs are required, but in actual
edit, that means rest of the data was being lost on edits

Allow partial updates to params edit API !!

This PR makes sure prev objects is fetched and merged with new data
hence allowing partial updates !!

We are also allowing the ability to view value of the secret once it's
saved via API !!

### Value is hidden
Param value will not be visible unless user is `super_user` or
`kibana_admin`, though user can assign new value.

---------

Co-authored-by: Justin Kambic <[email protected]>
(cherry picked from commit 0ff9a8a)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Oct 25, 2024
#195866) (#197850)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Synthetics] Fixes partial updates for params and params viewing
(#195866)](#195866)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT
[{"author":{"name":"Shahzad","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-25T15:06:52Z","message":"[Synthetics]
Fixes partial updates for params and params viewing (#195866)\n\n##
Summary\r\n\r\nFixes
https://github.com/elastic/kibana/issues/167781\r\n\r\nIn docs we says
that only key/value pairs are required, but in actual\r\nedit, that
means rest of the data was being lost on edits\r\n\r\nAllow partial
updates to params edit API !!\r\n\r\nThis PR makes sure prev objects is
fetched and merged with new data\r\nhence allowing partial updates
!!\r\n\r\nWe are also allowing the ability to view value of the secret
once it's\r\nsaved via API !!\r\n\r\n### Value is hidden\r\nParam value
will not be visible unless user is `super_user` or\r\n`kibana_admin`,
though user can assign new
value.\r\n\r\n---------\r\n\r\nCo-authored-by: Justin Kambic
<[email protected]>","sha":"0ff9a8a9d9ff2bdb99562eeca29152bd0a0c4385","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","backport:prev-minor","ci:project-deploy-observability","Team:obs-ux-management"],"title":"[Synthetics]
Fixes partial updates for params and params
viewing","number":195866,"url":"https://github.com/elastic/kibana/pull/195866","mergeCommit":{"message":"[Synthetics]
Fixes partial updates for params and params viewing (#195866)\n\n##
Summary\r\n\r\nFixes
https://github.com/elastic/kibana/issues/167781\r\n\r\nIn docs we says
that only key/value pairs are required, but in actual\r\nedit, that
means rest of the data was being lost on edits\r\n\r\nAllow partial
updates to params edit API !!\r\n\r\nThis PR makes sure prev objects is
fetched and merged with new data\r\nhence allowing partial updates
!!\r\n\r\nWe are also allowing the ability to view value of the secret
once it's\r\nsaved via API !!\r\n\r\n### Value is hidden\r\nParam value
will not be visible unless user is `super_user` or\r\n`kibana_admin`,
though user can assign new
value.\r\n\r\n---------\r\n\r\nCo-authored-by: Justin Kambic
<[email protected]>","sha":"0ff9a8a9d9ff2bdb99562eeca29152bd0a0c4385"}},"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/195866","number":195866,"mergeCommit":{"message":"[Synthetics]
Fixes partial updates for params and params viewing (#195866)\n\n##
Summary\r\n\r\nFixes
https://github.com/elastic/kibana/issues/167781\r\n\r\nIn docs we says
that only key/value pairs are required, but in actual\r\nedit, that
means rest of the data was being lost on edits\r\n\r\nAllow partial
updates to params edit API !!\r\n\r\nThis PR makes sure prev objects is
fetched and merged with new data\r\nhence allowing partial updates
!!\r\n\r\nWe are also allowing the ability to view value of the secret
once it's\r\nsaved via API !!\r\n\r\n### Value is hidden\r\nParam value
will not be visible unless user is `super_user` or\r\n`kibana_admin`,
though user can assign new
value.\r\n\r\n---------\r\n\r\nCo-authored-by: Justin Kambic
<[email protected]>","sha":"0ff9a8a9d9ff2bdb99562eeca29152bd0a0c4385"}}]}]
BACKPORT-->

Co-authored-by: Shahzad <[email protected]>
shahzad31 added a commit to shahzad31/kibana that referenced this pull request Oct 30, 2024
…stic#195866)

## Summary

Fixes elastic#167781

In docs we says that only key/value pairs are required, but in actual
edit, that means rest of the data was being lost on edits

Allow partial updates to params edit API !!

This PR makes sure prev objects is fetched and merged with new data
hence allowing partial updates !!

We are also allowing the ability to view value of the secret once it's
saved via API !!

### Value is hidden
Param value will not be visible unless user is `super_user` or
`kibana_admin`, though user can assign new value.

---------

Co-authored-by: Justin Kambic <[email protected]>
(cherry picked from commit 0ff9a8a)
@shahzad31
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
8.16

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

shahzad31 added a commit that referenced this pull request Nov 1, 2024
…ng (#195866) (#198284)

# Backport

This will backport the following commits from `main` to `8.16`:
- [[Synthetics] Fixes partial updates for params and params viewing
(#195866)](#195866)

<!--- Backport version: 8.9.8 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT
[{"author":{"name":"Shahzad","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-25T15:06:52Z","message":"[Synthetics]
Fixes partial updates for params and params viewing (#195866)\n\n##
Summary\r\n\r\nFixes
https://github.com/elastic/kibana/issues/167781\r\n\r\nIn docs we says
that only key/value pairs are required, but in actual\r\nedit, that
means rest of the data was being lost on edits\r\n\r\nAllow partial
updates to params edit API !!\r\n\r\nThis PR makes sure prev objects is
fetched and merged with new data\r\nhence allowing partial updates
!!\r\n\r\nWe are also allowing the ability to view value of the secret
once it's\r\nsaved via API !!\r\n\r\n### Value is hidden\r\nParam value
will not be visible unless user is `super_user` or\r\n`kibana_admin`,
though user can assign new
value.\r\n\r\n---------\r\n\r\nCo-authored-by: Justin Kambic
<[email protected]>","sha":"0ff9a8a9d9ff2bdb99562eeca29152bd0a0c4385","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","backport:prev-minor","ci:project-deploy-observability","Team:obs-ux-management","v8.16.0","v8.17.0"],"number":195866,"url":"https://github.com/elastic/kibana/pull/195866","mergeCommit":{"message":"[Synthetics]
Fixes partial updates for params and params viewing (#195866)\n\n##
Summary\r\n\r\nFixes
https://github.com/elastic/kibana/issues/167781\r\n\r\nIn docs we says
that only key/value pairs are required, but in actual\r\nedit, that
means rest of the data was being lost on edits\r\n\r\nAllow partial
updates to params edit API !!\r\n\r\nThis PR makes sure prev objects is
fetched and merged with new data\r\nhence allowing partial updates
!!\r\n\r\nWe are also allowing the ability to view value of the secret
once it's\r\nsaved via API !!\r\n\r\n### Value is hidden\r\nParam value
will not be visible unless user is `super_user` or\r\n`kibana_admin`,
though user can assign new
value.\r\n\r\n---------\r\n\r\nCo-authored-by: Justin Kambic
<[email protected]>","sha":"0ff9a8a9d9ff2bdb99562eeca29152bd0a0c4385"}},"sourceBranch":"main","suggestedTargetBranches":["8.16"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/195866","number":195866,"mergeCommit":{"message":"[Synthetics]
Fixes partial updates for params and params viewing (#195866)\n\n##
Summary\r\n\r\nFixes
https://github.com/elastic/kibana/issues/167781\r\n\r\nIn docs we says
that only key/value pairs are required, but in actual\r\nedit, that
means rest of the data was being lost on edits\r\n\r\nAllow partial
updates to params edit API !!\r\n\r\nThis PR makes sure prev objects is
fetched and merged with new data\r\nhence allowing partial updates
!!\r\n\r\nWe are also allowing the ability to view value of the secret
once it's\r\nsaved via API !!\r\n\r\n### Value is hidden\r\nParam value
will not be visible unless user is `super_user` or\r\n`kibana_admin`,
though user can assign new
value.\r\n\r\n---------\r\n\r\nCo-authored-by: Justin Kambic
<[email protected]>","sha":"0ff9a8a9d9ff2bdb99562eeca29152bd0a0c4385"}},{"branch":"8.16","label":"v8.16.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.x","label":"v8.17.0","labelRegex":"^v8.17.0$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/197850","number":197850,"state":"MERGED","mergeCommit":{"sha":"4c22558e576bd78deb97474b4a514f6eca9e598e","message":"[8.x]
[Synthetics] Fixes partial updates for params and params viewing
(#195866) (#197850)\n\n# Backport\n\nThis will backport the following
commits from `main` to `8.x`:\n- [[Synthetics] Fixes partial updates for
params and params
viewing\n(#195866)](https://github.com/elastic/kibana/pull/195866)\n\n<!---
Backport version: 9.4.3 -->\n\n### Questions ?\nPlease refer to the
[Backport
tool\ndocumentation](https://github.com/sqren/backport)\n\n<!--BACKPORT\n[{\"author\":{\"name\":\"Shahzad\",\"email\":\"[email protected]\"},\"sourceCommit\":{\"committedDate\":\"2024-10-25T15:06:52Z\",\"message\":\"[Synthetics]\nFixes
partial updates for params and params viewing
(#195866)\\n\\n##\nSummary\\r\\n\\r\\nFixes\nhttps://github.com//issues/167781\\r\\n\\r\\nIn
docs we says\nthat only key/value pairs are required, but in
actual\\r\\nedit, that\nmeans rest of the data was being lost on
edits\\r\\n\\r\\nAllow partial\nupdates to params edit API
!!\\r\\n\\r\\nThis PR makes sure prev objects is\nfetched and merged
with new data\\r\\nhence allowing partial updates\n!!\\r\\n\\r\\nWe are
also allowing the ability to view value of the secret\nonce
it's\\r\\nsaved via API !!\\r\\n\\r\\n### Value is hidden\\r\\nParam
value\nwill not be visible unless user is `super_user`
or\\r\\n`kibana_admin`,\nthough user can assign
new\nvalue.\\r\\n\\r\\n---------\\r\\n\\r\\nCo-authored-by: Justin
Kambic\n<[email protected]>\",\"sha\":\"0ff9a8a9d9ff2bdb99562eeca29152bd0a0c4385\",\"branchLabelMapping\":{\"^v9.0.0$\":\"main\",\"^v8.17.0$\":\"8.x\",\"^v(\\\\d+).(\\\\d+).\\\\d+$\":\"$1.$2\"}},\"sourcePullRequest\":{\"labels\":[\"release_note:skip\",\"v9.0.0\",\"backport:prev-minor\",\"ci:project-deploy-observability\",\"Team:obs-ux-management\"],\"title\":\"[Synthetics]\nFixes
partial updates for params and
params\nviewing\",\"number\":195866,\"url\":\"https://github.com/elastic/kibana/pull/195866\",\"mergeCommit\":{\"message\":\"[Synthetics]\nFixes
partial updates for params and params viewing
(#195866)\\n\\n##\nSummary\\r\\n\\r\\nFixes\nhttps://github.com//issues/167781\\r\\n\\r\\nIn
docs we says\nthat only key/value pairs are required, but in
actual\\r\\nedit, that\nmeans rest of the data was being lost on
edits\\r\\n\\r\\nAllow partial\nupdates to params edit API
!!\\r\\n\\r\\nThis PR makes sure prev objects is\nfetched and merged
with new data\\r\\nhence allowing partial updates\n!!\\r\\n\\r\\nWe are
also allowing the ability to view value of the secret\nonce
it's\\r\\nsaved via API !!\\r\\n\\r\\n### Value is hidden\\r\\nParam
value\nwill not be visible unless user is `super_user`
or\\r\\n`kibana_admin`,\nthough user can assign
new\nvalue.\\r\\n\\r\\n---------\\r\\n\\r\\nCo-authored-by: Justin
Kambic\n<[email protected]>\",\"sha\":\"0ff9a8a9d9ff2bdb99562eeca29152bd0a0c4385\"}},\"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/195866\",\"number\":195866,\"mergeCommit\":{\"message\":\"[Synthetics]\nFixes
partial updates for params and params viewing
(#195866)\\n\\n##\nSummary\\r\\n\\r\\nFixes\nhttps://github.com//issues/167781\\r\\n\\r\\nIn
docs we says\nthat only key/value pairs are required, but in
actual\\r\\nedit, that\nmeans rest of the data was being lost on
edits\\r\\n\\r\\nAllow partial\nupdates to params edit API
!!\\r\\n\\r\\nThis PR makes sure prev objects is\nfetched and merged
with new data\\r\\nhence allowing partial updates\n!!\\r\\n\\r\\nWe are
also allowing the ability to view value of the secret\nonce
it's\\r\\nsaved via API !!\\r\\n\\r\\n### Value is hidden\\r\\nParam
value\nwill not be visible unless user is `super_user`
or\\r\\n`kibana_admin`,\nthough user can assign
new\nvalue.\\r\\n\\r\\n---------\\r\\n\\r\\nCo-authored-by: Justin
Kambic\n<[email protected]>\",\"sha\":\"0ff9a8a9d9ff2bdb99562eeca29152bd0a0c4385\"}}]}]\nBACKPORT-->\n\nCo-authored-by:
Shahzad <[email protected]>"}}]}] BACKPORT-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) ci:project-deploy-observability Create an Observability project release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-management Observability Management User Experience Team v8.16.0 v8.17.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Synthetics] Remove Ability to View Synthetics Global Parameters from Kibana Synthetics App
5 participants