Skip to content

Commit

Permalink
Improvements to config handling
Browse files Browse the repository at this point in the history
  • Loading branch information
robgonnella committed Aug 9, 2023
1 parent 58097b3 commit 424c4ce
Show file tree
Hide file tree
Showing 19 changed files with 125 additions and 232 deletions.
26 changes: 10 additions & 16 deletions internal/config/interface.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package config

import (
"time"

"gorm.io/datatypes"
)

Expand All @@ -25,11 +23,10 @@ type SSHConfig struct {

// Config represents the data structure of our user provided json configuration
type Config struct {
ID int
Name string
SSH SSHConfig
Targets []string `json:"targets"`
Loaded time.Time
ID int
Name string
SSH SSHConfig
CIDR string
}

// SSHConfigModel represents the ssh config stored in the database
Expand All @@ -41,31 +38,28 @@ type SSHConfigModel struct {

// ConfigModel represents the config stored in the database
type ConfigModel struct {
ID int `gorm:"primaryKey"`
Name string `gorm:"uniqueIndex"`
SSH SSHConfigModel `gorm:"embedded"`
Targets datatypes.JSON
Loaded time.Time `gorm:"index:,sort:desc"`
ID int `gorm:"primaryKey"`
Name string `gorm:"uniqueIndex"`
SSH SSHConfigModel `gorm:"embedded"`
CIDR string `gorm:"column:cidr"`
}

// Repo interface representing access to stored configs
type Repo interface {
Get(id int) (*Config, error)
GetAll() ([]*Config, error)
GetByCIDR(cidr string) (*Config, error)
Create(conf *Config) (*Config, error)
Update(conf *Config) (*Config, error)
Delete(id int) error
SetLastLoaded(id int) error
LastLoaded() (*Config, error)
}

// Service interface for manipulating configurations
type Service interface {
Get(id int) (*Config, error)
GetAll() ([]*Config, error)
GetByCIDR(cidr string) (*Config, error)
Create(conf *Config) (*Config, error)
Update(conf *Config) (*Config, error)
Delete(id int) error
SetLastLoaded(id int) error
LastLoaded() (*Config, error)
}
39 changes: 4 additions & 35 deletions internal/config/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package config
import (
"encoding/json"
"errors"
"time"

"github.com/robgonnella/ops/internal/exception"
"gorm.io/datatypes"
Expand Down Expand Up @@ -38,10 +37,6 @@ func (r *SqliteRepo) Get(id int) (*Config, error) {
return nil, result.Error
}

if result := r.db.Save(&confModel); result.Error != nil {
return nil, result.Error
}

return modelToConfig(&confModel)
}

Expand Down Expand Up @@ -122,24 +117,11 @@ func (r *SqliteRepo) Delete(id int) error {
return r.db.Delete(&ConfigModel{ID: id}).Error
}

// SetLastLoaded updates a configs "loaded" field to the current timestamp
func (r *SqliteRepo) SetLastLoaded(id int) error {
confModel := ConfigModel{ID: id}

if result := r.db.First(&confModel); result.Error != nil {
return result.Error
}

confModel.Loaded = time.Now()

return r.db.Save(&confModel).Error
}

// LastLoaded returns the most recently loaded config
func (r *SqliteRepo) LastLoaded() (*Config, error) {
func (r *SqliteRepo) GetByCIDR(cidr string) (*Config, error) {
confModel := ConfigModel{}

if result := r.db.Order("loaded desc").First(&confModel); result.Error != nil {
if result := r.db.First(&confModel, "cidr = ?", cidr); result.Error != nil {
if errors.Is(result.Error, gorm.ErrRecordNotFound) {
return nil, exception.ErrRecordNotFound
}
Expand All @@ -158,12 +140,6 @@ func modelToConfig(model *ConfigModel) (*Config, error) {
return nil, err
}

targets := []string{}

if err := json.Unmarshal([]byte(model.Targets.String()), &targets); err != nil {
return nil, err
}

return &Config{
ID: model.ID,
Name: model.Name,
Expand All @@ -172,8 +148,7 @@ func modelToConfig(model *ConfigModel) (*Config, error) {
Identity: model.SSH.Identity,
Overrides: overrides,
},
Targets: targets,
Loaded: model.Loaded,
CIDR: model.CIDR,
}, nil
}

Expand All @@ -184,12 +159,6 @@ func configToModel(conf *Config) (*ConfigModel, error) {
return nil, err
}

targetsBytes, err := json.Marshal(conf.Targets)

if err != nil {
return nil, err
}

return &ConfigModel{
ID: conf.ID,
Name: conf.Name,
Expand All @@ -198,6 +167,6 @@ func configToModel(conf *Config) (*ConfigModel, error) {
Identity: conf.SSH.Identity,
Overrides: datatypes.JSON(overridesBytes),
},
Targets: datatypes.JSON(targetsBytes),
CIDR: conf.CIDR,
}, nil
}
42 changes: 19 additions & 23 deletions internal/config/repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func TestConfigSqliteRepo(t *testing.T) {
},
},
},
Targets: []string{"target"},
CIDR: "172.2.2.1/32",
}

newConf, err := repo.Create(conf)
Expand All @@ -84,7 +84,7 @@ func TestConfigSqliteRepo(t *testing.T) {
Identity: newConf.SSH.Identity,
Overrides: newConf.SSH.Overrides,
},
Targets: newConf.Targets,
CIDR: newConf.CIDR,
}

updatedConf, err := repo.Update(toUpdate)
Expand All @@ -107,19 +107,19 @@ func TestConfigSqliteRepo(t *testing.T) {
conf1 := &config.Config{
Name: "test2",
SSH: config.SSHConfig{
User: "test-user1",
Identity: "test-identity1",
User: "test-user2",
Identity: "test-identity2",
},
Targets: []string{"target1"},
CIDR: "172.2.2.2/32",
}

conf2 := &config.Config{
Name: "test3",
SSH: config.SSHConfig{
User: "test-user2",
Identity: "test-identity2",
User: "test-user3",
Identity: "test-identity3",
},
Targets: []string{"target2"},
CIDR: "172.2.2.3/32",
}

_, err := repo.Create(conf1)
Expand All @@ -144,40 +144,36 @@ func TestConfigSqliteRepo(t *testing.T) {

})

t.Run("gets last loaded", func(st *testing.T) {
t.Run("gets by cidr", func(st *testing.T) {
conf1 := &config.Config{
Name: "test4",
SSH: config.SSHConfig{
User: "test-user1",
Identity: "test-identity1",
User: "test-user4",
Identity: "test-identity4",
},
Targets: []string{"target1"},
CIDR: "172.2.2.4/32",
}

conf2 := &config.Config{
Name: "test5",
SSH: config.SSHConfig{
User: "test-user2",
Identity: "test-identity2",
User: "test-user5",
Identity: "test-identity5",
},
Targets: []string{"target2"},
CIDR: "172.2.2.5/32",
}

newConf1, err := repo.Create(conf1)

assert.NoError(st, err)

_, err = repo.Create(conf2)
_, err := repo.Create(conf1)

assert.NoError(st, err)

err = repo.SetLastLoaded(newConf1.ID)
newConf2, err := repo.Create(conf2)

assert.NoError(st, err)

lastLoaded, err := repo.LastLoaded()
foundConf, err := repo.GetByCIDR("172.2.2.5/32")

assert.NoError(st, err)
assertEqualConf(st, newConf1, lastLoaded)
assertEqualConf(st, newConf2, foundConf)
})
}
14 changes: 4 additions & 10 deletions internal/config/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ func (s *ConfigService) GetAll() ([]*Config, error) {
return s.repo.GetAll()
}

func (s *ConfigService) GetByCIDR(cidr string) (*Config, error) {
return s.repo.GetByCIDR(cidr)
}

// Create creates a new config
func (s *ConfigService) Create(conf *Config) (*Config, error) {
return s.repo.Create(conf)
Expand All @@ -34,13 +38,3 @@ func (s *ConfigService) Update(conf *Config) (*Config, error) {
func (s *ConfigService) Delete(id int) error {
return s.repo.Delete(id)
}

// SetLasLoaded sets the "loaded" field for a config to current timestamp
func (s *ConfigService) SetLastLoaded(id int) error {
return s.repo.SetLastLoaded(id)
}

// LastLoaded retrieves the most recently loaded config
func (s *ConfigService) LastLoaded() (*Config, error) {
return s.repo.LastLoaded()
}
20 changes: 11 additions & 9 deletions internal/config/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func TestConfigService(t *testing.T) {
User: "user",
Identity: "identity",
},
Targets: []string{"target"},
CIDR: "172.2.2.2/32",
}

mockRepo.EXPECT().Get(expectedConfig.ID).Return(expectedConfig, nil)
Expand All @@ -44,7 +44,7 @@ func TestConfigService(t *testing.T) {
User: "user",
Identity: "identity",
},
Targets: []string{"target"},
CIDR: "172.2.2.2/32",
}

conf2 := &config.Config{
Expand All @@ -53,7 +53,7 @@ func TestConfigService(t *testing.T) {
User: "user",
Identity: "identity",
},
Targets: []string{"target"},
CIDR: "172.2.2.3/32",
}

expectedConfs := []*config.Config{conf1, conf2}
Expand All @@ -73,7 +73,7 @@ func TestConfigService(t *testing.T) {
User: "user",
Identity: "identity",
},
Targets: []string{"target"},
CIDR: "172.2.2.2/32",
}

mockRepo.EXPECT().Create(conf).Return(conf, nil)
Expand All @@ -91,7 +91,7 @@ func TestConfigService(t *testing.T) {
User: "user",
Identity: "identity",
},
Targets: []string{"target"},
CIDR: "172.2.2.2/32",
}

mockRepo.EXPECT().Update(conf).Return(conf, nil)
Expand All @@ -112,19 +112,21 @@ func TestConfigService(t *testing.T) {
assert.NoError(st, err)
})

t.Run("gets last loaded config", func(st *testing.T) {
t.Run("gets last config by cidr", func(st *testing.T) {
cidr := "172.2.2.2/32"

expectedConfig := &config.Config{
Name: "test",
SSH: config.SSHConfig{
User: "user",
Identity: "identity",
},
Targets: []string{"target"},
CIDR: cidr,
}

mockRepo.EXPECT().LastLoaded().Return(expectedConfig, nil)
mockRepo.EXPECT().GetByCIDR(cidr).Return(expectedConfig, nil)

foundConf, err := service.LastLoaded()
foundConf, err := service.GetByCIDR(cidr)

assert.NoError(st, err)
assert.Equal(st, expectedConfig, foundConf)
Expand Down
8 changes: 0 additions & 8 deletions internal/core/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,6 @@ func (c *Core) UpdateConfig(conf config.Config) error {
return err
}

if err := c.configService.SetLastLoaded(updated.ID); err != nil {
return err
}

c.conf = updated

return nil
Expand All @@ -128,10 +124,6 @@ func (c *Core) SetConfig(id int) error {
return err
}

if err := c.configService.SetLastLoaded(conf.ID); err != nil {
return err
}

c.conf = conf

return nil
Expand Down
Loading

0 comments on commit 424c4ce

Please sign in to comment.