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

Disabling in-cluster does not prevent the usage of in-cluster #21207

Open
agaudreault opened this issue Dec 16, 2024 · 3 comments · May be fixed by #21208
Open

Disabling in-cluster does not prevent the usage of in-cluster #21207

agaudreault opened this issue Dec 16, 2024 · 3 comments · May be fixed by #21208
Assignees
Labels
bug Something isn't working component:api API bugs and enhancements component:application-controller good first issue Good for newcomers security Security related
Milestone

Comments

@agaudreault
Copy link
Member

agaudreault commented Dec 16, 2024

Describe the bug

When disabling the in-cluster via cluster.inClusterEnabled: 'false', the cluster is properly not showing from the Argo CD cluster list. However, Argo CD can still sync existing applications using this cluster and create resources on the in-cluster if it has k8s permissions via RoleBindings. Furthermore, applications can still be created using https://kubernetes.default.svc cluster url.

See the missing validation on inClusterEnabled usage in https://github.com/agaudreault/argo-cd/blob/0074bb4b486312e552f96f325f26148b3bbabbc0/util/db/cluster.go#L228-L230.

Another impact of this is that the controller will start to watch all resources on the local cluster when inClusterEnabled=false and no "in-cluster" secret exist, because the liveStateCache calls c.db.GetCluster, which returns the local cluster with default configuration. The cluster watches also do not handle the in-cluster secret to be disabled, so it will never be removed from the watches.

The usage of variable KubernetesInternalAPIServerAddr should be analyzed and probably the settings should be checked too when necessary in other code paths.

To Reproduce

  1. Create an Application using https://kubernetes.default.svc
  2. Update the configMap to use cluster.inClusterEnabled: 'false' and deleted the in-cluster secret.
  3. Restart controller
  4. Sync the app or create a new app

Expected behavior

  • An error should be returned that "cluster https://kubernetes.default.svc is disabled" when an Application is created/updated.
  • An error should be returned when the application is reconciled and/or synced.

Version

Latest master (2.13.1)

@todaywasawesome
Copy link
Contributor

This is potentially a breaking change becuase people could have this set to false and have been declaratively deploying applications. This would put their apps into unknown status until they updated the setting. Either 2.15 or 3.0 depending on how disruptive we think that is and if we can do 3.0 instead of 2.15.

@todaywasawesome todaywasawesome added this to the v.2.15 milestone Dec 19, 2024
@todaywasawesome
Copy link
Contributor

No plans to cherry-pick back given it's breaking nature.

@agaudreault
Copy link
Member Author

agaudreault commented Dec 19, 2024

Another impact of this bug

Another impact of this is that the controller will start to watch all resources on the local cluster when inClusterEnabled=false and no "in-cluster" secret exist, because the liveStateCache calls c.db.GetCluster, which returns the local cluster with default configuration. The cluster watches also do not handle the in-cluster secret to be disabled, so it will never be removed from the watches.

I do think it should be fixed in a minor version without needing a major. cluster.inClusterEnabled: 'true' can be set by anyone relying on in-cluster.

For people using cluster.inClusterEnabled: 'false', it actually decreases the security of argo, allowing anyone with application's create or update permissions to target in-cluster (assuming app project doesn't deny it) and take over any resources in that cluster allowed by the Kubernetes role bindings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working component:api API bugs and enhancements component:application-controller good first issue Good for newcomers security Security related
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants