diff --git a/CHANGELOG.md b/CHANGELOG.md index 553c0e02a6..e627576152 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ 1. [#5882](https://github.com/influxdata/chronograf/pull/5882): Repair table visualization of string values. 1. [#5913](https://github.com/influxdata/chronograf/pull/5913): Improve InfluxDB Enterprise detection. +1. [#5917](https://github.com/influxdata/chronograf/pull/5917): Improve InfluxDB Enterprise user creation process. +1. [#5917](https://github.com/influxdata/chronograf/pull/5917): Avoid stale reads in communication with InfluxDB Enterprise meta nodes. ### Other diff --git a/enterprise/meta.go b/enterprise/meta.go index 2eb4d51aa0..7f28a67bd0 100644 --- a/enterprise/meta.go +++ b/enterprise/meta.go @@ -12,6 +12,7 @@ import ( "net" "net/http" "net/url" + "sync" "time" "github.com/influxdata/chronograf" @@ -36,12 +37,11 @@ var ( ) type client interface { - Do(URL *url.URL, path, method string, authorizer influx.Authorizer, params map[string]string, body io.Reader) (*http.Response, error) + Do(path, method string, authorizer influx.Authorizer, params map[string]string, body io.Reader) (*http.Response, error) } // MetaClient represents a Meta node in an Influx Enterprise cluster type MetaClient struct { - URL *url.URL client client authorizer influx.Authorizer } @@ -49,10 +49,7 @@ type MetaClient struct { // NewMetaClient represents a meta node in an Influx Enterprise cluster func NewMetaClient(url *url.URL, InsecureSkipVerify bool, authorizer influx.Authorizer) *MetaClient { return &MetaClient{ - URL: url, - client: &defaultClient{ - InsecureSkipVerify: InsecureSkipVerify, - }, + client: newDefaultClient(url, InsecureSkipVerify), authorizer: authorizer, } } @@ -206,6 +203,18 @@ func (m *MetaClient) RemoveUserPerms(ctx context.Context, name string, perms Per return m.Post(ctx, "/user", a, nil) } +// AddUserPerms adds permissions for a user in Influx Enterprise +func (m *MetaClient) AddUserPerms(ctx context.Context, name string, perms Permissions) error { + a := &UserAction{ + Action: "add-permissions", + User: &User{ + Name: name, + Permissions: perms, + }, + } + return m.Post(ctx, "/user", a, nil) +} + // SetUserPerms removes permissions not in set and then adds the requested perms func (m *MetaClient) SetUserPerms(ctx context.Context, name string, perms Permissions) error { user, err := m.User(ctx, name) @@ -226,14 +235,7 @@ func (m *MetaClient) SetUserPerms(ctx context.Context, name string, perms Permis // ... next, add any permissions the user should have if len(add) > 0 { - a := &UserAction{ - Action: "add-permissions", - User: &User{ - Name: name, - Permissions: add, - }, - } - return m.Post(ctx, "/user", a, nil) + return m.AddUserPerms(ctx, name, add) } return nil } @@ -475,14 +477,46 @@ func (m *MetaClient) Post(ctx context.Context, path string, action interface{}, type defaultClient struct { InsecureSkipVerify bool + URL *url.URL + + // masterURL is setup when doing redirects, communication with a master + // node prevents stale reads from follower nodes after master node modifications + masterURL *url.URL + mu sync.Mutex +} + +func newDefaultClient(URL *url.URL, InsecureSkipVerify bool) *defaultClient { + return &defaultClient{ + URL: URL, + InsecureSkipVerify: InsecureSkipVerify, + } +} + +func (d *defaultClient) setMasterURL(URL *url.URL) { + if URL.Host != "" && URL.Scheme != "" { + d.mu.Lock() + defer d.mu.Unlock() + d.masterURL = &url.URL{Host: URL.Host, Scheme: URL.Scheme} + } +} + +func (d *defaultClient) getMasterURL() url.URL { + d.mu.Lock() + defer d.mu.Unlock() + if d.masterURL != nil { + return *d.masterURL + } + return *d.URL } // Do is a helper function to interface with Influx Enterprise's Meta API -func (d *defaultClient) Do(URL *url.URL, path, method string, authorizer influx.Authorizer, params map[string]string, body io.Reader) (*http.Response, error) { +func (d *defaultClient) Do(path, method string, authorizer influx.Authorizer, params map[string]string, body io.Reader) (*http.Response, error) { p := url.Values{} for k, v := range params { p.Add(k, v) } + // prefer communication with the master node, to avoid stale reads + URL := d.getMasterURL() URL.Path = path URL.RawQuery = p.Encode() @@ -537,6 +571,7 @@ func (d *defaultClient) Do(URL *url.URL, path, method string, authorizer influx. // AuthedCheckRedirect tries to follow the Influx Enterprise pattern of // redirecting to the leader but preserving authentication headers. func (d *defaultClient) AuthedCheckRedirect(req *http.Request, via []*http.Request) error { + d.setMasterURL(req.URL) if len(via) >= 10 { return errors.New("too many redirects") } else if len(via) == 0 { @@ -558,7 +593,7 @@ func (m *MetaClient) Do(ctx context.Context, path, method string, authorizer inf resps := make(chan (result)) go func() { - resp, err := m.client.Do(m.URL, path, method, authorizer, params, body) + resp, err := m.client.Do(path, method, authorizer, params, body) resps <- result{resp, err} }() diff --git a/enterprise/meta_test.go b/enterprise/meta_test.go index 9f0dd8ff54..34c57c59ef 100644 --- a/enterprise/meta_test.go +++ b/enterprise/meta_test.go @@ -18,9 +18,8 @@ import ( func TestMetaClient_ShowCluster(t *testing.T) { type fields struct { - URL *url.URL client interface { - Do(URL *url.URL, path, method string, authorizer influx.Authorizer, params map[string]string, body io.Reader) (*http.Response, error) + Do(path, method string, authorizer influx.Authorizer, params map[string]string, body io.Reader) (*http.Response, error) } } tests := []struct { @@ -32,10 +31,6 @@ func TestMetaClient_ShowCluster(t *testing.T) { { name: "Successful Show Cluster", fields: fields{ - URL: &url.URL{ - Host: "twinpinesmall.net:8091", - Scheme: "https", - }, client: NewMockClient( http.StatusOK, []byte(`{"data":[{"id":2,"version":"1.1.0-c1.1.0","tcpAddr":"data-1.twinpinesmall.net:8088","httpAddr":"data-1.twinpinesmall.net:8086","httpScheme":"https","status":"joined"}],"meta":[{"id":1,"addr":"meta-0.twinpinesmall.net:8091","httpScheme":"http","tcpAddr":"meta-0.twinpinesmall.net:8089","version":"1.1.0-c1.1.0"}]}`), @@ -66,10 +61,6 @@ func TestMetaClient_ShowCluster(t *testing.T) { { name: "Failed Show Cluster", fields: fields{ - URL: &url.URL{ - Host: "twinpinesmall.net:8091", - Scheme: "https", - }, client: NewMockClient( http.StatusBadGateway, nil, @@ -82,10 +73,6 @@ func TestMetaClient_ShowCluster(t *testing.T) { { name: "Bad JSON from Show Cluster", fields: fields{ - URL: &url.URL{ - Host: "twinpinesmall.net:8091", - Scheme: "https", - }, client: NewMockClient( http.StatusOK, []byte(`{data}`), @@ -98,7 +85,6 @@ func TestMetaClient_ShowCluster(t *testing.T) { } for _, tt := range tests { m := &MetaClient{ - URL: tt.fields.URL, client: tt.fields.client, } got, err := m.ShowCluster(context.Background()) @@ -129,9 +115,8 @@ func TestMetaClient_ShowCluster(t *testing.T) { func TestMetaClient_Users(t *testing.T) { type fields struct { - URL *url.URL client interface { - Do(URL *url.URL, path, method string, authorizer influx.Authorizer, params map[string]string, body io.Reader) (*http.Response, error) + Do(path, method string, authorizer influx.Authorizer, params map[string]string, body io.Reader) (*http.Response, error) } } type args struct { @@ -148,10 +133,6 @@ func TestMetaClient_Users(t *testing.T) { { name: "Successful Show users", fields: fields{ - URL: &url.URL{ - Host: "twinpinesmall.net:8091", - Scheme: "https", - }, client: NewMockClient( http.StatusOK, []byte(`{"users":[{"name":"admin","hash":"1234","permissions":{"":["ViewAdmin","ViewChronograf"]}}]}`), @@ -179,10 +160,6 @@ func TestMetaClient_Users(t *testing.T) { { name: "Successful Show users single user", fields: fields{ - URL: &url.URL{ - Host: "twinpinesmall.net:8091", - Scheme: "https", - }, client: NewMockClient( http.StatusOK, []byte(`{"users":[{"name":"admin","hash":"1234","permissions":{"":["ViewAdmin","ViewChronograf"]}}]}`), @@ -210,10 +187,6 @@ func TestMetaClient_Users(t *testing.T) { { name: "Failure Show users", fields: fields{ - URL: &url.URL{ - Host: "twinpinesmall.net:8091", - Scheme: "https", - }, client: NewMockClient( http.StatusOK, []byte(`{"users":[{"name":"admin","hash":"1234","permissions":{"":["ViewAdmin","ViewChronograf"]}}]}`), @@ -230,10 +203,6 @@ func TestMetaClient_Users(t *testing.T) { { name: "Bad JSON from Show users", fields: fields{ - URL: &url.URL{ - Host: "twinpinesmall.net:8091", - Scheme: "https", - }, client: NewMockClient( http.StatusOK, []byte(`{foo}`), @@ -250,7 +219,6 @@ func TestMetaClient_Users(t *testing.T) { } for _, tt := range tests { m := &MetaClient{ - URL: tt.fields.URL, client: tt.fields.client, } got, err := m.Users(tt.args.ctx, tt.args.name) @@ -266,9 +234,8 @@ func TestMetaClient_Users(t *testing.T) { func TestMetaClient_User(t *testing.T) { type fields struct { - URL *url.URL client interface { - Do(URL *url.URL, path, method string, authorizer influx.Authorizer, params map[string]string, body io.Reader) (*http.Response, error) + Do(path, method string, authorizer influx.Authorizer, params map[string]string, body io.Reader) (*http.Response, error) } } type args struct { @@ -285,10 +252,6 @@ func TestMetaClient_User(t *testing.T) { { name: "Successful Show users", fields: fields{ - URL: &url.URL{ - Host: "twinpinesmall.net:8091", - Scheme: "https", - }, client: NewMockClient( http.StatusOK, []byte(`{"users":[{"name":"admin","hash":"1234","permissions":{"":["ViewAdmin","ViewChronograf"]}}]}`), @@ -312,10 +275,6 @@ func TestMetaClient_User(t *testing.T) { { name: "No such user", fields: fields{ - URL: &url.URL{ - Host: "twinpinesmall.net:8091", - Scheme: "https", - }, client: NewMockClient( http.StatusNotFound, []byte(`{"error":"user not found"}`), @@ -332,10 +291,6 @@ func TestMetaClient_User(t *testing.T) { { name: "Bad JSON", fields: fields{ - URL: &url.URL{ - Host: "twinpinesmall.net:8091", - Scheme: "https", - }, client: NewMockClient( http.StatusNotFound, []byte(`{BAD}`), @@ -351,7 +306,6 @@ func TestMetaClient_User(t *testing.T) { } for _, tt := range tests { m := &MetaClient{ - URL: tt.fields.URL, client: tt.fields.client, } got, err := m.User(tt.args.ctx, tt.args.name) @@ -367,9 +321,8 @@ func TestMetaClient_User(t *testing.T) { func TestMetaClient_CreateUser(t *testing.T) { type fields struct { - URL *url.URL client interface { - Do(URL *url.URL, path, method string, authorizer influx.Authorizer, params map[string]string, body io.Reader) (*http.Response, error) + Do(path, method string, authorizer influx.Authorizer, params map[string]string, body io.Reader) (*http.Response, error) } } type args struct { @@ -387,10 +340,6 @@ func TestMetaClient_CreateUser(t *testing.T) { { name: "Successful Create User", fields: fields{ - URL: &url.URL{ - Host: "twinpinesmall.net:8091", - Scheme: "https", - }, client: NewMockClient( http.StatusOK, nil, @@ -408,7 +357,6 @@ func TestMetaClient_CreateUser(t *testing.T) { } for _, tt := range tests { m := &MetaClient{ - URL: tt.fields.URL, client: tt.fields.client, } if err := m.CreateUser(tt.args.ctx, tt.args.name, tt.args.passwd); (err != nil) != tt.wantErr { @@ -438,9 +386,8 @@ func TestMetaClient_CreateUser(t *testing.T) { func TestMetaClient_ChangePassword(t *testing.T) { type fields struct { - URL *url.URL client interface { - Do(URL *url.URL, path, method string, authorizer influx.Authorizer, params map[string]string, body io.Reader) (*http.Response, error) + Do(path, method string, authorizer influx.Authorizer, params map[string]string, body io.Reader) (*http.Response, error) } } type args struct { @@ -458,10 +405,6 @@ func TestMetaClient_ChangePassword(t *testing.T) { { name: "Successful Change Password", fields: fields{ - URL: &url.URL{ - Host: "twinpinesmall.net:8091", - Scheme: "https", - }, client: NewMockClient( http.StatusOK, nil, @@ -479,7 +422,6 @@ func TestMetaClient_ChangePassword(t *testing.T) { } for _, tt := range tests { m := &MetaClient{ - URL: tt.fields.URL, client: tt.fields.client, } if err := m.ChangePassword(tt.args.ctx, tt.args.name, tt.args.passwd); (err != nil) != tt.wantErr { @@ -510,9 +452,8 @@ func TestMetaClient_ChangePassword(t *testing.T) { func TestMetaClient_DeleteUser(t *testing.T) { type fields struct { - URL *url.URL client interface { - Do(URL *url.URL, path, method string, authorizer influx.Authorizer, params map[string]string, body io.Reader) (*http.Response, error) + Do(path, method string, authorizer influx.Authorizer, params map[string]string, body io.Reader) (*http.Response, error) } } type args struct { @@ -529,10 +470,6 @@ func TestMetaClient_DeleteUser(t *testing.T) { { name: "Successful delete User", fields: fields{ - URL: &url.URL{ - Host: "twinpinesmall.net:8091", - Scheme: "https", - }, client: NewMockClient( http.StatusOK, nil, @@ -549,7 +486,6 @@ func TestMetaClient_DeleteUser(t *testing.T) { } for _, tt := range tests { m := &MetaClient{ - URL: tt.fields.URL, client: tt.fields.client, } if err := m.DeleteUser(tt.args.ctx, tt.args.name); (err != nil) != tt.wantErr { @@ -579,9 +515,8 @@ func TestMetaClient_DeleteUser(t *testing.T) { func TestMetaClient_SetUserPerms(t *testing.T) { type fields struct { - URL *url.URL client interface { - Do(URL *url.URL, path, method string, authorizer influx.Authorizer, params map[string]string, body io.Reader) (*http.Response, error) + Do(path, method string, authorizer influx.Authorizer, params map[string]string, body io.Reader) (*http.Response, error) } } type args struct { @@ -600,10 +535,6 @@ func TestMetaClient_SetUserPerms(t *testing.T) { { name: "Remove all permissions for a user", fields: fields{ - URL: &url.URL{ - Host: "twinpinesmall.net:8091", - Scheme: "https", - }, client: NewMockClient( http.StatusOK, []byte(`{"users":[{"name":"admin","hash":"1234","permissions":{"":["ViewAdmin","ViewChronograf"]}}]}`), @@ -620,10 +551,6 @@ func TestMetaClient_SetUserPerms(t *testing.T) { { name: "Remove some permissions and add others", fields: fields{ - URL: &url.URL{ - Host: "twinpinesmall.net:8091", - Scheme: "https", - }, client: NewMockClient( http.StatusOK, []byte(`{"users":[{"name":"admin","hash":"1234","permissions":{"":["ViewAdmin","ViewChronograf"]}}]}`), @@ -646,7 +573,6 @@ func TestMetaClient_SetUserPerms(t *testing.T) { } for _, tt := range tests { m := &MetaClient{ - URL: tt.fields.URL, client: tt.fields.client, } if err := m.SetUserPerms(tt.args.ctx, tt.args.name, tt.args.perms); (err != nil) != tt.wantErr { @@ -700,9 +626,8 @@ func TestMetaClient_SetUserPerms(t *testing.T) { func TestMetaClient_Roles(t *testing.T) { type fields struct { - URL *url.URL client interface { - Do(URL *url.URL, path, method string, authorizer influx.Authorizer, params map[string]string, body io.Reader) (*http.Response, error) + Do(path, method string, authorizer influx.Authorizer, params map[string]string, body io.Reader) (*http.Response, error) } } type args struct { @@ -719,10 +644,6 @@ func TestMetaClient_Roles(t *testing.T) { { name: "Successful Show role", fields: fields{ - URL: &url.URL{ - Host: "twinpinesmall.net:8091", - Scheme: "https", - }, client: NewMockClient( http.StatusOK, []byte(`{"roles":[{"name":"admin","users":["marty"],"permissions":{"":["ViewAdmin","ViewChronograf"]}}]}`), @@ -751,10 +672,6 @@ func TestMetaClient_Roles(t *testing.T) { { name: "Successful Show role single role", fields: fields{ - URL: &url.URL{ - Host: "twinpinesmall.net:8091", - Scheme: "https", - }, client: NewMockClient( http.StatusOK, []byte(`{"roles":[{"name":"admin","users":["marty"],"permissions":{"":["ViewAdmin","ViewChronograf"]}}]}`), @@ -783,7 +700,6 @@ func TestMetaClient_Roles(t *testing.T) { } for _, tt := range tests { m := &MetaClient{ - URL: tt.fields.URL, client: tt.fields.client, } got, err := m.Roles(tt.args.ctx, tt.args.name) @@ -799,9 +715,8 @@ func TestMetaClient_Roles(t *testing.T) { func TestMetaClient_Role(t *testing.T) { type fields struct { - URL *url.URL client interface { - Do(URL *url.URL, path, method string, authorizer influx.Authorizer, params map[string]string, body io.Reader) (*http.Response, error) + Do(path, method string, authorizer influx.Authorizer, params map[string]string, body io.Reader) (*http.Response, error) } } type args struct { @@ -818,10 +733,6 @@ func TestMetaClient_Role(t *testing.T) { { name: "Successful Show role", fields: fields{ - URL: &url.URL{ - Host: "twinpinesmall.net:8091", - Scheme: "https", - }, client: NewMockClient( http.StatusOK, []byte(`{"roles":[{"name":"admin","users":["marty"],"permissions":{"":["ViewAdmin","ViewChronograf"]}}]}`), @@ -846,10 +757,6 @@ func TestMetaClient_Role(t *testing.T) { { name: "No such role", fields: fields{ - URL: &url.URL{ - Host: "twinpinesmall.net:8091", - Scheme: "https", - }, client: NewMockClient( http.StatusNotFound, []byte(`{"error":"user not found"}`), @@ -866,7 +773,6 @@ func TestMetaClient_Role(t *testing.T) { } for _, tt := range tests { m := &MetaClient{ - URL: tt.fields.URL, client: tt.fields.client, } got, err := m.Role(tt.args.ctx, tt.args.name) @@ -882,9 +788,8 @@ func TestMetaClient_Role(t *testing.T) { func TestMetaClient_UserRoles(t *testing.T) { type fields struct { - URL *url.URL client interface { - Do(URL *url.URL, path, method string, authorizer influx.Authorizer, params map[string]string, body io.Reader) (*http.Response, error) + Do(path, method string, authorizer influx.Authorizer, params map[string]string, body io.Reader) (*http.Response, error) } } type args struct { @@ -901,10 +806,6 @@ func TestMetaClient_UserRoles(t *testing.T) { { name: "Successful Show all roles", fields: fields{ - URL: &url.URL{ - Host: "twinpinesmall.net:8091", - Scheme: "https", - }, client: NewMockClient( http.StatusOK, []byte(`{"roles":[{"name":"timetravelers","users":["marty","docbrown"],"permissions":{"":["ViewAdmin","ViewChronograf"]}},{"name":"mcfly","users":["marty","george"],"permissions":{"":["ViewAdmin","ViewChronograf"]}}]}`), @@ -970,7 +871,6 @@ func TestMetaClient_UserRoles(t *testing.T) { } for _, tt := range tests { m := &MetaClient{ - URL: tt.fields.URL, client: tt.fields.client, } got, err := m.UserRoles(tt.args.ctx) @@ -986,9 +886,8 @@ func TestMetaClient_UserRoles(t *testing.T) { func TestMetaClient_CreateRole(t *testing.T) { type fields struct { - URL *url.URL client interface { - Do(URL *url.URL, path, method string, authorizer influx.Authorizer, params map[string]string, body io.Reader) (*http.Response, error) + Do(path, method string, authorizer influx.Authorizer, params map[string]string, body io.Reader) (*http.Response, error) } } type args struct { @@ -1005,10 +904,6 @@ func TestMetaClient_CreateRole(t *testing.T) { { name: "Successful Create Role", fields: fields{ - URL: &url.URL{ - Host: "twinpinesmall.net:8091", - Scheme: "https", - }, client: NewMockClient( http.StatusOK, nil, @@ -1025,7 +920,6 @@ func TestMetaClient_CreateRole(t *testing.T) { } for _, tt := range tests { m := &MetaClient{ - URL: tt.fields.URL, client: tt.fields.client, } if err := m.CreateRole(tt.args.ctx, tt.args.name); (err != nil) != tt.wantErr { @@ -1052,9 +946,8 @@ func TestMetaClient_CreateRole(t *testing.T) { func TestMetaClient_DeleteRole(t *testing.T) { type fields struct { - URL *url.URL client interface { - Do(URL *url.URL, path, method string, authorizer influx.Authorizer, params map[string]string, body io.Reader) (*http.Response, error) + Do(path, method string, authorizer influx.Authorizer, params map[string]string, body io.Reader) (*http.Response, error) } } type args struct { @@ -1071,10 +964,6 @@ func TestMetaClient_DeleteRole(t *testing.T) { { name: "Successful delete role", fields: fields{ - URL: &url.URL{ - Host: "twinpinesmall.net:8091", - Scheme: "https", - }, client: NewMockClient( http.StatusOK, nil, @@ -1091,7 +980,6 @@ func TestMetaClient_DeleteRole(t *testing.T) { } for _, tt := range tests { m := &MetaClient{ - URL: tt.fields.URL, client: tt.fields.client, } if err := m.DeleteRole(tt.args.ctx, tt.args.name); (err != nil) != tt.wantErr { @@ -1121,9 +1009,8 @@ func TestMetaClient_DeleteRole(t *testing.T) { func TestMetaClient_SetRolePerms(t *testing.T) { type fields struct { - URL *url.URL client interface { - Do(URL *url.URL, path, method string, authorizer influx.Authorizer, params map[string]string, body io.Reader) (*http.Response, error) + Do(path, method string, authorizer influx.Authorizer, params map[string]string, body io.Reader) (*http.Response, error) } } type args struct { @@ -1142,10 +1029,6 @@ func TestMetaClient_SetRolePerms(t *testing.T) { { name: "Remove all roles from user", fields: fields{ - URL: &url.URL{ - Host: "twinpinesmall.net:8091", - Scheme: "https", - }, client: NewMockClient( http.StatusOK, []byte(`{"roles":[{"name":"admin","users":["marty"],"permissions":{"":["ViewAdmin","ViewChronograf"]}}]}`), @@ -1162,10 +1045,6 @@ func TestMetaClient_SetRolePerms(t *testing.T) { { name: "Remove some users and add permissions to other", fields: fields{ - URL: &url.URL{ - Host: "twinpinesmall.net:8091", - Scheme: "https", - }, client: NewMockClient( http.StatusOK, []byte(`{"roles":[{"name":"admin","users":["marty"],"permissions":{"":["ViewAdmin","ViewChronograf"]}}]}`), @@ -1188,7 +1067,6 @@ func TestMetaClient_SetRolePerms(t *testing.T) { } for _, tt := range tests { m := &MetaClient{ - URL: tt.fields.URL, client: tt.fields.client, } if err := m.SetRolePerms(tt.args.ctx, tt.args.name, tt.args.perms); (err != nil) != tt.wantErr { @@ -1242,9 +1120,8 @@ func TestMetaClient_SetRolePerms(t *testing.T) { func TestMetaClient_SetRoleUsers(t *testing.T) { type fields struct { - URL *url.URL client interface { - Do(URL *url.URL, path, method string, authorizer influx.Authorizer, params map[string]string, body io.Reader) (*http.Response, error) + Do(path, method string, authorizer influx.Authorizer, params map[string]string, body io.Reader) (*http.Response, error) } } type args struct { @@ -1262,10 +1139,6 @@ func TestMetaClient_SetRoleUsers(t *testing.T) { { name: "Successful set users role (remove user from role)", fields: fields{ - URL: &url.URL{ - Host: "twinpinesmall.net:8091", - Scheme: "https", - }, client: NewMockClient( http.StatusOK, []byte(`{"roles":[{"name":"admin","users":["marty"],"permissions":{"":["ViewAdmin","ViewChronograf"]}}]}`), @@ -1282,10 +1155,6 @@ func TestMetaClient_SetRoleUsers(t *testing.T) { { name: "Successful set single user role", fields: fields{ - URL: &url.URL{ - Host: "twinpinesmall.net:8091", - Scheme: "https", - }, client: NewMockClient( http.StatusOK, []byte(`{"roles":[{"name":"admin","users":[],"permissions":{"":["ViewAdmin","ViewChronograf"]}}]}`), @@ -1305,7 +1174,6 @@ func TestMetaClient_SetRoleUsers(t *testing.T) { } for _, tt := range tests { m := &MetaClient{ - URL: tt.fields.URL, client: tt.fields.client, } if err := m.SetRoleUsers(tt.args.ctx, tt.args.name, tt.args.users); (err != nil) != tt.wantErr { @@ -1346,6 +1214,7 @@ func TestMetaClient_SetRoleUsers(t *testing.T) { } type MockClient struct { + URL *url.URL Code int // HTTP Status code Body []byte HeaderMap http.Header @@ -1356,6 +1225,10 @@ type MockClient struct { func NewMockClient(code int, body []byte, headers http.Header, err error) *MockClient { return &MockClient{ + URL: &url.URL{ + Host: "twinpinesmall.net:8091", + Scheme: "https", + }, Code: code, Body: body, HeaderMap: headers, @@ -1364,13 +1237,10 @@ func NewMockClient(code int, body []byte, headers http.Header, err error) *MockC } } -func (c *MockClient) Do(URL *url.URL, path, method string, authorizer influx.Authorizer, params map[string]string, body io.Reader) (*http.Response, error) { +func (c *MockClient) Do(path, method string, authorizer influx.Authorizer, params map[string]string, body io.Reader) (*http.Response, error) { if c == nil { return nil, fmt.Errorf("NIL MockClient") } - if URL == nil { - return nil, fmt.Errorf("NIL url") - } if c.Err != nil { return nil, c.Err } @@ -1381,6 +1251,7 @@ func (c *MockClient) Do(URL *url.URL, path, method string, authorizer influx.Aut p.Add(k, v) } + URL := *c.URL URL.Path = path URL.RawQuery = p.Encode() @@ -1401,17 +1272,31 @@ func (c *MockClient) Do(URL *url.URL, path, method string, authorizer influx.Aut }, nil } +type mockAuthorizer struct { + set func(req *http.Request) error +} + +func (a *mockAuthorizer) Set(req *http.Request) error { + return a.set(req) +} + func Test_AuthedCheckRedirect_Do(t *testing.T) { - var ts2URL string + path := "/test" + var ts2URL *url.URL + ts1Called := 0 ts1 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ts1Called++ want := http.Header{ - "Referer": []string{ts2URL}, "Accept-Encoding": []string{"gzip"}, "Authorization": []string{"hunter2"}, } + if ts1Called == 1 { + // referer is filled when doing a first redirect + want.Add("Referer", ts2URL.String()+path) + } for k, v := range want { if !reflect.DeepEqual(r.Header[k], v) { - t.Errorf("Request.Header = %#v; want %#v", r.Header[k], v) + t.Errorf("Request.Header[%s] = %#v; want %#v", k, r.Header[k], v) } } if t.Failed() { @@ -1422,38 +1307,44 @@ func Test_AuthedCheckRedirect_Do(t *testing.T) { })) defer ts1.Close() + ts2Called := 0 ts2 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ts2Called++ http.Redirect(w, r, ts1.URL, http.StatusFound) })) defer ts2.Close() - ts2URL = ts2.URL + ts2URL, _ = url.Parse(ts2.URL) - tr := &http.Transport{} - defer tr.CloseIdleConnections() - d := &defaultClient{} - c := &http.Client{ - Transport: tr, - CheckRedirect: d.AuthedCheckRedirect, + d := newDefaultClient(ts2URL, true) + authorizer := &mockAuthorizer{ + set: func(req *http.Request) error { + req.Header.Add("Cookie", "foo=bar") + req.Header.Add("Authorization", "hunter2") + req.Header.Add("Howdy", "doody") + req.Header.Set("User-Agent", "Darth Vader, an extraterrestrial from the Planet Vulcan") + return nil + }, } - - req, _ := http.NewRequest("GET", ts2.URL, nil) - req.Header.Add("Cookie", "foo=bar") - req.Header.Add("Authorization", "hunter2") - req.Header.Add("Howdy", "doody") - req.Header.Set("User-Agent", "Darth Vader, an extraterrestrial from the Planet Vulcan") - - res, err := c.Do(req) - if err != nil { - t.Fatal(err) + repetitions := 3 + for i := 0; i < repetitions; i++ { + res, err := d.Do(path, "GET", authorizer, nil, nil) + if err != nil { + t.Fatal(err) + } + defer res.Body.Close() + if res.StatusCode != 200 { + t.Fatal(res.Status) + } + if got := res.Header.Get("Result"); got != "ok" { + t.Errorf("result = %q; want ok", got) + } } - defer res.Body.Close() - if res.StatusCode != 200 { - t.Fatal(res.Status) + if ts1Called != repetitions { + t.Errorf("Master server called %v; expected %v", ts1Called, repetitions) } - - if got := res.Header.Get("Result"); got != "ok" { - t.Errorf("result = %q; want ok", got) + if ts2Called != 1 { + t.Errorf("Follower server called %v; expected 1", ts2Called) } } @@ -1514,9 +1405,9 @@ func Test_defaultClient_Do(t *testing.T) { })) defer ts.Close() - d := &defaultClient{} u, _ := url.Parse(ts.URL) - _, err := d.Do(u, tt.args.path, tt.args.method, tt.args.authorizer, tt.args.params, tt.args.body) + d := newDefaultClient(u, true) + _, err := d.Do(tt.args.path, tt.args.method, tt.args.authorizer, tt.args.params, tt.args.body) if (err != nil) != tt.wantErr { t.Errorf("defaultClient.Do() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/enterprise/users.go b/enterprise/users.go index d16f160841..45a5b51268 100644 --- a/enterprise/users.go +++ b/enterprise/users.go @@ -3,6 +3,7 @@ package enterprise import ( "context" "fmt" + "time" "github.com/influxdata/chronograf" ) @@ -18,11 +19,35 @@ func (c *UserStore) Add(ctx context.Context, u *chronograf.User) (*chronograf.Us if err := c.Ctrl.CreateUser(ctx, u.Name, u.Passwd); err != nil { return nil, err } - perms := ToEnterprise(u.Permissions) + // fix #5840: eventual consistency can cause delays in user creation, + // wait for the user to become available + _, err := c.Ctrl.User(ctx, u.Name) + timer := time.NewTimer(2_000_000_000) // retry at most 2 seconds + defer timer.Stop() + for err != nil { + if err.Error() != "user not found" { + return nil, err + } + // wait before the next attempt + select { + case <-ctx.Done(): + return nil, err + case <-timer.C: + return nil, err + case <-time.After(50_000_000): + break + } + _, err = c.Ctrl.User(ctx, u.Name) + } - if err := c.Ctrl.SetUserPerms(ctx, u.Name, perms); err != nil { - return nil, err + // add permissions + perms := ToEnterprise(u.Permissions) + if len(perms) > 0 { + if err := c.Ctrl.SetUserPerms(ctx, u.Name, perms); err != nil { + return nil, err + } } + // add roles for _, role := range u.Roles { if err := c.Ctrl.AddRoleUsers(ctx, role.Name, []string{u.Name}); err != nil { return nil, err diff --git a/enterprise/users_test.go b/enterprise/users_test.go index c715287ef0..076bd6fd3b 100644 --- a/enterprise/users_test.go +++ b/enterprise/users_test.go @@ -2,6 +2,7 @@ package enterprise_test import ( "context" + "errors" "fmt" "reflect" "testing" @@ -175,6 +176,48 @@ func TestClient_Add(t *testing.T) { } } +// TestClient_Add_UserNotFound verifies fix of defect #5840, the API has to wait +// for the creation of the user OOTB +func TestClient_Add_UserNotFound(t *testing.T) { + notFoundAttempts := 1 + c := &enterprise.UserStore{ + Ctrl: &mockCtrl{ + createUser: func(ctx context.Context, name, passwd string) error { + return nil + }, + user: func(ctx context.Context, name string) (*enterprise.User, error) { + if notFoundAttempts > 0 { + notFoundAttempts-- + return nil, errors.New("user not found") + } + return &enterprise.User{ + Name: "pavel", + Permissions: map[string][]string{}, + }, nil + }, + userRoles: func(ctx context.Context) (map[string]enterprise.Roles, error) { + return map[string]enterprise.Roles{}, nil + }, + }, + } + got, err := c.Add(context.Background(), &chronograf.User{ + Name: "pavel", + Passwd: "levap", + }) + if err != nil { + t.Errorf("Client.Add() error = %v", err) + return + } + want := &chronograf.User{ + Name: "pavel", + Permissions: chronograf.Permissions{}, + Roles: []chronograf.Role{}, + } + if !reflect.DeepEqual(got, want) { + t.Errorf("Client.Add() = \n%#v\n, want \n%#v\n", got, want) + } +} + func TestClient_Delete(t *testing.T) { type fields struct { Ctrl *mockCtrl diff --git a/server/sources.go b/server/sources.go index ed3422c51e..6908e37d28 100644 --- a/server/sources.go +++ b/server/sources.go @@ -544,11 +544,6 @@ func (s *Service) NewSourceUser(w http.ResponseWriter, r *http.Request) { return } - if err != nil { - Error(w, http.StatusBadRequest, err.Error(), s.Logger) - return - } - su := newSourceUserResponse(srcID, res.Name).WithPermissions(res.Permissions) if _, hasRoles := s.hasRoles(ctx, ts); hasRoles { su.WithRoles(srcID, res.Roles)