Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introducing Role Based Access Control (RBAC) - build RBAC cache #1263

Merged
merged 1 commit into from
Feb 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 19 additions & 9 deletions cmd/onos-config/onos-config.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import (
"github.com/onosproject/onos-config/pkg/northbound/diags"
"github.com/onosproject/onos-config/pkg/northbound/gnmi"
"github.com/onosproject/onos-config/pkg/store/change/device"
"github.com/onosproject/onos-config/pkg/store/change/device/rbac"
"github.com/onosproject/onos-config/pkg/store/change/device/state"
"github.com/onosproject/onos-config/pkg/store/change/network"
devicestore "github.com/onosproject/onos-config/pkg/store/device"
Expand All @@ -66,6 +67,9 @@ import (
// OIDCServerURL - address of an OpenID Connect server
const OIDCServerURL = "OIDC_SERVER_URL"

// RbacVersionedID - the internal device where RBAC rules are configured
const RbacVersionedID = "rbac:1.0.0"

type arrayFlags []string

func (i *arrayFlags) String() string {
Expand Down Expand Up @@ -166,9 +170,23 @@ func main() {
}
log.Infof("Topology service connected with endpoint %s", *topoEndpoint)

authorization := false
var rbacCache rbac.Cache
if oidcURL := os.Getenv(OIDCServerURL); oidcURL != "" {
authorization = true
rbacCache, err = rbac.NewRbacCache(deviceChangesStore, deviceSnapshotStore, RbacVersionedID)
if err != nil {
log.Fatal("Cannot create RBAC cache %v", err)
}
log.Infof("Authorization enabled. %s=%s", OIDCServerURL, oidcURL)
// OIDCServerURL is also referenced in jwt.go (from onos-lib-go)
} else {
log.Infof("Authorization not enabled %s", os.Getenv(OIDCServerURL))
}

mgr := manager.NewManager(leadershipStore, mastershipStore, deviceChangesStore,
deviceStateStore, deviceStore, deviceCache, networkChangesStore, networkSnapshotStore,
deviceSnapshotStore, *allowUnvalidatedConfig)
deviceSnapshotStore, *allowUnvalidatedConfig, rbacCache)
log.Info("Manager created")

defer func() {
Expand All @@ -188,14 +206,6 @@ func main() {
}

mgr.Run()
authorization := false
if oidcURL := os.Getenv(OIDCServerURL); oidcURL != "" {
authorization = true
log.Infof("Authorization enabled. %s=%s", OIDCServerURL, oidcURL)
// OIDCServerURL is also referenced in jwt.go (from onos-lib-go)
} else {
log.Infof("Authorization not enabled %s", os.Getenv(OIDCServerURL))
}

err = startServer(*caPath, *keyPath, *certPath, authorization)
if err != nil {
Expand Down
4 changes: 0 additions & 4 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,17 @@ module github.com/onosproject/onos-config
go 1.14

require (
cloud.google.com/go v0.43.0 // indirect
github.com/Pallinder/go-randomdata v1.2.0
github.com/atomix/go-client v0.4.1
github.com/cenkalti/backoff v2.2.1+incompatible
github.com/docker/docker v1.13.1 // indirect
github.com/gogo/protobuf v1.3.1
github.com/golang/mock v1.4.4
github.com/golang/protobuf v1.4.3
github.com/google/pprof v0.0.0-20190723021845-34ac40c74b70 // indirect
github.com/google/uuid v1.1.2
github.com/googleapis/gnostic v0.3.0 // indirect
github.com/gregjones/httpcache v0.0.0-20190611155906-901d90724c79 // indirect
github.com/grpc-ecosystem/go-grpc-middleware v1.2.0
github.com/kr/pty v1.1.8 // indirect
github.com/mattn/go-colorable v0.1.4 // indirect
github.com/mattn/go-isatty v0.0.12 // indirect
github.com/onosproject/config-models/modelplugin/devicesim-1.0.0 v0.0.0-20201130213019-492043aed0df
Expand All @@ -36,7 +33,6 @@ require (
github.com/spf13/viper v1.6.2 // indirect
github.com/stretchr/testify v1.5.1
go.uber.org/multierr v1.4.0 // indirect
golang.org/x/mobile v0.0.0-20190806162312-597adff16ade // indirect
golang.org/x/tools v0.0.0-20200522201501-cb1345f3a375 // indirect
google.golang.org/grpc v1.33.2
gopkg.in/yaml.v2 v2.2.8
Expand Down
2 changes: 1 addition & 1 deletion pkg/manager/getconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func (m *Manager) GetTargetConfig(deviceID devicetype.ID, version devicetype.Ver
return configValues, nil
}
filteredValues := make([]*devicechange.PathValue, 0)
pathRegexp := utils.MatchWildcardRegexp(path)
pathRegexp := utils.MatchWildcardRegexp(path, false)
for _, cv := range configValues {
if pathRegexp.MatchString(cv.Path) {
filteredValues = append(filteredValues, cv)
Expand Down
2 changes: 1 addition & 1 deletion pkg/manager/getstate.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func (m *Manager) GetTargetState(target string, path string) []*devicechange.Pat
log.Info("Getting State for ", target, path)
configValues := make([]*devicechange.PathValue, 0)
//First check the cache, if it's not empty for this path we read that and return,
pathRegexp := utils.MatchWildcardRegexp(path)
pathRegexp := utils.MatchWildcardRegexp(path, false)
m.OperationalStateCacheLock.RLock()
for pathCache, value := range m.OperationalStateCache[topodevice.ID(target)] {
if pathRegexp.MatchString(pathCache) {
Expand Down
42 changes: 5 additions & 37 deletions pkg/manager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package manager

import (
"fmt"
"github.com/onosproject/onos-config/pkg/store/change/device/rbac"
"sync"

devicechange "github.com/onosproject/onos-api/go/onos/config/change/device"
Expand Down Expand Up @@ -72,14 +73,15 @@ type Manager struct {
Dispatcher *dispatcher.Dispatcher
OperationalStateCache map[topodevice.ID]devicechange.TypedValueMap
OperationalStateCacheLock *sync.RWMutex
RbacCache rbac.Cache
allowUnvalidatedConfig bool
}

// NewManager initializes the network config manager subsystem.
func NewManager(leadershipStore leadership.Store, mastershipStore mastership.Store, deviceChangesStore device.Store,
deviceStateStore state.Store, deviceStore devicestore.Store, deviceCache cache.Cache,
networkChangesStore network.Store, networkSnapshotStore networksnap.Store,
deviceSnapshotStore devicesnap.Store, allowUnvalidatedConfig bool) *Manager {
deviceSnapshotStore devicesnap.Store, allowUnvalidatedConfig bool, rbacCache rbac.Cache) *Manager {
log.Info("Creating Manager")

modelReg := &modelregistry.ModelRegistry{
Expand Down Expand Up @@ -110,7 +112,9 @@ func NewManager(leadershipStore leadership.Store, mastershipStore mastership.Sto
OperationalStateCache: make(map[topodevice.ID]devicechange.TypedValueMap),
OperationalStateCacheLock: &sync.RWMutex{},
allowUnvalidatedConfig: allowUnvalidatedConfig,
RbacCache: rbacCache,
}

return &mgr
}

Expand Down Expand Up @@ -186,42 +190,6 @@ func GetManager() *Manager {
return &mgr
}

// ComputeDeviceChange computes a given device change the given updates and deletes, according to the path
// on the configuration for the specified target
func (m *Manager) ComputeDeviceChange(deviceName devicetype.ID, version devicetype.Version,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just moved this to setconfig.go in the same package - it's only ever used from there so did not need to be in this file

deviceType devicetype.Type, updates devicechange.TypedValueMap,
deletes []string, description string) (*devicechange.Change, error) {

var newChanges = make([]*devicechange.ChangeValue, 0)
//updates
for path, value := range updates {
updateValue, err := devicechange.NewChangeValue(path, value, false)
if err != nil {
log.Warnf("Error creating value for %s %v", path, err)
continue
}
newChanges = append(newChanges, updateValue)
}
//deletes
for _, path := range deletes {
deleteValue, _ := devicechange.NewChangeValue(path, devicechange.NewTypedValueEmpty(), true)
newChanges = append(newChanges, deleteValue)
}
//description := fmt.Sprintf("Originally created as part of %s", description)
//if description == "" {
// description = fmt.Sprintf("Created at %s", time.Now().Format(time.RFC3339))
//}
//TODO lost description of Change
changeElement := &devicechange.Change{
DeviceID: deviceName,
DeviceVersion: version,
DeviceType: deviceType,
Values: newChanges,
}

return changeElement, nil
}

// CheckCacheForDevice checks against the device cache (of the device change store
// to see if a device of that name is already present)
func (m *Manager) CheckCacheForDevice(target devicetype.ID, deviceType devicetype.Type,
Expand Down
2 changes: 1 addition & 1 deletion pkg/manager/manager_deep_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ func setUpDeepTest(t *testing.T) (*Manager, *AllMocks) {
assert.NilError(t, err)

mgrTest = NewManager(leadershipStore, mastershipStore, deviceChangesStore, deviceStateStore,
mockDeviceStore, deviceCache, networkChangesStore, networkSnapshotStore, deviceSnapshotStore, true)
mockDeviceStore, deviceCache, networkChangesStore, networkSnapshotStore, deviceSnapshotStore, true, nil)

modelData1 := gnmi.ModelData{
Name: "test1",
Expand Down
3 changes: 2 additions & 1 deletion pkg/manager/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,8 @@ func setUp(t *testing.T) (*Manager, *AllMocks) {
mockNetworkChangesStore,
mockNetworkSnapshotStore,
mockDeviceSnapshotStore,
true)
true,
nil)

mgrTest.Run()

Expand Down
42 changes: 39 additions & 3 deletions pkg/manager/setconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const SetConfigAlreadyApplied = "Already applied:"
func (m *Manager) ValidateNetworkConfig(deviceName devicetype.ID, version devicetype.Version,
deviceType devicetype.Type, updates devicechange.TypedValueMap, deletes []string, lastWrite networkchange.Revision) error {

chg, err := m.ComputeDeviceChange(deviceName, version, deviceType, updates, deletes, "Generated for validation")
chg, err := computeDeviceChange(deviceName, version, deviceType, updates, deletes, "Generated for validation")
if err != nil {
return err
}
Expand Down Expand Up @@ -128,7 +128,7 @@ func (m *Manager) computeNetworkConfig(targetUpdates map[devicetype.ID]devicecha
//FIXME this is a sequential job, not parallelized
version := deviceInfo[target].Version
deviceType := deviceInfo[target].Type
newChange, err := m.ComputeDeviceChange(
newChange, err := computeDeviceChange(
target, version, deviceType, updates, targetRemoves[target], description)
if err != nil {
log.Error("Error in setting config: ", newChange, " for target ", err)
Expand All @@ -143,7 +143,7 @@ func (m *Manager) computeNetworkConfig(targetUpdates map[devicetype.ID]devicecha
for target, removes := range targetRemoves {
version := deviceInfo[target].Version
deviceType := deviceInfo[target].Type
newChange, err := m.ComputeDeviceChange(
newChange, err := computeDeviceChange(
target, version, deviceType, make(devicechange.TypedValueMap), removes, description)
if err != nil {
log.Error("Error in setting config: ", newChange, " for target ", err)
Expand All @@ -154,3 +154,39 @@ func (m *Manager) computeNetworkConfig(targetUpdates map[devicetype.ID]devicecha
}
return deviceChanges, nil
}

// computeDeviceChange computes a given device change the given updates and deletes, according to the path
// on the configuration for the specified target
func computeDeviceChange(deviceName devicetype.ID, version devicetype.Version,
deviceType devicetype.Type, updates devicechange.TypedValueMap,
deletes []string, description string) (*devicechange.Change, error) {

var newChanges = make([]*devicechange.ChangeValue, 0)
//updates
for path, value := range updates {
updateValue, err := devicechange.NewChangeValue(path, value, false)
if err != nil {
log.Warnf("Error creating value for %s %v", path, err)
continue
}
newChanges = append(newChanges, updateValue)
}
//deletes
for _, path := range deletes {
deleteValue, _ := devicechange.NewChangeValue(path, devicechange.NewTypedValueEmpty(), true)
newChanges = append(newChanges, deleteValue)
}
//description := fmt.Sprintf("Originally created as part of %s", description)
//if description == "" {
// description = fmt.Sprintf("Created at %s", time.Now().Format(time.RFC3339))
//}
//TODO lost description of Change
changeElement := &devicechange.Change{
DeviceID: deviceName,
DeviceVersion: version,
DeviceType: deviceType,
Values: newChanges,
}

return changeElement, nil
}
6 changes: 4 additions & 2 deletions pkg/modelregistry/jsonvalues/jsonToValues.go
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,8 @@ func indicesOfPath(modelROpaths modelregistry.ReadOnlyPathMap,
pathNoIndices := modelregistry.RemovePathIndices(path)
// Find a short path
if pathNoIndices[:strings.LastIndex(pathNoIndices, slash)] == searchpathNoIndices {
return modelregistry.ExtractIndexNames(path)
idxNames, _ := modelregistry.ExtractIndexNames(path)
return idxNames
}
}

Expand All @@ -500,7 +501,8 @@ func indicesOfPath(modelROpaths modelregistry.ReadOnlyPathMap,
pathNoIndices := modelregistry.RemovePathIndices(fullpath)
// Find a short path
if pathNoIndices[:strings.LastIndex(pathNoIndices, slash)] == searchpathNoIndices {
return modelregistry.ExtractIndexNames(fullpath)
idxNames, _ := modelregistry.ExtractIndexNames(fullpath)
return idxNames
}
}
}
Expand Down
9 changes: 6 additions & 3 deletions pkg/modelregistry/modelregistry.go
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ func ExtractPaths(deviceEntry *yang.Entry, parentState yang.TriState, parentPath
// Need to copy the index of the list across to the RO list too
roIdxName := k[:strings.LastIndex(k, "/")]
roIdxSubPath := k[strings.LastIndex(k, "/"):]
indices := ExtractIndexNames(itemPath[strings.LastIndex(itemPath, "/"):])
indices, _ := ExtractIndexNames(itemPath[strings.LastIndex(itemPath, "/"):])
kIsIdxAttr := false
for _, idx := range indices {
if roIdxSubPath == fmt.Sprintf("/%s", idx) {
Expand Down Expand Up @@ -403,14 +403,17 @@ func RemovePathIndices(path string) string {
}

// ExtractIndexNames - get an ordered array of index names
func ExtractIndexNames(path string) []string {
func ExtractIndexNames(path string) ([]string, []string) {
indexNames := make([]string, 0)
indexValues := make([]string, 0)
jsonMatches := rOnIndex.FindAllStringSubmatch(path, -1)
for _, m := range jsonMatches {
idxName := m[1][1:strings.LastIndex(m[1], "=")]
indexNames = append(indexNames, idxName)
idxValue := m[1][strings.LastIndex(m[1], "=")+1 : len(m[1])-1]
indexValues = append(indexValues, idxValue)
}
return indexNames
return indexNames, indexValues
}

func formatName(dirEntry *yang.Entry, isList bool, parentPath string, subpathPrefix string) string {
Expand Down
10 changes: 9 additions & 1 deletion pkg/modelregistry/modelregistry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -700,13 +700,21 @@ func Test_formatName2(t *testing.T) {
}

func Test_extractIndexNames(t *testing.T) {
// TODO: check this is how multiple indices are used in real life
const modelPath = "/p/q/r[a=*]/s/t[b=*][c=*]/u/v[d=*][e=*][f=*]/w"
indexNames := ExtractIndexNames(modelPath)
indexNames, indexValues := ExtractIndexNames(modelPath)
assert.Equal(t, 6, len(indexNames))
assert.Equal(t, 6, len(indexValues))
assert.Equal(t, "a", indexNames[0])
assert.Equal(t, "*", indexValues[0])
assert.Equal(t, "b", indexNames[1])
assert.Equal(t, "*", indexValues[1])
assert.Equal(t, "c", indexNames[2])
assert.Equal(t, "*", indexValues[2])
assert.Equal(t, "d", indexNames[3])
assert.Equal(t, "*", indexValues[3])
assert.Equal(t, "e", indexNames[4])
assert.Equal(t, "*", indexValues[4])
assert.Equal(t, "f", indexNames[5])
assert.Equal(t, "*", indexValues[5])
}
2 changes: 1 addition & 1 deletion pkg/northbound/admin/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ func (s Server) ListSnapshots(r *admin.ListSnapshotsRequest, stream admin.Config
log.Infof("ListSnapshots called with %s. Subscribe %v", r.ID, r.Subscribe)

// There may be a wildcard given - we only want to reply with changes that match
matcher := utils.MatchWildcardChNameRegexp(string(r.ID))
matcher := utils.MatchWildcardChNameRegexp(string(r.ID), false)

if r.Subscribe {
eventCh := make(chan streams.Event)
Expand Down
3 changes: 2 additions & 1 deletion pkg/northbound/admin/admin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ func setUpServer(t *testing.T) (*manager.Manager, *grpc.ClientConn, admin.Config
mockstore.NewMockNetworkChangesStore(ctrl),
mockstore.NewMockNetworkSnapshotStore(ctrl),
mockstore.NewMockDeviceSnapshotStore(ctrl),
true)
true,
nil)

return mgrTest, conn, client, s
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/northbound/diags/diags.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ func (s Server) ListNetworkChanges(r *diags.ListNetworkChangeRequest, stream dia
log.Infof("ListNetworkChanges called with %s. Subscribe %v", r.ChangeID, r.Subscribe)

// There may be a wildcard given - we only want to reply with changes that match
matcher := utils.MatchWildcardChNameRegexp(string(r.ChangeID))
matcher := utils.MatchWildcardChNameRegexp(string(r.ChangeID), false)
var watchOpts []network.WatchOption
if !r.WithoutReplay {
watchOpts = append(watchOpts, network.WithReplay())
Expand Down
3 changes: 2 additions & 1 deletion pkg/northbound/diags/diags_new_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ func setUpServer(t *testing.T) (*manager.Manager, *grpc.ClientConn, diags.Change
mockstore.NewMockNetworkChangesStore(ctrl),
mockstore.NewMockNetworkSnapshotStore(ctrl),
mockstore.NewMockDeviceSnapshotStore(ctrl),
true)
true,
nil)

mgrTest.DeviceStore = mockstore.NewMockDeviceStore(ctrl)

Expand Down
3 changes: 2 additions & 1 deletion pkg/northbound/gnmi/gnmi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,8 @@ func setUp(t *testing.T) (*Server, *manager.Manager, *AllMocks) {
mockStores.NetworkChangesStore,
mockStores.NetworkSnapshotStore,
mockStores.DeviceSnapshotStore,
true)
true,
nil)

mgr.DeviceStore = mockStores.DeviceStore
mgr.DeviceChangesStore = mockStores.DeviceChangesStore
Expand Down
2 changes: 1 addition & 1 deletion pkg/northbound/gnmi/set.go
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@ func extractModelForTarget(target devicetype.ID,
func findPathFromModel(path string, rwPaths modelregistry.ReadWritePathMap) (*modelregistry.ReadWritePathElem, error) {
searchpathNoIndices := modelregistry.RemovePathIndices(path)
if strings.HasSuffix(path, "]") { //Ends with index
indices := modelregistry.ExtractIndexNames(path)
indices, _ := modelregistry.ExtractIndexNames(path)
// Add on the last index
searchpathNoIndices = fmt.Sprintf("%s/%s", searchpathNoIndices, indices[len(indices)-1])
}
Expand Down