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

Update WinRM related deps #275

Closed
wants to merge 2 commits into from
Closed

Update WinRM related deps #275

wants to merge 2 commits into from

Conversation

lbajolet-hashicorp
Copy link
Contributor

Supersedes #270, essentially re-applies the changes on top of the latest main, as the PR had conflicts.
Since we're releasing a new version of the SDK this week, I opted to open this one as to expedite the process, hope this is OK @bdwyertech!

@lbajolet-hashicorp lbajolet-hashicorp added the dependencies Pull requests that update a dependency file label Jan 20, 2025
@lbajolet-hashicorp lbajolet-hashicorp requested a review from a team as a code owner January 20, 2025 19:58
@lbajolet-hashicorp
Copy link
Contributor Author

Update: @bdwyertech unfortunately the tests fail, I suspect the changes to ParseExecuteCommandResponse might be the reason why they do, and I'm quite surprised this didn't happen on your branch at the time to be honest, but because of this, I cannot merge this PR today.
Since we are aiming for a release tomorrow for SDK + Packer CLI, this will likely have to be included in a future release of the SDK, sorry about this :/

Feel free to let me know what I can do to help, in the meantime I will close this PR, and let you reroll yours, we can continue our developments on this one.

@lbajolet-hashicorp lbajolet-hashicorp deleted the winrm-update branch January 20, 2025 21:37
@bdwyertech
Copy link

bdwyertech commented Jan 21, 2025

Update: @bdwyertech unfortunately the tests fail, I suspect the changes to ParseExecuteCommandResponse might be the reason why they do, and I'm quite surprised this didn't happen on your branch at the time to be honest, but because of this, I cannot merge this PR today. Since we are aiming for a release tomorrow for SDK + Packer CLI, this will likely have to be included in a future release of the SDK, sorry about this :/

Feel free to let me know what I can do to help, in the meantime I will close this PR, and let you reroll yours, we can continue our developments on this one.

I have a feeling it is the mocks within winrmtest, however it could also be something in https://github.com/packer-community/winrmcp. It appears much of the code has not been touched in a very long time.

https://github.com/dylanmei/winrmtest/blob/fbc9ae56efb6053a528b6002497191a22cbe0269/wsman.go#L55

Can test this theory with my updated fork.

go.mod

replace github.com/dylanmei/winrmtest => github.com/bdwyertech/winrmtest v0.0.0-20250121225421-1ff8c86b075c

I'm no WinRM expert, but the responses did not seem to align with Microsoft

dylanmei/winrmtest@master...bdwyertech:winrmtest:mocks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants