Expose clear transients in ecctl #649
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cmd/deployment/show.go
Outdated
| @@ -112,4 +123,5 @@ func initShowFlags() { | |||
| showCmd.Flags().BoolP("metadata", "m", false, "Shows the deployment metadata") | |||
| showCmd.Flags().BoolP("settings", "s", false, "Shows the deployment settings") | |||
| showCmd.Flags().Bool("generate-update-payload", false, "Outputs JSON which can be used as an argument for the --file flag with the update command.") | |||
| showCmd.Flags().Bool("clear-transient", false, "Removes the `transient` field in order to make read - edit - write loop safe") | |||
There was a problem hiding this comment.
should the comment explain that its default is true when generate-update-payload is enabled?
There was a problem hiding this comment.
Yeah I think so. I updated documentation.
gigerdo
reviewed
May 16, 2024
docs/ecctl_deployment_show.adoc
Outdated
| --plans Shows the deployment plans | ||
| --ref-id string Optional deployment kind RefId, if not set, the RefId will be auto-discovered | ||
| -s, --settings Shows the deployment settings | ||
| --clear-transient transient Removes the transient field in order to make read - edit - write loop safer. The default value of clear-transient depends on the value of `generate-update-payload`. If `generate-update-payload` is true `clear-transient` defaults to true. Otherwise defaults to false. |
There was a problem hiding this comment.
Looks like some issue with the docs generator: It adds a transient that shouldn't be there --clear-transient transient Removes the
It seems like it is due to `` in the text. Maybe some special syntax 🤔
gigerdo
reviewed
May 16, 2024
| @@ -192,6 +192,7 @@ func Test_showCmd(t *testing.T) { | |||
| Path: "/api/v1/deployments/29337f77410e23ab30e15c280060facf", | |||
| Host: api.DefaultMockHost, | |||
| Query: url.Values{ | |||
| "clear_transient": {"true"}, | |||
There was a problem hiding this comment.
There is another test below succeeds with show flags set where the --clear-transient flag can be added to test that that works as well.
gigerdo
approved these changes
May 16, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Exposes new
clear_transientsfield for ecctl for show deployment.The default value depends on on the value of
generate-update-payloadin order to facilitate safe usage of the "read - edit - write" loop. Ifgenerate-update-payload=truethenclear-transientsalso defaults to true and is false otherwise. If the flag is manually passed then that value takes precedence.Related Issues
https://elasticco.atlassian.net/browse/CP-4726
Motivation and Context
This change makes reading and writing deployments safer. We ran into this during the last security remediation and almost overwrote someone’s cluster from their most recent snapsho
How Has This Been Tested?
This has been tested by using local build to operate against QA ESS. I have verified that with
--generate-update-payload=truethe transient properties are not included (default behavior). If--generate-update-payload=trueand--clear-transient=falseis explicitly passed then transient properties are included.Types of Changes
Readiness Checklist