Argo CD attempts to unmarshal and deploy non Kubernetes JSON files

[rumstead]

Checklist:

• [x] I've searched in the docs and FAQ for my answer: https://bit.ly/argocd-faq.
• [x] I've included steps to reproduce the bug.
• [x] I've pasted the output of argocd version.

Describe the bug We store our ApplicationSets and GIT config in the same "bootstrap" repo. We have historically only used *.yaml files and have not run into any issues. ApplicationSet GIT generators also support JSON and a team used a JSON based config file over yaml. The bootstrap repo failed to sync because Argo CD tried to unmarshal the ApplicationSet's JSON config.

Issue #4695 and PR #4729 were created to address Argo CD attempting to unmarshal files that "look like" Kubernetes manifests, however, Argo CD is still attempting to unmarshal JSON files even though they do not contain "apiVersion, Kind, or Metadata".

Also relevant: #2638

To Reproduce Unit test reproducing

=== RUN   TestRecurseManifestsInDirAppSet
time="2022-10-19T15:29:00-04:00" level=info msg="manifest cache miss: &ApplicationSource{RepoURL:,Path:./testdata/appset-yaml-json,TargetRevision:,Helm:nil,Kustomize:nil,Directory:&ApplicationSourceDirectory{Recurse:true,Jsonnet:ApplicationSourceJsonnet{ExtVars:[]JsonnetVar{},TLAs:[]JsonnetVar{},Libs:[],},Exclude:,Include:,},Plugin:nil,Chart:,}/mock.Anything"
time="2022-10-19T15:29:00-04:00" level=info msg="manifest cache miss: &ApplicationSource{RepoURL:,Path:./testdata/appset-yaml-json,TargetRevision:,Helm:nil,Kustomize:nil,Directory:&ApplicationSourceDirectory{Recurse:true,Jsonnet:ApplicationSourceJsonnet{ExtVars:[]JsonnetVar{},TLAs:[]JsonnetVar{},Libs:[],},Exclude:,Include:,},Plugin:nil,Chart:,}/mock.Anything"
    repository_test.go:321: 
            Error Trace:    repository_test.go:321
            Error:          Expected nil, but got: &status.Error{s:(*status.Status)(0xc000cc6788)}
            Test:           TestRecurseManifestsInDirAppSet
            Messages:       rpc error: code = FailedPrecondition desc = Failed to unmarshal "config.json": Object 'Kind' is missing in '{
                              "path": "kustomize/dev",
                              "name": "dev-cool-thing",
                              "version": "HEAD"
                            }'
--- FAIL: TestRecurseManifestsInDirAppSet (0.01s)

FAIL

Expected behavior

JSON files containing non-Kubernetes resources should not be attempted to be unmarshalled.

Screenshots

Version

❯ argocd version
argocd: v2.4.15+05acf7a.dirty
  BuildDate: 2022-10-17T22:52:44Z
  GitCommit: 05acf7a52e377eacfee29c68e3e5e79a172ea013
  GitTreeState: dirty
  GoVersion: go1.19.2
  Compiler: gc
  Platform: darwin/amd64
argocd-server: v2.4.14+029be59
  BuildDate: 2022-10-05T17:15:37Z
  GitCommit: 029be590bfd5003d65ddabb4d4cb8a31bff29c18
  GitTreeState: clean
  GoVersion: go1.18.7
  Compiler: gc
  Platform: linux/amd64
  Kustomize Version: v4.5.5 2022-05-20T20:25:42Z
  Helm Version: v3.8.1+g5cb9af4
  Kubectl Version: v0.23.1
  Jsonnet Version: v0.18.0

Logs

rpc error: code = Unknown desc = Manifest generation error (cached): rpc error: code = FailedPrecondition desc = Failed to unmarshal "config.json": Object 'Kind' is missing in '{ "name": "<redacted cluster name>", "version": "HEAD" }'

[crenshaw-dev]

@rumstead the slight downside to the proposed change is that you won't get the current "oops you forgot to set kind" error you will now. I think it's a very small downside, but worth considering.

Would setting the exclude field to ignore non-manifest JSON do the job?

[rumstead]

@rumstead the slight downside to the proposed change is that you won't get the current "oops you forgot to set kind" error you will now. I think it's a very small downside, but worth considering.

Agreed, you wouldn't. IMHO, the difference in behavior between JSON and YAML was confusing and a little misleading.

Would setting the exclude field to ignore non-manifest JSON do the job?

The short answer is yes but painfully at scale.

The long answer is we have a "bootstrap" repo that contains all our developer's application sets. We gatekeep this repo because of some security concerns with application sets (really applications) and the ability to release to production. We have OPA policies and automated checks so team leads can approve PRs. I give that background so readers don't think I am crazy :). In addition to holding the application sets, they also hold any config files that the application sets can reference using the git generators. Our folder structure is more something like the one below.

├── app1
|   ├── appset.yaml  
│   ├── dev
│   │   └── config.yaml
│   ├── qa
│   │   └── config.yaml
│   └── tst
│       └── config.yaml
└── app2
    ├── appset.yaml  
    ├── dev
    │   └── config.yaml
    ├── qa
    │   └── config.yaml
    └── tst
        └── config.yaml

Here is where the issue lies. Someone decided to use JSON over YAML and we ran into the issue. So yes, we could come up with a standard and exclude using globs but it feels brittle and hard to scale.

Maybe we are using application sets incorrectly.

[cleverhu]

@crenshaw-dev @rumstead I pulled a request for this, what do you think of it?

[rumstead]

@crenshaw-dev @rumstead I pulled a request for this, what do you think of it?

looks better than mine

[clearlybaffled]

Just stumbled across this since I was having the same issue, but it didn't looked like it was merged yet.. The docs mention that

A directory-type application loads plain manifest files from .yml, .yaml, and .json files.

so it seemed like a way around it would be by naming my ApplicationSet config files so something other than config.json.. i went with config.js. Works great now. The config file and regular manifests get picked up all together.