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

Fail istioctl install on explicitly empty revision #26940

Closed
therealmitchconnors opened this issue Aug 31, 2020 · 3 comments · Fixed by #26959
Closed

Fail istioctl install on explicitly empty revision #26940

therealmitchconnors opened this issue Aug 31, 2020 · 3 comments · Fixed by #26959
Assignees
Labels
area/environments area/user experience feature/Multi-control-plane issues related with multi-control-plane support in a cluster

Comments

@therealmitchconnors
Copy link
Contributor

therealmitchconnors commented Aug 31, 2020

Bug description
istioctl install fails to distinguish between unset revision and empty string revision. In both cases, an in-place upgrade is applied. This can cause problems when the user has a typo in their bash session such as:

export REVISION=1.6.4
istioctl install --revision $REVISION_NAME

In this case, the user expects a revision-based upgrade which poses no risk to the existing mesh, but because of their typo they get a destructive in-place upgrade. When the revision flag is supplied, but an invalid value is associated with it (such as empty string, not sure if other scenarios exist), the command should fail, not fall back to in-place upgrade. Users should have 100% certainty which sort of upgrade they are performing.

[ ] Docs
[ x ] Installation
[ ] Networking
[ ] Performance and Scalability
[ ] Extensions and Telemetry
[ ] Security
[ ] Test and Release
[ x ] User Experience
[ ] Developer Infrastructure

Expected behavior
Multi Control Plane upgrade fails.

Version (include the output of istioctl version --remote and kubectl version and helm version if you used Helm)
istio 1.6+

@istio-policy-bot istio-policy-bot added area/environments area/user experience feature/Multi-control-plane issues related with multi-control-plane support in a cluster labels Aug 31, 2020
@therealmitchconnors therealmitchconnors self-assigned this Aug 31, 2020
@esnible
Copy link
Contributor

esnible commented Aug 31, 2020

According to Kubernetes, "Valid label values must be 63 characters or less and must be empty or begin and end with an alphanumeric character ([a-z0-9A-Z]) with dashes (-), underscores (_), dots (.), and alphanumerics between."

As Istio stores the revision in a label value, there should not be a problem validating that the user's supplied value has those characteristics.

@howardjohn
Copy link
Member

Does cobra distinguish unset vs empty flags if the default is ""?

@therealmitchconnors
Copy link
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/environments area/user experience feature/Multi-control-plane issues related with multi-control-plane support in a cluster
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants