Set-SafeguardPatch and Install-SafeguardPatch return JSON string but don't throw exceptions for errors

[JeffHarkavy]

This may be intentional?

When excuting either Set- or Install-SafegaurdPatch if the staging returns anyting in Errors or Warnings the command still "succeeds" without and exception being raised. While this might make sense for Warnings, should we be throwing an exception when Errors are detected? Perhaps use the absence/presenceof the -Force option to throw/not throw?

[petrsnd]

This is probably because they use a custom web request client. That change was made to support progress and better upload UX. I think there is probably a way to get these cmdlets to throw proper exceptions, and they should if we can do it.

[Goelrachit]

Fixed in 6.8.568-pre.

[Goelrachit]

@JeffHarkavy please verify this in the latest 6.8 prerelease. Thanks!

[Kevin-Andrew]

@petrsnd , @JeffHarkavy , @Goelrachit

This is probably because they use a custom web request client. That change was made to support progress and better upload UX.

The change you are referring to is PR #200, “Install-SafeguardPatch uses too much memory”. However, it did not the cause the question at hand. Allow me to try and go through some history, the best I can piece it together. What is happening and being referred to here comes from a feature that was first released in Safeguard 6.0, “Separate OS Patches”, which included a change that would return “Patch Precondition” checks in the initial response after uploading a patch. The “Patch Preconditions” check itself, was a feature added to Safeguard in version 2.10, released in late October, 2019. And as for Safeguard 6.0, it was released in April, 2020. The change to Safeguard-ps being referred to in the quote above, as mentioned is PR #200, was committed on November 5, 2019, and first appeared in Safeguard-ps version 2.10.391-pre. So it couldn’t have known about the "Patch Precondition" checks being returned in the response. In the Safeguard-ps release just prior to that, 2.10.387-pre, there still hadn’t been any support for “Patch Precondition” checks implemented at all. The first I see of “Patch Preconditions” in Safeguard-ps is PR #252, from April 16, 2020.

Regardless of all of that, the current question at hand:

… if the staging returns anyting [sic] in Errors or Warnings the command still "succeeds" …

has not been addressed. This commit, from PR #295 does not address the question. That commit just rethrows an exception when one has already happened, instead of returning any JSON data. In the case of the “Separate OS Patches” and “Patch Preconditions”, the server will return a successful HTTP 200 status code even when there are errors to the “Patch Preconditions”.

Here is an example of Install-SafeguardPatch using the version of Safeguard-ps just prior to PR #295:

In that example, we are trying to upload/stage an LTS patch to a Safeguard that is current already on a feature branch. You cannot patch from a feature branch to an LTS branch. That test is done with a “Patch Precondition” check. In this case, with the Install-SafeguardPatch, it happens after Safeguard-ps automatically makes a call to distribute the patch throughout the cluster. That patch distribution can be time consuming and in the end, still results in the patch not being able to be applied. So, from my understanding, what Jeff is asking is should we look at the response of the patch upload for any “Patch Precondition” errors or warnings before distributing the patch?

Note that with the latest code from PR #295, the behavior is exactly the same:

With the Set-SafeguardInstall method, the output is a little different, giving us a better understanding of why Jeff may have asked the question in the first place:

Here we can see that the server already knows that the patch can’t be applied, but we go ahead and ask the user if they still want to distribute it.

A few things need to be done.

1. Revert the commit, from PR Bug fixes #295. It can essentially be done as part of the next suggested changes.
2. The Set-SafeguardPatch method needs to be updated with a few changes, so that the code that calls the “UploadFileStream” looks like the code in the Install-SafeguardPatch method. Here is the code from the Install-SafeguardPatch method with a few key points called out:
   

And here is the code from the Set-SafeguardPatch method. It needs to be modified to look more like the code in the Install-SafeguardPatch method.

3. We can then make a decision on what to do about the real question at hand. Should the “Patch Precondition” checks be performed before distributing the patch to the cluster? And what to do about any warnings or errors? My suggestion would be to first setup a test environment cluster and see how the Safeguard Desktop Client application behaves.