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

vendir succeeds when includePath list item does not exist? #114

Open
caphrim007 opened this issue Sep 29, 2021 · 2 comments
Open

vendir succeeds when includePath list item does not exist? #114

caphrim007 opened this issue Sep 29, 2021 · 2 comments
Labels
awaiting-input discussion This issue is not a bug or feature and a conversation is needed to find an appropriate resolution

Comments

@caphrim007
Copy link

imho this is a bug, but I'm asking first in case this is a "functions as designed". If it is functioning as designed, then might I humbly request that design be revisited.

I have a config which is similar to what is shown below (git repos are different)

---
apiVersion: vendir.k14s.io/v1alpha1
kind: Config
directories:
- path: .vendir
  contents:
    - path: service
      git:
        url: git@github.com:org/software.git
        ref: 40ef6a
      includePaths:
        - k8s/modules/base/*
        - this/*
        - does/not/exist/*

I can guarantee that there are files in the first list item (the k8s path). The other two paths I made up.

When I run vendir sync, the output that I get is

Fetching: .vendir + service (git from git@github.com:org/software.git@40ef6a)

  --> git init
 ...bunch of git command output here...


Lock config

apiVersion: vendir.k14s.io/v1alpha1
directories:
- contents:
  - git:
      commitTitle: 'fix: remove sharder annotations'
      sha: 40ef6a85a4fa84a23a0193db9ea2773ba3b5614e
    path: service
  path: .vendir
kind: LockConfig

Succeeded

I can verify this is successful with the output of $? as well

$ echo $?
0

Is this expected behavior?

If so, I think this can very likely lead to some (bad) unintended consequences. The worst being that the end-user trusts that vendir will arrange some files correctly, when it actually doesn't.

For example, consider kubernetes manifests via ytt being vendir'd. Due to "reasons" (network hiccup, fat fingered path, path not existing at a particular version, etc) the paths include invalid paths. vendir produces a set of files and ignores the paths it couldn't find. k8s app gets deployed without a subset of content. The world spirals into chaos.

Thoughts?

@caphrim007 caphrim007 added the carvel-triage This issue has not yet been reviewed for validity label Sep 29, 2021
@cppforlife
Copy link
Contributor

@caphrim007 currently that's intentional behaviour.

Due to "reasons" (network hiccup, fat fingered path, path not existing at a particular version, etc) the paths include invalid paths. vendir produces a set of files and ignores the paths it couldn't find

network hiccup would result in a much earlier failure at asset download time. includePaths/excludePaths are filters are all has been successfully downloaded.

making a mistake is definitely a possibility though. i dont think we can change default behaviour at this point; but, ill have to think about how to enhance this feature to provide you with extra safety. (globs are typically meant to represent 0+, but in your case, you really want 1+.)

@github-actions
Copy link

This issue is being marked as stale due to a long period of inactivity and will be closed in 5 days if there is no response.

@github-actions github-actions bot added the stale This issue has had no activity for a while and will be closed soon label Nov 11, 2021
@cppforlife cppforlife added discussion This issue is not a bug or feature and a conversation is needed to find an appropriate resolution and removed carvel-triage This issue has not yet been reviewed for validity stale This issue has had no activity for a while and will be closed soon labels Nov 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-input discussion This issue is not a bug or feature and a conversation is needed to find an appropriate resolution
Projects
Status: Unprioritized
Development

No branches or pull requests

3 participants