fix: CMPv2 does not allow symlinks to adjacent files in same git repo. Fixes #13342 #13360
Conversation
…Fixes argoproj#13342 Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com>
jiachengxu
changed the title
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #13360 +/- ##
=======================================
Coverage 49.14% 49.15%
=======================================
Files 248 248
Lines 42891 42891
=======================================
+ Hits 21079 21082 +3
+ Misses 19693 19691 -2
+ Partials 2119 2118 -1
☔ View full report in Codecov by Sentry. |
Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com>
Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com>
Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com>
jiachengxu
force-pushed
the
fix-13342
branch
2 times, most recently
from
c3cd3e7
to
b3610bc
Compare
|
Can you check why |
| @@ -7,4 +7,4 @@ spec: | |||
| generate: | |||
| command: [sh, -c, 'echo "{\"kind\": \"ConfigMap\", \"apiVersion\": \"v1\", \"metadata\": { \"name\": \"$ARGOCD_APP_NAME\", \"namespace\": \"$ARGOCD_APP_NAMESPACE\", \"annotations\": {\"Foo\": \"$FOO\", \"KubeVersion\": \"$KUBE_VERSION\", \"KubeApiVersion\": \"$KUBE_API_VERSIONS\",\"Bar\": \"baz\"}}}"'] | |||
| discover: | |||
| fileName: "subdir/s*.yaml" | |||
| fileName: "cmp-fileName/subdir/s*.yaml" | |||
There was a problem hiding this comment.
Can you check why TestCMPDiscoverWithFileName is failing please ? Looks like it is related to changes and not flakiness.
@alexmt It turned out that this test failed because of the test case is wrong.
As stated on the config management plugin doc, the filename should be applied to the repository root not the application source:
fileName is a glob pattern (https://pkg.go.dev/path/filepath#Glob) that is applied to the repository's root directory (not the Application source directory). If there is a match, this plugin may be used for the repository.
and in the test case, the testdata is the repo, so we need to have cmp-fileName as part of the fileName.
It was not caught because we only passed appPath but not repoPath so the appPath was considered as repoPath and cmp-fileName was not needed.
There was a problem hiding this comment.
This change is causing issues, so I'm going to try to revert this change without reverting your whole bugfix. #13940
| @@ -0,0 +1 @@ | |||
| ../guestbook/guestbook-ui-deployment.yaml | |||
There was a problem hiding this comment.
I have to put all test files related to symlinks to a dedicated folder testdata2 otherwise it will fail other tests with repository contains out-of-bounds symlinks errors. I think that is because in the codebase we deny all absolute symlinks (https://github.com/argoproj/argo-cd/blob/master/util/app/path/path.go#L67), however, in our tests, repo is located in /tmp/ folder and that means all files are absolute paths.
|
/cherry-pick release-2.6 |
|
Cherry-pick failed with |
|
/cherry-pick release-2.7 |
…Fixes argoproj#13342 (argoproj#13360) fix: CMPv2 does not allow symlinks to adjacent files in same git repo. Fixes argoproj#13342 (argoproj#13360) Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com>
…Fixes argoproj#13342 (argoproj#13360) (argoproj#13669) fix: CMPv2 does not allow symlinks to adjacent files in same git repo. Fixes argoproj#13342 (argoproj#13360) Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com> Co-authored-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com> Signed-off-by: schakrad <58915923+schakrad@users.noreply.github.com>
…Fixes argoproj#13342 (argoproj#13360) fix: CMPv2 does not allow symlinks to adjacent files in same git repo. Fixes argoproj#13342 (argoproj#13360) Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com>
Fixes: #13342
This PR fixes the issue that CMP v2 doesn't work with applications that have symlinks to adjacent file in the same git repo.
The root cause is that when calling
detectConfigManagementPlugin, we should pass repo path and app path, otherwise, the check can fail since the symlinks can be pointing to files out of the app path but in the repo path.Note on DCO:
If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.
Checklist:
Please see Contribution FAQs if you have questions about your pull-request.