diff --git a/resource/user/user.go b/resource/user/user.go index 9628cdea5..f62997c72 100644 --- a/resource/user/user.go +++ b/resource/user/user.go @@ -131,19 +131,10 @@ func NewUser(system SystemUtils) *User { // Check if a user user exists func (u *User) Check(context.Context, resource.Renderer) (resource.TaskStatus, error) { - var ( - userByID *user.User - uidErr error - ) - - // lookup the user by name and lookup the user by uid - // the lookups return ErrUnsupported if the system is not supported + // lookup the user by name + // ErrUnsupported is returned if the system is not supported // Lookup returns user.UnknownUserError if the user is not found - // LookupID returns user.UnknownUserIdError if the uid is not found userByName, nameErr := u.system.Lookup(u.Username) - if u.UID != "" { - userByID, uidErr = u.system.LookupID(u.UID) - } status := resource.NewStatus() @@ -152,10 +143,10 @@ func (u *User) Check(context.Context, resource.Renderer) (resource.TaskStatus, e return status, ErrUnsupported } + _, nameNotFound := nameErr.(user.UnknownUserError) + switch u.State { case StatePresent: - _, nameNotFound := nameErr.(user.UnknownUserError) - switch { case nameNotFound: _, err := u.DiffAdd(status) @@ -175,37 +166,12 @@ func (u *User) Check(context.Context, resource.Renderer) (resource.TaskStatus, e } } case StateAbsent: - switch { - case u.UID == "": - _, nameNotFound := nameErr.(user.UnknownUserError) - - switch { - case nameNotFound: - status.AddMessage(fmt.Sprintf("user %s does not exist", u.Username)) - case userByName != nil: - status.RaiseLevel(resource.StatusWillChange) - status.AddDifference("user", fmt.Sprintf("user %s", u.Username), fmt.Sprintf("<%s>", string(StateAbsent)), "") - } - case u.UID != "": - _, nameNotFound := nameErr.(user.UnknownUserError) - _, uidNotFound := uidErr.(user.UnknownUserIdError) - - switch { - case nameNotFound && uidNotFound: - status.AddMessage(fmt.Sprintf("user %s and uid %s do not exist", u.Username, u.UID)) - case nameNotFound: - status.RaiseLevel(resource.StatusCantChange) - return status, fmt.Errorf("cannot delete user %s with uid %s: user does not exist", u.Username, u.UID) - case uidNotFound: - status.RaiseLevel(resource.StatusCantChange) - return status, fmt.Errorf("cannot delete user %s with uid %s: uid does not exist", u.Username, u.UID) - case userByName != nil && userByID != nil && userByName.Name != userByID.Name || userByName.Uid != userByID.Uid: - status.RaiseLevel(resource.StatusCantChange) - return status, fmt.Errorf("cannot delete user %s with uid %s: user and uid belong to different users", u.Username, u.UID) - case userByName != nil && userByID != nil && *userByName == *userByID: - status.RaiseLevel(resource.StatusWillChange) - status.AddDifference("user", fmt.Sprintf("user %s with uid %s", u.Username, u.UID), fmt.Sprintf("<%s>", string(StateAbsent)), "") - } + err := u.DiffDel(status, userByName, nameNotFound) + if err != nil { + return status, errors.Wrapf(err, "cannot delete user %s", u.Username) + } + if resource.AnyChanges(status.Differences) { + status.AddMessage("delete user") } default: status.RaiseLevel(resource.StatusFatal) @@ -217,19 +183,10 @@ func (u *User) Check(context.Context, resource.Renderer) (resource.TaskStatus, e // Apply changes for user func (u *User) Apply(context.Context) (resource.TaskStatus, error) { - var ( - userByID *user.User - uidErr error - ) - - // lookup the user by name and lookup the user by uid - // the lookups return ErrUnsupported if the system is not supported + // lookup the user by name + // ErrUnsupported is returned if the system is not supported // Lookup returns user.UnknownUserError if the user is not found - // LookupID returns user.UnknownUserIdError if the uid is not found userByName, nameErr := u.system.Lookup(u.Username) - if u.UID != "" { - userByID, uidErr = u.system.LookupID(u.UID) - } status := resource.NewStatus() @@ -238,10 +195,10 @@ func (u *User) Apply(context.Context) (resource.TaskStatus, error) { return status, ErrUnsupported } + _, nameNotFound := nameErr.(user.UnknownUserError) + switch u.State { case StatePresent: - _, nameNotFound := nameErr.(user.UnknownUserError) - switch { case nameNotFound: options, err := u.DiffAdd(status) @@ -273,40 +230,18 @@ func (u *User) Apply(context.Context) (resource.TaskStatus, error) { } } case StateAbsent: - switch { - case u.UID == "": - _, nameNotFound := nameErr.(user.UnknownUserError) - - switch { - case !nameNotFound && userByName != nil: - err := u.system.DelUser(u.Username) - if err != nil { - status.RaiseLevel(resource.StatusFatal) - status.AddMessage(fmt.Sprintf("error deleting user %s", u.Username)) - return status, errors.Wrap(err, "user delete") - } - status.AddMessage(fmt.Sprintf("deleted user %s", u.Username)) - default: - status.RaiseLevel(resource.StatusCantChange) - return status, fmt.Errorf("will not attempt to delete user %s", u.Username) - } - case u.UID != "": - _, nameNotFound := nameErr.(user.UnknownUserError) - _, uidNotFound := uidErr.(user.UnknownUserIdError) - - switch { - case !nameNotFound && !uidNotFound && userByName != nil && userByID != nil && *userByName == *userByID: - err := u.system.DelUser(u.Username) - if err != nil { - status.RaiseLevel(resource.StatusFatal) - status.AddMessage(fmt.Sprintf("error deleting user %s with uid %s", u.Username, u.UID)) - return status, errors.Wrap(err, "user delete") - } - status.AddMessage(fmt.Sprintf("deleted user %s with uid %s", u.Username, u.UID)) - default: - status.RaiseLevel(resource.StatusCantChange) - return status, fmt.Errorf("will not attempt to delete user %s with uid %s", u.Username, u.UID) + err := u.DiffDel(status, userByName, nameNotFound) + if err != nil { + return status, errors.Wrapf(err, "will not attempt to delete user %s", u.Username) + } + if resource.AnyChanges(status.Differences) { + err = u.system.DelUser(u.Username) + if err != nil { + status.RaiseLevel(resource.StatusFatal) + status.AddMessage(fmt.Sprintf("error deleting user %s", u.Username)) + return status, errors.Wrap(err, "user delete") } + status.AddMessage(fmt.Sprintf("deleted user %s", u.Username)) } default: status.RaiseLevel(resource.StatusFatal) @@ -401,6 +336,39 @@ func (u *User) DiffAdd(status *resource.Status) (*AddUserOptions, error) { return options, nil } +// DiffDel checks for differences between the current and desired state for the +// user to be deleted indicated by the User fields. +func (u *User) DiffDel(status *resource.Status, userByName *user.User, nameNotFound bool) error { + if nameNotFound || userByName == nil { + return nil + } + + switch { + case u.UID == "": + status.AddDifference("user", u.Username, fmt.Sprintf("<%s>", string(StateAbsent)), "") + case u.UID != "": + userByID, err := user.LookupId(u.UID) + _, uidNotFound := err.(user.UnknownUserIdError) + + switch { + case uidNotFound: + status.RaiseLevel(resource.StatusCantChange) + return fmt.Errorf("uid %s does not exist", u.UID) + case userByID != nil && *userByID != *userByName: + status.RaiseLevel(resource.StatusCantChange) + return fmt.Errorf("uid %s belongs to different user", u.UID) + case userByID != nil && *userByID == *userByName: + status.AddDifference("user", u.Username, fmt.Sprintf("<%s>", string(StateAbsent)), "") + } + } + + if resource.AnyChanges(status.Differences) { + status.RaiseLevel(resource.StatusWillChange) + } + + return nil +} + // DiffMod checks for differences between the user associated with u.Username // and the desired modifications of that user indicated by the other User // fields. The options to be used for the modify command are set. diff --git a/resource/user/user_test.go b/resource/user/user_test.go index a097ab8e2..ce3a7528f 100644 --- a/resource/user/user_test.go +++ b/resource/user/user_test.go @@ -223,110 +223,32 @@ func TestCheck(t *testing.T) { u := user.NewUser(new(user.System)) u.State = user.StateAbsent - t.Run("uid not provided", func(t *testing.T) { - t.Run("no delete-user does not exist", func(t *testing.T) { - u.Username = fakeUsername - status, err := u.Check(context.Background(), fakerenderer.New()) - - if runtime.GOOS == "linux" { - assert.NoError(t, err) - assert.Equal(t, resource.StatusNoChange, status.StatusCode()) - assert.Equal(t, fmt.Sprintf("user %s does not exist", u.Username), status.Messages()[0]) - assert.False(t, status.HasChanges()) - } else { - assert.EqualError(t, err, "user: not supported on this system") - } - }) - - t.Run("delete user", func(t *testing.T) { - u.Username = currUsername - status, err := u.Check(context.Background(), fakerenderer.New()) + t.Run("no delete-user does not exist", func(t *testing.T) { + u.Username = fakeUsername + status, err := u.Check(context.Background(), fakerenderer.New()) - if runtime.GOOS == "linux" { - assert.NoError(t, err) - assert.Equal(t, resource.StatusWillChange, status.StatusCode()) - assert.Equal(t, fmt.Sprintf("user %s", u.Username), status.Diffs()["user"].Original()) - assert.Equal(t, fmt.Sprintf("<%s>", string(user.StateAbsent)), status.Diffs()["user"].Current()) - assert.True(t, status.HasChanges()) - } else { - assert.EqualError(t, err, "user: not supported on this system") - } - }) + if runtime.GOOS == "linux" { + assert.NoError(t, err) + assert.Equal(t, resource.StatusNoChange, status.StatusCode()) + assert.False(t, status.HasChanges()) + } else { + assert.EqualError(t, err, "user: not supported on this system") + } }) - t.Run("uid provided", func(t *testing.T) { - t.Run("no delete-user name and uid do not exist", func(t *testing.T) { - u.Username = fakeUsername - u.UID = fakeUID - status, err := u.Check(context.Background(), fakerenderer.New()) - - if runtime.GOOS == "linux" { - assert.NoError(t, err) - assert.Equal(t, resource.StatusNoChange, status.StatusCode()) - assert.Equal(t, fmt.Sprintf("user %s and uid %s do not exist", u.Username, u.UID), status.Messages()[0]) - assert.False(t, status.HasChanges()) - } else { - assert.EqualError(t, err, "user: not supported on this system") - } - }) - - t.Run("no delete-user name does not exist", func(t *testing.T) { - u.Username = fakeUsername - u.UID = currUID - status, err := u.Check(context.Background(), fakerenderer.New()) - - if runtime.GOOS == "linux" { - assert.EqualError(t, err, fmt.Sprintf("cannot delete user %s with uid %s: user does not exist", u.Username, u.UID)) - assert.Equal(t, resource.StatusCantChange, status.StatusCode()) - assert.True(t, status.HasChanges()) - } else { - assert.EqualError(t, err, "user: not supported on this system") - } - }) - - t.Run("no delete-user uid does not exist", func(t *testing.T) { - u.Username = currUsername - u.UID = fakeUID - status, err := u.Check(context.Background(), fakerenderer.New()) - - if runtime.GOOS == "linux" { - assert.EqualError(t, err, fmt.Sprintf("cannot delete user %s with uid %s: uid does not exist", u.Username, u.UID)) - assert.Equal(t, resource.StatusCantChange, status.StatusCode()) - assert.True(t, status.HasChanges()) - } else { - assert.EqualError(t, err, "user: not supported on this system") - } - }) - - t.Run("no delete-user name and uid belong to different users", func(t *testing.T) { - u.Username = currUsername - u.UID = existingUID - status, err := u.Check(context.Background(), fakerenderer.New()) - - if runtime.GOOS == "linux" { - assert.EqualError(t, err, fmt.Sprintf("cannot delete user %s with uid %s: user and uid belong to different users", u.Username, u.UID)) - assert.Equal(t, resource.StatusCantChange, status.StatusCode()) - assert.True(t, status.HasChanges()) - } else { - assert.EqualError(t, err, "user: not supported on this system") - } - }) - - t.Run("delete user with uid", func(t *testing.T) { - u.Username = currUsername - u.UID = currUID - status, err := u.Check(context.Background(), fakerenderer.New()) + t.Run("delete user", func(t *testing.T) { + u.Username = currUsername + status, err := u.Check(context.Background(), fakerenderer.New()) - if runtime.GOOS == "linux" { - assert.NoError(t, err) - assert.Equal(t, resource.StatusWillChange, status.StatusCode()) - assert.Equal(t, fmt.Sprintf("user %s with uid %s", u.Username, u.UID), status.Diffs()["user"].Original()) - assert.Equal(t, fmt.Sprintf("<%s>", string(user.StateAbsent)), status.Diffs()["user"].Current()) - assert.True(t, status.HasChanges()) - } else { - assert.EqualError(t, err, "user: not supported on this system") - } - }) + if runtime.GOOS == "linux" { + assert.NoError(t, err) + assert.Equal(t, resource.StatusWillChange, status.StatusCode()) + assert.Equal(t, u.Username, status.Diffs()["user"].Original()) + assert.Equal(t, fmt.Sprintf("<%s>", string(user.StateAbsent)), status.Diffs()["user"].Current()) + assert.True(t, status.HasChanges()) + } else { + assert.EqualError(t, err, "user: not supported on this system") + } }) }) @@ -500,127 +422,62 @@ func TestApply(t *testing.T) { }) t.Run("state=absent", func(t *testing.T) { - t.Run("uid not provided", func(t *testing.T) { - t.Run("delete user", func(t *testing.T) { - usr := &os.User{ - Username: fakeUsername, - } - m := &MockSystem{} - u := user.NewUser(m) - u.Username = usr.Username - u.State = user.StateAbsent - - m.On("Lookup", u.Username).Return(usr, nil) - m.On("DelUser", u.Username).Return(nil) - status, err := u.Apply(context.Background()) - - m.AssertCalled(t, "DelUser", u.Username) - assert.NoError(t, err) - assert.Equal(t, fmt.Sprintf("deleted user %s", u.Username), status.Messages()[0]) - }) - - t.Run("no delete-error deleting user", func(t *testing.T) { - usr := &os.User{ - Username: fakeUsername, - } - m := &MockSystem{} - u := user.NewUser(m) - u.Username = usr.Username - u.State = user.StateAbsent - - m.On("Lookup", u.Username).Return(usr, nil) - m.On("DelUser", u.Username).Return(fmt.Errorf("")) - status, err := u.Apply(context.Background()) - - m.AssertCalled(t, "DelUser", u.Username) - assert.EqualError(t, err, "user delete: ") - assert.Equal(t, resource.StatusFatal, status.StatusCode()) - assert.Equal(t, fmt.Sprintf("error deleting user %s", u.Username), status.Messages()[0]) - }) - - t.Run("no delete-will not attempt delete", func(t *testing.T) { - usr := &os.User{ - Username: fakeUsername, - } - m := &MockSystem{} - u := user.NewUser(m) - u.Username = usr.Username - u.State = user.StateAbsent + t.Run("delete user", func(t *testing.T) { + usr := &os.User{ + Username: fakeUsername, + } + m := &MockSystem{} + u := user.NewUser(m) + u.Username = usr.Username + u.State = user.StateAbsent - m.On("Lookup", u.Username).Return(usr, os.UnknownUserError("")) - m.On("DelUser", u.Username).Return(nil) - status, err := u.Apply(context.Background()) + m.On("Lookup", u.Username).Return(usr, nil) + m.On("DelUser", u.Username).Return(nil) + status, err := u.Apply(context.Background()) - m.AssertNotCalled(t, "DelUser", u.Username) - assert.EqualError(t, err, fmt.Sprintf("will not attempt to delete user %s", u.Username)) - assert.Equal(t, resource.StatusCantChange, status.StatusCode()) - }) + m.AssertCalled(t, "DelUser", u.Username) + assert.NoError(t, err) + assert.Equal(t, fmt.Sprintf("deleted user %s", u.Username), status.Messages()[0]) + assert.Equal(t, u.Username, status.Diffs()["user"].Original()) + assert.Equal(t, fmt.Sprintf("<%s>", string(user.StateAbsent)), status.Diffs()["user"].Current()) + assert.True(t, status.HasChanges()) }) - t.Run("uid provided", func(t *testing.T) { - t.Run("delete user with uid", func(t *testing.T) { - usr := &os.User{ - Username: fakeUsername, - Uid: fakeUID, - } - m := &MockSystem{} - u := user.NewUser(m) - u.Username = usr.Username - u.UID = usr.Uid - u.State = user.StateAbsent - - m.On("Lookup", u.Username).Return(usr, nil) - m.On("LookupID", u.UID).Return(usr, nil) - m.On("DelUser", u.Username).Return(nil) - status, err := u.Apply(context.Background()) - - m.AssertCalled(t, "DelUser", u.Username) - assert.NoError(t, err) - assert.Equal(t, fmt.Sprintf("deleted user %s with uid %s", u.Username, u.UID), status.Messages()[0]) - }) - - t.Run("no delete-error deleting user", func(t *testing.T) { - usr := &os.User{ - Username: fakeUsername, - Uid: fakeUID, - } - m := &MockSystem{} - u := user.NewUser(m) - u.Username = usr.Username - u.UID = usr.Uid - u.State = user.StateAbsent + t.Run("no delete-error deleting user", func(t *testing.T) { + usr := &os.User{ + Username: fakeUsername, + } + m := &MockSystem{} + u := user.NewUser(m) + u.Username = usr.Username + u.State = user.StateAbsent - m.On("Lookup", u.Username).Return(usr, nil) - m.On("LookupID", u.UID).Return(usr, nil) - m.On("DelUser", u.Username).Return(fmt.Errorf("")) - status, err := u.Apply(context.Background()) + m.On("Lookup", u.Username).Return(usr, nil) + m.On("DelUser", u.Username).Return(fmt.Errorf("")) + status, err := u.Apply(context.Background()) - m.AssertCalled(t, "DelUser", u.Username) - assert.EqualError(t, err, "user delete: ") - assert.Equal(t, resource.StatusFatal, status.StatusCode()) - assert.Equal(t, fmt.Sprintf("error deleting user %s with uid %s", u.Username, u.UID), status.Messages()[0]) - }) + m.AssertCalled(t, "DelUser", u.Username) + assert.EqualError(t, err, "user delete: ") + assert.Equal(t, resource.StatusFatal, status.StatusCode()) + assert.Equal(t, fmt.Sprintf("error deleting user %s", u.Username), status.Messages()[0]) + }) - t.Run("no delete-will not attempt delete", func(t *testing.T) { - usr := &os.User{ - Username: fakeUsername, - Uid: fakeUID, - } - m := &MockSystem{} - u := user.NewUser(m) - u.Username = usr.Username - u.UID = usr.Uid - u.State = user.StateAbsent + t.Run("no delete-will not attempt delete", func(t *testing.T) { + usr := &os.User{ + Username: fakeUsername, + } + m := &MockSystem{} + u := user.NewUser(m) + u.Username = usr.Username + u.State = user.StateAbsent - m.On("Lookup", u.Username).Return(usr, os.UnknownUserError("")) - m.On("LookupID", u.UID).Return(usr, nil) - m.On("DelUser", u.Username).Return(nil) - status, err := u.Apply(context.Background()) + m.On("Lookup", u.Username).Return(usr, os.UnknownUserError("")) + m.On("DelUser", u.Username).Return(nil) + status, err := u.Apply(context.Background()) - m.AssertNotCalled(t, "DelUser", u.Username) - assert.EqualError(t, err, fmt.Sprintf("will not attempt to delete user %s with uid %s", u.Username, u.UID)) - assert.Equal(t, resource.StatusCantChange, status.StatusCode()) - }) + m.AssertNotCalled(t, "DelUser", u.Username) + assert.NoError(t, err) + assert.Equal(t, resource.StatusNoChange, status.StatusCode()) }) }) @@ -1019,6 +876,109 @@ func TestDiffAdd(t *testing.T) { }) } +// TestDiffDel tests DiffDel for user +func TestDiffDel(t *testing.T) { + t.Parallel() + + t.Run("user does not exist", func(t *testing.T) { + u := user.NewUser(new(user.System)) + u.Username = fakeUsername + u.State = user.StateAbsent + status := resource.NewStatus() + + usr := os.User{} + + err := u.DiffDel(status, &usr, true) + + assert.NoError(t, err) + assert.Equal(t, resource.StatusNoChange, status.StatusCode()) + assert.False(t, status.HasChanges()) + }) + + t.Run("delete user", func(t *testing.T) { + u := user.NewUser(new(user.System)) + u.Username = currUsername + u.State = user.StateAbsent + status := resource.NewStatus() + + usr := os.User{Username: currUsername} + + err := u.DiffDel(status, &usr, false) + + assert.NoError(t, err) + assert.Equal(t, resource.StatusWillChange, status.StatusCode()) + assert.Equal(t, u.Username, status.Diffs()["user"].Original()) + assert.Equal(t, fmt.Sprintf("<%s>", string(user.StateAbsent)), status.Diffs()["user"].Current()) + assert.True(t, status.HasChanges()) + }) + + t.Run("uid provided", func(t *testing.T) { + t.Run("user does not exist", func(t *testing.T) { + u := user.NewUser(new(user.System)) + u.Username = fakeUsername + u.UID = fakeUID + u.State = user.StateAbsent + status := resource.NewStatus() + + usr := os.User{} + + err := u.DiffDel(status, &usr, true) + + assert.NoError(t, err) + assert.Equal(t, resource.StatusNoChange, status.StatusCode()) + assert.False(t, status.HasChanges()) + }) + + t.Run("uid does not exist", func(t *testing.T) { + u := user.NewUser(new(user.System)) + u.Username = currUsername + u.UID = fakeUID + u.State = user.StateAbsent + status := resource.NewStatus() + + usr := os.User{Username: currUsername, Uid: currUID} + + err := u.DiffDel(status, &usr, false) + + assert.EqualError(t, err, fmt.Sprintf("uid %s does not exist", u.UID)) + assert.Equal(t, resource.StatusCantChange, status.StatusCode()) + assert.True(t, status.HasChanges()) + }) + + t.Run("uid belongs to different user", func(t *testing.T) { + u := user.NewUser(new(user.System)) + u.Username = currUsername + u.UID = existingUID + u.State = user.StateAbsent + status := resource.NewStatus() + + usr := os.User{Username: currUsername, Uid: currUID} + + err := u.DiffDel(status, &usr, false) + + assert.EqualError(t, err, fmt.Sprintf("uid %s belongs to different user", u.UID)) + assert.Equal(t, resource.StatusCantChange, status.StatusCode()) + assert.True(t, status.HasChanges()) + }) + + t.Run("delete user", func(t *testing.T) { + u := user.NewUser(new(user.System)) + u.Username = currUsername + u.UID = currUID + u.State = user.StateAbsent + status := resource.NewStatus() + + err := u.DiffDel(status, currUser, false) + + assert.NoError(t, err) + assert.Equal(t, resource.StatusWillChange, status.StatusCode()) + assert.Equal(t, u.Username, status.Diffs()["user"].Original()) + assert.Equal(t, fmt.Sprintf("<%s>", string(user.StateAbsent)), status.Diffs()["user"].Current()) + assert.True(t, status.HasChanges()) + }) + }) +} + // TestDiffMod tests DiffMod for user func TestDiffMod(t *testing.T) { t.Parallel()