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

kustomize 3.5.5 breaks installing ArgoCD from URL #2538

Closed
chancez opened this issue May 26, 2020 · 36 comments
Closed

kustomize 3.5.5 breaks installing ArgoCD from URL #2538

chancez opened this issue May 26, 2020 · 36 comments
Labels
area/dependency Issues or PRs related to dependency changes kind/bug Categorizes issue or PR as related to a bug. triage/needs-information Indicates an issue needs more information in order to work on it.

Comments

@chancez
Copy link
Contributor

chancez commented May 26, 2020

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 chancez changed the title Kustomize 3.5.5 breaks installing ArgoCD from URL kustomize 3.5.5 breaks installing ArgoCD from URL May 26, 2020
@chancez
Copy link
Contributor Author

chancez commented May 26, 2020

May be related to #2494

@monopole
Copy link
Contributor

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
Copy link
Contributor Author

chancez commented May 27, 2020

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
Copy link
Contributor Author

chancez commented May 27, 2020

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

@Shell32-Natsu
Copy link
Contributor

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
Copy link
Contributor

seans3 commented May 27, 2020

Re-opening so that we can triage it.

/reopen

@k8s-ci-robot
Copy link
Contributor

@seans3: Reopened this issue.

In response to this:

Re-opening so that we can triage it.

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot reopened this May 27, 2020
@chancez
Copy link
Contributor Author

chancez commented May 27, 2020

@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
Copy link
Contributor

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

@jameshochadel
Copy link

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
Copy link
Contributor

Shell32-Natsu commented Jun 1, 2020

@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 monopole added the triage/needs-information Indicates an issue needs more information in order to work on it. label Jun 1, 2020
@monopole
Copy link
Contributor

monopole commented Jun 1, 2020

@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
Copy link

@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
Copy link
Contributor

@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
Copy link

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

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 6, 2020
@docwhat
Copy link

docwhat commented Sep 30, 2020

/remove-lifecycle stale

@thatsmydoing
Copy link
Contributor

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
Copy link
Contributor

Shell32-Natsu commented Nov 16, 2020

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Member

yujunz commented Nov 18, 2020

A brief history of go-getter support

  1. a full version of go-getter library was integrated in kustomize at the very beginning
  2. go-getter was replaced by git exec later on because of massive transitive dependency
  3. a script based plugin was created to integrate go-getter as binary
  4. 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
Copy link
Member

yujunz commented Nov 18, 2020

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
Copy link
Contributor

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
Copy link
Member

yujunz commented Nov 19, 2020

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
Copy link
Member

yujunz commented Nov 19, 2020

@yujunz
Copy link
Member

yujunz commented Nov 20, 2020

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

Proposed a fix in #3253

@HariSekhon
Copy link

HariSekhon commented Jun 23, 2021

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
Copy link
Contributor

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
Copy link

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
Copy link
Contributor

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

@HariSekhon
Copy link

HariSekhon commented Jun 24, 2021

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
Copy link
Contributor

KnVerey commented Jul 12, 2022

/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
Copy link
Contributor

@KnVerey: Closing this issue.

In response to this:

/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. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot removed the lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. label Jul 12, 2022
@ZiaUrRehman-GBI
Copy link

ZiaUrRehman-GBI commented Dec 1, 2022

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'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dependency Issues or PRs related to dependency changes kind/bug Categorizes issue or PR as related to a bug. triage/needs-information Indicates an issue needs more information in order to work on it.
Projects
None yet
Development

No branches or pull requests