Merge pull request #1742 from rambohe-ch/improve-yurt-manager-secret-…

…rbac

improve rabc settings of secrets for yurt-manager component

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:

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:

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

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

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

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

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,

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

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
 }

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)

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

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,