search

argoproj / argo-cd

Declarative Continuous Deployment for Kubernetes
https://argo-cd.readthedocs.io
Apache License 2.0
18.04k stars 5.51k forks source link

argocd app diff for app-of-apps #4706

Open mikebryant opened 4 years ago

mikebryant commented 4 years ago

Summary

argocd app diff --new-application-file app.yaml should give a diff of what would be generated for a change to an Application object

Motivation

I'm trying to build a CI pipeline using the app-of-apps pattern, so my git repo contains a bunch of yaml files each of which contains an Application definition. The root app applies this whole directory.

I want to be able to (pre-merge, in a PR), get a diff of the actual changes that would happen, so I want to be able to run a command and pass it a file (or directory would also work), and generate the diff of what would be generated from the new Application object vs the one on the cluster. The current argocd app diff --local is very hard to use, because these charts are all being pulled from helm repositories (some internal, some external), so I don't have a convenient chart directory that's matching the version the new application is trying to install.

Proposal

New CLI flags / command?

mikebryant commented 4 years ago

Also the current diff behaviour only works for changes to the chart, it doesn't work for changes to Application.spec.source.helm.values

musabmasood commented 3 years ago

We also have this scenario where an application was already deployed in the cluster with helm. We wanted to manage it with ArgoCD.

We first create just the application (application.yaml) in the first PR , merging results in UNKNOWN status which is expected since the chart is not there yet in the repo.

Now that the application is in ArgoCD, if you run:

argocd app diff <app> --revision='<branch-with-chart-and-values>'

I expect it to show only the minimal diff since the helm chart is already deployed. Instead, the argocd cli results in a big diff. For example if you're only adding an annotation to a Deployment manifest, the cli outputs as if it wants to recreate the whole Deployment.

gw0 commented 2 years ago

@musabmasood I guess this is the expected behavior? You had just an App without a chart deployed, so the live state that ArgoCD is aware of is empty (i.e. there were no resources marked as being part of the App). In your branch you had added the chart, so the target state contains all the resources. The difference is to apply them all (which would probably just add some annotations, but leave them as-is).

gw0 commented 2 years ago

Oh, I found a manual workaround for the OP issue... So, the goal is to get a diff on all manifests generated by an App when you make a local change to the App manifest.

gw0 commented 2 years ago

Lets script this workaround:

$ APP_NAME="my-app"

## render root and extract the modified App
$ kustomize --enable-helm --load-restrictor=LoadRestrictionsNone my-root/ > root-local.yaml
$ yq "[.] | select(.[].metadata.name==\"${APP_NAME}\") | .[0]" root-local.yaml > modified.yaml

## create a temporary modified App with disabled finalizers and sync
$ yq -i '.metadata.name="tmp-modified" | del(.metadata.finalizers) | del(.spec.syncPolicy.automated)' modified.yaml
$ argocd app create -f modified.yaml

## render all manifests by the current App and the modified App
$ argocd app manifests "${APP_NAME}" --source=git > all-current.yaml
$ argocd app manifests tmp-modified --source=git > all-modified.yaml

## remove the temporary modified App
$ argocd app delete -y tmp-modified

## clean-up
$ YQ_FILTER='
  del(.metadata.creationTimestamp) |
  del(.metadata.generation) |
  del(.metadata.resourceVersion) |
  del(.metadata.uid) |
  del(.metadata.managedFields) |
  del(.metadata.annotations."kubectl.kubernetes.io/last-applied-configuration") |
  del(.metadata.annotations."deployment.kubernetes.io/revision") |
  del(.metadata.labels."app.kubernetes.io/instance") |
  del(.status)
' \
  GREP_FILTER='^\{\}$|^  annotations: \{\}$|^  labels: \{\}$|  vault/last-refresh:'
$ yq "${YQ_FILTER}" all-current.yaml | egrep -v "${GREP_FILTER}" > all-current-cleanup.yaml
$ yq "${YQ_FILTER}" all-modified.yaml | egrep -v "${GREP_FILTER}" > all-modified-cleanup.yaml

## diff
$ diff -u --color all-current-cleanup.yaml all-modified-cleanup.yaml
MeNsaaH commented 2 years ago

Currently what I'm doing is having a python script that loops over all the modified manifests and then runs argocd diff against them. For remote charts, the script downloads it to a tmp directory and runs the diff with --local set to the directory. This is not very ideal. Running diff against an app-of-apps pattern, should show the tree diff 🤦🏿

MeNsaaH commented 2 years ago

@jessesuen I'm interested in picking this up and running with it. Is there any issue with that?

crenshaw-dev commented 1 year ago

I gave it a try: https://github.com/argoproj/argo-cd/pull/12813

My current implementation requires applications, update access. Please take a look at my justification in the description and let me know y'alls thoughts.

imbgar commented 1 year ago

Currently what I'm doing is having a python script that loops over all the modified manifests and then runs argocd diff against them. For remote charts, the script downloads it to a tmp directory and runs the diff with --local set to the directory. This is not very ideal. Running diff against an app-of-apps pattern, should show the tree diff 🤦🏿

I'm happy to see active development on this issue, but I do have question in case I can make a similar workaround in the meantime.

My Application references a remote chart as well. When I run argocd app diff $APP_NAME --local $APP_PATH --server-side-generate I receive this error:

FATA[0002] rpc error: code = Unknown desc = failed parsing dependencies: open /tmp/0dd012cf-1358-44cd-b115-88b11d941f82/Chart.yaml: no such file or directory

The hash in /tmp/ is random every time, but it sounds like you were able to populate the charts locally. For me --local is pointing to local directory containing modified yaml of Application in yaml and not a tmp directory with chart. Any insight as to how you did this? 🙏

I can appreciate if you don't want random workaround code floating around, but I'd like to take a peek at a gist if you can make one with your script!

gw0 commented 1 year ago

For me --local is pointing to local directory containing modified yaml of Application

Afaik it needs to point to the local copy of the Helm chart itself (not Application resource).

akhy commented 6 months ago

Also the current diff behaviour only works for changes to the chart, it doesn't work for changes to Application.spec.source.helm.values

Also came across this issue. For this, I propose for a new flag like --helm-values (or --local-helm-values?)

argocd app diff my-app  --helm-values ./values.yaml

Edit: After some thought, I think it would be better if the argocd app diff subcommand accept similar flags like argocd app set, for example:

The point is, I expect app diff command to be used like app set and app sync but in dry-run mode without actually editing any Application's spec.

andrii-korotkov-verkada commented 1 week ago

I'll try to help with the implementation

gw0 commented 1 week ago

I noticed argocd app manifests has a --local parameter that can point to a local modified application.yaml. This should render all the manifests generated by the modified app (including all in-line values or referenced values files) which would simplify generating a diff.