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
argocd app sync/diff --local
doesn't account for sidecar CMPs
#8145
Comments
Proposal: server-side diffing for local codeLocal diff/syncing is currently implemented by running repo-server code on the CLI's host machine. It's the user's responsibility to install the necessary CLIs (like Helm or Kustomize). We shouldn't expect users to configure their local machine to run the same CMPs as those installed on the repo-server. Instead, we should have the CLI send the user's local files to the repo-server (via the Argo CD API), and have the repo-server send back the generated diff. Steps
Security considerationsNoted above, but repeated here:
|
Signed-off-by: notfromstatefarm <86763948+notfromstatefarm@users.noreply.github.com>
* feat: server-side manifest generation for diff (#8145) Signed-off-by: notfromstatefarm <86763948+notfromstatefarm@users.noreply.github.com> * fix docs, mocks, ineffectual err Signed-off-by: notfromstatefarm <86763948+notfromstatefarm@users.noreply.github.com> * fix CMPs, ineffectual err Signed-off-by: notfromstatefarm <86763948+notfromstatefarm@users.noreply.github.com> * refactor Signed-off-by: notfromstatefarm <86763948+notfromstatefarm@users.noreply.github.com> * add unit tests Signed-off-by: notfromstatefarm <86763948+notfromstatefarm@users.noreply.github.com> * handle err Signed-off-by: notfromstatefarm <86763948+notfromstatefarm@users.noreply.github.com> * add size limits and inclusion filters Signed-off-by: notfromstatefarm <86763948+notfromstatefarm@users.noreply.github.com> * fix docs Signed-off-by: notfromstatefarm <86763948+notfromstatefarm@users.noreply.github.com> * fix errors, increase defaults Signed-off-by: notfromstatefarm <86763948+notfromstatefarm@users.noreply.github.com> * use quantity, wrap errors, add security fields to logs, deprecation warning Signed-off-by: notfromstatefarm <86763948+notfromstatefarm@users.noreply.github.com> * have e2e test use server side generation Signed-off-by: notfromstatefarm <86763948+notfromstatefarm@users.noreply.github.com> * nits Signed-off-by: notfromstatefarm <86763948+notfromstatefarm@users.noreply.github.com> * remove unused import Signed-off-by: notfromstatefarm <86763948+notfromstatefarm@users.noreply.github.com> * fix merge conflict Signed-off-by: notfromstatefarm <86763948+notfromstatefarm@users.noreply.github.com> * fix conflicts Signed-off-by: notfromstatefarm <86763948+notfromstatefarm@users.noreply.github.com> * fix e2e test Signed-off-by: notfromstatefarm <86763948+notfromstatefarm@users.noreply.github.com> * add deprecation/breaking change info Signed-off-by: notfromstatefarm <86763948+notfromstatefarm@users.noreply.github.com> * remove security logging stuff, will be in a separate PR Signed-off-by: notfromstatefarm <86763948+notfromstatefarm@users.noreply.github.com> * more specific docs Signed-off-by: notfromstatefarm <86763948+notfromstatefarm@users.noreply.github.com> * add security logging Signed-off-by: notfromstatefarm <86763948+notfromstatefarm@users.noreply.github.com> Signed-off-by: notfromstatefarm <86763948+notfromstatefarm@users.noreply.github.com>
Not totally fixed. We should do this: #10936 |
i'm seeing this error
when running this is the argocd application object
|
You get that consistently? The checksum check is meant to prevent tampering or data corruption. I'd be really surprised to see it consistently. Can you run a hard refresh to make sure you don't keep getting a cached error? |
i tried
argocd log logs
is there an example of the directory context and files to include when using |
Following the original motivation, what do you think of |
Also seeing the "file checksum validation error" consistently when running
Seems the crc is not being properly computed or there's a difference in the way it's being generated. |
@cilindrox can you check the server-side log and post the checksum values? I did a deep dive on this error with @joshuasimon-taulia and found that when the API server receives the streamed file it receives metadata but no actual file contents. So its checksum is always the checksum of an empty array of bytes. I still have no idea why that is happening. |
Thanks for the quick reply @crenshaw-dev - I'm not seeing any relevant logs on Here's the crc client-side:
Should I be running this with a Edit: lack of logs got me thinking - could this be getting filtered at the ingress level? ie: the payload's not reaching the server? |
Yep,
Wouldn't hurt, but I don't think there will be much more info. I think the issue is pretty low-level.
Quite possibly. I doubt that the server is receiving the bytes and then just ignoring them. I bet they're not making it over the network. |
we tried the same exercise using |
I hit the same issue in a scenario where we are using For now, we'll skirt around this issue in our use case (which is to show the diffs on pull requests pre-merge) by using the |
Hi! I also got into the error of checksums not matching today when trying to figure out the diff from local path. I see that the issue referenced in this PR was removed from the v2.6 milestone but the warning about not using Does it mean that this issue will be addressed before v2.6 is released? Since otherwise once v2.6 would be released |
Having encountered this checksum problem myself, I can contribute to the confusion a bit: in my environment I could narrow this particular problem down to incorrectly setup non-web gRPC ingress. After fixing my setup, I could verify that |
For my case it works if if I use |
I'm also getting this issue where the file streaming part of diff --server-side-generate through the grpc-web just doesn't work and the end result is a different checksum. We cannot use pure grpc as our ArgoCD is behind Cloudflare Access which doesn't support any grpc. |
Spreading the knowledge: grpc-web doesn't support client streaming, except by websocket: #12032 (comment) |
Describe the bug
argocd app diff <appname> --local .
fails when<appname>
is handled by a sidecar CMP. Specifically, it fails with this error:argocd app sync <appname> --local .
fails with the same error.To Reproduce
argocd app diff <appname> --local .
from the directory in the git clone where the app is defined.Expected behavior
I would expect a diff to be generated. Since the example CMP always produces the same manifest, I'd expect the diff to be empty.
Version
The text was updated successfully, but these errors were encountered: