Skip to content

Commit

Permalink
Incrementally Build Config (#241) =
Browse files Browse the repository at this point in the history
* build configuration incrementally
* bug fixes and formatting
* clean up roles with no associations
* Improve error trace
* improve logging trace
* do not remove users and groups on config import
  • Loading branch information
BruceMacD authored Oct 20, 2021
1 parent 1ab50f8 commit 2f82548
Show file tree
Hide file tree
Showing 3 changed files with 155 additions and 100 deletions.
133 changes: 75 additions & 58 deletions internal/registry/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package registry

import (
"errors"
"fmt"
"regexp"
"strings"

Expand Down Expand Up @@ -81,7 +82,7 @@ func ImportSources(db *gorm.DB, sources []ConfigSource) error {

var source Source
if err := db.FirstOrCreate(&source, &Source{Kind: SourceKindOkta}).Error; err != nil {
return err
return fmt.Errorf("create config source: %w", err)
}

source.ClientId = s.ClientId
Expand All @@ -91,29 +92,27 @@ func ImportSources(db *gorm.DB, sources []ConfigSource) error {
source.ApiToken = s.ApiToken

if err := db.Save(&source).Error; err != nil {
return err
return fmt.Errorf("save source: %w", err)
}

idsToKeep = append(idsToKeep, source.Id)
case "":
logging.L.Sugar().Errorf("skipping a source with no kind set in configuration")
default:
logging.L.Sugar().Errorf("skipping invalid source kind in configuration: %s" + s.Kind)
logging.L.Sugar().Errorf("skipping invalid source kind in configuration: %s", s.Kind)
}
}

if len(idsToKeep) == 0 {
logging.L.Debug("no valid sources found in configuration, ensure the required fields are specified correctly")
// clear the sources
return db.Where("1 = 1").Delete(&Source{}).Error
}

if err := db.Where("1 = 1").Not(idsToKeep).Delete(&Source{}).Error; err != nil {
return err
}

return nil
return db.Not(idsToKeep).Delete(&Source{}).Error
}

func ImportGroupMapping(db *gorm.DB, groups []ConfigGroupMapping) error {
func ApplyGroupMappings(db *gorm.DB, groups []ConfigGroupMapping) (modifiedRoleIDs []string, err error) {
for _, g := range groups {
// get the source from the datastore that this group specifies
var source Source
Expand All @@ -126,44 +125,48 @@ func ImportGroupMapping(db *gorm.DB, groups []ConfigGroupMapping) error {
continue
}

return srcReadErr
return nil, fmt.Errorf("group read source: %w", srcReadErr)
}

var group Group

grpReadErr := db.Where(&Group{Name: g.Name, SourceId: source.Id}).First(&group).Error
grpReadErr := db.Preload("Users").Where(&Group{Name: g.Name, SourceId: source.Id}).First(&group).Error
if grpReadErr != nil {
if errors.Is(grpReadErr, gorm.ErrRecordNotFound) {
// skip this group, if they're created these roles will be added later
logging.L.Debug("skipping group in config import that has not yet been provisioned")
continue
}

return grpReadErr
return nil, fmt.Errorf("group read: %w", grpReadErr)
}

// import the roles on this group from the datastore
var roles []Role

roles, err := importRoles(db, g.Roles)
var grpRoleIDs []string

roles, grpRoleIDs, err = importRoles(db, g.Roles)
if err != nil {
return err
return nil, fmt.Errorf("group import roles: %w", err)
}

modifiedRoleIDs = append(modifiedRoleIDs, grpRoleIDs...)

// add the new group associations to the roles
for i, role := range roles {
if db.Model(&group).Where(&Role{Id: role.Id}).Association("Roles").Count() == 0 {
if err = db.Model(&group).Where(&Role{Id: role.Id}).Association("Roles").Append(&roles[i]); err != nil {
return err
return nil, fmt.Errorf("group role assocations: %w", err)
}
}
}
}

return nil
return modifiedRoleIDs, nil
}

func ImportUserMapping(db *gorm.DB, users []ConfigUserMapping) error {
func ApplyUserMappings(db *gorm.DB, users []ConfigUserMapping) (modifiedRoleIDs []string, err error) {
for _, u := range users {
var user User

Expand All @@ -175,29 +178,68 @@ func ImportUserMapping(db *gorm.DB, users []ConfigUserMapping) error {
continue
}

return usrReadErr
return nil, fmt.Errorf("read user: %w", usrReadErr)
}

var roles []Role

// add direct user to role mappings
roles, err := importRoles(db, u.Roles)
var usrRoleIDs []string

roles, usrRoleIDs, err = importRoles(db, u.Roles)
if err != nil {
return err
return nil, fmt.Errorf("import user roles: %w", err)
}

modifiedRoleIDs = append(modifiedRoleIDs, usrRoleIDs...)

// for all roles attached to this user update their user associations now that we have made sure they exist
// important: do not create the association on the user, that runs an upsert that creates a concurrent write because User.AfterCreate() calls this function
for i, role := range roles {
if db.Model(&user).Where(&Role{Id: role.Id}).Association("Roles").Count() == 0 {
if err = db.Model(&user).Where(&Role{Id: role.Id}).Association("Roles").Append(&roles[i]); err != nil {
return err
return nil, fmt.Errorf("user role associations: %w", err)
}
}
}
}

return modifiedRoleIDs, nil
}

// ImportRoleMappings iterates over user and group config and applies a role mapping to them
func ImportRoleMappings(db *gorm.DB, groups []ConfigGroupMapping, users []ConfigUserMapping) error {
groupRoleIDs, err := ApplyGroupMappings(db, groups)
if err != nil {
return fmt.Errorf("apply group mappings: %w", err)
}

userRoleIDs, err := ApplyUserMappings(db, users)
if err != nil {
return fmt.Errorf("apply user mappings: %w", err)
}

var roleIDs []string
roleIDs = append(roleIDs, groupRoleIDs...)
roleIDs = append(roleIDs, userRoleIDs...)

var rolesRemoved []Role
if err := db.Not(roleIDs).Find(&rolesRemoved).Error; err != nil {
return fmt.Errorf("find roles removed in config: %w", err)
}

if len(rolesRemoved) > 0 {
if err := db.Delete(rolesRemoved).Error; err != nil {
return fmt.Errorf("delete config removed role: %w", err)
}
}

logging.L.Sugar().Debugf("importing configuration removed %d role(s)", len(rolesRemoved))

return nil
}

// ImportConfig tries to import all valid fields in a config file
// ImportConfig tries to import all valid fields in a config file and removes old config
func ImportConfig(db *gorm.DB, bs []byte) error {
var config Config
if err := yaml.Unmarshal(bs, &config); err != nil {
Expand All @@ -207,43 +249,16 @@ func ImportConfig(db *gorm.DB, bs []byte) error {
initialConfig = config

return db.Transaction(func(tx *gorm.DB) error {
// gorm blocks global delete by default: https://gorm.io/docs/delete.html#Block-Global-Delete
if err := tx.Where("1 = 1").Delete(&Role{}).Error; err != nil {
return err
}

var users []User
if err := tx.Find(&users).Error; err != nil {
return err
}
if err := tx.Where("1 = 1").Delete(&users).Error; err != nil {
return err
}

var groups []Group
if err := tx.Find(&groups).Error; err != nil {
return err
}
if err := tx.Where("1 = 1").Delete(&groups).Error; err != nil {
return err
}

if err := ImportSources(tx, config.Sources); err != nil {
return err
}

if err := ImportGroupMapping(tx, config.Groups); err != nil {
return err
}

return ImportUserMapping(tx, config.Users)
return ImportRoleMappings(tx, config.Groups, config.Users)
})
}

// import roles creates roles specified in the config, or updates their associations
func importRoles(db *gorm.DB, roles []ConfigRoleKubernetes) ([]Role, error) {
var rolesImported []Role

func importRoles(db *gorm.DB, roles []ConfigRoleKubernetes) (rolesImported []Role, importedRoleIDs []string, err error) {
for _, r := range roles {
if r.Name == "" {
logging.L.Error("invalid role found in configuration, name is a required field")
Expand All @@ -268,36 +283,38 @@ func importRoles(db *gorm.DB, roles []ConfigRoleKubernetes) ([]Role, error) {

var dest Destination

err := db.Where(&Destination{Name: destination.Name}).First(&dest).Error
if err != nil {
if errors.Is(err, gorm.ErrRecordNotFound) {
destErr := db.Where(&Destination{Name: destination.Name}).First(&dest).Error
if destErr != nil {
if errors.Is(destErr, gorm.ErrRecordNotFound) {
// when a destination is added then the config import will be retried, skip for now
logging.L.Debug("skipping role binding for destination in config import that has not yet been discovered")
continue
}

return nil, err
return nil, nil, fmt.Errorf("find role destination: %w", destErr)
}

if len(destination.Namespaces) > 0 {
for _, namespace := range destination.Namespaces {
var role Role
if err = db.FirstOrCreate(&role, &Role{Name: r.Name, Kind: r.Kind, Namespace: namespace, DestinationId: dest.Id}).Error; err != nil {
return nil, err
return nil, nil, fmt.Errorf("group read source: %w", err)
}

rolesImported = append(rolesImported, role)
importedRoleIDs = append(importedRoleIDs, role.Id)
}
} else {
var role Role
if err = db.FirstOrCreate(&role, &Role{Name: r.Name, Kind: r.Kind, DestinationId: dest.Id}).Error; err != nil {
return nil, err
return nil, nil, fmt.Errorf("role find create: %w", err)
}

rolesImported = append(rolesImported, role)
importedRoleIDs = append(importedRoleIDs, role.Id)
}
}
}

return rolesImported, nil
return rolesImported, importedRoleIDs, nil
}
50 changes: 18 additions & 32 deletions internal/registry/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,22 @@ import (

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gopkg.in/yaml.v2"
"gorm.io/gorm"
)

var db *gorm.DB

var (
fakeOktaSource = Source{Kind: SourceKindOkta, Domain: "test.example.com", ClientSecret: "okta-secrets/client-secret", ApiToken: "okta-secrets/api-token"}
adminUser = User{Id: "001", Email: "admin@example.com"}
standardUser = User{Id: "002", Email: "user@example.com"}
iosDevUser = User{Id: "003", Email: "woz@example.com"}
iosDevGroup = Group{Name: "ios-developers"}
macAdminGroup = Group{Name: "mac-admins"}
clusterA = Destination{Name: "cluster-AAA"}
clusterB = Destination{Name: "cluster-BBB"}
clusterC = Destination{Name: "cluster-CCC"}
fakeOktaSource = Source{Id: "001", Kind: SourceKindOkta, Domain: "test.example.com", ClientSecret: "okta-secrets/client-secret", ApiToken: "okta-secrets/api-token"}
adminUser = User{Id: "001", Email: "admin@example.com"}
standardUser = User{Id: "002", Email: "user@example.com"}
iosDevUser = User{Id: "003", Email: "woz@example.com"}
iosDevGroup = Group{Name: "ios-developers", SourceId: fakeOktaSource.Id}
macAdminGroup = Group{Name: "mac-admins", SourceId: fakeOktaSource.Id}
notInConfigRole = Role{Name: "does-not-exist"}
clusterA = Destination{Name: "cluster-AAA"}
clusterB = Destination{Name: "cluster-BBB"}
clusterC = Destination{Name: "cluster-CCC"}
)

func setup() error {
Expand Down Expand Up @@ -87,8 +87,6 @@ func setup() error {
return err
}

iosDevGroup := &Group{Name: "ios-developers", SourceId: fakeOktaSource.Id}

err = db.Create(&iosDevGroup).Error
if err != nil {
return err
Expand All @@ -101,30 +99,12 @@ func setup() error {
return err
}

err = db.Create(&Group{Name: "mac-admins", SourceId: fakeOktaSource.Id}).Error
err = db.Create(&macAdminGroup).Error
if err != nil {
return err
}

// need to do the config import here so that the manual test modifications are not deleted
var config Config
if err := yaml.Unmarshal(confFile, &config); err != nil {
return err
}

initialConfig = config

return db.Transaction(func(tx *gorm.DB) error {
if err := ImportSources(tx, config.Sources); err != nil {
return err
}

if err := ImportGroupMapping(tx, config.Groups); err != nil {
return err
}

return ImportUserMapping(tx, config.Users)
})
return ImportConfig(db, confFile)
}

func TestMain(m *testing.M) {
Expand Down Expand Up @@ -177,6 +157,12 @@ func TestImportRolesForUnknownDestinationsAreIgnored(t *testing.T) {
}
}

func TestImportRemovesUnusedRoles(t *testing.T) {
var unused Role
err := db.Where(&Role{Name: notInConfigRole.Name}).First(&unused).Error
assert.ErrorIs(t, err, gorm.ErrRecordNotFound)
}

func TestExistingSourceIsOverridden(t *testing.T) {
// this source comes second in the config so it will override the one before it
var importedOkta Source
Expand Down
Loading

0 comments on commit 2f82548

Please sign in to comment.