Browse code

Set user fullname correctly, initialize identity mappings fully

Jordan Liggitt authored on 2015/02/06 05:53:19
Showing 5 changed files
... ...
@@ -41,7 +41,7 @@
41 41
           <li class="dropdown" ng-if="user">
42 42
             <a href="#" class="dropdown-toggle" data-toggle="dropdown">
43 43
               <span class="pficon pficon-user"></span>
44
-              {{user.metadata.name}} <b class="caret"></b>
44
+              {{user.fullName || user.metadata.name}} <b class="caret"></b>
45 45
             </a>
46 46
             <ul class="dropdown-menu">
47 47
               <!--
... ...
@@ -11784,7 +11784,7 @@ func images_touch_icon_precomposed_png() ([]byte, error) {
11784 11784
 	return _images_touch_icon_precomposed_png, nil
11785 11785
 }
11786 11786
 
11787
-var _index_html = []byte(`<!doctype html> <html class="no-js"> <head> <meta charset="utf-8"> <base href="/"> <title>OpenShift Management Console</title> <meta name="description" content=""> <meta name="viewport" content="width=device-width"> <!-- Place favicon.ico and apple-touch-icon.png in the root directory --> <link rel="stylesheet" href="styles/main.css"> <style type="text/css"></style>  <body class="console-os"> <!-- Add your site or application content here --> <nav class="navbar navbar-default navbar-pf" role="navigation"> <div class="navbar-header"> <button type="button" class="navbar-toggle" data-toggle="collapse" data-target=".navbar-collapse-1"> <span class="sr-only">Toggle navigation</span> <span class="icon-bar"></span> <span class="icon-bar"></span> <span class="icon-bar"></span> </button> <a class="navbar-brand" href="/"> <img src="images/logo-origin.svg" alt="OpenShift Origin"> </a> </div> <div class="collapse navbar-collapse navbar-collapse-1"> <ul class="nav navbar-nav navbar-utility"> <li> <a href="http://docs.openshift.org/latest/welcome/index.html">Documentation</a> </li> <li class="dropdown" ng-if="user"> <a href="#" class="dropdown-toggle" data-toggle="dropdown"> <span class="pficon pficon-user"></span> {{user.metadata.name}} <b class="caret"></b> </a> <ul class="dropdown-menu"> <!--
11787
+var _index_html = []byte(`<!doctype html> <html class="no-js"> <head> <meta charset="utf-8"> <base href="/"> <title>OpenShift Management Console</title> <meta name="description" content=""> <meta name="viewport" content="width=device-width"> <!-- Place favicon.ico and apple-touch-icon.png in the root directory --> <link rel="stylesheet" href="styles/main.css"> <style type="text/css"></style>  <body class="console-os"> <!-- Add your site or application content here --> <nav class="navbar navbar-default navbar-pf" role="navigation"> <div class="navbar-header"> <button type="button" class="navbar-toggle" data-toggle="collapse" data-target=".navbar-collapse-1"> <span class="sr-only">Toggle navigation</span> <span class="icon-bar"></span> <span class="icon-bar"></span> <span class="icon-bar"></span> </button> <a class="navbar-brand" href="/"> <img src="images/logo-origin.svg" alt="OpenShift Origin"> </a> </div> <div class="collapse navbar-collapse navbar-collapse-1"> <ul class="nav navbar-nav navbar-utility"> <li> <a href="http://docs.openshift.org/latest/welcome/index.html">Documentation</a> </li> <li class="dropdown" ng-if="user"> <a href="#" class="dropdown-toggle" data-toggle="dropdown"> <span class="pficon pficon-user"></span> {{user.fullName || user.metadata.name}} <b class="caret"></b> </a> <ul class="dropdown-menu"> <!--
11788 11788
               <li>
11789 11789
                 <a href="#">Account</a>
11790 11790
               </li>
... ...
@@ -13,13 +13,11 @@ func NewDefaultUserInitStrategy() Initializer {
13 13
 
14 14
 // InitializeUser implements Initializer
15 15
 func (*DefaultUserInitStrategy) InitializeUser(identity *api.Identity, user *api.User) error {
16
-	if identity.Extra == nil {
17
-		return nil
16
+	user.FullName = identity.UserName
17
+	if identity.Extra != nil {
18
+		if name, ok := identity.Extra["name"]; ok && len(name) > 0 {
19
+			user.FullName = name
20
+		}
18 21
 	}
19
-	name, ok := identity.Extra["name"]
20
-	if !ok {
21
-		name = identity.UserName
22
-	}
23
-	user.FullName = name
24 22
 	return nil
25 23
 }
... ...
@@ -4,11 +4,10 @@ import (
4 4
 	"errors"
5 5
 	"fmt"
6 6
 
7
-	"code.google.com/p/go-uuid/uuid"
8 7
 	etcderrs "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors/etcd"
9 8
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
10 9
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/tools"
11
-	"github.com/GoogleCloudPlatform/kubernetes/pkg/types"
10
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
12 11
 
13 12
 	"github.com/openshift/origin/pkg/user"
14 13
 	"github.com/openshift/origin/pkg/user/api"
... ...
@@ -53,7 +52,6 @@ func (r *Etcd) GetUserIdentityMapping(name string) (mapping *api.UserIdentityMap
53 53
 func (r *Etcd) CreateOrUpdateUserIdentityMapping(mapping *api.UserIdentityMapping) (*api.UserIdentityMapping, bool, error) {
54 54
 	// Create Identity.Name by combining Provider and UserName
55 55
 	name := fmt.Sprintf("%s:%s", mapping.Identity.Provider, mapping.Identity.UserName)
56
-	mapping.Identity.Name = name
57 56
 	key := makeUserKey(name)
58 57
 
59 58
 	// track the object we set into etcd to return
... ...
@@ -64,18 +62,36 @@ func (r *Etcd) CreateOrUpdateUserIdentityMapping(mapping *api.UserIdentityMappin
64 64
 		existing := *in.(*api.UserIdentityMapping)
65 65
 
66 66
 		// did not previously exist
67
-		if existing.Identity.Name == "" {
68
-			uid := uuid.New()
69
-			existing.User.UID = types.UID(uid)
67
+		if existing.Name == "" {
68
+			now := util.Now()
69
+
70
+			// TODO: move these initializations the rest layer once we stop using the registry directly
71
+			existing.Name = name
72
+			existing.UID = util.NewUUID()
73
+			existing.CreationTimestamp = now
74
+			existing.Identity = mapping.Identity
75
+
76
+			identityuid := util.NewUUID()
77
+			existing.Identity.Name = name
78
+			existing.Identity.UID = identityuid
79
+			existing.Identity.CreationTimestamp = now
80
+
81
+			useruid := util.NewUUID()
70 82
 			existing.User.Name = name
71
-			if err := r.initializer.InitializeUser(&mapping.Identity, &existing.User); err != nil {
83
+			existing.User.UID = useruid
84
+			existing.User.CreationTimestamp = now
85
+
86
+			if err := r.initializer.InitializeUser(&existing.Identity, &existing.User); err != nil {
72 87
 				return in, err
73 88
 			}
74 89
 
75 90
 			// set these again to prevent bad initialization from messing up data
76
-			existing.User.UID = types.UID(uid)
91
+			existing.Identity.Name = name
92
+			existing.Identity.UID = identityuid
93
+			existing.Identity.CreationTimestamp = now
77 94
 			existing.User.Name = name
78
-			existing.Identity = mapping.Identity
95
+			existing.User.UID = useruid
96
+			existing.User.CreationTimestamp = now
79 97
 
80 98
 			found = &existing
81 99
 			created = true
... ...
@@ -72,17 +72,58 @@ func TestUserInitialization(t *testing.T) {
72 72
 	}
73 73
 
74 74
 	expectedUser := api.User{
75
-		ObjectMeta: kapi.ObjectMeta{Name: ":test"},
76
-		FullName:   "Mr. Test",
75
+		ObjectMeta: kapi.ObjectMeta{
76
+			Name: ":test",
77
+			// Copy the UID and timestamp from the actual one
78
+			UID:               actual.User.UID,
79
+			CreationTimestamp: actual.User.CreationTimestamp,
80
+		},
81
+		FullName: "Mr. Test",
77 82
 	}
78
-	expectedUser.Name = ":test"
79
-	expectedUser.UID = actual.User.UID
83
+	// Copy the UID and timestamp from the actual one
84
+	mapping.Identity.UID = actual.Identity.UID
85
+	mapping.Identity.CreationTimestamp = actual.Identity.CreationTimestamp
80 86
 	expected := &api.UserIdentityMapping{
87
+		ObjectMeta: kapi.ObjectMeta{
88
+			Name: ":test",
89
+			// Copy the UID and timestamp from the actual one
90
+			UID:               actual.UID,
91
+			CreationTimestamp: actual.CreationTimestamp,
92
+		},
81 93
 		Identity: mapping.Identity,
82 94
 		User:     expectedUser,
83 95
 	}
84 96
 	compareIgnoringSelfLinkAndVersion(t, expected, actual)
85 97
 
98
+	// Make sure uid, name, and creation timestamp get initialized
99
+	if len(actual.UID) == 0 {
100
+		t.Fatalf("Expected UID to be set")
101
+	}
102
+	if len(actual.Name) == 0 {
103
+		t.Fatalf("Expected Name to be set")
104
+	}
105
+	if actual.CreationTimestamp.IsZero() {
106
+		t.Fatalf("Expected CreationTimestamp to be set")
107
+	}
108
+	if len(actual.User.UID) == 0 {
109
+		t.Fatalf("Expected UID to be set")
110
+	}
111
+	if len(actual.User.Name) == 0 {
112
+		t.Fatalf("Expected Name to be set")
113
+	}
114
+	if actual.User.CreationTimestamp.IsZero() {
115
+		t.Fatalf("Expected CreationTimestamp to be set")
116
+	}
117
+	if len(actual.Identity.UID) == 0 {
118
+		t.Fatalf("Expected UID to be set")
119
+	}
120
+	if len(actual.Identity.Name) == 0 {
121
+		t.Fatalf("Expected Name to be set")
122
+	}
123
+	if actual.Identity.CreationTimestamp.IsZero() {
124
+		t.Fatalf("Expected CreationTimestamp to be set")
125
+	}
126
+
86 127
 	user, err := userRegistry.GetUser(expected.User.Name)
87 128
 	if err != nil {
88 129
 		t.Fatalf("unexpected error: %v", err)
... ...
@@ -158,12 +199,24 @@ func TestUserLookup(t *testing.T) {
158 158
 		// TODO: t.Errorf("expected created to be true")
159 159
 	}
160 160
 	expectedUser := api.User{
161
-		ObjectMeta: kapi.ObjectMeta{Name: ":test"},
162
-		FullName:   "Mr. Test",
161
+		ObjectMeta: kapi.ObjectMeta{
162
+			Name: ":test",
163
+			// Copy the UID and timestamp from the actual one
164
+			UID:               actual.User.UID,
165
+			CreationTimestamp: actual.User.CreationTimestamp,
166
+		},
167
+		FullName: "Mr. Test",
163 168
 	}
164
-	expectedUser.Name = ":test"
165
-	expectedUser.UID = actual.User.UID
169
+	// Copy the UID and timestamp from the actual one
170
+	mapping.Identity.UID = actual.Identity.UID
171
+	mapping.Identity.CreationTimestamp = actual.Identity.CreationTimestamp
166 172
 	expected := &api.UserIdentityMapping{
173
+		ObjectMeta: kapi.ObjectMeta{
174
+			Name: ":test",
175
+			// Copy the UID and timestamp from the actual one
176
+			UID:               actual.UID,
177
+			CreationTimestamp: actual.CreationTimestamp,
178
+		},
167 179
 		Identity: mapping.Identity,
168 180
 		User:     expectedUser,
169 181
 	}