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

fix(argocd_cluster): use cluster list api to avoid 403 issues with cluster get api at cluster read time #399

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

w4rgrum
Copy link

@w4rgrum w4rgrum commented Jun 12, 2024

This PR fixes #266

Coded it with below comment in mind, meaning it is backward compatible: #266 (comment)

  • code does a Get as usual first (legacy code)
  • if it fails with a PermissionDenied error (i.e. 403) the fix is triggered (in a backward compatible way for servers <v2.8, see below)
    In that case a call is issued to the List api, reusing some code used for create cluster existence checks (that already uses this List api). Noe that this code is able to filter the cluster in the resulting list if not yet done by the ArgoCD server (servers filtering on List api does not work if <v2.8, see comments in linked issue and/or in the code ccomments)

Also added a non-regression test to validate a server-side deletion of the cluster triggers the fix.

@w4rgrum w4rgrum force-pushed the fix/argocd_cluster/issue-266-use-list-api-to-read branch 5 times, most recently from d2ffd2a to d5d059c Compare June 13, 2024 14:01
…uster get api at cluster read time

Signed-off-by: Hugo HARS <54102154+w4rgrum@users.noreply.github.com>
@w4rgrum w4rgrum force-pushed the fix/argocd_cluster/issue-266-use-list-api-to-read branch from d5d059c to ca58239 Compare June 13, 2024 14:11
@w4rgrum
Copy link
Author

w4rgrum commented Jun 13, 2024

@onematchfox PR is ready to be reviewed :)

@morhekil
Copy link

any chance to have this released as new version of the provider? This seems to be a blocker when using the provider with recent ArgoCD versions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ArgoCD clusters should list to find the cluster instead of get and check for NotFound
2 participants