Skip to content

feat(cli): allow to reject any ssm plugin install/update#2060

Merged
mergify[bot] merged 2 commits intoaws:mainlinefrom
iamhopaul123:exec-yes
Mar 16, 2021
Merged

feat(cli): allow to reject any ssm plugin install/update#2060
mergify[bot] merged 2 commits intoaws:mainlinefrom
iamhopaul123:exec-yes

Conversation

@iamhopaul123
Copy link
Copy Markdown
Contributor

This PR makes it possible to reject any SSM plugin install/update with --yes=false. Technically not many users want to use this feature. It is mainly for the e2e tests so that we can skip without updating the SSM plugin.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@iamhopaul123 iamhopaul123 requested a review from a team as a code owner March 15, 2021 23:56
@iamhopaul123 iamhopaul123 requested a review from bvtujo March 15, 2021 23:56
@iamhopaul123
Copy link
Copy Markdown
Contributor Author

$ copi svc exec --yes=false

Found only one deployed service frontend in environment test
Execute `/bin/sh` in container frontend in task 297c0a2ef8034100a343788d3864ef9b.
✘ execute command /bin/sh in container frontend: start session ecs-execute-command-0eb7e61e13bc503bb using ssm plugin: start session: exec: "session-manager-plugin": executable file not found in $PATH

Comment thread internal/pkg/cli/exec.go Outdated
taskID string
containerName string
skipConfirmation bool
yes *bool
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I wonder if we should make this flag more descriptive, like ssmPlugin or updateSSM or something?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we need the new yes field? (see below) I think we can get away by making this a pointer and assigning a local variable instead of storing this as a field in the struct

Suggested change
yes *bool
skipConfirmation *bool // If nil, we will prompt to upgrade the ssm plugin.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok done!

Comment thread internal/pkg/cli/flag.go Outdated
Comment thread internal/pkg/cli/exec.go Outdated
taskID string
containerName string
skipConfirmation bool
yes *bool
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we need the new yes field? (see below) I think we can get away by making this a pointer and assigning a local variable instead of storing this as a field in the struct

Suggested change
yes *bool
skipConfirmation *bool // If nil, we will prompt to upgrade the ssm plugin.

Comment thread internal/pkg/cli/svc_exec.go Outdated
Copy link
Copy Markdown
Contributor

@bvtujo bvtujo left a comment

Choose a reason for hiding this comment

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

Clever clever

@mergify mergify Bot merged commit 6d3fa3d into aws:mainline Mar 16, 2021
thrau pushed a commit to localstack/copilot-cli-local that referenced this pull request Dec 9, 2022
<!-- Provide summary of changes -->
This PR makes it possible to reject any SSM plugin install/update with `--yes=false`. Technically not many users want to use this feature. It is mainly for the e2e tests so that we can skip without updating the SSM plugin.
<!-- Issue number, if available. E.g. "Fixes aws#31", "Addresses aws#42, 77" -->

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants