Skip to content
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

feat(cli): add cmd to preview generated apps of appsets (#10895) #16781

Merged
merged 40 commits into from
Jun 18, 2024

Conversation

agaudreault
Copy link
Member

@agaudreault agaudreault commented Jan 8, 2024

Closes #10895

The goal of this PR is to update the argocd appset create command to add a --dry-run argument. When it is specified, the server will call the applicationSet templating and app generation logic to evaluate and transform all the generator. It will use the resulting applications to generate a list of "generated" application.

To avoid creating a hard dependency between the server and applicationSet, which is not a default component of ArgoCD, the server does the templating based on the same configuration the ApplicationSet adds. If not configured the same, the generation may be different. Once ApplicationSet is a native components of Argo, the templating could be exposed as an api call, similar to how it is done with the repo-server component.

./dist/argocd appset create --dry-run ./appset.yaml -o json | jq -r '.status.resources[].name' 
ApplicationSet 'tests.simple-application' unchanged (dry-run)
tests.simple-application.dev
tests.simple-application.production-1
tests.simple-application.production-2
tests.simple-application.staging

The output of this command will allow to compare the list of resulting applications with the output of argocd appset get tests.simple-application -o json | jq -r '.status.resources[].name to know which app will be added or removed once the manifest is applied.

Suggestions

I think another CLI command could be argocd appset template ./appset.yaml -o json and return the list of templated applications with the full object and not only their name. Some people expressed the need to have the whole applications so they could validate the logic client-side.

With the current implementation, only the app name is returned in create and the get.

An implementation to be able to do client-side diff would be to have the template command return the current AppSet with all the applications templated, and modify the get to return the full templated application as well (this would avoid an argocd app get for each generated apps).

Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com>
Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com>
Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com>
Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com>
Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com>
Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com>
Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com>
Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com>
…ist-apps

Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com>
@agaudreault agaudreault self-assigned this Jan 9, 2024
Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com>
Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com>
Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com>
Copy link

codecov bot commented Jan 10, 2024

Codecov Report

Attention: Patch coverage is 62.45955% with 116 lines in your changes missing coverage. Please review.

Project coverage is 50.24%. Comparing base (4c6ad9d) to head (19b0871).
Report is 11 commits behind head on master.

Current head 19b0871 differs from pull request most recent head 02bb52a

Please upload reports for the commit 02bb52a to get more accurate results.

Files Patch % Lines
cmd/argocd-server/commands/argocd_server.go 0.00% 46 Missing ⚠️
cmd/argocd/commands/applicationset.go 0.00% 27 Missing ⚠️
cmd/argocd/commands/headless/headless.go 0.00% 22 Missing ⚠️
applicationset/controllers/template/template.go 83.01% 7 Missing and 2 partials ⚠️
server/applicationset/applicationset.go 82.22% 4 Missing and 4 partials ⚠️
applicationset/generators/scm_provider.go 70.00% 3 Missing ⚠️
applicationset/generators/pull_request.go 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #16781      +/-   ##
==========================================
- Coverage   50.28%   50.24%   -0.05%     
==========================================
  Files         312      315       +3     
  Lines       43020    43120     +100     
==========================================
+ Hits        21633    21665      +32     
- Misses      18905    18971      +66     
- Partials     2482     2484       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@agaudreault agaudreault changed the title WIP: feat(cli): add cmd to preview generated apps of appsets (#10895) feat(cli): add cmd to preview generated apps of appsets (#10895) Jan 15, 2024
Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com>
@haiwu
Copy link

haiwu commented Jan 22, 2024

This feature is really needed in order to do any meaningful dryrun if using applicationset.

In production environment, we have to review the dryrun changes before any updates to applicationset could be landed. With this PR in place, we could get a list of applications, and then we could iterate the list of applications to get its diff via "argocd app diff" command at least.

Without this PR in place, there's no way to do any meaningful dryrun for applicationset.

This is the most needed feature for argocd applicationset.

@agaudreault
Copy link
Member Author

agaudreault commented Jan 22, 2024

@argoproj/argocd-approvers what do you think if we merge this as an "alpha" feature, and when the dependency is merged, we can change the output of the command to use the new fields?
Maybe with a warning in the CLI doc?
Never mind, I forgot that the current get command does not return the list of applications when progressive sync is not present. So it does need the dependency.

The dependency seems to be coupled with a few other PRs and may take some time to merge.
@alexymantha @csantanapr @reggie-k

@agaudreault agaudreault marked this pull request as ready for review January 22, 2024 21:06
@agaudreault agaudreault requested review from a team as code owners January 22, 2024 21:06
@agaudreault agaudreault marked this pull request as draft January 22, 2024 21:10
@cheskayang
Copy link

👍 Long-waited feature to allow simple and reliable validations on any non-trivial manifests repo that uses appset

Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
@agaudreault agaudreault marked this pull request as ready for review May 30, 2024 16:33
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
@crenshaw-dev crenshaw-dev enabled auto-merge (squash) June 18, 2024 14:11
@crenshaw-dev crenshaw-dev merged commit 70755aa into argoproj:master Jun 18, 2024
26 checks passed
Comment on lines +12 to +13
//go:generate go run github.com/vektra/mockery/v2@v2.40.2 --name=Generator

Copy link
Contributor

@Jack-R-lantern Jack-R-lantern Jun 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think --name=generator or make codegen again
Currently, the go generate command generates Generator.go, not generator.go.

Comment on lines +28 to +29
//go:generate go run github.com/vektra/mockery/v2@v2.40.2 --name=Renderer

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think --name=renderer or make codegen again
Currently, the go generate command generates Renderer.go, not renderer.go.

ishitasequeira pushed a commit to ishitasequeira/argo-cd that referenced this pull request Jul 2, 2024
…) (argoproj#16781)

* feat(cli): add cmd to preview generated apps

Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com>

* fix build

Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com>

* fix local proto gen

Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com>

* dry run client

Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com>

* fix: allow to run codegen outside GOPATH

Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com>

* clientgen

Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com>

* openapigen

Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com>

* remove ensure-gopath

Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com>

* fix tests and templatePatch

Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com>

* fix build

Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com>

* convert to interfaces

Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com>

* codegen

Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>

* extract common code

Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>

* use appset params in server

Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>

* codegen

Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>

* fix test build

Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>

* unit tests

Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>

* move test to new package

Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>

* move to correct folders

Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>

* fix build

Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>

* review

Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>

* lint

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* fix test

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* fix lint

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* auto generate mocks

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* better error handling

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* more docs

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* more docs

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* lint

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

---------

Signed-off-by: Alexandre Gaudreault <alexandre.gaudreault@logmein.com>
Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Completed
Development

Successfully merging this pull request may close these issues.

ApplicationSet generators diff with CLI
8 participants