admin/kubesphere
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix NPE in user_controller

Browse Source
This commit is contained in:
hongming
2021-10-11 17:15:42 +08:00
committed by hongming
parent 1956f83af0
commit 776593001e
5 changed files with 52 additions and 85 deletions
Download Patch File Download Diff File Expand all files Collapse all files

68
pkg/controller/user/user_controller.go
View File

@@ -198,12 +198,12 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco
// update user status if not managed by kubefed
managedByKubefed := user.Labels[constants.KubefedManagedLabel] == "true"
if !managedByKubefed {
if user, err = r.encryptPassword(ctx, user); err != nil {
if err = r.encryptPassword(ctx, user); err != nil {
klog.Error(err)
r.Recorder.Event(user, corev1.EventTypeWarning, failedSynced, fmt.Sprintf(syncFailMessage, err))
return ctrl.Result{}, err
}
if user, err = r.syncUserStatus(ctx, user); err != nil {
if err = r.syncUserStatus(ctx, user); err != nil {
klog.Error(err)
r.Recorder.Event(user, corev1.EventTypeWarning, failedSynced, fmt.Sprintf(syncFailMessage, err))
return ctrl.Result{}, err
@@ -238,15 +238,15 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco
return ctrl.Result{}, nil
}
func (r *Reconciler) encryptPassword(ctx context.Context, user *iamv1alpha2.User) (*iamv1alpha2.User, error) {
// encryptPassword Encrypt and update the user password
func (r *Reconciler) encryptPassword(ctx context.Context, user *iamv1alpha2.User) error {
// password is not empty and not encrypted
if user.Spec.EncryptedPassword != "" && !isEncrypted(user.Spec.EncryptedPassword) {
password, err := encrypt(user.Spec.EncryptedPassword)
if err != nil {
klog.Error(err)
return nil, err
return err
}
user = user.DeepCopy()
user.Spec.EncryptedPassword = password
if user.Annotations == nil {
user.Annotations = make(map[string]string)
@@ -256,11 +256,10 @@ func (r *Reconciler) encryptPassword(ctx context.Context, user *iamv1alpha2.User
delete(user.Annotations, corev1.LastAppliedConfigAnnotation)
err = r.Update(ctx, user, &client.UpdateOptions{})
if err != nil {
return nil, err
return err
}
return user, nil
}
return user, nil
return nil
}
func (r *Reconciler) ensureNotControlledByKubefed(ctx context.Context, user *iamv1alpha2.User) error {
@@ -268,11 +267,11 @@ func (r *Reconciler) ensureNotControlledByKubefed(ctx context.Context, user *iam
if user.Labels == nil {
user.Labels = make(map[string]string, 0)
}
user = user.DeepCopy()
user.Labels[constants.KubefedManagedLabel] = "false"
err := r.Update(ctx, user, &client.UpdateOptions{})
if err != nil {
klog.Error(err)
return err
}
}
return nil
@@ -453,57 +452,51 @@ func (r *Reconciler) deleteLoginRecords(ctx context.Context, user *iamv1alpha2.U
return r.Client.DeleteAllOf(ctx, loginRecord, client.MatchingLabels{iamv1alpha2.UserReferenceLabel: user.Name})
}
// syncUserStatus will reconcile user state based on user login records
func (r *Reconciler) syncUserStatus(ctx context.Context, user *iamv1alpha2.User) (*iamv1alpha2.User, error) {
// syncUserStatus Update the user status
func (r *Reconciler) syncUserStatus(ctx context.Context, user *iamv1alpha2.User) error {
if user.Spec.EncryptedPassword == "" {
if user.Labels[iamv1alpha2.IdentifyProviderLabel] != "" {
// mapped user from other identity provider always active until disabled
if user.Status.State == nil || *user.Status.State != iamv1alpha2.UserActive {
expected := user.DeepCopy()
active := iamv1alpha2.UserActive
expected.Status = iamv1alpha2.UserStatus{
user.Status = iamv1alpha2.UserStatus{
State: &active,
LastTransitionTime: &metav1.Time{Time: time.Now()},
}
err := r.Update(ctx, expected, &client.UpdateOptions{})
err := r.Update(ctx, user, &client.UpdateOptions{})
if err != nil {
return nil, err
return err
}
return expected, nil
}
} else {
// becomes disabled after setting a blank password
if user.Status.State == nil || *user.Status.State != iamv1alpha2.UserDisabled {
expected := user.DeepCopy()
disabled := iamv1alpha2.UserDisabled
expected.Status = iamv1alpha2.UserStatus{
user.Status = iamv1alpha2.UserStatus{
State: &disabled,
LastTransitionTime: &metav1.Time{Time: time.Now()},
}
err := r.Update(ctx, expected, &client.UpdateOptions{})
err := r.Update(ctx, user, &client.UpdateOptions{})
if err != nil {
return nil, err
return err
}
return expected, nil
}
}
return user, nil
return nil
}
// becomes active after password encrypted
if isEncrypted(user.Spec.EncryptedPassword) {
if user.Status.State == nil || *user.Status.State == iamv1alpha2.UserDisabled {
expected := user.DeepCopy()
active := iamv1alpha2.UserActive
expected.Status = iamv1alpha2.UserStatus{
user.Status = iamv1alpha2.UserStatus{
State: &active,
LastTransitionTime: &metav1.Time{Time: time.Now()},
}
err := r.Update(ctx, expected, &client.UpdateOptions{})
err := r.Update(ctx, user, &client.UpdateOptions{})
if err != nil {
return nil, err
return err
}
return expected, nil
}
}
@@ -511,18 +504,17 @@ func (r *Reconciler) syncUserStatus(ctx context.Context, user *iamv1alpha2.User)
if user.Status.State != nil && *user.Status.State == iamv1alpha2.UserAuthLimitExceeded {
if user.Status.LastTransitionTime != nil &&
user.Status.LastTransitionTime.Add(r.AuthenticationOptions.AuthenticateRateLimiterDuration).Before(time.Now()) {
expected := user.DeepCopy()
// unblock user
active := iamv1alpha2.UserActive
expected.Status = iamv1alpha2.UserStatus{
user.Status = iamv1alpha2.UserStatus{
State: &active,
LastTransitionTime: &metav1.Time{Time: time.Now()},
}
err := r.Update(ctx, expected, &client.UpdateOptions{})
err := r.Update(ctx, user, &client.UpdateOptions{})
if err != nil {
return nil, err
return err
}
return expected, nil
return nil
}
}
@@ -531,7 +523,7 @@ func (r *Reconciler) syncUserStatus(ctx context.Context, user *iamv1alpha2.User)
err := r.List(ctx, records, client.MatchingLabels{iamv1alpha2.UserReferenceLabel: user.Name})
if err != nil {
klog.Error(err)
return nil, err
return err
}
// count failed login attempts during last AuthenticateRateLimiterDuration
@@ -546,22 +538,20 @@ func (r *Reconciler) syncUserStatus(ctx context.Context, user *iamv1alpha2.User)
// block user if failed login attempts exceeds maximum tries setting
if failedLoginAttempts >= r.AuthenticationOptions.AuthenticateRateLimiterMaxTries {
expected := user.DeepCopy()
limitExceed := iamv1alpha2.UserAuthLimitExceeded
expected.Status = iamv1alpha2.UserStatus{
user.Status = iamv1alpha2.UserStatus{
State: &limitExceed,
Reason: fmt.Sprintf("Failed login attempts exceed %d in last %s", failedLoginAttempts, r.AuthenticationOptions.AuthenticateRateLimiterDuration),
LastTransitionTime: &metav1.Time{Time: time.Now()},
}
err = r.Update(ctx, expected, &client.UpdateOptions{})
err = r.Update(ctx, user, &client.UpdateOptions{})
if err != nil {
return nil, err
return err
}
return expected, nil
}
return user, nil
return nil
}
func encrypt(password string) (string, error) {

19
pkg/controller/user/user_controller_test.go
View File

@@ -99,7 +99,7 @@ func TestDoNothing(t *testing.T) {
t.Fatal(err)
}
_, err = c.Reconcile(context.Background(), reconcile.Request{
result, err := c.Reconcile(context.Background(), reconcile.Request{
NamespacedName: types.NamespacedName{Name: user.Name},
})
if err != nil {
@@ -108,22 +108,15 @@ func TestDoNothing(t *testing.T) {
// append finalizer
updateEvent := <-w.ResultChan()
assert.Equal(t, updateEvent.Type, watch.Modified)
assert.Equal(t, watch.Modified, updateEvent.Type)
assert.NotNil(t, updateEvent.Object)
user = updateEvent.Object.(*iamv1alpha2.User)
assert.NotNil(t, user)
assert.NotEmpty(t, user.Finalizers)
result, err := c.Reconcile(context.Background(), reconcile.Request{
NamespacedName: types.NamespacedName{Name: user.Name},
})
if err != nil {
t.Fatal(err)
}
updateEvent = <-w.ResultChan()
// encrypt password
assert.Equal(t, updateEvent.Type, watch.Modified)
assert.Equal(t, watch.Modified, updateEvent.Type)
assert.NotNil(t, updateEvent.Object)
user = updateEvent.Object.(*iamv1alpha2.User)
assert.NotNil(t, user)
@@ -132,12 +125,12 @@ func TestDoNothing(t *testing.T) {
// becomes active after password encrypted
updateEvent = <-w.ResultChan()
user = updateEvent.Object.(*iamv1alpha2.User)
assert.Equal(t, *user.Status.State, iamv1alpha2.UserActive)
assert.Equal(t, iamv1alpha2.UserActive, *user.Status.State)
// block user
updateEvent = <-w.ResultChan()
user = updateEvent.Object.(*iamv1alpha2.User)
assert.Equal(t, *user.Status.State, iamv1alpha2.UserAuthLimitExceeded)
assert.Equal(t, iamv1alpha2.UserAuthLimitExceeded, *user.Status.State)
assert.True(t, result.Requeue)
time.Sleep(result.RequeueAfter + time.Second)
@@ -151,5 +144,5 @@ func TestDoNothing(t *testing.T) {
// unblock user
updateEvent = <-w.ResultChan()
user = updateEvent.Object.(*iamv1alpha2.User)
assert.Equal(t, *user.Status.State, iamv1alpha2.UserActive)
assert.Equal(t, iamv1alpha2.UserActive, *user.Status.State)
}

7
pkg/controller/workspacerole/workspacerole_controller.go
View File

@@ -122,7 +122,6 @@ func (r *Reconciler) bindWorkspace(ctx context.Context, logger logr.Logger, work
return client.IgnoreNotFound(err)
}
if !metav1.IsControlledBy(workspaceRole, &workspace) {
workspaceRole = workspaceRole.DeepCopy()
workspaceRole.OwnerReferences = k8sutil.RemoveWorkspaceOwnerReference(workspaceRole.OwnerReferences)
if err := controllerutil.SetControllerReference(&workspace, workspaceRole, r.Scheme); err != nil {
logger.Error(err, "set controller reference failed")
@@ -151,6 +150,7 @@ func (r *Reconciler) multiClusterSync(ctx context.Context, logger logr.Logger, w
logger.Error(err, "create federated workspace role failed")
return err
}
return nil
}
}
logger.Error(err, "get federated workspace role failed")
@@ -174,10 +174,6 @@ func (r *Reconciler) multiClusterSync(ctx context.Context, logger logr.Logger, w
func newFederatedWorkspaceRole(workspaceRole *iamv1alpha2.WorkspaceRole) (*typesv1beta1.FederatedWorkspaceRole, error) {
federatedWorkspaceRole := &typesv1beta1.FederatedWorkspaceRole{
TypeMeta: metav1.TypeMeta{
Kind: typesv1beta1.FederatedWorkspaceRoleKind,
APIVersion: typesv1beta1.SchemeGroupVersion.String(),
},
ObjectMeta: metav1.ObjectMeta{
Name: workspaceRole.Name,
},
@@ -206,7 +202,6 @@ func (r *Reconciler) ensureNotControlledByKubefed(ctx context.Context, logger lo
if workspaceRole.Labels == nil {
workspaceRole.Labels = make(map[string]string)
}
workspaceRole = workspaceRole.DeepCopy()
workspaceRole.Labels[constants.KubefedManagedLabel] = "false"
if err := r.Update(ctx, workspaceRole); err != nil {
logger.Error(err, "update kubefed managed label failed")

17
pkg/controller/workspacerolebinding/workspacerolebinding_controller.go
View File

@@ -123,7 +123,6 @@ func (r *Reconciler) bindWorkspace(ctx context.Context, logger logr.Logger, work
}
// owner reference not match workspace label
if !metav1.IsControlledBy(workspaceRoleBinding, workspace) {
workspaceRoleBinding := workspaceRoleBinding.DeepCopy()
workspaceRoleBinding.OwnerReferences = k8sutil.RemoveWorkspaceOwnerReference(workspaceRoleBinding.OwnerReferences)
if err := controllerutil.SetControllerReference(workspace, workspaceRoleBinding, r.Scheme); err != nil {
logger.Error(err, "set controller reference failed")
@@ -145,7 +144,7 @@ func (r *Reconciler) multiClusterSync(ctx context.Context, logger logr.Logger, w
federatedWorkspaceRoleBinding := &typesv1beta1.FederatedWorkspaceRoleBinding{}
if err := r.Client.Get(ctx, types.NamespacedName{Name: workspaceRoleBinding.Name}, federatedWorkspaceRoleBinding); err != nil {
if errors.IsNotFound(err) {
if federatedWorkspaceRoleBinding, err := newFederatedWorkspaceRole(workspaceRoleBinding); err != nil {
if federatedWorkspaceRoleBinding, err := newFederatedWorkspaceRoleBinding(workspaceRoleBinding); err != nil {
logger.Error(err, "generate federated workspace role binding failed")
return err
} else {
@@ -153,6 +152,7 @@ func (r *Reconciler) multiClusterSync(ctx context.Context, logger logr.Logger, w
logger.Error(err, "create federated workspace role binding failed")
return err
}
return nil
}
}
logger.Error(err, "get federated workspace role binding failed")
@@ -176,12 +176,8 @@ func (r *Reconciler) multiClusterSync(ctx context.Context, logger logr.Logger, w
return nil
}
func newFederatedWorkspaceRole(workspaceRoleBinding *iamv1alpha2.WorkspaceRoleBinding) (*typesv1beta1.FederatedWorkspaceRoleBinding, error) {
federatedWorkspaceRole := &typesv1beta1.FederatedWorkspaceRoleBinding{
TypeMeta: metav1.TypeMeta{
Kind: typesv1beta1.FederatedWorkspaceRoleBindingKind,
APIVersion: typesv1beta1.SchemeGroupVersion.String(),
},
func newFederatedWorkspaceRoleBinding(workspaceRoleBinding *iamv1alpha2.WorkspaceRoleBinding) (*typesv1beta1.FederatedWorkspaceRoleBinding, error) {
federatedWorkspaceRoleBinding := &typesv1beta1.FederatedWorkspaceRoleBinding{
ObjectMeta: metav1.ObjectMeta{
Name: workspaceRoleBinding.Name,
},
@@ -198,10 +194,10 @@ func newFederatedWorkspaceRole(workspaceRoleBinding *iamv1alpha2.WorkspaceRoleBi
},
},
}
if err := controllerutil.SetControllerReference(workspaceRoleBinding, federatedWorkspaceRole, scheme.Scheme); err != nil {
if err := controllerutil.SetControllerReference(workspaceRoleBinding, federatedWorkspaceRoleBinding, scheme.Scheme); err != nil {
return nil, err
}
return federatedWorkspaceRole, nil
return federatedWorkspaceRoleBinding, nil
}
func (r *Reconciler) ensureNotControlledByKubefed(ctx context.Context, logger logr.Logger, workspaceRoleBinding *iamv1alpha2.WorkspaceRoleBinding) error {
@@ -209,7 +205,6 @@ func (r *Reconciler) ensureNotControlledByKubefed(ctx context.Context, logger lo
if workspaceRoleBinding.Labels == nil {
workspaceRoleBinding.Labels = make(map[string]string)
}
workspaceRoleBinding = workspaceRoleBinding.DeepCopy()
workspaceRoleBinding.Labels[constants.KubefedManagedLabel] = "false"
logger.V(4).Info("update kubefed managed label")
if err := r.Update(ctx, workspaceRoleBinding); err != nil {

26
pkg/controller/workspacetemplate/workspacetemplate_controller.go
View File

@@ -230,10 +230,6 @@ func (r *Reconciler) multiClusterSync(ctx context.Context, logger logr.Logger, w
func newFederatedWorkspace(template *tenantv1alpha2.WorkspaceTemplate) (*typesv1beta1.FederatedWorkspace, error) {
federatedWorkspace := &typesv1beta1.FederatedWorkspace{
TypeMeta: metav1.TypeMeta{
Kind: typesv1beta1.FederatedWorkspaceRoleKind,
APIVersion: typesv1beta1.SchemeGroupVersion.String(),
},
ObjectMeta: metav1.ObjectMeta{
Name: template.Name,
Labels: template.Labels,
@@ -301,7 +297,6 @@ func (r *Reconciler) ensureNotControlledByKubefed(ctx context.Context, logger lo
if workspaceTemplate.Labels == nil {
workspaceTemplate.Labels = make(map[string]string)
}
workspaceTemplate = workspaceTemplate.DeepCopy()
workspaceTemplate.Labels[constants.KubefedManagedLabel] = "false"
logger.V(4).Info("update kubefed managed label")
if err := r.Update(ctx, workspaceTemplate); err != nil {
@@ -326,8 +321,8 @@ func (r *Reconciler) initWorkspaceRoles(ctx context.Context, logger logr.Logger,
expected.Labels = make(map[string]string)
}
expected.Labels[tenantv1alpha1.WorkspaceLabel] = workspace.Name
var existed iamv1alpha2.WorkspaceRole
if err := r.Get(ctx, types.NamespacedName{Name: expected.Name}, &existed); err != nil {
workspaceRole := &iamv1alpha2.WorkspaceRole{}
if err := r.Get(ctx, types.NamespacedName{Name: expected.Name}, workspaceRole); err != nil {
if errors.IsNotFound(err) {
logger.V(4).Info("create workspace role", "workspacerole", expected.Name)
if err := r.Create(ctx, &expected); err != nil {
@@ -340,15 +335,14 @@ func (r *Reconciler) initWorkspaceRoles(ctx context.Context, logger logr.Logger,
return err
}
}
if !reflect.DeepEqual(expected.Labels, existed.Labels) ||
!reflect.DeepEqual(expected.Annotations, existed.Annotations) ||
!reflect.DeepEqual(expected.Rules, existed.Rules) {
updated := existed.DeepCopy()
updated.Labels = expected.Labels
updated.Annotations = expected.Annotations
updated.Rules = expected.Rules
logger.V(4).Info("update workspace role", "workspacerole", updated.Name)
if err := r.Update(ctx, updated); err != nil {
if !reflect.DeepEqual(expected.Labels, workspaceRole.Labels) ||
!reflect.DeepEqual(expected.Annotations, workspaceRole.Annotations) ||
!reflect.DeepEqual(expected.Rules, workspaceRole.Rules) {
workspaceRole.Labels = expected.Labels
workspaceRole.Annotations = expected.Annotations
workspaceRole.Rules = expected.Rules
logger.V(4).Info("update workspace role", "workspacerole", workspaceRole.Name)
if err := r.Update(ctx, workspaceRole); err != nil {
logger.Error(err, "update workspace role failed")
return err
}