Report changed files for Argo steps#1771
Conversation
zentron
left a comment
zentron
left a comment
There was a problem hiding this comment.
Im unsure about some of the data model used here, though I know that's not tied to your specific change.
I don't like overloading the ProcessApplicationResult to be used in two different scenarios with different nulled properties. Feels a bit off.
Just a question about the tests
| """; | ||
|
|
||
| var resultRepo = RepositoryHelpers.CloneOrigin(tempDirectory, OriginPath, argoCDBranchName); | ||
| AssertFileContents(resultRepo, "files/values.yml", updatedValuesFile); |
There was a problem hiding this comment.
Do we need to assert the file looks as we expected? I would have thought this would be handled by another test already and this could focus on the jsonpatch info.
There was a problem hiding this comment.
Yeah, that's a good call out for a copy and paste job. I'll scope down those assertions
Yeah, you've got a point with this. I think the model we are passing back to server via the service message works alright - but I did feel a bit uneasy as I was making changes through each of the install conventions. There are places where a misplaced parameter could get through as a bug. I'll merge this as is, but it's something that I might be able to improve. |
2 participants
In #1749 we introduced a service message to report changed files when doing Argo CD deployments, but it did not include the actual changed files/patches that we want to capture.
This PR adds to that existing service message:
The JSON patch functionality will be completed once we finish up changes to include a JSON patch generator in Calamari.
We will avoid sending sensitive data back to server through:
Example data (from server side)
Closes MD-1511
Relates to MD-1512