carvel-dev / vendir

Easy way to vendor portions of git repos, github releases, helm charts, docker image contents, etc. declaratively
https://carvel.dev/vendir
Apache License 2.0
268 stars 46 forks source link

vendir sync --json swallows error #357

Open hans-d opened 5 months ago

hans-d commented 5 months ago

What steps did you take:

What happened: produces no error, but does exit with code 1

What did you expect: error message like without --json, in json and/or to stderr

Environment:


Vote on this request

This is an invitation to the community to vote on issues, to help us prioritize our backlog. Use the "smiley face" up to the right of this comment to vote.

👍 "I would like to see this addressed as soon as possible" 👎 "There are other more important things to focus on right now"

We are also happy to receive and review Pull Requests if you want to help working on this issue.

kumaritanushree commented 5 months ago

@hans-d I tried to reproduce the issue with config

apiVersion: vendir.k14s.io/v1alpha1
kind: Config
directories:
- path: sponnet
  contents:
  - path: xyz
    http:
      url: https://github.com/spinnaker/sponnet/archive/refs/tags/v1.26.0.tar.gz123

First I run without --json and got error message in output:

$ vendir sync -f vendir.yaml 
Fetching: sponnet + xyz (http from https://github.com/spinnaker/sponnet/archive/refs/tags/v1.26.0.tar.gz123)

vendir: Error: Syncing directory 'sponnet':
  Syncing directory 'xyz' with HTTP contents:
    Downloading URL:
      Expected 200 OK, but was '404 Not Found'

With --json, it exited without any error or any log:

$ vendir sync -f vendir.yaml --json

Did you get something different output if yes can you please add here your config and steps you followed? (It will be helpful in future)

After trying out above config I can see there is bug in vendir sync --json output. In case of failure with flag --json as well it should show error message.

hans-d commented 5 months ago

This is the kind of thing i'm facing. It should produce an error (stderr or in json), and exit with a non-zero status

Zebradil commented 5 months ago

@Zebradil what do you think about this?

@100mik I think that, in case of an error, vendir --json should:

joaopapereira commented 5 months ago

Hey @hans-d can you provide a sample vendir.yaml for us to look at? It sounds like you have ../ as part of a path, is that correct?

hans-d commented 5 months ago

The '../' is part of the source directory:

apiVersion: vendir.k14s.io/v1alpha1
kind: Config

directories:

  - path: somewhere
    contents:
      - path: over/the/rainbow
        directory:
          path: ../src/foo # non-existing
renuy commented 5 months ago

This issue is seen in kbld & imgpkg too?

hans-d commented 5 months ago

Not using those, so can't say.

kumaritanushree commented 4 months ago

Same issues seen in imgpkg and kbld as well. Issues created are: https://github.com/carvel-dev/kbld/issues/455 https://github.com/carvel-dev/imgpkg/issues/631