-
Notifications
You must be signed in to change notification settings - Fork 90
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
Add compatiblity for "source_namespaces" introduced in ArgoCD v2.5.0 #212
Conversation
…engine v0.7.1-0.20220804190909-2bc3fef13e07
So I just came across this issue: argoproj/gitops-engine#447, and proceeded to pin the gitops-engine version as follows:
With this it compiles correctly. Removing WIP tag. EDIT: No, keeping WIP for now, hasn't been tested properly yet of course! |
Ok, I've now added Tests all run through for me locally. |
I've run into an issue in live testing which isn't picked up by the acceptance tests. To read an application it needs to now be referenced as In other words, queries such as this:
will have to be updated to:
I didn't build any acceptance tests that specifically deploy to other namespaces. The defaults ones still passed because they all deploy to the root namespace and can therefore just use the shorthand |
I accidentally added source_namespaces to schemas V0 and V1. Have removed it now. |
Ok, I've now dealt with the fact that Applications need to be referenced as To support this I also updated the test environment, as follows:
I've also updated the GitHub Actions pipeline to use ArgoCD 2.5.0. Not sure if I am supposed to leave the old ones in though? It fails for some reason though; I am not sure why. It seems to not be able to resolve the argocd Go package. Local tests run through. My live testing also hasn't revealed any further issues. |
I have now bumped the Go version to 1.19. This resolves the issue with not being able to find the the right go package. Acceptance tests now actually run, but there are some failures. I don't really get why the "argocd_cluster" tests are failing, unless there is something fundamentally different in 2.5.0 there that I haven't accounted for... EDIT: just to be sure I've redone my local testing env and run acceptance tests again. Locally everything runs through. |
Have now added back the tests on the older ArgoCD versions. I am pretty sure I am missing something here. How do I set it up so that certain tests only run for certain versions? And am I actually supposed to create a new applicationSpecSchema (v3), since the spec has now has an additional field? |
Hi @karlschriek, Thanks for taking this on. I had hoped to get to looking at this in more detail today but just didn't get the chance. However, I see you have already picked up on the first issue - upgrading the |
Just seen that I missed a bunch of comments and additional work since I starting writing that comment - which now isn't necessarily valid. Hopefully my reference to |
@onematchfox yeah, the "argocd_cluster" cluster test problem seems to have gone away. There are other things failing though, in particular on the older versions, although 2.5.0 also fails (although in the latest run it fails on something to do with TLS, for which there is a comment that this doesn't always work). I think it is almost there now, but two things in particular I am having a hard time figuring out and it might actually be more productive if someone closer to the project takes a look at them:
I've also just removed the Anyway, I am going to leave it there for now. |
@onematchfox have you had a chance to look at this yet? |
- Mark source_namespaces as Optional - Add feature/version constraint - Add test for project with source namespaces
4c7b0d9
to
21e8a56
Compare
HI @karlschriek, Apologies for the delay. I have been trying to look at this when/where possible but between travelling and falling ill I'm afraid I haven't had much time to dedicate to it.
I've pushed a commit that fixes up some of the changes to the project schema although this is by no means complete yet. To specifically address your points.
Outstanding from my side at the moment is wrapping my head around the rest of the changes. Namely:
|
aa810d5
to
3d84814
Compare
To be able to import existing resources in custom namespaces we need to store the namespace as part of the ID on the ArgoCD application resource.
@karlschriek Think I finally managed to get everything working here. @oboukili, do you want to take a look at this to double-check I haven't missed anything? One thing worth noting is that I had to adjust the structure of the internal ID for the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@oboukili Do you want to double-check this one or can I go ahead and merge?
@onematchfox big thanks for handling this PR! @karlschriek we're getting there! |
Co-authored-by: Olivier Boukili <[email protected]>
fed6c9d
to
d78df16
Compare
Schema for `application.spec` has not changed.
d78df16
to
255b2b7
Compare
ArgoCD 2.5.0 has just been released.
One of the features included therein, is the ability to deploy ArgoCD Applications to namespaces outside of the controller's own namespace (which is usually
argocd
). In order to prevent permission escalation, a new field "source_namespaces" was added to the AppProject resource, which constrains namespaces within which Applications for that AppProject can be deployed and still be picked up by the conroller.A full discussion on the feature can be found here: argoproj/argo-cd#9755
This PR updates the schemas and other relevant specs by adding support for "source_namespaces".
It also bumps up the argocd-cd Go package to:
github.com/argoproj/argo-cd/v2 v2.5.0
In order to accomodate this, I had to also manually bump the pinned k8s.io dependencies, i.e.
k8s.io/api => k8s.io/api v0.23.1
becomesk8s.io/api => k8s.io/api v0.24.2
and so forth.An issue remains though, as there seems to be some kind of incompatibility with
github.com/argoproj/[email protected]
, which is still the latest version.go build
will cause:I do not have a good enough understanding of the underlying dependencies to figure out where exactly this problem originates and how to fix or work around it. I am therefore putting this PR up so long in the hope that someone with more context would be able to quickly identify how to deal with this.