-
Notifications
You must be signed in to change notification settings - Fork 41
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
Kubernetes Secret Kubeconfig integration #66
Conversation
Thanks for taking this on @stevendborrelli! This is our most wanted feature request. Is there anything you want to discuss before doing further work here? |
I am still doing some testing and refactoring. There are a couple of things I'd like some clarity on:
is causing cluster deletes to be skipped: if meta.WasDeleted(cr) && meta.GetExternalName(cr) == observedCluster.Name {
// ArgoCD Cluster resource ignores the name field. This detects the deletion of the default cluster resource.
return managed.ExternalObservation{}, nil
} |
d0eb591
to
265bc82
Compare
@janwillies this PR is ready for review. My main concern is with the deletion code. Without the change my clusters would not delete. |
To test this PR, you can use the EKS example for the AWS official providers at : https://github.com/upbound/provider-aws/blob/main/examples/eks/cluster.yaml. The secret will be written to The example in |
Thanks for your PR @stevendborrelli! Can you take a look at the remarks and also solve the conflicts? We would be happy to see this feature in |
@MisterMX I made an initial pass changing fields to optional but there are some issues in the Mock function causing tests to fail. I'll try to get something by Friday. |
81419f2
to
da35ce5
Compare
@stevendborrelli @MisterMX Sorry for barging in here like this. |
Sorry for the delay everyone, I needed to add the ability to track changes to the kubeconfig, as cloud providers rotate the credentials. I've pushed a new commit that includes this tracking. |
The PR has been updated and is ready for review. |
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.
Thank you very much for your contribition @stevendborrelli. I added two remarks to your code. Can you take a look at them?
Also, can you squash your changes into a single commit and sign it?
Signed-off-by: Steven Borrelli <steve@borrelli.org>
662d30a
to
991d2e4
Compare
@MisterMX I've squashed the changes for the PR, and addressed your feedback. |
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.
LGTM. Thank you very much for your contribution @stevendborrelli!
Description of your changes
Ready for review.
kubeconfig
secret generated by Crossplane.Name
,Server
, andConfig.TLSClientConfig
to pointers as they are optional.Fixes #13
I have:
make reviewable test
to ensure this PR is ready for review.How has this code been tested