Merge pull request #2304 from influxdata/multitenancy_expose_source_for_superadmin

Fix ValidSourceRequest to modify pointer when needed
pull/10616/head
Michael Desa 2017-11-10 12:28:07 -05:00 committed by GitHub
commit 7ef6078c98
3 changed files with 514 additions and 5 deletions

View File

@ -76,7 +76,7 @@ func (s *Service) NewSource(w http.ResponseWriter, r *http.Request) {
return
}
if err := ValidSourceRequest(src, fmt.Sprintf("%d", defaultOrg.ID)); err != nil {
if err := ValidSourceRequest(&src, fmt.Sprintf("%d", defaultOrg.ID)); err != nil {
invalidData(w, err, s.Logger)
return
}
@ -270,7 +270,7 @@ func (s *Service) UpdateSource(w http.ResponseWriter, r *http.Request) {
return
}
if err := ValidSourceRequest(src, fmt.Sprintf("%d", defaultOrg.ID)); err != nil {
if err := ValidSourceRequest(&src, fmt.Sprintf("%d", defaultOrg.ID)); err != nil {
invalidData(w, err, s.Logger)
return
}
@ -290,8 +290,11 @@ func (s *Service) UpdateSource(w http.ResponseWriter, r *http.Request) {
encodeJSON(w, http.StatusOK, newSourceResponse(src), s.Logger)
}
// ValidSourceRequest checks if name, url and type are valid
func ValidSourceRequest(s chronograf.Source, defaultOrgID string) error {
// ValidSourceRequest checks if name, url, type, and role are valid
func ValidSourceRequest(s *chronograf.Source, defaultOrgID string) error {
if s == nil {
return fmt.Errorf("source must be non-nil")
}
// Name and URL areq required
if s.URL == "" {
return fmt.Errorf("url required")
@ -318,6 +321,14 @@ func ValidSourceRequest(s chronograf.Source, defaultOrgID string) error {
if s.Role == "" {
s.Role = roles.ViewerRoleName
}
switch s.Role {
case roles.ViewerRoleName, roles.EditorRoleName, roles.AdminRoleName:
return nil
default:
return fmt.Errorf("Unknown role %s. Valid roles are 'viewer', 'editor', and 'admin'", s.Role)
}
return nil
}
@ -345,7 +356,7 @@ func (s *Service) HandleNewSources(ctx context.Context, input string) error {
}
for _, sk := range srcsKaps {
if err := ValidSourceRequest(sk.Source, fmt.Sprintf("%d", defaultOrg.ID)); err != nil {
if err := ValidSourceRequest(&sk.Source, fmt.Sprintf("%d", defaultOrg.ID)); err != nil {
return err
}
// Add any new sources and kapacitors as specified via server flag

View File

@ -11,11 +11,216 @@ import (
"testing"
"github.com/bouk/httprouter"
"github.com/google/go-cmp/cmp"
"github.com/influxdata/chronograf"
"github.com/influxdata/chronograf/log"
"github.com/influxdata/chronograf/mocks"
"github.com/influxdata/chronograf/roles"
)
func Test_ValidSourceRequest(t *testing.T) {
type args struct {
source *chronograf.Source
defaultOrgID string
}
type wants struct {
err error
source *chronograf.Source
}
tests := []struct {
name string
args args
wants wants
}{
{
name: "nil source",
args: args{},
wants: wants{
err: fmt.Errorf("source must be non-nil"),
},
},
{
name: "missing url",
args: args{
source: &chronograf.Source{
ID: 1,
Name: "I'm a really great source",
Type: chronograf.InfluxDB,
Username: "fancy",
Password: "i'm so",
SharedSecret: "supersecret",
MetaURL: "http://www.so.meta.com",
InsecureSkipVerify: true,
Default: true,
Telegraf: "telegraf",
Organization: "0",
Role: roles.ViewerRoleName,
},
},
wants: wants{
err: fmt.Errorf("url required"),
},
},
{
name: "invalid source type",
args: args{
source: &chronograf.Source{
ID: 1,
Name: "I'm a really great source",
Type: "non-existent-type",
Username: "fancy",
Password: "i'm so",
SharedSecret: "supersecret",
URL: "http://www.any.url.com",
MetaURL: "http://www.so.meta.com",
InsecureSkipVerify: true,
Default: true,
Telegraf: "telegraf",
Organization: "0",
Role: roles.ViewerRoleName,
},
},
wants: wants{
err: fmt.Errorf("invalid source type non-existent-type"),
},
},
{
name: "set organization to be default org if not specified",
args: args{
defaultOrgID: "2",
source: &chronograf.Source{
ID: 1,
Name: "I'm a really great source",
Type: chronograf.InfluxDB,
Username: "fancy",
Password: "i'm so",
SharedSecret: "supersecret",
URL: "http://www.any.url.com",
MetaURL: "http://www.so.meta.com",
InsecureSkipVerify: true,
Default: true,
Telegraf: "telegraf",
Role: roles.ViewerRoleName,
},
},
wants: wants{
source: &chronograf.Source{
ID: 1,
Name: "I'm a really great source",
Type: chronograf.InfluxDB,
Username: "fancy",
Password: "i'm so",
SharedSecret: "supersecret",
URL: "http://www.any.url.com",
MetaURL: "http://www.so.meta.com",
InsecureSkipVerify: true,
Default: true,
Organization: "2",
Telegraf: "telegraf",
Role: roles.ViewerRoleName,
},
},
},
{
name: "bad url",
args: args{
source: &chronograf.Source{
ID: 1,
Name: "I'm a really great source",
Type: chronograf.InfluxDB,
Username: "fancy",
Password: "i'm so",
SharedSecret: "supersecret",
URL: "im a bad url",
MetaURL: "http://www.so.meta.com",
InsecureSkipVerify: true,
Organization: "0",
Default: true,
Telegraf: "telegraf",
Role: roles.ViewerRoleName,
},
},
wants: wants{
err: fmt.Errorf("invalid source URI: parse im a bad url: invalid URI for request"),
},
},
{
name: "set Role to be viewer if not specified",
args: args{
source: &chronograf.Source{
ID: 1,
Name: "I'm a really great source",
Type: chronograf.InfluxDB,
Username: "fancy",
Password: "i'm so",
SharedSecret: "supersecret",
URL: "http://www.any.url.com",
MetaURL: "http://www.so.meta.com",
InsecureSkipVerify: true,
Default: true,
Telegraf: "telegraf",
Organization: "0",
},
},
wants: wants{
source: &chronograf.Source{
ID: 1,
Name: "I'm a really great source",
Type: chronograf.InfluxDB,
Username: "fancy",
Password: "i'm so",
SharedSecret: "supersecret",
URL: "http://www.any.url.com",
MetaURL: "http://www.so.meta.com",
InsecureSkipVerify: true,
Default: true,
Organization: "0",
Telegraf: "telegraf",
Role: roles.ViewerRoleName,
},
},
},
{
name: "bad role type",
args: args{
source: &chronograf.Source{
ID: 1,
Name: "I'm a really great source",
Type: chronograf.InfluxDB,
Username: "fancy",
Password: "i'm so",
SharedSecret: "supersecret",
URL: "http://www.any.url.com",
MetaURL: "http://www.so.meta.com",
InsecureSkipVerify: true,
Default: true,
Telegraf: "telegraf",
Organization: "0",
Role: "superperson",
},
},
wants: wants{
err: fmt.Errorf("Unknown role superperson. Valid roles are 'viewer', 'editor', and 'admin'"),
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := ValidSourceRequest(tt.args.source, tt.args.defaultOrgID)
if err == nil && tt.wants.err == nil {
if diff := cmp.Diff(tt.args.source, tt.wants.source); diff != "" {
t.Errorf("%q. ValidSourceRequest():\n-got/+want\ndiff %s", tt.name, diff)
}
return
}
if err.Error() != tt.wants.err.Error() {
t.Errorf("%q. ValidSourceRequest() = %q, want %q", tt.name, err, tt.wants.err)
}
})
}
}
func Test_newSourceResponse(t *testing.T) {
tests := []struct {
name string

View File

@ -370,3 +370,296 @@ func TestStore_SourcesGet(t *testing.T) {
})
}
}
func TestStore_SourcesAll(t *testing.T) {
type fields struct {
SourcesStore chronograf.SourcesStore
}
type args struct {
organization string
role string
}
type wants struct {
sources []chronograf.Source
err bool
}
tests := []struct {
name string
fields fields
args args
wants wants
}{
{
name: "Get viewer sources as viewer",
fields: fields{
SourcesStore: &mocks.SourcesStore{
AllF: func(ctx context.Context) ([]chronograf.Source, error) {
return []chronograf.Source{
{
ID: 1,
Name: "my sweet name",
Organization: "0",
Role: "viewer",
},
}, nil
},
},
},
args: args{
organization: "0",
role: "viewer",
},
wants: wants{
sources: []chronograf.Source{
{
ID: 1,
Name: "my sweet name",
Organization: "0",
Role: "viewer",
},
},
},
},
{
name: "Get viewer sources as viewer - multiple orgs and multiple roles",
fields: fields{
SourcesStore: &mocks.SourcesStore{
AllF: func(ctx context.Context) ([]chronograf.Source, error) {
return []chronograf.Source{
{
ID: 1,
Name: "my sweet name",
Organization: "0",
Role: "viewer",
},
{
ID: 2,
Name: "A bad source",
Organization: "0",
Role: "editor",
},
{
ID: 3,
Name: "A good source",
Organization: "0",
Role: "admin",
},
{
ID: 4,
Name: "a source I can has",
Organization: "0",
Role: "viewer",
},
{
ID: 5,
Name: "i'm in the wrong org",
Organization: "1",
Role: "viewer",
},
}, nil
},
},
},
args: args{
organization: "0",
role: "viewer",
},
wants: wants{
sources: []chronograf.Source{
{
ID: 1,
Name: "my sweet name",
Organization: "0",
Role: "viewer",
},
{
ID: 4,
Name: "a source I can has",
Organization: "0",
Role: "viewer",
},
},
},
},
{
name: "Get editor sources as editor - multiple orgs and multiple roles",
fields: fields{
SourcesStore: &mocks.SourcesStore{
AllF: func(ctx context.Context) ([]chronograf.Source, error) {
return []chronograf.Source{
{
ID: 1,
Name: "my sweet name",
Organization: "0",
Role: "viewer",
},
{
ID: 2,
Name: "A bad source",
Organization: "0",
Role: "editor",
},
{
ID: 3,
Name: "A good source",
Organization: "0",
Role: "admin",
},
{
ID: 4,
Name: "a source I can has",
Organization: "0",
Role: "viewer",
},
{
ID: 5,
Name: "i'm in the wrong org",
Organization: "1",
Role: "viewer",
},
{
ID: 2,
Name: "i'm an editor, but wrong org",
Organization: "3",
Role: "editor",
},
}, nil
},
},
},
args: args{
organization: "0",
role: "editor",
},
wants: wants{
sources: []chronograf.Source{
{
ID: 1,
Name: "my sweet name",
Organization: "0",
Role: "viewer",
},
{
ID: 2,
Name: "A bad source",
Organization: "0",
Role: "editor",
},
{
ID: 4,
Name: "a source I can has",
Organization: "0",
Role: "viewer",
},
},
},
},
{
name: "Get admin sources as admin - multiple orgs and multiple roles",
fields: fields{
SourcesStore: &mocks.SourcesStore{
AllF: func(ctx context.Context) ([]chronograf.Source, error) {
return []chronograf.Source{
{
ID: 1,
Name: "my sweet name",
Organization: "0",
Role: "viewer",
},
{
ID: 2,
Name: "A bad source",
Organization: "0",
Role: "editor",
},
{
ID: 3,
Name: "A good source",
Organization: "0",
Role: "admin",
},
{
ID: 4,
Name: "a source I can has",
Organization: "0",
Role: "viewer",
},
{
ID: 5,
Name: "i'm in the wrong org",
Organization: "1",
Role: "viewer",
},
{
ID: 2,
Name: "i'm an editor, but wrong org",
Organization: "3",
Role: "editor",
},
}, nil
},
},
},
args: args{
organization: "0",
role: "admin",
},
wants: wants{
sources: []chronograf.Source{
{
ID: 1,
Name: "my sweet name",
Organization: "0",
Role: "viewer",
},
{
ID: 2,
Name: "A bad source",
Organization: "0",
Role: "editor",
},
{
ID: 3,
Name: "A good source",
Organization: "0",
Role: "admin",
},
{
ID: 4,
Name: "a source I can has",
Organization: "0",
Role: "viewer",
},
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
store := &Store{
SourcesStore: tt.fields.SourcesStore,
}
ctx := context.Background()
if tt.args.organization != "" {
ctx = context.WithValue(ctx, organizations.ContextKey, tt.args.organization)
}
if tt.args.role != "" {
ctx = context.WithValue(ctx, roles.ContextKey, tt.args.role)
}
sources, err := store.Sources(ctx).All(ctx)
if (err != nil) != tt.wants.err {
t.Errorf("%q. Store.Sources().Get() error = %v, wantErr %v", tt.name, err, tt.wants.err)
return
}
if diff := cmp.Diff(sources, tt.wants.sources); diff != "" {
t.Errorf("%q. Store.Sources().Get():\n-got/+want\ndiff %s", tt.name, diff)
}
})
}
}