Add sudo propagation check for azure #147
Merged
+81
−25
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
GGonryun
changed the title
gergas3
approved these changes
Nov 27, 2024
GGonryun
deleted the
miguelcampos/eng-3149-azure-ssh-add-pretestaccesspropagationargs-for-sudo-access
branch
GGonryun
added a commit
that referenced
this pull request
Dec 5, 2024
We recently introduced `validAccessPatterns` in #147 which checks to make sure a certain termination string exists when performing propagation checks. Consequentially for Azure, when we exit the terminal we're now always attempting to verify that an ephemeral access message is valid and failing the check. Two fixes were possible, 1) we either only check `isValidError` if we're in a propagation check or 2) we default `isValidError` to true and we only set it to false if we encounter an error. We didn't go with the first option in #147 because we want to remove the concept of `isAccessPropagationPreTest` from the `accessPropagationGuard`, but checking for valid error messages should only apply during the pretest phase. This PR instead only passes `validAccessPatterns` if we're performing a pre-test to the propagation guard.
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.
This PR implements Azure's
preTestAccessPropagationArgsand adds a new parameter to theSSHProvidertype to support valid access errors.Sudo access to an azure node can sometimes take an unexpectedly long time to propagate fully and if a user connects before sudo access propagates then they'll need to restart their session in order to pick up the sudo access. Therefore it is necessary to validate sudo access before letting a user complete their connection.
We get an error code of 1 instead of an error code 0 after sudo access is granted when performing the pretest for an Azure instance. This means that we needed to extend the
SSHProviderinterface a little to include potentially valid error messages and override the error code of 1.