kustomize 3.5.5 breaks installing ArgoCD from URL

chancez commented 4 years ago

With kustomize 3.5.4, the following command succeeds and produces the YAML as expected:

kustomize build 'github.com/argoproj/argo-cd//manifests/ha/cluster-install?ref=v1.5.2

With kustomize 3.5.5 I now get the following error when running the command:

$ kustomize build 'github.com/argoproj/argo-cd//manifests/ha/cluster-install?ref=v1.5.2'
Error: accumulating resources: accumulateFile "accumulating resources from '../namespace-install': evalsymlink failure on '/private/var/folders/0_/gjp5mdvn64lgly0qsrlzq1vc0000gp/T/namespace-install' : lstat /private/var/folders/0_/gjp5mdvn64lgly0qsrlzq1vc0000gp/T/namespace-install: no such file or directory", loader.New "Error loading ../namespace-install with git: url lacks host: ../namespace-install, dir: evalsymlink failure on '/private/var/folders/0_/gjp5mdvn64lgly0qsrlzq1vc0000gp/T/namespace-install' : lstat /private/var/folders/0_/gjp5mdvn64lgly0qsrlzq1vc0000gp/T/namespace-install: no such file or directory, get: invalid source string: ../namespace-install"

chancez commented 4 years ago

monopole commented 4 years ago

likely a dupe of #2444. Fix is in #2537, which is in v3.6.1 please give it a try. reopen if it doesn't work.

@Shell32-Natsu FYI

chancez commented 4 years ago

This doesn't seem resolved:

~/p/kubernetes-gitops(⎈ |dev-2447-us-west-2:jupyterhub) czibolski ❯❯❯ ~/Downloads/kustomize version                                                                          ⏎staging_prod_shared_logging_overlay ✭
{Version:kustomize/v3.6.1 GitCommit:c97fa946d576eb6ed559f17f2ac43b3b5a8d5dbd BuildDate:2020-05-27T20:47:35Z GoOs:darwin GoArch:amd64}
~/p/kubernetes-gitops(⎈ |dev-2447-us-west-2:jupyterhub) czibolski ❯❯❯ ~/Downloads/kustomize build 'github.com/argoproj/argo-cd//manifests/ha/cluster-install?ref=v1.5.2'      staging_prod_shared_logging_overlay ✭
Error: accumulating resources: accumulateFile "accumulating resources from '../namespace-install': evalsymlink failure on '/private/var/folders/0_/gjp5mdvn64lgly0qsrlzq1vc0000gp/T/kustomize-731707729/namespace-install' : lstat /private/var/folders/0_/gjp5mdvn64lgly0qsrlzq1vc0000gp/T/kustomize-731707729/namespace-install: no such file or directory", loader.New "Error loading ../namespace-install with git: url lacks host: ../namespace-install, dir: evalsymlink failure on '/private/var/folders/0_/gjp5mdvn64lgly0qsrlzq1vc0000gp/T/kustomize-731707729/namespace-install' : lstat /private/var/folders/0_/gjp5mdvn64lgly0qsrlzq1vc0000gp/T/kustomize-731707729/namespace-install: no such file or directory, get: invalid source string: ../namespace-install"

chancez commented 4 years ago

And I cannot seem to re-open this issue either.

Shell32-Natsu commented 4 years ago

I think the issue is different from #2444. Kustomize has changed to use go-getter by default. From the documentation of go-getter:

If you want to download only a specific subdirectory from a downloaded directory, you can specify a subdirectory after a double-slash //. go-getter will first download the URL specified before the double-slash (as if you didn't specify a double-slash), but will then copy the path after the double slash into the target directory.

So only manifests/ha/cluster-install will be copied and ../namespace-install is not there. You can try to replace // with /.

seans3 commented 4 years ago

Re-opening so that we can triage it.

/reopen

k8s-ci-robot commented 4 years ago

@seans3: Reopened this issue.

In response to [this](https://github.com/kubernetes-sigs/kustomize/issues/2538#issuecomment-634972250): >Re-opening so that we can triage it. > >/reopen Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.

chancez commented 4 years ago

@Shell32-Natsu That does seem to make a difference. I'll try to make this change to upstream argo-cd install docs so that it's hopefully avoided for others. Though I imagine this might be an issue for other remote bases/overlays that people could be using.

monopole commented 4 years ago

Thanks @chancez and @seans3, and thanks @Shell32-Natsu for taking this on.

jameshochadel commented 4 years ago

I am encountering this issue on 3.6.1, but I am not using // anywhere in my URLs. However, the issue only occurs when the protocol is not specified.

E.g., where my-repo/dev/kustomization.yaml references a resource ../base, this fails:

kustomize build my-github-org/my-repo/dev

but this succeeds:

kustomize build https://my-github-org/my-repo/dev

This was not previously (before 3.6.1) the case.

Shell32-Natsu commented 4 years ago

@jameshochadel Hi James, thanks for the feedback. It looks like this is a go-getter issue. I have created issue here.

For your case, the problem is go-getter has bug with sub directory. Adding https works since go-getter fails with https scheme as well (because it's ambiguous I guess) and a normal git clone happens.

And the reason for why @chancez can fix the issue by replace // with / is go-getter doesn't work when you are using / and ref at the same time. So actually there is a normal git clone as well.

I am surprised that go-getter has these problems. I think we may need to clearly document the url pattern if we want to continue using go-getter.

monopole commented 4 years ago

@jameshochadel

please provide

provide the output of the tree command,
include the contents of the kustomization files (at least the resources: and/or bases: field)
full command and value of working directory when running kustomize build
error message

jameshochadel commented 4 years ago

@monopole

Here is a repo that reproduces the issue: https://github.com/jameshochadel/kustomize-issue-2538

Instructions are included in the readme (just running kustomize build against the repo with and without the protocol specified).

I think that covers everything. tree for completeness:

.
├── README.md
├── base
│   └── kustomization.yaml
├── dev
│   └── kustomization.yaml
├── no-https.sh
└── with-https.sh

Shell32-Natsu commented 4 years ago

@jameshochadel My speculation was correct. In no-https.sh , go-getter finishes with no error and only download the path (currently, / and // work nearly same in go-getter, which is different from their doc). If you add https, the go-getter fails and kustomize will clone the entire repo.

fejta-bot commented 4 years ago

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /lifecycle stale

docwhat commented 4 years ago

/remove-lifecycle stale

thatsmydoing commented 3 years ago

Is it even possible using valid go-getter syntax to work with relative imports from parent directories? In the example, using github.com/jameshochadel/kustomize-issue-2538/dev results in only getting the dev folder which would cause referencing ../base to fail.

For now, the workaround seems to be to use a url that go-getter rejects so that it falls back to kustomize's old git clone strategy, but how is this supposed to actually work?

Should kustomize properly resolve ../base relative to the original url? In this case, to github.com/jameschochadel/kustomize-issue-2538/base? This seems like how I would expect it to work.

Otherwise, it seems like we need to specify 2 separate things, the go-getter url to fetch, and then the subdirectory within to use for the build.

Shell32-Natsu commented 3 years ago

@thatsmydoing Copy directory dev from the repo github.com/jameshochadel/kustomize-issue-2538. That is how go-getter works now. As they said in my issue these problems may be addressed in go-getter v2 but v2 is not released yet.

thatsmydoing commented 3 years ago

I think we're talking about 2 different things. The problem I'm trying to highlight is that when you have a resource like github.com/jameshochadel/kustomize-issue-2538//dev, what go-getter does is give you just the dev folder hence resources like ../base will fail. The issue linked seems to just be fixing issues where it's not parsing urls correctly, but in the end doesn't change the behavior of only getting the subdirectory.

What I instead usually want is to copy the whole repo github.com/jameshochadel/kustomize-issue-2538 and then build the kustomization at folder dev. This is how the cloner does it and that's why putting in an invalid go-getter url works.

I want to clarify because it all seems by chance that any of this works at all. Using go-getter syntax necessarily means that we are only getting the contents of a subdirectory (which does not work for anything that remotely uses the recommended kustomize project structure).

The way I see it, there are a couple ways forward here:

separately specify what we want copied with go-getter and then the subdirectory we want to use inside what was copied
make the loader smart by taking into account the parent loader when resolving relative imports

As it is now, saying that kustomize uses go-getter syntax just causes confusion because actually using legitimate go-getter syntax breaks anything that uses relative imports. This also means that when go-getter starts accepting more urls that it currently doesn't now, we'll run into this entire issue again of kustomize breaking relative imports.

Shell32-Natsu commented 3 years ago

Since go-getter has some annoying issues like this, I am happy to remove it from kustomize temporarily. What I am not sure about is the performance reduction that caused by calling external git exec. And also user will need to have a git executable in they PATH.

Perhaps using a go git library like go-git is better.

thatsmydoing commented 3 years ago

I'm not proposing getting rid of go-getter at all, removing it would probably break someone else's workflow which I think we want to avoid doing any more of. Note that go-getter itself uses external git so I'm not sure there's that much benefit to using go-git.

I think it would be better to aim for a proper fix that is consistent. I believe that requires clarifying the semantics of what should happen when you have remote resources and what happens with parent imports.

thatsmydoing commented 3 years ago

I did a bit more digging into this and I might be more persuaded to just drop go-getter at this point. Here is how kustomize currently handles fetching certain urls with go-getter. All of these urls work with the cloner. I don't know what the extent of usage of the following urls are, but I've certainly used some variation of these that doesn't work with go-getter.

URL	go-getter result
github.com/jameshochadel/kustomize-issue-2538	fetch success
github.com/jameshochadel/kustomize-issue-2538/	fetch success
github.com/jameshochadel/kustomize-issue-2538/dev	fetch success
github.com/jameshochadel/kustomize-issue-2538//dev	fetch success
https://github.com/jameshochadel/kustomize-issue-2538	wrong fetch
https://github.com/jameshochadel/kustomize-issue-2538/	fallback to cloner
https://github.com/jameshochadel/kustomize-issue-2538/dev	fallback to cloner
https://github.com/jameshochadel/kustomize-issue-2538//dev	wrong fetch
git@github.com:jameshochadel/kustomize-issue-2538	fetch success
git@github.com:jameshochadel/kustomize-issue-2538/	fallback to cloner
git@github.com:jameshochadel/kustomize-issue-2538/dev	fallback to cloner
git@github.com:jameshochadel/kustomize-issue-2538//dev	fetch success
git@github.com/jameshochadel/kustomize-issue-2538	fallback to cloner
git@github.com/jameshochadel/kustomize-issue-2538/	fallback to cloner
git@github.com/jameshochadel/kustomize-issue-2538/dev	fallback to cloner
git@github.com/jameshochadel/kustomize-issue-2538//dev	fallback to cloner
ssh://git@github.com:jameshochadel/kustomize-issue-2538	fallback to cloner
ssh://git@github.com:jameshochadel/kustomize-issue-2538/	fallback to cloner
ssh://git@github.com:jameshochadel/kustomize-issue-2538/dev	fallback to cloner
ssh://git@github.com:jameshochadel/kustomize-issue-2538//dev	fallback to cloner
ssh://git@github.com/jameshochadel/kustomize-issue-2538	fallback to cloner
ssh://git@github.com/jameshochadel/kustomize-issue-2538/	fallback to cloner
ssh://git@github.com/jameshochadel/kustomize-issue-2538/dev	fallback to cloner
ssh://git@github.com/jameshochadel/kustomize-issue-2538//dev	fallback to cloner
git::ssh://git@github.com/jameshochadel/kustomize-issue-2538	fetch success
git::ssh://git@github.com/jameshochadel/kustomize-issue-2538/	fallback to cloner
git::ssh://git@github.com/jameshochadel/kustomize-issue-2538/dev	fallback to cloner
git::ssh://git@github.com/jameshochadel/kustomize-issue-2538//dev	fetch success

This means that even with #3244, the following urls will still be broken

https://github.com/jameshochadel/kustomize-issue-2538
https://github.com/jameshochadel/kustomize-issue-2538//dev

go-getter does not treat these as git repositories but as just standard http downloads.

There can obviously be more variations of this, and this doesn't cover the special handling kustomize has for azure and AWS repositories. It has been suggested that the special handling can be made into a go-getter detector but this doesn't perfectly work either because go-getter treats urls with schemes (https://, ssh://) as already normalized so they don't get transformed anymore. Additionally, even if kustomize worked around this by transforming the url itself before passing it to go-getter then there doesn't seem to be much point using go-getter at all.

Where go-getter works but the cloner doesn't is for archive downloads, but even then the usage of the feature is not obvious.

URL	go-getter result
https://github.com/jameshochadel/kustomize-issue-2538/archive/master.zip	wrong fetch
https://github.com/jameshochadel/kustomize-issue-2538/archive/master.zip//kustomize-issue-2538-master	fetch success

I see that @yujunz added go-getter into kustomize, maybe they could shed more light on how they use this?

yujunz commented 3 years ago

A brief history of go-getter support

a full version of go-getter library was integrated in kustomize at the very beginning
go-getter was replaced by git exec later on because of massive transitive dependency
a script based plugin was created to integrate go-getter as binary
a lite version of go-getter is integrated into kustomize

In fact go-getter can support git clone. But we tried to keep backward compatibility to old cloner URLs when implementing step 4. I think that could be one reason of the chaos we are facing. Dropping go-getter will probably break the backward compatibility to current version.

Let me have a look of current implementation to see what is the best way out.

yujunz commented 3 years ago

I see that @yujunz added go-getter into kustomize, maybe they could shed more light on how they use this?

It was tested against the examples in https://github.com/kubernetes-sigs/kustomize/blob/master/examples/remoteBuild.md#url-format . But I roughly remembered the example tests were once excluded from CI. Not sure if it is covered now or not.

See also some history conversion in the two PRs for the problems go-getter tries to resolve and the tradeoffs

thatsmydoing commented 3 years ago

Dropping go-getter will probably break the backward compatibility to current version.

Aside from archive downloads and urls that use the depth query parameter, I don't think any url will be broken if go-getter is removed.

Either way though, I do think kustomize needs to be clear what the supported urls are. If the official position is that only go-getter urls are supported, then we would have just dropped the old cloner entirely but that didn't pan out. Some people have suggested removing go-getter but even then, there should be a spec as to what urls are supported since it accepts way too many variations and any kind of refactoring would be difficult without breaking someone's workflow. The repospec code is very convoluted and it's difficult to tell what kinds of urls it accepts and how it transforms them.

As a user, I just want to know what url formats are supported and that I can reasonably expect to never break and that kustomize will immediately fix if broken.

yujunz commented 3 years ago

Either way though, I do think kustomize needs to be clear what the supported urls are.

Yeah. It should be documented and covered by integration test.

We may need an exhaustive list in https://github.com/kubernetes-sigs/kustomize/blob/master/examples/remoteBuild.md#url-format which is covered by https://github.com/kubernetes-sigs/kustomize/blob/master/hack/testExamplesAgainstKustomize.sh

yujunz commented 3 years ago

The test seems triggered by https://github.com/kubernetes/test-infra/blob/master/config/jobs/kubernetes-sigs/kustomize/kustomize-presubmit-master.yaml before submit any PR to master.

yujunz commented 3 years ago

I just realized the tests in remote build were skipped because of an extra blank line...

Proposed a fix in https://github.com/kubernetes-sigs/kustomize/pull/3253

HariSekhon commented 3 years ago

I had this issue on ArgoCD 2.0.3 with Kustomize 3.9.4

I solved it by just replacing the base url double slashes // from the docs with a single slash / in my kustomization.yaml- I tried this with and without https:// protocol prefix and both worked.

kustomization.yaml:

bases:
#- github.com/argoproj/argo-cd//manifests/cluster-install?ref=v2.0.3
- github.com/argoproj/argo-cd/manifests/cluster-install?ref=v2.0.3

EDIT: upgrading Kustomize to 4 in argocd-repo-server also solves this problem - I include this patch in my kustomizaton.yaml to get a specific version of Kustomize now.

thatsmydoing commented 3 years ago

Are you sure it's not working for kustomize 4.1.3? I tried it locally and both urls work. Pre version 4, you might be hitting the go-getter path which is known to be broken for certain urls.

HariSekhon commented 3 years ago

It worked locally in Kustomize 4.1.3 but not in ArgoCD, even when I upgraded the Kustomize version inside the container via an init container shared emptyDir as seen in the patch I published above. I even exec'd into the argocd-server container and did a Kustomize version and it showed 4.1.3

thatsmydoing commented 3 years ago

I think evaluation happens in argocd-repo-server instead. Which is probably still using the old kustomize.

HariSekhon commented 3 years ago

Yeah I just woke up this morning and realized that! Will adjust the patch... I knew something was off.

Updated my patch and comment above with both solutions.

KnVerey commented 2 years ago

/remove-lifecycle frozen /close

Closing since this is about a very old version of Kustomize, and comments indicate it was caused by a dependency we no longer have.

k8s-ci-robot commented 2 years ago

@KnVerey: Closing this issue.

In response to [this](https://github.com/kubernetes-sigs/kustomize/issues/2538#issuecomment-1182551346): >/remove-lifecycle frozen >/close > >Closing since this is about a very old version of Kustomize, and comments indicate it was caused by a dependency we no longer have. Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.

ZiaUrRehman-GBI commented 1 year ago

Kustomize version {Version:kustomize/v4.5.7 GitCommit:56d82a8378dfc8dc3b3b1085e5a6e67b82966bd7 BuildDate:2022-08-02T16:28:01Z GoOs:darwin GoArch:arm64}

 kustomize build .      
github.com/argoproj/argo-cd/manifests/ha/cluster-install?ref=v2.5.2

Error: accumulating resources: accumulation err='accumulating resources from 'github.com/argoproj/argo-cd/manifests/ha/cluster-install?ref=v2.5.2': evalsymlink failure on ...
no such file or directory': hit 27s timeout running '/usr/bin/git fetch --depth=1 origin v2.5.2'

kustomize build .  
https://github.com/argoproj/argo-cd/manifests/ha/cluster-install?ref=v2.5.2

Error: accumulating resources: accumulation err='accumulating resources from 'https://github.com/argoproj/argo-cd/manifests/ha/cluster-install?ref=v2.5.2': URL is a git repository': hit 27s timeout running '/usr/bin/git fetch --depth=1 origin v2.5.2'

kubernetes-sigs / kustomize

kustomize 3.5.5 breaks installing ArgoCD from URL #2538