From 5b314d3fce30040a133a9b76de4cb51053908cd3 Mon Sep 17 00:00:00 2001 From: rambohe-ch Date: Mon, 21 Aug 2023 20:10:58 +0800 Subject: [PATCH] improve rabc settings of secrets for yurt-manager component --- .../yurt-manager-auto-generated.yaml | 54 ++++++++----------- .../yurt-manager/templates/yurt-manager.yaml | 21 +++++++- .../yurtappdaemon/yurtappdaemon_controller.go | 2 - .../yurtappset/yurtappset_controller.go | 2 - .../cert/yurtcoordinatorcert_controller.go | 2 +- pkg/yurtmanager/webhook/server.go | 2 +- .../util/controller/webhook_controller.go | 23 -------- .../webhook/util/writer/certwriter.go | 12 ++--- pkg/yurtmanager/webhook/util/writer/error.go | 17 +++--- pkg/yurtmanager/webhook/util/writer/secret.go | 10 ++-- test/e2e/cmd/init/constants/constants.go | 24 +++++++++ .../cmd/init/util/kubernetes/apply_addons.go | 10 ++++ test/e2e/cmd/init/util/kubernetes/util.go | 28 ++++++++++ 13 files changed, 127 insertions(+), 80 deletions(-) diff --git a/charts/yurt-manager/templates/yurt-manager-auto-generated.yaml b/charts/yurt-manager/templates/yurt-manager-auto-generated.yaml index 1b452b66bcb..3f77b32e328 100644 --- a/charts/yurt-manager/templates/yurt-manager-auto-generated.yaml +++ b/charts/yurt-manager/templates/yurt-manager-auto-generated.yaml @@ -7,29 +7,43 @@ --- apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRole +kind: Role metadata: creationTimestamp: null name: yurt-manager-role + namespace: {{ .Release.Namespace }} rules: - apiGroups: - "" resources: - - configmaps + - secrets verbs: + - create - get - - list - - watch + - patch + - update - apiGroups: - "" resources: - - secret + - secrets verbs: - - create - get - - list - - patch - update +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + creationTimestamp: null + name: yurt-manager-role +rules: +- apiGroups: + - "" + resources: + - configmaps + verbs: + - get + - list + - watch - apiGroups: - admissionregistration.k8s.io resources: @@ -317,18 +331,6 @@ rules: - patch - update - watch -- apiGroups: - - "" - resources: - - persistentvolumeclaims - verbs: - - create - - delete - - get - - list - - patch - - update - - watch - apiGroups: - "" resources: @@ -347,18 +349,6 @@ rules: - pods/status verbs: - update -- apiGroups: - - "" - resources: - - secrets - verbs: - - create - - delete - - get - - list - - patch - - update - - watch - apiGroups: - "" resources: diff --git a/charts/yurt-manager/templates/yurt-manager.yaml b/charts/yurt-manager/templates/yurt-manager.yaml index 210338e384c..9472b85572a 100644 --- a/charts/yurt-manager/templates/yurt-manager.yaml +++ b/charts/yurt-manager/templates/yurt-manager.yaml @@ -1,4 +1,10 @@ apiVersion: v1 +kind: Secret +metadata: + name: yurt-manager-webhook-certs + namespace: {{ .Release.Namespace }} +--- +apiVersion: v1 kind: ServiceAccount metadata: name: yurt-manager @@ -19,7 +25,20 @@ subjects: name: yurt-manager namespace: {{ .Release.Namespace }} --- - +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: yurt-manager-role-binding + namespace: {{ .Release.Namespace }} +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: yurt-manager-role +subjects: + - kind: ServiceAccount + name: yurt-manager + namespace: {{ .Release.Namespace }} +--- apiVersion: v1 kind: Service metadata: diff --git a/pkg/yurtmanager/controller/yurtappdaemon/yurtappdaemon_controller.go b/pkg/yurtmanager/controller/yurtappdaemon/yurtappdaemon_controller.go index d36bb905cb5..e2c8635cf94 100644 --- a/pkg/yurtmanager/controller/yurtappdaemon/yurtappdaemon_controller.go +++ b/pkg/yurtmanager/controller/yurtappdaemon/yurtappdaemon_controller.go @@ -136,8 +136,6 @@ func newReconciler(mgr manager.Manager) reconcile.Reconciler { // +kubebuilder:rbac:groups=apps,resources=deployments/status,verbs=get;update;patch // +kubebuilder:rbac:groups=core,resources=events,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=apps,resources=controllerrevisions,verbs=get;list;watch;create;update;patch;delete -// +kubebuilder:rbac:groups=core,resources=persistentvolumeclaims,verbs=get;list;watch;create;update;patch;delete -// +kubebuilder:rbac:groups=coordination.k8s.io,resources=leases,verbs=get;list;watch;create;update;patch;delete // Reconcile reads that state of the cluster for a YurtAppDaemon object and makes changes based on the state read // and what is in the YurtAppDaemon.Spec diff --git a/pkg/yurtmanager/controller/yurtappset/yurtappset_controller.go b/pkg/yurtmanager/controller/yurtappset/yurtappset_controller.go index 8180aad3db7..042d334d6ec 100644 --- a/pkg/yurtmanager/controller/yurtappset/yurtappset_controller.go +++ b/pkg/yurtmanager/controller/yurtappset/yurtappset_controller.go @@ -150,8 +150,6 @@ type ReconcileYurtAppSet struct { // +kubebuilder:rbac:groups=apps,resources=deployments/status,verbs=get;update;patch // +kubebuilder:rbac:groups=core,resources=events,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=apps,resources=controllerrevisions,verbs=get;list;watch;create;update;patch;delete -// +kubebuilder:rbac:groups=core,resources=persistentvolumeclaims,verbs=get;list;watch;create;update;patch;delete -// +kubebuilder:rbac:groups=coordination.k8s.io,resources=leases,verbs=get;list;watch;create;update;patch;delete // Reconcile reads that state of the cluster for a YurtAppSet object and makes changes based on the state read // and what is in the YurtAppSet.Spec diff --git a/pkg/yurtmanager/controller/yurtcoordinator/cert/yurtcoordinatorcert_controller.go b/pkg/yurtmanager/controller/yurtcoordinator/cert/yurtcoordinatorcert_controller.go index dc127095225..9023c2ce35b 100644 --- a/pkg/yurtmanager/controller/yurtcoordinator/cert/yurtcoordinatorcert_controller.go +++ b/pkg/yurtmanager/controller/yurtcoordinator/cert/yurtcoordinatorcert_controller.go @@ -301,7 +301,7 @@ func (r *ReconcileYurtCoordinatorCert) InjectConfig(cfg *rest.Config) error { } // +kubebuilder:rbac:groups=certificates.k8s.io,resources=certificatesigningrequests,verbs=create -// +kubebuilder:rbac:groups="",resources=secret,verbs=get;update;patch;create;list +// +kubebuilder:rbac:groups="",namespace=kube-system,resources=secrets,verbs=get;update;create;patch // +kubebuilder:rbac:groups="",resources=configmaps,verbs=get;watch;list // todo: make customized certificate for each yurtcoordinator pod diff --git a/pkg/yurtmanager/webhook/server.go b/pkg/yurtmanager/webhook/server.go index 7b43a79c414..35a6f93728b 100644 --- a/pkg/yurtmanager/webhook/server.go +++ b/pkg/yurtmanager/webhook/server.go @@ -140,7 +140,7 @@ func SetupWithManager(c *config.CompletedConfig, mgr manager.Manager) error { type GateFunc func() (enabled bool) -// +kubebuilder:rbac:groups=core,resources=secrets,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=core,namespace=kube-system,resources=secrets,verbs=get;update // +kubebuilder:rbac:groups=admissionregistration.k8s.io,resources=mutatingwebhookconfigurations,verbs=get;list;watch;update;patch // +kubebuilder:rbac:groups=admissionregistration.k8s.io,resources=validatingwebhookconfigurations,verbs=get;list;watch;update;patch // +kubebuilder:rbac:groups=apiextensions.k8s.io,resources=customresourcedefinitions,verbs=get;list;watch;update;patch diff --git a/pkg/yurtmanager/webhook/util/controller/webhook_controller.go b/pkg/yurtmanager/webhook/util/controller/webhook_controller.go index 11eed0df2ad..dad23492209 100644 --- a/pkg/yurtmanager/webhook/util/controller/webhook_controller.go +++ b/pkg/yurtmanager/webhook/util/controller/webhook_controller.go @@ -35,7 +35,6 @@ import ( "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/informers" admissionregistrationinformers "k8s.io/client-go/informers/admissionregistration/v1" - coreinformers "k8s.io/client-go/informers/core/v1" clientset "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" "k8s.io/client-go/tools/cache" @@ -54,8 +53,6 @@ const ( ) var ( - secretName = webhookutil.GetSecretName() - uninit = make(chan struct{}) onceInit = sync.Once{} ) @@ -91,8 +88,6 @@ func New(handlers map[string]struct{}, cc *config.CompletedConfig, restCfg *rest } c.informerFactory = informers.NewSharedInformerFactory(c.kubeClient, 0) - - secretInformer := coreinformers.New(c.informerFactory, webhookutil.GetNamespace(), nil).Secrets() admissionRegistrationInformer := admissionregistrationinformers.New(c.informerFactory, v1.NamespaceAll, nil) extensionsClient, err := apiextensionsclientset.NewForConfig(restCfg) @@ -121,23 +116,6 @@ func New(handlers map[string]struct{}, cc *config.CompletedConfig, restCfg *rest c.extensionsClient = extensionsClient c.extensionsLister = crdInformer.Lister() - secretInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ - AddFunc: func(obj interface{}) { - secret := obj.(*v1.Secret) - if secret.Name == secretName { - klog.Infof("Secret %s added", secretName) - c.queue.Add("") - } - }, - UpdateFunc: func(old, cur interface{}) { - secret := cur.(*v1.Secret) - if secret.Name == secretName { - klog.Infof("Secret %s updated", secretName) - c.queue.Add("") - } - }, - }) - admissionRegistrationInformer.MutatingWebhookConfigurations().Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ AddFunc: func(obj interface{}) { conf := obj.(*admissionregistrationv1.MutatingWebhookConfiguration) @@ -173,7 +151,6 @@ func New(handlers map[string]struct{}, cc *config.CompletedConfig, restCfg *rest }) c.synced = []cache.InformerSynced{ - secretInformer.Informer().HasSynced, admissionRegistrationInformer.MutatingWebhookConfigurations().Informer().HasSynced, admissionRegistrationInformer.ValidatingWebhookConfigurations().Informer().HasSynced, crdInformer.Informer().HasSynced, diff --git a/pkg/yurtmanager/webhook/util/writer/certwriter.go b/pkg/yurtmanager/webhook/util/writer/certwriter.go index 1795ac57d0d..b52e7d6a4b8 100644 --- a/pkg/yurtmanager/webhook/util/writer/certwriter.go +++ b/pkg/yurtmanager/webhook/util/writer/certwriter.go @@ -54,7 +54,7 @@ func handleCommon(dnsName string, ch certReadWriter) (*generator.Artifacts, bool return nil, false, errors.New("certReaderWriter should not be nil") } - certs, changed, err := createIfNotExists(ch) + certs, changed, err := updateIfNotExists(ch) if err != nil { return nil, changed, err } @@ -72,16 +72,12 @@ func handleCommon(dnsName string, ch certReadWriter) (*generator.Artifacts, bool return certs, changed, nil } -func createIfNotExists(ch certReadWriter) (*generator.Artifacts, bool, error) { +func updateIfNotExists(ch certReadWriter) (*generator.Artifacts, bool, error) { // Try to read first certs, err := ch.read() - if isNotFound(err) { + if isNotExist(err) { // Create if not exists - certs, err = ch.write() - // This may happen if there is another racer. - if isAlreadyExists(err) { - certs, err = ch.read() - } + certs, err = ch.overwrite(certs.ResourceVersion) return certs, true, err } return certs, false, err diff --git a/pkg/yurtmanager/webhook/util/writer/error.go b/pkg/yurtmanager/webhook/util/writer/error.go index 188243abb8c..de432d2f666 100644 --- a/pkg/yurtmanager/webhook/util/writer/error.go +++ b/pkg/yurtmanager/webhook/util/writer/error.go @@ -24,11 +24,6 @@ func (e notFoundError) Error() string { return e.err.Error() } -func isNotFound(err error) bool { - _, ok := err.(notFoundError) - return ok -} - type alreadyExistError struct { err error } @@ -37,7 +32,15 @@ func (e alreadyExistError) Error() string { return e.err.Error() } -func isAlreadyExists(err error) bool { - _, ok := err.(alreadyExistError) +type notExistError struct { + err error +} + +func (e notExistError) Error() string { + return e.err.Error() +} + +func isNotExist(err error) bool { + _, ok := err.(notExistError) return ok } diff --git a/pkg/yurtmanager/webhook/util/writer/secret.go b/pkg/yurtmanager/webhook/util/writer/secret.go index 781c8801f3d..32e602b92df 100644 --- a/pkg/yurtmanager/webhook/util/writer/secret.go +++ b/pkg/yurtmanager/webhook/util/writer/secret.go @@ -19,6 +19,7 @@ package writer import ( "context" "errors" + "fmt" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -129,13 +130,16 @@ func (s *secretCertWriter) overwrite(resourceVersion string) (*generator.Artifac func (s *secretCertWriter) read() (*generator.Artifacts, error) { secret, err := s.Clientset.CoreV1().Secrets(s.Secret.Namespace).Get(context.TODO(), s.Secret.Name, metav1.GetOptions{}) - if apierrors.IsNotFound(err) { - return nil, notFoundError{err} - } if err != nil { return nil, err } + certs := secretToCerts(secret) + if secret.Data == nil || len(secret.Data[CAKeyName]) == 0 || len(secret.Data[CACertName]) == 0 || + len(secret.Data[ServerCertName]) == 0 || len(secret.Data[ServerKeyName]) == 0 { + return certs, notExistError{fmt.Errorf("no certificate exists in secret %s", s.Secret.Name)} + } + if certs.CACert != nil && certs.CAKey != nil { // Store the CA for next usage. s.CertGenerator.SetCA(certs.CAKey, certs.CACert) diff --git a/test/e2e/cmd/init/constants/constants.go b/test/e2e/cmd/init/constants/constants.go index 8362d30818c..6ed60d6d748 100644 --- a/test/e2e/cmd/init/constants/constants.go +++ b/test/e2e/cmd/init/constants/constants.go @@ -34,6 +34,14 @@ const ( YurthubNamespace = "kube-system" YurthubCmName = "yurt-hub-cfg" + YurtManagerCertsSecret = ` +apiVersion: v1 +kind: Secret +metadata: + name: yurt-manager-webhook-certs + namespace: kube-system +` + YurtManagerServiceAccount = ` apiVersion: v1 kind: ServiceAccount @@ -52,6 +60,22 @@ roleRef: kind: ClusterRole name: yurt-manager-role subjects: +- kind: ServiceAccount + name: yurt-manager + namespace: kube-system +` + + YurtManagerRoleBinding = ` +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: yurt-manager-role-binding + namespace: kube-system +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: yurt-manager-role +subjects: - kind: ServiceAccount name: yurt-manager namespace: kube-system diff --git a/test/e2e/cmd/init/util/kubernetes/apply_addons.go b/test/e2e/cmd/init/util/kubernetes/apply_addons.go index 6fb62f8680e..cd0baa661cd 100644 --- a/test/e2e/cmd/init/util/kubernetes/apply_addons.go +++ b/test/e2e/cmd/init/util/kubernetes/apply_addons.go @@ -84,6 +84,10 @@ func DeleteYurthubSetting(client kubeclientset.Interface) error { } func CreateYurtManager(client kubeclientset.Interface, yurtManagerImage string) error { + if err := CreateSecretFromYaml(client, SystemNamespace, constants.YurtManagerCertsSecret); err != nil { + return err + } + if err := CreateServiceAccountFromYaml(client, SystemNamespace, constants.YurtManagerServiceAccount); err != nil { return err @@ -95,6 +99,12 @@ func CreateYurtManager(client kubeclientset.Interface, yurtManagerImage string) return err } + // bind the role + if err := CreateRoleBindingFromYaml(client, + constants.YurtManagerRoleBinding); err != nil { + return err + } + // create the Service if err := CreateServiceFromYaml(client, SystemNamespace, diff --git a/test/e2e/cmd/init/util/kubernetes/util.go b/test/e2e/cmd/init/util/kubernetes/util.go index 2c158c2b110..04eedda72f4 100644 --- a/test/e2e/cmd/init/util/kubernetes/util.go +++ b/test/e2e/cmd/init/util/kubernetes/util.go @@ -75,6 +75,20 @@ func processCreateErr(kind string, name string, err error) error { return nil } +// CreateSecretFromYaml creates the Secret from the yaml template. +func CreateSecretFromYaml(cliSet kubeclientset.Interface, ns, saTmpl string) error { + obj, err := YamlToObject([]byte(saTmpl)) + if err != nil { + return err + } + se, ok := obj.(*corev1.Secret) + if !ok { + return fmt.Errorf("fail to assert secret: %w", err) + } + _, err = cliSet.CoreV1().Secrets(ns).Create(context.Background(), se, metav1.CreateOptions{}) + return processCreateErr("secret", se.Name, err) +} + // CreateServiceAccountFromYaml creates the ServiceAccount from the yaml template. func CreateServiceAccountFromYaml(cliSet kubeclientset.Interface, ns, saTmpl string) error { obj, err := YamlToObject([]byte(saTmpl)) @@ -117,6 +131,20 @@ func CreateClusterRoleBindingFromYaml(cliSet kubeclientset.Interface, crbTmpl st return processCreateErr("clusterrolebinding", crb.Name, err) } +// CreateRoleBindingFromYaml creates the RoleBinding from the yaml template. +func CreateRoleBindingFromYaml(cliSet kubeclientset.Interface, crbTmpl string) error { + obj, err := YamlToObject([]byte(crbTmpl)) + if err != nil { + return err + } + rb, ok := obj.(*rbacv1.RoleBinding) + if !ok { + return fmt.Errorf("fail to assert rolebinding: %w", err) + } + _, err = cliSet.RbacV1().RoleBindings("kube-system").Create(context.Background(), rb, metav1.CreateOptions{}) + return processCreateErr("rolebinding", rb.Name, err) +} + // CreateConfigMapFromYaml creates the ConfigMap from the yaml template. func CreateConfigMapFromYaml(cliSet kubeclientset.Interface, ns, cmTmpl string) error { obj, err := YamlToObject([]byte(cmTmpl))