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

Gardener components no longer use admin kubeconfig for local garden dev setup #3901

Merged
merged 19 commits into from
Apr 23, 2021

Conversation

rfranzke
Copy link
Member

How to categorize this PR?

/area dev-productivity
/kind enhancement

What this PR does / why we need it:
Today, all Gardener components are using the admin kubeconfig for the local garden development setup. This has the downside that the RBAC rules which we maintain in

Which issue(s) this PR fixes:
Softly related to #1723 and #1724.

Special notes for your reviewer:
/squash

Release note:

When using the local garden development environment, the Gardener components do now use dedicated kubeconfigs constrained by RBAC rules (earlier, they were always using the admin kubeconfig).

…er components

On the way, I renamed "default-controller-manager" to "default-kube-controller-manager".
(First bug found due to improved local dev env :))
@rfranzke rfranzke requested a review from a team as a code owner April 20, 2021 11:05
@gardener-robot gardener-robot added area/dev-productivity Developer productivity related (how to improve development) kind/enhancement Enhancement, improvement, extension merge/squash size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Apr 20, 2021
@rfranzke
Copy link
Member Author

@vpnachev - an idea as follow-up of this PR to also make the gardenlet use the RBAC privileges in the seed that you plan to apply with #3763:

In https://github.com/gardener/gardener/blob/master/hack/local-development/start-gardenlet,

  1. create a ServiceAccount in the garden namespace of all the seed clusters (by reading all Seeds and using the kubeconfig referenced by .spec.secretRef)
  2. apply the RBAC resources for the gardenlet in the seeds
  3. create a new, temporary kubeconfig which uses the token of the ServiceAccount created in 1.
  4. update the kubeconfig in the .spec.secretRef of the various seeds

It might have drawbacks, just brain-storming here. WDYT?

@vpnachev
Copy link
Member

@vpnachev - an idea as follow-up of this PR to also make the gardenlet use the RBAC privileges in the seed that you plan to apply with #3763:

In https://github.com/gardener/gardener/blob/master/hack/local-development/start-gardenlet,

1. create a `ServiceAccount` in the `garden` namespace of all the seed clusters (by reading all `Seed`s and using the kubeconfig referenced by `.spec.secretRef`)

2. apply the RBAC resources for the gardenlet in the seeds

3. create a new, temporary kubeconfig which uses the token of the `ServiceAccount` created in 1.

4. update the kubeconfig in the `.spec.secretRef` of the various seeds

It might have drawbacks, just brain-storming here. WDYT?

Thanks for the suggestion! However, I am still thinking that it should be as it is today. Why

  1. A new seed can be added at any time and gardenlet will use the configured kubeconfig. On restart of gardenlet, it will create the service account and bound the (cluster)roles to it. This can lead to authorization issues if new permissions are required, or if the seed is deleted with no restart of gardenlet such missing permissions will not be detected.
  2. It still possible to run gardenlet remotely in productive landscapes with the whatever kubeconfig is configured by the landscape administrator. Should we implement such automation in this case - I think no. It would be a bit weird gardenlet to amend externally configured kubeconfig.

For now I will not add anything like this in #3763 as it has been prolonged too much and a lot of conflicts needs to be resolved. Such automation can be contributed later, if we find a suitable solution.

@rfranzke
Copy link
Member Author

@vpnachev My recommendation was only for the local development environment. Of course, in production code we should not have anything like this. I just think we should find a way to make sure to detect issues in the gardenlet's RBAC rules already when developing locally. We don't have to follow above approach (it was just some short brain-storming/food for thought), and we don't have to do anything now with #3763, but I think we should eventually have some solution for it.

@stoyanr
Copy link
Contributor

stoyanr commented Apr 21, 2021

/assign

Copy link
Contributor

@stoyanr stoyanr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few minor nits and some questions regarding why is this enabled only for local-garden / $NODELESS as opposed to other local environment types, e.g. remote-garden / $REMOTE.

Copy link
Contributor

@stoyanr stoyanr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@rfranzke rfranzke merged commit ec0c9b9 into gardener:master Apr 23, 2021
@rfranzke rfranzke deleted the enh/rbac-local-garden branch April 23, 2021 05:14
krgostev pushed a commit to krgostev/gardener that referenced this pull request Apr 21, 2022
…ev setup (gardener#3901)

* Extract ClusterRoleBinding/gardener.cloud:admin into dedicated file

* Extract ClusterRoleBinding/gardener.cloud:apiserver:auth-delegator into dedicated file

* Extract RoleBinding/gardener.cloud:apiserver:auth-reader into dedicated file

* Extract ClusterRoleBinding/gardener.cloud:apiserver:admin into dedicated file

* Extract ClusterRoleBinding/gardener.cloud:controller-manager:admin into dedicated file

* Extract ClusterRole/gardener.cloud:system:admission-controller into dedicated file

* Extract ClusterRoleBinding/gardener.cloud:admission-controller into dedicated file

* Extract ClusterRole/gardener.cloud:system:gardener-scheduler into dedicated file

* Extract ClusterRoleBinding/gardener.cloud:system:gardener-scheduler into dedicated file

* Extract ClusterRole/gardener.cloud:system:seeds into dedicated file

* Extract ClusterRoleBinding/gardener.cloud:system:seeds into dedicated file

* Extract ClusterRole/gardener.cloud:system:seed-bootstrapper into dedicated file

* Extract ClusterRoleBinding/gardener.cloud:system:seed-bootstrapper into dedicated file

* Adapt local-garden cert generator to generate client certs for Gardener components

On the way, I renamed "default-controller-manager" to "default-kube-controller-manager".

* Generate client certificates for Gardener components for local-garden

* Deploy ClusterRole{Binding}s for Gardener components for local garden

and use dedicated kubeconfigs

* Fix missing `s` in GAC's ClusterRole

(First bug found due to improved local dev env :))

* Address PR review feedback of @stoyanr

* Add disclaimer about admin privileges for remote garden
krgostev pushed a commit to krgostev/gardener that referenced this pull request Jul 5, 2022
…ev setup (gardener#3901)

* Extract ClusterRoleBinding/gardener.cloud:admin into dedicated file

* Extract ClusterRoleBinding/gardener.cloud:apiserver:auth-delegator into dedicated file

* Extract RoleBinding/gardener.cloud:apiserver:auth-reader into dedicated file

* Extract ClusterRoleBinding/gardener.cloud:apiserver:admin into dedicated file

* Extract ClusterRoleBinding/gardener.cloud:controller-manager:admin into dedicated file

* Extract ClusterRole/gardener.cloud:system:admission-controller into dedicated file

* Extract ClusterRoleBinding/gardener.cloud:admission-controller into dedicated file

* Extract ClusterRole/gardener.cloud:system:gardener-scheduler into dedicated file

* Extract ClusterRoleBinding/gardener.cloud:system:gardener-scheduler into dedicated file

* Extract ClusterRole/gardener.cloud:system:seeds into dedicated file

* Extract ClusterRoleBinding/gardener.cloud:system:seeds into dedicated file

* Extract ClusterRole/gardener.cloud:system:seed-bootstrapper into dedicated file

* Extract ClusterRoleBinding/gardener.cloud:system:seed-bootstrapper into dedicated file

* Adapt local-garden cert generator to generate client certs for Gardener components

On the way, I renamed "default-controller-manager" to "default-kube-controller-manager".

* Generate client certificates for Gardener components for local-garden

* Deploy ClusterRole{Binding}s for Gardener components for local garden

and use dedicated kubeconfigs

* Fix missing `s` in GAC's ClusterRole

(First bug found due to improved local dev env :))

* Address PR review feedback of @stoyanr

* Add disclaimer about admin privileges for remote garden
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dev-productivity Developer productivity related (how to improve development) kind/enhancement Enhancement, improvement, extension size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants