feat: argocd app manifests --local option (#5525) #7658
Conversation
Codecov Report
@@ Coverage Diff @@
## master #7658 +/- ##
==========================================
- Coverage 45.89% 42.28% -3.61%
==========================================
Files 227 176 -51
Lines 27327 22815 -4512
==========================================
- Hits 12541 9647 -2894
+ Misses 13084 11792 -1292
+ Partials 1702 1376 -326
Continue to review full report at Codecov.
|
Cylix
force-pushed
the
5525-cli-app-manifests-local
branch
from
1ae13f5 to
292bb39
Compare
Cylix
force-pushed
the
5525-cli-app-manifests-local
branch
from
0ec889e to
667e8ea
Compare
Signed-off-by: Simon Ninon <simon.ninon@gmail.com>
Cylix
force-pushed
the
5525-cli-app-manifests-local
branch
from
667e8ea to
4102f81
Compare
| cluster, err := clusterIf.Get(context.Background(), &clusterpkg.ClusterQuery{Name: app.Spec.Destination.Name, Server: app.Spec.Destination.Server}) | ||
| errors.CheckError(err) | ||
|
|
||
| unstructureds = getLocalObjects(app, local, localRepoRoot, argoSettings.AppLabelKey, cluster.ServerVersion, cluster.Info.APIVersions, argoSettings.KustomizeOptions, argoSettings.ConfigManagementPlugins, argoSettings.TrackingMethod) |
There was a problem hiding this comment.
Is local really useful for the manifests use case ?
Couldn't we use app.Spec.Source.Path instead of having to provide its value with local?
Then regarding the if local != "" condition to enable this behavior I would say checking if len(localRepoRoot) > 0 instead would make sense.
There was a problem hiding this comment.
I validated the commands are working as expected. There is some question of how useful this is but it has been implemented as designed in #5525 which had several requesters. So I think it's reasonable to merge.
|
@todaywasawesome, @Cylix I'm reopening here to fix a merge conflict I missed: #10061 |
Context
Closes #5525:
Changes
I mostly followed how
argo app diff --localis implemented for consistency and tried to reuse the existinggetLocalObjectsthat does what we need.Testing
I started an Argo server with
helm install charts/argo-cd -n argocd-helm --generate-nameand configured a sample helm app.Among other things, I verified the following:
argocd app manifests helm-sample-app --helpargocd app manifests helm-sample-app --source liveargocd app manifests helm-sample-app --source gitargocd app manifests helm-sample-app --source git --local charts/helm-exampleThere does not seem to be existing unit tests for
NewApplicationManifestsCommand, but let me know if that's something you'd like me to add.Checklist